Move it away from http2 to generic core crate.
And use it for DCERPC (and SMB)
And remove the C version.
Main change in API is the free function is not free itself, but
a rust wrapper around unbox.
Ticket: #4112
Without dangerous snprintf pattern identified by CodeQL
even if this pattern is not a problem in those precise cases,
it may easily get copy pasted in a dangerous place, so better
get rid of it and make CodeQL happy
Adds a PDU frame to the DNS parser. For UDP this is the DNS payload
portion of the DNS packet, for TCP this is the payload minus the leading
legth field.
Ticket: 4984
Wrap the calls behind frames to C code if a `cfg!(not(test))` so they
don't get compiled when running Rust unit tests. Linkage to C functions
is not yet available for Rust unit tests, and this will keep the check
out of individual parsers.
Ticket: 4984
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
Fuzzers found a possible integer overflow bug when parsing response
messages. To fix that, removed the case where we incremented the parsed
field length and created a new message type for situations where Suri
parsers an Unknown message. This is good because there may happen that
an unknown message to Suri is valid, and in this case, we would still be
able to log it.
Philippe Antoine found the bug while fuzzing with rust debug assertions.
Bug #5016
To recognize a protocol, Suricata first looks for
patterns, which can be confirmed by a probing parser.
If this does not work, Suricata can try to run
some probing parsers on some ports.
This is the case for SMB.
This commit makes handling the confirming and the probing
paser differently even if they share much code.
The confirmation parser knows that a pattern has been found.
So, it must not do the midstream case of looking for this
pattern in the whole buffer, but only check it at the beginning.
But it must reverse direction if needed.
The DNS name parser will error out with an error even if the
error is incomplete. Instead of manually generating errors,
use '?' to let the nom error ripple up the error handling chain.
The reason this wasn't done in the first place is this code
predates the ? operator, or we were not aware of it at the time.
This prevents the case where probing fails when there is enough data to
parse the header, but not enough to complete name parser. In such a case
a parse error is returned (instead of incomplete) resulting in the
payload not being detected as DNS.
Ticket #5034
If there is more data than a header, but not enough for a complete DNS
message, the hostname parser could return an error causing the probe to
fail on valid DNS messages.
So only parse the complete message if we have enough input data. This is
reliable for TCP as DNS messages are prefixed, but for UDP its just
going to be the size of the input buffer presented to the parser, so
incomplete could still happen.
Ticket #5034
Allow limiting in-flight out or order data chunks per size or count.
Implemented for read and writes separately:
app-layer.protocols.smb.max-write-queue-size
app-layer.protocols.smb.max-write-queue-cnt
app-layer.protocols.smb.max-read-queue-size
app-layer.protocols.smb.max-read-queue-cnt
This addresses Redmine bug #5018 by ensuring that the parser
never requests additional data via the Incomplete error, but to
raise an actual parse error, since it is supposed to have all
the data as specified by the message length in the header already.
Instead of closing files in both direction when receiving a close request,
close only toserver files for the request and close toclient on receiving
a response.
If an SMB record is seen in the wrong direction, set an event on the PDU
frame and don't process the record in the state.
No error is returned, so the next record will be processed.
The bits were being parsed in the order they're displayed in Wireshark,
rather than the order they were being seen on the wire, resulting in
direction and async being 0 more often than they should be.
Instead of bits, take the 4 bytes as an le_u32 and just use bit masks to
extract what we need into a struct, I think its easier to reason about
this way when comparing to the Microsoft documentation.
There should be no remaining data after parsing the partial
RPC record, so don't handle it but instead add a debug validation
bug on.
Successful processing for NFSv3 read/write records returns
AppLayerResult::ok() directly as all data is consumed.
With these, the portion of code within the tags should be included
in the related code-snippets (for frame support documentation) w/o
errors, even if the code within changes. The tags can also work as
a reminder that the existing code is being shown elsewhere, so folks
know documentation might need updates, in case of major changes.
Our current rust code isn't always documentation friendly when it
comes to using code snippets. Used rustfmt to apply rust default
formatting on functions that we wanted to show in our documentation
for Frame support
When we want to share our code in our documentation pages, the current
rust formatting isn't so nice to read. Formatted just the portion of
the code that will be shown, for now.
cargo vendor has been part of the core cargo command since Rust 1.37,
and are minimum Rust version is not 1.41, so remove the check. Its
always available now.
Frames:
- sip.pdu
- sip.request_line
- sip.response_line
- sip.request_headers
- sip.response_headers
- sip.request_body
- sip.response_body
The `sip.pdu` frame is always created, the rest only if the record
parser succeeded.
Ticket: #5036.
max-streams and max-table-size
Allows users to find balance between completeness of decoding
and increases resource consumption, which can DOS suricata.
http2_parse_var_uint can overflow the variable-length
integer it is decoding. In this case, it now returns an error
of kind LengthValue.
The new function http2_parse_headers_blocks, which factorizes
the code loop for headers, push promise, and continuation, will
check for this specific error, and instead of erroring itself,
will return the list of so far parsed headers, plus another one
with HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow
This status is then checked by process_headers to create an
app-layer event.
The smb dce_iface keyword must match for all those dcerpc requests
and responses sent in the context of the given interface. They are
not matching as the current bind interfaces are deleted by any
non bind message.
Ticket: 4767
The smb dce_iface keyword must match for all those dcerpc requests and
responses sent in the context of the given interface. They are not
matching because in rs_smb_tx_get_dce_iface, x.req_cmd is erroneously
compared with 1. Fix this by comparing with DCERPC_TYPE_REQUEST instead.
Ticket: 4767
The smb dce_opnum matches all the opnums that are higher that the
indicated opnum. This is due the range comparison if was put in the
exact comparison context, and in case the opnum doesn't match exactly,
then the range comparison is triggered (the upper limit is always true).
Move the erroneus if to the outer context, as else option of the block
checks if comparison should be exact or range.
Ticket: 4767
The smb dce_opnum keyword doesn't match the dcerpc requests/responses.
This occurs because in the rs_smb_tx_match_dce_opnum function, the
x.req_cmd is matched against the erroneous code 1. Fix this by using
DCERPC_TYPE_REQUEST for the comparison instead.
Ticket: 4767
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
Pgsql was using bitwise operations to assign password output config to
its context flags, but mixing that with logic negation of the default
value, resulting in the expressions having a constant value as result.
Bug: #5007
- add nom parsers for decoding most messages from StartupPhase and
SimpleQuery subprotocols
- add unittests
- tests/fuzz: add pgsql to confyaml
Feature: #4241
SMB1 record parsing code simplification.
Frames:
nbss.pdu
nbss.hdr
nbss.data
smb1.pdu
smb1.hdr
smb1.data
smb2.pdu
smb2.hdr
smb2.data
smb3.pdu
smb3.hdr
smb3.data
The smb* frames are created for valid SMB records.
Export the AppLayerEvent derive macro so plugin (or library code) can
use it as expected, for example:
use suricata::applayer::AppLayerEvent;
enum MyEvent {
EventOne,
EventTwo,
}
The macro was generating code that references names use the "crate"
prefix which will fail if the macro is used by a library user or plugin.
Dynamically check where we are running an use the correct import paths
as needed.
Rename the Rust lib to simply "suricata" instead of "suricata_rust".
This allows Rust plugin/library code to use it under the name "suricata"
which is what should be expected.
The name was only "suricata_rust" to prevent on-disk conflict with the C
code, so just rename the file on disk, which doesn't affect how the code
is interacted with from an API layer.
The function macro existed so it would only be enabled on Rust
versions that supported. Now that our MSRV is 1.41, which is
greater than 1.38 we can assume we always have support for
this macro.
Add new methods to set a value as a base64 encoded string of
a byte array. This uses the Rust base64 crate and encodes
directly into the JsonBuilder buffer with no intermediate
buffer required.
jb_set_base64: set a field on an object
jb_append_base64: append a value to an array
It appears that DNS servers will still process a DNS request even if the
z-bit is set, our parser will fail the transaction. So create the
transaction, but still set the event.
Ticket #4924
Ticket: 4862
A transaction to client is always considered
complete in the direction to server and vice versa.
Otherwise, transactions are never complete for
AppLayerParserTransactionsCleanup
Ticket: 4811
Completes commit c023116857
state.free should also close files with ranges
as state.free_tx did already
And file_range field should be reset so that there is no
use after free.
If a HTTP2 transaction gets freed before the end of the range
request, we need to have the files container which is in
the state, to transfer owernship of this file to the files
container.
Ticket: 4811
The `count` combinator preallocates a number of bytes. Since the value
is untrusted, this can result in an Out Of Memory allocation.
Use a maximum value, large enough to cover all current implementations.
rustdoc was complaining about the format of the URL in a comment
while trying to generate documentation. Convert the comment to a
non-rustdoc comment for now to satisfy rustdoc.
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.
Create traits for app-layer State and Transaction that allow
a generic implementation of a transaction iterator that parser
can use when the follow the common pattern for iterating
transactions.
Also convert DNS to use the generic for testing purposes.
As was done only for HTTP1 in previous commit
The verification part stays separated from the parsing part,
as we want to keep on logging invalid ranges values.
When there are a lot of open transactions, as is possible with
modbus, the default tx_iterator will loop for the whole
transacations vector to find each transaction, that means
quadratic complexity.
Reusing the tx_iterator from the template, and keeping as a state
the last index where to start looking avoids this quadratic
complexity.
DNP3, ENIP, HTTP2 and Modbus are supposed to be disabled
by default. That means the default configuration does it,
but that also means that, if they are not in suricata.yaml,
the protocol should stay disabled.
The stream depth setting was broken since it was moved to Rust because
of a missing parser for memory values in configuration.
Use get_memval fn from conf.rs to calculate and fetch the correct
values.
If an HTTP2 file was within only ont DATA frame, the filetracker
would open it and close it in the same call, preventing the
firther call to incr_files_opened
Also includes rustfmt again for all HTTP2 files
After a file has been closed (CLOSE, COMMIT command or EOF/SYNC part of
READ/WRITE data block) mark it as such so that new file commands on that
file do not reuse the transaction.
When a file transfer is completed it will be flagged as such and not be
found anymore by the NFSState::get_file_tx_by_handle() method. This forces
a new transaction to be created.
Add generation of wrapper functions for get_event_info
and get_event_info_by_id to the derive macro. Eliminates
the need for the wrapper method to be created by the parser
author.
Provide generic functions for get_event_info and
get_event_info_by_id. These functions can be used by any app-layer
event enum that implements AppLayerEvent.
Unfortunately the parser registration cannot use these functions
directly as generic functions cannot be #[no_mangle]. So they
do need small extern "C" wrappers around them.
Currently has one derive, AppLayerEvent to be used like:
#[derive(AppLayerEvent)]
pub enum DNSEvent {
MalformedData,
NotRequest,
NotResponse,
ZFlagSet,
}
Code will be generated to:
- Convert enum to a c type string
- Convert string to enum variant
- Convert id to enum variant
Calling a function in unwrap_or causes that function to always
be called even when not needed. Instead use unwrap_or_else with
a closure which will only be called when needed.
Based on the Rust clippy lint that recommends that any public
function that dereferences a raw pointer, mark all FFI functions
that reference raw pointers with build_slice and cast_pointer
as unsafe.
This commits starts by removing the unsafe wrapper inside
the build_slice and cast_pointer macros then marks all
functions that use these macros as unsafe.
Then fix all not_unsafe_ptr_arg_deref warnings from clippy.
Fixes clippy lint:
https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
All cases of our transmute can be replaced with more idiomatic
solutions and do no require the power of transmute.
When returning an object to C for life-time management, use
Box::into_raw to convert the boxed object to pointer and use
Box::from_raw to convert back.
For cases where we're just returning a pointer to Rust managed
data, use a cast.
Registering parsers in Rust requires signatures to be a certain way and
compatible with C. Change signatures of all the functions.
Probe fn has also been changed to return AppProto as required by the new
fn signature.
So that we do not have an endless loop casting index to
u16 and having more than 65536 buffers in one transaction
Changes for all protocols, even ones where it is impossible
to have such a pattern, so as to avoid bad pattern copy/paste
in the future
Before, even if there were no outputs, all the arguments
were evaluated, which could turn expensive
All variables which are used only in certain build configurations
are now prefixed by underscore to avoid warnings
Move tests in a seperate commit so that we can use the previous one for
regression testing. This also gets rid of the temporary glue that made
the C tests work with the rust implementation.
Adds a new rust modbus app layer parser and detection module.
Moves the C module to rust but leaves the test cases in place to
regression test the new rust module.
It doesn't look like flood protection is required with the
stateless parser anymore. It actually can get in the way of TCP
DNS when a large number of requests end-up in the same segment
where a TX can get purged before it has a chance to go through
the normal TX life-cycle.
Lately, some of the TLS data was misdetected as DCERPC/TCP because of
the pattern |05 00|. Add more checks in DCERPC probe function to ensure
that it is in fact DCERPC/TCP.