diff --git a/contrib/check-config.sh b/contrib/check-config.sh index af7d8a7de5..3e910ae956 100755 --- a/contrib/check-config.sh +++ b/contrib/check-config.sh @@ -128,6 +128,16 @@ check_device() { fi } +check_sysctl() { + val=$(sysctl -n $1) + want=$2 + if [ "$val" = "$want" ]; then + wrap_good "sysctl $1" "enabled" + else + wrap_bad "sysctl $1" "disabled" + fi +} + if [ ! -e "$CONFIG" ]; then wrap_warning "warning: $CONFIG does not exist, searching other paths for kernel config ..." for tryConfig in $possibleConfigs; do @@ -343,6 +353,10 @@ if ! is_set EXT4_FS || ! is_set EXT4_FS_POSIX_ACL || ! is_set EXT4_FS_SECURITY; fi echo '- Network Drivers:' +echo " - \"$(wrap_color 'bridge' blue)\":" +check_sysctl net.ipv4.ip_forward 1 | sed 's/^/ - /' +check_sysctl net.ipv6.conf.all.forwarding 1 | sed 's/^/ - /' +check_sysctl net.ipv6.conf.default.forwarding 1 | sed 's/^/ - /' echo " - \"$(wrap_color 'overlay' blue)\":" check_flags VXLAN BRIDGE_VLAN_FILTERING | sed 's/^/ /' echo ' Optional (for encrypted networks):' diff --git a/contrib/dockerd-rootless.sh b/contrib/dockerd-rootless.sh index f798cd09dd..3ded28f6dd 100755 --- a/contrib/dockerd-rootless.sh +++ b/contrib/dockerd-rootless.sh @@ -199,5 +199,12 @@ else mount_directory /etc/ssl "--rbind" fi + # When running with --firewall-backend=nftables, IP forwarding needs to be enabled + # because the daemon won't enable it. IP forwarding is harmless in the rootless + # netns, there's only a single external interface and only Docker uses the netns. + # So, always enable IPv4 and IPv6 forwarding. + sysctl -w net.ipv4.ip_forward=1 + sysctl -w net.ipv6.conf.all.forwarding=1 + exec "$dockerd" "$@" fi diff --git a/daemon/libnetwork/drivers/bridge/bridge_linux.go b/daemon/libnetwork/drivers/bridge/bridge_linux.go index 84e66a7715..8af622e318 100644 --- a/daemon/libnetwork/drivers/bridge/bridge_linux.go +++ b/daemon/libnetwork/drivers/bridge/bridge_linux.go @@ -556,7 +556,7 @@ var newFirewaller = func(ctx context.Context, config firewaller.Config) (firewal // cleaner can't clean up network or port-specific rules that may have been added // to iptables built-in chains. So, if cleanup is needed, give the cleaner to // the nftabler. Then, it'll use it to delete old rules as networks are restored. - fw.(firewaller.FirewallCleanerSetter).SetFirewallCleaner(iptabler.NewCleaner(ctx, config)) + fw.SetFirewallCleaner(iptabler.NewCleaner(ctx, config)) return fw, nil } @@ -880,14 +880,28 @@ func (d *driver) createNetwork(ctx context.Context, config *networkConfiguration config.EnableIPv4 && d.config.EnableIPForwarding, "setupIPv4Forwarding", func(*networkConfiguration, *bridgeInterface) error { - return setupIPv4Forwarding(d.firewaller, d.config.EnableIPTables && !d.config.DisableFilterForwardDrop) + ffd, ok := d.firewaller.(filterForwardDropper) + if !ok { + // The firewaller can't drop non-Docker forwarding. It's up to the user to enable + // forwarding on their host, and configure their firewall appropriately. + return checkIPv4Forwarding() + } + // Enable forwarding and set a default-drop forwarding policy if necessary. + return setupIPv4Forwarding(ffd, d.config.EnableIPTables && !d.config.DisableFilterForwardDrop) }, }, { config.EnableIPv6 && d.config.EnableIPForwarding, "setupIPv6Forwarding", func(*networkConfiguration, *bridgeInterface) error { - return setupIPv6Forwarding(d.firewaller, d.config.EnableIP6Tables && !d.config.DisableFilterForwardDrop) + ffd, ok := d.firewaller.(filterForwardDropper) + if !ok { + // The firewaller can't drop non-Docker forwarding. It's up to the user to enable + // forwarding on their host, and configure their firewall appropriately. + return checkIPv6Forwarding() + } + // Enable forwarding and set a default-drop forwarding policy if necessary. + return setupIPv6Forwarding(ffd, d.config.EnableIP6Tables && !d.config.DisableFilterForwardDrop) }, }, diff --git a/daemon/libnetwork/drivers/bridge/bridge_store.go b/daemon/libnetwork/drivers/bridge/bridge_store.go index 92a7dc20d1..78dce7e53d 100644 --- a/daemon/libnetwork/drivers/bridge/bridge_store.go +++ b/daemon/libnetwork/drivers/bridge/bridge_store.go @@ -13,7 +13,7 @@ import ( "github.com/containerd/log" "github.com/moby/moby/v2/daemon/internal/otelutil" "github.com/moby/moby/v2/daemon/libnetwork/datastore" - "github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/firewaller" + "github.com/moby/moby/v2/daemon/libnetwork/drivers/bridge/internal/nftabler" "github.com/moby/moby/v2/daemon/libnetwork/portmapperapi" "github.com/moby/moby/v2/daemon/libnetwork/types" "go.opentelemetry.io/otel" @@ -43,8 +43,8 @@ func (d *driver) initStore() error { // If there's a firewall cleaner, it's done its job by cleaning up rules // belonging to the restored networks. So, drop it. - if fcs, ok := d.firewaller.(firewaller.FirewallCleanerSetter); ok { - fcs.SetFirewallCleaner(nil) + if nft, ok := d.firewaller.(*nftabler.Nftabler); ok { + nft.SetFirewallCleaner(nil) } return nil diff --git a/daemon/libnetwork/drivers/bridge/internal/firewaller/firewaller.go b/daemon/libnetwork/drivers/bridge/internal/firewaller/firewaller.go index 743b03c23e..9ab68aaa58 100644 --- a/daemon/libnetwork/drivers/bridge/internal/firewaller/firewaller.go +++ b/daemon/libnetwork/drivers/bridge/internal/firewaller/firewaller.go @@ -76,9 +76,6 @@ type Firewaller interface { // NewNetwork returns an object that can be used to add published ports and legacy // links for a bridge network. NewNetwork(ctx context.Context, nc NetworkConfig) (Network, error) - // FilterForwardDrop sets the default policy of the FORWARD chain in the filter - // table to DROP. - FilterForwardDrop(ctx context.Context, ipv IPVersion) error } // Network can be used to manipulate firewall rules for a bridge network. @@ -110,12 +107,6 @@ type Network interface { DelLink(ctx context.Context, parentIP, childIP netip.Addr, ports []types.TransportPort) } -// FirewallCleanerSetter is an optional interface for a Firewaller. -type FirewallCleanerSetter interface { - // SetFirewallCleaner replaces the FirewallCleaner (possibly with 'nil'). - SetFirewallCleaner(FirewallCleaner) -} - // FirewallCleaner is used to delete rules created by previous incarnations of // the daemon. On startup, once a Firewaller implementation has been selected, if // rules may have been left behind by a different Firewaller implementation, get diff --git a/daemon/libnetwork/drivers/bridge/internal/firewaller/stub.go b/daemon/libnetwork/drivers/bridge/internal/firewaller/stub.go index cf884edd26..f09df48415 100644 --- a/daemon/libnetwork/drivers/bridge/internal/firewaller/stub.go +++ b/daemon/libnetwork/drivers/bridge/internal/firewaller/stub.go @@ -15,7 +15,6 @@ import ( type StubFirewaller struct { Config Networks map[string]*StubFirewallerNetwork - FFD map[IPVersion]bool // filter forward drop } func NewStubFirewaller(config Config) *StubFirewaller { @@ -24,7 +23,6 @@ func NewStubFirewaller(config Config) *StubFirewaller { // A real Firewaller shouldn't hold on to its own networks, the bridge driver is doing that. // But, for unit tests cross-checking the driver, this is useful. Networks: make(map[string]*StubFirewallerNetwork), - FFD: make(map[IPVersion]bool), } } @@ -41,11 +39,6 @@ func (fw *StubFirewaller) NewNetwork(_ context.Context, nc NetworkConfig) (Netwo return nw, nil } -func (fw *StubFirewaller) FilterForwardDrop(_ context.Context, ipv IPVersion) error { - fw.FFD[ipv] = true - return nil -} - type stubFirewallerLink struct { parentIP netip.Addr childIP netip.Addr diff --git a/daemon/libnetwork/drivers/bridge/internal/iptabler/cleaner.go b/daemon/libnetwork/drivers/bridge/internal/iptabler/cleaner.go index 98f3f5f340..1de527b08b 100644 --- a/daemon/libnetwork/drivers/bridge/internal/iptabler/cleaner.go +++ b/daemon/libnetwork/drivers/bridge/internal/iptabler/cleaner.go @@ -17,7 +17,7 @@ type iptablesCleaner struct { } // NewCleaner checks for iptables rules left behind by an old daemon that was using -// the iptabler. +// the Iptabler. // // If there are old rules present, it deletes as much as possible straight away // (user-defined chains and jumps from the built-in chains). @@ -46,6 +46,13 @@ func NewCleaner(ctx context.Context, config firewaller.Config) firewaller.Firewa _ = t.DeleteJumpRule(iptables.Filter, "FORWARD", DockerForwardChain) _ = deleteLegacyTopLevelRules(ctx, t, ipv) removeIPChains(ctx, ipv) + // The iptables chains will no longer have Docker's ACCEPT rules. So, if the + // filter-FORWARD chain has policy DROP (possibly set by Docker when it enabled + // IP forwarding), packets accepted by nftables chains will still be processed by + // iptables and dropped. It's the user's responsibility to sort that out. + if t.HasPolicy("filter", "FORWARD", iptables.Drop) { + log.G(ctx).WithField("ipv", ipv).Warn("Network traffic for published ports may be dropped, iptables chain FORWARD has policy DROP.") + } return true } cleaned4 := clean(iptables.IPv4, config.IPv4) @@ -62,7 +69,7 @@ func (ic iptablesCleaner) DelNetwork(ctx context.Context, nc firewaller.NetworkC } n := network{ config: nc, - ipt: &iptabler{config: ic.config}, + ipt: &Iptabler{config: ic.config}, } if ic.config.IPv4 && nc.Config4.Prefix.IsValid() { _ = deleteLegacyFilterRules(iptables.IPv4, nc.IfName) @@ -77,7 +84,7 @@ func (ic iptablesCleaner) DelNetwork(ctx context.Context, nc firewaller.NetworkC func (ic iptablesCleaner) DelEndpoint(ctx context.Context, nc firewaller.NetworkConfig, epIPv4, epIPv6 netip.Addr) { n := network{ config: nc, - ipt: &iptabler{config: ic.config}, + ipt: &Iptabler{config: ic.config}, } if n.ipt.config.IPv4 && epIPv4.IsValid() { _ = n.filterDirectAccess(ctx, iptables.IPv4, n.config.Config4, epIPv4, false) @@ -90,7 +97,7 @@ func (ic iptablesCleaner) DelEndpoint(ctx context.Context, nc firewaller.Network func (ic iptablesCleaner) DelPorts(ctx context.Context, nc firewaller.NetworkConfig, pbs []types.PortBinding) { n := network{ config: nc, - ipt: &iptabler{config: ic.config}, + ipt: &Iptabler{config: ic.config}, } _ = n.DelPorts(ctx, pbs) } diff --git a/daemon/libnetwork/drivers/bridge/internal/iptabler/iptabler.go b/daemon/libnetwork/drivers/bridge/internal/iptabler/iptabler.go index 74df0ba8b8..0a51582e3b 100644 --- a/daemon/libnetwork/drivers/bridge/internal/iptabler/iptabler.go +++ b/daemon/libnetwork/drivers/bridge/internal/iptabler/iptabler.go @@ -35,12 +35,12 @@ const ( isolationChain2 = "DOCKER-ISOLATION-STAGE-2" ) -type iptabler struct { +type Iptabler struct { config firewaller.Config } -func NewIptabler(ctx context.Context, config firewaller.Config) (firewaller.Firewaller, error) { - ipt := &iptabler{config: config} +func NewIptabler(ctx context.Context, config firewaller.Config) (*Iptabler, error) { + ipt := &Iptabler{config: config} if ipt.config.IPv4 { removeIPChains(ctx, iptables.IPv4) @@ -91,7 +91,8 @@ func NewIptabler(ctx context.Context, config firewaller.Config) (firewaller.Fire return ipt, nil } -func (ipt *iptabler) FilterForwardDrop(ctx context.Context, ipv firewaller.IPVersion) error { +// FilterForwardDrop sets the default policy of the FORWARD chain in the filter table to DROP. +func (ipt *Iptabler) FilterForwardDrop(ctx context.Context, ipv firewaller.IPVersion) error { var iptv iptables.IPVersion switch ipv { case firewaller.IPv4: diff --git a/daemon/libnetwork/drivers/bridge/internal/iptabler/network.go b/daemon/libnetwork/drivers/bridge/internal/iptabler/network.go index f80802fb35..5eef4d6690 100644 --- a/daemon/libnetwork/drivers/bridge/internal/iptabler/network.go +++ b/daemon/libnetwork/drivers/bridge/internal/iptabler/network.go @@ -22,11 +22,11 @@ type ( type network struct { config firewaller.NetworkConfig - ipt *iptabler + ipt *Iptabler cleanFuncs iptablesCleanFuncs } -func (ipt *iptabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) { +func (ipt *Iptabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) { n := &network{ ipt: ipt, config: nc, diff --git a/daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go b/daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go index 7efbfe7a91..c57c36d654 100644 --- a/daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go +++ b/daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go @@ -11,7 +11,7 @@ import ( "github.com/moby/moby/v2/daemon/libnetwork/internal/nftables" ) -// Cleanup deletes all rules created by nftabler; it's intended to be used +// Cleanup deletes all rules created by Nftabler; it's intended to be used // during startup, to clean up rules created by an old incarnation of the daemon // after switching to a different Firewaller implementation. func Cleanup(ctx context.Context, config firewaller.Config) { @@ -31,6 +31,6 @@ func Cleanup(ctx context.Context, config firewaller.Config) { } } -func (nft *nftabler) SetFirewallCleaner(fc firewaller.FirewallCleaner) { +func (nft *Nftabler) SetFirewallCleaner(fc firewaller.FirewallCleaner) { nft.cleaner = fc } diff --git a/daemon/libnetwork/drivers/bridge/internal/nftabler/network.go b/daemon/libnetwork/drivers/bridge/internal/nftabler/network.go index 996cc86128..b5dd54bf27 100644 --- a/daemon/libnetwork/drivers/bridge/internal/nftabler/network.go +++ b/daemon/libnetwork/drivers/bridge/internal/nftabler/network.go @@ -18,10 +18,10 @@ import ( type network struct { config firewaller.NetworkConfig cleaner func(ctx context.Context) error - fw *nftabler + fw *Nftabler } -func (nft *nftabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) { +func (nft *Nftabler) NewNetwork(ctx context.Context, nc firewaller.NetworkConfig) (_ firewaller.Network, retErr error) { n := &network{ fw: nft, config: nc, diff --git a/daemon/libnetwork/drivers/bridge/internal/nftabler/nftabler.go b/daemon/libnetwork/drivers/bridge/internal/nftabler/nftabler.go index 9d6f15c3e6..e0fe055bd9 100644 --- a/daemon/libnetwork/drivers/bridge/internal/nftabler/nftabler.go +++ b/daemon/libnetwork/drivers/bridge/internal/nftabler/nftabler.go @@ -45,15 +45,15 @@ const ( rawPreroutingPortsRuleGroup = iota + initialRuleGroup + 1 ) -type nftabler struct { +type Nftabler struct { config firewaller.Config cleaner firewaller.FirewallCleaner table4 nftables.TableRef table6 nftables.TableRef } -func NewNftabler(ctx context.Context, config firewaller.Config) (firewaller.Firewaller, error) { - nft := &nftabler{config: config} +func NewNftabler(ctx context.Context, config firewaller.Config) (*Nftabler, error) { + nft := &Nftabler{config: config} if nft.config.IPv4 { var err error @@ -85,23 +85,8 @@ func NewNftabler(ctx context.Context, config firewaller.Config) (firewaller.Fire return nft, nil } -func (nft *nftabler) getTable(ipv firewaller.IPVersion) nftables.TableRef { - if ipv == firewaller.IPv4 { - return nft.table4 - } - return nft.table6 -} - -func (nft *nftabler) FilterForwardDrop(ctx context.Context, ipv firewaller.IPVersion) error { - table := nft.getTable(ipv) - if err := table.Chain(ctx, forwardChain).SetPolicy("drop"); err != nil { - return err - } - return nftApply(ctx, table) -} - // init creates the bridge driver's nftables table for IPv4 or IPv6. -func (nft *nftabler) init(ctx context.Context, family nftables.Family) (nftables.TableRef, error) { +func (nft *Nftabler) init(ctx context.Context, family nftables.Family) (nftables.TableRef, error) { // Instantiate the table. table, err := nftables.NewTable(family, dockerTable) if err != nil { diff --git a/daemon/libnetwork/drivers/bridge/setup_ip_forwarding.go b/daemon/libnetwork/drivers/bridge/setup_ip_forwarding.go index 4f2845d0f5..0ebd4eb17d 100644 --- a/daemon/libnetwork/drivers/bridge/setup_ip_forwarding.go +++ b/daemon/libnetwork/drivers/bridge/setup_ip_forwarding.go @@ -4,6 +4,7 @@ package bridge import ( "context" + "errors" "fmt" "os" @@ -17,7 +18,24 @@ const ( ipv6ForwardConfAll = "/proc/sys/net/ipv6/conf/all/forwarding" ) -func setupIPv4Forwarding(fw firewaller.Firewaller, wantFilterForwardDrop bool) (retErr error) { +type filterForwardDropper interface { + FilterForwardDrop(context.Context, firewaller.IPVersion) error +} + +func checkIPv4Forwarding() error { + enabled, err := getKernelBoolParam(ipv4ForwardConf) + if err != nil { + return fmt.Errorf("checking IPv4 forwarding: %w", err) + } + if enabled { + return nil + } + // It's the user's responsibility to enable forwarding and secure their host. Or, + // start docker with --ip-forward=false to disable this check. + return errors.New("IPv4 forwarding is disabled: check your host's firewalling and set sysctl net.ipv4.ip_forward=1, or disable this check using daemon option --ip-forward=false") +} + +func setupIPv4Forwarding(ffd filterForwardDropper, wantFilterForwardDrop bool) (retErr error) { changed, err := configureIPForwarding(ipv4ForwardConf, '1') if err != nil { return err @@ -34,16 +52,35 @@ func setupIPv4Forwarding(fw firewaller.Firewaller, wantFilterForwardDrop bool) ( // When enabling ip_forward set the default policy on forward chain to drop. if changed && wantFilterForwardDrop { - if err := fw.FilterForwardDrop(context.TODO(), firewaller.IPv4); err != nil { + if err := ffd.FilterForwardDrop(context.TODO(), firewaller.IPv4); err != nil { return err } } return nil } -func setupIPv6Forwarding(fw firewaller.Firewaller, wantFilterForwardDrop bool) (retErr error) { +func checkIPv6Forwarding() error { + enabledDef, err := getKernelBoolParam(ipv6ForwardConfDefault) + if err != nil { + return fmt.Errorf("checking IPv6 default forwarding: %w", err) + } + enabledAll, err := getKernelBoolParam(ipv6ForwardConfAll) + if err != nil { + return fmt.Errorf("checking IPv6 global forwarding: %w", err) + } + if enabledDef && enabledAll { + return nil + } + + // It's the user's responsibility to enable forwarding and secure their host. Or, + // start docker with --ip-forward=false to disable this check. + return errors.New("IPv6 global forwarding is disabled: check your host's firewalling and set sysctls net.ipv6.conf.all.forwarding=1 and net.ipv6.conf.default.forwarding=1, or disable this check using daemon option --ip-forward=false") +} + +func setupIPv6Forwarding(ffd filterForwardDropper, wantFilterForwardDrop bool) (retErr error) { // Set IPv6 default.forwarding, if needed. - // FIXME(robmry) - is it necessary to set this, setting "all" (below) does the job? + // Setting "all" (below) sets "default" as well, but need to check that "default" is + // set even if "all" is already set. changedDef, err := configureIPForwarding(ipv6ForwardConfDefault, '1') if err != nil { return err @@ -74,7 +111,7 @@ func setupIPv6Forwarding(fw firewaller.Firewaller, wantFilterForwardDrop bool) ( } if (changedAll || changedDef) && wantFilterForwardDrop { - if err := fw.FilterForwardDrop(context.TODO(), firewaller.IPv6); err != nil { + if err := ffd.FilterForwardDrop(context.TODO(), firewaller.IPv6); err != nil { return err } } diff --git a/daemon/libnetwork/drivers/bridge/setup_ip_forwarding_test.go b/daemon/libnetwork/drivers/bridge/setup_ip_forwarding_test.go index 65da1b9f79..7567c169de 100644 --- a/daemon/libnetwork/drivers/bridge/setup_ip_forwarding_test.go +++ b/daemon/libnetwork/drivers/bridge/setup_ip_forwarding_test.go @@ -14,19 +14,20 @@ import ( is "gotest.tools/v3/assert/cmp" ) -type ffdTestFirewaller struct { - ffd firewaller.IPVersion -} - -// NewNetwork is part of interface [firewaller.Firewaller]. -func (f *ffdTestFirewaller) NewNetwork(_ context.Context, _ firewaller.NetworkConfig) (firewaller.Network, error) { - return nil, nil +type ffDropper struct { + ffDrop4 bool + ffDrop6 bool } // FilterForwardDrop is part of interface [firewaller.Firewaller]. Just enough to check // it was called with the expected IPVersion. -func (f *ffdTestFirewaller) FilterForwardDrop(_ context.Context, ipv firewaller.IPVersion) error { - f.ffd = ipv +func (f *ffDropper) FilterForwardDrop(_ context.Context, ipv firewaller.IPVersion) error { + switch ipv { + case firewaller.IPv4: + f.ffDrop4 = true + case firewaller.IPv6: + f.ffDrop6 = true + } return nil } @@ -36,21 +37,16 @@ func TestSetupIPForwarding(t *testing.T) { for _, wantFFD := range []bool{true, false} { t.Run(fmt.Sprintf("wantFFD=%v", wantFFD), func(t *testing.T) { // Disable IP Forwarding if enabled - _, err := configureIPForwarding(ipv4ForwardConf, '0') - assert.NilError(t, err) + setForwarding(t, '0') // Set IP Forwarding - fw := &ffdTestFirewaller{} - err = setupIPv4Forwarding(fw, wantFFD) + ffd := &ffDropper{} + err := setupIPv4Forwarding(ffd, wantFFD) assert.NilError(t, err) // Check what the firewaller was told. - if wantFFD { - assert.Check(t, is.Equal(fw.ffd, firewaller.IPv4)) - } else { - var noVer firewaller.IPVersion - assert.Check(t, is.Equal(fw.ffd, noVer)) - } + assert.Check(t, is.Equal(ffd.ffDrop4, wantFFD)) + assert.Check(t, !ffd.ffDrop6) // Read new setting procSetting, err := os.ReadFile(ipv4ForwardConf) @@ -65,23 +61,15 @@ func TestSetupIP6Forwarding(t *testing.T) { for _, wantFFD := range []bool{true, false} { t.Run(fmt.Sprintf("wantFFD=%v", wantFFD), func(t *testing.T) { - _, err := configureIPForwarding(ipv6ForwardConfDefault, '0') - assert.NilError(t, err) - _, err = configureIPForwarding(ipv6ForwardConfAll, '0') - assert.NilError(t, err) + // Disable IP Forwarding if enabled + setForwarding(t, '0') // Set IP Forwarding - fw := &ffdTestFirewaller{} - err = setupIPv6Forwarding(fw, wantFFD) + ffd := &ffDropper{} + err := setupIPv6Forwarding(ffd, wantFFD) assert.NilError(t, err) - - // Check what the firewaller was told. - if wantFFD { - assert.Check(t, is.Equal(fw.ffd, firewaller.IPv6)) - } else { - var noVer firewaller.IPVersion - assert.Check(t, is.Equal(fw.ffd, noVer)) - } + assert.Check(t, !ffd.ffDrop4) + assert.Check(t, is.Equal(ffd.ffDrop6, wantFFD)) // Read new setting procSetting, err := os.ReadFile(ipv6ForwardConfDefault) @@ -93,3 +81,30 @@ func TestSetupIP6Forwarding(t *testing.T) { }) } } + +func TestCheckForwarding(t *testing.T) { + defer netnsutils.SetupTestOSContext(t)() + + setForwarding(t, '0') + err := checkIPv4Forwarding() + assert.Check(t, is.ErrorContains(err, "IPv4 forwarding is disabled")) + err = checkIPv6Forwarding() + assert.Check(t, is.ErrorContains(err, "IPv6 global forwarding is disabled")) + + setForwarding(t, '1') + err = checkIPv4Forwarding() + assert.Check(t, err) + err = checkIPv6Forwarding() + assert.Check(t, err) +} + +func setForwarding(t *testing.T, val byte) { + for _, sysctl := range []string{ + ipv4ForwardConf, + ipv6ForwardConfDefault, + ipv6ForwardConfAll, + } { + err := os.WriteFile(sysctl, []byte{val, '\n'}, 0o644) + assert.NilError(t, err) + } +} diff --git a/daemon/libnetwork/iptables/iptables.go b/daemon/libnetwork/iptables/iptables.go index f2d473cc93..dd46c8ef06 100644 --- a/daemon/libnetwork/iptables/iptables.go +++ b/daemon/libnetwork/iptables/iptables.go @@ -3,6 +3,7 @@ package iptables import ( + "bytes" "context" "errors" "fmt" @@ -411,6 +412,16 @@ func (iptable IPTable) SetDefaultPolicy(table Table, chain string, policy Policy return nil } +// HasPolicy returns true if the chain exists and has the given policy. +func (iptable IPTable) HasPolicy(table Table, chain string, policy Policy) bool { + out, err := iptable.Raw("-t", string(table), "-L", chain) + if err != nil { + return false + } + firstLine, _, _ := bytes.Cut(out, []byte("\n")) + return strings.Contains(string(firstLine), "policy "+string(policy)) +} + // AddReturnRule adds a return rule for the chain in the filter table func (iptable IPTable) AddReturnRule(table Table, chain string) error { if iptable.Exists(table, chain, "-j", "RETURN") { diff --git a/hack/make/.integration-daemon-start b/hack/make/.integration-daemon-start index 78f056356f..76d2efecdf 100644 --- a/hack/make/.integration-daemon-start +++ b/hack/make/.integration-daemon-start @@ -67,6 +67,13 @@ fi dockerd="dockerd" +# When running with the nftables backend, dockerd will not enable IP forwarding (by default, it +# will error on network creation if forwarding is not enabled). +if [ "$DOCKER_FIREWALL_BACKEND" = "nftables" ]; then + sysctl -w net.ipv4.ip_forward=1 > /dev/null + sysctl -w net.ipv6.conf.all.forwarding=1 > /dev/null +fi + if [ -n "$DOCKER_ROOTLESS" ]; then if [ -z "$TEST_SKIP_INTEGRATION_CLI" ]; then echo >&2 '# DOCKER_ROOTLESS requires TEST_SKIP_INTEGRATION_CLI to be set' diff --git a/integration/internal/testutils/networking/firewall.go b/integration/internal/testutils/networking/firewall.go index c7ba4e48ce..0c03087fc3 100644 --- a/integration/internal/testutils/networking/firewall.go +++ b/integration/internal/testutils/networking/firewall.go @@ -1,7 +1,6 @@ package networking import ( - "fmt" "os/exec" "regexp" "strings" @@ -9,45 +8,29 @@ import ( "github.com/moby/moby/v2/testutil/daemon" "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/icmd" "gotest.tools/v3/poll" ) -const ( - // The name of the bridge driver's nftables tables. - nftTable = "docker-bridges" - // The name of the filter-FORWARD chain in nftTable. - nftFFChain = "filter-FORWARD" -) - // Find the policy in, for example "Chain FORWARD (policy ACCEPT)". var rePolicy = regexp.MustCompile("policy ([A-Za-z]+)") // SetFilterForwardPolicies sets the default policy for the FORWARD chain in -// the filter tables for both IPv4 and IPv6. The original policy is restored -// using t.Cleanup(). +// the iptables filter tables for both IPv4 and IPv6. The original policy is +// restored using t.Cleanup(). // // There's only one filter-FORWARD policy, so this won't behave well if used by // tests running in parallel in a single network namespace that expect different // behaviour. -func SetFilterForwardPolicies(t *testing.T, firewallBackend string, policy string) { - t.Helper() - if strings.HasPrefix(firewallBackend, "iptables") { - setIptablesFFP(t, policy) - return - } - if strings.HasPrefix(firewallBackend, "nftables") { - setNftablesFFP(t, policy) - return - } - t.Fatalf("unknown firewall backend %s", firewallBackend) -} - -func setIptablesFFP(t *testing.T, policy string) { +func SetFilterForwardPolicies(t *testing.T, policy string) { t.Helper() for _, iptablesCmd := range []string{"iptables", "ip6tables"} { - origPolicy, err := getChainPolicy(t, exec.Command(iptablesCmd, "-L", "FORWARD")) - assert.NilError(t, err, "failed to get iptables policy") + out, err := exec.Command(iptablesCmd, "-L", "FORWARD").Output() + assert.NilError(t, err, "failed to get %s policy", iptablesCmd) + opMatch := rePolicy.FindSubmatch(out) + assert.Assert(t, is.Len(opMatch, 2), "searching for policy: %w", err) + origPolicy := string(opMatch[1]) if origPolicy == policy { continue } @@ -62,42 +45,6 @@ func setIptablesFFP(t *testing.T, policy string) { } } -func setNftablesFFP(t *testing.T, policy string) { - t.Helper() - policy = strings.ToLower(policy) - for _, family := range []string{"ip", "ip6"} { - origPolicy, err := getChainPolicy(t, exec.Command("nft", "list", "chain", family, nftTable, nftFFChain)) - assert.NilError(t, err, "failed to get nftables policy") - if origPolicy == policy { - continue - } - cmd := func(p string) *exec.Cmd { - return exec.Command("nft", "add", "chain", family, nftTable, nftFFChain, "{", "policy", p, ";", "}") - } - if err := cmd(policy).Run(); err != nil { - t.Fatalf("Failed to set %s filter-FORWARD policy: %v", family, err) - } - t.Cleanup(func() { - if err := cmd(origPolicy).Run(); err != nil { - t.Logf("Failed to restore %s filter-FORWARD policy: %v", family, err) - } - }) - } -} - -func getChainPolicy(t *testing.T, cmd *exec.Cmd) (string, error) { - t.Helper() - out, err := cmd.Output() - if err != nil { - return "", fmt.Errorf("getting policy: %w", err) - } - opMatch := rePolicy.FindSubmatch(out) - if len(opMatch) != 2 { - return "", fmt.Errorf("searching for policy: %w", err) - } - return string(opMatch[1]), nil -} - // FirewalldRunning returns true if "firewall-cmd --state" reports "running". func FirewalldRunning() bool { state, err := exec.Command("firewall-cmd", "--state").CombinedOutput() diff --git a/integration/internal/testutils/networking/l3_segment_linux.go b/integration/internal/testutils/networking/l3_segment_linux.go index 26b4a69659..eb923eff2a 100644 --- a/integration/internal/testutils/networking/l3_segment_linux.go +++ b/integration/internal/testutils/networking/l3_segment_linux.go @@ -77,6 +77,8 @@ func (l3 *L3Segment) AddHost(t *testing.T, hostname, nsName, ifname string, addr l3.bridge.MustRun(t, "ip", "link", "set", hostname, "up", "master", l3.bridge.Iface) host.MustRun(t, "ip", "link", "set", host.Iface, "up") host.MustRun(t, "ip", "link", "set", "lo", "up") + host.MustRun(t, "sysctl", "-w", "net.ipv4.ip_forward=1") + host.MustRun(t, "sysctl", "-w", "net.ipv6.conf.all.forwarding=1") for _, addr := range addrs { host.MustRun(t, "ip", "addr", "add", addr.String(), "dev", host.Iface, "nodad") diff --git a/integration/network/bridge/bridge_linux_test.go b/integration/network/bridge/bridge_linux_test.go index 85a89bac13..671d744479 100644 --- a/integration/network/bridge/bridge_linux_test.go +++ b/integration/network/bridge/bridge_linux_test.go @@ -246,6 +246,8 @@ func TestIPRangeAt64BitLimit(t *testing.T) { func TestFilterForwardPolicy(t *testing.T) { skip.If(t, testEnv.IsRootless, "rootless has its own netns") skip.If(t, networking.FirewalldRunning(), "can't use firewalld in host netns to add rules in L3Segment") + skip.If(t, strings.HasPrefix(testEnv.FirewallBackendDriver(), "nftables"), "no policy is set for nftables") + ctx := setupTest(t) // Set up a netns for each test to avoid sysctl and iptables pollution. @@ -305,30 +307,17 @@ func TestFilterForwardPolicy(t *testing.T) { ) host := l3.Hosts[hostname] - getFwdPolicy := func(usingNftables bool, fam string) string { + getFwdPolicy := func(cmd string) string { t.Helper() - if usingNftables { - out := host.MustRun(t, "nft", "list chain "+fam+" docker-bridges filter-FORWARD") - if strings.Contains(out, "policy accept") { - return "ACCEPT" - } - if strings.Contains(out, "policy drop") { - return "DROP" - } - t.Fatalf("Failed to determine nftables filter-FORWARD policy: %s", out) - return "" - } else { - cmd := fam + "tables" - out := host.MustRun(t, cmd, "-S", "FORWARD") - if strings.HasPrefix(out, "-P FORWARD ACCEPT") { - return "ACCEPT" - } - if strings.HasPrefix(out, "-P FORWARD DROP") { - return "DROP" - } - t.Fatalf("Failed to determine %s FORWARD policy: %s", cmd, out) - return "" + out := host.MustRun(t, cmd, "-S", "FORWARD") + if strings.HasPrefix(out, "-P FORWARD ACCEPT") { + return "ACCEPT" } + if strings.HasPrefix(out, "-P FORWARD DROP") { + return "DROP" + } + t.Fatalf("Failed to determine %s FORWARD policy: %s", cmd, out) + return "" } type sysctls struct{ v4, v6def, v6all string } @@ -354,22 +343,21 @@ func TestFilterForwardPolicy(t *testing.T) { d.StartWithBusybox(ctx, t, tc.daemonArgs...) t.Cleanup(func() { d.Stop(t) }) }) - usingNftables := d.FirewallBackendDriver(t) == "nftables" c := d.NewClientT(t) t.Cleanup(func() { c.Close() }) // If necessary, the IPv4 policy should have been updated when the default bridge network was created. - assert.Check(t, is.Equal(getFwdPolicy(usingNftables, "ip"), tc.expPolicy)) + assert.Check(t, is.Equal(getFwdPolicy("iptables"), tc.expPolicy)) // IPv6 policy should not have been updated yet. - assert.Check(t, is.Equal(getFwdPolicy(usingNftables, "ip6"), "ACCEPT")) + assert.Check(t, is.Equal(getFwdPolicy("ip6tables"), "ACCEPT")) assert.Check(t, is.Equal(getSysctls(), sysctls{tc.expForwarding, tc.initForwarding, tc.initForwarding})) // If necessary, creating an IPv6 network should update the sysctls and policy. const netName = "testnetffp" network.CreateNoError(ctx, t, c, netName, network.WithIPv6()) t.Cleanup(func() { network.RemoveNoError(ctx, t, c, netName) }) - assert.Check(t, is.Equal(getFwdPolicy(usingNftables, "ip"), tc.expPolicy)) - assert.Check(t, is.Equal(getFwdPolicy(usingNftables, "ip6"), tc.expPolicy)) + assert.Check(t, is.Equal(getFwdPolicy("iptables"), tc.expPolicy)) + assert.Check(t, is.Equal(getFwdPolicy("ip6tables"), tc.expPolicy)) assert.Check(t, is.Equal(getSysctls(), sysctls{tc.expForwarding, tc.expForwarding, tc.expForwarding})) }) } diff --git a/integration/networking/bridge_linux_test.go b/integration/networking/bridge_linux_test.go index 47ec967f97..919d5d0871 100644 --- a/integration/networking/bridge_linux_test.go +++ b/integration/networking/bridge_linux_test.go @@ -358,7 +358,6 @@ func TestBridgeINCRouted(t *testing.T) { d := daemon.New(t) d.StartWithBusybox(ctx, t) t.Cleanup(func() { d.Stop(t) }) - firewallBackend := d.FirewallBackendDriver(t) c := d.NewClientT(t) t.Cleanup(func() { c.Close() }) @@ -457,10 +456,12 @@ func TestBridgeINCRouted(t *testing.T) { }, } - for _, fwdPolicy := range []string{"ACCEPT", "DROP"} { - networking.SetFilterForwardPolicies(t, firewallBackend, fwdPolicy) + runTests := func(testName, policy string) { networking.FirewalldReload(t, d) - t.Run(fwdPolicy, func(t *testing.T) { + t.Run(testName, func(t *testing.T) { + if policy != "" { + networking.SetFilterForwardPolicies(t, policy) + } for _, tc := range testcases { t.Run(tc.name+"/v4/ping", func(t *testing.T) { t.Parallel() @@ -497,6 +498,13 @@ func TestBridgeINCRouted(t *testing.T) { } }) } + + if strings.HasPrefix(d.FirewallBackendDriver(t), "iptables") { + runTests("iptables-ACCEPT", "ACCEPT") + runTests("iptables-DROP", "DROP") + } else { + runTests("nftables", "") + } } // TestAccessToPublishedPort checks that a container in one network can diff --git a/integration/networking/port_mapping_linux_test.go b/integration/networking/port_mapping_linux_test.go index 231258b4db..1993878d5e 100644 --- a/integration/networking/port_mapping_linux_test.go +++ b/integration/networking/port_mapping_linux_test.go @@ -648,7 +648,6 @@ func TestDirectRoutingOpenPorts(t *testing.T) { d := daemon.New(t) d.StartWithBusybox(ctx, t) t.Cleanup(func() { d.Stop(t) }) - firewallBackend := d.FirewallBackendDriver(t) c := d.NewClientT(t) t.Cleanup(func() { c.Close() }) @@ -770,9 +769,11 @@ func TestDirectRoutingOpenPorts(t *testing.T) { // Run the ping and http tests in two parallel groups, rather than waiting for // ping/http timeouts separately. (The iptables filter-FORWARD policy affects the // whole host, so ACCEPT/DROP tests can't be parallelized). - for _, fwdPolicy := range []string{"ACCEPT", "DROP"} { - networking.SetFilterForwardPolicies(t, firewallBackend, fwdPolicy) - t.Run(fwdPolicy, func(t *testing.T) { + runTests := func(testName, policy string) { + t.Run(testName, func(t *testing.T) { + if policy != "" { + networking.SetFilterForwardPolicies(t, policy) + } for gwMode := range networks { t.Run(gwMode+"/v4/ping", func(t *testing.T) { testPing(t, "ping", networks[gwMode].ipv4, expPingExit[gwMode]) @@ -795,6 +796,13 @@ func TestDirectRoutingOpenPorts(t *testing.T) { } }) } + + if strings.HasPrefix(d.FirewallBackendDriver(t), "iptables") { + runTests("iptables-ACCEPT", "ACCEPT") + runTests("iptables-DROP", "DROP") + } else { + runTests("nftables", "") + } } func TestAcceptFwMark(t *testing.T) {