Commit Graph

98 Commits (1e21db66aa209989fa5750fb2c8b3540fb6873f9)

Author SHA1 Message Date
Giuseppe Longo 8c9bca92fb rdp: treat tpkt payload as complete input
parse_t123_tpkt runs the X.224 and X.223/MCS sub-parsers over the
length-bounded tpkt payload, but they use streaming nom parsers.
An inner length field overrunning the payload, such as an oversized
CS_CORE block, escaped as Err::Incomplete although the tpkt was fully
present.

This patch wraps the child parsers in complete() so Incomplete from within the
bounded payload becomes an Error.
The tpkt is then returned as an unrecognized Raw child, which the caller skips,
and parsing resumes at the next tpkt.

Ticket #8838
2 months ago
Victor Julien 9555e3add6 rdp: fix tx id handling
Tx ID handling did not take the required + 1 into account.

From a report:

        RDP can skip cleanup because its id convention does not match the
        generic Rust iterator. The generic iterator in applayer.rs returns
        tx.id() - 1, and cleanup trusts that id when calling StateTransactionFree
        in app-layer-parser.c. RDP registers that iterator in rdp.rs, but
        RdpTransaction::id() returns the stored id unchanged in rdp.rs, while
        free_tx also compares against the raw stored id in rdp.rs. For a single
        freeable RDP tx with stored id 1, the iterator returns C id 0; cleanup
        calls free_tx(0), nothing is removed, then has_next == false allows
        min_id to advance to total_txs in app-layer-parser.c. That leaves the
        tx live but now below min_id, so later cleanup will not revisit it.

This patch brings the handling in line with the other parsers.

Bug: #8717.
4 months ago
Jeff Lucovsky 5af13e3eb0 rdp: mark transactions single-directional
Mark the direction into RDP transactions at creation time,
so the tx carries SKIP_INSPECT for the direction it is
never seen in, matching DHCP and the other single-direction
parsers. This lets cleanup free completed transactions and
stops a tx from being inspected (and alerting) twice, once per
direction.

RDP bounds its transactions to connection setup and stops
parsing once bypass_parsing is set.

Issue: 8621
4 months ago
Philippe Antoine 9d9d8df4cd rust: format rdp files
Ticket: 3836
5 months ago
Philippe Antoine 64d29fcd1c rust: move AppLayerStateData definition to C
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
9 months ago
Philippe Antoine c960b7d7c1 app-layer: AppLayerParserFPtr uses a mut pointer to local storage
So, fix the fn prototype in rust ParseFn
9 months ago
Philippe Antoine 455b7d1b3b rust: use snake case for applayer_register_protocol_detection 11 months ago
Jason Ish 399f96b07d rust/rdp: convert to nom 8
Ticket: #8044
1 year ago
Philippe Antoine bca6cbfb04 rust: fix prototype for AppLayerParserTriggerRawStreamInspection
Ticket: 7762

As it modifies the flow indeed
1 year ago
Philippe Antoine 6dbc421825 rust: bindgen AppLayerParserConfParserEnabled
Ticket: 7667
1 year ago
Philippe Antoine 49b2a2be5d rust: bindgen SCAppLayerParserRegisterLogger
Ticket: 7667
1 year ago
Philippe Antoine b29d46d81f rust: bindgen SCAppLayerParserStateIssetFlag
Ticket: 7667
1 year ago
Philippe Antoine 7bb0c94ae9 rust: bindgen app-layer-detect-proto.h
Ticket: 7667
1 year ago
Shivani Bhardwaj 44e098b7b3 rdp: trigger raw stream inspection
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.

RDP parser creates a transaction per request or response. Appropriate calls
to trigger raw stream inspection have been added on completion of each request
and response.

Task 7026
Bug 7004
1 year ago
Victor Julien add7d3fcf5 app-layer: API for mapping progress name vs id 2 years ago
Jason Ish b67917489d rust/rdp: namespace and visibility cleanups
Ticket: #7498
2 years ago
Philippe Antoine 3bc2a14fbf rust: fix clippy 1.85 precedence warnings
warning: operator precedence can trip the unwary
   --> src/jsonbuilder.rs:781:36
    |
781 |                 buf[offset] = HEX[(x >> 4 & 0xf) as usize];
    |                                    ^^^^^^^^^^^^ help: consider parenthesizing your expression: `(x >> 4) & 0xf`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#precedence
    = note: `#[warn(clippy::precedence)]` on by default
2 years ago
Jason Ish 4554c4778d rust: use AppProto from generated bindings instead of duplicating
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
2 years ago
Jason Ish c86b2608aa rust/applayer: clean visibility of export macros
Both the macros export_tx_data_get and export_state_data_get can
generate non-pub functions as the function they generate is only used
as a pointer during registration.

Remove "pub" and "no_mangle" from the generated functions and update
the names of the generated functions to follow Rust rules as they are
no longer exported into the global C namespace.

