fix: implement conditional mandatory BGP attribute validation

CHANGELOG.md

@@ -4,6 +4,27 @@ All notable changes to this project will be documented in this file.
 
 ## Unreleased
 
+### Fixed
+
+* **RFC 4760 mandatory attribute validation**: Fixed validation to properly handle multiprotocol BGP (MP-BGP) conditional mandatory attributes:
+  - Pure withdrawals (no announcements): no mandatory attributes required
+  - MP_REACH_NLRI only: ORIGIN and AS_PATH required, NEXT_HOP not required (embedded in MP_REACH_NLRI)
+  - IPv4 NLRI: ORIGIN, AS_PATH, and NEXT_HOP all required
+  - Mixed (IPv4 NLRI + MP_REACH_NLRI): ORIGIN, AS_PATH required; NEXT_HOP required because IPv4 NLRI is present
+
+### Added
+
+* **`Attributes::check_mandatory_attributes()`**: New method to validate mandatory attributes with proper NLRI context awareness. Must be called after NLRI parsing to correctly determine requirements.
+* **`Attributes::attr_mask`**: Internal `[u64; 4]` bitmask field for O(1) attribute presence tracking (32 bytes vs 256 bytes for `[bool; 256]`)
+* **Integration tests**: Added `tests/test_bgp_update_validation.rs` with comprehensive scenario coverage for all validation cases
+
+### Changed
+
+* **`parse_attributes()`**: Removed inline mandatory attribute validation; validation now performed by callers via `Attributes::check_mandatory_attributes()` after NLRI context is known
+* **`Attributes::has_attr()`**: Now O(1) via bitmask lookup instead of O(n) vector scan
+* **`Attributes` Debug impl**: Changed from derived to manual implementation (excludes internal `attr_mask` field)
+* **All `Attributes` constructors**: Updated to compute and populate `attr_mask` from `inner` vector
+
 ## v0.16.0 - 2026-04-07
 
 ### Breaking changes

src/models/bgp/attributes/mod.rs

@@ -134,18 +134,30 @@ pub fn get_deprecated_attr_type(attr_type: u8) -> Option<&'static str> {
 }
 
 /// Convenience wrapper for a list of attributes
-#[derive(Debug, PartialEq, Clone, Default, Eq)]
+#[derive(PartialEq, Clone, Default, Eq)]
 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<Attribute>,
     /// RFC 7606 validation warnings collected during parsing
     pub(crate) validation_warnings: Vec<BgpValidationWarning>,
