]> git.ipfire.org Git - thirdparty/suricata.git/log
thirdparty/suricata.git
6 months agoldap: implement abandon request 12390/head 12396/head
Philippe Antoine [Thu, 9 Jan 2025 14:21:20 +0000 (15:21 +0100)] 
ldap: implement abandon request

Ticket: #7477

6 months agoldap: update ldap-parser crate
Philippe Antoine [Thu, 9 Jan 2025 14:07:19 +0000 (15:07 +0100)] 
ldap: update ldap-parser crate

so that we can implement abandon request support

Ticket: #7477

6 months agodetect: add vlan.layers keyword 12393/head
Alice Akaki [Wed, 8 Jan 2025 21:03:24 +0000 (17:03 -0400)] 
detect: add vlan.layers keyword

vlan.layers matches on the number of VLAN layers per packet
It is an unsigned 8-bit integer
Valid range = [0-3]
Supports prefiltering

Ticket: #1065

6 months agodetect: add vlan.id keyword
Alice Akaki [Thu, 7 Nov 2024 20:46:33 +0000 (16:46 -0400)] 
detect: add vlan.id keyword

vlan.id matches on Virtual Local Area Network IDs
It is an unsigned 16-bit integer
Valid range = [0-4095]
Supports prefiltering

Ticket: #1065

6 months agofuzz: remove unused macro 12383/head 12389/head
Philippe Antoine [Mon, 13 Jan 2025 12:39:21 +0000 (13:39 +0100)] 
fuzz: remove unused macro

6 months agofuzz: better init for signature parsing harness
Philippe Antoine [Fri, 10 Jan 2025 16:27:55 +0000 (17:27 +0100)] 
fuzz: better init for signature parsing harness

It needs app-layer registration for the names

6 months agoplugins: app-layer plugins
Philippe Antoine [Fri, 10 Jan 2025 15:57:51 +0000 (16:57 +0100)] 
plugins: app-layer plugins

Ticket: 5053

6 months agoapp-layer: make number of alprotos dynamic
Philippe Antoine [Mon, 11 Nov 2024 06:26:11 +0000 (07:26 +0100)] 
app-layer: make number of alprotos dynamic

Ticket: 5053

The names are now dynamically registered at runtime.
The AppProto alproto enum identifiers are still static for now.

This is the final step before app-layer plugins.

6 months agoapp-layer: move ALPROTO_FAILED definition
Philippe Antoine [Mon, 11 Nov 2024 06:21:03 +0000 (07:21 +0100)] 
app-layer: move ALPROTO_FAILED definition

Because some alprotos will remain static and defined as a constant,
such as ALPROTO_UNKNOWN=0, or ALPROTO_FAILED.

The regular already used protocols keep for now their static
identifier such as ALPROTO_SNMP, but this could be made more
dynamic in a later commit.

ALPROTO_FAILED was used in comparison and these needed to change to use
either ALPROTO_MAX or use standard function AppProtoIsValid

6 months agolua/datasets: factor out into its own file 12379/head
Jason Ish [Fri, 10 Jan 2025 23:23:35 +0000 (17:23 -0600)] 
lua/datasets: factor out into its own file

This is mainly for header sanitization to avoid pulling in detect
modules into the Lua sandbox definition.

Plus if we namespace modules with names like "suricata.dataset", it
probably makes sense to keep those modules in their own files.

6 months agolua/datasets: rework to be a "required" module
Jason Ish [Fri, 10 Jan 2025 21:40:26 +0000 (15:40 -0600)] 
lua/datasets: rework to be a "required" module

Re-work the Lua dataset lib to be required into a user script like:

    local dataset = require("suricata.data")

The main difference from loading it into global space is providing a
custom require function (as we removed it in the sandbox) and load it on
demand, returning a table to the module.

6 months agodetect/lua: add support for datasets
Victor Julien [Thu, 11 Apr 2024 14:10:34 +0000 (16:10 +0200)] 
detect/lua: add support for datasets