Ticket: 7498
2 years ago
Jason Ish 7ef4caf90e rust/flow: move flow support to its own file (cleanup)
Move the Rust Flow support from core.rs to flow.rs.
2 years ago
Philippe Antoine af4e1e8888 rust: remove unneeded mut for tx in loggers
Loggers do not change transactions, they read only.
2 years ago
Shivani Bhardwaj f2de3e01cb src: remove truncate fn and glue code
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
2 years ago
Philippe Antoine 3643b6ed4b output: generic simple tx json logger
Ticket: 3827
3 years ago
Philippe Antoine 9a84681bd9 rust: fix vec_init_then_push
warning: calls to `push` immediately after creation
    --> src/pgsql/parser.rs:1179:9
     |
1179 | /         let mut database_param: Vec<PgsqlParameter> = Vec::new();
1180 | |         database_param.push(database);
     | |______________________________________^
help: consider using the `vec![]` macro: `let database_param: Vec<PgsqlParameter> = vec![..];`
3 years ago
Daniel Olatunji 5c0af0b203 rust/doc: add docstring to rust module files.
Issue: #4584
3 years ago
Philippe Antoine 5bdbc1a313 rdp: do not use zero-bit bitflag
cf https://docs.rs/bitflags/latest/bitflags/#zero-bit-flags

As warned by clippy 1.72.0
3 years ago
Philippe Antoine 668501c225 rust: remove unused 3 years ago
Victor Julien ee7ed99b6f rust: spelling 4 years ago
Jason Ish 0e55307c1d app-layer: remove APP_LAYER_PARSER_OPT_UNIDIR_TXS
This flag is no longer needed as a parser can now create a transaction
as unidirectional.

Setting this flag also doesn't make sense on parsers that may have
request/reply and some unidirectional messaging.
4 years ago
Jason Ish d2fb958e28 rust: fix clippy lint for assert
Fix done automatically by clippy --fix
4 years ago
Jason Ish 359d5fcb7e rust/clippy: fix lint: needless_lifetimes 4 years ago
Jason Ish f15ffbc869 rust/clippy: fix lint: single_match
Allow this lint in some cases where a match statement adds clarity.
4 years ago
Jason Ish 925bc74c1f rust/clippy: fix lint: while_let_loop 4 years ago
Alice Akaki ccdc992a71 rust: fix lint warnings about mixed case hex literals
Ticket: #4593
4 years ago
Jason Ish b6cc0e25b1 rust: fix clippy lints for clippy::redundant_static_lifetimes 4 years ago
Jason Ish 7ba1d3e300 rust: fix clippy lints for clippy::nonminimal_bool 4 years ago
Jason Ish 7ebdfa539a rust: fix clippy lints for clippy::manual_find 4 years ago
Jason Ish 3ec435a703 rust: fix clippy lints for clippy::manual_range_contains 4 years ago
Jason Ish f342d4aacd rust: fix clippy lints for clippy::len_zero 4 years ago
Jason Ish c4034dafa1 rust: fix clippy lints for clippy::derive_partial_eq_without_eq 4 years ago
Victor Julien 79499e4769 app-layer: move files into transactions
Update APIs to store files in transactions instead of the per flow state.

Goal is to avoid the overhead of matching up files and transactions in
cases where there are many of both.

Update all protocol implementations to support this.

Update file logging logic to account for having files in transactions. Instead
of it acting separately on file containers, it is now tied into the
transaction logging.

Update the filestore keyword to consider a match if filestore output not
enabled.
4 years ago
Victor Julien c27df6304d app-layer: introduce common AppLayerStateData API
Add per state structure for storing flags and other variables.
4 years ago
Pierre Chifflier b31c72c06a rust/rdp: convert parsers to nom7 4 years ago
Pierre Chifflier 49520b2143 rust/rdp: upgrade dependency on tls-parser 4 years ago
Pierre Chifflier 3aace49649 rust/x509: update dependency on x509-parser 4 years ago
Philippe Antoine 2294e9cdbc rdp: bump up tls-parser crate version
so that we can use new functions in quic parser
4 years ago
Jason Ish 8790968281 mqtt, rdp: fix copyright dates 5 years ago
Jason Ish 9b0b2beac1 pgsql: convert transaction list to vecdeque
Allows for more efficient removal from front of the list.

Ticket: #5297
5 years ago
Jason Ish 4e0ad5e0bd rdp: convert transaction list to vecdeque
Allows for more efficient removal from front of the list.

Ticket: #5295
5 years ago
Jason Ish 7b11b4d3a1 app-layer: more generic state trait
Instead of a method that is required to return a slice of transactions,
use 2 methods, one to return the number of transactions in the
collection, and another to get a transaction by its index in the
collection.

This allows for the transaction collection to not be a contiguous array
and instead can be a VecDeque, or possibly another collection type that
supports retrieval by index.

Ticket #5278
5 years ago