Commit Graph

16687 Commits (beec1eac2f6bc9f037a13f557ea00d01f80626fd)
 

Author SHA1 Message Date
Jason Ish 604e3c7422 rust/jsonbuilder: make set_uint generic over Into<u64>
Allow `set_uint` to accept any number value that can be converted to a
u64. Prevents callers from having to do `as u64`.

This required fixing up any callers that used `.into()` to just pass in
their value without the into conversion.

Most calls using `as u64` can have that cast removed, with the exception
of `usize` values which must still be cast is conversion can't be
guaranteed to be non-fallible.
2 months ago
Philippe Antoine 146314b0c5 ci: run CodeQL only on python changes
Ticket: 7358
2 months ago
Jeff Lucovsky 91d5b77316 doc/commit
Describe how to use the git commit template. The template helps ensure
that the information needed for evaluation and context is included in
the commit message.

Ticket: <Redmine ticket number>
2 months ago
Jeff Lucovsky 1b88f2189e git: Add commit template
Issue: none

This commit adds a template that identifies commit message elements that
we find important. The Suricata development team requests that
contributions use the template to help improve commit messages. We
reserve the right to strictly enforce adherence to the template in the
future.

Configure git to use this template with:
    git config commit.template ..github/commit-template.txt
2 months ago
Philippe Antoine 5da2b5d7a4 dns/probe: check that a request has at least one query
Ticket: 7279
2 months ago
Philippe Antoine 9b40446bea dns/probe: adds check for 0 records and big size
Ticket: 7279

Make dns probing function stricter to avoid matching on non-DNS
on port 53 and later returning a app-layer error.
2 months ago
Shivani Bhardwaj 2c0d3b83c4 dns: fix clippy lint warnings
Fix vector lint issues:
- same_item_push
- vec_init_then_push
3 months ago
Jeff Lucovsky 89c9cec0bb general/typo: Fix typo in stacksize msg 3 months ago
Victor Julien d11e8a8ee7 doc/userguide: document TCP urgent policy 3 months ago
Victor Julien 779f9d8ba3 eve/flow: add per flow TCP oob urg data counter
If TCP urgent handling is set to the OOB processing, the number of OOB
bytes is tracked for SEQ offset calculations. If this offset is
non-zero, add the field to the flow record.

Ticket: #7411.
3 months ago
Victor Julien 6882bcb3e5 stream: add TCP urgent handling options
TCP urgent handling is a complex topic due to conflicting RFCs and
implementations.

Until now the URG flag and urgent pointer values were simply ignored,
leading to an effective "inline" processing of urgent data. Many
implementations however, do not default to this behavior.

Many actual implementations use the urgent mechanism to send 1 byte of
data out of band to the application.

Complicating the matter is that the way the urgent logic is handled is
generally configurable both of the OS and the app level. So from the
network it is impossible to know with confidence what the settings are.

This patch adds the following policies:

`stream.reassembly.urgent.policy`:

- drop: drop URG packets before they affect the stream engine

- inline: ignore the urgent pointer and process all data inline

- oob (out of band): treat the last byte as out of band

- gap: skip the last byte, but do no adjust sequence offsets, leading to
       gaps in the data

For the `oob` option, tracking of a sequence number offset is required,
as the OOB data does "consume" sequence number space. This is limited to
64k. For this reason, there is a second policy:

`stream.reassembly.urgent.oob-limit-policy`:

- drop: drop URG packets before they affect the stream engine

- inline: ignore the urgent pointer and process all data inline

- gap: skip the last byte, but do no adjust sequence offsets, leading to
       gaps in the data

Bug: #7411.
3 months ago
Victor Julien 4c1554f4f6 stream: remove unused function argument
Sequence number is taken from seg, not the func arg.
3 months ago
Victor Julien ac02a71479 decode/tcp: count urg flag 3 months ago
Victor Julien d1b0d00478 flow/timeout: add frame awareness
If there are still frames in the flow, the detection and logging logic
needs to be able to evaluate them. To do this, make the flow timeout
logic aware of the frames. If frames still exist in a direction, trigger
a FFR packet to be created.

Ticket: #7440.
3 months ago
Victor Julien 0de9eee04d eve/frame: require frame length to be known
Or reach logging threshold.

Avoids logging too early.

Ticket: #7440.
3 months ago
Victor Julien 95ac92f9aa eve/frame: run logging for flow end packets
If there are frames in the flow the flow manager will create flow
timeout packets to log the remaining frames. This requires the logger to
run for those flow timeout packets.

Ticket: #7440.
3 months ago
Victor Julien db589765e4 eve/frame: remove unreachable if branch 3 months ago
Jason Ish 19cf0f8133 dns: provide events for recoverable parse errors
Add events for the following resource name parsing issues:

- name truncated as its too long
- maximum number of labels reached
- infinite loop

Currently these events are only registered when encountered, but
recoverable. That is where we are able to return some of the name,
usually in a truncated state.

