Instead of inheriting from flow, because encrypted protocols like TLS
and SSH may have just set the flow flags to indicate rest of stream is
encrypted and does not need to run stream inspection. But inspection
still needs to be run detection on this last flushing packet.
Ticket: #7235.
(cherry picked from commit 976dec7f33)
Ticket: 7191
So as to avoid quadratic complexity in libhtp.
Make the limit configurable from suricata.yaml,
and have an event when network traffic goes over the limit.
(cherry picked from commit bb714c9178)
Ticket: 7067
This off by one could lead to an empty fragment being inserted
in the rb tree, which led to integer underflow
(cherry picked from commit 9203656496)
If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.
It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.
This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.
This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.
Bug: #7264.
(cherry picked from commit 45eb7e4881)
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)
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)
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)
(cherry picked from commit 9c3669b03f)
An additional change was made to correct an ASAN issue -- the membuffer
is reset following allocation in MemBufferCreateNew().
Log using stream callback API, meaning that data will also
be logged if there are GAPs.
Also implement GAP indicators: '[123 bytes missing]'.
(cherry picked from commit 6e10c66078)
This avoids looping over partly duplicate segments that cause
output data corruption by logging parts of the stream data multiple
times.
For data with GAPs now add a indicator '[4 bytes missing]' similar
to how Wireshark does it.
Bug: #6553.
(cherry picked from commit 43858f70ad)
As stream frame is not always created,
hence the first frame is not always a stream frame :
If stream frame is not enabled, it does not get created,
and other enabled frames may be created first.
See use of FrameConfigTypeIsEnabled
This resulted that this other frame got its length updated
on stream end, which led to false positives.
Also checking FRAME_STREAM_TYPE is more consistent.
Not a clean cherry-pick as AppLayerFrameGetLastOpenByType
does not exist in main7
Ticket: 7213
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)
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)
Don't return true for negated protocol check if no protocol has been
evaluated due to ALPROTO_UNKNOWN in the packet direction.
This leads to false positives for negated matching, as an expression
like "!tls" will match if checked against ALPROTO_UNKNOWN.
This patch readds missing check. The keyword returns no match as
long as the alproto is ALPROTO_UNKNOWN.
Fixes: bf9bbdd612 ("detect: fix app-layer-protocol keyword for HTTP")
Ticket: #7242.
Ticket: 6634
That means take only the first client hello into account.
This way, we do not end with ja3 string with 9 commas...
(cherry picked from commit 84735251b5)
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)
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)
This ensures that the correct spelling of the `security_result` EVE
field for RFB (as opposed to `security-result`) is also reflected in the
documentation.
Ticket: #7210
(cherry picked from commit cb14e44780)
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
The lookup function was not taking into account that we can have
an IPv4 or an IPv6 address as parameters and that this addresses
need to be converted to Suricata internal storage.
By using the already defined dedicated parsing function, we are
fixing the issue.
Issue: #6969
(cherry picked from commit 4668c95513)
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)
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)
Don't set an ACK value if ACK flag is no longer set. This avoids a bogus
`pkt_broken_ack` event set.
Fixes: ebf465a11b ("tcp: do not assign TCP flags to pseudopackets")
Ticket: #7158.
(cherry picked from commit a404fd26af)
The "eve.version" field is not always logged. Update the schema to
enforce that it is, and fix it for records that don't log it.
Ticket: #7167
(cherry picked from commit fcc1b1067b)
Shivani Bhardwaj
Ilya Bakhtin
Sascha Steinbiss
Philippe Antoine
Victor Julien
Juliana Fajardini
Juliana Fajardini
Eric Leblond
Jason Ish