dilim/moby
1
0 Fork 0
mirror of https://github.com/moby/moby.git synced 2026-11-06 05:17:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

libnetwork/d/bridge: remove leftover DOCKER chain

Browse Source 
When the daemon started with the nftables backend after a release older
than 28.0.0 had run with iptables, NewCleaner deleted the jumps from
filter-FORWARD to DOCKER-FORWARD and DOCKER-ISOLATION-STAGE-1, then
removed Docker's chains. Those releases also jumped from FORWARD to the
filter table's DOCKER chain for each bridge ("-o docker0 -j DOCKER").
iptables refuses to delete a chain that a rule still jumps to, and
removeIPChains ignores the error. The cleaner's DelNetwork deleted the
per-bridge jump later, when the bridge driver restored the network, but
nothing tried to remove the chain again. The empty DOCKER chain
remained.

When the bridge driver has restored its networks and endpoints, call
the cleaner's new Done method, which removes the chains again.

Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
Cory Snider committed 2026-11-04 12:25:59 -05:00
1 parent 3f2a441dfd
commit 50416cf892
6 files changed
+149 -7
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

daemon/libnetwork/drivers/bridge/bridge_store.go
+3 -3
View File
@@ -41,10 +41,10 @@ func (d *driver) initStore() error {
return err
}
// If there's a firewall cleaner, it's done its job by cleaning up rules
// belonging to the restored networks. So, drop it.
// If there's a firewall cleaner, it's cleaned up rules belonging to the
// restored networks. Let it finish, then drop it.
if nft, ok := d.firewaller.(*nftabler.Nftabler); ok {
nft.SetFirewallCleaner(nil)
nft.FinishCleanup(context.TODO())
}
return nil
daemon/libnetwork/drivers/bridge/internal/firewaller/firewaller.go
+5
View File
@@ -138,6 +138,11 @@ type FirewallCleaner interface {
// DelPorts removes firewall rules associated with the specified port bindings.
// It should be called by the new Firewaller when adding new port mappings.
DelPorts(ctx context.Context, nc NetworkConfig, pbs []types.PortBinding)
// Done deletes rules that could only be deleted after the calls to
// DelNetwork, DelEndpoint and DelPorts for restored networks and
// endpoints. It should be called by the new Firewaller once they have
// been restored.
Done(ctx context.Context)
// DelLink removes firewall rules associated with a legacy link.
// It should be called by the new Firewaller when adding a new legacy link.
// (Excluded from the interface at present, it's not required by any current
daemon/libnetwork/drivers/bridge/internal/iptabler/cleaner.go
+19 -3
View File
@@ -13,7 +13,9 @@ import (
)
type iptablesCleaner struct {
config firewaller.Config
config firewaller.Config
cleaned4 bool
cleaned6 bool
}
// NewCleaner checks for iptables rules left behind by an old daemon that was using
@@ -27,7 +29,8 @@ type iptablesCleaner struct {
// this stage, interface names, addresses, etc. are unknown. So, also return
// FirewallCleaner that the new Firewaller can use to delete those rules while
// it's setting up those networks/ports (probably on replay from persistent
// storage).
// storage). A chain that one of those rules jumps to can't be removed until
// the rule is deleted, so the FirewallCleaner's Done removes the chains again.
//
// If there are no old rules to clean up, return nil.
func NewCleaner(ctx context.Context, config firewaller.Config) firewaller.FirewallCleaner {
@@ -60,7 +63,7 @@ func NewCleaner(ctx context.Context, config firewaller.Config) firewaller.Firewa
if !cleaned4 && !cleaned6 {
return nil
}
return &iptablesCleaner{config: config}
return &iptablesCleaner{config: config, cleaned4: cleaned4, cleaned6: cleaned6}
}
func (ic iptablesCleaner) DelNetwork(ctx context.Context, nc firewaller.NetworkConfig) {
@@ -102,3 +105,16 @@ func (ic iptablesCleaner) DelPorts(ctx context.Context, nc firewaller.NetworkCon
}
_ = n.DelPorts(ctx, pbs)
}
// Done removes the chains that NewCleaner couldn't remove. Daemons older than
// 28.0.0 jumped from filter-FORWARD to DOCKER for each bridge ("-o <bridge> -j
// DOCKER"), and iptables can't remove a chain that a rule jumps to. DelNetwork
// deletes those jumps for restored networks.
func (ic iptablesCleaner) Done(ctx context.Context) {
if ic.cleaned4 {
removeIPChains(ctx, iptables.IPv4)
}
if ic.cleaned6 {
removeIPChains(ctx, iptables.IPv6)
}
}
daemon/libnetwork/drivers/bridge/internal/iptabler/iptabler_test.go
+9 -1
View File
@@ -106,7 +106,9 @@ func TestCleanupOldIngressChains(t *testing.T) {
// TestCleanerPerBridgeRules checks that the cleaner's DelNetwork deletes the
// per-bridge rules that daemons older than 28.0.1 left in filter-FORWARD, for
// internal and non-internal networks.
// internal and non-internal networks. Then, that Done removes the filter
// table's DOCKER chain, which NewCleaner can't remove while filter-FORWARD
// has per-bridge jumps to it.
func TestCleanerPerBridgeRules(t *testing.T) {
skip.If(t, iptables.UsingFirewalld(), "firewalld is running in the host netns, it can't modify rules in the test's netns")
@@ -149,6 +151,12 @@ func TestCleanerPerBridgeRules(t *testing.T) {
assert.NilError(t, err)
assert.Check(t, is.Equal(string(out), "-P FORWARD ACCEPT\n"), "version:%s", version)
}
fc.Done(t.Context())
for _, version := range versions {
iptable := iptables.GetIptable(version)
assert.Check(t, !iptable.ExistChain(dockerChain, iptables.Filter), "version:%s", version)
}
}
// TestIptabler tests combinations of firewaller options against golden results.
daemon/libnetwork/drivers/bridge/internal/nftabler/cleaner.go
+9
View File
@@ -37,3 +37,12 @@ func tryCleanup(ctx context.Context, family nftables.Family, label string) {
func (nft *Nftabler) SetFirewallCleaner(fc firewaller.FirewallCleaner) {
nft.cleaner = fc
}
// FinishCleanup tells the FirewallCleaner, if there is one, that networks and
// endpoints have been restored. Then, it drops the FirewallCleaner.
func (nft *Nftabler) FinishCleanup(ctx context.Context) {
if nft.cleaner != nil {
nft.cleaner.Done(ctx)
nft.cleaner = nil
}
}
integration/network/bridge/bridge_linux_test.go
+104
View File
@@ -1125,6 +1125,110 @@ func TestFirewallBackendSwitch(t *testing.T) {
assert.Check(t, !nftablesTablesExist(), "nftables tables exist after running with iptables")
}
// TestFirewallBackendSwitchFrom27 checks that, when the daemon starts with the
// nftables backend after a 27.x release ran with iptables, it deletes the 27.x
// release's chains and rules. In that layout, the filter-FORWARD chain has
// per-bridge rules, including a jump to the DOCKER chain.
func TestFirewallBackendSwitchFrom27(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")
_ = setupTest(t)
// Run in a clean netns.
addr4 := netip.MustParseAddr("192.168.126.1")
addr6 := netip.MustParseAddr("fd76:c828:41fa::1")
l3 := networking.NewL3Segment(t, "test-fwbe27",
netip.PrefixFrom(addr4, 24),
netip.PrefixFrom(addr6, 64),
)
defer l3.Destroy(t)
addr4, addr6 = addr4.Next(), addr6.Next()
const hostname = "fwbe27"
l3.AddHost(t, hostname, hostname+"-netns", "eth0",
netip.PrefixFrom(addr4, 24),
netip.PrefixFrom(addr6, 64),
)
host := l3.Hosts[hostname]
// Run without OTel because there's no routing from this netns for it.
d := daemon.New(t, daemon.WithEnvVars("OTEL_EXPORTER_OTLP_ENDPOINT="))
// A start with the iptables backend leaves the default bridge network in
// the bridge driver's store, so the next start restores it, as it would
// after an upgrade from 27.x.
host.Do(t, func() {
d.Start(t, "--firewall-backend=iptables")
d.Stop(t)
})
// Replace the rules with those a 27.x release created for the default
// bridge network, from the first generated copy of
// iptablesdoc/generated/new-daemon.md. A rule with cmd set is only for
// that command. The default bridge network had no IPv6, so ip6tables has
// no per-bridge rules.
rules27 := []struct {
cmd string
table string
args []string
}{
{table: "filter", args: []string{"-N", "DOCKER"}},
{table: "filter", args: []string{"-N", "DOCKER-ISOLATION-STAGE-1"}},
{table: "filter", args: []string{"-N", "DOCKER-ISOLATION-STAGE-2"}},
{table: "filter", args: []string{"-N", "DOCKER-USER"}},
{table: "filter", args: []string{"-A", "FORWARD", "-j", "DOCKER-USER"}},
{table: "filter", args: []string{"-A", "FORWARD", "-j", "DOCKER-ISOLATION-STAGE-1"}},
{cmd: "iptables", table: "filter", args: []string{"-A", "FORWARD", "-o", "docker0", "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT"}},
{cmd: "iptables", table: "filter", args: []string{"-A", "FORWARD", "-o", "docker0", "-j", "DOCKER"}},
{cmd: "iptables", table: "filter", args: []string{"-A", "FORWARD", "-i", "docker0", "!", "-o", "docker0", "-j", "ACCEPT"}},
{cmd: "iptables", table: "filter", args: []string{"-A", "FORWARD", "-i", "docker0", "-o", "docker0", "-j", "ACCEPT"}},
{cmd: "iptables", table: "filter", args: []string{"-A", "DOCKER-ISOLATION-STAGE-1", "-i", "docker0", "!", "-o", "docker0", "-j", "DOCKER-ISOLATION-STAGE-2"}},
{table: "filter", args: []string{"-A", "DOCKER-ISOLATION-STAGE-1", "-j", "RETURN"}},
{cmd: "iptables", table: "filter", args: []string{"-A", "DOCKER-ISOLATION-STAGE-2", "-o", "docker0", "-j", "DROP"}},
{table: "filter", args: []string{"-A", "DOCKER-ISOLATION-STAGE-2", "-j", "RETURN"}},
{table: "filter", args: []string{"-A", "DOCKER-USER", "-j", "RETURN"}},
{table: "nat", args: []string{"-N", "DOCKER"}},
{table: "nat", args: []string{"-A", "PREROUTING", "-m", "addrtype", "--dst-type", "LOCAL", "-j", "DOCKER"}},
{cmd: "iptables", table: "nat", args: []string{"-A", "OUTPUT", "!", "-d", "127.0.0.0/8", "-m", "addrtype", "--dst-type", "LOCAL", "-j", "DOCKER"}},
{cmd: "ip6tables", table: "nat", args: []string{"-A", "OUTPUT", "!", "-d", "::1/128", "-m", "addrtype", "--dst-type", "LOCAL", "-j", "DOCKER"}},
{cmd: "iptables", table: "nat", args: []string{"-A", "POSTROUTING", "-s", "172.17.0.0/16", "!", "-o", "docker0", "-j", "MASQUERADE"}},
{cmd: "iptables", table: "nat", args: []string{"-A", "DOCKER", "-i", "docker0", "-j", "RETURN"}},
}
for _, cmd := range []string{"iptables", "ip6tables"} {
for _, table := range []string{"filter", "nat"} {
host.MustRun(t, cmd, "-t", table, "-F")
host.MustRun(t, cmd, "-t", table, "-X")
}
for _, r := range rules27 {
if r.cmd != "" && r.cmd != cmd {
continue
}
host.MustRun(t, cmd, append([]string{"-t", r.table}, r.args...)...)
}
}
host.Do(t, func() {
d.Start(t, "--firewall-backend=nftables")
d.Stop(t)
})
// The daemon doesn't delete DOCKER-USER, or the jump to it. Everything
// else in the 27.x rules is Docker's, and must be gone.
for _, cmd := range []string{"iptables-save", "ip6tables-save"} {
dump := host.MustRun(t, cmd)
var remaining []string
for line := range strings.SplitSeq(dump, "\n") {
if strings.Contains(line, "DOCKER-USER") {
continue
}
if strings.HasPrefix(line, ":DOCKER") || strings.HasPrefix(line, "-A ") {
remaining = append(remaining, line)
}
}
assert.Check(t, is.Len(remaining, 0), "%s:\n%s", cmd, dump)
}
}
func TestEmptyPortBindingsBC(t *testing.T) {
ctx := setupTest(t)
d := daemon.New(t)