Fw updates/v19 by victorjulien · Pull Request #15306 · OISF/suricata

victorjulien · 2026-05-01T13:56:02Z

Replaces #15300:

minor fixups to get CI green

Tickets:

https://redmine.openinfosecfoundation.org/issues/8497
https://redmine.openinfosecfoundation.org/issues/8495
https://redmine.openinfosecfoundation.org/issues/8480
https://redmine.openinfosecfoundation.org/issues/8444
https://redmine.openinfosecfoundation.org/issues/8387
https://redmine.openinfosecfoundation.org/issues/8390

- tls.cert_chain_len - datarep - dataset - dns.opcode Part of Ticket OISF#8387

The firewall enabling flag for tcp.flags was being overwritten by another line of code. Related to Ticket OISF#8387

Clean up host mode tracking, which is used by reject to control how rejects are sent. Before this patch there were 2 modes: sniffer only and router. This patch introduces a bridge mode that is automatically set by the bridge modes. In bridge mode the `Packet::livedev` is used. Ticket: OISF#8390.

So a value of 0 means no device.

Most code uses an opague type for LiveDevice, so add an id getter.

In prep for storing both directions for IPS.

Update ctx caching to take direction into account.

Use an enum for the firewall related flow control, to improve readability of the firewall inspection logic.

If a ruleset would use `dns:request_complete` but not have a rule for `dns:request_started`, the `request_started` hook default policy would not get invoked. Add a check to make sure it is invoked. Ticket: OISF#8495.

For non-UDP (so TCP), don't allow `accept:packet` or `drop:packet` as this makes the evaluation of other rule hooks unpredictable. Ticket: OISF#8497.

When there are no rules after prefilter the default policy needs to be invoked.

Fixes: 232276a ("detect: ethernet/arp matching")

For firewall rules, allow multiple actions to be specified in a list accept:flow,pass:flow,alert accept:flow,alert accept:flow,pass:flow It is mandatory to make the first action the primary firewall policy action: accept, drop, reject. Ticket: OISF#8480.

For improved readability.

Previously a `accept:flow` action would act as both a firewall "accept" and a threat detection "pass" for the rest of the flow. This patch changes that. The `accept:flow` action now only accepts the rest of the packets for the firewall ruleset, but does still continue threat detection rule evaluation. Ticket: OISF#8444.

suricata-qa · 2026-05-01T16:35:03Z

Information: QA ran without warnings.

Pipeline = 31153

codecov · 2026-05-01T18:23:22Z

Codecov Report

❌ Patch coverage is 71.56863% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.65%. Comparing base (18f742f) to head (8529d8f).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15306      +/-   ##
==========================================
- Coverage   82.65%   82.65%   -0.01%     
==========================================
  Files         993      993              
  Lines      270997   271136     +139     
==========================================
+ Hits       223985   224099     +114     
- Misses      47012    47037      +25

Flag	Coverage Δ
fuzzcorpus	`61.01% <26.62%> (-0.04%)`	⬇️
livemode	`18.38% <23.56%> (-0.01%)`	⬇️
netns	`22.65% <49.49%> (+0.01%)`	⬆️
pcap	`45.23% <23.63%> (-0.03%)`	⬇️
suricata-verify	`66.39% <64.72%> (+<0.01%)`	⬆️
unittests	`58.56% <20.26%> (-0.03%)`	⬇️

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.

jasonish · 2026-05-05T16:32:53Z

Observation 1: accept:flow can still skip threat detection rules.

firewall.rules:

accept:packet arp:all any any -> any any (sid:1;)
accept:hook tcp:all any any <> any any (sid:1000001;)
accept:hook http1:request_started any any -> any any (sid:80001;)
accept:hook http1:request_line any any -> any any (sid:80002;)
accept:flow http1:request_headers any any -> any any (http.host; content:"testmyids.org"; sid:1000004;)

ips.rules

alert http any any -> any any (msg:"GPL ATTACK_RESPONSE id check returned root"; http.response_body; content:"uid=0|28|root|29|"; sid:2100498; rev:7;)

The rule in ips.rules never alerts, but I believe we expect it to. Alerting on something like the http.host in ips.rules does alert, its alerting on something in a state after http1:request_headers that does not alert, appears to be bypassed.

jasonish · 2026-05-05T16:36:14Z

Should we expect the following to apply threat detection rules as well?

accept:packet tcp:all any any <> any any (sid:3;)

My understanding of ticket 8444 is that we should, but this still appears to skip thread detection, and there is no commit that addresses this case.

jasonish · 2026-05-05T16:54:53Z

A general note on commit firewall: support multi-action statements in rules.

This needs need documentation. And this documentation note is probably out date now:

the action pass is not available in firewall rules due to ambiguity around the existing meaning for threat detection rules.

victorjulien · 2026-05-05T17:26:59Z

Should we expect the following to apply threat detection rules as well?
accept:packet tcp:all any any <> any any (sid:3;)
My understanding of ticket 8444 is that we should, but this still appears to skip thread detection, and there is no commit that addresses this case.

Yeah this is a todo.

victorjulien · 2026-05-05T17:27:15Z

Observation 1: accept:flow can still skip threat detection rules.

firewall.rules:
accept:packet arp:all any any -> any any (sid:1;)
accept:hook tcp:all any any <> any any (sid:1000001;)
accept:hook http1:request_started any any -> any any (sid:80001;)
accept:hook http1:request_line any any -> any any (sid:80002;)
accept:flow http1:request_headers any any -> any any (http.host; content:"testmyids.org"; sid:1000004;)
ips.rules
alert http any any -> any any (msg:"GPL ATTACK_RESPONSE id check returned root"; http.response_body; content:"uid=0|28|root|29|"; sid:2100498; rev:7;)
The rule in ips.rules never alerts, but I believe we expect it to. Alerting on something like the http.host in ips.rules does alert, its alerting on something in a state after http1:request_headers that does not alert, appears to be bypassed.

