Skip to content

Fw updates/v4#14955

Closed
victorjulien wants to merge 8 commits intoOISF:mainfrom
victorjulien:fw-updates/v4
Closed

Fw updates/v4#14955
victorjulien wants to merge 8 commits intoOISF:mainfrom
victorjulien:fw-updates/v4

Conversation

@victorjulien
Copy link
Copy Markdown
Member

Draft to introduce a way to accept arp packets in fw bridge mode.

SV_BRANCH=OISF/suricata-verify#2953

Some notable things:

  • introduces alert ether ... . Not sure about the name, could also be eth or ethernet?
  • limits alert pkthdr to just decoder event rules. Previously it was just an alias for alert ip.

@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 30012

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 79.29% (+0.007%) from 79.283%
when pulling d171405 on victorjulien:fw-updates/v4
into 7e97dfd on OISF:main.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 95.17766% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.58%. Comparing base (1d06103) to head (5d5d9be).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14955      +/-   ##
==========================================
- Coverage   82.60%   82.58%   -0.03%     
==========================================
  Files         990      992       +2     
  Lines      271506   271659     +153     
==========================================
+ Hits       224290   224355      +65     
- Misses      47216    47304      +88     
Flag Coverage Δ
fuzzcorpus 60.99% <71.54%> (-0.01%) ⬇️
livemode 18.40% <29.26%> (-0.04%) ⬇️
netns 22.11% <45.52%> (+3.74%) ⬆️
pcap 45.20% <35.77%> (-0.04%) ⬇️
suricata-verify 66.10% <74.39%> (-0.03%) ⬇️
unittests 58.82% <76.64%> (ø)

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
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 30026

@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 30034

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 ( \
                eth.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.
@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 30102

@catenacyber
Copy link
Copy Markdown
Contributor

Why is this PR a draft ?

@catenacyber
Copy link
Copy Markdown
Contributor

Not sure I get the intention of alert pkthdr, even after looking at https://docs.suricata.io/en/latest/rules/intro.html#protocol : I guess it needs more doc

@catenacyber
Copy link
Copy Markdown
Contributor

Not sure about the name, could also be eth or ethernet?

My 2 cents : short is good

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

Replaced by #15029.

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.

4 participants