From ba781265a4d5c05bd5cde397dd4c1891994125aa Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 26 Nov 2020 11:03:21 +0100 Subject: [PATCH] dcerpc/udp: fix transaction handling and logging Implement missing transaction handling. Fix logging wrongly casting 'state' to DCERPCState instead of DCERPCUDPState leading to crashes and malformed output. Remove unused fields from DCERPCUDPState. --- rust/src/dcerpc/dcerpc_udp.rs | 75 +++++++++++++++++------------------ rust/src/dcerpc/log.rs | 64 +++++++++++++++++++++++++----- src/app-layer-dcerpc-udp.c | 4 +- src/output-json-dcerpc.c | 10 ++++- 4 files changed, 100 insertions(+), 53 deletions(-) diff --git a/rust/src/dcerpc/dcerpc_udp.rs b/rust/src/dcerpc/dcerpc_udp.rs index 9488a8b749..959b02483e 100644 --- a/rust/src/dcerpc/dcerpc_udp.rs +++ b/rust/src/dcerpc/dcerpc_udp.rs @@ -20,7 +20,7 @@ use std::mem::transmute; use crate::applayer::{AppLayerResult, AppLayerTxData}; use crate::core; use crate::dcerpc::dcerpc::{ - DCERPCTransaction, DCERPCUuidEntry, DCERPC_TYPE_REQUEST, DCERPC_TYPE_RESPONSE, PFCL1_FRAG, PFCL1_LASTFRAG, + DCERPCTransaction, DCERPC_TYPE_REQUEST, DCERPC_TYPE_RESPONSE, PFCL1_FRAG, PFCL1_LASTFRAG, }; use crate::dcerpc::parser; @@ -53,26 +53,14 @@ pub struct DCERPCHdrUdp { #[derive(Debug)] pub struct DCERPCUDPState { pub tx_id: u64, - pub header: Option, pub transactions: Vec, - pub fraglenleft: u16, - pub uuid_entry: Option, - pub uuid_list: Vec, - pub de_state: Option<*mut core::DetectEngineState>, - pub tx_data: AppLayerTxData, } impl DCERPCUDPState { pub fn new() -> DCERPCUDPState { return DCERPCUDPState { tx_id: 0, - header: None, transactions: Vec::new(), - fraglenleft: 0, - uuid_entry: None, - uuid_list: Vec::new(), - de_state: None, - tx_data: AppLayerTxData::new(), }; } @@ -107,6 +95,26 @@ impl DCERPCUDPState { } } + /// Get transaction as per the given transaction ID. Transaction ID with + /// which the lookup is supposed to be done as per the calls from AppLayer + /// parser in C. This requires an internal transaction ID to be maintained. + /// + /// Arguments: + /// * `tx_id`: + /// description: internal transaction ID to track transactions + /// + /// Return value: + /// Option mutable reference to DCERPCTransaction + pub fn get_tx(&mut self, tx_id: u64) -> Option<&mut DCERPCTransaction> { + for tx in &mut self.transactions { + let found = tx.id == tx_id; + if found { + return Some(tx); + } + } + None + } + fn find_incomplete_tx(&mut self, hdr: &DCERPCHdrUdp) -> Option<&mut DCERPCTransaction> { for tx in &mut self.transactions { if tx.seqnum == hdr.seqnum && tx.activityuuid == hdr.activityuuid { @@ -182,10 +190,6 @@ impl DCERPCUDPState { if !self.handle_fragment_data(&header, &leftover_bytes[..header.fraglen as usize]) { return AppLayerResult::err(); } - let mut uuidentry = DCERPCUuidEntry::new(); - let auuid = header.activityuuid.to_vec(); - uuidentry.uuid = auuid; - self.uuid_list.push(uuidentry); } Err(nom::Err::Incomplete(_)) => { // Insufficient data. @@ -239,7 +243,7 @@ pub extern "C" fn rs_dcerpc_udp_state_transaction_free( pub extern "C" fn rs_dcerpc_udp_get_tx_detect_state( vtx: *mut std::os::raw::c_void, ) -> *mut core::DetectEngineState { - let dce_state = cast_pointer!(vtx, DCERPCUDPState); + let dce_state = cast_pointer!(vtx, DCERPCTransaction); match dce_state.de_state { Some(ds) => ds, None => std::ptr::null_mut(), @@ -250,7 +254,7 @@ pub extern "C" fn rs_dcerpc_udp_get_tx_detect_state( pub extern "C" fn rs_dcerpc_udp_set_tx_detect_state( vtx: *mut std::os::raw::c_void, de_state: *mut core::DetectEngineState, ) -> u8 { - let dce_state = cast_pointer!(vtx, DCERPCUDPState); + let dce_state = cast_pointer!(vtx, DCERPCTransaction); dce_state.de_state = Some(de_state); 0 } @@ -260,33 +264,29 @@ pub extern "C" fn rs_dcerpc_udp_get_tx_data( tx: *mut std::os::raw::c_void) -> *mut AppLayerTxData { - let tx = cast_pointer!(tx, DCERPCUDPState); + let tx = cast_pointer!(tx, DCERPCTransaction); return &mut tx.tx_data; } #[no_mangle] pub extern "C" fn rs_dcerpc_udp_get_tx( - state: *mut std::os::raw::c_void, _tx_id: u64, -) -> *mut DCERPCUDPState { + state: *mut std::os::raw::c_void, tx_id: u64, +) -> *mut DCERPCTransaction { let dce_state = cast_pointer!(state, DCERPCUDPState); - dce_state -} - -#[no_mangle] -pub extern "C" fn rs_dcerpc_udp_get_tx_cnt(_state: *mut std::os::raw::c_void) -> u8 { - 1 -} - -#[no_mangle] -pub extern "C" fn rs_dcerpc_udp_get_alstate_progress( - _tx: *mut std::os::raw::c_void, _direction: u8, -) -> u8 { - 0 + match dce_state.get_tx(tx_id) { + Some(tx) => { + return unsafe{transmute(tx)}; + }, + None => { + return std::ptr::null_mut(); + } + } } #[no_mangle] -pub extern "C" fn rs_dcerpc_udp_get_alstate_progress_completion_status(_direction: u8) -> u8 { - 1 +pub extern "C" fn rs_dcerpc_udp_get_tx_cnt(vtx: *mut std::os::raw::c_void) -> u64 { + let dce_state = cast_pointer!(vtx, DCERPCUDPState); + dce_state.tx_id } #[cfg(test)] @@ -465,7 +465,6 @@ mod tests { AppLayerResult::ok(), dcerpcudp_state.handle_input_data(request) ); - assert_eq!(0, dcerpcudp_state.fraglenleft); assert_eq!( 1392, dcerpcudp_state.transactions[0].stub_data_buffer_ts.len() diff --git a/rust/src/dcerpc/log.rs b/rust/src/dcerpc/log.rs index 9f69fe5a4b..317fab2559 100644 --- a/rust/src/dcerpc/log.rs +++ b/rust/src/dcerpc/log.rs @@ -17,9 +17,10 @@ use uuid::Uuid; use crate::dcerpc::dcerpc::*; +use crate::dcerpc::dcerpc_udp::*; use crate::jsonbuilder::{JsonBuilder, JsonError}; -fn log_dcerpc_header( +fn log_dcerpc_header_tcp( jsb: &mut JsonBuilder, state: &DCERPCState, tx: &DCERPCTransaction, ) -> Result<(), JsonError> { if tx.req_done == true && tx.req_lost == false { @@ -71,14 +72,7 @@ fn log_dcerpc_header( } if let Some(ref hdr) = state.header { - if hdr.rpc_vers != 4 { - jsb.set_uint("call_id", tx.call_id as u64)?; - } else { - let activityuuid = Uuid::from_slice(tx.activityuuid.as_slice()); - let activityuuid = activityuuid.map(|uuid| uuid.to_hyphenated().to_string()).unwrap(); - jsb.set_string("activityuuid", &activityuuid)?; - jsb.set_uint("seqnum", tx.seqnum as u64)?; - } + jsb.set_uint("call_id", tx.call_id as u64)?; let vstr = format!("{}.{}", hdr.rpc_vers, hdr.rpc_vers_minor); jsb.set_string("rpc_version", &vstr)?; } @@ -86,9 +80,57 @@ fn log_dcerpc_header( return Ok(()); } +fn log_dcerpc_header_udp( + jsb: &mut JsonBuilder, _state: &DCERPCUDPState, tx: &DCERPCTransaction, +) -> Result<(), JsonError> { + if tx.req_done == true && tx.req_lost == false { + jsb.set_string("request", &dcerpc_type_string(tx.req_cmd))?; + match tx.req_cmd { + DCERPC_TYPE_REQUEST => { + jsb.open_object("req")?; + jsb.set_uint("opnum", tx.opnum as u64)?; + jsb.set_uint("frag_cnt", tx.frag_cnt_ts as u64)?; + jsb.set_uint("stub_data_size", tx.stub_data_buffer_ts.len() as u64)?; + jsb.close()?; + } + _ => {} + } + } else { + jsb.set_string("request", "REQUEST_LOST")?; + } + + if tx.resp_done == true && tx.resp_lost == false { + jsb.set_string("response", &dcerpc_type_string(tx.resp_cmd))?; + match tx.resp_cmd { + DCERPC_TYPE_RESPONSE => { + jsb.open_object("res")?; + jsb.set_uint("frag_cnt", tx.frag_cnt_tc as u64)?; + jsb.set_uint("stub_data_size", tx.stub_data_buffer_tc.len() as u64)?; + jsb.close()?; + } + _ => {} // replicating behavior from smb + } + } else { + jsb.set_string("response", "UNREPLIED")?; + } + let activityuuid = Uuid::from_slice(tx.activityuuid.as_slice()); + let activityuuid = activityuuid.map(|uuid| uuid.to_hyphenated().to_string()).unwrap(); + jsb.set_string("activityuuid", &activityuuid)?; + jsb.set_uint("seqnum", tx.seqnum as u64)?; + jsb.set_string("rpc_version", "4.0")?; + return Ok(()); +} + #[no_mangle] -pub extern "C" fn rs_dcerpc_log_json_record( +pub extern "C" fn rs_dcerpc_log_json_record_tcp( state: &DCERPCState, tx: &DCERPCTransaction, mut jsb: &mut JsonBuilder, ) -> bool { - log_dcerpc_header(&mut jsb, state, tx).is_ok() + log_dcerpc_header_tcp(&mut jsb, state, tx).is_ok() +} + +#[no_mangle] +pub extern "C" fn rs_dcerpc_log_json_record_udp( + state: &DCERPCUDPState, tx: &DCERPCTransaction, mut jsb: &mut JsonBuilder, +) -> bool { + log_dcerpc_header_udp(&mut jsb, state, tx).is_ok() } diff --git a/src/app-layer-dcerpc-udp.c b/src/app-layer-dcerpc-udp.c index 6c2e96b53d..d38378ae07 100644 --- a/src/app-layer-dcerpc-udp.c +++ b/src/app-layer-dcerpc-udp.c @@ -94,12 +94,12 @@ static uint64_t RustDCERPCUDPGetTxCnt(void *state) static int RustDCERPCUDPGetAlstateProgressCompletionStatus(uint8_t direction) { - return rs_dcerpc_udp_get_alstate_progress_completion_status(direction); + return rs_dcerpc_get_alstate_progress_completion_status(direction); } static int RustDCERPCUDPGetAlstateProgress(void *tx, uint8_t direction) { - return rs_dcerpc_udp_get_alstate_progress(tx, direction); + return rs_dcerpc_get_alstate_progress(tx, direction); } static int DCERPCUDPRegisterPatternsForProtocolDetection(void) diff --git a/src/output-json-dcerpc.c b/src/output-json-dcerpc.c index 76737bd78f..0bd56a548f 100644 --- a/src/output-json-dcerpc.c +++ b/src/output-json-dcerpc.c @@ -52,8 +52,14 @@ static int JsonDCERPCLogger(ThreadVars *tv, void *thread_data, } jb_open_object(jb, "dcerpc"); - if (!rs_dcerpc_log_json_record(state, tx, jb)) { - goto error; + if (p->proto == IPPROTO_TCP) { + if (!rs_dcerpc_log_json_record_tcp(state, tx, jb)) { + goto error; + } + } else { + if (!rs_dcerpc_log_json_record_udp(state, tx, jb)) { + goto error; + } } jb_close(jb); -- 2.47.2