Commit Graph

83 Commits (3bc2a14fbfefddbfc241bf6d961a57c3b24835a3)

Author SHA1 Message Date
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
Philippe Antoine ef044b208c dcerpc: prevent integer underflow
in case a fragment has a length lesser than DCERPC_HDR_LEN

Fixes: 9daf8528b7 ("dcerpc: tidy up code")

Ticket: 7548
2 years ago
Shivani Bhardwaj c73299a298 dcerpc/tcp: add frames support
Frames of the following types have been added for toserver direction:
1. Pdu: The entire Protocol Data Unit
2. Hdr: Header of the request
3. Data: PDU data

Feature 4904
2 years ago
Shivani Bhardwaj 9daf8528b7 dcerpc: tidy up code
- remove unneeded variables
- remove unnecessary tracking of bytes in state
- modify calculations as indicated by failing tests
2 years ago
Shivani Bhardwaj 4790da1825 dcerpc: remove fragmented data tests
With the introduction of AppLayerResult::incomplete API, fragmented data
is no longer handled fully in the dcerpc code. Given that these code
paths are already covered by the following s-v tests, these tests can now be
safely removed.
- dce-gap-handling
- dcerpc-dce-iface-*

Ticket 5699
2 years ago
Shivani Bhardwaj 74de1042a9 dcerpc: use AppLayerResult::incomplete API
Instead of own internal mechanism of buffering in case of fragmented
data, use AppLayerResult::incomplete API to let the AppLayer Parser take
care of it. This makes the memory use more efficient.
Remove any unneeded variables and code with the introduction of this
API.

Ticket 5699
2 years ago
Shivani Bhardwaj fc88e61c7f dcerpc: save version info in tx
to make it available for logging.
2 years ago
Shivani Bhardwaj 0d6017d174 dcerpc: do not assume an upper bound on data
TCP data can be presented to the protocol parser in any way e.g. one
byte at a time, single complete PDU, fragmented PDU, multiple PDUs at
once. A limit of 1MB can be easily reached in some of such scenarios.
Remove the check that rejects data that is more than 1MB.
2 years ago
Jason Ish 2bcc66da58 rust/log: move rust log and debug utils to debug module
Move Rust logging, and debug_validation to a debug module to mirrow
the C side.
2 years ago
Jason Ish f62be374ea rust/direction: move direction to own file (cleanup)
Move the implementation of Direction to its own file, direction.rs.
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 b02557ac7d 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
2 years ago
Shivani Bhardwaj fbb97c51e4 dcerpc: return error on invalid header
DCERPC/TCP tends to return the same values for invalid and incomplete
headers. As a result of this, invalid headers and any traffic following
it is buffered and processed later on assumed to be valid DCERPC traffic.
Fix this by clearly defining error and incomplete data and taking
appropriate actions.

Bug 7230
2 years ago
Victor Julien 65392c02f5 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.
2 years ago
Victor Julien a0bf282963 rust: address clippy errors 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 37a9003736 rust/probing: safety check for null input
Ticket: 7013

Done consistently for all protocols

This may change some protocols behaviors which failed early
if they found there was not enough data...
2 years ago
Shivani Bhardwaj 8770431986 dcerpc: accept ALTER_CONTEXT as a valid request
So far, if only the starting request was a DCERPC request, it would be
considered DCERPC traffic. Since ALTER_CONTEXT is a valid request type,
it should be accepted too.

Reported and patch proposed in the following Redmine ticket by
InterNALXz.

Bug 6191
3 years ago
Shivani Bhardwaj d4e674b390 rust: fix clippy warnings 3 years ago
Philippe Antoine d40dca5e55 dcerpc: maximum number of live transactions also for UDP
Ticket: #6129

Avoids that quadratic complexity gets too bad
3 years ago
Philippe Antoine 668501c225 rust: remove unused 3 years ago
Victor Julien ee7ed99b6f rust: spelling 4 years ago
Jason Ish d2fb958e28 rust: fix clippy lint for assert
Fix done automatically by clippy --fix
4 years ago
Philippe Antoine b52293b609 dcerpc: config limit maximum number of live transactions
As is done for other protocols

Ticket: #5779
4 years ago
Jason Ish 4940dfb3bd rust/clippy: fix lint: len_without_is_empty 4 years ago
Jason Ish 5d62995e26 rust/clippy: fix lint: explicit_counter_loop 4 years ago
Philippe Antoine 29f40c9e07 dcerpc: fix integer underflow
as input.len() can be 65536, it cannot be directly cast to u16

Ticket: #5557
4 years ago
Philippe Antoine bc287018e5 rust: cargo clippy --all-features --fix --allow-no-vcs 4 years ago
Jason Ish 04f0ee0151 rust: fix clippy lints for clippy::unnecessary_cast 4 years ago
Jason Ish b6cc0e25b1 rust: fix clippy lints for clippy::redundant_static_lifetimes 4 years ago
Jason Ish 6b71d69356 rust: fix clippy lints for clippy::bool_comparison 4 years ago
Jason Ish f342d4aacd rust: fix clippy lints for clippy::len_zero 4 years ago
Jason Ish 29a4a7fddc rust: fix clippy lints for clippy::assign_op_pattern 4 years ago
Jason Ish c4034dafa1 rust: fix clippy lints for clippy::derive_partial_eq_without_eq 4 years ago
Jason Ish c503ca62e2 rust: fix clippy lint for needless_late_init 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
Philippe Antoine d745d28d4a dcerpc: use vecdeque tx iterator
Ticket: #5321
5 years ago
Jason Ish dfe76bb905 dcerpc: convert transaction list to vecdeque
Allows for more efficient removal from front of the list.

Ticket: #5271
5 years ago
Philippe Antoine 704bc878ea dcerpc: store consumed_bytes as i32
As it can grow bigger than u16
5 years ago
Pierre Chifflier 8dc3431d86 rust/dcerpc: convert parser to nom7 functions 5 years ago
Eloy Pérez González 15f493f516 dcerpc: remove prev_tx_call_id and clear_bind_cache from DCERPCState
remove those fields since are not used because of the removal of
handle_bind_cache.
5 years ago
Eloy Pérez González 1aca2676a6 dcerpc: avoid delete the rpc state interface context
The bug:
The dcerpc dce_iface keyword just match the packet following the bind. Only the
next request after the rpc is sent will match. However the expected behaviour it
that all the rpc requests/responses sent under the context of the given
interface would match.

In the Open Group c706 the following is indicated:

In 2.2.1 Binding-related Operations, indicates that one category of binding
operations are those that "operations that establish internal call routing
information for the server." (The other are to establish the protocol which is
not relevant here). And the following statement can be found:

Operations in the second category establish a set of mappings that the server
can use to route calls internally to the appropriate manager routine. This
routing is based on the interface and version, operation and any object
requested by the call.

It indicates that server routes (to call methods) are based on the operation,
interface and object.

- Operation: To indicate the method to call, and operation number is
             specified as indicated in the second step of 2.3.3.2 (Client
             Binding Steps).
- Interface: An interface is a set of remotely callable operations offered by a
             server and invokable by clients. (2.1.1.1)
- Object: Is the manager that implements the interface, as stated in section
          Interface and Manager Selection of 2.3.3.3. It is not mandatory, can
          be nil.

To call a method, a client must send a request message as defined in 2.6.4.9,
that contains these identifiers:

- opnum: The opnum field identifies the operation being invoked within the
         interface.
- p_cont_id (Context ID in Wireshark): The p_cont_id field holds a presentation
                                       context identifier that identifies the
                                       data representation and interface, as
                                       defined in 12.6.3.4 (Context Identifiers).
- object: The object field is contained if the PFC_OBJECT_UUID is set. (Could be
          interesting to create a keyword dce_object for matching this UUID)

Therefore, to get the correct method to invoke, the server must map the context
to the correct interface. This is negotiated by the bind request

Interfaces are first negotiated using the bind message (12.6.4.3), contained in
the p_context_elem array. Then they are accepted or rejected using the bind_ack
message (12.6.4.4).

Once these contexts are established, both client and server can use the context
id, which is the index of the p_context_elem array, to refer the interface they
are using.

Moreover, in the middle of the connection, the context can be changed with the
alter_context message.

This is way suricata shouldn't delete the bindack attribute, that contains
the contexts, used by match_backuuid. This is the only way to know the interface
a request message is referring to.

ticket: 4769
https://redmine.openinfosecfoundation.org/issues/4769
5 years ago
Victor Julien e6f49e5a05 app/frames: implement name to id API for frames 5 years ago
Victor Julien c073d5cfbf app-layer: use StreamSlice as input to parsers
Remove input, input_len and flags in favor of stream slice.
5 years ago
Victor Julien 6466296b32 app-layer: add StreamSlice to pass data to parsers
Since object to contain relevant pointer, length, offset, flags to make
it easy to pass these to the parsers.
5 years ago
Jason Ish 7732efbec2 app-layer: include decoder events in app-layer tx data
As most parsers use an events structure we can include it in the
tx_data structure to reduce some boilerplate/housekeeping code
in app-layer parsers.
5 years ago
Jason Ish 9c67c634c1 app-layer: include DetectEngineState in AppLayerTxData
Every transaction has an existing mandatory field, tx_data. As
DetectEngineState is also mandatory, include it in tx_data.

This allows us to remove the boilerplate every app-layer has
for managing detect engine state.
5 years ago
Shivani Bhardwaj a866499bca dcerpc: use Direction enum 5 years ago
Sam Muhammed 922a453da5 rust(lint): use is_null() instead of ptr::null_mut()
Bug: #4594
5 years ago