and bindgen it to rust
Will make easier the bindgen of RustParser structure which uses
a callback which uses AppLayerTxData
Move also the free function to C SCAppLayerTxDataCleanup
As suricata-sys crate defines AppLayerTxData for rust,
It must itself implement the Drop trait, and thus,
We need to define a feature surest
and bindgen it to rust
Will make easier the bindgen of RustParser structure which uses
a callback which uses AppLayerResult
Keep From<> impl in sys crate that defines it
and bindgen it to rust, and use default trait instead of new
Will make easier the bindgen of RustParser structure which uses
a callback which uses AppLayerStateData
Unittests test_parse_bind_pdu_infinite_loop and
test_parse_bindack_pdu_infinite_loop seem to have artificially made up
header which does not hold up to the strict calculations enforced by the
parser now. Their headers mark the fraglens as 64 and 72 respectively
which are not enough to hold the kind of bind(ack) items that are expected.
It worked so far as the parser passed the entire input slice around but
with the bugfix for issue 7546, the input passed around is strictly
restricted to the fraglen parsed in the header.
Bug 7546
So far, the fraglen defined in the header was used inconsistently in
certain places to define bounds on input length. Make it consistent by
making sure that only a slice up until fraglen is passed around as that
is the maximum length the fragment should have.
With the help of Applayer::incomplete API, the case when the
stream_slice passed to the parser is smaller than the header defined
fraglen is already handled.
Bug 7546
The parser could receive an input that consists of arbitrary data post
gap. This is handled in the beginning of the fn handle_input_data.
However, the rest of the calculation does not take into account the
bytes that were consumed at this stage. Fix the indices and calculations
to consider a new DCERPC fragment beginning post these consumed bytes.
warning: called `unwrap` on `rd.pipe` after checking its variant with `is_some`
--> src/smb/smb1.rs:858:28
|
857 | if rd.pipe.is_some() {
| -------------------- help: try: `if let Some(<item>) = rd.pipe`
858 | let pipe = rd.pipe.unwrap();
| ^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#unnecessary_unwrap
= note: `#[warn(clippy::unnecessary_unwrap)]` on by default
Header handling is wrong in the case
packet A to server is fragmented (return AppLayerResult::incomplete)
packet B is to client, but uses the header of the to_server packet
Bug 7547
DCERPC parsers had no upper bounds when it came to extending the stub
data buffer. Traffic can be crafted to bypass some internal parser
conditions to create an indefinite buffering in the stub_data array that
can make Suricata crash.
Add a default limit of 1MiB and make it configurable for the user.
Security 8182
Co-authored-by: Philippe Antoine <pantoine@oisf.net>
TLS parsers use x509-parser crate which parses X.509 certificates that
use ASN.1 DER encoding that can allow arbitrary byte sequences. An
attacker could inject null byte in a certificate anywhere to stump the
common language parsers terminating the string at a null byte leading to
a bypass of a possibly malicious certificate.
So far, the rust TLS parser for "subjectaltname" used a pattern that involved:
-> Get ASN.1 DER encoded raw data from the x509-parser crate
-> Convert this raw data to a decoded string (Rust)
-> Convert the Rust string to CString
-- The problem lies here. CString only accepts proper strings/byte
buffers and converts it into an owned C-compatible, null-terminated
string. However, if any null byte occurs in the string passed to the
CString then it panics.
In the rust TLS parser, this panic is handled by returning NULL.
This means that the parser will error out during the decoding of the
certificate. However, Suricata must be able to detect the null byte
injection attack being an IDS/IPS.
Hence, replace all such string patterns w.r.t. TLS SAN with a byte
array.
Bug 7887
TLS parsers use x509-parser crate which parses X.509 certificates that
use ASN.1 DER encoding that can allow arbitrary byte sequences. An
attacker could inject null byte in a certificate anywhere to stump the
common language parsers terminating the string at a null byte leading to
a bypass of a possibly malicious certificate.
So far, the rust TLS parser for "issuerdn" used a pattern that involved:
-> Get ASN.1 DER encoded raw data from the x509-parser crate
-> Convert this raw data to a decoded string (Rust)
-> Convert the Rust string to CString
-- The problem lies here. CString only accepts proper strings/byte
buffers and converts it into an owned C-compatible, null-terminated
string. However, if any null byte occurs in the string passed to the
CString then it panics.
In the rust TLS parser, this panic is handled by returning NULL.
This means that the parser will error out during the decoding of the
certificate. However, Suricata must be able to detect the null byte
injection attack being an IDS/IPS.
Hence, replace all such string patterns w.r.t. TLS IssuerDN with a byte
array.
Bug 7887
TLS parsers use x509-parser crate which parses X.509 certificates that
use ASN.1 DER encoding that can allow arbitrary byte sequences. An
attacker could inject null byte in a certificate anywhere to stump the
common language parsers terminating the string at a null byte leading to
a bypass of a possibly malicious certificate.
So far, the rust TLS parser for "Subject" used a pattern that involved:
-> Get ASN.1 DER encoded raw data from the x509-parser crate
-> Convert this raw data to a decoded string (Rust)
-> Convert the Rust string to CString
-- The problem lies here. CString only accepts proper strings/byte
buffers and converts it into an owned C-compatible, null-terminated
string. However, if any null byte occurs in the string passed to the
CString then it panics.
In the rust TLS parser, this panic is handled by returning NULL.
This means that the parser will error out during the decoding of the
certificate. However, Suricata must be able to detect the null byte
injection attack being an IDS/IPS.
Hence, replace all such string patterns w.r.t. TLS Subject with a byte
array.
Bug 7887
The functionality of "both" can already be easily achieved by using both
"toclient" and "toserver" in a rule. This just adds the ease of
expression in rule. As it is added now, check the direction for the
pre-existing users of direction as well.
While debug_validate_bug_on is still used, it does not need to be
imported directly, as that macro is marked with `macro_export`, making
it globally available to the crate.
Ticket: 6724
Allows to use numerical values for example
Also fixes some unit tests that were returning 1 after goto error
FlagsTestParse05 especially took this path as
de->ignored_flags != (TH_SYN|TH_RST) was false
we had de->ignored_flags == 0xff ^ (TH_SYN|TH_RST)
And then, we had a match, instead of what the not-run code
was supposing.
Ticket: 6724
Allows sugar syntax for bitflags keywords.
While the expressivity does not increase, because we could already
use numerial values with all generic integer modes, this modifier
prefix is used with the strings, and follows the syntax
that is already used for fragbits and tcp.flags keyword.
So that http2.frametype and http2.errorcode can use EnumString
without changing the format, as they used UPPERCASE instead of
the default EnumString snake_case
IKE attributes are an array of TLV style objects, this means there can
be duplicate types seen on the wire. However, Suricata logs these as a
mapping with the type as the key. This can result in the JSON
containing duplicate keys.
To address this, log the attributes as an array of objects, allow
duplicates to exist, for example:
"client": {
"proposals": [
{
"sa_life_duration": "Unknown",
"sa_life_duration_raw": 86400,
}
}
}
is now logged as:
"client": {
"proposals": [
{"key": "sa_life_duration", "value": "Unknown", "raw": 86400}
]
}
Also adds `"version": 2` to each IKE record to note the change of
format from previous versions.
Ticket: #7902
email.body_md5 matches on md5 hash generated from email body
This keyword maps to the EVE field email.body_md5
It is a sticky buffer
Supports prefiltering
Ticket: #7587
Issue: 7853
Support the use of `from_base64` with no optional values. In this case,
the default values for:
- mode RFC4648
- offset: 0
- bytes: buffer size
will be used.
Fix new warning present in Rust 1.89.
warning: hiding a lifetime that's elided elsewhere is confusing
--> src/ldap/types.rs:191:30
= help: the same lifetime is referred to in inconsistent ways, making the signature confusing
= note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
Ticket: 7658
Suricata will not handle well if we open a file for this tx,
do not close it, but set the transaction state to completed.
RFC 9113 section 6.1 states:
If a DATA frame is received whose Stream Identifier field is 0x00,
the recipient MUST respond with a connection error (Section 5.4.1)
of type PROTOCOL_ERROR.
For example:
error: lifetime flowing from input to output with different syntax can be confusing
--> htp/src/headers.rs:475:16
|
475 | fn null(input: &[u8]) -> IResult<&[u8], ParsedBytes> {
| ^^^^^ ----- ----------- the lifetimes get resolved as `'_`
| | |
| | the lifetimes get resolved as `'_`
| this lifetime flows to the output
|
note: the lint level is defined here
--> htp/src/lib.rs:3:9
This currently only happens when using the Rust nightly compiler, which
we use for our fuzz builds.
There shouldn't be duplicated messages in the requests Vec. And thus
the parser shouldn't log duplicated keys nor messages. Add debug
validations to ensure this.
With PGSQL's current state machine, most frontend/ client messages will
lead to the creation of a new transaction - which would prevent
duplicated messages being pushed to the requests array and reaching the
logger.
The current exceptions for that are:
- CopyDataIn
- CopyDone
- CopyFail
Thus, debug statements were added for those cases.
CopyDone and CopyFail, per the documentation, shouldn't be seen
duplicated on the wire for the same transaction. CopyDataIn -- yes, but
we consolidate those, so the expectation is that they won't be
duplicated in the requests array or when reaching the logger either.
Related to
Task #7645