refactor NLRI parsing for add-path issue

This refactor enables parsing of NLRI block with potentially wrong
add-path type. In some cases, the BGP message with add-path might be
wrapped in a non-add-path message and causing trouble parsing NLRI block
in BGP message and in attributes.

This patch implements a workaround that can more intelligently "guess"
the add-path intention and revert back to regular parsing when guess
fails. Here is a brief description of the workaround:

- If add-path is not set (i.e. not the add-path BGP msgs) and the first
byte of an NLRI entry is 0, it is likely an add-path msg wrapped in
non-add-path msg, treat as add-path. This covers both IPv4 and IPv6
cases.
- If an error happens when parsing, revert back to the original add-path
setting (this only happens for `0.0.0.0/0` or `0::/0` prefixes without
add-path, which is pretty rare) and re-parse the whole NLRI
block. (needs to go back a few bytes to the beginning).
- The unhandled cases are the ones where the first byte of path_id is
not zero. In those cases, we will, unfortunately, parse the msg without
path-id, and depending on the msg, we might see parsing errors
later (e.g. not enough bytes, or extra bytes).

Test run show that the parsing of one problematic file is resolved back
to normal now. See #31 for
more details.

src/parser/bgp/attributes.rs

@@ -4,13 +4,13 @@ use std::convert::TryFrom;
 use bgp_models::bgp::attributes::*;
 use bgp_models::bgp::community::*;
 use bgp_models::network::*;
-use log::warn;
+use log::{warn,debug};
 
 use byteorder::{BigEndian, ReadBytesExt};
 
 use num_traits::FromPrimitive;
 
-use crate::parser::ReadUtils;
+use crate::parser::{parse_nlri_list, ReadUtils};
 use crate::error::ParserErrorKind;
 
 
@@ -257,6 +257,7 @@ impl AttributeParser {
     }
 
     ///
+    /// <https://datatracker.ietf.org/doc/html/rfc4760#section-3>
     /// The attribute is encoded as shown below:
     /// +---------------------------------------------------------+
     /// | Address Family Identifier (2 octets)                    |
@@ -322,19 +323,23 @@ impl AttributeParser {
                 if first_byte_zero {
                     if reachable {
                         // skip reserved byte for reachable NRLI
-                        let _should_be_zero = buf_input.read_u8()?;
+                        if buf_input.read_u8()? !=0 {
+                            warn!("NRLI reserved byte not 0");
+                        }
                     }
-                    self.parse_mp_prefix_list(&mut buf_input, &afi)?
+                    parse_nlri_list(&mut buf_input, self.additional_paths, &afi)?
                 } else {
                     pfxs.to_owned()
                 }
             },
             None => {
                 if reachable {
                     // skip reserved byte for reachable NRLI
-                    buf_input.read_u8()?;
+                    if buf_input.read_u8()? !=0 {
+                        warn!("NRLI reserved byte not 0");
+                    }
                 }
-                self.parse_mp_prefix_list(&mut buf_input, &afi)?
+                parse_nlri_list(&mut buf_input, self.additional_paths, &afi)?
             }
         };
 
@@ -367,24 +372,6 @@ impl AttributeParser {
         Ok(output)
     }
 
-    fn parse_mp_prefix_list<T: Read>(
-        &self,
-        input: &mut Take<T>,
-        afi: &Afi,
-    ) -> Result<Vec<NetworkPrefix>, ParserErrorKind> {
-        let mut output = Vec::new();
-        while input.limit() > 0 {
-            let path_id = if self.additional_paths {
-                input.read_u32::<BigEndian>()
-            } else {
-                Ok(0)
-            }?;
-            let prefix = input.read_nlri_prefix(afi, path_id)?;
-            output.push(prefix);
-        }
-        Ok(output)
-    }
-
     fn parse_large_communities<T: Read>(
         &self,
         input: &mut Take<T>,

src/parser/bgp/messages.rs

@@ -1,11 +1,13 @@
-use std::io::Read;
+use std::io::{Read, Take};
 
 use bgp_models::bgp::*;
 use bgp_models::network::*;
+use byteorder::ReadBytesExt;
 use num_traits::FromPrimitive;
 
 use crate::error::ParserErrorKind;
-use crate::parser::{AttributeParser, ReadUtils};
+use crate::parser::{AttributeParser, parse_nlri_list, ReadUtils};
+use log::warn;
 
 /// BGP message
 ///
@@ -122,25 +124,46 @@ pub fn parse_bgp_open_message<T: Read>(input: &mut T, ) -> Result<BgpOpenMessage
         })
 }
 
