Detect http trailers 8256 v2 by catenacyber · Pull Request #14724 · OISF/suricata

catenacyber · 2026-01-30T08:46:38Z

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/8256

Describe changes:

detect: http.headers works on trailers even if it is not fast_pattern
To do so :
convert many variables/fields/args describing a tx_progress to use u8 instead of int or other
adds a max_progress field to DetectEngineAppInspectionEngine
adds a DetectAppLayerInspectEngineRegisterMax function to register an app engine with a min_progress < max_progress

#14717 clean and rebased

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.

Function to register a app engine with a min and max progress

as it registers the app engine up to the trailers progress Ticket: 8256

and make scan-build happy

codecov · 2026-01-30T10:47:23Z

Codecov Report

❌ Patch coverage is 94.79167% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.17%. Comparing base (81572cb) to head (e42618b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14724      +/-   ##
==========================================
- Coverage   82.17%   82.17%   -0.01%     
==========================================
  Files        1008     1008              
  Lines      263916   263941      +25     
==========================================
+ Hits       216868   216882      +14     
- Misses      47048    47059      +11

Flag	Coverage Δ
fuzzcorpus	`60.20% <89.58%> (+<0.01%)`	⬆️
livemode	`18.73% <43.75%> (+0.02%)`	⬆️
netns	`18.51% <43.75%> (-0.01%)`	⬇️
pcap	`44.64% <65.62%> (+0.02%)`	⬆️
suricata-verify	`65.36% <94.79%> (-0.01%)`	⬇️
unittests	`59.36% <72.91%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

suricata-qa · 2026-01-30T12:55:03Z

ERROR:

ERROR: QA failed on SURI_TLPR1_alerts_cmp.

Pipeline = 29352

victorjulien

QA failure needs investigating. Further comments inline. We need more docs on this code in general, but for this update specifically to be able to understand how it affects things.

victorjulien · 2026-02-03T05:57:56Z

src/detect.c

-                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 "


these messages need updating as well

actually this can be removed...

victorjulien · 2026-02-03T05:59:24Z

src/detect.c

+                            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) {


not sure how this relates to max_progress now, should it be >= min progress && <= max_progress?

I will add a comment

If it were >= min progress && <= max_progress, with no other changes, it fails DetectHttpClientBodyTest14

DetectHttpClientBodyTest14 uses method, header and body in signature with a header being the fast_pattern.

So, we may match the fast_pattern, then the method, then have not enough data for additional header, but if we do not store the signature, we never go back to rerunning the prefilter which already matched...

victorjulien · 2026-02-03T06:00:23Z

src/detect.h

    uint16_t sm_list;
    uint16_t sm_list_base; /**< base buffer being transformed */
-    uint8_t progress;
+    uint8_t min_progress;


need explanations here

victorjulien · 2026-02-03T06:02:14Z

src/detect-engine.c

    }
 }

+void DetectAppLayerInspectEngineRegisterMax(const char *name, AppProto alproto, uint32_t dir,


should have docs here

victorjulien · 2026-02-03T06:04:07Z

src/detect-http-header.c

    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,


commit message should explain better why this is needed

victorjulien · 2026-02-03T06:05:20Z

src/detect-engine-analyzer.c

            SCJbSetBool(ctx.js, "is_mpm", app->mpm);
            SCJbSetString(ctx.js, "app_proto", AppProtoToString(app->alproto));
-            SCJbSetUint(ctx.js, "progress", app->progress);
+            SCJbSetUint(ctx.js, "progress", app->min_progress);


should be named progress_min?

I did not want to break compatibility, I will just add min and max if they are different

catenacyber · 2026-02-04T15:06:58Z

@ct0br0 asking you help about this QA result ;-)

catenacyber · 2026-02-04T15:10:53Z

Status : I have the review changes ready in detect-http-trailers-8256-v3
But I need some more data (TLPR pcaps) to investigate the QA different result (2 sigs with http.header keyword, one triggering more, one triggering less)

catenacyber added 5 commits January 30, 2026 09:45

detect: convert tx_progress to uint8_t

358999d

detect: introduce DetectAppLayerInspectEngineRegisterMax

3b8d059

Function to register a app engine with a min and max progress

detect: http.headers now works for trailers

dc3edee

as it registers the app engine up to the trailers progress Ticket: 8256

detect/file: reorder struct fields to reduce padding

e42618b

and make scan-build happy

catenacyber requested review from jasonish, jufajardini and victorjulien as code owners January 30, 2026 08:46

catenacyber mentioned this pull request Jan 30, 2026

Detect http trailers 8256 v1 #14717

Closed

victorjulien requested changes Feb 3, 2026

View reviewed changes

Conversation

catenacyber commented Jan 30, 2026

Uh oh!

codecov bot commented Jan 30, 2026

Codecov Report

Uh oh!

suricata-qa commented Jan 30, 2026

Uh oh!

victorjulien left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

catenacyber commented Feb 4, 2026

Uh oh!

catenacyber commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants