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.
While this could be considered minor, they were not just bad, but
misleading names, as the variables weren't really `dummy` responses,
but consolidating several messages.
pgsql: add initial support to CopyIn mode/subproto
This sub-protocol inspects messages sent mainly from the frontend to
the backend after a 'COPY FROM STDIN' has been processed by the
backend.
Parses new messages:
- CopyInResponse -- initiates copy-in mode/sub-protocol
- CopyData (In) -- data transfer message, from frontend to backend
- CopyDone -- signals that no more CopyData messages will be seen from
the frontend, for the current transaction
- CopyFail -- used by the frontend to signal some failure to proceed
with sending CopyData messages
Jason Ish [Mon, 2 Jun 2025 21:47:03 +0000 (15:47 -0600)]
lua/streaming: provide streaming buffer as argument
When setting up a Lua output script for streaming data, we're
explicitly requesting stream data. Just pass the streaming data as
arguments, rather than requiring the script to make an extra call to
get the stream data.
The streaming data will be passed in the "stream" field of the args
passed to the log function.
Jason Ish [Mon, 2 Jun 2025 20:37:57 +0000 (14:37 -0600)]
lua/bytevar: convert SCByteVar to Lua lib
Similar to flowvars and flowints, but a byte var cannot be registered
from a Lua script, but it still needs to be setup. Instead provide an
"map" function that sets it up, or errors out if the byte var is
unknown.
This also required passing the signature into the Lua init method, as
the state of the Signature object and the time of loading the Lua
keyword is required.
Jason Ish [Mon, 2 Jun 2025 15:07:20 +0000 (09:07 -0600)]
lua: fix coverity unchecked return
CID 1648351: (#1 of 1): Unchecked return value (CHECKED_RETURN)
1. check_return: Calling lua_getstack without checking return value (as is done elsewhere 9 out of 10 times).
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.
Shivani Bhardwaj [Thu, 29 May 2025 09:04:57 +0000 (14:34 +0530)]
detect/analyzer: use note instead of warning
It is invalid to issue a warning based purely on pattern matching
between the matched content and certain http methods. Without a check
for which keyword exists in the rule, it makes more sense to issue this
as a general note.
util/flow-rate: fix sum calc on index next to base
When the buffer is wrapped around, for any new index, the calculation
must subtract the previous value stored in the buffer. So far, the code
ended up adding to the existing buffer value on the index unless it was
the first index after wrapping around. This is incorrect and would end
up flagging a flow as elephant a lot before than it should be.
Jason Ish [Wed, 28 May 2025 18:43:05 +0000 (12:43 -0600)]
mdns: add mdns parser, logger and detection
The mDNS support is based heavily on the DNS support, reusing the
existing DNS parser where possible. This meant adding variations on
DNS, as mDNS is a little different. Mainly being that *all* mDNS
traffic is to_server, yet there is still the concept of request and
responses.
They are mostly in-line with the DNS keywords, except
mdns.answers.rdata which is a better than that mdns.response.rrname,
as its actually looking at the rdata, and not rrnames.
mDNS has its own logger that differs from the DNS logger:
- No grouped logging
- In answers/additionals/authorities, the rdata is logged in a field
that is named after the rdata type. For example, "txt" data is no
longer logged in the "rdata" field, but instead a "txt" field. We
currently already did this in DNS for fields that were not a single
buffer, like SOA, SRV, etc. So this makes things more consistent. And
gives query like semantics that the "grouped" object was trying to
provide.
- Types are logged in lower case ("txt" instead of "TXT")
- Flags are logged as an array: "flags": ["aa", "z"]
Jason Ish [Tue, 27 May 2025 17:27:23 +0000 (11:27 -0600)]
dns: parse multiple txt segments into an array
A DNS TXT answer record can actually be made of up multiple TXT
entries in a single record. Suricata currently expands these into
multiple TXT records, however that is not very representative of the
actualy DNS message.
Instead, if a TXT record contains multiple labels, parse them into an
array.
We still expand multiple TXT segements into multiple TXT records at
logging time for compatibility, but this will allow something like
MDNS to log more accurately to the protocol.
Shivani Bhardwaj [Tue, 27 May 2025 13:17:43 +0000 (18:47 +0530)]
doc: move upgrade note to correct section
The PR for the behavior change of dealing with spaces in http URI and
protocol was started in 2019 and merged in 2024. When the PR was
created, it belonged to the correct upgrade section, however, by the
time it was merged, it was 8.0.x branch already.
Move it to upgrade notes from 7 to 8 for correctness.
Jason Ish [Fri, 23 May 2025 15:42:18 +0000 (09:42 -0600)]
clang-format: don't reformat schema on rewrite-branch
Add --extensions c,h flag to git clang-format when reformatting
commits on a branch. This prevents clang-format from modifying
non-C files like JSON schemas.
The extensions is used elsewhere in this script, so I suspect it was
just an oversight.
Also, ./scripts/schema-order.py --in-place will consistently format the
schema, while sorting the keys.
Philippe Antoine [Wed, 21 May 2025 13:34:26 +0000 (15:34 +0200)]
detect/files: increment local_file_id even if buffer is NULL
Ticket: 7579
Otherwise, we will keep on calling again and again GetDataCallback
with the same local_file_id, and we will always get a NULL
buffer even if the next local_file_id would return a non-NULL buffer.