From 9f66a134dc95a08ff7ffe903fa1ec46b9c2a3ecd Mon Sep 17 00:00:00 2001 From: Konrad Kollnig <5175206+kasnder@users.noreply.github.com> Date: Sun, 6 Sep 2026 12:53:14 +0200 Subject: [PATCH] Recover DNS detection across split TCP responses --- .github/workflows/test.yml | 8 +- app/src/main/jni/netguard/dns.c | 13 ++- app/src/main/jni/netguard/dns_frame.c | 84 +++++++++++++-- app/src/main/jni/netguard/dns_frame.h | 46 ++++---- app/src/main/jni/netguard/netguard.h | 2 + app/src/main/jni/netguard/tcp.c | 11 +- .../test/native/dns_frame_allocation_test.c | 59 ++++++++++ app/src/test/native/dns_frame_record_test.c | 101 +++++++++++++++++ app/src/test/native/dns_frame_test.c | 102 +++++++++++++++++- app/src/test/native/run_defensive_tests.sh | 2 +- app/src/test/native/run_dns_frame_tests.sh | 23 ++++ app/src/test/native/tcp_defensive_test.c | 10 +- 12 files changed, 411 insertions(+), 50 deletions(-) create mode 100644 app/src/test/native/dns_frame_allocation_test.c create mode 100644 app/src/test/native/dns_frame_record_test.c create mode 100755 app/src/test/native/run_dns_frame_tests.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5e9b5428d..a82aa4195 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -72,12 +72,8 @@ jobs: # the C ABI the NetGuard engine calls (wgbridge-rs/tests/). cargo test --manifest-path wgbridge-rs/Cargo.toml --workspace --locked --offline - - name: Run DNS-over-TCP framing host tests - run: | - cc -Wall -Wextra -Werror -Iapp/src/main/jni/netguard \ - -o /tmp/dns_frame_test \ - app/src/test/native/dns_frame_test.c app/src/main/jni/netguard/dns_frame.c - /tmp/dns_frame_test + - name: Run DNS-over-TCP framing and detection regression tests + run: bash app/src/test/native/run_dns_frame_tests.sh - name: Run IPv6 extension header walk host tests run: | diff --git a/app/src/main/jni/netguard/dns.c b/app/src/main/jni/netguard/dns.c index b21c47b0a..d552e6303 100644 --- a/app/src/main/jni/netguard/dns.c +++ b/app/src/main/jni/netguard/dns.c @@ -22,6 +22,7 @@ struct tcdns_ctx { const struct arguments *args; const struct ng_session *s; + int detection_only; }; static void tcdns_record_answer(void *opaque, const char *qname, const char *aname, @@ -32,7 +33,7 @@ static void tcdns_record_answer(void *opaque, const char *qname, const char *ana static int tcdns_is_domain_blocked(void *opaque, const char *qname) { const struct tcdns_ctx *ctx = (const struct tcdns_ctx *) opaque; - return is_domain_blocked(ctx->args, qname) != 0; + return !ctx->detection_only && is_domain_blocked(ctx->args, qname) != 0; } static uint8_t tcdns_blocked_rcode(void *opaque) { @@ -43,6 +44,8 @@ static uint8_t tcdns_blocked_rcode(void *opaque) { static void tcdns_on_blanked(void *opaque, const char *qname, uint16_t qtype, uint8_t rcode) { const struct tcdns_ctx *ctx = (const struct tcdns_ctx *) opaque; + if (ctx->detection_only) + return; const struct arguments *args = ctx->args; const struct ng_session *s = ctx->s; @@ -115,3 +118,11 @@ void parse_dns_partial_response(const struct arguments *args, const struct ng_se if (result != TCDNS_UNCHANGED) *blanked = 1; } + +// This is a completed private copy: recording is useful, but policy/logging +// cannot claim to have rewritten bytes already forwarded to the client. +void record_dns_response(const struct arguments *args, uint8_t *data, size_t datalen) { + struct tcdns_ctx ctx = { .args = args, .s = NULL, .detection_only = 1 }; + tcdns_callbacks cb = TCDNS_CALLBACKS_INIT; + (void) tcdns_process_response(data, datalen, &cb, &ctx); +} diff --git a/app/src/main/jni/netguard/dns_frame.c b/app/src/main/jni/netguard/dns_frame.c index e5ae2f7f7..b581644df 100644 --- a/app/src/main/jni/netguard/dns_frame.c +++ b/app/src/main/jni/netguard/dns_frame.c @@ -17,10 +17,68 @@ Copyright 2015-2019 by Marcel Bokhorst (M66B) */ +#include #include #include "dns_frame.h" +static void clear_frame_buffer(struct dns_stream_state *state) { + free(state->frame_buffer); + state->frame_buffer = NULL; + state->frame_length = 0; + state->frame_received = 0; +} + +void dns_frame_reset(struct dns_stream_state *state) { + if (state == NULL) + return; + + free(state->frame_buffer); + memset(state, 0, sizeof(*state)); +} + +static void start_frame_buffer(struct dns_stream_state *state, size_t frame_len, + const uint8_t *data, size_t bytes) { + clear_frame_buffer(state); + if (frame_len == 0) + return; + + state->frame_buffer = malloc(frame_len); + if (state->frame_buffer == NULL) + return; + + state->frame_length = (uint32_t) frame_len; + if (bytes > frame_len) + bytes = frame_len; + if (bytes > 0) + memcpy(state->frame_buffer, data, bytes); + state->frame_received = (uint32_t) bytes; +} + +static void append_frame_buffer(struct dns_stream_state *state, + const uint8_t *data, size_t bytes) { + if (state->frame_buffer == NULL || bytes == 0) + return; + + size_t available = state->frame_length - state->frame_received; + if (bytes > available) + bytes = available; + memcpy(state->frame_buffer + state->frame_received, data, bytes); + state->frame_received += (uint32_t) bytes; +} + +static void parse_completed_buffer(struct dns_stream_state *state, + dns_frame_parse_fn parse, void *ctx) { + if (state->frame_buffer != NULL && + state->frame_received == state->frame_length) { + int blank_rest = 0; + (void) parse(ctx, state->frame_buffer, state->frame_length, + DNS_FRAME_REPLAY, + &blank_rest); + } + clear_frame_buffer(state); +} + size_t dns_frame_process_stream(uint8_t *buffer, size_t bytes, struct dns_stream_state *state, dns_frame_parse_fn parse, void *ctx) { @@ -32,18 +90,21 @@ size_t dns_frame_process_stream(uint8_t *buffer, size_t bytes, size_t end = bytes; // bytes to forward; only ever shrinks, never below cursor // 1. Continuation of a frame whose earlier bytes were already forwarded in - // a previous recv(). Those bytes are neither parsed (their DNS header is - // gone) nor rewritten (their prefix is already on the wire). + // a previous recv(). Preserve the original bytes for a complete parse + // before applying any carried blanking decision to the forwarded copy. if (state->frame_remaining > 0) { size_t remaining = end - cursor; size_t skip = (state->frame_remaining < remaining ? (size_t) state->frame_remaining : remaining); + append_frame_buffer(state, buffer + cursor, skip); if (state->blank_remaining != 0 && skip > 0) memset(buffer + cursor, 0, skip); state->frame_remaining -= (uint32_t) skip; cursor += skip; - if (state->frame_remaining == 0) + if (state->frame_remaining == 0) { state->blank_remaining = 0; + parse_completed_buffer(state, parse, ctx); + } if (cursor >= end) return end; } @@ -61,15 +122,18 @@ size_t dns_frame_process_stream(uint8_t *buffer, size_t bytes, size_t avail = end - cursor; if (frame_len > avail) { int blank_rest = 0; + start_frame_buffer(state, frame_len, buffer + cursor, avail); if (avail > 0) - (void) parse(ctx, buffer + cursor, avail, 1, &blank_rest); + (void) parse(ctx, buffer + cursor, avail, DNS_FRAME_PARTIAL, + &blank_rest); state->frame_remaining = (uint32_t) (frame_len - avail); state->blank_remaining = (uint8_t) (blank_rest != 0); return end; } if (frame_len > 0) { int blank_rest = 0; - (void) parse(ctx, buffer + cursor, frame_len, 1, &blank_rest); // shrink ignored + (void) parse(ctx, buffer + cursor, frame_len, DNS_FRAME_PARTIAL, + &blank_rest); // shrink ignored cursor += frame_len; } } @@ -98,10 +162,13 @@ size_t dns_frame_process_stream(uint8_t *buffer, size_t bytes, if (frame_len > avail) { // Frame runs past this read: parse what is visible (blanking only, - // any shrink is ignored) and remember the overflow. + // any shrink is ignored), retain its original bytes for detection + // when complete, and remember the overflow. int blank_rest = 0; + start_frame_buffer(state, frame_len, buffer + cursor, avail); if (avail > 0) - (void) parse(ctx, buffer + cursor, avail, 1, &blank_rest); + (void) parse(ctx, buffer + cursor, avail, DNS_FRAME_PARTIAL, + &blank_rest); state->frame_remaining = (uint32_t) (frame_len - avail); state->blank_remaining = (uint8_t) (blank_rest != 0); return end; @@ -110,7 +177,8 @@ size_t dns_frame_process_stream(uint8_t *buffer, size_t bytes, // Complete frame, prefix and payload both inside this buffer: nothing // here has been forwarded yet, so it may be shortened. int blank_rest = 0; - size_t new_dlen = parse(ctx, buffer + cursor, frame_len, 0, &blank_rest); + size_t new_dlen = parse(ctx, buffer + cursor, frame_len, + DNS_FRAME_COMPLETE, &blank_rest); if (new_dlen > frame_len) new_dlen = frame_len; // defensive: a parser must never grow a frame diff --git a/app/src/main/jni/netguard/dns_frame.h b/app/src/main/jni/netguard/dns_frame.h index d0abe2ebf..ceb19a659 100644 --- a/app/src/main/jni/netguard/dns_frame.h +++ b/app/src/main/jni/netguard/dns_frame.h @@ -21,7 +21,7 @@ #define DNS_FRAME_H /* - * Bufferless cursor over a DNS-over-TCP (port 53) byte stream, extracted out + * Cursor over a DNS-over-TCP (port 53) byte stream, extracted out * of check_tcp_socket() (tcp.c) so it can be unit-tested on the host without * pulling in JNI/session dependencies. This header and its implementation * (dns_frame.c) must only depend on libc: no netguard.h, no JNI. @@ -35,19 +35,11 @@ * - a lone byte that is the first half of a length prefix. * * dns_frame_process_stream() walks every frame boundary inside one recv() - * buffer and carries enough state to stay aligned into the next call without - * buffering stream bytes. A frame is offered to the DNS parser only in the - * recv() where its prefix is completed. If its payload continues into later - * reads, only the bytes visible in that first call are parsed; the parser is - * told that the frame is partial, and a blocking result causes the visible - * answer tail and all later continuation bytes to be zeroed in place. Policy - * enforcement for such split frames therefore remains best-effort: once the - * answer section is cut short, per-answer detection stops at the truncation - * point and SVCB-triggered blanking is unavailable, leaving the domain of the - * question as the only signal the block decision can use. That signal needs the - * header and the whole question inside the visible bytes, so a read split - * within the first few bytes of a response blocks nothing at all and the frame - * passes through intact. + * buffer and carries alignment into the next call. A split payload is also + * copied into a bounded buffer (the DNS-over-TCP length field limits it to + * 65535 bytes). The visible first part is parsed immediately for enforcement; + * once the payload is complete, the copy is parsed again so answers that were + * beyond the first recv() still reach tracker detection. * * A blanked split frame keeps its original 2-byte length prefix, so what * reaches the client is a DNS message with all three counts cleared followed by @@ -90,19 +82,35 @@ struct dns_stream_state { uint8_t prefix_hi; /* stashed first byte of a length prefix split across recv()s */ uint8_t have_prefix_hi; /* nonzero when prefix_hi is valid */ + uint8_t *frame_buffer; /* original bytes of a split frame */ + uint32_t frame_length; + uint32_t frame_received; +}; + +/* Releases any split-frame buffer and restores the all-zero initial state. */ +void dns_frame_reset(struct dns_stream_state *state); + +enum dns_frame_parse_mode { + DNS_FRAME_COMPLETE = 0, + DNS_FRAME_PARTIAL = 1, + DNS_FRAME_REPLAY = 2, }; /* * Called for each DNS payload (or the visible part of one) found in the - * buffer; stands in for parse_dns_response(). partial != 0 means the frame is - * not wholly inside this buffer, so the callback must not shrink it. On a - * partial call, *blank_rest is set nonzero when the callback blanked the - * visible part and the caller must blank the frame's later continuation bytes. + * buffer; stands in for parse_dns_response(). mode is DNS_FRAME_COMPLETE for + * a complete frame, DNS_FRAME_PARTIAL for the first visible part of a split + * frame, and DNS_FRAME_REPLAY when replaying the completed buffered copy of a + * split frame for detection. Partial and replay calls must not shrink the + * forwarded stream. On a partial call, *blank_rest is set nonzero when the + * callback blanked the visible part and the caller must blank the frame's later + * continuation bytes. * Returns the possibly-shrunk payload length for a complete frame; a return * > dlen must be treated by the caller as "unchanged" (defensive clamp). */ typedef size_t (*dns_frame_parse_fn)(void *ctx, uint8_t *data, size_t dlen, - int partial, int *blank_rest); + enum dns_frame_parse_mode mode, + int *blank_rest); /* * Processes one recv() buffer of a DNS-over-TCP stream in place. diff --git a/app/src/main/jni/netguard/netguard.h b/app/src/main/jni/netguard/netguard.h index 4d33152a4..eec7563e0 100644 --- a/app/src/main/jni/netguard/netguard.h +++ b/app/src/main/jni/netguard/netguard.h @@ -414,6 +414,8 @@ void check_udp_socket(const struct arguments *args, const struct epoll_event *ev void parse_dns_response(const struct arguments *args, const struct ng_session *session, uint8_t *data, size_t *datalen); +void record_dns_response(const struct arguments *args, uint8_t *data, size_t datalen); + void parse_dns_partial_response(const struct arguments *args, const struct ng_session *session, uint8_t *data, size_t *datalen, int *blanked); diff --git a/app/src/main/jni/netguard/tcp.c b/app/src/main/jni/netguard/tcp.c index 03f487ced..f066901ac 100644 --- a/app/src/main/jni/netguard/tcp.c +++ b/app/src/main/jni/netguard/tcp.c @@ -28,6 +28,7 @@ extern char socks5_password[127 + 1]; extern FILE *pcap_file; void clear_tcp_data(struct tcp_session *cur) { + dns_frame_reset(&cur->dns_stream); struct segment *s = cur->forward; while (s != NULL) { struct segment *p = s; @@ -103,6 +104,7 @@ int check_tcp_session(const struct arguments *args, struct ng_session *s, s->tcp.time = time(NULL); s->tcp.state = TCP_CLOSE; + dns_frame_reset(&s->tcp.dns_stream); } if ((s->tcp.state == TCP_CLOSING || s->tcp.state == TCP_CLOSE) && @@ -249,10 +251,15 @@ struct dns_stream_parse_ctx { // Adapter matching dns_frame_parse_fn: complete frames use // parse_dns_response() and may shrink; partial frames use the in-place path // and report whether their later continuation bytes must be blanked. +// Replay parses only a private copy; its result cannot rewrite forwarded bytes. static size_t tcp_dns_parse_frame(void *ctx, uint8_t *data, size_t dlen, - int partial, int *blank_rest) { + enum dns_frame_parse_mode mode, int *blank_rest) { struct dns_stream_parse_ctx *pctx = (struct dns_stream_parse_ctx *) ctx; - if (partial != 0) { + if (mode == DNS_FRAME_REPLAY) { + record_dns_response(pctx->args, data, dlen); + return dlen; + } + if (mode == DNS_FRAME_PARTIAL) { int blanked = 0; parse_dns_partial_response(pctx->args, pctx->s, data, &dlen, &blanked); *blank_rest = blanked; diff --git a/app/src/test/native/dns_frame_allocation_test.c b/app/src/test/native/dns_frame_allocation_test.c new file mode 100644 index 000000000..7e1b622a3 --- /dev/null +++ b/app/src/test/native/dns_frame_allocation_test.c @@ -0,0 +1,59 @@ +/* Deterministic allocation failure and lifetime tests for the stream buffer. */ +#include +#include +#include "dns_frame.h" + +static int fail_allocation; +static size_t live_allocations, last_allocation, replays; +static void *frame_alloc(size_t size) { + last_allocation = size; + if (fail_allocation) return NULL; + void *p = malloc(size); + assert(p != NULL); + live_allocations++; + return p; +} +static void frame_free(void *p) { + if (p != NULL) { assert(live_allocations > 0); live_allocations--; } + free(p); +} +#define malloc frame_alloc +#define free frame_free +#include "dns_frame.c" +#undef malloc +#undef free + +static size_t parse(void *ctx, uint8_t *data, size_t len, + enum dns_frame_parse_mode mode, int *blank_rest) { + (void)ctx; (void)data; + *blank_rest = 0; + if (mode == DNS_FRAME_REPLAY) replays++; + return len; +} +int main(void) { + struct dns_stream_state state = {0}; + uint8_t first[] = {0, 5, 1}, rest[] = {2, 3, 4, 5}; + assert(dns_frame_process_stream(first, sizeof(first), &state, parse, NULL) == sizeof(first)); + assert(live_allocations == 1 && last_allocation == 5); + assert(dns_frame_process_stream(rest, sizeof(rest), &state, parse, NULL) == sizeof(rest)); + assert(replays == 1 && live_allocations == 0 && state.frame_buffer == NULL); + dns_frame_reset(&state); + + /* A connection closes before the announced maximum frame arrives. */ + uint8_t maximum[] = {255, 255}; + dns_frame_process_stream(maximum, sizeof(maximum), &state, parse, NULL); + assert(last_allocation == 65535 && live_allocations == 1); + dns_frame_reset(&state); + dns_frame_reset(&state); + assert(live_allocations == 0 && state.frame_remaining == 0); + + /* OOM must preserve forwarding/alignment and the previous partial parser. */ + fail_allocation = 1; + assert(dns_frame_process_stream(first, sizeof(first), &state, parse, NULL) == sizeof(first)); + assert(state.frame_buffer == NULL && state.frame_remaining == 4); + assert(dns_frame_process_stream(rest, sizeof(rest), &state, parse, NULL) == sizeof(rest)); + assert(replays == 1 && state.frame_remaining == 0 && live_allocations == 0); + assert(first[2] == 1 && rest[0] == 2 && rest[3] == 5); + dns_frame_reset(&state); + return 0; +} diff --git a/app/src/test/native/dns_frame_record_test.c b/app/src/test/native/dns_frame_record_test.c new file mode 100644 index 000000000..fdc7af7e8 --- /dev/null +++ b/app/src/test/native/dns_frame_record_test.c @@ -0,0 +1,101 @@ +/* Real framing + real tc-dns C ABI, with only the Java record sink replaced. */ +#include +#include +#include +#include +#include "dns_frame.h" +#include "netguard.h" + +static unsigned records, policy_calls, log_calls; +static int force_block; +static struct arguments args = { .rcode = 3 }; +static struct ng_session session = { .protocol = IPPROTO_UDP, .udp = { .version = 4 } }; +void dns_resolved(const struct arguments *unused, const char *q, const char *a, + const char *ip, int ttl) { + (void)unused; + assert(strcmp(q, "doubleclick.net") == 0); + assert(strcmp(a, "doubleclick.net") == 0); + assert(strcmp(ip, "203.0.113.5") == 0); + assert(ttl == 300); + records++; +} +jboolean is_domain_blocked(const struct arguments *unused, const char *q) { + (void)unused; (void)q; policy_calls++; return force_block; +} +jobject create_packet(const struct arguments *unused, jint version, jint protocol, + const char *flags, const char *source, jint sport, + const char *dest, jint dport, const char *data, jint uid, + jboolean allowed) { + (void)unused; (void)version; (void)protocol; (void)flags; (void)source; + (void)sport; (void)dest; (void)dport; (void)data; (void)uid; (void)allowed; + return NULL; +} +void log_packet(const struct arguments *unused, jobject packet) { + (void)unused; (void)packet; log_calls++; +} +static size_t parse(void *ctx, uint8_t *data, size_t len, + enum dns_frame_parse_mode mode, int *blank_rest) { + (void)ctx; + *blank_rest = 0; + if (mode == DNS_FRAME_REPLAY) record_dns_response(&args, data, len); + else if (mode == DNS_FRAME_PARTIAL) + parse_dns_partial_response(&args, &session, data, &len, blank_rest); + else parse_dns_response(&args, &session, data, &len); + return len; +} +static const uint8_t frame[] = { + 0,49, /* TCP length */ + 0x12,0x34,0x81,0x80,0,1,0,1,0,0,0,0, + 11,'d','o','u','b','l','e','c','l','i','c','k',3,'n','e','t',0, + 0,1,0,1, + 0xc0,0x0c,0,1,0,1,0,0,1,0x2c,0,4,203,0,113,5 +}; +static unsigned run(size_t split, int bytewise) { + uint8_t buffer[sizeof(frame)]; + memcpy(buffer, frame, sizeof(buffer)); + struct dns_stream_state state = {0}; + records = 0; + if (bytewise) { + for (size_t i=0; ibase = base; } -static size_t stub_parse(void *ctx, uint8_t *data, size_t dlen, int partial, +static size_t stub_parse(void *ctx, uint8_t *data, size_t dlen, + enum dns_frame_parse_mode partial, int *blank_rest) { struct parse_recorder *r = (struct parse_recorder *) ctx; *blank_rest = 0; + if (partial == DNS_FRAME_REPLAY && !r->record_replays) + return dlen; if (r->calls < MAX_CALLS) { r->off[r->calls] = (size_t) (data - r->base); r->len[r->calls] = dlen; @@ -108,7 +114,8 @@ static size_t stub_parse(void *ctx, uint8_t *data, size_t dlen, int partial, * the configured marker. This models a rule-based parser outcome without * depending on callback invocation order. */ static size_t stub_parse_by_marker(void *ctx, uint8_t *data, size_t dlen, - int partial, int *blank_rest) { + enum dns_frame_parse_mode partial, + int *blank_rest) { struct parse_recorder *r = (struct parse_recorder *) ctx; (void) stub_parse(ctx, data, dlen, partial, blank_rest); if (r->shrink_on_marker && dlen > 0 && data[0] == r->shrink_marker) @@ -118,7 +125,8 @@ static size_t stub_parse_by_marker(void *ctx, uint8_t *data, size_t dlen, static int state_is_clean(const struct dns_stream_state *state) { return state->frame_remaining == 0 && state->blank_remaining == 0 && - state->have_prefix_hi == 0; + state->have_prefix_hi == 0 && state->frame_buffer == NULL && + state->frame_length == 0 && state->frame_received == 0; } static size_t append_frame(uint8_t *buffer, size_t offset, size_t frame_len, @@ -189,6 +197,7 @@ static void run_coalesced_shrink_case(const char *label, label); expected_offset += 2 + new_lens[i]; } + dns_frame_reset(&state); } /* A four-frame coalesced read verifies every callback pointer against the @@ -238,6 +247,7 @@ static void test_coalesced_multiple_frames(void) { static const size_t new_lens[] = {1, 2, 3, 4}; run_coalesced_shrink_case("coalesced shrink: several frames", new_lens); } + dns_frame_reset(&state); } /* 1. A single complete frame in the read, shortened by the parser: the @@ -264,6 +274,7 @@ static void test_single_frame_shortened(void) { CHECK(out == 22, "single frame shortened: forwards 22 bytes"); CHECK(read_prefix(buffer) == 20, "single frame shortened: prefix rewritten to 20"); CHECK(state_is_clean(&state), "single frame shortened: no carry-over state"); + dns_frame_reset(&state); } /* 2. A single complete frame the parser leaves alone: forwarded verbatim. @@ -301,6 +312,8 @@ static void test_single_frame_unchanged(void) { CHECK(out2 == sizeof(buffer), "grow clamped: bytes untouched"); CHECK(read_prefix(buffer) == frame_len, "grow clamped: prefix untouched"); CHECK(state_is_clean(&state2), "grow clamped: no carry-over state"); + dns_frame_reset(&state); + dns_frame_reset(&state2); } /* 3. Coalesced read: two complete frames in one recv(). Both are parsed @@ -334,6 +347,7 @@ static void test_coalesced_read(void) { CHECK(out == sizeof(copy), "coalesced read: bytes untouched"); CHECK(memcmp(copy, buffer, sizeof(buffer)) == 0, "coalesced read: buffer untouched"); CHECK(state_is_clean(&state), "coalesced read: no carry-over state"); + dns_frame_reset(&state); } /* Then: the first frame is shortened to 10 -- the second frame slides @@ -359,6 +373,7 @@ static void test_coalesced_read(void) { CHECK(copy[14] == 0xDD && copy[14 + second - 1] == 0xDD, "coalesced shrink: second payload moved down intact"); CHECK(state_is_clean(&state), "coalesced shrink: no carry-over state"); + dns_frame_reset(&state); } } @@ -394,6 +409,7 @@ static void test_shrink_to_zero_alignment(void) { CHECK(memcmp(buffer + out, original + out, bytes - out) == 0, "shrink to zero: bytes after output unchanged"); CHECK(state_is_clean(&state), "shrink to zero: clean state"); + dns_frame_reset(&state); } /* A payload can span three reads: the visible fragment is parsed once, the @@ -463,6 +479,7 @@ static void test_payload_spanning_three_reads(void) { "three-read payload: final continuation untouched"); CHECK(state_is_clean(&state), "three-read payload: clean state"); } + dns_frame_reset(&state); } /* A blocked partial frame is blanked in the first read and its continuation @@ -501,6 +518,7 @@ static void test_blocked_split_frames(void) { "blocked two-read: continuation blanked"); CHECK(state.frame_remaining == 0 && state.blank_remaining == 0, "blocked two-read: blank continuation cleared at frame end"); + dns_frame_reset(&state); } { @@ -548,6 +566,7 @@ static void test_blocked_split_frames(void) { "blocked three-read: next frame parsed after exact drain"); CHECK(state_is_clean(&state), "blocked three-read: blank continuation cleared at frame end"); + dns_frame_reset(&state); } } @@ -620,6 +639,7 @@ static void test_split_prefix_payload_handoff(void) { CHECK(out == sizeof(buffer), "split-prefix handoff: final count unchanged"); CHECK(state_is_clean(&state), "split-prefix handoff: clean state"); } + dns_frame_reset(&state); } /* Legal zero-length frames between real frames must forward their prefixes @@ -647,6 +667,7 @@ static void test_zero_frames_between_real_frames(void) { "zero frames between: real-frame offsets and lengths"); CHECK(out == sizeof(buffer), "zero frames between: all bytes forwarded"); CHECK(state_is_clean(&state), "zero frames between: clean state"); + dns_frame_reset(&state); } /* 4. Split frame: recv() got fewer bytes than the prefix declares. The @@ -695,6 +716,7 @@ static void test_split_frame(void) { "split continuation: exactly 50 carry-over bytes skipped"); CHECK(out2 == sizeof(next), "split continuation: all bytes forwarded"); CHECK(state_is_clean(&state), "split continuation: state drained"); + dns_frame_reset(&state); } /* 5. frame_len == 0 is a legal no-op frame: its two bytes forward as-is and @@ -719,6 +741,7 @@ static void test_zero_frame_len(void) { CHECK(read_prefix(buffer) == 0, "zero frame_len: no-op frame forwarded as-is"); CHECK(read_prefix(buffer + 2) == 3, "zero frame_len: following prefix rewritten"); CHECK(state_is_clean(&state), "zero frame_len: no carry-over state"); + dns_frame_reset(&state); } /* 6. Minimal edges: a 3-byte read holding one payload byte, complete or @@ -742,6 +765,7 @@ static void test_minimal_reads(void) { CHECK(out == 2, "bytes==3 complete: shrunk to a bare prefix"); CHECK(read_prefix(buffer) == 0, "bytes==3 complete: prefix rewritten to 0"); CHECK(state_is_clean(&state), "bytes==3 complete: no carry-over state"); + dns_frame_reset(&state); } /* Split: the prefix declares more than the single available byte. */ @@ -760,6 +784,7 @@ static void test_minimal_reads(void) { "bytes==3 split: parse capped to the single available byte"); CHECK(out == sizeof(buffer), "bytes==3 split: bytes untouched"); CHECK(state.frame_remaining == 4, "bytes==3 split: 4 payload bytes still owed"); + dns_frame_reset(&state); } /* A lone byte: the high half of a length prefix. It cannot be withheld, @@ -778,6 +803,7 @@ static void test_minimal_reads(void) { CHECK(state.have_prefix_hi != 0 && state.prefix_hi == 0x01, "lone prefix byte: stashed for the next read"); CHECK(state.frame_remaining == 0, "lone prefix byte: no payload owed"); + dns_frame_reset(&state); } /* Exactly two bytes can hold a nonzero prefix but no payload. The parser @@ -799,6 +825,7 @@ static void test_minimal_reads(void) { "bytes==2 no payload: full frame length owed"); CHECK(memcmp(buffer, snapshot, sizeof(buffer)) == 0, "bytes==2 no payload: buffer untouched"); + dns_frame_reset(&state); } /* A one-byte continuation is similarly just consumed from the owed @@ -808,7 +835,7 @@ static void test_minimal_reads(void) { uint8_t snapshot[sizeof(buffer)]; memcpy(snapshot, buffer, sizeof(buffer)); - struct dns_stream_state state = {5, 0, 0, 0}; + struct dns_stream_state state = {.frame_remaining = 5}; struct parse_recorder r; recorder_init(&r, buffer); size_t out = dns_frame_process_stream(buffer, sizeof(buffer), &state, @@ -818,6 +845,7 @@ static void test_minimal_reads(void) { "bytes==1 continuation: one owed byte consumed"); CHECK(memcmp(buffer, snapshot, sizeof(buffer)) == 0, "bytes==1 continuation: buffer untouched"); + dns_frame_reset(&state); } } @@ -850,6 +878,7 @@ static void test_frame_len_u16_boundary(void) { CHECK(read_prefix(buffer) == 65435, "u16 boundary complete: prefix rewritten"); CHECK(state_is_clean(&state), "u16 boundary complete: no carry-over state"); free(buffer); + dns_frame_reset(&state); } } @@ -875,6 +904,7 @@ static void test_frame_len_u16_boundary(void) { CHECK(memcmp(buffer, snapshot, sizeof(buffer)) == 0, "u16 boundary split: buffer untouched"); CHECK(state.frame_remaining == frame_len - avail, "u16 boundary split: overflow remembered"); + dns_frame_reset(&state); } } @@ -985,6 +1015,7 @@ static void test_multi_call_no_desync(void) { CHECK(read_prefix(buffer) == 4, "read 4: prefix rewritten"); CHECK(state_is_clean(&state), "read 4: no carry-over state"); } + dns_frame_reset(&state); } #define CHUNKING_STREAM_BYTES 30 @@ -1096,6 +1127,7 @@ static void run_chunk_partition(const uint8_t *stream, size_t stream_bytes, CHECK(chunk_start == stream_bytes, "chunking property: all reference bytes consumed"); CHECK(state_is_clean(&state), "chunking property: clean final state"); + dns_frame_reset(&state); } /* Every two-way and three-way partition exercises prefix splits, payload @@ -1120,6 +1152,65 @@ static void test_exhaustive_chunking(void) { } } +struct replay_recorder { + uint8_t expected[6]; + size_t calls; + size_t first_len; + size_t replay_len; + int replay_matches; +}; + +static size_t record_split_replay(void *ctx, uint8_t *data, size_t dlen, + enum dns_frame_parse_mode mode, + int *blank_rest) { + struct replay_recorder *r = (struct replay_recorder *) ctx; + *blank_rest = 0; + r->calls++; + if (mode == DNS_FRAME_PARTIAL) + r->first_len = dlen; + else if (mode == DNS_FRAME_REPLAY) { + r->replay_len = dlen; + r->replay_matches = + dlen == sizeof(r->expected) && + memcmp(data, r->expected, sizeof(r->expected)) == 0; + } + return dlen; +} + +static void test_split_frame_replayed_when_complete(void) { + struct dns_stream_state state = {0}; + struct replay_recorder r = { + .expected = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15}, + }; + uint8_t first[] = {0x00, 0x06, 0x10, 0x11}; + uint8_t second[] = {0x12, 0x13, 0x14, 0x15}; + + size_t first_out = dns_frame_process_stream( + first, sizeof(first), &state, record_split_replay, &r); + CHECK(first_out == sizeof(first), "split replay: first read unchanged"); + CHECK(r.calls == 1 && r.first_len == 2, + "split replay: visible prefix parsed immediately"); + CHECK(state.frame_buffer != NULL && state.frame_length == 6 && + state.frame_received == 2, + "split replay: incomplete frame retained"); + + size_t second_out = dns_frame_process_stream( + second, sizeof(second), &state, record_split_replay, &r); + CHECK(second_out == sizeof(second), "split replay: second read unchanged"); + CHECK(r.calls == 2 && r.replay_len == 6 && r.replay_matches, + "split replay: complete original payload detected"); + CHECK(state_is_clean(&state), "split replay: buffer released on completion"); + + uint8_t third[] = {0x00, 0x06, 0x20}; + (void) dns_frame_process_stream(third, sizeof(third), &state, + record_split_replay, &r); + CHECK(state.frame_buffer != NULL, + "split replay reset: incomplete frame retained"); + dns_frame_reset(&state); + CHECK(state_is_clean(&state), "split replay reset: buffer released"); + dns_frame_reset(&state); +} + int main(void) { test_single_frame_shortened(); test_single_frame_unchanged(); @@ -1136,6 +1227,7 @@ int main(void) { test_frame_len_u16_boundary(); test_multi_call_no_desync(); test_exhaustive_chunking(); + test_split_frame_replayed_when_complete(); if (failures == 0) { printf("dns_frame_test: all tests passed\n"); diff --git a/app/src/test/native/run_defensive_tests.sh b/app/src/test/native/run_defensive_tests.sh index 77ccc99f7..f6e02cfb1 100644 --- a/app/src/test/native/run_defensive_tests.sh +++ b/app/src/test/native/run_defensive_tests.sh @@ -19,7 +19,7 @@ compile() { if [[ ${BUILD_ONLY:-0} != 1 ]]; then "$OUT/$name"; fi } compile tcp_defensive_test app/src/test/native/tcp_defensive_test.c \ - app/src/main/jni/netguard/tcp.c \ + app/src/main/jni/netguard/tcp.c app/src/main/jni/netguard/dns_frame.c \ -Wl,--wrap=close -Wl,--wrap=connect -Wl,--wrap=send compile udp_defensive_test app/src/test/native/udp_defensive_test.c \ app/src/main/jni/netguard/udp.c \ diff --git a/app/src/test/native/run_dns_frame_tests.sh b/app/src/test/native/run_dns_frame_tests.sh new file mode 100755 index 000000000..1bff0f557 --- /dev/null +++ b/app/src/test/native/run_dns_frame_tests.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Host framing, buffer lifetime and real Rust-parser regression tests. +set -euo pipefail +cd "$(dirname "$0")/../../../.." +OUT=${OUT:-$(mktemp -d)} +mkdir -p "$OUT" +COMMON=(-O1 -g -Wall -Wextra -Werror -fsanitize=address,undefined + -fno-sanitize-recover=all -fno-omit-frame-pointer + -Iapp/src/main/jni/netguard) +"${CC:-cc}" "${COMMON[@]}" app/src/test/native/dns_frame_test.c \ + app/src/main/jni/netguard/dns_frame.c -o "$OUT/dns_frame_test" +"$OUT/dns_frame_test" +"${CC:-cc}" "${COMMON[@]}" app/src/test/native/dns_frame_allocation_test.c \ + -o "$OUT/dns_frame_allocation_test" +"$OUT/dns_frame_allocation_test" +export CARGO_TARGET_DIR="$PWD/wgbridge-rs/target" +cargo build --manifest-path wgbridge-rs/Cargo.toml --lib --locked --offline +"${CC:-cc}" "${COMMON[@]}" app/src/test/native/dns_frame_record_test.c \ + app/src/main/jni/netguard/dns_frame.c app/src/main/jni/netguard/dns.c \ + -D_GNU_SOURCE -include app/src/test/native/host_compat/linux_test.h \ + -idirafter app/src/test/native/host_compat -L"$CARGO_TARGET_DIR/debug" \ + -lwgbridge -Wl,-rpath,"$CARGO_TARGET_DIR/debug" -o "$OUT/dns_frame_record_test" +"$OUT/dns_frame_record_test" diff --git a/app/src/test/native/tcp_defensive_test.c b/app/src/test/native/tcp_defensive_test.c index 9cb390ef4..da3f0b1d1 100644 --- a/app/src/test/native/tcp_defensive_test.c +++ b/app/src/test/native/tcp_defensive_test.c @@ -185,14 +185,8 @@ void parse_dns_partial_response(const struct arguments *args, *blanked = 0; } -size_t dns_frame_process_stream(uint8_t *buffer, size_t bytes, - struct dns_stream_state *state, - dns_frame_parse_fn parse, void *ctx) { - (void) buffer; - (void) state; - (void) parse; - (void) ctx; - return bytes; +void record_dns_response(const struct arguments *args, uint8_t *data, size_t datalen) { + (void) args; (void) data; (void) datalen; } const char *strstate(const int state) {