Commit Graph

632 Commits (a83d18d3f71d6e071293d293e8532ba51f620c14)

Author SHA1 Message Date
Jeff Lucovsky a83d18d3f7 detect: pass transform errors on to content inspection
The file.name, file.magic and file.data engines built their content
inspection flags without buffer->flags, and DetectGetMultiData and the
dns.response getter overwrote buffer->flags after the transforms ran,
so a transform's error flag never reached content inspection.

OR buffer->flags into the flags passed on, and OR the single-buffer
bits into buffer->flags instead of assigning them.

Issue: 7313
6 days 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 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 59d08a717e detect: make the completion state final at the end state
eof is C > P, so an engine registered at a parser's completion state
never becomes complete: there is no next state to enter. A firewall
rule hooking such a state (e.g. tls:client_finished) stayed pending
forever and was optimistically accepted.

Treat progress == engine progress == tx end state as complete. This
also finalizes the buffer engines' eof on completion-state buffers, so
absent and bsize decisions now happen there, and it covers every parser
whose engine progress equals its completion state (dns, ntp, quic,
rdp-style two-state protocols), not only the firewall path that
motivated the change.

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
Philippe Antoine 5284906338 doh2: use right dns protocol to interpret tx
Ticket: 8916

Avoid type confusion and possible overread when calling
AppLayerParserGetStateProgress

DetectEngineInspectMultiBufferGeneric callback may be called with
the inner DNS tx of a HTTP2/DOH2 tx, so we should use dns as its
protocol
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
Dmitry Uryvchikov d2497790f9 detect-engine-mpm: change transforms->cnt to uint8_t 3 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 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 2e1003a026 detect: add debug validation checks
Check that sub state handling is correct.
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 1bb843d83f detect/http2: don't double register engines
Each keyword supporting DOH2 must register explicitly
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
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
Philippe Antoine 7d024a840c rust: bindgen detect ThreadCtx keyword functions 4 months ago
Philippe Antoine 89dbce1b46 detect: convert tx_progress to uint8_t 4 months ago
Samaresh Kumar Singh df353242bd detect: fail thread init on keyword ctx error
DetectEngineThreadCtxInitKeywords returns TM_ECODE_FAILED when a per-thread keyword init fails (for example DetectFilemagicThreadInit), but ThreadCtxDoInit discarded that result and still returned OK. The detect thread then ran with a partially initialized keyword context array, producing indeterminate results. Propagate the failure so the callers abort thread init and clean up.

Add a unit test that registers a keyword whose thread init fails and verifies that DetectEngineThreadCtxInit reports the failure.

Ticket: #8237
5 months ago
Philippe Antoine 6bb271cee9 conf: uses SCConfGetNonNull
Ticke: 8651

Uses it in place when we dereferenced the value straight away
after checking SCConfGet result but not its value
5 months ago
Victor Julien d154484cc6 detect/firewall: support hook LTE mode for built-in hooks
LTE support depended on registered hook names, but did not support the
built-in names. This commit adds the support.

Ticket: #8645.
5 months ago
Victor Julien b02fa53134 detect: add helper for getting hook name
To fetch hook name from app-layer, or use built-ins.
5 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 17151571b8 detect: guard rate filter callback registration, return bool on failure
SCDetectEngineRegisterRateFilterCallback() dereferences the result of
DetectEngineGetCurrent() unconditionally. Add a NULL guard with
SCLogError and return false so callers can detect registration failure.

Flagged by Svace and confirmed by gcc -fanalyzer.

Ticket: 8560
5 months ago
Victor Julien c27868a8c5 threshold: use seed consistently in hashing
Use for IPv6 in the main hash.

Update IPv4 thread cache to also use a seed. For this the thread
initialization is done at thread init instead on the first packet. The
thread ctx is no longer stored in a thread_local, but instead uses the
thread storage API to store in the ThreadVars. This way it will stay
active during rule reloads as well.

Ticket: #8617.
5 months ago
Philippe Antoine 94a3e03903 util/mpm: add MpmCtx argument to MpmInitThreadCtx
Ticket: 8001

Will allow algorithms such as ac to use the MpmCtx, especially
the number of patterns, to preallocate some structures.

For detect engine PatternMatchThreadPrepare, create a dummy
MpmCtx out of all the MpmCtx in the DetectEngineCtx,
and get the max value for the number of patterns
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
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 02f6a4fd87 detect: work around scan-build warning
detect-engine.c:2350:69: warning: Out of bound access to memory after the end of 'new_det_ctx' [security.ArrayBound]
 2350 |             FlowWorkerReplaceDetectCtx(SC_ATOMIC_GET(s->slot_data), new_det_ctx[i]);
      |                                                                     ^~~~~~~~~~~~~~
