Skip to content

Fw updates/v6#15079

Closed
victorjulien wants to merge 10 commits intoOISF:mainfrom
victorjulien:fw-updates/v6
Closed

Fw updates/v6#15079
victorjulien wants to merge 10 commits intoOISF:mainfrom
victorjulien:fw-updates/v6

Conversation

@victorjulien
Copy link
Copy Markdown
Member

SV_BRANCH=OISF/suricata-verify#2953

#15029 rebased, with eth.hdr renamed to ether.hdr.

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.
@victorjulien victorjulien requested review from a team, jasonish and jufajardini as code owners March 21, 2026 21:05
@victorjulien victorjulien mentioned this pull request Mar 21, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 95.20202% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.60%. Comparing base (6587e36) to head (fd54696).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main   #15079    +/-   ##
========================================
  Coverage   82.59%   82.60%            
========================================
  Files         990      992     +2     
  Lines      271761   271916   +155     
========================================
+ Hits       224465   224609   +144     
- Misses      47296    47307    +11     
Flag Coverage Δ
fuzzcorpus 61.02% <71.77%> (+<0.01%) ⬆️
livemode 18.36% <30.24%> (-0.01%) ⬇️
netns 22.59% <46.37%> (+4.21%) ⬆️
pcap 45.23% <36.69%> (-0.03%) ⬇️
suricata-verify 66.14% <74.59%> (-0.01%) ⬇️
unittests 58.83% <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.

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 79.321% (+0.006%) from 79.315%
when pulling fd54696 on victorjulien:fw-updates/v6
into 6587e36 on OISF:main.

@suricata-qa
Copy link
Copy Markdown

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.flow.ftp-data 703 681 96.87%

Pipeline = 30462

Copy link
Copy Markdown
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

Looks good, from my perspective.
The pkthdr info made me wonder if something on this alias aspect should be added to the 80x docs, but not sure.

Comment on lines +146 to +149

Up until Suricata 8 this protocol was an alias for `alert ip`, but in Suricata 9 it
is only to be used in decoder event rules.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL

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.

Functionally seems OK, I tested the arp passing on an AFP firewall.

This should probably get documented sooner than later though.

One observation: "alert ether" rules.. They don't have much context, not even the ethernet address. Would it be odd to enable ethernet header logging automatically on such rules?

@victorjulien
Copy link
Copy Markdown
Member Author

One observation: "alert ether" rules.. They don't have much context, not even the ethernet address. Would it be odd to enable ethernet header logging automatically on such rules?

Was thinking about doing it more generically: if we're in bridge mode (IPS/fw), enable it by default.

@jasonish
Copy link
Copy Markdown
Member

One observation: "alert ether" rules.. They don't have much context, not even the ethernet address. Would it be odd to enable ethernet header logging automatically on such rules?

Was thinking about doing it more generically: if we're in bridge mode (IPS/fw), enable it by default.

Yes, I think that makes sense. At least its how I would like it.

@victorjulien victorjulien mentioned this pull request Mar 26, 2026
@victorjulien
Copy link
Copy Markdown
Member Author

One observation: "alert ether" rules.. They don't have much context, not even the ethernet address. Would it be odd to enable ethernet header logging automatically on such rules?

Was thinking about doing it more generically: if we're in bridge mode (IPS/fw), enable it by default.

Yes, I think that makes sense. At least its how I would like it.

https://redmine.openinfosecfoundation.org/issues/8407

@victorjulien
Copy link
Copy Markdown
Member Author

replaced by #15108

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.

5 participants