+    /// Bitmask of seen attributes to allow O(1) checks. Fits in 4 u64s.
+    pub(crate) attr_mask: [u64; 4],
+}
+
+impl std::fmt::Debug for Attributes {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Attributes")
+            .field("inner", &self.inner)
+            .field("validation_warnings", &self.validation_warnings)
+            .finish()
+    }
 }
 
 impl Attributes {
     pub fn has_attr(&self, ty: AttrType) -> bool {
-        self.inner.iter().any(|x| x.value.attr_type() == ty)
+        let attr = u8::from(ty);
+        (self.attr_mask[(attr / 64) as usize] & (1u64 << (attr % 64))) != 0
     }
 
     pub fn get_attr(&self, ty: AttrType) -> Option<Attribute> {
@@ -156,9 +168,48 @@ impl Attributes {
     }
 
     pub fn add_attr(&mut self, attr: Attribute) {
+        let ty = u8::from(attr.value.attr_type());
+        self.attr_mask[(ty / 64) as usize] |= 1u64 << (ty % 64);
         self.inner.push(attr);
     }
 
+    /// Check for missing well-known mandatory attributes.
+    ///
+    /// RFC 4271 (BGP-4) and RFC 4760 (MP-BGP) define which attributes are mandatory.
+    /// - Pure Withdrawals: NO path attributes are required.
+    /// - Announcements: ORIGIN and AS_PATH are strictly required.
+    /// - NEXT_HOP is required if standard IPv4 NLRI is present or if no MP_REACH_NLRI is present.
+    pub fn check_mandatory_attributes(&mut self, is_announcement: bool, has_standard_nlri: bool) {
+        if !is_announcement {
+            return;
+        }
+
+        // ORIGIN and AS_PATH are universally mandatory for all announcements.
+        if !self.has_attr(AttrType::ORIGIN) {
+            self.validation_warnings
+                .push(BgpValidationWarning::MissingWellKnownAttribute {
+                    attr_type: AttrType::ORIGIN,
+                });
+        }
+        if !self.has_attr(AttrType::AS_PATH) {
+            self.validation_warnings
+                .push(BgpValidationWarning::MissingWellKnownAttribute {
+                    attr_type: AttrType::AS_PATH,
+                });
+        }
+
+        // NEXT_HOP is required if this is an IPv4 announcement (has standard NLRI)
+        // or if we haven't seen MP_REACH_NLRI,
+        // which implies standard NLRI is expected, since is_announcement.
+        let has_mp_reach = self.has_attr(AttrType::MP_REACHABLE_NLRI);
+        if (has_standard_nlri || !has_mp_reach) && !self.has_attr(AttrType::NEXT_HOP) {
+            self.validation_warnings
+                .push(BgpValidationWarning::MissingWellKnownAttribute {
+                    attr_type: AttrType::NEXT_HOP,
+                });
+        }
+    }
+
     /// Add a validation warning to the attributes
     pub fn add_validation_warning(&mut self, warning: BgpValidationWarning) {
         self.validation_warnings.push(warning);
@@ -324,27 +375,43 @@ impl Iterator for MetaCommunitiesIter<'_> {
     }
 }
 
+fn compute_mask(inner: &[Attribute]) -> [u64; 4] {
+    let mut attr_mask = [0; 4];
+    for attr in inner {
+        let ty = u8::from(attr.value.attr_type());
+        attr_mask[(ty / 64) as usize] |= 1u64 << (ty % 64);
+    }
+    attr_mask
+}
+
 impl FromIterator<Attribute> for Attributes {
     fn from_iter<T: IntoIterator<Item = Attribute>>(iter: T) -> Self {
+        let inner: Vec<Attribute> = iter.into_iter().collect();
+        let attr_mask = compute_mask(&inner);
         Attributes {
-            inner: iter.into_iter().collect(),
+            inner,
             validation_warnings: Vec::new(),
+            attr_mask,
         }
     }
 }
 
 impl From<Vec<Attribute>> for Attributes {
     fn from(value: Vec<Attribute>) -> Self {
+        let attr_mask = compute_mask(&value);
         Attributes {
             inner: value,
             validation_warnings: Vec::new(),
+            attr_mask,
         }
     }
 }
 
 impl Extend<Attribute> for Attributes {
     fn extend<T: IntoIterator<Item = Attribute>>(&mut self, iter: T) {
-        self.inner.extend(iter)
+        for attr in iter {
+            self.add_attr(attr);
+        }
     }
 }
 
@@ -356,9 +423,12 @@ impl Extend<AttributeValue> for Attributes {
 
 impl FromIterator<AttributeValue> for Attributes {
     fn from_iter<T: IntoIterator<Item = AttributeValue>>(iter: T) -> Self {
+        let inner: Vec<Attribute> = iter.into_iter().map(Attribute::from).collect();
+        let attr_mask = compute_mask(&inner);
         Attributes {
-            inner: iter.into_iter().map(Attribute::from).collect(),
+            inner,
             validation_warnings: Vec::new(),
+            attr_mask,
         }
     }
 }
@@ -400,9 +470,12 @@ mod serde_impl {
         where
             D: Deserializer<'de>,
         {
+            let inner = <Vec<Attribute>>::deserialize(deserializer)?;
+            let attr_mask = compute_mask(&inner);
             Ok(Attributes {
-                inner: <Vec<Attribute>>::deserialize(deserializer)?,
+                inner,
                 validation_warnings: Vec::new(),
+                attr_mask,
             })
         }
     }

src/parser/bgp/attributes/mod.rs

@@ -185,8 +185,15 @@ pub fn parse_attributes(
     let estimated_attrs = (data.remaining() / 3).min(256);
     let mut attributes: Vec<Attribute> = Vec::with_capacity(estimated_attrs.max(8));
     let mut validation_warnings: Vec<BgpValidationWarning> = Vec::new();
-    // boolean flags for seen attributes - small dataset in hot loop.
-    let mut seen_attributes: [bool; 256] = [false; 256];
+
+    // Bitmask for seen attributes (256 bits total, using 4x u64). Fits in registers/cache easily.
+    let mut seen_attributes: [u64; 4] = [0; 4];
+    let has_attr = |seen: &[u64; 4], attr: u8| -> bool {
+        (seen[(attr / 64) as usize] & (1u64 << (attr % 64))) != 0
+    };
+    let set_attr = |seen: &mut [u64; 4], attr: u8| {
+        seen[(attr / 64) as usize] |= 1u64 << (attr % 64);
+    };
 
     while data.remaining() >= 3 {
         // each attribute is at least 3 bytes: flag(1) + type(1) + length(1)
@@ -223,13 +230,13 @@ pub fn parse_attributes(
         let parsed_attr_type = AttrType::from(attr_type);
 
         // RFC 7606: Check for duplicate attributes
-        if seen_attributes[attr_type as usize] {
+        if has_attr(&seen_attributes, attr_type) {
             validation_warnings.push(BgpValidationWarning::DuplicateAttribute {
                 attr_type: parsed_attr_type,
             });
             // Continue processing - don't skip duplicate for now
         }
-        seen_attributes[attr_type as usize] = true;
+        set_attr(&mut seen_attributes, attr_type);
 
         // Validate attribute flags and length
         validate_attribute_flags(parsed_attr_type, flag, &mut validation_warnings);
@@ -378,25 +385,11 @@ 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
-    ];
-
-    for &mandatory_attr in &mandatory_attributes {
-        if !seen_attributes[u8::from(mandatory_attr) as usize] {
-            validation_warnings.push(BgpValidationWarning::MissingWellKnownAttribute {
-                attr_type: mandatory_attr,
-            });
-        }
-    }
-
-    let mut result = Attributes::from(attributes);
-    result.validation_warnings = validation_warnings;
-    Ok(result)
+    Ok(Attributes {
+        inner: attributes,
+        validation_warnings,
+        attr_mask: seen_attributes,
+    })
 }
 
 impl Attribute {
@@ -527,19 +520,25 @@ mod tests {
 
     #[test]
     fn test_rfc7606_missing_mandatory_attribute() {
-        // Empty attributes - should have warnings for missing mandatory attributes
-        let data = Bytes::from(vec![]);
+        // Attributes with only LOCAL_PREF (missing ORIGIN, AS_PATH, NEXT_HOP)
+        let data = Bytes::from(vec![
+            0x40, 0x05, 0x04, 0x00, 0x00, 0x00, 0x64, // LOCAL_PREF = 100
+        ]);
         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();
+        let mut attributes =
+            parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap();
+        // Manually trigger mandatory check as an announcement with standard NLRI
+        attributes.check_mandatory_attributes(true, true);
 
         // Should have warnings for missing mandatory attributes
         assert!(attributes.has_validation_warnings());
         let warnings = attributes.validation_warnings();
+        // LOCAL_PREF is not a withdrawal, so ORIGIN, AS_PATH, NEXT_HOP are required
         assert_eq!(warnings.len(), 3); // ORIGIN, AS_PATH, NEXT_HOP
 
         for warning in warnings {
@@ -555,6 +554,69 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_mp_reach_no_next_hop() {
+        // Attributes with MP_REACH_NLRI (missing ORIGIN, AS_PATH)
+        // MP_REACH_NLRI is type 14 (0x0E).
+        // We just need a dummy MP_REACH_NLRI.
+        let data = Bytes::from(vec![
+            0x80, 0x0E, 0x06, 0x00, 0x01, 0x01, // AFI=1, SAFI=1
+            0x00, // Next Hop Len = 0 (invalid for parsing, but enough to trigger logic)
+            0x00, // Reserved
+            0x00, // NLRI
+        ]);
+        let asn_len = AsnLength::Bits16;
+        let add_path = false;
+        let afi = None;
+        let safi = None;
+        let prefixes = None;
+
+        let mut attributes =
+            parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap();
+        // Manually trigger mandatory check as an announcement but NO standard NLRI (MP only)
+        attributes.check_mandatory_attributes(true, false);
+
+        // Should NOT have NEXT_HOP warning because MP_REACH_NLRI is present and has_standard_nlri is false
+        let warnings = attributes.validation_warnings();
+        let has_next_hop_warning = warnings.iter().any(|w| {
+            matches!(
+                w,
+                BgpValidationWarning::MissingWellKnownAttribute {
+                    attr_type: AttrType::NEXT_HOP
+                }
+            )
+        });
+        assert!(!has_next_hop_warning);
+    }
+
+    #[test]
+    fn test_pure_withdrawal_no_warnings() {
+        // Empty attributes - pure withdrawal
+        let data = Bytes::from(vec![]);
+        let asn_len = AsnLength::Bits16;
+        let add_path = false;
+        let afi = None;
+        let safi = None;
+        let prefixes = None;
+
+        let mut attributes =
+            parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap();
+        // Manually trigger mandatory check as a withdrawal
+        attributes.check_mandatory_attributes(false, false);
+
+        // Should have NO warnings
+        assert!(!attributes.has_validation_warnings());
+
+        // Attributes with only MP_UNREACH_NLRI - pure withdrawal
+        let data = Bytes::from(vec![
+            0x80, 0x0F, 0x03, 0x00, 0x01, 0x01, // AFI=1, SAFI=1
+        ]);
+        let mut attributes =
+            parse_attributes(data, &asn_len, add_path, afi, safi, prefixes).unwrap();
+        attributes.check_mandatory_attributes(false, false);
+        assert!(!attributes.has_validation_warnings());
+    }
+
     #[test]
     fn test_rfc7606_duplicate_attribute() {
         // Create two ORIGIN attributes

src/parser/bgp/messages.rs

@@ -468,12 +468,18 @@ 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 = 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)?;
 
+    // validate mandatory attributes
+    let is_announcement =
+        !announced_prefixes.is_empty() || attributes.has_attr(AttrType::MP_REACHABLE_NLRI);
+    let has_standard_nlri = !announced_prefixes.is_empty();
+    attributes.check_mandatory_attributes(is_announcement, has_standard_nlri);
+
     Ok(BgpUpdateMessage {
         withdrawn_prefixes,
         attributes,

src/parser/mrt/messages/table_dump.rs

@@ -99,9 +99,12 @@ 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 mut attributes =
         parse_attributes(attr_data_slice, &AsnLength::Bits16, false, None, None, None)?;
 
+    // validate mandatory attributes (TABLE_DUMP is always an announcement)
+    attributes.check_mandatory_attributes(true, afi == Afi::Ipv4);
+
     Ok(TableDumpMessage {
         view_number,
         sequence_number,