Eric Leblond [Sun, 15 Oct 2023 13:39:40 +0000 (15:39 +0200)]
eve: revert ethernet addresses when needed
EVE logging has a direction parameter that can cause the logging
of an application layer to be done in a direction that is not linked
to the packet. As a result the source IP addres could be assigned the
MAC address of the destination IP and reverse.
This patch addresses this by propagating the direction to the ethernet
logging function and using it there to define the correct mapping.
Victor Julien [Wed, 29 May 2024 05:03:24 +0000 (07:03 +0200)]
threads: give threads more time to get ready
In certain conditions, it can take a long time for threads to start up.
For example in af-packet, setting up the socket, rings, etc has been
observed to take close to half a second per thread, and since the
threads go one by one in a preset order, this means the start up can
take a lot of time if there are many threads. The old logic would just
allow a hard coded 60s. This was not always enough when the number of
threads was high.
This patch makes the wait time take the number of threads into account.
It adds a second of time budget to the base 60s for each thread.
So as an example, if a system has 112 af-packet threads, it would wait
172 seconds (60 + 112) for the threads to get ready.
Victor Julien [Mon, 27 May 2024 15:12:09 +0000 (17:12 +0200)]
threads: optimize start up check
When starting a large amount of threads, the loop was inefficient. It
would loop over the threads and if one wasn't yet ready it would sleep a
bit and then reevaluate all the threads. This reevaluation of threads
already checked was inefficient, and could lead to the time budget
running out.
This patch splits the check, and keeps track of the threads that have
already passed. This avoids the rescanning of already checked threads.
Shivani Bhardwaj [Wed, 28 Feb 2024 14:29:04 +0000 (19:59 +0530)]
detect/port: remove SigGroupHead* ops
The functions in detect-engine-port.c are only being used at the time of
parsing the ports from rules initially. Since there are no SGHs at that
point, remove the ops related to them too.
Shivani Bhardwaj [Mon, 25 Mar 2024 13:38:31 +0000 (19:08 +0530)]
detect/port: handle range and upper boundary ports
So far, if a port was found to be single which was earlier a part of the
range, port + 1 was added to the list to honor the range that it was a
part of. But, this is incorrect in case the port is 65535 or if the port
was found to be of range when it was earlier a single port.
If a port point is single but later on also a part of a range, it ends
up only creating the port groups for single points and not the range.
Fix it by adding the port next to current single one to unique points
and marking it a range port.
Victor Julien [Mon, 26 Feb 2024 16:08:21 +0000 (21:38 +0530)]
detect/port: use qsort instead of insert sort
Instead of using in place insertion sort on linked list based on two
keys, convert the linked list to an array, perform sorting on it using
qsort and convert it back to a linked list. This turns out to be much
faster.
Shivani Bhardwaj [Wed, 21 Feb 2024 06:42:30 +0000 (12:12 +0530)]
detect/port: merge port ranges for same signatures
To avoid getting multiple entries in the final port list and to also
make the next step more efficient by reducing the size of the items to
traverse over.
Shivani Bhardwaj [Tue, 20 Feb 2024 16:22:38 +0000 (21:52 +0530)]
detect/port: create list of small port ranges
Using the unique port points, create a list of small port ranges which
contain the DetectPort objects and the designated SGHs found by finding
the overlaps with the existing ports and copying the SGHs accordingly.
Shivani Bhardwaj [Fri, 16 Feb 2024 09:18:46 +0000 (14:48 +0530)]
detect/port: create a tree of given ports
After all the SGHs have been appropriately copied to the designated
ports, create an interval tree out of it for a faster lookup when later
a search for overlaps is made.
Shivani Bhardwaj [Fri, 16 Feb 2024 08:57:52 +0000 (14:27 +0530)]
detect/port: find unique port points
In order to create the smallest possible port ranges, it is convenient
to first have a list of unique ports. Then, the work becomes simple. See
below:
Given, a port range P1 = [1, 8]; SGH1
and another, P2 = [3, 94]; SGH2
right now, the code will follow a logic of recursively cutting port
ranges until we create the small ranges. But, with the help of unique
port points, we get, unique_port_points = [1, 3, 8, 94]
So, now, in a later stage, we can create the ranges as
[1, 2], [3, 7], [8, 8], [9, 94] and copy the designated SGHs where they
belong. Note that the intervals are closed which means that the range
is inclusive of both the points.
The final result becomes:
1. [1, 2]; SGH1
2. [3, 7]; SGH1 + SGH2
3. [8, 8]; SGH1 + SGH2
4. [9, 94]; SGH2
There would be 3 unique rule groups made for the case above.
Group 1: [1, 2]
Group 2: [3, 7], [8, 8]
Group 3: [9, 94]
Warning was:
src/util-port-interval-tree.c:50:1: warning: Either the condition 'tmp!=NULL' is redundant or there is possible null pointer dereference: tmp. [nullPointerRedundantCheck]
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Assuming that condition 'tmp!=NULL' is not redundant
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Null pointer dereference
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: warning: Either the condition 'oleft!=NULL' is redundant or there is possible null pointer dereference: oleft. [nullPointerRedundantCheck]
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Assuming that condition 'oleft!=NULL' is not redundant
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Null pointer dereference
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: warning: Either the condition 'oright!=NULL' is redundant or there is possible null pointer dereference: oright. [nullPointerRedundantCheck]
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Assuming that condition 'oright!=NULL' is not redundant
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Null pointer dereference
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: warning: Either the condition 'left!=NULL' is redundant or there is possible null pointer dereference: left. [nullPointerRedundantCheck]
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Assuming that condition 'left!=NULL' is not redundant
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
src/util-port-interval-tree.c:50:1: note: Null pointer dereference
IRB_GENERATE(PI, SCPortIntervalNode, irb, SCPortIntervalCompareAndUpdate);
^
Shivani Bhardwaj [Fri, 16 Feb 2024 08:07:23 +0000 (13:37 +0530)]
util/interval-tree: add utility fns
Add new utility files to deal with the interval trees. These cover the
basic ops:
1. Creation/Destruction of the tree
2. Creation/Destruction of the nodes
It also adds the support for finding overlaps for a given set of ports.
This function is used by the detection engine is the Stage 2 of
signature preparation.
Shivani Bhardwaj [Mon, 29 Jan 2024 06:08:51 +0000 (11:38 +0530)]
interval-tree: add augmentation fns to the tree
An interval tree uses red-black tree as its base data structure and
follows all the properties of a usual red-black tree. The additional
params are:
1. An interval such as [low, high] per node.
2. A max attribute per node. This attribute stores the maximum high
value of any subtree rooted at this node.
At any point in time, an inorder traversal of an interval tree should
give the port ranges sorted by the low key in ascending order.
This commit modifies the IRB_AUGMENT macro and it's call sites to make
sure that on every insertion, the max attribute of the tree is properly
updated.
Victor Julien [Fri, 12 Jan 2024 07:03:06 +0000 (12:33 +0530)]
detect/engine: fix whitelisting check
In the commit 4a00ae607, the whitelisting check was updated in a quest
to make use of the conditional better but it made things worse as every
range would be whitelisted as long as it had any of the default
whitelisted port which is very common.
Philippe Antoine [Thu, 18 Apr 2024 09:54:34 +0000 (11:54 +0200)]
detect: use direction-based tx for app-layer logging
When we only have stream matches.
Ticket: 6846
This solves the case where another transaction was created
by parsing data in the other direction, before running the
detection.
Like
1. get data in direction 1
2. acked data: parse it, but do not run detection in dir 1
3. other data in direction 2
4. other data acked : parse it and create new tx,
then run detection for direction 1 with data from first packet
Callers to these allocators often use ``sc_errno`` to provide context of
the error. And in the case of the above bug, they return ``sc_errno``,
but as it has not been set ``sc_errno = 0; == SC_OK``.
This patch simply sets this variable to ensure there is context provided
upon error.
Victor Julien [Tue, 21 May 2024 12:13:11 +0000 (14:13 +0200)]
pcap-log: use correct pkthdr size for limit enforcement
The on-disk pcap pkthdr is 16 bytes. This was calculated using
`sizeof(struct pcap_pkthdr)`, which is 24 bytes on 64 bit Linux. On
Macos, it's even worse, as a comment field grows the struct to 280
bytes.
Victor Julien [Mon, 20 May 2024 20:09:06 +0000 (22:09 +0200)]
time: only consider packet threads
In offline mode, a timestamp is kept per thread, and the lowest
timestamp of the active threads is used. This was also considering the
non-packet threads, which could lead to the used timestamp being further
behind that needed. This would happen at the start of the program, as
the non-packet threads were set up the same way as the packet threads.
This patch both no longer sets up the timestamp for non-packet threads
as well as not considering non-packet threads during timestamp
retrieval.
Jeff Lucovsky [Sat, 16 Mar 2024 12:58:11 +0000 (08:58 -0400)]
profiling/rules: Improve dynamic rule handling
Issue: 6861
Without this commit, disabling rule profiling via suricatasc's command
'ruleset-profile-stop' may crash because profiling_rules_entered becomes
negative.
This can happen because
- There can be multiple rules evaluated for a single packet
- Each rule is profiled individually.
- Starting profiling is gated by a configuration setting and rule
profiling being active
- Ending profiling is gated by the same configuration setting and
whether the packet was marked as profiling.
The crash can occur when a rule is being profiled and rule profiling
is then disabled after one at least one rule was profiled for the packet
(which marks the packet as being profiled).
In this scenario, the value of profiling_rules_entered was
not incremented so the BUG_ON in the end profiling macro trips
because it is 0.
The changes to fix the problem are:
- In the profiling end macro, gate the actions taken there by the same
configuration setting and use the profiling_rues_entered (instead of
the per-packet profiling flag). Since the start and end macros are
tightly coupled, this will permit profiling to "finish" if started.
- Modify SCProfileRuleStart to only check the sampling values if the
packet hasn't been marked for profiling already. This change makes all
rules for a packet (once selected) to be profiled (without this change
sampling is applied to each *rule* that applies to the packet.
Philippe Antoine [Fri, 17 May 2024 07:39:52 +0000 (09:39 +0200)]
http: fix nul deref on memcap reached
HttpRangeOpenFileAux may return NULL in different cases, including
when memcap is reached.
But is only caller did not check it before calling HttpRangeAppendData
which would dereference the NULL value.
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`
Victor Julien [Thu, 25 Apr 2024 15:07:52 +0000 (17:07 +0200)]
detect/iprep: allow 0 as a reputation value
Rules would allow checking against value 0, but internally the value
was used to indicate "no value". To address this, the internals now
return negative values for not found. This way value 0 can be fully
supported.
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.