userguide/exceptions: clarify when stats are logged
The stats for exception policies are only logged/ present when any of
the exception policies are enabled (which means any value other than
"auto" or "ignore" in IDS mode, or "ignore" in IPS mode).
With the addition of exception policy stats counters, the human readable
version of the sats log was mis-aligned, when counters for per-app-proto
were enabled.
Width change made large enough to accomodate a counter as long as
"app_layer.error.bittorrent-dht.exception_policy.pass_packet" which
could be valid.
While our documentation indicated what were the possible configuration
settings for exception policies, our yaml only explicitly mentioned
exception policy for the master switch. Clearly indicate which config
settings are about exception policies.
Some exception policies can only be applied to entire flows or
individual packets, for some exception scenarios. Make this easier to
read, in the documentation.
We will register stats counters for all policies, even though for now
Suri only uses one possible configuration policy at a time. The idea is
that this could change in the near future, so we want to have this
ready.
Jason Ish [Tue, 22 Jul 2025 14:29:08 +0000 (08:29 -0600)]
github-ci: add flto build
Ubuntu and Fedora packing system build with -flto=auto by default, so
update one test to use -flto=auto. Also build with -O2 as that
combination can cause issues such as
https://redmine.openinfosecfoundation.org/issues/7824.
'nbytes' may be used uninitialized [-Werror=maybe-uninitialized]
532 | if (!DetectBytetestValidateNbytes(data, nbytes, optstr)) {
| ^
detect-bytetest.c:336:14: note: 'nbytes' was declared here
336 | uint32_t nbytes;
| ^
Jason Ish [Thu, 7 Aug 2025 15:47:42 +0000 (09:47 -0600)]
rust: fix mismatched_lifetime_syntaxes warning
Fix new warning present in Rust 1.89, for example:
warning: hiding a lifetime that's elided elsewhere is confusing
--> src/ike/parser.rs:295:30
295 | pub fn parse_key_exchange(i: &[u8], length: u16) -> IResult<&[u8], KeyExchangePayload> {
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
A flow with IPv4 IP in IP traffic won't handle this tunneling case
properly.
This leads to potential malicious traffic not triggering alerts, as well
as other inaccuracies in the logs.
Add section label and doc reference, add another term to Common terms
section.
Tried to also improve readability for the Midstream behavior tables:
- Highlight key-words when differences are only in `do` vs `no`.
- Change order of sentences in certain descriptions, to align with the
steps those happen for the engine.
The terms 'inspection' and 'detection' were being used to signify
different engine actions in this document, while throughout the
documentation and code they're many times interchangeable.
Replace 'inspection' with 'parsing' or even 'decoding and parsing' as
more appropriate.
Add a small glossary to clarify what we mean with those terms.
Philippe Antoine [Tue, 15 Apr 2025 10:34:37 +0000 (12:34 +0200)]
http2: forbid data on stream 0
Ticket: 7658
Suricata will not handle well if we open a file for this tx,
do not close it, but set the transaction state to completed.
RFC 9113 section 6.1 states:
If a DATA frame is received whose Stream Identifier field is 0x00,
the recipient MUST respond with a connection error (Section 5.4.1)
of type PROTOCOL_ERROR.
Philippe Antoine [Fri, 27 Jun 2025 07:31:24 +0000 (09:31 +0200)]
rust/dns: fix clippy char_indices_as_byte_indices
error: indexing into a string with a character position where a byte index is expected
--> src/dns/detect.rs:45:39
|
45 | let code: u8 = opcode[i..].parse().map_err(|_| ())?;
| ^
|
= note: a character can take up more than one byte, so they are not interchangeable
note: position comes from the enumerate iterator
--> src/dns/detect.rs:36:10
|
36 | for (i, c) in opcode.chars().enumerate() {
| ^ ^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#char_indices_as_byte_indices
= note: `#[deny(clippy::char_indices_as_byte_indices)]` on by default
help: consider using `.char_indices()` instead
|
36 - for (i, c) in opcode.chars().enumerate() {
36 + for (i, c) in opcode.char_indices() {
Victor Julien [Tue, 10 Jun 2025 09:33:03 +0000 (11:33 +0200)]
threading: fix shutdown of IPS autofp modes
For IPS modes with a verdict thread in autofp there was an issue with
the verdict thread not shutting down, leading to a long shutdown time
until an error condition was reached.
The problem was that when the packet threads, of which the verdict
thread is one, were told to enter their flow timeout loop the verdict
thread got stuck as it immediately progressed to THV_RUNNING_DONE
instead of the expected THV_FLOW_LOOP.
This patch updates the shutdown logic to only apply the flow timeout
logic to the relevant threads, and skip the verdict thread(s).
Add TM_FLAG_VERDICT_TM to indicate a thread has a verdict module to more
explicitly shut it down.
Victor Julien [Wed, 30 Apr 2025 08:20:10 +0000 (10:20 +0200)]
threads: fix autofp shutdown race condition
Sometimes a single flow pcap would log 2 flows. It turns out FlowWorkToDoCleanup
ran before all the packet threads had processed their "wire" packets. It then
removed a flow that a wire packet would still have needed, leading to the worker
thread creating a new flow for it.
This could happen due to the logic in TmThreadDisableReceiveThreads which calls
TmThreadDrainPacketThreads to made sure it only returns when all autofp-workers
have processed all the packets the autofp-capture thread fed to them.
However, the way it checked this is by checking the size of the autofp-worker's
input queue. If 0, it assumes it is done.
What this missed, is that a worker thread could have just taken the last packet
from the input queue, but it is not yet done processing it. If then the
FlowWorkToDoCleanup is ran as well, it would race the worker thread to the flow
handling logic. When it won, the flow was evicted and the packet thread
created a new flow.
This patch improves the shutdown logic to force the worker threads to
enter a "flow loop" (THV_FLOW_LOOP) state before moving on to the
FlowWorkToDoCleanup step. This makes sure that any in progress packets
in the worker threads have been processed.
flow-manager: move time check after RUNNNING state change
When running in pcap-file mode and with a continous directory
reading, the provided directory can be empty.
By having no packets and being in offline mode, the initial packet timestamp
is never set. However, Flow Manager waited until the timestamp was set
to proceed with transferring its state to "RUNNING".
Other pcap-related threads (RX / workers) are set in RUNNING state while
waiting for the PCAP to appear in the directory.
As a result, the main Suricata-Main thread timed out after the default
60 seconds budget for threads to turn from INIT_DONE to RUNNING state.
Jeff Lucovsky [Tue, 17 Dec 2024 12:56:42 +0000 (07:56 -0500)]
detect/content: account for distance variables
Under some cases (below), the depth and offset values are used
twice. This commit disregards the distance variable (if any), when
computing the final depth.
These rules are logically equivalent::
1. alert tcp any any -> any 8080 (msg:"distance name"; flow:to_server; content:"Authorization:"; content:"5f71ycy"; distance:0; byte_extract:1,0,option_len,string,relative; content:!"|38|"; distance:option_len; within:1; content:"|37|"; distance:-1; within:1; content:"|49|"; distance:option_len; within:1; sid:1;)
2. alert tcp any any -> any 8080 (msg:"distance number"; flow:to_server; content:"Authorization:"; content:"5f71ycy"; distance:0; byte_extract:1,0,option_len,string,relative; content:!"|38|"; distance:7; within:1; content:"|37|"; distance:-1; within:1; content:"|49|"; distance:option_len; within:1; sid:2;)
The differences:
Rule 1: content:!"|38|"; distance:option_len; within:1; //option_len == 7
Rule 2: content:!"|38|"; distance:7; within:1;
Without this commit, rule 2 triggers an alert but rule 1 doesn't.
warning: using `contains()` instead of `iter().any()` is more efficient
--> src/http2/http2.rs:267:20
|
267 | if block.value.iter().any(|&x| x == b'@') {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `block.value.contains(&b'@')`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains
= note: `#[warn(clippy::manual_contains)]` on by default
Victor Julien [Mon, 17 Mar 2025 20:19:13 +0000 (21:19 +0100)]
datasets: set higher hashsize limits
To avoid possible upgrade issues, allow higher defaults than in the
master branch. Add some upgrade guidance and a note that defaults will
probably be further reduced.
Philippe Antoine [Tue, 18 Mar 2025 09:55:39 +0000 (10:55 +0100)]
detect: add configurable limits for datasets
Ticket: 7615
Avoids signatures setting extreme hash sizes, which would lead to very
high memory use.
Default to allowing:
- 65536 per dataset
- 16777216 total
To override these built-in defaults:
```yaml
datasets:
# Limits for per rule dataset instances to avoid rules using too many
# resources.
limits:
# Max value for per dataset `hashsize` setting
#single-hashsize: 65536
# Max combined hashsize values for all datasets.
#total-hashsizes: 16777216
```
Jason Ish [Mon, 17 Mar 2025 16:35:57 +0000 (10:35 -0600)]
af-packet: delay setting default-packet-size for af-packet
AF_PACKET needs more information about its configuration before we can
set the default packet size, so on startup, leave unset in suricata.c
if in AF_PACKET mode.
If defrag is enabled, use a default packet size of 9k for tpacket-v2.
This can still lead to truncation events, then the user can increase
their 'default-packet-size'.
Tpacket-v3 does not need an increased packet size as it will handle
any size of packet that is smaller than the configured block size
which now has a default of 128k.
9k for the snap is somewhat arbitrary but is large enough for the
common 9000 jumbo frame plus some extra headers including tpacket
headers.