Commit Graph

1325 Commits (de5b8ae0b45636352e641cff7eeb4f1fc89c8129)

Author SHA1 Message Date
Philippe Antoine f2a18e91c4 rust: define AppLayerEventType only in rust
And detect.h does no longer depend on app-layer-events.h
2 years ago
Philippe Antoine 668501c225 rust: remove unused 2 years ago
Philippe Antoine 7ca43e7e1f output/snmp: log version from tx
and not the one from state

If a SNMP flow starts with a V2 version transaction,
then there is a V3i version transaction,
we will now log V3 for the second transaction
2 years ago
Philippe Antoine 0ec0d8de67 output/rfb: remove unused function parameters 2 years ago
Philippe Antoine 24c2702a05 output/mqtt: remove unused function parameters 2 years ago
Philippe Antoine 09d364b32f output/krb5: remove unused function parameters 2 years ago
Lancer Cheng abc76e27de smb: fix data padding logic in writeAndX parser
Bug: #6008
2 years ago
Lancer Cheng 000eb91078 smb: fix wrong data offset when wct = 12
Bug: #6008
2 years ago
Philippe Antoine 6350736882 http2: avoid quadratic complexity in headers
When adding an element to the dynamic headers table, the oldest
ones may get evicted. When multiple elements get evicted, they
should get evicted all at once with drain, instead of one by one
as there will be a massive move each time.

Ticket: #6103
2 years ago
Philippe Antoine 7d3aa91bf4 mqtt: fix quadratic complexity
get_tx_by_pkt_id loops only over the last transactions
in case there is a transaction flood

Ticket: #6100
2 years ago
Haleema Khan 8e19906afa mqtt: rustfmt mqtt.rs 2 years ago
Haleema Khan e474858e25 mqtt: add mqtt frames
Adds PDU, Header and Data frame to the MQTT parser.
Ticket: 5731
2 years ago
Jason Ish 33827beae5 jsonbuilder: check buffer growth
Use try_reserve before growing the internal buffer, and the internal
state vector. This allows allocation errors to be caught and an error
returned instead of just aborting the process.

Ticket: #6057
2 years ago
Jason Ish 95cfc2b34f jsonbuilder: rustfmt
Some very minor changes to formatting.
2 years ago
Jason Ish 039c27789b rust: use 2021 edition
With the MSRV being bumped to 1.62 for 7.0, we can move the edition up
to 2021.
2 years ago
Jason Ish c30fff8bcb rust/doc: restore comment with code example, but ignore
Use backticks for proper markdown processing. As Rust code in
backticks is compiled, and this is a non-complete example, tag the
code sample to be ignored.
2 years ago
Philippe Antoine 5391f0a8a0 detect: http_response_line for HTTP2
Ticket: #4067

Synthetized as HTTP/2 <STAT>\r\n
2 years ago
Philippe Antoine 0dca8cc796 detect: http_request_line support for HTTP2
Ticket: #4067

Synthetized as <METHOD> <URI> HTTP/2\r\n
2 years ago
Jason Ish 13fe957b7e rust/doc: wrap some code examples in backticks 2 years ago
Victor Julien d4c60924f1 rust/doc: fix doc compile issues 2 years ago
Eloy Pérez González ed91d689f2 krb5: use req_type instead of msg_type to get request type 2 years ago
Eloy Pérez González a9b7241417 krb5: set msg_type for KRB-ERROR messages to MessageType::KRB_ERROR 2 years ago
Eloy Pérez González 511dbfe171 krb5: add AS-REQ and TGS-REQ transactions
Fix bug in ticket #4529
2 years ago
Victor Julien f9276fdf00 rust: spelling fixes
Thanks to Josh Soref.
2 years ago
Victor Julien cdd3251982 snmp: fix spelling
Thanks to Josh Soref.
2 years ago
Victor Julien ee7ed99b6f rust: spelling 2 years ago
Victor Julien d630f0fa34 rust: rustfmt files with recent new tests 2 years ago
Victor Julien 77f1658c2a rust: fix new clippy warnings 2 years ago
Lancer Cheng 0cf742a9ca smb: add unit tests
Issue: 4865
2 years ago
tianjinshan 2c0c6cb0a5 smb/ntlmssp: fix parsing of negotiate flags
Ticket: #5783
2 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.
2 years ago
Jason Ish f8ec993401 rust/time: add note why this needs to be pinned 2 years ago
Jason Ish 5925b63d82 rust: update x509-parser to 0.15.0 2 years ago
William Correia e378aa8d15 modbus: bump crate version
sawp 0.12 is available and addresses future compilation failures in
dependent crates.
Updated modbus test case to expect 12 bytes needed instead of 15. This
aligns with expectations as the test case slices 3 bytes off the end of
a 12 byte message so needing 12 bytes is correct.