dataset.new
  create a dataset object in lua

<dataset>:get
  gets a reference to an existing dataset

<dataset>:add
  returns 1 if a new entry was added
  returns 0 if entry was already in the set

Example:
```
function init (args)
    local needs = {}
    needs["packet"] = tostring(true)
    return needs
end

function thread_init (args)
    conn_new, dataset.new()
    ret, err conn_new:get("conn-seen")
    if err ~= nil then
        SCLogWarning("dataset warning: " .. err)
        return 0
    end
end

function match (args)
    ipver, srcip, dstip, proto, sp, dp = SCFlowTuple()
    str = ipver .. ":<" .. srcip .. ">:<" .. dstip .. ">:" .. dp

    ret, err = conn_new:add(str, #str);
    if ret == 1 then
        SCLogInfo(str .. " => " .. ret)
    end
    return ret
end
```

Ticket: #7243.

6 months agodetect/lua: add thread_init
Victor Julien [Mon, 10 Jun 2024 18:34:40 +0000 (20:34 +0200)] 
detect/lua: add thread_init

Add optional `thread_init` function support. This function is called per
script, per thread to allow a user to initialize the lua state.

6 months agodetect/lua: improve stack dumping
Victor Julien [Wed, 6 Nov 2024 08:13:10 +0000 (09:13 +0100)] 
detect/lua: improve stack dumping

Only useful when debugging. Add a prefix and a stack size indication.

6 months agolog/file: Ensure file ctx pointer is returned . 12375/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.

6 months agoflow/pkts: allow matching on either direction 12373/head
Shivani Bhardwaj [Fri, 23 Aug 2024 06:57:35 +0000 (12:27 +0530)] 
flow/pkts: allow matching on either direction

For flow.bytes and flow.pkts keywords, allow matching in either
direction.

Feature 5646

6 months agodoc: update syntax for flow.pkts & flow.bytes
Shivani Bhardwaj [Tue, 8 Oct 2024 07:13:27 +0000 (12:43 +0530)] 
doc: update syntax for flow.pkts & flow.bytes

6 months ago flow/pkts: make syntax cleaner and compact
Shivani Bhardwaj [Fri, 29 Nov 2024 08:31:12 +0000 (14:01 +0530)] 
 flow/pkts: make syntax cleaner and compact

Currently, the syntax includes direction as a part of the keyword which
is against how usually keywords are done. By making direction as a
mandatory argument, it is possible to make the syntax cleaner and the
implementation more compact and easily extendable.
Pros:
- Registration table sees lesser entries if newer options are added
- If the options have to be extended, it can be done trivially
- In accordance w existing keyword implementations

Note that this commit also retains the existing direction specific
keywords.

6 months agostream: RST no longer acks all data 12371/head
Victor Julien [Fri, 29 Nov 2024 13:37:08 +0000 (14:37 +0100)] 
stream: RST no longer acks all data

Since forever (1578ef1e3e8a24d0cc615430c4e6bec1fefdad28) a valid RST
would update the internal `last_ack` representation to include all
unack'd data. This was originally done to make sure the unACK'd data was
inspected/processed at flow timeout.

It was observed however, that if GAPs existed in this unACK'd data, a
GAP could be reported in the stats and a GAP event would be raised. This
doesn't make sense, as missing segments in the unACK'd part of the
stream are completely normal. Segments simply do not all arrive in
order.

It turns out that the original behavior of updating `last_ack` to
include all unACK'd data is no longer needed.

For raw stream inspection, the detection engine will already include the
unACK'd data on flow end.

For app-layer updates the unACK'd data is often harmful, as the data
often has GAPs. Parser like the http parser would report these GAPs and
could also get confused about the post-GAP data being a new transaction
including a file. This lead to many reported errors and fantom txs and
files.

Since the GAP detection uses `last_ack` to determine GAPs, not moving
`last_ack` addresses the GAP false positives.

Ticket: #7422.

