]> 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)
committerShivani Bhardwaj <shivanib134@gmail.com>
Fri, 4 Mar 2022 05:38:17 +0000 (11:08 +0530)
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.

Comments from Shivani Bhardwaj: This is was more than a cherry pick and
some of the modifications were not directly portable to the nom macros.
So, parsers were changed to make sure the functionality remained same while
making the transition to nom5 while keeping the diff minimal.

(cherry picked from commit 64d8a1e16e07148a8b5839452be3f7481e4e3623)

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

index 4df4969e4a6aed29871c77be229245e0d28443eb..487ec9faee71f9720bf666fcdb5a4e6dc33e0826 100644 (file)
@@ -1062,7 +1062,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, cur_i.len(), false) {
                     Ok((remaining, ref hdr)) => {
                         match parse_nfs3_request_write(hdr.prog_data) {
                             Ok((_, ref w)) => {
@@ -1155,28 +1155,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, cur_i.len(), true) {
                         Ok((_, ref rpc_record)) => {
-                            cur_i = &cur_i[rec_size..];
                             self.process_request_record(rpc_record);
-                        },
+                        }
                         Err(nom::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(nom::Err::Error(_e)) |
                         Err(nom::Err::Failure(_e)) => {
                             self.set_event(NFSEvent::MalformedData);
                             SCLogDebug!("Parsing failed: {:?}", _e);
-                            return AppLayerResult::err();
-                        },
+                        }
                     }
-                },
+                    cur_i = &cur_i[rec_size..];
+                }
                 Err(nom::Err::Incomplete(needed)) => {
                     if let nom::Needed::Size(n) = needed {
                         SCLogDebug!("Not enough data for partial RPC header {:?}", needed);
@@ -1186,13 +1182,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(nom::Err::Error(_e)) |
                 Err(nom::Err::Failure(_e)) => {
                     self.set_event(NFSEvent::MalformedData);
                     SCLogDebug!("Parsing failed: {:?}", _e);
                     return AppLayerResult::err();
-                },
+                }
             }
         };
 
