detect: allow bypass keyword for fw rules in firewall mode (only) - v5 - #16077
Closed
jufajardini wants to merge 15 commits into
Closed
detect: allow bypass keyword for fw rules in firewall mode (only) - v5#16077jufajardini wants to merge 15 commits into
jufajardini wants to merge 15 commits into
Conversation
If the point is to check the docs for added changes, it makes sense to that used binary is the one that comes with the doc changes. Thus, try using the local binary first, before falling back to usr/bin installed Suricata.
Previously, a rule that had firewall-only syntax or keywords would fail the script check. Since we can't guarantee that a firewall rule will look different than a detection one, run rule examples against both scenarios before failing them.
In case a keyword should work in firewall mode, with firewall rules only. The engine errors out if threat detection rules use the given keyword. Part of Ticket OISF#8459
The bypass keyword should work in firewall mode, with firewall rules, only. The engine errors out if threat detection rules use said keyword. Ticket OISF#8459
If we add more firewall-related rule options, we can keep this opaque to SigParseOptions. Part of Ticket OISF#8459
This allows banning variations of `action`:`scope` for specific keywords. Mostly having firewall rules in mind. Done by introducing several SIGMATCH flags, to cover: actions: - config - drop - reject action scopes: - packet - tx - hook `accept` and `flow` were left out as they would not be used for the work at hand. Part of Ticket OISF#8459
A firewall rule only accepts the `bypass` keyword with the combination of `accept:flow`. Thus, ban: `drop`, `reject`, `config`, `hook`, `tx` and `packet` from firewall usage for this keyword. Part of Ticket OISF#8459
Especially related to firewall mode. As part of Ticket OISF#8459
If a firewall rule sets a flow to be bypassed, the triggering packet could still be inspected by a threat detection rule with a drop. Avoid that the `accept` from the firewall rule would still allow a TD `drop` to be applied to the first packet. This also implies that the stats for accept in such cases will now differ between firewall and IPS, as the firewall accepted+bypassed packet is never seen by ips (so can't be accepted). Related to Ticket OISF#8459
A packet bypassed by the firewall can't lead to ips stats counters increments. For a accept+bypass from the firewall, this implies that the stats for accept in such cases will now differ between firewall and IPS, as the firewall accepted+bypassed packet is never seen by ips (so can't be counted as accepted). Part of Ticket OISF#8459
Part of Ticket OISF#8459
Part of Ticket OISF#8459
Related to Ticket OISF#8459
As a firewall rule skips the TD branch during PacketAlertQueue finalizing, we must account for supressed alerts elsewhere, for firewall rules.
PacketAlertHandle returns 2 if the alert is to be suppressed by threshold, but actions should be applied. But the FirewallRule check was adding rules to the alert queue if results were > 0.
jufajardini
requested review from
a team,
jasonish and
victorjulien
as code owners
August 21, 2026 23:46
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16077 +/- ##
==========================================
- Coverage 83.04% 83.02% -0.02%
==========================================
Files 1004 1004
Lines 277359 277460 +101
==========================================
+ Hits 230322 230368 +46
- Misses 47037 47092 +55
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 33232 |
Contributor
Author
|
Must fix CI docs failure, and rebase (so I don't need the check-docs commits) |
Contributor
Author
|
Replaced by: #16120 |
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
2 participants
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.
Previous PR: #16063
Describe changes:
Probably need tickets:
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/8459
SV_BRANCH=OISF/suricata-verify#3299