Commit Graph

15387 Commits (edfda9f69fb2f095a195860b382eed94655238be)
 

Author SHA1 Message Date
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
1 year 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.
1 year ago
Jeff Lucovsky 9fe00ff710 config/jansson: Remove excess libjansson mentions
Issue: 6712

Remove multiple occurrences of libjansson installation packages.
1 year ago
Jeff Lucovsky ee6208be9d config/nss: Remove libnspr/libnss traces
Issue: 6712
1 year 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.
1 year 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
1 year ago
Victor Julien 7e4dba7dfb detect/http: report error on alloc failure 1 year ago
Philippe Antoine b48ec8a039 detect/http_header: fix leak on realloc failure 1 year 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.
1 year 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
1 year ago
Philippe Antoine 7f5e98e6df ci: authors check using OISF repo
As flagged critical by codescan
1 year 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
1 year 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
1 year 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
1 year 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.
1 year 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
1 year ago
Philippe Antoine 8f73a0ac55 smtp: config limit maximum number of live transactions
Ticket: #6477
1 year ago
Philippe Antoine 4175680a8a http1: configurable max number of live tx per flow
Ticket: #5921

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

Ticket: #5921
1 year 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.
1 year 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
1 year ago
Shivani Bhardwaj 7f89aaf772 detect: remove unneeded max_idx 1 year 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.
1 year 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.
1 year ago
Philippe Antoine 6de885c603 ci: update scorecard analysis workflow 1 year ago
Philippe Antoine f6e1a20215 detect: dns.opcode as first-class integer
Ticket: 5446

That means it can accept ranges
1 year ago
Shivani Bhardwaj 8fc0faf5c2 util/streaming-buffer: remove unneeded fn param
StreamingBuffer is not required to find the intersecting regions, so,
don't pass it as a param to the fn.
1 year ago
Juliana Fajardini 244a35d539 userguide: fix explanation about bsize ranges
Our code handles Uint ranges as exclusive, but for bsize, our
documentation stated that they're inclusive.

Cf. from uint.rs:

    DetectUintMode::DetectUintModeRange => {
        if val > x.arg1 && val < x.arg2 {
            return true;
        }
    }

Task #6708
1 year ago
Philippe Antoine b8bc2c7e0f doc: integer keywords
Ticket: 6628

Document the generic detection capabilities for integer keywords.
and make every integer keyword pointing to this section.
1 year ago
Philippe Antoine d05f3ac791 detect: integer keywords now accept bitmasks
Ticket: 6648

Like &0x40=0x40 to test for a specific bit set
1 year ago
Philippe Antoine 370ac05419 detect/integer: rust derive for enumerations
Ticket: 6647

Allows keywords using integers to use strings in signature
parsing based on a rust enumeration with a derive.
1 year ago
Philippe Antoine 06c5dd3133 detect: integer keywords now accept negated ranges
Ticket: 6646
1 year ago
Philippe Antoine 3b65a2bb61 detect: integer keywords now support hexadecimal
So that we can write enip.revision: 0x203

Ticket: 6645
1 year ago
Philippe Antoine d73ccd0f52 ci: run clippy without all features 1 year ago
Philippe Antoine 38db51b878 rust: make cargo clippy clean
Fixing single_match and manual_find intertwined with SCLogDebug
1 year ago
Philippe Antoine 89936b6530 mqtt: fix logic when setting event
Especially sets transactions to complete when we get a response
without having seen the request, so that the transactions
end up getting cleaned (instead of living/leaking in the state).

Also try to set the event on the relevant transaction, instead
of creating a new transaction just for the purpose of having
the event.

Ticket: #6299
1 year ago
Philippe Antoine 2fb50598f2 detect: do not store state without flags
If flags are zero, there is nothing to store and remember.

Stored signatures will be reused on a later packet, and
qsorted (which may be expensive), with newer matches candidates.

Avoiding to store, leads to avoid the call to qsort.
1 year ago
Philippe Antoine 5bb8800588 detect: merge sorted lists instead of qsort
Ticket: #6299

Simply because it is faster (just linear).

This is for merging match_array into tx_candidates
1 year ago
Philippe Antoine 9240ae250c detect: avoids case of useless detection on txs
When a TCP flow packet has not led to app-layer updates,
it is useless to run DetectRunTx, as there cannot be new
matches.

This happens for instance, when one side sends in a row multiple
packets which are not acked (and thus not parsed in IDS mode).

Doing so requires to move up the call to
AppLayerParserSetTransactionInspectId
so that it is run the same times DetectRunTx is run, and not in the
case where the transaction was not updated.

Ticket: 6299
1 year ago
Jason Ish c3b3c11e30 requirements: use libhtp 0.5.x
Move to libhtp to the 0.5.x branch instead of 0.5.45.
1 year ago
Jason Ish 8bf8131c31 doc: note what version "requires" was added in 1 year ago
Jason Ish de3cbe4c90 detect/requires: reset sigerror flags for each rule
"sigerror_ok" and "sigerror_requires" were not being reset after each
rule which could lead to a rule load error being incorrectly tracked
as skipped rather than failed.

Also initialize "skippedsigs" to 0 along with "goodsigs" and
"badsigs", while not directly related to this issue, could also throw
off some stats.

Ticket: #6710
1 year ago
jason taylor 3cb7112aa5 detect: update smb.version keyword
Signed-off-by: jason taylor <jtfas90@gmail.com>
1 year ago
jason taylor bfc0790d87 rust: fix rustfmt warnings for smb detect
Signed-off-by: jason taylor <jtfas90@gmail.com>
1 year ago
Eloy Pérez González a4901a1f70 smb: add smb.keyword documentation 1 year ago
Eloy Pérez González 415722dab2 smb: add smb.version keyword
Ticket: #5075

Signed-off-by: jason taylor <jtfas90@gmail.com>
1 year ago
Juliana Fajardini df6444822e userguide: clarify midstream exception policy
The description of behavior when midstream is enabled and exception
policy is set to ignore wasn't descriptive enough.

Fix typos.
1 year ago
Lukas Sismis 6e4cc79b39 doc: remove references to prehistoric versions
Remove references that are mentioning Suricata 3 or less
As a note - only one Suricata 4 reference found:
(suricata-yaml.rst:"In 4.1.x")
Fast pattern selection criteria can be internally found by inspecting
SupportFastPatternForSigMatchList and SigTableSetup functions.

Ticket: #6570
1 year ago
Lukas Sismis 2a2898053c dpdk: add interrupt (power-saving) mode
When the packet load is low, Suricata can run in interrupt
mode. This more resembles the classic approach of processing
packets - CPU cores run low and only fetch packets
on interrupt.

Ticket: #5839
1 year ago
Lukas Sismis ca6f7c2d00 dpdk: rework hugepage hints to use per-numa information
Previous integration of hugepage analysis only fetched data
from /proc/meminfo. However this proved to be often
deceiving mainly for providing only global information and
not taking into account different hugepage sizes (e.g. 1GB
hugepages) and different NUMA nodes.

Ticket: #6419
1 year ago