]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
nfs/rpc: update full record parsers to be more exact
authorVictor Julien <vjulien@oisf.net>
Sun, 20 Feb 2022 09:00:48 +0000 (10:00 +0100)
committerVictor Julien <vjulien@oisf.net>
Mon, 21 Feb 2022 19:25:29 +0000 (20:25 +0100)
Instead of 'take'ing all data for the RPC prog_data and then
letting the higher level parsers figure out which part to use
take the exact amount.

rust/src/nfs/nfs.rs
rust/src/nfs/rpc_records.rs

index f200cdd227200016555b23651ab140e338e024ec..c9b0cd6128b23edc6e04161dc366a09eb70a7914 100644 (file)
@@ -998,7 +998,7 @@ impl NFSState {
                 SCLogDebug!("CONFIRMED WRITE: large record {}, file chunk xfer", rec_size);
 
                 // lets try to parse the RPC record. Might fail with Incomplete.
-                match parse_rpc(cur_i) {
+                match parse_rpc(cur_i, false) {
                     Ok((remaining, ref hdr)) => {
                         match parse_nfs3_request_write(hdr.prog_data) {
                             Ok((_, ref w)) => {
@@ -1091,28 +1091,24 @@ impl NFSState {
                     }
 
                     // we have the full records size worth of data,
-                    // let's parse it
-                    match parse_rpc(&cur_i[..rec_size]) {
+                    // let's parse it. Errors lead to event, but are
+                    // not fatal as we already have enough info to
+                    // go to the next record.
+                    match parse_rpc(cur_i, true) {
                         Ok((_, ref rpc_record)) => {
-                            cur_i = &cur_i[rec_size..];
                             self.process_request_record(rpc_record);
-                        },
+                        }
                         Err(Err::Incomplete(_)) => {
-                            cur_i = &cur_i[rec_size..]; // progress input past parsed record
-
-                            // we shouldn't get incomplete as we have the full data
-                            // so if we got incomplete anyway it's the data that is
-                            // bad.
                             self.set_event(NFSEvent::MalformedData);
-                        },
+                        }
                         Err(Err::Error(_e)) |
                         Err(Err::Failure(_e)) => {
                             self.set_event(NFSEvent::MalformedData);
                             SCLogDebug!("Parsing failed: {:?}", _e);
-                            return AppLayerResult::err();
-                        },
+                        }
                     }
-                },
+                    cur_i = &cur_i[rec_size..];
+                }
                 Err(Err::Incomplete(needed)) => {
                     if let Needed::Size(n) = needed {
                         SCLogDebug!("Not enough data for partial RPC header {:?}", needed);
@@ -1123,13 +1119,15 @@ impl NFSState {
                         return AppLayerResult::incomplete((i.len() - cur_i.len()) as u32, need as u32);
                     }
                     return AppLayerResult::err();
-                },
+                }
+                /* This error is fatal. If we failed to parse the RPC hdr we don't
+                 * have a length and we don't know where the next record starts. */
                 Err(Err::Error(_e)) |
                 Err(Err::Failure(_e)) => {
                     self.set_event(NFSEvent::MalformedData);
                     SCLogDebug!("Parsing failed: {:?}", _e);
                     return AppLayerResult::err();
-                },
+                }
             }
         };
 
