Merge pull request #1304 from shivkr6/bridge-unmanaged-dns-fix

bridge: unmanaged aardvark-dns fix

Author: openshift-merge-bot[bot]

examples/host-device-plugin.rs

@@ -50,7 +50,7 @@ impl Plugin for Exec {
             }
         }
 
-        let addresses = host.netlink.dump_addresses()?;
+        let addresses = host.netlink.dump_addresses(None)?;
         let mut subnets = Vec::new();
         for address in addresses {
             if address.header.index == link.header.index {

src/network/bridge.rs

@@ -1,12 +1,5 @@
 use std::{collections::HashMap, fs, net::IpAddr, os::fd::BorrowedFd};
 
-use ipnet::IpNet;
-use log::{debug, error};
-use netlink_packet_route::link::{
-    BridgeVlanInfoFlags, InfoBridge, InfoData, InfoKind, InfoVeth, LinkAttribute, LinkInfo,
-    LinkMessage,
-};
-
 use crate::dns::aardvark::SafeString;
 use crate::network::core_utils::get_default_route_interface;
 use crate::network::dhcp::{dhcp_teardown, get_dhcp_lease};
@@ -20,6 +13,14 @@ use crate::{
     },
     network::{constants, sysctl::disable_ipv6_autoconf, types},
 };
+use ipnet::IpNet;
+use log::{debug, error};
+use netlink_packet_route::address::{AddressAttribute, AddressScope};
+use netlink_packet_route::link::{
+    BridgeVlanInfoFlags, InfoBridge, InfoData, InfoKind, InfoVeth, LinkAttribute, LinkInfo,
+    LinkMessage,
+};
+use netlink_packet_route::AddressFamily;
 
 use super::{
     constants::{
@@ -79,6 +80,16 @@ pub struct Bridge<'a> {
     data: Option<InternalData>,
 }
 
+struct CreateInterfacesResult {
+    /// The MAC address of the container's veth interface.
+    mac_address: String,
+    /// An optional writer for creating a sysctl.d config file.
+    /// This is only created when a new bridge is created.
+    sysctl_writer: Option<sysctl::SysctlDWriter<'static, String, String>>,
+    /// The interface index of the bridge.
+    bridge_index: u32,
+}
+
 impl<'a> Bridge<'a> {
     pub fn new(info: DriverInfo<'a>) -> Self {
         Bridge { info, data: None }
@@ -166,7 +177,11 @@ impl driver::NetworkDriver for Bridge<'_> {
 
         let (host_sock, netns_sock) = netlink_sockets;
 
-        let (container_veth_mac, sysctl_writer) = create_interfaces(
+        let CreateInterfacesResult {
+            mac_address: container_veth_mac,
+            sysctl_writer,
+            bridge_index,
+        } = create_interfaces(
             host_sock,
             netns_sock,
             data,
@@ -252,18 +267,49 @@ impl driver::NetworkDriver for Bridge<'_> {
                 }
             }
 
-            let gw = data
-                .ipam
-                .gateway_addresses
-                .iter()
-                .map(|ipnet| ipnet.addr())
-                .collect();
+            // Fixes #1177: In unmanaged mode, the gateway IP may not be on the host.
+            // We need to find an IP on the bridge itself for aardvark-dns to bind to.
+            let bind_addr: Vec<IpAddr> = if data.mode == BridgeMode::Unmanaged {
+                let addr_msgs = host_sock.dump_addresses(Some(bridge_index))?;
+
+                let addresses: Vec<IpAddr> = addr_msgs
+                    .into_iter()
+                    .filter_map(|addr_msg| {
+                        // address is either a IPv4 address, or it's an IPv6 address that is not a link-local address.
+                        if (addr_msg.header.family == AddressFamily::Inet6
+                            && addr_msg.header.scope != AddressScope::Link)
+                            || addr_msg.header.family == AddressFamily::Inet
+                        {
+                            addr_msg.attributes.into_iter().find_map(|attr| {
+                                if let AddressAttribute::Address(ip) = attr {
+                                    Some(ip)
+                                } else {
+                                    None
+                                }
+                            })
+                        } else {
+                            None
+                        }
+                    })
+                    .collect();
+                if addresses.is_empty() {
+                    return Err(NetavarkError::msg(format!("bridge '{}' in unmanaged mode has no usable IP addresses. Aardvark-dns requires at least one address (should not be an IPv6 link-local address) to bind to. Please add an IP address or disable DNS for this network (--disable-dns).", data.bridge_interface_name)));
+                }
+                response.dns_server_ips = Some(addresses.clone());
+                addresses
+            } else {
+                data.ipam
+                    .gateway_addresses
+                    .iter()
+                    .map(|ipnet| ipnet.addr())
+                    .collect()
+            };
 
             match self.info.container_id.as_str().try_into() {
                 Ok(id) => Some(AardvarkEntry {
                     network_name: &self.info.network.name,
                     container_id: id,
-                    network_gateways: gw,
+                    network_gateways: bind_addr,
                     network_dns_servers: &self.info.network.network_dns_servers,
                     container_ips_v4: ipv4,
                     container_ips_v6: ipv6,
@@ -553,10 +599,7 @@ fn create_interfaces(
     rootless: bool,
     hostns_fd: BorrowedFd<'_>,
     netns_fd: BorrowedFd<'_>,
-) -> NetavarkResult<(
-    String,
-    Option<sysctl::SysctlDWriter<'static, String, String>>,
-)> {
+) -> NetavarkResult<CreateInterfacesResult> {
     let mut sysctl_writer = None;
     let (bridge_index, mtu, mac) = match host.get_link(netlink::LinkID::Name(
         data.bridge_interface_name.to_string(),
@@ -738,7 +781,11 @@ fn create_interfaces(
         netns_fd,
         mtu,
     )?;
-    Ok((mac, sysctl_writer))
+    Ok(CreateInterfacesResult {
+        mac_address: mac,
+        sysctl_writer,
+        bridge_index,
+    })
 }
 
 /// return the container veth mac address

src/network/netlink.rs

@@ -114,6 +114,8 @@ impl Socket {
     pub fn new() -> NetavarkResult<Socket> {
         let mut socket = wrap!(netlink_sys::Socket::new(NETLINK_ROUTE), "open")?;
         let addr = &SocketAddr::new(0, 0);
+        // Needs to be enabled for dump filtering to work
+        socket.set_netlink_get_strict_chk(true)?;
         wrap!(socket.bind(addr), "bind")?;
         wrap!(socket.connect(addr), "connect")?;
 
@@ -409,11 +411,19 @@ impl Socket {
         Ok(links)
     }
 
-    pub fn dump_addresses(&mut self) -> NetavarkResult<Vec<AddressMessage>> {
-        let msg = AddressMessage::default();
+    // If filtering options are supplied, then only the ip addresses satisfying the filter are returned. Otherwise all ip addresses of all interfaces are returned
+    pub fn dump_addresses(
+        &mut self,
+        interface_id_filter: Option<u32>,
+    ) -> NetavarkResult<Vec<AddressMessage>> {
+        let mut msg = AddressMessage::default();
 
-        let results = self
-            .make_netlink_request(RouteNetlinkMessage::GetAddress(msg), NLM_F_DUMP | NLM_F_ACK)?;
+        if let Some(id) = interface_id_filter {
+            msg.header.index = id;
+        }
+
+        let results =
+            self.make_netlink_request(RouteNetlinkMessage::GetAddress(msg), NLM_F_DUMP)?;
 
         let mut addresses = Vec::with_capacity(results.len());
 

src/test/netlink.rs

@@ -175,7 +175,7 @@ mod tests {
         eprintln!("{}", String::from_utf8(out.stderr).unwrap());
         assert!(out.status.success(), "failed to set up lo via ip");
 
-        let addresses = sock.dump_addresses().expect("dump_addresses failed");
+        let addresses = sock.dump_addresses(None).expect("dump_addresses failed");
         for nla in addresses[0].attributes.iter() {
             if let address::AddressAttribute::Address(ip) = nla {
                 assert_eq!(ip, &IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)))
@@ -187,4 +187,38 @@ mod tests {
             }
         }
     }
+    #[test]
+    fn test_dump_addr_filter() {
+        test_setup!();
+        let mut sock = Socket::new().expect("Socket::new()");
+
+        let out = run_command!("ip", "link", "add", "test1", "type", "dummy");
+        eprintln!("{}", String::from_utf8(out.stderr).unwrap());
+        assert!(out.status.success(), "failed to add link via ip");
+
+        let net = "10.0.0.2/24";
+
+        let out = run_command!("ip", "addr", "add", net, "dev", "test1");
+        eprintln!("{}", String::from_utf8(out.stderr).unwrap());
+        assert!(out.status.success(), "failed to add addr via ip");
+
+        let out = run_command!("ip", "link", "set", "up", "lo");
+        eprintln!("{}", String::from_utf8(out.stderr).unwrap());
+        assert!(out.status.success(), "failed to set up lo via ip");
+
+        let bridge_id: u32 = sock
+            .get_link(LinkID::Name("test1".to_string()))
+            .expect("get_link failed")
+            .header
+            .index;
+
+        let addresses = sock
+            .dump_addresses(Some(bridge_id))
+            .expect("dump_address_filter failed");
+        for nla in addresses[0].attributes.iter() {
+            if let address::AddressAttribute::Address(ip) = nla {
+                assert_eq!(ip, &IpAddr::V4(Ipv4Addr::new(10, 0, 0, 2)))
+            }
+        }
+    }
 }

test/620-bridge-mode.bats

@@ -44,3 +44,30 @@ load helpers
     expected_rc=1 run_netavark --file ${TESTSDIR}/testfiles/bridge-managed-dhcp.json setup $(get_container_netns_path)
     assert_json ".error" "cannot use dhcp ipam driver without using the option mode=unmanaged" "dhcp error"
 }
+
+@test bridge - unmanaged mode with aardvark-dns no bridge ip {
+    run_in_host_netns ip link add brtest0 type bridge
+    run_in_host_netns ip link set brtest0 up
+    run_in_host_netns ip -j --details link show brtest0
+    link_info="$output"
+    assert_json "$link_info" '.[].flags[] | select(.=="UP")' == "UP" "Host bridge interface is up"
+
+    expected_rc=1 run_netavark --file ${TESTSDIR}/testfiles/bridge-unmanaged-dns.json setup $(get_container_netns_path)
+    assert_json ".error" "bridge 'brtest0' in unmanaged mode has no usable IP addresses. Aardvark-dns requires at least one address (should not be an IPv6 link-local address) to bind to. Please add an IP address or disable DNS for this network (--disable-dns)."
+}
+
+@test bridge - unmanaged mode with aardvark-dns bridge ip {
+    run_in_host_netns ip link add brtest0 type bridge
+    run_in_host_netns sysctl -w net.ipv6.conf.brtest0.accept_dad=0
+    run_in_host_netns ip link set brtest0 up
+    run_in_host_netns ip -j --details link show brtest0
+    link_info="$output"
+    assert_json "$link_info" '.[].flags[] | select(.=="UP")' == "UP" "Host bridge interface is up"
+
+    run_in_host_netns ip addr add 10.88.0.100/16 dev brtest0
+    run_in_host_netns ip addr add 2001:db8:abcd:000a:50a6:6bff:fe84:255a dev brtest0
+    run_netavark --file ${TESTSDIR}/testfiles/bridge-unmanaged-dns.json setup $(get_container_netns_path)
+
+    run_helper cat "$NETAVARK_TMPDIR/config/aardvark-dns/podman"
+    assert "${lines[0]}" == "10.88.0.100,2001:db8:abcd:a:50a6:6bff:fe84:255a" "aardvark-dns should bind to the unmanaged bridge IP"
+}

test/testfiles/bridge-unmanaged-dns.json

@@ -0,0 +1,32 @@
+{
+	"container_id": "6ce776ea58b5",
+	"container_name": "testcontainer",
+	"networks": {
+		"podman": {
+			"interface_name": "eth0",
+			"static_ips": ["10.88.0.2", "2001:db8:abcd:0::2"]
+		}
+	},
+	"network_info": {
+		"podman": {
+			"dns_enabled": true,
+			"driver": "bridge",
+			"id": "53ce4390f2adb1681eb1a90ec8b48c49c015e0a8d336c197637e7f65e365fa9e",
+			"internal": false,
+			"ipv6_enabled": false,
+			"name": "podman",
+			"network_interface": "brtest0",
+			"subnets": [
+				{
+					"subnet": "10.88.0.0/16"
+				},
+				{
+					"subnet": "2001:db8:abcd::/48"
+				}
+			],
+			"options": {
+				"mode": "unmanaged"
+			}
+		}
+	}
+}