Fw updates/v7 by victorjulien · Pull Request #15108 · OISF/suricata

victorjulien · 2026-03-26T12:07:15Z

#15079 rebased with review comments addressed. Also added a NFQ based fw test that doesn't need the arp support, so that can be backported more easily.

Allows for quickly checking if sig operates on frames during parsing.

Make `Signature::proto` an optional member, meaning that if it is NULL we can skip the check. This can be done for `alert ip`, as no check is needed, and for `alert tcp` and `alert udp` as having a rule in a sgh for those means that the protocol matches. Some exceptions are rules that require: - ipv4/ipv6 specific matching - frames, due to sharing prefilter between tcp and udp - ip-only rules, due to those not being per sgh

Support `alert ether` for matching all ethernet packets. Add `alert arp` for matching ARP packets. Ticket: OISF#8313.

Sticky buffer to inspect the ethernet header. Example rule: alert ether any any -> any any ( \ ether.hdr; content:"|08 06|"; offset:12; depth:2; \ sid:1;) Ticket: OISF#8327.

To indicate it's not just like `alert ip`.

`alert pkthdr` was initially just an alias for `alert ip`, as that was really just a way of stating that "any" should be matched. However with the Ethernet matching in place, it no long makes sense to treat `alert ip` as "any". Since `pkthdr` is used to match on decoder events, also for packets that completely failed to parse, it should no longer be treated as `alert ip` but rather as it's own distinct logic.

Additionally, use bsize, pcre and urilen. Ticket: OISF#8204. Ticket: OISF#8397.

Ticket: OISF#8397.

Cannot be combined with --firewall-rules-exclusive

codecov · 2026-03-26T13:04:58Z

Codecov Report

❌ Patch coverage is 95.20202% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.61%. Comparing base (dce2dee) to head (f7eb068).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15108      +/-   ##
==========================================
- Coverage   82.63%   82.61%   -0.03%     
==========================================
  Files         990      992       +2     
  Lines      271599   271754     +155     
==========================================
+ Hits       224429   224497      +68     
- Misses      47170    47257      +87

Flag	Coverage Δ
fuzzcorpus	`61.05% <71.77%> (-0.02%)`	⬇️
livemode	`18.39% <30.24%> (+<0.01%)`	⬆️
netns	`22.59% <46.37%> (+4.19%)`	⬆️
pcap	`45.28% <36.69%> (-0.01%)`	⬇️
suricata-verify	`66.17% <74.59%> (-0.01%)`	⬇️
unittests	`58.84% <77.27%> (-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-03-26T14:30:03Z

Information: QA ran without warnings.

Pipeline = 30558

catenacyber

Thanks for the work,

CI : ✅
Code : looking
Commits segmentation : looks cool
Commit messages : nice
Git ID set : looks fine for me
CLA : you already contributed :-p
Doc update : seems nice
Redmine ticket(s) : ok
Rustfmt : no rust
Tests : ⚠️ there is commit enable pcre and urilen for firewall mode but no SV test about it and I heard we absolutely must add SV tests before marking a keyword supported for firewall mode
Dependencies added: none

catenacyber · 2026-03-29T19:09:21Z

-    /** addresses, ports and proto this sig matches on */
-    DetectProto proto;
+    /** rule protocol: can be NULL if the check can be skipped */
+    DetectProto *proto;


Is the memory gain really worth it ?

I have the feeling it gains like 1 Mbyte on a ruleset with 50k signatures...

And I think you feel this is right and I trust you, but the commit message may bring some more numbers...

It was worth it at the time, but it's been over 2 years.

catenacyber · 2026-03-29T19:12:54Z

+static bool SigValidateEthernet(const Signature *s)
+{
+    if (s->init_data->proto.flags & DETECT_PROTO_ETHERNET) {
+        if ((s->flags & (SIG_FLAG_SP_ANY | SIG_FLAG_DP_ANY)) !=


Should we not also test that ip addresses are any ?

And reject alert arp 192.168.1.1 any -> ...

catenacyber · 2026-03-29T19:17:50Z

+    { "ip", 0, 0, DETECT_PROTO_ANY, 0 },
+    { "pkthdr", 0, 0, DETECT_PROTO_ANY, 0 },
+    { "ether", 0, 0, DETECT_PROTO_ETHERNET, 0 },
+    { "arp", 0, 0, DETECT_PROTO_ETHERNET, ETHERNET_TYPE_ARP },


This arp and ether should be in the docs doc/userguide/rules/intro.rst section Protocol

catenacyber · 2026-03-29T19:19:35Z

+    InspectionBuffer *buffer = InspectionBufferGet(det_ctx, list_id);
+    if (buffer->inspect == NULL) {
+        if (!PacketIsEthernet(p)) {
+            // DETECT_PROTO_ETHERNET does not prefilter


Should it ?
Is there not a mask somewhere ?

For now I'm mostly caring about correctness than we can express what we need. Such optimizations can follow later.

catenacyber · 2026-03-29T19:24:43Z

+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Traditionally the rule language was only about matching on IP packets. For packets
+that caused decoder events in the layers before IP a special protocol `pkthdr` was


the layers before IP

We also have like `msg:"SURICATA ICMPv4 packet too small"; decode-event:icmpv4.pkt_too_small``

