From 39ab39327417e1feebbc48a98748579ff8872e45 Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Mon, 9 Jun 2025 19:39:21 +0100 Subject: [PATCH] Add daemon option --firewall-backend Signed-off-by: Rob Murray --- daemon/command/config_unix.go | 1 + daemon/config/config.go | 4 ++++ daemon/config/config_linux.go | 12 ++++++++++ daemon/config/config_windows.go | 4 ++++ daemon/daemon.go | 1 + daemon/libnetwork/config/config.go | 8 +++++++ daemon/libnetwork/controller.go | 4 +++- daemon/libnetwork/firewall_linux.go | 24 ++++++++++++------- daemon/libnetwork/firewall_others.go | 4 +++- .../internal/nftables/nftables_linux.go | 12 ++++++---- .../internal/nftables/nftables_linux_test.go | 4 ++-- hack/make/.integration-daemon-start | 1 + hack/make/run | 1 + hack/test/e2e-run.sh | 1 + .../network/bridge/bridge_linux_test.go | 3 +-- testutil/daemon/daemon.go | 13 ++++++---- 16 files changed, 74 insertions(+), 23 deletions(-) diff --git a/daemon/command/config_unix.go b/daemon/command/config_unix.go index 42a7cfffc3..fa9e3c5f9f 100644 --- a/daemon/command/config_unix.go +++ b/daemon/command/config_unix.go @@ -51,6 +51,7 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) { flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers") flags.StringVar(&conf.IpcMode, "default-ipc-mode", conf.IpcMode, `Default mode for containers ipc ("shareable" | "private")`) flags.Var(&conf.NetworkConfig.DefaultAddressPools, "default-address-pool", "Default address pools for node specific local networks") + flags.StringVar(&conf.NetworkConfig.FirewallBackend, "firewall-backend", "", "Firewall backend to use, iptables or nftables") // rootless needs to be explicitly specified for running "rootful" dockerd in rootless dockerd (#38702) // Note that conf.BridgeConfig.UserlandProxyPath and honorXDG are configured according to the value of rootless.RunningWithRootlessKit, not the value of --rootless. flags.BoolVar(&conf.Rootless, "rootless", conf.Rootless, "Enable rootless mode; typically used with RootlessKit") diff --git a/daemon/config/config.go b/daemon/config/config.go index 51797ea785..9ff57a6873 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -148,6 +148,10 @@ type NetworkConfig struct { NetworkControlPlaneMTU int `json:"network-control-plane-mtu,omitempty"` // Default options for newly created networks DefaultNetworkOpts map[string]map[string]string `json:"default-network-opts,omitempty"` + // FirewallBackend overrides the daemon's default selection of firewall + // implementation. Currently only used on Linux, it is an error to + // supply a value for other platforms. + FirewallBackend string `json:"firewall-backend,omitempty"` } // TLSOptions defines TLS configuration for the daemon server. diff --git a/daemon/config/config_linux.go b/daemon/config/config_linux.go index 97db38b665..10478c76ae 100644 --- a/daemon/config/config_linux.go +++ b/daemon/config/config_linux.go @@ -243,6 +243,10 @@ func validatePlatformConfig(conf *Config) error { return errors.Wrap(err, "invalid fixed-cidr-v6") } + if err := validateFirewallBackend(conf.FirewallBackend); err != nil { + return errors.Wrap(err, "invalid firewall-backend") + } + return verifyDefaultCgroupNsMode(conf.CgroupNamespaceMode) } @@ -294,6 +298,14 @@ func verifyDefaultIpcMode(mode string) error { return nil } +func validateFirewallBackend(val string) error { + switch val { + case "", "iptables", "nftables": + return nil + } + return errors.New(`allowed values are "iptables" and "nftables"`) +} + func verifyDefaultCgroupNsMode(mode string) error { cm := container.CgroupnsMode(mode) if !cm.Valid() { diff --git a/daemon/config/config_windows.go b/daemon/config/config_windows.go index cdbd713521..32696b25a9 100644 --- a/daemon/config/config_windows.go +++ b/daemon/config/config_windows.go @@ -2,6 +2,7 @@ package config import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -88,6 +89,9 @@ func validatePlatformConfig(conf *Config) error { if conf.MTU != 0 && conf.MTU != DefaultNetworkMtu { log.G(context.TODO()).Warn(`WARNING: MTU for the default network is not configurable on Windows, and this option will be ignored.`) } + if conf.FirewallBackend != "" { + return errors.New("firewall-backend can only be configured on Linux") + } return nil } diff --git a/daemon/daemon.go b/daemon/daemon.go index 987524e4bc..0312975a85 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1457,6 +1457,7 @@ func (daemon *Daemon) networkOptions(conf *config.Config, pg plugingetter.Plugin nwconfig.OptionDefaultNetwork(network.DefaultNetwork), nwconfig.OptionLabels(conf.Labels), nwconfig.OptionNetworkControlPlaneMTU(conf.NetworkControlPlaneMTU), + nwconfig.OptionFirewallBackend(conf.FirewallBackend), driverOptions(conf), } diff --git a/daemon/libnetwork/config/config.go b/daemon/libnetwork/config/config.go index 96a89660b5..ff6b51f86b 100644 --- a/daemon/libnetwork/config/config.go +++ b/daemon/libnetwork/config/config.go @@ -42,6 +42,7 @@ type Config struct { DatastoreBucket string ActiveSandboxes map[string]any PluginGetter plugingetter.PluginGetter + FirewallBackend string } // New creates a new Config and initializes it with the given Options. @@ -154,3 +155,10 @@ func OptionActiveSandboxes(sandboxes map[string]any) Option { c.ActiveSandboxes = sandboxes } } + +// OptionFirewallBackend returns an option setter for selection of the firewall backend. +func OptionFirewallBackend(val string) Option { + return func(c *Config) { + c.FirewallBackend = val + } +} diff --git a/daemon/libnetwork/controller.go b/daemon/libnetwork/controller.go index a1ae4145cf..5c30f06080 100644 --- a/daemon/libnetwork/controller.go +++ b/daemon/libnetwork/controller.go @@ -168,7 +168,9 @@ func New(ctx context.Context, cfgOptions ...config.Option) (_ *Controller, retEr diagnosticServer: diagnostic.New(), } - c.selectFirewallBackend() + if err := c.selectFirewallBackend(); err != nil { + return nil, err + } c.drvRegistry.Notify = c // External plugins don't need config passed through daemon. They can diff --git a/daemon/libnetwork/firewall_linux.go b/daemon/libnetwork/firewall_linux.go index ed34d281ac..2ab12b3c55 100644 --- a/daemon/libnetwork/firewall_linux.go +++ b/daemon/libnetwork/firewall_linux.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os" "github.com/containerd/log" "github.com/docker/docker/daemon/libnetwork/internal/nftables" @@ -13,16 +12,23 @@ import ( const userChain = "DOCKER-USER" -func (c *Controller) selectFirewallBackend() { - // Don't try to enable nftables if firewalld is running. - if iptables.UsingFirewalld() { - return +func (c *Controller) selectFirewallBackend() error { + // If explicitly configured to use iptables, don't consider nftables. + if c.cfg.FirewallBackend == "iptables" { + return nil } - // Only try to use nftables if explicitly enabled by env-var. - // TODO(robmry) - command line options? - if os.Getenv("DOCKER_FIREWALL_BACKEND") == "nftables" { - _ = nftables.Enable() + // If configured to use nftables, but it can't be initialised, return an error. + if c.cfg.FirewallBackend == "nftables" { + // Don't try to enable nftables if firewalld is running. + if iptables.UsingFirewalld() { + return errors.New("firewall-backend is set to nftables, but firewalld is running") + } + if err := nftables.Enable(); err != nil { + return fmt.Errorf("firewall-backend is set to nftables: %v", err) + } + return nil } + return nil } // Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6) that's diff --git a/daemon/libnetwork/firewall_others.go b/daemon/libnetwork/firewall_others.go index fb5996f7ba..6e3bfa3b31 100644 --- a/daemon/libnetwork/firewall_others.go +++ b/daemon/libnetwork/firewall_others.go @@ -2,6 +2,8 @@ package libnetwork -func (c *Controller) selectFirewallBackend() {} +func (c *Controller) selectFirewallBackend() error { + return nil +} func (c *Controller) setupUserChains() {} diff --git a/daemon/libnetwork/internal/nftables/nftables_linux.go b/daemon/libnetwork/internal/nftables/nftables_linux.go index 9fbee5d643..ca99134f92 100644 --- a/daemon/libnetwork/internal/nftables/nftables_linux.go +++ b/daemon/libnetwork/internal/nftables/nftables_linux.go @@ -62,6 +62,8 @@ var ( // nftPath is the path of the "nft" tool, set by [Enable] and left empty if the tool // is not present - in which case, nftables is disabled. nftPath string + // Error returned by Enable if nftables could not be initialised. + nftEnableError error // incrementalUpdateTempl is a parsed text/template, used to apply incremental updates. incrementalUpdateTempl *template.Template // reloadTempl is a parsed text/template, used to apply a whole table. @@ -134,21 +136,23 @@ const ( nftTypeIfname nftType = "ifname" ) -// Enable checks whether the "nft" tool is available, and returns true if it is. -// Subsequent calls to [Enabled] will return the same result. -func Enable() bool { +// Enable tries once to initialise nftables. +func Enable() error { enableOnce.Do(func() { path, err := exec.LookPath("nft") if err != nil { log.G(context.Background()).WithError(err).Warnf("Failed to find nft tool") + nftEnableError = fmt.Errorf("failed to find nft tool: %w", err) + return } if err := parseTemplate(); err != nil { log.G(context.Background()).WithError(err).Error("Internal error while initialising nftables") + nftEnableError = fmt.Errorf("internal error while initialising nftables: %w", err) return } nftPath = path }) - return nftPath != "" + return nftEnableError } // Enabled returns true if the "nft" tool is available and [Enable] has been called. diff --git a/daemon/libnetwork/internal/nftables/nftables_linux_test.go b/daemon/libnetwork/internal/nftables/nftables_linux_test.go index 9eb38869ab..526fe89956 100644 --- a/daemon/libnetwork/internal/nftables/nftables_linux_test.go +++ b/daemon/libnetwork/internal/nftables/nftables_linux_test.go @@ -14,7 +14,7 @@ import ( func testSetup(t *testing.T) func() { t.Helper() - if !Enable() { + if err := Enable(); err != nil { // Make sure it didn't fail because of a bug in the text/template. assert.NilError(t, parseTemplate()) // If this is not CI, skip. @@ -22,7 +22,7 @@ func testSetup(t *testing.T) func() { t.Skip("Cannot enable nftables, no 'nft' command in $PATH ?") } // In CI, nft should always be installed, fail the test. - t.Fatal("Failed to enable nftables") + t.Fatalf("Failed to enable nftables: %s", err) } cleanupContext := netnsutils.SetupTestOSContext(t) return func() { diff --git a/hack/make/.integration-daemon-start b/hack/make/.integration-daemon-start index b3e1837896..f0f1ea5b5a 100644 --- a/hack/make/.integration-daemon-start +++ b/hack/make/.integration-daemon-start @@ -118,6 +118,7 @@ if [ -z "$DOCKER_TEST_HOST" ]; then --storage-driver "$DOCKER_GRAPHDRIVER" \ --pidfile "$DEST/docker.pid" \ --userland-proxy="$DOCKER_USERLANDPROXY" \ + --firewall-backend="$DOCKER_FIREWALL_BACKEND" \ ${storage_params} \ ${extra_params} \ &> "$DEST/docker.log" diff --git a/hack/make/run b/hack/make/run index 16d9febc34..a528d18c07 100644 --- a/hack/make/run +++ b/hack/make/run @@ -58,6 +58,7 @@ args=( --host="unix://${socket}" --storage-driver="${DOCKER_GRAPHDRIVER}" --userland-proxy="${DOCKER_USERLANDPROXY}" + --firewall-backend="${DOCKER_FIREWALL_BACKEND}" --tls=false $storage_params $extra_params diff --git a/hack/test/e2e-run.sh b/hack/test/e2e-run.sh index 744ca8ceb1..ec0a9c13c9 100755 --- a/hack/test/e2e-run.sh +++ b/hack/test/e2e-run.sh @@ -47,6 +47,7 @@ test_env() { DOCKER_CERT_PATH="$DOCKER_TEST_CERT_PATH" \ DOCKER_GRAPHDRIVER="$DOCKER_GRAPHDRIVER" \ DOCKER_USERLANDPROXY="$DOCKER_USERLANDPROXY" \ + DOCKER_FIREWALL_BACKEND="$DOCKER_FIREWALL_BACKEND" \ DOCKER_HOST="$DOCKER_HOST" \ DOCKER_REMAP_ROOT="$DOCKER_REMAP_ROOT" \ DOCKER_REMOTE_DAEMON="$DOCKER_REMOTE_DAEMON" \ diff --git a/integration/network/bridge/bridge_linux_test.go b/integration/network/bridge/bridge_linux_test.go index 0a69c64d09..a689418174 100644 --- a/integration/network/bridge/bridge_linux_test.go +++ b/integration/network/bridge/bridge_linux_test.go @@ -856,9 +856,8 @@ func TestFirewallBackendSwitch(t *testing.T) { networkCreated := false runDaemon := func(backend string) { - d.SetEnvVar("DOCKER_FIREWALL_BACKEND", backend) host.Do(t, func() { - d.StartWithBusybox(ctx, t) + d.StartWithBusybox(ctx, t, "--firewall-backend="+backend) defer d.Stop(t) // Create a network (and its firewall rules) first time through. diff --git a/testutil/daemon/daemon.go b/testutil/daemon/daemon.go index c693323c36..48284643c4 100644 --- a/testutil/daemon/daemon.go +++ b/testutil/daemon/daemon.go @@ -234,10 +234,6 @@ func New(t testing.TB, ops ...Option) *Daemon { } ops = append(ops, WithOOMScoreAdjust(-500)) - if val, ok := os.LookupEnv("DOCKER_FIREWALL_BACKEND"); ok { - ops = append(ops, WithEnvVars("DOCKER_FIREWALL_BACKEND="+val)) - } - d, err := NewDaemon(dest, ops...) assert.NilError(t, err, "could not create daemon at %q", dest) if d.rootlessUser != nil && d.dockerdBinary != defaultDockerdBinary { @@ -534,6 +530,15 @@ func (d *Daemon) StartWithLogFile(out *os.File, providedArgs ...string) error { d.args = append(d.args, "--storage-driver", d.storageDriver) } + hasFwBackendArg := !slices.ContainsFunc(providedArgs, func(s string) bool { + return strings.HasPrefix(s, "--firewall-backend") + }) + if hasFwBackendArg { + if fw := os.Getenv("DOCKER_FIREWALL_BACKEND"); fw != "" { + d.args = append(d.args, "--firewall-backend="+fw) + } + } + d.args = append(d.args, providedArgs...) cmd := exec.Command(dockerdBinary, d.args...) cmd.Env = append(os.Environ(), "DOCKER_SERVICE_PREFER_OFFLINE_IMAGE=1")