]> git.ipfire.org Git - thirdparty/suricata.git/log
thirdparty/suricata.git
6 months agoprotodetect: finish probing parser sooner 12463/head
Philippe Antoine [Thu, 16 Jan 2025 08:26:30 +0000 (09:26 +0100)] 
protodetect: finish probing parser sooner

Ticket: 7495

We want to finish also if we tested all the expected protocols
in mask, or if we tested even more.

There can be one more protocol coming from pe0, which can be
the protocol already found in the other direction.

(cherry picked from commit b5094b00b62ada4d8825f3c45898fc4c5e9d5b1f)

6 months agoflow/var: Release key storage 12392/head 12404/head
Jeff Lucovsky [Sun, 22 Dec 2024 13:04:58 +0000 (08:04 -0500)] 
flow/var: Release key storage

Issue: 7466

This commit releases the memory for the flow variable "key" when
the flow variable is of type string. The key is allocated in the Lua
extension logic.

(cherry picked from commit 2d9df5a1aea4407846ce7e7e2fac58f65eb19073)

6 months agolog/file: Ensure file ctx pointer is returned . 12391/head
Jeff Lucovsky [Sat, 11 Jan 2025 14:23:50 +0000 (09:23 -0500)] 
log/file: Ensure file ctx pointer is returned .

The fix for issue 7447 introduced an error with threaded eve output.

The changes that were committed for that issue mishandled the return
value when a file is being opened for the 2nd or higher time.

Instead of returning the existing file context, null was returned.

(cherry picked from commit 1d996c5aed820c34add5693a7a1bd4be00a1e304)

6 months agooutput/log: Remove extraneous error message
Jeff Lucovsky [Thu, 12 Dec 2024 13:49:20 +0000 (08:49 -0500)] 
output/log: Remove extraneous error message

Issue: 7447

When the output file can't be opened, 2 error messages are displayed
for the same problem. The second message doesn't add value and lacks
context (error reason, e.g., "Permission denied").

Retaining the second message as a debug message.

Without this commit:

Error: logopenfile: Error opening file: "/home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.1.json": Permission denied [SCLogOpenFileFp:util-logopenfile.c:428]
Error: logopenfile: Unable to open slot 1 for file /home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.json [LogFileEnsureExists:util-logopenfile.c:737]
Error: runmodes: unable to initialize sub-module eve-log.stats [RunModeInitializeEveOutput:runmodes.c:692]

With commit:

Error: logopenfile: Error opening file: "/home/jlucovsky/src/jal/suricata-verify/tests/bug-5198/output/noperms/eve.1.json": Permission denied [SCLogOpenFileFp:util-logopenfile.c:428]
Error: runmodes: unable to initialize sub-module eve-log.stats [RunModeInitializeEveOutput:runmodes.c:692]
(cherry picked from commit d853972c744c789abbb61fa8e2fb567f4f6456ec)

6 months agooutput/log: Improve error handling
Jeff Lucovsky [Wed, 11 Dec 2024 13:26:01 +0000 (08:26 -0500)] 
output/log: Improve error handling

This commit improves error handling for cases when file(s) cannot be
opened.
- Return NULL if file object can't be opened
- checks whether the file object has been opened before
  dereferencing the per-file context.

Issue: 7447
(cherry picked from commit e72fc39f831e5c03d7884caa447eb758a345d1c8)

6 months agooutput/json: check 5-tuple values prior to logging 12355/head
Giuseppe Longo [Wed, 20 Mar 2024 17:31:42 +0000 (18:31 +0100)] 
output/json: check 5-tuple values prior to logging

This commit enhances the JSON output by introducing a feature for conditional port logging.
Now, port logging is dependent on the underlying protocol
(such as TCP, UDP, or SCTP), where port information is pertinent, while it
avoids unnecessary logging for protocols where a port is not utilized (e.g. ARP).

Furthermore, this update ensures that IP addresses and the protocol have
meaningful values set, rather than being logged as empty strings.

These changes will make each log entry more precise, eliminating cases where
5-tuple fields are empty or set to zero, indicating the absence of a field.

Backported to address ticket: https://redmine.openinfosecfoundation.org/issues/7460

Ticket: #7460

(cherry picked from commit a1c6328156ffa1e3d690fa180b0395f9dcd9f52e)

7 months agoflow/manager: fix multi instance row tracking 12302/head
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.

Bug: #7365.

Fixes: e9d2417e0ff3 ("flow/manager: adaptive hash eviction timing")
(cherry picked from commit ae072d5c07e0f5e02a8583127c7b3edd97f93d54)

7 months agoflow/manager: add fn docs
Shivani Bhardwaj [Thu, 13 Jun 2024 13:45:54 +0000 (19:15 +0530)] 
flow/manager: add fn docs

(cherry picked from commit f97b4ec1e80cbf1fb18c6ca34dddbf1b43b03cbb)

7 months agostream: apply clang formatting 12274/head 12283/head
Shivani Bhardwaj [Fri, 13 Dec 2024 06:45:22 +0000 (12:15 +0530)] 
stream: apply clang formatting

