Commit Graph

1242 Commits (6190913a461e9943e83f24d78a368043c22f0039)

Author SHA1 Message Date
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
1 year 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
1 year 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
1 year 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.
1 year 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