From: Victor Julien Date: Tue, 21 Jan 2020 11:20:40 +0000 (+0100) Subject: smb: handle file transactions post-GAP X-Git-Tag: suricata-6.0.0-beta1~827 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d41aeccea459e52fc2c1a6fa2ce1000973651001;p=thirdparty%2Fsuricata.git smb: handle file transactions post-GAP After a GAP all normal transactions are closed. File transactions are left open as they can handle GAPs in principle. However, the GAP might have contained the closing of a file and therefore it may remain active until the end of the flow. This patch introduces a time based heuristic for these transactions. After the GAP all file transactions are stamped with the current timestamp. If 60 seconds later a file has seen no update, its marked as closed. This is meant to fix resource starvation issues observed in long running SMB sessions where packet loss was causing GAPs. --- diff --git a/rust/src/smb/files.rs b/rust/src/smb/files.rs index 29f051fb99..b8667e3fb8 100644 --- a/rust/src/smb/files.rs +++ b/rust/src/smb/files.rs @@ -30,6 +30,9 @@ pub struct SMBTransactionFile { pub file_name: Vec, pub share_name: Vec, pub file_tracker: FileTransferTracker, + /// after a gap, this will be set to a time in the future. If the file + /// receives no updates before that, it will be considered complete. + pub post_gap_ts: u64, } impl SMBTransactionFile { @@ -40,6 +43,7 @@ impl SMBTransactionFile { file_name: Vec::new(), share_name: Vec::new(), file_tracker: FileTransferTracker::new(), + post_gap_ts: 0, } } } @@ -201,6 +205,11 @@ impl SMBState { } } + // reset timestamp if we get called after a gap + if tdf.post_gap_ts > 0 { + tdf.post_gap_ts = 0; + } + let file_data = &data[0..data_to_handle_len]; let cs = tdf.file_tracker.update(files, flags, file_data, gap_size); cs diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs index b74bf2e914..2939108dd5 100644 --- a/rust/src/smb/smb.rs +++ b/rust/src/smb/smb.rs @@ -1,4 +1,4 @@ -/* Copyright (C) 2017 Open Information Security Foundation +/* Copyright (C) 2017-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 @@ -784,6 +784,10 @@ pub struct SMBState<> { pub ts_trunc: bool, // no more data for TOSERVER pub tc_trunc: bool, // no more data for TOCLIENT + /// true as long as we have file txs that are in a post-gap + /// state. It means we'll do extra house keeping for those. + check_post_gap_file_txs: bool, + /// transactions list pub transactions: Vec, @@ -798,6 +802,10 @@ pub struct SMBState<> { /// dcerpc interfaces, stored here to be able to match /// them while inspecting DCERPC REQUEST txs pub dcerpc_ifaces: Option>, + + /// Timestamp in seconds of last update. This is packet time, + /// potentially coming from pcaps. + ts: u64, } impl SMBState { @@ -824,11 +832,13 @@ impl SMBState { tc_gap: false, ts_trunc: false, tc_trunc: false, + check_post_gap_file_txs: false, transactions: Vec::new(), tx_id:0, dialect:0, dialect_vec: None, dcerpc_ifaces: None, + ts: 0, } } @@ -1143,9 +1153,29 @@ impl SMBState { (&name, is_dcerpc) } + fn post_gap_housekeeping_for_files(&mut self) + { + let mut post_gap_txs = false; + for tx in &mut self.transactions { + if let Some(SMBTransactionTypeData::FILE(ref f)) = tx.type_data { + if f.post_gap_ts > 0 { + if self.ts > f.post_gap_ts { + tx.request_done = true; + tx.response_done = true; + } else { + post_gap_txs = true; + } + } + } + } + self.check_post_gap_file_txs = post_gap_txs; + } + /* after a gap we will consider all transactions complete for our * direction. File transfer transactions are an exception. Those - * can handle gaps. */ + * can handle gaps. For the file transactions we set the current + * (flow) time and prune them in 60 seconds if no update for them + * was received. */ fn post_gap_housekeeping(&mut self, dir: u8) { if self.ts_ssn_gap && dir == STREAM_TOSERVER { @@ -1154,8 +1184,13 @@ impl SMBState { SCLogDebug!("post_gap_housekeeping: done"); break; } - if let Some(SMBTransactionTypeData::FILE(_)) = tx.type_data { - // leaving FILE txs open as they can deal with gaps. + if let Some(SMBTransactionTypeData::FILE(ref mut f)) = tx.type_data { + // leaving FILE txs open as they can deal with gaps. We + // remove them after 60 seconds of no activity though. + if f.post_gap_ts == 0 { + f.post_gap_ts = self.ts + 60; + self.check_post_gap_file_txs = true; + } } else { SCLogDebug!("post_gap_housekeeping: tx {} marked as done TS", tx.id); tx.request_done = true; @@ -1167,8 +1202,13 @@ impl SMBState { SCLogDebug!("post_gap_housekeeping: done"); break; } - if let Some(SMBTransactionTypeData::FILE(_)) = tx.type_data { - // leaving FILE txs open as they can deal with gaps. + if let Some(SMBTransactionTypeData::FILE(ref mut f)) = tx.type_data { + // leaving FILE txs open as they can deal with gaps. We + // remove them after 60 seconds of no activity though. + if f.post_gap_ts == 0 { + f.post_gap_ts = self.ts + 60; + self.check_post_gap_file_txs = true; + } } else { SCLogDebug!("post_gap_housekeeping: tx {} marked as done TC", tx.id); tx.request_done = true; @@ -1457,6 +1497,9 @@ impl SMBState { }; self.post_gap_housekeeping(STREAM_TOSERVER); + if self.check_post_gap_file_txs { + self.post_gap_housekeeping_for_files(); + } 0 } @@ -1684,6 +1727,9 @@ impl SMBState { } }; self.post_gap_housekeeping(STREAM_TOCLIENT); + if self.check_post_gap_file_txs { + self.post_gap_housekeeping_for_files(); + } self._debug_tx_stats(); 0 } @@ -1783,7 +1829,7 @@ pub extern "C" fn rs_smb_state_free(state: *mut std::os::raw::c_void) { /// C binding parse a SMB request. Returns 1 on success, -1 on failure. #[no_mangle] -pub extern "C" fn rs_smb_parse_request_tcp(_flow: *mut Flow, +pub extern "C" fn rs_smb_parse_request_tcp(flow: &mut Flow, state: &mut SMBState, _pstate: *mut std::os::raw::c_void, input: *const u8, @@ -1800,6 +1846,7 @@ pub extern "C" fn rs_smb_parse_request_tcp(_flow: *mut Flow, state.ts_gap = true; } + state.ts = flow.get_last_time().as_secs(); if state.parse_tcp_data_ts(buf) == 0 { return 1; } else { @@ -1821,7 +1868,7 @@ pub extern "C" fn rs_smb_parse_request_tcp_gap( #[no_mangle] -pub extern "C" fn rs_smb_parse_response_tcp(_flow: *mut Flow, +pub extern "C" fn rs_smb_parse_response_tcp(flow: &mut Flow, state: &mut SMBState, _pstate: *mut std::os::raw::c_void, input: *const u8, @@ -1838,6 +1885,7 @@ pub extern "C" fn rs_smb_parse_response_tcp(_flow: *mut Flow, state.tc_gap = true; } + state.ts = flow.get_last_time().as_secs(); if state.parse_tcp_data_tc(buf) == 0 { return 1; } else {