Add docker interfaces to firewalld docker zone#2548
Conversation
|
PTAL @yrro @cpuguy83 @thaJeztah |
7378ec4 to
f4b58e7
Compare
|
IMO, Docker should follow libvirt's example and create its own I think you're modifying firewalld's runtime configuration only, so interfaces will be moved back to the default zone when firewalld is reloaded or restarted. libvirt handles this by listening to firewalld's Reloaded signal, and the normal D-Bus |
|
It occurs to me that a user might want different interfaces to be created in different zones. A |
f4b58e7 to
cd39849
Compare
|
@yrro thanks for taking a look !
|
ed91f60 to
d0d4036
Compare
|
updated the PR to add docker interfaces to a new docker zone |
erig0
left a comment
There was a problem hiding this comment.
First, thanks for adding firewalld integration. This will please users. In general this looks functional. I have various cleanup type comments. But those are non-blocking.
I realize that masquerading and port forwarding is still done via iptables. In the future I think firewalld could be used for those as well. Upstream is currently working on support for forward/output filtering.
|
|
||
| settings := getDockerZoneSettings() | ||
| // Permanent | ||
| if err := connection.sysConfObj.Call(dbusInterface+".config.addZone", 0, dockerZone, settings).Err; err != nil { |
There was a problem hiding this comment.
Just a note that the addZone() method has been deprecated. The next firewalld feature release introduces addZone2() which is dictionary based and doesn't require all fields be filled. That being said, docker has to support older versions of firewalld so this is the correct one to call. In the future support for addZone2() should be considered.
There was a problem hiding this comment.
FWIW, libvirt ships a static zone definition as part of the package. Therefore they avoid the addZone() and reload(). They also allow other things; dhcp, dns, icmp, ipv6-icmp. But they're also filtering (denying) other packets destined to the host.
| // Check if interface is already added to the zone | ||
| if err := connection.sysObj.Call(dbusInterface+".zone.getInterfaces", 0, dockerZone).Store(&intfs); err != nil { | ||
| return err | ||
| } | ||
| // Return if interface is already part of the zone | ||
| if contains(intfs, intf) { | ||
| logrus.Infof("Firewalld: interface %s already part of %s zone, returning", intf, dockerZone) | ||
| return nil | ||
| } | ||
|
|
||
| logrus.Debugf("Firewalld: adding %s interface to %s zone", intf, dockerZone) | ||
| // Runtime | ||
| if err := connection.sysObj.Call(dbusInterface+".zone.addInterface", 0, dockerZone, intf).Err; err != nil { | ||
| return err | ||
| } | ||
| return nil |
There was a problem hiding this comment.
You're probably aware, but this is racey. It's possible the interface gets deleted/added by another entity/user between the getInterfaces() and addInterface() calls. It may be simpler to always call addInterface() and handle the error codes appropriately. e.g. ALREADY_ENABLED.
There was a problem hiding this comment.
I'd like to keep it this way, since AFAIK I would need to compare the error string to handle the above case, which might change in the future
| if err := connection.sysObj.Call(dbusInterface+".zone.getInterfaces", 0, dockerZone).Store(&intfs); err != nil { | ||
| return err | ||
| } | ||
| // Remove interface if it exists | ||
| if !contains(intfs, intf) { | ||
| return fmt.Errorf("Firewalld: unable to find interface %s in %s zone", intf, dockerZone) | ||
| } | ||
|
|
||
| logrus.Debugf("Firewalld: removing %s interface from %s zone", intf, dockerZone) | ||
| // Runtime | ||
| if err := connection.sysObj.Call(dbusInterface+".zone.removeInterface", 0, dockerZone, intf).Err; err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Similar comment as above. Maybe always call removeInterface() and handle the errors.
There was a problem hiding this comment.
same comment as above
57b7633 to
902a280
Compare
If firewalld is running, create a new docker zone and add the docker interfaces to the docker zone to allow container networking for distros with firewalld enabled Fixes: moby#2496 Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com>
902a280 to
7a72092
Compare
|
What happens here if someone already has a pre-configured |
|
If a user manually created a docker zone, libnetwork would just reuse that and insert interfaces into that zone. However exact behavior of networking depends on the zone settings (which libnetwork will not try and override) |
|
So I believe that should happen if the user sets Edit - tested this with |
|
I think the idea was to not have to set iptables=false but allow users to enable the specific firewalld enhancement (or rather disable it as needed), for example cases where they are already managing this. I just know that firewall changes Docker does always ends up being problematic. |
|
@cpuguy83 even w/o this change, we have been pushing iptables rules via firewalld |
|
We've been pushing rules such that firewalld ensures they exist in iptables, but this is changing how firewalld is configured entirely. |
|
@cpuguy83 do you prefer having a |
full diff: moby/libnetwork@2e24aed...9e99af2 - moby/libnetwork#2548 Add docker interfaces to firewalld docker zone - fixes docker/for-linux#957 DNS Not Resolving under Network [CentOS8] - fixes moby/libnetwork#2496 Port Forwarding does not work on RHEL 8 with Firewalld running with FirewallBackend=nftables - store.getNetworksFromStore() remove unused error return - moby/libnetwork#2554 Fix 'failed to get network during CreateEndpoint' - fixes/addresses docker/for-linux#888 failed to get network during CreateEndpoint - moby/libnetwork#2558 [master] bridge: disable IPv6 router advertisements - moby/libnetwork#2563 log error instead if disabling IPv6 router advertisement failed - fixes docker/for-linux#1033 Shouldn't be fatal: Unable to disable IPv6 router advertisement: open /proc/sys/net/ipv6/conf/docker0/accept_ra: read-only file system Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: moby/libnetwork@2e24aed...9e99af2 - moby/libnetwork#2548 Add docker interfaces to firewalld docker zone - fixes docker/for-linux#957 DNS Not Resolving under Network [CentOS8] - fixes moby/libnetwork#2496 Port Forwarding does not work on RHEL 8 with Firewalld running with FirewallBackend=nftables - store.getNetworksFromStore() remove unused error return - moby/libnetwork#2554 Fix 'failed to get network during CreateEndpoint' - fixes/addresses docker/for-linux#888 failed to get network during CreateEndpoint - moby/libnetwork#2558 [master] bridge: disable IPv6 router advertisements - moby/libnetwork#2563 log error instead if disabling IPv6 router advertisement failed - fixes docker/for-linux#1033 Shouldn't be fatal: Unable to disable IPv6 router advertisement: open /proc/sys/net/ipv6/conf/docker0/accept_ra: read-only file system Signed-off-by: Sebastiaan van Stijn <github@gone.nl> Upstream-commit: 219e7e7ddcf5f0314578d2a517fc0832f03622c1 Component: engine
https://build.opensuse.org/request/show/851816 by user mrostecki + dimstar_suse - Add a patch which makes Docker compatible with firewalld with nftables backend. Backport of moby/libnetwork#2548 (boo#1178801, SLE-16460) * boo1178801-0001-Add-docker-interfaces-to-firewalld-docker-zone.patch
|
Rocky 9 All docker containers listening on 0.0.0.0:PORT are accessible from the outside via servername:PORT |
This is expected. This what docker requests. I recently wrote a blog about this and how to do strict filtering of docker containers. |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

If firewalld is running, create a new docker zone and
add the docker interfaces to the docker zone to allow
container networking for distros with Firewalld enabled
Fixes: #2496
Addresses docker/for-linux#957
Debug output
Signed-off-by: Arko Dasgupta arko.dasgupta@docker.com