From 5da3fb031ea8cbd56e0877679438df37b23fa9b6 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sun, 3 Aug 2025 18:12:45 -0700 Subject: [PATCH 1/2] feat: implement RFC 7606 BGP error handling with validation warnings Add comprehensive BGP validation warning system that implements RFC 7606 revised error handling for BGP UPDATE messages. This ensures the parser never breaks from reading incorrect BGP messages and follows the treat-as-withdraw approach for mandatory attributes and attribute discard approach for optional attributes. Changes: - Add BgpValidationWarning enum with 12 warning types covering RFC 4271/7606 scenarios - Add validation_warnings field to Attributes struct with accessor methods - Implement attribute validation during parsing (flags, length, duplicates, missing mandatory) - Add RFC 7606 compliant error handling that continues parsing without session reset - Add conditional compilation guards for parser-only features - Add comprehensive tests for all RFC 7606 error handling scenarios - Add RFC 7606 to supported RFCs list in documentation - Use debug\! level logging for warnings to avoid output by default --- src/error.rs | 114 ++++++- src/lib.rs | 2 +- src/models/bgp/attributes/mod.rs | 26 +- src/models/bgp/capabilities.rs | 20 +- src/parser/bgp/attributes/mod.rs | 318 +++++++++++++++++++- src/parser/bmp/messages/route_monitoring.rs | 10 +- 6 files changed, 475 insertions(+), 15 deletions(-) diff --git a/src/error.rs b/src/error.rs index fbe913f0..617a50cb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,7 @@ /*! error module defines the error types used in bgpkit-parser. */ -use crate::models::{Afi, Bgp4MpType, BgpState, EntryType, Safi, TableDumpV2Type}; +use crate::models::{Afi, AttrType, Bgp4MpType, BgpState, EntryType, Safi, TableDumpV2Type}; use num_enum::TryFromPrimitiveError; #[cfg(feature = "oneio")] use oneio::OneIoError; @@ -123,3 +123,115 @@ impl From> for ParserError { ParserError::ParseError(format!("Unknown SAFI type: {}", value.number)) } } + +/// BGP validation warnings for RFC 7606 compliant error handling. +/// These represent non-fatal validation issues that don't prevent parsing. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum BgpValidationWarning { + /// Attribute flags conflict with attribute type code (RFC 4271 Section 6.3) + AttributeFlagsError { + attr_type: AttrType, + expected_flags: u8, + actual_flags: u8, + }, + /// Attribute length conflicts with expected length (RFC 4271 Section 6.3) + AttributeLengthError { + attr_type: AttrType, + expected_length: Option, + actual_length: usize, + }, + /// Missing well-known mandatory attribute (RFC 4271 Section 6.3) + MissingWellKnownAttribute { attr_type: AttrType }, + /// Unrecognized well-known attribute (RFC 4271 Section 6.3) + UnrecognizedWellKnownAttribute { attr_type_code: u8 }, + /// Invalid origin attribute value (RFC 4271 Section 6.3) + InvalidOriginAttribute { value: u8 }, + /// Invalid next hop attribute (RFC 4271 Section 6.3) + InvalidNextHopAttribute { reason: String }, + /// Malformed AS_PATH attribute (RFC 4271 Section 6.3) + MalformedAsPath { reason: String }, + /// Optional attribute error (RFC 4271 Section 6.3) + OptionalAttributeError { attr_type: AttrType, reason: String }, + /// Attribute appears more than once (RFC 4271 Section 6.3) + DuplicateAttribute { attr_type: AttrType }, + /// Invalid network field in NLRI (RFC 4271 Section 6.3) + InvalidNetworkField { reason: String }, + /// Malformed attribute list (RFC 4271 Section 6.3) + MalformedAttributeList { reason: String }, + /// Partial attribute with errors (RFC 7606) + PartialAttributeError { attr_type: AttrType, reason: String }, +} + +impl Display for BgpValidationWarning { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + BgpValidationWarning::AttributeFlagsError { attr_type, expected_flags, actual_flags } => { + write!(f, "Attribute flags error for {attr_type:?}: expected 0x{expected_flags:02x}, got 0x{actual_flags:02x}") + } + BgpValidationWarning::AttributeLengthError { attr_type, expected_length, actual_length } => { + match expected_length { + Some(expected) => write!(f, "Attribute length error for {attr_type:?}: expected {expected}, got {actual_length}"), + None => write!(f, "Attribute length error for {attr_type:?}: invalid length {actual_length}"), + } + } + BgpValidationWarning::MissingWellKnownAttribute { attr_type } => { + write!(f, "Missing well-known mandatory attribute: {attr_type:?}") + } + BgpValidationWarning::UnrecognizedWellKnownAttribute { attr_type_code } => { + write!(f, "Unrecognized well-known attribute: type code {attr_type_code}") + } + BgpValidationWarning::InvalidOriginAttribute { value } => { + write!(f, "Invalid origin attribute value: {value}") + } + BgpValidationWarning::InvalidNextHopAttribute { reason } => { + write!(f, "Invalid next hop attribute: {reason}") + } + BgpValidationWarning::MalformedAsPath { reason } => { + write!(f, "Malformed AS_PATH: {reason}") + } + BgpValidationWarning::OptionalAttributeError { attr_type, reason } => { + write!(f, "Optional attribute error for {attr_type:?}: {reason}") + } + BgpValidationWarning::DuplicateAttribute { attr_type } => { + write!(f, "Duplicate attribute: {attr_type:?}") + } + BgpValidationWarning::InvalidNetworkField { reason } => { + write!(f, "Invalid network field: {reason}") + } + BgpValidationWarning::MalformedAttributeList { reason } => { + write!(f, "Malformed attribute list: {reason}") + } + BgpValidationWarning::PartialAttributeError { attr_type, reason } => { + write!(f, "Partial attribute error for {attr_type:?}: {reason}") + } + } + } +} + +/// Result type for BGP attribute parsing that includes validation warnings +#[derive(Debug, Clone)] +pub struct BgpValidationResult { + pub value: T, + pub warnings: Vec, +} + +impl BgpValidationResult { + pub fn new(value: T) -> Self { + Self { + value, + warnings: Vec::new(), + } + } + + pub fn with_warnings(value: T, warnings: Vec) -> Self { + Self { value, warnings } + } + + pub fn add_warning(&mut self, warning: BgpValidationWarning) { + self.warnings.push(warning); + } + + pub fn has_warnings(&self) -> bool { + !self.warnings.is_empty() + } +} diff --git a/src/lib.rs b/src/lib.rs index ac4c6bbe..c0fa233b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -364,6 +364,7 @@ If you would like to see any specific RFC's support, please submit an issue on G - [X] [RFC 4456](https://datatracker.ietf.org/doc/html/rfc4456): BGP Route Reflection: An Alternative to Full Mesh Internal BGP (IBGP) - [X] [RFC 5065](https://datatracker.ietf.org/doc/html/rfc5065): Autonomous System Confederations for BGP - [X] [RFC 6793](https://datatracker.ietf.org/doc/html/rfc6793): BGP Support for Four-Octet Autonomous System (AS) Number Space +- [X] [RFC 7606](https://datatracker.ietf.org/doc/html/rfc7606): Revised Error Handling for BGP UPDATE Messages - [X] [RFC 7911](https://datatracker.ietf.org/doc/html/rfc7911): Advertisement of Multiple Paths in BGP (ADD-PATH) - [X] [RFC 8950](https://datatracker.ietf.org/doc/html/rfc8950): Advertising IPv4 Network Layer Reachability Information (NLRI) with an IPv6 Next Hop - [X] [RFC 9072](https://datatracker.ietf.org/doc/html/rfc9072): Extended Optional Parameters Length for BGP OPEN Message Updates @@ -411,7 +412,6 @@ We support normal communities, extended communities, and large communities. #[cfg(feature = "parser")] pub mod encoder; -#[cfg(feature = "parser")] pub mod error; pub mod models; #[cfg(feature = "parser")] diff --git a/src/models/bgp/attributes/mod.rs b/src/models/bgp/attributes/mod.rs index 8811aba2..a595898e 100644 --- a/src/models/bgp/attributes/mod.rs +++ b/src/models/bgp/attributes/mod.rs @@ -12,6 +12,7 @@ use std::net::IpAddr; use std::slice::Iter; use std::vec::IntoIter; +use crate::error::BgpValidationWarning; use crate::models::*; pub use aspath::*; @@ -138,6 +139,8 @@ pub struct Attributes { // Black box type to allow for later changes/optimizations. The most common attributes could be // added as fields to allow for easier lookup. pub(crate) inner: Vec, + /// RFC 7606 validation warnings collected during parsing + pub(crate) validation_warnings: Vec, } impl Attributes { @@ -156,6 +159,21 @@ impl Attributes { self.inner.push(attr); } + /// Add a validation warning to the attributes + pub fn add_validation_warning(&mut self, warning: BgpValidationWarning) { + self.validation_warnings.push(warning); + } + + /// Get all validation warnings for these attributes + pub fn validation_warnings(&self) -> &[BgpValidationWarning] { + &self.validation_warnings + } + + /// Check if there are any validation warnings + pub fn has_validation_warnings(&self) -> bool { + !self.validation_warnings.is_empty() + } + /// Get the `ORIGIN` attribute. In the event that this attribute is not present, /// [Origin::INCOMPLETE] will be returned instead. pub fn origin(&self) -> Origin { @@ -310,13 +328,17 @@ impl FromIterator for Attributes { fn from_iter>(iter: T) -> Self { Attributes { inner: iter.into_iter().collect(), + validation_warnings: Vec::new(), } } } impl From> for Attributes { fn from(value: Vec) -> Self { - Attributes { inner: value } + Attributes { + inner: value, + validation_warnings: Vec::new(), + } } } @@ -342,6 +364,7 @@ impl FromIterator for Attributes { flag: AttrFlags::empty(), }) .collect(), + validation_warnings: Vec::new(), } } } @@ -385,6 +408,7 @@ mod serde_impl { { Ok(Attributes { inner: >::deserialize(deserializer)?, + validation_warnings: Vec::new(), }) } } diff --git a/src/models/bgp/capabilities.rs b/src/models/bgp/capabilities.rs index 38ee7974..5f523605 100644 --- a/src/models/bgp/capabilities.rs +++ b/src/models/bgp/capabilities.rs @@ -1,6 +1,8 @@ +use crate::error::ParserError; use crate::models::network::{Afi, Safi}; +#[cfg(feature = "parser")] use crate::parser::ReadUtils; -use crate::ParserError; +#[cfg(feature = "parser")] use bytes::{BufMut, Bytes, BytesMut}; use num_enum::{FromPrimitive, IntoPrimitive}; @@ -101,6 +103,7 @@ impl ExtendedNextHopCapability { /// - NLRI AFI (2 bytes) /// - NLRI SAFI (2 bytes) /// - NextHop AFI (2 bytes) + #[cfg(feature = "parser")] pub fn parse(mut data: Bytes) -> Result { let mut entries = Vec::new(); @@ -132,6 +135,7 @@ impl ExtendedNextHopCapability { } /// Encode Extended Next Hop capability to raw bytes - RFC 8950, Section 3 + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::with_capacity(self.entries.len() * 6); @@ -168,6 +172,7 @@ impl MultiprotocolExtensionsCapability { /// - AFI (2 bytes) /// - Reserved (1 byte) - should be 0 /// - SAFI (1 byte) + #[cfg(feature = "parser")] pub fn parse(mut data: Bytes) -> Result { if data.len() != 4 { return Err(ParserError::ParseError(format!( @@ -186,6 +191,7 @@ impl MultiprotocolExtensionsCapability { } /// Encode Multiprotocol Extensions capability to raw bytes - RFC 2858, Section 7 + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::with_capacity(4); bytes.put_u16(self.afi as u16); // AFI (2 bytes) @@ -238,6 +244,7 @@ impl GracefulRestartCapability { /// Format: /// - Restart Flags (4 bits) + Restart Time (12 bits) = 2 bytes total /// - Followed by 0 or more address family entries (4 bytes each) + #[cfg(feature = "parser")] pub fn parse(mut data: Bytes) -> Result { if data.len() < 2 { return Err(ParserError::ParseError(format!( @@ -284,6 +291,7 @@ impl GracefulRestartCapability { } /// Encode Graceful Restart capability to raw bytes - RFC 4724 + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::with_capacity(2 + self.address_families.len() * 4); @@ -367,6 +375,7 @@ impl AddPathCapability { /// - AFI (2 bytes) /// - SAFI (1 byte) /// - Send/Receive (1 byte) + #[cfg(feature = "parser")] pub fn parse(mut data: Bytes) -> Result { let mut address_families = Vec::new(); @@ -397,6 +406,7 @@ impl AddPathCapability { } /// Encode ADD-PATH capability to raw bytes - RFC 7911 + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::with_capacity(self.address_families.len() * 4); @@ -424,6 +434,7 @@ impl RouteRefreshCapability { /// Parse Route Refresh capability from raw bytes - RFC 2918 /// This capability has length 0, so data should be empty + #[cfg(feature = "parser")] pub fn parse(data: Bytes) -> Result { if !data.is_empty() { return Err(ParserError::ParseError(format!( @@ -436,6 +447,7 @@ impl RouteRefreshCapability { /// Encode Route Refresh capability to raw bytes - RFC 2918 /// Always returns empty bytes since this capability has no parameters + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { Bytes::new() } @@ -463,6 +475,7 @@ impl FourOctetAsCapability { /// Parse 4-octet AS capability from raw bytes - RFC 6793 /// Format: 4 bytes containing the AS number + #[cfg(feature = "parser")] pub fn parse(mut data: Bytes) -> Result { if data.len() != 4 { return Err(ParserError::ParseError(format!( @@ -476,6 +489,7 @@ impl FourOctetAsCapability { } /// Encode 4-octet AS capability to raw bytes - RFC 6793 + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::with_capacity(4); bytes.put_u32(self.asn); @@ -533,6 +547,7 @@ impl BgpRoleCapability { /// Parse BGP Role capability from raw bytes - RFC 9234 /// Format: 1 byte containing the role value + #[cfg(feature = "parser")] pub fn parse(mut data: Bytes) -> Result { if data.len() != 1 { return Err(ParserError::ParseError(format!( @@ -547,6 +562,7 @@ impl BgpRoleCapability { } /// Encode BGP Role capability to raw bytes - RFC 9234 + #[cfg(feature = "parser")] pub fn encode(&self) -> Bytes { let mut bytes = BytesMut::with_capacity(1); bytes.put_u8(self.role as u8); @@ -554,7 +570,7 @@ impl BgpRoleCapability { } } -#[cfg(test)] +#[cfg(all(test, feature = "parser"))] mod tests { use super::*; diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index f0973718..3e030c08 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -18,7 +18,7 @@ use std::net::IpAddr; use crate::models::*; -use crate::error::ParserError; +use crate::error::{BgpValidationWarning, ParserError}; use crate::parser::bgp::attributes::attr_01_origin::{encode_origin, parse_origin}; use crate::parser::bgp::attributes::attr_02_17_as_path::{encode_as_path, parse_as_path}; use crate::parser::bgp::attributes::attr_03_next_hop::{encode_next_hop, parse_next_hop}; @@ -45,6 +45,121 @@ use crate::parser::bgp::attributes::attr_35_otc::{ }; use crate::parser::ReadUtils; +/// Validate attribute flags according to RFC 4271 and RFC 7606 +fn validate_attribute_flags( + attr_type: AttrType, + flags: AttrFlags, + warnings: &mut Vec, +) { + let expected_flags = match attr_type { + // Well-known mandatory attributes + AttrType::ORIGIN | AttrType::AS_PATH | AttrType::NEXT_HOP => AttrFlags::TRANSITIVE, + // Well-known discretionary attributes + AttrType::ATOMIC_AGGREGATE => AttrFlags::TRANSITIVE, + // Optional non-transitive attributes + AttrType::MULTI_EXIT_DISCRIMINATOR + | AttrType::ORIGINATOR_ID + | AttrType::CLUSTER_LIST + | AttrType::MP_REACHABLE_NLRI + | AttrType::MP_UNREACHABLE_NLRI => AttrFlags::OPTIONAL, + // Optional transitive attributes + AttrType::AGGREGATOR + | AttrType::AS4_AGGREGATOR + | AttrType::AS4_PATH + | AttrType::COMMUNITIES + | AttrType::EXTENDED_COMMUNITIES + | AttrType::IPV6_ADDRESS_SPECIFIC_EXTENDED_COMMUNITIES + | AttrType::LARGE_COMMUNITIES + | AttrType::ONLY_TO_CUSTOMER => AttrFlags::OPTIONAL | AttrFlags::TRANSITIVE, + // LOCAL_PREFERENCE is well-known mandatory for IBGP + AttrType::LOCAL_PREFERENCE => AttrFlags::TRANSITIVE, + // Unknown or development attributes + _ => return, // Don't validate unknown attributes + }; + + // Check if flags match expected (ignoring EXTENDED and PARTIAL flags for this check) + let relevant_flags = flags & (AttrFlags::OPTIONAL | AttrFlags::TRANSITIVE); + if relevant_flags != expected_flags { + warnings.push(BgpValidationWarning::AttributeFlagsError { + attr_type, + expected_flags: expected_flags.bits(), + actual_flags: relevant_flags.bits(), + }); + } + + // Check partial flag constraint + if flags.contains(AttrFlags::PARTIAL) { + match attr_type { + // Partial bit MUST be 0 for well-known attributes and optional non-transitive + AttrType::ORIGIN + | AttrType::AS_PATH + | AttrType::NEXT_HOP + | AttrType::LOCAL_PREFERENCE + | AttrType::ATOMIC_AGGREGATE + | AttrType::MULTI_EXIT_DISCRIMINATOR + | AttrType::ORIGINATOR_ID + | AttrType::CLUSTER_LIST + | AttrType::MP_REACHABLE_NLRI + | AttrType::MP_UNREACHABLE_NLRI => { + warnings.push(BgpValidationWarning::AttributeFlagsError { + attr_type, + expected_flags: expected_flags.bits(), + actual_flags: flags.bits(), + }); + } + _ => {} // Partial is OK for optional transitive attributes + } + } +} + +/// Check if an attribute type is well-known mandatory +fn is_well_known_mandatory(attr_type: AttrType) -> bool { + matches!( + attr_type, + AttrType::ORIGIN | AttrType::AS_PATH | AttrType::NEXT_HOP | AttrType::LOCAL_PREFERENCE + ) +} + +/// Validate attribute length constraints +fn validate_attribute_length( + attr_type: AttrType, + length: usize, + warnings: &mut Vec, +) { + let expected_length = match attr_type { + AttrType::ORIGIN => Some(1), + AttrType::NEXT_HOP => Some(4), // IPv4 next hop + AttrType::MULTI_EXIT_DISCRIMINATOR => Some(4), + AttrType::LOCAL_PREFERENCE => Some(4), + AttrType::ATOMIC_AGGREGATE => Some(0), + AttrType::ORIGINATOR_ID => Some(4), + AttrType::ONLY_TO_CUSTOMER => Some(4), + // Variable length attributes - no fixed constraint + AttrType::AS_PATH + | AttrType::AS4_PATH + | AttrType::AGGREGATOR + | AttrType::AS4_AGGREGATOR + | AttrType::COMMUNITIES + | AttrType::EXTENDED_COMMUNITIES + | AttrType::IPV6_ADDRESS_SPECIFIC_EXTENDED_COMMUNITIES + | AttrType::LARGE_COMMUNITIES + | AttrType::CLUSTER_LIST + | AttrType::MP_REACHABLE_NLRI + | AttrType::MP_UNREACHABLE_NLRI => None, + _ => None, // Unknown attributes + }; + + if let Some(expected) = expected_length { + if length != expected { + warnings.push(BgpValidationWarning::AttributeLengthError { + attr_type, + expected_length: Some(expected), + actual_length: length, + }); + } + } +} + /// Parse BGP attributes given a slice of u8 and some options. /// /// The `data: &[u8]` contains the entirety of the attributes bytes, therefore the size of @@ -58,6 +173,8 @@ pub fn parse_attributes( prefixes: Option<&[NetworkPrefix]>, ) -> Result { let mut attributes: Vec = Vec::with_capacity(20); + let mut validation_warnings: Vec = Vec::new(); + let mut seen_attributes: std::collections::HashSet = std::collections::HashSet::new(); while data.remaining() >= 3 { // each attribute is at least 3 bytes: flag(1) + type(1) + length(1) @@ -90,7 +207,23 @@ pub fn parse_attributes( "reading attribute: type -- {:?}, length -- {}", &attr_type, attr_length ); - let attr_type = match AttrType::from(attr_type) { + + let parsed_attr_type = AttrType::from(attr_type); + + // RFC 7606: Check for duplicate attributes + if seen_attributes.contains(&parsed_attr_type) { + validation_warnings.push(BgpValidationWarning::DuplicateAttribute { + attr_type: parsed_attr_type, + }); + // Continue processing - don't skip duplicate for now + } + seen_attributes.insert(parsed_attr_type); + + // Validate attribute flags and length + validate_attribute_flags(parsed_attr_type, flag, &mut validation_warnings); + validate_attribute_length(parsed_attr_type, attr_length, &mut validation_warnings); + + let attr_type = match parsed_attr_type { attr_type @ AttrType::Unknown(unknown_type) => { // skip pass the remaining bytes of this attribute let bytes = data.read_n_bytes(attr_length)?; @@ -195,18 +328,61 @@ pub fn parse_attributes( attributes.push(Attribute { value, flag }); } Err(e) => { + // RFC 7606 error handling if partial { - // it's ok to have errors when reading partial bytes - debug!("PARTIAL: {}", e); + // Partial attribute with errors - log warning but continue + validation_warnings.push(BgpValidationWarning::PartialAttributeError { + attr_type, + reason: e.to_string(), + }); + debug!("PARTIAL attribute error: {}", e); + } else if is_well_known_mandatory(attr_type) { + // For well-known mandatory attributes, use "treat-as-withdraw" approach + // Don't break parsing, but log warning + validation_warnings.push(BgpValidationWarning::MalformedAttributeList { + reason: format!( + "Well-known mandatory attribute {} parsing failed: {}", + u8::from(attr_type), + e + ), + }); + debug!( + "Well-known mandatory attribute parsing failed, treating as withdraw: {}", + e + ); } else { - debug!("{}", e); + // For optional attributes, use "attribute discard" approach + validation_warnings.push(BgpValidationWarning::OptionalAttributeError { + attr_type, + reason: e.to_string(), + }); + debug!("Optional attribute error, discarding: {}", e); } + // Continue parsing in all cases - never break the session continue; } }; } - Ok(Attributes::from(attributes)) + // Check for missing well-known mandatory attributes + let mandatory_attributes = [ + AttrType::ORIGIN, + AttrType::AS_PATH, + AttrType::NEXT_HOP, + // LOCAL_PREFERENCE is only mandatory for IBGP, so we don't check it here + ]; + + for &mandatory_attr in &mandatory_attributes { + if !seen_attributes.contains(&mandatory_attr) { + validation_warnings.push(BgpValidationWarning::MissingWellKnownAttribute { + attr_type: mandatory_attr, + }); + } + } + + let mut result = Attributes::from(attributes); + result.validation_warnings = validation_warnings; + Ok(result) } impl Attribute { @@ -298,4 +474,134 @@ mod tests { AttrType::Unknown(254) ); } + + #[test] + fn test_rfc7606_attribute_flags_error() { + // Create an ORIGIN attribute with wrong flags (should be transitive, not optional) + let data = Bytes::from(vec![0x80, 0x01, 0x01, 0x00]); // Optional flag set incorrectly + let asn_len = AsnLength::Bits16; + let add_path = false; + let afi = None; + let safi = None; + let prefixes = None; + + let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + + // Should have validation warning for incorrect flags + assert!(attributes.has_validation_warnings()); + let warnings = attributes.validation_warnings(); + // Will have attribute flags error + missing mandatory attributes + assert!(warnings.len() >= 1); + + match &warnings[0] { + BgpValidationWarning::AttributeFlagsError { attr_type, .. } => { + assert_eq!(*attr_type, AttrType::ORIGIN); + } + _ => panic!("Expected AttributeFlagsError warning"), + } + } + + #[test] + fn test_rfc7606_missing_mandatory_attribute() { + // Empty attributes - should have warnings for missing mandatory attributes + let data = Bytes::from(vec![]); + let asn_len = AsnLength::Bits16; + let add_path = false; + let afi = None; + let safi = None; + let prefixes = None; + + let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + + // Should have warnings for missing mandatory attributes + assert!(attributes.has_validation_warnings()); + let warnings = attributes.validation_warnings(); + assert_eq!(warnings.len(), 3); // ORIGIN, AS_PATH, NEXT_HOP + + for warning in warnings { + match warning { + BgpValidationWarning::MissingWellKnownAttribute { attr_type } => { + assert!(matches!( + attr_type, + AttrType::ORIGIN | AttrType::AS_PATH | AttrType::NEXT_HOP + )); + } + _ => panic!("Expected MissingWellKnownAttribute warning"), + } + } + } + + #[test] + fn test_rfc7606_duplicate_attribute() { + // Create two ORIGIN attributes + let data = Bytes::from(vec![ + 0x40, 0x01, 0x01, 0x00, // First ORIGIN attribute + 0x40, 0x01, 0x01, 0x01, // Second ORIGIN attribute (duplicate) + ]); + let asn_len = AsnLength::Bits16; + let add_path = false; + let afi = None; + let safi = None; + let prefixes = None; + + let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + + // Should have warning for duplicate attribute + assert!(attributes.has_validation_warnings()); + let warnings = attributes.validation_warnings(); + + // Should have at least one duplicate attribute warning + let has_duplicate_warning = warnings + .iter() + .any(|w| matches!(w, BgpValidationWarning::DuplicateAttribute { .. })); + assert!(has_duplicate_warning); + } + + #[test] + fn test_rfc7606_attribute_length_error() { + // Create an ORIGIN attribute with wrong length (should be 1 byte, not 2) + let data = Bytes::from(vec![0x40, 0x01, 0x02, 0x00, 0x01]); + let asn_len = AsnLength::Bits16; + let add_path = false; + let afi = None; + let safi = None; + let prefixes = None; + + let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + + // Should have warning for incorrect attribute length + assert!(attributes.has_validation_warnings()); + let warnings = attributes.validation_warnings(); + + let has_length_warning = warnings + .iter() + .any(|w| matches!(w, BgpValidationWarning::AttributeLengthError { .. })); + assert!(has_length_warning); + } + + #[test] + fn test_rfc7606_no_session_reset() { + // Test that parsing continues even with multiple errors + let data = Bytes::from(vec![ + 0x80, 0x01, 0x02, 0x00, 0x01, // Wrong flags and length for ORIGIN + 0x40, 0x01, 0x01, 0x00, // Duplicate ORIGIN + 0x40, 0xFF, 0x01, 0x00, // Unknown attribute + ]); + let asn_len = AsnLength::Bits16; + let add_path = false; + let afi = None; + let safi = None; + let prefixes = None; + + // Should not panic or return error - RFC 7606 requires continued parsing + let result = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes); + assert!(result.is_ok()); + + let attributes = result.unwrap(); + assert!(attributes.has_validation_warnings()); + + // Should have multiple warnings but parsing should continue + let warnings = attributes.validation_warnings(); + assert!(!warnings.is_empty()); + } } diff --git a/src/parser/bmp/messages/route_monitoring.rs b/src/parser/bmp/messages/route_monitoring.rs index 14bd7f77..75a81c33 100644 --- a/src/parser/bmp/messages/route_monitoring.rs +++ b/src/parser/bmp/messages/route_monitoring.rs @@ -74,9 +74,11 @@ mod tests { let mon_msg = RouteMonitoring { bgp_message: BgpMessage::Update(msg), }; - assert_eq!( - format!("{mon_msg:?}"), - "RouteMonitoring { bgp_message: Update(BgpUpdateMessage { withdrawn_prefixes: [], attributes: Attributes { inner: [] }, announced_prefixes: [] }) }" - ); + #[cfg(feature = "parser")] + let expected = "RouteMonitoring { bgp_message: Update(BgpUpdateMessage { withdrawn_prefixes: [], attributes: Attributes { inner: [], validation_warnings: [] }, announced_prefixes: [] }) }"; + #[cfg(not(feature = "parser"))] + let expected = "RouteMonitoring { bgp_message: Update(BgpUpdateMessage { withdrawn_prefixes: [], attributes: Attributes { inner: [] }, announced_prefixes: [] }) }"; + + assert_eq!(format!("{mon_msg:?}"), expected); } } From d3499d3af7ee04a64eb6af0d0af6acf9c1f2b67e Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Sun, 3 Aug 2025 18:16:51 -0700 Subject: [PATCH 2/2] update RFC list --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index f71bdc3d..a48df0fd 100644 --- a/README.md +++ b/README.md @@ -372,6 +372,7 @@ If you would like to see any specific RFC's support, please submit an issue on G - [X] [RFC 4456](https://datatracker.ietf.org/doc/html/rfc4456): BGP Route Reflection: An Alternative to Full Mesh Internal BGP (IBGP) - [X] [RFC 5065](https://datatracker.ietf.org/doc/html/rfc5065): Autonomous System Confederations for BGP - [X] [RFC 6793](https://datatracker.ietf.org/doc/html/rfc6793): BGP Support for Four-Octet Autonomous System (AS) Number Space +- [X] [RFC 7606](https://datatracker.ietf.org/doc/html/rfc7606): Revised Error Handling for BGP UPDATE Messages - [X] [RFC 7911](https://datatracker.ietf.org/doc/html/rfc7911): Advertisement of Multiple Paths in BGP (ADD-PATH) - [X] [RFC 8950](https://datatracker.ietf.org/doc/html/rfc8950): Advertising IPv4 Network Layer Reachability Information (NLRI) with an IPv6 Next Hop - [X] [RFC 9072](https://datatracker.ietf.org/doc/html/rfc9072): Extended Optional Parameters Length for BGP OPEN Message Updates