7 months agoversion: start development towards 7.0.9
Shivani Bhardwaj [Fri, 13 Dec 2024 06:10:47 +0000 (11:40 +0530)] 
version: start development towards 7.0.9

7 months agorelease: 7.0.8; update changelog suricata-7.0.8
Shivani Bhardwaj [Thu, 12 Dec 2024 10:09:00 +0000 (15:39 +0530)] 
release: 7.0.8; update changelog

7 months agostream: mark urgent experimental; set safe defaults
Victor Julien [Wed, 11 Dec 2024 19:51:58 +0000 (20:51 +0100)] 
stream: mark urgent experimental; set safe defaults

Uncomment in default config. This will make the policy "inline",
which is the same behavior as prior to the urgent policy support.

Add line to docs that this is an experimental feature.

7 months agodoc/userguide: document TCP urgent policy
Victor Julien [Wed, 11 Dec 2024 09:09:19 +0000 (10:09 +0100)] 
doc/userguide: document TCP urgent policy

(cherry picked from commit d11e8a8ee7fb2cb2da0567de16bde344e1313f36)

7 months agoeve/flow: add per flow TCP oob urg data counter
Victor Julien [Tue, 10 Dec 2024 09:16:51 +0000 (10:16 +0100)] 
eve/flow: add per flow TCP oob urg data counter

If TCP urgent handling is set to the OOB processing, the number of OOB
bytes is tracked for SEQ offset calculations. If this offset is
non-zero, add the field to the flow record.

Ticket: #7411.
(cherry picked from commit 779f9d8ba35c3f9b5abfa327d3a4209861bd2eb8)

7 months agostream: add TCP urgent handling options
Victor Julien [Thu, 10 Oct 2024 14:12:09 +0000 (16:12 +0200)] 
stream: add TCP urgent handling options

TCP urgent handling is a complex topic due to conflicting RFCs and
implementations.

Until now the URG flag and urgent pointer values were simply ignored,
leading to an effective "inline" processing of urgent data. Many
implementations however, do not default to this behavior.

Many actual implementations use the urgent mechanism to send 1 byte of
data out of band to the application.

Complicating the matter is that the way the urgent logic is handled is
generally configurable both of the OS and the app level. So from the
network it is impossible to know with confidence what the settings are.

This patch adds the following policies:

`stream.reassembly.urgent.policy`:

- drop: drop URG packets before they affect the stream engine

- inline: ignore the urgent pointer and process all data inline

- oob (out of band): treat the last byte as out of band

- gap: skip the last byte, but do no adjust sequence offsets, leading to
       gaps in the data

For the `oob` option, tracking of a sequence number offset is required,
as the OOB data does "consume" sequence number space. This is limited to
64k. For this reason, there is a second policy:

`stream.reassembly.urgent.oob-limit-policy`:

- drop: drop URG packets before they affect the stream engine

- inline: ignore the urgent pointer and process all data inline

- gap: skip the last byte, but do no adjust sequence offsets, leading to
       gaps in the data

Bug: #7411.
(cherry picked from commit 6882bcb3e51bd3cf509fb6569cc30f48d7bb53d7)

7 months agostream: remove unused function argument
Victor Julien [Thu, 5 Dec 2024 09:00:14 +0000 (10:00 +0100)] 
stream: remove unused function argument

Sequence number is taken from seg, not the func arg.

(cherry picked from commit 4c1554f4f690a7e89c4190b0fa3f31a19d0bbedc)

7 months agodecode/tcp: count urg flag
Victor Julien [Thu, 10 Oct 2024 12:56:21 +0000 (14:56 +0200)] 
decode/tcp: count urg flag

(cherry picked from commit ac02a71479c06d06a92683274987f7f898fb2c1c)

7 months agodns: provide events for recoverable parse errors
Jason Ish [Fri, 1 Nov 2024 17:39:23 +0000 (11:39 -0600)] 
dns: provide events for recoverable parse errors

Add events for the following resource name parsing issues:

- name truncated as its too long
- maximum number of labels reached
- infinite loop

Currently these events are only registered when encountered, but
recoverable. That is where we are able to return some of the name,
usually in a truncated state.

As name parsing has many code paths, we pass in a pointer to a flag
field that can be updated by the name parser, this is done in
addition to the flags being set on a specific name as when logging we
want to designate which fields are truncated, etc. But for alerts, we
just care that something happened during the parse. It also reduces
errors as it won't be forgotten to check for the flags and set the
event if some new parser is written that also parses names.

Ticket: #7280

(cherry picked from commit 19cf0f81335d9f787d587450f7105ad95a648951)

7 months agoeve/dns: add truncation flags for fields that are truncated
Jason Ish [Thu, 31 Oct 2024 21:46:35 +0000 (15:46 -0600)] 
eve/dns: add truncation flags for fields that are truncated

If rrname, rdata or mname are truncated, set a flag field like
'rrname_truncated: true' to indicate that the name is truncated.

Ticket: #7280

(cherry picked from commit 37f4c52b22fcdde4adf9b479cb5700f89d00768d)

7 months agodns: truncate names larger than 1025 characters
Jason Ish [Thu, 31 Oct 2024 21:40:40 +0000 (15:40 -0600)] 
dns: truncate names larger than 1025 characters