@@ -1157,7 +1155,7 @@ impl NFSState {
                 SCLogDebug!("CONFIRMED large READ record {}, likely file chunk xfer", rec_size);
 
                 // we should have enough data to parse the RPC record
-                match parse_rpc_reply(cur_i) {
+                match parse_rpc_reply(cur_i, false) {
                     Ok((remaining, ref hdr)) => {
                         match parse_nfs3_reply_read(hdr.prog_data) {
                             Ok((_, ref r)) => {
@@ -1250,27 +1248,24 @@ impl NFSState {
                     }
 
                     // we have the full data of the record, lets parse
-                    match parse_rpc_reply(&cur_i[..rec_size]) {
+                    match parse_rpc_reply(cur_i, true) {
                         Ok((_, ref rpc_record)) => {
-                            cur_i = &cur_i[rec_size..]; // progress input past parsed record
                             self.process_reply_record(rpc_record);
-                        },
+                        }
                         Err(Err::Incomplete(_)) => {
-                            cur_i = &cur_i[rec_size..]; // progress input past parsed record
-
                             // we shouldn't get incomplete as we have the full data
                             // so if we got incomplete anyway it's the data that is
                             // bad.
                             self.set_event(NFSEvent::MalformedData);
-                        },
+                        }
                         Err(Err::Error(_e)) |
                         Err(Err::Failure(_e)) => {
                             self.set_event(NFSEvent::MalformedData);
                             SCLogDebug!("Parsing failed: {:?}", _e);
-                            return AppLayerResult::err();
                         }
                     }
-                },
+                    cur_i = &cur_i[rec_size..]; // progress input past parsed record
+                }
                 Err(Err::Incomplete(needed)) => {
                     if let Needed::Size(n) = needed {
                         SCLogDebug!("Not enough data for partial RPC header {:?}", needed);
@@ -1281,13 +1276,15 @@ impl NFSState {
                         return AppLayerResult::incomplete((i.len() - cur_i.len()) as u32, need as u32);
                     }
                     return AppLayerResult::err();
-                },
+                }
+                /* This error is fatal. If we failed to parse the RPC hdr we don't
+                 * have a length and we don't know where the next record starts. */
                 Err(Err::Error(_e)) |
                 Err(Err::Failure(_e)) => {
                     self.set_event(NFSEvent::MalformedData);
                     SCLogDebug!("Parsing failed: {:?}", _e);
                     return AppLayerResult::err();
-                },
+                }
             }
         };
         self.post_gap_housekeeping(Direction::ToClient);
