A v4 COMPOUND carried with the GSS integrity service (creds procedure 0 =
RPCSEC_GSS_DATA, service 2) is wrapped in an envelope: length of the
wrapped body (sequence number and arguments), sequence number, arguments,
checksum (RFC 2203 5.3.2.2). When that cannot be unwrapped the compound
parser is never reached, and the arm ended with set_event(MalformedData).
set_event stamps the newest transaction of the state, and returns without
doing anything when the state has none. Two consequences: on a flow whose
first RPC is such a call, the reject produced no transaction, no anomaly
and nothing in EVE, so a record no server would ever send went by
unremarked; and on a reply, the event landed on whichever transaction
happened to be last, which on an async mount is a different call than the
one being rejected, while the tx owning the xid stayed open forever.
Give both arms a transaction to carry the event, like the partial-record
rejects already do. The request side reuses the malformed-record helper
(the xidmap is registered by the caller, so the gss creds survive and the
reply of the xid is still known to be wrapped). The reply side marks the
tx owning the xid, and falls back to a new one only when there is none -
which is the common branch, as a compound with no main-opcode result
(PUTFH+GETATTR, a SEQUENCE-led v4.1 one) opens no tx; that one therefore
carries the procedure the xidmap recorded, not the 0 that EVE reads as
NULL. Either way the record is skipped to its RPC boundary, never
buffered toward the claimed length.
Ticket: #9265.
The error arm of a COMPOUND that is fully in hand only set a state event.
set_event marks the newest transaction, and there is none when the
record's first op is the one that fails: the record was then consumed,
nothing was logged, and nothing asked for the missing bytes. A v4 WRITE
whose count runs past the end of its record is that case - the parser
gives up at the data, so the size check in the WRITE handling, which
needs a parsed count, is never reached.
Reject what cannot be walked the way a partial record is rejected: scan
the bytes the parser was given, and log an oversized WRITE as a
write_request_too_large file transaction or another un-walkable record as
a malformed one, on a transaction of its own. What the parser skips is
not in that set: a defined op this parser does not implement keeps base's
event on the newest tx, and an RPCSEC_GSS privacy compound, whose result
is opaque by RFC 2203 on the call and the reply alike, is left unparsed
with no event at all. Either way the record is bounded by its own marker,
and a flow that carries un-parseable v4 traffic leaves a trace in EVE.
Ticket: #9265.
The record scanners unwrapped the (0, 2) combination and took every other
one as a bare compound. A control procedure or a privacy compound was
then scanned as if it were a COMPOUND: the envelope length was read as
the first tag, so the scan either stayed incomplete and buffered toward
the claim, or rejected a record that has nothing to reject.
Scan by the combination the call recorded: hand the credential pair to
gss_compound_scan_data, which leaves (0, 1) raw because rpc_gss_svc_none
wraps nothing, unwraps (0, 2), and reports a privacy (0, 3) compound as
opaque. There is nothing to scan in an encrypted compound, so the record
is skipped to its boundary and neither parsed nor scored: the request
path keeps the call in the request map for its reply, the reply path
drops it. Any other combination is one the full record path rejects as
well, so it is malformed, boundedly and without buffering. The v4
request path records the combination whatever it is, not only when it
unwraps.
Ticket: #9263.
The args of a WRITE request and of a READ reply were trusted to fit the
record that carries them. Where the claim fell below the extent the parse
covers, the file data ran past the record boundary into the next RPC,
which was then read as a record. A max-tx of 0 was accepted and evicted
every transaction on insert.
Compare the claim against that extent: for a WRITE the handle, offset,
count, stable flag and its data, for a READ the status, the optional
attributes, the count, the eof flag and its data. A record whose claim is
below the extent gets a transaction with the malformed data event and a
skip to the record boundary, as an oversized claim already does. Only the
v3 records arrive in pieces, so process_read_record now learns which
reply it is given and the check is v3's. Reject a max-tx of 0 at config
load.
Ticket: #9263.
A partial v3 reply that carries no file data is skipped rather than
buffered toward its claim, which for LOOKUP, READDIRPLUS and CREATE drops
the handle to name mapping their replies establish. The exception to that
reused max-read-size, so those procedures depended on a knob that
documents READ data, and lowering it for READs silently dropped the
mapping of any directory reply above it; the mapping depends on
segmentation, as the same reply parses when it arrives in one piece.
Give them a limit of their own, max-metadata-size, with a 16 MiB default,
and use it to decide whether such a reply may still buffer toward its
record claim. A value of 0 falls back to the default, so the buffering
stays bounded and the limit cannot be disabled. The WRITE and READ limits
share that default through NFS_MAX_RW_SIZE_DEFAULT. Documented in the
userguide.
Ticket: #9263.
The queue-limit work computed the record tail and the chunk promise from
the declared size, and added a count or a data length to it. A padded or
segmented record therefore left a promise armed past the end of its
record: a continuation could run on into the next RPC, an input that
carries only record slack grew the out-of-order queue and truncated a
file whose queued data fits, and the backstop walked the transactions of
a call that holds no file transaction at all.
Take the promise from the record itself, with record_tail(): what its
marker claims minus what the parser holds. Keep its two halves apart:
the stream side is the record tail, the queue side is the file data the
tracker still misses, so a continuation queues no more than its own
bytes. A pad a record carries is inside its claim, so the tail drains
it; it is neither file data nor fill. Release a promise whose
transaction is gone instead of consuming toward the claim, run the
backstop only when the call carries data and the state holds a file
transaction, and count a chunk only for input that carries file data. A
consume past the end of an input is now a debug assertion, with the
advance clamped to the input.
Ticket: #9263.
A WRITE request or READ reply with a count of 0 has no file data, and
processing it returned early. For a partially received record this left the
rest of the record unaccounted for, while the input was reported as consumed.
The rest of the record was then parsed as the next RPC record, which failed
and disabled the parser for the flow.
Like the complete record path, skip the rest of the record and continue with
the next record. For a READ reply this includes removing its request from the
request map, as is done for a complete reply.
Ticket: #9188
When a partially received record failed to parse, for example a WRITE with an
invalid stable value or a READ reply with an invalid eof, the parser returned
an error, disabling the parser for the flow. Yet the RPC record marker had
already given the size of the record.
Like the complete record path already does, set the malformed data event,
skip the rest of the record and continue with the next record. For a READ
reply this includes removing its request from the request map. A record
marker that cannot be parsed is still a fatal error, as the boundary of the
next record is unknown.
Ticket: #9190
The continuation of a partial WRITE request or READ reply only covered the
file data, not the XDR padding that follows it in the record. When a delivery
of stream data ended on the last byte of file data, or inside the padding,
the remaining padding was parsed as the start of a new RPC record, which
failed and disabled the parser for the flow.
Keep the chunk active while its record still has unconsumed bytes, as tracked
by the record tail, and consume exactly those bytes: from the input that
completes the file data and, for what is not there yet, from the next
deliveries. If the file is still open the padding is passed to its file
tracker, which consumes it as fill bytes, never as file data.
A partial READ reply that already holds all of its file data now also has its
request removed from the request map, as is done when a chunk completes.
Builds on the file handle of the in-progress READ chunk added for #9192.
Ticket: #9194
The continuation of a partially received READ reply looked up its file handle
in the request map by the reply's xid. As the request map is bounded and
evicts the least recently used entries, enough other requests on the flow
could evict the entry while the reply was still in progress. The continuation
then consumed nothing and its file data was parsed as a new RPC record, which
failed and disabled the parser for the flow.
Keep the file handle of the in-progress to-client chunk in the state, like
is done for the to-server WRITE chunk, and clear it along with the rest of
the chunk state.
The file transaction itself can also go away while its chunk is in progress,
for example when the max-tx limit forces it closed, after which a READ request
for the same handle creates a new transaction. A transaction found for the
handle whose file is not open, which is also the case after a complete READ
reply with eof set, cannot take the chunk either. In these cases consume the
chunk up to its known record boundary instead of parsing its data as RPC
records or passing it to that transaction.
Ticket: #9192
A NFSv3 WRITE with stable set to FILE_SYNC is assumed to be the last chunk of
its file, and its file transaction was closed as soon as the record was seen.
If the record was only partially received, the continuation could no longer
find the transaction by its file handle. Its file data was then not consumed
but parsed as a new RPC record, which failed and disabled the parser for the
flow, and the file was truncated.
Like the READ reply path already does, only close the transaction right away
if the record holds all of its file data. Otherwise filetracker_update closes
it once the file tracker is done or, as a truncated tracker is never done,
once the data of the last chunk is complete. The latter also applies to a
partial READ reply with eof set.
The NFSv4 WRITE path makes the same FILE_SYNC assumption, but it only handles
complete records, so it is not affected.
This is also the root cause of Redmine #8009.
Ticket: #9196
block_scrutinee now warns on a block-wrapped if let scrutinee, since
the block changes when temporaries drop. The file config reads in the
nfs, smb and http2 gettxfiles handlers were wrapped in a pointless
single-expression block; remove it.
The chunk-continuation queue-limit rejection accounted its stream
consumption and skip from the count-based chunk promise
(chunk_left), but the parser accepts file_len < count and the
tracker only ever consumes file_len bytes. With count > file_len
the consume/skip ran past the record's actual tail into the
following RPC record and the stream desynced.
Consume/skip from the record's unbuffered tail plus the record's
XDR padding.
Ticket: #8791.
Rework the queue-limit gap backstop: fall back to a 1 GiB / 1024
chunk hard cap when the limits are disabled, run it ahead of the
chunk_left == 0 early return, and scan every open file tx in the
direction (an interleaved handle can hide a first file's queue).
TruncatedFileData is attributed to the truncated tx.
Ticket: #8791.
The chunk path (filetracker_update) enqueued per-segment data with no
queue limit. Check the limits on every enqueuing path, projected onto
actual queue growth: only OOO data at a new offset grows the queue,
and only a new OOO chunk adds a map entry. Overflow raises the queue
event on the affected tx and skips to the record boundary.
Replace the 2 MiB gap backstop with a queue-limit one (the chunk-count
cap and all-open-file scan come next), and tag the session gapped on
every gap (even one fully covered by a skip), since the backstop is
gated on that flag. Rejected and evicted tx now release their file
resources so the file logger can clean them up.
Ticket: #8791.
Replace the hardcoded 16 MiB record caps with configurable
max-{write,read}-size and observable events: an oversized WRITE
request or READ response is rejected, logged, and its payload skipped
to keep the stream in sync. 0 disables the check.
For v4 records, the issue is more complex due to the COMPOUND type:
A partial v4 COMPOUND record whose frag_len claim is huge must not be
buffered toward that claim (31-bit, attacker-controlled → OOM). The
full parser can't help: on a partial buffer it only returns Incomplete
("need more bytes"), with no information about what's coming. The scan
instead walks the buffered prefix's ops by wire extent and reads the
file op's header words alone, answering the gate's four-way question:
oversized claim detected (reject + skip to the record boundary before
any data buffers), data not yet present (buffer normally), structurall
malformed (bounded rejection), or no oversized op (fall through). It
exists because rejecting before buffering requires the claimed length,
which only a prefix walk can extract.
Ticket: #8791.
Bound the per-file-transaction OOO queue, mirroring SMB: add
max-{read,write}-queue-{size,cnt} (0 disables, default 64 MiB / 64),
raise a queue event on overflow, and skip the truncated record's
remainder from the stream.
Ticket: #8791.
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, and use default trait instead of new
Will make easier the bindgen of RustParser structure which uses
a callback which uses AppLayerStateData
Internals
---------
Suricata's stream engine returns data for inspection to the detection
engine from the stream when the chunk size is reached.
Bug
---
Inspection triggered only in the specified chunk sizes may be too late
when it comes to inspection of smaller protocol specific data which
could result in delayed inspection, incorrect data logged with a transaction
and logs misindicating the pkt that triggered an alert.
Fix
---
Fix this by making an explicit call from all respective applayer parsers to
trigger raw stream inspection which shall make the data available for inspection
in the following call of the stream engine. This needs to happen per direction
on the completion of an entity like a request or a response.
Important notes
---------------
1. The above mentioned behavior with and without this patch is
affected internally by the following conditions.
- inspection depth
- stream depth
In these special cases, the inspection window will be affected and
Suricata may not consider all the data that could be expected to be
inspected.
2. This only applies to applayer protocols running over TCP.
3. The inspection window is only considered up to the ACK'd data.
4. This entire issue is about IDS mode only.
NFS has a classic request response model where each request is mapped to
a corresponding response. Additionally, there's also file tracking and
handling done as a part of these transactions. Appropriate calls to
trigger raw stream inspection have been added on completion of each
request and response.
Task 7026
Bug 7004
Deranged v0.4.1 (a dependency of the time crate) has implemented
PartialOrd for some integer types that conflict with the
implementation in the standard library creating an ambiguity as such
implementation are global. For more info see
https://github.com/jhpratt/deranged/issues/18.
To fix, use "::from" directly, instead of using .into() which is where
we run into amgibuity.
Have bindgen generate bindings for app-layer-protos.h, then use the
generated definitions of AppProto/AppProtoEnum instead if defining
them ourselves.
This header was chosen as its used by Rust, and its a simple header
with no circular dependencies.
Ticket: #7341
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
truncate fn is only active and used by dcerpc and smb parsers. In case
stream depth is reached for any side, truncate fn is supposed to set the
tx entity (request/response) in the same direction as complete so the
other side is not forever waiting for data.
However, whether the stream depth is reached is already checked by
AppLayerParserGetStateProgress fn which is called by:
- DetectTx
- DetectEngineInspectBufferGeneric
- AppLayerParserSetTransactionInspectId
- OutputTxLog
- AppLayerParserTransactionsCleanup
and, in such a case, StateGetProgressCompletionStatus is returned for
the respective direction. This fn following efc9a7a, always returns 1
as long as the direction is valid meaning that the progress for the
current direction is marked complete. So, there is no need for the additional
callback to mark the entities as done in case of depth or a gap.
Remove all such glue code and callbacks for truncate fns.
Bug 7044
Ticket: 7013
Done consistently for all protocols
This may change some protocols behaviors which failed early
if they found there was not enough data...
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
Victor Julien
Jason Ish
Giuseppe Longo
Philippe Antoine
Shivani Bhardwaj