dcerpc: don't reuse completed tx #11590
Conversation
In the DCERPC over TCP pcap, logging and rule matching is disrupted by adding a simple rule:
alert tcp any any -> any any (flow:to_server,established; \
dce_iface:5d2b62aa-ee0a-4a95-91ae-b064fdb471fc; dce_opnum:1; \
dce_stub_data; content:"|42 77 4E 6F 64 65 49 50 2E 65 78 65 20|"; \
content:!"|00|"; within:100; distance:97; sid:1; rev:1; )
Works: alert + 3 dcerpc records.
But when adding a trivial rule:
alert tcp any any -> any any (flow:to_server,established; \
dce_iface:5d2b62aa-ee0a-4a95-91ae-b064fdb471fc; dce_opnum:1; \
dce_stub_data; content:"|42 77 4E 6F 64 65 49 50 2E 65 78 65 20|"; \
content:!"|00|"; within:100; distance:97; sid:1; rev:1; )
alert tcp any any -> any any (dsize:3; sid:2; rev:1; )
The alert for sid:1 disappears and also there is one dcerpc event less.
In the single rule case we can aggressively free the transactions, as there
is only an sgh in the toserver direction.
This means that when we encounter the 2nd REQUEST, the first 2 transactions
have already been processed and freed. So for the 2nd REQUEST we open a new
TX and run inspection and logging on it.
When the 2nd rule is added, it adds toclient sgh as well. This means that we
will now slightly delay the freeing of the transactions.
As a consequence we still have the TX for the first REQUEST when the 2nd REQUEST
is parsed. This leads to the 2nd REQUEST re-using the TX. Since the TX is
already marked as inspected, it means the toserver rule now no longer matches.
Also we're not logging this TX correctly now.
This commit fixes the issue by not "finding" a TX that as already been
marked complete in the search direction.
Bug OISF#7187.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11590 +/- ##
==========================================
- Coverage 82.50% 82.50% -0.01%
==========================================
Files 923 923
Lines 248721 248725 +4
==========================================
- Hits 205215 205203 -12
- Misses 43506 43522 +16
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
WARNING:
Pipeline 21804 |
Given the changes, this is an improvement. |
There was a problem hiding this comment.
Thanks for the work.
CI : ✅
Code : good
Commits segmentation : ok
Commit messages : ok, but let me be nitpicky : all the details in the commit message hide the real surprise : there is real dcerpc traffic with call_id reuse (for instance in HTTP2 it is clearly defined as a protocol violation in the RFC)
Git ID set : looks fine for me
CLA : you already contributed
Doc update : not needed
Redmine ticket : ok
Rustfmt : looks good
Tests : good for me
Dependencies added: none
How? 🤔 |
Sorry for the remainder in the copy/paste : I approve this PR |
In the DCERPC over TCP pcap, logging and rule matching is disrupted by adding a simple rule:
Works: alert + 3 dcerpc records.
But when adding a trivial rule:
The alert for sid:1 disappears and also there is one dcerpc event less.
In the single rule case we can aggressively free the transactions, as there is only an sgh in the toserver direction.
This means that when we encounter the 2nd REQUEST, the first 2 transactions have already been processed and freed. So for the 2nd REQUEST we open a new TX and run inspection and logging on it.
When the 2nd rule is added, it adds toclient sgh as well. This means that we will now slightly delay the freeing of the transactions.
As a consequence we still have the TX for the first REQUEST when the 2nd REQUEST is parsed. This leads to the 2nd REQUEST re-using the TX. Since the TX is already marked as inspected, it means the toserver rule now no longer matches. Also we're not logging this TX correctly now.
This commit fixes the issue by not "finding" a TX that as already been marked complete in the search direction.
Bug #7187.
SV_BRANCH=OISF/suricata-verify#1995