Giuseppe Longo [Tue, 1 Aug 2023 19:20:58 +0000 (21:20 +0200)]
rust/sip: add direction to transaction
This patch permits to set a direction when a new transaction is created in order
to avoid 'signature shadowing' as reported by Eric Leblond in commit 5aaf50760f546e9047da508f725f43a7ad9b8a35
Jason Ish [Thu, 22 Feb 2024 17:24:52 +0000 (11:24 -0600)]
examples/lib: work with bundled libhtp
The simple example Makefile.am was unconditionally including
$(HTP_LDADD) which might be empty resulting in "../.." ending up in the
Makefile causing the build to fail.
Instead, also make HTP_LDADD a conditional, so we can only include it
when actually set, and its only set when libhtp is bundled.
The reason this Makefile needs to include the path components "../.."
is because the HTP_LDADD value is relative to the top level "src/"
directory.
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.
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.
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);
^
Jason Ish [Thu, 15 Feb 2024 15:19:14 +0000 (09:19 -0600)]
build: remove autoconf.h from dist
We were including "autoconf.h" in our dist releases, however this file
is generated by ./configure. This can be problematic in out of tree
builds as the one included in the dist, not the generated one may be
used.
Also using "autoconf.h" instead of <autoconf.h> so it is searched for
relative to the file including it which is necessary if a library/plugin
does something like "#include "suricata/foo.h".
Jason Ish [Mon, 5 Feb 2024 22:49:45 +0000 (16:49 -0600)]
examples: program linking against library
Provide an example of an extremely simple application that links
against Suricata. This provides a Makefile integrated with the
Suricata build system for in-tree building, as well as an example
Makefile for building out of tree.
Currently this application just wraps SuricataMain and does nothing
else.
Jason Ish [Wed, 14 Feb 2024 21:23:44 +0000 (15:23 -0600)]
examples/plugin: simplify Makefile
Simplify the Makefile by avoiding automake and providing our own
Makefile.in that is suitable for in-tree builds of the plugin and can
also serve as an example for standalone plugins.
But the bigger benefit of this is to allow building the example plugin
even with --disable-shared provided to configure, as this is just a
phony limitation imposed by automake/libtool.
Jason Ish [Tue, 13 Feb 2024 19:38:57 +0000 (13:38 -0600)]
cocci/run-check: log if parallel command is not found
If CONCURRENCY_LEVEL was set, the script would log a concurrency level
even if the parallel command was not available. Not log if parallel is
not available and set concurrency to 1.
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.
Fixes: 79499e476979 ("app-layer: move files into transactions")
Victor Julien [Tue, 13 Feb 2024 08:51:15 +0000 (09:51 +0100)]
multi-tenant: fix coverity warning
Rework locking logic to avoid the following coverity warning.
** CID 1591966: Concurrent data access violations (MISSING_LOCK)
/src/detect-engine-loader.c: 475 in DetectLoadersSync()
474 SCCtrlMutexLock(loader->tv->ctrl_mutex);
>>> CID 1591966: Concurrent data access violations (MISSING_LOCK)
>>> Accessing "loader->tv" without holding lock "DetectLoaderControl_.m". Elsewhere, "DetectLoaderControl_.tv" is written to with "DetectLoaderControl_.m" held 1 out of 1 times (1 of these accesses strongly imply that it is necessary).
475 pthread_cond_broadcast(loader->tv->ctrl_cond);
476 SCCtrlMutexUnlock(loader->tv->ctrl_mutex);
Jason Ish [Tue, 13 Feb 2024 16:08:37 +0000 (10:08 -0600)]
dependabot: disable rust checks
As we don't have a Cargo.toml and a Cargo.lock, dependabot for Rust
hasn't been working correctly. Disable, as we now have our own cargo
audit and update workflows.
Multiple uploads can no longer use the same name, so give the cbindgen
artifact its own name of "cbindgen". Requires an additional download
for each build depending on this cbindgen artifact.
Jason Ish [Mon, 12 Feb 2024 06:02:32 +0000 (00:02 -0600)]
github-ci: move centos-7 build to its own workflow
CentOS 7 requires older actions due to newer GitHub actions depending
on a newer glibc. So move to its own workflow file so the main builds
can move forward to newer versions of actions.
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.
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
Instead of just setting the old transactions to a drop state so
that they get later cleaned up by Suricata, fail creating new ones.
This is because one call to app-layer parsing can create many
transactions, and quadratic complexity could happen in one
single app-layer parsing because of find_or_create_tx
Philippe Antoine [Tue, 17 Oct 2023 20:01:27 +0000 (22:01 +0200)]
pgsql: parse auth message within its bound
If the next PDU is already in the slice next, do not use it and
restrict ourselves to the length of this PDU.
Avoids overconsumption of memory by quadratic complexity, when
having many small PDUS in one big chunk being parsed
detect: fixes use-after-free with http.request_header
Ticket: #6441
This keyword and the response one use a multiple inspection buffer.
But the different instances point to the same memory address
that comes from HttpHeaderGetBufferSpace and is not owned
by the transaction, and is rebuilt, which is a functional
bug in itself.
As it gets crafted, it can get reallocated if one header
is over 1024 bytes, while the previous freed pointer will still get
used for the previous headers.
Philippe Antoine [Tue, 14 Nov 2023 20:51:37 +0000 (21:51 +0100)]
smtp: avoid creating empty transaction
Ticket: 6477
So as to avoid ending up with too many empty transactions.
This happens when Suricata sees a DATA command in the current
transaction but did not have a confirmation response for it.
Then, if Suricata receives another DATA command, it will
create another new transaction, even if the previous one
is empty. And so, a malicious client can create many empty
transactions by just sending a repeated amount of DATA commands
without having a confirmation code for them.
Suricata cannot use state->current_command == SMTP_COMMAND_DATA
to prevent this attack and needs to resort to a new boolean
is_data because the malicious client may send another dummy command
after each DATA command.
This patch leaves only one call to SMTPTransactionCreate
Philippe Antoine [Thu, 25 Jan 2024 15:01:14 +0000 (16:01 +0100)]
http2: handle reassembly for continuation frames
Ticket: 5926
HTTP2 continuation frames are defined in RFC 9113.
They allow header blocks to be split over multiple HTTP2 frames.
For Suricata to process correctly these header blocks, it
must do the reassembly of the payload of these HTTP2 frames.
Otherwise, we get incomplete decoding for headers names and/or
values while decoding a single frame.
Design is to add a field to the HTTP2 state, as the RFC states that
these continuation frames form a discrete unit :
> Field blocks MUST be transmitted as a contiguous sequence of frames,
> with no interleaved frames of any other type or from any other stream.
So, we do not have to duplicate this reassembly field per stream id.
Another design choice is to wait for the reassembly to be complete
before doing any decoding, to avoid quadratic complexity on partially
decoding of the data.
Present scenario
----------------
Currently, as a part of setting signature count per SGH, a max_idx is
passed which could be as high as the highest signature number (internal
ID).
Issue
-----
Not every SGH needs to evaluate all the signatures while setting
the signature count or while creating the match_array.
In a nonideal scenario, when say, there are 2 SGHs and one SGH has 2
signatures and the other one has 60k, given the current scheme of
evaluating max_idx, the max_idx will be set to 60k, and this shall
later be passed on to SigGroupHeadSetSigCnt or
SigGroupHeadBuildMatchArra which shall traverse over all the 60k sigs
for either SGHs.
Other info
----------
This is a very fast operation as the internal arithmetic is done
bitwise.
Patch
-----
The functions SigGroupHeadSetSigCnt and SigGroupHeadBuildMatchArray can
be optimized by storing the max signature id (internal) per SGH (which
also seemed to be the initial intention as per fn comments).
As a result of this, the sig_array is only walked up until the max sig
id of that respective SGH.
detect: remove unused port in SigGroupHeadInitData
port is not used and logically makes sense to not be in this struct as
this struct is already referenced by DetectPort itself as a part of
SigGroupHead.
projects / thirdparty / suricata.git / log
