Skip to content

next/1410/20260626/v1#15740

Merged
victorjulien merged 17 commits into
OISF:mainfrom
victorjulien:next/1410/20260626/v1
Jun 27, 2026
Merged

next/1410/20260626/v1#15740
victorjulien merged 17 commits into
OISF:mainfrom
victorjulien:next/1410/20260626/v1

Conversation

@victorjulien

Copy link
Copy Markdown
Member

catenacyber and others added 17 commits June 26, 2026 21:50
for SV to run tests based on the presence of this feature
Completes commit 7e725c6

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
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
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
Ticket: 7929

One commit as they share the same code
Ticket: 7929

There is only one msg_type by tx
Ticket: 7929

As both syntaxes share comma as delimiter
So, that if a keyword advertises uint16, it can indeed parse
a uint16 and is not just a uint8
@victorjulien victorjulien requested review from a team, jasonish and jufajardini as code owners June 27, 2026 01:54
@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.71271% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.95%. Comparing base (09f0851) to head (17dc065).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15740      +/-   ##
==========================================
- Coverage   82.96%   82.95%   -0.02%     
==========================================
  Files        1003     1003              
  Lines      275031   275096      +65     
==========================================
+ Hits       228192   228217      +25     
- Misses      46839    46879      +40     
Flag Coverage Δ
fuzzcorpus 61.48% <85.27%> (+0.01%) ⬆️
livemode 18.36% <17.05%> (-0.02%) ⬇️
netns 22.70% <24.80%> (-0.05%) ⬇️
pcap 45.38% <53.48%> (-0.02%) ⬇️
suricata-verify 66.96% <86.82%> (+0.01%) ⬆️
unittests 58.45% <53.59%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa

Copy link
Copy Markdown

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.flow.ftp_data 604 688 113.91%
.app_layer.error.ftp.parser 17 0 -

Pipeline = 32281

@jasonish jasonish left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Staging looks OK.

CI issue doesn't seem related, but the FreeBSD build is reporting:

  pkg: No packages available to install matching 'py311-pyyaml' have been found in the repositories

@victorjulien

victorjulien commented Jun 27, 2026

Copy link
Copy Markdown
Member Author

Staging looks OK.

CI issue doesn't seem related, but the FreeBSD build is reporting:

  pkg: No packages available to install matching 'py311-pyyaml' have been found in the repositories

Yeah, it's a bit odd. My own FreeBSD 15 vm is fine. Just updated it to 15.1 and it is still fine. Strangely, I can't get py312-pyyaml installed there, only py311-pyyaml. It's odd because this suggests py312-pyyaml should exist: https://ports.freebsd.org/cgi/ports.cgi?query=pyyaml&stype=all&sektion=all

@victorjulien victorjulien merged commit 17dc065 into OISF:main Jun 27, 2026
153 of 156 checks passed
@victorjulien victorjulien deleted the next/1410/20260626/v1 branch June 27, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants