Philippe Antoine [Sun, 24 Mar 2024 20:12:15 +0000 (21:12 +0100)]
detect/parse: set limits for pcre2
Ticket: 6889
To avoid regexp dos with too much backtracking.
This is already done on pcre keyword, and pcrexform transform.
We use the same default limits for rules parsing.
Philippe Antoine [Wed, 27 Mar 2024 13:33:54 +0000 (14:33 +0100)]
http2: use a reference counter for headers
Ticket: 6892
As HTTP hpack header compression allows one single byte to
express a previously seen arbitrary-size header block (name+value)
we should avoid to copy the vectors data, but just point
to the same data, while reamining memory safe, even in the case
of later headers eviction from the dybnamic table.
Rust std solution is Rc, and the use of clone, so long as the
data is accessed by only one thread.
Jason Ish [Fri, 12 Jan 2024 17:09:59 +0000 (11:09 -0600)]
defrag: fix check for complete packet
The list of fragments may still contain overlaps, so adding up the
fragment lengths is flawed. Instead track the largest size of
contiguous data that can be re-assembled.
Jason Ish [Thu, 7 Dec 2023 22:44:56 +0000 (16:44 -0600)]
defrag: fix subsequent overlap of start of original (bsd)
Fix the BSD policy case where a subsequent fragment starts before an
original fragment and overlaps the beginning of the original
fragment. In this case the overlapping data from the new fragment is
preferred.
Suricata was preferring the data from the original fragment, but it
should only do that when the original fragment has an offset <= to the
new fragment.
Jason Ish [Tue, 28 Nov 2023 18:35:26 +0000 (12:35 -0600)]
defrag: check next fragment for overlap before stopping re-assembly
Instead of breaking the loop when the current fragment does not have
any more fragments, set a flag and continue to the next fragment as
the next fragment may have data that occurs before this fragment, but
overlaps it.
Then break if the next fragment does not overlap the previous.
Victor Julien [Thu, 28 Mar 2024 12:46:23 +0000 (13:46 +0100)]
streaming/buffer: improve integer handling safety
Unsafe handling of buffer offset and to be inserted data's length
could lead to a integer overflow. This in turn would skip growing
the target buffer, which then would be memcpy'd into, leading to
an out of bounds write.
This issue shouldn't be reachable through any of the consumers of
the API, but to be sure some debug validation checks have been
added.
Andreas Herz [Tue, 16 Apr 2024 15:22:20 +0000 (17:22 +0200)]
dataset: cleanup datasets that hit the memcap while loading
Datasets that hit the memcap limit need to be discarded if the memcap is
hit or otherwise the datasets are still loaded with partial data while
the signature is not loaded due to the memcap error.
When adding many sequence nodes, either from start or scalar event
We add "sequence nodes" whose name is an integer cf sequence_node_name
and then run ConfNodeLookupChild to see if it had been already set
(from the command line cf comment in the code)
And ConfNodeLookupChild iterates the whole linked list...
1. We add node 1
2. To add node 2, we check if node 1 equals this new node
3. To add node 3, we check if nodes 1, or 2 equals this new node's name
And so on...
This commits avoids these checks ig the list is empty at the beginning
Jeff Lucovsky [Mon, 15 Apr 2024 14:17:17 +0000 (10:17 -0400)]
flow/inject: Select thread_id by flow flag
Issue: 6957
Rather than selecting the thread_id index by packets traveling to the
server, use the flow flags. If the flow has been reversed, the second
slot is represents the thread id to be used.
Arne Welzel [Sat, 17 Feb 2024 17:19:27 +0000 (18:19 +0100)]
stats: Fix non-worker stats missing
Commit b8b8aa69b49ac0dd222446c28d00a50f9fd7d716 used tm_name of the
first StatsRecord of a thread block as key for the "threads" object.
However, depending on the type of thread, tm_name can be NULL and would
result in no entry being included for that thread at all. This caused
non-worker metrics to vanish from the "threads" object in the
dump-counters output.
This patch fixes this by remembering the first occurrence of a valid
tm_name within the per-thread block and adds another unittest to
cover this scenario.
Victor Julien [Sat, 23 Mar 2024 19:17:54 +0000 (20:17 +0100)]
defrag: fix wrong datalink being logged
Eve's packet_info.linktype should correctly indicated what the `packet`
field contains. Until now it was using DLT_RAW even if Ethernet or other
L2+ headers were present.
This commit records the datalink of the packet creating the first
fragment, which can include the L2+ header data.
Victor Julien [Thu, 4 Apr 2024 15:51:48 +0000 (17:51 +0200)]
host-info: remove pcre2_substring_list_free use
Function prototype has changed in a recent release. Rather than dealing
with detecting that, fall back to our regular pattern of using
pcre2_substring_copy_bynumber().
Jason Ish [Fri, 5 Apr 2024 16:33:14 +0000 (10:33 -0600)]
configure: .git can be a file as well
In worktree scenarios, .git is a file. Assuming its a directory causes
the release date to check the ChangeLog instead of the last commit,
while not a big issue, can be confusing.
Jason Ish [Mon, 1 Apr 2024 16:37:49 +0000 (10:37 -0600)]
configure: export release date for documentation
Sphinx embeds a date in the generated man pages, and to provide
reproducible builds this date needs to be provided to Sphinx,
otherwise it will use the current date.
If building from Git, extract the date from the most recent commit. In
a release, this commit would be the commit that sets the version so is
accurate.
If .git does not exist, use the most recent data found in the
ChangeLog.
The ChangeLog is not used when building from git, as the main/master
branch may not have recent enough timestamps.
This should provide a consistent date when re-building the
distribution from the same non-git archive, or from the same git
commit.
Jason Ish [Mon, 1 Apr 2024 16:35:39 +0000 (10:35 -0600)]
docs/userguide: use a consistent date for reproducible builds
By default, when Sphinx generates the man pages, the current date will
be embedded in them. This can be set to a specific date with the
"today" variable. Typically the date embedded in manpages in the
release date.
To achieve this, attempt to use the environment variable, RELEASE_DATE
to set the "today" variable, reverting back to the empty string if not
set. It is up to our build system to properly set this date.
When the packet load is low, Suricata can run in interrupt
mode. This more resembles the classic approach of processing
packets - CPU cores run low and only fetch packets
on interrupt.
Jeff Lucovsky [Mon, 11 Mar 2024 18:57:16 +0000 (14:57 -0400)]
flow/inject: Ensure initialized thread value used
Issue: 6835
When injecting a flow, ensure that the selected thread_id has been
initialized. When a flow is picked up midstream, the initialized thread
can be the second thread element.
Philippe Antoine [Thu, 21 Mar 2024 21:45:41 +0000 (22:45 +0100)]
rust/mqtt: fix clippy 1.77 warning
error: creating a mutable reference to mutable static is discouraged
--> src/mqtt/mqtt.rs:752:23
|
752 | let max_msg_len = &mut MAX_MSG_LEN;
| ^^^^^^^^^^^^^^^^ mutable reference to mutable static
|
= note: for more information, see issue #114447 <https://github.com/rust-lang/rust/issues/114447>
= note: this will be a hard error in the 2024 edition
= note: this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior
Philippe Antoine [Thu, 21 Mar 2024 15:15:53 +0000 (16:15 +0100)]
rust/smb: fix clippy nightly warning
error: unnecessary use of `to_vec`
--> src/smb/smb.rs:1048:62
|
1048 | let (name, is_dcerpc) = match self.guid2name_map.get(&guid.to_vec()) {
| ^^^^^^^^^^^^^^ help: replace it with: `guid`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
= note: `#[deny(clippy::unnecessary_to_owned)]` implied by `#[deny(warnings)]`
Philippe Antoine [Thu, 21 Mar 2024 15:02:23 +0000 (16:02 +0100)]
rust: fix clippy 1.77 warning
Ticket: 6883
error: field `0` is never read
--> src/asn1/mod.rs:36:14
|
36 | BerError(Err<der_parser::error::BerError>),
| -------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| field in this variant
|
Philippe Antoine [Thu, 22 Feb 2024 09:14:36 +0000 (10:14 +0100)]
ssh: avoid quadratic complexity from long banner
Ticket: 6799
When we find an overlong banner, we get into the state just
waiting for end of line, and we just want to skip the bytes
until then.
Returning AppLayerResult::incomplete made TCP engine retain
the bytes and grow the buffer that we parsed again and again...
When running FlowWorkerStreamTCPUpdate, one of the dequeued packet
may set the flow action to drop, without updating the not-pseudo
packet action, as is done usually with a previous call to
FlowHandlePacketUpdate
Jason Ish [Tue, 27 Feb 2024 22:07:33 +0000 (16:07 -0600)]
thread modules: separate initialization from registration
Move the zero'ing to the thread module array InitGlobal in an effort
to fix capture modules.
At some point device validation moved to a point in startup before
plugins are loaded meaning that capture plugins could not be
used. Moving plugin registration early enough caused some of their
registration to be wiped out as clearing the array was done after.
We only have more logging to do if the app update was fresh,
ie if p->app_update_direction != 0
If we have data acknowledged in one direction,
and then many packets in the other direction,
the APP_UPDATED flow flags did not get reset because we did not
run detection yet in this direction,
but there is nothing more to do after the first packet in the
other direction.
Lukas Sismis [Sat, 10 Feb 2024 19:04:55 +0000 (20:04 +0100)]
hugepages: run hugepage check only on DPDK runmode and on Linux
Previous implementation allowed FreeBSD to enter into the hugepage
analysis. It then failed with an error message because hugepage/
NUMA node paths that are used in the codebase to retrieve info about
the system are not the same with the structure in Linux.
Additionally, the messages were logged on error level. It has been
demoted to info level because the whole hugepage analysis checkup is
only for informational purposes and does not affect Suricata operation.
The hugepage analysis and the hugepage snapshots are now limited to
only run in the DPDK runmode.
Jeff Lucovsky [Sat, 25 Nov 2023 14:22:19 +0000 (09:22 -0500)]
cppcheck: Address cpcheck report of an FP
Issue: 6527
Address the FP raised by cppcheck -- note that although the code
corectly checks to ensure that `to_shift != &sb->reqion`, the logic was
detected as a FP. Rework the code to eliminate the FP.
Victor Julien [Sat, 17 Feb 2024 09:52:59 +0000 (10:52 +0100)]
runmode/unix-socket: fix cppcheck warnings
src/runmode-unix-socket.c:547:9: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(tstr, sizeof(tstr), "%d", cfile->tenant_id);
^
src/runmode-unix-socket.c:1040:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(prefix, sizeof(prefix), "multi-detect.%d", tenant_id);
^
src/runmode-unix-socket.c:1189:5: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(prefix, sizeof(prefix), "multi-detect.%d", tenant_id);
^
Lukas Sismis [Tue, 6 Feb 2024 10:38:19 +0000 (11:38 +0100)]
tcp: do not assign TCP flags to pseudopackets
Previously pseudopackets were assigned with ACK flag which falsely turned
"SYN" flows to "SYN/ACK" flows when Suricata ran with raw content-matching
rules. The problem occured during the flow timeout or Suricata shutdown,
essentially, when the flow was being kicked out (with a pseudopacket).
When Suricata ran without raw content-matching rules (the ruleset did not
contain content matching keywords or it only contained keywords that are
app-layer content-matching) then raw stream reassembly tracking is turned off
(SignatureHasStreamContent()).
This in turn disabled a check in StreamNeedsReassembly() and the right edge
was not checked with the raw stream progress. In turn, it did not generate
a pseudopacket that would go through the detection engine. Suricata with
raw content-matching keywords would therefore on a flow with SYN packet only
return STREAM_HAS_UNPROCESSED_SEGMENTS_NEED_ONLY_DETECTION which would generate
the pseudopacket.
In Suricata versions <= 6.0.x, the flow output was correct because
only the commit 1bb6f44ff01363fa29488f1ae83b9368e33c2770 started to
differentiate the right edge calculation between the raw and application
layer streams. The older Suricata versions used only the application layer
right edge equation and therefore did not generate a pseudopacket.
Victor Julien [Sun, 11 Feb 2024 08:29:38 +0000 (09:29 +0100)]
multi-tenant: fix loader dead lock
A dead lock could occur at start up, where a loader thread would
get stuck on it's condition variable, while the main thread was
polling the loaders task results.
Between the main thread unlocking the loader and waking up the
threads, it is possible that the loader has already moved ahead
but not yet entered its conditional wait. The main thread sends
its condition signal, but since the loader isn't yet waiting on
it the signal is ignored. Then when the loader does enter its
conditional wait, the signal is not sent again.
This patch updates the logic to send signals much more often.
It also makes sure that the signal is sent under lock, as the
API requires.
Arne Welzel [Mon, 5 Feb 2024 16:45:30 +0000 (17:45 +0100)]
stats: Do not expand dots of tm_name
When an interface with dots is used, per worker stats are nested by the
dot-separated-components of the interface due to the usage of
OutputStats2Json().
Prevent this by using OutputStats2Json() on a per-thread specific object
and setting this object into the threads object using the
json_object_set_new() which won't do the dot expansion.
This was tested by creating an interface with dots in the name
and checking the stats.
ip link add name a.b.c type dummy
With Suricata 7.0.2, sniffing on the a.b.c interface results in the
following worker stats format:
Simon Dugas [Fri, 29 Dec 2023 16:58:50 +0000 (11:58 -0500)]
detect-engine-iponly: improve ip list performance
The runtime complexity of insertion sort is approx. O(h*n)^2 where
h is the size of the HOME_NET and n is the number of ip only rules
that use the HOME_NET.
Replacing this with qsort significantly improves rule load time when
a large HOME_NET is used in combination with a moderate amount of ip
only rules.
Philippe Antoine [Thu, 25 Jan 2024 13:26:09 +0000 (14:26 +0100)]
detect: respect directionality for filestore
Ticket: 6617
So that rules with keyword like `filestore:to_server,flow`
only store the files to server and not the ones to client...
Directionality only worked with the default scope, ie the
current file, and not the scope tx or scope flow.
For non-default scope, tx or flow, both directions were stored
whatever the directionality specified.
For these non-default scopes, this commit keeps a default
of both directions, but use only one direction if specified.
Need to split flag FLOWFILE_STORE per direction, so that Suricata
can retain this (optional) directional info from the filestore
keyword.
Jason Ish [Thu, 8 Feb 2024 19:21:11 +0000 (13:21 -0600)]
detect-http: add superfluous alloc check for cocci
Add not-needed SCCalloc return check to satisfy our Cocci malloc
checks as it can't see that the caller immediately checks the return
value of this simple wrapper around SCCalloc.
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
--> src/dns/log.rs:371:29
|
371 | pub fn dns_print_addr(addr: &Vec<u8>) -> std::string::String {
| ^^^^^^^^ help: change this to: `&[u8]`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg