From: Ilya Bakhtin Date: Tue, 10 Nov 2020 10:05:18 +0000 (+0530) Subject: dcerpc/udp: Fix pairing of request response X-Git-Tag: suricata-6.0.1~54 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6916b63f09e335baec3dd05d1def4c439a13a7b4;p=thirdparty%2Fsuricata.git dcerpc/udp: Fix pairing of request response So far, request and response were paired with serial number fields in the header. This is incorrect. According to https://pubs.opengroup.org/onlinepubs/9629399/chap12.htm, "Together, the activity UUID and the sequence number uniquely identify a remote procedure call." Hence, add activity uuid and sequence number to the transaction and pair the request accordingly. Remove incorrect handling of this and fix tests. --- diff --git a/rust/src/dcerpc/dcerpc.rs b/rust/src/dcerpc/dcerpc.rs index 308ec58e42..0ec31e792e 100644 --- a/rust/src/dcerpc/dcerpc.rs +++ b/rust/src/dcerpc/dcerpc.rs @@ -179,6 +179,8 @@ pub struct DCERPCTransaction { pub resp_lost: bool, pub req_cmd: u8, pub resp_cmd: u8, + pub activityuuid: Vec, + pub seqnum: u32, pub tx_data: AppLayerTxData, pub de_state: Option<*mut core::DetectEngineState>, } @@ -206,6 +208,8 @@ impl DCERPCTransaction { resp_lost: false, req_cmd: DCERPC_TYPE_REQUEST, resp_cmd: DCERPC_TYPE_RESPONSE, + activityuuid: Vec::new(), + seqnum: 0, tx_data: AppLayerTxData::new(), de_state: None, }; diff --git a/rust/src/dcerpc/dcerpc_udp.rs b/rust/src/dcerpc/dcerpc_udp.rs index 68e9f33a35..4fd37567f7 100644 --- a/rust/src/dcerpc/dcerpc_udp.rs +++ b/rust/src/dcerpc/dcerpc_udp.rs @@ -20,10 +20,9 @@ use std::mem::transmute; use crate::applayer::{AppLayerResult, AppLayerTxData}; use crate::core; use crate::dcerpc::dcerpc::{ - DCERPCTransaction, DCERPCUuidEntry, DCERPC_TYPE_REQUEST, DCERPC_TYPE_RESPONSE, PFC_FIRST_FRAG, + DCERPCTransaction, DCERPCUuidEntry, DCERPC_TYPE_REQUEST, DCERPC_TYPE_RESPONSE, PFCL1_FRAG, PFCL1_LASTFRAG, }; use crate::dcerpc::parser; -use std::cmp; // Constant DCERPC UDP Header length pub const DCERPC_UDP_HDR_LEN: i32 = 80; @@ -77,12 +76,12 @@ impl DCERPCUDPState { }; } - fn create_tx(&mut self, serial_no: u16) -> DCERPCTransaction { + fn create_tx(&mut self, hdr: &DCERPCHdrUdp) -> DCERPCTransaction { let mut tx = DCERPCTransaction::new(); - let endianness = self.get_hdr_drep_0() & 0x10; tx.id = self.tx_id; - tx.call_id = serial_no as u32; - tx.endianness = endianness; + tx.endianness = hdr.drep[0] & 0x10; + tx.activityuuid = hdr.activityuuid.to_vec(); + tx.seqnum = hdr.seqnum; self.tx_id += 1; tx } @@ -108,208 +107,101 @@ impl DCERPCUDPState { } } - fn evaluate_serial_no(&mut self) -> u16 { - let mut serial_no: u16; - let mut serial_hi: u8 = 0; - let mut serial_lo: u8 = 0; - let endianness = self.get_hdr_drep_0(); - if let Some(ref hdr) = &self.header { - serial_hi = hdr.serial_hi; - serial_lo = hdr.serial_lo; - } - if endianness & 0x10 == 0 { - serial_no = serial_lo as u16; - serial_no = serial_no.rotate_left(8) | serial_hi as u16; - } else { - serial_no = serial_hi as u16; - serial_no = serial_no.rotate_left(8) | serial_lo as u16; - } - serial_no - } - - fn find_tx(&mut self, serial_no: u16) -> Option<&mut DCERPCTransaction> { + fn find_incomplete_tx(&mut self, hdr: &DCERPCHdrUdp) -> Option<&mut DCERPCTransaction> { for tx in &mut self.transactions { - let found = tx.call_id == (serial_no as u32); - if found { - return Some(tx); + if tx.seqnum == hdr.seqnum && tx.activityuuid == hdr.activityuuid { + if (hdr.pkt_type == DCERPC_TYPE_REQUEST && !tx.req_done) || + (hdr.pkt_type == DCERPC_TYPE_RESPONSE && !tx.resp_done) { + SCLogDebug!("found tx id {}, last tx_id {}, {} {}", tx.id, self.tx_id, tx.seqnum, tx.activityuuid[0]); + return Some(tx); + } } } None } - fn get_hdr_pkt_type(&self) -> Option { - debug_validate_bug_on!(self.header.is_none()); - if let Some(ref hdr) = &self.header { - return Some(hdr.pkt_type); + pub fn handle_fragment_data(&mut self, hdr: &DCERPCHdrUdp, input: &[u8]) -> bool { + if hdr.pkt_type != DCERPC_TYPE_REQUEST && hdr.pkt_type != DCERPC_TYPE_RESPONSE { + SCLogDebug!("Unrecognized packet type"); + return false; } - // Shouldn't happen - None - } - fn get_hdr_flags1(&self) -> Option { - debug_validate_bug_on!(self.header.is_none()); - if let Some(ref hdr) = &self.header { - return Some(hdr.flags1); + let mut otx = self.find_incomplete_tx(hdr); + if otx.is_none() { + let ntx = self.create_tx(hdr); + SCLogDebug!("new tx id {}, last tx_id {}, {} {}", ntx.id, self.tx_id, ntx.seqnum, ntx.activityuuid[0]); + self.transactions.push(ntx); + otx = self.transactions.last_mut(); } - // Shouldn't happen - None - } - fn get_hdr_drep_0(&self) -> u8 { - debug_validate_bug_on!(self.header.is_none()); - if let Some(ref hdr) = &self.header { - return hdr.drep[0]; + if let Some(tx) = otx { + let done = (hdr.flags1 & PFCL1_FRAG) == 0 || (hdr.flags1 & PFCL1_LASTFRAG) != 0; + + match hdr.pkt_type { + DCERPC_TYPE_REQUEST => { + tx.stub_data_buffer_ts.extend_from_slice(&input); + tx.frag_cnt_ts += 1; + if done { + tx.req_done = true; + } + return true; + } + DCERPC_TYPE_RESPONSE => { + tx.stub_data_buffer_tc.extend_from_slice(&input); + tx.frag_cnt_tc += 1; + if done { + tx.resp_done = true; + } + return true; + } + _ => { + // unreachable + } + } } - 0 + return false; // unreachable } - pub fn get_hdr_fraglen(&self) -> Option { - debug_validate_bug_on!(self.header.is_none()); - if let Some(ref hdr) = &self.header { - return Some(hdr.fraglen); - } - // Shouldn't happen - None - } - - pub fn handle_fragment_data(&mut self, input: &[u8], input_len: u16) -> u16 { - let retval: u16; - let hdrflags1 = self.get_hdr_flags1().unwrap_or(0); - let fraglenleft = self.fraglenleft; - let hdrtype = self.get_hdr_pkt_type().unwrap_or(0); - let serial_no = self.evaluate_serial_no(); - let tx; - if let Some(transaction) = self.find_tx(serial_no) { - tx = transaction; - } else { - SCLogDebug!( - "No transaction found matching the serial number: {:?}", - serial_no - ); - return 0; - } - - // Update the stub params based on the packet type - match hdrtype { - DCERPC_TYPE_REQUEST => { - retval = evaluate_stub_params( - input, - input_len, - hdrflags1, - fraglenleft, - &mut tx.stub_data_buffer_ts, - &mut tx.stub_data_buffer_len_ts, - ); - tx.req_done = true; - tx.frag_cnt_ts += 1; - } - DCERPC_TYPE_RESPONSE => { - retval = evaluate_stub_params( - input, - input_len, - hdrflags1, - fraglenleft, - &mut tx.stub_data_buffer_tc, - &mut tx.stub_data_buffer_len_tc, - ); - tx.resp_done = true; - tx.frag_cnt_tc += 1; - } - _ => { - SCLogDebug!("Unrecognized packet type"); - return 0; - } + pub fn handle_input_data(&mut self, input: &[u8]) -> AppLayerResult { + // Input length should at least be header length + if (input.len() as i32) < DCERPC_UDP_HDR_LEN { + return AppLayerResult::err(); } - // Update the remaining fragment length - self.fraglenleft -= retval; - - retval - } - pub fn process_header(&mut self, input: &[u8]) -> i32 { + // Call header parser first match parser::parse_dcerpc_udp_header(input) { Ok((leftover_bytes, header)) => { if header.rpc_vers != 4 { SCLogDebug!("DCERPC UDP Header did not validate."); - return -1; + return AppLayerResult::err(); + } + if leftover_bytes.len() < header.fraglen as usize { + SCLogDebug!("Insufficient data: leftover_bytes {}, fraglen {}", leftover_bytes.len(), header.fraglen); + return AppLayerResult::err(); + } + 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); - self.header = Some(header); - (input.len() - leftover_bytes.len()) as i32 } Err(nom::Err::Incomplete(_)) => { // Insufficient data. SCLogDebug!("Insufficient data while parsing DCERPC request"); - -1 + return AppLayerResult::err(); } Err(_) => { // Error, probably malformed data. SCLogDebug!("An error occurred while parsing DCERPC request"); - -1 - } - } - } - - pub fn handle_input_data(&mut self, input: &[u8]) -> AppLayerResult { - // Input length should at least be header length - if (input.len() as i32) < DCERPC_UDP_HDR_LEN { - return AppLayerResult::err(); - } - // Call header parser first - let mut parsed = self.process_header(input); - if parsed == -1 { - return AppLayerResult::err(); - } - - let mut input_left = input.len() as i32 - parsed; - let fraglen = self.get_hdr_fraglen().unwrap_or(0); - self.fraglenleft = fraglen; - let serial_no = self.evaluate_serial_no(); - let tx = self.create_tx(serial_no); - self.transactions.push(tx); - // Parse rest of the body - while parsed >= DCERPC_UDP_HDR_LEN && parsed < fraglen as i32 && input_left > 0 { - let retval = self.handle_fragment_data(&input[parsed as usize..], input_left as u16); - if retval > 0 && retval <= input_left as u16 { - parsed += retval as i32; - input_left -= retval as i32; - } else if input_left > 0 { - SCLogDebug!("Error parsing DCERPC UDP Fragment Data"); - parsed -= input_left; - input_left = 0; + return AppLayerResult::err(); } } return AppLayerResult::ok(); } } -fn evaluate_stub_params( - input: &[u8], input_len: u16, hdrflags: u8, lenleft: u16, stub_data_buffer: &mut Vec, - stub_data_buffer_len: &mut u32, -) -> u16 { - let stub_len: u16; - stub_len = cmp::min(lenleft, input_len); - if stub_len == 0 { - return 0; - } - // If the UDP frag is the the first frag irrespective of it being a part of - // a multi frag PDU or not, it indicates the previous PDU's stub would - // have been buffered and processed and we can use the buffer to hold - // frags from a fresh request/response - if hdrflags & PFC_FIRST_FRAG > 0 { - *stub_data_buffer_len = 0; - } - - let input_slice = &input[..stub_len as usize]; - stub_data_buffer.extend_from_slice(&input_slice); - *stub_data_buffer_len += stub_len as u32; - - stub_len -} - #[no_mangle] pub extern "C" fn rs_dcerpc_udp_parse( _flow: *mut core::Flow, state: &mut DCERPCUDPState, _pstate: *mut std::os::raw::c_void, @@ -401,6 +293,7 @@ pub extern "C" fn rs_dcerpc_udp_get_alstate_progress_completion_status(_directio mod tests { use crate::applayer::AppLayerResult; use crate::dcerpc::dcerpc_udp::DCERPCUDPState; + use crate::dcerpc::parser; #[test] fn test_process_header_udp_incomplete_hdr() { @@ -410,8 +303,12 @@ mod tests { 0x1c, 0x7d, 0xcf, 0x11, ]; - let mut dcerpcudp_state = DCERPCUDPState::new(); - assert_eq!(-1, dcerpcudp_state.process_header(request)); + match parser::parse_dcerpc_udp_header(request) { + Ok((_rem, _header)) => { + { assert!(false); } + } + _ => {} + } } #[test] @@ -424,8 +321,13 @@ mod tests { 0x79, 0xbe, 0x01, 0x34, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x68, 0x00, 0x00, 0x00, 0x0a, 0x00, ]; - let mut dcerpcudp_state = DCERPCUDPState::new(); - assert_eq!(80, dcerpcudp_state.process_header(request)); + match parser::parse_dcerpc_udp_header(request) { + Ok((rem, header)) => { + assert_eq!(4, header.rpc_vers); + assert_eq!(80, request.len() - rem.len()); + } + _ => { assert!(false); } + } } #[test] @@ -438,12 +340,14 @@ mod tests { 0x79, 0xbe, 0x01, 0x34, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x68, 0x00, 0x00, 0x00, 0x0a, 0x00, ]; - let mut dcerpcudp_state = DCERPCUDPState::new(); - assert_eq!(80, dcerpcudp_state.process_header(request)); - assert_eq!( - 0, - dcerpcudp_state.handle_fragment_data(request, request.len() as u16) - ); + match parser::parse_dcerpc_udp_header(request) { + Ok((rem, header)) => { + assert_eq!(4, header.rpc_vers); + assert_eq!(80, request.len() - rem.len()); + assert_eq!(0, rem.len()); + } + _ => { assert!(false); } + } } #[test] @@ -564,7 +468,7 @@ mod tests { assert_eq!(0, dcerpcudp_state.fraglenleft); assert_eq!( 1392, - dcerpcudp_state.transactions[0].stub_data_buffer_len_ts + dcerpcudp_state.transactions[0].stub_data_buffer_ts.len() ); } }