firewall: add default-policy to policy config v3 by lukashino · Pull Request #15950 · OISF/suricata

lukashino · 2026-07-29T09:48:56Z

Follow-up of #15944

Add a default-policy setting 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-filter becomes packet.filter and <proto> becomes app.<proto>.
Action scopes are now validated against the hook they reach. A global accept:tx arriving at a packet hook is a startup error instead of being applied silently.

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

Describe changes:
v3:

commit message update

v1/v2:

new suricata.yaml policy config structure
policy hook validation and hints
multi-layer default policy for firewall policies

SV_BRANCH=OISF/suricata-verify#3252

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.

AppProtoToString(ALPROTO_HTTP1) returns "http", so an HTTP/1 policy had to be written as `http:` while its rule hooks were already spelled `http1:`. Use the same name in both places. Ticket: 8712

The policy config was a flat map mixing packet hooks and app-layer protocols: `packet-filter` next to `dns`. There was no node that meant "the packet hooks" or "the app-layer hooks", so a setting could not be scoped to one group. Move each group under its own node: packet-filter -> packet.filter packet-pre-flow -> packet.pre-flow packet-pre-stream -> packet.pre-stream <proto>.<hook> -> app.<proto>.<hook> Ticket: 8712

Every hook has a built-in default policy, but expressing anything other than the built-in meant naming each hook explicitly. Add a `default-policy` setting that covers all hooks below it, so unlisted hooks still get a policy. For any hook the most specific setting present wins: app.<proto>.<sub state>.<hook> app.<proto>.<sub state>.default-policy app.<proto>.default-policy app.default-policy default-policy built-in The packet hooks follow the same pattern under `packet`. Resolution moves into ResolveFirewallPolicy(), which walks the candidate paths most-specific-first and stops at the first one that is configured. A path that is present but empty is now a startup error rather than being treated as unset. DoParseAppSubStatePolicy() collapses into DoParseAppPolicy() as a sub state hook only differs by an extra path segment. Path assembly and hook-name normalisation move to helpers now that both are needed in more places. 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, naming the config path and the scopes that would be accepted there. A global `accept:tx` is now a startup error. Ticket: 8712

Copilot

Pull request overview

This PR updates Suricata’s experimental firewall policy configuration to support hierarchical default-policy inheritance and to validate action scopes against the hook class they apply to, while also restructuring the YAML layout to separate packet hooks from app-layer hooks.

Changes:

Introduces multi-level default-policy resolution (most-specific match wins) across global, packet, and app protocol/sub-state policy nodes.
Validates action scopes so incompatible scopes (e.g., accept:tx applied to packet hooks) fail at startup instead of being applied silently.
Updates example configuration and user guide documentation to reflect the new packet.* and app.* policy structure.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
suricata.yaml.in	Updates commented example config to show hierarchical `default-policy` and the new `packet`/`app` nodes.
src/detect.h	Adds a firewall policy “class” enum to support class-specific validation.
src/detect-parse.c	Implements hierarchical policy resolution, scope validation, and new packet/app policy loading logic.
doc/userguide/firewall/firewall-example.rst	Updates the HTTP example config snippet to use `policies.app.http1.*`.
doc/userguide/firewall/firewall-design.rst	Documents the new config structure, precedence rules, and scope-validity constraints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-07-29T10:36:15Z

Codecov Report

❌ Patch coverage is 93.61702% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.99%. Comparing base (8455efd) to head (04309fb).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15950      +/-   ##
==========================================
- Coverage   83.03%   82.99%   -0.04%     
==========================================
  Files        1003     1003              
  Lines      276582   276602      +20     
==========================================
- Hits       229647   229553      -94     
- Misses      46935    47049     +114

Flag	Coverage Δ
fuzzcorpus	`61.60% <0.00%> (+0.03%)`	⬆️
livemode	`18.44% <0.00%> (-0.03%)`	⬇️
netns	`22.89% <62.41%> (-0.03%)`	⬇️
pcap	`45.38% <0.00%> (-0.08%)`	⬇️
suricata-verify	`67.05% <93.61%> (-0.03%)`	⬇️
unittests	`58.46% <0.00%> (-0.02%)`	⬇️

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 · 2026-07-29T11:20:03Z