1 warning generated.
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 32afba47dd packet: store livedev by id 5 months ago
Philippe Antoine 3d56472d16 detect: bindgen more functions
To prepare to move dcerpc.stub_data keyword to full rust
6 months ago
Jeff Lucovsky d0ba1c4c5e output/flush: Remove pkt-based flush logic
Remove packet-based flush logic in favor of simpler solution

Issue: 8286
7 months ago
Philippe Antoine 7e97dfd521 detect: replace stack alloc by member of DetectEngineThreadCtx
To avoid running a big (when many signatures) stack allocation
on each detection loop with postmatches

Ticket: 8001
8 months ago
Philippe Antoine 14c78d8405 detect: generic setup callback DetectLowerSetupCallback 9 months ago
Philippe Antoine 24e33944aa detect: SetupCallback uses DetectBufferType argument
Will allow for generic callbacks, that can be used from rust
9 months ago
Lukas Sismis 15c83be61a hs: prune stale MPM cache files
Hyperscan MPM can cache the compiled contexts to files.
This however grows as rulesets change and leads to bloating
the system. This addition prunes the stale cache files based
on their modified file timestamp.

Part of this work incorporates new model for MPM cache stats
to split it out from the cache save function and aggregate
cache-related stats in one place (newly added pruning).

Ticket: 7830
9 months ago
Lukas Sismis 08f5abe5e9 detect-engine: make mpm & spm part of MT stub ctx
As a intermediary step for Hyperscan (MPM) caching,
the MPM config initialization should be part of the default
detect engine context for later dynamic retrieval.

Ticket: 7830
9 months ago
Victor Julien 7c0776abeb counters: remove ThreadVars from counter registration
Just pass the StatsThreadContext pointer directly.
11 months ago
Jeff Lucovsky 9a7a30f18e detect/dbv: Assert MT lock held
Add a DBV assert to validate that the master->lock is held.

Suppress missing master->lock warning

Issue: 7819
1 year ago
Jeff Lucovsky 46a53aee73 engine/mt: Ensure master lock held for reload
Issue: 7819

DetectEngineReload must hold the `master->lock`; recent changes changed
the locking usages to avoid deadlock when registering/handling tenants.
These changes added the presumption that the master lock is held at a
higher level. Coverity highlighted that the lock is not held
consistently.
1 year ago
Victor Julien 178f3baf4a detect: improve path handling for tenants
** CID 1666423:       Error handling issues  (CHECKED_RETURN)
/src/detect-engine.c: 4413           in DetectEngineMultiTenantSetup()

_____________________________________________________________________________________________
*** CID 1666423:         Error handling issues  (CHECKED_RETURN)
/src/detect-engine.c: 4413             in DetectEngineMultiTenantSetup()
4407                         goto bad_tenant;
4408                     }
4409                     SCLogDebug("tenant id: %u, %s", tenant_id, yaml_node->val);
4410
4411                     char yaml_path[PATH_MAX] = "";
4412                     if (path) {
>>>     CID 1666423:         Error handling issues  (CHECKED_RETURN)
>>>     Calling "PathMerge" without checking return value (as is done elsewhere 19 out of 21 times).
4413                         PathMerge(yaml_path, PATH_MAX, path, yaml_node->val);
4414                     } else {
4415                         strlcpy(yaml_path, yaml_node->val, sizeof(yaml_path));
4416                     }
4417                     SCLogDebug("tenant path: %s", yaml_path);
4418
1 year ago
Philippe Antoine c52121586a rust: move AppLayerEvents funcs out of RustContext
Ticket: 7667

just use it directly, with bindgen, and cfg not test
1 year ago
Philippe Antoine b7056cd2dc src: remove some unused functions 1 year ago
Jeff Lucovsky e57eeb24b5 misc: fix typos
Fix typos in src/detect-engine.c

Issue: 7819
1 year ago
Jeff Lucovsky 06e89736b3 detect/mt: Prevent deadlock when adding tenants
This commit modifies the call path for registering MT tenants to avoid
deadlocks on the master->lock

When performing tenant operations, e.g., using suricatasc to send a
register-tenant command, a deadlock occurs when

- DetectEngineMTApply: acquires master->lock
- Calls DetectEngineReloadThreads
- Within DetectEngineReloadThreads, calls DetectEngineMultiTenantEnabled
- Which first acquires master->lock

Commit 2bea5af introduced changes to the master->lock usage leading to
the deadlock situation.

Issue: 7819
1 year ago