]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
smb: convert to return AppLayerResult
authorVictor Julien <victor@inliniac.net>
Mon, 9 Mar 2020 18:33:38 +0000 (19:33 +0100)
committerVictor Julien <victor@inliniac.net>
Tue, 17 Mar 2020 21:02:19 +0000 (22:02 +0100)
Support returning 'incomplete' and remove the buffering
code from the parser.

rust/src/smb/debug.rs
rust/src/smb/smb.rs
src/app-layer-smb.c

index da1a94fe7a1ded55de0883c6008b5171357dbc09..8c2178c255f192ffb67826da2011764c0dd7f22e 100644 (file)
@@ -1,4 +1,4 @@
-/* Copyright (C) 2018 Open Information Security Foundation
+/* Copyright (C) 2018-2020 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -73,6 +73,6 @@ impl SMBState {
 
     #[cfg(feature = "debug")]
     pub fn _debug_state_stats(&self) {
-        SCLogDebug!("ssn2vec_map {} guid2name_map {} ssn2vecoffset_map {} ssn2tree_map {} ssnguid2vec_map {} tcp_buffer_ts {} tcp_buffer_tc {} file_ts_guid {} file_tc_guid {} transactions {}", self.ssn2vec_map.len(), self.guid2name_map.len(), self.ssn2vecoffset_map.len(), self.ssn2tree_map.len(), self.ssnguid2vec_map.len(), self.tcp_buffer_ts.len(), self.tcp_buffer_tc.len(), self.file_ts_guid.len(), self.file_tc_guid.len(), self.transactions.len());
+        SCLogDebug!("ssn2vec_map {} guid2name_map {} ssn2vecoffset_map {} ssn2tree_map {} ssnguid2vec_map {} file_ts_guid {} file_tc_guid {} transactions {}", self.ssn2vec_map.len(), self.guid2name_map.len(), self.ssn2vecoffset_map.len(), self.ssn2tree_map.len(), self.ssnguid2vec_map.len(), self.file_ts_guid.len(), self.file_tc_guid.len(), self.transactions.len());
     }
 }
index 565c800fa568c6580b5e5792294e9f4bd778fb1d..4e1a3e7fa5000ed34972715a6f2e348dd607cb77 100644 (file)
@@ -38,6 +38,7 @@ use crate::core::*;
 use crate::log::*;
 use crate::applayer;
 use crate::applayer::LoggerFlags;
+use crate::parser::AppLayerResult;
 
 use crate::smb::nbss_records::*;
 use crate::smb::smb1_records::*;
@@ -761,10 +762,6 @@ pub struct SMBState<> {
     // requests for DCERPC.
     pub ssnguid2vec_map: HashMap<SMBHashKeyHdrGuid, Vec<u8>>,
 
-    /// TCP segments defragmentation buffer
-    pub tcp_buffer_ts: Vec<u8>,
-    pub tcp_buffer_tc: Vec<u8>,
-
     pub files: SMBFiles,
 
     skip_ts: u32,
@@ -817,8 +814,6 @@ impl SMBState {
             ssn2vecoffset_map:HashMap::new(),
             ssn2tree_map:HashMap::new(),
             ssnguid2vec_map:HashMap::new(),
-            tcp_buffer_ts:Vec::new(),
-            tcp_buffer_tc:Vec::new(),
             files: SMBFiles::new(),
             skip_ts:0,
             skip_tc:0,
@@ -1328,6 +1323,7 @@ impl SMBState {
 
                                 }
                             } else if smb.version == 0xfe_u8 { // SMB2
+                                SCLogDebug!("NBSS record {:?}", nbss_part_hdr);
                                 SCLogDebug!("SMBv2 record");
                                 match parse_smb2_request_record(&nbss_part_hdr.data) {
                                     Ok((_, ref smb_record)) => {
@@ -1336,6 +1332,7 @@ impl SMBState {
                                         if smb_record.command == SMB2_COMMAND_WRITE {
                                             smb2_write_request_record(self, smb_record);
                                             let consumed = input.len() - output.len();
+                                            SCLogDebug!("consumed {}", consumed);
                                             return consumed;
                                         }
                                     },
@@ -1355,35 +1352,14 @@ impl SMBState {
     }
 
     /// Parsing function, handling TCP chunks fragmentation
-    pub fn parse_tcp_data_ts<'b>(&mut self, i: &'b[u8]) -> u32
+    pub fn parse_tcp_data_ts<'b>(&mut self, i: &'b[u8]) -> AppLayerResult
     {
-        let mut v : Vec<u8>;
-        //println!("parse_tcp_data_ts ({})",i.len());
-        //println!("{:?}",i);
-        // Check if TCP data is being defragmented
-        let tcp_buffer = match self.tcp_buffer_ts.len() {
-            0 => i,
-            _ => {
-                v = self.tcp_buffer_ts.split_off(0);
-                if self.tcp_buffer_ts.len() + i.len() > 100000 {
-                    self.set_event(SMBEvent::RecordOverflow);
-                    return 1;
-                };
-                v.extend_from_slice(i);
-                v.as_slice()
-            },
-        };
-        //println!("tcp_buffer ({})",tcp_buffer.len());
-        let mut cur_i = tcp_buffer;
-        if cur_i.len() > 1000000 {
-            self.set_event(SMBEvent::RecordOverflow);
-            return 1;
-        }
+        let mut cur_i = i;
         let consumed = self.handle_skip(STREAM_TOSERVER, cur_i.len() as u32);
         if consumed > 0 {
             if consumed > cur_i.len() as u32 {
                 self.set_event(SMBEvent::InternalError);
-                return 1;
+                return AppLayerResult::err();
             }
             cur_i = &cur_i[consumed as usize..];
         }
@@ -1393,30 +1369,42 @@ impl SMBState {
         if consumed > 0 {
             if consumed > cur_i.len() as u32 {
                 self.set_event(SMBEvent::InternalError);
-                return 1;
+                return AppLayerResult::err();
             }
             cur_i = &cur_i[consumed as usize..];
         }
+        if cur_i.len() == 0 {
+            return AppLayerResult::ok();
+        }
         // gap
         if self.ts_gap {
             SCLogDebug!("TS trying to catch up after GAP (input {})", cur_i.len());
-            match search_smb_record(cur_i) {
-                Ok((_, pg)) => {
-                    SCLogDebug!("smb record found");
-                    let smb2_offset = cur_i.len() - pg.len();
-                    if smb2_offset < 4 {
-                        return 0;
-                    }
-                    let nbss_offset = smb2_offset - 4;
-                    cur_i = &cur_i[nbss_offset..];
+            while cur_i.len() > 0 { // min record size
+                match search_smb_record(cur_i) {
+                    Ok((_, pg)) => {
+                        SCLogDebug!("smb record found");
+                        let smb2_offset = cur_i.len() - pg.len();
+                        if smb2_offset < 4 {
+                            cur_i = &cur_i[smb2_offset+4..];
+                            continue; // see if we have another record in our data
+                        }
+                        let nbss_offset = smb2_offset - 4;
+                        cur_i = &cur_i[nbss_offset..];
 
-                    self.ts_gap = false;
-                },
-                _ => {
-                    SCLogDebug!("smb record NOT found");
-                    self.tcp_buffer_ts.extend_from_slice(cur_i);
-                    return 0;
-                },
+                        self.ts_gap = false;
+                        break;
+                    },
+                    _ => {
+                        let mut consumed = i.len();
+                        if consumed < 4 {
+                            consumed = 0;
+                        } else {
+                            consumed = consumed - 3;
+                        }
+                        SCLogDebug!("smb record NOT found");
+                        return AppLayerResult::incomplete(consumed as u32, 8);
+                    },
+                }
             }
         }
         while cur_i.len() > 0 { // min record size
@@ -1436,7 +1424,7 @@ impl SMBState {
                                         },
                                         _ => {
                                             self.set_event(SMBEvent::MalformedData);
-                                            return 1;
+                                            return AppLayerResult::err();
                                         },
                                     }
                                 } else if smb.version == 0xfe_u8 { // SMB2
@@ -1452,7 +1440,7 @@ impl SMBState {
                                             },
                                             _ => {
                                                 self.set_event(SMBEvent::MalformedData);
-                                                return 1;
+                                                return AppLayerResult::err();
                                             },
                                         }
                                     }
@@ -1466,7 +1454,7 @@ impl SMBState {
                                             },
                                             _ => {
                                                 self.set_event(SMBEvent::MalformedData);
-                                                return 1;
+                                                return AppLayerResult::err();
                                             },
                                         }
                                     }
@@ -1474,7 +1462,7 @@ impl SMBState {
                             },
                             _ => {
                                 self.set_event(SMBEvent::MalformedData);
-                                return 1;
+                                return AppLayerResult::err();
                             },
                         }
                     } else {
@@ -1482,16 +1470,34 @@ impl SMBState {
                     }
                     cur_i = rem;
                 },
-                Err(nom::Err::Incomplete(_)) => {
-                    let consumed = self.parse_tcp_data_ts_partial(cur_i);
-                    cur_i = &cur_i[consumed ..];
-
-                    self.tcp_buffer_ts.extend_from_slice(cur_i);
-                    break;
+                Err(nom::Err::Incomplete(needed)) => {
+                    if let nom::Needed::Size(n) = needed {
+                        // 512 is the minimum for parse_tcp_data_ts_partial
+                        if n >= 512 && cur_i.len() < 512 {
+                            let total_consumed = i.len() - cur_i.len();
+                            return AppLayerResult::incomplete(total_consumed as u32, 512);
+                        }
+                        let consumed = self.parse_tcp_data_ts_partial(cur_i);
+                        if consumed == 0 {
+                            // if we consumed none we will buffer the entire record
+                            let total_consumed = i.len() - cur_i.len();
+                            SCLogDebug!("setting consumed {} need {} needed {:?} total input {}",
+                                    total_consumed, n, needed, i.len());
+                            let need = n + 4; // Incomplete returns size of data minus NBSS header
+                            return AppLayerResult::incomplete(total_consumed as u32, need as u32);
+                        }
+                        // tracking a write record, which we don't need to
+                        // queue up at the stream level, but can feed to us
+                        // in small chunks
+                        return AppLayerResult::ok();
+                    } else {
+                        self.set_event(SMBEvent::InternalError);
+                        return AppLayerResult::err();
+                    }
                 },
                 Err(_) => {
                     self.set_event(SMBEvent::MalformedData);
-                    return 1;
+                    return AppLayerResult::err();
                 },
             }
         };
@@ -1500,7 +1506,7 @@ impl SMBState {
         if self.check_post_gap_file_txs {
             self.post_gap_housekeeping_for_files();
         }
-        0
+        AppLayerResult::ok()
     }
 
     /// return bytes consumed
@@ -1584,33 +1590,14 @@ impl SMBState {
     }
 
     /// Parsing function, handling TCP chunks fragmentation
-    pub fn parse_tcp_data_tc<'b>(&mut self, i: &'b[u8]) -> u32
+    pub fn parse_tcp_data_tc<'b>(&mut self, i: &'b[u8]) -> AppLayerResult
     {
-        let mut v : Vec<u8>;
-        // Check if TCP data is being defragmented
-        let tcp_buffer = match self.tcp_buffer_tc.len() {
-            0 => i,
-            _ => {
-                v = self.tcp_buffer_tc.split_off(0);
-                if self.tcp_buffer_tc.len() + i.len() > 100000 {
-                    self.set_event(SMBEvent::RecordOverflow);
-                    return 1;
-                };
-                v.extend_from_slice(i);
-                v.as_slice()
-            },
-        };
-        let mut cur_i = tcp_buffer;
-        SCLogDebug!("cur_i.len {}", cur_i.len());
-        if cur_i.len() > 100000 {
-            self.set_event(SMBEvent::RecordOverflow);
-            return 1;
-        }
+        let mut cur_i = i;
         let consumed = self.handle_skip(STREAM_TOCLIENT, cur_i.len() as u32);
         if consumed > 0 {
             if consumed > cur_i.len() as u32 {
                 self.set_event(SMBEvent::InternalError);
-                return 1;
+                return AppLayerResult::err();
             }
             cur_i = &cur_i[consumed as usize..];
         }
@@ -1620,30 +1607,42 @@ impl SMBState {
         if consumed > 0 {
             if consumed > cur_i.len() as u32 {
                 self.set_event(SMBEvent::InternalError);
-                return 1;
+                return AppLayerResult::err();
             }
             cur_i = &cur_i[consumed as usize..];
         }
+        if cur_i.len() == 0 {
+            return AppLayerResult::ok();
+        }
         // gap
         if self.tc_gap {
             SCLogDebug!("TC trying to catch up after GAP (input {})", cur_i.len());
-            match search_smb_record(cur_i) {
-                Ok((_, pg)) => {
-                    SCLogDebug!("smb record found");
-                    let smb2_offset = cur_i.len() - pg.len();
-                    if smb2_offset < 4 {
-                        return 0;
-                    }
-                    let nbss_offset = smb2_offset - 4;
-                    cur_i = &cur_i[nbss_offset..];
+            while cur_i.len() > 0 { // min record size
+                match search_smb_record(cur_i) {
+                    Ok((_, pg)) => {
+                        SCLogDebug!("smb record found");
+                        let smb2_offset = cur_i.len() - pg.len();
+                        if smb2_offset < 4 {
+                            cur_i = &cur_i[smb2_offset+4..];
+                            continue; // see if we have another record in our data
+                        }
+                        let nbss_offset = smb2_offset - 4;
+                        cur_i = &cur_i[nbss_offset..];
 
-                    self.tc_gap = false;
-                },
-                _ => {
-                    SCLogDebug!("smb record NOT found");
-                    self.tcp_buffer_tc.extend_from_slice(cur_i);
-                    return 0;
-                },
+                        self.tc_gap = false;
+                        break;
+                    },
+                    _ => {
+                        let mut consumed = i.len();
+                        if consumed < 4 {
+                            consumed = 0;
+                        } else {
+                            consumed = consumed - 3;
+                        }
+                        SCLogDebug!("smb record NOT found");
+                        return AppLayerResult::incomplete(consumed as u32, 8);
+                    },
+                }
             }
         }
         while cur_i.len() > 0 { // min record size
@@ -1663,7 +1662,7 @@ impl SMBState {
                                         },
                                         _ => {
                                             self.set_event(SMBEvent::MalformedData);
-                                            return 1;
+                                            return AppLayerResult::err();
                                         },
                                     }
                                 } else if smb.version == 0xfe_u8 { // SMB2
@@ -1677,7 +1676,7 @@ impl SMBState {
                                             },
                                             _ => {
                                                 self.set_event(SMBEvent::MalformedData);
-                                                return 1;
+                                                return AppLayerResult::err();
                                             },
                                         }
                                     }
@@ -1691,7 +1690,7 @@ impl SMBState {
                                             },
                                             _ => {
                                                 self.set_event(SMBEvent::MalformedData);
-                                                return 1;
+                                                return AppLayerResult::err();
                                             },
                                         }
                                     }
@@ -1703,7 +1702,7 @@ impl SMBState {
                             },
                             Err(_) => {
                                 self.set_event(SMBEvent::MalformedData);
-                                return 1;
+                                return AppLayerResult::err();
                             },
                         }
                     } else {
@@ -1713,16 +1712,33 @@ impl SMBState {
                 },
                 Err(nom::Err::Incomplete(needed)) => {
                     SCLogDebug!("INCOMPLETE have {} needed {:?}", cur_i.len(), needed);
-                    let consumed = self.parse_tcp_data_tc_partial(cur_i);
-                    cur_i = &cur_i[consumed ..];
-
-                    SCLogDebug!("INCOMPLETE have {}", cur_i.len());
-                    self.tcp_buffer_tc.extend_from_slice(cur_i);
-                    break;
+                    if let nom::Needed::Size(n) = needed {
+                        // 512 is the minimum for parse_tcp_data_tc_partial
+                        if n >= 512 && cur_i.len() < 512 {
+                            let total_consumed = i.len() - cur_i.len();
+                            return AppLayerResult::incomplete(total_consumed as u32, 512);
+                        }
+                        let consumed = self.parse_tcp_data_tc_partial(cur_i);
+                        if consumed == 0 {
+                            // if we consumed none we will buffer the entire record
+                            let total_consumed = i.len() - cur_i.len();
+                            SCLogDebug!("setting consumed {} need {} needed {:?} total input {}",
+                                    total_consumed, n, needed, i.len());
+                            let need = n + 4; // Incomplete returns size of data minus NBSS header
+                            return AppLayerResult::incomplete(total_consumed as u32, need as u32);
+                        }
+                        // tracking a read record, which we don't need to
+                        // queue up at the stream level, but can feed to us
+                        // in small chunks
+                        return AppLayerResult::ok();
+                    } else {
+                        self.set_event(SMBEvent::InternalError);
+                        return AppLayerResult::err();
+                    }
                 },
                 Err(_) => {
                     self.set_event(SMBEvent::MalformedData);
-                    return 1;
+                    return AppLayerResult::err();
                 },
             }
         };
@@ -1731,15 +1747,12 @@ impl SMBState {
             self.post_gap_housekeeping_for_files();
         }
         self._debug_tx_stats();
-        0
+        AppLayerResult::ok()
     }
 
     /// handle a gap in the TOSERVER direction
     /// returns: 0 ok, 1 unrecoverable error
     pub fn parse_tcp_data_ts_gap(&mut self, gap_size: u32) -> u32 {
-        if self.tcp_buffer_ts.len() > 0 {
-            self.tcp_buffer_ts.clear();
-        }
         let consumed = self.handle_skip(STREAM_TOSERVER, gap_size);
         if consumed < gap_size {
             let new_gap_size = gap_size - consumed;
@@ -1761,9 +1774,6 @@ impl SMBState {
     /// handle a gap in the TOCLIENT direction
     /// returns: 0 ok, 1 unrecoverable error
     pub fn parse_tcp_data_tc_gap(&mut self, gap_size: u32) -> u32 {
-        if self.tcp_buffer_tc.len() > 0 {
-            self.tcp_buffer_tc.clear();
-        }
         let consumed = self.handle_skip(STREAM_TOCLIENT, gap_size);
         if consumed < gap_size {
             let new_gap_size = gap_size - consumed;
@@ -1785,7 +1795,6 @@ impl SMBState {
     pub fn trunc_ts(&mut self) {
         SCLogDebug!("TRUNC TS");
         self.ts_trunc = true;
-        self.tcp_buffer_ts.clear();
 
         for tx in &mut self.transactions {
             if !tx.request_done {
@@ -1797,7 +1806,6 @@ impl SMBState {
     pub fn trunc_tc(&mut self) {
         SCLogDebug!("TRUNC TC");
         self.tc_trunc = true;
-        self.tcp_buffer_tc.clear();
 
         for tx in &mut self.transactions {
             if !tx.response_done {
@@ -1836,7 +1844,7 @@ pub extern "C" fn rs_smb_parse_request_tcp(flow: &mut Flow,
                                        input_len: u32,
                                        _data: *mut std::os::raw::c_void,
                                        flags: u8)
-                                       -> i8
+                                       -> AppLayerResult
 {
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
     SCLogDebug!("parsing {} bytes of request data", input_len);
@@ -1847,11 +1855,7 @@ pub extern "C" fn rs_smb_parse_request_tcp(flow: &mut Flow,
     }
 
     state.ts = flow.get_last_time().as_secs();
-    if state.parse_tcp_data_ts(buf) == 0 {
-        return 0;
-    } else {
-        return -1;
-    }
+    state.parse_tcp_data_ts(buf)
 }
 
 #[no_mangle]
@@ -1875,7 +1879,7 @@ pub extern "C" fn rs_smb_parse_response_tcp(flow: &mut Flow,
                                         input_len: u32,
                                         _data: *mut std::os::raw::c_void,
                                         flags: u8)
-                                        -> i8
+                                        -> AppLayerResult
 {
     SCLogDebug!("parsing {} bytes of response data", input_len);
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
@@ -1886,11 +1890,7 @@ pub extern "C" fn rs_smb_parse_response_tcp(flow: &mut Flow,
     }
 
     state.ts = flow.get_last_time().as_secs();
-    if state.parse_tcp_data_tc(buf) == 0 {
-        return 0;
-    } else {
-        return -1;
-    }
+    state.parse_tcp_data_tc(buf)
 }
 
 #[no_mangle]
index 234738d4b4ad2126da617f0b9966f57f8eb0317f..d3b2d3416aab309425b7babb393f6d390ff28933 100644 (file)
@@ -38,19 +38,20 @@ static AppLayerResult SMBTCPParseRequest(Flow *f, void *state,
     uint16_t file_flags = FileFlowToFlags(f, STREAM_TOSERVER);
     rs_smb_setfileflags(0, state, file_flags|FILE_USE_DETECT);
 
-    int res;
     if (input == NULL && input_len > 0) {
-        res = rs_smb_parse_request_tcp_gap(state, input_len);
+        int res = rs_smb_parse_request_tcp_gap(state, input_len);
+        SCLogDebug("SMB request GAP of %u bytes, retval %d", input_len, res);
+        if (res != 0) {
+            SCReturnStruct(APP_LAYER_ERROR);
+        }
+        SCReturnStruct(APP_LAYER_OK);
     } else {
-        res = rs_smb_parse_request_tcp(f, state, pstate, input, input_len,
-            local_data, flags);
-    }
-    if (res != 0) {
+        AppLayerResult res = rs_smb_parse_request_tcp(f, state, pstate,
+                input, input_len, local_data, flags);
         SCLogDebug("SMB request%s of %u bytes, retval %d",
-                (input == NULL && input_len > 0) ? " is GAP" : "", input_len, res);
-        SCReturnStruct(APP_LAYER_ERROR);
+                (input == NULL && input_len > 0) ? " is GAP" : "", input_len, res.status);
+        SCReturnStruct(res);
     }
-    SCReturnStruct(APP_LAYER_OK);
 }
 
 static AppLayerResult SMBTCPParseResponse(Flow *f, void *state,
@@ -62,19 +63,19 @@ static AppLayerResult SMBTCPParseResponse(Flow *f, void *state,
     rs_smb_setfileflags(1, state, file_flags|FILE_USE_DETECT);
 
     SCLogDebug("SMBTCPParseResponse %p/%u", input, input_len);
-    int res;
     if (input == NULL && input_len > 0) {
-        res = rs_smb_parse_response_tcp_gap(state, input_len);
+        int res = rs_smb_parse_response_tcp_gap(state, input_len);
+        if (res != 0) {
+            SCLogDebug("SMB response%s of %u bytes, retval %d",
+                    (input == NULL && input_len > 0) ? " is GAP" : "", input_len, res);
+            SCReturnStruct(APP_LAYER_ERROR);
+        }
+        SCReturnStruct(APP_LAYER_OK);
     } else {
-        res = rs_smb_parse_response_tcp(f, state, pstate, input, input_len,
-            local_data, flags);
-    }
-    if (res != 0) {
-        SCLogDebug("SMB response%s of %u bytes, retval %d",
-                (input == NULL && input_len > 0) ? " is GAP" : "", input_len, res);
-        SCReturnStruct(APP_LAYER_ERROR);
+        AppLayerResult res = rs_smb_parse_response_tcp(f, state, pstate,
+                input, input_len, local_data, flags);
+        SCReturnStruct(res);
     }
-    SCReturnStruct(APP_LAYER_OK);
 }
 
 static uint16_t SMBTCPProbe(Flow *f, uint8_t direction,