Commit Graph

15568 Commits (2dc39d31c669abcb38d57bce5cbd7764da53a4e9)
 

Author SHA1 Message Date
Arne Welzel b8b8aa69b4 stats: Do not expand dots of tm_name
When an interface with dots is used, per worker stats are nested by the
dot-separated-components of the interface due to the usage of
OutputStats2Json().

Prevent this by using OutputStats2Json() on a per-thread specific object
and setting this object into the threads object using the
json_object_set_new() which won't do the dot expansion.

This was tested by creating an interface with dots in the name
and checking the stats.

    ip link add name a.b.c type dummy

With Suricata 7.0.2, sniffing on the a.b.c interface results in the
following worker stats format:

    "threads": {
      "W#01-a": {
        "b": {
          "c": {
            "capture": {
              "kernel_packets": 0,

After this fix, the output looks as follows:

    "threads": {
      "W#01-a.b.c": {
        "capture": {
          "kernel_packets": 0,

Ticket: #6732
2 years ago
Simon Dugas 17f9d7aecc detect-engine-iponly: improve ip list performance
The runtime complexity of insertion sort is approx. O(h*n)^2 where
h is the size of the HOME_NET and n is the number of ip only rules
that use the HOME_NET.

Replacing this with qsort significantly improves rule load time when
a large HOME_NET is used in combination with a moderate amount of ip
only rules.
2 years ago
Philippe Antoine 63caa0b40a detect: respect directionality for filestore
Ticket: 6617

So that rules with keyword like `filestore:to_server,flow`
only store the files to server and not the ones to client...

Directionality only worked with the default scope, ie the
current file, and not the scope tx or scope flow.
For non-default scope, tx or flow, both directions were stored
whatever the directionality specified.

For these non-default scopes, this commit keeps a default
of both directions, but use only one direction if specified.

Need to split flag FLOWFILE_STORE per direction, so that Suricata
can retain this (optional) directional info from the filestore
keyword.

Fixes: 79499e4769 ("app-layer: move files into transactions")
2 years ago
Philippe Antoine 41a621178f ci: right sha for authors check 2 years ago
Victor Julien 2d7c3d8d59 multi-tenant: fix coverity warning
Rework locking logic to avoid the following coverity warning.

** CID 1591966:  Concurrent data access violations  (MISSING_LOCK)
/src/detect-engine-loader.c: 475 in DetectLoadersSync()

    474                     SCCtrlMutexLock(loader->tv->ctrl_mutex);
    >>>     CID 1591966:  Concurrent data access violations  (MISSING_LOCK)
    >>>     Accessing "loader->tv" without holding lock "DetectLoaderControl_.m". Elsewhere, "DetectLoaderControl_.tv" is written to with "DetectLoaderControl_.m" held 1 out of 1 times (1 of these accesses strongly imply that it is necessary).
    475                     pthread_cond_broadcast(loader->tv->ctrl_cond);
    476                     SCCtrlMutexUnlock(loader->tv->ctrl_mutex);

The warning itself is harmless.
2 years ago
Jeff Lucovsky f9a20dafc6 mqtt: Improve frame parsing w/mult. PDUs
This commit improves the mqtt parsing of frames to handle multiple PDUs.

Issue: 6592
2 years ago
dependabot[bot] fa98c48e65 github-actions: bump github/codeql-action from 2.24.0 to 3.24.1
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2.24.0 to 3.24.1.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Commits](https://github.com/github/codeql-action/compare/v2.24.0...v3.24.1)

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

Signed-off-by: dependabot[bot] <support@github.com>
2 years ago
Victor Julien 3c06457b74 detect/tls.certs: fix direction handling
Direction flag was checked against wrong field, leading to undefined behavior.

Bug: #6778.
2 years ago
Jason Ish 2242d10fa0 github-ci: fix authors check with special characters
Dependabot is always getting flagged as a new author even tho it uses
a consistent author of:

dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

But this doesn't work with plain grep. Fix by telling grep to treat
the value as a fixed string instead of a regular expression.
2 years ago
Jason Ish 5c686af149 dependabot: disable rust checks
As we don't have a Cargo.toml and a Cargo.lock, dependabot for Rust
hasn't been working correctly. Disable, as we now have our own cargo
audit and update workflows.
2 years ago
Jason Ish c7cb3e92a6 dependabot: ignore actions/{cache,checkout} v3
The CentOS 7 build requires older GitHub actions, try to make
dependabot ignore these older versions.
2 years ago
Jason Ish a87943d9bf github-ci: apply read-only permissions to more workflows
- authors.yml
- codeql.yml
- scan-build.yml
2 years ago
Victor Julien abbd507b5c security: update policy wrt CVE ID's
To match that we'll now request CVE ID's ourselves as well,
and we can do it for reported issues as well.

See also:
https://forum.suricata.io/t/security-new-cve-policy/4473
2 years ago
Lukas Sismis 356f9ffa13 doc: mention the limited number of RX/TX descriptors on Intel NICs
Ticket: 6748
2 years ago
Lukas Sismis c65ff35819 dpdk: max cache size should be lower than one of the constraints
Ticket: 6741
2 years ago
Lukas Sismis cc2eb2d8b7 dpdk: sanitize integer overflow in the configuration
Ticket: #6737
2 years ago
Philippe Antoine 3a7a4cd581 http: code simplification
removing function unused parameter tx_id in HTPFileOpen
And using directly tx instead of its id in HTPFileOpenWithRange
2 years ago
Philippe Antoine c99d93c257 app-layer/template: use a max number of txs
Ticket: 6773
2 years ago
Jeff Lucovsky 2a1a70b308 threads/mutex: Ensure mutex held before signaling
Ensure that the mutex protecting the condition variable is held before
signaling it. This ensures that the thread(s) awaiting the signal are
notified.

Issue: 6569
2 years ago
jason taylor e891ef3d4e doc: add pcap file logging variable details
Signed-off-by: jason taylor <jtfas90@gmail.com>
2 years ago
Daniel Olatunji f9a4e9c588 codeql: add security-extended query suite
Add the CodeQL security-extended suite to
the CodeQL workflow configuration.
2 years ago
dependabot[bot] 7881e85088
github-actions: bump github/codeql-action from 2 to 3
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2 to 3.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Commits](https://github.com/github/codeql-action/compare/v2...v3)

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

Signed-off-by: dependabot[bot] <support@github.com>
2 years ago
dependabot[bot] be07d96c3d github-actions: bump codecov/codecov-action from 3.1.1 to 4.0.1
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.1 to 4.0.1.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](d9f34f8cd5...e0b68c6749)

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

Signed-off-by: dependabot[bot] <support@github.com>
2 years ago
Jason Ish 7c98134624 github-ci: cancel previous job for all workflows
Previously only enabled in build.yml, apply cancen-in-progress to all
workflow files.
2 years ago
Jason Ish d5a3bfcab6 github-ci: don't depend on cbindgen when installed from package 2 years ago
Jason Ish 49834eabf1 github-ci: update actions/github-script 2 years ago
Jason Ish e786297497 github-ci: update actions/checkout 2 years ago
Jason Ish 32d55febed github-ci: update actions/cache 2 years ago
Jason Ish 5bfaeb3bf5 github-ci: update {download,upload} artifact actions
Multiple uploads can no longer use the same name, so give the cbindgen
artifact its own name of "cbindgen". Requires an additional download
for each build depending on this cbindgen artifact.
2 years ago
Jason Ish 8522256aaa github-ci: use all cores available
GitHub action Linux runners now have 4 cores, instead of hardcoding
the number, use nproc to determine how many cores are available and
use them.
2 years ago
Jason Ish 6922fef4ab github-ci: move centos-7 build to its own workflow
CentOS 7 requires older actions due to newer GitHub actions depending
on a newer glibc. So move to its own workflow file so the main builds
can move forward to newer versions of actions.
2 years ago
Jason Ish edfda9f69f rust: weekly cargo audit and update
Add GitHub actions to perform:

- cargo audit: catch new warnings in dependendent packages
- cargo update: catch updated dependencies that depend on a new MSRV
    than we use
2 years ago
Victor Julien 7956fa5242 multi-tenant: fix loader dead lock
A dead lock could occur at start up, where a loader thread would
get stuck on it's condition variable, while the main thread was
polling the loaders task results.

The vector to the dead lock is as follows:

main	                        loader
DetectEngineMultiTenantSetup
-DetectLoaderSetupLoadTenant
--DetectLoaderQueueTask
---lock loader
---add task
---unlock loader
	                        lock loader
	                        check/exec tasks
	                        unlock loader
---wake up threads
	                        lock ctrl mutx
	                        cond wait ctrl
	                        unlock ctrl
-DetectLoadersSync
--lock loader
--check tasks
--unlock loader

Between the main thread unlocking the loader and waking up the
threads, it is possible that the loader has already moved ahead
but not yet entered its conditional wait. The main thread sends
its condition signal, but since the loader isn't yet waiting on
it the signal is ignored. Then when the loader does enter its
conditional wait, the signal is not sent again.

This patch updates the logic to send signals much more often.
It also makes sure that the signal is sent under lock, as the
API requires.

Bug: #6766.
2 years ago
Jeff Lucovsky 9fe00ff710 config/jansson: Remove excess libjansson mentions
Issue: 6712

Remove multiple occurrences of libjansson installation packages.
2 years ago
Jeff Lucovsky ee6208be9d config/nss: Remove libnspr/libnss traces
Issue: 6712
2 years ago
Jeff Lucovsky 364adeeb04 netmap: Release lock to avoid deadlock
Issue: 6755

When NetmapOpen encounters an error opening the netmap device, it'll
retry a bit. When the retry limit is reached, it'll shutdown Suricata.

This commit ensures that the device list lock is not held when before
closing all open devices before terminating Suricata.
2 years ago
Alexey Simakov 231c892bef util/mime: fix memory leak
Fix memory leak at util-decode-mime:MimeDecInitParser, which
root cause is not-freeing allocated memory for mimeMsg

Bug: #6745
2 years ago
Victor Julien 7e4dba7dfb detect/http: report error on alloc failure 2 years ago
Philippe Antoine b48ec8a039 detect/http_header: fix leak on realloc failure 2 years ago
Jason Ish f800ed0f90 detect-http: add superfluous alloc check for cocci
Add not-needed SCCalloc return check to satisfy our Cocci malloc
checks as it can't see that the caller immediately checks the return
value of this simple wrapper around SCCalloc.
2 years ago
Philippe Antoine 68b0052018 rust: fix clippy ptr_arg warnings
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
   --> src/dns/log.rs:371:29
    |
371 | pub fn dns_print_addr(addr: &Vec<u8>) -> std::string::String {
    |                             ^^^^^^^^ help: change this to: `&[u8]`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
2 years ago
Philippe Antoine 7f5e98e6df ci: authors check using OISF repo
As flagged critical by codescan
2 years ago
Philippe Antoine 80abc22f64 http2: limit number of concurrent transactions
Ticket: 6481

Instead of just setting the old transactions to a drop state so
that they get later cleaned up by Suricata, fail creating new ones.

This is because one call to app-layer parsing can create many
transactions, and quadratic complexity could happen in one
single app-layer parsing because of find_or_create_tx
2 years ago
Philippe Antoine 86de7cffa7 pgsql: parse only PDU when type is unknown
A next PDU may already be in the slice to parse.
Do not skip its parsing, ie do not use rest, but take just
the length of the pdu
2 years ago
Philippe Antoine f52c033e56 pgsql: parse auth message within its bound
If the next PDU is already in the slice next, do not use it and
restrict ourselves to the length of this PDU.
Avoids overconsumption of memory by quadratic complexity, when
having many small PDUS in one big chunk being parsed

Ticket: #6411
2 years ago
Philippe Antoine bc422c17d6 detect: fixes use-after-free with http.request_header
Ticket: #6441

This keyword and the response one use a multiple inspection buffer.
But the different instances point to the same memory address
that comes from HttpHeaderGetBufferSpace and is not owned
by the transaction, and is rebuilt, which is a functional
bug in itself.

As it gets crafted, it can get reallocated if one header
is over 1024 bytes, while the previous freed pointer will still get
used for the previous headers.
2 years ago
Philippe Antoine 61f2e4e1e5 smtp: avoid creating empty transaction
Ticket: 6477

So as to avoid ending up with too many empty transactions.

This happens when Suricata sees a DATA command in the current
transaction but did not have a confirmation response for it.
Then, if Suricata receives another DATA command, it will
create another new transaction, even if the previous one
is empty. And so, a malicious client can create many empty
transactions by just sending a repeated amount of DATA commands
without having a confirmation code for them.

Suricata cannot use state->current_command == SMTP_COMMAND_DATA
to prevent this attack and needs to resort to a new boolean
is_data because the malicious client may send another dummy command
after each DATA command.

This patch leaves only one call to SMTPTransactionCreate
2 years ago
Philippe Antoine 8f73a0ac55 smtp: config limit maximum number of live transactions
Ticket: #6477
2 years ago
Philippe Antoine 4175680a8a http1: configurable max number of live tx per flow
Ticket: #5921

Co-authored-by: Jason Ish <jason.ish@oisf.net>
2 years ago
Philippe Antoine 8f63a8f3bf http1: remove transactions from their list
instead of keeping a NULL pointer in an array

Ticket: #5921
2 years ago