Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,14 @@ cargo fmt -- --check
- The `bgpkit-parser` binary requires the `cli` feature (`required-features = ["cli"]`).
- Examples under `examples/` may require specific features (see `[[example]]` entries in `Cargo.toml`).
- Benchmarks use Criterion (`[[bench]]` entries).

## Finding Missing Implementations

When looking for incomplete or missing BGP attribute implementations, check `src/parser/bgp/attributes/README.md`. This file contains:

1. **Unit Test Coverage** table — lists fully implemented attributes with RFC references
2. **Known Limitations** table — lists attributes that are:
- Type defined in `AttrType` enum but have no parser (e.g., PMSI_TUNNEL, BGPSEC_PATH)
- Model structs exist but parser/encoder not yet implemented (e.g., AIGP, ATTR_SET)

Use this file to identify which attributes need implementation work and their corresponding RFCs.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,20 @@ All notable changes to this project will be documented in this file.
* **Enhanced warning messages**: Added context to 14 warning messages across BGP, BMP, and MRT parsers to help identify which parsing stage encountered an issue. Also fixed typo in NLRI warning ("NRLI" → "NLRI").
* **Codecov configuration**: Added `require_base: true` to ensure Codecov only posts coverage reports when a valid base commit coverage exists, preventing comparisons against outdated baselines.

### Security improvements

* **AS_PATH segment count validation**: Added bounds check to prevent buffer over-read when parsing AS_PATH segments. Validates that `count * asn_len.bytes()` does not exceed remaining buffer size (CWE-126).
* **BGP message length underflow protection**: Added saturating arithmetic to prevent integer underflow when calculating message length (CWE-191).
* **NLRI prefix length validation**: Enforce maximum prefix lengths (32 bits for IPv4, 128 bits for IPv6) immediately after reading length byte (CWE-20).
* **BGP marker RFC 4271 compliance**: Fixed marker encoding to use 16 bytes of 0xFF as required by RFC 4271. Added validation on parse with warning for non-compliant markers.
* **Communities divisibility validation**: Validate that Communities (4 bytes), Extended Communities (8 bytes), and Large Communities (12 bytes) attribute lengths are properly divisible by their respective sizes.
* **FlowSpec traffic rate validation**: Added NaN and infinity checks for FlowSpec Traffic Rate extended communities to prevent undefined behavior in downstream calculations.
* **FlowSpec DSCP byte offset**: Corrected Traffic Marking extended community parsing per RFC 5575 (DSCP at proper byte offset).
* **Production code safety**: Replaced `assert_eq!` with `debug_assert_eq!` in production parsing code to prevent panics on malformed input.
* **Timestamp overflow handling**: Fixed timestamp truncation for values beyond 2106 (u32::MAX) using proper bounds checking.
* **Encoding truncation checks**: Added overflow warnings for `as u8`/`as u16` casts in encoding functions.
* **IPv4 enforcement for AGGREGATOR/ORIGINATOR_ID**: Encoders now reject IPv6 addresses for these attributes (BGP specifications require IPv4 only).

### Bug fixes

* **WASM `only_to_customer` serialization**: Added test to verify that messages without the OTC (Only To Customer) attribute correctly serialize `only_to_customer` as `null` (not `0`) in JSON output. This ensures JavaScript consumers can properly distinguish between "no OTC attribute" (`null`) and "OTC with AS 0" (`0`).
Expand Down
68 changes: 68 additions & 0 deletions src/models/bgp/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,66 @@ impl From<AttributeValue> for Attribute {
}
}

/// AIGP TLV (Type-Length-Value) entry - RFC 7311
#[derive(Debug, PartialEq, Clone, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct AigpTlv {
pub tlv_type: u8,
pub length: u16,
pub value: Vec<u8>,
}

/// AIGP (Accumulated IGP Metric) Attribute - RFC 7311
///
/// Type 26, optional non-transitive attribute containing TLVs.
/// The AIGP TLV (Type=1) contains an 8-octet accumulated metric value.
#[derive(Debug, PartialEq, Clone, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct Aigp {
pub tlvs: Vec<AigpTlv>,
}

impl Aigp {
/// Get the accumulated metric from the first AIGP TLV (Type=1)
pub fn accumulated_metric(&self) -> Option<u64> {
self.tlvs
.iter()
.find(|tlv| tlv.tlv_type == 1)
.and_then(|tlv| {
if tlv.value.len() >= 8 {
Some(u64::from_be_bytes([
tlv.value[0],
tlv.value[1],
tlv.value[2],
tlv.value[3],
tlv.value[4],
tlv.value[5],
tlv.value[6],
tlv.value[7],
]))
} else {
None
}
})
}
}