Once a name has gone over 1025 chars it will be truncated to 1025
chars and no more labels will be added to it, however the name will
continue to be parsed up to the label limit in attempt to find the end
so parsing can continue.

This introduces a new struct, DNSName which contains the name and any
flags which indicate any name parsing errors which should not error
out parsing the complete message, for example, infinite recursion
after some labels are parsed can continue, or truncation of name where
compression was used so we know the start of the next data to be
parsed.

This limits the logged DNS messages from being over our maximum size
of 10Mb in the case of really long names.

Ticket: #7280

(cherry picked from commit 3a5671739f5b25e5dd973a74ca5fd8ea40e1ae2d)

7 months agoutil/streaming-buffer: add extra safety check
Philippe Antoine [Thu, 21 Nov 2024 14:20:44 +0000 (15:20 +0100)] 
util/streaming-buffer: add extra safety check

Ticket: 7393

Check if GrowRegionToSize is called with an argument
trying to shrink the region size, and if so do nothing,
ie do not try to shrink, and just return ok.

This way, we avoid a buffer overflow from memeset using an
unsigned having underflowed.

(cherry picked from commit 9a53ec43b13f0039a083950511a18bf6f408e432)

7 months agoutil/streaming-buffer: check need to grow region
Philippe Antoine [Thu, 21 Nov 2024 14:17:21 +0000 (15:17 +0100)] 
util/streaming-buffer: check need to grow region

Ticket: 7393

As it was possible before earlier patches to get here
with mem_size lesser than start->buf_size,
which caused then an unsigned underflow and a buffer overflow.

(cherry picked from commit 8900041405dbb5f9584edae994af2100733fb4be)

7 months agoutil/streaming-buffer: fix regions intersection
Philippe Antoine [Thu, 21 Nov 2024 13:55:32 +0000 (14:55 +0100)] 
util/streaming-buffer: fix regions intersection

This was not a problem for current callers in Suricata,
as RegionsIntersect is only called through StreamingBufferInsertAt
which is only used by TCP...

And TCP uses default region gap = 256kb, and only calls
StreamingBufferInsertAt with a u16, so TCP never inserts a new
data that will strictly contain an existing region augmented
with region gap, which was the only case where RegionsIntersect
returned the wrong result, which could later lead to a
buffer overflow.

Ticket: 7393
(cherry picked from commit 282509f70c4ce805098e59535af445362e3e9ebd)

7 months agodoc: improve documentation about guess-applayer-tx 12270/head
Philippe Antoine [Wed, 11 Dec 2024 14:11:29 +0000 (15:11 +0100)] 
doc: improve documentation about guess-applayer-tx

Ticket: 7199

7 months agodetect: re-add app-layer to alerts on stream matches
Victor Julien [Wed, 11 Dec 2024 11:04:34 +0000 (12:04 +0100)] 
detect: re-add app-layer to alerts on stream matches

The `guess-applayer-tx` work also removed the stream match condition
for adding app-layer metadata to alerts. This is a behavior change that
is not desired at this point, so this commit reverts that part of the
changes.

We keep the exising logging of app-layer metadata if the match was in
the stream.

7 months agodetect: log app-layer metadata in alert with single tx
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.

(cherry picked from commit f2c37763149f16da17326cb313750052e5a2117d)

7 months agodetect: don't run pkt sigs on ffr pkts
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.

Ticket: #7318.
(cherry picked from commit 0e4faba79ae7e9fbe36815956cfd44af6d7f9378)

7 months agodpdk: set ice PMD RSS key length to 52 bytes for all DPDK versions 12262/head
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.

Ticket: 7445
(cherry picked from commit 18ab9a6ccd2392f7e94db9623bb3383731038937)

7 months agodetect/flowing: apply clang format changes 12244/head
Juliana Fajardini [Fri, 6 Dec 2024 13:26:41 +0000 (10:26 -0300)] 
detect/flowing: apply clang format changes

Related to
Task #7426

7 months agoflowint: add isnotset support
Juliana Fajardini [Wed, 4 Dec 2024 19:59:28 +0000 (16:59 -0300)] 
flowint: add isnotset support

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.

Task #7426

(cherry picked from commit 6e4a501e7c96a705df084fd1e74668bc70a00b89)

7 months agoeve/alert: enrich decoder event
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.

Ticket: #7433.
(cherry picked from commit 2fe2cf855310678141896195e3191fa582752e6c)

7 months agodetect: fix decoder only events
Victor Julien [Tue, 3 Dec 2024 15:51:26 +0000 (16:51 +0100)] 
detect: fix decoder only events

Add missing setup part of the decoder event sgh.

Bug: #7414.
(cherry picked from commit b23fa51e331141a2743ef7ded4ec01db951ec697)

7 months agorequires: add option to ignore unknown requirements
Jason Ish [Wed, 4 Dec 2024 17:28:17 +0000 (11:28 -0600)] 
requires: add option to ignore unknown requirements

The new behavior in 8, and backported is to treat unknown requirements
as unsatisfied requirements.

For 7.0.8, add a configuration option, "ignore-unknown-requirements"
to completely ignore unknown requirements, effectively treating them
as available.

Ticket: #7434