@@ -1602,7 +1599,7 @@ fn nfs_probe_dir(i: &[u8], rdir: *mut u8) -> i8 {
 
 pub fn nfs_probe(i: &[u8], direction: Direction) -> i32 {
     if direction == Direction::ToClient {
-        match parse_rpc_reply(i) {
+        match parse_rpc_reply(i, false) {
             Ok((_, ref rpc)) => {
                 if rpc.hdr.frag_len >= 24 && rpc.hdr.frag_len <= 35000 && rpc.hdr.msgtype == 1 && rpc.reply_state == 0 && rpc.accept_state == 0 {
                     SCLogDebug!("TC PROBE LEN {} XID {} TYPE {}", rpc.hdr.frag_len, rpc.hdr.xid, rpc.hdr.msgtype);
@@ -1634,7 +1631,7 @@ pub fn nfs_probe(i: &[u8], direction: Direction) -> i32 {
             },
         }
     } else {
-        match parse_rpc(i) {
+        match parse_rpc(i, false) {
             Ok((_, ref rpc)) => {
                 if rpc.hdr.frag_len >= 40 && rpc.hdr.msgtype == 0 &&
                    rpc.rpcver == 2 && (rpc.progver == 3 || rpc.progver == 4) &&
index 8a141416cd66a7eea4e1bf6b36cbe87a9fd753da..7e01f6b7b7e2c5f229270d5e0567ddf05b484f29 100644 (file)
@@ -20,7 +20,7 @@
 use crate::common::nom7::bits;
 use nom7::bits::streaming::take as take_bits;
 use nom7::bytes::streaming::take;
-use nom7::combinator::{cond, verify};
+use nom7::combinator::{cond, verify, rest};
 use nom7::multi::length_data;
 use nom7::number::streaming::{be_u32};
 use nom7::sequence::tuple;
@@ -155,6 +155,7 @@ pub struct RpcReplyPacket<'a> {
     pub reply_state: u32,
     pub accept_state: u32,
 
+    pub prog_data_size: u32,
     pub prog_data: &'a [u8],
 }
 
@@ -201,11 +202,24 @@ pub struct RpcPacket<'a> {
     pub verifier_len: u32,
     pub verifier: &'a [u8],
 
+    pub prog_data_size: u32,
     pub prog_data: &'a [u8],
 }
 
-pub fn parse_rpc(i: &[u8]) -> IResult<&[u8], RpcPacket> {
-    let (i, hdr) = parse_rpc_packet_header(i)?;
+/// Parse request RPC record.
+///
+/// Can be called from 2 paths:
+///  1. we have all data -> do full parsing
+///  2. we have partial data (large records) -> allow partial prog_data parsing
+///
+/// Arguments:
+/// * `complete`:
+///           type: bool
+///    description: do full parsing, including of `prog_data`
+///
+pub fn parse_rpc(start_i: &[u8], complete: bool) -> IResult<&[u8], RpcPacket> {
+    let (i, hdr) = parse_rpc_packet_header(start_i)?;
+    let rec_size = hdr.frag_len as usize + 4;
 
     let (i, rpcver) = be_u32(i)?;
     let (i, program) = be_u32(i)?;
@@ -225,7 +239,14 @@ pub fn parse_rpc(i: &[u8]) -> IResult<&[u8], RpcPacket> {
     let (i, verifier_len) = verify(be_u32, |&size| size < RPC_MAX_VERIFIER_SIZE)(i)?;
     let (i, verifier) = take(verifier_len as usize)(i)?;
 
-    let (i, prog_data) = (&[], i);
+    let consumed = start_i.len() - i.len();
+    let data_size : u32 = (rec_size as usize - consumed) as u32;
+    let (i, prog_data) = if !complete {
+        rest(i)?
+    } else {
+        take(data_size)(i)?
+    };
+
     let packet = RpcPacket {
         hdr,
 
@@ -241,14 +262,26 @@ pub fn parse_rpc(i: &[u8]) -> IResult<&[u8], RpcPacket> {
         verifier_len,
         verifier,
 
+        prog_data_size: data_size,
         prog_data,
     };
     Ok((i, packet))
 }
 
-// to be called with data <= hdr.frag_len + 4. Sending more data is undefined.
-pub fn parse_rpc_reply(i: &[u8]) -> IResult<&[u8], RpcReplyPacket> {
-    let (i, hdr) = parse_rpc_packet_header(i)?;
+/// Parse reply RPC record.
+///
+/// Can be called from 2 paths:
+///  1. we have all data -> do full parsing
+///  2. we have partial data (large records) -> allow partial prog_data parsing
+///
+/// Arguments:
+/// * `complete`:
+///           type: bool
+///    description: do full parsing, including of `prog_data`
+///
+pub fn parse_rpc_reply(start_i: &[u8], complete: bool) -> IResult<&[u8], RpcReplyPacket> {
+    let (i, hdr) = parse_rpc_packet_header(start_i)?;
+    let rec_size = hdr.frag_len + 4;
 
     let (i, reply_state) = be_u32(i)?;
 
@@ -257,7 +290,14 @@ pub fn parse_rpc_reply(i: &[u8]) -> IResult<&[u8], RpcReplyPacket> {
     let (i, verifier) = cond(verifier_len > 0, take(verifier_len as usize))(i)?;
 
     let (i, accept_state) = be_u32(i)?;
-    let (i, prog_data) = (&[], i);
+
+    let consumed = start_i.len() - i.len();
+    let data_size : u32 = (rec_size as usize - consumed) as u32;
+    let (i, prog_data) = if !complete {
+        rest(i)?
+    } else {
+        take(data_size)(i)?
+    };
     let packet = RpcReplyPacket {
         hdr,
 
@@ -268,6 +308,7 @@ pub fn parse_rpc_reply(i: &[u8]) -> IResult<&[u8], RpcReplyPacket> {
         reply_state,
         accept_state,
 
+        prog_data_size: data_size,
         prog_data,
     };
     Ok((i, packet))
@@ -307,7 +348,8 @@ pub fn parse_rpc_udp_request(i: &[u8]) -> IResult<&[u8], RpcPacket> {
     let (i, verifier_len) = verify(be_u32, |&size| size < RPC_MAX_VERIFIER_SIZE)(i)?;
     let (i, verifier) = take(verifier_len as usize)(i)?;
 
-    let (i, prog_data) = (&[], i);
+    let data_size : u32 = i.len() as u32;
+    let (i, prog_data) = rest(i)?;
     let packet = RpcPacket {
         hdr,
 
@@ -323,6 +365,7 @@ pub fn parse_rpc_udp_request(i: &[u8]) -> IResult<&[u8], RpcPacket> {
         verifier_len,
         verifier,
 
+        prog_data_size: data_size,
         prog_data,
     };
     Ok((i, packet))
@@ -337,7 +380,9 @@ pub fn parse_rpc_udp_reply(i: &[u8]) -> IResult<&[u8], RpcReplyPacket> {
 
     let (i, reply_state) = be_u32(i)?;
     let (i, accept_state) = be_u32(i)?;
-    let (i, prog_data) = (&[], i);
+
+    let data_size : u32 = i.len() as u32;
+    let (i, prog_data) = rest(i)?;
     let packet = RpcReplyPacket {
         hdr,
 
@@ -348,6 +393,7 @@ pub fn parse_rpc_udp_reply(i: &[u8]) -> IResult<&[u8], RpcReplyPacket> {
         reply_state,
         accept_state,
 
+        prog_data_size: data_size,
         prog_data,
     };
     Ok((i, packet))