/// ATTR_SET Attribute - RFC 6368
///
/// Used in BGP/MPLS IP VPNs to transparently carry customer BGP path attributes
/// through the VPN core. Acts as a stack where attributes are "pushed" at the
/// PE ingress and "popped" at the PE egress.
///
/// Type 128, optional transitive attribute.
#[derive(Debug, PartialEq, Clone, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct AttrSet {
/// Origin AS number (customer network AS)
pub origin_as: Asn,
/// Nested path attributes
pub attributes: Attributes,
}

/// The `AttributeValue` enum represents different kinds of Attribute values.
#[derive(Debug, PartialEq, Clone, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down Expand Up @@ -477,6 +537,10 @@ pub enum AttributeValue {
Development(Vec<u8>),
Deprecated(AttrRaw),
Unknown(AttrRaw),
/// AIGP (Accumulated IGP Metric) attribute - RFC 7311
Aigp(Aigp),
/// ATTR_SET attribute - RFC 6368
AttrSet(AttrSet),
}

impl From<Origin> for AttributeValue {
Expand Down Expand Up @@ -534,6 +598,8 @@ impl AttributeValue {
AttributeValue::TunnelEncapsulation(_) => AttrType::TUNNEL_ENCAPSULATION,
AttributeValue::Development(_) => AttrType::DEVELOPMENT,
AttributeValue::Deprecated(x) | AttributeValue::Unknown(x) => x.attr_type,
AttributeValue::Aigp(_) => AttrType::AIGP,
AttributeValue::AttrSet(_) => AttrType::ATTR_SET,
}
}

Expand All @@ -559,6 +625,8 @@ impl AttributeValue {
AttributeValue::MpReachNlri(_) => Some(OptionalNonTransitive),
AttributeValue::MpUnreachNlri(_) => Some(OptionalNonTransitive),
AttributeValue::LinkState(_) => Some(OptionalNonTransitive),
AttributeValue::Aigp(_) => Some(OptionalNonTransitive),
AttributeValue::AttrSet(_) => Some(OptionalTransitive),
_ => None,
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/models/network/asn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ impl AsnLength {
AsnLength::Bits32 => true,
}
}

/// Return the number of bytes used to encode an ASN of this length.
pub const fn bytes(&self) -> usize {
match self {
AsnLength::Bits16 => 2,
AsnLength::Bits32 => 4,
}
}
}

/// ASN -- Autonomous System Number
Expand Down
29 changes: 29 additions & 0 deletions src/parser/bgp/attributes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,40 @@
| Large Community | [RFC8092][rfc8092] | 32 | Yes |
| Only To Customer | [RFC9234][rfc9234] | 35 | Yes |

## Known Limitations

| Path Attribute | RFC | Type Code | Status | Notes |
|---------------------------------|-------------------------------|-----------|-----------------------------|----------------------------------|
| AIGP (Accumulated IGP Metric) | [RFC7311][rfc7311] | 26 | Type defined, model only | Parser and encoding not yet implemented |
| ATTR_SET | [RFC6368][rfc6368] | 128 | Type defined, model only | Parser and encoding not yet implemented |
| PMSI_TUNNEL | [RFC6514][rfc6514] | 22 | Type defined in enum | Parser not implemented |
| TRAFFIC_ENGINEERING | [RFC5543][rfc5543] | 24 | Type defined in enum | Parser not implemented |
| IPv6_EXT_COMMUNITIES | [RFC5701][rfc5701] | 25 | ✅ Implemented | Listed in main table above |
| PE_DISTINGUISHER_LABELS | [RFC6514][rfc6514] | 27 | Type defined in enum | Parser not implemented |
| BGPSEC_PATH | [RFC8205][rfc8205] | 33 | Type defined in enum | Parser not implemented |
| SFP_ATTRIBUTE | [RFC9015][rfc9015] | 37 | Type defined in enum | Parser not implemented |
| BFD_DISCRIMINATOR | [RFC9026][rfc9026] | 38 | Type defined in enum | Parser not implemented |
| BGP_PREFIX_SID | [RFC8669][rfc8669] | 40 | Type defined in enum | Parser not implemented |

**Legend:**
- **Type defined**: Attribute type code is defined in `AttrType` enum (models)
- **Model only**: Data structures exist but parser/encoder not implemented
- **Type defined in enum**: Only the type code exists, no parser or model