6 months agoflow/manager: improve doc; minor cleanup
Victor Julien [Fri, 20 Dec 2024 07:53:23 +0000 (08:53 +0100)] 
flow/manager: improve doc; minor cleanup

Explain meaning of `ts` in flow managers main loop.

6 months agoflow: skip lock for skippable flows
Victor Julien [Wed, 18 Sep 2024 20:19:17 +0000 (22:19 +0200)] 
flow: skip lock for skippable flows

Some checks can be done w/o holding a lock:
- seeing if the flow matches the packet
- if the hash row needs a timeout check

This patch skips taking a lock in these conditions.

6 months agothreads: align struct to CLS to avoid false sharing
Victor Julien [Wed, 18 Sep 2024 09:50:59 +0000 (11:50 +0200)] 
threads: align struct to CLS to avoid false sharing

Since `Thread` objects are part of a big allocation, more than one
Thread could be on a single cache line, leading to false sharing. Atomic
updates to one `Thread` could then lead to poor performance accessing
another `Thread`. Align to CLS (cache line size) to avoid this.

6 months agothreads: seal after setup; unseal at shutdown
Victor Julien [Wed, 18 Sep 2024 08:48:29 +0000 (10:48 +0200)] 
threads: seal after setup; unseal at shutdown

The idea of sealing the thread store is that its members can be accessed
w/o holding a lock to the whole store at runtime.

6 months agothreads: fine grained locking for Thread
Victor Julien [Tue, 17 Sep 2024 09:02:00 +0000 (11:02 +0200)] 
threads: fine grained locking for Thread

Until now many accesses to the Thread structure required taking a global
lock, leading to performance issues. In practice this only happened in
offline mode.

This patch adds a finer grained locking scheme. It assumes that the
Thread object itself cannot disappear, and adds a spinlock to protect
updates to the structure.

Additionally, the `pktts` field is made an atomic, so that it can be
read w/o taking the spinlock. Updates to it are still done under lock.

6 months agoflow/manager: in offline mode, use owning threads time
Victor Julien [Tue, 17 Sep 2024 18:52:14 +0000 (20:52 +0200)] 
flow/manager: in offline mode, use owning threads time

As this may mean that a threads ts is a bit ahead of the minimum time
the flow manager normally uses, it can evict flows a bit faster.

Ticket: #7455.

6 months agoflow/worker: improve flow timeout time accuracy
Victor Julien [Wed, 18 Sep 2024 10:03:46 +0000 (12:03 +0200)] 
flow/worker: improve flow timeout time accuracy

When timing out flows, use the timestamp from the "owning" thread. This
avoids problems with threads being out of sync with each other.

Ticket: #7455.

6 months agoflow: fix flow bucket timestamp optimization
Victor Julien [Mon, 16 Sep 2024 06:54:43 +0000 (08:54 +0200)] 
flow: fix flow bucket timestamp optimization

Flow Manager skips rows based on a minimized tracker that tracks the
next second at which the first flow may time out.

If seconds match a flow can still be timing out.

6 months agothreads: use sleeping threads for minimum time a bit longer
Victor Julien [Wed, 18 Sep 2024 10:21:42 +0000 (12:21 +0200)] 
threads: use sleeping threads for minimum time a bit longer

If a thread doesn't receive packets for a while the packet timestamp
will no longer be used to determine a reasonable minimum timestamp for
flow timeout handling.

To avoid issues with the minimum timestamp to be set a bit too
aggressively, increase the time a thread can be inactive.

6 months agotime: thread time update after flow update
Victor Julien [Sun, 15 Sep 2024 17:15:56 +0000 (19:15 +0200)] 
time: thread time update after flow update

The flow worker needs to get the opportunity to run the flow update
before globally making it's current timestamp available. This is to
avoid another thread using the time to evict the flow that is about to
get a legitimate update.

Ticket: #7455.

6 months agoflow: exact flow timeout
Victor Julien [Sun, 15 Sep 2024 10:20:21 +0000 (12:20 +0200)] 
flow: exact flow timeout