As name parsing has many code paths, we pass in a pointer to a flag
field that can be updated by the name parser, this is done in
addition to the flags being set on a specific name as when logging we
want to designate which fields are truncated, etc. But for alerts, we
just care that something happened during the parse. It also reduces
errors as it won't be forgotten to check for the flags and set the
event if some new parser is written that also parses names.

Ticket: #7280
3 months ago
Jason Ish 37f4c52b22 eve/dns: add truncation flags for fields that are truncated
If rrname, rdata or mname are truncated, set a flag field like
'rrname_truncated: true' to indicate that the name is truncated.

Ticket: #7280
3 months ago
Jason Ish 3a5671739f dns: truncate names larger than 1025 characters
Once a name has gone over 1025 chars it will be truncated to 1025
chars and no more labels will be added to it, however the name will
continue to be parsed up to the label limit in attempt to find the end
so parsing can continue.

This introduces a new struct, DNSName which contains the name and any
flags which indicate any name parsing errors which should not error
out parsing the complete message, for example, infinite recursion
after some labels are parsed can continue, or truncation of name where
compression was used so we know the start of the next data to be
parsed.

This limits the logged DNS messages from being over our maximum size
of 10Mb in the case of really long names.

Ticket: #7280
3 months ago
Philippe Antoine 9a53ec43b1 util/streaming-buffer: add extra safety check
Ticket: 7393

Check if GrowRegionToSize is called with an argument
trying to shrink the region size, and if so do nothing,
ie do not try to shrink, and just return ok.

This way, we avoid a buffer overflow from memeset using an
unsigned having underflowed.
3 months ago
Philippe Antoine 8900041405 util/streaming-buffer: check need to grow region
Ticket: 7393

As it was possible before earlier patches to get here
with mem_size lesser than start->buf_size,
which caused then an unsigned underflow and a buffer overflow.
3 months ago
Philippe Antoine 282509f70c util/streaming-buffer: fix regions intersection
This was not a problem for current callers in Suricata,
as RegionsIntersect is only called through StreamingBufferInsertAt
which is only used by TCP...

And TCP uses default region gap = 256kb, and only calls
StreamingBufferInsertAt with a u16, so TCP never inserts a new
data that will strictly contain an existing region augmented
with region gap, which was the only case where RegionsIntersect
returned the wrong result, which could later lead to a
buffer overflow.

Ticket: 7393
3 months ago
Victor Julien 0e4faba79a detect: don't run pkt sigs on ffr pkts
Last packet from the TLS TCP session moves TCP state to CLOSED.

This flags the app-layer with APP_LAYER_PARSER_EOF_TS or
APP_LAYER_PARSER_EOF_TC depending on the direction of the final packet.
This flag will just have been set in a single direction.

This leads to the last packet updating the inspect id in that packets
direction.

At the end of the TLS session a pseudo packet is created, because:
 - flow has ended
 - inspected tx id == 0, for at least one direction
 - total txs is 1

Then a packet rule matches:

```
alert tcp any any -> any 443 (flow: to_server;                  \
        flowbits:isset,tls_error;                               \
        sid:09901033; rev:1;                                    \
        msg:"Allow TLS error handling (outgoing packet)"; )
```

The `SIG_MASK_REQUIRE_REAL_PKT` is not preventing the match, as the
`flowbits` keyword doesn't set it.

To avoid this match. This patch skips signatures of the `SIG_TYPE_PKT`
for flow end packets.

Ticket: #7318.
3 months ago
Philippe Antoine f426ee3ee2 detect: rename stream_log variables
to better reflect their true meaning
3 months ago
Philippe Antoine f2c3776314 detect: log app-layer metadata in alert with single tx
Ticket: 7199

Uses a config parameter detect.guess-applayer-tx to enable
this behavior (off by default)

This feature is requested for use cases with signatures not
using app-layer keywords but still targetting application
layer transactions, such as pass/drop rule combination,
or lua usage.

This overrides the previous behavior of checking if the signature
has a content match, by checking if there is only one live
transaction, in addition to the config parameter being set.
3 months ago
Lukas Sismis 18ab9a6ccd dpdk: set ice PMD RSS key length to 52 bytes for all DPDK versions
ICE driver (Intel E810 NIC) requires/supports 52-byte long RSS key.
The 52 byte key length was mandatory from DPDK 23.11 when Suricata
was starting with independently configured ice PMD.

However, Suricata failed to start when ice PMD was part of
net_bonding PMD, requiring 52 byte RSS key even in DPDK versions
lower than 23.11. Since the support for the longer key is present
since DPDK 19.11 the key is set to 52 bytes for all versions.

Ticket: 7444
3 months 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
3 months ago
Philippe Antoine e62c7d733b rust/ftp: handle NULL inputs
Completes Ticket 7013
3 months ago
Philippe Antoine 38d7900fa9 sip: remove UPDATE method for detection
As it is also used for HTTP/1
Remove it only for TCP and keep it for UDP.

