Skip to content

SCTP decoder and sticky buffers v6 - #15534

Closed
glongo wants to merge 9 commits into
OISF:mainfrom
glongo:dev-4251-sctp-v6
Closed

SCTP decoder and sticky buffers v6#15534
glongo wants to merge 9 commits into
OISF:mainfrom
glongo:dev-4251-sctp-v6

Conversation

@glongo

@glongo glongo commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/4251

Describe changes:

  • Added events and descriptions for SCTP_MAX_TRACKED_CHUNKS and SCTP_MAX_DATA_CHUNKS
  • Fixed SCTPVars layout
  • Added DEBUG_VALIDATE_BUG_ON
  • Removed chunk_idx passed in GetBuffer for sctp.chunk_data
  • Removed MIN
  • Replaced (const SCTPChunkHdr *)(pkt + offset); with memcpy

Previous PR: #15497

SV_BRANCH=OISF/suricata-verify#2999

glongo added 9 commits June 3, 2026 15:37
Extend the SCTP decoder to parse chunk headers after the 12-byte common
header. Each chunk is validated for minimum header size and length
consistency per RFC 4960 sec 3.2.

Add SCTPChunkHdr and SCTPVars structs to track per-packet chunk metadata

Add five new decoder events for protocol violations:
- SCTP_CHUNK_TOO_SMALL: insufficient data for a chunk header
- SCTP_CHUNK_LEN_INVALID: chunk length < 4 or exceeds packet
- SCTP_INIT_CHUNK_NOT_ALONE: INIT/INIT_ACK bundled (RFC 4960 sec 6.10)
- SCTP_INIT_WITH_NON_ZERO_VTAG: INIT with vtag != 0 (RFC 4960 sec 8.5.1)
- SCTP_DATA_WITH_ZERO_VTAG: DATA chunk with vtag == 0

Ticket OISF#4251
Implement a sticky buffer to match the raw SCTP header
(common header + chunks)

Ticket OISF#4251
Add a U8 numeric keyword to match the first SCTP chunk type in a packet
with prefilter support.

Ticket OISF#4251
Add a U8 numeric keyword to match the number of SCTP chunks
parsed in a packet with prefilter support.

Ticket OISF#4251
Add a U32 numeric keyword to match the SCTP verification tag
from the common header with prefilter support.

Ticket OISF#4251
Log SCTP-specific fields in the EVE JSON "sctp" object for alert events.

Ticket OISF#4251
Track the first DATA chunk's data offset and length during chunk iteration,
then reassign p->payload to point at the user data.

When no DATA chunk is present (INIT, SACK, HEARTBEAT, etc.),
payload_len is set to 0 since there is no application data.

Ticket OISF#4251
Add a sctp.data sticky multi-buffer that allows content matching on the bytes
inside any of the SCTP DATA chunks extracted.

Ticket OISF#4251
Add documentation for all sctp keywords.

Ticket OISF#4251
@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.09257% with 99 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.85%. Comparing base (26bb18c) to head (16f17fd).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15534      +/-   ##
==========================================
- Coverage   82.85%   82.85%   -0.01%     
==========================================
  Files         999     1006       +7     
  Lines      272609   273370     +761     
