Bladeren bron

Merge pull request #895 from thvdveld/fix-ipv6-src-addr-selection

fix: don't panic if no suitable IPv6 src_addr is found
Thibaut Vandervelden 1 jaar geleden
bovenliggende
commit
9bd836ba65
5 gewijzigde bestanden met toevoegingen van 240 en 51 verwijderingen
  1. 2 2
      examples/ping.rs
  2. 25 13
      src/iface/interface/ipv6.rs
  3. 2 2
      src/iface/interface/mod.rs
  4. 209 24
      src/iface/interface/tests/ipv6.rs
  5. 2 10
      src/socket/icmp.rs

+ 2 - 2
examples/ping.rs

@@ -187,7 +187,7 @@ fn main() {
                         remote_addr
                     );
                     icmp_repr.emit(
-                        &iface.get_source_address_ipv6(&address).unwrap(),
+                        &iface.get_source_address_ipv6(&address),
                         &address,
                         &mut icmp_packet,
                         &device_caps.checksum,
@@ -221,7 +221,7 @@ fn main() {
                     let icmp_packet = Icmpv6Packet::new_checked(&payload).unwrap();
                     let icmp_repr = Icmpv6Repr::parse(
                         &address,
-                        &iface.get_source_address_ipv6(&address).unwrap(),
+                        &iface.get_source_address_ipv6(&address),
                         &icmp_packet,
                         &device_caps.checksum,
                     )

+ 25 - 13
src/iface/interface/ipv6.rs

@@ -21,8 +21,13 @@ impl Default for HopByHopResponse<'_> {
 impl InterfaceInner {
     /// Return the IPv6 address that is a candidate source address for the given destination
     /// address, based on RFC 6724.
+    ///
+    /// # Panics
+    /// This function panics if the destination address is unspecified.
     #[allow(unused)]
-    pub(crate) fn get_source_address_ipv6(&self, dst_addr: &Ipv6Address) -> Option<Ipv6Address> {
+    pub(crate) fn get_source_address_ipv6(&self, dst_addr: &Ipv6Address) -> Ipv6Address {
+        assert!(!dst_addr.is_unspecified());
+
         // See RFC 6724 Section 4: Candidate source address
         fn is_candidate_source_address(dst_addr: &Ipv6Address, src_addr: &Ipv6Address) -> bool {
             // For all multicast and link-local destination addresses, the candidate address MUST
@@ -39,10 +44,10 @@ impl InterfaceInner {
                 return false;
             }
 
-            // Loopback addresses and multicast address can not be in the candidate source address
+            // Unspecified addresses and multicast address can not be in the candidate source address
             // list. Except when the destination multicast address has a link-local scope, then the
             // source address can also be link-local multicast.
-            if src_addr.is_loopback() || src_addr.is_multicast() {
+            if src_addr.is_unspecified() || src_addr.is_multicast() {
                 return false;
             }
 
@@ -67,18 +72,28 @@ impl InterfaceInner {
             bits as usize
         }
 
-        // Get the first address that is a candidate address.
+        // If the destination address is a loopback address, or when there are no IPv6 addresses in
+        // the interface, then the loopback address is the only candidate source address.
+        if dst_addr.is_loopback()
+            || self
+                .ip_addrs
+                .iter()
+                .filter(|a| matches!(a, IpCidr::Ipv6(_)))
+                .count()
+                == 0
+        {
+            return Ipv6Address::LOOPBACK;
+        }
+
         let mut candidate = self
             .ip_addrs
             .iter()
-            .filter_map(|a| match a {
+            .find_map(|a| match a {
                 #[cfg(feature = "proto-ipv4")]
                 IpCidr::Ipv4(_) => None,
-                #[cfg(feature = "proto-ipv6")]
                 IpCidr::Ipv6(a) => Some(a),
             })
-            .find(|a| is_candidate_source_address(dst_addr, &a.address()))
-            .unwrap();
+            .unwrap(); // NOTE: we check above that there is at least one IPv6 address.
 
         for addr in self.ip_addrs.iter().filter_map(|a| match a {
             #[cfg(feature = "proto-ipv4")]
@@ -121,7 +136,7 @@ impl InterfaceInner {
             }
         }
 
-        Some(candidate.address())
+        candidate.address()
     }
 
     /// Determine if the given `Ipv6Address` is the solicited node
@@ -441,11 +456,8 @@ impl InterfaceInner {
 
         let src_addr = if src_addr.is_unicast() {
             src_addr
-        } else if let Some(addr) = self.get_source_address_ipv6(&dst_addr) {
-            addr
         } else {
-            net_debug!("no suitable source address found");
-            return None;
+            self.get_source_address_ipv6(&dst_addr)
         };
 
         let ipv6_reply_repr = Ipv6Repr {

+ 2 - 2
src/iface/interface/mod.rs

@@ -328,7 +328,7 @@ impl Interface {
     /// Get an address from the interface that could be used as source address. The selection is
     /// based on RFC6724.
     #[cfg(feature = "proto-ipv6")]
-    pub fn get_source_address_ipv6(&self, dst_addr: &Ipv6Address) -> Option<Ipv6Address> {
+    pub fn get_source_address_ipv6(&self, dst_addr: &Ipv6Address) -> Ipv6Address {
         self.inner.get_source_address_ipv6(dst_addr)
     }
 
@@ -728,7 +728,7 @@ impl InterfaceInner {
             #[cfg(feature = "proto-ipv4")]
             IpAddress::Ipv4(addr) => self.get_source_address_ipv4(addr).map(|a| a.into()),
             #[cfg(feature = "proto-ipv6")]
-            IpAddress::Ipv6(addr) => self.get_source_address_ipv6(addr).map(|a| a.into()),
+            IpAddress::Ipv6(addr) => Some(self.get_source_address_ipv6(addr).into()),
         }
     }
 

+ 209 - 24
src/iface/interface/tests/ipv6.rs

@@ -880,8 +880,6 @@ fn get_source_address() {
     //   fd00::201:1:1:1:2/64
     //   fd01::201:1:1:1:2/64
     //   2001:db8:3::1/64
-    //   ::1/128
-    //   ::/128
     iface.update_ip_addrs(|addrs| {
         addrs.clear();
 
@@ -897,17 +895,10 @@ fn get_source_address() {
         addrs
             .push(IpCidr::Ipv6(Ipv6Cidr::new(OWN_GLOBAL_UNICAST_ADDR1, 64)))
             .unwrap();
-
-        // These should never be used:
-        addrs
-            .push(IpCidr::Ipv6(Ipv6Cidr::new(Ipv6Address::LOOPBACK, 128)))
-            .unwrap();
-        addrs
-            .push(IpCidr::Ipv6(Ipv6Cidr::new(Ipv6Address::UNSPECIFIED, 128)))
-            .unwrap();
     });
 
     // List of addresses we test:
+    //   ::1               -> ::1
     //   fe80::42          -> fe80::1
     //   fd00::201:1:1:1:1 -> fd00::201:1:1:1:2
     //   fd01::201:1:1:1:1 -> fd01::201:1:1:1:2
@@ -924,63 +915,257 @@ fn get_source_address() {
     const GLOBAL_UNICAST_ADDR2: Ipv6Address =
         Ipv6Address::new(0x2001, 0x0db9, 0x0003, 0, 0, 0, 0, 2);
 
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&Ipv6Address::LOOPBACK),
+        Ipv6Address::LOOPBACK
+    );
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&LINK_LOCAL_ADDR),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR1),
+        OWN_UNIQUE_LOCAL_ADDR1
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR2),
+        OWN_UNIQUE_LOCAL_ADDR2
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
+        OWN_UNIQUE_LOCAL_ADDR1
+    );
+    assert_eq!(
+        iface
+            .inner
+            .get_source_address_ipv6(&Ipv6Address::LINK_LOCAL_ALL_NODES),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR1),
+        OWN_GLOBAL_UNICAST_ADDR1
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR2),
+        OWN_GLOBAL_UNICAST_ADDR1
+    );
+
+    assert_eq!(
+        iface.get_source_address_ipv6(&LINK_LOCAL_ADDR),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR1),
+        OWN_UNIQUE_LOCAL_ADDR1
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR2),
+        OWN_UNIQUE_LOCAL_ADDR2
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
+        OWN_UNIQUE_LOCAL_ADDR1
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&Ipv6Address::LINK_LOCAL_ALL_NODES),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR1),
+        OWN_GLOBAL_UNICAST_ADDR1
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR2),
+        OWN_GLOBAL_UNICAST_ADDR1
+    );
+}
+
+#[cfg(feature = "medium-ip")]
+#[test]
+fn get_source_address_only_link_local() {
+    let (mut iface, _, _) = setup(Medium::Ip);
+
+    // List of addresses in the interface:
+    //   fe80::1/64
+    const OWN_LINK_LOCAL_ADDR: Ipv6Address = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 1);
+    iface.update_ip_addrs(|ips| {
+        ips.clear();
+        ips.push(IpCidr::Ipv6(Ipv6Cidr::new(OWN_LINK_LOCAL_ADDR, 64)))
+            .unwrap();
+    });
+
+    // List of addresses we test:
+    //   ::1               -> ::1
+    //   fe80::42          -> fe80::1
+    //   fd00::201:1:1:1:1 -> fe80::1
+    //   fd01::201:1:1:1:1 -> fe80::1
+    //   fd02::201:1:1:1:1 -> fe80::1
+    //   ff02::1           -> fe80::1
+    //   2001:db8:3::2     -> fe80::1
+    //   2001:db9:3::2     -> fe80::1
+    const LINK_LOCAL_ADDR: Ipv6Address = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 42);
+    const UNIQUE_LOCAL_ADDR1: Ipv6Address = Ipv6Address::new(0xfd00, 0, 0, 201, 1, 1, 1, 1);
+    const UNIQUE_LOCAL_ADDR2: Ipv6Address = Ipv6Address::new(0xfd01, 0, 0, 201, 1, 1, 1, 1);
+    const UNIQUE_LOCAL_ADDR3: Ipv6Address = Ipv6Address::new(0xfd02, 0, 0, 201, 1, 1, 1, 1);
+    const GLOBAL_UNICAST_ADDR1: Ipv6Address =
+        Ipv6Address::new(0x2001, 0x0db8, 0x0003, 0, 0, 0, 0, 2);
+    const GLOBAL_UNICAST_ADDR2: Ipv6Address =
+        Ipv6Address::new(0x2001, 0x0db9, 0x0003, 0, 0, 0, 0, 2);
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&Ipv6Address::LOOPBACK),
+        Ipv6Address::LOOPBACK
+    );
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&LINK_LOCAL_ADDR),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR1),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR2),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface
+            .inner
+            .get_source_address_ipv6(&Ipv6Address::LINK_LOCAL_ALL_NODES),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR1),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR2),
+        OWN_LINK_LOCAL_ADDR
+    );
+
+    assert_eq!(
+        iface.get_source_address_ipv6(&LINK_LOCAL_ADDR),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR1),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR2),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&Ipv6Address::LINK_LOCAL_ALL_NODES),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR1),
+        OWN_LINK_LOCAL_ADDR
+    );
+    assert_eq!(
+        iface.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR2),
+        OWN_LINK_LOCAL_ADDR
+    );
+}
+
+#[cfg(feature = "medium-ip")]
+#[test]
+fn get_source_address_empty_interface() {
+    let (mut iface, _, _) = setup(Medium::Ip);
+
+    iface.update_ip_addrs(|ips| ips.clear());
+
+    // List of addresses we test:
+    //   ::1               -> ::1
+    //   fe80::42          -> ::1
+    //   fd00::201:1:1:1:1 -> ::1
+    //   fd01::201:1:1:1:1 -> ::1
+    //   fd02::201:1:1:1:1 -> ::1
+    //   ff02::1           -> ::1
+    //   2001:db8:3::2     -> ::1
+    //   2001:db9:3::2     -> ::1
+    const LINK_LOCAL_ADDR: Ipv6Address = Ipv6Address::new(0xfe80, 0, 0, 0, 0, 0, 0, 42);
+    const UNIQUE_LOCAL_ADDR1: Ipv6Address = Ipv6Address::new(0xfd00, 0, 0, 201, 1, 1, 1, 1);
+    const UNIQUE_LOCAL_ADDR2: Ipv6Address = Ipv6Address::new(0xfd01, 0, 0, 201, 1, 1, 1, 1);
+    const UNIQUE_LOCAL_ADDR3: Ipv6Address = Ipv6Address::new(0xfd02, 0, 0, 201, 1, 1, 1, 1);
+    const GLOBAL_UNICAST_ADDR1: Ipv6Address =
+        Ipv6Address::new(0x2001, 0x0db8, 0x0003, 0, 0, 0, 0, 2);
+    const GLOBAL_UNICAST_ADDR2: Ipv6Address =
+        Ipv6Address::new(0x2001, 0x0db9, 0x0003, 0, 0, 0, 0, 2);
+
+    assert_eq!(
+        iface.inner.get_source_address_ipv6(&Ipv6Address::LOOPBACK),
+        Ipv6Address::LOOPBACK
+    );
+
     assert_eq!(
         iface.inner.get_source_address_ipv6(&LINK_LOCAL_ADDR),
-        Some(OWN_LINK_LOCAL_ADDR)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR1),
-        Some(OWN_UNIQUE_LOCAL_ADDR1)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR2),
-        Some(OWN_UNIQUE_LOCAL_ADDR2)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.inner.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
-        Some(OWN_UNIQUE_LOCAL_ADDR1)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface
             .inner
             .get_source_address_ipv6(&Ipv6Address::LINK_LOCAL_ALL_NODES),
