Only enable bridge netfiltering when needed by robmry · Pull Request #48492 · moby/moby

robmry · 2024-09-13T12:02:34Z

- What I did

fixes Docker bridge networks not automatically added to appropriate firewalld zone #48375
fixes Filtered packets between containers in bridged network after upgrade #48411

- How I did it

Kernel module br_netfilter is loaded when the daemon starts with either iptables or ip6tables enabled. That automatically sets:

net.bridge.bridge-nf-call-arptables = 1
net.bridge.bridge-nf-call-ip6tables = 1
net.bridge.bridge-nf-call-iptables = 1

So, when:

docker was running happily with iptables=false, and
no explicit ip6tables=false, and
br_netfilter was not loaded

... the change in moby 27.0 to enable ip6tables by default, resulted in net.bridge.bridge-nf-call-iptables being enabled, where it wasn't before.

If the host also had a firewall with default-drop on its forward chain - that resulted in packets getting dropped between containers on a bridge network.

So, only try to load br_netfilter when it's needed - it's only needed to implement --icc=false, which only works when iptables / ip6tables are enabled.

- How to verify it

On a freshly booted system (or at-least without br_netfilter loaded).
With iptables/nftables rules set up with a default DROP for forwarding.
Start dockerd with "iptables":false, and "ip6tables" left as default.
Create an IPv4-only network, docker network create br4.
With 26.x:
- Check that containers on network br4 can communicate. For example:
- docker run --rm -d --name c1 --network br4 nginx; docker run --rm -ti --network br4 alpine wget -O- http://c1
Upgrade to 27.x to repro the bug or a build with this fix, re-run the nginx/wget commands.
- Note that the fix will not work once br_netfilter has been loaded, rmmod br_netfilter or reboot to get rid of it.
With this fix:
- Check that br_netfilter hasn't been loaded.
  - For example sysctl -a | grep net.bridge.bridge-nf-call.
- Create a no-icc network...
  - For example docker network create --ipv6 -o com.docker.network.bridge.enable_icc=false iccf.
  - Check that br_netfilter has been loaded.

- Description for the changelog

Fix an issue that prevented communication between containers on an IPv4 bridge network
when running with `--iptables=false`, `--ip6tables=true` (the default), a firewall with a
DROP rule for forwarded packets on hosts where the `br_netfilter` kernel module was not
normally loaded.

thaJeztah · 2024-09-16T12:32:42Z

