Skip to content

fw: validate scope inheritance in policies and rules v2 - #16132

Closed
lukashino wants to merge 2 commits into
OISF:mainfrom
lukashino:bug/8954-action-scope-verify-v2
Closed

fw: validate scope inheritance in policies and rules v2#16132
lukashino wants to merge 2 commits into
OISF:mainfrom
lukashino:bug/8954-action-scope-verify-v2

Conversation

@lukashino

Copy link
Copy Markdown
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/8954

Describe changes:
v2:

  • validate pass only is used next to accept primary action

v1:

  • validate pass only inherits packet and flow scopes

SV_BRANCH=OISF/suricata-verify#3329

Lukas Sismis added 2 commits August 27, 2026 17:45
A secondary action given without an explicit scope inherits the scope of
the primary action, but was never validated against the scopes it
supports itself. `pass` only supports packet and flow scope but
the inheritance was never verified.

Ticket: 8954
Previously, hook sequences in both rules and default policies
allowed `pass` to be combined with incompatible hooks (e.g.
reject or drop).

This commit clamps pass to accept only.

Ticket: 8954

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens Suricata firewall rule action parsing by adding stricter validation around pass usage and how action scopes are inherited across multi-action firewall rules.

Changes:

  • Reject pass as a secondary action unless the rule’s primary action is accept.
  • When pass omits an explicit scope, validate that the inherited scope is limited to packet or flow.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/detect-parse.c
Comment thread src/detect-parse.c
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.05%. Comparing base (7c66954) to head (f32a565).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16132      +/-   ##
==========================================
- Coverage   83.06%   83.05%   -0.01%     
==========================================
  Files        1004     1004              
  Lines      277376   277415      +39     
==========================================
+ Hits       230389   230420      +31     
- Misses      46987    46995       +8     
Flag Coverage Δ
fuzzcorpus 61.49% <12.50%> (-0.01%) ⬇️
livemode 18.42% <12.50%> (-0.01%) ⬇️
netns 22.83% <25.00%> (-0.05%) ⬇️
pcap 45.37% <12.50%> (-0.05%) ⬇️
suricata-verify 67.17% <100.00%> (+0.01%) ⬆️
unittests 58.53% <12.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.

@lukashino
lukashino marked this pull request as draft August 27, 2026 19:19
@suricata-qa

Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 33419

@lukashino

Copy link
Copy Markdown
Contributor Author

Continues in #16138

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