Commit Graph

576 Commits (d4aa40df61ab08670b29391fc6448a9907923eae)

Author SHA1 Message Date
Juliana Fajardini d4aa40df61 detect: allow banning kw from td in firewall mode
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 #9041
3 weeks ago
Juliana Fajardini 3e3dcf4c9f firewall: don't use 'banned' when no fw support
Banned isn't the right messaging here, we decided.
Instead, go with "not supported".
3 weeks ago
Lukas Sismis a329e0a3b9 firewall: reject flow scope for the pre-flow hook
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.
Firewall rules on the pre_flow hook were not validated at all, so `flow`
and `tx` scopes were accepted there as well.

This commit adds extra validation to scope parsing for the pre_flow
hook default policy and validates pre_flow firewall rules against the
same scopes.

Ticket: 8967
3 weeks ago
Philippe Antoine 8461e3f3e7 src: do not include "util-unittest.h" directly
It is included in suricata-common.h

Ticket: 945
3 weeks ago
Victor Julien bf167418bf detect: reject the '<' hook form for non-firewall rules
The '<' lower bound hook form is a firewall policy construct: the rule
covers its own hook and every prior one. For a threat detection rule the
form has no such meaning, and the flag it sets leaked into the firewall
policy walk, where the rule could influence the per-state decisions.

Reject the form at load for rules without SIG_FLAG_FIREWALL.

Ticket: #8944.
4 weeks ago
Victor Julien 2630c6a5d3 detect: log the hook for sub-state firewall rules
Alerts and drops for firewall rules hooked at a sub-state (http2
stream/global) had no firewall.hook field: the logger resolved the state
name without the sub-state, so the lookup found nothing.

Keep the rule's sub-state on the Signature next to app_progress_hook,
set it from the hook and for the default policy signatures, resolve the
name in the sub-state table, and fall back to the plain state name when
that lookup fails.

Ticket: #8944.
4 weeks ago
Victor Julien 5c81ab1e0a ssh: rebuild the state machine as per-direction states
The old ssh state scale was coupled across the two directions: the
terminal finished state was only reached after both sides sent NewKeys,
and the progress accessor clamped every per-direction phase to a
shared value. A rule hooking the old top state therefore meant "the
flow completed", not "this direction completed the handshake", and
there was no room above the terminal state for a true completion
marker.

Replace the scale with per-direction phases:

    banner(0) -> banner_wait_eol(1) -> kex(2) -> session(3)

NewKeys now sets session on the packet's own direction only, banner
completion sets kex, and the progress accessor is identity: each
direction reports its own raw phase. Add the registered completion
state done(4), never assigned during stream parsing: it stays out
of both name tables, so a rule referencing it fails to load, the
hook listing omits it and the firewall policy loader does not
consult it; the progress accessor reports it only on flows
disrupted by stream depth or async reassembly.

The legacy state names (in_progress, banner_done, finished) no
longer resolve through the name table (pre-production - breaking
changes allowed); the name-table unit test pins the rejection.
The old firewall policy state keys are added to the rename hint
table (like the TLS migration), so a config using them fails
loudly with the replacement named instead of degrading to a bare
warning. The non-prefilter engine-name registration now guards
the null id-to-name result (a hook-only rule at the completion
progress previously formatted a null name into the engine
name).

Add unit tests for the state name table (new names, rejected legacy
names, the unhookable completion state, id-to-name) and for the
per-direction session state.

The eve log condition compares the per-direction states with
>= rather than ==: a direction at session has necessarily passed
kex, and a single delivery can take a direction
banner -> kex -> session, so a final {session, kex}
pair must still admit the log (done is never assigned, so >=
changes only the kex branch).

Ticket: #9046.
4 weeks ago
Victor Julien 1d77a2b910 detect/parse: report unknown firewall policy keys
The app-layer firewall policy config resolves per-state keys
against the current state name table only, so keys renamed or
removed in this release are silently ignored: the state falls back
to the implicit default policy (drop in firewall mode) with
nothing logged. A suricata upgrade without updating
firewall.policies.app.tls would then drop traffic with no
diagnostic.

Walk the policy config at load time and compare every per-state
key against the state names of the protocol section it sits in,
mirroring the enumeration the policy loader consults (the plain
state tables and the sub-state sections). An unknown key logs a
warning with the config path. A key matching a state name renamed
in this release (a deliberately name-based, per-protocol hint
list) escalates to an error with the new name suggested, and the
load aborts under init-failure-fatal.

Ticket: #9047.
4 weeks ago
Philippe Antoine 429cc7d07c detect: warn on missing closing parenthesis
Ticket: 8579
4 weeks ago
Lukas Sismis 17e2e7f892 fw: disallow config to be used as a default policy
Ticket: 8954
2 months ago
Lukas Sismis d6521adf4e fw: allow pass as a secondary action only for accept
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
2 months ago
Lukas Sismis 4bafe61ecf fw: validate scope inheritance in policies and rules
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
2 months ago
Lukas Sismis 092ae272e2 firewall: validate action scope against the hook class
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
2 months ago
Lukas Sismis 0aaa80d4f3 firewall: add default-policy to policy config
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
2 months ago
Lukas Sismis 1aa0259876 firewall: share the generic app hook name helper
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
2 months ago
Shivani Bhardwaj 63daa73032 detect/ipv6: classify ipv6 ranges as like_ip_only
IPOnly Engine does not support parsing and recording IPv6 ranges. So,
passing a range of ipv6 there just results in non-fatal errors in
parsing the IP addresses which means they fail to be added to the radix
tree struct.
Re-classify the ipv6 ranges as like_ip_only so they fall out of the
IPOnly engine and are processed per packet.

Ticket: 8847
2 months ago
Lukas Sismis b82b4496af detect/firewall: group policies under packet and app nodes
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
2 months ago
Lukas Sismis 04c9fd9e22 detect/firewall: address HTTP/1 policies as http1
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
2 months ago
Lukas Sismis 3adbd677e8 app-layer: query raw protocol names
To avoid back-and-forth of http1 conversion
a second query function was added to support
existing use cases.

This change will be handy for the upcoming
default-policy for firewall settings
2 months ago
Victor Julien 8a5e8c63b2 detect: strict validation of buffers
Make sure that every buffer in a signature is actually used in the
setup of the engines. Reject sigs that are locked to a certain substate
and use buffers that require another.
4 months ago
Victor Julien a824d97d71 detect: use sub state to select inspect engines
When building the per signature app_inspect list, only add engines
that match the sub state specified in the hook.
4 months ago
Victor Julien f8f28c6f6f detect: sync prefilter and app inspect alproto logic
In firewall mode the alproto logic when building prefilter and rule
engines is strict, but was out of sync between prefilter and rule app
inspect engines.

For SIGNATURE_HOOK_TYPE_APP rules the logic is strict, with an exception
for HTTP/2 and DOH2. DOH2 is not a full protocol implementation, but
rather HTTP/2 with a different alproto.

Fixes: d64954a873 ("detect: don't register unrelated inspect engines")
4 months ago
Victor Julien 7517ba9f98 detect: give clear errors for http2 w/o substate
http2 states were not yet supported even if the built-in states could
already work. Clearly error out on a hook w/o substate.
4 months ago
Victor Julien 2511070a58 detect/firewall: support lte mode for substate 4 months ago
Victor Julien 17a3e0d44f app-layer: use macro for progress ceiling 4 months ago
Victor Julien 3b6b381f90 detect/firewall: add default alert signature to policy object
Now that there is a policy hash, make the alert signature object a
member of that to avoid another hash table lookup.
4 months ago
Victor Julien bffea309db detect/firewall: implement app policy as hash table
With substate support the fixed table approach for policies was no
longer a good fit, so convert to a hash table. The policies are stored
per alproto, sub_state, progress and direction.
4 months ago
Victor Julien eb87137471 detect/parse: allow bigger protocol/hook specifications 4 months ago
Victor Julien 956030452f detect/parse: don't setup sub states if protocol doesn't support it 4 months ago
Victor Julien 3ef0d2ce16 detect/parse: parse sub state hooks
Register a generic list for each sub state / progress combo.
4 months ago
Victor Julien 116a060092 detect/firewall: policy for substates
For the most part hard coded for HTTP/2 for now.
4 months ago
Victor Julien c943de4fac detect/parse: initial http2 substate support
Hard coded for now.
4 months ago
Victor Julien 59e2c5a40c detect/parse: initial substate support 4 months ago
Shivani Bhardwaj 06db2e2698 detect/flowbits: add max usage per signature limit
Add a limit on the maximum number of times flowbits keyword that can be used in a
signature.

Task 8596
4 months ago
Victor Julien 7fe8f63a89 detect/parse: tighten hook parsing
Don't allow trailing :
4 months ago
Philippe Antoine 89dbce1b46 detect: convert tx_progress to uint8_t 4 months ago
Victor Julien d64954a873 detect: don't register unrelated inspect engines
For rules that specify an explicit app-layer hook,
e.g. http1:request_headers, don't register inspect engines for
other protocols like HTTP/2. These have their own progress tracking,
so should be excluded from these rules.
5 months ago
Denis Balashov 4d4c6227e1 detect/parse: assert HashListTableLookup results in duplicate sig check
Add DEBUG_VALIDATE_BUG_ON() at four sites in DetectEngineSignatureIsDuplicate()
where HashListTableLookup() is assumed to return non-NULL. Documents the
invariant that every Signature in sig_list has a dup_sig_hash_table entry
and catches violations in debug builds.