+		if out, err := exec.Command("modprobe", "-va", "bridge", "br_netfilter").CombinedOutput(); err != nil {
+			log.G(context.TODO()).WithError(err).Warnf("Running modprobe bridge br_netfilter failed with message: %s", out)


Wondering if we should start looking at plumbing through context to these networking options (both for OTEL, as well as potentially context-cancellation (e.g. this could use exec.CommandContext).

Definitely not for this PR, but curious if it would make sense to start looking at.

Yes, it would make sense.

thaJeztah · 2024-09-16T12:34:03Z

+	if _, err := os.Stat("/proc/sys/net/bridge"); err != nil {
+		if out, err := exec.Command("modprobe", "-va", "bridge", "br_netfilter").CombinedOutput(); err != nil {
+			log.G(context.TODO()).WithError(err).Warnf("Running modprobe bridge br_netfilter failed with message: %s", out)
+			return fmt.Errorf("modprobe br_netfilter failed: %w", err)


This now both returns and logs the error, whereas previously we would only log it; is there a risk that we're changing behavior?

(Also wondering if the logging should happen on the caller side now that we return these)?

As it's an error now; perhaps the Warnf should become an Errorf now as well 🤔 (we tend to use "warning" for errors that we can recover from, but as we now no longer recover, it's an Error I guess?

This now both returns and logs the error, whereas previously we would only log it; is there a risk that we're changing behavior?

If the modprobe failed on startup before, network creation with icc=false would have failed here, with the "cannot restrict inter-container communication" error (just below). I'll add "cannot restrict inter-container communication" to the modprobe error.

So, I don't think there's any change in behaviour, apart from the text in the error message.

(Also wondering if the logging should happen on the caller side now that we return these)?

I think that'd mean duplicating the log lines, would it be an improvement? We could just get rid of the log line, but it might still provide a useful clue for something.

As it's an error now; perhaps the Warnf should become an Errorf now as well 🤔 (we tend to use "warning" for errors that we can recover from, but as we now no longer recover, it's an Error I guess?

I'll change it to an Errorf. (If the modprobe fails, it now means an network create "icc=false" didn't work - before it meant a future network create with "icc=false" would fail if attempted. Recovery is the same as before, just reject the network creation. The rest of the daemon wouldn't be affected in either case.)

Kernel module br_netfilter is loaded when the daemon starts with either iptables or ip6tables enabled. That automatically sets: net.bridge.bridge-nf-call-arptables = 1 net.bridge.bridge-nf-call-ip6tables = 1 net.bridge.bridge-nf-call-iptables = 1 So, when: - docker was running happily with iptables=false, and - no explicit ip6tables=false, and - br_netfilter was not loaded ... the change in moby 27.0 to enable ip6tables by default, resulted in net.bridge.bridge-nf-call-iptables being enabled. If the host also had a firewall with default-drop on its forward chain - that resulted in packets getting dropped between containers on a bridge network. So, only try to load br_netfilter when it's needed - it's only needed to implement "--icc=false", which can only be used when iptables or ip6tables is enabled. Signed-off-by: Rob Murray <[email protected]>

thaJeztah

LGTM, thanks!

Docker versions prior to 27.3.1 automatically loaded the br_netfilter kernel module at daemon startup. When loaded, br_netfilter causes bridge traffic to pass through the host's iptables FORWARD chain, where DOCKER-ISOLATION-STAGE-1 drops packets from --internal networks destined for external IPs. This silently breaks membrane's transparent proxy; the agent's TCP connections time out with no useful error. Insert a narrow iptables rule into DOCKER-USER (which runs before DOCKER-ISOLATION-STAGE-1) that accepts forwarded traffic from the membrane internal bridge. The rule is scoped to the per-session bridge interface and is removed on session teardown. If cleanup fails, the orphaned rule references a nonexistent interface and is harmless; Docker rewrites its chains on restart anyway. This fix makes membrane work correctly regardless of Docker version or whether br_netfilter is loaded on the host. Ref: moby/moby#48492 (Docker stopped auto-loading br_netfilter in 27.3.1)

Docker versions prior to 27.3.1 automatically loaded the br_netfilter kernel module at daemon startup. When loaded, br_netfilter causes bridge traffic to pass through the host's iptables FORWARD chain, where DOCKER-ISOLATION-STAGE-1 drops packets from --internal networks destined for external IPs. This silently breaks membrane's transparent proxy; the agent's TCP connections time out with no useful error. Insert a narrow iptables rule into DOCKER-USER (which runs before DOCKER-ISOLATION-STAGE-1) that accepts forwarded traffic from the membrane internal bridge. The rule is scoped to the per-session bridge interface and is removed on session teardown. If cleanup fails, the orphaned rule references a nonexistent interface and is harmless; Docker rewrites its chains on restart anyway. Only inject the rule when br_netfilter is actually loaded, checked via /proc/modules. On hosts where it is not loaded (Docker 27.3.1+ default), membrane works without touching iptables at all. Use sudo for the iptables commands since membrane does not run as root. This fix makes membrane work correctly regardless of Docker version or whether br_netfilter is loaded on the host. Ref: moby/moby#48492 (Docker stopped auto-loading br_netfilter in 27.3.1)

Docker versions prior to 27.3.1 automatically loaded the br_netfilter kernel module at daemon startup. When loaded, br_netfilter causes bridge traffic to pass through the host's iptables FORWARD chain, where DOCKER-ISOLATION-STAGE-1 drops packets from --internal networks destined for external IPs. This silently breaks membrane's transparent proxy; the agent's TCP connections time out with no useful error. Insert a narrow iptables rule into DOCKER-USER (which runs before DOCKER-ISOLATION-STAGE-1) that accepts forwarded traffic from the membrane internal bridge. The rule is scoped to the per-session bridge interface and is removed on session teardown. If cleanup fails, the orphaned rule references a nonexistent interface and is harmless; Docker rewrites its chains on restart anyway. Only inject the rule when br_netfilter is actually loaded, checked via /proc/modules. On hosts where it is not loaded (Docker 27.3.1+ default), membrane works without touching iptables at all. Use sudo for the iptables commands since membrane does not run as root. Tighten the DOCKER-USER rule to also restrict the outbound interface to the external bridge (-o br-<external>), so packets from the internal bridge can only be forwarded toward the handler and not to any other interface on the host. This fix makes membrane work correctly regardless of Docker version or whether br_netfilter is loaded on the host. Ref: moby/moby#48492 (Docker stopped auto-loading br_netfilter in 27.3.1)

Docker versions prior to 27.3.1 automatically loaded the br_netfilter kernel module at daemon startup. When loaded, br_netfilter causes bridge traffic to pass through the host's iptables FORWARD chain, where DOCKER-ISOLATION-STAGE-1 drops packets from --internal networks destined for external IPs. This silently breaks membrane's transparent proxy; the agent's TCP connections time out with no useful error. When br_netfilter is detected in /proc/modules, insert a rule into DOCKER-USER (-i br-<internal> -j ACCEPT) before session start. DOCKER-USER is evaluated before DOCKER-ISOLATION-STAGE-1, so the ACCEPT prevents the DROP from firing. The rule is scoped to the per-session internal bridge interface and is removed on session teardown. If cleanup fails, the orphaned rule references a nonexistent interface and is harmless; Docker rewrites its chains on restart anyway. The rule uses only -i (inbound interface) without an outbound constraint. The agent's packets are dnat'd by the handler's nftables PREROUTING rule before the forwarding decision, rewriting the destination back into the internal subnet, so adding -o would prevent the rule from ever matching. Ref: moby/moby#48492 (Docker stopped auto-loading br_netfilter in 27.3.1)

robmry added area/networking Networking impact/changelog kind/bugfix PR's that fix bugs area/networking/ipv6 Networking area/networking/firewalling Networking area/networking/d/bridge Networking process/cherry-pick/27.x labels Sep 13, 2024

robmry added this to the 28.0.0 milestone Sep 13, 2024

robmry self-assigned this Sep 13, 2024

robmry force-pushed the 48375_bridge_netfiltering branch from 09119a0 to 0677efa Compare September 13, 2024 13:40

robmry marked this pull request as ready for review September 13, 2024 14:30

robmry requested review from akerouanton and corhere September 13, 2024 14:30

akerouanton approved these changes Sep 16, 2024

View reviewed changes

thaJeztah reviewed Sep 16, 2024

View reviewed changes

robmry force-pushed the 48375_bridge_netfiltering branch from 0677efa to db25b0d Compare September 16, 2024 13:32

thaJeztah approved these changes Sep 16, 2024

View reviewed changes

robmry mentioned this pull request Sep 16, 2024

[27.x backport] Only enable bridge netfiltering when needed #48511

Merged

thaJeztah merged commit 83d8064 into moby:master Sep 16, 2024

thaJeztah added process/cherry-picked and removed process/cherry-pick/27.x labels Sep 16, 2024

robmry deleted the 48375_bridge_netfiltering branch September 16, 2024 14:55

robmry mentioned this pull request Nov 4, 2024

[epic] IPv6 improvements for moby 28.0 #48814

Closed

18 tasks

This was referenced Nov 27, 2024

br_netfilter module is not loaded by docker #48948

Closed

Dockerfile: make sure modprobe is available in dev-container #48992

Closed

[master] deb,rpm: add kmod as recommended dependency to provide modprobe docker/docker-ce-packaging#1118

Merged

thaJeztah mentioned this pull request Dec 4, 2024

[27.x backport] deb,rpm: add kmod as recommended dependency to provide modprobe docker/docker-ce-packaging#1121

Merged

thaJeztah mentioned this pull request Dec 13, 2024

daemon: info: remove bridge-nf-call-iptables / ip6tables warnings #49089

Merged

robmry mentioned this pull request Jan 17, 2025

Allow users to ignore missing br_netfilter #49293

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only enable bridge netfiltering when needed#48492

Only enable bridge netfiltering when needed#48492
thaJeztah merged 1 commit intomoby:masterfrom
robmry:48375_bridge_netfiltering

robmry commented Sep 13, 2024 •

edited by thaJeztah

Loading

Uh oh!

thaJeztah Sep 16, 2024

Uh oh!

robmry Sep 16, 2024

Uh oh!

thaJeztah Sep 16, 2024

Uh oh!

thaJeztah Sep 16, 2024

Uh oh!

robmry Sep 16, 2024

Uh oh!

thaJeztah left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if out, err := exec.Command("modprobe", "-va", "bridge", "br_netfilter").CombinedOutput(); err != nil {
		log.G(context.TODO()).WithError(err).Warnf("Running modprobe bridge br_netfilter failed with message: %s", out)

Conversation

robmry commented Sep 13, 2024 • edited by thaJeztah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

robmry Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

robmry Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robmry commented Sep 13, 2024 •

edited by thaJeztah

Loading