Use a more precise calculation for timing out flows, using both the
seconds and the micro seconds.

Ticket: #7455.

6 months agotime: getter for SCTime_t timestamp of a thread
Victor Julien [Wed, 18 Sep 2024 09:15:00 +0000 (11:15 +0200)] 
time: getter for SCTime_t timestamp of a thread

6 months agostream: rename tcp reuse flag
Victor Julien [Fri, 13 Dec 2024 09:35:13 +0000 (10:35 +0100)] 
stream: rename tcp reuse flag

Rename to be consistent with other naming:

STREAM_PKT_FLAG_TCP_PORT_REUSE -> STREAM_PKT_FLAG_TCP_SESSION_REUSE

6 months agoeve/stream: add tcp-session-reuse trigger
Victor Julien [Wed, 11 Sep 2024 19:11:09 +0000 (21:11 +0200)] 
eve/stream: add tcp-session-reuse trigger

Can be used to log when the tcp session reuse logic triggers.

6 months agoflow: improve thread safety during timeout checks
Victor Julien [Sat, 14 Sep 2024 19:26:45 +0000 (21:26 +0200)] 
flow: improve thread safety during timeout checks

Timeout checks would access certain fields w/o locking, which could lead
to thread safety issues.

6 months agoeve/flow: log tcp reuse as 'reason'
Victor Julien [Fri, 13 Sep 2024 18:26:53 +0000 (20:26 +0200)] 
eve/flow: log tcp reuse as 'reason'

Ticket: #7482.

6 months agounix/socket: cleanup start up logic
Victor Julien [Thu, 31 Oct 2024 16:41:26 +0000 (17:41 +0100)] 
unix/socket: cleanup start up logic

No longer init then deinit part of the engine at startup of the unix
socket mode.

6 months agothreads: include name in error message
Victor Julien [Thu, 31 Oct 2024 19:26:08 +0000 (20:26 +0100)] 
threads: include name in error message

When a thread fails to spawn, include the thread name in the error
message.

6 months agodns: improved handling of corrupt additionals
Philippe Antoine [Tue, 10 Sep 2024 13:31:00 +0000 (15:31 +0200)] 
dns: improved handling of corrupt additionals

Ticket: 7228

That means log the rest of queries and answers, even if the
final field additionals is corrupt.
Set an event in this case.

6 months agoldap: truly enforce max-tx 12358/head
Philippe Antoine [Tue, 17 Dec 2024 10:20:51 +0000 (11:20 +0100)] 
ldap: truly enforce max-tx

Ticket: 7465

If a bug chunk of data is parsed in one go, we could create many
transactions even if marking them as complete, and have
quadratic complexity calling find_request.

Proposed solution is to fail on creating a new transaction if too
many already exist.

6 months agoprofiling: use dynamic number of app-layer protos
Philippe Antoine [Tue, 29 Oct 2024 10:27:30 +0000 (11:27 +0100)] 
profiling: use dynamic number of app-layer protos

Ticket: 5053

6 months agoapp-layer/parser: use dynamic number of app-layer protos
Philippe Antoine [Mon, 2 Sep 2024 09:00:37 +0000 (11:00 +0200)] 
app-layer/parser: use dynamic number of app-layer protos

Ticket: 5053

6 months agofuzz: use dynamic number of app-layer protos
Philippe Antoine [Mon, 2 Sep 2024 09:13:21 +0000 (11:13 +0200)] 
fuzz: use dynamic number of app-layer protos

Ticket: 5053

delay after initialization so that StringToAppProto works

6 months agoapp-layer/stats: use dynamic number of app-layer protos
Philippe Antoine [Mon, 2 Sep 2024 08:52:04 +0000 (10:52 +0200)] 
app-layer/stats: use dynamic number of app-layer protos

Ticket: 5053

6 months agoutil: parenthesis for macro
Philippe Antoine [Mon, 2 Sep 2024 11:16:00 +0000 (13:16 +0200)] 
util: parenthesis for macro