Ticket #5989
2 years ago
Jason Ish b4f0d3c741 rust: update der-parser to 8.2.0
Minimal modifications required on the Suricata side, mainly for fields
becoming private and needing an accessor instead.

Note: As the kerberos parser still depends on der-parser 6.0, we still
have to depend on that so it is depended on, but renamed to
der-parser6. There is not an udpated kerberos-parser yet that uses
der-parser 8.2.0.

Ticket: #5991
2 years ago
Jason Ish 3d9fc3bf1d rust: update snmp-parser to 0.9.0
Updating snmp-parser required directly depending on the asn1-rs crate
for the Oid type, as snmp-parser does not re-export this type anymore.

Ticket: #5992
2 years ago
Jason Ish 0d6628c64e rust: update cargo.lock
Update Cargo.lock, most importantly the Nom 5.1.3 update which will
prevent future breakage by Rustc.
2 years ago
Jason Ish d2fb958e28 rust: fix clippy lint for assert
Fix done automatically by clippy --fix
2 years ago
Haleema Khan 3531a4abaa rfb: rustfmt rfb.rs 2 years ago
Haleema Khan 3eee311350 rfb: add rfb frames, update tests
Adds a PDU frame to the RFB parser.
Update function signature in tests to reflect frames

Ticket: 5717
2 years ago
Victor Julien 0bbc411743 nfs: fix newline in debug messages 2 years ago
Philippe Antoine 9adb59bcdb http2: faster when reducing dynamic headers size
avoid quadratic complexity from removing the first element
and copying all the contents a big number fo times.

Ticket: #5909
2 years ago
Jason Ish 8ef410e284 app-layer: add direction to transaction creation where needed
Build on Eric's but set the direction on transaction creation when
needed. I think this makes it a little more clear, and easier to
document when creating single direction transactions.

This also somewhat abstracts the inner-workings of a directional
transaction from the implementation.

Ticket: #4759
2 years ago
Eric Leblond 9f4ca26962 sip: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 8b0d56c414 nfs: TX are not unidirectional
NFS transactions are not unidirectional so we should not declare
them as such.
2 years ago
Eric Leblond 19174de4f3 quic: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 44482a68b0 ntp: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond c64e4526cd krb: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 322f3896ba mqtt: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 7ce557a44c ike: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 8926d82465 dns: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 744fccee17 bittorrent_dht: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond a82a5aa84b snmp: add TX orientation
Set no inspection in the opposite side of the transaction.

Ticket: #5799
2 years ago
Eric Leblond 5aaf50760f app-layer: add flag to skip detection on TX
Stamus team did discover a problem were a signature can shadow
other signatures.

For example, on a PCAP only containing Kerberos protocol and where the
following signature is matching:

alert krb5 $HOME_NET any -> any any (msg:"krb match"; krb5_cname; content:"marlo"; sid:3; rev:1;)

If we add the following signature to the list of signature

alert ssh $HOME_NET any -> any any (msg:"rr"; content:"rr"; flow:established,to_server; sid:4; rev:2;)

Then the Kerberos signature is not matching anymore.

To understand this case, we need some information:

- The krb5_cname is a to_client keyword
- The signal on ssh is to_server
- Kerberos has unidirectional transaction
- kerberos application state progress is a function always returning 1

As the two signatures are in opposite side, they end up in separate
sig group head.

Another fact is that, in the PCAP, the to_server side of the session
is sent first to the detection. It thus hit the sig group head of
the SSH signature. When Suricata runs detection in this direction
the Kerberos application layer send the transaction as it is existing
and because the alstate progress function just return 1 if the transaction
exists. So Suricata runs DetectRunTx() and stops when it sees that
sgh->tx_engines is NULL.

But the transaction is consumed by the engine as it has been evaluated
in one direction and the kerberos transaction are unidirectional so
there is no need to continue looking at it.

This results in no matching of the kerberos signature as the match
should occur in the evaluation of the other side but the transaction
with the data is already seen has been handled.

This problem was discovered on this Kerberos signature but all
the application layer with unidirectional transaction are impacted.

This patch introduces a flag that can be used by application layer
to signal that the TX should not be inspected. By using this flag
on the directional detect_flags_[ts|tc] the application layer can
prevent the TX to be consumed in the wrong direction.

