defrag: regression test for first-fragment RB tree search

[ssam18]

Companion test for OISF/suricata#15403 (Redmine #8232).

What it tests

Adds tests/defrag/bug-8232-defrag-first-frag-out-of-order/.

The pcap, generated by frag-ip-tcp-payload.py with scapy, carries an IPv4 TCP packet with a 40 byte payload, fragmented into eight 8 byte fragments. The fragments are written in reverse order on the wire so the offset-0 fragment lands while the RB tree is already populated. That exercises the neighbour search at src/defrag.c:657 whose key was changed from frag_offset - 1 to frag_offset in the companion PR.

The rule alerts on the reassembled TCP payload via a content match. The test expects exactly one alert.

Honest caveat

The reassembled output is identical with or without the suricata-side fix. Fragment offsets are always 8-byte aligned (IPV4_GET_RAW_FRAGOFFSET << 3 for IPv4, the fragment header offset field for IPv6), so no node ever lives at frag_offset - 1, and RB_NFIND returns the same neighbour for keys frag_offset and frag_offset - 1 on every realistic input. The fix only removes a uint16_t underflow on frag_offset == 0, which used to wrap the key to 65535 and reach the same end state via an unintended fallback.

So this test does not fail on the pre-fix code. It is regression coverage of the affected code path so future changes to the defrag neighbour search keep reassembly correct in this ordering pattern, not a fail-on-old/pass-on-new demonstrator. Verified locally that the test passes both with and without the defrag.c change.

Verification

Ran locally against a Suricata built from the companion PR branch and against the pre-fix code:

===> defrag/bug-8232-defrag-first-frag-out-of-order: OK
PASSED:  1
FAILED:  0
SKIPPED: 0

[victorjulien]

@ssam18 why does it pass as-is?

[ssam18]

@victorjulien @inashivb thanks for the review.

To Victor's "why does it pass as-is?": with the original code the uint16_t key wraps to 65535 when frag_offset is 0. No real fragment offset can be that high (IPv4 13-bit field shifted by 3 caps at 65528, IPv6 follows the same alignment), so RB_NFIND returns NULL either way and the existing RB_MIN fallback walks the tree forward. The reassembled output is the same, so the test as originally written only exercised the affected path without distinguishing the two states.

To inashivb's suggestion to add DEBUG_VALIDATE_BUG_ON(frag_offset == 0): one subtle issue with placing it right before the .offset = frag_offset - 1 line is that after the fix removes the -1, frag_offset == 0 becomes a legitimate input to that block (a first fragment arriving into a non-empty tree, e.g. when fragments arrive out of order). The BUG_ON would then fire on every such case in debug-validation builds and break the fix.

I pushed an update to OISF/suricata#15403 that takes the same idea but restructures the block so the assertion stays valid on the fixed code:

if (!RB_EMPTY(&tracker->fragment_tree)) {
    if (frag_offset == 0) {
        prev = RB_MIN(IP_FRAGMENTS, &tracker->fragment_tree);
        next = IP_FRAGMENTS_RB_NEXT(prev);
    } else {
        DEBUG_VALIDATE_BUG_ON(frag_offset == 0);
        Frag key = { .offset = frag_offset };
        next = RB_NFIND(IP_FRAGMENTS, &tracker->fragment_tree, &key);
        ...
    }
}

The first-fragment branch uses the same RB_MIN initialisation that the no-strict-greater fallback uses, so behaviour is identical to the previous code on every input. The BUG_ON in the else branch is a real invariant now (the else is only reached for non-zero frag_offset).

Verified locally with --enable-debug-validation: on a build that mirrors the pre-fix code (no special case, -1 in the key) plus the BUG_ON guard, this SV test exits -6 (SIGABRT from BUG_ON):

===> bug-8232-defrag-first-frag-out-of-order: FAILED: got exit code -6, expected 0

On the fixed code under the same debug-validation build the test passes. All 35 defrag unit tests still pass on the fixed code under --enable-unittests. PR #15403 body now references SV_REPO=ssam18/suricata-verify and SV_BRANCH=bug-8232-defrag-first-frag-regress so the OISF CI exercises this SV test against the patched Suricata.