Raw BMP Parser does not honor peer capbilities when parsing RouteMonitoring messages

[ahmadkkhaled]

Hi folks,

I recently started using bgpkit-parser for a project with the aims of parsing a static dataset of raw bmp messages stored on a file system. After some exploration I came across this function which seems to be the only public function in the crate that supports parsing raw bmp messages

bgpkit-parser/src/parser/bmp/mod.rs

Line 25 in 6713331

pub fn parse_bmp_msg(data: &mut Bytes) -> Result<BmpMessage, ParserBmpError> {

With further exploration I learned that this function is stateless, i.e. it does not maintain the state of peer capabilities negotiated in the OpenMessage.

At this point it is safe to assume that bgpkit-parser does not actually support accurate parsing of RouteMonitoring messages including RFC 7911: Advertisements of Multiple Paths in BGP (ADD-PATH) as promised in this crate's documentation. Also evident by the hard cording of add_path = false here

bgpkit-parser/src/parser/bmp/messages/route_monitoring.rs

Line 16 in 6713331

let bgp_update = parse_bgp_message(data, false, asn_len)?;

Could you please clarify if my understanding is correct? and if so, could you reflect that in the READMe/Docs?

[digizeph]

You are correct that the parsing of BMP messages are stateless in that it currently does not maintain capabilities negotiated in the OpenMessage. The same goes for parsing RouteMirroring messages.

However, in terms of add_path, the parser actually will attempt to fix incorrect add_path value during parsing and it should actually be able to parse add_path routes with add_path==false. Can you clarify the errors you encounter?

Here is the function that add_path is used and it does attempt to fix incorrect values:

bgpkit-parser/src/parser/utils.rs

Lines 227 to 275 in 6713331

	pub fn parse_nlri_list(
	mut input: Bytes,
	add_path: bool,
	afi: &Afi,
	) -> Result<Vec<NetworkPrefix>, ParserError> {
	let mut is_add_path = add_path;
	let mut prefixes = vec![];
	let mut retry = false;
	let mut guessed = false;
	let mut input_copy = None;
	while input.remaining() > 0 {
	if !is_add_path && input[0] == 0 {
	// it's likely that this is a add-path wrongfully wrapped in non-add-path msg
	debug!("not add-path but with NLRI size to be 0, likely add-path msg in wrong msg type, treat as add-path now");
	// cloning the data bytes
	is_add_path = true;
	guessed = true;
	input_copy = Some(input.clone());
	}
	let prefix = match input.read_nlri_prefix(afi, is_add_path) {
	Ok(p) => p,
	Err(e) => {
	if guessed {
	retry = true;
	break;
	} else {
	return Err(e);
	}
	}
	};
	prefixes.push(prefix);
	}
	if retry {
	prefixes.clear();
	// try again without attempt to guess add-path
	// if we reach here (retry==true), input_copy must be Some
	let mut input_2 = input_copy.unwrap();
	while input_2.remaining() > 0 {
	let prefix = input_2.read_nlri_prefix(afi, add_path)?;
	prefixes.push(prefix);
	}
	}
	Ok(prefixes)
	}

Of course, I am happy to work on a better support for stateful BMP parser to address this issue. This is also relevant to #137 where better BGP OpenMessage support is part of the goal.

[ahmadkkhaled]

Thanks for the prompt response! I will provide examples as soon as I can, additionally, if I find the time, I can create a pull request to support parsing with capabilities as an optional parameter, such that maintaining the state becomes the responsibility of the client code, but using the state (or making sense of it) is a property of the parser. What do you think?

[digizeph]

Happy to take a PR!

FYI, here is another example that parses BMP from a live connection: https://github.com/bgpkit/bgpkit-parser/blob/main/examples/bmp_listener.rs