7 months agorequires: treat unknown requires keywords as unmet requirements
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.

Ticket: #7418
(cherry picked from commit 820a3e51b752867da1322f29d542e5844bb6e727)

8 months agorust: remove unnecessary lifetimes 12223/head 12231/head
Jason Ish [Wed, 4 Dec 2024 18:31:37 +0000 (12:31 -0600)] 
rust: remove unnecessary lifetimes

Fix provided by cargo clipy --fix.

Backport of 7bdbe7ed32d220abae62c0fc6ed8fcbeba886454.

8 months agorust/smb: fix rustdoc line
Jason Ish [Thu, 28 Nov 2024 15:54:12 +0000 (09:54 -0600)] 
rust/smb: fix rustdoc line

'///' style rust comments/documentation come before the item being
documented.

Spotted by clippy.

(cherry picked from commit aa6e94fc73be4843e4379d3539ba4fee38b63aaf)

8 months agorust: allow static_mut_refs for now
Jason Ish [Thu, 28 Nov 2024 16:06:29 +0000 (10:06 -0600)] 
rust: allow static_mut_refs for now

But we should fix all these soon.

(cherry picked from commit 4c12165816681e97959be3e1e1cf28c6195a84da)

8 months agorust: update num-derive to 0.4.2
Jason Ish [Wed, 4 Dec 2024 18:22:28 +0000 (12:22 -0600)] 
rust: update num-derive to 0.4.2

Includes Cargo.lock.in generated for just this single crate update
(minimal atomic update to keep Cargo.lock in sync with Cargo.toml).

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)

Backport of 8e408d37306e03d28888aa390e91fae09b28f392.

8 months agorust: sync Cargo.lock with Cargo.toml
Jason Ish [Wed, 4 Dec 2024 18:20:58 +0000 (12:20 -0600)] 
rust: sync Cargo.lock with Cargo.toml

This just updates some internal dependencies to our own crates in the
work-space.

8 months agoutil-buffer: expand by multiples of 4k 12204/head 12207/head
Jason Ish [Thu, 28 Nov 2024 15:20:18 +0000 (09:20 -0600)] 
util-buffer: expand by multiples of 4k

(cherry picked from commit 287d8360e7a7263e23194470ed59e08982f76e31)

8 months agooutput-json: drop eve records that are too long
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.

Ticket: #7300
(cherry picked from commit d39e42728a5b84d9cefbd4329034064d71c4e268)

8 months agomqtt: look for a reason code in all messages 12188/head
Philippe Antoine [Wed, 27 Nov 2024 15:08:05 +0000 (16:08 +0100)] 
mqtt: look for a reason code in all messages

instead of stopping on the first message if it does not
have a reason code, like conn and conn_ack

Was fixed in master by big refactor 0a1062fad2ece8f900113c381147e8e8bdd1c009

8 months agomqtt: double-check detection directions
Jason Ish [Tue, 26 Nov 2024 23:16:58 +0000 (17:16 -0600)] 
mqtt: double-check detection directions

Backport of commit 5d8252117f3a6643be5867c6f1f19caa316fd76d.

Ticket: 7323

8 months agomqtt: add reason code support for SUBACK
Sascha Steinbiss [Sun, 20 Oct 2024 09:27:51 +0000 (11:27 +0200)] 
mqtt: add reason code support for SUBACK

Ticket: #7323
(cherry picked from commit 377d4705e15aa54ae26176822b23eec0a98bbc59)

8 months agodetect/transform: handle overlapping dotprefix 12154/head 12160/head
Philippe Antoine [Tue, 26 Nov 2024 21:34:13 +0000 (22:34 +0100)] 
detect/transform: handle overlapping dotprefix

If there is a transform before dotprefix, it operates in place
in a single buffer, and must therefore use memmove instead of memcpy
to avoid UB.

Ticket: 7229

8 months agodetect/sip.stat_code: Correct sticky buffer name 12146/head
Jeff Lucovsky [Sat, 23 Nov 2024 14:03:29 +0000 (09:03 -0500)] 
detect/sip.stat_code: Correct sticky buffer name

Issue: 7295

The sticky buffer name was incorrectly set to method; this commit fixes
the name typo with stat_code.

8 months agodetect/transforms: write directly in inspect buffer 12125/head 12143/head
Philippe Antoine [Thu, 7 Nov 2024 16:49:45 +0000 (17:49 +0100)] 
detect/transforms: write directly in inspect buffer

instead of writing to a temporary buffer and then copying,
to save the cost of copying.

Ticket: 7229

Not a cherry-pick as we do not put the transforms in rust,
but just do this optimization in C

8 months agohttp2: rename event variant to match rule 12106/head
Jason Ish [Fri, 1 Nov 2024 15:58:33 +0000 (09:58 -0600)] 
http2: rename event variant to match rule

Rename InvalidHTTP1Settings to InvalidHttp1Settings so it gets the
expected name transformation of "invalid_http1_settings".

Ticket: #7361
(cherry picked from commit b1c26dccf3d361109ddf5b163a8a4268423f0a9c)