so that we can use safely EXCEPTION_POLICY_MAX*sizeof(x)

6 months agoframes: use dynamic number of app-layer protos
Philippe Antoine [Mon, 2 Sep 2024 08:47:07 +0000 (10:47 +0200)] 
frames: use dynamic number of app-layer protos

Ticket: 5053

6 months agoprotodetect: use dynamic number of app-layer protos
Philippe Antoine [Mon, 2 Sep 2024 08:40:19 +0000 (10:40 +0200)] 
protodetect: use dynamic number of app-layer protos

for alproto_names

Ticket: 5053

6 months agoprotodetect: use dynamic number of app-layer protos
Philippe Antoine [Mon, 2 Sep 2024 08:35:11 +0000 (10:35 +0200)] 
protodetect: use dynamic number of app-layer protos

for expectation_proto

Ticket: 5053

6 months agoprotodetect: make expectation_proto part of AppLayerProtoDetectCtx
Philippe Antoine [Mon, 2 Sep 2024 08:28:04 +0000 (10:28 +0200)] 
protodetect: make expectation_proto part of AppLayerProtoDetectCtx

instead of a global variable.

For easier initialization with dynamic number of protocols

6 months agofuzz: use lower pcre limits
Philippe Antoine [Tue, 7 Jan 2025 15:55:35 +0000 (16:55 +0100)] 
fuzz: use lower pcre limits

to avoid timeouts

instead of forbidding pcre signatures on stream

Ticket: 4858

6 months agodetect: move fields around to fill memory holes
Philippe Antoine [Tue, 7 Jan 2025 12:57:25 +0000 (13:57 +0100)] 
detect: move fields around to fill memory holes

to make scan-build happy avoiding its warning :

Excessive padding in 'struct DetectEngineThreadCtx_'
(33 padding bytes, where 1 is optimal)

6 months agostats: remove unused pseudo_failed stat
Philippe Antoine [Tue, 17 Dec 2024 21:21:13 +0000 (22:21 +0100)] 
stats: remove unused pseudo_failed stat

6 months agosrc: remove unused struct fields
Philippe Antoine [Tue, 17 Dec 2024 20:36:32 +0000 (21:36 +0100)] 
src: remove unused struct fields

Even if they get defined

6 months agogithub-ci: update Fedora 39 jobs to 41
Victor Julien [Mon, 6 Jan 2025 13:42:31 +0000 (14:42 +0100)] 
github-ci: update Fedora 39 jobs to 41

6 months agodetect: improve tx_id guessing for unidirectional protocols
Philippe Antoine [Wed, 11 Dec 2024 13:50:49 +0000 (14:50 +0100)] 
detect: improve tx_id guessing for unidirectional protocols

So we get:
1. request arrives - buffered due to not ackd
2. response arrives, acks request - request is now parsed, response isn't
3. ack for response, response parsed. Then detect runs for request,
generates alert. We now have 2 txs. txid will be 0 from AppLayerParserGetTransactionInspectId

But txid 1 is unidirectional in the other way, so we can use txid 0
metadata for logging

Ticket: 7449

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

Ticket: 7199

6 months agoflow/var: Release key storage
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.

6 months agogithub-actions: bump actions/upload-artifact from 4.4.3 to 4.5.0 12339/head
dependabot[bot] [Wed, 1 Jan 2025 19:32:50 +0000 (19:32 +0000)] 
github-actions: bump actions/upload-artifact from 4.4.3 to 4.5.0

Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.4.3 to 4.5.0.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](https://github.com/actions/upload-artifact/compare/b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882...6f51ac03b9356f520e9adb1b1b7802705f340c2b)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
6 months agogithub-actions: bump github/codeql-action from 3.27.5 to 3.28.0
dependabot[bot] [Wed, 1 Jan 2025 19:32:47 +0000 (19:32 +0000)] 
github-actions: bump github/codeql-action from 3.27.5 to 3.28.0

Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.27.5 to 3.28.0.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Commits](https://github.com/github/codeql-action/compare/v3.27.5...v3.28.0)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
6 months agogithub-actions: bump codecov/codecov-action from 5.0.7 to 5.1.2
dependabot[bot] [Wed, 1 Jan 2025 19:32:31 +0000 (19:32 +0000)] 
github-actions: bump codecov/codecov-action from 5.0.7 to 5.1.2

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.0.7 to 5.1.2.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](https://github.com/codecov/codecov-action/compare/015f24e6818733317a2da2edd6290ab26238649a...1e68e06f1dbfde0e4cefc87efeba9e4643565303)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
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]

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

