diff --git a/src/commands/setup.rs b/src/commands/setup.rs index 9d025c32..9f2e3344 100644 --- a/src/commands/setup.rs +++ b/src/commands/setup.rs @@ -3,8 +3,8 @@ use crate::commands::get_config_dir; use crate::dns::aardvark::Aardvark; use crate::error::{NetavarkError, NetavarkResult}; use crate::firewall; -use crate::network::driver::{get_network_driver, DriverInfo}; -use crate::network::netlink::LinkID; +use crate::network::driver::{get_network_driver, DriverInfo, NetworkDriver}; +use crate::network::netlink::{self, LinkID}; use crate::network::{self}; use crate::network::{core_utils, types}; @@ -109,17 +109,11 @@ impl Setup { Ok((s, a)) => (s, a), Err(e) => { // now teardown the already setup drivers - for dri in drivers.iter().take(i) { - match dri.teardown((&mut hostns.netlink, &mut netns.netlink)) { - Ok(_) => {} - Err(e) => { - error!( - "failed to cleanup previous networks after setup failed: {}", - e - ) - } - }; - } + teardown_drivers( + drivers.iter().take(i), + &mut hostns.netlink, + &mut netns.netlink, + ); return Err(e); } }; @@ -139,22 +133,19 @@ impl Setup { // ignore error when path already exists Err(ref e) if e.kind() == std::io::ErrorKind::AlreadyExists => {} Err(e) => { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("failed to create aardvark-dns directory: {e}"), - ) - .into()); + teardown_drivers(drivers.iter(), &mut hostns.netlink, &mut netns.netlink); + return Err(NetavarkError::wrap( + format!("failed to create aardvark-dns directory {}", path.display()), + NetavarkError::Io(e), + )); } } let aardvark_interface = Aardvark::new(path, rootless, aardvark_bin, dns_port); if let Err(er) = aardvark_interface.commit_netavark_entries(aardvark_entries) { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("Error while applying dns entries: {er}"), - ) - .into()); + teardown_drivers(drivers.iter(), &mut hostns.netlink, &mut netns.netlink); + return Err(NetavarkError::wrap("error while applying dns entries", er)); } } else { info!( @@ -170,3 +161,18 @@ impl Setup { Ok(()) } } + +fn teardown_drivers<'a, I>(drivers: I, host: &mut netlink::Socket, netns: &mut netlink::Socket) +where + I: Iterator>, +{ + for driver in drivers { + if let Err(e) = driver.teardown((host, netns)) { + error!( + "failed to cleanup network {} after setup failed: {}", + driver.network_name(), + e + ); + }; + } +} diff --git a/test/100-bridge-iptables.bats b/test/100-bridge-iptables.bats index 7f8f0bc2..2ac2dade 100644 --- a/test/100-bridge-iptables.bats +++ b/test/100-bridge-iptables.bats @@ -1088,3 +1088,12 @@ function check_simple_bridge_iptables() { assert "${lines[3]}" == "-A NETAVARK_FORWARD -s 10.88.0.0/16 -j ACCEPT" "NETAVARK_FORWARD rule 3" assert "${#lines[@]}" = 4 "too many NETAVARK_FORWARD rules" } + +@test "$fw_driver - aardvark-dns error cleanup" { + expected_rc=1 run_netavark -a /usr/bin/false --file ${TESTSDIR}/testfiles/dualstack-bridge-custom-dns-server.json setup $(get_container_netns_path) + assert_json ".error" "error while applying dns entries: IO error: aardvark-dns exited unexpectedly without error message" "aardvark-dns error" + run_in_host_netns iptables -S + assert "$output" !~ "10.89.3.0/24" "leaked network iptables rules after setup error" + run_in_host_netns iptables -S -t nat + assert "$output" !~ "10.89.3.0/24" "leaked network iptables NAT rules after setup error" +} diff --git a/test/250-bridge-nftables.bats b/test/250-bridge-nftables.bats index 8c30f17d..06496ce3 100644 --- a/test/250-bridge-nftables.bats +++ b/test/250-bridge-nftables.bats @@ -1026,5 +1026,13 @@ function check_simple_bridge_nftables() { expected_rc=1 run_in_host_netns nft list chain inet netavark $chain run_in_host_netns nft list chain inet netavark NETAVARK-HOSTPORT-DNAT assert "$output" == $'table inet netavark {\n\tchain NETAVARK-HOSTPORT-DNAT {\n\t}\n}' "NETAVARK-HOSTPORT-DNAT chain must be empty" +} + +@test "$fw_driver - aardvark-dns error cleanup" { + expected_rc=1 run_netavark -a /usr/bin/false --file ${TESTSDIR}/testfiles/dualstack-bridge-custom-dns-server.json setup $(get_container_netns_path) + assert_json ".error" "error while applying dns entries: IO error: aardvark-dns exited unexpectedly without error message" "aardvark-dns error" + run_in_host_netns nft list table inet netavark + assert "$output" !~ "10.89.3.0/24" "leaked network nft rules after setup error" + assert "$output" !~ "fd10:88:a::/64" "leaked network nft rules after setup error" }