Victor Julien [Mon, 21 Oct 2024 13:24:50 +0000 (15:24 +0200)]
detect: don't run pkt sigs on ffr pkts
Last packet from the TLS TCP session moves TCP state to CLOSED.
This flags the app-layer with APP_LAYER_PARSER_EOF_TS or
APP_LAYER_PARSER_EOF_TC depending on the direction of the final packet.
This flag will just have been set in a single direction.
This leads to the last packet updating the inspect id in that packets
direction.
At the end of the TLS session a pseudo packet is created, because:
- flow has ended
- inspected tx id == 0, for at least one direction
- total txs is 1
Then a packet rule matches:
```
alert tcp any any -> any 443 (flow: to_server; \
flowbits:isset,tls_error; \
sid:09901033; rev:1; \
msg:"Allow TLS error handling (outgoing packet)"; )
```
The `SIG_MASK_REQUIRE_REAL_PKT` is not preventing the match, as the
`flowbits` keyword doesn't set it.
To avoid this match. This patch skips signatures of the `SIG_TYPE_PKT`
for flow end packets.
Philippe Antoine [Tue, 26 Nov 2024 20:44:45 +0000 (21:44 +0100)]
detect: log app-layer metadata in alert with single tx
Ticket: 7199
Uses a config parameter detect.guess-applayer-tx to enable
this behavior (off by default)
This feature is requested for use cases with signatures not
using app-layer keywords but still targetting application
layer transactions, such as pass/drop rule combination,
or lua usage.
This overrides the previous behavior of checking if the signature
has a content match, by checking if there is only one live
transaction, in addition to the config parameter being set.
Lukas Sismis [Mon, 9 Dec 2024 15:07:57 +0000 (16:07 +0100)]
dpdk: set ice PMD RSS key length to 52 bytes for all DPDK versions
ICE driver (Intel E810 NIC) requires/supports 52-byte long RSS key.
The 52 byte key length was mandatory from DPDK 23.11 when Suricata
was starting with independently configured ice PMD.
However, Suricata failed to start when ice PMD was part of
net_bonding PMD, requiring 52 byte RSS key even in DPDK versions
lower than 23.11. Since the support for the longer key is present
since DPDK 19.11 the key is set to 52 bytes for all versions.
Philippe Antoine [Fri, 31 May 2024 08:39:16 +0000 (10:39 +0200)]
app-layer: track modified/processed txs
To optimize detection, and logging, to avoid going through
all the live transactions when only a few were modified.
Two boolean fields are added to the tx data: updated_tc and ts
The app-layer parsers are now responsible to set these when
needed, and the logging and detection uses them to skip
transactions that were not updated.
There may some more optimization remaining by when we set
both updated_tc and updated_ts in functions returning
a mutable transaction, by checking if all the callers
are called in one direction only (request or response)
Victor Julien [Tue, 3 Dec 2024 10:36:27 +0000 (11:36 +0100)]
flow/manager: fix multi instance row tracking
In multi instance flow manager setups, each flow manager gets a slice
of the hash table to manage. Due to a logic error in the chunked
scanning of the hash slice, instances beyond the first would always
rescan the same (first) subslice of their slice.
The `pos` variable that is used to keep the state of what the starting
position for the next scan was supposed to be, was treated as if it held
a relative value. Relative to the bounds of the slice. It was however,
holding an absolute position. This meant that when doing it's bounds
check it was always considered out of bounds. This would reset the sub-
slice to be scanned to the first part of the instances slice.
This patch addresses the issue by correctly handling the fact that the
value is absolute.
Similar keywords use `isnotset`, while `flowint` only accepted `notset`
Opted to change the code, not only the regex, to keep the underlying
code also following the same patterns.
Victor Julien [Tue, 3 Dec 2024 15:55:38 +0000 (16:55 +0100)]
eve/alert: enrich decoder event
Default decoder event alert was very sparse, not even logging packet
type and pcap_cnt. Expand support for this record type. It will be more
useful with the ethernet headers and packet field, but these are still
disabled by default.
Victor Julien [Thu, 30 May 2024 14:02:28 +0000 (16:02 +0200)]
af-packet: speed up thread sync during startup
Threads are initialized sequentially to allow for a predictable mapping
of threads and queues. Not all parts of the start up need to be done
sequentially. The setting up of the rings can be very expensive, taking
of a couple of hundred milliseconds. The ring setup doesn't need to be
done sequentially though.
This patch releases the thread early, after bind but before the ring
setups.
Jason Ish [Wed, 20 Nov 2024 16:46:38 +0000 (10:46 -0600)]
requires: treat unknown requires keywords as unmet requirements
For example, "requires: foo bar" is an unknown requirement, however
its not tracked, nor an error as it follows the syntax. Instead,
record these unknown keywords, and fail the requirements check if any
are present.
A future version of Suricata may have new requires keywords, for
example a check for keywords.
Jason Ish [Thu, 28 Nov 2024 15:51:24 +0000 (09:51 -0600)]
rust: update num-derive to 0.4.2
This prevents the clippy warning:
508 | #[derive(FromPrimitive, Debug)]
| ^------------
| |
| `FromPrimitive` is not local
| move the `impl` block outside of this constant `_IMPL_NUM_FromPrimitive_FOR_IsakmpPayloadType`
509 | pub enum IsakmpPayloadType {
| ----------------- `IsakmpPayloadType` is not local
|
= note: the derive macro `FromPrimitive` defines the non-local `impl`, and may need to be changed
= note: the derive macro `FromPrimitive` may come from an old version of the `num_derive` crate, try updating your dependency with `cargo update -p num_derive`
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration for the purpose of this lint
= note: this warning originates in the derive macro `FromPrimitive` (in Nightly builds, run with -Z macro-backtrace for more info)
Jason Ish [Fri, 22 Nov 2024 21:26:49 +0000 (15:26 -0600)]
output-json: drop eve records that are too long
In the situation where the mem buffer cannot be expanded to the
requested size, drop the log message.
For each JSON log context, a warning will be emitted once with a partial
bit of the log record being dropped to identify what event types may be
leading to large log records.
This also fixes the call to MemBufferExpand which is supposed be
passed the amount to expand by, not the new size required.
- DetectEngineInspectBufferHttpHeader is only used with ALPROTO_HTTP1
- engine->progress should be HTP_REQUEST_HEADERS or HTP_RESPONSE_HEADERS based on the direction
Victor Julien [Wed, 18 May 2022 12:32:35 +0000 (14:32 +0200)]
radix: implement more compact trees
Implement a more compact set of trees specifically for IPv4
and IPv6 addresses. This allows for more compact data structures
and fewer memory allocations.
Jason Ish [Tue, 19 Nov 2024 17:28:03 +0000 (11:28 -0600)]
rust: put all rust/cargo env vars in CARGO_ENV
To ensure that all calls to cargo use the same environment variables,
put the environment variables in CARGO_ENV so every call to cargo can
easily use the same vars.
The Cargo build system is smarter than make, it can detect a change in
an environment variable that affects the build, and the setting of
SURICATA_LUA_SYS_HEADER_DST changing could cause a rebuild.
Also update suricata-lua-sys, which is smarter about copying headers. It
will only copy if the destination does not exist, or the source header
is newer than the target, which can also prevent unnecessary rebuilds.
This is mainly to fix an issue where subsequent builds may fail,
especially when running an editor with a LSP enabled:
Update lua crate to 0.1.0-alpha.5. This update will force a rewrite of
the headers if the env var SURICATA_LUA_SYS_HEADER_DST changes. This
fixes the issue where the headers may not be written.
The cause is that Rust dependencies are cached, and if your editor is
using rust-analyzer, it might cache the build without this var being
set, so these headers are not available to Suricata. This crate update
forces the re-run of the Lua build.rs if this env var changes, fixing
this issue.
Jason Ish [Fri, 18 Oct 2024 14:46:42 +0000 (08:46 -0600)]
make: install-headers: rust-bindings.h
rust-bindings.h was not being installed with "make install-headers",
and its now pulled in by a header used for plugin support, so make
sure its installed.
We first attempt to install the "dist" version if exists, otherwise
install the "gen" one. Also install the "gen" even if the "dist" one
exists, as its going to be newer.
Jason Ish [Fri, 11 Oct 2024 19:21:14 +0000 (13:21 -0600)]
eve: user callbacks for adding additional data
Provide a way for library/plugin users to register a callback that
will be called prior to an EVE record being closed. The callback will
be passed ThreadVars, Packet, and Flow pointers if available, as well
as private user data.
Philippe Antoine [Tue, 15 Oct 2024 19:05:19 +0000 (21:05 +0200)]
detect/http: fix progress for headers keywords
Ticket: 7326
Having a lower progress than one where we actually can get
occurences of the multibuffer made prefilter
bail out too early, not having found a buffer in the multi-buffer
that matiched the prefilter.
For example, we registered http_request_header with progress 0
instad of progress HTP_REQUEST_HEADERS==2, and if the first
packet had only the request line, we would consider
that signatures with http_request_header as prefilter/fast_pattern
could not match for this transaction, even if they in fact
could have a later packet with matching headers.
Hence, we got false negatives, if http.request_header or
http.response_header was used as fast pattern, and if the request
or response came in multiple packets, and the first of these packets
did not have enough data (like only http request line),
and the next packets did have the matching data.
Philippe Antoine [Tue, 29 Oct 2024 10:00:15 +0000 (11:00 +0100)]
app-layer: remove ALPROTO_TEST and tests
These tests purpose seems to have been lost.
Registering a alproto with a parser function that always fails,
and just testing that AppLayerParserParse returned -1...
We would get the same result without registering a parser function,
or using ALPROTO_FAILED as argument to AppLayerParserParse
The comment says "Test the deallocation of app layer parser memory
on occurrence of error in the parsing process."
but I do not see how this is tested.
Eric Leblond [Fri, 25 Oct 2024 21:34:53 +0000 (23:34 +0200)]
misc: fix build of rules profiling
The patch a0fc2b8628d8a281ef7a2943614b507498c80ca3 has removed the
declaration of functions used when building with ruleset profiling
only (without --enable-profiling). This is causing a build failure.
This patch moves the declaration to the rules profiling section to
be sure it is always there.