Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dcerpc: don't reuse completed tx #11820

Closed

Conversation

victorjulien
Copy link
Member

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 #7187.

(cherry picked from commit 65392c0)

SV_BRANCH=OISF/suricata-verify#2054

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.

(cherry picked from commit 65392c0)
@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.tx.dcerpc_tcp 5949 6289 105.72%

Pipeline 22786

@victorjulien victorjulien added the needs baseline update QA will need a new base line label Sep 24, 2024
@victorjulien
Copy link
Member Author

Merged in #11827, thanks!

@victorjulien victorjulien deleted the backports-for-707/v1 branch January 15, 2025 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs baseline update QA will need a new base line
Development

Successfully merging this pull request may close these issues.

3 participants