8 months agorules/modbus: remove rule for event that not longer exists
Jason Ish [Fri, 1 Nov 2024 15:46:58 +0000 (09:46 -0600)] 
rules/modbus: remove rule for event that not longer exists

The event "modbus.invalid_unit_identifier" no longer exists.

Ticket: #7361
(cherry picked from commit a55960e6ba78d9ea2b9adbdb269e776d4759e958)

8 months agorules/ike: fix ike event names that have changed
Jason Ish [Fri, 1 Nov 2024 15:46:11 +0000 (09:46 -0600)] 
rules/ike: fix ike event names that have changed

- weak_crypto_nodh -> weak_crypto_no_dh
- weak_crypto_noauth -> weak_crypto_no_auth

Ticket: #7361
(cherry picked from commit b44ba3224f774344ae4307ed82d98b8d92d6e7db)

8 months agorules/dns: fix dns event names that have changed
Jason Ish [Fri, 1 Nov 2024 15:45:24 +0000 (09:45 -0600)] 
rules/dns: fix dns event names that have changed

- not_a_request to not_request
- not_a_response to not_reponse

Ticket: #7361
(cherry picked from commit 833c7c64beae56e79bb1349933a9dc97b7bfe987)

8 months agorust/applayer: return -1 if event info was not found
Jason Ish [Fri, 1 Nov 2024 15:32:12 +0000 (09:32 -0600)] 
rust/applayer: return -1 if event info was not found

The returned event_id was being set to -1, but the function wasn't
returning -1 to indicate error.

Ticket: #7361

8 months agodetect/http: fix progress for headers keywords
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.

(cherry picked from commit cca59cdaa9dd896a92a2dd4f30a6ebd5ba2cd000)

8 months agosuricata/bpf: fix -Wshorten-64-to-32 warning 12096/head
Philippe Antoine [Mon, 4 Nov 2024 16:09:32 +0000 (17:09 +0100)] 
suricata/bpf: fix -Wshorten-64-to-32 warning

Ticket: 7366
Ticket: 6186
(cherry picked from commit dd71ef0af222a566e54dfc479dd1951dd17d7ceb)

9 months agorust/applayer: use c_int as return type for get_info_by_id 12059/head 12067/head
Jason Ish [Tue, 22 Oct 2024 16:46:13 +0000 (10:46 -0600)] 
rust/applayer: use c_int as return type for get_info_by_id

Rust was using i8 as the return type, while C uses int. As of Rust
1.82, the return value is turned to garbage over the FFI boundary.

Ticket: #7338
(cherry picked from commit 45384ef969d180d962f4b50f19556c5e2c5cfccc)

9 months agoeve/schema: add missing field "code" anomaly events
Jason Ish [Tue, 22 Oct 2024 16:26:22 +0000 (10:26 -0600)] 
eve/schema: add missing field "code" anomaly events

(cherry picked from commit b44fc62e6023784949f0a5ea0707e36e7f9614da)

9 months agostream/reassembly: optimize GetBlock 12041/head
Noah Liu [Mon, 23 Sep 2024 03:07:47 +0000 (11:07 +0800)] 
stream/reassembly: optimize GetBlock

Current GetBlock degrees the sbb search from rb tree to
line, which costs much cpu time, and could be replaced by
SBB_RB_FIND_INCLUSIVE. It reduces time complexity from
O(nlogn) to O(logn).

Ticket: 7208.
(cherry picked from commit 951bcff9702f1f39c00293d68d286fd77008b037)

9 months agoprofiling: Correct profiling data array size
Jeff Lucovsky [Wed, 16 Oct 2024 14:37:20 +0000 (10:37 -0400)] 
profiling: Correct profiling data array size

The profiling arrays are incorrectly sized by the number of thread
modules. Since they contain app-layer protocol data, they should be
sized by ALPROTO_MAX.

(cherry picked from commit 799822c3db46648edadae570404d74f6ec42efdd)

9 months agoschema/tls: add missing custom fields chain/cert
Juliana Fajardini [Fri, 27 Sep 2024 13:49:21 +0000 (10:49 -0300)] 
schema/tls: add missing custom fields chain/cert

Task #7287

(cherry picked from commit 2eefc4dac84a186b788b670a8c21f05417002952)

9 months agoconf: init parser after check with stat() 12005/head
Zemeteri Kamimizu [Thu, 3 Oct 2024 09:50:31 +0000 (12:50 +0300)] 
conf: init parser after check with stat()

Commit changes are made to avoid possible memory leaks. If the parser
is initialized before configuration file checking, there was no deinit
call before function return. Do check config file existance and type
before YAML parser initialization, so we don't need to deinit parser
before exiting the function.

Bug: #7302
(cherry picked from commit 87e6e9374ff40847d3e7408afcd404374b629337)

9 months agodetect: add new_de_ctx release in case of errors in initialization 12001/head
Zemeteri Kamimizu [Thu, 3 Oct 2024 10:05:55 +0000 (13:05 +0300)] 
detect: add new_de_ctx release in case of errors in initialization

Detect engine tenant reloading function hasn't got engine release call
under error label, so it is possible memory leak in case of errors in
further new detect engine initialization.

Bug: #7303
(cherry picked from commit adcac9ee0f8a20b68ca394ce0628063bc5c2ce7c)

