More fixes after fuzzing

src/bgp/message/nlri.rs

@@ -314,12 +314,12 @@ where Octs: AsRef<[u8]>,
 impl<Octs: AsRef<[u8]>> Eq for Labels<Octs> { }
 
 
-impl<Octs: Octets> Labels<Octs> {
-    fn len(&self) -> usize {
+impl<Octs: AsRef<[u8]>> Labels<Octs> {
+    pub fn len(&self) -> usize {
         self.octets.as_ref().len()
     }
 
-    fn as_ref(&self) -> &[u8] {
+    pub fn as_ref(&self) -> &[u8] {
         self.octets.as_ref()
     }
 
@@ -491,6 +491,20 @@ pub struct MplsVpnNlri<Octs> {
     rd: RouteDistinguisher,
 }
 
+impl<T> MplsVpnNlri<T> {
+    pub fn basic(&self) -> BasicNlri {
+        self.basic
+    }
+
+    pub fn labels(&self) -> &Labels<T> {
+        &self.labels
+    }
+
+    pub fn rd(&self) -> RouteDistinguisher {
+        self.rd
+    }
+}
+
 impl<Octs, Other> PartialEq<MplsVpnNlri<Other>> for MplsVpnNlri<Octs>
 where Octs: AsRef<[u8]>,
       Other: AsRef<[u8]>
@@ -569,6 +583,12 @@ impl<Octs> EvpnNlri<Octs> {
     }
 }
 
+impl<T: AsRef<[u8]>> EvpnNlri<T> {
+    fn compose_len(&self) -> usize {
+        1 + self.raw.as_ref().len()
+    }
+}
+
 typeenum!(
     EvpnRouteType, u8,
     {
@@ -938,8 +958,12 @@ impl<Octs: AsRef<[u8]>> Nlri<Octs> {
     pub fn compose_len(&self) -> usize {
         match self {
             Nlri::Unicast(b) | Nlri::Multicast(b) => b.compose_len(),
+            Nlri::Mpls(m) => m.compose_len(),
+            Nlri::MplsVpn(m) => m.compose_len(),
+            Nlri::Vpls(v) => v.compose_len(),
             Nlri::FlowSpec(f) => f.compose_len(),
-            _ => todo!()
+            Nlri::RouteTarget(r) => r.compose_len(),
+            Nlri::Evpn(e) => e.compose_len(),
         }
     }
 }
@@ -1263,18 +1287,26 @@ impl<Octs: Octets> MplsVpnNlri<Octs> {
         let mut prefix_bits = parser.parse_u8()?;
         let labels = Labels::parse(parser)?;
 
-        // Check whether we can safely subtract the labels length from the
-        // prefix size. If there is an unexpected path id, we might silently
-        // subtract too much, because there is no 'subtract with overflow'
-        // warning when built in release mode.
-        if 8 * labels.len() as u8 > prefix_bits {
+        // Check whether we can safely subtract both the labels length and the
+        // route Distinguisher length from the prefix size. If there is an
+        // unexpected path id, we might silently subtract too much, because
+        // there is no 'subtract with overflow' warning when built in release
+        // mode.
+
+        if
+            u8::try_from(8 * (8 + labels.len()))
+                .map_err(|_| ParseError::form_error(
+                        "MplsVpnNlri labels/rd too long"
+                ))? > prefix_bits
+        {
             return Err(ParseError::ShortInput);
         }
 
         prefix_bits -= 8 * labels.len() as u8;
 
         let rd = RouteDistinguisher::parse(parser)?;
-        prefix_bits -= 8*std::mem::size_of::<RouteDistinguisher>() as u8;
+
+        prefix_bits -= 8*8;
 
         let prefix = parse_prefix_for_len(parser, prefix_bits, afi)?;
 
@@ -1283,6 +1315,12 @@ impl<Octs: Octets> MplsVpnNlri<Octs> {
     }
 }
 
+impl<T: AsRef<[u8]>> MplsVpnNlri<T> {
+    fn compose_len(&self) -> usize {
+        self.basic.compose_len() + self.labels.len() + self.rd.bytes.len()
+    }
+}
+
 impl<Octs: Octets> MplsNlri<Octs> {
     pub fn check(
         parser: &mut Parser<Octs>,
@@ -1334,7 +1372,10 @@ impl<Octs: Octets> MplsNlri<Octs> {
         // prefix size. If there is an unexpected path id, we might silently
         // subtract too much, because there is no 'subtract with overflow'
         // warning when built in release mode.
-        if 8 * labels.len() as u8 > prefix_bits {
+
+        if u8::try_from(8 * labels.len())
+            .map_err(|_| ParseError::form_error("MplsNlri labels too long"))?
+            > prefix_bits {
             return Err(ParseError::ShortInput);
         }
 
@@ -1351,6 +1392,15 @@ impl<Octs: Octets> MplsNlri<Octs> {
     }
 }
 
+impl<T: AsRef<[u8]>> MplsNlri<T> {
+    fn compose_len(&self) -> usize {
+        self.basic.compose_len() + self.labels.len()
+    }
+    pub fn labels(&self) -> &Labels<T> {
+        &self.labels
+    }
+}
+
 impl VplsNlri {
     pub fn check<Octs: Octets>(parser: &mut Parser<Octs>)
         -> Result<(), ParseError>
@@ -1384,6 +1434,10 @@ impl VplsNlri {
             }
         )
     }
+
+    fn compose_len(&self) -> usize {
+        8 + 2 + 2 + 2 + 4
+    }
 }
 
 impl<Octs: Octets> FlowSpecNlri<Octs> {
@@ -1521,6 +1575,12 @@ impl<Octs: Octets> RouteTargetNlri<Octs> {
     }
 }
 
+impl<T: AsRef<[u8]>> RouteTargetNlri<T> {
+    fn compose_len(&self) -> usize {
+        self.raw.as_ref().len()
+    }
+}
+
 impl<Octs: Octets> EvpnNlri<Octs> {
     pub fn parse<'a, R>(parser: &mut Parser<'a, R>)
         -> Result<Self, ParseError>

src/bgp/message/update.rs

@@ -1331,15 +1331,20 @@ mod tests {
 
     use super::*;
     use std::str::FromStr;
+    use std::net::Ipv6Addr;
     use crate::bgp::communities::{
         StandardCommunity,
         ExtendedCommunityType,
         ExtendedCommunitySubType,
         Tag, Wellknown,
     };
-    use crate::bgp::message::{Message, nlri::PathId};
+    use crate::bgp::message::{Message, nlri::{
+        Labels, PathId, RouteDistinguisher
+    }};
     use crate::addr::Prefix;
 
+
+
     use bytes::Bytes;
 
     #[allow(dead_code)]
@@ -1373,11 +1378,11 @@ mod tests {
     //   x chained iter
     // - MP NLRI types:
     //   announcements:
-    //   - v4 mpls unicast
+    //   x v4 mpls unicast
     //   - v4 mpls unicast unreach **missing**
     //   - v4 mpls vpn unicast
     //   - v6 mpls unicast addpath 
-    //   - v6 mpls vpn unicast
+    //   X v6 mpls vpn unicast
     //   - multicast **missing
     //   - vpls
     //   - flowspec
@@ -2380,4 +2385,156 @@ mod tests {
 
         assert_eq!(update.fmt_pcap_string(), update2.fmt_pcap_string());
     }
+
+    #[test]
+    fn v4_mpls_unicast() {
+        let raw = vec![
+            0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+            0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+            0x00, 0x5c, 0x02, 0x00, 0x00, 0x00, 0x45, 0x80,
+            0x0e, 0x31, 0x00, 0x01, 0x04, 0x04, 0x0a, 0x07,
+            0x08, 0x08, 0x00, 0x38, 0x01, 0xf4, 0x01, 0x0a,
+            0x00, 0x00, 0x09, 0x32, 0x01, 0xf4, 0x11, 0xc6,
+            0x33, 0x64, 0x00, 0x32, 0x01, 0xf4, 0x21, 0xc6,
+            0x33, 0x64, 0x40, 0x32, 0x01, 0xf4, 0x31, 0xc6,
+            0x33, 0x64, 0x80, 0x32, 0x01, 0xf4, 0x91, 0xc6,
+            0x33, 0x64, 0xc0, 0x40, 0x01, 0x01, 0x00, 0x40,
+            0x02, 0x0a, 0x02, 0x02, 0x00, 0x00, 0x01, 0x2c,
+            0x00, 0x00, 0x01, 0xf4
+        ];
+
+        let upd = UpdateMessage::from_octets(
+            &raw,
+            SessionConfig::modern()
+        ).unwrap();
+        if let Ok(Some(NextHop::Unicast(a))) = upd.mp_next_hop() {
+            assert_eq!(a, Ipv4Addr::from_str("10.7.8.8").unwrap());
+        } else {
+            panic!("wrong");
+        }
+        let mut ann = upd.mp_announcements().unwrap().unwrap().iter();
+        if let Some(Ok(Nlri::Mpls(n1))) = ann.next() {
+            assert_eq!(
+                n1.basic().prefix(),
+                Prefix::from_str("10.0.0.9/32").unwrap()
+            );
+            assert_eq!(
+                n1.labels().as_ref(),
+                &[0x01, 0xf4, 0x01] // single label: [2012]
+                //Labels::from(..),
+            );
+        } else {
+            panic!("wrong");
+        }
+
+        // and 4 more:
+        assert_eq!(ann.count(), 4);
+    }
+
+    #[test]
+    fn v6_mpls_vpn_unicast() {
+
+        // BGP UPDATE for 2/128, one single announced NLRI 
+        let raw = vec![
+            0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+            0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+            0x00, 0x9a, 0x02, 0x00, 0x00, 0x00, 0x83, 0x80,
+            0x0e, 0x39, 0x00, 0x02, 0x80, 0x18, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0xff, 0xff, 0x0a, 0x00, 0x00, 0x02, 0x00, 0xd8,
+            0x00, 0x7d, 0xc1, 0x00, 0x00, 0x00, 0x64, 0x00,
+            0x00, 0x00, 0x01, 0xfc, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+            0x00, 0x00, 0x01, 0x40, 0x01, 0x01, 0x00, 0x40,
+            0x02, 0x06, 0x02, 0x01, 0x00, 0x00, 0x00, 0x01,
+            0x80, 0x04, 0x04, 0x00, 0x00, 0x00, 0x00, 0x40,
+            0x05, 0x04, 0x00, 0x00, 0x00, 0x64, 0xc0, 0x10,
+            0x18, 0x00, 0x02, 0x00, 0x64, 0x00, 0x00, 0x00,
+            0x01, 0x00, 0x09, 0x00, 0x64, 0x00, 0x00, 0x00,
+            0x00, 0x01, 0x0b, 0x0a, 0x00, 0x00, 0x02, 0x00,
+            0x01, 0xc0, 0x14, 0x0e, 0x00, 0x01, 0x00, 0x00,
+            0x00, 0x64, 0x00, 0x00, 0x00, 0x01, 0x0a, 0x00,
+            0x00, 0x02
+        ];
+
+        let upd = UpdateMessage::from_octets(
+            &raw,
+            SessionConfig::modern()
+        ).unwrap();
+        if let Ok(Some(NextHop::Ipv6MplsVpnUnicast(rd, a))) = upd.mp_next_hop() {
+            assert_eq!(rd, RouteDistinguisher::new(&[0; 8]));
+            assert_eq!(a, Ipv6Addr::from_str("::ffff:10.0.0.2").unwrap());
+        } else {
+            panic!("wrong");
+        }
+        let mut ann = upd.mp_announcements().unwrap().unwrap().iter();
+        if let Some(Ok(Nlri::MplsVpn(n1))) = ann.next() {
+            assert_eq!(
+                n1.basic().prefix(),
+                Prefix::from_str("fc00::1/128").unwrap()
+            );
+            assert_eq!(
+                n1.labels().as_ref(),
+                &[0x00, 0x7d, 0xc1] // single label: [2012]
+                //Labels::from([2012]),
+            );
+            assert_eq!(
+                n1.rd(),
+                //RouteDistinguisher::from_str("100:1".unwrap())
+                RouteDistinguisher::new(&[0, 0, 0, 100, 0, 0, 0, 1])
+            );
+        } else {
+            panic!("wrong");
+        }
+
+        assert!(ann.next().is_none());
+    }
+
+
+
+
+    #[test]
+    fn debug_fuzz_unknown_afi_safi() {
+        let raw = vec![
+            255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
+            255, 255, 255, 0, 36, 0, 0, 0, 0, 13, 255, 255, 0, 0, 0, 14, 6, 1,
+            0, 4, 0, 1, 0, 0
+        ];
+
+        let upd = UpdateMessage::from_octets(&raw, SessionConfig::modern())
+            .unwrap();
+
+        if let Ok(pas) = upd.path_attributes() {
+            for pa in pas.into_iter() {
+                dbg!(&pa);
+                if let Ok(pa) = pa {
+                    let _ = pa.to_owned();
+                }
+            }
+        }
+    }
+
+
+    #[test]
+    fn debug_fuzz_assert_flowspecnlri() {
+        let raw = vec![
+            255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255,
+            255, 255, 255, 0, 36, 0, 0, 0, 0, 13, 255, 255, 0, 0, 0, 15, 6, 0,
+            2, 133, 43, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 1, 0, 255, 255, 255, 254
+        ];
+
+        let upd = UpdateMessage::from_octets(&raw, SessionConfig::modern())
+            .unwrap();
+
+        if let Ok(pas) = upd.path_attributes() {
+            for pa in pas.into_iter() {
+                dbg!(&pa);
+                if let Ok(pa) = pa {
+                    let _ = pa.to_owned();
+                }
+            }
+        }
+    }
 }

src/bgp/message/update_builder.rs

@@ -1172,12 +1172,18 @@ impl NextHop {
             NextHop::Unicast(IpAddr::V4(_)) | NextHop::Multicast(IpAddr::V4(_)) => 4, 
             NextHop::Unicast(IpAddr::V6(_)) | NextHop::Multicast(IpAddr::V6(_)) => 16, 
             NextHop::Ipv6LL(_, _) => 32,
-            NextHop::Ipv4MplsVpnUnicast(_rd, _ip4) => todo!(),
-            NextHop::Ipv6MplsVpnUnicast(_rd, _ip6) => todo!(),
+            NextHop::Ipv4MplsVpnUnicast(_rd, _ip4) => 8 + 4,
+            NextHop::Ipv6MplsVpnUnicast(_rd, _ip6) => 8 + 16,
             NextHop::Empty => 0, // FlowSpec
-            NextHop::Evpn(IpAddr::V4(_)) => todo!(),
-            NextHop::Evpn(IpAddr::V6(_)) => todo!(),
-            NextHop::Unimplemented(_afi, _safi) => todo!()
+            NextHop::Evpn(IpAddr::V4(_)) => 4, 
+            NextHop::Evpn(IpAddr::V6(_)) => 16,
+            NextHop::Unimplemented(_afi, _safi) => {
+                warn!(
+                    "unexpected compose_len called on NextHop::Unimplemented \
+                    returning usize::MAX, this will cause failure."
+                );
+                usize::MAX
+            }
         }
     }