[rfc1997]: https://datatracker.ietf.org/doc/html/rfc1997
[rfc4271]: https://datatracker.ietf.org/doc/html/rfc4271#section-4.3
[rfc4360]: https://datatracker.ietf.org/doc/html/rfc4360
[rfc4456]: https://datatracker.ietf.org/doc/html/rfc4456
[rfc4760]: https://datatracker.ietf.org/doc/html/rfc4760
[rfc5543]: https://datatracker.ietf.org/doc/html/rfc5543
[rfc5701]: https://datatracker.ietf.org/doc/html/rfc5701
[rfc6368]: https://datatracker.ietf.org/doc/html/rfc6368
[rfc6514]: https://datatracker.ietf.org/doc/html/rfc6514
[rfc7311]: https://datatracker.ietf.org/doc/html/rfc7311
[rfc8092]: https://datatracker.ietf.org/doc/html/rfc8092
[rfc8205]: https://datatracker.ietf.org/doc/html/rfc8205
[rfc8669]: https://datatracker.ietf.org/doc/html/rfc8669
[rfc9015]: https://datatracker.ietf.org/doc/html/rfc9015
[rfc9026]: https://datatracker.ietf.org/doc/html/rfc9026
[rfc9234]: https://datatracker.ietf.org/doc/html/rfc9234
[iana-bgp]: https://www.iana.org/assignments/bgp-parameters/bgp-parameters.xhtml
48 changes: 48 additions & 0 deletions src/parser/bgp/attributes/attr_02_17_as_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ fn parse_as_path_segment(
) -> Result<AsPathSegment, ParserError> {
let segment_type = input.read_u8()?;
let count = input.read_u8()? as usize;

// Validate AS_PATH segment count against remaining buffer size
// to prevent reading beyond buffer bounds
let required_bytes = count
.checked_mul(asn_len.bytes())
.ok_or_else(|| ParserError::ParseError("AS path segment too large".to_string()))?;
if input.remaining() < required_bytes {
return Err(ParserError::TruncatedMsg(format!(
"AS_PATH segment requires {} bytes, only {} remaining",
required_bytes,
input.remaining()
)));
}

let path = input.read_asns(asn_len, count)?;
match segment_type {
AS_PATH_AS_SET => Ok(AsPathSegment::AsSet(path)),
Expand Down Expand Up @@ -242,4 +256,38 @@ mod tests {
let encoded_bytes = encode_as_path(&path, AsnLength::Bits16);
assert_eq!(data, encoded_bytes);
}

#[test]
fn test_as_path_count_validation() {
// Test that excessive count value is rejected (16-bit ASN)
// count=100, but buffer only has 2 bytes
let data = Bytes::from(vec![
2, // sequence
100, // 100 ASes claimed, but buffer is too small
0, 1, // Only 2 bytes available (1 ASN, not 100)
]);
let result = parse_as_path(data, &AsnLength::Bits16);
assert!(result.is_err());
assert!(matches!(result, Err(ParserError::TruncatedMsg(_))));

// Test that excessive count value is rejected (32-bit ASN)
// count=50, but buffer only has 4 bytes
let data = Bytes::from(vec![
2, // sequence
50, // 50 ASes claimed (need 200 bytes), but buffer is too small
0, 0, 0, 1, // Only 4 bytes available (1 ASN, not 50)
]);
let result = parse_as_path(data, &AsnLength::Bits32);
assert!(result.is_err());
assert!(matches!(result, Err(ParserError::TruncatedMsg(_))));

// Test valid count passes
let data = Bytes::from(vec![
2, // sequence
2, // 2 ASes
0, 1, 0, 2, // 2 ASNs (4 bytes for 16-bit)
]);
let result = parse_as_path(data, &AsnLength::Bits16);
assert!(result.is_ok());
}
}
8 changes: 8 additions & 0 deletions src/parser/bgp/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,14 @@ impl Attribute {
AttributeValue::Development(v) => Bytes::from(v.to_owned()),
AttributeValue::Deprecated(v) => Bytes::from(v.bytes.to_owned()),
AttributeValue::Unknown(v) => Bytes::from(v.bytes.to_owned()),
AttributeValue::Aigp(_v) => {
// AIGP encoding not yet implemented - return empty bytes
Bytes::new()
}
AttributeValue::AttrSet(_v) => {
// ATTR_SET encoding not yet implemented - return empty bytes
Bytes::new()
}
};

match self.is_extended() {
Expand Down
Loading
Loading