@@ -1220,7 +1218,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, cur_i.len(), false) {
                     Ok((remaining, ref hdr)) => {
                         match parse_nfs3_reply_read(hdr.prog_data) {
                             Ok((_, ref r)) => {
@@ -1313,27 +1311,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, cur_i.len(), true) {
                         Ok((_, ref rpc_record)) => {
-                            cur_i = &cur_i[rec_size..]; // progress input past parsed record
                             self.process_reply_record(rpc_record);
-                        },
+                        }
                         Err(nom::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(nom::Err::Error(_e)) |
                         Err(nom::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(nom::Err::Incomplete(needed)) => {
                     if let nom::Needed::Size(n) = needed {
                         SCLogDebug!("Not enough data for partial RPC header {:?}", needed);
@@ -1343,13 +1338,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(nom::Err::Error(_e)) |
                 Err(nom::Err::Failure(_e)) => {
                     self.set_event(NFSEvent::MalformedData);
                     SCLogDebug!("Parsing failed: {:?}", _e);
                     return AppLayerResult::err();
-                },
+                }
             }
         };
         self.post_gap_housekeeping(STREAM_TOCLIENT);
@@ -1762,7 +1759,7 @@ fn nfs_probe_dir(i: &[u8], rdir: *mut u8) -> i8 {
 
 pub fn nfs_probe(i: &[u8], direction: u8) -> i8 {
     if direction == STREAM_TOCLIENT {
-        match parse_rpc_reply(i) {
+        match parse_rpc_reply(i, i.len(), 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);
@@ -1794,7 +1791,7 @@ pub fn nfs_probe(i: &[u8], direction: u8) -> i8 {
             },
         }
     } else {
-        match parse_rpc(i) {
+        match parse_rpc(i, i.len(), false) {
             Ok((_, ref rpc)) => {
                 if rpc.hdr.frag_len >= 40 && rpc.hdr.msgtype == 0 &&
                    rpc.rpcver == 2 && (rpc.progver == 3 || rpc.progver == 4) &&
index 1dee3847ad7db3dd8fde73cc28496ebadc6c877c..b48afc6a531869c8f1733cdb30f7a6ea701f7635 100644 (file)
@@ -18,7 +18,7 @@
 //! Nom parsers for RPCv2
 
 use nom::IResult;
-use nom::combinator::rest;
+use nom::combinator::{rest, rest_len};
 use nom::number::streaming::be_u32;
 
 pub const RPC_MAX_MACHINE_SIZE: u32 = 256; // Linux kernel defines 64.
@@ -158,7 +158,8 @@ pub struct RpcReplyPacket<'a> {
     pub reply_state: u32,
     pub accept_state: u32,
 
-    pub prog_data: &'a[u8],
+    pub prog_data_size: u32,
+    pub prog_data: &'a [u8],
 }
 
 // top of request packet, just to get to procedure
@@ -206,13 +207,24 @@ pub struct RpcPacket<'a> {
     pub verifier_len: u32,
     pub verifier: &'a[u8],
 
+    pub prog_data_size: u32,
     pub prog_data: &'a[u8],
 }
 
-named!(pub parse_rpc<RpcPacket>,
+// 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`
+//
+named_args!(pub parse_rpc(start_i: usize, complete: bool) <RpcPacket>,
    do_parse!(
        hdr: parse_rpc_packet_header
-
        >> rpcver: be_u32
        >> program: be_u32
        >> progver: be_u32
@@ -228,9 +240,10 @@ named!(pub parse_rpc<RpcPacket>,
        >> verifier_flavor: be_u32
        >> verifier_len: verify!(be_u32, |&size| size < RPC_MAX_VERIFIER_SIZE)
        >> verifier: take!(verifier_len as usize)
-
-       >> pl: rest
-
+       >> rem_len: rest_len
+       >> prog_data_size: value!(hdr.frag_len + 4 - (start_i - rem_len) as u32)
+       >> prog_data_f: cond!(complete, take!(prog_data_size))
+       >> prog_data_t: cond!(!complete, rest)
        >> (
            RpcPacket {
                 hdr:hdr,
@@ -247,13 +260,24 @@ named!(pub parse_rpc<RpcPacket>,
                 verifier_len:verifier_len,
                 verifier:verifier,
 
-                prog_data:pl,
+                prog_data_size:prog_data_size as u32,
+                prog_data: if complete { prog_data_f.unwrap() } else { prog_data_t.unwrap() },
            }
    ))
 );
 
-// to be called with data <= hdr.frag_len + 4. Sending more data is undefined.
-named!(pub parse_rpc_reply<RpcReplyPacket>,
+// 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`
+//
+named_args!(pub parse_rpc_reply(start_i: usize, complete: bool) <RpcReplyPacket>,
    do_parse!(
        hdr: parse_rpc_packet_header
 
@@ -264,8 +288,10 @@ named!(pub parse_rpc_reply<RpcReplyPacket>,
        >> verifier: cond!(verifier_len > 0, take!(verifier_len as usize))
 
        >> accept_state: be_u32
-
-       >> pl: rest
+       >> rem_len: rest_len
+       >> prog_data_size: value!(hdr.frag_len + 4 - (start_i - rem_len) as u32)
+       >> prog_data_f: cond!(complete, take!(prog_data_size))
+       >> prog_data_t: cond!(!complete, rest)
 
        >> (
            RpcReplyPacket {
@@ -278,7 +304,8 @@ named!(pub parse_rpc_reply<RpcReplyPacket>,
                 reply_state:reply_state,
                 accept_state:accept_state,
 
-                prog_data:pl,
+                prog_data_size:prog_data_size as u32,
+                prog_data: if complete { prog_data_f.unwrap() } else { prog_data_t.unwrap() },
            }
    ))
 );
@@ -334,7 +361,7 @@ named!(pub parse_rpc_udp_request<RpcPacket>,
                 verifier_flavor:verifier_flavor,
                 verifier_len:verifier_len,
                 verifier:verifier,
-
+                prog_data_size:pl.len() as u32,
                 prog_data:pl,
            }
    ))
@@ -363,8 +390,8 @@ named!(pub parse_rpc_udp_reply<RpcReplyPacket>,
 
                 reply_state:reply_state,
                 accept_state:accept_state,
-
-                prog_data:pl,
+                prog_data_size:pl.len() as u32,
+                prog_data:pl
            }
    ))
 );