Victor Julien [Tue, 10 Jun 2025 10:40:21 +0000 (12:40 +0200)]
threads: clean up module flags
Remove unused TM_FLAG_STREAM_TM.
Rename TM_FLAG_DETECT_TM to TM_FLAG_FLOWWORKER_TM as it was mostly used
to check if a thread is a flow worker. TM_FLAG_DETECT_TM was always set
for a flow worker, even when there was no detection in use.
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 [Thu, 5 Jun 2025 08:43:22 +0000 (10:43 +0200)]
detect: set detect table for non-firewall mode as well
This also exposed a difference between the handling of TD alerts in
firewall vs non-firewall mode. In firewall mode the table/hook is also
part of the alert ordering to make sure actions from packet:td are
applied before app:td. Handle that explicitly for now.
Jeff Lucovsky [Sat, 7 Jun 2025 13:25:52 +0000 (09:25 -0400)]
detect: Ensure byte* variable usages is for same buffers
Issue: 7549
Use the active buffer list to fetch SM variables to ensure that they are
part of the same list so a variable created with bytemath or byteextract
will have context when used with bytejump, e.g
Jason Ish [Thu, 5 Jun 2025 15:20:08 +0000 (09:20 -0600)]
dns: log addresses in order of packet
DNS logs have always been logged in flow direction, this can be
confusing as DNS responses have a src_ip of the client, but it makes
more sense to have the src_ip for the server, as that is the src_ip of
the response packet.
As this is a breaking change, limit it DNS v3 logging which was
introduced, and is the default for Suricata 8.0.
Jason Ish [Thu, 5 Jun 2025 21:41:40 +0000 (15:41 -0600)]
output: delayed initialization for custom loggers
When a plugin is first initialized, it is too early to register
transaction loggers. Instead, a plugin can register a callback to be
called when Suricata is ready for outputs like transaction loggers to
be registered.
Likewise for library users, there is a window in SuricataInit where
transaction loggers can be registered that library users don't have
access to. So a lifecycle callback useful here as well.
Internals
---------
Suricata's stream engine returns data for inspection to the detection
engine from the stream when the chunk size is reached.
Bug
---
Inspection triggered only in the specified chunk sizes may be too late
when it comes to inspection of smaller protocol specific data which
could result in delayed inspection, incorrect data logged with a transaction
and logs misindicating the pkt that triggered an alert.
Fix
---
Fix this by making an explicit call from all respective applayer parsers to
trigger raw stream inspection which shall make the data available for inspection
in the following call of the stream engine. This needs to happen per direction
on the completion of an entity like a request or a response.
Important notes
---------------
1. The above mentioned behavior with and without this patch is
affected internally by the following conditions.
- inspection depth
- stream depth
In these special cases, the inspection window will be affected and
Suricata may not consider all the data that could be expected to be
inspected.
2. This only applies to applayer protocols running over TCP.
3. The inspection window is only considered up to the ACK'd data.
4. This entire issue is about IDS mode only.
SMTP parser can handle multiple command lines per direction, however an
SMTP transaction comprises of the full communication starting from HELO
till there's a RST or QUIT request. Appropriate calls to trigger raw stream
inspection have been added on succesful parsing of each full request and response.
Shivani Bhardwaj [Fri, 23 May 2025 05:31:45 +0000 (11:01 +0530)]
dnp3: trigger raw stream inspection
Internals
---------
Suricata's stream engine returns data for inspection to the detection
engine from the stream when the chunk size is reached.
Bug
---
Inspection triggered only in the specified chunk sizes may be too late
when it comes to inspection of smaller protocol specific data which
could result in delayed inspection, incorrect data logged with a transaction
and logs misindicating the pkt that triggered an alert.
Fix
---
Fix this by making an explicit call from all respective applayer parsers to
trigger raw stream inspection which shall make the data available for inspection
in the following call of the stream engine. This needs to happen per direction
on the completion of an entity like a request or a response.
Important notes
---------------
1. The above mentioned behavior with and without this patch is
affected internally by the following conditions.
- inspection depth
- stream depth
In these special cases, the inspection window will be affected and
Suricata may not consider all the data that could be expected to be
inspected.
2. This only applies to applayer protocols running over TCP.
3. The inspection window is only considered up to the ACK'd data.
4. This entire issue is about IDS mode only.
DNP3 parser creates a transaction per direction. Appropriate calls to trigger
raw stream inspection have been added on succesful parsing of each request and
response.
Lukas Sismis [Fri, 3 Jan 2025 15:08:36 +0000 (16:08 +0100)]
threading: support thread autopinning and interface-specific affinity
Using the new configuration format, it is now possible to set CPU affinity
settings per interface.
The threading.autopin option has been added to automatically use CPUs from the
same NUMA node as the interface. The autopin option requires
hwloc-devel / hwloc-dev to be installed and --enable-hwloc flag in configure
script.
Lukas Sismis [Fri, 3 Jan 2025 12:08:49 +0000 (13:08 +0100)]
threading: support previous threading configuration format
Provide backward compatibility with the previous configuration
format to allow smooth transition to the new format.
The commit adds docs about the new format and the introduced changes.
Jason Ish [Fri, 6 Jun 2025 15:05:12 +0000 (09:05 -0600)]
rust: fix compiler warning for confusing lifetimes
For example:
error: lifetime flowing from input to output with different syntax can be confusing
--> htp/src/headers.rs:475:16
|
475 | fn null(input: &[u8]) -> IResult<&[u8], ParsedBytes> {
| ^^^^^ ----- ----------- the lifetimes get resolved as `'_`
| | |
| | the lifetimes get resolved as `'_`
| this lifetime flows to the output
|
note: the lint level is defined here
--> htp/src/lib.rs:3:9
This currently only happens when using the Rust nightly compiler, which
we use for our fuzz builds.
Jason Ish [Thu, 5 Jun 2025 17:47:04 +0000 (11:47 -0600)]
rust: update deps
Update all deps with cargo update. Additionally, apply the updated
versions to the Cargo.toml, which while not stricly required, does
make it more clear what the version in use is.
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.
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.
Jason Ish [Wed, 4 Jun 2025 17:26:52 +0000 (11:26 -0600)]
lua/output: access luastate within lock
Fixes Coverity issue:
CID 1648445: (#1 of 1): Data race condition (MISSING_LOCK)
4. missing_lock: Accessing td->lua_ctx->luastate without holding lock
LogLuaCtx_.m. Elsewhere, LogLuaCtx_.luastate is written to with
LogLuaCtx_.m held 41 out of 41 times.
pgsql: debug validation on duplicated request msgs
There shouldn't be duplicated messages in the requests Vec. And thus
the parser shouldn't log duplicated keys nor messages. Add debug
validations to ensure this.
With PGSQL's current state machine, most frontend/ client messages will
lead to the creation of a new transaction - which would prevent
duplicated messages being pushed to the requests array and reaching the
logger.
The current exceptions for that are:
- CopyDataIn
- CopyDone
- CopyFail
Thus, debug statements were added for those cases.
CopyDone and CopyFail, per the documentation, shouldn't be seen
duplicated on the wire for the same transaction. CopyDataIn -- yes, but
we consolidate those, so the expectation is that they won't be
duplicated in the requests array or when reaching the logger either.