From 6e5de55fc6211dfba41087d1d05bbb415c1a51da Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Tue, 21 Jan 2025 14:53:16 -0500 Subject: [PATCH] Add handling for firewalld's StrictForwardPorts setting This mostly amounts to throwing a sensible error in cases where the setting is enabled and the user has requested ports be forwarded. There are no tests at present because firewalld 2.3.x is currently restricted to Rawhide, and is required for effective testing. A manpage has also been added with details on how to work with the StrictForwardPorts setting (as well as some other tidbits on firewalld + Podman integration). Signed-off-by: Matt Heon --- .gitignore | 1 + docs/Makefile | 7 ++- docs/netavark-firewalld.7.md | 90 ++++++++++++++++++++++++++++++++ rpm/netavark.spec | 1 + src/commands/firewalld_reload.rs | 9 +++- src/firewall/firewalld.rs | 75 +++++++++++++++++++++++--- src/firewall/fwnone.rs | 12 ++++- src/firewall/iptables.rs | 16 ++++-- src/firewall/mod.rs | 12 ++++- src/firewall/nft.rs | 11 +++- src/network/bridge.rs | 9 +++- 11 files changed, 221 insertions(+), 22 deletions(-) create mode 100644 docs/netavark-firewalld.7.md diff --git a/.gitignore b/.gitignore index 6db072ba2..ca433255c 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ target/ targets/ *.swp netavark.1 +netavark-firewalld.7 vendor/ .idea/* contrib/systemd/*/*.service diff --git a/docs/Makefile b/docs/Makefile index 7587b40dd..72449fb2a 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -4,11 +4,14 @@ MANDIR ?= $(DATADIR)/man GO ?= go GOMD2MAN ?= go-md2man -docs: $(patsubst %.md,%,$(wildcard *.1.md)) +docs: $(patsubst %.md,%,$(wildcard *.[0-9].md)) %.1: %.1.md $(GOMD2MAN) -in $^ -out $@ +%.7: %.7.md + $(GOMD2MAN) -in $^ -out $@ + .PHONY: .install.md2man .install.md2man: $(GO) install github.com/cpuguy83/go-md2man/v2@latest @@ -17,6 +20,8 @@ docs: $(patsubst %.md,%,$(wildcard *.1.md)) install: install -d ${DESTDIR}/${MANDIR}/man1 install -m 0644 *.1 ${DESTDIR}/${MANDIR}/man1 + install -d ${DESTDIR}/${MANDIR}/man7 + install -m 0644 *.7 ${DESTDIR}/${MANDIR}/man7 .PHONY: clean clean: diff --git a/docs/netavark-firewalld.7.md b/docs/netavark-firewalld.7.md new file mode 100644 index 000000000..e3688bd10 --- /dev/null +++ b/docs/netavark-firewalld.7.md @@ -0,0 +1,90 @@ +% NETAVARK-FIREWALLD 7 Netavark Firewalld Interactions Man Page +% Matthew Heon +% January 2025 + +## Name + +netavark-firewalld - description of the interaction of Netavark and firewalld + +## Description + +Netavark can be used on systems with firewalld enabled without issue. +When using the default `nftables` or `iptables` firewall drivers, on systems where firewalld is running, firewalld will automatically be configured to allow connectivity to Podman containers. +All subnets of Podman-managed networks will be automatically added to the `trusted` zone to allow this access. + +### Firewalld Driver + +There is also a dedicated firewalld driver in Netavark. +This driver uses the firewalld DBus API to natively interact with firewalld. +It can be enabled by setting `firewall_driver` to `firewalld` in `containers.conf`. +The native firewalld driver offers better integration with firewalld, but presently suffers from several limitations. +It does not support isolation (the `--opt isolate=` option to `podman network create`. +Connections to ports forwarded by a container on the same host can only be made through the IPv4 localhost IP (`127.0.0.1`). +Using other IPs on the host will not work, unless the connection comes from a separate host. + +### Strict Port Forwarding + +Since firewalld version 2.3.0, a new setting, `StrictForwardPorts`, has been added. +The setting is located in `/etc/firewalld/firewalld.conf` and defaults to `no` (disabled). +When disabled, port forwarding with Podman works as normal. +When it is enabled (set to `yes`), port forwarding with root Podman will become nonfunctional. +Attempting to start a container or pod with the `-p` or `-P` options will return errors. +When StrictForwardPorts is enabled, all port forwarding must be done through firewalld using the firewall-cmd tool. +This ensures that containers cannot allow traffic through the firewall without administrator intervention. +Please note that rootless Podman is unaffected by this setting, and will function as it always has. + +Instead, containers should be started without forwarded ports specified, and preferably with static IPs. + +To forward a port externally, the following command should be run, substituting the desired host and container port numbers, protocol, and the container's IP. +``` +# firewall-cmd --permanent --zone {ZONE} --add-forward-port=port={HOST_PORT_NUMBER}:proto={PROTOCOL}:toport={CONTAINER_PORT_NUMBER}:toaddr={CONTAINER_IP} +``` + +If you are not sure which zone to use, the `public` zone should always work. + +If the container does not have a static IP, it can be found with `podman inspect`: +``` +# podman inspect -f '{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}' {CONTAINER_NAME_OR_ID} +``` + +Once the container is stopped or removed, the rule must be manually removed with the following command: +``` +# firewall-cmd --permanent --zone {ZONE} --remove-forward-port=port={HOST_PORT_NUMBER}:proto={PROTOCOL}:toport={CONTAINER_PORT_NUMBER}:toaddr={CONTAINER_IP} +``` + +To also allow forwarding via IPv4 localhost (`127.0.0.1`), a firewalld policy must be added, as well as a rich rule for each port requiring localhost forwarding. +Forwarding via IPv6 localhost is not possible due to kernel limitations. + +To add the policies required for IPv4 localhost forwarding, the following commands must be run. +This is only necessary once per system. +``` +# firewall-cmd --permanent --new-policy localhostForward +# firewall-cmd --permanent --policy localhostForward --add-ingress-zone HOST +# firewall-cmd --permanent --policy localhostForward --add-egress-zone ANY +``` + +A further rich rule for each container is required: +``` +# firewall-cmd --permanent --policy localhostForward --add-rich-rule='rule family=ipv4 destination address=127.0.0.0/8 forward-port port={HOST_PORT_NUMBER} protocol={PROTOCOL} to-port={CONTAINER_PORT_NUMBER} to-addr={CONTAINER_IP}' +``` + +These rules must be manually removed when the container is stopped or removed with the following command: +``` +# firewall-cmd --permanent --policy localhostForward --remove-rich-rule='rule family=ipv4 destination address=127.0.0.0/8 forward-port port={HOST_PORT_NUMBER} protocol={PROTOCOL} to-port={CONTAINER_PORT_NUMBER} to-addr={CONTAINER_IP}' +``` + +The associated `localhostForward` policy does not need to be removed. + +Please also note that, at present, it is only possible to access forwarded ports of a container on the same host via the IPv4 localhost IP (`127.0.0.1`), and only when the rich rule above has been applied. +Accessing via an IP that is not `127.0.0.1` from the same host is presently not possible, but we hope to address this with a future firewalld release. + +Please note that the firewalld driver presently bypasses this protection, and will still allow traffic through the firewall when `StrictForwardPorts` is enabled without manual forwarding through `firewall-cmd`. +This may be changed in a future release. + +## SEE ALSO + +firewalld(1), firewall-cmd(1), firewalld.conf(5), podman(1), containers.conf(5) + +## Authors + +Matthew Heon diff --git a/rpm/netavark.spec b/rpm/netavark.spec index 0e8b3f1b5..0ebc991b3 100644 --- a/rpm/netavark.spec +++ b/rpm/netavark.spec @@ -133,6 +133,7 @@ cd docs %dir %{_libexecdir}/podman %{_libexecdir}/podman/%{name}* %{_mandir}/man1/%{name}.1* +%{_mandir}/man7/%{name}-firewalld.7* %{_unitdir}/%{name}-dhcp-proxy.service %{_unitdir}/%{name}-dhcp-proxy.socket %{_unitdir}/%{name}-firewalld-reload.service diff --git a/src/commands/firewalld_reload.rs b/src/commands/firewalld_reload.rs index 5f3e00c43..f17b4d567 100644 --- a/src/commands/firewalld_reload.rs +++ b/src/commands/firewalld_reload.rs @@ -60,11 +60,16 @@ fn reload_rules_inner(config_dir: &Path) -> NetavarkResult<()> { if let Some(conf) = conf { let fw_driver = get_supported_firewall_driver(Some(conf.driver))?; + let dbus_conn = match zbus::blocking::Connection::system() { + Ok(c) => Some(c), + Err(_) => None, + }; + for net in conf.net_confs { - fw_driver.setup_network(net)?; + fw_driver.setup_network(net, &dbus_conn)?; } for port in &conf.port_confs { - fw_driver.setup_port_forward(port.into())?; + fw_driver.setup_port_forward(port.into(), &dbus_conn)?; } log::info!("Successfully reloaded firewall rules"); } diff --git a/src/firewall/firewalld.rs b/src/firewall/firewalld.rs index e4274d2e2..3954798c9 100644 --- a/src/firewall/firewalld.rs +++ b/src/firewall/firewalld.rs @@ -30,7 +30,11 @@ impl firewall::FirewallDriver for FirewallD { firewall::FIREWALLD } - fn setup_network(&self, network_setup: internal_types::SetupNetwork) -> NetavarkResult<()> { + fn setup_network( + &self, + network_setup: internal_types::SetupNetwork, + _dbus_con: &Option, + ) -> NetavarkResult<()> { let mut need_reload = false; need_reload |= match create_zone_if_not_exist(&self.conn, ZONENAME) { @@ -112,7 +116,11 @@ impl firewall::FirewallDriver for FirewallD { Ok(()) } - fn setup_port_forward(&self, setup_portfw: PortForwardConfig) -> Result<(), NetavarkError> { + fn setup_port_forward( + &self, + setup_portfw: PortForwardConfig, + _dbus_con: &Option, + ) -> Result<(), NetavarkError> { // NOTE: There is a serious TOCTOU risk in this function if netavark // is either run in parallel, or is not the only thing to edit this // policy. @@ -658,17 +666,17 @@ pub fn is_firewalld_running(conn: &Connection) -> bool { /// Ignore all errors, beyond possibly logging them. /// Not used within the firewalld driver, but by other drivers that may need to /// interact with firewalld. -pub fn add_firewalld_if_possible(net: &ipnet::IpNet) { - let conn = match Connection::system() { - Ok(conn) => conn, - Err(_) => return, +pub fn add_firewalld_if_possible(dbus_conn: &Option, net: &ipnet::IpNet) { + let conn = match dbus_conn { + Some(conn) => conn, + None => return, }; - if !is_firewalld_running(&conn) { + if !is_firewalld_running(conn) { return; } debug!("Adding firewalld rules for network {}", net.to_string()); - match add_source_subnets_to_zone(&conn, "trusted", &[*net]) { + match add_source_subnets_to_zone(conn, "trusted", &[*net]) { Ok(_) => {} Err(e) => warn!( "Error adding subnet {} from firewalld trusted zone: {}", @@ -706,3 +714,54 @@ pub fn rm_firewalld_if_possible(net: &ipnet::IpNet) { ), }; } + +/// Check whether firewalld's StrictForwardPorts setting is enabled. +/// Returns false if firewalld is not installed, not running, or there is any +/// error with the process. +pub fn is_firewalld_strict_forward_enabled(dbus_con: &Option) -> bool { + let conn = match dbus_con { + Some(conn) => conn, + None => return false, + }; + if !is_firewalld_running(conn) { + return false; + } + + // Fetch current running config + match conn.call_method( + Some("org.fedoraproject.FirewallD1"), + "/org/fedoraproject/FirewallD1/config", + Some("org.freedesktop.DBus.Properties"), + "Get", + &("org.fedoraproject.FirewallD1.config", "StrictForwardPorts"), + ) { + Ok(b) => b.body().deserialize().unwrap_or(false), + Err(_) => { + // Assume any error is related to the property not existing + // (As it will not on older firewalld versions) + // Return false given that. + false + } + } +} + +/// Check if firewalld's StrictForwardPorts setting is enabled and, if so, +/// whether the container has requested any ports be forwarded. If both are true +/// return a helpful error that port forwarding cannot be performed. +pub fn check_can_forward_ports( + dbus_conn: &Option, + setup_portfw: &PortForwardConfig, +) -> NetavarkResult<()> { + if is_firewalld_strict_forward_enabled(dbus_conn) { + let mut portfw_used = setup_portfw.dns_port != 53; + if let Some(ports) = setup_portfw.port_mappings { + portfw_used = portfw_used || !ports.is_empty(); + } + if portfw_used { + return Err(NetavarkError::msg( + "Port forwarding not possible as firewalld StrictForwardPorts enabled", + )); + } + } + Ok(()) +} diff --git a/src/firewall/fwnone.rs b/src/firewall/fwnone.rs index deab786f6..8c603e280 100644 --- a/src/firewall/fwnone.rs +++ b/src/firewall/fwnone.rs @@ -16,7 +16,11 @@ impl firewall::FirewallDriver for Fwnone { firewall::NONE } - fn setup_network(&self, _network_setup: SetupNetwork) -> NetavarkResult<()> { + fn setup_network( + &self, + _network_setup: SetupNetwork, + _dbus_conn: &Option, + ) -> NetavarkResult<()> { Ok(()) } @@ -26,7 +30,11 @@ impl firewall::FirewallDriver for Fwnone { Ok(()) } - fn setup_port_forward(&self, _setup_portfw: PortForwardConfig) -> NetavarkResult<()> { + fn setup_port_forward( + &self, + _setup_portfw: PortForwardConfig, + _dbus_conn: &Option, + ) -> NetavarkResult<()> { Ok(()) } diff --git a/src/firewall/iptables.rs b/src/firewall/iptables.rs index 8baaf0112..9cc018341 100644 --- a/src/firewall/iptables.rs +++ b/src/firewall/iptables.rs @@ -41,7 +41,11 @@ impl firewall::FirewallDriver for IptablesDriver { firewall::IPTABLES } - fn setup_network(&self, network_setup: SetupNetwork) -> NetavarkResult<()> { + fn setup_network( + &self, + network_setup: SetupNetwork, + dbus_con: &Option, + ) -> NetavarkResult<()> { if let Some(subnet) = network_setup.subnets { for network in subnet { let is_ipv6 = network.network().is_ipv6(); @@ -62,7 +66,7 @@ impl firewall::FirewallDriver for IptablesDriver { create_network_chains(chains)?; - firewalld::add_firewalld_if_possible(&network); + firewalld::add_firewalld_if_possible(dbus_con, &network); } } Ok(()) @@ -111,7 +115,13 @@ impl firewall::FirewallDriver for IptablesDriver { Result::Ok(()) } - fn setup_port_forward(&self, setup_portfw: PortForwardConfig) -> NetavarkResult<()> { + fn setup_port_forward( + &self, + setup_portfw: PortForwardConfig, + dbus_con: &Option, + ) -> NetavarkResult<()> { + firewalld::check_can_forward_ports(dbus_con, &setup_portfw)?; + if let Some(v4) = setup_portfw.container_ip_v4 { let subnet_v4 = match setup_portfw.subnet_v4 { Some(s) => s, diff --git a/src/firewall/mod.rs b/src/firewall/mod.rs index 9260b2086..0f185eedd 100644 --- a/src/firewall/mod.rs +++ b/src/firewall/mod.rs @@ -21,12 +21,20 @@ const NONE: &str = "none"; /// and port mappings. pub trait FirewallDriver { /// Set up firewall rules for the given network, - fn setup_network(&self, network_setup: SetupNetwork) -> NetavarkResult<()>; + fn setup_network( + &self, + network_setup: SetupNetwork, + dbus_con: &Option, + ) -> NetavarkResult<()>; /// Tear down firewall rules for the given network. fn teardown_network(&self, tear: TearDownNetwork) -> NetavarkResult<()>; /// Set up port-forwarding firewall rules for a given container. - fn setup_port_forward(&self, setup_pw: PortForwardConfig) -> NetavarkResult<()>; + fn setup_port_forward( + &self, + setup_pw: PortForwardConfig, + dbus_con: &Option, + ) -> NetavarkResult<()>; /// Tear down port-forwarding firewall rules for a single container. fn teardown_port_forward(&self, teardown_pf: TeardownPortForward) -> NetavarkResult<()>; diff --git a/src/firewall/nft.rs b/src/firewall/nft.rs index ef9722a51..2f8bf70a4 100644 --- a/src/firewall/nft.rs +++ b/src/firewall/nft.rs @@ -50,7 +50,11 @@ impl firewall::FirewallDriver for Nftables { firewall::NFTABLES } - fn setup_network(&self, network_setup: internal_types::SetupNetwork) -> NetavarkResult<()> { + fn setup_network( + &self, + network_setup: internal_types::SetupNetwork, + dbus_conn: &Option, + ) -> NetavarkResult<()> { let mut batch = Batch::new(); // Overall table @@ -342,7 +346,7 @@ impl firewall::FirewallDriver for Nftables { // Add us to firewalld if necessary. // Do this first, as firewalld doesn't wipe our rules - so after a reload, we skip everything below. - firewalld::add_firewalld_if_possible(&subnet); + firewalld::add_firewalld_if_possible(dbus_conn, &subnet); // Do we already have a chain for the subnet? if get_chain(&existing_rules, &chain).is_some() { @@ -557,7 +561,10 @@ impl firewall::FirewallDriver for Nftables { fn setup_port_forward( &self, setup_portfw: internal_types::PortForwardConfig, + dbus_conn: &Option, ) -> NetavarkResult<()> { + firewalld::check_can_forward_ports(dbus_conn, &setup_portfw)?; + let mut batch = Batch::new(); let existing_rules = get_netavark_rules()?; diff --git a/src/network/bridge.rs b/src/network/bridge.rs index d74c16f73..533ac00d2 100644 --- a/src/network/bridge.rs +++ b/src/network/bridge.rs @@ -417,7 +417,12 @@ impl<'a> Bridge<'a> { )?; } - self.info.firewall.setup_network(sn)?; + let system_dbus = match zbus::blocking::Connection::system() { + Ok(c) => Some(c), + Err(_) => None, + }; + + self.info.firewall.setup_network(sn, &system_dbus)?; if spf.port_mappings.is_some() { // Need to enable sysctl localnet so that traffic can pass @@ -432,7 +437,7 @@ impl<'a> Bridge<'a> { )?; } - self.info.firewall.setup_port_forward(spf)?; + self.info.firewall.setup_port_forward(spf, &system_dbus)?; Ok(()) }