Commit Graph

19281 Commits (4985eb9daea2f765b6ef59a58fa02e5c71c0a77c)
 

Author SHA1 Message Date
Philippe Antoine 4985eb9dae smtp/mime: consolidate a MimeStateSMTP restart function
Ticket: 8649

Fully resets all the fields before tackling an ecapsulated message
to avoid evasion, due to the encoding of the upper file
leaking into the next one...
1 month ago
Philippe Antoine 82c4190558 ftp: do not create more than max-tx transactions
Ticket: 8592

Fixes: 5ddd808e9b ("ftp: don't halt the flow when raising
too_many_transactions")

In the case we receive a big chunk of TCP data, we end up
creating much more than max-tx transactions, and have
quadratic complexity on this packet, even if all
these transactions get cleaned up at the end of the processing
of this packet.
1 month ago
Philippe Antoine 4a728464e6 ippair: check ip family when comparing
Ticket: 8559

Do not accept an IPv6 as an IPv4 (even if the hash is likely
different in the first place)
1 month ago
Jason Ish 40503f6602 github-ci: fix new authors check
Shortly after minimizing the new authors check to one workflow, github
released an action update that required more permissions due to an
attack scenario, which didn't really apply to us as we didn't run code
from the remotes fork.

However, to avoid extending permissions, rework the authors check to
pull the OISF repo, checkout the PR fork as history, then do the new
authors check. This safely allows the check to run in the context of our
repo without the fork being able to inject code execution.
1 month ago
Amir Boussejra f917926165 ebpf: raise RLIMIT_MEMLOCK only on kernel >= 5.11
EBPFLoadFile() unconditionally raised RLIMIT_MEMLOCK to infinity and
aborted the eBPF/XDP load if the call failed.

Since Linux 5.11 BPF map memory is memcg-accounted and is no longer
charged against RLIMIT_MEMLOCK (https://lwn.net/Articles/829307/), so
raising the limit is unnecessary on those kernels.

Ticket: 8719
1 month ago
Giuseppe Longo 499995bf0e sip: store frame lens as u32 to avoid body truncation
Body lengths were stored as u16, so a SIP body of 65536 bytes truncated
body_len to 0 and the RequestBody/ResponseBody frame was never created,
letting body content evade inspection. Widen the framing fields to u32.

Ticket #8582
1 month ago
Philippe Antoine d62acf5d75 detect: use only one non-prefilter frame for prefilter
Ticket: 8590
1 month ago
Shivani Bhardwaj 0f56783ec1 dcerpc: make tx id handling consistent
dcerpc parser creates a new tx with id 0 and compensates for the 1 based
index handling throughout the code by overriding that value in a trait
implementation. Make this consistent with other applayer parsers.

Task 8720
1 month ago
Adam Kiripolsky b8d7f894be flow/manager: no flow timeout when hash-size < 10
Fix Flow Manager error when flows were not being timed out
in case flow.hash-size < 10.
This happened because calculation of variable rows_per_sec in
flow-manager.c rounded the value to zero.

This commit ensures that variable rows_per_sec is at least 1.

Ticket: 8710
1 month ago
Victor Julien 9555e3add6 rdp: fix tx id handling
Tx ID handling did not take the required + 1 into account.

From a report:

        RDP can skip cleanup because its id convention does not match the
        generic Rust iterator. The generic iterator in applayer.rs returns
        tx.id() - 1, and cleanup trusts that id when calling StateTransactionFree
        in app-layer-parser.c. RDP registers that iterator in rdp.rs, but
        RdpTransaction::id() returns the stored id unchanged in rdp.rs, while
        free_tx also compares against the raw stored id in rdp.rs. For a single
        freeable RDP tx with stored id 1, the iterator returns C id 0; cleanup
        calls free_tx(0), nothing is removed, then has_next == false allows
        min_id to advance to total_txs in app-layer-parser.c. That leaves the
        tx live but now below min_id, so later cleanup will not revisit it.

This patch brings the handling in line with the other parsers.

Bug: #8717.
1 month ago
Victor Julien 9e31a21aba detect/firewall: apply accept if last tx was skipped
In firewall mode, a accept:hook or accept:tx needs to lead to a accept
packet when the action is applied to the last TX. For this the code
relied of the `DetectTransaction::is_last` field, where the assumption
was that there would always be an inspection on the last transaction.
This assumption was wrong however, as transactions can be skipped for a
few reasons: not updated, fully inspected, unidirectional for the other
direction. This would cause the accept not be applied to the packet,
leading to a default drop.

The reason this wasn't noticed before is that until now the work had
focused on protocols that used sequential transactions (http/tls),
and/or short lived sequential unidir transactions (dns)

This patch addresses the issue by making a simple assumption: if the
last available transaction in the main detection loop is skipped, it
means it has been accepted before. Therefore we can apply the "accept
packet" logic in this case.

Bug: #8698.
1 month ago
Victor Julien ee1b9b9212 detect: use next tx info from iterator
In non-sequential transaction handling, the is_last set could be
incorrect.
1 month ago
Victor Julien b065a4bf98 http1: fix transaction iterator
The has_next field was not correctly set.
1 month ago
Victor Julien 3b351e2574 detect: move packet alert logic into helper 1 month ago
Victor Julien 695c9d8d20 detect/alert: split append func per packet/tx 1 month ago
Adam Kiripolsky 74bc0feb1a af-packet: invert ebpf g_flowv4_ok log flag
Fix how error messages are printed in a case error occurs in the ebpf
flow table code. The log flag g_flowv4_ok was inverted,
so the error messages were never printed.

The fix follows g_flowv6_ok flag code structure.

Ticket: 8704
1 month ago
dependabot[bot] 6cbf511f65 github-actions: bump github/codeql-action/upload-sarif
Bumps [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) from eda5730a8bfb740e03a28087a958444c646e5842 to 1f34ec16430d82636d18716acc7aaa6d843b35a9.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](eda5730a8b...1f34ec1643)

---
updated-dependencies:
- dependency-name: github/codeql-action/upload-sarif
  dependency-version: 1f34ec16430d82636d18716acc7aaa6d843b35a9
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
1 month ago
dependabot[bot] 8002b59993 github-actions: bump actions/checkout from 6.0.2 to 7.0.0
Bumps [actions/checkout](https://github.com/actions/checkout) from 6.0.2 to 7.0.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Commits](https://github.com/actions/checkout/compare/v6.0.2...v7)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: 7.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
1 month ago
dependabot[bot] 0a2b024317 github-actions: bump vmactions/freebsd-vm from 1.4.6 to 1.5.0
Bumps [vmactions/freebsd-vm](https://github.com/vmactions/freebsd-vm) from 1.4.6 to 1.5.0.
- [Release notes](https://github.com/vmactions/freebsd-vm/releases)
- [Commits](a6de9343ef...5a72679103)

---
updated-dependencies:
- dependency-name: vmactions/freebsd-vm
  dependency-version: 1.5.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
1 month ago
dependabot[bot] 7a419071c8 github-actions: bump actions/cache from 5 to 6
Bumps [actions/cache](https://github.com/actions/cache) from 5 to 6.
- [Release notes](https://github.com/actions/cache/releases)
- [Commits](https://github.com/actions/cache/compare/v5...v6)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
1 month ago
Philippe Antoine aa59bb040b detect: allocate array on the heap, with a dedicated struct
buffer_type_id is a u32

We have a list per buffer of a list of AppProto
But the list of AppProto can grow maximum for the case of file.data
to all the alprotos supporting files.

Ticket: 8001
1 month ago
Lukas Sismis 376db8fc6b ci: upgrade FreeBSD Python package to 3.12
FreeBSD upgraded the default Python package to 3.12.
With that the pip packages also updated to the new
version.
1 month ago
Philippe Antoine 2205761bf6 doc: add missing "value" to entropy examples
Commit from Hydraze
1 month ago
Jason Ish e7e837c6af psl: update to latest version
Ticket: #8147
1 month ago
Victor Julien c1392d99a3 http/xff: harden code against http in detection-only 1 month ago
Victor Julien c01aa71b56 app-layer: treat pstate as non-NULL
In almost every case, if there is a alstate there is also a pstate. So
remove the conditional pstate handling, and replace it by
unconditionally using the pointer. Add debug validation to make sure the
assumption is and stays correct.

Explicitly handle the one exception in AppLayerParserParse, which
follows an error before the pstate is allocated, or when pstate
allocation itself fails.
1 month ago
Victor Julien 7cc25599de app-layer: enforce pstate as non-NULL 1 month ago
Victor Julien f4f521272a app-layer: cleanup inspect id getter
Since pstate can't be NULL, remove the conditional logic.
1 month ago
Victor Julien 5a770adc27 app-layer/parser: remove misleading comment
Fixes: 833a738dd1 ("http: fail tx creation if we cannot allocate user data")
1 month ago
Victor Julien 280a1e5a59 app-layer: minor code cleanup for GetStateProgress
It used the alstate name where it meant tx.
1 month ago
Victor Julien 7fe8f63a89 detect/parse: tighten hook parsing
Don't allow trailing :
1 month ago
Victor Julien f8daaac096 detect/file: rename file registration type 1 month ago
Victor Julien 0123cfd9ca detect/file.data: reduce scope for MPM datatype 1 month ago
Victor Julien 5ae427b146 detect/file: minor cleanup
Rename progress vars.
1 month ago
Victor Julien 0e3c946836 detect/file: remove unused registration fields 1 month ago
Victor Julien d81be73ba3 htp: remove duplicate entries in the event table 1 month ago
Philippe Antoine 17dc065326 ci: check uint keywords with their size
So, that if a keyword advertises uint16, it can indeed parse
a uint16 and is not just a uint8
1 month ago
Philippe Antoine 399ee1e360 ci: check multi uint keywords
Ticket: 7929
1 month ago
Philippe Antoine 3b994e807d detect/mqtt: bitflags do not support multi integer
Ticket: 7929

As both syntaxes share comma as delimiter
1 month ago
Philippe Antoine 9bf9d53c42 detect/sctp: sctp.chunk_type does not support multi integer
Ticket: 7929

As it supports prefilter
1 month ago
Philippe Antoine 06855cd832 detect/nfs: nfs_procedure does not support multi integer
Ticket: 7929
1 month ago
Philippe Antoine 4649bb3838 detect/file: filesize does not support multi integer
Ticket: 7929
1 month ago
Philippe Antoine cb0ecb1722 detect/krb5: msg_type is not a multi integer
Ticket: 7929

There is only one msg_type by tx
1 month ago
Philippe Antoine be4c036071 detect/enip: status are not multi integers
Ticket: 7929
1 month ago
Philippe Antoine e08a19a88a detect/enip: cip_class and cip_instance are now a multi-integer
Ticket: 7929

One commit as they share the same code
1 month ago
Philippe Antoine 0bdfe3f571 detect/enip: cip_attribute keyword is now a multi-integer
Ticket: 7929
1 month ago
Jeff Lucovsky 5af13e3eb0 rdp: mark transactions single-directional
Mark the direction into RDP transactions at creation time,
so the tx carries SKIP_INSPECT for the direction it is
never seen in, matching DHCP and the other single-direction
parsers. This lets cleanup free completed transactions and
stops a tx from being inspected (and alerting) twice, once per
direction.

RDP bounds its transactions to connection setup and stops
parsing once bypass_parsing is set.

Issue: 8621
1 month ago
Jeff Lucovsky 3f86c56c80 dhcp: mark transactions single-directional to fix tx leak
DHCP is a stateless parser where each datagram is its own standalone,
single-direction transaction. It was creating transactions with
AppLayerTxData::new(), which leaves both SKIP_INSPECT bits clear, so the
engine treats every tx as still needing inspection in both directions.

For a flow that only ever carries one direction (broadcast DHCP, or a
relay seeing one side), the never-observed direction's inspect bit can
never be set, so AppLayerParserTransactionsCleanup() never frees the tx.
The per-flow transaction Vec then grows without bound and every packet
re-scans the whole list, giving O(n^2) CPU and unbounded memory on a
busy DHCP aggregation point.

Use AppLayerTxData::for_direction() like every other stateless parser
(DNS, SNMP, NTP, IKE, KRB5, MQTT, QUIC, SIP, WebSocket, bittorrent-dht)
so the tx carries SKIP_INSPECT for the direction it will never be seen
in. This lets cleanup free completed transactions and also stops the tx
from being inspected (and alerting) twice, once per direction.

Issue: 8621
1 month ago
Philippe Antoine a6fcbb1ab7 flow: factorize duplicated code for hashing 1 month ago
Philippe Antoine fe728d97d8 flow: use livedev for ftp-hash
Completes commit 7e725c650d

autofp-scheduler with value ftp-hash ends up using
FlowGetIpPairProtoHash which ignores the ports for ftp-looking
flows so that the ftp and ftp-data flow get processed by the
same thread.

As for the other cases, we want to use every other parameter
to compute the flow hash, inclusing the live device
1 month ago