Skip to content

Fw updates/v9#15122

Closed
victorjulien wants to merge 15 commits intoOISF:mainfrom
victorjulien:fw-updates/v9
Closed

Fw updates/v9#15122
victorjulien wants to merge 15 commits intoOISF:mainfrom
victorjulien:fw-updates/v9

Conversation

@victorjulien
Copy link
Copy Markdown
Member

SV_BRANCH=OISF/suricata-verify#2953

#15108 with review comments addressed and more tests in the SV test.

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.
Cannot be combined with --firewall-rules-exclusive
@victorjulien victorjulien requested review from a team, jasonish and jufajardini as code owners March 30, 2026 15:16
@victorjulien victorjulien mentioned this pull request Mar 30, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 95.00000% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.68%. Comparing base (dce2dee) to head (35315a5).
⚠️ Report is 32 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15122      +/-   ##
==========================================
+ Coverage   82.63%   82.68%   +0.05%     
==========================================
  Files         990      992       +2     
  Lines      271599   271852     +253     
==========================================
+ Hits       224429   224777     +348     
+ Misses      47170    47075      -95     
Flag Coverage Δ
fuzzcorpus 61.11% <74.20%> (+0.04%) ⬆️
livemode 18.35% <29.36%> (-0.04%) ⬇️
netns 22.61% <46.42%> (+4.21%) ⬆️
pcap 45.25% <39.28%> (-0.04%) ⬇️
suricata-verify 66.27% <74.60%> (+0.09%) ⬆️
unittests 58.85% <78.50%> (+<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.

@jasonish
Copy link
Copy Markdown
Member

2 observations:

  • accept:packet arp:all will accept IP addresses, should those fail validation?
  • accept:packet ether:all will accept IP addresses, and they appear to work, but not accept ports. ip:all will accept ports, even though IP can have protocols with no concept of ports. I'm not sure I see ether as being much different

@victorjulien
Copy link
Copy Markdown
Member Author

2 observations:

* `accept:packet arp:all` will accept IP addresses, should those fail validation?

* `accept:packet ether:all` will accept IP addresses, and they appear to work, but not accept ports. `ip:all` will accept ports, even though IP can have protocols with no concept of ports. I'm not sure I see `ether` as being much different

Yeah this will need some further work. I suppose ultimately we want to allow MAC addresses to be used in the source/dest fields of these rules.

@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 30600

Copy link
Copy Markdown
Member

@jasonish jasonish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes my AFP firewall setup; needed to move forward. Any thoughts on backporting? Some of the commits clearly mention a ticket marked for backporting, but not all reference a ticket.

@victorjulien victorjulien added this to the 9.0 milestone Mar 31, 2026
@victorjulien
Copy link
Copy Markdown
Member Author

Merged in #15127, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants