Commit Graph

1160 Commits (6fe3a3dfc3f9fbb8ab955ff931865809d8a88400)

Author SHA1 Message Date
Lukas Sismis 6fe3a3dfc3 firewall: apply flow scope to the default packet policy drop
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 effect.

Ticket: 8969
3 weeks ago
Victor Julien e6eac91974 detect: unify the transaction end state helper
AppLayerParserGetTxEndState() replaces the remaining copies of the
transaction end state computation in the firewall tx setup, the
app-layer-state and app-layer-event keywords and the completion check.
One tolerated a missing tx data pointer, the others asserted or
dereferenced it, so they could disagree.

Move DetectTxCompleted() above its first use while there, so it no
longer needs a forward declaration, and shorten its comment.

Ticket: #8944.
3 weeks ago
Victor Julien 0e297d26ff detect: honor the P+1 eof rule for generic hook list engines
DetectEngineInspectGenericList returned NO_MATCH for every keyword miss,
also when the tx had moved past the phase the engine is registered at.
Unlike the buffer engines it never produced a terminal no match, so a
firewall LTE rule using such a keyword (tls.version on a hook) stayed
provisional and was accepted for the rest of the session.

Make a miss terminal at P + 1. This engine also backs the lua
on-request/on-response hooks and the tls_cert_*, dnp3/dnp3_ind and
ftpdata/ftp mode keywords, so a plain IDS rule using them keeps its
verdict once the tx has moved past the registered phase.

Export the parser's end state computation as AppLayerParserGetTxEndState()
here, so the version keyword can use it; the remaining call sites are
converted later.

Ticket: #8944.
3 weeks ago
Victor Julien 9d4d448a90 detect/firewall: fix later LTE rule confusing policy checks
A non-matching rule followed by an LTE rule (<hook) with an overlapping
hook can be decided by the wrong default policy. E.g.:

        drop:flow tls:client_hello .. tls.sni; content:"match" \
                content:"no-match";
        accept:flow tls:<client_hello .. tls.sni; content:"match"

Here the `tls:client_started` state needs to be accepted by the second
rule, which acts as `accept:hook` on that state.

A failing `<` rule defers the per-state defaults of the states it
promised to cover; the last pending covering rule decides them, applying
its own hook default and sweeping forward from its own hook. There is no
backward sweep, so the states below that hook get no default of their
own.

The pending LTE coverage accounting was the problem: it counted
candidates before their header predicates were checked, compared against
a max that included the rule being evaluated, and kept rules that
resolved during the pass. Count only rules that pass the header checks,
keep per-hook coverage counts in per-thread scratch, drop a rule from
the counts once it resolves at or past its hook, and let the hook checks
consult the counts instead of the raw LTE flag.

Also fixes two latent issues in the same walk: the candidate list is now
sorted whenever the tx state store adds entries - the old condition
skipped the sort when the prefilter contributed nothing, so the
last-for-progress peek could see an unsorted list; and rule profiling
now receives `r == 1` instead of the raw return code, which used to
decrement the match counter on a no-match.

Ticket: #8944.
3 weeks ago
Victor Julien 6792375d62 ssh: expose the parse failure reason and state in the eve ssh log
A flow that hit an unrecoverable ssh parse error (invalid banner
or invalid record) was never visible in the eve ssh output: the
log condition only admitted a successful handshake (both
directions at kex or session), and the tx logger is one-shot, so
the successful-handshake log fired before a later failure could
be reported and the failure was then silent - the reason was
only visible through the app-layer events.

The log condition is now failure-only: the failure latches, so
the object is emitted exactly once - at the failure, carrying
the error field (invalid_banner / invalid_record) and the state
field (the state the failure occurred in - banner, kex, or
session if a later record fails after NewKeys) - and successful
flows are logged at the flow-end flush, where the engine logs
unconditionally. Logging on the success condition mid-flow would
consume the one-shot tx log and drop a later failure, so it is
not done. The state field is emitted only alongside the error.
The object may be error + state only, when no banner was
parsed. A unit test pins the condition for a failed, a
banner-only and a both-at-kex flow (the failure admits the log
on the failing delivery); the user guide and eve reference doc
commits that follow carry the field docs.

That flow-end flush is skipped for flows that end without a
further app-layer update, though: in IPS/firewall mode a drop
disables the app layer (StreamTcpCheckFlowDrops ->
StreamTcpDisableAppLayer), after which no app-layer update
reaches the tx loggers, and a bypassed flow gets no flow-end
packet either. A successful ssh record - e.g. one that only saw
the banners - was therefore never emitted for those flows. The
hook is the app-layer end: on the transition to app-layer EOF,
StreamTcpDisableAppLayer raises FLOW_APP_LAYER_FLUSH_PENDING,
and FlowUpdateState raises it on the transition to a bypassed
state; OutputTxLog runs the tx loggers once for the next packet,
also when that packet itself has no app-layer update and is not
marked established (the disabling packet returns from the
stream engine before that), and flow-worker clears the flag when
it frees the app layer of a bypassed flow. The flush is
deliberately not a flow-end flush: when the flush is the only
reason to run the loggers, eof is not forced, so each logger's
condition decides and no other parser starts logging records it
suppressed; a disabling packet that does carry an app-layer
update keeps the existing EOF behaviour. SSHTxLogCondition
admits an IPS drop or a bypass, so the pending record is emitted
there; an IDS drop does not end the app layer, so the condition
stays failure-only - a later failure still wins, and the success
record is emitted by the flow-end flush, which the flow timeout
path now also runs for IDS-dropped flows. The unrelated
schema-sort ordering of the new state keys is fixed up as well.

Ticket: #9046.
3 weeks ago
Victor Julien 76f7871783 detect/firewall: sweep defaults up to progress
When the tx progress crosses more than one app state and the last rule
for a direction sits below the current progress, the no-match path set
fw_next_progress_missing but never applied the default policies of the
states in between (only the match path did). With the hello phase now
entered at the start of the message, the record that completes a hello
walks over hello -> cert: without the missing-state policies a packet
whose last rule was at a lower hook was decided by the packet default
policy even though the app policy for the current state says accept.

Apply the missing-state policies up to the final state on the no-match
path as well, mirroring the match path - but only when the policy of
the current hook does not already carry the verdict: an accept with tx
or flow scope has decided the flow, and sweeping the config-less states
above it would hit the implicit drop:flow default and kill a flow the
rule just accepted.

Bound the sweep at the state the packet actually reached (tx_progress),
at both call sites: a state beyond the current progress was not walked
over, so its configured default must not decide this packet. Also make
a drop from the no-match sweep end the walk, like a policy drop at the
current hook does.

Ticket: #9047.
3 weeks ago
Victor Julien fa76410603 detect: fix heap buffer overflow in DetectRunTx post-rule match
PrefilterPostRuleMatch() repopulates det_ctx->pmq with flowbits:isset
candidates after a matching flowbits:set rule is found. The subsequent
append loop writes directly into det_ctx->tx_candidates[] without first
checking that the array has room.

The buffer was initially sized for prefilter results only not accounting
for post-rule additions.  When array_idx already equals tx_candidates_size,
the writes go past the end of the heap-allocated block - a heap buffer
overflow followed by qsort() over OOB memory, causing a crash (DoS).

Addressed by adding a capacity check + expand before the append loop,
matching the existing pattern at all other candidate append logic.

Ticket: #8866.
2 months ago
Jason Ish 358b0b621b flow: include cleanup
Include flow-bindgen.h into flow.h and use flow.h in our source.
flow-bindgen.h exists for the purposes of bindgen only.
2 months ago
Victor Julien c5d4a250b5 detect/firewall: avoid passing state match for policy 4 months ago
Victor Julien 110e74642e detect/firewall: harden policy lookup logic 4 months ago
Victor Julien cfc98aeeaf detect: add debug validation checks to assert assumptions 4 months ago
Victor Julien 7e60c3205e detect/alert: store sub-state in PacketAlert
In preparation of being able to log it.
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 116a060092 detect/firewall: policy for substates
For the most part hard coded for HTTP/2 for now.
4 months ago
Victor Julien 5735b87c3a detect: change how DOH2 inspection works
DNS/HTTP2 no longer automatically registers all keywords also for DOH2.

Instead, the DNS keywords and HTTP/2 stream keywords are registered for
DOH2 explicitly as well.

- flow alproto DOH2 + engine DOH2  -> inspect inner DNS
- flow alproto DOH2 + engine HTTP2 -> inspect outer HTTP/2
- flow alproto DOH2 + engine UNKNOWN -> inspect outer HTTP/2
4 months ago
Victor Julien 95365ce05b detect: support per tx sub states
Support for per transaction sub states: different state machines per
transaction type.

Skip engines not belonging to our substate.

Store tx_type in DetectTransaction.

Each protocol supporting sub states will register states from 1 and up.

To support:

Ticket: #8386.
4 months ago
Victor Julien 9e31a21aba detect/firewall: apply accept if last tx was skipped
In firewall mode, a accept:hook or accept:tx needs to lead to a accept
packet when the action is applied to the last TX. For this the code
relied of the `DetectTransaction::is_last` field, where the assumption
was that there would always be an inspection on the last transaction.
This assumption was wrong however, as transactions can be skipped for a
few reasons: not updated, fully inspected, unidirectional for the other
direction. This would cause the accept not be applied to the packet,
leading to a default drop.

The reason this wasn't noticed before is that until now the work had
focused on protocols that used sequential transactions (http/tls),
and/or short lived sequential unidir transactions (dns)

This patch addresses the issue by making a simple assumption: if the
last available transaction in the main detection loop is skipped, it
means it has been accepted before. Therefore we can apply the "accept
packet" logic in this case.

Bug: #8698.
4 months ago
Victor Julien ee1b9b9212 detect: use next tx info from iterator
In non-sequential transaction handling, the is_last set could be
incorrect.
4 months ago
Victor Julien 3b351e2574 detect: move packet alert logic into helper 4 months ago
Victor Julien 695c9d8d20 detect/alert: split append func per packet/tx 4 months ago
Philippe Antoine 29795be5a2 fuzz: make more unit test utilities available for fuzzing 5 months ago
Victor Julien e0b501a4b7 detect: cleanup last tx logic
Move into DetectTransaction.
5 months ago
Victor Julien 3d00e73d91 detect: make progress values uint8_t
Also reflect that it can be >= 48, like in prefilter.
5 months ago
Victor Julien a9b3ad49ea detect/firewall: minor code cleanup 5 months ago
Victor Julien 4db84cfa28 detect/firewall: clean up apply accept logic
Use end state from tx.

Handle flow control from inside the function consistently.
5 months ago
Victor Julien 68885e75e1 detect/firewall: refactor per tx rule result handling
Break out the 3 options: match, partial match, no match for firewall
into separate functions.

Additionally, handle the re-match case for matches on a hook where the
progress value didn't yet progress further. In this case the continue
inspection logic revisits the rule and the accept needs to be
re-applied.
5 months ago
Victor Julien 9c76480ac3 detect/firewall: further simplify flow control
For the last for progress case we can just break on a firewall drop.

For the accept:flow and accept:tx cases the next sig (if any) will check
the flow/tx flag and manage flow control from there.
5 months ago
Victor Julien bb6dc93789 detect/firewall: clean up per rule run check 5 months ago
Victor Julien 035e8bc851 detect/firewall: minor flow control cleanup 5 months ago
Victor Julien bb7aff36a8 detect/firewall: simplify pre-check flow
Simplify pre-check logic. Pre-check takes care of enforcing
FLOW_ACTION_ACCEPT, APP_LAYER_TX_ACCEPT and default policy enforcement
before the current rule's hook. This should be done in firewall mode for
each rule regardless whether it is a firewall or TD rule.

No need to track verdict state anymore.
5 months ago
Victor Julien 68f73302ce detect/firewall: clean up tx inspection loop 5 months ago
Victor Julien a86fe01cc3 detect/firewall: consolidate action handling
Step towards consolidating all action handling into a central location.
5 months ago
Victor Julien 1ad9c3ec29 detect/firewall: clean up pre-check policy logic 5 months ago
Victor Julien 52967c69ec detect/firewall: fix accept:tx,alert in default policy 5 months ago
Victor Julien 247c6a2333 detect/firewall: inject alert before default policy
If default policy is invoked because of missing rules for next hooks,
make sure to inject an alert before the next hook policies might do so.
5 months ago
Victor Julien f1090da254 detect/firewall: log alert for app default with alert
Fix missing alert on drop:flow,alert
5 months ago
Victor Julien 485f5243d5 detect/firewall: improve handle fw alert handling
More clearly define the relationship between PacketAlerts for firewall
and threat detection events.

Also no longer count firewall_discarded if drop rule came before a
another rule, causing the later rule to not be evaluated/alerted.

When packet:filter and app:filter alert appear in a single alert queue,
handle accept:hook by keeping track of the detect_table.
5 months ago
Victor Julien fb9747df8e detect/firewall: minor code cleanup 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 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
Victor Julien f6dc772677 detect/firewall: fix last for progress handling
In last_for_progress handling set accept only on packet if it was also
triggered on the last tx.

If there are more transactions, the accept can be set later (if policy
allows).
5 months ago
Juliana Fajardini 312967f291 detect: add flow drop by firewall as drop reason
To track flow drops triggered by the firewall.
Add flow drop by firewall as drop reason.

As part of
Ticket #7699
5 months ago
Juliana Fajardini 5b488feef5 detect/firewall: add dedicated stats counters
Add a `firewall` stats counter aggregator for all firewall-related
stats.
De-overload "detect.alert_queue_overflow", by adding
"firewall.discarded_alerts" to account for discarded drops in
Firewall mode.
Add Debug statements for tracking corner cases where it can be
difficult to know where a drop is coming from.

Added counters:
- stats.firewall.blocked
- stats.firewall.accepted
- stats.firewall.rejected
- stats.firewall.drop_reason.default_app_policy
- stats.firewall.drop_reason.default_packet_policy
- stats.firewall.drop_reason.flow_drop
- stats.firewall.drop_reason.pre_flow_hook
- stats.firewall.drop_reason.pre_stream_hook
- stats.firewall.drop_reason.rules
- stats.firewall.discarded_alerts

Ticket #7699
5 months ago
Victor Julien 367ca7f430 detect/tx: minor debug additions and fixes 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.
5 months ago
Victor Julien eaacb41aaf firewall: accept:flow no longer implies pass:flow
Previously a `accept:flow` action would act as both a firewall "accept" and
a threat detection "pass" for the rest of the flow.

This patch changes that. The `accept:flow` action now only accepts the
rest of the packets for the firewall ruleset, but does still continue
threat detection rule evaluation.

Ticket: #8444.
5 months ago
Victor Julien 0093bd6123 detect/firewall: rename flow control variable
For improved readability.
5 months ago