So, this seems more than "the layers before IP", right ?

A bad ICMP event should use alert ip as we require a valid IP before checking ICMP. alert pkthdr is really meant for when alert ip doesn't make sense as there is no or no valid IP.

catenacyber · 2026-03-29T19:28:40Z

+
+    accept:packet arp:all any any -> any any (sid:200;)
+
+Other ethernet types can be accepted by using generic ethernet rules, with the ``ether.hdr`` keyword.


What happens for a packet which has ethernet/vlan/arp ?

Or a packet that is vxlan encapsulating ARP ?

Good question. The vlan/arp case currently doesn't work, as it is essentially just checking the ether_type. Looking into it.

For VXLAN we split out the encapsulated packet, so that should work, but it didn't. Fixing as well.

victorjulien · 2026-03-30T16:04:16Z

Comments addressed in #15122

victorjulien added 13 commits March 26, 2026 09:38

detect/frame: set frame flag during init

637dbc0

Allows for quickly checking if sig operates on frames during parsing.

detect: ethernet/arp matching

1c2bf05

Support `alert ether` for matching all ethernet packets. Add `alert arp` for matching ARP packets. Ticket: OISF#8313.

qa: add script to test firewall bridge mode

b519825

detect: add ether.hdr keyword

be4d72f

Sticky buffer to inspect the ethernet header. Example rule: alert ether any any -> any any ( \ ether.hdr; content:"|08 06|"; offset:12; depth:2; \ sid:1;) Ticket: OISF#8327.

detect: add track pkthdr as any l2 protocol

b4f3a85

To indicate it's not just like `alert ip`.

doc/userguide: improve pkthdr docs

173a064

qa: add rule reload step to firewall test

622d23f

Additionally, use bsize, pcre and urilen. Ticket: OISF#8204. Ticket: OISF#8397.

detect: enable pcre and urilen for firewall mode

3ccaf9f

Ticket: OISF#8397.

qa: add nfq based firewall test with live reload

36d745b

doc/userguide: document L2 firewall handling of ARP

38ff319

doc/userguide: add note on rule reloads

f7eb068

Cannot be combined with --firewall-rules-exclusive

victorjulien requested review from a team, jasonish and jufajardini as code owners March 26, 2026 12:07

victorjulien mentioned this pull request Mar 26, 2026

Fw updates/v6 #15079

Closed

jufajardini reviewed Mar 27, 2026

View reviewed changes

Comment thread doc/userguide/firewall/firewall-design.rst

victorjulien marked this pull request as draft March 27, 2026 12:54

catenacyber reviewed Mar 29, 2026

View reviewed changes

victorjulien mentioned this pull request Mar 30, 2026

Fw updates/v9 #15122

Closed

victorjulien closed this Mar 30, 2026


		accept:packet arp:all any any -> any any (sid:200;)

		Other ethernet types can be accepted by using generic ethernet rules, with the ``ether.hdr`` keyword.

Conversation

victorjulien commented Mar 26, 2026

Uh oh!

codecov Bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

suricata-qa commented Mar 26, 2026

Uh oh!

Uh oh!

catenacyber 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!

catenacyber Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

victorjulien commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Mar 26, 2026 •

edited

Loading

catenacyber Mar 29, 2026 •

edited

Loading