Skip to content

Commit

Permalink
rust: remove unneeded truncate fn calls
Browse files Browse the repository at this point in the history
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
  • Loading branch information
inashivb committed Oct 8, 2024
1 parent 153d48b commit f7bdfd3
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 49 deletions.
31 changes: 0 additions & 31 deletions rust/src/dcerpc/dcerpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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..) {
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 1 addition & 18 deletions rust/src/smb/smb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
};
Expand Down

0 comments on commit f7bdfd3

Please sign in to comment.