Application layers with unidirectional TX will be updated
in separate commits to set the flag on the direction opposite
to the one they are.

Ticket: #5799
2 years ago
Eric Leblond 236869bc58 detect: remove STREAM_FLUSH
It is unused in the code so can be removed.

Ticket: #5799
2 years ago
Jason Ish 60e67db452 rust: don't suppress vendor output
There appears to be some errors happening in CI and this may be hiding
the source of the error.
2 years ago
Jason Ish 6f14aed0e6 rust: bundle Cargo.lock
Cargo.lock has to be provided as template, Cargo.lock.in so it can
live beside Cargo.lock in out of tree automake builds, like distcheck.

This will pin Rust dependencies even for git builds, updating
Cargo.lock will now be a manual process that we'll have to take care
of periodically.
2 years ago
Jason Ish d4418034d1 rust: update aes and aes-gcm crates
Addresses RUSTSEC-2021-0059, RUSTSEC-2021-0060.
2 years ago
Jason Ish 159b72c101 rust/clippy: allow derivable impls
The latest Rust will automatically "fix" derivable default
implementation, which is nice, but makes changes that don't meet our
current MSRV, so allow derivable impls for now.
2 years ago
Lancer Cheng 9207012e4b smb: fix parser of ntlmssp negotiateflags
Fix endian-conversion bug in function parse_ntlm_auth_nego_flags

Bug OISF#5783
2 years ago
Haleema Khan 6b55e53ff5 rfb: add unittests to rfb.rs
Task: #5741
2 years ago
Philippe Antoine 233ab11148 smb: handles records with trailing nbss data
If a file (read/write) SMB record has padding/trailing data
after the buffer being read or written, and that Suricata falls
in one case where it skips the data, it should skip until
the very end of the NBSS record, meaning it should also skip the
padding/trailing data.

Otherwise, an attacker may smuggle some NBSS/SMB record in this
trailing data, that will be interpreted by Suricata, but not
by the SMB client/server, leading to evasions.

Ticket: #5786
2 years ago
Philippe Antoine c1b7befb18 smb: checks against nbss records length
When Suricata handles files over SMB, it does not wait for the
NBSS record to be complete, and can stream the payload to the
file... But it did not check the consistency of the SMB record
length being read or written against the NBSS record length.

This could lead to an evasion where an attacker crafts a SMB
write with a too big Length field, and then sends its evil
payload, even if the server returned an error for the write request.

Ticket: #5770
2 years ago
Jason Ish f15f092a69 rfb: remove duplicate logging of depth
The "depth" field in the "pixel_format" object was being logged twice.

Issue: 5813
2 years ago
Jason Ish 717e2b0248 smb: fix duplicate interface logging
An array of interfaces was being logged without creating an array,
resulting in duplicate "interface" objects being logged. Instead put
these interfaces into an array like already done elsewhere.

Issue: 5814
2 years ago
Jason Ish 67baab573b smb: remove duplicate tree_id logging
Remove the second occurrence of tree_id logging which appears to
always be a duplicate of the first tree_id logged, even though they
come from different data structures.

Issue: 5811
2 years ago
Jason Ish e21ae88e05 rust: utility function to copy Rust strings to C strings
As there are a few places where a Rust string is copied into a provided
C string buffer, create a utility function to take care of these
details.
2 years ago
Jason Ish 6344501dba tls: fix date logging for dates before 1970
The Rust time crate used by the x509-parser crate represents dates
before 1970 as negative numbers which do not survive the conversion to
SCTime_t and formatting with the current time formatting functions.

Instead of fixing our formatting functions to handle such dates,
create a Rust function for logging TLS dates directly to JSON using
the time crate that handles such dates properly.

Also add a FFI function for formatting to a provided C buffer for the
legacy tls-log.

Issue: 5817
2 years ago
Jason Ish 64cb687a65 rust: suppress specific manual_flatten list
In this case of debug code, the explicit iterator seems to make more
sense.
2 years ago
Jason Ish 7080ecbb76 rust: remove explicit lifetimes where not needed 2 years ago
Jason Ish e7f5bd047d rust: fix needless borrows of references
Fixed automatically by cargo clippy --fix.
2 years ago
Jason Ish 29f345af1a rust: allow uninlined_format_args
Newer versions of Rust/clippy are getting picky about format strings.
We should allow and use the new style, but also not prevent the old
style.
2 years ago
Jason Ish 3f4dad8676 ftp: add events for command too long
Issue: 5235
2 years ago
Jason Ish 48920bd784 rust/derive: allow event name to be set as attribute
When deriving AppLayerEvent, allow the event name to be set with the
"name" attribute in cases where the transformed name is not suitable.

