diff --git a/src/bgp/message/update.rs b/src/bgp/message/update.rs index b68f0853..93f9f73a 100644 --- a/src/bgp/message/update.rs +++ b/src/bgp/message/update.rs @@ -15,7 +15,7 @@ use crate::bgp::message::nlri::{ }; use core::iter::Peekable; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::fmt; use crate::util::parser::{parse_ipv4addr, ParseError}; @@ -2137,8 +2137,9 @@ impl UpdateBuilder { } Some(ref builder) => { - if builder.afi_safi() != (AFI::Ipv6, SAFI::Unicast) - || builder.addpath_enabled() != b.is_addpath() { + if !builder.valid_combination( + AFI::Ipv6, SAFI::Unicast, b.is_addpath() + ) { // We are already constructing a // MP_UNREACH_NLRI but for a different // AFI,SAFI than the prefix in `withdrawal`, @@ -2248,24 +2249,84 @@ impl UpdateBuilder { } pub fn set_nexthop(&mut self, addr: IpAddr) -> Result<(), ComposeError> { - if self.nexthop.is_none() { - // Check if this goes over the total PDU len. That will happen - // before we will go over the u16 for Total Path Attributes Length - // so we don't have to check for that. - let new_total = self.total_pdu_len + 7; + // Depending on the variant of `addr` we add/update either: + // - the conventional NEXT_HOP path attribute (IPv4); or + // - we update/create a MpReachNlriBuilder (IPv6) + match addr { + IpAddr::V4(a) => { + if self.nexthop.is_none() { + // NEXT_HOP path attribute is 7 bytes long. + let new_total = self.total_pdu_len + 7; + if new_total > Self::MAX_PDU { + return Err(ComposeError::PduTooLarge(new_total)); + } else { + self.total_pdu_len = new_total; + self.attributes_len += 7 + } + self.nexthop = Some(new_pas::NextHop::new(a)); + } + } + IpAddr::V6(a) => { + if let Some(ref mut builder) = self.mp_reach_nlri_builder { + // Given that we have a builder, there is already either a + // NextHop::Ipv6 or a NextHop::Ipv6LL. Setting the + // non-link-local address does not change the length of + // the next hop part, so there is no need to update the + // total_pdu_len and the attributes_len. + builder.set_nexthop(a); + } else { + let builder = MpReachNlriBuilder::new( + AFI::Ipv6, SAFI::Unicast, NextHop::Ipv6(a), false + ); + let new_bytes = builder.compose_len_empty(); + let new_total = self.total_pdu_len + new_bytes; + if new_total > Self::MAX_PDU { + return Err(ComposeError::PduTooLarge(new_total)); + } + self.attributes_len += new_bytes; + self.total_pdu_len = new_total; + self.mp_reach_nlri_builder = Some(builder); + } + } + } + Ok(()) + } + + pub fn set_nexthop_ll(&mut self, addr: Ipv6Addr) + -> Result<(), ComposeError> + { + // XXX We could/should check for addr.is_unicast_link_local() once + // that lands in stable. + + if let Some(ref mut builder) = self.mp_reach_nlri_builder { + let new_bytes = match builder.get_nexthop() { + NextHop::Ipv6(_) => 16, + NextHop::Ipv6LL(_,_) => 0, + _ => unreachable!() + }; + + let new_total = self.total_pdu_len + new_bytes; if new_total > Self::MAX_PDU { return Err(ComposeError::PduTooLarge(new_total)); - } else { - self.total_pdu_len = new_total; - self.attributes_len += 7 } - } - match addr { - IpAddr::V4(a) => { - self.nexthop = Some(new_pas::NextHop::new(a)) + builder.set_nexthop_ll(addr); + self.attributes_len += new_bytes; + self.total_pdu_len = new_total; + } else { + let builder = MpReachNlriBuilder::new( + AFI::Ipv6, SAFI::Unicast, NextHop::Ipv6LL(0.into(), addr), + false + ); + let new_bytes = builder.compose_len_empty(); + let new_total = self.total_pdu_len + new_bytes; + if new_total > Self::MAX_PDU { + return Err(ComposeError::PduTooLarge(new_total)); } - IpAddr::V6(_a) => todo!() // goes in MP_REACH_NLRI + self.attributes_len += new_bytes; + self.total_pdu_len = new_total; + self.mp_reach_nlri_builder = Some(builder); } + Ok(()) } @@ -2307,7 +2368,8 @@ impl UpdateBuilder { nexthop, b.is_addpath() ); - let res = builder.compose_len(announcement); + let res = builder.compose_len_empty() + + builder.compose_len(announcement); self.mp_reach_nlri_builder = Some(builder); res @@ -2320,19 +2382,16 @@ impl UpdateBuilder { //in the outer match anyway, as that is never a //conventional announcement anyway? - if builder.afi_safi() != (AFI::Ipv6, SAFI::Unicast) - || builder.addpath_enabled() != b.is_addpath() { - // We are already constructing a - // MP_REACH_NLRI but for a different - // AFI,SAFI than the prefix in `announcement`, - // or we are mixing addpath with non-addpath. + if !builder.valid_combination( + AFI::Ipv6, SAFI::Unicast, b.is_addpath() + ) { return Err(ComposeError::IllegalCombination); } + builder.compose_len(announcement) } }; - println!("new_bytes_num: {new_bytes_num}"); let new_total = self.total_pdu_len + new_bytes_num; if new_total > Self::MAX_PDU { @@ -2381,6 +2440,9 @@ pub enum ComposeError{ AttributeTooLarge(PathAttributeType, usize), AttributesTooLarge(usize), IllegalCombination, + EmptyMpReachNlri, + EmptyMpUnreachNlri, + WrongAddressType, /// Variant for `octseq::builder::ShortBuf` ShortBuf, @@ -2416,6 +2478,15 @@ impl fmt::Display for ComposeError { ComposeError::IllegalCombination => { write!(f, "illegal combination of prefixes/attributes") } + ComposeError::EmptyMpReachNlri => { + write!(f, "missing NLRI in MP_REACH_NLRI") + } + ComposeError::EmptyMpUnreachNlri => { + write!(f, "missing NLRI in MP_UNREACH_NLRI") + } + ComposeError::WrongAddressType => { + write!(f, "wrong address type") + } ComposeError::ShortBuf => { ShortBuf.fmt(f) } @@ -2609,12 +2680,32 @@ where Target: FreezeBuilder, ::Octets: Octets, { + self.is_valid()?; Ok(UpdateMessage::from_octets( self.finish().map_err(|_| ShortBuf)?, SessionConfig::modern() )?) } - pub fn finish(mut self) + // Check whether the combination of NLRI and attributes would produce a + // valid UPDATE pdu. + fn is_valid(&self) -> Result<(), ComposeError> { + // If we have builders for MP_(UN)REACH_NLRI, they should carry + // prefixes. + if let Some(ref builder) = self.mp_reach_nlri_builder { + if builder.is_empty() { + return Err(ComposeError::EmptyMpReachNlri); + } + } + if let Some(ref builder) = self.mp_unreach_nlri_builder { + if builder.is_empty() { + return Err(ComposeError::EmptyMpUnreachNlri); + } + } + + Ok(()) + } + + fn finish(mut self) -> Result<::Octets, Target::AppendError> { let mut header = Header::for_slice_mut(self.target.as_mut()); @@ -3739,7 +3830,7 @@ mod tests { let mut iter = prefixes.into_iter().peekable(); builder.announcements_from_iter(&mut iter).unwrap(); builder.set_origin(OriginType::Igp).unwrap(); - //builder.set_nexthop("fe80:1:2;3::".parse().unwrap()).unwrap(); + builder.set_nexthop("fe80:1:2:3::".parse().unwrap()).unwrap(); let path = HopPath::from([ Asn::from_u32(100), Asn::from_u32(200), @@ -3749,9 +3840,84 @@ mod tests { let raw = builder.finish().unwrap(); print_pcap(&raw); + } + #[test] + fn build_announcements_mp_missing_nlri() { + use crate::bgp::aspath::HopPath; + + let mut builder = UpdateBuilder::new_vec(); + builder.set_origin(OriginType::Igp).unwrap(); + builder.set_nexthop("fe80:1:2:3::".parse().unwrap()).unwrap(); + assert!(builder.mp_reach_nlri_builder.is_some()); + let path = HopPath::from([ + Asn::from_u32(100), + Asn::from_u32(200), + Asn::from_u32(300), + ]); + builder.set_aspath::>(path.to_as_path().unwrap()).unwrap(); + + assert!(matches!( + builder.into_message(), + Err(ComposeError::EmptyMpReachNlri) + )); } + #[test] + fn build_announcements_mp_link_local() { + use crate::bgp::aspath::HopPath; + + let mut builder = UpdateBuilder::new_vec(); + + let prefixes = [ + "2001:db8:1::/48", + "2001:db8:2::/48", + "2001:db8:3::/48", + ].map(|p| Nlri::unicast_from_str(p).unwrap()); + + + let mut iter = prefixes.into_iter().peekable(); + + builder.announcements_from_iter(&mut iter).unwrap(); + builder.set_origin(OriginType::Igp).unwrap(); + //builder.set_nexthop("2001:db8::1".parse().unwrap()).unwrap(); + builder.set_nexthop_ll("fe80:1:2:3::".parse().unwrap()).unwrap(); + + + let path = HopPath::from([ + Asn::from_u32(100), + Asn::from_u32(200), + Asn::from_u32(300), + ]); + builder.set_aspath::>(path.to_as_path().unwrap()).unwrap(); + + //let unchecked = builder.finish().unwrap(); + //print_pcap(unchecked); + let msg = builder.into_message().unwrap(); + msg.print_pcap(); + } + + #[test] + fn build_announcements_mp_ll_no_nlri() { + use crate::bgp::aspath::HopPath; + + let mut builder = UpdateBuilder::new_vec(); + builder.set_origin(OriginType::Igp).unwrap(); + //builder.set_nexthop("2001:db8::1".parse().unwrap()).unwrap(); + builder.set_nexthop_ll("fe80:1:2:3::".parse().unwrap()).unwrap(); + assert!(builder.mp_reach_nlri_builder.is_some()); + let path = HopPath::from([ + Asn::from_u32(100), + Asn::from_u32(200), + Asn::from_u32(300), + ]); + builder.set_aspath::>(path.to_as_path().unwrap()).unwrap(); + + assert!(matches!( + builder.into_message(), + Err(ComposeError::EmptyMpReachNlri) + )); + } #[test] diff --git a/src/bgp/message/update_builder.rs b/src/bgp/message/update_builder.rs index c9e2e839..6ce9fc0b 100644 --- a/src/bgp/message/update_builder.rs +++ b/src/bgp/message/update_builder.rs @@ -1,3 +1,5 @@ +use std::net::Ipv6Addr; + use octseq::OctetsBuilder; use crate::bgp::message::nlri::Nlri; @@ -9,6 +11,8 @@ use super::update::ComposeError; #[allow(dead_code)] pub mod new_pas { + use std::net::Ipv4Addr; + // eventually we work towards // enum PathAttribute { // ... @@ -76,7 +80,6 @@ pub mod new_pas { //--- NextHop - use std::net::Ipv4Addr; #[derive(Debug)] pub struct NextHop(Ipv4Addr); @@ -131,7 +134,14 @@ pub struct MpReachNlriBuilder { } impl MpReachNlriBuilder { - pub fn new( + + + // Minimal required size for a meaningful MP_REACH_NLRI. This comprises + // the attribute flags/size/type (3 bytes), a IPv6 nexthop (17), reserved + // byte (1) and then space for at least an IPv6 /48 announcement (7) + //pub const MIN_SIZE: usize = 3 + 17 + 1 + 7; + + pub(crate) fn new( afi: AFI, safi: SAFI, nexthop: NextHop, @@ -158,19 +168,47 @@ impl MpReachNlriBuilder { } } - pub fn afi_safi(&self) -> (AFI, SAFI) { - (self.afi, self.safi) + pub(crate) fn is_empty(&self) -> bool { + self.announcements.is_empty() } - pub fn addpath_enabled(&self) -> bool { - self.addpath_enabled + pub(crate) fn get_nexthop(&self) -> &NextHop { + &self.nexthop } - pub fn set_nexthop(&mut self, nexthop: NextHop) { - self.nexthop = nexthop; + pub(crate) fn set_nexthop(&mut self, addr: Ipv6Addr) { + match self.nexthop { + NextHop::Ipv6(_) => self.nexthop = NextHop::Ipv6(addr), + NextHop::Ipv6LL(_, ll) => { + self.nexthop = NextHop::Ipv6LL(addr, ll) + } + _ => unimplemented!() + } } - pub fn add_announcement(&mut self, announcement: &Nlri>) + pub(crate) fn set_nexthop_ll(&mut self, addr: Ipv6Addr) { + match self.nexthop { + NextHop::Ipv6(a) => { + self.nexthop = NextHop::Ipv6LL(a, addr); + self.len += 16; + } + NextHop::Ipv6LL(a, _ll) => { + self.nexthop = NextHop::Ipv6LL(a, addr); + } + _ => unreachable!() + } + } + + pub(crate) fn valid_combination( + &self, afi: AFI, safi: SAFI, is_addpath: bool + ) -> bool { + self.afi == afi + && self.safi == safi + && (self.announcements.is_empty() + || self.addpath_enabled == is_addpath) + } + + pub(crate) fn add_announcement(&mut self, announcement: &Nlri>) -> Result<(), ComposeError> { let announcement_len = announcement.compose_len(); @@ -182,16 +220,13 @@ impl MpReachNlriBuilder { Ok(()) } - pub fn compose_len(&self, announcement: &Nlri>) -> usize { - let announcement_len = announcement.compose_len(); - if self.announcements.is_empty() { - // First announcement to be added, so the total number of - // required octets includes the path attribute flags and - // length, the AFI/SAFI, the nexthop field, and the reserved byte. - let nh_len = self.nexthop.compose_len(); - return announcement_len + 3 + 3 + nh_len + 1; - } + pub(crate) fn compose_len_empty(&self) -> usize { + 3 + 3 + self.nexthop.compose_len() + 1 + } + + pub(crate) fn compose_len(&self, announcement: &Nlri>) -> usize { + let announcement_len = announcement.compose_len(); if !self.extended && self.len + announcement_len > 255 { // Adding this announcement would make the path attribute exceed // 255 and thus require the Extended Length bit to be set. @@ -202,7 +237,7 @@ impl MpReachNlriBuilder { announcement_len } - pub fn compose(&self, target: &mut Target) + pub(crate) fn compose(&self, target: &mut Target) -> Result<(), Target::AppendError> { let len = self.len.to_be_bytes(); @@ -253,7 +288,7 @@ impl NextHop { } } - pub fn compose(&self, target: &mut Target) + pub(crate) fn compose(&self, target: &mut Target) -> Result<(), Target::AppendError> { target.append_slice(&[u8::try_from(self.compose_len()).unwrap() - 1])?; @@ -297,7 +332,7 @@ pub struct MpUnreachNlriBuilder { } impl MpUnreachNlriBuilder { - pub fn new(afi: AFI, safi: SAFI, addpath_enabled: bool) -> Self { + pub(crate) fn new(afi: AFI, safi: SAFI, addpath_enabled: bool) -> Self { MpUnreachNlriBuilder { withdrawals: vec![], len: 3, // 3 bytes for AFI+SAFI @@ -308,15 +343,20 @@ impl MpUnreachNlriBuilder { } } - pub fn afi_safi(&self) -> (AFI, SAFI) { - (self.afi, self.safi) + pub(crate) fn is_empty(&self) -> bool { + self.withdrawals.is_empty() } - pub fn addpath_enabled(&self) -> bool { - self.addpath_enabled + pub(crate) fn valid_combination( + &self, afi: AFI, safi: SAFI, is_addpath: bool + ) -> bool { + self.afi == afi + && self.safi == safi + && (self.withdrawals.is_empty() + || self.addpath_enabled == is_addpath) } - pub fn add_withdrawal(&mut self, withdrawal: &Nlri>) + pub(crate) fn add_withdrawal(&mut self, withdrawal: &Nlri>) -> Result<(), ComposeError> { let withdrawal_len = withdrawal.compose_len(); @@ -328,7 +368,7 @@ impl MpUnreachNlriBuilder { Ok(()) } - pub fn compose_len(&self, withdrawal: &Nlri>) -> usize { + pub(crate) fn compose_len(&self, withdrawal: &Nlri>) -> usize { let withdrawal_len = withdrawal.compose_len(); if self.withdrawals.is_empty() { // First withdrawal to be added, so the total number of @@ -347,7 +387,7 @@ impl MpUnreachNlriBuilder { withdrawal_len } - pub fn compose(&self, target: &mut Target) + pub(crate) fn compose(&self, target: &mut Target) -> Result<(), Target::AppendError> { let len = self.len.to_be_bytes();