7 months agodoc: add guide for ticket title 12314/head
Shivani Bhardwaj [Mon, 25 Nov 2024 09:47:23 +0000 (15:17 +0530)] 
doc: add guide for ticket title

Explain with examples what a good ticket title looks like and
why is it important to have ticket titles convey the correct issues.

7 months agohttp2: complete json schema
Philippe Antoine [Wed, 18 Dec 2024 20:28:01 +0000 (21:28 +0100)] 
http2: complete json schema

git grep js.set_string rust/src/http2/logger.rs | grep '"' |
cut -d'"' -f2 | sort | uniq | while read i;
do echo -n $i; git grep $i etc/schema.json | wc -l; done

shows only has_multiple was missing

7 months agodoh2: really enforce 65K dns message limit
Philippe Antoine [Tue, 17 Dec 2024 09:30:45 +0000 (10:30 +0100)] 
doh2: really enforce 65K dns message limit

Ticket: #7464

7 months agorust/jsonbuilder: make set_uint generic over Into<u64>
Jason Ish [Wed, 18 Dec 2024 17:36:20 +0000 (11:36 -0600)] 
rust/jsonbuilder: make set_uint generic over Into<u64>

Allow `set_uint` to accept any number value that can be converted to a
u64. Prevents callers from having to do `as u64`.

This required fixing up any callers that used `.into()` to just pass in
their value without the into conversion.

Most calls using `as u64` can have that cast removed, with the exception
of `usize` values which must still be cast is conversion can't be
guaranteed to be non-fallible.

7 months agoci: run CodeQL only on python changes
Philippe Antoine [Tue, 17 Dec 2024 15:44:49 +0000 (16:44 +0100)] 
ci: run CodeQL only on python changes

Ticket: 7358

7 months agodoc/commit
Jeff Lucovsky [Tue, 19 Nov 2024 14:30:23 +0000 (09:30 -0500)] 
doc/commit

Describe how to use the git commit template. The template helps ensure
that the information needed for evaluation and context is included in
the commit message.

Ticket: <Redmine ticket number>

7 months agogit: Add commit template
Jeff Lucovsky [Tue, 16 Jul 2024 13:43:12 +0000 (09:43 -0400)] 
git: Add commit template

Issue: none

This commit adds a template that identifies commit message elements that
we find important. The Suricata development team requests that
contributions use the template to help improve commit messages. We
reserve the right to strictly enforce adherence to the template in the
future.

Configure git to use this template with:
    git config commit.template ..github/commit-template.txt

7 months agodns/probe: check that a request has at least one query
Philippe Antoine [Thu, 3 Oct 2024 13:16:44 +0000 (15:16 +0200)] 
dns/probe: check that a request has at least one query

Ticket: 7279

7 months agodns/probe: adds check for 0 records and big size
Philippe Antoine [Wed, 2 Oct 2024 12:30:15 +0000 (14:30 +0200)] 
dns/probe: adds check for 0 records and big size

Ticket: 7279

Make dns probing function stricter to avoid matching on non-DNS
on port 53 and later returning a app-layer error.

7 months agodns: fix clippy lint warnings 12287/head
Shivani Bhardwaj [Fri, 13 Dec 2024 07:03:08 +0000 (12:33 +0530)] 
dns: fix clippy lint warnings

