Property tests and fuzz targets for the parsers, and the bugs they found

Property tests (run by `cargo test`):
- crates/packet/tests/properties.rs: every kind of message, generated,
  must be written, be as long as wire_len says, and read back equal; and
  noise, noise behind a valid header, and valid messages with bytes damaged
  must never do more than return an error. Anything the decoder accepts
  must be showable and survive being written and read again.
- IpPrefix: text round trip, noise as text, `contains` against a bitwise
  model.
- The session's framing: a stream of messages yields the same messages
  however it is cut into reads.

Fuzz targets (fuzz/, cargo-fuzz on nightly, a minute each in CI): raw
messages, message bodies behind a valid header, prefix text, the
configuration file.

Bugs found, each with a named regression test:
- A prefix longer than its address family (e.g. an IPv4 NLRI of length 99)
  was accepted, and showing it panicked. The session logs every route, so
  one UPDATE from a peer ended the session task. Such a prefix is now
  malformed, and showing a prefix cannot fail.
- Bits past a prefix's length were kept, so the same prefix sent with stray
  bits was a different key in the RIB. Prefixes now have one form.
- An opaque extended community was read as seven octets instead of eight,
  putting every community after it out of step.
- AGGREGATOR could be neither read from nor written to a peer with
  two-octet AS numbers.
- The Outbound Route Filter and Long-Lived Graceful Restart capabilities
  were written differently from how they are read.
- A capability value or unknown attribute too long for its length field
  was written with the length wrapped around; it is now an error, or for
  the attribute the two-octet length.

Also: ROUTE-REFRESH carries its AFI and SAFI instead of being modelled as
an empty message, and one with a body of the wrong length is refused.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Rayhaan JaufeerallyandClaude Fable 5.1 committed 2026-10-01 22:54:37 +02:00
1 parent b0762874e0
commit 39632e3c38
62 files changed
+1437 -103

No files matched your search

