From 8fb08a4f3a4892812f76b72d6523c3ed8ad926b4 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 29 Jan 2026 15:40:36 +0100 Subject: [PATCH 1/5] detect: convert tx_progress to uint8_t --- .../extending/app-layer/transactions.rst | 2 +- rust/src/ssh/detect.rs | 12 ++++++------ rust/sys/src/sys.rs | 6 ++---- src/app-layer-parser.c | 6 +++--- src/app-layer-parser.h | 2 +- src/detect-app-layer-state.c | 6 +++--- src/detect-engine-file.h | 2 +- src/detect-engine-helper.c | 5 +++-- src/detect-engine-helper.h | 5 +++-- src/detect-engine-mpm.c | 8 ++++---- src/detect-engine-mpm.h | 6 +++--- src/detect-engine-prefilter.c | 16 ++++++++-------- src/detect-engine-prefilter.h | 4 ++-- src/detect-engine.c | 15 +++++++-------- src/detect-engine.h | 6 +++--- src/detect-file-data.c | 6 +++--- src/detect-parse.c | 16 ++++++++-------- src/detect.c | 2 +- src/detect.h | 6 +++--- 19 files changed, 65 insertions(+), 66 deletions(-) diff --git a/doc/userguide/devguide/extending/app-layer/transactions.rst b/doc/userguide/devguide/extending/app-layer/transactions.rst index f46cda9543d3..52ca3bcc9460 100644 --- a/doc/userguide/devguide/extending/app-layer/transactions.rst +++ b/doc/userguide/devguide/extending/app-layer/transactions.rst @@ -72,7 +72,7 @@ Rule Matching Transaction progress is also used for certain keywords to know what is the minimum state before we can expect a match: until that, Suricata won't even try to look for the patterns. -As seen in ``DetectAppLayerMpmRegister`` that has ``int progress`` as parameter, and ``DetectAppLayerInspectEngineRegister``, which expects ``int tx_min_progress``, for instance. In the code snippet, +As seen in ``DetectAppLayerMpmRegister`` that has ``int progress`` as parameter, and ``DetectAppLayerInspectEngineRegister``, which expects ``uint8_t tx_min_progress``, for instance. In the code snippet, ``HTTP2StateDataClient``, ``HTTP2StateDataServer`` and ``0`` are the values passed to the functions - in the last example, for ``FTPDATA``, the existence of a transaction implies that a file is being transferred. Hence the ``0`` value. diff --git a/rust/src/ssh/detect.rs b/rust/src/ssh/detect.rs index 84ee5b27d62b..10b46648f256 100644 --- a/rust/src/ssh/detect.rs +++ b/rust/src/ssh/detect.rs @@ -298,7 +298,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER | STREAM_TOCLIENT, Some(SCSshTxGetSoftware), - SSHConnectionState::SshStateBannerDone as c_int, + SSHConnectionState::SshStateBannerDone as u8, ); SCDetectHelperKeywordAliasRegister( ssh_software_kw_id, @@ -340,7 +340,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER | STREAM_TOCLIENT, Some(SCSshTxGetProtocol), - SSHConnectionState::SshStateBannerDone as c_int, + SSHConnectionState::SshStateBannerDone as u8, ); SCDetectHelperKeywordAliasRegister( ssh_proto_kw_id, @@ -360,7 +360,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER, Some(SCSshTxGetHasshString), - SSHConnectionState::SshStateBannerDone as c_int, + SSHConnectionState::SshStateBannerDone as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH_STRING, @@ -380,7 +380,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOCLIENT, Some(SCSshTxGetHasshString), - SSHConnectionState::SshStateBannerDone as c_int, + SSHConnectionState::SshStateBannerDone as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH_SERVER_STRING, @@ -400,7 +400,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER, Some(SCSshTxGetHassh), - SSHConnectionState::SshStateBannerDone as c_int, + SSHConnectionState::SshStateBannerDone as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH, @@ -421,7 +421,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOCLIENT, Some(SCSshTxGetHassh), - SSHConnectionState::SshStateBannerDone as c_int, + SSHConnectionState::SshStateBannerDone as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH_SERVER, diff --git a/rust/sys/src/sys.rs b/rust/sys/src/sys.rs index 35ee3e633a02..da4ccc4f4f20 100644 --- a/rust/sys/src/sys.rs +++ b/rust/sys/src/sys.rs @@ -823,8 +823,7 @@ extern "C" { extern "C" { pub fn SCDetectHelperBufferProgressMpmRegister( name: *const ::std::os::raw::c_char, desc: *const ::std::os::raw::c_char, - alproto: AppProto, direction: u8, GetData: InspectionSingleBufferGetDataPtr, - progress: ::std::os::raw::c_int, + alproto: AppProto, direction: u8, GetData: InspectionSingleBufferGetDataPtr, progress: u8, ) -> ::std::os::raw::c_int; } extern "C" { @@ -836,8 +835,7 @@ extern "C" { extern "C" { pub fn SCDetectHelperMultiBufferProgressMpmRegister( name: *const ::std::os::raw::c_char, desc: *const ::std::os::raw::c_char, - alproto: AppProto, direction: u8, GetData: InspectionMultiBufferGetDataPtr, - progress: ::std::os::raw::c_int, + alproto: AppProto, direction: u8, GetData: InspectionMultiBufferGetDataPtr, progress: u8, ) -> ::std::os::raw::c_int; } extern "C" { diff --git a/src/app-layer-parser.c b/src/app-layer-parser.c index 0ab84a8269de..e3c1a6f86eb7 100644 --- a/src/app-layer-parser.c +++ b/src/app-layer-parser.c @@ -1119,12 +1119,12 @@ void *AppLayerParserGetTx(uint8_t ipproto, AppProto alproto, void *alstate, uint SCReturnPtr(r, "void *"); } -int AppLayerParserGetStateProgressCompletionStatus(AppProto alproto, - uint8_t direction) +uint8_t AppLayerParserGetStateProgressCompletionStatus(AppProto alproto, uint8_t direction) { SCEnter(); int r = StateGetProgressCompletionStatus(alproto, direction); - SCReturnInt(r); + // TODO convert StateGetProgressCompletionStatus and more to uint8_t + return (uint8_t)r; } int AppLayerParserGetEventInfo(uint8_t ipproto, AppProto alproto, const char *event_name, diff --git a/src/app-layer-parser.h b/src/app-layer-parser.h index da39ae5dc4fb..177562b70de0 100644 --- a/src/app-layer-parser.h +++ b/src/app-layer-parser.h @@ -328,7 +328,7 @@ int AppLayerParserGetStateProgress(uint8_t ipproto, AppProto alproto, void *alstate, uint8_t direction); uint64_t AppLayerParserGetTxCnt(const Flow *, void *alstate); void *AppLayerParserGetTx(uint8_t ipproto, AppProto alproto, void *alstate, uint64_t tx_id); -int AppLayerParserGetStateProgressCompletionStatus(AppProto alproto, uint8_t direction); +uint8_t AppLayerParserGetStateProgressCompletionStatus(AppProto alproto, uint8_t direction); int AppLayerParserGetEventInfo(uint8_t ipproto, AppProto alproto, const char *event_name, uint8_t *event_id, AppLayerEventType *event_type); int AppLayerParserGetEventInfoById(uint8_t ipproto, AppProto alproto, uint8_t event_id, diff --git a/src/detect-app-layer-state.c b/src/detect-app-layer-state.c index 6ec8d013cdb8..37a997a38648 100644 --- a/src/detect-app-layer-state.c +++ b/src/detect-app-layer-state.c @@ -140,7 +140,7 @@ static uint8_t DetectEngineAptStateInspect(DetectEngineCtx *de_ctx, DetectEngine } // TODO dedup with detect-parse.c -static SignatureHook SetAppHook(const AppProto alproto, int progress) +static SignatureHook SetAppHook(const AppProto alproto, uint8_t progress) { SignatureHook h = { .type = SIGNATURE_HOOK_TYPE_APP, @@ -175,7 +175,7 @@ static int DetectAppLayerStateSetup(DetectEngineCtx *de_ctx, Signature *s, const IPPROTO_TCP /* TODO */, s->alproto, h, STREAM_TOSERVER); if (progress_ts >= 0) { s->flags |= SIG_FLAG_TOSERVER; - s->init_data->hook = SetAppHook(s->alproto, progress_ts); + s->init_data->hook = SetAppHook(s->alproto, (uint8_t)progress_ts); } else { const int progress_tc = AppLayerParserGetStateIdByName( IPPROTO_TCP /* TODO */, s->alproto, h, STREAM_TOCLIENT); @@ -183,7 +183,7 @@ static int DetectAppLayerStateSetup(DetectEngineCtx *de_ctx, Signature *s, const return -1; } s->flags |= SIG_FLAG_TOCLIENT; - s->init_data->hook = SetAppHook(s->alproto, progress_tc); + s->init_data->hook = SetAppHook(s->alproto, (uint8_t)progress_tc); } SCLogDebug("hook %u", s->init_data->hook.t.app.app_progress); return 0; diff --git a/src/detect-engine-file.h b/src/detect-engine-file.h index 10bcf5ca7dbc..555683e23d0e 100644 --- a/src/detect-engine-file.h +++ b/src/detect-engine-file.h @@ -29,6 +29,6 @@ uint8_t DetectFileInspectGeneric(DetectEngineCtx *de_ctx, DetectEngineThreadCtx uint8_t flags, void *_alstate, void *tx, uint64_t tx_id); void DetectFileRegisterProto( - AppProto alproto, int direction, int to_client_progress, int to_server_progress); + AppProto alproto, int direction, uint8_t to_client_progress, uint8_t to_server_progress); #endif /* SURICATA_DETECT_ENGINE_FILE_H */ diff --git a/src/detect-engine-helper.c b/src/detect-engine-helper.c index e3ff76c5ae26..634a62612ace 100644 --- a/src/detect-engine-helper.c +++ b/src/detect-engine-helper.c @@ -64,7 +64,7 @@ int SCDetectHelperBufferMpmRegister(const char *name, const char *desc, AppProto } int SCDetectHelperBufferProgressMpmRegister(const char *name, const char *desc, AppProto alproto, - uint8_t direction, InspectionSingleBufferGetDataPtr GetData, int progress) + uint8_t direction, InspectionSingleBufferGetDataPtr GetData, uint8_t progress) { if (direction & STREAM_TOSERVER) { DetectAppLayerInspectEngineRegisterSingle(name, alproto, SIG_FLAG_TOSERVER, progress, @@ -83,7 +83,8 @@ int SCDetectHelperBufferProgressMpmRegister(const char *name, const char *desc, } int SCDetectHelperMultiBufferProgressMpmRegister(const char *name, const char *desc, - AppProto alproto, uint8_t direction, InspectionMultiBufferGetDataPtr GetData, int progress) + AppProto alproto, uint8_t direction, InspectionMultiBufferGetDataPtr GetData, + uint8_t progress) { if (direction & STREAM_TOSERVER) { DetectAppLayerMultiRegister(name, alproto, SIG_FLAG_TOSERVER, progress, GetData, 2); diff --git a/src/detect-engine-helper.h b/src/detect-engine-helper.h index 6e8491527b84..a74680b63c2b 100644 --- a/src/detect-engine-helper.h +++ b/src/detect-engine-helper.h @@ -85,11 +85,12 @@ int SCDetectHelperBufferRegister(const char *name, AppProto alproto, uint8_t dir int SCDetectHelperBufferMpmRegister(const char *name, const char *desc, AppProto alproto, uint8_t direction, InspectionSingleBufferGetDataPtr GetData); int SCDetectHelperBufferProgressMpmRegister(const char *name, const char *desc, AppProto alproto, - uint8_t direction, InspectionSingleBufferGetDataPtr GetData, int progress); + uint8_t direction, InspectionSingleBufferGetDataPtr GetData, uint8_t progress); int SCDetectHelperMultiBufferMpmRegister(const char *name, const char *desc, AppProto alproto, uint8_t direction, InspectionMultiBufferGetDataPtr GetData); int SCDetectHelperMultiBufferProgressMpmRegister(const char *name, const char *desc, - AppProto alproto, uint8_t direction, InspectionMultiBufferGetDataPtr GetData, int progress); + AppProto alproto, uint8_t direction, InspectionMultiBufferGetDataPtr GetData, + uint8_t progress); int SCDetectHelperTransformRegister(const SCTransformTableElmt *kw); diff --git a/src/detect-engine-mpm.c b/src/detect-engine-mpm.c index c1af73c2fdd5..acbe7a379a28 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -89,7 +89,7 @@ static int g_mpm_list_cnt[DETECT_BUFFER_MPM_TYPE_SIZE] = { 0, 0, 0 }; static void RegisterInternal(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionBufferGetDataPtr GetData, InspectionSingleBufferGetDataPtr GetDataSingle, - InspectionMultiBufferGetDataPtr GetMultiData, AppProto alproto, int tx_min_progress) + InspectionMultiBufferGetDataPtr GetMultiData, AppProto alproto, uint8_t tx_min_progress) { SCLogDebug("registering %s/%d/%d/%p/%p/%u/%d", name, direction, priority, PrefilterRegister, GetData, alproto, tx_min_progress); @@ -151,7 +151,7 @@ static void RegisterInternal(const char *name, int direction, int priority, void DetectAppLayerMpmRegister(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionBufferGetDataPtr GetData, - AppProto alproto, int tx_min_progress) + AppProto alproto, uint8_t tx_min_progress) { RegisterInternal(name, direction, priority, PrefilterRegister, GetData, NULL, NULL, alproto, tx_min_progress); @@ -159,7 +159,7 @@ void DetectAppLayerMpmRegister(const char *name, int direction, int priority, void DetectAppLayerMpmRegisterSingle(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionSingleBufferGetDataPtr GetData, - AppProto alproto, int tx_min_progress) + AppProto alproto, uint8_t tx_min_progress) { RegisterInternal(name, direction, priority, PrefilterRegister, NULL, GetData, NULL, alproto, tx_min_progress); @@ -167,7 +167,7 @@ void DetectAppLayerMpmRegisterSingle(const char *name, int direction, int priori void DetectAppLayerMpmMultiRegister(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionMultiBufferGetDataPtr GetData, - AppProto alproto, int tx_min_progress) + AppProto alproto, uint8_t tx_min_progress) { RegisterInternal(name, direction, priority, PrefilterRegister, NULL, NULL, GetData, alproto, tx_min_progress); diff --git a/src/detect-engine-mpm.h b/src/detect-engine-mpm.h index 6bde23a2020c..25892ce6a44c 100644 --- a/src/detect-engine-mpm.h +++ b/src/detect-engine-mpm.h @@ -86,13 +86,13 @@ typedef int (*PrefilterRegisterFunc)(DetectEngineCtx *de_ctx, SigGroupHead *sgh, */ void DetectAppLayerMpmRegister(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionBufferGetDataPtr GetData, - AppProto alproto, int tx_min_progress); + AppProto alproto, uint8_t tx_min_progress); void DetectAppLayerMpmRegisterSingle(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionSingleBufferGetDataPtr GetData, - AppProto alproto, int tx_min_progress); + AppProto alproto, uint8_t tx_min_progress); void DetectAppLayerMpmMultiRegister(const char *name, int direction, int priority, PrefilterRegisterFunc PrefilterRegister, InspectionMultiBufferGetDataPtr GetData, - AppProto alproto, int tx_min_progress); + AppProto alproto, uint8_t tx_min_progress); void DetectAppLayerMpmRegisterByParentId( DetectEngineCtx *de_ctx, const int id, const int parent_id, diff --git a/src/detect-engine-prefilter.c b/src/detect-engine-prefilter.c index 5a4124af183b..0e01b54987b4 100644 --- a/src/detect-engine-prefilter.c +++ b/src/detect-engine-prefilter.c @@ -350,7 +350,7 @@ int PrefilterAppendPayloadEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, } int PrefilterAppendTxEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - PrefilterTxFn PrefilterTxFunc, AppProto alproto, int tx_min_progress, void *pectx, + PrefilterTxFn PrefilterTxFunc, AppProto alproto, uint8_t tx_min_progress, void *pectx, void (*FreeFunc)(void *pectx), const char *name) { if (sgh == NULL || PrefilterTxFunc == NULL || pectx == NULL) @@ -366,7 +366,7 @@ int PrefilterAppendTxEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, e->alproto = alproto; // TODO change function prototype ? DEBUG_VALIDATE_BUG_ON(tx_min_progress > INT8_MAX); - e->tx_min_progress = (uint8_t)tx_min_progress; + e->tx_min_progress = tx_min_progress; e->Free = FreeFunc; if (sgh->init->tx_engines == NULL) { @@ -700,7 +700,7 @@ static void NonPFNamesFree(void *data) struct TxNonPFData { AppProto alproto; int dir; /**< 0: toserver, 1: toclient */ - int progress; /**< progress state value to register at */ + uint8_t progress; /**< progress state value to register at */ int sig_list; /**< special handling: normally 0, but for special cases (app-layer-state, app-layer-event) use the list id to create separate engines */ uint32_t sigs_cnt; @@ -730,7 +730,7 @@ static void TxNonPFFree(void *data) } static int TxNonPFAddSig(DetectEngineCtx *de_ctx, HashListTable *tx_engines_hash, - const AppProto alproto, const int dir, const int16_t progress, const int sig_list, + const AppProto alproto, const int dir, const uint8_t progress, const int sig_list, const char *name, const Signature *s) { const uint32_t max_sids = DetectEngineGetMaxSigId(de_ctx); @@ -1005,8 +1005,8 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) dir == 0 ? STREAM_TOSERVER : STREAM_TOCLIENT); if (TxNonPFAddSig(de_ctx, tx_engines_hash, s->alproto, dir, - (int16_t)s->init_data->hook.t.app.app_progress, s->init_data->hook.sm_list, - pname, s) != 0) { + s->init_data->hook.t.app.app_progress, s->init_data->hook.sm_list, pname, + s) != 0) { goto error; } tx_non_pf = true; @@ -1082,10 +1082,10 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) } /* register special progress value to indicate we need to run it all the time */ - int engine_progress = t->progress; + uint8_t engine_progress = t->progress; if (t->sig_list == app_state_list_id) { SCLogDebug("engine %s for state list", t->engine_name); - engine_progress = -1; + engine_progress = UINT8_MAX; } struct PrefilterNonPFDataTx *data = diff --git a/src/detect-engine-prefilter.h b/src/detect-engine-prefilter.h index b1ebb0b35ffd..b01fe11bf893 100644 --- a/src/detect-engine-prefilter.h +++ b/src/detect-engine-prefilter.h @@ -40,7 +40,7 @@ typedef struct DetectTransaction_ { /* original value to track changes. */ const uint8_t detect_progress_orig; - const int tx_progress; + const uint8_t tx_progress; const int tx_end_state; } DetectTransaction; @@ -63,7 +63,7 @@ void PrefilterPostRuleMatch( int PrefilterAppendPayloadEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, PrefilterPktFn PrefilterFunc, void *pectx, void (*FreeFunc)(void *pectx), const char *name); int PrefilterAppendTxEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - PrefilterTxFn PrefilterTxFunc, const AppProto alproto, const int tx_min_progress, + PrefilterTxFn PrefilterTxFunc, const AppProto alproto, const uint8_t tx_min_progress, void *pectx, void (*FreeFunc)(void *pectx), const char *name); int PrefilterAppendFrameEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, PrefilterFrameFn PrefilterFrameFunc, AppProto alproto, uint8_t frame_type, void *pectx, diff --git a/src/detect-engine.c b/src/detect-engine.c index eeea4483e0c5..848a53f1406d 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -195,7 +195,7 @@ void DetectPktInspectEngineRegister(const char *name, * * \note errors are fatal */ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alproto, uint32_t dir, - int progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData, + uint8_t progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData, InspectionSingleBufferGetDataPtr GetDataSingle, InspectionMultiBufferGetDataPtr GetMultiData) { @@ -209,8 +209,7 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp SCLogDebug("name %s id %d", name, sm_list); if ((alproto == ALPROTO_FAILED) || (!(dir == SIG_FLAG_TOSERVER || dir == SIG_FLAG_TOCLIENT)) || - (sm_list < DETECT_SM_LIST_MATCH) || (sm_list >= SHRT_MAX) || - (progress < 0 || progress >= SHRT_MAX) || (Callback == NULL)) { + (sm_list < DETECT_SM_LIST_MATCH) || (sm_list >= SHRT_MAX) || (Callback == NULL)) { SCLogError("Invalid arguments"); BUG_ON(1); } else if (Callback == DetectEngineInspectBufferGeneric && GetData == NULL) { @@ -248,7 +247,7 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp new_engine->dir = direction; new_engine->sm_list = (uint16_t)sm_list; new_engine->sm_list_base = (uint16_t)sm_list; - new_engine->progress = (int16_t)progress; + new_engine->progress = progress; new_engine->v2.Callback = Callback; if (Callback == DetectEngineInspectBufferGeneric) { new_engine->v2.GetData = GetData; @@ -271,7 +270,7 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp } void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uint32_t dir, - int progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData) + uint8_t progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData) { /* before adding, check that we don't add a duplicate entry, which will * propagate all the way into the packet runtime if allowed. */ @@ -293,7 +292,7 @@ void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uin } void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alproto, uint32_t dir, - int progress, InspectEngineFuncPtr Callback, InspectionSingleBufferGetDataPtr GetData) + uint8_t progress, InspectEngineFuncPtr Callback, InspectionSingleBufferGetDataPtr GetData) { /* before adding, check that we don't add a duplicate entry, which will * propagate all the way into the packet runtime if allowed. */ @@ -876,7 +875,7 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature DetectEngineAppInspectionEngine t = { .alproto = s->init_data->hook.t.app.alproto, - .progress = (uint16_t)s->init_data->hook.t.app.app_progress, + .progress = s->init_data->hook.t.app.app_progress, .sm_list = (uint16_t)s->init_data->hook.sm_list, .sm_list_base = (uint16_t)s->init_data->hook.sm_list, .dir = dir, @@ -2096,7 +2095,7 @@ uint8_t DetectEngineInspectBufferGeneric(DetectEngineCtx *de_ctx, DetectEngineTh // wrapper for both DetectAppLayerInspectEngineRegister and DetectAppLayerMpmRegister // with cast of callback function -void DetectAppLayerMultiRegister(const char *name, AppProto alproto, uint32_t dir, int progress, +void DetectAppLayerMultiRegister(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectionMultiBufferGetDataPtr GetData, int priority) { AppLayerInspectEngineRegisterInternal(name, alproto, dir, progress, diff --git a/src/detect-engine.h b/src/detect-engine.h index 5aca85a6881d..1e90760ae1c5 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -164,12 +164,12 @@ int DetectEngineInspectPktBufferGeneric( * \param Callback The engine callback. */ void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uint32_t dir, - int progress, InspectEngineFuncPtr Callback2, InspectionBufferGetDataPtr GetData); + uint8_t progress, InspectEngineFuncPtr Callback2, InspectionBufferGetDataPtr GetData); void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alproto, uint32_t dir, - int progress, InspectEngineFuncPtr Callback2, InspectionSingleBufferGetDataPtr GetData); + uint8_t progress, InspectEngineFuncPtr Callback2, InspectionSingleBufferGetDataPtr GetData); -void DetectAppLayerMultiRegister(const char *name, AppProto alproto, uint32_t dir, int progress, +void DetectAppLayerMultiRegister(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectionMultiBufferGetDataPtr GetData, int priority); void DetectPktInspectEngineRegister(const char *name, diff --git a/src/detect-file-data.c b/src/detect-file-data.c index b4eb213726ca..a6e4bffe061d 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -70,8 +70,8 @@ int PrefilterMpmFiledataRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, Mpm typedef struct { AppProto alproto; int direction; - int to_client_progress; - int to_server_progress; + uint8_t to_client_progress; + uint8_t to_server_progress; } DetectFileHandlerProtocol_t; /* Table with all filehandler registrations */ @@ -97,7 +97,7 @@ DetectFileHandlerProtocol_t al_protocols[ALPROTO_WITHFILES_MAX] = { }; void DetectFileRegisterProto( - AppProto alproto, int direction, int to_client_progress, int to_server_progress) + AppProto alproto, int direction, uint8_t to_client_progress, uint8_t to_server_progress) { size_t i = 0; while (i < ALPROTO_WITHFILES_MAX && al_protocols[i].alproto != ALPROTO_UNKNOWN) { diff --git a/src/detect-parse.c b/src/detect-parse.c index c2f513ae37ce..87264cc7db02 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1156,9 +1156,9 @@ void DetectRegisterAppLayerHookLists(void) alproto_name = "http1"; SCLogDebug("alproto %u/%s", a, alproto_name); - const int max_progress_ts = + const uint8_t max_progress_ts = AppLayerParserGetStateProgressCompletionStatus(a, STREAM_TOSERVER); - const int max_progress_tc = + const uint8_t max_progress_tc = AppLayerParserGetStateProgressCompletionStatus(a, STREAM_TOCLIENT); char ts_tx_started[64]; @@ -1191,7 +1191,7 @@ void DetectRegisterAppLayerHookLists(void) SCLogDebug("- hook %s:%s list %s (%u)", alproto_name, "response_name", tc_tx_complete, (uint32_t)strlen(tc_tx_complete)); - for (int p = 0; p <= max_progress_ts; p++) { + for (uint8_t p = 0; p <= max_progress_ts; p++) { const char *name = AppLayerParserGetStateNameById( IPPROTO_TCP /* TODO no ipproto */, a, p, STREAM_TOSERVER); if (name != NULL && !IsBuiltIn(name)) { @@ -1204,7 +1204,7 @@ void DetectRegisterAppLayerHookLists(void) list_name, a, SIG_FLAG_TOSERVER, p, DetectEngineInspectGenericList, NULL); } } - for (int p = 0; p <= max_progress_tc; p++) { + for (uint8_t p = 0; p <= max_progress_tc; p++) { const char *name = AppLayerParserGetStateNameById( IPPROTO_TCP /* TODO no ipproto */, a, p, STREAM_TOCLIENT); if (name != NULL && !IsBuiltIn(name)) { @@ -1299,7 +1299,7 @@ static int SigParseProtoHookPkt(Signature *s, const char *proto_hook, const char return 0; } -static SignatureHook SetAppHook(const AppProto alproto, int progress) +static SignatureHook SetAppHook(const AppProto alproto, uint8_t progress) { SignatureHook h = { .type = SIGNATURE_HOOK_TYPE_APP, @@ -1335,7 +1335,7 @@ static int SigParseProtoHookApp(Signature *s, const char *proto_hook, const char IPPROTO_TCP /* TODO */, s->alproto, h, STREAM_TOSERVER); if (progress_ts >= 0) { s->flags |= SIG_FLAG_TOSERVER; - s->init_data->hook = SetAppHook(s->alproto, progress_ts); + s->init_data->hook = SetAppHook(s->alproto, (uint8_t)progress_ts); } else { const int progress_tc = AppLayerParserGetStateIdByName( IPPROTO_TCP /* TODO */, s->alproto, h, STREAM_TOCLIENT); @@ -1343,7 +1343,7 @@ static int SigParseProtoHookApp(Signature *s, const char *proto_hook, const char return -1; } s->flags |= SIG_FLAG_TOCLIENT; - s->init_data->hook = SetAppHook(s->alproto, progress_tc); + s->init_data->hook = SetAppHook(s->alproto, (uint8_t)progress_tc); } } @@ -1360,7 +1360,7 @@ static int SigParseProtoHookApp(Signature *s, const char *proto_hook, const char SignatureHookTypeToString(s->init_data->hook.type), s->init_data->hook.t.app.alproto, s->init_data->hook.t.app.app_progress); - s->app_progress_hook = (uint8_t)s->init_data->hook.t.app.app_progress; + s->app_progress_hook = s->init_data->hook.t.app.app_progress; return 0; } diff --git a/src/detect.c b/src/detect.c index da7d32755dd2..850d5a0fc2fb 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1443,7 +1443,7 @@ static DetectTransaction GetDetectTx(const uint8_t ipproto, const AppProto alpro .de_state = tx_dir_state, .detect_progress = detect_progress, .detect_progress_orig = detect_progress, - .tx_progress = tx_progress, + .tx_progress = (uint8_t)tx_progress, .tx_end_state = tx_end_state, }; return tx; diff --git a/src/detect.h b/src/detect.h index 9f14e1216e25..52693e832a91 100644 --- a/src/detect.h +++ b/src/detect.h @@ -422,7 +422,7 @@ typedef struct DetectEngineAppInspectionEngine_ { bool match_on_null; uint16_t sm_list; uint16_t sm_list_base; /**< base buffer being transformed */ - int16_t progress; + uint8_t progress; struct { union { @@ -576,7 +576,7 @@ typedef struct SignatureHook_ { AppProto alproto; /** progress value of the app-layer hook specified in the rule. Sets the app_proto * specific progress value. */ - int app_progress; + uint8_t app_progress; } app; struct { enum SignatureHookPkt ph; @@ -784,7 +784,7 @@ typedef struct DetectBufferMpmRegistry_ { InspectionMultiBufferGetDataPtr GetMultiData; }; AppProto alproto; - int tx_min_progress; + uint8_t tx_min_progress; } app_v2; /* pkt matching: use if type == DETECT_BUFFER_MPM_TYPE_PKT */ From 500819009acb984e0d724c69eb57dc59fd95e81e Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 29 Jan 2026 15:54:11 +0100 Subject: [PATCH 2/5] detect: AppInspectionEngine can have a min and max progress instead of a single progress. Will help for keywords such as http.header which can act on headers and trailers progress Tx engines are inspected between min_progress and max_progress So, we do not give up and says a signature does not match when it will match on later max_progress And we can match as early as possible, especially in IPS mode. --- src/detect-engine-analyzer.c | 7 +++++- src/detect-engine-prefilter.c | 11 ++++----- src/detect-engine-prefilter.h | 2 +- src/detect-engine.c | 43 +++++++++++++++++++++-------------- src/detect-file-data.c | 2 +- src/detect-filemagic.c | 2 +- src/detect-filename.c | 2 +- src/detect-http-client-body.c | 4 ++-- src/detect-parse.c | 16 ++++++------- src/detect.c | 32 +++++++++++++++++--------- src/detect.h | 7 +++++- 11 files changed, 78 insertions(+), 50 deletions(-) diff --git a/src/detect-engine-analyzer.c b/src/detect-engine-analyzer.c index dc5cf98bb9f9..6272e7033c8f 100644 --- a/src/detect-engine-analyzer.c +++ b/src/detect-engine-analyzer.c @@ -1441,7 +1441,12 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s) SCJbSetString(ctx.js, "direction", direction); SCJbSetBool(ctx.js, "is_mpm", app->mpm); SCJbSetString(ctx.js, "app_proto", AppProtoToString(app->alproto)); - SCJbSetUint(ctx.js, "progress", app->progress); + if (app->max_progress != app->min_progress) { + SCJbSetUint(ctx.js, "min_progress", app->min_progress); + SCJbSetUint(ctx.js, "max_progress", app->max_progress); + } else { + SCJbSetUint(ctx.js, "progress", app->min_progress); + } if (app->v2.transforms != NULL) { SCJbOpenArray(ctx.js, "transforms"); diff --git a/src/detect-engine-prefilter.c b/src/detect-engine-prefilter.c index 0e01b54987b4..d1a3ba3926a6 100644 --- a/src/detect-engine-prefilter.c +++ b/src/detect-engine-prefilter.c @@ -350,7 +350,7 @@ int PrefilterAppendPayloadEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, } int PrefilterAppendTxEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - PrefilterTxFn PrefilterTxFunc, AppProto alproto, uint8_t tx_min_progress, void *pectx, + PrefilterTxFn PrefilterTxFunc, AppProto alproto, int8_t tx_min_progress, void *pectx, void (*FreeFunc)(void *pectx), const char *name) { if (sgh == NULL || PrefilterTxFunc == NULL || pectx == NULL) @@ -364,8 +364,6 @@ int PrefilterAppendTxEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, e->PrefilterTx = PrefilterTxFunc; e->pectx = pectx; e->alproto = alproto; - // TODO change function prototype ? - DEBUG_VALIDATE_BUG_ON(tx_min_progress > INT8_MAX); e->tx_min_progress = tx_min_progress; e->Free = FreeFunc; @@ -990,7 +988,7 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) if (list_id == app_state_list_id) sig_list = app_state_list_id; if (TxNonPFAddSig(de_ctx, tx_engines_hash, app->alproto, app->dir, - app->progress, sig_list, buf->name, s) != 0) { + app->min_progress, sig_list, buf->name, s) != 0) { goto error; } tx_non_pf = true; @@ -1082,10 +1080,11 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) } /* register special progress value to indicate we need to run it all the time */ - uint8_t engine_progress = t->progress; + DEBUG_VALIDATE_BUG_ON(t->progress > INT8_MAX); + int8_t engine_progress = (int8_t)t->progress; if (t->sig_list == app_state_list_id) { SCLogDebug("engine %s for state list", t->engine_name); - engine_progress = UINT8_MAX; + engine_progress = -1; } struct PrefilterNonPFDataTx *data = diff --git a/src/detect-engine-prefilter.h b/src/detect-engine-prefilter.h index b01fe11bf893..87ac299b9069 100644 --- a/src/detect-engine-prefilter.h +++ b/src/detect-engine-prefilter.h @@ -63,7 +63,7 @@ void PrefilterPostRuleMatch( int PrefilterAppendPayloadEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, PrefilterPktFn PrefilterFunc, void *pectx, void (*FreeFunc)(void *pectx), const char *name); int PrefilterAppendTxEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, - PrefilterTxFn PrefilterTxFunc, const AppProto alproto, const uint8_t tx_min_progress, + PrefilterTxFn PrefilterTxFunc, const AppProto alproto, const int8_t tx_min_progress, void *pectx, void (*FreeFunc)(void *pectx), const char *name); int PrefilterAppendFrameEngine(DetectEngineCtx *de_ctx, SigGroupHead *sgh, PrefilterFrameFn PrefilterFrameFunc, AppProto alproto, uint8_t frame_type, void *pectx, diff --git a/src/detect-engine.c b/src/detect-engine.c index 848a53f1406d..67baec1a36e0 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -247,7 +247,8 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp new_engine->dir = direction; new_engine->sm_list = (uint16_t)sm_list; new_engine->sm_list_base = (uint16_t)sm_list; - new_engine->progress = progress; + new_engine->min_progress = progress; + new_engine->max_progress = progress; new_engine->v2.Callback = Callback; if (Callback == DetectEngineInspectBufferGeneric) { new_engine->v2.GetData = GetData; @@ -280,7 +281,8 @@ void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uin const int sm_list = DetectBufferTypeGetByName(name); if (t->sm_list == sm_list && t->alproto == alproto && t_direction == dir && - t->progress == progress && t->v2.Callback == Callback && t->v2.GetData == GetData) { + t->min_progress == progress && t->v2.Callback == Callback && + t->v2.GetData == GetData) { DEBUG_VALIDATE_BUG_ON(1); return; } @@ -302,7 +304,7 @@ void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alprot const int sm_list = DetectBufferTypeGetByName(name); if (t->sm_list == sm_list && t->alproto == alproto && t_direction == dir && - t->progress == progress && t->v2.Callback == Callback && + t->min_progress == progress && t->v2.Callback == Callback && t->v2.GetDataSingle == GetData) { DEBUG_VALIDATE_BUG_ON(1); return; @@ -333,7 +335,8 @@ static void DetectAppLayerInspectEngineCopy( new_engine->sm_list = (uint16_t)new_list; /* use new list id */ DEBUG_VALIDATE_BUG_ON(sm_list < 0 || sm_list > UINT16_MAX); new_engine->sm_list_base = (uint16_t)sm_list; - new_engine->progress = t->progress; + new_engine->min_progress = t->min_progress; + new_engine->max_progress = t->max_progress; new_engine->v2 = t->v2; new_engine->v2.transforms = transforms; /* assign transforms */ @@ -366,7 +369,8 @@ static void DetectAppLayerInspectEngineCopyListToDetectCtx(DetectEngineCtx *de_c new_engine->dir = t->dir; new_engine->sm_list = t->sm_list; new_engine->sm_list_base = t->sm_list; - new_engine->progress = t->progress; + new_engine->min_progress = t->min_progress; + new_engine->max_progress = t->max_progress; new_engine->v2 = t->v2; if (list == NULL) { @@ -585,7 +589,8 @@ static void AppendStreamInspectEngine( new_engine->sm_list_base = DETECT_SM_LIST_PMATCH; new_engine->smd = stream; new_engine->v2.Callback = DetectEngineInspectStream; - new_engine->progress = 0; + new_engine->min_progress = 0; + new_engine->max_progress = 0; /* append */ if (s->app_inspect == NULL) { @@ -738,7 +743,8 @@ static void AppendAppInspectEngine(DetectEngineCtx *de_ctx, new_engine->sm_list_base = t->sm_list_base; new_engine->smd = smd; new_engine->match_on_null = smd ? DetectContentInspectionMatchOnAbsentBuffer(smd) : false; - new_engine->progress = t->progress; + new_engine->min_progress = t->min_progress; + new_engine->max_progress = t->max_progress; new_engine->v2 = t->v2; SCLogDebug("sm_list %d new_engine->v2 %p/%p/%p", new_engine->sm_list, new_engine->v2.Callback, new_engine->v2.GetData, new_engine->v2.transforms); @@ -755,7 +761,8 @@ static void AppendAppInspectEngine(DetectEngineCtx *de_ctx, } /* prepend engine if forced or if our engine has a lower progress. */ - } else if (prepend || (!(*head_is_mpm) && s->app_inspect->progress > new_engine->progress)) { + } else if (prepend || + (!(*head_is_mpm) && s->app_inspect->min_progress > new_engine->min_progress)) { new_engine->next = s->app_inspect; s->app_inspect = new_engine; if (new_engine->sm_list == files_id) { @@ -770,7 +777,7 @@ static void AppendAppInspectEngine(DetectEngineCtx *de_ctx, } else { DetectEngineAppInspectionEngine *a = s->app_inspect; while (a->next != NULL) { - if (a->next && a->next->progress > new_engine->progress) { + if (a->next && a->next->min_progress > new_engine->min_progress) { break; } a = a->next; @@ -875,7 +882,8 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature DetectEngineAppInspectionEngine t = { .alproto = s->init_data->hook.t.app.alproto, - .progress = s->init_data->hook.t.app.app_progress, + .min_progress = s->init_data->hook.t.app.app_progress, + .max_progress = s->init_data->hook.t.app.app_progress, .sm_list = (uint16_t)s->init_data->hook.sm_list, .sm_list_base = (uint16_t)s->init_data->hook.sm_list, .dir = dir, @@ -907,9 +915,9 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature #ifdef DEBUG const DetectEngineAppInspectionEngine *iter = s->app_inspect; while (iter) { - SCLogDebug("%u: engine %s id %u progress %d %s", s->id, - DetectEngineBufferTypeGetNameById(de_ctx, iter->sm_list), iter->id, iter->progress, - iter->sm_list == mpm_list ? "MPM" : ""); + SCLogDebug("%u: engine %s id %u progress %d-%d %s", s->id, + DetectEngineBufferTypeGetNameById(de_ctx, iter->sm_list), iter->id, + iter->min_progress, iter->max_progress, iter->sm_list == mpm_list ? "MPM" : ""); iter = iter->next; } #endif @@ -1991,8 +1999,8 @@ uint8_t DetectEngineInspectBufferSingle(DetectEngineCtx *de_ctx, DetectEngineThr const int list_id = engine->sm_list; SCLogDebug("running inspect on %d", list_id); - const bool eof = - (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > engine->progress); + const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > + engine->max_progress); SCLogDebug("list %d mpm? %s transforms %p", engine->sm_list, engine->mpm ? "true" : "false", engine->v2.transforms); @@ -2052,7 +2060,8 @@ uint8_t DetectEngineInspectBufferGeneric(DetectEngineCtx *de_ctx, DetectEngineTh const int list_id = engine->sm_list; SCLogDebug("running inspect on %d", list_id); - const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > engine->progress); + const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > + engine->max_progress); SCLogDebug("list %d mpm? %s transforms %p", engine->sm_list, engine->mpm ? "true" : "false", engine->v2.transforms); @@ -2174,7 +2183,7 @@ uint8_t DetectEngineInspectMultiBufferGeneric(DetectEngineCtx *de_ctx, if (local_id == 0) { // That means we did not get even one buffer value from the multi-buffer const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > - engine->progress); + engine->max_progress); if (eof && engine->match_on_null) { return DETECT_ENGINE_INSPECT_SIG_MATCH; } diff --git a/src/detect-file-data.c b/src/detect-file-data.c index a6e4bffe061d..1f6d377a8314 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -491,7 +491,7 @@ uint8_t DetectEngineInspectFiledata(DetectEngineCtx *de_ctx, DetectEngineThreadC } if (ffc->head == NULL) { const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > - engine->progress); + engine->max_progress); if (eof && engine->match_on_null) { return DETECT_ENGINE_INSPECT_SIG_MATCH; } diff --git a/src/detect-filemagic.c b/src/detect-filemagic.c index 263737218005..2f0781958f51 100644 --- a/src/detect-filemagic.c +++ b/src/detect-filemagic.c @@ -311,7 +311,7 @@ static uint8_t DetectEngineInspectFilemagic(DetectEngineCtx *de_ctx, DetectEngin FileContainer *ffc = files.fc; if (ffc == NULL || ffc->head == NULL) { const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > - engine->progress); + engine->max_progress); if (eof && engine->match_on_null) { return DETECT_ENGINE_INSPECT_SIG_MATCH; } diff --git a/src/detect-filename.c b/src/detect-filename.c index fec04a48fb42..9513d0ae64b2 100644 --- a/src/detect-filename.c +++ b/src/detect-filename.c @@ -248,7 +248,7 @@ static uint8_t DetectEngineInspectFilename(DetectEngineCtx *de_ctx, DetectEngine FileContainer *ffc = files.fc; if (ffc == NULL || ffc->head == NULL) { const bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > - engine->progress); + engine->max_progress); if (eof && engine->match_on_null) { return DETECT_ENGINE_INSPECT_SIG_MATCH; } diff --git a/src/detect-http-client-body.c b/src/detect-http-client-body.c index af1e2caf5220..af80514d4d62 100644 --- a/src/detect-http-client-body.c +++ b/src/detect-http-client-body.c @@ -312,8 +312,8 @@ static uint8_t DetectEngineInspectBufferHttpBody(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx, const DetectEngineAppInspectionEngine *engine, const Signature *s, Flow *f, uint8_t flags, void *alstate, void *txv, uint64_t tx_id) { - bool eof = - (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > engine->progress); + bool eof = (AppLayerParserGetStateProgress(f->proto, f->alproto, txv, flags) > + engine->max_progress); const InspectionBuffer *buffer = HttpRequestBodyGetDataCallback( det_ctx, engine->v2.transforms, f, flags, txv, engine->sm_list, engine->sm_list_base); if (buffer == NULL || buffer->inspect == NULL) { diff --git a/src/detect-parse.c b/src/detect-parse.c index 87264cc7db02..aaef0cf9abf0 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -2620,17 +2620,17 @@ static int SigValidateCheckBuffers( /* only allow rules to use the hook for engines at that * exact progress for now. */ if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP) { - if ((s->flags & SIG_FLAG_TOSERVER) && (app->dir == 0) && - app->progress != s->init_data->hook.t.app.app_progress) { - SCLogError("engine progress value %d doesn't match hook %u", app->progress, + bool samedir = ((s->flags & SIG_FLAG_TOSERVER) && (app->dir == 0)) || + ((s->flags & SIG_FLAG_TOCLIENT) && (app->dir == 1)); + + if (samedir && + (app->min_progress > s->init_data->hook.t.app.app_progress || + app->max_progress > s->init_data->hook.t.app.app_progress)) { + SCLogError("engine progress value %d-%d doesn't match hook %u", + app->min_progress, app->max_progress, s->init_data->hook.t.app.app_progress); SCReturnInt(0); } - if ((s->flags & SIG_FLAG_TOCLIENT) && (app->dir == 1) && - app->progress != s->init_data->hook.t.app.app_progress) { - SCLogError("engine progress value doesn't match hook"); - SCReturnInt(0); - } } } } diff --git a/src/detect.c b/src/detect.c index 850d5a0fc2fb..4328a0be602e 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1242,23 +1242,33 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, /* engines are sorted per progress, except that the one with * mpm/prefilter enabled is first */ - if (tx->tx_progress < engine->progress) { - SCLogDebug("tx progress %d < engine progress %d", - tx->tx_progress, engine->progress); + if (tx->tx_progress < engine->min_progress) { + SCLogDebug("tx progress %d < engine progress %d", tx->tx_progress, + engine->min_progress); break; } if (engine->mpm) { - if (tx->tx_progress > engine->progress) { + if (tx->tx_progress > engine->max_progress) { TRACE_SID_TXS(s->id, tx, - "engine->mpm: t->tx_progress %u > engine->progress %u, so set " + "engine->mpm: t->tx_progress %u > engine->max_progress %u, so set " "mpm_before_progress", - tx->tx_progress, engine->progress); + tx->tx_progress, engine->max_progress); mpm_before_progress = true; - } else if (tx->tx_progress == engine->progress) { + } else if (tx->tx_progress == engine->min_progress) { + // do not use >= min progress && <= max_progress here + // because mpm_in_progress is used to optimize and + // not store partially (so far) matching signature as next + // packet with next progress will re-run mpm. + // But for a rule with + // a http1 header as fast_pattern/mpm + // a http1 method, and some content on client body + // when we are in body progress and do not match yet the content + // we will not re-run http_header prefilter for headers progress + // so we need to store signature TRACE_SID_TXS(s->id, tx, - "engine->mpm: t->tx_progress %u == engine->progress %u, so set " + "engine->mpm: t->tx_progress %u == engine->min_progress %u, so set " "mpm_in_progress", - tx->tx_progress, engine->progress); + tx->tx_progress, engine->min_progress); mpm_in_progress = true; } } @@ -1275,13 +1285,13 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, } else if (engine->v2.Callback == NULL) { /* TODO is this the cleanest way to support a non-app sig on a app hook? */ - if (tx->tx_progress > engine->progress) { + if (tx->tx_progress > engine->max_progress) { mpm_before_progress = true; // TODO needs a new name now } /* we don't have to store a "hook" match, also don't want to keep any state to make * sure the hook gets invoked again until tx progress progresses. */ - if (tx->tx_progress <= engine->progress) + if (tx->tx_progress <= engine->max_progress) return DETECT_ENGINE_INSPECT_SIG_MATCH; /* if progress > engine progress, track state to avoid additional matches */ diff --git a/src/detect.h b/src/detect.h index 52693e832a91..5d8a2973a07f 100644 --- a/src/detect.h +++ b/src/detect.h @@ -422,7 +422,12 @@ typedef struct DetectEngineAppInspectionEngine_ { bool match_on_null; uint16_t sm_list; uint16_t sm_list_base; /**< base buffer being transformed */ - uint8_t progress; + // An app-layer inspection engine may run at different progresses. + // Even if most engines run at only one progress (min_progress=max_progress) + // http_header keyword may run for headers and trailers progress for example + uint8_t min_progress; + // max_progress allows us to not give up on matching too soon + uint8_t max_progress; struct { union { From 5df6422f09daaa3dced4d8dbaa429a76ecf8fb89 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 29 Jan 2026 16:02:53 +0100 Subject: [PATCH 3/5] detect: introduce DetectAppLayerInspectEngineRegisterMax Function to register a app engine with a min and max progress --- src/detect-engine.c | 56 +++++++++++++++++++++++++++++++++++++-------- src/detect-engine.h | 4 ++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/detect-engine.c b/src/detect-engine.c index 67baec1a36e0..4c98b9ababe9 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -195,11 +195,12 @@ void DetectPktInspectEngineRegister(const char *name, * * \note errors are fatal */ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alproto, uint32_t dir, - uint8_t progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData, - InspectionSingleBufferGetDataPtr GetDataSingle, + uint8_t min_progress, uint8_t max_progress, InspectEngineFuncPtr Callback, + InspectionBufferGetDataPtr GetData, InspectionSingleBufferGetDataPtr GetDataSingle, InspectionMultiBufferGetDataPtr GetMultiData) { - BUG_ON(progress >= 48); + BUG_ON(min_progress >= 48 || max_progress >= 48); + BUG_ON(max_progress < min_progress); DetectBufferTypeRegister(name); const int sm_list = DetectBufferTypeGetByName(name); @@ -234,8 +235,8 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp } // every DNS or HTTP2 can be accessed from DOH2 if (alproto == ALPROTO_HTTP2 || alproto == ALPROTO_DNS) { - AppLayerInspectEngineRegisterInternal( - name, ALPROTO_DOH2, dir, progress, Callback, GetData, GetDataSingle, GetMultiData); + AppLayerInspectEngineRegisterInternal(name, ALPROTO_DOH2, dir, min_progress, max_progress, + Callback, GetData, GetDataSingle, GetMultiData); } DetectEngineAppInspectionEngine *new_engine = @@ -247,8 +248,8 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp new_engine->dir = direction; new_engine->sm_list = (uint16_t)sm_list; new_engine->sm_list_base = (uint16_t)sm_list; - new_engine->min_progress = progress; - new_engine->max_progress = progress; + new_engine->min_progress = min_progress; + new_engine->max_progress = max_progress; new_engine->v2.Callback = Callback; if (Callback == DetectEngineInspectBufferGeneric) { new_engine->v2.GetData = GetData; @@ -270,6 +271,41 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp } } +/** + * \brief register an app-layer inspect engine with a min and max progress + * + * \param name the buffer name + * \param alproto app-layer protocol + * \param dir direction + * \param min_progress minimum progress to run the engine + * \param max_progress maximum progress up to which we can run the engine + * \param Callback callback function, can be generic DetectEngineInspectBufferGeneric + * \param GetData seconday callback in case of use of DetectEngineInspectBufferGeneric + */ +void DetectAppLayerInspectEngineRegisterMax(const char *name, AppProto alproto, uint32_t dir, + uint8_t min_progress, uint8_t max_progress, InspectEngineFuncPtr Callback, + InspectionBufferGetDataPtr GetData) +{ + /* before adding, check that we don't add a duplicate entry, which will + * propagate all the way into the packet runtime if allowed. */ + DetectEngineAppInspectionEngine *t = g_app_inspect_engines; + while (t != NULL) { + const uint32_t t_direction = t->dir == 0 ? SIG_FLAG_TOSERVER : SIG_FLAG_TOCLIENT; + const int sm_list = DetectBufferTypeGetByName(name); + + if (t->sm_list == sm_list && t->alproto == alproto && t_direction == dir && + t->min_progress == min_progress && t->v2.Callback == Callback && + t->v2.GetData == GetData) { + DEBUG_VALIDATE_BUG_ON(1); + return; + } + t = t->next; + } + + AppLayerInspectEngineRegisterInternal( + name, alproto, dir, min_progress, max_progress, Callback, GetData, NULL, NULL); +} + void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData) { @@ -290,7 +326,7 @@ void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uin } AppLayerInspectEngineRegisterInternal( - name, alproto, dir, progress, Callback, GetData, NULL, NULL); + name, alproto, dir, progress, progress, Callback, GetData, NULL, NULL); } void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alproto, uint32_t dir, @@ -313,7 +349,7 @@ void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alprot } AppLayerInspectEngineRegisterInternal( - name, alproto, dir, progress, Callback, NULL, GetData, NULL); + name, alproto, dir, progress, progress, Callback, NULL, GetData, NULL); } /* copy an inspect engine with transforms to a new list id. */ @@ -2107,7 +2143,7 @@ uint8_t DetectEngineInspectBufferGeneric(DetectEngineCtx *de_ctx, DetectEngineTh void DetectAppLayerMultiRegister(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectionMultiBufferGetDataPtr GetData, int priority) { - AppLayerInspectEngineRegisterInternal(name, alproto, dir, progress, + AppLayerInspectEngineRegisterInternal(name, alproto, dir, progress, progress, DetectEngineInspectMultiBufferGeneric, NULL, NULL, GetData); DetectAppLayerMpmMultiRegister( name, dir, priority, PrefilterMultiGenericMpmRegister, GetData, alproto, progress); diff --git a/src/detect-engine.h b/src/detect-engine.h index 1e90760ae1c5..4e8d540b31d8 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -166,6 +166,10 @@ int DetectEngineInspectPktBufferGeneric( void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectEngineFuncPtr Callback2, InspectionBufferGetDataPtr GetData); +void DetectAppLayerInspectEngineRegisterMax(const char *name, AppProto alproto, uint32_t dir, + uint8_t min_progress, uint8_t max_progress, InspectEngineFuncPtr Callback2, + InspectionBufferGetDataPtr GetData); + void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectEngineFuncPtr Callback2, InspectionSingleBufferGetDataPtr GetData); From f728a4e01d94ae66cfeaa7dba46b57cd989f62b8 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 29 Jan 2026 16:04:59 +0100 Subject: [PATCH 4/5] detect: http.headers now works for trailers as it registers the app engine up to the trailers progress Ticket: 8256 --- src/detect-http-header.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/detect-http-header.c b/src/detect-http-header.c index 817df4d89814..cf71b9359fd2 100644 --- a/src/detect-http-header.c +++ b/src/detect-http-header.c @@ -383,14 +383,16 @@ void DetectHttpHeaderRegister(void) sigmatch_table[DETECT_HTTP_HEADER].flags |= SIGMATCH_NOOPT; sigmatch_table[DETECT_HTTP_HEADER].flags |= SIGMATCH_INFO_STICKY_BUFFER; - DetectAppLayerInspectEngineRegister("http_header", ALPROTO_HTTP1, SIG_FLAG_TOSERVER, - HTP_REQUEST_PROGRESS_HEADERS, DetectEngineInspectBufferGeneric, GetData1); + DetectAppLayerInspectEngineRegisterMax("http_header", ALPROTO_HTTP1, SIG_FLAG_TOSERVER, + HTP_REQUEST_PROGRESS_HEADERS, HTP_REQUEST_PROGRESS_TRAILER, + DetectEngineInspectBufferGeneric, GetData1); DetectAppLayerMpmRegister("http_header", SIG_FLAG_TOSERVER, 2, PrefilterMpmHttpHeaderRequestRegister, NULL, ALPROTO_HTTP1, 0); /* not used, registered twice: HEADERS/TRAILER */ - DetectAppLayerInspectEngineRegister("http_header", ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, - HTP_RESPONSE_PROGRESS_HEADERS, DetectEngineInspectBufferGeneric, GetData1); + DetectAppLayerInspectEngineRegisterMax("http_header", ALPROTO_HTTP1, SIG_FLAG_TOCLIENT, + HTP_RESPONSE_PROGRESS_HEADERS, HTP_RESPONSE_PROGRESS_TRAILER, + DetectEngineInspectBufferGeneric, GetData1); DetectAppLayerMpmRegister("http_header", SIG_FLAG_TOCLIENT, 2, PrefilterMpmHttpHeaderResponseRegister, NULL, ALPROTO_HTTP1, 0); /* not used, registered twice: HEADERS/TRAILER */ From 7a3bfbaf497573a63c4fb054dde31f2011f19aab Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 29 Jan 2026 22:32:04 +0100 Subject: [PATCH 5/5] detect/file: reorder struct fields to reduce padding and make scan-build happy --- src/detect-file-data.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detect-file-data.c b/src/detect-file-data.c index 1f6d377a8314..a95b2214f475 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -68,8 +68,8 @@ int PrefilterMpmFiledataRegister(DetectEngineCtx *de_ctx, SigGroupHead *sgh, Mpm // file protocols with common file handling typedef struct { - AppProto alproto; int direction; + AppProto alproto; uint8_t to_client_progress; uint8_t to_server_progress; } DetectFileHandlerProtocol_t;