From e5341ea6398cc7d9e72ec38000dfd3cfe22021c8 Mon Sep 17 00:00:00 2001 From: Rayhaan Jaufeerally Date: Tue, 6 Oct 2026 11:15:58 +0200 Subject: [PATCH] Packet: the wire format's shared pieces in one module What the parsers and writers of messages, attributes and capabilities had each written for themselves is one module (wire.rs): the shape of a parser's result and its error, reading a list to the end, checking that nothing was left over, a non-empty list of fixed-size records (the community attributes and CLUSTER_LIST, which now fail alike), an AS number at the session's width, an AFI/SAFI pair, and a length written before what it counts. open.rs loses its fail! macro and attributes.rs its own copies of the same. Tests for what had none: AS4_AGGREGATOR kept opaque and discarded when malformed; IPv4 routes with IPv6 next hops (RFC 8950) in the property tests; and what a part-level parser accepts re-encodes to what it read, but for the fields the decoders normalise. Nlri::wire_len had no callers; the mask of a prefix's last octet is one function; a test's name is spelled right; a comment is on the test it describes. Co-Authored-By: Claude Fable 5.1 --- crates/packet/src/attributes.rs | 237 +++++++++++------------------ crates/packet/src/lib.rs | 1 + crates/packet/src/message.rs | 68 +++------ crates/packet/src/open.rs | 76 +++------ crates/packet/src/update_decode.rs | 94 +++++++++--- crates/packet/src/wire.rs | 189 +++++++++++++++++++++++ crates/packet/tests/properties.rs | 86 +++++++++-- crates/primitives/src/ip_prefix.rs | 18 ++- 8 files changed, 479 insertions(+), 290 deletions(-) create mode 100644 crates/packet/src/wire.rs diff --git a/crates/packet/src/attributes.rs b/crates/packet/src/attributes.rs index 72dd7d4..2a0198f 100644 --- a/crates/packet/src/attributes.rs +++ b/crates/packet/src/attributes.rs @@ -9,17 +9,20 @@ use std::net::{Ipv4Addr, Ipv6Addr}; use bitfield::bitfield; use bytes::{BufMut, BytesMut}; -use nom::Err::Failure; -use nom::number::complete::{be_u8, be_u16, be_u32}; -use nom::{IResult, Parser}; +use nom::Parser; +use nom::number::complete::{be_u8, be_u32}; use serde::Serialize; use serde_repr::Serialize_repr; -use bgp_primitives::constants::{Afi, AfiSafi, IpVersion, Safi}; +use bgp_primitives::constants::{AfiSafi, IpVersion, Safi}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use bgp_primitives::parser::{BgpParserError, EncodeError, ParserContext}; -use crate::message::{AS_TRANS, write_nlri}; +use crate::message::write_nlri; +use crate::wire::{ + Parsed, list, malformed, parse_afi_safi, parse_asn, put_afi_safi, put_asn, put_length, records, + whole, +}; bitfield! { #[derive(Clone, Debug, Serialize, PartialEq, Eq)] @@ -109,18 +112,6 @@ pub(crate) fn defined_flags(type_code: u8) -> Option<(bool, bool)> { }) } -/// A parsed value, provided the parser used all of it. -fn whole((rest, parsed): (&[u8], T)) -> Result>> { - if rest.is_empty() { - Ok(parsed) - } else { - Err(Failure(BgpParserError::Malformed(format!( - "{} octets left over", - rest.len() - )))) - } -} - /// One attribute as it lies on the wire, split out but not interpreted. #[derive(Debug, Clone)] pub struct RawAttribute<'a> { @@ -228,12 +219,8 @@ impl PathAttribute { /// Parses one attribute, header and value, from the front of `buf`. /// The flags are not checked against the attribute's definition; /// [`crate::message::UpdateMessage::decode`] does that for attributes off a session. - pub fn from_wire<'a>( - ctx: &ParserContext, - buf: &'a [u8], - ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let (raw, rest) = - split_attribute(buf).map_err(|e| Failure(BgpParserError::Malformed(e.to_string())))?; + pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { + let (raw, rest) = split_attribute(buf).map_err(|e| malformed(e.to_string()))?; let attribute = Self::parse_value(ctx, raw.flags, raw.type_code, raw.value)?; Ok((rest, attribute)) } @@ -249,32 +236,50 @@ impl PathAttribute { ) -> Result>> { use attr_type::*; Ok(match type_code { - ORIGIN => Self::Origin(whole(OriginPathAttribute::from_wire(value)?)?), - AS_PATH => Self::AsPath(whole(AsPathAttribute::from_wire(ctx, value)?)?), - NEXT_HOP => Self::NextHop(whole(NextHopPathAttribute::from_wire(value)?)?), - MULTI_EXIT_DISC => { - Self::MultiExitDisc(whole(MultiExitDiscPathAttribute::from_wire(value)?)?) - } - LOCAL_PREF => Self::LocalPref(whole(LocalPrefPathAttribute::from_wire(value)?)?), - ATOMIC_AGGREGATE => { - Self::AtomicAggregate(whole((value, AtomicAggregatePathAttribute {}))?) - } - AGGREGATOR => Self::Aggregator(whole(AggregatorPathAttribute::from_wire(ctx, value)?)?), - COMMUNITIES => Self::Communities(whole(CommunitiesPathAttribute::from_wire(value)?)?), - MP_REACH_NLRI => { - Self::MpReachNlri(whole(MpReachNlriPathAttribute::from_wire(ctx, value)?)?) - } - MP_UNREACH_NLRI => { - Self::MpUnreachNlri(whole(MpUnreachNlriPathAttribute::from_wire(ctx, value)?)?) - } + ORIGIN => Self::Origin(whole("ORIGIN", OriginPathAttribute::from_wire(value)?)?), + AS_PATH => Self::AsPath(whole("AS_PATH", AsPathAttribute::from_wire(ctx, value)?)?), + NEXT_HOP => Self::NextHop(whole("NEXT_HOP", NextHopPathAttribute::from_wire(value)?)?), + MULTI_EXIT_DISC => Self::MultiExitDisc(whole( + "MULTI_EXIT_DISC", + MultiExitDiscPathAttribute::from_wire(value)?, + )?), + LOCAL_PREF => Self::LocalPref(whole( + "LOCAL_PREF", + LocalPrefPathAttribute::from_wire(value)?, + )?), + ATOMIC_AGGREGATE => Self::AtomicAggregate(whole( + "ATOMIC_AGGREGATE", + (value, AtomicAggregatePathAttribute {}), + )?), + AGGREGATOR => Self::Aggregator(whole( + "AGGREGATOR", + AggregatorPathAttribute::from_wire(ctx, value)?, + )?), + COMMUNITIES => Self::Communities(whole( + "COMMUNITIES", + CommunitiesPathAttribute::from_wire(value)?, + )?), + MP_REACH_NLRI => Self::MpReachNlri(whole( + "MP_REACH_NLRI", + MpReachNlriPathAttribute::from_wire(ctx, value)?, + )?), + MP_UNREACH_NLRI => Self::MpUnreachNlri(whole( + "MP_UNREACH_NLRI", + MpUnreachNlriPathAttribute::from_wire(ctx, value)?, + )?), EXTENDED_COMMUNITIES => Self::ExtendedCommunities(whole( + "EXTENDED_COMMUNITIES", ExtendedCommunitiesPathAttribute::from_wire(value)?, )?), // Always four octets wide, whatever the session's AS_PATH is. - AS4_PATH => Self::As4Path(whole(AsPathAttribute::from_wire_with_width(true, value)?)?), - LARGE_COMMUNITIES => { - Self::LargeCommunities(whole(LargeCommunitiesPathAttribute::from_wire(value)?)?) - } + AS4_PATH => Self::As4Path(whole( + "AS4_PATH", + AsPathAttribute::from_wire_with_width(true, value)?, + )?), + LARGE_COMMUNITIES => Self::LargeCommunities(whole( + "LARGE_COMMUNITIES", + LargeCommunitiesPathAttribute::from_wire(value)?, + )?), ONLY_TO_CUSTOMER => { let asn = <[u8; 4]>::try_from(value).map_err(|_| { malformed(format!( @@ -293,22 +298,12 @@ impl PathAttribute { })?; Self::OriginatorId(Ipv4Addr::from(id)) } - CLUSTER_LIST => { - if value.is_empty() || !value.len().is_multiple_of(4) { - return Err(malformed(format!( - "CLUSTER_LIST is {} octets long, not a multiple of four", - value.len() - ))); - } - Self::ClusterList( - value - .as_chunks::<4>() - .0 - .iter() - .map(|id| Ipv4Addr::from(*id)) - .collect(), - ) - } + CLUSTER_LIST => Self::ClusterList( + records::<4>(value, "CLUSTER_LIST")? + .iter() + .map(|id| Ipv4Addr::from(*id)) + .collect(), + ), _ => Self::UnknownPathAttribute { flags, type_code, @@ -325,26 +320,21 @@ impl PathAttribute { let mut flags = self.flags(); out.put_u8(0); out.put_u8(self.type_code()); + let length_at = out.len(); out.put_u8(0); - let value_start = out.len(); self.value_to_wire(ctx, out)?; - let length = out.len() - value_start; + let length = out.len() - length_at - 1; if let Ok(short) = u8::try_from(length) && !flags.extended_length() { - out[start + 2] = short; + out[length_at] = short; } else { - let long = u16::try_from(length).map_err(|_| EncodeError::TooLong { - what: "path attribute", - len: length, - max: u16::MAX.into(), - })?; flags.set_extended_length(true); // Move the value up by one octet to make room. out.put_u8(0); - out.copy_within(value_start..value_start + length, value_start + 1); - out[start + 2..start + 4].copy_from_slice(&long.to_be_bytes()); + out.copy_within(length_at + 1..length_at + 1 + length, length_at + 2); + put_length(out, length_at, 2, length, u16::MAX.into(), "path attribute")?; } out[start] = flags.0; Ok(()) @@ -394,12 +384,6 @@ pub fn only_to_customer_wire(asn: u32) -> [u8; 7] { ] } -type Parsed<'a, T> = IResult<&'a [u8], T, BgpParserError<&'a [u8]>>; - -fn malformed<'a>(detail: String) -> nom::Err> { - Failure(BgpParserError::Malformed(detail)) -} - /// ORIGIN: how the route came into BGP (RFC 4271 §5.1.1). #[derive(Debug, PartialEq, Eq, Copy, Clone, Serialize_repr)] #[repr(u8)] @@ -518,16 +502,11 @@ impl AsPathAttribute { ))); } if len == 0 { - return Err(malformed("AS path segment with no AS numbers".into())); + return Err(malformed("AS path segment with no AS numbers")); } let count = usize::from(len); - let (buf, path): (_, Vec) = if four_octet { - nom::multi::many_m_n(count, count, be_u32).parse(buf)? - } else { - nom::multi::many_m_n(count, count, be_u16) - .parse(buf) - .map(|(buf, asns)| (buf, asns.into_iter().map(u32::from).collect()))? - }; + let (buf, path) = + nom::multi::many_m_n(count, count, |buf| parse_asn(four_octet, buf)).parse(buf)?; Ok(( buf, AsPathSegment { @@ -536,16 +515,7 @@ impl AsPathAttribute { }, )) }; - - // Not many0: that stops quietly at a segment it cannot read, and a - // truncated segment is an error. - let mut segments = Vec::new(); - let mut buf = buf; - while !buf.is_empty() { - let (rest, segment) = parse_segment(buf)?; - segments.push(segment); - buf = rest; - } + let (buf, segments) = list(buf, parse_segment)?; Ok((buf, Self { segments })) } @@ -567,11 +537,7 @@ impl AsPathAttribute { })?; out.put_u8(count); for asn in &segment.path { - if four_octet { - out.put_u32(*asn); - } else { - out.put_u16(u16::try_from(*asn).unwrap_or(AS_TRANS)); - } + put_asn(four_octet, *asn, out); } } Ok(()) @@ -645,11 +611,7 @@ impl AggregatorPathAttribute { /// The AS number is as wide as the session's AS numbers (RFC 6793), so /// the attribute is six or eight octets. pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { - let (buf, asn) = if ctx.four_octet_asn { - be_u32(buf)? - } else { - be_u16(buf).map(|(buf, asn)| (buf, u32::from(asn)))? - }; + let (buf, asn) = parse_asn(ctx.four_octet_asn, buf)?; let (buf, ip) = be_u32(buf)?; Ok(( buf, @@ -663,11 +625,7 @@ impl AggregatorPathAttribute { /// On a two-octet session an AS number that does not fit is written as /// AS_TRANS (RFC 6793 §4.2.2). pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) { - if ctx.four_octet_asn { - out.put_u32(self.asn); - } else { - out.put_u16(u16::try_from(self.asn).unwrap_or(AS_TRANS)); - } + put_asn(ctx.four_octet_asn, self.asn, out); out.put_u32(self.ip.into()); } } @@ -676,10 +634,13 @@ impl AggregatorPathAttribute { pub struct CommunitiesPathAttribute(pub Vec<(u16, u16)>); impl CommunitiesPathAttribute { + /// Reads all of `buf` as communities, four octets each. pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { - // At least one: an empty COMMUNITIES is malformed (RFC 7606 §7.8). - let (buf, values) = nom::multi::many1(|i| (be_u16, be_u16).parse(i)).parse(buf)?; - Ok((buf, CommunitiesPathAttribute(values))) + let values = records::<4>(buf, "COMMUNITIES")? + .iter() + .map(|[a, b, c, d]| (u16::from_be_bytes([*a, *b]), u16::from_be_bytes([*c, *d]))) + .collect(); + Ok((&buf[buf.len()..], CommunitiesPathAttribute(values))) } pub fn to_wire(&self, out: &mut BytesMut) { @@ -697,16 +658,11 @@ pub struct ExtendedCommunitiesPathAttribute { } impl ExtendedCommunitiesPathAttribute { + /// Reads all of `buf` as extended communities, eight octets each. pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { - let (communities, rest) = buf.as_chunks::<8>(); - if communities.is_empty() || !rest.is_empty() { - return Err(malformed(format!( - "{} octets are not a list of extended communities", - buf.len() - ))); - } + let communities = records::<8>(buf, "EXTENDED_COMMUNITIES")?; Ok(( - rest, + &buf[buf.len()..], Self { extended_communities: communities.iter().copied().map(ExtendedCommunity).collect(), }, @@ -743,9 +699,17 @@ pub struct LargeCommunitiesPathAttribute { } impl LargeCommunitiesPathAttribute { + /// Reads all of `buf` as large communities, twelve octets each. pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { - let (buf, communities) = nom::multi::many1(LargeCommunity::from_wire).parse(buf)?; - Ok((buf, Self { communities })) + let communities = records::<12>(buf, "LARGE_COMMUNITIES")? + .iter() + .map(|community| { + let (_, community) = + LargeCommunity::from_wire(community).expect("twelve octets are one"); + community + }) + .collect(); + Ok((&buf[buf.len()..], Self { communities })) } pub fn to_wire(&self, out: &mut BytesMut) { @@ -864,16 +828,9 @@ fn parse_nlri(version: IpVersion, with_path_id: bool, buf: &[u8]) -> Parsed<'_, pub(crate) fn parse_nlri_list( version: IpVersion, with_path_id: bool, - mut buf: &[u8], + buf: &[u8], ) -> Parsed<'_, Vec> { - // Not many0: that stops quietly at a prefix it cannot read. - let mut nlri = Vec::new(); - while !buf.is_empty() { - let (rest, parsed) = parse_nlri(version, with_path_id, buf)?; - nlri.push(parsed); - buf = rest; - } - Ok((buf, nlri)) + list(buf, |buf| parse_nlri(version, with_path_id, buf)) } /// The IP version of the prefixes that make up the NLRI of `family`. @@ -892,12 +849,6 @@ fn ip_version_of_nlri<'a>( IpVersion::try_from(family.afi).map_err(|_| unsupported()) } -fn parse_family(buf: &[u8]) -> Parsed<'_, AfiSafi> { - let (buf, afi) = be_u16(buf)?; - let (buf, safi) = be_u8(buf)?; - Ok((buf, AfiSafi::new(Afi(afi), Safi(safi)))) -} - /// MP_REACH_NLRI: routes of a family other than the one in the message /// body, with their next hop (RFC 4760 §3). #[derive(Debug, Clone, Serialize, PartialEq, Eq)] @@ -909,7 +860,7 @@ pub struct MpReachNlriPathAttribute { impl MpReachNlriPathAttribute { pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { - let (buf, family) = parse_family(buf)?; + let (buf, family) = parse_afi_safi(buf)?; let version = ip_version_of_nlri(family)?; let (buf, next_hop_len) = be_u8(buf)?; @@ -942,8 +893,7 @@ impl MpReachNlriPathAttribute { } pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) { - out.put_u16(self.family.afi.0); - out.put_u8(self.family.safi.0); + put_afi_safi(self.family, out); out.put_u8(self.next_hop.wire_len()); self.next_hop.to_wire(out); out.put_u8(0); @@ -961,15 +911,14 @@ pub struct MpUnreachNlriPathAttribute { impl MpUnreachNlriPathAttribute { pub fn from_wire<'a>(ctx: &ParserContext, buf: &'a [u8]) -> Parsed<'a, Self> { - let (buf, family) = parse_family(buf)?; + let (buf, family) = parse_afi_safi(buf)?; let version = ip_version_of_nlri(family)?; let (buf, prefixes) = parse_nlri_list(version, ctx.has_add_path(family), buf)?; Ok((buf, Self { family, prefixes })) } pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) { - out.put_u16(self.family.afi.0); - out.put_u8(self.family.safi.0); + put_afi_safi(self.family, out); write_nlri(&self.prefixes, ctx.has_add_path(self.family), out); } } diff --git a/crates/packet/src/lib.rs b/crates/packet/src/lib.rs index fda6bfd..b7fcafa 100644 --- a/crates/packet/src/lib.rs +++ b/crates/packet/src/lib.rs @@ -4,3 +4,4 @@ pub mod errors; pub mod message; pub mod open; pub mod update_decode; +mod wire; diff --git a/crates/packet/src/message.rs b/crates/packet/src/message.rs index d347a10..bd7ad02 100644 --- a/crates/packet/src/message.rs +++ b/crates/packet/src/message.rs @@ -15,6 +15,7 @@ use crate::errors::DecodeError; use crate::errors::MsgHeaderSubcode; use crate::errors::OpenMsgSubcode; use crate::open::OpenMessage; +use crate::wire::{fill_length, put_length}; /// A sentinel AS number used to denote that the actual AS number is provided as /// a 4 byte value in the capabilities instead. @@ -162,30 +163,27 @@ impl Message { pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { let start = out.len(); out.put_slice(&BGP_MARKER); - // The length, filled in once the body is written. + // The length, filled in once the body is written. It counts the + // header too. + let length_at = out.len(); out.put_u16(0); out.put_u8(self.type_code()); let written = self.body_to_wire(ctx, out).and_then(|()| { let length = out.len() - start; - u16::try_from(length) - .ok() - .filter(|length| *length <= BGP_MAX_MSG_LEN) - .ok_or(EncodeError::TooLong { - what: "BGP message", - len: length, - max: BGP_MAX_MSG_LEN.into(), - }) + put_length( + out, + length_at, + 2, + length, + BGP_MAX_MSG_LEN.into(), + "BGP message", + ) }); - match written { - Ok(length) => { - out[start + 16..start + 18].copy_from_slice(&length.to_be_bytes()); - Ok(()) - } - Err(e) => { - out.truncate(start); - Err(e) - } + if let Err(e) = written { + out.truncate(start); + return Err(e); } + Ok(()) } fn body_to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { @@ -337,15 +335,19 @@ impl UpdateMessage { // The prefixes in the body are IPv4 unicast (RFC 4271 §4.3). let with_path_id = ctx.has_add_path(AfiSafi::IPV4_UNICAST); - let withdrawn = reserve_length(out); + // Each field's two-octet length is filled in once the field is + // written. + let withdrawn = out.len(); + out.put_u16(0); write_nlri(&self.withdrawn_routes, with_path_id, out); - fill_length(out, withdrawn, "withdrawn routes")?; + fill_length(out, withdrawn, 2, u16::MAX.into(), "withdrawn routes")?; - let attributes = reserve_length(out); + let attributes = out.len(); + out.put_u16(0); for attribute in &self.path_attributes { attribute.to_wire(ctx, out)?; } - fill_length(out, attributes, "path attributes")?; + fill_length(out, attributes, 2, u16::MAX.into(), "path attributes")?; // The NLRI take up the rest of the message. write_nlri(&self.announced_routes, with_path_id, out); @@ -353,26 +355,6 @@ impl UpdateMessage { } } -/// Leaves room at the end of `out` for a two-octet length that is known -/// only once what it counts has been written, and returns where. -fn reserve_length(out: &mut BytesMut) -> usize { - let at = out.len(); - out.put_u16(0); - at -} - -/// Fills in the length reserved at `at`: everything written since. -fn fill_length(out: &mut BytesMut, at: usize, what: &'static str) -> Result<(), EncodeError> { - let length = out.len() - at - 2; - let field = u16::try_from(length).map_err(|_| EncodeError::TooLong { - what, - len: length, - max: u16::MAX.into(), - })?; - out[at..at + 2].copy_from_slice(&field.to_be_bytes()); - Ok(()) -} - /// Writes prefixes as an UPDATE names them: for each, its path identifier /// if the session uses them for the family (RFC 7911 §3), its length in /// bits, and the bytes that length covers. @@ -807,7 +789,6 @@ mod tests { } ); - // Rejects trailing garbage after the NLRI section (a partial prefix). /// With ADD-PATH in use for a family, each of its prefixes has a /// four-octet path identifier in front (RFC 7911 §3): in the body for /// IPv4 unicast, in MP_REACH_NLRI and MP_UNREACH_NLRI for the others. @@ -1067,6 +1048,7 @@ mod tests { assert_eq!(narrow.as4_path(), None); } + /// Rejects trailing garbage after the NLRI section (a partial prefix). #[test] fn test_update_rejects_trailing_garbage() { let ctx = default_v6_context(); diff --git a/crates/packet/src/open.rs b/crates/packet/src/open.rs index 327e695..4df38a2 100644 --- a/crates/packet/src/open.rs +++ b/crates/packet/src/open.rs @@ -4,7 +4,7 @@ use bitfield::bitfield; use bytes::{BufMut, BytesMut}; use nom::{ Err::Failure, - IResult, Parser, + Parser, number::complete::{be_u8, be_u16, be_u24, be_u32}, }; use serde::Serialize; @@ -16,26 +16,8 @@ use bgp_primitives::{ parser::{BgpParserError, EncodeError, ProtocolErrorValues}, }; -type Parsed<'a, T> = IResult<&'a [u8], T, BgpParserError<&'a [u8]>>; - -macro_rules! fail { - ( $($arg:expr),* $(,)? ) => { - Failure(BgpParserError::Malformed(format!($($arg),*))) - }; -} - -/// Parses `buf` to its end as a list of `item`s. Unlike nom's `many0`, -/// which stops quietly at an item it cannot read, an error in an item is an -/// error of the list. -fn list<'a, T>(mut buf: &'a [u8], item: impl Fn(&'a [u8]) -> Parsed<'a, T>) -> Parsed<'a, Vec> { - let mut items = Vec::new(); - while !buf.is_empty() { - let (rest, parsed) = item(buf)?; - items.push(parsed); - buf = rest; - } - Ok((buf, items)) -} +use crate::message::BGP4_VERSION; +use crate::wire::{Parsed, fill_length, list, malformed, parse_afi_safi, put_afi_safi, whole}; /// The optional parameter type that marks an OPEN whose optional parameters /// have two-octet lengths (RFC 9072). @@ -62,7 +44,7 @@ impl OpenMessage { /// in a well-formed OPEN is nothing. pub fn from_wire(buf: &[u8]) -> Parsed<'_, Self> { let (buf, version) = be_u8(buf)?; - if version != 4 { + if version != BGP4_VERSION { return Err(Failure(BgpParserError::ProtocolError( ProtocolErrorValues::UnsupportedVersion(version), ))); @@ -316,12 +298,6 @@ fn afi_reserved_safi(buf: &[u8]) -> Parsed<'_, (Afi, Safi)> { Ok((buf, (Afi(afi), Safi(safi)))) } -fn afi_safi(buf: &[u8]) -> Parsed<'_, (Afi, Safi)> { - let (buf, afi) = be_u16(buf)?; - let (buf, safi) = be_u8(buf)?; - Ok((buf, (Afi(afi), Safi(safi)))) -} - impl Capability { fn type_code(&self) -> u8 { use cap_type::*; @@ -387,7 +363,7 @@ impl Capability { // else: no SAFI is larger than 255. let safi = u8::try_from(safi) .map(Safi) - .map_err(|_| fail!("SAFI {} does not fit in one octet", safi))?; + .map_err(|_| malformed(format!("SAFI {safi} does not fit in one octet")))?; Ok(( buf, ExtendedNextHop { @@ -398,7 +374,7 @@ impl Capability { )) })?; if entries.is_empty() { - return Err(fail!("Extended Next Hop capability with no entries")); + return Err(malformed("Extended Next Hop capability with no entries")); } (rest, Self::ExtendedNextHop { entries }) } @@ -416,12 +392,12 @@ impl Capability { } MULTI_LABEL => { let (rest, entries) = list(value, |buf| { - let (buf, (afi, safi)) = afi_safi(buf)?; + let (buf, AfiSafi { afi, safi }) = parse_afi_safi(buf)?; let (buf, count) = be_u8(buf)?; Ok((buf, MultiLabelEntry { afi, safi, count })) })?; if entries.is_empty() { - return Err(fail!("Multiple Labels capability with no entries")); + return Err(malformed("Multiple Labels capability with no entries")); } (rest, Self::MultiLabelCompat { entries }) } @@ -438,7 +414,7 @@ impl Capability { // Four bits of flags and twelve of restart time. let (rest, flags_restart_time) = be_u16(value)?; let (rest, entries) = list(rest, |buf| { - let (buf, (afi, safi)) = afi_safi(buf)?; + let (buf, AfiSafi { afi, safi }) = parse_afi_safi(buf)?; let (buf, flags) = be_u8(buf)?; Ok(( buf, @@ -485,7 +461,7 @@ impl Capability { ENHANCED_ROUTE_REFRESH => (value, Self::EnhancedRouteRefresh), LONG_LIVED_GRACEFUL_RESTART => { let (rest, entries) = list(value, |buf| { - let (buf, (afi, safi)) = afi_safi(buf)?; + let (buf, AfiSafi { afi, safi }) = parse_afi_safi(buf)?; let (buf, flags) = be_u8(buf)?; let (buf, stale_time) = be_u24(buf)?; Ok(( @@ -508,13 +484,7 @@ impl Capability { }, ), }; - if !rest.is_empty() { - return Err(fail!( - "{} octets left over in capability {type_code}", - rest.len() - )); - } - Ok(capability) + whole(format_args!("capability {type_code}"), (rest, capability)) } /// Writes the capability, with its type and length, to the end of @@ -525,13 +495,7 @@ impl Capability { let length_at = out.len(); out.put_u8(0); self.value_to_wire(out)?; - let length = out.len() - length_at - 1; - out[length_at] = u8::try_from(length).map_err(|_| EncodeError::TooLong { - what: "capability", - len: length, - max: u8::MAX.into(), - })?; - Ok(()) + fill_length(out, length_at, 1, u8::MAX.into(), "capability") } fn value_to_wire(&self, out: &mut BytesMut) -> Result<(), EncodeError> { @@ -562,8 +526,7 @@ impl Capability { } Capability::MultiLabelCompat { entries } => { for entry in entries { - out.put_u16(entry.afi.0); - out.put_u8(entry.safi.0); + put_afi_safi(AfiSafi::new(entry.afi, entry.safi), out); out.put_u8(entry.count); } } @@ -585,23 +548,20 @@ impl Capability { u16::from(*restart_state) << 15 | u16::from(*notification) << 14 | restart_time, ); for entry in entries { - out.put_u16(entry.afi.0); - out.put_u8(entry.safi.0); + put_afi_safi(AfiSafi::new(entry.afi, entry.safi), out); out.put_u8(u8::from(entry.preserve_forwarding) << 7); } } Capability::FourByteAsn { asn } => out.put_u32(*asn), Capability::AddPath { entries } => { for entry in entries { - out.put_u16(entry.afi.0); - out.put_u8(entry.safi.0); + put_afi_safi(entry.family(), out); out.put_u8(entry.send_recv as u8); } } Capability::LongLivedGracefulRestart(entries) => { for entry in entries { - out.put_u16(entry.afi.0); - out.put_u8(entry.safi.0); + put_afi_safi(AfiSafi::new(entry.afi, entry.safi), out); out.put_u8(entry.flags); out.put_slice(&entry.stale_time.to_be_bytes()[1..]); } @@ -630,7 +590,7 @@ impl OrfFamily { let (rest, orf_type) = be_u8(buf)?; let (rest, send_recv) = be_u8(rest)?; let send_recv = OrfSendRecv::try_from(send_recv) - .map_err(|e| fail!("Failed to parse ORF send/recv: {}", e))?; + .map_err(|e| malformed(format!("Failed to parse ORF send/recv: {e}")))?; orfs.push(OutboundRouteFilterEntry { r#type: OrfType(orf_type), send_recv, @@ -843,7 +803,7 @@ mod tests { } test_capability_roundtrip!( - test_multiptotocol, + test_multiprotocol, &[0x01, 0x04, 0x00, 0x01, 0x00, 0x01], Capability::MultiProtocol { afi: Afi::IPV4, diff --git a/crates/packet/src/update_decode.rs b/crates/packet/src/update_decode.rs index 317f55b..61d83bf 100644 --- a/crates/packet/src/update_decode.rs +++ b/crates/packet/src/update_decode.rs @@ -201,8 +201,9 @@ pub fn merge_as4_path(attributes: &mut Vec) { else { return; }; - let PathAttribute::As4Path(wide) = attributes.remove(at) else { - return; + let wide = match attributes.remove(at) { + PathAttribute::As4Path(wide) => wide, + other => unreachable!("{other:?} is not the AS4_PATH found at {at}"), }; for attribute in attributes { if let PathAttribute::AsPath(path) = attribute { @@ -501,12 +502,6 @@ mod tests { OriginPathAttribute, }; - const MED: u8 = 4; - const COMMUNITIES: u8 = 8; - const EXTENDED_COMMUNITIES: u8 = 16; - const LARGE_COMMUNITY: u8 = 32; - const ONLY_TO_CUSTOMER: u8 = 35; - const WELL_KNOWN: u8 = 0x40; const OPTIONAL: u8 = 0x80; const OPTIONAL_TRANSITIVE: u8 = 0xc0; @@ -602,7 +597,7 @@ mod tests { #[test] fn a_well_formed_update_is_left_alone() { - let decoded = announce_with(&ebgp(), attr(OPTIONAL, MED, &[0, 0, 0, 50])); + let decoded = announce_with(&ebgp(), attr(OPTIONAL, MULTI_EXIT_DISC, &[0, 0, 0, 50])); assert!(decoded.errors.is_empty()); assert_eq!(decoded.update.announced_routes, [route()]); assert_eq!( @@ -632,7 +627,7 @@ mod tests { // Nothing when the routes were withdrawn instead: there is no route // for the attributes to belong to. - let decoded = announce_with(&ebgp(), attr(OPTIONAL, MED, &[1])); + let decoded = announce_with(&ebgp(), attr(OPTIONAL, MULTI_EXIT_DISC, &[1])); assert_eq!(decoded.attributes_wire, None); let decoded = UpdateMessage::decode(&ebgp(), &body(&NLRI, &[], &[])).unwrap(); assert_eq!(decoded.attributes_wire, None); @@ -691,7 +686,11 @@ mod tests { #[test] fn a_malformed_attribute_withdraws_the_routes_and_keeps_the_session() { let cases: Vec<(&str, u8, Vec)> = vec![ - ("MED of three octets", MED, attr(OPTIONAL, MED, &[0, 0, 50])), + ( + "MED of three octets", + MULTI_EXIT_DISC, + attr(OPTIONAL, MULTI_EXIT_DISC, &[0, 0, 50]), + ), ( "COMMUNITIES with nothing in it", COMMUNITIES, @@ -709,13 +708,13 @@ mod tests { ), ( "LARGE_COMMUNITY of eleven octets", - LARGE_COMMUNITY, - attr(OPTIONAL_TRANSITIVE, LARGE_COMMUNITY, &[0; 11]), + LARGE_COMMUNITIES, + attr(OPTIONAL_TRANSITIVE, LARGE_COMMUNITIES, &[0; 11]), ), ( "MED marked transitive", - MED, - attr(OPTIONAL_TRANSITIVE, MED, &[0, 0, 0, 50]), + MULTI_EXIT_DISC, + attr(OPTIONAL_TRANSITIVE, MULTI_EXIT_DISC, &[0, 0, 0, 50]), ), // RFC 9234 §5. ( @@ -803,13 +802,13 @@ mod tests { fn an_attribute_that_overruns_the_attributes_withdraws_what_was_found() { // A MED that claims 200 octets. let mut attributes = mandatory(); - attributes.push(vec![OPTIONAL, MED, 200, 0, 0]); + attributes.push(vec![OPTIONAL, MULTI_EXIT_DISC, 200, 0, 0]); let decoded = UpdateMessage::decode(&ebgp(), &body(&[], &attributes, &NLRI)).unwrap(); - assert_withdrawn(&decoded, Some(MED), "overrun"); + assert_withdrawn(&decoded, Some(MULTI_EXIT_DISC), "overrun"); // Two stray octets after the last attribute: not even a header. let mut attributes = mandatory(); - attributes.push(vec![OPTIONAL, MED]); + attributes.push(vec![OPTIONAL, MULTI_EXIT_DISC]); let decoded = UpdateMessage::decode(&ebgp(), &body(&[], &attributes, &NLRI)).unwrap(); assert_withdrawn(&decoded, None, "stray octets"); } @@ -860,10 +859,13 @@ mod tests { #[test] fn a_repeated_attribute_keeps_its_first_value() { let mut attributes = mandatory(); - attributes.push(attr(OPTIONAL, MED, &[0, 0, 0, 50])); - attributes.push(attr(OPTIONAL, MED, &[0, 0, 0, 99])); + attributes.push(attr(OPTIONAL, MULTI_EXIT_DISC, &[0, 0, 0, 50])); + attributes.push(attr(OPTIONAL, MULTI_EXIT_DISC, &[0, 0, 0, 99])); let decoded = UpdateMessage::decode(&ebgp(), &body(&[], &attributes, &NLRI)).unwrap(); - assert_eq!(handled(&decoded), [(Handling::AttributeDiscard, Some(MED))]); + assert_eq!( + handled(&decoded), + [(Handling::AttributeDiscard, Some(MULTI_EXIT_DISC))] + ); assert!( decoded .update @@ -972,6 +974,56 @@ mod tests { assert_eq!(decoded.update.announced_routes, [route()]); } + /// AS4_AGGREGATOR (RFC 6793 §4.2.2) is kept as it came, once its + /// length is known to be the four octets of an AS number and the four + /// of an address. One of another length is dropped and the route kept + /// (RFC 6793 §6), like a malformed AGGREGATOR. + #[test] + fn as4_aggregator_is_kept_opaque_and_dropped_if_malformed() { + let ctx = ParserContext { + four_octet_asn: false, + ibgp: false, + add_path: Vec::new(), + }; + let attributes = |as4_aggregator: Vec| { + vec![ + origin(), + attr(WELL_KNOWN, AS_PATH, &[2, 1, 0x5b, 0xa0]), + next_hop(), + as4_aggregator, + ] + }; + // AS 4200000001, 192.0.2.9. + let value = [0xfa, 0x56, 0xea, 0x01, 192, 0, 2, 9]; + let good = attr(OPTIONAL_TRANSITIVE, AS4_AGGREGATOR, &value); + let decoded = UpdateMessage::decode(&ctx, &body(&[], &attributes(good), &NLRI)).unwrap(); + assert!(decoded.errors.is_empty()); + assert_eq!( + decoded.update.path_attributes.last(), + Some(&PathAttribute::UnknownPathAttribute { + flags: PathAttributeFlags(OPTIONAL_TRANSITIVE), + type_code: AS4_AGGREGATOR, + payload: value.to_vec(), + }) + ); + + for (case, bad) in [ + ("two-octet AS number", &value[2..]), + ("no address", &value[..4]), + ("nothing", &[][..]), + ] { + let bad = attr(OPTIONAL_TRANSITIVE, AS4_AGGREGATOR, bad); + let decoded = UpdateMessage::decode(&ctx, &body(&[], &attributes(bad), &NLRI)).unwrap(); + assert_eq!( + handled(&decoded), + [(Handling::AttributeDiscard, Some(AS4_AGGREGATOR))], + "{case}" + ); + assert_eq!(decoded.update.announced_routes, [route()], "{case}"); + assert_eq!(decoded.update.path_attributes.len(), 3, "{case}"); + } + } + /// RFC 6793 §4.2.3. AS4_PATH is the tail of the path; the front is /// kept from AS_PATH. One longer than the path is not used, and is /// gone all the same: nothing after the decoder is to see two paths, diff --git a/crates/packet/src/wire.rs b/crates/packet/src/wire.rs new file mode 100644 index 0000000..ddc9d20 --- /dev/null +++ b/crates/packet/src/wire.rs @@ -0,0 +1,189 @@ +//! What the readers and writers of the wire format share: the shape of a +//! parser's result and its errors, the fields that several messages and +//! attributes have in common, and the lengths that are written before what +//! they count. + +use bytes::{BufMut, BytesMut}; +use nom::Err::Failure; +use nom::IResult; +use nom::number::complete::{be_u8, be_u16, be_u32}; + +use bgp_primitives::constants::{Afi, AfiSafi, Safi}; +use bgp_primitives::parser::{BgpParserError, EncodeError}; + +use crate::message::AS_TRANS; + +/// A parser's result: what is left of the input, and what was read. +pub(crate) type Parsed<'a, T> = IResult<&'a [u8], T, BgpParserError<&'a [u8]>>; + +/// The error for input that does not follow the format; `detail` says how. +pub(crate) fn malformed<'a>(detail: impl Into) -> nom::Err> { + Failure(BgpParserError::Malformed(detail.into())) +} + +/// Parses `buf` to its end as a list of `item`s. Unlike nom's `many0`, +/// which stops quietly at an item it cannot read, an error in an item is an +/// error of the list. +pub(crate) fn list<'a, T>( + mut buf: &'a [u8], + item: impl Fn(&'a [u8]) -> Parsed<'a, T>, +) -> Parsed<'a, Vec> { + let mut items = Vec::new(); + while !buf.is_empty() { + let (rest, parsed) = item(buf)?; + items.push(parsed); + buf = rest; + } + Ok((buf, items)) +} + +/// A parsed value, provided the parser used all of the input. `what` names +/// the input in the error. +pub(crate) fn whole( + what: impl std::fmt::Display, + (rest, parsed): (&[u8], T), +) -> Result>> { + if rest.is_empty() { + Ok(parsed) + } else { + Err(malformed(format!( + "{} octets left over in {what}", + rest.len() + ))) + } +} + +/// `value` as a list of one or more records of `N` octets each: how the +/// community attributes and CLUSTER_LIST are laid out. An empty list is +/// malformed (RFC 7606 §7.8 for COMMUNITIES, and the others alike). `what` +/// names the attribute in the error. +pub(crate) fn records<'a, const N: usize>( + value: &'a [u8], + what: &str, +) -> Result<&'a [[u8; N]], nom::Err>> { + let (records, rest) = value.as_chunks::(); + if records.is_empty() || !rest.is_empty() { + return Err(malformed(format!( + "{what} is {} octets long, not a non-empty multiple of {N}", + value.len() + ))); + } + Ok(records) +} + +/// Parses an AS number as wide as the session's: four octets or two +/// (RFC 6793). +pub(crate) fn parse_asn(four_octet: bool, buf: &[u8]) -> Parsed<'_, u32> { + if four_octet { + be_u32(buf) + } else { + be_u16(buf).map(|(buf, asn)| (buf, u32::from(asn))) + } +} + +/// Writes an AS number as wide as the session's. On a two-octet session +/// one that does not fit is written as AS_TRANS (RFC 6793 §4.2.2). +pub(crate) fn put_asn(four_octet: bool, asn: u32, out: &mut BytesMut) { + if four_octet { + out.put_u32(asn); + } else { + out.put_u16(u16::try_from(asn).unwrap_or(AS_TRANS)); + } +} + +/// Parses an address family as the multiprotocol attributes and most +/// capabilities name it: a two-octet AFI and a one-octet SAFI. +pub(crate) fn parse_afi_safi(buf: &[u8]) -> Parsed<'_, AfiSafi> { + let (buf, afi) = be_u16(buf)?; + let (buf, safi) = be_u8(buf)?; + Ok((buf, AfiSafi::new(Afi(afi), Safi(safi)))) +} + +/// Writes an address family as [`parse_afi_safi`] reads it. +pub(crate) fn put_afi_safi(family: AfiSafi, out: &mut BytesMut) { + out.put_u16(family.afi.0); + out.put_u8(family.safi.0); +} + +/// Writes `length` into the field of `width` octets reserved at `at`, if +/// it is no more than `max`. `what` names the field in the error. +pub(crate) fn put_length( + out: &mut BytesMut, + at: usize, + width: usize, + length: usize, + max: usize, + what: &'static str, +) -> Result<(), EncodeError> { + if length > max { + return Err(EncodeError::TooLong { + what, + len: length, + max, + }); + } + let bytes = length.to_be_bytes(); + out[at..at + width].copy_from_slice(&bytes[bytes.len() - width..]); + Ok(()) +} + +/// Fills in the field of `width` octets reserved at `at` with how much +/// has been written after it, if that is no more than `max`: for a length +/// that is known only once what it counts has been written. `what` names +/// the field in the error. +pub(crate) fn fill_length( + out: &mut BytesMut, + at: usize, + width: usize, + max: usize, + what: &'static str, +) -> Result<(), EncodeError> { + let length = out.len() - at - width; + put_length(out, at, width, length, max, what) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_length_is_filled_in_after_what_it_counts() { + let mut out = BytesMut::new(); + out.put_u8(0xaa); + let at = out.len(); + out.put_u16(0); + out.put_slice(&[1, 2, 3]); + fill_length(&mut out, at, 2, u16::MAX.into(), "field").unwrap(); + assert_eq!(&out[..], [0xaa, 0, 3, 1, 2, 3]); + + let mut out = BytesMut::new(); + out.put_u8(0); + out.put_slice(&[0; 300]); + assert_eq!( + fill_length(&mut out, 0, 1, u8::MAX.into(), "field"), + Err(EncodeError::TooLong { + what: "field", + len: 300, + max: 255, + }) + ); + } + + #[test] + fn records_are_a_whole_number_of_them_and_at_least_one() { + assert_eq!( + records::<4>(&[1, 2, 3, 4, 5, 6, 7, 8], "X").unwrap().len(), + 2 + ); + for value in [&[][..], &[1, 2, 3][..], &[1, 2, 3, 4, 5][..]] { + let error = records::<4>(value, "X").unwrap_err(); + assert_eq!( + crate::message::describe(&error), + format!( + "X is {} octets long, not a non-empty multiple of 4", + value.len() + ) + ); + } + } +} diff --git a/crates/packet/tests/properties.rs b/crates/packet/tests/properties.rs index 9fd3d72..4f4528b 100644 --- a/crates/packet/tests/properties.rs +++ b/crates/packet/tests/properties.rs @@ -137,19 +137,6 @@ fn extended_community() -> impl Strategy { fn mp_reach(session: Session) -> impl Strategy { let ip_safi = || prop_oneof![Just(Safi::UNICAST), Just(Safi::MULTICAST)]; - let v4 = ip_safi() - .prop_flat_map(move |safi| { - ( - Just(safi), - any::<[u8; 4]>(), - nlri(session, IpVersion::Ipv4, safi), - ) - }) - .prop_map(|(safi, next_hop, prefixes)| MpReachNlriPathAttribute { - family: AfiSafi::new(Afi::IPV4, safi), - next_hop: NlriNextHop::Ipv4(Ipv4Addr::from(next_hop)), - prefixes, - }); let v6_next_hop = || { prop_oneof![ any::<[u8; 16]>().prop_map(|a| NlriNextHop::Ipv6(Ipv6Addr::from(a))), @@ -161,6 +148,27 @@ fn mp_reach(session: Session) -> impl Strategy }), ] }; + // IPv4 routes have an IPv4 next hop, or an IPv6 one over a link that + // has only IPv6 addresses (RFC 8950). + let v4_next_hop = move || { + prop_oneof![ + 3 => any::<[u8; 4]>().prop_map(|a| NlriNextHop::Ipv4(Ipv4Addr::from(a))), + 1 => v6_next_hop(), + ] + }; + let v4 = ip_safi() + .prop_flat_map(move |safi| { + ( + Just(safi), + v4_next_hop(), + nlri(session, IpVersion::Ipv4, safi), + ) + }) + .prop_map(|(safi, next_hop, prefixes)| MpReachNlriPathAttribute { + family: AfiSafi::new(Afi::IPV4, safi), + next_hop, + prefixes, + }); let v6 = ip_safi() .prop_flat_map(move |safi| { ( @@ -692,7 +700,9 @@ proptest! { check_decoding(&ctx, &damaged(&wire, &damage))?; } - /// The parsers below the message level are public too. + /// The parsers below the message level are public too. What they + /// accept must write back as it was read, where the format leaves + /// nothing for the reader to normalise. #[test] fn noise_is_never_more_than_an_error_for_the_parts( session in session(), @@ -700,8 +710,52 @@ proptest! { ) { let ctx = session.ctx(); let _ = OpenMessage::from_wire(&bytes); - let _ = Capability::from_wire(&bytes); let _ = UpdateMessage::decode(&ctx, &bytes); - let _ = PathAttribute::from_wire(&ctx, &bytes); + + if let Ok((rest, capability)) = Capability::from_wire(&bytes) { + let consumed = &bytes[..bytes.len() - rest.len()]; + let mut rewritten = BytesMut::new(); + capability.to_wire(&mut rewritten) + .map_err(|e| TestCaseError::fail(format!("accepted but cannot be written: {e}")))?; + // ADD-PATH families with a Send/Receive value nobody defined + // are left out (RFC 7911 §4), and the reserved octet of + // Multiprotocol and ORF and the reserved bits of Graceful + // Restart are read as zero whatever they were. Everything else + // is written as it was read. + let normalised = matches!( + capability, + Capability::AddPath { .. } + | Capability::MultiProtocol { .. } + | Capability::OutboundRouteFilter(_) + | Capability::GracefulRestart { .. } + ); + if !normalised { + prop_assert_eq!(&rewritten[..], consumed, "{:?}", capability); + } + let again = Capability::from_wire(&rewritten) + .map_err(|e| TestCaseError::fail(format!("written but cannot be read: {e:?}")))?; + prop_assert_eq!(again, (&[][..], capability)); + } + + if let Ok((rest, attribute)) = PathAttribute::from_wire(&ctx, &bytes) { + let consumed = &bytes[..bytes.len() - rest.len()]; + let mut rewritten = BytesMut::new(); + attribute.to_wire(&ctx, &mut rewritten) + .map_err(|e| TestCaseError::fail(format!("accepted but cannot be written: {e}")))?; + // The flags are not checked against the attribute's definition + // here, and are written as defined; and the bits of a prefix + // past its length are cleared. With the flags as defined and + // no prefixes, the attribute is written as it was read. + let names_prefixes = matches!( + attribute, + PathAttribute::MpReachNlri(_) | PathAttribute::MpUnreachNlri(_) + ); + if rewritten[0] == consumed[0] && !names_prefixes { + prop_assert_eq!(&rewritten[..], consumed, "{:?}", attribute); + } + let again = PathAttribute::from_wire(&ctx, &rewritten) + .map_err(|e| TestCaseError::fail(format!("written but cannot be read: {e:?}")))?; + prop_assert_eq!(again, (&[][..], attribute)); + } } } diff --git a/crates/primitives/src/ip_prefix.rs b/crates/primitives/src/ip_prefix.rs index 93b28ef..5b8ae05 100644 --- a/crates/primitives/src/ip_prefix.rs +++ b/crates/primitives/src/ip_prefix.rs @@ -64,7 +64,7 @@ impl IpPrefix { let mut address = [0u8; 16]; address[..needed].copy_from_slice(&prefix[..needed]); if let (Some(last), rem @ 1..) = (address[..needed].last_mut(), length % 8) { - *last &= u8::MAX << (8 - rem); + *last &= high_bits_mask(rem); } Ok(Self { address_family, @@ -98,7 +98,7 @@ impl IpPrefix { let whole = usize::from(length / 8); address[..whole].copy_from_slice(&self.address[..whole]); if !length.is_multiple_of(8) { - address[whole] = self.address[whole] & (u8::MAX << (8 - length % 8)); + address[whole] = self.address[whole] & high_bits_mask(length % 8); } Self { address_family: self.address_family, @@ -122,11 +122,18 @@ impl IpPrefix { if rem_bits == 0 { return true; } - let mask = u8::MAX << (8 - rem_bits); + let mask = high_bits_mask(rem_bits); (self.address[full_bytes] & mask) == (other.address[full_bytes] & mask) } } +/// The mask that keeps the first `bits` bits of an octet and clears the +/// rest, for `bits` from 1 to 7: what the last octet of a prefix whose +/// length is not a whole number of octets is masked with. +fn high_bits_mask(bits: u8) -> u8 { + u8::MAX << (8 - bits) +} + /// A prefix as an UPDATE names it: the prefix and, on a session where /// ADD-PATH is in use for the address family (RFC 7911), the identifier /// that tells apart the several routes a peer may announce for it. @@ -144,11 +151,6 @@ impl Nlri { pub fn plain(prefixes: impl IntoIterator) -> Vec { prefixes.into_iter().map(Self::from).collect() } - - /// The bytes this takes on the wire, with or without its identifier. - pub fn wire_len(&self, with_path_id: bool) -> usize { - 1 + self.prefix.bytes().len() + if with_path_id { 4 } else { 0 } - } } impl From for Nlri {