Skip to content

8.0.x: smtp back ports - v1 - #16043

Open
jasonish wants to merge 11 commits into
OISF:main-8.0.xfrom
jasonish:8.0.x-smtp/v1
Open

8.0.x: smtp back ports - v1#16043
jasonish wants to merge 11 commits into
OISF:main-8.0.xfrom
jasonish:8.0.x-smtp/v1

Conversation

@jasonish

@jasonish jasonish commented Aug 18, 2026

Copy link
Copy Markdown
Member

SMTP backports related to https://redmine.openinfosecfoundation.org/issues/8393.

The impact to existing 8.0.x users is minimal, with more correctness and
accuracy being the main issue.

  • if looking closely, some transactions and alerts may happen on different
    pcap_cnt's due to finer grained progress tracking
  • not transaction just for a quit
  • fewer duplicate helo alerts; new tx created per spec

SV_BRANCH=OISF/suricata-verify#3287

Add minimal SMTP progress states to support envelope validation before
moving to data.

Update SMTP, file and email keywords to hook into the appropriate
states.

Purposefully kept minimal for now as to not break the current idea of an
SMTP transaction, which is probably not ideal for firewall mode.

Ticket: OISF#8393
(cherry picked from commit c2728ee)
Add directionality to completion states, and replace tx->done by checking for
both directions being complete.

This means that the transaction is now not complete until the server responds
to the clients of data marker, previously the tx was completed when the client
send end of data without waiting for the server response.

This keeps smtp:response_complete from being exposed before the server response
is parsed.

Ticket: OISF#8393
(cherry picked from commit 7b31f41)
Track the transaction id for each queued SMTP command so replies can update the
transaction that created the command instead of always using the current
transaction.

Ticket: OISF#8393
(cherry picked from commit e2a62dd)
Also ensures that a quit or rset without a helo still creates a tx.

Ticket: OISF#8728
(cherry picked from commit 842b14e)
Make sure the transaction still exists before completing it in the
to-client direction. A pipelined RSET reply may already have completed and
freed it while a later end-of-DATA marker still refers to it.

Found by OSS-Fuzz testcase 5498180758994944.

Bug OISF#8739.

(cherry picked from commit 62fdb77)
Simply makes the follow diff a little easier to read.

(cherry picked from commit 2676d1b)
Track the BDAT LAST marker so the final chunk and its reply complete the
transaction in each direction, preventing a following MAIL FROM from
being merged into the previous transaction.

Ticket: OISF#8741
(cherry picked from commit ec0fec1)
A BDAT command that failed to parse, such as "BDAT 5 X", returned -1,
disabling SMTP parsing for the rest of the flow. A server may reject the
command and continue the session, leaving following messages
uninspected.

Instead raise a decoder event and queue the command as an ordinary
command.

Ticket: OISF#8741
(cherry picked from commit 60e0df6)
SMTPGetLine assigned response frames to the current transaction even when a
queued reply belonged to an older transaction.

Use the queued command owner so frame EVE output and frame-based detection
receive the correct transaction id.

Includes some re-org to avoid prototypes for static functions.

Ticket: OISF#8741
(cherry picked from commit 57ae571)
RFC 5321 says a mid-session EHLO should work just like RSET. We more or
less ignored it, which meant transaction state could carry over.

Treat a HELO/EHLO received during a transaction as RSET once the server
accepts it.

Ticket: OISF#8715
(cherry picked from commit 0be6e34)
Scan-build reports a possible NULL dereference that is not reachable.

(cherry picked from commit 4481f27)
@jasonish
jasonish requested review from a team and victorjulien as code owners August 18, 2026 23:08
@jasonish

jasonish commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

Not rushing for 8.0.x, just with the last merge to main, it was ready for a complete backport, and it was in my hot cache still.

@jasonish

Copy link
Copy Markdown
Member Author

AI-generated review posted automatically by Suricata ai-review.

Verdict: approve

No high- or medium-severity findings.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.61%. Comparing base (27ff8c8) to head (0cb09e7).

Additional details and impacted files
@@              Coverage Diff               @@
##           main-8.0.x   #16043      +/-   ##
==============================================
- Coverage       81.68%   81.61%   -0.07%     
==============================================
  Files            1016     1016              
  Lines          276981   277124     +143     
==============================================
- Hits           226245   226186      -59     
- Misses          50736    50938     +202     
Flag Coverage Δ
fuzzcorpus 63.91% <89.06%> (+0.01%) ⬆️
livemode 18.63% <16.66%> (-0.05%) ⬇️
netns 20.14% <16.66%> (-0.02%) ⬇️
pcap 44.47% <85.93%> (-0.03%) ⬇️
suricata-verify 65.05% <92.70%> (-0.15%) ⬇️
unittests 58.60% <74.87%> (-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.tx.smtp 613619 386561 63.0%
IPS_AFP_stats_chk
.app_layer.tx.smtp 74520 37800 50.72%
TREX_GENERIC_stats_chk
.app_layer.tx.smtp 29542 14771 50.0%

Pipeline = 33089

@victorjulien victorjulien added this to the 8.0 milestone Aug 19, 2026
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.

3 participants