Commit Graph

1408 Commits (suricata-7.0.9)

Author SHA1 Message Date
Philippe Antoine 782f35c5cf 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
(cherry picked from commit b02557ac7d)
5 months ago
Philippe Antoine 05bf4a8dec quic: discard late retry packets
Ticket: 7556

See RFC 9000 section 17.2.5.2 :
After the client has received and processed an Initial
or Retry packet from the server,
it MUST discard any subsequent Retry packets that it receives.

(cherry picked from commit 726de5520f)
5 months ago
Philippe Antoine 530f1a40e4 quic: decrypt only initial packets
Ticket: 7556

Avoids failed_decrypt events when the first packet seen is not
a Quic Initial packet

(cherry picked from commit d61f36c66f)
5 months ago
Philippe Antoine ac6dcd6fbf quic: handle retry packets
Ticket: 7556
(cherry picked from commit 6d8910d245)
5 months ago
Philippe Antoine 31d57ef7fc quic: handle fragmented hello over multiple packets
Ticket: 7556

To do so, we need to add 2 buffers (one for each direction)
to the QuicState structure, so that on parsing the second packet
with hello/crypto fragment, we still have the data of the first
hello/crypto fragment.

Use a hardcoded limit so that these buffers cannot grow indefinitely
and set an event when reaching the limit

(cherry picked from commit f295cc059d)
5 months ago
Philippe Antoine ce90ff187e quic: parse ack frame number 3
cf rfc9000 section 19.3. ACK Frames

Ticket: 7556
(cherry picked from commit 68adc87bd2)
5 months ago
Philippe Antoine 26a1d02722 quic: move all_consuming check to callee
Will alow to have decode_frames accept one additional parameter
with past fragment data

(cherry picked from commit ee04d667b5)
5 months ago
Jason Ish 13a76e0710 rust: fixes for new clippy warnings
Fixes provided by cargo clippy --fix.
5 months ago
Philippe Antoine 7fce4ef077 detect/krb5: avoid integer underflow with krb5.ticket_encryption
Ticket: 7560

When passing INT32_MIN aka 0x80000000, we cannot compute -vali
as it does not fit into a i32

(cherry picked from commit 8ae5665767)
5 months ago
Jason Ish ac62d1bc46 dns: refactor tests to avoid assert!(false)
Mostly just unwrap instead of match as unwrap provides good
context. And replace a few assert!(false) with a descriptive panic.
5 months ago
Jason Ish 6666555a09 rust: allow clippy::unused_unit for tests that use the test macro
The cause of the issue comes from the macro, which is provided by a
crate. Bust just to allow this for now.
5 months ago
Shivani Bhardwaj 17b8f1f7d7 dns: fix clippy lint warnings
Fix vector lint issues:
- same_item_push
- vec_init_then_push

(cherry picked from commit 2c0d3b83c4)
5 months ago
Philippe Antoine 57111f35c3 rust: fix assertions_on_constants for assert!(true)
Which will be optimized away by the compiler

(cherry picked from commit c49463c86f)
5 months ago
Philippe Antoine ab089b0859 rust: fix single_binding
error: this match could be written as a `let` statement
   --> src/nfs/nfs3_records.rs:747:9
    |
747 | /         match result {
748 | |             (r, request) => {
749 | |                 assert_eq!(r.len(), 0);
750 | |                 assert_eq!(request.handle, expected_handle);
751 | |                 assert_eq!(request.name_vec, br#"bln"#);
752 | |             }
753 | |         }
    | |_________^

(cherry picked from commit 259cdf169e)
5 months ago
Philippe Antoine a40b37ba44 rust: fix assertions_on_constants for assert!(false)
using panic! instead with a string message

(cherry picked from commit a8199bf2ca)
5 months ago
Jason Ish c7ff76cac5 rust: allow vec_init_then_push in tests
To supress the clippy lint in tests.
5 months ago
Jason Ish 2b6e5f822c mqtt: always use 0x notation for hex in tests
Fixes clippy lint for zero_prefixed_literal.
5 months ago
Giuseppe Longo b3e6a8f15d sip/parser: enforce valid chars for sip version
The `is_version_char` function incorrectly allowed characters that are not
part of the valid SIP version "SIP/2.0".

For instance, 'HTTP/1.1' was mistakenly accepted as a valid SIP version,
although it's not.

This commit fixes the issue by updating the condition to strictly
check for the correct version string.

cherry-picked from commit 69f841c998
6 months ago
Giuseppe Longo aabaa95913 sip/parser: accept valid chars
Accepts valid characters as defined in RFC3261.

cherry-picked from commit 7e993d5081
6 months ago
Giuseppe Longo bfcbe48e72 rust/sip: rustfmt sip module
cherry-picked from commit 8ff80cb84d
6 months ago
Jason Ish 71212b78bd 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 19cf0f8133)
8 months ago
Jason Ish 5edb84fe23 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 37f4c52b22)
8 months ago
Jason Ish 58c41a7fa9 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 3a5671739f)
8 months ago
Jason Ish ccc61f6294 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
8 months ago
Jason Ish eac4854636 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 820a3e51b7)
8 months ago
Jason Ish 825eadf1c5 rust: remove unnecessary lifetimes
Fix provided by cargo clipy --fix.

Backport of 7bdbe7ed32.
8 months ago
Jason Ish 60dd1d566c rust/smb: fix rustdoc line
'///' style rust comments/documentation come before the item being
documented.

Spotted by clippy.

(cherry picked from commit aa6e94fc73)
8 months ago
Jason Ish a502c188c5 rust: allow static_mut_refs for now
But we should fix all these soon.

(cherry picked from commit 4c12165816)
8 months ago
Jason Ish 9edac554eb 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 8e408d3730.
8 months ago
Jason Ish 4632510308 rust: sync Cargo.lock with Cargo.toml
This just updates some internal dependencies to our own crates in the
work-space.
8 months ago
Philippe Antoine 1bae761818 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 0a1062fad2
8 months ago
Sascha Steinbiss 378b9bb55d mqtt: add reason code support for SUBACK
Ticket: #7323
(cherry picked from commit 377d4705e1)
8 months ago
Jason Ish c3aa3ae102 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 b1c26dccf3)
9 months ago
Jason Ish bcd3523cc7 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
9 months ago
Jason Ish 3000e963d5 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 45384ef969)
9 months ago
Philippe Antoine 34a4625156 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 1e152d1f10)
10 months ago
Juliana Fajardini 1fbe985177 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 2b1ad81cf5)
10 months ago
Juliana Fajardini 5034ae5e44 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 dcccbb1196)
10 months ago
Juliana Fajardini 629b5d7298 pgsql: use new API style for extern C functions
(cherry picked from commit 2c7824a41f)
10 months ago
Juliana Fajardini add17d15c6 pgsql: order StateProgress enum per direction
Related to
Bug #7113

(cherry picked from commit 3ba179422d)
10 months ago
Victor Julien 19006560a1 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 65392c02f5)
10 months ago
Philippe Antoine 9571df8936 rust/detect: fix too_long_first_doc_paragraph clippy warning
warning: first doc comment paragraph is too long
  --> src/detect/iprep.rs:57:1
   |
57 | / /// value matching is done use `DetectUintData` logic.
58 | | /// isset matching is done using special `DetectUintData` value ">= 0"
59 | | /// isnotset matching bypasses `DetectUintData` and is handled directly
60 | | /// in the match function (in C).
   | |_
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
   = note: `#[warn(clippy::too_long_first_doc_paragraph)]` on by default
help: add an empty line

(cherry picked from commit dc3c048b49)
10 months ago
Philippe Antoine d3927afb70 rust/dcerpc: fix single_match clippy warning
warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
  --> src/dcerpc/log.rs:36:33
   |
36 |               DCERPC_TYPE_BIND => match &state.bind {
   |  _________________________________^
37 | |                 Some(bind) => {
38 | |                     jsb.open_array("interfaces")?;
39 | |                     for uuid in &bind.uuid_list {
...  |
51 | |                 None => {}
52 | |             },
   | |_____________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
   = note: `#[warn(clippy::single_match)]` on by default

(cherry picked from commit 2a984e3b13)
10 months ago
Philippe Antoine ded2082416 rust/ike: fix collapsible_match clippy warning
warning: this `match` can be collapsed into the outer `match`
help: the outer pattern can be modified to include the inner pattern
(cherry picked from commit 42e5e556e5)
11 months ago
Philippe Antoine bc1b906a7b rust: fix byte_char_slices clippy warnings
warning: can be more succinctly written as a byte str
   --> src/mime/smtp.rs:762:37
    |
762 |     mime_smtp_find_url_strings(ctx, &[b'\n']);
    |                                     ^^^^^^^^ help: try: `b"\n"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#byte_char_slices
    = note: `#[warn(clippy::byte_char_slices)]` on by default

(cherry picked from commit 564f685eea)
11 months ago
Sascha Steinbiss 8c8abbf7fa rust/rfb: use consistent key name for security_result
A typo caused a slightly different key (`security-result`) to be used
for the case in which the result was `FAIL`. This commit addresses this
by ensuring the same string is used for all cases.

Ticket: #7198
11 months ago
Juliana Fajardini 37ec6251e9 pgsql: check for eol when parsing response
It was brought to my attention by GLongo that Pgsql parser handled eof
diffrently for requests and responses, and apparently there isn't a good
reason for such a difference therefore, apply same logic used for
rs_pgsql_parse_request for checking for eof when parsing a response.

(cherry picked from commit ce1556cefd)
11 months ago
Juliana Fajardini f5cc23464b pgsql/logger: open json object from logger function
Before, the JsonBuilder object for the pgsql event was being created
from the C-side function that actually called the Rust logger.

This resulted that if another module - such as the Json Alert called the
PGSQL logger, we wouldn't have the `pgsql` key present in the log output
- only its inner fields.

Bug #6983

(cherry picked from commit 69e26de197)
11 months ago
Philippe Antoine d72ec89c37 rust: compatibility with cbindgen 0.27
Ticket: 7206

Cbindgen 0.27 now handles extern blocks as extern "C" blocks.
The way to differentiate them is to use a special comment
before the block.

(cherry picked from commit 304271e63a)
12 months ago
Jason Ish 1e0c64187f rust/dcerpc: fix rustdoc indentation
Fixes clippy lint:

error: doc list item missing indentation
   --> src/dcerpc/dcerpc.rs:511:9
    |
511 |     ///  description: direction of the flow
    |         ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
12 months ago