9 months agogithub-ci: don't install cbindgen on centos stream 9 11982/head
Jason Ish [Wed, 16 Oct 2024 19:28:10 +0000 (13:28 -0600)] 
github-ci: don't install cbindgen on centos stream 9

Not needed as this is a build from make dist, and providing cbindgen
will force regeneration of the bindings.

9 months agogithub-ci: pin actions/upload-artifact to newer version
Jason Ish [Wed, 16 Oct 2024 18:59:30 +0000 (12:59 -0600)] 
github-ci: pin actions/upload-artifact to newer version

Required for the newer version of download-artifact in use due to sync
with master.

9 months agogithub-ci: sync almalinux builds with master
Jason Ish [Wed, 16 Oct 2024 18:57:50 +0000 (12:57 -0600)] 
github-ci: sync almalinux builds with master

9 months agogithub-ci: pin missed actions/checkout
Jason Ish [Wed, 16 Oct 2024 18:52:54 +0000 (12:52 -0600)] 
github-ci: pin missed actions/checkout

Pin actions missed as part of the sync with master as they most likely
don't exist in master.

9 months agogithub-ci: pin missed actions/cache
Jason Ish [Wed, 16 Oct 2024 18:49:48 +0000 (12:49 -0600)] 
github-ci: pin missed actions/cache

In job: ubuntu-22-04-cov-fuzz

9 months agogithub-ci: add ubuntu 24.04 cocci build
Jason Ish [Wed, 16 Oct 2024 18:47:19 +0000 (12:47 -0600)] 
github-ci: add ubuntu 24.04 cocci build

As cocci was removed from the Fedora builds due to issues in Cocci on
Fedora.

9 months agogithub-ci: remove Fedora 38, add Fedora 40
Jason Ish [Wed, 16 Oct 2024 18:45:16 +0000 (12:45 -0600)] 
github-ci: remove Fedora 38, add Fedora 40

Sync with master on Fedora builds.

9 months agogithub-ci: sync with master branch
Jason Ish [Wed, 16 Oct 2024 18:36:17 +0000 (12:36 -0600)] 
github-ci: sync with master branch

Mainly hashes of actions and other minor changes.

9 months agogithub-action: share cargo registry cache
Victor Julien [Thu, 10 Oct 2024 04:48:33 +0000 (06:48 +0200)] 
github-action: share cargo registry cache

(cherry picked from commit 97d525d18d8b648b9732560da1ddd5dc2b37eb44)

9 months agogithub-action: share cargo cache for windows jobs
Victor Julien [Wed, 9 Oct 2024 16:54:37 +0000 (18:54 +0200)] 
github-action: share cargo cache for windows jobs

Modification by Jason Ish: Keep removal of duplicate checkout action
while resolving conflict.

(cherry picked from commit d574d88bcafa630e0a112322e8c2dbf66d9ec49a)

9 months agogithub-ci: install prepared cbindgen on rpm distros
Jason Ish [Wed, 9 Oct 2024 16:22:35 +0000 (10:22 -0600)] 
github-ci: install prepared cbindgen on rpm distros

Currently cbindgen from system packages is broken, for now use the
cbindgen artifact we build.

(cherry picked from commit 09d604f7c3acd7d71e5cb57cb2aa263ad28c8466)

9 months agogithub-ci: break out cbindgen installation to action
Jason Ish [Wed, 9 Oct 2024 15:33:20 +0000 (09:33 -0600)] 
github-ci: break out cbindgen installation to action

(cherry picked from commit a5e13d0deeacf34402e3145f377987bba6c9eb26)

9 months agogithub-ci: run macos python jobs in virtualenv
Jason Ish [Mon, 7 Oct 2024 15:52:30 +0000 (09:52 -0600)] 
github-ci: run macos python jobs in virtualenv

With the latest brew changes, a virtualenv is required to install
pyyaml.

(cherry picked from commit 2b16369071da1976c853b7682eb062c5485338c0)

9 months agoyaml: Add check of allocation for node object
Alexey Simakov [Mon, 23 Sep 2024 18:24:48 +0000 (21:24 +0300)] 
yaml: Add check of allocation for node object

Fix potential dereference of nullptr in case
of unsuccessful allocation of memory for
list node

Bug: #7270
(cherry picked from commit c72404e5546635a6239bdd7302fe945f47ce1927)

9 months agohttp: fix condition check
Philippe Antoine [Tue, 8 Oct 2024 11:51:32 +0000 (13:51 +0200)] 
http: fix condition check

Ticket: 7309

Do not use a constant expression in a condition

(cherry picked from commit 76527dde79a5825299c9534e8da1fabb7fb4279e)

9 months agotemplate: remove -rust references 11931/head
Philippe Antoine [Wed, 9 Oct 2024 12:55:54 +0000 (14:55 +0200)] 
template: remove -rust references

Ticket: 7315

Completes commit 4a7567b3f04075f02543762717dbff9dd5b5c1f3

Allows keyword template.buffer to work properly when template
protocol is enabled

(cherry picked from commit 58556b7f8b65b26f97e4e8a95e95ce304fded6c8)

