Commit Graph

15613 Commits (2d0ceedeba1866063b80e73e6cdf062e07fdaf37)
 

Author SHA1 Message Date
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
Philippe Antoine aff54f29f8 http2: handle reassembly for continuation frames
Ticket: 5926

HTTP2 continuation frames are defined in RFC 9113.
They allow header blocks to be split over multiple HTTP2 frames.
For Suricata to process correctly these header blocks, it
must do the reassembly of the payload of these HTTP2 frames.
Otherwise, we get incomplete decoding for headers names and/or
values while decoding a single frame.

Design is to add a field to the HTTP2 state, as the RFC states that
these continuation frames form a discrete unit :
> Field blocks MUST be transmitted as a contiguous sequence of frames,
> with no interleaved frames of any other type or from any other stream.
So, we do not have to duplicate this reassembly field per stream id.

Another design choice is to wait for the reassembly to be complete
before doing any decoding, to avoid quadratic complexity on partially
decoding of the data.
2 years ago
Philippe Antoine db99c45d23 detect: errors on 65k filestore signatures
Errors when a detection engine gets 65k filestore signatures to
avoid the hard limit to have 65k filestore per signature group
head

Ticket: #6393
2 years ago
Shivani Bhardwaj 7f89aaf772 detect: remove unneeded max_idx 2 years ago
Shivani Bhardwaj 395c74d81e detect/engine: set max sig ID per SGH
Present scenario
----------------
Currently, as a part of setting signature count per SGH, a max_idx is
passed which could be as high as the highest signature number (internal
ID).

Issue
-----
Not every SGH needs to evaluate all the signatures while setting
the signature count or while creating the match_array.
In a nonideal scenario, when say, there are 2 SGHs and one SGH has 2
signatures and the other one has 60k, given the current scheme of
evaluating max_idx, the max_idx will be set to 60k, and this shall
later be passed on to SigGroupHeadSetSigCnt or
SigGroupHeadBuildMatchArra which shall traverse over all the 60k sigs
for either SGHs.

Other info
----------
This is a very fast operation as the internal arithmetic is done
bitwise.

Patch
-----
The functions SigGroupHeadSetSigCnt and SigGroupHeadBuildMatchArray can
be optimized by storing the max signature id (internal) per SGH (which
also seemed to be the initial intention as per fn comments).
As a result of this, the sig_array is only walked up until the max sig
id of that respective SGH.
2 years ago
Shivani Bhardwaj 264101ba22 detect: remove unused port in SigGroupHeadInitData
port is not used and logically makes sense to not be in this struct as
this struct is already referenced by DetectPort itself as a part of
SigGroupHead.
2 years ago