From 3d52cd3168d0e8239c9b614e08b3322be9d32945 Mon Sep 17 00:00:00 2001 From: MorganaFuture Date: Fri, 29 May 2026 12:36:06 +0300 Subject: [PATCH] Implement RFC 7607 handling for AS 0 AS 0 is reserved and must never appear on the wire. Codify the restriction on rdb::Asn (is_reserved) and enforce it at the BGP boundaries: - reject a peer OPEN whose My Autonomous System is 0 with an OPEN Message Error / Bad Peer AS notification; - treat AS 0 in AS_PATH/AS4_PATH/AGGREGATOR/AS4_AGGREGATOR as a malformed UPDATE, routed through the existing RFC 7606 handling (treat-as-withdraw for the paths, attribute-discard for the aggregators); - refuse to configure a local ASN of 0, and skip any such router persisted by an earlier version at startup. Fixes: #706 --- bgp/src/error.rs | 3 + bgp/src/messages.rs | 184 +++++++++++++++++++ bgp/src/proptest.rs | 3 +- bgp/src/session.rs | 9 + mg-api-types/versions/src/impls/bgp/parse.rs | 9 + mgd/src/bgp_admin.rs | 7 + mgd/src/main.rs | 8 + rdb/src/types.rs | 16 ++ 8 files changed, 238 insertions(+), 1 deletion(-) diff --git a/bgp/src/error.rs b/bgp/src/error.rs index b2cdf221..ce9d4de1 100644 --- a/bgp/src/error.rs +++ b/bgp/src/error.rs @@ -169,6 +169,9 @@ pub enum Error { #[error("Unexpected ASN: {0}")] UnexpectedAsn(ExpectationMismatch), + #[error("Peer used reserved AS 0 (RFC 7607)")] + ReservedPeerAsn, + #[error("Hold time too small")] HoldTimeTooSmall, diff --git a/bgp/src/messages.rs b/bgp/src/messages.rs index c344b613..8503b148 100644 --- a/bgp/src/messages.rs +++ b/bgp/src/messages.rs @@ -11,6 +11,7 @@ use nom::{ number::complete::{be_u8, be_u16, be_u32, u8 as parse_u8}, }; use path_attribute_flags::*; +use rdb::Asn; use std::{ collections::{BTreeSet, HashSet}, fmt::{Display, Formatter}, @@ -1373,6 +1374,11 @@ pub fn path_attribute_value_from_wire( segments.push(seg); input = out; } + if as_path_contains_reserved_asn(&segments) { + return Err(UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::AsPath, + }); + } Ok(PathAttributeValue::As4Path(segments)) } PathAttributeTypeCode::NextHop => { @@ -1425,6 +1431,11 @@ pub fn path_attribute_value_from_wire( segments.push(seg); input = out; } + if as_path_contains_reserved_asn(&segments) { + return Err(UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::As4Path, + }); + } Ok(PathAttributeValue::As4Path(segments)) } PathAttributeTypeCode::Communities => { @@ -1506,6 +1517,11 @@ pub fn path_attribute_value_from_wire( detail: e, } })?; + if Asn::from(agg.asn).is_reserved() { + return Err(UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::Aggregator, + }); + } Ok(PathAttributeValue::Aggregator(agg)) } PathAttributeTypeCode::As4Aggregator => { @@ -1524,6 +1540,11 @@ pub fn path_attribute_value_from_wire( detail: e, } })?; + if Asn::from(agg.asn).is_reserved() { + return Err(UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::As4Aggregator, + }); + } Ok(PathAttributeValue::As4Aggregator(agg)) } } @@ -1555,6 +1576,14 @@ pub fn as4_path_segment_to_wire( Ok(buf) } +/// RFC 7607: AS 0 is reserved and MUST NOT appear in an AS path attribute. +fn as_path_contains_reserved_asn(segments: &[As4PathSegment]) -> bool { + segments + .iter() + .flat_map(|seg| seg.value.iter()) + .any(|asn| Asn::from(*asn).is_reserved()) +} + pub fn as4_path_segment_from_wire( input: &[u8], ) -> Result<(&[u8], As4PathSegment), Error> { @@ -3087,6 +3116,161 @@ mod tests { assert!(!msg.nlri.is_empty(), "Expected NLRI to be present"); } + // ========================================================================= + // RFC 7607 (AS 0) tests + // ========================================================================= + + /// Append a path-attributes length and NLRI to a partial UPDATE buffer, + /// patching the length field at `len_offset`. + fn finish_update(buf: &mut Vec, len_offset: usize, attrs_start: usize) { + let attrs_len = (buf.len() - attrs_start) as u16; + buf[len_offset..len_offset + 2] + .copy_from_slice(&attrs_len.to_be_bytes()); + // NLRI: 198.51.100.0/24 + buf.push(24); + buf.extend_from_slice(&[198, 51, 100]); + } + + /// RFC 7607: AS 0 in AS_PATH makes the UPDATE malformed (treat-as-withdraw). + #[test] + fn as_path_reserved_as_zero_treated_as_withdraw() { + let mut buf = Vec::new(); + buf.extend_from_slice(&0u16.to_be_bytes()); // withdrawn routes length + let len_offset = buf.len(); + buf.extend_from_slice(&0u16.to_be_bytes()); // path attrs length + let attrs_start = buf.len(); + + // ORIGIN (IGP) + buf.extend_from_slice(&[0x40, 1, 1, 0]); + // AS_PATH: AS_SEQUENCE with a single AS of 0 + buf.extend_from_slice(&[0x40, 2, 6, 2, 1]); + buf.extend_from_slice(&0u32.to_be_bytes()); + // NEXT_HOP + buf.extend_from_slice(&[0x40, 3, 4, 192, 0, 2, 1]); + + finish_update(&mut buf, len_offset, attrs_start); + + let msg = update_message_from_wire(&buf) + .expect("parse should succeed with treat-as-withdraw"); + assert!( + treat_as_withdraw(&msg.errors), + "AS 0 in AS_PATH should be treat-as-withdraw" + ); + assert!( + msg.errors.iter().any(|(reason, action)| matches!( + reason, + UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::AsPath + } + ) && matches!( + action, + AttributeAction::TreatAsWithdraw + )), + "expected ReservedAsZero(AsPath) treat-as-withdraw, got {:?}", + msg.errors + ); + } + + /// RFC 7607: AS 0 in AGGREGATOR makes the UPDATE malformed; per RFC 7606 + /// the AGGREGATOR is an informational attribute, so it is discarded while + /// the rest of the UPDATE is retained. + #[test] + fn aggregator_reserved_as_zero_discarded() { + let mut buf = Vec::new(); + buf.extend_from_slice(&0u16.to_be_bytes()); + let len_offset = buf.len(); + buf.extend_from_slice(&0u16.to_be_bytes()); + let attrs_start = buf.len(); + + // ORIGIN, AS_PATH (AS 65000), NEXT_HOP - all valid + buf.extend_from_slice(&[0x40, 1, 1, 0]); + buf.extend_from_slice(&[0x40, 2, 6, 2, 1]); + buf.extend_from_slice(&65000u32.to_be_bytes()); + buf.extend_from_slice(&[0x40, 3, 4, 192, 0, 2, 1]); + // AGGREGATOR: 2-byte AS of 0 + IPv4 + buf.extend_from_slice(&[0xc0, 7, 6, 0, 0, 192, 0, 2, 1]); + + finish_update(&mut buf, len_offset, attrs_start); + + let msg = update_message_from_wire(&buf).expect("parse should succeed"); + assert!( + !treat_as_withdraw(&msg.errors), + "AGGREGATOR error must not withdraw the route" + ); + assert!( + msg.errors.iter().any(|(reason, action)| matches!( + reason, + UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::Aggregator + } + ) && matches!( + action, + AttributeAction::Discard + )), + "expected ReservedAsZero(Aggregator) discard, got {:?}", + msg.errors + ); + assert!( + !msg.path_attributes.iter().any(|pa| matches!( + pa.value, + PathAttributeValue::Aggregator(_) + )), + "AGGREGATOR with AS 0 should be discarded" + ); + } + + /// RFC 7607: AS 0 in AS4_AGGREGATOR is handled the same as AGGREGATOR. + #[test] + fn as4_aggregator_reserved_as_zero_discarded() { + let mut buf = Vec::new(); + buf.extend_from_slice(&0u16.to_be_bytes()); + let len_offset = buf.len(); + buf.extend_from_slice(&0u16.to_be_bytes()); + let attrs_start = buf.len(); + + buf.extend_from_slice(&[0x40, 1, 1, 0]); + buf.extend_from_slice(&[0x40, 2, 6, 2, 1]); + buf.extend_from_slice(&65000u32.to_be_bytes()); + buf.extend_from_slice(&[0x40, 3, 4, 192, 0, 2, 1]); + // AS4_AGGREGATOR: 4-byte AS of 0 + IPv4 + buf.extend_from_slice(&[0xc0, 18, 8]); + buf.extend_from_slice(&0u32.to_be_bytes()); + buf.extend_from_slice(&[192, 0, 2, 1]); + + finish_update(&mut buf, len_offset, attrs_start); + + let msg = update_message_from_wire(&buf).expect("parse should succeed"); + assert!(!treat_as_withdraw(&msg.errors)); + assert!( + msg.errors.iter().any(|(reason, action)| matches!( + reason, + UpdateParseErrorReason::ReservedAsZero { + type_code: PathAttributeTypeCode::As4Aggregator + } + ) && matches!( + action, + AttributeAction::Discard + )), + "expected ReservedAsZero(As4Aggregator) discard, got {:?}", + msg.errors + ); + assert!(!msg.path_attributes.iter().any(|pa| matches!( + pa.value, + PathAttributeValue::As4Aggregator(_) + ))); + } + + /// RFC 7607: a peer that advertises AS 0 in its OPEN must be detected as + /// using a reserved ASN (the session layer rejects it with Bad Peer AS). + #[test] + fn open_peer_as_zero_is_reserved() { + let om = OpenMessage::new4(0, 30, 0xaabbccdd, false); + assert!(Asn::from(om.asn()).is_reserved()); + + let om = OpenMessage::new2(0, 30, 0xaabbccdd, false); + assert!(Asn::from(om.asn()).is_reserved()); + } + // ========================================================================= // BgpNexthop tests // ========================================================================= diff --git a/bgp/src/proptest.rs b/bgp/src/proptest.rs index c2e855d9..1832bbef 100644 --- a/bgp/src/proptest.rs +++ b/bgp/src/proptest.rs @@ -92,7 +92,8 @@ fn path_origin_strategy() -> impl Strategy { fn as_path_segment_strategy() -> impl Strategy { ( prop_oneof![Just(AsPathType::AsSet), Just(AsPathType::AsSequence)], - prop::collection::vec(any::(), 1..5), + // RFC 7607: AS 0 is reserved and rejected on parse, so never generate it. + prop::collection::vec(1..=u32::MAX, 1..5), ) .prop_map(|(typ, value)| As4PathSegment { typ, value }) } diff --git a/bgp/src/session.rs b/bgp/src/session.rs index e0144ce4..6eea856d 100644 --- a/bgp/src/session.rs +++ b/bgp/src/session.rs @@ -7173,6 +7173,15 @@ impl SessionRunner { /// Handle an open message fn handle_open(&self, conn: &Cnx, om: &OpenMessage) -> Result<(), Error> { let remote_asn = om.asn(); + if Asn::from(remote_asn).is_reserved() { + self.send_notification( + conn, + ErrorCode::Open, + ErrorSubcode::Open(OpenErrorSubcode::BadPeerAS), + ); + self.unregister_conn(conn.id()); + return Err(Error::ReservedPeerAsn); + } if let Some(expected_remote_asn) = lock!(self.session).remote_asn && remote_asn != expected_remote_asn { diff --git a/mg-api-types/versions/src/impls/bgp/parse.rs b/mg-api-types/versions/src/impls/bgp/parse.rs index a96af8ee..ad5ff4df 100644 --- a/mg-api-types/versions/src/impls/bgp/parse.rs +++ b/mg-api-types/versions/src/impls/bgp/parse.rs @@ -59,6 +59,8 @@ pub enum UpdateParseErrorReason { InvalidOriginValue { value: u8 }, /// AS_PATH attribute is malformed MalformedAsPath { detail: String }, + /// Reserved AS 0 found in an AS path or aggregator attribute (RFC 7607) + ReservedAsZero { type_code: PathAttributeTypeCode }, /// Attribute flags are invalid for this type InvalidAttributeFlags { type_code: u8, flags: u8 }, @@ -159,6 +161,13 @@ impl Display for UpdateParseErrorReason { Self::InvalidOriginValue { value } => { write!(f, "invalid ORIGIN value: {}", value) } + Self::ReservedAsZero { type_code } => { + write!( + f, + "reserved AS 0 in {:?} attribute (RFC 7607)", + type_code + ) + } Self::MalformedAsPath { detail } => { write!(f, "malformed AS_PATH: {}", detail) } diff --git a/mgd/src/bgp_admin.rs b/mgd/src/bgp_admin.rs index fdec1a44..fb57f7c2 100644 --- a/mgd/src/bgp_admin.rs +++ b/mgd/src/bgp_admin.rs @@ -2371,6 +2371,13 @@ pub(crate) mod helpers { id: rq.id, }; + if cfg.asn.is_reserved() { + return Err(Error::InvalidRequest( + "AS 0 is reserved and cannot be used as a local ASN (RFC 7607)" + .into(), + )); + } + let db = ctx.db.clone(); let router = Arc::new(Router::::new( diff --git a/mgd/src/main.rs b/mgd/src/main.rs index 4e7fd276..aec78a6d 100644 --- a/mgd/src/main.rs +++ b/mgd/src/main.rs @@ -305,6 +305,14 @@ fn start_bgp_routers( dlog!(context.log, info, "starting bgp routers: {routers:#?}"); let mut guard = context.bgp.router.lock().expect("lock bgp routers"); for (asn, info) in routers { + if rdb::Asn::FourOctet(asn).is_reserved() { + dlog!( + context.log, + warn, + "skipping persisted BGP router with reserved AS {asn} (RFC 7607)" + ); + continue; + } bgp_admin::helpers::add_router( context.clone(), mg_api_types::bgp::config::Router { diff --git a/rdb/src/types.rs b/rdb/src/types.rs index 1bac4856..f97632fe 100644 --- a/rdb/src/types.rs +++ b/rdb/src/types.rs @@ -252,12 +252,19 @@ impl From for Asn { } impl Asn { + /// AS 0 is reserved by IANA and MUST NOT appear on the wire (RFC 7607). + pub const RESERVED: u32 = 0; + pub fn as_u32(&self) -> u32 { match self { Self::TwoOctet(value) => u32::from(*value), Self::FourOctet(value) => *value, } } + + pub fn is_reserved(&self) -> bool { + self.as_u32() == Self::RESERVED + } } pub fn to_buf(value: &T) -> Result> { @@ -410,6 +417,15 @@ mod test { } } + /// AS 0 is reserved (RFC 7607) regardless of two- or four-octet encoding. + #[test] + fn asn_reserved_is_zero_only() { + assert!(Asn::TwoOctet(0).is_reserved()); + assert!(Asn::FourOctet(0).is_reserved()); + assert!(!Asn::TwoOctet(1).is_reserved()); + assert!(!Asn::FourOctet(65000).is_reserved()); + } + /// BGP path identity is purely PeerId. Two paths with the /// same PeerId are Equal regardless of all other fields. /// Different PeerIds are never Equal, even when everything