10 months agoutil/thash: fix formatting 11849/head 11851/head
Shivani Bhardwaj [Tue, 1 Oct 2024 11:03:39 +0000 (16:33 +0530)] 
util/thash: fix formatting

10 months agoversion: start development towards 7.0.8
Shivani Bhardwaj [Tue, 1 Oct 2024 11:00:04 +0000 (16:30 +0530)] 
version: start development towards 7.0.8

10 months agorelease: 7.0.7; update changelog suricata-7.0.7
Shivani Bhardwaj [Tue, 1 Oct 2024 06:08:36 +0000 (11:38 +0530)] 
release: 7.0.7; update changelog

10 months agodetect: pseudo-packets inherit inspect flags from parent packet
Ilya Bakhtin [Sat, 31 Aug 2024 11:44:25 +0000 (13:44 +0200)] 
detect: pseudo-packets inherit inspect flags from parent packet

Instead of inheriting from flow, because encrypted protocols like TLS
and SSH may have just set the flow flags to indicate rest of stream is
encrypted and does not need to run stream inspection. But inspection
still needs to be run detection on this last flushing packet.

Ticket: #7235.
(cherry picked from commit 976dec7f332624e31f57a936e6e6275c01dd8da5)

10 months agotls: do not break custom fields when enabling JA4
Sascha Steinbiss [Fri, 27 Sep 2024 09:24:26 +0000 (11:24 +0200)] 
tls: do not break custom fields when enabling JA4

Ticket: 7286

10 months agoutil/hash: use randomized hash algorithm
Philippe Antoine [Sun, 22 Sep 2024 19:38:50 +0000 (21:38 +0200)] 
util/hash: use randomized hash algorithm

For datasets and http ranges

Ticket: 7209

Prevents abusive hash collisions from known djb2 algorithm

(cherry picked from commit 26da953f6dad3793d29f27ce7ab6628a2db8f471)

10 months agohttp: have a headers limit
Philippe Antoine [Mon, 9 Sep 2024 07:34:39 +0000 (09:34 +0200)] 
http: have a headers limit

Ticket: 7191

So as to avoid quadratic complexity in libhtp.
Make the limit configurable from suricata.yaml,
and have an event when network traffic goes over the limit.

(cherry picked from commit bb714c917878ed13aab9e314a026f71570e84f37)

10 months agoja4: handles non alphanumeric alpn
Philippe Antoine [Mon, 23 Sep 2024 09:30:19 +0000 (11:30 +0200)] 
ja4: handles non alphanumeric alpn

Ticket: 7267

Follows more closely the specification :
https://github.com/FoxIO-LLC/ja4/blob/main/technical_details/JA4.md#alpn-extension-value

Also fixes the case with a single-char alpn.

(cherry picked from commit 1e152d1f1060a5afd39496d4f2556e7159cd22cc)

10 months agodefrag: fix off by one
Philippe Antoine [Mon, 15 Jul 2024 07:52:00 +0000 (09:52 +0200)] 
defrag: fix off by one

Ticket: 7067

This off by one could lead to an empty fragment being inserted
in the rb tree, which led to integer underflow

(cherry picked from commit 9203656496c4081260817cce018a0d8fd57869b5)

10 months agodetect/dataset: abort only in debug mode 11838/head
Philippe Antoine [Tue, 13 Aug 2024 14:53:53 +0000 (16:53 +0200)] 
detect/dataset: abort only in debug mode

Ticket: 7195
(cherry picked from commit c55c7d6c27f5386ad0297cf1113b291787c3f09e)

10 months agodetect/datasets: implement unset command
Philippe Antoine [Mon, 12 Aug 2024 07:54:43 +0000 (09:54 +0200)] 
detect/datasets: implement unset command

Ticket: 7195

Otherwise, Suricata aborted on such a rule

(cherry picked from commit e47598110a557bb9f87ea498d85ba91a45bb0cb6)

10 months agodatasets: restrict scope of macro/enum
Philippe Antoine [Thu, 1 Aug 2024 18:50:28 +0000 (20:50 +0200)] 
datasets: restrict scope of macro/enum

(cherry picked from commit 1352ed68c77dd9cd7d9fa708d68e8ec787958258)

10 months agostream: improve 3whs completed by ACK with data 11835/head
Victor Julien [Fri, 20 Sep 2024 07:54:57 +0000 (09:54 +0200)] 
stream: improve 3whs completed by ACK with data

If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.

This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: #7264.
(cherry picked from commit 45eb7e48817f0435040c3efc15e66383d78ed71f)

10 months agossl/ja3: better check for ja3 being enabled 11830/head
Philippe Antoine [Mon, 23 Sep 2024 12:03:04 +0000 (14:03 +0200)] 
ssl/ja3: better check for ja3 being enabled

Ticket: 6634

Completes commit 84735251b577a284af3795708786974fd30720b0

Avoids error log in Ja3BufferAddValue about NULL buffer

(cherry picked from commit 1d32f117456bb6d220ca3f7e99b4680ec7fbd549)

10 months agopgsql: trigger raw stream reassembly at tx completion 11827/head
Juliana Fajardini [Wed, 17 Jul 2024 20:22:04 +0000 (17:22 -0300)] 
pgsql: trigger raw stream reassembly at tx completion

