From e92f3e0c3447627a0b32652718e0d0ae88c29766 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Tue, 9 Jun 2026 10:00:49 -0400 Subject: [PATCH] tests: add DHCP single-direction transaction tests for bug 8621 DHCP is a stateless parser where each datagram is its own standalone, single-direction transaction. Verify that a request/reply exchange alerts once per datagram rather than once per direction, and that directional rules only match the transaction for their own direction. bug-8621-01 checks a generic dhcp rule fires exactly once on the request and once on the reply. bug-8621-02 checks a flow:to_server rule only matches the request and a flow:to_client rule only matches the reply. Issue: 8621 --- tests/bug-8621/README.md | 49 +++++++++++++++++++++++ tests/bug-8621/bug-8621-01/README.md | 11 +++++ tests/bug-8621/bug-8621-01/input.pcap | Bin 0 -> 748 bytes tests/bug-8621/bug-8621-01/suricata.yaml | 11 +++++ tests/bug-8621/bug-8621-01/test.rules | 1 + tests/bug-8621/bug-8621-01/test.yaml | 34 ++++++++++++++++ tests/bug-8621/bug-8621-02/README.md | 11 +++++ tests/bug-8621/bug-8621-02/suricata.yaml | 11 +++++ tests/bug-8621/bug-8621-02/test.rules | 2 + tests/bug-8621/bug-8621-02/test.yaml | 44 ++++++++++++++++++++ 10 files changed, 174 insertions(+) create mode 100644 tests/bug-8621/README.md create mode 100644 tests/bug-8621/bug-8621-01/README.md create mode 100644 tests/bug-8621/bug-8621-01/input.pcap create mode 100644 tests/bug-8621/bug-8621-01/suricata.yaml create mode 100644 tests/bug-8621/bug-8621-01/test.rules create mode 100644 tests/bug-8621/bug-8621-01/test.yaml create mode 100644 tests/bug-8621/bug-8621-02/README.md create mode 100644 tests/bug-8621/bug-8621-02/suricata.yaml create mode 100644 tests/bug-8621/bug-8621-02/test.rules create mode 100644 tests/bug-8621/bug-8621-02/test.yaml diff --git a/tests/bug-8621/README.md b/tests/bug-8621/README.md new file mode 100644 index 000000000..e2a3e9e36 --- /dev/null +++ b/tests/bug-8621/README.md @@ -0,0 +1,49 @@ +# Issue 8621 — single-direction app-layer transactions + +DHCP (and RDP) build each transaction from a message observed in a single +direction, but created them with both `SKIP_INSPECT` bits clear. The engine +then treated every transaction as still needing inspection in the direction it +would never be seen in. Two things followed: a transaction could be inspected — +and alert — twice, once per direction; and on a flow that only ever carries one +direction the never-observed direction's inspect bit could never be set, so +`AppLayerParserTransactionsCleanup` never freed the transaction (unbounded tx +growth and O(n^2) CPU). The fix creates transactions with +`AppLayerTxData::for_direction()` so the unseen direction carries `SKIP_INSPECT`. + +Related tickets: +- https://redmine.openinfosecfoundation.org/issues/8621 +- https://redmine.openinfosecfoundation.org/issues/8658 + +## Test cases covered + +These tests cover the DHCP half of the fix — specifically the per-direction +inspection correctness, which is the part observable in suricata-verify. + +- **bug-8621-01** — a DHCP request/reply exchange and a generic `alert dhcp` + rule must produce one alert per datagram (two total), not one per direction + (four). Guards against a transaction being inspected/alerting in both + directions. + +- **bug-8621-02** — the same request/reply pcap with directional rules + (`flow:to_server` and `flow:to_client`). The to-server rule must match only + the request and the to-client rule only the reply; neither may fire on the + other transaction. Guards that each transaction carries the correct + per-direction `SKIP_INSPECT` bit. + +Both tests use a bidirectional (request + reply) pcap by necessity: the reply +packet is what drives to-client inspection, which is required to expose the +wrong-direction inspection before the fix. A unidirectional pcap would not +surface the bug at the alert level. + +## Not covered here (and why) + +- **RDP is unit-test only.** RDP receives the same `for_direction()` change, but + it has no observable suricata-verify delta: it has no per-transaction + detection keyword, a bare `alert rdp` over TCP matches per packet (not per + transaction), and RDP bounds its transactions to connection setup so it does + not leak. Running identical rules on the RDP-fixed and RDP-unfixed binaries + produces byte-identical alerts, so an s-v test would pass equally before and + after the fix. The RDP change is covered by the Rust unit test + `test_tx_skip_inspect_direction` in `rust/src/rdp/rdp.rs`, which asserts the + per-direction `SKIP_INSPECT` flags directly. The DHCP unit test of the same + name in `rust/src/dhcp/dhcp.rs` does the same for DHCP. diff --git a/tests/bug-8621/bug-8621-01/README.md b/tests/bug-8621/bug-8621-01/README.md new file mode 100644 index 000000000..80499c23b --- /dev/null +++ b/tests/bug-8621/bug-8621-01/README.md @@ -0,0 +1,11 @@ +A DHCP request/reply exchange must produce one alert per datagram, not +one per direction. + +DHCP is a stateless parser where each datagram is a standalone, +single-direction transaction. Before the fix for issue 8621 the +transactions were created without the per-direction SKIP_INSPECT bits, so +each transaction was inspected in both the toserver and toclient +directions and a matching rule alerted twice. + +Related ticket: +https://redmine.openinfosecfoundation.org/issues/8621 diff --git a/tests/bug-8621/bug-8621-01/input.pcap b/tests/bug-8621/bug-8621-01/input.pcap new file mode 100644 index 0000000000000000000000000000000000000000..93617129fa5bf026e7a42a9ed3a01f6293d9230b GIT binary patch literal 748 zc-p&ic+)~A1{MYcU}0bclF@fo#<|LHGlT&-AiThAzK@RvgP@VP=LZf3R|ZB81_lQP z2SMdVE&)asAZBE6VQ^+N*}R&Ok&WSfO1KNiERY-nG63sm7&aieDLC1bk(q}zFR|E= z!JM5@hKZS any any (msg:"DHCP transaction"; sid:1; rev:1;) diff --git a/tests/bug-8621/bug-8621-01/test.yaml b/tests/bug-8621/bug-8621-01/test.yaml new file mode 100644 index 000000000..30334d7f4 --- /dev/null +++ b/tests/bug-8621/bug-8621-01/test.yaml @@ -0,0 +1,34 @@ +requires: + min-version: 9 + +# Issue 8621: each DHCP datagram is its own standalone, single-direction +# transaction. Before the fix the transactions were created with both +# SKIP_INSPECT bits clear, so every tx was inspected -- and could alert -- +# in both directions. A request and a reply must therefore each alert once, +# not twice. +checks: + # Exactly two DHCP alerts for the two datagrams: one per transaction, + # not one per direction. + - filter: + count: 2 + match: + event_type: alert + alert.signature_id: 1 + + # The request (toserver) datagram alerts exactly once. + - filter: + count: 1 + match: + event_type: alert + alert.signature_id: 1 + pcap_cnt: 1 + dhcp.type: request + + # The reply (toclient) datagram alerts exactly once. + - filter: + count: 1 + match: + event_type: alert + alert.signature_id: 1 + pcap_cnt: 2 + dhcp.type: reply diff --git a/tests/bug-8621/bug-8621-02/README.md b/tests/bug-8621/bug-8621-02/README.md new file mode 100644 index 000000000..c57bcdc9f --- /dev/null +++ b/tests/bug-8621/bug-8621-02/README.md @@ -0,0 +1,11 @@ +Directional rules must only match the DHCP transaction for their own +direction. + +Each DHCP datagram is a standalone, single-direction transaction. With the +fix for issue 8621 the transaction carries the SKIP_INSPECT bit for the +direction it will never be seen in, so a toserver rule only matches the +request and a toclient rule only matches the reply. Before the fix both +directional rules could fire on both transactions. + +Related ticket: +https://redmine.openinfosecfoundation.org/issues/8621 diff --git a/tests/bug-8621/bug-8621-02/suricata.yaml b/tests/bug-8621/bug-8621-02/suricata.yaml new file mode 100644 index 000000000..7f2bae708 --- /dev/null +++ b/tests/bug-8621/bug-8621-02/suricata.yaml @@ -0,0 +1,11 @@ +%YAML 1.1 +--- + +outputs: + - eve-log: + enabled: yes + types: + - alert + - dhcp: + enabled: yes + extended: yes diff --git a/tests/bug-8621/bug-8621-02/test.rules b/tests/bug-8621/bug-8621-02/test.rules new file mode 100644 index 000000000..4b7d6a076 --- /dev/null +++ b/tests/bug-8621/bug-8621-02/test.rules @@ -0,0 +1,2 @@ +alert dhcp any any -> any any (msg:"DHCP toserver"; flow:to_server; sid:1; rev:1;) +alert dhcp any any -> any any (msg:"DHCP toclient"; flow:to_client; sid:2; rev:1;) diff --git a/tests/bug-8621/bug-8621-02/test.yaml b/tests/bug-8621/bug-8621-02/test.yaml new file mode 100644 index 000000000..8f7a6a60b --- /dev/null +++ b/tests/bug-8621/bug-8621-02/test.yaml @@ -0,0 +1,44 @@ +requires: + min-version: 9 + +pcap: + ../bug-8621-01/input.pcap + +# Issue 8621: a single-direction DHCP transaction must only be inspected in +# the direction it was seen. The toserver rule may only match the request +# and the toclient rule may only match the reply. Before the fix the +# transactions lacked the per-direction SKIP_INSPECT bits, so the reply tx +# was also inspected toserver (and the request tx toclient), letting the +# directional rules fire on the wrong transaction. +checks: + # The toserver rule matches the request exactly once. + - filter: + count: 1 + match: + event_type: alert + alert.signature_id: 1 + dhcp.type: request + + # The toclient rule matches the reply exactly once. + - filter: + count: 1 + match: + event_type: alert + alert.signature_id: 2 + dhcp.type: reply + + # The toserver rule never fires on the reply transaction. + - filter: + count: 0 + match: + event_type: alert + alert.signature_id: 1 + dhcp.type: reply + + # The toclient rule never fires on the request transaction. + - filter: + count: 0 + match: + event_type: alert + alert.signature_id: 2 + dhcp.type: request -- 2.47.3