Skip to content

smb: trigger raw stream inspection#14649

Draft
inashivb wants to merge 1 commit intoOISF:mainfrom
inashivb:smb-raw-inspection/v1
Draft

smb: trigger raw stream inspection#14649
inashivb wants to merge 1 commit intoOISF:mainfrom
inashivb:smb-raw-inspection/v1

Conversation

@inashivb
Copy link
Member

Previous PR: #14641

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

SV_BRANCH=OISF/suricata-verify#2859

Changes since v0:

  • completed the inspection calls where appropriate

Q: In case of too many txs, see fn new_tx() in smb/smb.rs, should the inspection be triggered once after processing all old txs in both the directions or each time we find a tx that had either or both sides incomplete?

Internals
---------
Suricata's stream engine returns data for inspection to the detection
engine from the stream when the chunk size is reached.

Bug
---
Inspection triggered only in the specified chunk sizes may be too late
when it comes to inspection of smaller protocol specific data which
could result in delayed inspection, incorrect data logged with a transaction
and logs misindicating the pkt that triggered an alert.

Fix
---
Fix this by making an explicit call from all respective applayer parsers to
trigger raw stream inspection which shall make the data available for inspection
in the following call of the stream engine. This needs to happen per direction
on the completion of an entity like a request or a response.

Important notes
---------------
1. The above mentioned behavior with and without this patch is
affected internally by the following conditions.
- inspection depth
- stream depth
In these special cases, the inspection window will be affected and
Suricata may not consider all the data that could be expected to be
inspected.
2. This only applies to applayer protocols running over TCP.
3. The inspection window is only considered up to the ACK'd data.
4. This entire issue is about IDS mode only.

SMB parser creates a transaction per request-response pair. Appropriate calls to trigger
raw stream inspection have been added on succesful parsing of each request and
response.

Task 7863
Bug 7004
@inashivb inashivb marked this pull request as draft January 19, 2026 10:05
@inashivb
Copy link
Member Author

Q: In case of too many txs, see fn new_tx() in smb/smb.rs, should the inspection be triggered once after processing all old txs in both the directions or each time we find a tx that had either or both sides incomplete?

Going with all at once. It'll anyway be processed at once in the next call of stream engine.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline = 29178

@inashivb
Copy link
Member Author

inashivb commented Jan 21, 2026

Failing test bug-5162 seems to have incorrect excessive alerts now.
e.g. for rule with sid:1:
Packets 2941, 2981 -- correct
Packets 2955, 2991 -- incorrect TCP FINACK packets w no matches for the rules but the FINACK packets of the respective streams that did have the matches

@inashivb
Copy link
Member Author

inashivb commented Jan 22, 2026

Failing test bug-5162 seems to have incorrect excessive alerts now. e.g. for rule with sid:1: Packets 2941, 2981 -- correct Packets 2955, 2991 -- incorrect TCP FINACK packets w no matches for the rules but the FINACK packets of the respective streams that did have the matches

bug-5162 and dcerpc-smb-fail were failing due to missing inspection calls in smb/dcerpc parser. I'm not very sure how yet but this tampered with the inspection window such that there were multiple alerts for a rule that should only match once.

streamsize-keyword* tests still fail..

@inashivb
Copy link
Member Author

streamsize-keyword* tests still fail..

This is an issue with how the engine perceives and deals with applayer and stream when they're not in sync. So, that's a different issue with the internals but that's what I'm looking at now.

Keeping this open for now.

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.

2 participants