Commit Graph

15560 Commits (cc5511c09427c13a8e3c4be2b71ca14bf95ed438)
 

Author SHA1 Message Date
Jason Ish cc5511c094 github-ci: use current directory for unit test logging
/tmp appears to exist when you make it, but doesn't appear to actually
exist after msys translation, so just use "."

(cherry picked from commit 11cef2980b)
11 months ago
Victor Julien 8a32965609 detect: don't set conflicting packet/flow actions
If for the same a packet a drop rule and a pass rule would match,
the applying of actions could be contradictionary:

- the drop would be applied to the packet
- the pass rule would also be considered, not overriding the drop,
  but still setting the flow pass flag.

This would lead to the packet being dropped, but the rest of the
flow getting passed, including retransmissions of the dropped
packet.

This patch only sets drop/pass actions if no conflicting action
has been set on the packet before. It respects the action-order.

Bug: #7653.

Fix based on:
57b17fb3b2 ("detect: don't set conflicting packet/flow actions")
11 months ago
Philippe Antoine 155487e79d detect: do not bug on tx data being NULL
Ticket: 7610

As this can happen for HTTP1 in Suricata 7

This was fixed in Suricata 8 by f301cd3702
and 833a738dd1 from ticket 5739
12 months ago
Philippe Antoine e23aa06196 dnp3: mark tx as updated when creating it
Ticket: 7668

We should set updated_tx when allocating a dnp3 tx

(cherry picked from commit e41c28f7c9)
1 year ago
Philippe Antoine 529bec83b2 ftp: mark tx as updated when creating it
Ticket: 7668

We should set updated_tx when allocating a ftp tx

Was already done right for updated_tc

(cherry picked from commit f24d3ffb74)
1 year ago
Philippe Antoine a954066812 http1: always mark tx as updated on request/response start
Ticket: 7668

We should set updated_tx when allocating HtpTxUserData

(cherry picked from commit a5b987266b)
1 year ago
Philippe Antoine 858739519d rust: fix clippy warning manual_contains
warning: using `contains()` instead of `iter().any()` is more efficient
   --> src/http2/http2.rs:267:20
    |
