diff --git a/Changelog.md b/Changelog.md index 97034f4..2f5ffac 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,6 +9,8 @@ Released yyyy-mm-dd. ## Bug fixes +* Accept 16-byte and 32-byte IPv6 next hops for IPv4 unicast MP_REACH_NLRI. + ## New ## Other changes diff --git a/src/bgp/message/update.rs b/src/bgp/message/update.rs index d4af281..b572392 100644 --- a/src/bgp/message/update.rs +++ b/src/bgp/message/update.rs @@ -1155,6 +1155,25 @@ impl PduParseInfo { Self { four_octet_asns: FourOctetAsns(false), pdu_addpaths: PduAddpaths::default() } } + /// Creates parse info from the ASN and independent per-PDU ADD-PATH flags. + /// + /// Useful when restoring context persisted alongside raw path attributes. + pub fn from_flags( + four_octet_asns: bool, + conventional_addpath: bool, + mp_reach_addpath: bool, + mp_unreach_addpath: bool, + ) -> Self { + Self { + four_octet_asns: FourOctetAsns(four_octet_asns), + pdu_addpaths: PduAddpaths { + conventional: conventional_addpath, + mp_reach: mp_reach_addpath, + mp_unreach: mp_unreach_addpath, + }, + } + } + /// Creates a `PduParseInfo` from a `SessionConfig` and address families. /// /// This constructor takes two optional address families, describing the @@ -2339,6 +2358,81 @@ mod tests { assert_eq!(upd.mp_withdrawals().unwrap().iter().count(), 1); } + #[test] + fn ipv4_unicast_with_extended_next_hop() { + let global: Ipv6Addr = "2001:db8::1".parse().unwrap(); + let link_local: Ipv6Addr = "fe80::1".parse().unwrap(); + for with_link_local in [false, true] { + let mut reach = vec![0, 1, 1, if with_link_local { 32 } else { 16 }]; + reach.extend_from_slice(&global.octets()); + if with_link_local { + reach.extend_from_slice(&link_local.octets()); + } + // Reserved octet followed by IPv4 NLRI 192.0.2.0/24. + reach.extend_from_slice(&[0, 24, 192, 0, 2]); + let mut attrs = vec![ + 0x40, 1, 1, 0, // ORIGIN: IGP + 0x40, 2, 0, // Empty AS_PATH + 0x80, 14, reach.len() as u8, + ]; + attrs.extend_from_slice(&reach); + let mut wire = vec![0xff; 16]; + wire.extend_from_slice(&((23 + attrs.len()) as u16).to_be_bytes()); + wire.extend_from_slice(&[2, 0, 0]); // UPDATE, no withdrawals + wire.extend_from_slice(&(attrs.len() as u16).to_be_bytes()); + wire.extend_from_slice(&attrs); + + let upd = UpdateMessage::from_octets(&wire, &SessionConfig::modern()) + .unwrap(); + let expected = if with_link_local { + NextHop::Ipv6LL { global, link_local } + } else { + NextHop::Unicast(global.into()) + }; + assert_eq!(upd.mp_next_hop().unwrap(), Some(expected)); + assert_eq!(upd.find_next_hop(AfiSafiType::Ipv4Unicast).unwrap(), expected); + let announcements: Vec<_> = upd.announcements().unwrap() + .map(|nlri| nlri.unwrap()).collect(); + assert_eq!(announcements.len(), 1); + assert_eq!(announcements[0].afi_safi(), AfiSafiType::Ipv4Unicast); + } + } + + #[test] + fn evpn_with_ipv6_next_hop() { + let next_hop: Ipv6Addr = "2001:db8::1".parse().unwrap(); + let mut reach = vec![0, 25, 70, 16]; + reach.extend_from_slice(&next_hop.octets()); + reach.push(0); // Reserved octet. + // Inclusive Multicast Ethernet Tag route: RD, Ethernet Tag ID, + // and originating router's IPv4 address. + reach.extend_from_slice(&[ + 3, 17, 0, 0, 0, 0, 0, 0, 0, 1, + 0, 0, 0, 0, 32, 192, 0, 2, 1, + ]); + let mut attrs = vec![ + 0x40, 1, 1, 0, // ORIGIN: IGP + 0x40, 2, 0, // Empty AS_PATH + 0x80, 14, reach.len() as u8, + ]; + attrs.extend_from_slice(&reach); + let mut wire = vec![0xff; 16]; + wire.extend_from_slice(&((23 + attrs.len()) as u16).to_be_bytes()); + wire.extend_from_slice(&[2, 0, 0]); // UPDATE, no withdrawals. + wire.extend_from_slice(&(attrs.len() as u16).to_be_bytes()); + wire.extend_from_slice(&attrs); + + let upd = UpdateMessage::from_octets(&wire, &SessionConfig::modern()) + .unwrap(); + let expected = NextHop::Unicast(next_hop.into()); + assert_eq!(upd.mp_next_hop().unwrap(), Some(expected)); + assert_eq!(upd.find_next_hop(AfiSafiType::L2VpnEvpn).unwrap(), expected); + let announcements: Vec<_> = upd.announcements().unwrap() + .map(|nlri| nlri.unwrap()).collect(); + assert_eq!(announcements.len(), 1); + assert_eq!(announcements[0].afi_safi(), AfiSafiType::L2VpnEvpn); + } + #[test] fn evpn() { let buf = vec![ diff --git a/src/bgp/nlri/nexthop.rs b/src/bgp/nlri/nexthop.rs index 318b178..5916d96 100644 --- a/src/bgp/nlri/nexthop.rs +++ b/src/bgp/nlri/nexthop.rs @@ -95,19 +95,22 @@ impl NextHop { let res = match afisafi { - Ipv4Unicast | - Ipv4Multicast | + Ipv4Multicast | Ipv4RouteTarget | - L2VpnVpls | - L2VpnEvpn + L2VpnVpls => { match len { 4 => NextHop::Unicast(parse_ipv4addr(parser)?.into()), _ => error!() } } - Ipv6Unicast => { + // Extended next hops allow IPv4 unicast NLRI with an IPv6 + // next hop, optionally followed by its link-local address. + Ipv4Unicast | Ipv6Unicast => { match len { + 4 if afisafi == Ipv4Unicast => { + NextHop::Unicast(parse_ipv4addr(parser)?.into()) + } 16 => NextHop::Unicast(parse_ipv6addr(parser)?.into()), 32 => NextHop::Ipv6LL{ global: parse_ipv6addr(parser)?, @@ -124,7 +127,8 @@ impl NextHop { } // RFC4684: the nexthop for MPLS can be of the other AFI than the // NLRI themselves are. - Ipv4MplsUnicast | Ipv6MplsUnicast => { + // RFC7432: EVPN next hops are the IPv4 or IPv6 address of the PE. + Ipv4MplsUnicast | Ipv6MplsUnicast | L2VpnEvpn => { match len { 4 => NextHop::Unicast(parse_ipv4addr(parser)?.into()), 16 => NextHop::Unicast(parse_ipv6addr(parser)?.into()), @@ -173,6 +177,98 @@ mod tests { #[allow(unused_imports)] use super::*; + #[test] + fn evpn_next_hops() { + for ip in ["192.0.2.1", "2001:db8::1"] { + let ip: IpAddr = ip.parse().unwrap(); + let bytes = match ip { + IpAddr::V4(ip) => ip.octets().to_vec(), + IpAddr::V6(ip) => ip.octets().to_vec(), + }; + let mut wire = vec![bytes.len() as u8]; + wire.extend_from_slice(&bytes); + wire.push(0xab); + let mut parser = Parser::from_ref(&wire); + assert_eq!(NextHop::parse(&mut parser, AfiSafi::L2VpnEvpn), + Ok(NextHop::Unicast(ip))); + assert_eq!(parser.parse_u8().unwrap(), 0xab); + } + } + + #[test] + fn evpn_invalid_next_hops() { + for len in 0..=255u8 { + if [4, 16].contains(&len) { + for available in 0..usize::from(len) { + let mut wire = vec![len]; + wire.resize(1 + available, 0); + assert_eq!( + NextHop::parse(&mut Parser::from_ref(&wire), AfiSafi::L2VpnEvpn), + Err(ParseError::ShortInput), + ); + } + } else { + let mut wire = vec![len]; + wire.resize(1 + usize::from(len), 0); + assert_eq!( + NextHop::parse(&mut Parser::from_ref(&wire), AfiSafi::L2VpnEvpn), + Err(ParseError::Unsupported), + ); + } + } + } + + #[test] + fn ipv4_unicast_next_hop_lengths() { + let global: Ipv6Addr = "2001:db8::1".parse().unwrap(); + let link_local: Ipv6Addr = "fe80::1".parse().unwrap(); + let cases = [ + (vec![192, 0, 2, 1], NextHop::Unicast(Ipv4Addr::new(192, 0, 2, 1).into())), + (global.octets().to_vec(), NextHop::Unicast(global.into())), + ( + [global.octets(), link_local.octets()].concat(), + NextHop::Ipv6LL { global, link_local }, + ), + ]; + for (bytes, expected) in cases { + let mut wire = vec![bytes.len() as u8]; + wire.extend_from_slice(&bytes); + wire.push(0xab); + let mut parser = Parser::from_ref(&wire); + assert_eq!(NextHop::parse(&mut parser, AfiSafi::Ipv4Unicast), Ok(expected)); + assert_eq!(parser.parse_u8().unwrap(), 0xab); + } + } + + #[test] + fn ipv4_unicast_invalid_next_hops() { + for len in 0..=255u8 { + if [4, 16, 32].contains(&len) { + continue; + } + let mut wire = vec![len]; + wire.resize(1 + usize::from(len), 0); + assert_eq!( + NextHop::parse(&mut Parser::from_ref(&wire), AfiSafi::Ipv4Unicast), + Err(ParseError::Unsupported), + ); + } + for len in [4, 16, 32] { + for available in 0..len { + let mut wire = vec![len as u8]; + wire.resize(1 + available, 0); + assert_eq!( + NextHop::parse(&mut Parser::from_ref(&wire), AfiSafi::Ipv4Unicast), + Err(ParseError::ShortInput), + ); + } + } + assert_eq!( + NextHop::parse(&mut Parser::from_ref(&&[4, 192, 0, 2, 1][..]), AfiSafi::Ipv6Unicast), + Err(ParseError::Unsupported), + ); + } + #[cfg(feature = "serde")] #[test] fn nexthop_serialize() { diff --git a/src/bgp/path_attributes.rs b/src/bgp/path_attributes.rs index 5daa880..ebae0f4 100644 --- a/src/bgp/path_attributes.rs +++ b/src/bgp/path_attributes.rs @@ -1524,7 +1524,7 @@ impl Attribute for MpReachNextHop { fn validate( _flags: Flags, parser: &mut Parser<'_, Octs>, - ppi: PduParseInfo + _ppi: PduParseInfo ) -> Result<(), ParseError> { @@ -1532,12 +1532,10 @@ impl Attribute for MpReachNextHop { let safi= parser.parse_u8()?; let afisafi = AfiSafiType::from((afi, safi)); let _next_hop = crate::bgp::types::NextHop::parse(parser, afisafi)?; - if parser.parse_u8()? != 0 { - return Err(ParseError::form_error( - "MP_REACH_NLRI reserved byte is non-zero" - )); - } - validate_mp_nlri(*parser, afisafi, ppi.mp_reach_addpath()) + // This attribute retains only the next hop, just like parse below. + // Trailing reserved/NLRI bytes belong to the full MP_REACH attribute + // and may require ADD-PATH context unavailable to stored attributes. + Ok(()) } fn parse<'a, Octs: 'a + Octets>(parser: &mut Parser<'a, Octs>, _ppi: PduParseInfo) @@ -2385,6 +2383,30 @@ mod tests { use crate::bgp::aspath::HopPath; use crate::bgp::types::OriginType; + #[test] + #[cfg(feature = "serde")] + fn mp_reach_next_hop_json_with_addpath_nlri() { + let mut value = vec![0, 1, 1, 16]; + value.extend_from_slice(&"2001:db8::3".parse::().unwrap().octets()); + // Check the retained prefix, ordinary NLRI, and ADD-PATH NLRI whose + // path ID would be an invalid prefix length without ADD-PATH context. + for tail in [vec![], vec![0, 24, 192, 0, 2], vec![0, 255, 0, 0, 1, 24, 192, 0, 2]] { + let mut raw = vec![0x80, 14, (value.len() + tail.len()) as u8]; + raw.extend_from_slice(&value); + raw.extend_from_slice(&tail); + let mut parser = Parser::from_ref(&raw); + let attr = WireformatPathAttribute::parse(&mut parser, PduParseInfo::modern()) + .unwrap().to_owned().unwrap(); + assert_eq!(serde_json::to_value(attr).unwrap(), serde_json::json!({ + "mpReachNlri": {"nextHop": {"ipv6Unicast": "2001:db8::3"}} + })); + } + // Next-hop truncation still fails validation. + let truncated = &value[..value.len() - 1]; + let mut parser = Parser::from_ref(&truncated); + assert!(MpReachNextHop::validate(Flags(0x80), &mut parser, PduParseInfo::modern()).is_err()); + } + #[test] fn wireformat_to_owned_and_back() { use super::PathAttribute as PA;