[27.x backport] Fix: setup user chains even if there are running containers #48714

robmry · 2024-10-21T11:44:27Z

backport Fix: setup user chains during libnetwork controller initialization #48577

Currently, the DOCKER-USER chains are set up when the firewall is reloaded or when a network is created:

Lines 709 to 715 in 4001d07

    
           // Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6) 
        
           // that"s enabled in the controller"s configuration. 
        
           for _, ipVersion := range c.enabledIptablesVersions() { 
        
           	if err := setupUserChain(ipVersion); err != nil { 
        
           		log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name) 
        
           	} 
        
           }

During a normal startup, the daemon creates the bridge network, so the DOCKER-USER chains are set up.

But when live-restore is enabled, there may be running containers when the daemon starts. If that"s the case, the configureNetworking function will not be called:

moby/daemon/daemon_unix.go

Lines 848 to 852 in 4001d07

    
           if len(activeSandboxes) > 0 { 
        
           	log.G(context.TODO()).Info("there are running containers, updated network configuration will not take affect") 
        
           } else if err := configureNetworking(daemon.netController, cfg); err != nil { 
        
           	return err 
        
           }

configureNetworking calls initBridgeDriver, which calls NewNetwork, which calls setupUserChain, so if configureNetworking isn"t called the user chains won"t be set up.

This is a problem if the iptables rules change while the daemon is stopped.

- What I did

I made sure the user chains are set up on startup, even if the configureNetworking function is not called

- How I did it

I put the logic for setting up user chains for IPv4 and IPv6 in a separate function, which is called in the original place in NewNetwork, but also in initNetworkController even if there are running containers.

- How to verify it

I still didn"t write an integration test, I"ll add it when I have some time.

To manually test:

dockerd --live-restore
Create a dummy container: docker run -d busybox sleep 300
Stop dockerd
Flush the FORWARD chain: iptables -F FORWARD
Run again dockerd --live-restore
List the rules: iptables -S FORWARD

-A FORWARD -j DOCKER-USER should be there

- Description for the changelog

After a daemon restart with live-restore, ensure an iptables jump to the DOCKER-USER chain is placed before other rules.

- A picture of a cute animal (not mandatory but encouraged)

Currently, the DOCKER-USER chains are set up on firewall reload or network creation. If there are running containers at startup, configureNetworking won"t be called (daemon/daemon_unix.go), so the user chains won"t be setup. This commit puts the setup logic on a separate function, and calls it on the original place and on initNetworkController. Signed-off-by: Andrés Maldonado <[email protected]> (cherry picked from commit a8bfa83) Signed-off-by: Rob Murray <[email protected]>

robmry added area/networking kind/bugfix PR's that fix bugs area/networking/firewalling labels Oct 21, 2024

robmry added this to the 27.4.0 milestone Oct 21, 2024

robmry self-assigned this Oct 21, 2024

akerouanton approved these changes Oct 21, 2024

View reviewed changes

austinvazquez approved these changes Oct 21, 2024

View reviewed changes

robmry merged commit b6b3898 into moby:27.x Oct 21, 2024
141 checks passed

robmry deleted the 27.x-backport/48560-setup-user-chains branch October 21, 2024 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[27.x backport] Fix: setup user chains even if there are running containers #48714

[27.x backport] Fix: setup user chains even if there are running containers #48714

robmry commented Oct 21, 2024

	// Sets up the DOCKER-USER chain for each iptables version (IPv4, IPv6)
	// that"s enabled in the controller"s configuration.
	for _, ipVersion := range c.enabledIptablesVersions() {
	if err := setupUserChain(ipVersion); err != nil {
	log.G(context.TODO()).WithError(err).Warnf("Controller.NewNetwork %s:", name)
	}
	}

	if len(activeSandboxes) > 0 {
	log.G(context.TODO()).Info("there are running containers, updated network configuration will not take affect")
	} else if err := configureNetworking(daemon.netController, cfg); err != nil {
	return err
	}

[27.x backport] Fix: setup user chains even if there are running containers #48714

[27.x backport] Fix: setup user chains even if there are running containers #48714

Conversation

robmry commented Oct 21, 2024