From 0b2215fe6cc4a7982bf6e96fbd5584616919c031 Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Mon, 7 Oct 2024 15:13:58 +0530 Subject: [PATCH 1/2] applayer: remove truncation logic as its functionality is already covered by the generic code. This leaves APP_LAYER_PARSER_TRUNC_TC and APP_LAYER_PARSER_TRUNC_TS to avoid introducing any breaking changes. FlowGetDisruptionFlags sets STREAM_DEPTH flag in case the respective stream depth was reached. This flag tells that whether all the open files should be truncated or not. Bug 7044 --- rust/src/applayer.rs | 4 ++-- src/app-layer-parser.c | 34 +++------------------------------- src/app-layer-parser.h | 4 ++++ src/output-tx.c | 6 ++---- 4 files changed, 11 insertions(+), 37 deletions(-) diff --git a/rust/src/applayer.rs b/rust/src/applayer.rs index ff19d8763234..74ffc5787c98 100644 --- a/rust/src/applayer.rs +++ b/rust/src/applayer.rs @@ -504,8 +504,8 @@ pub const APP_LAYER_PARSER_NO_INSPECTION_PAYLOAD : u16 = BIT_U16!(3); pub const APP_LAYER_PARSER_BYPASS_READY : u16 = BIT_U16!(4); pub const APP_LAYER_PARSER_EOF_TS : u16 = BIT_U16!(5); pub const APP_LAYER_PARSER_EOF_TC : u16 = BIT_U16!(6); -pub const APP_LAYER_PARSER_TRUNC_TS : u16 = BIT_U16!(7); -pub const APP_LAYER_PARSER_TRUNC_TC : u16 = BIT_U16!(8); +pub const APP_LAYER_PARSER_TRUNC_TS : u16 = BIT_U16!(7); /* unused flag. see https://redmine.openinfosecfoundation.org/issues/7044 */ +pub const APP_LAYER_PARSER_TRUNC_TC : u16 = BIT_U16!(8); /* unused flag. see https://redmine.openinfosecfoundation.org/issues/7044 */ pub const APP_LAYER_PARSER_OPT_ACCEPT_GAPS: u32 = BIT_U32!(0); diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index a051616dd3b9..0fd5c84fd8ad 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -209,9 +209,6 @@ FramesContainer *AppLayerFramesSetupContainer(Flow *f) return f->alparser->frames; } -static inline void AppLayerParserStreamTruncated(AppLayerParserState *pstate, const uint8_t ipproto, - const AppProto alproto, void *alstate, const uint8_t direction); - #ifdef UNITTESTS void UTHAppLayerParserStateGetIds(void *ptr, uint64_t *i1, uint64_t *i2, uint64_t *log, uint64_t *min) { @@ -968,11 +965,8 @@ void AppLayerParserTransactionsCleanup(Flow *f, const uint8_t pkt_dir) AppLayerTxData *txd = AppLayerParserGetTxData(ipproto, alproto, tx); if (txd != NULL && AppLayerParserHasFilesInDir(txd, pkt_dir)) { if (pkt_dir_trunc == -1) - pkt_dir_trunc = - AppLayerParserStateIssetFlag(f->alparser, - (pkt_dir == STREAM_TOSERVER) ? APP_LAYER_PARSER_TRUNC_TS - : APP_LAYER_PARSER_TRUNC_TC) != 0; - + pkt_dir_trunc = IS_DISRUPTED( + (pkt_dir == STREAM_TOSERVER) ? ts_disrupt_flags : tc_disrupt_flags); AppLayerParserFileTxHousekeeping(f, tx, pkt_dir, (bool)pkt_dir_trunc); } @@ -1334,7 +1328,7 @@ int AppLayerParserParse(ThreadVars *tv, AppLayerParserThreadCtx *alp_tctx, Flow if (!(p->option_flags & APP_LAYER_PARSER_OPT_ACCEPT_GAPS)) { SCLogDebug("app-layer parser does not accept gaps"); if (f->alstate != NULL && !FlowChangeProto(f)) { - AppLayerParserStreamTruncated(pstate, f->proto, alproto, f->alstate, flags); + AppLayerParserTriggerRawStreamReassembly(f, direction); } AppLayerIncGapErrorCounter(tv, f); goto error; @@ -1494,10 +1488,6 @@ int AppLayerParserParse(ThreadVars *tv, AppLayerParserThreadCtx *alp_tctx, Flow AppLayerIncTxCounter(tv, f, cur_tx_cnt - p_tx_cnt); } - /* stream truncated, inform app layer */ - if (flags & STREAM_DEPTH) - AppLayerParserStreamTruncated(pstate, f->proto, alproto, f->alstate, flags); - end: /* update app progress */ if (consumed != input_len && f->proto == IPPROTO_TCP && f->protoctx != NULL) { @@ -1814,24 +1804,6 @@ uint16_t AppLayerParserStateIssetFlag(AppLayerParserState *pstate, uint16_t flag SCReturnUInt(pstate->flags & flag); } -static inline void AppLayerParserStreamTruncated(AppLayerParserState *pstate, const uint8_t ipproto, - const AppProto alproto, void *alstate, const uint8_t direction) -{ - SCEnter(); - - if (direction & STREAM_TOSERVER) { - AppLayerParserStateSetFlag(pstate, APP_LAYER_PARSER_TRUNC_TS); - } else { - AppLayerParserStateSetFlag(pstate, APP_LAYER_PARSER_TRUNC_TC); - } - - if (alp_ctx.ctxs[FlowGetProtoMapping(ipproto)][alproto].Truncate != NULL) { - alp_ctx.ctxs[FlowGetProtoMapping(ipproto)][alproto].Truncate(alstate, direction); - } - - SCReturn; -} - #ifdef DEBUG void AppLayerParserStatePrintDetails(AppLayerParserState *pstate) { diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h index d27a08c85119..9751833b99c8 100644 --- a/src/app-layer-parser.h +++ b/src/app-layer-parser.h @@ -38,8 +38,12 @@ #define APP_LAYER_PARSER_BYPASS_READY BIT_U16(4) #define APP_LAYER_PARSER_EOF_TS BIT_U16(5) #define APP_LAYER_PARSER_EOF_TC BIT_U16(6) + +/* unused flags APP_LAYER_PARSER_TRUNC_*. see https://redmine.openinfosecfoundation.org/issues/7044 + */ #define APP_LAYER_PARSER_TRUNC_TS BIT_U16(7) #define APP_LAYER_PARSER_TRUNC_TC BIT_U16(8) + #define APP_LAYER_PARSER_SFRAME_TS BIT_U16(9) #define APP_LAYER_PARSER_SFRAME_TC BIT_U16(10) diff --git a/src/output-tx.c b/src/output-tx.c index 042b424adec9..f3080ec16431 100644 --- a/src/output-tx.c +++ b/src/output-tx.c @@ -379,10 +379,8 @@ static TmEcode OutputTxLog(ThreadVars *tv, Packet *p, void *thread_data) SCLogDebug("pcap_cnt %" PRIu64, p->pcap_cnt); const bool last_pseudo = (p->flowflags & FLOW_PKT_LAST_PSEUDO) != 0; - const bool ts_eof = AppLayerParserStateIssetFlag(f->alparser, - (APP_LAYER_PARSER_EOF_TS | APP_LAYER_PARSER_TRUNC_TS)) != 0; - const bool tc_eof = AppLayerParserStateIssetFlag(f->alparser, - (APP_LAYER_PARSER_EOF_TC | APP_LAYER_PARSER_TRUNC_TC)) != 0; + const bool ts_eof = AppLayerParserStateIssetFlag(f->alparser, APP_LAYER_PARSER_EOF_TS) != 0; + const bool tc_eof = AppLayerParserStateIssetFlag(f->alparser, APP_LAYER_PARSER_EOF_TC) != 0; const bool eof = last_pseudo || (ts_eof && tc_eof); SCLogDebug("eof %d last_pseudo %d ts_eof %d tc_eof %d", eof, last_pseudo, ts_eof, tc_eof); From edec53885b25b288ea460c7d36bf67369b30ff5f Mon Sep 17 00:00:00 2001 From: Shivani Bhardwaj Date: Mon, 7 Oct 2024 13:41:08 +0530 Subject: [PATCH 2/2] rust: remove unneeded truncate fn calls truncate fn is only active and used by dcerpc and smb parsers. In case stream depth is reached for any side, truncate fn is supposed to set the tx entity (request/response) in the same direction as complete so the other side is not forever waiting for data. However, whether the stream depth is reached is already checked by AppLayerParserGetStateProgress fn which is called by: - DetectTx - DetectEngineInspectBufferGeneric - AppLayerParserSetTransactionInspectId - OutputTxLog - AppLayerParserTransactionsCleanup and, in such a case, StateGetProgressCompletionStatus is returned for the respective direction. This fn following efc9a7a, always returns 1 as long as the direction is valid meaning that the progress for the current direction is marked complete. So, there is no need for the additional callback to mark the entities as done in case of depth or a gap. Remove all calls to the truncate fn but leave the registration fn intact to avoid introducing any breaking API changes. Bug 7044 --- rust/src/dcerpc/dcerpc.rs | 31 ------------------------------- rust/src/smb/smb.rs | 19 +------------------ 2 files changed, 1 insertion(+), 49 deletions(-) diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs index 345d46656624..ad1e97e28644 100644 --- a/rust/src/dcerpc/dcerpc.rs +++ b/rust/src/dcerpc/dcerpc.rs @@ -322,8 +322,6 @@ pub struct DCERPCState { pub tc_gap: bool, pub ts_ssn_gap: bool, pub tc_ssn_gap: bool, - pub ts_ssn_trunc: bool, /// true if Truncated in this direction - pub tc_ssn_trunc: bool, pub flow: Option<*const core::Flow>, state_data: AppLayerStateData, } @@ -354,8 +352,6 @@ impl DCERPCState { tx.call_id = call_id; tx.endianness = endianness; self.tx_id += 1; - tx.req_done = self.ts_ssn_trunc; - tx.resp_done = self.tc_ssn_trunc; if self.transactions.len() > unsafe { DCERPC_MAX_TX } { let mut index = self.tx_index_completed; for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) { @@ -1193,33 +1189,6 @@ pub unsafe extern "C" fn rs_dcerpc_state_transaction_free(state: *mut std::os::r dce_state.free_tx(tx_id); } -#[no_mangle] -pub unsafe extern "C" fn rs_dcerpc_state_trunc(state: *mut std::os::raw::c_void, direction: u8) { - let dce_state = cast_pointer!(state, DCERPCState); - match direction.into() { - Direction::ToServer => { - dce_state.ts_ssn_trunc = true; - for tx in &mut dce_state.transactions { - tx.req_done = true; - if let Some(flow) = dce_state.flow { - sc_app_layer_parser_trigger_raw_stream_reassembly(flow, Direction::ToServer as i32); - } - } - SCLogDebug!("dce_state.ts_ssn_trunc = true; txs {}", dce_state.transactions.len()); - } - Direction::ToClient => { - dce_state.tc_ssn_trunc = true; - for tx in &mut dce_state.transactions { - tx.resp_done = true; - if let Some(flow) = dce_state.flow { - sc_app_layer_parser_trigger_raw_stream_reassembly(flow, Direction::ToClient as i32); - } - } - SCLogDebug!("dce_state.tc_ssn_trunc = true; txs {}", dce_state.transactions.len()); - } - } -} - #[no_mangle] pub unsafe extern "C" fn rs_dcerpc_get_tx( vtx: *mut std::os::raw::c_void, tx_id: u64, diff --git a/rust/src/smb/smb.rs b/rust/src/smb/smb.rs index e64e0a8e6f0b..cd1a83acafee 100644 --- a/rust/src/smb/smb.rs +++ b/rust/src/smb/smb.rs @@ -2207,23 +2207,6 @@ pub unsafe extern "C" fn rs_smb_get_tx_data( return &mut tx.tx_data; } - -#[no_mangle] -pub unsafe extern "C" fn rs_smb_state_truncate( - state: *mut std::ffi::c_void, - direction: u8) -{ - let state = cast_pointer!(state, SMBState); - match direction.into() { - Direction::ToServer => { - state.trunc_ts(); - } - Direction::ToClient => { - state.trunc_tc(); - } - } -} - #[no_mangle] pub unsafe extern "C" fn rs_smb_state_get_event_info_by_id( event_id: std::os::raw::c_int, @@ -2333,7 +2316,7 @@ pub unsafe extern "C" fn rs_smb_register_parser() { get_state_data: rs_smb_get_state_data, apply_tx_config: None, flags: APP_LAYER_PARSER_OPT_ACCEPT_GAPS, - truncate: Some(rs_smb_state_truncate), + truncate: None, get_frame_id_by_name: Some(SMBFrameType::ffi_id_from_name), get_frame_name_by_id: Some(SMBFrameType::ffi_name_from_id), };