This allows us to use enum variant names like
"FtpEventRequestCommandTooLong" for direct use in C, but is also a
name that doesn't transform well to an event name in rules, where we
want to see "request_command_too_long".
2 years ago
Philippe Antoine b52293b609 dcerpc: config limit maximum number of live transactions
As is done for other protocols

Ticket: #5779
2 years ago
Philippe Antoine ba99241957 http2: fix leak with range files
Ticket: #5808

May have been introduced by a24d7dc45c

Function http2_range_open expects to be called only when
tx.file_range is nil. One condition to ensure this is to check
that we are beginning the files contents. The filetracker field
file_open is not fit for this, as it may be reset to false.
2 years ago
Victor Julien 37f13a4fc7 smb: set defaults for file transfer limits
Ticket: #5782.
2 years ago
Jason Ish fab3f36b8c dns: never return error on UDP DNS
UDP parsers should never return error as it should indicate to Suricata
that an unrecoverable error has occurred.  UDP being record based for
the most part is almost always recoverable, at least for protocols like
DNS.
2 years ago
Jason Ish d720ead470 dns: split header and body parsing
As part of extra header validation, split out DNS body parsing to
avoid the overhead of parsing the header twice.
2 years ago
Jason Ish 595700ab7e dns: validate header on every incoming message
As UDP streams getting probed, a stream that does not appear to be DNS
at first, may have a single packet that does look close enough to DNS
to be picked up as DNS causing every subsequent packet to result in a
parser error.

To mitigate this, probe every incoming DNS message header for validity
before continuing onto the body.  If the header doesn't validate as
DNS, just ignore the packet so no parse error is registered.
2 years ago
Jason Ish c98c49d4ba dns: parse and alert on invalid opcodes
Accept DNS messages with an invalid opcode that are otherwise
valid. Such DNS message will create a parser event.

This is a change of behavior, previously an invalid opcode would cause
the DNS message to not be detected or parsed as DNS.

Issue: #5444
2 years ago
Jason Ish 7afc2e3aed dns: rustfmt 2 years ago
Jason Ish 39d2524bf6 dns: mark test buffers with rustfmt::skip 2 years ago
Victor Julien 6cc9811edd files: move FileContainer into FileTransferTracker
Update SMB, NFS, HTTP2.
2 years ago
Victor Julien e3e55406a7 files: update API and callers to take stream config
This is to allow not storing the stream buffer config in each file.
2 years ago
Victor Julien 71bc9e75f5 app-layer: get sbconfg with files 2 years ago
Victor Julien a1a221066f files: remove filecontainer drop trait
In preparation of it becoming impossible to use due to the free
function getting an cfg argument.
2 years ago
Victor Julien 0320c03f8c http2: explicity free files
In preparation of adding an argument to the free functions which
means the drop trait can't be used anymore.
2 years ago
Victor Julien 4b1e9f7c21 smb: explicity free files
In preparation of adding an argument to the free functions which
means the drop trait can't be used anymore.
2 years ago
Victor Julien 3a24cce289 nfs: explicity free files
In preparation of adding an argument to the free functions which
means the drop trait can't be used anymore.
2 years ago
Victor Julien 4bfeac6591 nfs: file handling cleanups 2 years ago
Victor Julien 33f6a16290 smb: file handling cleanups 2 years ago
Victor Julien d57510a10f files: remove unused Rust binding for file pruning 2 years ago
Victor Julien a24d7dc45c smb: fix post-trunc chunk behavior
After a gap in a file transaction, the file tracker is truncated. However
this did not clear any stored out of order chunks from memory or stop more
chunks to be stored, leading to accumulation of a large number of chunks.

This patches fixes this be clearing the stored chunks on trunc. It also
makes sure no more chunks are stored in the tracker after the trunc.

Bug: #5781.
2 years ago
Philippe Antoine 55c4834e4e smb: configurable max number of transactions per flow
Ticket: #5753
2 years ago
Philippe Antoine 1d9183638f smb: convert transaction list to vecdeque
Allows for more efficient removal from front of the list.

Ticket: #5753
2 years ago
Philippe Antoine cb89192ec3 smb: fix typo in comment 2 years ago
Haleema Khan cfcb7df9dc mqtt: rustfmt parser.rs 2 years ago
Haleema Khan 23acb89653 mqtt: add unittests for nom7 parsers
Ticket: #5742
2 years ago
Haleema Khan cdc5ccd7f7 rfb: rustfmt parser.rs 2 years ago