firewall: add default-policy to policy config v5.2 - #16091
Conversation
Mapping an app-layer progress state to its generic request-/response- hook alias was hardcoded in multiple places. This commit adds a wrapper to unify it to a single function. It returns the config-form (hyphenated) name, or NULL for an intermediate state. Ticket: 8712
Every hook has a built-in default policy, but expressing anything other than the built-in meant naming each hook explicitly. This commit adds a `default-policy` setting that covers all hooks below it. For any hook the most specific setting present wins. DoParseAppSubStatePolicy() collapses into DoParseAppPolicy() as a sub state hook only differs by an extra path segment. Ticket: 8712
Validate the resolved scope against the class of hook it is being applied to and fail at startup if it does not fit. Ticket: 8712
There was a problem hiding this comment.
Pull request overview
This PR extends Suricata’s experimental firewall-mode policy configuration by adding hierarchical default-policy inheritance and validating that action scopes are compatible with the hook class they ultimately apply to (e.g., preventing accept:tx from reaching packet hooks). It also updates documentation/examples to reflect the new configuration structure and semantics.
Changes:
- Add multi-level
default-policyinheritance for firewall hook policies (global → packet/app → protocol → sub-state → hook). - Enforce action-scope compatibility by hook class during policy resolution, turning previously silent misconfigurations into startup errors.
- Refactor/commonize app-hook “generic” hook naming and update docs/examples accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| suricata.yaml.in | Updates the firewall policy configuration example/comments to introduce default-policy. |
| src/detect-parse.h | Exposes a helper to derive generic app hook names for reuse across modules. |
| src/detect-parse.c | Implements policy inheritance chain resolution + scope validation; refactors packet/app policy loading. |
| src/detect-engine-analyzer.c | Reuses the shared generic-hook naming helper for analyzer output. |
| doc/userguide/firewall/firewall-design.rst | Documents the new hierarchical policy structure, precedence rules, and scope validation behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| #policies: | ||
| # default-policy: ["drop:flow"] | ||
| # packet: | ||
| # filter: ["drop:packet"] | ||
| # app: |
There was a problem hiding this comment.
Well this was supposed to just show possibilities. But it is true that we try to comment out the values that are actually default.
Perhaps this could be removed/leave just actual defaults. Related to it but irrelevant in this context, with that we could remove the per-interface-cpu-affinity node.
|
Information: QA ran without warnings. Pipeline = 33262 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16091 +/- ##
==========================================
- Coverage 83.05% 83.04% -0.01%
==========================================
Files 1004 1004
Lines 277359 277366 +7
==========================================
- Hits 230370 230352 -18
- Misses 46989 47014 +25
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Merged in #16099, thanks! |
Follow-up of pre-approved #16069
Add a
default-policysetting that applies to all hooks below it. For any hook the most specific setting present wins.The flat layout had no way to express a default covering only the packet hooks or only the app hooks, so both move under their own node:
packet-filterbecomespacket.filterand<proto>becomesapp.<proto>.Action scopes are now validated against the hook they reach. A global
accept:txarriving at a packet hook is a startup error instead of being applied silently.Link to ticket: https://redmine.openinfosecfoundation.org/issues/8712
Describe changes:
v5.2:
v5.1:
v5:
v4:
v3:
v1/v2:
SV_BRANCH=OISF/suricata-verify#3307
To consider:
As we changed the config structure for default policies, we can consider adding upgrade checks.
But since the firewall mode is experimental in 8, I avoided any conversion checks from the previous versions to keep the code simpler.
Visit https://redmine.openinfosecfoundation.org/issues/8712#note-17 for an updated configuration structure (could be worth it)