Victor Julien [Tue, 4 Jun 2024 12:43:22 +0000 (14:43 +0200)]
defrag: don't use completed tracker
When a Tracker is set up for a IPID, frags come in for it and it's
reassembled and complete, the `DefragTracker::remove` flag is set. This
is mean to tell the hash cleanup code to recyle the tracker and to let
the lookup code skip the tracker during lookup.
A logic error lead to the following scenario:
1. there are sufficient frag trackers to make sure the hash table is
filled with trackers
2. frags for a Packet with IPID X are processed correctly (X1)
3. frags for a new Packet that also has IPID X come in quickly after the
first (X2).
4. during the lookup, the frag for X2 hashes to a hash row that holds
more than one tracker
5. as the trackers in hash row are evaluated, it finds the tracker for
X1, but since the `remove` bit is not checked, it is returned as the
tracker for X2.
6. reassembly fails, as the tracker is already complete
The logic error is that only for the first tracker in a row the `remove`
bit was checked, leading to reuse to a closed tracker if there were more
trackers in the hash row.
Philippe Antoine [Thu, 23 May 2024 12:05:08 +0000 (14:05 +0200)]
rust: return empty slice without using from_raw_parts
As this triggers rustc 1.78
unsafe precondition(s) violated: slice::from_raw_parts requires
the pointer to be aligned and non-null,
and the total size of the slice not to exceed `isize::MAX`
Jeff Lucovsky [Thu, 7 Sep 2023 14:49:23 +0000 (10:49 -0400)]
detect/alert: Drop packet if rule is pkt only
This commit modifies the logic used to determine the disposition of a
flow/packet.
If the rule doesn't require a stream and only contains properties for
packet matching, then the alert is not marked as applying to the
flow and hence, the flow won't be dropped.
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.
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.
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 [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.
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: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
|
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
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
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.
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, 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.
Jeff Lucovsky [Thu, 3 Aug 2023 14:06:47 +0000 (10:06 -0400)]
detect/analysis: Move globals to engine ctx
Issue: 6239
This commit moves the global variables associated with engine analysis
into the detect engine context. Doing so provides encapsulation of the
analysis components as well as thread-safe operation in a multi-tenant
(context) deployment.
Jason Ish [Fri, 27 Oct 2023 16:19:31 +0000 (10:19 -0600)]
dns/eve: use default formats if formats is empty
If the configuration field "formats" is empty, DNS response records do
not have any relevant information other than that there was a
response, but not much about the response.
I'm pretty sure the intention here was to log the response details if
no formats were provided, which is what happens when the field is
commented out.
So if no formats are specified, use the default of all.
Our documentation was failing to build, seems connected to the new way
of indicating build options (cf
https://readthedocs.org/projects/suricata/builds/22112658/,
https://docs.readthedocs.io/en/stable/config-file/v2.html#build,
and https://docs.readthedocs.io/en/stable/config-file/v2.html#build-os).
Added the build.os required new field, and adjusted the way python
version is passed.
For the new configuration style for read the docs, one of the ways to
pass extra configuration for python is having a requirements file.
email_ctx->fields only gets populated when smtp.custom setting is on.
The fn EveEmailLogJSONCustom is called when either
1. smtp.extended setting is on or,
2. email_ctx->fields is populated which means smtp.custom setting is on
In case neither of these are set in suricata.yaml, no call should
ideally be made to the fn EveEmailLogJSONCustom.
However, it turns out that email_ctx->fields is unset and then set only
after the smtp config was found. This leads to email_ctx->fields
sometimes contain value even when no config was given to the smtp
section and can lead to unexpected output.
Fix this by using SCCalloc while initializing OutputJsonEmailCtx struct
instead of SCMalloc.
Victor Julien [Fri, 13 Oct 2023 11:47:05 +0000 (13:47 +0200)]
detect: inspect all packets in multi-layer tunneling
When the decoders encounter multiple layers of tunneling, multiple tunnel
packets are created. These are then stored in ThreadVars::decode_pq, where
they are processed after the current thread "slot" is done. However, due
to a logic error, the tunnel packets after the first, where not called
for the correct position in the packet pipeline. This would lead to these
packets not going through the FlowWorker module, so skipping everything
from flow tracking, detection and logging.
This would only happen for single and workers, due to how the pipelines
are constructed.
The "slot" holding the decoder, would contain 2 packets in
ThreadVars::decode_pq. Then it would call the pipeline on the first
packet with the next slot of the pipeline through a indirect call to
TmThreadsSlotVarRun(), so it would be called for the FlowWorker.
However when that first (the most inner) packet was done, the call
to TmThreadsSlotVarRun() would again service the ThreadVars::decode_pq
and process it, again moving the slot pointer forward, so past the
FlowWorker.
This patch addresses the issue by making sure only a "decode" thread
slot will service the ThreadVars::decode_pq, thus never moving the
slot past the FlowWorker.
Victor Julien [Mon, 15 May 2023 08:02:26 +0000 (10:02 +0200)]
flowworker: simplify pseudo packet use
Pseudo packets originating in the flow worker do not need to leave the
flow worker. Putting those in the ThreadVars::decode_pq will make them
be evaluated by the next steps in the pipeline, but those will all
ignore pseudo packets.
Instead, this patch returns them to the packet pool, while still honoring
the IPS verdict logic.
Philippe Antoine [Wed, 30 Aug 2023 19:35:08 +0000 (21:35 +0200)]
smtp: fix null deref with config option body md5
Ticket: #6279
If we have the smtp body beginning without headers, we need to
create the md5 context and right away and supply data to it.
Otherwise, on the next line being processed, md5_ctx will be
NULL but body_begin will have been reset to 0
Victor Julien [Wed, 2 Aug 2023 06:37:45 +0000 (08:37 +0200)]
var-names: reimplement var name handling
Implement a new design for handling var name id's. The old logic
was aware of detection engine versions and generally didn't work
well for multi-tenancy cases. Other than memory leaks and crashes,
logging of var names worked or failed based on which tenant was
loaded last.
This patch implements a new approach, where there is a global store
of vars and their id's for the lifetime of the program.
Overall Design:
Base Store: "base"
Used during keyword registration. Operates under lock. Base is shared
between all detect engines, detect engine versions and tenants.
Each variable name is ref counted.
During the freeing of a detect engine / tenant, unregistration decreases
the ref cnt.
Base has both a string to id and a id to string hash table. String to
id is used during parsing/registration. id to string during unregistration.
Active Store Pointer (atomic)
The "active" store atomic pointer points to the active lookup store. The call
to `VarNameStoreActivate` will build a new lookup store and hot swap
the pointer.
Ensuring memory safety. During the hot swap, the pointer is replaced, so
any new call to the lookup functions will automatically use the new store.
This leaves the case of any lookup happening concurrently with the pointer
swap. For this case we add the old store to a free list. It gets a timestamp
before which it cannot be freed.
Free List
The free list contains old stores that are waiting to get removed. They
contain a timestamp that is checked before they are freed.
Arne Welzel [Sun, 20 Aug 2023 15:32:47 +0000 (17:32 +0200)]
community-id: Fix IPv6 address sorting not respecting byte order
When comparing IPv6 addresses based on uint32_t chunks, one needs to
apply ntohl() conversion to the individual parts, otherwise on little
endian systems individual bytes are compared in the wrong order.
Avoid this all and leverage memcmp(), it'll short circuit on the first
differing byte and its return values tells us which address sorts lower.
projects / thirdparty / suricata.git / log
