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.
DNS no longer requires a logger to be registered for to-client and
to-server directions. This has not been required with the stateless
design of the Rust DNS parser.
So that lexical-core, needed by nom, and using bitflags
is used with version 0.7.5 instead of version 0.7.0
which fixed the fact that BITS is now a reserved keyword
in nightly version
Renaming was done with shell commands, git mv for moving the files and content like
find -iname '*.c' | xargs sed -i 's/ikev1/ike/g' respecting the different mixes of upper/lower case.
Lately, Wireguard proto starting w pattern |04 00| is misdetected as
DCERPC/UDP which also starts with the same pattern, add more checks
to make sure that it is the best guess for packet to be dcerpc/udp.
The book defines transmute as "This is really, truly, the most horribly unsafe
thing you can do in Rust. The guardrails here are dental floss."
Transmute can result into mind boggling undefined behaviors. Get rid of
it wherever possible.
As we don't install the libraries by default, provide a make target,
"install-library" to install the libsuricata library files.
If shared library support exists, both the static and shared
libraries will be installed, otherwise only the static libraries
will be installed.
AppLayerRegisterParser was creating a link error when attempting
to use a convenience library for the Suricata C code, then linking
the library of C code with the library of Rust code into a final
Suricata executable, or use with fuzz targets.
By moving AppLayerRegisterParser to the context structure and
calling it like a callback the circular reference is removed
allowing the convenience libraries to work again.
This is also a stepping block to proving a Suricata library
as a single .a or .so file.
The "md-5" crate is part of the RustCrypto project that also
uses the sha1 and sha256 crates we are using. These all implement
the Digest trait for a common API.
Add a Rust module that exposes Rust implementations of
sha256, sha1 and md5 from the RustCrypto project.
This is an experiment in replacing the libnss hash functions with
pure Rust versions that will allow us to remove nss as a compile
time option.
Initial tests are good, even with a 10% or so performance
improvement when being called from C.
Also trying a module naming scheme where modules under the ffi
modules are purely for exports to C, as it doesn't make any
sense to use this new hashing module directly from Rust.
The cbindgen generated header should not include rust.h as
rust.h already includes the generated binding.
Fixup C source code that only pulled the generated include, it
should instead pull in "rust.h" which includes the generated
binding plus other misc. stuff.
Since the completion status was a constant for all parsers, remove the
callback logic and instead register the values themselves. This should
avoid a lot of unnecessary callback calls.
Update all parsers to take advantage of this.
Prior to Rust 1.44, Cargo would name static libs with the .lib
extension. 1.44 changes this extension to .a when running under
a GNU environment on Windows like msys to make it more similar
to other unix environments.
Now assume static library name to be the same on Windows and
unix, but rename the .lib if found to still support older
versions of Rust on Windows.
Implement missing transaction handling.
Fix logging wrongly casting 'state' to DCERPCState instead of
DCERPCUDPState leading to crashes and malformed output.
Remove unused fields from DCERPCUDPState.
warning: getting the inner pointer of a temporary `CString`
this `CString` is deallocated at the end of the statement,
bind it to a variable to extend its lifetime
Expand macros in the do_log macro after checking the log level
instead of each log macro (ie: SCLogDebug) expanding the macros
then passing off to do_log to have the log level check.
Will eliminate any expense of expanding macros if this log level
does not permit the given message to be logged.
Redmine issue:
https://redmine.openinfosecfoundation.org/issues/4114
Log fields that only are meant to be in a PDU for a particular RPC
version. Since DCERPC/UDP works on RPC version 4 and DCERPC/TCP works on
RPC version 5, there are certain fields that are particular to each
version.
Remove call_id from the logger for UDP.
Add activityuuid and seqnum fields to the logger for UDP.
call_id and (activityuuid + seqnum) fields are used to uniquely pair a
request with response for RPC versions 5 and 4 respectively.
So far, request and response were paired with serial number fields in
the header. This is incorrect. According to
https://pubs.opengroup.org/onlinepubs/9629399/chap12.htm,
"Together, the activity UUID and the sequence number uniquely identify
a remote procedure call."
Hence, add activity uuid and sequence number to the transaction and pair
the request accordingly. Remove incorrect handling of this and fix
tests.
warning: variable does not need to be mutable
--> src/dcerpc/dcerpc.rs:1036:42
|
1036 | let tx = if let Some(mut tx) = self.get_tx_by_call_id(current_call_id, core::STREAM_TOCLIENT) {
| ----^^
| |
| help: remove this `mut`
|
= note: `#[warn(unused_mut)]` on by default
warning: variable does not need to be mutable
--> src/dcerpc/dcerpc.rs:1061:30
|
1061 | Some(mut tx) => {
| ----^^
| |
| help: remove this `mut`
The headers table from client to server
and the one from server to client
may have different maximum sizes
(even if both endpoints have to keep both tables)
Scenario is use of dummy padding in write AndX request
or other similar commands using a data offset.
Parsing skips now these dummy bytes, and generates one event
Evasion scenario is
- a first dummy write of one byte at offset 0 is done
- the second full write of EICAR at offset 0 is then done
and does not trigger detection
The last write had the final value, and as we cannot "cancel"
the previous write, we set an event which is then transformed into
an app-layer decoder alert
This patch addresses issues discovered by redmine ticket 3896. With the
approach of finding latest record, there was a chance that no record was
found at all and consumed + needed became input length.
e.g.
input_len = 1000
input = 01 05 00 02 00 03 a5 56 00 00 .....
There exists no |05 00| identifier in the rest of the record. After
having parsed |05 00|, there was a search for another record with the
leftover data. Current data length at this point would be 997. Since the
identifier was not found in the data, we calculate the consumed bytes at
this point i.e. consumed = current_data.len() - 1 which would be 996.
Needed bytes still stay at a constant of 2. So, consumed + needed = 996
+ 2 = 998 which is lesser than initial input length of 1000 and hence
the assertion fails.
There could be two fixes to this problem.
1. Finding the latest record but making use of the last found record in
case no new record was found.
2. Always use the earliest record.
This patch takes the approach (2). It also makes sure that the gap and
current direction are the same.
When one side of the connection reaches the STREAM_DEPTH condition the
parser should be aware of this. Otherwise transactions will forever be
waiting for data in that direction.
This parameter is NULL or the pointer to the previous state
for the previous protocol in the case of a protocol change,
for instance from HTTP1 to HTTP2
This way, the new protocol can use the old protocol context.
For instance, HTTP2 mimicks the HTTP1 request, to have a HTTP2
transaction with both request and response
To signal incomplete data, we must return the number of
consumed bytes. When we get a banner and some records, we have
to take into account the number of bytes already consumed by
the banner parsing before reaching an incomplete record.
As is documented in RFC 7541, section 6.1
The index value of 0 is not used. It MUST be treated as a decoding
error if found in an indexed header field representation.
Added `dns_parse_rdata_soa` to parse SOA fields into an `DNSRDataSOA`
struct.
Added logging for answer and authority SOA records in both version
1 & 2, as well as grouped formats.
Borrow a macro from https://github.com/popzxc/stdext-rs that
will give us the Rust function name in SCLog messages in Rust.
As this trick only works on Rust 1.38 and newer, keep the old
macro around and set a feature based on a Rust version test
done during ./configure.
Functions written in Rust will need to suricata::plugin::init()
to bootstrap themselves. This bootstrap process sets the log level
within the Rust address space, and hooks up function pointers
that are expected to be set during normal runs of Suricata.
Make sure the log level is setup with a pure Rust function, so
when it is set, its set within the address space of the caller.
This is important for Rust plugins where the Rust modules are not
in the address space of the Suricata main process.
This commit logs http2 as an http event. The idea is to somewhat
normalize http/http2 so common info can be version agnostic.
This puts the http2 specific fields in an "http2" object inside
the "http" object.
HTTP2 headers/values that are in common with HTTP1 are logged
under the "http" object to be compatible with HTTP1 logging.
Only run cbindgen when necessary. This is a bit tricky. When
building a dist we want to unconditionally build the headers.
When going through a "make; sudo make install" type process,
cbindgen should not be run as the headers already exist, are
valid, and the environment under sudo is more often than
not suitable to pick up the Rust toolchains when installed
with rustup.
For the normal "make" case we have the gen/rust-bindings.h file
depend on library file, this will cause it to only be rebuilt
if the code was modified.
For "make dist" we unconditionally create "dist/rust-bindings.h".
This means the generated file could be in 2 locations, so update
configure.ac, and the library search find to find it.
The "gen/rust-bindings.h" should be picked up first if it exists,
for those who develop from a dist archive where "dist/rust-bindings.h"
also exists.
Not completely happy having the same file in 2 locations, but not
sure how else to get the dependency tracking correct.
Elastic search didn't accept the 'hassh' and 'hassh.string'. It would
see the first 'hassh' as a string and split the second key into a
object 'hassh' with a string member 'string'. So two different types
for 'hassh', so it rejected it.
This patch mimics the ja3(s) logging by creating a 'hassh' object
with 2 members: 'hash', which holds the md5 representation, and
'string' which holds the string representation.