+22
View File
@@ -26,3 +26,25 @@ jobs:
run: cargo clippy --workspace --all-targets -- -D warnings
- name: Test
run: cargo test --workspace
# A short run of each fuzz target: enough to notice a parser change that
# breaks on input the seeds are close to. Long runs are done by hand; see
# fuzz/README.md.
fuzz:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
- name: Install cargo-fuzz
run: cargo install cargo-fuzz --locked
- name: Fuzz
working-directory: fuzz
# cargo-fuzz sets its own flags; the workspace's -D warnings would
# replace them.
env:
RUSTFLAGS: ""
run: |
for target in message message_body prefix_text config; do
mkdir -p "corpus/$target"
cargo fuzz run "$target" "corpus/$target" "seeds/$target" -- -max_total_time=60
done
Generated
+148 -2
View File
@@ -182,6 +182,7 @@ dependencies = [
"bytes",
"eyre",
"nom",
"proptest",
"serde",
"serde_repr",
"strum",
@@ -195,6 +196,7 @@ dependencies = [
"bitfield",
"bytes",
"nom",
"proptest",
"serde",
"serde_repr",
"strum",
@@ -224,6 +226,7 @@ dependencies = [
"clap",
"futures",
"prometheus",
"proptest",
"protobuf",
"protobuf-codegen",
"thiserror 2.0.12",
@@ -248,6 +251,21 @@ dependencies = [
"tracing",
]
[[package]]
name = "bit-set"
version = "0.8.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3"
dependencies = [
"bit-vec",
]
[[package]]
name = "bit-vec"
version = "0.8.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7"
[[package]]
name = "bitfield"
version = "0.19.0"
@@ -270,9 +288,9 @@ dependencies = [
[[package]]
name = "bitflags"
version = "2.9.0"
version = "2.13.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5c8214115b7bf84099f1309324e63141d4c5d7cc26862f97a0a857dbefe165bd"
checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06"
[[package]]
name = "bumpalo"
@@ -863,6 +881,15 @@ dependencies = [
"windows-sys 0.61.2",
]
[[package]]
name = "num-traits"
version = "0.2.19"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841"
dependencies = [
"autocfg",
]
[[package]]
name = "once_cell"
version = "1.21.1"
@@ -919,6 +946,15 @@ dependencies = [
"zerovec",
]
[[package]]
name = "ppv-lite86"
version = "0.2.21"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9"
dependencies = [
"zerocopy",
]
[[package]]
name = "proc-macro2"
version = "1.0.94"
@@ -943,6 +979,25 @@ dependencies = [
"thiserror 2.0.12",
]
[[package]]
name = "proptest"
version = "1.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4b45fcc2344c680f5025fe57779faef368840d0bd1f42f216291f0dc4ace4744"
dependencies = [
"bit-set",
"bit-vec",
"bitflags",
"num-traits",
"rand",
"rand_chacha",
"rand_xorshift",
"regex-syntax",
"rusty-fork",
"tempfile",
"unarray",
]
[[package]]
name = "protobuf"
version = "3.7.2"
@@ -994,6 +1049,12 @@ dependencies = [
"thiserror 1.0.69",
]
[[package]]
name = "quick-error"
version = "1.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0"
[[package]]
name = "quote"
version = "1.0.47"
@@ -1009,6 +1070,44 @@ version = "5.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f"
[[package]]
name = "rand"
version = "0.9.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b9ef1d0d795eb7d84685bca4f72f3649f064e6641543d3a8c415898726a57b41"
dependencies = [
"rand_chacha",
"rand_core",
]
[[package]]
name = "rand_chacha"
version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb"
dependencies = [
"ppv-lite86",
"rand_core",
]
[[package]]
name = "rand_core"
version = "0.9.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "76afc826de14238e6e8c374ddcc1fa19e374fd8dd986b0d2af0d02377261d83c"
dependencies = [
"getrandom",
]
[[package]]
name = "rand_xorshift"
version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "513962919efc330f829edb2535844d1b912b0fbe2ca165d613e4e8788bb05a5a"
dependencies = [
"rand_core",
]
[[package]]
name = "redox_syscall"
version = "0.5.10"
@@ -1110,6 +1209,18 @@ version = "1.0.20"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eded382c5f5f786b989652c49544c4877d9f015cc22e145a5ea8ea66c2921cd2"
[[package]]
name = "rusty-fork"
version = "0.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cc6bf79ff24e648f6da1f8d1f011e9cac26491b619e6b9280f2b47f1774e6ee2"
dependencies = [
"fnv",
"quick-error",
"tempfile",
"wait-timeout",
]
[[package]]
name = "ryu"
version = "1.0.23"
@@ -1554,6 +1665,12 @@ version = "0.2.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b"
[[package]]
name = "unarray"
version = "0.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94"
[[package]]
name = "unicode-ident"
version = "1.0.18"
@@ -1590,6 +1707,15 @@ version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65"
[[package]]
name = "wait-timeout"
version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "09ac3b126d3914f9849036f826e054cbabdc8519970b8998ddaf3b5bd3c65f11"
dependencies = [
"libc",
]
[[package]]
name = "want"
version = "0.3.1"
@@ -1814,6 +1940,26 @@ dependencies = [
"synstructure",
]
[[package]]
name = "zerocopy"
version = "0.8.59"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6df92bf3d9227be3d53173901ddbffac2babc27ae50f397776ffd6dc33f800cb"
dependencies = [
"zerocopy-derive",
]
[[package]]
name = "zerocopy-derive"
version = "0.8.59"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ac4f328cf2f05d084e496c3e9c3f33ed0a183656a16e1fcec4d464d8373aec82"
dependencies = [
"proc-macro2",
"quote",
"syn 2.0.100",
]
[[package]]
name = "zerofrom"
version = "0.1.8"
+3
View File
@@ -1,6 +1,8 @@
[workspace]
default-members = ["bin/bgp"]
members = ["bin/*", "crates/*"]
# Fuzz targets need a nightly toolchain; see fuzz/README.md.
exclude = ["fuzz"]
resolver = "3"
[workspace.package]
@@ -46,6 +48,7 @@ strum = { version = "0.27.1", features = ["derive"] }
tracing = "0.1.44"
tracing-subscriber = { version = "0.3.22", features = ["env-filter"] }
prometheus = "0.14.0"
proptest = "1.7"
protobuf = { package = "protobuf", version = "3.7.2" }
protobuf-codegen = { package = "protobuf-codegen", version = "3.7.2" }
clap = { version = "4.5.53", features = ["derive"] }
+11
View File
@@ -41,6 +41,17 @@ cargo test --workspace
Lints are configured once, under `[workspace.lints]` in the root `Cargo.toml`.
`unsafe` code is forbidden throughout.
`cargo test` includes property tests: generated messages must survive being
written and read back, and no input, however damaged, may make a parser
panic. They take about a second; for a longer hunt:
```
PROPTEST_CASES=1000000 cargo test --release -p bgp-packet --test properties
```
Coverage-guided fuzz targets for the same parsers are in `fuzz/`; see
`fuzz/README.md`. CI runs each of them for a minute.
## License
Apache License 2.0; see `LICENSE`.
+1
View File
@@ -20,6 +20,7 @@ bgp-primitives.workspace = true
[dev-dependencies]
eyre.workspace = true
proptest.workspace = true
[lints]
workspace = true
+175 -30
View File
@@ -45,7 +45,7 @@ pub const BGP_MAX_MSG_LEN: u16 = 4096;
const BGP_MARKER: [u8; 16] = [0xFF; 16];
/// Message represents the top-level messages in the BGP protocol.
#[derive(Debug, Serialize, Deserialize, EnumDiscriminants)]
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, EnumDiscriminants)]
#[repr(u8)]
pub enum Message {
Open(OpenMessage) = 1,
@@ -53,7 +53,7 @@ pub enum Message {
Notification(NotificationMessage) = 3,
KeepAlive = 4,
/// BGP Route Refresh message (RFC 2918).
RouteRefresh = 5,
RouteRefresh(RouteRefreshMessage) = 5,
}
impl Message {
@@ -148,7 +148,14 @@ impl Message {
.map_err(|e| bad_length(&format!("malformed NOTIFICATION ({})", describe(&e)))),
4 if body.is_empty() => Ok(Self::KeepAlive),
4 => Err(bad_length("KEEPALIVE with a body")),
5 => Ok(Self::RouteRefresh),
5 => match body {
[afi_hi, afi_lo, subtype, safi] => Ok(Self::RouteRefresh(RouteRefreshMessage {
afi: Afi(u16::from_be_bytes([*afi_hi, *afi_lo])),
subtype: *subtype,
safi: Safi(*safi),
})),
_ => Err(bad_length("ROUTE-REFRESH")),
},
other => Err(header_error(
MsgHeaderSubcode::BadMessageType,
vec![other],
@@ -181,7 +188,12 @@ impl Message {
Self::Open(m) => m.to_wire(ctx, &mut body)?,
Self::Update(m) => m.to_wire(ctx, &mut body)?,
Self::Notification(m) => m.to_wire(ctx, &mut body)?,
Self::KeepAlive | Self::RouteRefresh => {}
Self::RouteRefresh(m) => {
body.put_u16(m.afi.0);
body.put_u8(m.subtype);
body.put_u8(m.safi.0);
}
Self::KeepAlive => {}
}
let total_len = usize::from(BGP_HEADER_LEN) + body.len();
@@ -206,7 +218,8 @@ impl Message {
Self::Open(m) => m.wire_len(ctx)?,
Self::Update(m) => m.wire_len(ctx)?,
Self::Notification(m) => m.wire_len(),
Self::KeepAlive | Self::RouteRefresh => 0,
Self::RouteRefresh(_) => 4,
Self::KeepAlive => 0,
};
Ok(BGP_HEADER_LEN + body_len)
}
@@ -217,11 +230,36 @@ impl Message {
Self::Update(_) => 2,
Self::Notification(_) => 3,
Self::KeepAlive => 4,
Self::RouteRefresh => 5,
Self::RouteRefresh(_) => 5,
}
}
}
/// A request to send again every route of one address family (RFC 2918).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub struct RouteRefreshMessage {
pub afi: Afi,
/// Zero for a plain request. RFC 7313 uses the octet, reserved in
/// RFC 2918, to mark the beginning and end of a refresh.
pub subtype: u8,
pub safi: Safi,
}
impl RouteRefreshMessage {
/// A plain request for the routes of `family`.
pub fn request(family: AfiSafi) -> Self {
Self {
afi: family.afi,
subtype: 0,
safi: family.safi,
}
}
pub fn family(&self) -> AfiSafi {
AfiSafi::new(self.afi, self.safi)
}
}
/// Represents a BGP Update message.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct UpdateMessage {
@@ -661,12 +699,23 @@ impl PathAttribute {
type_code,
payload,
} => {
// The flags are sent as they came, except that a payload too
// long for a one-octet length gets the two-octet one.
let length = u16::try_from(payload.len()).map_err(|_| EncodeError::TooLong {
what: "path attribute",
len: payload.len(),
max: u16::MAX.into(),
})?;
let mut flags = flags.clone();
if length > 255 {
flags.set_extended_length(true);
}
out.put_u8(flags.0);
out.put_u8(*type_code);
if flags.extended_length() {
out.put_u16(payload.len() as u16);
out.put_u16(length);
} else {
out.put_u8(payload.len() as u8);
out.put_u8(length as u8);
}
out.put(payload.as_slice());
}
@@ -693,16 +742,16 @@ impl PathAttribute {
PathAttribute::As4Path(a) => a.wire_len_with_width(true)?,
PathAttribute::LargeCommunities(a) => a.wire_len(ctx)?,
PathAttribute::UnknownPathAttribute { flags, payload, .. } => {
// An unknown attribute preserves its original extended_length
// flag on re-serialization, so use it verbatim here rather than
// re-deriving it from the payload length.
// An unknown attribute keeps the length width it came with,
// unless its payload needs the wider one.
let payload_len =
u16::try_from(payload.len()).map_err(|_| EncodeError::TooLong {
what: "path attribute",
len: payload.len(),
max: u16::MAX.into(),
})?;
let header_len: u16 = if flags.extended_length() { 4 } else { 3 };
let extended = flags.extended_length() || payload_len > 255;
let header_len: u16 = if extended { 4 } else { 3 };
return Ok(header_len + payload_len);
}
};
@@ -1002,17 +1051,21 @@ pub struct AggregatorPathAttribute {
}
impl AggregatorPathAttribute {
/// The AS number is as wide as the session's AS numbers (RFC 6793), so
/// the attribute is six or eight octets.
pub fn from_wire<'a>(
ctx: &ParserContext,
buf: &'a [u8],
) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> {
if ctx.four_octet_asn.is_none_or(|val| !val) {
return Err(nom::Err::Failure(BgpParserError::Malformed(
"AggregatorPathAttribute can only be parsed for four_octet_asn enabled peers"
.into(),
)));
}
let (buf, asn) = be_u32(buf)?;
let (buf, asn) = match ctx.four_octet_asn {
Some(true) => be_u32(buf)?,
Some(false) => be_u16(buf).map(|(buf, asn)| (buf, u32::from(asn)))?,
None => {
return Err(nom::Err::Failure(BgpParserError::Malformed(
"Context must set four_octet_asn before being used".into(),
)));
}
};
let (buf, ip) = be_u32(buf)?;
Ok((
buf,
@@ -1023,19 +1076,20 @@ impl AggregatorPathAttribute {
))
}
/// On a two-octet session an AS number that does not fit is written as
/// AS_TRANS (RFC 6793 §4.2.2).
pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> {
if ctx.four_octet_asn.is_none_or(|val| !val) {
return Err(EncodeError::Unencodable(
"AGGREGATOR can only be sent to peers with four-octet AS numbers",
));
if width(ctx)? {
out.put_u32(self.asn);
} else {
out.put_u16(u16::try_from(self.asn).unwrap_or(AS_TRANS));
}
out.put_u32(self.asn);
out.put_u32(self.ip.into());
Ok(())
}
pub fn wire_len(&self, _: &ParserContext) -> Result<u16, EncodeError> {
Ok(8)
pub fn wire_len(&self, ctx: &ParserContext) -> Result<u16, EncodeError> {
Ok(if width(ctx)? { 8 } else { 6 })
}
}
@@ -1120,7 +1174,8 @@ pub enum ExtendedCommunity {
Opaque {
typ: u8,
sub_typ: u8,
value: [u8; 5],
/// The six octets after the type and sub-type.
value: [u8; 6],
},
/// Route Target Community as specified in Section 4 of RFC4360.
RouteTarget {
@@ -1203,10 +1258,10 @@ impl ExtendedCommunity {
)
}
_ => {
let (buf, payload) = nom::bytes::take(5_usize).parse(buf)?;
let value: [u8; 5] = payload.try_into().map_err(|_| {
let (buf, payload) = nom::bytes::take(6_usize).parse(buf)?;
let value: [u8; 6] = payload.try_into().map_err(|_| {
Failure(BgpParserError::Malformed(
"Expected exactly 5 bytes from the parser".into(),
"Expected exactly 6 bytes from the parser".into(),
))
})?;
(
@@ -2027,6 +2082,96 @@ mod tests {
);
// Rejects trailing garbage after the NLRI section (a partial prefix).
// Found by the property tests in tests/properties.rs.
/// A prefix longer than its address family allows was accepted, and
/// showing it (as the session does when it logs a route) panicked. A
/// peer could end the session task with one UPDATE.
#[test]
fn a_prefix_longer_than_an_address_is_malformed() {
let ctx = default_v6_context();
// No withdrawn routes, no attributes, NLRI of length 99.
let mut body = vec![0, 0, 0, 0, 99];
body.extend_from_slice(&[0xff; 13]);
assert!(UpdateMessage::from_wire(&ctx, &body).is_err());
let mut unreach = vec![0x00, 0x02, 0x01, 129];
unreach.extend_from_slice(&[0xff; 17]);
assert!(MpUnreachNlriPathAttribute::from_wire(&ctx, &unreach).is_err());
}
/// The bits of a prefix past its length mean nothing (RFC 4271 §4.3).
/// They used to be kept, which made 10.0.0.0/8 sent with stray bits a
/// different route from 10.0.0.0/8.
#[test]
fn bits_past_the_prefix_length_are_cleared() {
let ctx = default_v6_context();
// 10.255.0.0/12 as sent, with the low four bits of the second octet set.
let body = [0, 0, 0, 0, 12, 10, 0xff];
let (_, update) = UpdateMessage::from_wire(&ctx, &body).unwrap();
assert_eq!(update.announced_routes, vec![v4_prefix(12, &[10, 0xf0])]);
assert_eq!(update.announced_routes[0].to_string(), "10.240.0.0/12");
}
/// An extended community is eight octets whatever its type. The opaque
/// kind was read as seven, which put every community after it out of
/// step.
#[test]
fn an_opaque_extended_community_is_eight_octets() {
let ctx = default_v6_context();
#[rustfmt::skip]
let wire = [
0x03, 0x0c, 1, 2, 3, 4, 5, 6, // opaque
0x00, 0x02, 0, 0, 0xfd, 0xe8, 0, 100, // route target 65000:100
];
let (rest, parsed) = ExtendedCommunitiesPathAttribute::from_wire(&ctx, &wire).unwrap();
assert!(rest.is_empty());
assert_eq!(
parsed.extended_communities,
vec![
ExtendedCommunity::Opaque {
typ: 0x03,
sub_typ: 0x0c,
value: [1, 2, 3, 4, 5, 6],
},
ExtendedCommunity::RouteTarget {
typ: 0x00,
sub_typ: 0x02,
global_admin: 65000,
local_admin: 100,
},
]
);
let mut out = BytesMut::new();
parsed.to_wire(&ctx, &mut out).unwrap();
assert_eq!(&out[..], &wire[..]);
}
/// AGGREGATOR carries an AS number as wide as the session's. It could
/// neither be read from nor written to a two-octet peer, so one such
/// attribute ended the session.
#[test]
fn aggregator_is_six_octets_on_a_two_octet_session() {
let two_octet = ParserContext {
four_octet_asn: Some(false),
address_family: None,
};
let wire = [0xfd, 0xe8, 192, 0, 2, 1];
let (rest, parsed) = AggregatorPathAttribute::from_wire(&two_octet, &wire).unwrap();
assert!(rest.is_empty());
assert_eq!(
parsed,
AggregatorPathAttribute {
asn: 65000,
ip: Ipv4Addr::new(192, 0, 2, 1),
}
);
let mut out = BytesMut::new();
parsed.to_wire(&two_octet, &mut out).unwrap();
assert_eq!(&out[..], &wire[..]);
assert_eq!(parsed.wire_len(&two_octet).unwrap(), 6);
}
/// The reserved octet after the next hop is there for every family. It
/// used to be skipped only for IPv6, so an IPv4 MP_REACH_NLRI gained a
/// 0.0.0.0/0 route that the peer never sent.
+79 -19
View File
@@ -25,7 +25,7 @@ macro_rules! fail {
/// Contains messages related to the BGP Open message,
/// i.e. the Open message itself and options / capabilities.
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)]
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct OpenMessage {
/// Version of the BGP protocol in use.
pub version: u8,
@@ -149,7 +149,7 @@ impl OpenMessage {
}
}
#[derive(Debug, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)]
#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)]
#[repr(u8)]
pub enum OpenOption {
Capabilities(Vec<Capability>) = 2,
@@ -231,7 +231,7 @@ pub struct MultiLabelEntry {
pub count: u8,
}
#[derive(Debug, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)]
#[derive(Debug, Clone, Serialize, Deserialize, EnumDiscriminants, PartialEq, Eq)]
#[repr(u8)]
pub enum Capability {
/// MultiProtocol Extension (RFC 2858).
@@ -583,7 +583,7 @@ impl Capability {
pub fn to_wire(&self, _ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> {
out.put_u8(self.discriminant());
out.put_u8(self.payload_len());
out.put_u8(self.payload_len()?);
match self {
Capability::MultiProtocol { afi, safi } => {
out.put_u16(afi.0);
@@ -595,6 +595,8 @@ impl Capability {
out.put_u16(afi.0);
out.put_u8(0); // Reserved byte.
out.put_u8(safi.0);
// payload_len checked that there are few enough to count.
out.put_u8(orfs.len() as u8);
for orf in orfs {
out.put_u8(orf.r#type.clone().into());
out.put_u8(orf.send_recv.clone() as u8);
@@ -677,28 +679,33 @@ impl Capability {
}
pub fn wire_len(&self, _ctx: &ParserContext) -> Result<u16, EncodeError> {
Ok(2 + self.payload_len() as u16)
Ok(2 + u16::from(self.payload_len()?))
}
fn payload_len(&self) -> u8 {
match self {
/// The length of the capability's value, which has to fit the one octet
/// that carries it.
fn payload_len(&self) -> Result<u8, EncodeError> {
let len = match self {
Capability::MultiProtocol { .. } => 4,
Capability::RouteRefresh => 0,
Capability::OutboundRouteFilter { orfs, .. } => 4 + 2 * orfs.len() as u8,
Capability::ExtendedNextHop { entries } => 6 * entries.len() as u8,
Capability::OutboundRouteFilter { orfs, .. } => 5 + 2 * orfs.len(),
Capability::ExtendedNextHop { entries } => 6 * entries.len(),
Capability::ExtendedMessage => 0,
Capability::BgpSec { .. } => 3,
Capability::MultiLabelCompat { entries } => 4 * entries.len() as u8,
Capability::MultiLabelCompat { entries } => 4 * entries.len(),
Capability::BgpRole { .. } => 1,
Capability::GracefulRestart { entries, .. } => 2 + 4 * entries.len() as u8,
Capability::GracefulRestart { entries, .. } => 2 + 4 * entries.len(),
Capability::FourByteAsn { .. } => 4,
Capability::AddPath { .. } => 4,
Capability::EnhancedRouteRefresh => 0,
Capability::LongLivedGracefulRestart(long_lived_graceful_restarts) => {
8 * long_lived_graceful_restarts.len() as u8
}
Capability::UnknownCapability { value, .. } => value.len() as u8,
}
Capability::LongLivedGracefulRestart(entries) => 7 * entries.len(),
Capability::UnknownCapability { value, .. } => value.len(),
};
u8::try_from(len).map_err(|_| EncodeError::TooLong {
what: "capability",
len,
max: u8::MAX.into(),
})
}
}
@@ -876,12 +883,12 @@ bitfield! {
/// Represents an entry in the BGP Graceful Restart capability.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct GracefulRestartEntry {
afi: Afi,
safi: Safi,
pub afi: Afi,
pub safi: Safi,
/// Forwarding State (F) bit, which can be used to indicate whether the
/// forwarding state for routes that were advertised with the given AFI
/// and SAFI has indeed been preserved during the previous BGP restart.
preserve_forwarding: bool,
pub preserve_forwarding: bool,
}
#[cfg(test)]
@@ -1147,6 +1154,59 @@ mod tests {
assert!(Capability::from_wire(&ctx, &[0x01, 0x03, 0x00, 0x01, 0x00]).is_err());
}
// Found by the property tests in tests/properties.rs: both were written
// differently from how they are read, so an OPEN of ours carrying one
// would have been malformed.
#[test]
fn outbound_route_filter_capability_is_written_with_its_count() {
let ctx = ParserContext::default();
// IPv4 unicast, one ORF: address prefix (64), both directions.
let wire = [0x03, 0x07, 0x00, 0x01, 0x00, 0x01, 0x01, 0x40, 0x03];
let (rest, cap) = Capability::from_wire(&ctx, &wire).unwrap();
assert!(rest.is_empty());
let mut out = BytesMut::new();
cap.to_wire(&ctx, &mut out).unwrap();
assert_eq!(&out[..], &wire[..]);
}
#[test]
fn long_lived_graceful_restart_entries_are_seven_octets() {
let ctx = ParserContext::default();
// IPv4 unicast, no flags, stale time 3600 s.
let wire = [0x47, 0x07, 0x00, 0x01, 0x01, 0x00, 0x00, 0x0e, 0x10];
let (rest, cap) = Capability::from_wire(&ctx, &wire).unwrap();
assert!(rest.is_empty());
let mut out = BytesMut::new();
cap.to_wire(&ctx, &mut out).unwrap();
assert_eq!(&out[..], &wire[..]);
assert_eq!(cap.wire_len(&ctx).unwrap(), 9);
}
/// A capability's value has a one-octet length. One that does not fit
/// used to be written with the length wrapped around.
#[test]
fn a_capability_too_long_for_its_length_octet_is_refused() {
let ctx = ParserContext::default();
let entry = ExtendedNextHop {
afi: Afi::IPV4,
safi: Safi::UNICAST,
nh_afi: Afi::IPV6,
};
let fits = Capability::ExtendedNextHop {
entries: vec![entry.clone(); 42],
};
let mut out = BytesMut::new();
fits.to_wire(&ctx, &mut out).unwrap();
assert_eq!(out.len(), 2 + 252);
let too_long = Capability::ExtendedNextHop {
entries: vec![entry; 43],
};
assert!(too_long.wire_len(&ctx).is_err());
assert!(too_long.to_wire(&ctx, &mut BytesMut::new()).is_err());
}
#[test]
fn capability_parsers_leave_the_following_bytes() {
let ctx = ParserContext::default();
+568
View File
@@ -0,0 +1,568 @@
//! Property tests for the wire codec.
//!
//! Two things are checked for every kind of message:
//!
//! - **Round trip.** A message that can be written can be read back, comes
//! back equal, and is exactly as long as `wire_len` said.
//! - **Hostile input.** Whatever bytes arrive, decoding returns a message or
//! an error: it never panics. What it does accept can be shown, written
//! again, and read back as the same message.
//!
//! The second is what a peer can do to us, so it is run on three kinds of
//! input: plain noise, noise behind a valid header (so it reaches the body
//! parsers), and valid messages with a few bytes damaged (so it reaches deep
//! into them).
use std::net::{Ipv4Addr, Ipv6Addr};
use bgp_packet::errors::BgpError;
use bgp_packet::message::{
AggregatorPathAttribute, AsPathAttribute, AsPathSegment, AtomicAggregatePathAttribute,
BGP_HEADER_LEN, BGP_MAX_MSG_LEN, CommunitiesPathAttribute, ExtendedCommunitiesPathAttribute,
ExtendedCommunity, LargeCommunitiesPathAttribute, LargeCommunity, LocalPrefPathAttribute,
Message, MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, MultiExitDiscPathAttribute,
NextHopPathAttribute, NlriNextHop, NotificationMessage, OriginPathAttribute, PathAttribute,
PathAttributeFlags, RouteRefreshMessage, UpdateMessage,
};
use bgp_packet::open::{
AddPathSendRecv, BgpRole, BgpSecHeader, Capability, ExtendedNextHop, GracefulRestartEntry,
LongLivedGracefulRestart, MultiLabelEntry, OpenMessage, OpenOption, OrfSendRecv, OrfType,
OutboundRouteFilterEntry,
};
use bgp_primitives::constants::{AddressFamilyId, Afi, Safi};
use bgp_primitives::ip_prefix::IpPrefix;
use bgp_primitives::parser::{EncodeError, ParserContext};
use bytes::BytesMut;
use proptest::collection::vec;
use proptest::prelude::*;
fn ctx(four_octet_asn: bool) -> ParserContext {
ParserContext {
four_octet_asn: Some(four_octet_asn),
address_family: None,
}
}
// ---------------------------------------------------------------------------
// Generators
// ---------------------------------------------------------------------------
fn prefix(version: AddressFamilyId) -> impl Strategy<Value = IpPrefix> {
let max_len = match version {
AddressFamilyId::Ipv4 => 32u8,
AddressFamilyId::Ipv6 => 128,
};
(0..=max_len, any::<[u8; 16]>()).prop_map(move |(length, bytes)| {
IpPrefix::new(version, bytes.to_vec(), length).expect("16 bytes are enough for any prefix")
})
}
fn afi() -> impl Strategy<Value = Afi> {
any::<u16>().prop_map(Afi)
}
fn safi() -> impl Strategy<Value = Safi> {
any::<u8>().prop_map(Safi)
}
/// AS numbers that fit the session: any on a four-octet one, two-octet ones
/// otherwise (wider ones are replaced by AS_TRANS on the wire, by design).
fn asn(four_octet_asn: bool) -> BoxedStrategy<u32> {
if four_octet_asn {
any::<u32>().boxed()
} else {
any::<u16>().prop_map(u32::from).boxed()
}
}
fn as_path(asn: BoxedStrategy<u32>) -> impl Strategy<Value = AsPathAttribute> {
vec((any::<bool>(), vec(asn, 0..6)), 0..4).prop_map(|segments| AsPathAttribute {
segments: segments
.into_iter()
.map(|(ordered, path)| AsPathSegment { ordered, path })
.collect(),
})
}
/// Extended communities are classified by their type octets when read, so
/// they are generated from the eight octets they are on the wire.
fn extended_community() -> impl Strategy<Value = ExtendedCommunity> {
any::<[u8; 8]>().prop_map(|bytes| {
ExtendedCommunity::from_wire(&ctx(true), &bytes)
.expect("any eight octets are an extended community")
.1
})
}
fn mp_reach() -> impl Strategy<Value = MpReachNlriPathAttribute> {
let ip_safi = prop_oneof![Just(Safi::UNICAST), Just(Safi::MULTICAST)];
let v4 = (
ip_safi.clone(),
any::<[u8; 4]>(),
vec(prefix(AddressFamilyId::Ipv4), 0..5),
)
.prop_map(|(safi, next_hop, prefixes)| MpReachNlriPathAttribute {
afi: Afi::IPV4,
safi,
next_hop: NlriNextHop::Ipv4(Ipv4Addr::from(next_hop)),
prefixes,
});
let v6_next_hop = prop_oneof![
any::<[u8; 16]>().prop_map(|a| NlriNextHop::Ipv6(Ipv6Addr::from(a))),
(any::<[u8; 16]>(), any::<[u8; 16]>()).prop_map(|(global, link_local)| {
NlriNextHop::Ipv6WithLl {
global: Ipv6Addr::from(global),
link_local: Ipv6Addr::from(link_local),
}
}),
];
let v6 = (
ip_safi,
v6_next_hop,
vec(prefix(AddressFamilyId::Ipv6), 0..5),
)
.prop_map(|(safi, next_hop, prefixes)| MpReachNlriPathAttribute {
afi: Afi::IPV6,
safi,
next_hop,
prefixes,
});
prop_oneof![v4, v6]
}
fn mp_unreach() -> impl Strategy<Value = MpUnreachNlriPathAttribute> {
let of = |version: AddressFamilyId| {
vec(prefix(version), 0..5).prop_map(move |prefixes| MpUnreachNlriPathAttribute {
afi: version.into(),
safi: Safi::UNICAST,
prefixes,
})
};
prop_oneof![of(AddressFamilyId::Ipv4), of(AddressFamilyId::Ipv6)]
}
/// Type codes that have their own variant; an unknown attribute has none of
/// these.
const KNOWN_ATTRIBUTES: [u8; 13] = [1, 2, 3, 4, 5, 6, 7, 8, 14, 15, 16, 17, 32];
fn unknown_attribute() -> impl Strategy<Value = PathAttribute> {
(
any::<u8>().prop_filter("a type code without a variant", |code| {
!KNOWN_ATTRIBUTES.contains(code)
}),
any::<u8>(),
vec(any::<u8>(), 0..300),
)
.prop_map(|(type_code, flags, payload)| {
// Bit 4 selects the two-octet length, which a payload this
// long needs.
let flags = PathAttributeFlags(if payload.len() > 255 {
flags | 0x10
} else {
flags
});
PathAttribute::UnknownPathAttribute {
flags,
type_code,
payload,
}
})
}
fn path_attribute(four_octet_asn: bool) -> impl Strategy<Value = PathAttribute> {
prop_oneof![
prop_oneof![
Just(OriginPathAttribute::IGP),
Just(OriginPathAttribute::EGP),
Just(OriginPathAttribute::INCOMPLETE)
]
.prop_map(PathAttribute::Origin),
as_path(asn(four_octet_asn)).prop_map(PathAttribute::AsPath),
any::<[u8; 4]>()
.prop_map(|a| PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::from(a)))),
any::<u32>().prop_map(|v| PathAttribute::MultiExitDisc(MultiExitDiscPathAttribute(v))),
any::<u32>().prop_map(|v| PathAttribute::LocalPref(LocalPrefPathAttribute(v))),
Just(PathAttribute::AtomicAggregate(
AtomicAggregatePathAttribute {}
)),
(asn(four_octet_asn), any::<[u8; 4]>()).prop_map(|(asn, ip)| {
PathAttribute::Aggregator(AggregatorPathAttribute {
asn,
ip: Ipv4Addr::from(ip),
})
}),
vec(any::<(u16, u16)>(), 0..8)
.prop_map(|c| PathAttribute::Communities(CommunitiesPathAttribute(c))),
mp_reach().prop_map(PathAttribute::MpReachNlri),
mp_unreach().prop_map(PathAttribute::MpUnreachNlri),
vec(extended_community(), 1..6).prop_map(|extended_communities| {
PathAttribute::ExtendedCommunities(ExtendedCommunitiesPathAttribute {
extended_communities,
})
}),
// AS4_PATH is four octets wide on every session.
as_path(any::<u32>().boxed()).prop_map(PathAttribute::As4Path),
vec(any::<(u32, u32, u32)>(), 1..6).prop_map(|communities| {
PathAttribute::LargeCommunities(LargeCommunitiesPathAttribute {
communities: communities
.into_iter()
.map(|(global_admin, data_1, data_2)| LargeCommunity {
global_admin,
data_1,
data_2,
})
.collect(),
})
}),
unknown_attribute(),
]
}
fn update(four_octet_asn: bool) -> impl Strategy<Value = UpdateMessage> {
(
vec(prefix(AddressFamilyId::Ipv4), 0..5),
vec(path_attribute(four_octet_asn), 0..8),
vec(prefix(AddressFamilyId::Ipv4), 0..5),
)
.prop_map(
|(withdrawn_routes, path_attributes, announced_routes)| UpdateMessage {
withdrawn_routes,
path_attributes,
announced_routes,
},
)
}
/// Capability codes that have their own variant.
const KNOWN_CAPABILITIES: [u8; 13] = [1, 2, 3, 5, 6, 7, 8, 9, 64, 65, 69, 70, 71];
fn capability() -> impl Strategy<Value = Capability> {
let orf = (
any::<u8>().prop_map(OrfType::from),
prop_oneof![
Just(OrfSendRecv::Receive),
Just(OrfSendRecv::Send),
Just(OrfSendRecv::Both)
],
)
.prop_map(|(r#type, send_recv)| OutboundRouteFilterEntry { r#type, send_recv });
let add_path = prop_oneof![
Just(AddPathSendRecv::Receive),
Just(AddPathSendRecv::Send),
Just(AddPathSendRecv::Both)
];
prop_oneof![
(afi(), safi()).prop_map(|(afi, safi)| Capability::MultiProtocol { afi, safi }),
Just(Capability::RouteRefresh),
(afi(), safi(), vec(orf, 0..4))
.prop_map(|(afi, safi, orfs)| Capability::OutboundRouteFilter { afi, safi, orfs }),
vec((afi(), safi(), afi()), 1..4).prop_map(|entries| Capability::ExtendedNextHop {
entries: entries
.into_iter()
.map(|(afi, safi, nh_afi)| ExtendedNextHop { afi, safi, nh_afi })
.collect(),
}),
Just(Capability::ExtendedMessage),
(any::<u8>(), afi()).prop_map(|(header, afi)| Capability::BgpSec {
header: BgpSecHeader(header),
afi,
}),
vec((afi(), safi(), any::<u8>()), 1..4).prop_map(|entries| {
Capability::MultiLabelCompat {
entries: entries
.into_iter()
.map(|(afi, safi, count)| MultiLabelEntry { afi, safi, count })
.collect(),
}
}),
any::<u8>().prop_map(|role| Capability::BgpRole {
role: BgpRole::from(role),
}),
(
any::<bool>(),
0u16..4096,
vec((afi(), safi(), any::<bool>()), 0..4)
)
.prop_map(|(restart_state, restart_time, entries)| {
Capability::GracefulRestart {
restart_state,
restart_time,
entries: entries
.into_iter()
.map(|(afi, safi, preserve_forwarding)| GracefulRestartEntry {
afi,
safi,
preserve_forwarding,
})
.collect(),
}
}),
any::<u32>().prop_map(|asn| Capability::FourByteAsn { asn }),
(afi(), safi(), add_path).prop_map(|(afi, safi, send_recv)| Capability::AddPath {
afi,
safi,
send_recv,
}),
Just(Capability::EnhancedRouteRefresh),
vec((afi(), safi(), any::<u8>(), 0u32..(1 << 24)), 0..4).prop_map(|entries| {
Capability::LongLivedGracefulRestart(
entries
.into_iter()
.map(|(afi, safi, flags, stale_time)| LongLivedGracefulRestart {
afi,
safi,
flags,
stale_time,
})
.collect(),
)
}),
(
any::<u8>().prop_filter("a code without a variant", |code| {
!KNOWN_CAPABILITIES.contains(code)
}),
vec(any::<u8>(), 0..20)
)
.prop_map(|(type_code, value)| Capability::UnknownCapability {
type_code,
length: value.len() as u8,
value,
}),
]
}
fn open() -> impl Strategy<Value = OpenMessage> {
(
any::<u16>(),
any::<u16>(),
any::<[u8; 4]>(),
// Enough to go past 255 octets of options now and then, which
// switches to the extended length of RFC 9072.
vec(vec(capability(), 0..10), 0..8),
)
.prop_map(|(asn, hold_time, identifier, options)| OpenMessage {
version: 4,
asn,
hold_time,
identifier: Ipv4Addr::from(identifier),
options: options.into_iter().map(OpenOption::Capabilities).collect(),
})
}
fn notification() -> impl Strategy<Value = NotificationMessage> {
(1u8..=8, 0u8..12, vec(any::<u8>(), 0..40)).prop_filter_map(
"an error code and subcode that are defined",
|(code, subcode, data)| {
BgpError::from_code_subcode(code, subcode)
.ok()
.map(|error| NotificationMessage { error, data })
},
)
}
fn message(four_octet_asn: bool) -> impl Strategy<Value = Message> {
prop_oneof![
open().prop_map(Message::Open),
update(four_octet_asn).prop_map(Message::Update),
notification().prop_map(Message::Notification),
Just(Message::KeepAlive),
(afi(), any::<u8>(), safi()).prop_map(|(afi, subtype, safi)| {
Message::RouteRefresh(RouteRefreshMessage { afi, subtype, safi })
}),
]
}
/// A session's AS number width, and a message that fits it.
fn session_and_message() -> impl Strategy<Value = (bool, Message)> {
any::<bool>().prop_flat_map(|four_octet_asn| (Just(four_octet_asn), message(four_octet_asn)))
}
// ---------------------------------------------------------------------------
// Properties
// ---------------------------------------------------------------------------
/// What must hold for anything the decoder accepts: it can be shown,
/// written, and read back as itself.
fn check_accepted(ctx: &ParserContext, msg: &Message) -> Result<(), TestCaseError> {
// Display of what a peer sent must not be able to panic: it is logged.
let _ = format!("{msg:?}");
if let Message::Update(update) = msg {
for prefix in update
.withdrawn_routes
.iter()
.chain(&update.announced_routes)
{
let _ = prefix.to_string();
}
for attribute in &update.path_attributes {
match attribute {
PathAttribute::MpReachNlri(mp) => {
mp.prefixes.iter().for_each(|p| drop(p.to_string()))
}
PathAttribute::MpUnreachNlri(mp) => {
mp.prefixes.iter().for_each(|p| drop(p.to_string()))
}
_ => {}
}
}
}
let mut wire = BytesMut::new();
msg.to_wire(ctx, &mut wire)
.map_err(|e| TestCaseError::fail(format!("accepted but cannot be written: {e}")))?;
prop_assert_eq!(usize::from(msg.wire_len(ctx).unwrap()), wire.len());
let again = Message::decode(ctx, &wire)
.map_err(|e| TestCaseError::fail(format!("written but cannot be read: {e}")))?;
prop_assert_eq!(&again, msg);
Ok(())
}
/// Writes a generated message. `None` when it is too big for one BGP
/// message, which the generators can produce and the encoder must refuse;
/// any other refusal is a bug, since everything generated is meant to fit
/// the session.
fn written(ctx: &ParserContext, msg: &Message) -> Result<Option<BytesMut>, TestCaseError> {
let mut wire = BytesMut::new();
match msg.to_wire(ctx, &mut wire) {
Ok(()) => Ok(Some(wire)),
Err(EncodeError::TooLong { .. }) => Ok(None),
Err(e) => Err(TestCaseError::fail(format!("cannot be written: {e}"))),
}
}
fn framed(msg_type: u8, body: &[u8]) -> Vec<u8> {
let mut wire = vec![0xff; 16];
let length = u16::try_from(usize::from(BGP_HEADER_LEN) + body.len()).unwrap();
wire.extend_from_slice(&length.to_be_bytes());
wire.push(msg_type);
wire.extend_from_slice(body);
wire
}
/// Ways to damage a message.
#[derive(Debug, Clone)]
enum Damage {
Set { at: prop::sample::Index, to: u8 },
Remove { at: prop::sample::Index },
Insert { at: prop::sample::Index, byte: u8 },
Truncate { to: prop::sample::Index },
}
fn damage() -> impl Strategy<Value = Damage> {
prop_oneof![
(any::<prop::sample::Index>(), any::<u8>()).prop_map(|(at, to)| Damage::Set { at, to }),
any::<prop::sample::Index>().prop_map(|at| Damage::Remove { at }),
(any::<prop::sample::Index>(), any::<u8>())
.prop_map(|(at, byte)| Damage::Insert { at, byte }),
any::<prop::sample::Index>().prop_map(|to| Damage::Truncate { to }),
]
}
/// Applies `damage` to the body of `wire` and makes the header's length
/// right again, so that the damage is the body parsers' problem.
fn damaged(wire: &[u8], damage: &[Damage]) -> Vec<u8> {
let header = usize::from(BGP_HEADER_LEN);
let msg_type = wire[header - 1];
let mut body = wire[header..].to_vec();
for d in damage {
match d {
Damage::Set { at, to } if !body.is_empty() => {
let i = at.index(body.len());
body[i] = *to;
}
Damage::Remove { at } if !body.is_empty() => {
body.remove(at.index(body.len()));
}
Damage::Insert { at, byte } => body.insert(at.index(body.len() + 1), *byte),
Damage::Truncate { to } if !body.is_empty() => body.truncate(to.index(body.len())),
_ => {}
}
}
body.truncate(usize::from(BGP_MAX_MSG_LEN - BGP_HEADER_LEN));
framed(msg_type, &body)
}
/// 2000 cases a property, which takes about a second. For a longer hunt
/// set `PROPTEST_CASES`, e.g. `PROPTEST_CASES=1000000 cargo test --release
/// -p bgp-packet --test properties`.
fn config() -> ProptestConfig {
let cases = std::env::var("PROPTEST_CASES")
.ok()
.and_then(|cases| cases.parse().ok())
.unwrap_or(2000);
ProptestConfig {
cases,
// Failures are not saved to a file: one that is found becomes a
// named test next to the code it broke.
failure_persistence: None,
..ProptestConfig::default()
}
}
proptest! {
#![proptest_config(config())]
#[test]
fn a_message_that_can_be_written_reads_back_equal((four_octet_asn, msg) in session_and_message()) {
let ctx = ctx(four_octet_asn);
let Some(wire) = written(&ctx, &msg)? else {
return Ok(());
};
prop_assert_eq!(usize::from(msg.wire_len(&ctx).unwrap()), wire.len());
let decoded = Message::decode(&ctx, &wire)
.map_err(|e| TestCaseError::fail(format!("{e}")))?;
prop_assert_eq!(&decoded, &msg);
let (rest, _) = Message::from_wire(&ctx, &wire).unwrap();
prop_assert!(rest.is_empty());
}
#[test]
fn noise_is_never_more_than_an_error(four_octet_asn: bool, bytes in vec(any::<u8>(), 0..200)) {
let ctx = ctx(four_octet_asn);
if let Ok(msg) = Message::decode(&ctx, &bytes) {
check_accepted(&ctx, &msg)?;
}
}
#[test]
fn noise_behind_a_valid_header_is_never_more_than_an_error(
four_octet_asn: bool,
msg_type in 0u8..7,
body in vec(any::<u8>(), 0..300),
) {
let ctx = ctx(four_octet_asn);
if let Ok(msg) = Message::decode(&ctx, &framed(msg_type, &body)) {
check_accepted(&ctx, &msg)?;
}
}
#[test]
fn a_damaged_message_is_never_more_than_an_error(
(four_octet_asn, msg) in session_and_message(),
damage in vec(damage(), 1..4),
) {
let ctx = ctx(four_octet_asn);
let Some(wire) = written(&ctx, &msg)? else {
return Ok(());
};
if let Ok(msg) = Message::decode(&ctx, &damaged(&wire, &damage)) {
check_accepted(&ctx, &msg)?;
}
}
/// The parsers below the message level are public too.
#[test]
fn noise_is_never_more_than_an_error_for_the_parts(
four_octet_asn: bool,
bytes in vec(any::<u8>(), 0..120),
) {
let ctx = ctx(four_octet_asn);
let _ = OpenMessage::from_wire(&ctx, &bytes);
let _ = OpenOption::from_wire(&ctx, &bytes);
let _ = Capability::from_wire(&ctx, &bytes);
let _ = UpdateMessage::from_wire(&ctx, &bytes);
let _ = PathAttribute::from_wire(&ctx, &bytes);
let _ = NotificationMessage::from_wire(&ctx, &bytes);
}
}
+3
View File
@@ -17,5 +17,8 @@ serde_repr.workspace = true
thiserror.workspace = true
strum.workspace = true
[dev-dependencies]
proptest.workspace = true
[lints]
workspace = true
+10
View File
@@ -18,6 +18,16 @@ pub enum AddressFamilyId {
Ipv6 = 2,
}
impl AddressFamilyId {
/// How many bits an address of this version has.
pub fn bits(self) -> u8 {
match self {
Self::Ipv4 => 32,
Self::Ipv6 => 128,
}
}
}
impl TryFrom<u16> for AddressFamilyId {
type Error = UnknownAfi;
+99 -49
View File
@@ -40,6 +40,10 @@ impl IpPrefix {
prefix: Vec<u8>,
length: u8,
) -> Result<Self, PrefixError> {
let max = address_family.bits();
if length > max {
return Err(PrefixError::LengthTooLong { length, max });
}
// Ensure that the prefix we are given contains the right number of bytes corresponding to the prefix length.
let needed = usize::from(length.div_ceil(8));
if prefix.len() < needed {
@@ -49,11 +53,26 @@ impl IpPrefix {
got: prefix.len(),
});
}
Ok(Self {
Ok(Self::normalised(address_family, prefix, length))
}
/// Builds the one representation of a prefix: exactly the bytes its
/// length covers, with the bits past the length cleared. Those bits mean
/// nothing (RFC 4271 §4.3), so two prefixes that differ only there are
/// the same prefix and must compare equal.
///
/// `length` must fit the address family and `prefix` must have at least
/// the bytes it covers.
fn normalised(address_family: AddressFamilyId, mut prefix: Vec<u8>, length: u8) -> Self {
prefix.truncate(usize::from(length.div_ceil(8)));
if let (Some(last), rem @ 1..) = (prefix.last_mut(), length % 8) {
*last &= u8::MAX << (8 - rem);
}
Self {
address_family,
prefix,
length,
})
}
}
/// Returns true if `other` is equal to or more specific than `self`, i.e.
@@ -75,6 +94,16 @@ impl IpPrefix {
(self.prefix[full_bytes] & mask) == (other.prefix[full_bytes] & mask)
}
/// The prefix as a whole address, zero where the prefix says nothing.
/// The fields are public, so this cannot assume `prefix` has a sensible
/// length: showing a prefix must never fail, whatever is in it.
fn padded<const N: usize>(&self) -> [u8; N] {
let mut address = [0u8; N];
let len = self.prefix.len().min(N);
address[..len].copy_from_slice(&self.prefix[..len]);
address
}
pub fn to_wire(&self, _: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> {
// Verify that there is enough space to write the IpPrefix.
if out.remaining() < (self.prefix.len() + 1) {
@@ -98,36 +127,14 @@ impl TryFrom<&str> for IpPrefix {
let (addr, length) = value.split_once('/').ok_or_else(format)?;
let length: u8 = length.parse().map_err(|_| format())?;
let (afi, mut octets, max) = if addr.contains(':') {
let (afi, octets) = if addr.contains(':') {
let addr = Ipv6Addr::from_str(addr).map_err(|_| address())?;
(AddressFamilyId::Ipv6, addr.octets().to_vec(), 128)
(AddressFamilyId::Ipv6, addr.octets().to_vec())
} else {
let addr = Ipv4Addr::from_str(addr).map_err(|_| address())?;
(AddressFamilyId::Ipv4, addr.octets().to_vec(), 32)
(AddressFamilyId::Ipv4, addr.octets().to_vec())
};
if length > max {
return Err(PrefixError::LengthTooLong { length, max });
}
// Truncate the octets we have to the right number of bytes to match the prefix length..
if length.is_multiple_of(8) {
// We can cleanly truncate the number of bytes since we are at a byte boundary.
octets.truncate((length / 8).into());
} else {
// We need to keep length % 8 bits of the last byte.
let num_bytes = (length / 8) + 1;
let mask = u8::MAX << (8 - (length % 8));
octets.truncate(num_bytes.into());
// Fix up the last byte.
let last_pos = octets.len() - 1;
octets[last_pos] &= mask;
}
Ok(IpPrefix {
address_family: afi,
prefix: octets,
length,
})
IpPrefix::new(afi, octets, length)
}
}
@@ -135,29 +142,10 @@ impl Display for IpPrefix {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.address_family {
AddressFamilyId::Ipv4 => {
// Pad the prefix with 0 bytes if it's less than 4 bytes long.
let bytes = &mut self.prefix.clone();
if bytes.len() < 4 {
bytes.extend(std::iter::repeat_n(0, 4 - bytes.len()));
}
let four_bytes: [u8; 4] = bytes
.as_slice()
.try_into()
.map_err(|_| std::fmt::Error {})?;
let ipv4_addr = Ipv4Addr::from(four_bytes);
write!(f, "{}/{}", ipv4_addr, self.length)
write!(f, "{}/{}", Ipv4Addr::from(self.padded()), self.length)
}
AddressFamilyId::Ipv6 => {
let bytes = &mut self.prefix.clone();
if bytes.len() < 16 {
bytes.extend(std::iter::repeat_n(0, 16 - bytes.len()));
}
let sixteen_bytes: [u8; 16] = bytes
.as_slice()
.try_into()
.map_err(|_| std::fmt::Error {})?;
let ipv6_addr = Ipv6Addr::from(sixteen_bytes);
write!(f, "{}/{}", ipv6_addr, self.length)
write!(f, "{}/{}", Ipv6Addr::from(self.padded()), self.length)
}
}
}
@@ -167,6 +155,8 @@ impl Display for IpPrefix {
mod tests {
use crate::constants::AddressFamilyId;
use proptest::prelude::*;
use super::{IpPrefix, PrefixError};
macro_rules! verify_roundtrip {
@@ -273,6 +263,66 @@ mod tests {
);
}
#[test]
fn a_prefix_has_one_representation() {
// Extra bytes and stray bits past the length are dropped.
let noisy = IpPrefix::new(AddressFamilyId::Ipv4, vec![10, 0xff, 3, 4], 12).unwrap();
assert_eq!(noisy, IpPrefix::try_from("10.240.0.0/12").unwrap());
assert_eq!(noisy.prefix, vec![10, 0xf0]);
assert_eq!(
IpPrefix::new(AddressFamilyId::Ipv4, vec![0xff; 13], 99),
Err(PrefixError::LengthTooLong {
length: 99,
max: 32
})
);
}
/// The fields are public, so a prefix can be put together wrongly.
/// Showing it must still not fail: prefixes are shown in log lines.
#[test]
fn showing_a_prefix_never_fails() {
let wrong = IpPrefix {
address_family: AddressFamilyId::Ipv4,
prefix: vec![1, 2, 3, 4, 5, 6],
length: 200,
};
assert_eq!(wrong.to_string(), "1.2.3.4/200");
}
proptest! {
#[test]
fn text_round_trips(version: bool, bytes: [u8; 16], length in 0u8..=128) {
let version = if version { AddressFamilyId::Ipv6 } else { AddressFamilyId::Ipv4 };
let length = length.min(version.bits());
let prefix = IpPrefix::new(version, bytes.to_vec(), length).unwrap();
let text = prefix.to_string();
prop_assert_eq!(IpPrefix::try_from(text.as_str()), Ok(prefix.clone()));
prop_assert!(prefix.contains(&prefix));
}
#[test]
fn text_that_is_noise_is_an_error_or_a_prefix_that_round_trips(text in "\\PC{0,40}") {
if let Ok(prefix) = IpPrefix::try_from(text.as_str()) {
let shown = prefix.to_string();
prop_assert_eq!(IpPrefix::try_from(shown.as_str()), Ok(prefix));
}
}
/// `contains` agrees with comparing the leading bits by hand.
#[test]
fn contains_is_a_comparison_of_leading_bits(
a: [u8; 4], a_len in 0u8..=32, b: [u8; 4], b_len in 0u8..=32,
) {
let outer = IpPrefix::new(AddressFamilyId::Ipv4, a.to_vec(), a_len).unwrap();
let inner = IpPrefix::new(AddressFamilyId::Ipv4, b.to_vec(), b_len).unwrap();
let mask = if a_len == 0 { 0 } else { u32::MAX << (32 - a_len) };
let expected = b_len >= a_len
&& (u32::from_be_bytes(a) & mask) == (u32::from_be_bytes(b) & mask);
prop_assert_eq!(outer.contains(&inner), expected);
}
}
#[test]
fn contains_covers_more_specifics() {
let p = |s: &str| IpPrefix::try_from(s).unwrap();
+3
View File
@@ -24,5 +24,8 @@ thiserror.workspace = true
[build-dependencies]
protobuf-codegen.workspace = true
[dev-dependencies]
proptest.workspace = true
[lints]
workspace = true
+60
View File
@@ -431,6 +431,10 @@ fn prefix_counts(update: &UpdateMessage) -> (usize, usize) {
mod tests {
use std::time::Duration;
use bgp_packet::errors::CeaseSubcode;
use bgp_packet::message::NotificationMessage;
use proptest::collection::vec;
use proptest::prelude::*;
use tokio::io::{AsyncWriteExt, duplex};
use super::*;
@@ -482,6 +486,62 @@ mod tests {
assert!(matches!(frames.next(&ctx).await, Ok(Message::KeepAlive)));
}
proptest! {
/// TCP delivers a stream, not messages: however the bytes of a run
/// of messages are cut into reads, the same messages come out.
#[test]
fn framing_does_not_depend_on_how_the_stream_is_cut(
bodies in vec(vec(any::<u8>(), 0..60), 1..8),
cuts in vec(1usize..40, 0..30),
) {
// NOTIFICATIONs, since their body may be any bytes after a
// valid code and subcode.
let sent: Vec<Message> = bodies
.into_iter()
.map(|data| {
Message::Notification(NotificationMessage {
error: BgpError::Cease(CeaseSubcode::AdminShutdown),
data,
})
})
.collect();
let ctx = ParserContext::default();
let mut stream = BytesMut::new();
for msg in &sent {
msg.to_wire(&ctx, &mut stream).unwrap();
}
let runtime = tokio::runtime::Builder::new_current_thread().build().unwrap();
let received = runtime.block_on(async {
let (mut peer, ours) = duplex(8192);
let mut frames = FrameReader::new(ours);
let writer = async {
let mut rest = &stream[..];
for cut in cuts.iter().copied().chain(std::iter::repeat(40)) {
if rest.is_empty() {
break;
}
let (now, later) = rest.split_at(cut.min(rest.len()));
peer.write_all(now).await.unwrap();
// Let the reader see this piece on its own.
tokio::task::yield_now().await;
rest = later;
}
drop(peer);
};
let reader = async {
let mut received = Vec::new();
while let Ok(msg) = frames.next(&ctx).await {
received.push(msg);
}
received
};
tokio::join!(writer, reader).1
});
prop_assert_eq!(received, sent);
}
}
#[tokio::test]
async fn an_impossible_length_is_rejected_from_the_header_alone() {
let (mut peer, ours) = duplex(4096);
+5 -2
View File
@@ -227,7 +227,7 @@ impl Fsm {
actions.push(Action::DeliverUpdate(update));
actions
}
(Phase::Established(negotiated), Message::RouteRefresh) => {
(Phase::Established(negotiated), Message::RouteRefresh(_)) => {
let mut actions = heard_from_peer(negotiated);
actions.push(Action::RouteRefreshRequested);
actions
@@ -372,6 +372,7 @@ mod tests {
use bgp_packet::errors::{CeaseSubcode, FsmSubcode, MsgHeaderSubcode};
use bgp_packet::message::{
MpReachNlriPathAttribute, NlriNextHop, NotificationMessage, PathAttribute,
RouteRefreshMessage,
};
use bgp_primitives::constants::{Afi, AfiSafi, Safi};
use bgp_primitives::ip_prefix::IpPrefix;
@@ -790,7 +791,9 @@ mod tests {
[Action::ResetHoldTimer(d), Action::DeliverUpdate(_)] if *d == hold
));
let actions = fsm.handle(Event::Message(Message::RouteRefresh));
let actions = fsm.handle(Event::Message(Message::RouteRefresh(
RouteRefreshMessage::request(AfiSafi::IPV4_UNICAST),
)));
assert!(matches!(
&actions[..],
[Action::ResetHoldTimer(d), Action::RouteRefreshRequested] if *d == hold
+1 -1
View File
@@ -82,6 +82,6 @@ pub fn message_type_name(msg: &Message) -> &'static str {
Message::Update(_) => "update",
Message::Notification(_) => "notification",
Message::KeepAlive => "keepalive",
Message::RouteRefresh => "route_refresh",
Message::RouteRefresh(_) => "route_refresh",
}
}
+6
View File
@@ -0,0 +1,6 @@
# What a fuzzing run produces. Seeds worth keeping live in seeds/.
target/
corpus/
artifacts/
coverage/
Cargo.lock
+54
View File
@@ -0,0 +1,54 @@
# Coverage-guided fuzz targets, run with cargo-fuzz on a nightly toolchain.
# See README.md in this directory.
[package]
name = "bgp-fuzz"
version = "0.0.0"
edition = "2024"
license = "Apache-2.0"
publish = false
[package.metadata]
cargo-fuzz = true
# Not part of the main workspace: libfuzzer needs nightly, the workspace
# builds on stable.
[workspace]
[dependencies]
bgp-packet = { path = "../crates/packet" }
bgp-primitives = { path = "../crates/primitives" }
bgp-server = { path = "../crates/server" }
bytes = "1.10.1"
libfuzzer-sys = "0.4"
protobuf = "3.7.2"
[lib]
path = "src/lib.rs"
[[bin]]
name = "message"
path = "fuzz_targets/message.rs"
test = false
doc = false
bench = false
[[bin]]
name = "message_body"
path = "fuzz_targets/message_body.rs"
test = false
doc = false
bench = false
[[bin]]
name = "prefix_text"
path = "fuzz_targets/prefix_text.rs"
test = false
doc = false
bench = false
[[bin]]
name = "config"
path = "fuzz_targets/config.rs"
test = false
doc = false
bench = false
+47
View File
@@ -0,0 +1,47 @@
# Fuzz targets
Coverage-guided fuzzing of everything that reads input we do not control.
The property tests (`cargo test`, e.g. `crates/packet/tests/properties.rs`)
check the same things on generated input and run on every build; these
targets go further by letting libFuzzer follow code coverage for as long as
you leave them running.
| Target | Input |
|----------------|------------------------------------------------------------------|
| `message` | Raw bytes as they come off a TCP connection |
| `message_body` | A message body behind a correct header (reaches the body parsers) |
| `prefix_text` | A prefix as text |
| `config` | The configuration file |
For the two message targets a finding is either a panic, or a message the
decoder accepted that cannot be written and read back as itself.
## Running
libFuzzer needs a nightly toolchain and `cargo-fuzz`:
```bash
rustup toolchain install nightly
cargo install cargo-fuzz
```
Then, from this directory:
```bash
cargo +nightly fuzz run message_body corpus/message_body seeds/message_body -- -max_total_time=300
```
The first directory is the working corpus, which the fuzzer adds to and
which is not checked in. `seeds/` holds a few valid inputs to start from.
Leave out `-max_total_time` to run until interrupted.
## When it finds something
The input is saved under `artifacts/<target>/`. Reproduce it with
```bash
cargo +nightly fuzz run message_body artifacts/message_body/crash-...
```
Fix the bug, and add a unit test with the input next to the code it broke,
so that `cargo test` keeps it fixed without anyone having to run the fuzzer.
+13
View File
@@ -0,0 +1,13 @@
//! The configuration file. It is written by the operator and not by a peer,
//! but a typo in it should be an error message, never a panic at startup.
#![no_main]
use bgp_server::config::Config;
use bgp_server::proto::config::BgpConfig;
use libfuzzer_sys::fuzz_target;
fuzz_target!(|text: &str| {
if let Ok(proto) = protobuf::text_format::parse_from_str::<BgpConfig>(text) {
let _ = Config::from_proto(proto);
}
});
+12
View File
@@ -0,0 +1,12 @@
//! Raw bytes, as they come off a TCP connection. The first byte picks the
//! session's AS number width.
#![no_main]
use libfuzzer_sys::fuzz_target;
fuzz_target!(|data: &[u8]| {
let Some((selector, wire)) = data.split_first() else {
return;
};
bgp_fuzz::decode_and_check(selector & 1 == 1, wire);
});
+24
View File
@@ -0,0 +1,24 @@
//! A message body behind a correct header, so that the fuzzer spends its
//! time in the OPEN, UPDATE and NOTIFICATION parsers and not on guessing
//! the marker and length. The first byte picks the AS number width and the
//! message type.
#![no_main]
use libfuzzer_sys::fuzz_target;
/// The longest body that fits a BGP message (4096 less the 19-byte header).
const MAX_BODY: usize = 4096 - 19;
fuzz_target!(|data: &[u8]| {
let Some((selector, body)) = data.split_first() else {
return;
};
let body = &body[..body.len().min(MAX_BODY)];
let msg_type = 1 + (selector >> 1) % 5;
let mut wire = vec![0xff; 16];
wire.extend_from_slice(&((19 + body.len()) as u16).to_be_bytes());
wire.push(msg_type);
wire.extend_from_slice(body);
bgp_fuzz::decode_and_check(selector & 1 == 1, &wire);
});
+12
View File
@@ -0,0 +1,12 @@
//! Prefixes as text, as they appear in the configuration file.
#![no_main]
use bgp_primitives::ip_prefix::IpPrefix;
use libfuzzer_sys::fuzz_target;
fuzz_target!(|text: &str| {
if let Ok(prefix) = IpPrefix::try_from(text) {
let shown = prefix.to_string();
assert_eq!(IpPrefix::try_from(shown.as_str()), Ok(prefix));
}
});
+38
View File
@@ -0,0 +1,38 @@
# A worked example; the fields are documented in
# crates/server/proto/config.proto.
router_id: "10.0.0.1"
local_asn: 65000
# Where to accept BGP connections. Port 179 needs privileges; pick another
# port to run unprivileged against a peer that dials a custom port.
listen_endpoints { address: "127.0.0.1" port: 179 }
listen_endpoints { address: "::1" port: 179 }
# A peer we connect to (and accept connections from).
peers {
name: "r1"
endpoint { address: "127.0.0.1" port: 1791 }
remote_asn: 65001
hold_time_seconds: 30
# Offer only these families to the peer. Left out, all are offered.
address_families: IPV4_UNICAST
address_families: IPV6_UNICAST
}
connect_retry_seconds: 5
# Let a restarted peer replace a stale Established session immediately
# (RFC 4271 CollisionDetectEstablishedState). Default false.
collision_detect_established_state: false
# Routes this router originates, announced to every peer.
originate { prefix: "10.100.0.0/16" }
originate {
prefix: "2001:db8:100::/48"
med: 50
communities: "65000:100"
large_communities: "65000:1:2"
}
# Read-only status page, JSON API and metrics. It has no authentication:
# keep it on loopback or behind a reverse proxy.
http_endpoint { address: "127.0.0.1" port: 8080 }
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
+1
View File
@@ -0,0 +1 @@
bye
+1
View File
@@ -0,0 +1 @@
bye
+1
View File
@@ -0,0 +1 @@

+1
View File
@@ -0,0 +1 @@

Binary file not shown.
Binary file not shown.
+1
View File
@@ -0,0 +1 @@
10.0.0.0/8
+1
View File
@@ -0,0 +1 @@
2001:db8::/32
+1
View File
@@ -0,0 +1 @@
0.0.0.0/0
+1
View File
@@ -0,0 +1 @@
::/0
+1
View File
@@ -0,0 +1 @@
192.0.2.1/32
+1
View File
@@ -0,0 +1 @@
10.1.2.3/19
+33
View File
@@ -0,0 +1,33 @@
//! What the fuzz targets have in common.
use bgp_packet::message::Message;
use bgp_primitives::parser::ParserContext;
use bytes::BytesMut;
/// Decodes `wire` as a peer's message on a session with the given AS number
/// width. Whatever the bytes, this must return: a panic is a way for a peer
/// to take a session down.
///
/// A message that is accepted must also be one we can show, write, and read
/// back as itself. Anything else means the decoder built a value the rest
/// of the program cannot rely on.
pub fn decode_and_check(four_octet_asn: bool, wire: &[u8]) {
let ctx = ParserContext {
four_octet_asn: Some(four_octet_asn),
address_family: None,
};
let Ok(msg) = Message::decode(&ctx, wire) else {
return;
};
let _ = format!("{msg:?}");
let mut rewritten = BytesMut::new();
msg.to_wire(&ctx, &mut rewritten)
.expect("an accepted message can be written");
assert_eq!(
usize::from(msg.wire_len(&ctx).expect("and has a length")),
rewritten.len()
);
let again = Message::decode(&ctx, &rewritten).expect("and read back");
assert_eq!(again, msg);
}