==========================================
+ Hits       225881   226508     +627     
- Misses      46728    46862     +134     
Flag Coverage Δ
fuzzcorpus 61.42% <41.33%> (-0.05%) ⬇️
livemode 18.39% <14.48%> (-0.05%) ⬇️
netns 22.77% <14.48%> (-0.02%) ⬇️
pcap 45.15% <36.34%> (-0.05%) ⬇️
suricata-verify 66.55% <72.68%> (-0.02%) ⬇️
unittests 58.51% <65.31%> (+0.03%) ⬆️

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_TLPW2_single_stats_chk
.decoder.event.sctp.chunk_too_small - 0 -
.decoder.event.sctp.chunk_len_invalid - 0 -
.decoder.event.sctp.init_chunk_not_alone - 0 -
.decoder.event.sctp.init_with_non_zero_vtag - 0 -
.decoder.event.sctp.data_with_zero_vtag - 0 -
.decoder.event.sctp.too_many_chunks - 0 -
.decoder.event.sctp.too_many_data_chunks - 0 -
.sctp.init - 0 -
.sctp.init_ack - 0 -
.sctp.data - 0 -
.sctp.abort - 0 -
.sctp.shutdown - 0 -
SURI_TLPW2_autofp_stats_chk
.decoder.event.sctp.chunk_too_small - 0 -
.decoder.event.sctp.chunk_len_invalid - 0 -
.decoder.event.sctp.init_chunk_not_alone - 0 -
.decoder.event.sctp.init_with_non_zero_vtag - 0 -
.decoder.event.sctp.data_with_zero_vtag - 0 -
.decoder.event.sctp.too_many_chunks - 0 -
.decoder.event.sctp.too_many_data_chunks - 0 -
.sctp.init - 0 -
.sctp.init_ack - 0 -
.sctp.data - 0 -
.sctp.abort - 0 -
.sctp.shutdown - 0 -
SURI_TLPW1_stats_chk
.decoder.event.sctp.chunk_too_small - 0 -
.decoder.event.sctp.chunk_len_invalid - 0 -
.decoder.event.sctp.init_chunk_not_alone - 0 -
.decoder.event.sctp.init_with_non_zero_vtag - 0 -
.decoder.event.sctp.data_with_zero_vtag - 0 -
.decoder.event.sctp.too_many_chunks - 0 -
.decoder.event.sctp.too_many_data_chunks - 0 -
.sctp.init - 0 -
.sctp.init_ack - 0 -
.sctp.data - 0 -
.sctp.abort - 0 -
.sctp.shutdown - 0 -
SURI_TLPR1_stats_chk
.decoder.event.sctp.chunk_too_small - 0 -
.decoder.event.sctp.chunk_len_invalid - 0 -
.decoder.event.sctp.init_chunk_not_alone - 0 -
.decoder.event.sctp.init_with_non_zero_vtag - 0 -
.decoder.event.sctp.data_with_zero_vtag - 0 -
.decoder.event.sctp.too_many_chunks - 0 -
.decoder.event.sctp.too_many_data_chunks - 0 -
.sctp.init - 0 -
.sctp.init_ack - 0 -
.sctp.data - 0 -
.sctp.abort - 0 -
.sctp.shutdown - 0 -
IPS_AFP_stats_chk
.decoder.event.sctp.chunk_too_small - 0 -
.decoder.event.sctp.chunk_len_invalid - 0 -
.decoder.event.sctp.init_chunk_not_alone - 0 -
.decoder.event.sctp.init_with_non_zero_vtag - 0 -
.decoder.event.sctp.data_with_zero_vtag - 0 -
.decoder.event.sctp.too_many_chunks - 0 -
.decoder.event.sctp.too_many_data_chunks - 0 -
.sctp.init - 0 -
.sctp.init_ack - 0 -
.sctp.data - 0 -
.sctp.abort - 0 -
.sctp.shutdown - 0 -
TREX_GENERIC_stats_chk
.decoder.event.sctp.chunk_too_small - 0 -
.decoder.event.sctp.chunk_len_invalid - 0 -
.decoder.event.sctp.init_chunk_not_alone - 0 -
.decoder.event.sctp.init_with_non_zero_vtag - 0 -
.decoder.event.sctp.data_with_zero_vtag - 0 -
.decoder.event.sctp.too_many_chunks - 0 -
.decoder.event.sctp.too_many_data_chunks - 0 -
.sctp.init - 0 -
.sctp.init_ack - 0 -
.sctp.data - 0 -
.sctp.abort - 0 -
.sctp.shutdown - 0 -

Pipeline = 31840

Comment thread etc/schema.json
]
}
},
"has_abort": {

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.

should these also reference keyword sctp.chunk_type?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was wondering whether it makes sense to log these boolean fields. Thoughts?

Comment thread etc/schema.json
"type": "integer",
"description": "SCTP DATA chunk with verification tag == 0"
},
"init_chunk_not_alone": {

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.

don't have a suggestion, but this doesn't sound great

Comment thread rules/sctp-events.rules
alert sctp any any -> any any (msg:"SURICATA SCTP INIT with non-zero vtag"; decode-event:sctp.init_with_non_zero_vtag; classtype:protocol-command-decode; sid:2239005; rev:1;)
alert sctp any any -> any any (msg:"SURICATA SCTP DATA with zero vtag"; decode-event:sctp.data_with_zero_vtag; classtype:protocol-command-decode; sid:2239006; rev:1;)
alert sctp any any -> any any (msg:"SURICATA SCTP too many chunks"; decode-event:sctp.too_many_chunks; classtype:protocol-command-decode; sid:2239007; rev:1;)
alert sctp any any -> any any (msg:"SURICATA SCTP too many data chunks"; decode-event:sctp.too_many_data_chunks; classtype:protocol-command-decode; sid:2239008; rev:1;)

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.

nit: we generally have a next sid comment at the bottom of these files

Comment thread rules/sctp-events.rules
# SCTP decoder event rules.
# SID's fall in the 2239000+ range. See rules/README.md

alert sctp any any -> any any (msg:"SURICATA SCTP packet too small"; decode-event:sctp.pkt_too_small; classtype:protocol-command-decode; sid:2239001; rev:1;)

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.

is the packet still marked as sctp in this case? Is there a SV test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the packet is still identified as SCTP. Adding a SV test.

Comment thread src/decode-sctp.c
return -1;
}

if (p->l4.vars.sctp.data_chunk_cnt > 0) {

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.

can you add a comment about what p->payload is after this assignment? Is this also reflected in the docs?

@victorjulien

Copy link
Copy Markdown
Member

Looking at codecov it seems we have tests exercising the prefilter support, so that needs to be added.

@glongo

glongo commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Replaced with #15580

@glongo glongo closed this Jun 8, 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