Once we are tracking tx progress per-direction for PGSQL, we can trigger
the raw stream reassembly, for detection purposes, as soon as the
transactions are completed in the given direction.

Task #7000

(cherry picked from commit 2b1ad81cf587fb46392d751f740a55139795ec56)

10 months agopgsql: track transaction progress per direction
Juliana Fajardini [Thu, 29 Aug 2024 21:02:15 +0000 (18:02 -0300)] 
pgsql: track transaction progress per direction

PGSQL's current implementation tracks the transaction progress without
taking into consideration flow direction, and also has indirections
that make it harder to understand how the progress is tracked, as well
as when a request or response is actually complete.

This patch introduces tracking such progress per direction and adds
completion status per direction, too. This will help when triggering
raw stream reassembly or for unidirectional transactions, and may be
useful when we implement sub-protocols that can have multiple requests
per transaction, as well.

CancelRequests and TerminationRequests are examples of unidirectional
transactions. There won't be any responses to those requests, so we can
also mark the response side as done, and set their transactions as
completed.

Bug #7113

(cherry picked from commit dcccbb11963b350a5f81b47f53f64f4b4a082ce3)

10 months agopgsql: use new API style for extern C functions
Juliana Fajardini [Mon, 23 Sep 2024 18:56:49 +0000 (15:56 -0300)] 
pgsql: use new API style for extern C functions

(cherry picked from commit 2c7824a41f4c28895ce581b9b3e444f94f86a339)

10 months agopgsql: order StateProgress enum per direction
Juliana Fajardini [Wed, 28 Aug 2024 20:15:01 +0000 (17:15 -0300)] 
pgsql: order StateProgress enum per direction

Related to
Bug #7113

(cherry picked from commit 3ba179422d53b47ad074bb114bcaca9f1ab44703)

10 months agodcerpc: don't reuse completed tx
Victor Julien [Wed, 31 Jul 2024 11:58:29 +0000 (13:58 +0200)] 
dcerpc: don't reuse completed tx

In the DCERPC over TCP pcap, logging and rule matching is disrupted by adding a simple rule:

        alert tcp any any -> any any (flow:to_server,established; \
                dce_iface:5d2b62aa-ee0a-4a95-91ae-b064fdb471fc; dce_opnum:1; \
                dce_stub_data; content:"|42 77 4E 6F 64 65 49 50 2E 65 78 65 20|"; \
                content:!"|00|"; within:100; distance:97; sid:1; rev:1; )

Works: alert + 3 dcerpc records.

But when adding a trivial rule:

        alert tcp any any -> any any (flow:to_server,established; \
                dce_iface:5d2b62aa-ee0a-4a95-91ae-b064fdb471fc; dce_opnum:1; \
                dce_stub_data; content:"|42 77 4E 6F 64 65 49 50 2E 65 78 65 20|"; \
                content:!"|00|"; within:100; distance:97; sid:1; rev:1; )
        alert tcp any any -> any any (dsize:3; sid:2; rev:1; )

The alert for sid:1 disappears and also there is one dcerpc event less.

In the single rule case we can aggressively free the transactions, as there
is only an sgh in the toserver direction.

This means that when we encounter the 2nd REQUEST, the first 2 transactions
have already been processed and freed. So for the 2nd REQUEST we open a new
TX and run inspection and logging on it.

When the 2nd rule is added, it adds toclient sgh as well. This means that we
will now slightly delay the freeing of the transactions.

As a consequence we still have the TX for the first REQUEST when the 2nd REQUEST
is parsed. This leads to the 2nd REQUEST re-using the TX. Since the TX is
already marked as inspected, it means the toserver rule now no longer matches.
Also we're not logging this TX correctly now.

This commit fixes the issue by not "finding" a TX that as already been
marked complete in the search direction.

Bug #7187.

(cherry picked from commit 65392c02f5632d7a8faf30285fb8f5a946cbe9a4)

10 months agoeve/alert: fix validation check
Victor Julien [Wed, 20 Mar 2024 06:18:44 +0000 (07:18 +0100)] 
eve/alert: fix validation check

Bug: #6875.
(cherry picked from commit 0be3ba802e1433632e48a7160cc6ae9fbe4c239e)

10 months agomembuffer: annotate printf style function
Victor Julien [Fri, 24 Nov 2023 16:06:20 +0000 (17:06 +0100)] 
membuffer: annotate printf style function

(cherry picked from commit ff8597d50bebe92a9bf25df61d091f530c30791d)

10 months agoeve/alert: break out of payload logging callback if buffer is full
Victor Julien [Fri, 24 Nov 2023 15:02:14 +0000 (16:02 +0100)] 
eve/alert: break out of payload logging callback if buffer is full

(cherry picked from commit 926c6e3addad81cb696e478c8648abb4d7384fbe)

10 months agoeve/frame: break out of logging callback if buffer is full
Victor Julien [Fri, 24 Nov 2023 14:53:23 +0000 (15:53 +0100)] 
eve/frame: break out of logging callback if buffer is full

(cherry picked from commit 1dea4fea0b3989f6a76d5ea012588f32e20702ac)