Do you have this as a SV test?

jasonish · 2026-05-05T17:28:16Z

Observation 1: accept:flow can still skip threat detection rules.
firewall.rules:
accept:packet arp:all any any -> any any (sid:1;)
accept:hook tcp:all any any <> any any (sid:1000001;)
accept:hook http1:request_started any any -> any any (sid:80001;)
accept:hook http1:request_line any any -> any any (sid:80002;)
accept:flow http1:request_headers any any -> any any (http.host; content:"testmyids.org"; sid:1000004;)
ips.rules
alert http any any -> any any (msg:"GPL ATTACK_RESPONSE id check returned root"; http.response_body; content:"uid=0|28|root|29|"; sid:2100498; rev:7;)
The rule in ips.rules never alerts, but I believe we expect it to. Alerting on something like the http.host in ips.rules does alert, its alerting on something in a state after http1:request_headers that does not alert, appears to be bypassed.
Do you have this as a SV test?

No, just a live test. Will convert.

jasonish · 2026-05-05T17:44:58Z

Observation 1: accept:flow can still skip threat detection rules.
firewall.rules:
accept:packet arp:all any any -> any any (sid:1;)
accept:hook tcp:all any any <> any any (sid:1000001;)
accept:hook http1:request_started any any -> any any (sid:80001;)
accept:hook http1:request_line any any -> any any (sid:80002;)
accept:flow http1:request_headers any any -> any any (http.host; content:"testmyids.org"; sid:1000004;)
ips.rules
alert http any any -> any any (msg:"GPL ATTACK_RESPONSE id check returned root"; http.response_body; content:"uid=0|28|root|29|"; sid:2100498; rev:7;)
The rule in ips.rules never alerts, but I believe we expect it to. Alerting on something like the http.host in ips.rules does alert, its alerting on something in a state after http1:request_headers that does not alert, appears to be bypassed.
Do you have this as a SV test?

Last commit of OISF/suricata-verify#3068.

victorjulien · 2026-05-05T21:21:26Z

Fix in #15320, but that will need a bit more work before it gets in a non-draft state.

jufajardini and others added 23 commits May 1, 2026 15:48

detect: opt-in keywords for firewall mode

c726add

- tls.cert_chain_len - datarep - dataset - dns.opcode Part of Ticket OISF#8387

detect/tcp: opt-in tcp.flags for firewall mode

039ef3c

The firewall enabling flag for tcp.flags was being overwritten by another line of code. Related to Ticket OISF#8387

device: add O(1) lookup by id

90a726f

device: start id space at 1

ecc98bd

So a value of 0 means no device.

device: add get id func

51bb9ba

Most code uses an opague type for LiveDevice, so add an id getter.

flow: store livedev by id

c67fd4a

In prep for storing both directions for IPS.

packet: store livedev by id

3d7411d

af-packet: shrink AFPPeer struct by better layout

8442ecf

packet: track outgoing livedev id

4a3e1d0

reject/respond: use dst livedev for rejectdst

f06b1f4

Update ctx caching to take direction into account.

detect/firewall: clean up flow control

c6e12eb

Use an enum for the firewall related flow control, to improve readability of the firewall inspection logic.

firewall: fix hooks getting skipped in some rulesets

74704c5

If a ruleset would use `dns:request_complete` but not have a rule for `dns:request_started`, the `request_started` hook default policy would not get invoked. Add a check to make sure it is invoked. Ticket: OISF#8495.

detect: minor code cleanup

355506e

detect: clean up of last_tx check

00a8a4a

detect: clean up firewall rule match handling

3eb3c88

detect/firewall: add single call for applying app default policy

6971dd6

firewall: limit action scope packet for app-hooks

a07449d

For non-UDP (so TCP), don't allow `accept:packet` or `drop:packet` as this makes the evaluation of other rule hooks unpredictable. Ticket: OISF#8497.

detect/firewall: apply default policy in no rules case

6e6d94b

When there are no rules after prefilter the default policy needs to be invoked.

detect: suppress noisy debug messages

85eca17

Fixes: 232276a ("detect: ethernet/arp matching")

detect/firewall: rename flow control variable

bf5cece

For improved readability.

victorjulien requested review from a team and jasonish as code owners May 1, 2026 13:56

victorjulien mentioned this pull request May 1, 2026

Fw updates/v18 #15300

Closed

jasonish mentioned this pull request May 5, 2026

fw accept flow issue/v1 OISF/suricata-verify#3068

Closed

victorjulien mentioned this pull request May 5, 2026

Fw updates/v21 #15320

Closed

victorjulien closed this May 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fw updates/v19#15306

Fw updates/v19#15306
victorjulien wants to merge 23 commits into
OISF:mainfrom
victorjulien:fw-updates/v19

victorjulien commented May 1, 2026

Uh oh!

suricata-qa commented May 1, 2026

Uh oh!

codecov Bot commented May 1, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

victorjulien commented May 5, 2026

Uh oh!

victorjulien commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

victorjulien commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

victorjulien commented May 1, 2026

Uh oh!

suricata-qa commented May 1, 2026

Uh oh!

codecov Bot commented May 1, 2026

Codecov Report

Uh oh!

jasonish commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

victorjulien commented May 5, 2026

Uh oh!

victorjulien commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

jasonish commented May 5, 2026

Uh oh!

victorjulien commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants