diff --git a/src/detect-engine-analyzer.c b/src/detect-engine-analyzer.c index 387fc76f6f..ad26449e62 100644 --- a/src/detect-engine-analyzer.c +++ b/src/detect-engine-analyzer.c @@ -1478,10 +1478,15 @@ 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->sub_state) SCJbSetString(ctx.js, "sub_state", AppLayerParserGetSubStateName(app->alproto, app->sub_state)); + 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 6368382a00..7dc909ce5c 100644 --- a/src/detect-engine-prefilter.c +++ b/src/detect-engine-prefilter.c @@ -1082,9 +1082,16 @@ static int SetupNonPrefilter(DetectEngineCtx *de_ctx, SigGroupHead *sgh) const bool run_always = buf != NULL && buf->run_always; const uint8_t sub_state = app->sub_state; if (TxNonPFAddSig(de_ctx, tx_engines_hash, app->alproto, sub_state, app->dir, - app->progress, sig_list, buf->name, s, run_always) != 0) { + app->min_progress, sig_list, buf->name, s, run_always) != 0) { goto error; } + if (app->max_progress > app->min_progress) { + if (TxNonPFAddSig(de_ctx, tx_engines_hash, app->alproto, sub_state, + app->dir, app->max_progress, sig_list, buf->name, s, + run_always) != 0) { + goto error; + } + } tx_non_pf = true; } } diff --git a/src/detect-engine.c b/src/detect-engine.c index f8ce5d9cf3..c2b764d1ce 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -197,8 +197,9 @@ void DetectPktInspectEngineRegister(const char *name, * * \note errors are fatal */ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alproto, uint32_t dir, - uint8_t sub_state, uint8_t progress, InspectEngineFuncPtr Callback, - InspectionBufferGetDataPtr GetData, InspectionSingleBufferGetDataPtr GetDataSingle, + uint8_t sub_state, uint8_t min_progress, uint8_t max_progress, + InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData, + InspectionSingleBufferGetDataPtr GetDataSingle, InspectionMultiBufferGetDataPtr GetMultiData) { /* ignore special case unknown */ @@ -206,7 +207,8 @@ static void AppLayerInspectEngineRegisterInternal(const char *name, AppProto alp DEBUG_VALIDATE_BUG_ON(AppLayerParserSupportsSubStates(alproto) && sub_state == 0); DEBUG_VALIDATE_BUG_ON(!AppLayerParserSupportsSubStates(alproto) && sub_state != 0); } - BUG_ON(progress >= APP_LAYER_MAX_PROGRESS); + BUG_ON(max_progress >= APP_LAYER_MAX_PROGRESS); + BUG_ON(max_progress < min_progress); DetectBufferTypeRegister(name); const int sm_list = DetectBufferTypeGetByName(name); @@ -216,8 +218,8 @@ 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 >= 48) || - (Callback == NULL)) { + (sm_list < DETECT_SM_LIST_MATCH) || (sm_list >= SHRT_MAX) || + (max_progress >= APP_LAYER_MAX_PROGRESS) || (Callback == NULL)) { SCLogError("Invalid arguments"); BUG_ON(1); } else if (Callback == DetectEngineInspectBufferGeneric && GetData == NULL) { @@ -249,8 +251,9 @@ 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->sub_state = sub_state; + 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; @@ -272,6 +275,45 @@ 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 progress1 first progress to run the engine + * \param progress2 second progress where we can run the engine + * \param Callback callback function, can be generic DetectEngineInspectBufferGeneric + * \param GetData seconday callback in case of use of DetectEngineInspectBufferGeneric + */ +void DetectAppLayerInspectEngineRegister2(const char *name, AppProto alproto, uint32_t dir, + uint8_t progress1, uint8_t progress2, 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; + if (progress1 >= progress2) { + FatalError( + "Bad order for progresses for %s : %d should be > %d", name, progress1, progress2); + } + 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 == progress1 && t->v2.Callback == Callback && + t->v2.GetData == GetData) { + DEBUG_VALIDATE_BUG_ON(1); + return; + } + t = t->next; + } + + AppLayerInspectEngineRegisterInternal( + name, alproto, dir, 0, progress1, progress2, Callback, GetData, NULL, NULL); +} + void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectEngineFuncPtr Callback, InspectionBufferGetDataPtr GetData) { @@ -283,7 +325,7 @@ 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->sub_state == 0 && t->progress == progress && t->v2.Callback == Callback && + t->sub_state == 0 && t->min_progress == progress && t->v2.Callback == Callback && t->v2.GetData == GetData) { DEBUG_VALIDATE_BUG_ON(1); return; @@ -292,7 +334,7 @@ void DetectAppLayerInspectEngineRegister(const char *name, AppProto alproto, uin } AppLayerInspectEngineRegisterInternal( - name, alproto, dir, 0, (uint8_t)progress, Callback, GetData, NULL, NULL); + name, alproto, dir, 0, progress, progress, Callback, GetData, NULL, NULL); } void DetectAppLayerInspectEngineRegisterSubState(const char *name, AppProto alproto, uint32_t dir, @@ -307,7 +349,7 @@ void DetectAppLayerInspectEngineRegisterSubState(const char *name, AppProto alpr const int sm_list = DetectBufferTypeGetByName(name); if (t->sm_list == sm_list && t->alproto == alproto && t_direction == dir && - t->sub_state == sub_state && t->progress == progress && + t->sub_state == sub_state && t->min_progress == progress && t->v2.Callback == Callback && t->v2.GetData == GetData) { DEBUG_VALIDATE_BUG_ON(1); return; @@ -316,7 +358,7 @@ void DetectAppLayerInspectEngineRegisterSubState(const char *name, AppProto alpr } AppLayerInspectEngineRegisterInternal( - name, alproto, dir, sub_state, progress, Callback, GetData, NULL, NULL); + name, alproto, dir, sub_state, progress, progress, Callback, GetData, NULL, NULL); } void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectEngineFuncPtr Callback, InspectionSingleBufferGetDataPtr GetData) @@ -329,7 +371,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; @@ -338,7 +380,7 @@ void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alprot } AppLayerInspectEngineRegisterInternal( - name, alproto, dir, 0, (uint8_t)progress, Callback, NULL, GetData, NULL); + name, alproto, dir, 0, progress, progress, Callback, NULL, GetData, NULL); } /* copy an inspect engine with transforms to a new list id. */ @@ -360,7 +402,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->sub_state = t->sub_state; new_engine->v2 = t->v2; new_engine->v2.transforms = transforms; /* assign transforms */ @@ -394,7 +437,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->sub_state = t->sub_state; new_engine->v2 = t->v2; @@ -614,7 +658,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) { @@ -783,7 +828,20 @@ 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; + // A firewall rule such as + // accept:flow,alert http1: any any (http.header; content:"xyz"; + // should not have max_progress response_trailers so that DetectTxCompleted returns true at + // response_body + if ((s->flags & SIG_FLAG_FW_HOOK_LTE) && s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP && + new_engine->max_progress > s->app_progress_hook) { + new_engine->max_progress = new_engine->min_progress; + } else if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_APP && + (s->flags & SIG_FLAG_FW_HOOK_LTE) == 0 && + new_engine->min_progress < s->app_progress_hook) { + new_engine->min_progress = new_engine->max_progress; + } new_engine->sub_state = t->sub_state; new_engine->v2 = t->v2; SCLogDebug("sm_list %d new_engine->v2 %p/%p/%p", new_engine->sm_list, new_engine->v2.Callback, @@ -801,7 +859,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) { @@ -816,7 +875,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; @@ -966,7 +1025,8 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature DetectEngineAppInspectionEngine t = { .alproto = s->init_data->hook.t.app.alproto, - .progress = state, + .min_progress = state, + .max_progress = state, .sub_state = s->init_data->hook.t.app.sub_state, .sm_list = (uint16_t)sm_list, .sm_list_base = (uint16_t)sm_list, @@ -1049,7 +1109,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, .sub_state = s->init_data->hook.t.app.sub_state, .sm_list = (uint16_t)s->init_data->hook.sm_list, .sm_list_base = (uint16_t)s->init_data->hook.sm_list, @@ -1082,9 +1143,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 @@ -2143,10 +2204,10 @@ static bool DetectTxCompleted( if (progress < 0) { return false; } - if (progress > engine->progress) { + if (progress > engine->max_progress) { return true; } - if (progress < engine->progress) { + if (progress < engine->max_progress) { return false; } return progress == AppLayerParserGetTxEndState(f->proto, f->alproto, txv, flags); @@ -2318,7 +2379,7 @@ void DetectAppLayerMultiRegisterSubState(const char *name, AppProto alproto, uin uint8_t sub_state, uint8_t progress, InspectionMultiBufferGetDataPtr GetData, int priority) { BUG_ON(AppLayerParserSupportsSubStates(alproto) && sub_state == 0); - AppLayerInspectEngineRegisterInternal(name, alproto, dir, sub_state, progress, + AppLayerInspectEngineRegisterInternal(name, alproto, dir, sub_state, progress, progress, DetectEngineInspectMultiBufferGeneric, NULL, NULL, GetData); DetectAppLayerMpmMultiRegisterSubState(name, dir, priority, PrefilterMultiGenericMpmRegister, GetData, alproto, sub_state, progress); @@ -2330,7 +2391,7 @@ void DetectAppLayerMultiRegister(const char *name, AppProto alproto, uint32_t di InspectionMultiBufferGetDataPtr GetData, int priority) { BUG_ON(AppLayerParserSupportsSubStates(alproto)); - AppLayerInspectEngineRegisterInternal(name, alproto, dir, 0, (uint8_t)progress, + AppLayerInspectEngineRegisterInternal(name, alproto, dir, 0, 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 ff788ef7cb..5e330ef7c6 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -172,6 +172,10 @@ void DetectAppLayerInspectEngineRegisterSubState(const char *name, AppProto alpr uint8_t type, uint8_t progress, InspectEngineFuncPtr Callback2, InspectionBufferGetDataPtr GetData); +void DetectAppLayerInspectEngineRegister2(const char *name, AppProto alproto, uint32_t dir, + uint8_t progress1, uint8_t progress2, InspectEngineFuncPtr Callback2, + InspectionBufferGetDataPtr GetData); + void DetectAppLayerInspectEngineRegisterSingle(const char *name, AppProto alproto, uint32_t dir, uint8_t progress, InspectEngineFuncPtr Callback2, InspectionSingleBufferGetDataPtr GetData); diff --git a/src/detect-file-data.c b/src/detect-file-data.c index 293b826c73..537c4c4f75 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -493,8 +493,8 @@ uint8_t DetectEngineInspectFiledata(DetectEngineCtx *de_ctx, DetectEngineThreadC if (ffc == NULL) { return DETECT_ENGINE_INSPECT_SIG_CANT_MATCH_FILES; } - 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); if (ffc->head == NULL) { 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 336edc9204..4e686d62d6 100644 --- a/src/detect-filemagic.c +++ b/src/detect-filemagic.c @@ -310,7 +310,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 ad5484b641..a2903eb798 100644 --- a/src/detect-filename.c +++ b/src/detect-filename.c @@ -245,7 +245,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 44a03c2208..adfd4f073b 100644 --- a/src/detect-http-client-body.c +++ b/src/detect-http-client-body.c @@ -311,8 +311,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-http-header.c b/src/detect-http-header.c index 89ea1d199e..46e7366450 100644 --- a/src/detect-http-header.c +++ b/src/detect-http-header.c @@ -386,14 +386,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); + DetectAppLayerInspectEngineRegister2("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); + DetectAppLayerInspectEngineRegister2("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 */ diff --git a/src/detect-parse.c b/src/detect-parse.c index 045f660f04..a60ab91dea 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -3084,17 +3084,14 @@ static int SigValidateCheckBuffers( } 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 doesn't match hook %u", app->min_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); - } } buffer_consumed = true; diff --git a/src/detect.c b/src/detect.c index 4e5e41d5f4..fe363ea479 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1370,14 +1370,14 @@ static int DetectRunTxInspectRule(ThreadVars *tv, DetectEngineCtx *de_ctx, "skip because engine alproto %s sub_state %u != tx_type %u (engine " "progress %u)", AppProtoToString(engine->alproto), engine->sub_state, tx->tx_type, - engine->progress); + engine->min_progress); engine = engine->next; continue; } TRACE_SID_TXS(s->id, tx, "inspecting engine alproto %s sub_state %u == tx_type %u (engine progress %u)", AppProtoToString(engine->alproto), engine->sub_state, tx->tx_type, - engine->progress); + engine->min_progress); void *tx_ptr = DetectGetInnerTx(tx->tx_ptr, f->alproto, engine->alproto, flow_flags); if (tx_ptr == NULL) { @@ -1396,23 +1396,33 @@ static int DetectRunTxInspectRule(ThreadVars *tv, DetectEngineCtx *de_ctx, /* 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); if ((p->flags & PKT_PSEUDO_DETECTLOG_FLUSH) == 0) { mpm_in_progress = true; } @@ -1431,13 +1441,14 @@ static int DetectRunTxInspectRule(ThreadVars *tv, DetectEngineCtx *de_ctx, } 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 ((s->flags & SIG_FLAG_FW_HOOK_LTE) == 0 && tx->tx_progress <= engine->progress) { + if ((s->flags & SIG_FLAG_FW_HOOK_LTE) == 0 && + tx->tx_progress <= engine->max_progress) { return 1; // DETECT_ENGINE_INSPECT_SIG_MATCH; } diff --git a/src/detect.h b/src/detect.h index 605b54693a..18926fa425 100644 --- a/src/detect.h +++ b/src/detect.h @@ -427,8 +427,13 @@ typedef struct DetectEngineAppInspectionEngine_ { bool match_on_null; uint16_t sm_list; uint16_t sm_list_base; /**< base buffer being transformed */ - uint8_t progress; uint8_t sub_state; /**< matches tx type */ + // 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 {