-        Some(OWN_LINK_LOCAL_ADDR)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.inner.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR1),
-        Some(OWN_GLOBAL_UNICAST_ADDR1)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.inner.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR2),
-        Some(OWN_GLOBAL_UNICAST_ADDR1)
+        Ipv6Address::LOOPBACK
     );
 
     assert_eq!(
         iface.get_source_address_ipv6(&LINK_LOCAL_ADDR),
-        Some(OWN_LINK_LOCAL_ADDR)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR1),
-        Some(OWN_UNIQUE_LOCAL_ADDR1)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR2),
-        Some(OWN_UNIQUE_LOCAL_ADDR2)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.get_source_address_ipv6(&UNIQUE_LOCAL_ADDR3),
-        Some(OWN_UNIQUE_LOCAL_ADDR1)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.get_source_address_ipv6(&Ipv6Address::LINK_LOCAL_ALL_NODES),
-        Some(OWN_LINK_LOCAL_ADDR)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR1),
-        Some(OWN_GLOBAL_UNICAST_ADDR1)
+        Ipv6Address::LOOPBACK
     );
     assert_eq!(
         iface.get_source_address_ipv6(&GLOBAL_UNICAST_ADDR2),
-        Some(OWN_GLOBAL_UNICAST_ADDR1)
+        Ipv6Address::LOOPBACK
     );
 }

+ 2 - 10
src/socket/icmp.rs

@@ -594,16 +594,8 @@ impl<'a> Socket<'a> {
                 }
                 #[cfg(feature = "proto-ipv6")]
                 IpAddress::Ipv6(dst_addr) => {
-                    let src_addr = match cx.get_source_address_ipv6(&dst_addr) {
-                        Some(addr) => addr,
-                        None => {
-                            net_trace!(
-                                "icmp:{}: not find suitable source address, dropping",
-                                remote_endpoint
-                            );
-                            return Ok(());
-                        }
-                    };
+                    let src_addr = cx.get_source_address_ipv6(&dst_addr);
+
                     let packet = Icmpv6Packet::new_unchecked(&*packet_buf);
                     let repr = match Icmpv6Repr::parse(
                         &src_addr,