Fix vector lint issues:
- same_item_push
- vec_init_then_push

7 months agogeneral/typo: Fix typo in stacksize msg 12282/head
Jeff Lucovsky [Fri, 13 Dec 2024 15:11:27 +0000 (10:11 -0500)] 
general/typo: Fix typo in stacksize msg

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

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.

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.

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.

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

7 months agoflow/timeout: add frame awareness
Victor Julien [Fri, 6 Dec 2024 13:15:34 +0000 (14:15 +0100)] 
flow/timeout: add frame awareness

If there are still frames in the flow, the detection and logging logic
needs to be able to evaluate them. To do this, make the flow timeout
logic aware of the frames. If frames still exist in a direction, trigger
a FFR packet to be created.

Ticket: #7440.

7 months agoeve/frame: require frame length to be known
Victor Julien [Fri, 6 Dec 2024 13:13:14 +0000 (14:13 +0100)] 
eve/frame: require frame length to be known

Or reach logging threshold.

Avoids logging too early.

Ticket: #7440.

7 months agoeve/frame: run logging for flow end packets
Victor Julien [Fri, 6 Dec 2024 13:11:38 +0000 (14:11 +0100)] 
eve/frame: run logging for flow end packets

If there are frames in the flow the flow manager will create flow
timeout packets to log the remaining frames. This requires the logger to
run for those flow timeout packets.

Ticket: #7440.

7 months agoeve/frame: remove unreachable if branch
Victor Julien [Fri, 6 Dec 2024 13:09:33 +0000 (14:09 +0100)] 
eve/frame: remove unreachable if branch

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

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

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

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.

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.

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

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

7 months agodetect: rename stream_log variables
Philippe Antoine [Mon, 2 Dec 2024 10:00:31 +0000 (11:00 +0100)] 
detect: rename stream_log variables

to better reflect their true meaning

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.

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

7 months agoapp-layer: track modified/processed txs
Philippe Antoine [Fri, 31 May 2024 08:39:16 +0000 (10:39 +0200)] 
app-layer: track modified/processed txs

To optimize detection, and logging, to avoid going through
all the live transactions when only a few were modified.

Two boolean fields are added to the tx data: updated_tc and ts
The app-layer parsers are now responsible to set these when
needed, and the logging and detection uses them to skip
transactions that were not updated.

There may some more optimization remaining by when we set
both updated_tc and updated_ts in functions returning
a mutable transaction, by checking if all the callers
are called in one direction only (request or response)

Ticket: 7087

7 months agorust/ftp: handle NULL inputs
Philippe Antoine [Tue, 10 Sep 2024 09:20:29 +0000 (11:20 +0200)] 
rust/ftp: handle NULL inputs

Completes Ticket 7013

7 months agosip: remove UPDATE method for detection 12245/head
Philippe Antoine [Tue, 29 Oct 2024 21:29:06 +0000 (22:29 +0100)] 
sip: remove UPDATE method for detection

As it is also used for HTTP/1
Remove it only for TCP and keep it for UDP.

Ticket: 7436

7 months agofuzz: simplify target for protocol detection
Philippe Antoine [Tue, 29 Oct 2024 21:28:18 +0000 (22:28 +0100)] 
fuzz: simplify target for protocol detection

As too many cases are found when splitting tcp payload

7 months agofuzz: better init for protocol detection
Philippe Antoine [Tue, 29 Oct 2024 21:26:37 +0000 (22:26 +0100)] 
fuzz: better init for protocol detection

Ticket: 7435

7 months agoflow/manager: add chunk debug output
Victor Julien [Wed, 4 Dec 2024 09:51:06 +0000 (10:51 +0100)] 
flow/manager: add chunk debug output

7 months agoflow/manager: fix multi instance row tracking
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")
7 months agodetect/engine/flowint: apply clang format changes 12235/head
Juliana Fajardini [Thu, 5 Dec 2024 01:50:09 +0000 (22:50 -0300)] 
detect/engine/flowint: 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