From: Pierre Chifflier <chifflier@wzdftpd.net>
Date: Tue, 11 Jan 2022 14:50:55 +0000 (+0100)
Subject: rust: upgrade versions of BER/DER, Kerberos and SNMP parsers
X-Git-Tag: suricata-7.0.0-beta1~167
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=beadd090b89979b904c882e2857c24cf91aae1bb;p=thirdparty%2Fsuricata.git

rust: upgrade versions of BER/DER, Kerberos and SNMP parsers
---

diff --git a/rust/Cargo.toml.in b/rust/Cargo.toml.in
index 7201555c89..c299dd3a37 100644
--- a/rust/Cargo.toml.in
+++ b/rust/Cargo.toml.in
@@ -41,11 +41,11 @@ aes-gcm = "~0.8.0"
 
 sawp-modbus = "~0.11.0"
 sawp = "~0.11.0"
-der-parser = "~4.0.2"
+der-parser = "~6.0"
 kerberos-parser = "~0.7.1"
 ntp-parser = "~0.6.0"
 ipsec-parser = "~0.7.0"
-snmp-parser = "~0.6.0"
+snmp-parser = "~0.8.0"
 tls-parser = "~0.11.0"
 x509-parser = "~0.14.0"
 libc = "~0.2.82"
diff --git a/rust/src/asn1/mod.rs b/rust/src/asn1/mod.rs
index 7d33bc0a9b..5a2b4d15de 100644
--- a/rust/src/asn1/mod.rs
+++ b/rust/src/asn1/mod.rs
@@ -83,10 +83,13 @@ impl<'a> Asn1<'a> {
 
     /// Checks a BerObject and subnodes against the Asn1 checks
     fn check_object(obj: &BerObject, ad: &DetectAsn1Data) -> Option<Asn1Check> {
+        // get length
+        // Note that if length is indefinite (BER), this will return None
+        let len = obj.header.len.primitive().ok()?;
         // oversize_length will check if a node has a length greater than
         // the user supplied length
         if let Some(oversize_length) = ad.oversize_length {
-            if obj.header.len > oversize_length as u64
+            if len > oversize_length as usize
                 || obj.content.as_slice().unwrap_or(&[]).len() > oversize_length as usize
             {
                 return Some(Asn1Check::OversizeLength);
@@ -101,8 +104,8 @@ impl<'a> Asn1<'a> {
                 && obj.header.is_primitive())
         {
             if let BerObjectContent::BitString(bits, _v) = &obj.content {
-                if obj.header.len > 0
-                    && *bits as u64 > obj.header.len.saturating_mul(8)
+                if len > 0
+                    && *bits as usize > len.saturating_mul(8)
                 {
                     return Some(Asn1Check::BitstringOverflow);
                 }
@@ -118,10 +121,10 @@ impl<'a> Asn1<'a> {
                 && obj.header.is_primitive())
         {
             if let Ok(data) = obj.content.as_slice() {
-                if obj.header.len > 0
+                if len > 0
                     && !data.is_empty()
                     && data[0] & 0xC0 == 0
-                    && (obj.header.len > 256 || data.len() > 256)
+                    && (len > 256 || data.len() > 256)
                 {
                     return Some(Asn1Check::DoubleOverflow);
                 }
diff --git a/rust/src/kerberos.rs b/rust/src/kerberos.rs
index c980e0fd68..c4f1fca499 100644
--- a/rust/src/kerberos.rs
+++ b/rust/src/kerberos.rs
@@ -15,15 +15,14 @@
  * 02110-1301, USA.
  */
 
+use nom7::IResult;
+use nom7::error::{ErrorKind, ParseError};
+use nom7::number::streaming::le_u16;
 use der_parser;
 use der_parser::der::parse_der_oid;
 use der_parser::error::BerError;
 use kerberos_parser::krb5::{ApReq, PrincipalName, Realm};
 use kerberos_parser::krb5_parser::parse_ap_req;
-use nom;
-use nom::error::{ErrorKind, ParseError};
-use nom::number::complete::le_u16;
-use nom::IResult;
 
 #[derive(Debug)]
 pub enum SecBlobError {
@@ -55,22 +54,19 @@ pub struct Kerberos5Ticket {
     pub sname: PrincipalName,
 }
 
-fn parse_kerberos5_request_do(blob: &[u8]) -> IResult<&[u8], ApReq, SecBlobError> {
-    let (_, b) = der_parser::parse_der(blob).map_err(nom::Err::convert)?;
-    let blob = b
-        .as_slice()
-        .or(Err(nom::Err::Error(SecBlobError::KrbFmtError)))?;
-    let (blob, _) = parse_der_oid(blob).map_err(nom::Err::convert)?;
-    let (blob, _) = le_u16(blob)?;
-    // Should be parse_ap_req(blob).map_err(nom::Err::convert)
-    // But upgraded kerberos parser uses a newer der_parser crate
-    // Hence the enum `der_parser::error::BerError` are different
-    // and we cannot convert to SecBlobError with the From impl
-    // Next is to upgrade the der_parser crate (and nom to nom7 by the way)
-    match parse_ap_req(blob) {
-        Ok((blob, ap_req)) => Ok((blob, ap_req)),
-        _ => Err(nom::Err::Error(SecBlobError::KrbReqError)),
-    }
+fn parse_kerberos5_request_do(blob: &[u8]) -> IResult<&[u8], ApReq, SecBlobError>
+{
+    let (_,b) = der_parser::parse_der(blob).map_err(nom7::Err::convert)?;
+    let blob = b.as_slice().or(
+        Err(nom7::Err::Error(SecBlobError::KrbFmtError))
+    )?;
+    let parser = |i| {
+        let (i, _base_o) = parse_der_oid(i)?;
+        let (i, _tok_id) = le_u16(i)?;
+        let (i, ap_req) = parse_ap_req(i)?;
+        Ok((i, ap_req))
+    };
+    parser(blob).map_err(nom7::Err::convert)
 }
 
 pub fn parse_kerberos5_request(blob: &[u8]) -> IResult<&[u8], Kerberos5Ticket, SecBlobError> {
diff --git a/rust/src/krb/krb5.rs b/rust/src/krb/krb5.rs
index c7210238e7..fdee89d3ca 100644
--- a/rust/src/krb/krb5.rs
+++ b/rust/src/krb/krb5.rs
@@ -19,9 +19,8 @@
 
 use std;
 use std::ffi::CString;
-use nom;
-use nom::IResult;
-use nom::number::streaming::be_u32;
+use nom7::{Err, IResult};
+use nom7::number::streaming::be_u32;
 use der_parser::der::der_read_element_header;
 use der_parser::ber::BerClass;
 use kerberos_parser::krb5_parser;
@@ -186,7 +185,7 @@ impl KRB5State {
                 }
                 0
             },
-            Err(nom::Err::Incomplete(_)) => {
+            Err(Err::Incomplete(_)) => {
                 SCLogDebug!("Insufficient data while parsing KRB5 data");
                 self.set_event(KRB5Event::MalformedData);
                 -1
@@ -346,7 +345,7 @@ pub unsafe extern "C" fn rs_krb5_probing_parser(_flow: *const Flow,
             }
             return ALPROTO_FAILED;
         },
-        Err(nom::Err::Incomplete(_)) => {
+        Err(Err::Incomplete(_)) => {
             return ALPROTO_UNKNOWN;
         },
         Err(_) => {
@@ -370,7 +369,7 @@ pub unsafe extern "C" fn rs_krb5_probing_parser_tcp(_flow: *const Flow,
             return rs_krb5_probing_parser(_flow, direction,
                     rem.as_ptr(), rem.len() as u32, rdir);
         },
-        Err(nom::Err::Incomplete(_)) => {
+        Err(Err::Incomplete(_)) => {
             return ALPROTO_UNKNOWN;
         },
         Err(_) => {
@@ -442,7 +441,7 @@ pub unsafe extern "C" fn rs_krb5_parse_request_tcp(_flow: *const core::Flow,
                     state.record_ts = record as usize;
                     cur_i = rem;
                 },
-                Err(nom::Err::Incomplete(_)) => {
+                Err(Err::Incomplete(_)) => {
                     state.defrag_buf_ts.extend_from_slice(cur_i);
                     return AppLayerResult::ok();
                 }
@@ -500,7 +499,7 @@ pub unsafe extern "C" fn rs_krb5_parse_response_tcp(_flow: *const core::Flow,
                     state.record_tc = record as usize;
                     cur_i = rem;
                 },
-                Err(nom::Err::Incomplete(_)) => {
+                Err(Err::Incomplete(_)) => {
                     state.defrag_buf_tc.extend_from_slice(cur_i);
                     return AppLayerResult::ok();
                 }
diff --git a/rust/src/nfs/nfs4.rs b/rust/src/nfs/nfs4.rs
index d714ceadb8..7c4c48127e 100644
--- a/rust/src/nfs/nfs4.rs
+++ b/rust/src/nfs/nfs4.rs
@@ -17,9 +17,9 @@
 
 // written by Victor Julien
 
-use nom::bytes::streaming::take;
-use nom::number::streaming::be_u32;
-use nom7::Err;
+use nom7::bytes::streaming::take;
+use nom7::number::streaming::be_u32;
+use nom7::{Err, IResult};
 
 use crate::core::*;
 use crate::nfs::nfs::*;
@@ -30,8 +30,7 @@ use crate::nfs::types::*;
 
 use crate::kerberos::{parse_kerberos5_request, Kerberos5Ticket, SecBlobError};
 
-// use the old nom type until both SMB and NFS are migrated to nom 7
-fn parse_req_gssapi(i: &[u8]) -> nom::IResult<&[u8], Kerberos5Ticket, SecBlobError> {
+fn parse_req_gssapi(i: &[u8]) -> IResult<&[u8], Kerberos5Ticket, SecBlobError> {
     let (i, len) = be_u32(i)?;
     let (i, buf) = take(len as usize)(i)?;
     let (_, ap) = parse_kerberos5_request(buf)?;
diff --git a/rust/src/smb/auth.rs b/rust/src/smb/auth.rs
index bd15ca666e..f3efdf874a 100644
--- a/rust/src/smb/auth.rs
+++ b/rust/src/smb/auth.rs
@@ -20,26 +20,25 @@ use crate::kerberos::*;
 use crate::smb::ntlmssp_records::*;
 use crate::smb::smb::*;
 
-use nom;
-use nom::IResult;
+use nom7::{Err, IResult};
 use der_parser::ber::BerObjectContent;
 use der_parser::der::{parse_der_oid, parse_der_sequence};
 
 fn parse_secblob_get_spnego(blob: &[u8]) -> IResult<&[u8], &[u8], SecBlobError>
 {
-    let (rem, base_o) = der_parser::parse_der(blob).map_err(nom::Err::convert)?;
+    let (rem, base_o) = der_parser::parse_der(blob).map_err(Err::convert)?;
     SCLogDebug!("parse_secblob_get_spnego: base_o {:?}", base_o);
     let d = match base_o.content.as_slice() {
-        Err(_) => { return Err(nom::Err::Error(SecBlobError::NotSpNego)); },
+        Err(_) => { return Err(Err::Error(SecBlobError::NotSpNego)); },
         Ok(d) => d,
     };
-    let (next, o) = parse_der_oid(d).map_err(nom::Err::convert)?;
+    let (next, o) = parse_der_oid(d).map_err(Err::convert)?;
     SCLogDebug!("parse_secblob_get_spnego: sub_o {:?}", o);
 
     let oid = match o.content.as_oid() {
         Ok(oid) => oid,
         Err(_) => {
-            return Err(nom::Err::Error(SecBlobError::NotSpNego));
+            return Err(Err::Error(SecBlobError::NotSpNego));
         },
     };
     SCLogDebug!("oid {}", oid.to_string());
@@ -49,7 +48,7 @@ fn parse_secblob_get_spnego(blob: &[u8]) -> IResult<&[u8], &[u8], SecBlobError>
             SCLogDebug!("SPNEGO {}", oid);
         },
         _ => {
-            return Err(nom::Err::Error(SecBlobError::NotSpNego));
+            return Err(Err::Error(SecBlobError::NotSpNego));
         },
     }
 
@@ -60,14 +59,14 @@ fn parse_secblob_get_spnego(blob: &[u8]) -> IResult<&[u8], &[u8], SecBlobError>
 
 fn parse_secblob_spnego_start(blob: &[u8]) -> IResult<&[u8], &[u8], SecBlobError>
 {
-    let (rem, o) = der_parser::parse_der(blob).map_err(nom::Err::convert)?;
+    let (rem, o) = der_parser::parse_der(blob).map_err(Err::convert)?;
     let d = match o.content.as_slice() {
         Ok(d) => {
             SCLogDebug!("d: next data len {}",d.len());
             d
         },
         _ => {
-            return Err(nom::Err::Error(SecBlobError::NotSpNego));
+            return Err(Err::Error(SecBlobError::NotSpNego));
         },
     };
     Ok((rem, d))
diff --git a/rust/src/snmp/snmp.rs b/rust/src/snmp/snmp.rs
index 9e05c3d4ca..e6a7698204 100644
--- a/rust/src/snmp/snmp.rs
+++ b/rust/src/snmp/snmp.rs
@@ -26,9 +26,8 @@ use std::ffi::CString;
 use der_parser::ber::BerObjectContent;
 use der_parser::der::parse_der_sequence;
 use der_parser::oid::Oid;
-use nom;
-use nom::IResult;
-use nom::error::ErrorKind;
+use nom7::{Err, IResult};
+use nom7::error::{ErrorKind, make_error};
 
 #[derive(AppLayerEvent)]
 pub enum SNMPEvent {
@@ -343,11 +342,11 @@ fn parse_pdu_enveloppe_version(i:&[u8]) -> IResult<&[u8],u32> {
                 },
                 _ => ()
             };
-            Err(nom::Err::Error(error_position!(i, ErrorKind::Verify)))
+            Err(Err::Error(make_error(i, ErrorKind::Verify)))
         },
-        Err(nom::Err::Incomplete(i)) => Err(nom::Err::Incomplete(i)),
-        Err(nom::Err::Failure(_)) |
-        Err(nom::Err::Error(_))      => Err(nom::Err::Error(error_position!(i,ErrorKind::Verify)))
+        Err(Err::Incomplete(i)) => Err(Err::Incomplete(i)),
+        Err(Err::Failure(_)) |
+        Err(Err::Error(_))      => Err(Err::Error(make_error(i,ErrorKind::Verify)))
     }
 }
 
@@ -361,9 +360,9 @@ pub unsafe extern "C" fn rs_snmp_probing_parser(_flow: *const Flow,
     let alproto = ALPROTO_SNMP;
     if slice.len() < 4 { return ALPROTO_FAILED; }
     match parse_pdu_enveloppe_version(slice) {
-        Ok((_,_))                    => alproto,
-        Err(nom::Err::Incomplete(_)) => ALPROTO_UNKNOWN,
-        _                            => ALPROTO_FAILED,
+        Ok((_,_))               => alproto,
+        Err(Err::Incomplete(_)) => ALPROTO_UNKNOWN,
+        _                       => ALPROTO_FAILED,
     }
 }