Flagged by Svace static analyzer.

Ticket: 8635
5 months ago
Juliana Fajardini 6e71ff3013 detect/parse: allow banning keywords with fw mode
Related to
Ticket #8551
5 months ago
Juliana Fajardini 04a7674512 detect/parse: allow banning keywords from fw rules
Related to
Ticket #8551
5 months ago
Victor Julien 57b16c964e detect/firewall: drop in fw mode does not include alert
In TD mode the drop action also includes alert.

In firewall mode it should not to stay in line with accept.

Ticket: #8601.
5 months ago
Victor Julien dc4c22e906 detect/firewall: support alert in packet default policy
Support `alert` as a secondary action in packet firewall policies.

To implement this a Signature object is created per policy that uses
alert, and this is stored in a array table. When the policy is applied
the signature is looked up and used in the PacketAlert.

Ticket: #8566.
5 months ago
Victor Julien 2d4f1968b8 detect/firewall: support alert in default app policy
Support `alert` as a secondary action in app-layer firewall policies.

To implement this a Signature object is created per policy that uses
alert, and this is stored in a hash table. When the policy is applied
the signature is looked up and used in the PacketAlert.

Ticket: #8566.
5 months ago
Victor Julien 71a6a9a746 detect/firewall: limit auto accept notation
Limit to accept:flow, accept:tx and accept:hook.
5 months ago
Victor Julien 651afba883 detect/firewall: implement initial state range support
Allow a single rule to accept a hook and the hooks prior to it.

Example:

        accept:flow tls:<client_hello_done ... \
                tls.sni; content:"suricata.io"; endswith;

This will evaluate the SNI at the client_hello_done hook, but will
act as if there is a `accept:hook tls:client_in_progress ...` as well.

Implementation is currently specific to this `<` operator. During
parsing the sig gets flagged for this case. During setup this has 3 main
effects:

1. prefilter is disabled as we need to eval this right at the first
   state (0)
2. for state 0 a non-PF "prefilter" engine is setup to make sure the
   rule is flagged for evaluation
3. In the Signature::app_inspect list a dummy inspect engine is
   registered per state before Signature::app_progress_hook

The matching logic is building on the stateful rule handling. The
stateful rule handling can now tell the inspection loop that a partial
match occured. For this rule type the partial match will act as a match
with action accept:hook.

Next app updates will then use the continue detection logic to continue
the stateful match. When that fully matches, the final actions are
applied, like accept:flow or accept:tx.

Ticket: #8472.
5 months ago
Juliana Fajardini 6933602050 detect/parse: convert Notice Log into Debug 5 months ago
Victor Julien 7134592fea detect/firewall: configurable default policies
Allow configurable policies, including accept. For app-layer this
requires looping all available hooks to apply the policies.

Support configurable policies for packet-filter, pre-stream, pre-flow.

If there are no rules there is also no rule group (sgh). Make sure
the app hooks policies are correctly handled in this case by allowing
a NULL sgh to be handled as well.

For tx rule match actually apply drop directly. Previously this was
always handled by the default drop:flow policy.

Ticket: #7701.
6 months ago
Victor Julien e76728a536 firewall: support multi-action statements in rules
For firewall rules, allow multiple actions to be specified in a list

        accept:flow,pass:flow,alert
        accept:flow,alert
        accept:flow,pass:flow

It is mandatory to make the first action the primary firewall policy
action: accept, drop, reject.

Ticket: #8480.
6 months ago
Victor Julien 33b3793372 firewall: limit action scope packet for app-hooks
For non-UDP (so TCP), don't allow `accept:packet` or `drop:packet` as
this makes the evaluation of other rule hooks unpredictable.

Ticket: #8497.
6 months ago
Victor Julien 6fbd7483d6 detect/parse: limit pkthdr proto to decoder event rules
`alert pkthdr` was initially just an alias for `alert ip`, as that was
really just a way of stating that "any" should be matched. However with
the Ethernet matching in place, it no long makes sense to treat `alert
ip` as "any". Since `pkthdr` is used to match on decoder events, also
for packets that completely failed to parse, it should no longer be
treated as `alert ip` but rather as it's own distinct logic.
7 months ago