From c75c28ab0ba64df7a6dbeef6502471bef5532e80 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:05:27 -0700 Subject: [PATCH 1/7] fix: suppress NEXT_HOP warning when MP_REACH_NLRI is present (RFC 4760) Per RFC 4760 Section 3, UPDATE messages carrying no NLRI other than what's encoded in MP_REACH_NLRI should not carry a separate NEXT_HOP attribute, as the next hop is embedded within MP_REACH_NLRI itself. This eliminates false positive warnings for valid MP-BGP messages. Caveat: may produce false negatives for rare edge cases where an UPDATE contains both regular NLRI and MP_REACH_NLRI but lacks NEXT_HOP. --- CHANGELOG.md | 4 ++ src/parser/bgp/attributes/mod.rs | 102 +++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d6a5bf..6ba5881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Bug fixes + +* **RFC 4760 NEXT_HOP validation**: Suppress `MissingWellKnownAttribute { NEXT_HOP }` warning when `MP_REACH_NLRI` is present. Per RFC 4760 Section 3, UPDATE messages carrying no NLRI other than what's encoded in `MP_REACH_NLRI` should not carry a separate `NEXT_HOP` attribute (the next hop is embedded within `MP_REACH_NLRI`). This eliminates false positive warnings for valid MP-BGP messages. Note: may produce false negatives for rare edge cases where an UPDATE contains both regular NLRI and `MP_REACH_NLRI` but lacks `NEXT_HOP`. + ## v0.16.0 - 2026-04-07 ### Breaking changes diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index b8faa01..6ad8a25 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -386,8 +386,16 @@ pub fn parse_attributes( // LOCAL_PREFERENCE is only mandatory for IBGP, so we don't check it here ]; + // RFC 4760 Section 3: When MP_REACH_NLRI is present, the NEXT_HOP attribute + // should not be present (the next hop is carried within MP_REACH_NLRI itself). + let mp_reach_nlri_present = seen_attributes[u8::from(AttrType::MP_REACHABLE_NLRI) as usize]; + for &mandatory_attr in &mandatory_attributes { if !seen_attributes[u8::from(mandatory_attr) as usize] { + // Skip NEXT_HOP warning if MP_REACH_NLRI is present (RFC 4760) + if mandatory_attr == AttrType::NEXT_HOP && mp_reach_nlri_present { + continue; + } validation_warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: mandatory_attr, }); @@ -666,4 +674,98 @@ mod tests { let warnings = attributes.validation_warnings(); assert!(!warnings.is_empty()); } + + #[test] + fn test_rfc4760_mp_reach_nlri_no_next_hop_warning() { + // RFC 4760 Section 3: When MP_REACH_NLRI is present, the NEXT_HOP attribute + // should not be present (the next hop is carried within MP_REACH_NLRI itself). + // Therefore, no MissingWellKnownAttribute warning for NEXT_HOP should be generated. + let data = Bytes::from(vec![ + // ORIGIN attribute (type 1) - required + 0x40, 0x01, 0x01, 0x00, // flags (transitive), type 1, length 1, value IGP + // AS_PATH attribute (type 2) - required + 0x40, 0x02, 0x00, // flags (transitive), type 2, length 0 (empty AS_PATH) + // MP_REACH_NLRI attribute (type 14) with IPv4 unicast + 0x80, 0x0E, // flags (optional), type 14 + 0x0D, // length: 13 bytes (AFI(2)+SAFI(1)+NH-len(1)+NH(4)+Reserved(1)+Prefix-len(1)+Prefix(3)) + 0x00, 0x01, // AFI: IPv4 + 0x01, // SAFI: Unicast + 0x04, // Next hop length: 4 bytes + 0xC0, 0x00, 0x02, 0x01, // Next hop: 192.0.2.1 + 0x00, // Reserved + 0x18, // Prefix length: 24 bits + 0xC0, 0x00, + 0x02, // NLRI: 192.0.2.0/24 + // Note: No NEXT_HOP attribute (type 3) - this is valid per RFC 4760 + ]); + 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(); + + // MP_REACH_NLRI should be present + assert!(attributes.has_attr(AttrType::MP_REACHABLE_NLRI)); + // NEXT_HOP should not be present + assert!(!attributes.has_attr(AttrType::NEXT_HOP)); + + // No MissingWellKnownAttribute warning for NEXT_HOP should be generated + let has_next_hop_warning = attributes + .validation_warnings() + .iter() + .any(|w| matches!(w, BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP)); + assert!( + !has_next_hop_warning, + "Should not warn about missing NEXT_HOP when MP_REACH_NLRI is present (RFC 4760)" + ); + } + + #[test] + fn test_rfc4760_mp_reach_nlri_with_next_hop_present() { + // RFC 4760 Section 3: If NEXT_HOP is present when MP_REACH_NLRI is present, + // it should be ignored (no warning should be generated for having it) + let data = Bytes::from(vec![ + // ORIGIN attribute (type 1) - required + 0x40, 0x01, 0x01, 0x00, // flags (transitive), type 1, length 1, value IGP + // AS_PATH attribute (type 2) - required + 0x40, 0x02, 0x00, // flags (transitive), type 2, length 0 (empty AS_PATH) + // NEXT_HOP attribute (type 3) - present but will be ignored per RFC 4760 + 0x40, 0x03, 0x04, // flags (transitive), type 3, length 4 + 0xC0, 0x00, 0x02, 0x01, // Next hop: 192.0.2.1 + // MP_REACH_NLRI attribute (type 14) with IPv6 unicast + 0x80, 0x0E, // flags (optional), type 14 + 0x1E, // length: 30 bytes (AFI(2)+SAFI(1)+NH-len(1)+NH(16)+Reserved(1)+Prefix-len(1)+Prefix(8)) + 0x00, 0x02, // AFI: IPv6 + 0x01, // SAFI: Unicast + 0x10, // Next hop length: 16 bytes + // IPv6 next hop: 2001:db8::1 + 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x01, 0x00, // Reserved + 0x40, // Prefix length: 64 bits + 0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, // NLRI: 2001:db8::/64 + ]); + 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(); + + // Both NEXT_HOP and MP_REACH_NLRI should be present + assert!(attributes.has_attr(AttrType::NEXT_HOP)); + assert!(attributes.has_attr(AttrType::MP_REACHABLE_NLRI)); + + // No MissingWellKnownAttribute warning for NEXT_HOP + let has_next_hop_warning = attributes + .validation_warnings() + .iter() + .any(|w| matches!(w, BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP)); + assert!( + !has_next_hop_warning, + "Should not warn about NEXT_HOP when MP_REACH_NLRI is present" + ); + } } From 73bed4b509f4a33daf81b153127791f68e442ffb Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:18:50 -0700 Subject: [PATCH 2/7] refactor: move mandatory attribute validation after NLRI parsing Complete refactoring of BGP mandatory attribute validation to handle RFC 4760 (MP-BGP) semantics correctly: **Breaking change:** - parse_attributes() now returns (Attributes, [bool; 256]) tuple to track seen attribute types for post-parsing validation **New validation logic:** - ORIGIN and AS_PATH: required for any announcement (IPv4 or MP_REACH_NLRI) - NEXT_HOP: required only when IPv4 NLRI is present (RFC 4271) - No mandatory attributes for withdrawal-only messages (RFC 4760 Section 4) - MP_REACH_NLRI carries embedded next hop, no separate NEXT_HOP needed **Validation moved from parse_attributes() to parse_bgp_update_message()** after NLRI parsing, enabling proper context-aware checks. **Files changed:** - src/parser/bgp/attributes/mod.rs: return tuple, add validate_mandatory_attributes() - src/parser/bgp/messages.rs: call validation after NLRI parsing - src/parser/mrt/messages/table_dump*.rs: handle tuple return - CHANGELOG.md: document breaking change and bug fix --- CHANGELOG.md | 12 +- src/parser/bgp/attributes/mod.rs | 193 ++++++++++++------ src/parser/bgp/messages.rs | 23 ++- src/parser/mrt/messages/table_dump.rs | 2 +- .../messages/table_dump_v2/rib_afi_entries.rs | 2 +- 5 files changed, 164 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ba5881..fc0b6d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,19 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Breaking changes + +* **`parse_attributes()` signature change**: The function now returns a tuple `(Attributes, [bool; 256])` where the second element tracks which attribute types were seen during parsing. This enables proper mandatory attribute validation after NLRI parsing. Callers need to update to handle the tuple return. + ### Bug fixes -* **RFC 4760 NEXT_HOP validation**: Suppress `MissingWellKnownAttribute { NEXT_HOP }` warning when `MP_REACH_NLRI` is present. Per RFC 4760 Section 3, UPDATE messages carrying no NLRI other than what's encoded in `MP_REACH_NLRI` should not carry a separate `NEXT_HOP` attribute (the next hop is embedded within `MP_REACH_NLRI`). This eliminates false positive warnings for valid MP-BGP messages. Note: may produce false negatives for rare edge cases where an UPDATE contains both regular NLRI and `MP_REACH_NLRI` but lacks `NEXT_HOP`. +* **RFC 4760 mandatory attribute validation**: Moved mandatory attribute validation from `parse_attributes()` to after NLRI parsing in `parse_bgp_update_message()`. This allows proper context-aware validation per RFC 4271 and RFC 4760: + - `ORIGIN` and `AS_PATH`: required for any announcement (IPv4 NLRI or `MP_REACH_NLRI`) + - `NEXT_HOP`: required only when IPv4 NLRI is present (RFC 4271) + - No mandatory attributes required for withdrawal-only messages (RFC 4760 Section 4) + - `MP_REACH_NLRI` carries its own next hop, so no separate `NEXT_HOP` attribute needed (RFC 4760 Section 3) + + This eliminates false positive warnings for valid MP-BGP messages and properly handles the "conditionally mandatory" nature of BGP attributes. ## v0.16.0 - 2026-04-07 diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 6ad8a25..4296a8c 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -172,6 +172,10 @@ fn validate_attribute_length( /// /// The `data: &[u8]` contains the entirety of the attributes bytes, therefore the size of /// the slice is the total byte length of the attributes section of the message. +/// +/// Returns the parsed attributes and a boolean array tracking which attribute types were seen. +/// Mandatory attribute validation should be performed after NLRI parsing using +/// `validate_mandatory_attributes()` with proper context. pub fn parse_attributes( mut data: Bytes, asn_len: &AsnLength, @@ -179,7 +183,7 @@ pub fn parse_attributes( afi: Option, safi: Option, prefixes: Option<&[NetworkPrefix]>, -) -> Result { +) -> Result<(Attributes, [bool; 256]), ParserError> { // Estimate capacity from data size: each attribute is at least 3 bytes // (flag + type + length). Cap at 256 to avoid over-allocation for corrupted data. let estimated_attrs = (data.remaining() / 3).min(256); @@ -378,33 +382,60 @@ pub fn parse_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 - ]; - - // RFC 4760 Section 3: When MP_REACH_NLRI is present, the NEXT_HOP attribute - // should not be present (the next hop is carried within MP_REACH_NLRI itself). - let mp_reach_nlri_present = seen_attributes[u8::from(AttrType::MP_REACHABLE_NLRI) as usize]; - - for &mandatory_attr in &mandatory_attributes { - if !seen_attributes[u8::from(mandatory_attr) as usize] { - // Skip NEXT_HOP warning if MP_REACH_NLRI is present (RFC 4760) - if mandatory_attr == AttrType::NEXT_HOP && mp_reach_nlri_present { - continue; - } - validation_warnings.push(BgpValidationWarning::MissingWellKnownAttribute { - attr_type: mandatory_attr, - }); - } - } - let mut result = Attributes::from(attributes); result.validation_warnings = validation_warnings; - Ok(result) + Ok((result, seen_attributes)) +} + +/// Validate mandatory attributes based on NLRI context. +/// +/// This should be called after NLRI parsing to properly determine which attributes +/// are mandatory based on the message content per RFC 4271 and RFC 4760. +/// +/// Rules: +/// - ORIGIN and AS_PATH: required if any reachable prefixes are announced +/// (either IPv4 NLRI or MP_REACH_NLRI present) +/// - NEXT_HOP: required only if IPv4 NLRI is present (RFC 4271) +/// Not required if only MP_REACH_NLRI is present (RFC 4760 Section 3) +/// - MP_UNREACH_NLRI only (withdrawals): no attributes required (RFC 4760 Section 4) +pub fn validate_mandatory_attributes( + seen_attributes: &[bool; 256], + has_ipv4_nlri: bool, + has_mp_reach_nlri: bool, + _has_mp_unreach_nlri: bool, +) -> Vec { + let mut warnings = Vec::new(); + + // Check if any announcements exist + let has_announcements = has_ipv4_nlri || has_mp_reach_nlri; + + // If no announcements (withdrawals only), no mandatory attributes required + if !has_announcements { + return warnings; + } + + // ORIGIN and AS_PATH are required for any announcement + if !seen_attributes[u8::from(AttrType::ORIGIN) as usize] { + warnings.push(BgpValidationWarning::MissingWellKnownAttribute { + attr_type: AttrType::ORIGIN, + }); + } + + if !seen_attributes[u8::from(AttrType::AS_PATH) as usize] { + warnings.push(BgpValidationWarning::MissingWellKnownAttribute { + attr_type: AttrType::AS_PATH, + }); + } + + // NEXT_HOP is required only if IPv4 NLRI is present (RFC 4271) + // If only MP_REACH_NLRI is present, next hop is embedded in the attribute (RFC 4760) + if has_ipv4_nlri && !seen_attributes[u8::from(AttrType::NEXT_HOP) as usize] { + warnings.push(BgpValidationWarning::MissingWellKnownAttribute { + attr_type: AttrType::NEXT_HOP, + }); + } + + warnings } impl Attribute { @@ -497,9 +528,9 @@ mod tests { let afi = None; let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes); - assert!(attributes.is_ok()); - let attributes = attributes.unwrap(); + let result = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes); + assert!(result.is_ok()); + let (attributes, _seen_attributes) = result.unwrap(); assert_eq!(attributes.inner.len(), 1); assert_eq!( attributes.inner[0].value.attr_type(), @@ -517,12 +548,13 @@ mod tests { let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, _seen_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 + // Will have attribute flags error (mandatory warnings moved to post-NLRI validation) assert!(!warnings.is_empty()); match &warnings[0] { @@ -534,8 +566,9 @@ mod tests { } #[test] - fn test_rfc7606_missing_mandatory_attribute() { - // Empty attributes - should have warnings for missing mandatory attributes + fn test_mandatory_attributes_validation() { + // Test that validate_mandatory_attributes correctly requires + // ORIGIN, AS_PATH, and NEXT_HOP based on NLRI context let data = Bytes::from(vec![]); let asn_len = AsnLength::Bits16; let add_path = false; @@ -543,14 +576,17 @@ mod tests { let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (_attributes, seen_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 + // Empty attributes with no announcements should have no mandatory warnings + let warnings = validate_mandatory_attributes(&seen_attributes, false, false, false); + assert!(warnings.is_empty()); - for warning in warnings { + // Empty attributes with IPv4 NLRI should warn about all three + let warnings = validate_mandatory_attributes(&seen_attributes, true, false, false); + assert_eq!(warnings.len(), 3); // ORIGIN, AS_PATH, NEXT_HOP + for warning in &warnings { match warning { BgpValidationWarning::MissingWellKnownAttribute { attr_type } => { assert!(matches!( @@ -561,6 +597,21 @@ mod tests { _ => panic!("Expected MissingWellKnownAttribute warning"), } } + + // Empty attributes with MP_REACH_NLRI should only warn about ORIGIN and AS_PATH + let warnings = validate_mandatory_attributes(&seen_attributes, false, true, false); + assert_eq!(warnings.len(), 2); // ORIGIN, AS_PATH (no NEXT_HOP per RFC 4760) + for warning in &warnings { + match warning { + BgpValidationWarning::MissingWellKnownAttribute { attr_type } => { + assert!( + matches!(attr_type, AttrType::ORIGIN | AttrType::AS_PATH), + "Should only warn about ORIGIN or AS_PATH for MP_REACH_NLRI only" + ); + } + _ => panic!("Expected MissingWellKnownAttribute warning"), + } + } } #[test] @@ -576,7 +627,8 @@ mod tests { let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, _seen_attributes) = + parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); // Should have warning for duplicate attribute assert!(attributes.has_validation_warnings()); @@ -609,7 +661,8 @@ mod tests { data.extend_from_slice(&[0x40, 0xFF, 0x01, 0x00]); // development let data = Bytes::from(data); - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, _seen_attributes) = + parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); assert!(attributes.has_attr(AttrType::DEVELOPMENT)); assert!(!attributes.has_validation_warnings()); @@ -619,7 +672,8 @@ mod tests { data.extend_from_slice(&[0x40, 0x00, 0x01, 0x01]); // reserved let data = Bytes::from(data); - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, _seen_attributes) = + parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); // There is a validation warning about the reserved attribute assert!(attributes.validation_warnings.iter().any(|vw| { @@ -637,7 +691,8 @@ mod tests { let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, _seen_attributes) = + parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); // Should have warning for incorrect attribute length assert!(attributes.has_validation_warnings()); @@ -667,7 +722,7 @@ mod tests { let result = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes); assert!(result.is_ok()); - let attributes = result.unwrap(); + let (attributes, _seen_attributes) = result.unwrap(); assert!(attributes.has_validation_warnings()); // Should have multiple warnings but parsing should continue @@ -676,10 +731,10 @@ mod tests { } #[test] - fn test_rfc4760_mp_reach_nlri_no_next_hop_warning() { + fn test_rfc4760_mp_reach_nlri_no_next_hop_validation() { // RFC 4760 Section 3: When MP_REACH_NLRI is present, the NEXT_HOP attribute // should not be present (the next hop is carried within MP_REACH_NLRI itself). - // Therefore, no MissingWellKnownAttribute warning for NEXT_HOP should be generated. + // Test validate_mandatory_attributes with MP_REACH_NLRI context. let data = Bytes::from(vec![ // ORIGIN attribute (type 1) - required 0x40, 0x01, 0x01, 0x00, // flags (transitive), type 1, length 1, value IGP @@ -696,7 +751,7 @@ mod tests { 0x18, // Prefix length: 24 bits 0xC0, 0x00, 0x02, // NLRI: 192.0.2.0/24 - // Note: No NEXT_HOP attribute (type 3) - this is valid per RFC 4760 + // Note: No NEXT_HOP attribute (type 3) ]); let asn_len = AsnLength::Bits16; let add_path = false; @@ -704,21 +759,32 @@ mod tests { let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, seen_attributes) = + parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); // MP_REACH_NLRI should be present assert!(attributes.has_attr(AttrType::MP_REACHABLE_NLRI)); // NEXT_HOP should not be present assert!(!attributes.has_attr(AttrType::NEXT_HOP)); - // No MissingWellKnownAttribute warning for NEXT_HOP should be generated - let has_next_hop_warning = attributes - .validation_warnings() - .iter() - .any(|w| matches!(w, BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP)); + // Test with MP_REACH_NLRI only (no IPv4 NLRI) - should not warn about NEXT_HOP + let warnings = validate_mandatory_attributes(&seen_attributes, false, true, false); assert!( - !has_next_hop_warning, - "Should not warn about missing NEXT_HOP when MP_REACH_NLRI is present (RFC 4760)" + !warnings.iter().any(|w| matches!( + w, + BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP + )), + "Should not warn about missing NEXT_HOP when only MP_REACH_NLRI is present (RFC 4760)" + ); + + // Test with both IPv4 NLRI and MP_REACH_NLRI - should warn about missing NEXT_HOP + let warnings = validate_mandatory_attributes(&seen_attributes, true, true, false); + assert!( + warnings.iter().any(|w| matches!( + w, + BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP + )), + "Should warn about missing NEXT_HOP when IPv4 NLRI is present" ); } @@ -731,7 +797,7 @@ mod tests { 0x40, 0x01, 0x01, 0x00, // flags (transitive), type 1, length 1, value IGP // AS_PATH attribute (type 2) - required 0x40, 0x02, 0x00, // flags (transitive), type 2, length 0 (empty AS_PATH) - // NEXT_HOP attribute (type 3) - present but will be ignored per RFC 4760 + // NEXT_HOP attribute (type 3) - present 0x40, 0x03, 0x04, // flags (transitive), type 3, length 4 0xC0, 0x00, 0x02, 0x01, // Next hop: 192.0.2.1 // MP_REACH_NLRI attribute (type 14) with IPv6 unicast @@ -752,19 +818,20 @@ mod tests { let safi = None; let prefixes = None; - let attributes = parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); + let (attributes, seen_attributes) = + parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap(); // Both NEXT_HOP and MP_REACH_NLRI should be present assert!(attributes.has_attr(AttrType::NEXT_HOP)); assert!(attributes.has_attr(AttrType::MP_REACHABLE_NLRI)); - // No MissingWellKnownAttribute warning for NEXT_HOP - let has_next_hop_warning = attributes - .validation_warnings() - .iter() - .any(|w| matches!(w, BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP)); + // Validate with no IPv4 NLRI - should have no warnings (NEXT_HOP is optional here) + let warnings = validate_mandatory_attributes(&seen_attributes, false, true, false); assert!( - !has_next_hop_warning, + !warnings.iter().any(|w| matches!( + w, + BgpValidationWarning::MissingWellKnownAttribute { attr_type } if *attr_type == AttrType::NEXT_HOP + )), "Should not warn about NEXT_HOP when MP_REACH_NLRI is present" ); } diff --git a/src/parser/bgp/messages.rs b/src/parser/bgp/messages.rs index 3253c23..4e86173 100644 --- a/src/parser/bgp/messages.rs +++ b/src/parser/bgp/messages.rs @@ -10,7 +10,7 @@ use crate::models::capabilities::{ MultiprotocolExtensionsCapability, RouteRefreshCapability, }; use crate::models::error::BgpError; -use crate::parser::bgp::attributes::parse_attributes; +use crate::parser::bgp::attributes::{parse_attributes, validate_mandatory_attributes}; use crate::parser::{encode_nlri_prefixes, parse_nlri_list, ReadUtils}; use log::warn; use zerocopy::big_endian::{U16, U32}; @@ -468,12 +468,31 @@ pub fn parse_bgp_update_message( input.has_n_remaining(attribute_length)?; let attr_data_slice = input.split_to(attribute_length); - let attributes = parse_attributes(attr_data_slice, asn_len, add_path, None, None, None)?; + let (mut attributes, seen_attributes) = + parse_attributes(attr_data_slice, asn_len, add_path, None, None, None)?; // parse announced prefixes nlri. // the remaining bytes are announced prefixes. let announced_prefixes = read_nlri(input, &afi, add_path)?; + // Determine NLRI context for mandatory attribute validation + let has_ipv4_nlri = !announced_prefixes.is_empty(); + let has_mp_reach_nlri = seen_attributes[u8::from(AttrType::MP_REACHABLE_NLRI) as usize]; + let has_mp_unreach_nlri = seen_attributes[u8::from(AttrType::MP_UNREACHABLE_NLRI) as usize]; + + // Validate mandatory attributes based on NLRI context (RFC 4271, RFC 4760) + let mandatory_warnings = validate_mandatory_attributes( + &seen_attributes, + has_ipv4_nlri, + has_mp_reach_nlri, + has_mp_unreach_nlri, + ); + + // Add mandatory validation warnings to attributes + for warning in mandatory_warnings { + attributes.add_validation_warning(warning); + } + Ok(BgpUpdateMessage { withdrawn_prefixes, attributes, diff --git a/src/parser/mrt/messages/table_dump.rs b/src/parser/mrt/messages/table_dump.rs index 3f64e68..adac402 100644 --- a/src/parser/mrt/messages/table_dump.rs +++ b/src/parser/mrt/messages/table_dump.rs @@ -99,7 +99,7 @@ pub fn parse_table_dump_message( let attr_data_slice = data.split_to(attribute_length); // for TABLE_DUMP type, the AS number length is always 2-byte. - let attributes = + let (attributes, _seen_attributes) = parse_attributes(attr_data_slice, &AsnLength::Bits16, false, None, None, None)?; Ok(TableDumpMessage { diff --git a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs index c251313..1149b88 100644 --- a/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs +++ b/src/parser/mrt/messages/table_dump_v2/rib_afi_entries.rs @@ -140,7 +140,7 @@ pub fn parse_rib_entry( input.has_n_remaining(attribute_length)?; let attr_data_slice = input.split_to(attribute_length); - let attributes = parse_attributes( + let (attributes, _seen_attributes) = parse_attributes( attr_data_slice, &AsnLength::Bits32, is_add_path, From 90385dccceff9237464b6cc23f1f3029c762a462 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:22:21 -0700 Subject: [PATCH 3/7] docs: add RFC 4271 and RFC 4760 text to validate_mandatory_attributes Include verbatim RFC excerpts to help readers understand the actual validation rules and their RFC provenance: - RFC 4271 Section 6.3: ORIGIN, AS_PATH, NEXT_HOP as well-known mandatory - RFC 4760 Section 3: MP_REACH_NLRI requirements, NEXT_HOP omission - RFC 4760 Section 4: MP_UNREACH_NLRI requires no other attributes --- src/parser/bgp/attributes/mod.rs | 47 ++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 4296a8c..f450499 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -392,28 +392,59 @@ pub fn parse_attributes( /// This should be called after NLRI parsing to properly determine which attributes /// are mandatory based on the message content per RFC 4271 and RFC 4760. /// -/// Rules: -/// - ORIGIN and AS_PATH: required if any reachable prefixes are announced -/// (either IPv4 NLRI or MP_REACH_NLRI present) -/// - NEXT_HOP: required only if IPv4 NLRI is present (RFC 4271) +/// ## RFC 4271 (BGP-4) Section 6.3 - Well-known Mandatory Attributes +/// +/// > "ORIGIN (Type Code 1): +/// > [...] well-known mandatory attribute [...]" +/// +/// > "AS_PATH (Type Code 2): +/// > [...] well-known mandatory attribute [...]" +/// +/// > "NEXT_HOP (Type Code 3): +/// > [...] well-known mandatory attribute [...] defines the IP address of +/// > the border router that SHOULD be used as the next hop [...]" +/// +/// ## RFC 4760 Section 3 - Multiprotocol Reachable NLRI (MP_REACH_NLRI) +/// +/// > "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the +/// > ORIGIN and the AS_PATH attributes (both in EBGP and in IBGP exchanges). +/// > Moreover, in IBGP exchanges such a message MUST also carry the LOCAL_PREF +/// > attribute." +/// +/// > "An UPDATE message that carries no NLRI, other than the one encoded in +/// > the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP attribute. +/// > If such a message contains the NEXT_HOP attribute, the BGP speaker +/// > that receives the message SHOULD ignore this attribute." +/// +/// ## RFC 4760 Section 4 - Multiprotocol Unreachable NLRI (MP_UNREACH_NLRI) +/// +/// > "An UPDATE message that contains the MP_UNREACH_NLRI is not required +/// > to carry any other path attributes." +/// +/// ## Validation Rules Applied +/// +/// - **ORIGIN and AS_PATH**: required for any announcement +/// (either IPv4 NLRI or MP_REACH_NLRI present) - per RFC 4271 and RFC 4760 Section 3 +/// - **NEXT_HOP**: required only if IPv4 NLRI is present (RFC 4271) /// Not required if only MP_REACH_NLRI is present (RFC 4760 Section 3) -/// - MP_UNREACH_NLRI only (withdrawals): no attributes required (RFC 4760 Section 4) +/// - **Withdrawals only** (MP_UNREACH_NLRI without announcements): no mandatory attributes +/// per RFC 4760 Section 4 pub fn validate_mandatory_attributes( seen_attributes: &[bool; 256], has_ipv4_nlri: bool, has_mp_reach_nlri: bool, _has_mp_unreach_nlri: bool, ) -> Vec { - let mut warnings = Vec::new(); - // Check if any announcements exist let has_announcements = has_ipv4_nlri || has_mp_reach_nlri; // If no announcements (withdrawals only), no mandatory attributes required if !has_announcements { - return warnings; + return vec![]; } + let mut warnings = Vec::new(); + // ORIGIN and AS_PATH are required for any announcement if !seen_attributes[u8::from(AttrType::ORIGIN) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { From de9a1006953ecccf600c5130368e0b0ec7083d95 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:23:49 -0700 Subject: [PATCH 4/7] docs: move RFC text inline with validation checks Place RFC 4271 and RFC 4760 excerpts directly next to each check inside validate_mandatory_attributes() for better readability: - RFC 4760 Section 4 comment at withdrawals check - RFC 4271/4760 Section 3 comments at ORIGIN check - RFC 4271/4760 Section 3 comments at AS_PATH check - RFC 4271 Section 6.3 + RFC 4760 Section 3 comments at NEXT_HOP check --- src/parser/bgp/attributes/mod.rs | 79 ++++++++++++++------------------ 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index f450499..6af2009 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -391,75 +391,66 @@ pub fn parse_attributes( /// /// This should be called after NLRI parsing to properly determine which attributes /// are mandatory based on the message content per RFC 4271 and RFC 4760. -/// -/// ## RFC 4271 (BGP-4) Section 6.3 - Well-known Mandatory Attributes -/// -/// > "ORIGIN (Type Code 1): -/// > [...] well-known mandatory attribute [...]" -/// -/// > "AS_PATH (Type Code 2): -/// > [...] well-known mandatory attribute [...]" -/// -/// > "NEXT_HOP (Type Code 3): -/// > [...] well-known mandatory attribute [...] defines the IP address of -/// > the border router that SHOULD be used as the next hop [...]" -/// -/// ## RFC 4760 Section 3 - Multiprotocol Reachable NLRI (MP_REACH_NLRI) -/// -/// > "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the -/// > ORIGIN and the AS_PATH attributes (both in EBGP and in IBGP exchanges). -/// > Moreover, in IBGP exchanges such a message MUST also carry the LOCAL_PREF -/// > attribute." -/// -/// > "An UPDATE message that carries no NLRI, other than the one encoded in -/// > the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP attribute. -/// > If such a message contains the NEXT_HOP attribute, the BGP speaker -/// > that receives the message SHOULD ignore this attribute." -/// -/// ## RFC 4760 Section 4 - Multiprotocol Unreachable NLRI (MP_UNREACH_NLRI) -/// -/// > "An UPDATE message that contains the MP_UNREACH_NLRI is not required -/// > to carry any other path attributes." -/// -/// ## Validation Rules Applied -/// -/// - **ORIGIN and AS_PATH**: required for any announcement -/// (either IPv4 NLRI or MP_REACH_NLRI present) - per RFC 4271 and RFC 4760 Section 3 -/// - **NEXT_HOP**: required only if IPv4 NLRI is present (RFC 4271) -/// Not required if only MP_REACH_NLRI is present (RFC 4760 Section 3) -/// - **Withdrawals only** (MP_UNREACH_NLRI without announcements): no mandatory attributes -/// per RFC 4760 Section 4 pub fn validate_mandatory_attributes( seen_attributes: &[bool; 256], has_ipv4_nlri: bool, has_mp_reach_nlri: bool, _has_mp_unreach_nlri: bool, ) -> Vec { - // Check if any announcements exist + // RFC 4760 Section 4: + // "An UPDATE message that contains the MP_UNREACH_NLRI is not required + // to carry any other path attributes." + // + // RFC 4760 Section 3: + // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the + // ORIGIN and the AS_PATH attributes..." + // + // Therefore: if no announcements (withdrawals only), no mandatory attributes required. let has_announcements = has_ipv4_nlri || has_mp_reach_nlri; - - // If no announcements (withdrawals only), no mandatory attributes required if !has_announcements { return vec![]; } let mut warnings = Vec::new(); - // ORIGIN and AS_PATH are required for any announcement + // RFC 4271 Section 6.3: + // "ORIGIN (Type Code 1): + // [...] well-known mandatory attribute [...]" + // + // RFC 4760 Section 3: + // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the + // ORIGIN and the AS_PATH attributes..." if !seen_attributes[u8::from(AttrType::ORIGIN) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::ORIGIN, }); } + // RFC 4271 Section 6.3: + // "AS_PATH (Type Code 2): + // [...] well-known mandatory attribute [...]" + // + // RFC 4760 Section 3: + // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the + // ORIGIN and the AS_PATH attributes..." if !seen_attributes[u8::from(AttrType::AS_PATH) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::AS_PATH, }); } - // NEXT_HOP is required only if IPv4 NLRI is present (RFC 4271) - // If only MP_REACH_NLRI is present, next hop is embedded in the attribute (RFC 4760) + // RFC 4271 Section 6.3: + // "NEXT_HOP (Type Code 3): + // [...] well-known mandatory attribute [...] defines the IP address of + // the border router that SHOULD be used as the next hop [...]" + // + // RFC 4760 Section 3: + // "An UPDATE message that carries no NLRI, other than the one encoded in + // the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP attribute. + // If such a message contains the NEXT_HOP attribute, the BGP speaker + // that receives the message SHOULD ignore this attribute." + // + // Therefore: NEXT_HOP is required only if IPv4 NLRI is present. if has_ipv4_nlri && !seen_attributes[u8::from(AttrType::NEXT_HOP) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::NEXT_HOP, From f2ad9c42e1269549e062f087cd27bc7581b0b21b Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:25:10 -0700 Subject: [PATCH 5/7] docs: clarify ORIGIN/AS_PATH required for ANY announcement Update RFC comments to clarify that: - RFC 4271 Section 6.3 is the primary source (well-known mandatory) - RFC 4760 Section 3 clarifies this applies to MP_REACH_NLRI too - Therefore the check applies to ANY announcement (IPv4 or MP) --- src/parser/bgp/attributes/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 6af2009..c6c2f1f 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -417,9 +417,11 @@ pub fn validate_mandatory_attributes( // "ORIGIN (Type Code 1): // [...] well-known mandatory attribute [...]" // - // RFC 4760 Section 3: + // RFC 4760 Section 3 clarifies this applies to MP_REACH_NLRI announcements: // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the // ORIGIN and the AS_PATH attributes..." + // + // Therefore: ORIGIN is required for ANY announcement (IPv4 NLRI or MP_REACH_NLRI). if !seen_attributes[u8::from(AttrType::ORIGIN) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::ORIGIN, @@ -430,9 +432,11 @@ pub fn validate_mandatory_attributes( // "AS_PATH (Type Code 2): // [...] well-known mandatory attribute [...]" // - // RFC 4760 Section 3: + // RFC 4760 Section 3 clarifies this applies to MP_REACH_NLRI announcements: // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the // ORIGIN and the AS_PATH attributes..." + // + // Therefore: AS_PATH is required for ANY announcement (IPv4 NLRI or MP_REACH_NLRI). if !seen_attributes[u8::from(AttrType::AS_PATH) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::AS_PATH, From 41704179cfb6ee24ab23e66ef35230121adda9b8 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:29:22 -0700 Subject: [PATCH 6/7] docs: add RFC 4760 Section 3 text for ORIGIN/AS_PATH Explain why ORIGIN and AS_PATH are required for MP_REACH_NLRI using the actual RFC 4760 text: 'An UPDATE message that carries the MP_REACH_NLRI MUST also carry the ORIGIN and the AS_PATH attributes...' This clarifies that RFC 4271's mandatory requirement extends to multiprotocol announcements as well. --- src/parser/bgp/attributes/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index c6c2f1f..891a126 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -417,11 +417,11 @@ pub fn validate_mandatory_attributes( // "ORIGIN (Type Code 1): // [...] well-known mandatory attribute [...]" // - // RFC 4760 Section 3 clarifies this applies to MP_REACH_NLRI announcements: + // RFC 4760 Section 3: // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the // ORIGIN and the AS_PATH attributes..." // - // Therefore: ORIGIN is required for ANY announcement (IPv4 NLRI or MP_REACH_NLRI). + // This confirms ORIGIN is required for MP_REACH_NLRI announcements as well. if !seen_attributes[u8::from(AttrType::ORIGIN) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::ORIGIN, @@ -432,11 +432,11 @@ pub fn validate_mandatory_attributes( // "AS_PATH (Type Code 2): // [...] well-known mandatory attribute [...]" // - // RFC 4760 Section 3 clarifies this applies to MP_REACH_NLRI announcements: + // RFC 4760 Section 3: // "An UPDATE message that carries the MP_REACH_NLRI MUST also carry the // ORIGIN and the AS_PATH attributes..." // - // Therefore: AS_PATH is required for ANY announcement (IPv4 NLRI or MP_REACH_NLRI). + // This confirms AS_PATH is required for MP_REACH_NLRI announcements as well. if !seen_attributes[u8::from(AttrType::AS_PATH) as usize] { warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::AS_PATH, From aa37708b970b314c9202fcce464a99d57839cb05 Mon Sep 17 00:00:00 2001 From: Mingwei Zhang Date: Wed, 15 Apr 2026 10:31:40 -0700 Subject: [PATCH 7/7] docs: restructure NEXT_HOP validation comment for clarity Follow RFC 4760 Section 3 logic flow more naturally: 1. First quote RFC 4760 on when NEXT_HOP should NOT be present (when only MP_REACH_NLRI carries the NLRI) 2. Explain in plain English what this means for validation 3. Place RFC 4271 quote inside the check block, showing this is where we enforce the mandatory requirement for IPv4 NLRI This avoids the double-negative confusion and makes the RFC-to-code mapping clearer. --- src/parser/bgp/attributes/mod.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/parser/bgp/attributes/mod.rs b/src/parser/bgp/attributes/mod.rs index 891a126..da96565 100644 --- a/src/parser/bgp/attributes/mod.rs +++ b/src/parser/bgp/attributes/mod.rs @@ -443,19 +443,17 @@ pub fn validate_mandatory_attributes( }); } - // RFC 4271 Section 6.3: - // "NEXT_HOP (Type Code 3): - // [...] well-known mandatory attribute [...] defines the IP address of - // the border router that SHOULD be used as the next hop [...]" - // // RFC 4760 Section 3: // "An UPDATE message that carries no NLRI, other than the one encoded in - // the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP attribute. - // If such a message contains the NEXT_HOP attribute, the BGP speaker - // that receives the message SHOULD ignore this attribute." + // the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP attribute." // - // Therefore: NEXT_HOP is required only if IPv4 NLRI is present. + // This means NEXT_HOP is only needed when there is regular IPv4 NLRI. + // MP_REACH_NLRI carries its own next-hop internally, so no separate NEXT_HOP + // attribute is required for multiprotocol routes. if has_ipv4_nlri && !seen_attributes[u8::from(AttrType::NEXT_HOP) as usize] { + // RFC 4271 Section 6.3: + // "NEXT_HOP (Type Code 3): + // [...] well-known mandatory attribute [...]" warnings.push(BgpValidationWarning::MissingWellKnownAttribute { attr_type: AttrType::NEXT_HOP, });