+/// read nlri portion of a bgp update message.
+fn read_nlri<T: Read>(input: &mut Take<T>, length: usize, afi: &Afi, add_path: bool) -> Result<Vec<NetworkPrefix>, ParserErrorKind> {
+    if length==0{
+        return Ok(vec![])
+    }
+    if length==1 {
+        // 1 byte does not make sense
+        warn!("seeing strange one-byte NLRI field");
+        input.read_u8().unwrap();
+        return Ok(vec![])
+    }
+
+    let mut buf=Vec::with_capacity(length);
+    input.read_to_end(&mut buf)?;
+
+    let mut nlri_input = buf.as_slice().take(length as u64);
+
+    let prefixes = parse_nlri_list(&mut nlri_input, add_path, &afi)?;
+
+    Ok(prefixes)
+}
+
+/// read bgp update message.
 pub fn parse_bgp_update_message<T: Read>(input: &mut T, add_path:bool, afi: &Afi, asn_len: &AsnLength, bgp_msg_length: u64) -> Result<BgpUpdateMessage, ParserErrorKind> {
+    // parse withdrawn prefixes nlri
     let withdrawn_length = input.read_16b()? as u64;
-    let mut withdarwn_input = input.take(withdrawn_length);
-    let mut withdrawn_prefixes = vec!();
-    while withdarwn_input.limit()>0 {
-        withdrawn_prefixes.push(withdarwn_input.read_nlri_prefix(afi, 0)?);
-    }
+    let mut withdrawn_input = input.take(withdrawn_length);
+    let withdrawn_prefixes = read_nlri(&mut withdrawn_input, withdrawn_length as usize, afi, add_path)?;
 
+    // parse attributes
     let attribute_length = input.read_16b()? as u64;
     let attr_parser = AttributeParser::new(add_path);
     let mut attr_input = input.take(attribute_length);
     let attributes = attr_parser.parse_attributes(&mut attr_input, asn_len, None, None, None)?;
-    let mut announced_prefixes = vec!();
 
+    // parse announced prefixes nlri
     let nlri_length = bgp_msg_length - 4 - withdrawn_length - attribute_length;
     let mut nlri_input = input.take(nlri_length);
-    while nlri_input.limit()>0 {
-        announced_prefixes.push(nlri_input.read_nlri_prefix(afi, 0)?);
-    }
+    let announced_prefixes = read_nlri(&mut nlri_input, nlri_length as usize, afi, add_path)?;
+
     Ok(
         BgpUpdateMessage{
             withdrawn_prefixes,

src/parser/mrt/messages/table_dump_v2_message.rs

@@ -123,7 +123,7 @@ pub fn parse_rib_afi_entries<T: std::io::Read>(input: &mut Take<T>, rib_type: Ta
 
     let sequence_number = input.read_32b()?;
 
-    let prefix = input.read_nlri_prefix(&afi, 0)?;
+    let prefix = input.read_nlri_prefix(&afi, add_path)?;
     let prefixes = vec!(prefix.clone());
 
     let entry_count = input.read_16b()?;

src/parser/utils.rs

@@ -10,8 +10,9 @@ use std::{
 
 use num_traits::FromPrimitive;
 use std::net::IpAddr;
-use std::io::Read;
+use std::io::{Read, Take};
 use bgp_models::network::{Afi, Asn, AsnLength, NetworkPrefix, Safi};
+use log::warn;
 
 use crate::error::ParserErrorKind;
 
@@ -66,10 +67,19 @@ pub trait ReadUtils: io::Read {
         Ok(())
     }
 
-    /// Read announced prefix.
+    /// Read announced/withdrawn prefix.
     ///
     /// The length in bits is 1 byte, and then based on the IP version it reads different number of bytes.
-    fn read_nlri_prefix(&mut self, afi: &Afi, path_id: u32) -> Result<NetworkPrefix, ParserErrorKind> {
+    /// If the `add_path` is true, it will also first read a 4-byte path id first; otherwise, a path-id of 0
+    /// is automatically set.
+    fn read_nlri_prefix(&mut self, afi: &Afi, add_path: bool) -> Result<NetworkPrefix, ParserErrorKind> {
+
+        let path_id = if add_path {
+            self.read_u32::<BigEndian>()?
+        } else {
+            0
+        };
+
         // Length in bits
         let bit_len = self.read_8b()?;
 
@@ -199,5 +209,55 @@ pub trait ReadUtils: io::Read {
     }
 }
 
+pub(crate) fn parse_nlri_list(
+    input: &mut Take<&[u8]>,
+    add_path: bool,
+    afi: &Afi,
+) -> Result<Vec<NetworkPrefix>, ParserErrorKind> {
+
+    let mut bytes_copy = Vec::with_capacity(input.limit() as usize);
+    input.read_to_end(&mut bytes_copy)?;
+
+    let mut is_add_path = add_path;
+    let mut prefixes = Vec::new();
+
+    let mut retry = false;
+    let mut guessed = false;
+
+    let mut new_input = bytes_copy.take(bytes_copy.len() as u64);
+    while new_input.limit() > 0 {
+        if !is_add_path && new_input.get_ref()[0]==0 {
+            // it's likely that this is a add-path wrongfully wrapped in non-add-path msg
+            warn!("not add-path but with NLRI size to be 0, likely add-path msg in wrong msg type, treat as add-path now");
+            is_add_path = true;
+            guessed = true;
+        }
+        let prefix = match new_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
+        let mut new_input = bytes_copy.take(bytes_copy.len() as u64);
+        while new_input.limit() > 0 {
+            let prefix = new_input.read_nlri_prefix(afi, add_path)?;
+            prefixes.push(prefix);
+        }
+    }
+
+    Ok(prefixes)
+}
+
 // All types that implement Read can now read prefixes
 impl<R: io::Read> ReadUtils for R {}