fw: tighten pre-flow hook scopes and apply flow scope to packet drop v1 - #16129
Open
lukashino wants to merge 2 commits into
Open
fw: tighten pre-flow hook scopes and apply flow scope to packet drop v1#16129lukashino wants to merge 2 commits into
lukashino wants to merge 2 commits into
Conversation
added 2 commits
August 27, 2026 13:39
The packet default policy runtime only applied the action scope for `accept`. For `drop:flow` and `reject:flow` it dropped the packet and left the flow untouched, so the flow scope had no eiffect. Ticket: 8969
Action scopes for packet hook default policies were validated per hook class, so `flow` was accepted for every packet hook. The pre_flow hook is a special case as it is evaluated before Suricata obtains a flow record. This commit adds extra validation to scope parsing for the pre_flow hook. Ticket: 8967
There was a problem hiding this comment.
Pull request overview
This PR refines Suricata’s firewall default-policy handling by (1) tightening which action scopes are allowed for the pre-flow packet hook (since a Flow may not exist yet) and (2) ensuring flow-scoped default packet drops persist by marking the Flow when the default policy is hit.
Changes:
- Add a dedicated policy-class for packet pre-flow hooks to disallow
flowaction scope during policy resolution. - When the default packet policy is
drop:flowand a flow exists, setFLOW_ACTION_DROP | FLOW_ACTION_BY_FIREWALLon the flow so subsequent packets are dropped at flow-level. - Route pre-flow packet policy loading through the new policy-class so invalid scopes are rejected with appropriate hints.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/detect.c | Applies flow-scoped default packet drops to the flow by setting flow drop flags when applicable. |
| src/detect-parse.c | Introduces a pre-flow packet policy class and scope set; uses it when resolving the pre-flow default policy. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
lukashino
changed the title
|
Information: QA ran without warnings. Pipeline = 33414 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16129 +/- ##
==========================================
- Coverage 83.06% 83.04% -0.02%
==========================================
Files 1004 1004
Lines 277376 277421 +45
==========================================
- Hits 230389 230371 -18
- Misses 46987 47050 +63
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The commits cover these Redmine tickets:
Describe changes:
SV_BRANCH=OISF/suricata-verify#3327