Information: QA ran without warnings.

Pipeline = 32734

catenacyber · 2026-07-31T11:55:51Z

    }

-    const char *app_name = AppProtoToString(app_proto);
+    const char *app_name = (app_proto == ALPROTO_HTTP1) ? "http1" : AppProtoToString(app_proto);


Maybe time to have another function than AppProtoToString that does this

And use it in DetectRegisterAppLayerHookLists and DetectEngineAppHookToSmlist where we already do this

catenacyber · 2026-07-31T11:57:53Z

-        FatalError("internal error: failed to assemble firewall policy config string");
+    char scope[256];
+    if (sub_state_name != NULL) {
+        char sub[64];


I would like to see how this magic 64 is computed ;-)

jufajardini

Added some minor comments, basically related to documentation.

jufajardini · 2026-07-30T12:29:27Z


 If applayer is available, rules from the following tables apply. The tables for the
-application layer are per app layer protocol and per protocol state. e.g. ``http:request_line``.
+application layer are per app layer protocol and per protocol state. e.g. ``http1:request_line``.


To be confirmed, but if we already have stuff backported where http is an alias to http1, should we keep it that way? (or do we change in 9, and document?)

jufajardini · 2026-07-30T14:01:00Z

+          filter:     ["drop:packet"]
+          pre-flow:   ["accept:hook"]
+          pre-stream: ["accept:hook"]
+        app:


Maybe nitty, but as far as naming goes, I prefer proto instead of app.

jufajardini · 2026-07-30T14:12:59Z

+                return -1;
+            }
+            if (!FirewallScopeValidForClass(tmp.action_scope, pol_class)) {
+                char hint[32];


Minor, but maybe a comment on why 32 here, in case something changes ppl know what to take into account? Or in the fn...

Lukas Sismis added 4 commits July 28, 2026 18:28

detect/firewall: address HTTP/1 policies as http1

5391c44

AppProtoToString(ALPROTO_HTTP1) returns "http", so an HTTP/1 policy had to be written as `http:` while its rule hooks were already spelled `http1:`. Use the same name in both places. Ticket: 8712

Copilot AI review requested due to automatic review settings July 29, 2026 09:48

lukashino requested review from a team, jufajardini and victorjulien as code owners July 29, 2026 09:48

lukashino mentioned this pull request Jul 29, 2026

firewall: add default-policy to policy config v2 #15944

Closed

Copilot started reviewing on behalf of lukashino July 29, 2026 09:49 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

Comment thread src/detect.h

Comment thread src/detect-parse.c

Comment thread src/detect-parse.c

jufajardini mentioned this pull request Jul 30, 2026

firewall: add default-policy to policy config v1 #15942

Closed

catenacyber reviewed Jul 31, 2026

View reviewed changes

jufajardini reviewed Jul 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

firewall: add default-policy to policy config v3 - #15950

firewall: add default-policy to policy config v3#15950
lukashino wants to merge 4 commits into
OISF:mainfrom
lukashino:feat/8712-fw-default-policy-v3

lukashino commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 29, 2026

Uh oh!

suricata-qa commented Jul 29, 2026

Uh oh!

catenacyber Jul 31, 2026

Uh oh!

catenacyber Jul 31, 2026

Uh oh!

jufajardini left a comment

Uh oh!

jufajardini Jul 30, 2026

Uh oh!

jufajardini Jul 30, 2026

Uh oh!

jufajardini Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

lukashino commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 29, 2026

Codecov Report

Uh oh!

suricata-qa commented Jul 29, 2026

Uh oh!

catenacyber Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

catenacyber Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

jufajardini left a comment

Choose a reason for hiding this comment

Uh oh!

jufajardini Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jufajardini Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jufajardini Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants