From 3ff7418f551eb5c9e759157ab1cb8cea2bbc5643 Mon Sep 17 00:00:00 2001 From: Ilya Bakhtin Date: Sun, 21 Jul 2024 19:15:00 +0200 Subject: [PATCH 1/2] protodetect/dcerpc: improve DCERPC UDP probing parser Several additional checks are added to the probing parser to avoid false detection of DNS as DCERPC Ticket - 7111 --- rust/src/dcerpc/dcerpc_udp.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust/src/dcerpc/dcerpc_udp.rs b/rust/src/dcerpc/dcerpc_udp.rs index ab7f65cafbad..a4eb1adfbe3b 100644 --- a/rust/src/dcerpc/dcerpc_udp.rs +++ b/rust/src/dcerpc/dcerpc_udp.rs @@ -294,9 +294,11 @@ pub unsafe extern "C" fn rs_dcerpc_udp_get_tx_cnt(vtx: *mut std::os::raw::c_void /// Probe input to see if it looks like DCERPC. fn probe(input: &[u8]) -> (bool, bool) { match parser::parse_dcerpc_udp_header(input) { - Ok((_, hdr)) => { + Ok((leftover_bytes, hdr)) => { let is_request = hdr.pkt_type == 0x00; let is_dcerpc = hdr.rpc_vers == 0x04 && + hdr.fragnum == 0 && + leftover_bytes.len() >= hdr.fraglen as usize && (hdr.flags2 & 0xfc == 0) && (hdr.drep[0] & 0xee == 0) && (hdr.drep[1] <= 3); From fad7b6d8cd4dbbcfe890d7285012a430433fda06 Mon Sep 17 00:00:00 2001 From: Ilya Bakhtin Date: Sun, 21 Jul 2024 20:01:51 +0200 Subject: [PATCH 2/2] protodetect: simplify app-layer-detect-proto.c code since DCERPC UDP detection is improved Protocol detection code is simplified. Removed dependency on explicit alproto constants from the common part of code that must not be aware of the each specific protocol features. Ticket - 7111 --- src/app-layer-detect-proto.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/app-layer-detect-proto.c b/src/app-layer-detect-proto.c index a65a98c88a41..2cd0fc10bd0d 100644 --- a/src/app-layer-detect-proto.c +++ b/src/app-layer-detect-proto.c @@ -1401,7 +1401,6 @@ AppProto AppLayerProtoDetectGetProto(AppLayerProtoDetectThreadCtx *tctx, Flow *f (flags & STREAM_TOSERVER) ? "toserver" : "toclient"); AppProto alproto = ALPROTO_UNKNOWN; - AppProto pm_alproto = ALPROTO_UNKNOWN; if (!FLOW_IS_PM_DONE(f, flags)) { AppProto pm_results[ALPROTO_MAX]; @@ -1419,38 +1418,24 @@ AppProto AppLayerProtoDetectGetProto(AppLayerProtoDetectThreadCtx *tctx, Flow *f FLOW_RESET_PP_DONE(f, reverse_dir); } } - - /* HACK: if detected protocol is dcerpc/udp, we run PP as well - * to avoid misdetecting DNS as DCERPC. */ - if (!(ipproto == IPPROTO_UDP && alproto == ALPROTO_DCERPC)) - goto end; - - pm_alproto = alproto; - - /* fall through */ + SCReturnUInt(alproto); } } if (!FLOW_IS_PP_DONE(f, flags)) { - bool rflow = false; - alproto = AppLayerProtoDetectPPGetProto(f, buf, buflen, ipproto, flags, &rflow); + DEBUG_VALIDATE_BUG_ON(*reverse_flow); + alproto = AppLayerProtoDetectPPGetProto(f, buf, buflen, ipproto, flags, reverse_flow); if (AppProtoIsValid(alproto)) { - if (rflow) { - *reverse_flow = true; - } - goto end; + SCReturnUInt(alproto); } } /* Look if flow can be found in expectation list */ if (!FLOW_IS_PE_DONE(f, flags)) { + DEBUG_VALIDATE_BUG_ON(*reverse_flow); alproto = AppLayerProtoDetectPEGetProto(f, flags); } - end: - if (!AppProtoIsValid(alproto)) - alproto = pm_alproto; - SCReturnUInt(alproto); }