Ticket: 7436
3 months ago
Philippe Antoine e5b98be41f fuzz: simplify target for protocol detection
As too many cases are found when splitting tcp payload
3 months ago
Philippe Antoine 261c15d0e1 fuzz: better init for protocol detection
Ticket: 7435
3 months ago
Victor Julien 7fd707a876 flow/manager: add chunk debug output 3 months ago
Victor Julien ae072d5c07 flow/manager: fix multi instance row tracking
In multi instance flow manager setups, each flow manager gets a slice
of the hash table to manage. Due to a logic error in the chunked
scanning of the hash slice, instances beyond the first would always
rescan the same (first) subslice of their slice.

The `pos` variable that is used to keep the state of what the starting
position for the next scan was supposed to be, was treated as if it held
a relative value. Relative to the bounds of the slice. It was however,
holding an absolute position. This meant that when doing it's bounds
check it was always considered out of bounds. This would reset the sub-
slice to be scanned to the first part of the instances slice.

This patch addresses the issue by correctly handling the fact that the
value is absolute.

Bug: #7365.

Fixes: e9d2417e0f ("flow/manager: adaptive hash eviction timing")
3 months ago
Juliana Fajardini a9b36d88b8 detect/engine/flowint: apply clang format changes
Related to
Task #7426
3 months ago
Juliana Fajardini 6e4a501e7c flowint: add isnotset support
Similar keywords use `isnotset`, while `flowint` only accepted `notset`
Opted to change the code, not only the regex, to keep the underlying
code also following the same patterns.

Task #7426
3 months ago
Victor Julien 2fe2cf8553 eve/alert: enrich decoder event
Default decoder event alert was very sparse, not even logging packet
type and pcap_cnt. Expand support for this record type. It will be more
useful with the ethernet headers and packet field, but these are still
disabled by default.

Ticket: #7433.
3 months ago
Victor Julien b23fa51e33 detect: fix decoder only events
Add missing setup part of the decoder event sgh.

Bug: #7414.
3 months ago
Philippe Antoine 09ba69cfb0 output/http: log invalid status as a string
Ticket: 7311

If response_status_number is not a valid poisitive integer,
we should not try to parse it again, and fail again,
but just log the raw string.
3 months ago
Victor Julien 923ad6af77 af-packet: speed up thread sync during startup
Threads are initialized sequentially to allow for a predictable mapping
of threads and queues. Not all parts of the start up need to be done
sequentially. The setting up of the rings can be very expensive, taking
of a couple of hundred milliseconds. The ring setup doesn't need to be
done sequentially though.

This patch releases the thread early, after bind but before the ring
setups.

Ticket: #7272.
3 months ago
Philippe Antoine b58b886db7 detect/transforms: in place modifications of buffers
As is the case when chaining multiple transforms.
Avoids using memcpy in these cases.

Add tests for these cases.

Ticket: 7409
3 months ago
dependabot[bot] e9173f3b06 github-actions: bump github/codeql-action from 3.27.0 to 3.27.5
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.27.0 to 3.27.5.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Commits](https://github.com/github/codeql-action/compare/v3.27.0...v3.27.5)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
3 months ago
dependabot[bot] c73e90e305 github-actions: bump codecov/codecov-action from 4.6.0 to 5.0.7
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4.6.0 to 5.0.7.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](b9fd7d16f6...015f24e681)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
3 months ago
Jason Ish 289ff25f5b requires: support requires check for keyword
For example:

    requires: keyword foo;

Will require that Suricata supports the "foo" keyword.

Ticket: #7403
3 months ago
Jason Ish 8bcc844b6f sigtable: add function to test for a keyword
To be used by the requires keyword to check for keyword support.

Ticket: #7403
3 months ago
Jason Ish 820a3e51b7 requires: treat unknown requires keywords as unmet requirements
For example, "requires: foo bar" is an unknown requirement, however
its not tracked, nor an error as it follows the syntax. Instead,
record these unknown keywords, and fail the requirements check if any
are present.

A future version of Suricata may have new requires keywords, for
example a check for keywords.

Ticket: #7418
3 months ago
Victor Julien ae10fc3de4 codecov: expect 5 flags to be submitted
Flags are:
- unittests
- suricata-verify
- pcap
- livemode
- fuzzcorpus

This should make sure codecov only adds its report after receiving
the results for each of the flags.
3 months ago
Victor Julien 663fa0c518 github-actions: add basic commandline tests
Run various commandlines, checking that they don't error/crash.

Also counts towards coverage.
3 months ago
Victor Julien d7e1a8f7da github-actions: fix codecov for unittests
Don't overwrite ut coverage with later tests.
3 months ago
Jason Ish 4c12165816 rust: allow static_mut_refs for now
But we should fix all these soon.
3 months ago