267 |                 if block.value.iter().any(|&x| x == b'@') {
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `block.value.contains(&b'@')`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains
    = note: `#[warn(clippy::manual_contains)]` on by default

(cherry picked from commit 0f3932afb7)
1 year ago
Jason Ish 2826d670d6 rust: fix rustdoc indentation in lists
Ticket: #7652
1 year ago
Jason Ish 1504dcea4a rust: fix clippy warnings for unspecified extern ABI
Fix done by clippy --fix.

Ticket: #7652
1 year ago
Jason Ish f19e28f121 version: start development towards 7.0.11 1 year ago
Philippe Antoine cb38f509ce fuzz: set flow flags as in Suricata
Fixes: d8ddef4c14 ("detect: delay tx cleanup in some edge case")
(cherry picked from commit 09aed7e243)
1 year ago
Jason Ish 4a9d081490 github-ci: don't run builds on PR if only docs changed
(cherry picked from commit 3658d502ff)
1 year ago
Jason Ish cd5a961b36 github-ci: update Fedora non-root build to Fedora 41
(cherry picked from commit 65b863b087)
1 year ago
Jason Ish 8674befdc7 github-ci: remove fedora 40 builds where 41 exists
Remove Fedora 40 builds where there is a Fedora 41 equivalent.

(cherry picked from commit 70d5bae160)
1 year ago
Victor Julien 7816a51f98 github-ci: update Fedora 39 jobs to 41
(cherry picked from commit c56b741088)
1 year ago
Victor Julien 9d89bbd2dd github-ci: bump known rust ver to 1.85.1 1 year ago
Shivani Bhardwaj 9378707700 release: 7.0.10; update changelog 1 year ago
Victor Julien 9b02cd42d0 detect: add padding to suppress scan-build warning
Add to DetectEngineCtx to avoid:

./detect.h:840:16: warning: Excessive padding in 'struct DetectEngineCtx_' (32 padding bytes, where 0 is optimal). Optimal fields order: sig_list, srepCIDR_ctx, sig_array, sc_sig_order_funcs, sgh_hash_table, mpm_hash_table, pattern_hash_table, dup_sig_hash_table, spm_global_thread_ctx, mpm_ctx_factory_container, sgh_array, decoder_event_sgh, rule_file, sigerror, keyword_hash, next, dport_hash_table, tcp_whitelist, udp_whitelist, address_table, metadata_table, buffer_type_hash_name, buffer_type_hash_id, app_mpms_list, app_inspect_engines, pkt_inspect_engines, pkt_mpms_list, frame_inspect_engines, frame_mpms_list, prefilter_hash_table, fp_support_smlist_list, class_conf_ht, class_conf_regex, class_conf_regex_match, reference_conf_ht, reference_conf_regex, reference_conf_regex_match, ea, tenant_path, requirements, last_reload, sig_stat, ths_ctx, io_ctx, flow_gh, tenant_id, sig_cnt, srep_version, sig_array_size, sig_array_len, signum, non_pf_store_cnt_max, inspection_recursion_limit, filemagic_thread_ctx_id, max_fb_id, sgh_array_cnt, sgh_array_size, sgh_mpm_context_proto_tcp_packet, sgh_mpm_context_proto_udp_packet, sgh_mpm_context_proto_other_packet, sgh_mpm_context_stream, byte_extract_max_local_id, version, rule_line, keyword_id, type, ref_cnt, loader_id, prefilter_setting, buffer_type_id, app_mpms_list_cnt, pkt_mpms_list_cnt, frame_mpms_list_cnt, prefilter_id, filedata_config, max_uniq_toclient_groups, max_uniq_toserver_groups, base64_decode_max_len, filestore_cnt, failure_fatal, flags, mpm_matcher, spm_matcher, guess_applayer, sigerror_silent, sigerror_ok, sigerror_requires, filedata_config_initialized, sgh_mpm_ctx_cnf, config_prefix, sm_types_prefilter, sm_types_silent_error, consider reordering the fields or adding explicit padding members [optin.performance.Padding]
  840 | typedef struct DetectEngineCtx_ {
      |         ~~~~~~~^~~~~~~~~~~~~~~~~~
  841 |     bool failure_fatal;
      |     ~~~~~~~~~~~~~~~~~~~
  842 |     uint8_t flags;       /**< only DE_QUIET */
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  843 |     uint8_t mpm_matcher; /**< mpm matcher this ctx uses */
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  844 |     uint8_t spm_matcher; /**< spm matcher this ctx uses */
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  845 |
  846 |     uint32_t tenant_id;
      |     ~~~~~~~~~~~~~~~~~~~
  847 |
  848 |     Signature *sig_list;
      |     ~~~~~~~~~~~~~~~~~~~~
  849 |     uint32_t sig_cnt;
      |     ~~~~~~~~~~~~~~~~~
  850 |
  851 |     /* version of the srep data */
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  852 |     uint32_t srep_version;
      |     ~~~~~~~~~~~~~~~~~~~~~~
  853 |
  854 |     /* reputation for netblocks */
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  855 |     SRepCIDRTree *srepCIDR_ctx;
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
1 year ago
Victor Julien dba022d150 af-packet: use actual snaplen in bpf
Avoids setting a 0 snaplen in BPF, leading to an error.

Fixes: b8b6ed550a ("af-packet: delay setting default-packet-size for af-packet")

Ticket: #7618.
(cherry picked from commit 749ffbd06a)
1 year ago
Victor Julien c6fd3e69bc datasets: work around scan-build warning
datasets.c:493:27: warning: Dereference of null pointer [core.NullDereference]
  493 |     DEBUG_VALIDATE_BUG_ON(set->hash->config.hash_size != hashsize);
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
./util-validate.h:95:44: note: expanded from macro 'DEBUG_VALIDATE_BUG_ON'
   95 | #define DEBUG_VALIDATE_BUG_ON(exp) BUG_ON((exp))
      |                                            ^~~
./suricata-common.h:307:36: note: expanded from macro 'BUG_ON'
  307 |         #define BUG_ON(x) assert(!(x))
      |                                    ^
/usr/include/assert.h:109:7: note: expanded from macro 'assert'
  109 |     ((expr)                                                             \
      |       ^~~~
1 warning generated.

(cherry picked from commit c6fdf99cec)
1 year ago
Shivani Bhardwaj 77652d0f7a version: start development towards 7.0.10 1 year ago
Shivani Bhardwaj 76729e4266 release: 7.0.9; update changelog 1 year ago
Juliana Fajardini f6140df708 upgrade: list inspection recursion default limit
As the yaml indicated before that if no value was specified there were
no limits, and now there will be one.

(cherry picked from commit 3985b24e1b)
1 year ago
Victor Julien d86c5f9f0c datasets: set higher hashsize limits
To avoid possible upgrade issues, allow higher defaults than in the
master branch. Add some upgrade guidance and a note that defaults will
probably be further reduced.
1 year ago
Philippe Antoine e28c8c655a detect: add configurable limits for datasets
Ticket: 7615

Avoids signatures setting extreme hash sizes, which would lead to very
high memory use.

Default to allowing:
- 65536 per dataset
- 16777216 total

To override these built-in defaults:

```yaml
datasets:
  # Limits for per rule dataset instances to avoid rules using too many
  # resources.
  limits:
    # Max value for per dataset `hashsize` setting
    #single-hashsize: 65536
    # Max combined hashsize values for all datasets.
    #total-hashsizes: 16777216
```

(cherry picked from commit a7713db709)
1 year ago
Victor Julien 2f432c99a9 datasets: improve default hashsize handling
Make hashsize default local to dataset code, instead of relying on the
thash code.

Use the same default value as before.

(cherry picked from commit d32a39ca4b)
1 year ago
Jason Ish fc6022286c doc/userguide: af-packet upgrade notes
Add note about increased block size and how to change it back to old
defaults if needed.

Ticket: #7458
(cherry picked from commit c6d18fc871)
1 year ago
Jason Ish c3be2b29b5 af-packet: delay setting default-packet-size for af-packet
AF_PACKET needs more information about its configuration before we can
set the default packet size, so on startup, leave unset in suricata.c
if in AF_PACKET mode.

If defrag is enabled, use a default packet size of 9k for tpacket-v2.
This can still lead to truncation events, then the user can increase
their 'default-packet-size'.

Tpacket-v3 does not need an increased packet size as it will handle
any size of packet that is smaller than the configured block size
which now has a default of 128k.

9k for the snap is somewhat arbitrary but is large enough for the
common 9000 jumbo frame plus some extra headers including tpacket
headers.

Ticket: #7458
(cherry picked from commit b8b6ed550a)
1 year ago
Jason Ish cbd5bfbbc1 af-packet: warn that tpacket-v3 is better for non-inline usage
Ticket: #7458
(cherry picked from commit 8c7ac89791)
1 year ago
Jason Ish cd00499863 af-packet: add event for packets truncated by af-packet
Ticket: #7458
(cherry picked from commit d78f2c9a4e)
1 year ago
Jason Ish 916ed77121 af-packet: warn if v3 block size is not large enough for defrag
If using tpacket-v3 and defrag, warn if the block size is not large
enough for a fully defragmented packet.

Ticket: #7458
(cherry picked from commit 9f96975d55)
1 year ago
Jason Ish efc74ff9ed af-packet: warn if v2 block size not large enough for defrag
If using tpacket-v2, defrag and a user provided v2-block-size, warn if
the block size is not large enough to hold one fully defragmented
packet.

Ticket: #7458
(cherry picked from commit 320ef7b617)
1 year ago
Jason Ish f3d52ef8cf af-packet: make tpacket-v2 block size configurable
With the change of the default tpacket-v2 block size from 32k to 128k,
allow it to be configurable for users who may want to make it larger,
or revert it back to the pre 7.0.9 default of 32k.

Ticket: #7458
(cherry picked from commit 5871c6458c)
1 year ago
Jason Ish b2d2b70745 af-packet: increase default block size
Increase the default block size from 32k to 128k. This allows for a
fully defragmented packet to fit in the buffer.

Ticket: #7458
(cherry picked from commit c342b054f4)
1 year ago
Jason Ish 0f21d899f1 af-packet: warn if defrag not suitable for mode
AF_PACKET defrag should not be used for inline modes. Its possible that
a packet received could be larger than can be set when defrag is
enabled, so warn if disabled for inline use.

Likewise, warn if defrag is disabled for IDS use, or non-inline mode.

Ticket: #7458
(cherry picked from commit 808502d5ca)
1 year ago
Jason Ish 1dd4664714 af-packet: check defrag value even if cluster-type not set
If cluster-type was not set we default to "cluster_flow" with defrag
always on. Instead check for defrag value and disable defrag if disabled
by the user.

Ticket: #7458
(cherry picked from commit 25d0fba912)
1 year ago
Philippe Antoine bab716776b detect: limit base64_decode `bytes` to 64KiB
Ticket: 7613

Avoids potential large per-thread memory allocation. A buffer with the
size of the largest decode_base64 buffer size setting would be allocated
per thread. As this was a u32, it could mean a per-thread 4GiB memory
allocation.

64KiB was already the built-in default for cases where bytes size wasn't
specified.

(cherry picked from commit 32d0bd2bbb)
1 year ago
Philippe Antoine 0b39cf06f8 detect: non infinite default value for inspection-recursion-limit
So that empty config are protected by this setting as was intended.

Set to unlimited for fuzz testing.

(cherry picked from commit b9b797f1f4)
1 year ago
Philippe Antoine f6c9490e1f detect/pcre: avoid infinite loop after negated pcre
Ticket: 7526

The usage of negated pcre, followed by other relative payload
content keywords could lead to an infinite loop.

This is because regular (not negated) pcre can test multiple
occurences, but negated pcre should be tried only once.

(cherry picked from commit b14c67cbdf)
1 year ago
Victor Julien b81392dde5 stream: RST no longer acks all data
Since forever (1578ef1e3e) a valid RST
would update the internal `last_ack` representation to include all
unack'd data. This was originally done to make sure the unACK'd data was
inspected/processed at flow timeout.

It was observed however, that if GAPs existed in this unACK'd data, a
GAP could be reported in the stats and a GAP event would be raised. This
doesn't make sense, as missing segments in the unACK'd part of the
stream are completely normal. Segments simply do not all arrive in
order.

It turns out that the original behavior of updating `last_ack` to
include all unACK'd data is no longer needed.

For raw stream inspection, the detection engine will already include the
unACK'd data on flow end.

For app-layer updates the unACK'd data is often harmful, as the data
often has GAPs. Parser like the http parser would report these GAPs and
could also get confused about the post-GAP data being a new transaction
including a file. This lead to many reported errors and fantom txs and
files.

Since the GAP detection uses `last_ack` to determine GAPs, not moving
`last_ack` addresses the GAP false positives.

Ticket: #7422.
(cherry picked from commit bd1b9f6229)
1 year ago
Philippe Antoine b30f286a6e detect: delay tx cleanup in some edge case
Ticket: 7552

f->sgh_toserver may be NULL but because FLOW_SGH_TOSERVER is unset
and thus, we want to delay cleanup until detection has really been
run with the right signature group head.

This may happen for a rule using
`alert tcp any any -> any any` and
a app-layer keyword to client
with a app-layer supporting both udp and tcp
with stream.midstream=true
and with the first packet of a flow being a server response

In this case, we swap the flow and reset its signature group heads

(cherry picked from commit d8ddef4c14)

Additional fix in rfb unit test which moved to SV in suricata 8
1 year ago
Philippe Antoine 1907b9f225 detect: reset signature groups when reversing flow
Ticket: 7552

When we use midstream, and the first packet we see of a flow is
a response from server, and we want to match on some signature
to client :
- we had first set sgh_toserver/FLOW_SGH_TOSERVER as we first
  thought this was a packet to server
- we then swap/reverse the flow, so sgh_toclient becomes sgh_toserver
  but it contains signatures to server and cannot match our
  to_client signature

The detect engine with DetectRunSetup will set again the
signatures group heads properly

(cherry picked from commit d74bc774b7)
1 year ago
Philippe Antoine da092a5d83 files: append data on closing even with FILE_NOSTORE
Ticket: 7577

When HTTP1 post multipart handles a small file, it will call
HTPFileClose with some data
This data needs to be appended to the streaming buffer for usage
by file.data keyword even if we do not end up storing the file

(cherry picked from commit f68e2f5537)
1 year ago
Philippe Antoine 782f35c5cf 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
(cherry picked from commit b02557ac7d)
1 year ago
Philippe Antoine 05bf4a8dec quic: discard late retry packets
Ticket: 7556

See RFC 9000 section 17.2.5.2 :
After the client has received and processed an Initial
or Retry packet from the server,
it MUST discard any subsequent Retry packets that it receives.

(cherry picked from commit 726de5520f)
1 year ago
Philippe Antoine 530f1a40e4 quic: decrypt only initial packets
Ticket: 7556

Avoids failed_decrypt events when the first packet seen is not
a Quic Initial packet

(cherry picked from commit d61f36c66f)
1 year ago
Philippe Antoine ac6dcd6fbf quic: handle retry packets
Ticket: 7556
(cherry picked from commit 6d8910d245)
1 year ago
Philippe Antoine 31d57ef7fc quic: handle fragmented hello over multiple packets
Ticket: 7556

To do so, we need to add 2 buffers (one for each direction)
to the QuicState structure, so that on parsing the second packet
with hello/crypto fragment, we still have the data of the first
hello/crypto fragment.

Use a hardcoded limit so that these buffers cannot grow indefinitely
and set an event when reaching the limit

(cherry picked from commit f295cc059d)
1 year ago
Philippe Antoine ce90ff187e quic: parse ack frame number 3
cf rfc9000 section 19.3. ACK Frames

Ticket: 7556
(cherry picked from commit 68adc87bd2)
1 year ago
Philippe Antoine 26a1d02722 quic: move all_consuming check to callee
Will alow to have decode_frames accept one additional parameter
with past fragment data

(cherry picked from commit ee04d667b5)
1 year ago