From 0ccad8fd888fdd3bd6a7adeeab9ec0768d82d19c Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 12 Dec 2023 22:47:01 +0100 Subject: [PATCH] doh: make dns and http keywords for doh2 Ticket: 5773 --- rust/src/http2/http2.rs | 16 +++++++++++ src/app-layer-detect-proto.c | 9 ++++++ src/app-layer-protos.h | 52 +++++++++++++++++++++++++++++++++++ src/detect-engine-mpm.c | 5 ++++ src/detect-engine-prefilter.c | 10 +++++-- src/detect-engine.c | 5 ++++ src/detect-parse.c | 17 ++---------- src/detect.c | 36 +++++++++++++++++++----- src/detect.h | 1 + 9 files changed, 127 insertions(+), 24 deletions(-) diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs index 063f80a15d..6620c040de 100644 --- a/rust/src/http2/http2.rs +++ b/rust/src/http2/http2.rs @@ -1277,6 +1277,22 @@ impl HTTP2State { // C exports. +#[no_mangle] +pub unsafe extern "C" fn SCDoH2GetDnsTx( + tx: &HTTP2Transaction, flags: u8, +) -> *mut std::os::raw::c_void { + if flags & Direction::ToServer as u8 != 0 { + if let Some(ref dtx) = &tx.dns_request_tx { + return dtx as *const _ as *mut _; + } + } else if flags & Direction::ToClient as u8 != 0 { + if let Some(ref dtx) = &tx.dns_response_tx { + return dtx as *const _ as *mut _; + } + } + std::ptr::null_mut() +} + export_tx_data_get!(rs_http2_get_tx_data, HTTP2Transaction); export_state_data_get!(rs_http2_get_state_data, HTTP2State); diff --git a/src/app-layer-detect-proto.c b/src/app-layer-detect-proto.c index 298392313e..a65a98c88a 100644 --- a/src/app-layer-detect-proto.c +++ b/src/app-layer-detect-proto.c @@ -1844,6 +1844,12 @@ bool AppLayerRequestProtocolTLSUpgrade(Flow *f) return AppLayerRequestProtocolChange(f, 443, ALPROTO_TLS); } +/** \brief Forces a flow app-layer protocol change. + * Happens for instance when a HTTP2 flow is seen as DOH2 + * + * \param f flow to act on + * \param new_proto new app-layer protocol + */ void AppLayerForceProtocolChange(Flow *f, AppProto new_proto) { if (new_proto != f->alproto) { @@ -2035,6 +2041,9 @@ void AppLayerProtoDetectSupportedIpprotos(AppProto alproto, uint8_t *ipprotos) if (alproto == ALPROTO_HTTP) { AppLayerProtoDetectSupportedIpprotos(ALPROTO_HTTP1, ipprotos); AppLayerProtoDetectSupportedIpprotos(ALPROTO_HTTP2, ipprotos); + } else if (alproto == ALPROTO_DOH2) { + // DOH2 is not detected, just HTTP2 + AppLayerProtoDetectSupportedIpprotos(ALPROTO_HTTP2, ipprotos); } else { AppLayerProtoDetectPMGetIpprotos(alproto, ipprotos); AppLayerProtoDetectPPGetIpprotos(alproto, ipprotos); diff --git a/src/app-layer-protos.h b/src/app-layer-protos.h index 85c69b225e..348203ac2f 100644 --- a/src/app-layer-protos.h +++ b/src/app-layer-protos.h @@ -95,6 +95,16 @@ static inline bool AppProtoEquals(AppProto sigproto, AppProto alproto) return true; } switch (sigproto) { + case ALPROTO_DNS: + // a DNS signature matches on either DNS or DOH2 flows + return (alproto == ALPROTO_DOH2) || (alproto == ALPROTO_DNS); + case ALPROTO_HTTP2: + // a HTTP2 signature matches on either HTTP2 or DOH2 flows + return (alproto == ALPROTO_DOH2) || (alproto == ALPROTO_HTTP2); + case ALPROTO_DOH2: + // a DOH2 signature accepts dns, http2 or http generic keywords + return (alproto == ALPROTO_DOH2) || (alproto == ALPROTO_HTTP2) || + (alproto == ALPROTO_DNS) || (alproto == ALPROTO_HTTP); case ALPROTO_HTTP: return (alproto == ALPROTO_HTTP1) || (alproto == ALPROTO_HTTP2); case ALPROTO_DCERPC: @@ -103,6 +113,48 @@ static inline bool AppProtoEquals(AppProto sigproto, AppProto alproto) return false; } +// whether a signature AppProto matches a flow (or signature) AppProto +static inline AppProto AppProtoCommon(AppProto sigproto, AppProto alproto) +{ + switch (sigproto) { + case ALPROTO_SMB: + if (alproto == ALPROTO_DCERPC) { + // ok to have dcerpc keywords in smb sig + return ALPROTO_SMB; + } + break; + case ALPROTO_HTTP: + // we had a generic http sig, now version specific + if (alproto == ALPROTO_HTTP1) { + return ALPROTO_HTTP1; + } else if (alproto == ALPROTO_HTTP2) { + return ALPROTO_HTTP2; + } + break; + case ALPROTO_HTTP1: + // version-specific sig with a generic keyword + if (alproto == ALPROTO_HTTP) { + return ALPROTO_HTTP1; + } + break; + case ALPROTO_HTTP2: + if (alproto == ALPROTO_HTTP) { + return ALPROTO_HTTP2; + } + break; + case ALPROTO_DOH2: + // DOH2 accepts different protocol keywords + if (alproto == ALPROTO_HTTP || alproto == ALPROTO_HTTP2 || alproto == ALPROTO_DNS) { + return ALPROTO_DOH2; + } + break; + } + if (sigproto != alproto) { + return ALPROTO_FAILED; + } + return alproto; +} + /** * \brief Maps the ALPROTO_*, to its string equivalent. * diff --git a/src/detect-engine-mpm.c b/src/detect-engine-mpm.c index eca9807df1..5e8687e346 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -107,6 +107,11 @@ static void RegisterInternal(const char *name, int direction, int priority, FatalError("MPM engine registration for %s failed", name); } + // every HTTP2 can be accessed from DOH2 + if (alproto == ALPROTO_HTTP2 || alproto == ALPROTO_DNS) { + RegisterInternal(name, direction, priority, PrefilterRegister, GetData, GetMultiData, + ALPROTO_DOH2, tx_min_progress); + } DetectBufferMpmRegistry *am = SCCalloc(1, sizeof(*am)); BUG_ON(am == NULL); am->name = name; diff --git a/src/detect-engine-prefilter.c b/src/detect-engine-prefilter.c index 83ccb2afb2..feff1251e4 100644 --- a/src/detect-engine-prefilter.c +++ b/src/detect-engine-prefilter.c @@ -107,8 +107,12 @@ void DetectRunPrefilterTx(DetectEngineThreadCtx *det_ctx, PrefilterEngine *engine = sgh->tx_engines; do { - if (engine->alproto != alproto) + // based on flow alproto, and engine, we get right tx_ptr + void *tx_ptr = DetectGetInnerTx(tx->tx_ptr, alproto, engine->alproto, flow_flags); + if (tx_ptr == NULL) { + // incompatible engine->alproto with flow alproto goto next; + } if (engine->ctx.tx_min_progress > tx->tx_progress) break; if (tx->tx_progress > engine->ctx.tx_min_progress) { @@ -118,8 +122,8 @@ void DetectRunPrefilterTx(DetectEngineThreadCtx *det_ctx, } PREFILTER_PROFILING_START(det_ctx); - engine->cb.PrefilterTx(det_ctx, engine->pectx, p, p->flow, tx->tx_ptr, tx->tx_id, - tx->tx_data_ptr, flow_flags); + engine->cb.PrefilterTx( + det_ctx, engine->pectx, p, p->flow, tx_ptr, tx->tx_id, tx->tx_data_ptr, flow_flags); PREFILTER_PROFILING_END(det_ctx, engine->gid); if (tx->tx_progress > engine->ctx.tx_min_progress && engine->is_last_for_progress) { diff --git a/src/detect-engine.c b/src/detect-engine.c index 181b0023c8..c4e79682f1 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -200,6 +200,11 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp } else { direction = 1; } + // every DNS or HTTP2 can be accessed from DOH2 + if (alproto == ALPROTO_HTTP2 || alproto == ALPROTO_DNS) { + AppLayerInspectEngineRegisterInternal( + name, ALPROTO_DOH2, dir, progress, Callback, GetData, GetMultiData); + } DetectEngineAppInspectionEngine *new_engine = SCCalloc(1, sizeof(DetectEngineAppInspectionEngine)); diff --git a/src/detect-parse.c b/src/detect-parse.c index 61318521f4..984501c1dd 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1742,20 +1742,9 @@ int DetectSignatureSetAppProto(Signature *s, AppProto alproto) return -1; } - /* since AppProtoEquals is quite permissive wrt dcerpc and smb, make sure - * we refuse `alert dcerpc ... smb.share; content...` explicitly. */ - if (alproto == ALPROTO_SMB && s->alproto == ALPROTO_DCERPC) { - SCLogError("can't set rule app proto to %s: already set to %s", AppProtoToString(alproto), - AppProtoToString(s->alproto)); - return -1; - } - - if (s->alproto != ALPROTO_UNKNOWN && !AppProtoEquals(s->alproto, alproto)) { - if (AppProtoEquals(alproto, s->alproto)) { - // happens if alproto = HTTP_ANY and s->alproto = HTTP1 - // in this case, we must keep the most restrictive HTTP1 - alproto = s->alproto; - } else { + if (s->alproto != ALPROTO_UNKNOWN) { + alproto = AppProtoCommon(s->alproto, alproto); + if (alproto == ALPROTO_FAILED) { SCLogError("can't set rule app proto to %s: already set to %s", AppProtoToString(alproto), AppProtoToString(s->alproto)); return -1; diff --git a/src/detect.c b/src/detect.c index 5d6eb4df3e..0302a374b5 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1050,6 +1050,24 @@ DetectRunTxSortHelper(const void *a, const void *b) #define TRACE_SID_TXS(sid,txs,...) #endif +// Get inner transaction for engine +void *DetectGetInnerTx(void *tx_ptr, AppProto alproto, AppProto engine_alproto, uint8_t flow_flags) +{ + if (unlikely(alproto == ALPROTO_DOH2)) { + if (engine_alproto == ALPROTO_DNS) { + // need to get the dns tx pointer + tx_ptr = SCDoH2GetDnsTx(tx_ptr, flow_flags); + } else if (engine_alproto != ALPROTO_HTTP2) { + // incompatible engine->alproto with flow alproto + tx_ptr = NULL; + } + } else if (engine_alproto != alproto) { + // incompatible engine->alproto with flow alproto + tx_ptr = NULL; + } + return tx_ptr; +} + /** \internal * \brief inspect a rule against a transaction * @@ -1110,12 +1128,16 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, if (!(inspect_flags & BIT_U32(engine->id)) && direction == engine->dir) { - const bool skip_engine = (engine->alproto != 0 && engine->alproto != f->alproto); - /* special case: file_data on 'alert tcp' will have engines - * in the list that are not for us. */ - if (unlikely(skip_engine)) { - engine = engine->next; - continue; + void *tx_ptr = DetectGetInnerTx(tx->tx_ptr, f->alproto, engine->alproto, flow_flags); + if (tx_ptr == NULL) { + if (engine->alproto != ALPROTO_UNKNOWN) { + /* special case: file_data on 'alert tcp' will have engines + * in the list that are not for us. */ + engine = engine->next; + continue; + } else { + tx_ptr = tx->tx_ptr; + } } /* engines are sorted per progress, except that the one with @@ -1150,7 +1172,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, KEYWORD_PROFILING_SET_LIST(det_ctx, engine->sm_list); DEBUG_VALIDATE_BUG_ON(engine->v2.Callback == NULL); match = engine->v2.Callback( - de_ctx, det_ctx, engine, s, f, flow_flags, alstate, tx->tx_ptr, tx->tx_id); + de_ctx, det_ctx, engine, s, f, flow_flags, alstate, tx_ptr, tx->tx_id); TRACE_SID_TXS(s->id, tx, "engine %p match %d", engine, match); if (engine->stream) { can->stream_stored = true; diff --git a/src/detect.h b/src/detect.h index 3715018c1a..4df2dfa3d6 100644 --- a/src/detect.h +++ b/src/detect.h @@ -1586,6 +1586,7 @@ const SigGroupHead *SigMatchSignaturesGetSgh(const DetectEngineCtx *de_ctx, cons int DetectUnregisterThreadCtxFuncs(DetectEngineCtx *, void *data, const char *name); int DetectRegisterThreadCtxFuncs(DetectEngineCtx *, const char *name, void *(*InitFunc)(void *), void *data, void (*FreeFunc)(void *), int); void *DetectThreadCtxGetKeywordThreadCtx(DetectEngineThreadCtx *, int); +void *DetectGetInnerTx(void *tx_ptr, AppProto alproto, AppProto engine_alproto, uint8_t flow_flags); void RuleMatchCandidateTxArrayInit(DetectEngineThreadCtx *det_ctx, uint32_t size); void RuleMatchCandidateTxArrayFree(DetectEngineThreadCtx *det_ctx); -- 2.47.2