From 8fc147dae7aff4d95ef4eab1bd99be35d46de5f9 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 1eae0b0a61dac4424297949d87eda79bb6da3d39 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 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); }