diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 1649dce4a..ce751108a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -60,7 +60,18 @@ jobs: ./gradlew :app:prefetchOfflineBuildClasspaths - name: Run Rust unit tests - run: cargo test --manifest-path wgbridge-rs/Cargo.toml --locked --offline + run: | + cargo test --manifest-path wgbridge-rs/Cargo.toml --locked --offline + # The capi feature gates the C ABI the NetGuard engine calls; its + # FFI-boundary tests only compile with the feature enabled. + cargo test --manifest-path wgbridge-rs/Cargo.toml -p tc-dns --features capi --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 unit tests run: ./gradlew testFdroidDebugUnitTest --offline diff --git a/AGENTS.md b/AGENTS.md index 8165f5af4..98356af89 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,7 @@ covers — don't rely on what you remember of it. | `agents/docs/build-and-test.md` | anything beyond the commands below — prerequisites, flavours, the native C/Rust builds, the reproducibility flags | | `agents/docs/device-testing.md` | **before any `adb` command** — flavour choice, seeding away the permission prompts, editing preferences safely | | `agents/docs/triage.md` | reviewing, triaging or closing an issue — the verdict vocabulary and the two reusable close messages | -| `wgbridge-rs/README.md` | touching `wgbridge-rs/` or `net.kollnig.missioncontrol.wg*` — architecture, the JNI API surface, building/testing the crate | +| `wgbridge-rs/README.md` | touching `wgbridge-rs/`, `tc-dns`, DNS response rewriting, or `net.kollnig.missioncontrol.wg*` — architecture, the C/JNI API surfaces, building/testing the workspace | | `docs/RELEASING.md` | cutting a release — version bump, Fastlane changelog, tag-triggered unsigned build, local signing, smoke-test checklist | --- diff --git a/agents/docs/build-and-test.md b/agents/docs/build-and-test.md index d8bee8467..f58ec87e6 100644 --- a/agents/docs/build-and-test.md +++ b/agents/docs/build-and-test.md @@ -6,10 +6,10 @@ flavour matrix, the native builds, and the reproducibility flags. ## Prerequisites JDK 17, Android SDK (compile/target SDK 37, min SDK 23), NDK `27.2.12479018`, -CMake. For the WireGuard bridge you also need Rust ≥ 1.95 with the four Android -targets and `cargo-ndk` — but the Gradle build wires that in for you (it even -installs `cargo-ndk` if missing). See `wgbridge-rs/README.md` for the exact -`rustup target add …` list and F-Droid build metadata. +CMake. Native builds also need Rust ≥ 1.95 with the four Android targets; +the WireGuard bridge additionally needs `cargo-ndk`. Gradle wires both Rust +builds in but deliberately does not install tools or fetch crates. See +`wgbridge-rs/README.md` for setup and F-Droid build metadata. ## Flavours @@ -42,15 +42,20 @@ device, build **fdroid debug** instead — see `agents/docs/device-testing.md`. **Rust host tests** (config/DNS/key parsing — no device needed): ```bash -cd wgbridge-rs && cargo test +cd wgbridge-rs && cargo test --workspace +cd wgbridge-rs && cargo test -p tc-dns --features capi ``` ## Native code builds automatically with the app -- The **C engine** builds via CMake through AGP's `externalNativeBuild`. +- The **C engine** builds via CMake through AGP's `externalNativeBuild` and + calls `tc-dns` through the C ABI exported by `libwgbridge.so`. CMake + configure/build tasks depend on `wgbridgeBuild`; JVM compilation and unit + tests remain Rust-free. - The **Rust WireGuard bridge** builds via the `wgbridgeBuild` Gradle task, which - runs `cargo ndk` for all four ABIs and is a `dependsOn` of `preBuild`. It only - re-runs when `wgbridge-rs/src/**`, `Cargo.toml`, or `Cargo.lock` change. + runs `cargo ndk` for all four ABIs and is needed only by JNI-library packaging + tasks. It also tracks the shared `tc-dns` source because the bridge links the + same core. If Gradle can't find `cargo` (Android Studio sanitizes `PATH`), pass `-PcargoBin=/path/to/cargo` or put `~/.cargo/bin` on `PATH`. diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 9033f0785..36081df11 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -14,6 +14,7 @@ add_library( netguard src/main/jni/netguard/ip.c src/main/jni/netguard/tls.c src/main/jni/netguard/tcp.c + src/main/jni/netguard/dns_frame.c src/main/jni/netguard/udp.c src/main/jni/netguard/icmp.c src/main/jni/netguard/dns.c @@ -26,8 +27,20 @@ include_directories( src/main/jni/netguard/ ) find_library( log-lib log ) +if(NOT DEFINED WGBRIDGE_LIB_DIR) + message(FATAL_ERROR "WGBRIDGE_LIB_DIR not set -- build through Gradle, which runs wgbridgeBuild first.") +endif() + +add_library(wgbridge SHARED IMPORTED) +set_target_properties(wgbridge PROPERTIES + IMPORTED_LOCATION "${WGBRIDGE_LIB_DIR}/${ANDROID_ABI}/libwgbridge.so" + # cargo-ndk's cdylib has no DT_SONAME; use -lwgbridge so DT_NEEDED is + # the packaged basename instead of a build-directory-relative path. + IMPORTED_NO_SONAME TRUE) + target_link_libraries( netguard - ${log-lib} ) + ${log-lib} + wgbridge ) target_link_options(netguard PRIVATE "-Wl,-z,max-page-size=16384" diff --git a/app/build.gradle b/app/build.gradle index 9bf506a04..0550e748c 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -58,7 +58,8 @@ android { cppFlags "" arguments "-DANDROID_PLATFORM=android-22", "-DCMAKE_C_COMPILER_LAUNCHER=${ccacheLauncher}", - "-DCMAKE_CXX_COMPILER_LAUNCHER=${ccacheLauncher}" + "-DCMAKE_CXX_COMPILER_LAUNCHER=${ccacheLauncher}", + "-DWGBRIDGE_LIB_DIR=${buildDir}/rustJniLibs" // https://developer.android.com/ndk/guides/cmake.html } } diff --git a/app/gradle/wgbridge.gradle b/app/gradle/wgbridge.gradle index 2defcfd82..34964c117 100644 --- a/app/gradle/wgbridge.gradle +++ b/app/gradle/wgbridge.gradle @@ -78,61 +78,83 @@ def resolveCargoTargetDir = { -> return file("${System.getProperty('user.home')}/.cargo/tc-wgbridge-target") } +def assertCargoAvailable = { cargoBin -> + def onPath = System.getenv('PATH')?.split(File.pathSeparator)?.any { + file("$it/cargo").canExecute() + } + if ((cargoBin == 'cargo' && !onPath) || (cargoBin != 'cargo' && !file(cargoBin).canExecute())) { + throw new GradleException(""" + Could not find a `cargo` executable. Install Rust via rustup + (https://rustup.rs), then run: + ./scripts/setup_rust_android.sh + and either add ~/.cargo/bin to PATH (restart Android Studio), + or pass -PcargoBin=/path/to/cargo to Gradle. + """.stripIndent()) + } + return cargoBin +} + +// Build the cargo environment in one place. Keeping the target directory and +// encoded flags stable is load-bearing: otherwise cargo alternates between +// fingerprints and repeats the full cross-compile on every invocation. +def buildCargoEnv = { -> + def cargoBin = assertCargoAvailable(findCargoBinary()) + def cargoDir = file(cargoBin).parent + def env = new HashMap(System.getenv()) + def pathEntries = [] + if (cargoDir) pathEntries << cargoDir + pathEntries << "${System.getProperty('user.home')}/.cargo/bin" + if (env['PATH']) pathEntries << env['PATH'] + env['PATH'] = pathEntries.join(File.pathSeparator) + env['ANDROID_NDK_HOME'] = findNdkDirectory().absolutePath + + // Shared, checkout-independent target dir so worktrees reuse artifacts. + def cargoTargetDir = resolveCargoTargetDir() + cargoTargetDir.mkdirs() + env['CARGO_TARGET_DIR'] = cargoTargetDir.absolutePath + + // Android 15+ requires 16KB-aligned ELF LOAD segments. These flags are + // shared with every Rust build so cargo fingerprints stay stable. + def rustFlags = [] + def separator = '\u001f' + if (env['CARGO_ENCODED_RUSTFLAGS']) { + rustFlags.addAll(env['CARGO_ENCODED_RUSTFLAGS'].split(separator).findAll { !it.isEmpty() }) + } else if (env['RUSTFLAGS']) { + rustFlags.addAll(env['RUSTFLAGS'].trim().split(/\s+/)) + } + def cargoHome = env['CARGO_HOME'] ?: "${System.getProperty('user.home')}/.cargo" + rustFlags.addAll([ + '-Clink-arg=-Wl,-z,max-page-size=16384', + '-Clink-arg=-Wl,-z,common-page-size=16384', + '-Clink-arg=-Wl,--build-id=none', + "--remap-path-prefix=${file(cargoHome).absolutePath}=/cargo", + "--remap-path-prefix=${wgbridgeSrcDir.absolutePath}=/wgbridge-rs", + ]) + // Unlike RUSTFLAGS, the encoded form preserves spaces inside paths. + env['CARGO_ENCODED_RUSTFLAGS'] = rustFlags.join(separator) + env.remove('RUSTFLAGS') + + return [cargoBin: cargoBin, env: env, cargoTargetDir: cargoTargetDir] +} + tasks.register('wgbridgeBuild') { description = 'Builds libwgbridge.so via `cargo ndk`. Requires rustup with the four Android targets, cargo-ndk and the Android NDK.' group = 'build' inputs.files(fileTree(wgbridgeSrcDir) { include 'src/**' + include 'tc-dns/**' include 'Cargo.toml' include 'Cargo.lock' }).withPropertyName('wgbridgeSource').withPathSensitivity(PathSensitivity.RELATIVE) + inputs.file(file("$rootDir/rust-toolchain.toml")).withPropertyName('rustToolchain') outputs.dir(wgbridgeOutDir).withPropertyName('wgbridgeJniLibs') outputs.cacheIf { true } doLast { - def cargoBin = findCargoBinary() - if (cargoBin == 'cargo' && !System.getenv('PATH')?.split(File.pathSeparator)?.any { file("$it/cargo").canExecute() }) { - throw new GradleException(""" - Could not find a `cargo` executable. Install Rust via rustup - (https://rustup.rs), then run: - ./scripts/setup_rust_android.sh - and either add ~/.cargo/bin to PATH (restart Android Studio), - or pass -PcargoBin=/path/to/cargo to Gradle. - """.stripIndent()) - } - def cargoDir = file(cargoBin).parent - - def env = new HashMap(System.getenv()) - env['PATH'] = "${cargoDir}${File.pathSeparator}${System.getProperty('user.home')}/.cargo/bin${File.pathSeparator}${env['PATH'] ?: ''}" - env['ANDROID_NDK_HOME'] = findNdkDirectory().absolutePath - // Shared, checkout-independent target dir so worktrees reuse artifacts. - def cargoTargetDir = resolveCargoTargetDir() - cargoTargetDir.mkdirs() - env['CARGO_TARGET_DIR'] = cargoTargetDir.absolutePath - // Android 15+ requires 16KB-aligned ELF LOAD segments. - // --remap-path-prefix strips the checkout- and machine-specific - // absolute paths (cargo registry cache, wgbridge-rs checkout dir) - // that rustc otherwise bakes into panic/track_caller strings, so - // libwgbridge.so is reproducible across build machines. - def rustFlags = [] - if (env['CARGO_ENCODED_RUSTFLAGS']) { - rustFlags.addAll(env['CARGO_ENCODED_RUSTFLAGS'].split('').findAll { !it.isEmpty() }) - } else if (env['RUSTFLAGS']) { - // Cargo interprets RUSTFLAGS as whitespace-separated arguments. - rustFlags.addAll(env['RUSTFLAGS'].trim().split(/\s+/)) - } - def cargoHome = env['CARGO_HOME'] ?: "${System.getProperty('user.home')}/.cargo" - rustFlags.addAll([ - '-Clink-arg=-Wl,-z,max-page-size=16384', - '-Clink-arg=-Wl,-z,common-page-size=16384', - '-Clink-arg=-Wl,--build-id=none', - "--remap-path-prefix=${file(cargoHome).absolutePath}=/cargo", - "--remap-path-prefix=${wgbridgeSrcDir.absolutePath}=/wgbridge-rs", - ]) - // Unlike RUSTFLAGS, the encoded form preserves spaces inside paths. - env['CARGO_ENCODED_RUSTFLAGS'] = rustFlags.join('') - env.remove('RUSTFLAGS') + def cargo = buildCargoEnv() + def cargoBin = cargo.cargoBin + def env = cargo.env // Tool installation must happen before Gradle runs. In particular, // F-Droid disables network access for the actual build phase. @@ -150,7 +172,7 @@ tasks.register('wgbridgeBuild') { def args = [cargoBin, 'ndk'] wgbridgeAbis.each { args += ['-t', it] } args += ['--platform', '23', '-o', wgbridgeOutDir.absolutePath, - 'build', '--release', '--locked', '--offline'] + 'build', '-p', 'wgbridge', '--release', '--locked', '--offline'] wgbridgeExecOperations.exec { workingDir wgbridgeSrcDir @@ -191,6 +213,46 @@ tasks.register('wgbridgeBuild') { }.result.get().assertNormalExitValue() } } + + // tc-dns is an rlib dependency of the wgbridge cdylib; its + // #[no_mangle] pub extern "C" symbols (tcdns_process_response, + // tcdns_abi_version — see app/src/main/jni/netguard/tcdns.h) only + // reach libwgbridge.so because rustc currently chooses to keep them. + // libnetguard.so links against libwgbridge.so for these symbols (the + // IMPORTED_NO_SONAME wgbridge target in app/CMakeLists.txt), so a + // future LTO / --gc-sections / rustc change that drops the re-export + // would silently build but fail at runtime with an + // UnsatisfiedLinkError. Catch that here instead. + def requiredTcdnsSymbols = ['tcdns_process_response', 'tcdns_abi_version'] + def llvmNm = "${findNdkDirectory().absolutePath}/toolchains/llvm/prebuilt/${hostTag}/bin/llvm-nm" + wgbridgeAbis.each { abi -> + def lib = file("$wgbridgeOutDir/$abi/libwgbridge.so") + // A missing library is itself a failure here: this check exists + // precisely to stop an unusable artifact from shipping. + if (!lib.exists()) + throw new GradleException("libwgbridge.so ($abi) was not produced by `cargo ndk`.") + def symbolOutput = new ByteArrayOutputStream() + wgbridgeExecOperations.exec { + commandLine llvmNm, '--defined-only', '--dynamic', lib.absolutePath + standardOutput = symbolOutput + }.assertNormalExitValue() + def exportedSymbols = symbolOutput.toString() + requiredTcdnsSymbols.each { symbol -> + if (!exportedSymbols.contains(symbol)) { + throw new GradleException(""" + libwgbridge.so ($abi) is missing the expected dynamic + symbol `$symbol` from the tc-dns crate. libnetguard.so + has a hard runtime dependency on this symbol (see + app/src/main/jni/netguard/tcdns.h); without it the app + will crash with UnsatisfiedLinkError on device. + This likely means a linker/LTO/rustc change stopped + re-exporting tc-dns's #[no_mangle] symbols through the + wgbridge cdylib — check wgbridge-rs/Cargo.toml and + wgbridge-rs/tc-dns for crate-type / visibility changes. + """.stripIndent()) + } + } + } } } @@ -198,6 +260,8 @@ tasks.register('wgbridgeBuild') { // preBuild is important: JVM unit tests compile app code but never load the // Android .so, and should not cross-compile four release ABIs first. tasks.configureEach { task -> + if (task.name ==~ /(configure|build)CMake.*/) + task.dependsOn 'wgbridgeBuild' if (task.name ==~ /merge.*JniLibFolders/) task.dependsOn 'wgbridgeBuild' } diff --git a/app/src/main/jni/netguard/dns.c b/app/src/main/jni/netguard/dns.c index b61927ded..1f7568b62 100644 --- a/app/src/main/jni/netguard/dns.c +++ b/app/src/main/jni/netguard/dns.c @@ -19,221 +19,89 @@ #include "netguard.h" -int32_t get_qname(const uint8_t *data, const size_t datalen, uint16_t off, char *qname) { - *qname = 0; - - if (off >= datalen) - return -1; - - uint16_t c = 0; - uint8_t noff = 0; - uint16_t ptr = off; - uint8_t len = *(data + ptr); - uint8_t count = 0; - while (len) { - if (count++ > 25) - break; - - if (ptr + 1 < datalen && (len & 0xC0)) { - uint16_t jump = (uint16_t) ((len & 0x3F) * 256 + *(data + ptr + 1)); - if (jump >= datalen) { - log_android(ANDROID_LOG_DEBUG, "DNS invalid jump"); - break; - } - ptr = jump; - len = *(data + ptr); - log_android(ANDROID_LOG_DEBUG, "DNS qname compression ptr %d len %d", ptr, len); - if (!c) { - c = 1; - off += 2; - } - } else if (ptr + 1 + len < datalen && noff + len <= DNS_QNAME_MAX) { - memcpy(qname + noff, data + ptr + 1, len); - *(qname + noff + len) = '.'; - noff += (len + 1); - - uint16_t jump = (uint16_t) (ptr + 1 + len); - if (jump >= datalen) { - log_android(ANDROID_LOG_DEBUG, "DNS invalid jump"); - break; - } - ptr = jump; - len = *(data + ptr); - } else - break; - } - ptr++; - - if (len > 0 || noff == 0) { - log_android(ANDROID_LOG_ERROR, "DNS qname invalid len %d noff %d", len, noff); - return -1; - } +struct tcdns_ctx { + const struct arguments *args; + const struct ng_session *s; +}; + +static void tcdns_record_answer(void *opaque, const char *qname, const char *aname, + const char *resource, int32_t ttl) { + const struct tcdns_ctx *ctx = (const struct tcdns_ctx *) opaque; + dns_resolved(ctx->args, qname, aname, resource, ttl); +} - *(qname + noff - 1) = 0; - log_android(ANDROID_LOG_DEBUG, "qname %s", qname); +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 (c ? off : ptr); +static uint8_t tcdns_blocked_rcode(void *opaque) { + const struct tcdns_ctx *ctx = (const struct tcdns_ctx *) opaque; + return (uint8_t) ctx->args->rcode; } -void parse_dns_response(const struct arguments *args, const struct ng_session *s, - const uint8_t *data, size_t *datalen) { - if (*datalen < sizeof(struct dns_header) + 1) { - log_android(ANDROID_LOG_WARN, "DNS response length %d", *datalen); - return; +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; + const struct arguments *args = ctx->args; + const struct ng_session *s = ctx->s; + + int version; + char source[INET6_ADDRSTRLEN + 1]; + char dest[INET6_ADDRSTRLEN + 1]; + uint16_t sport; + uint16_t dport; + + if (s->protocol == IPPROTO_UDP) { + version = s->udp.version; + sport = ntohs(s->udp.source); + dport = ntohs(s->udp.dest); + if (s->udp.version == 4) { + inet_ntop(AF_INET, &s->udp.saddr.ip4, source, sizeof(source)); + inet_ntop(AF_INET, &s->udp.daddr.ip4, dest, sizeof(dest)); + } else { + inet_ntop(AF_INET6, &s->udp.saddr.ip6, source, sizeof(source)); + inet_ntop(AF_INET6, &s->udp.daddr.ip6, dest, sizeof(dest)); + } + } else { + version = s->tcp.version; + sport = ntohs(s->tcp.source); + dport = ntohs(s->tcp.dest); + if (s->tcp.version == 4) { + inet_ntop(AF_INET, &s->tcp.saddr.ip4, source, sizeof(source)); + inet_ntop(AF_INET, &s->tcp.daddr.ip4, dest, sizeof(dest)); + } else { + inet_ntop(AF_INET6, &s->tcp.saddr.ip6, source, sizeof(source)); + inet_ntop(AF_INET6, &s->tcp.daddr.ip6, dest, sizeof(dest)); + } } - // Check if standard DNS query - // TODO multiple qnames - struct dns_header *dns = (struct dns_header *) data; - int qcount = ntohs(dns->q_count); - int acount = ntohs(dns->ans_count); - if (dns->qr == 1 && dns->opcode == 0 && qcount > 0 && acount > 0) { - log_android(ANDROID_LOG_DEBUG, "DNS response qcount %d acount %d", qcount, acount); - if (qcount > 1) - log_android(ANDROID_LOG_WARN, "DNS response qcount %d acount %d", qcount, acount); - - // http://tools.ietf.org/html/rfc1035 - char name[DNS_QNAME_MAX + 1]; - int32_t off = sizeof(struct dns_header); - - uint16_t qtype; - uint16_t qclass; - char qname[DNS_QNAME_MAX + 1]; - - for (int q = 0; q < 1; q++) { - off = get_qname(data, *datalen, (uint16_t) off, name); - if (off > 0 && off + 4 <= *datalen) { - // TODO multiple qnames? - if (q == 0) { - strcpy(qname, name); - qtype = ntohs(*((uint16_t *) (data + off))); - qclass = ntohs(*((uint16_t *) (data + off + 2))); - log_android(ANDROID_LOG_DEBUG, - "DNS question %d qtype %d qclass %d qname %s", - q, qtype, qclass, qname); - } - off += 4; - } else { - log_android(ANDROID_LOG_WARN, - "DNS response Q invalid off %d datalen %d", off, *datalen); - return; - } - } + char name[DNS_QNAME_MAX + 40 + 1]; + (void) snprintf(name, sizeof(name), "qtype %u qname %s rcode %u", + qtype, qname, rcode); + jobject objPacket = create_packet( + args, version, s->protocol, "", + source, sport, dest, dport, + name, 0, 0); + log_packet(args, objPacket); +} - short svcb = 0; - int32_t aoff = off; - for (int a = 0; a < acount; a++) { - off = get_qname(data, *datalen, (uint16_t) off, name); - if (off > 0 && off + 10 <= *datalen) { - uint16_t qtype = ntohs(*((uint16_t *) (data + off))); - uint16_t qclass = ntohs(*((uint16_t *) (data + off + 2))); - uint32_t ttl = ntohl(*((uint32_t *) (data + off + 4))); - uint16_t rdlength = ntohs(*((uint16_t *) (data + off + 8))); - off += 10; - - if (off + rdlength <= *datalen) { - if (qclass == DNS_QCLASS_IN && - (qtype == DNS_QTYPE_A || qtype == DNS_QTYPE_AAAA)) { - - char rd[INET6_ADDRSTRLEN + 1]; - if (qtype == DNS_QTYPE_A) { - if (off + sizeof(__be32) <= *datalen) - inet_ntop(AF_INET, data + off, rd, sizeof(rd)); - else - return; - } else if (qclass == DNS_QCLASS_IN && qtype == DNS_QTYPE_AAAA) { - if (off + sizeof(struct in6_addr) <= *datalen) - inet_ntop(AF_INET6, data + off, rd, sizeof(rd)); - else - return; - } - - dns_resolved(args, qname, name, rd, ttl); - log_android(ANDROID_LOG_DEBUG, - "DNS answer %d qname %s qtype %d ttl %d data %s", - a, name, qtype, ttl, rd); - } else if (qclass == DNS_QCLASS_IN && - (qtype == DNS_SVCB || qtype == DNS_HTTPS)) { - // https://tools.ietf.org/id/draft-ietf-dnsop-svcb-https-01.html - svcb = 1; - log_android(ANDROID_LOG_WARN, - "SVCB answer %d qname %s qtype %d", a, name, qtype); - } else - log_android(ANDROID_LOG_DEBUG, - "DNS answer %d qname %s qclass %d qtype %d ttl %d length %d", - a, name, qclass, qtype, ttl, rdlength); - - off += rdlength; - } else { - log_android(ANDROID_LOG_WARN, - "DNS response A invalid off %d rdlength %d datalen %d", - off, rdlength, *datalen); - return; - } - } else { - log_android(ANDROID_LOG_WARN, - "DNS response A invalid off %d datalen %d", off, *datalen); - return; - } - } +static const tcdns_callbacks tcdns_callbacks_template = { + .abi_version = TCDNS_ABI_VERSION, + .record_answer = tcdns_record_answer, + .is_domain_blocked = tcdns_is_domain_blocked, + .blocked_rcode = tcdns_blocked_rcode, + .on_blanked = tcdns_on_blanked, + .log = NULL, +}; - if (qcount > 0 && - (svcb || is_domain_blocked(args, qname))) { - dns->qr = 1; - dns->aa = 0; - dns->tc = 0; - dns->rd = 0; - dns->ra = 0; - dns->z = 0; - dns->ad = 0; - dns->cd = 0; - dns->rcode = (uint16_t) args->rcode; - dns->ans_count = 0; - dns->auth_count = 0; - dns->add_count = 0; - *datalen = aoff; - - int version; - char source[INET6_ADDRSTRLEN + 1]; - char dest[INET6_ADDRSTRLEN + 1]; - uint16_t sport; - uint16_t dport; - - if (s->protocol == IPPROTO_UDP) { - version = s->udp.version; - sport = ntohs(s->udp.source); - dport = ntohs(s->udp.dest); - if (s->udp.version == 4) { - inet_ntop(AF_INET, &s->udp.saddr.ip4, source, sizeof(source)); - inet_ntop(AF_INET, &s->udp.daddr.ip4, dest, sizeof(dest)); - } else { - inet_ntop(AF_INET6, &s->udp.saddr.ip6, source, sizeof(source)); - inet_ntop(AF_INET6, &s->udp.daddr.ip6, dest, sizeof(dest)); - } - } else { - version = s->tcp.version; - sport = ntohs(s->tcp.source); - dport = ntohs(s->tcp.dest); - if (s->tcp.version == 4) { - inet_ntop(AF_INET, &s->tcp.saddr.ip4, source, sizeof(source)); - inet_ntop(AF_INET, &s->tcp.daddr.ip4, dest, sizeof(dest)); - } else { - inet_ntop(AF_INET6, &s->tcp.saddr.ip6, source, sizeof(source)); - inet_ntop(AF_INET6, &s->tcp.daddr.ip6, dest, sizeof(dest)); - } - } - - // Log qname - char name[DNS_QNAME_MAX + 40 + 1]; - sprintf(name, "qtype %d qname %s rcode %d", qtype, qname, dns->rcode); - jobject objPacket = create_packet( - args, version, s->protocol, "", - source, sport, dest, dport, - name, 0, 0); - log_packet(args, objPacket); - } - } else if (acount > 0) - log_android(ANDROID_LOG_WARN, - "DNS response qr %d opcode %d qcount %d acount %d", - dns->qr, dns->opcode, qcount, acount); +#define TCDNS_CALLBACKS_INIT tcdns_callbacks_template + +void parse_dns_response(const struct arguments *args, const struct ng_session *s, + uint8_t *data, size_t *datalen) { + struct tcdns_ctx ctx = { .args = args, .s = s }; + tcdns_callbacks cb = TCDNS_CALLBACKS_INIT; + size_t new_len = tcdns_process_response(data, *datalen, &cb, &ctx); + if (new_len != TCDNS_UNCHANGED) + *datalen = new_len; } diff --git a/app/src/main/jni/netguard/dns_frame.c b/app/src/main/jni/netguard/dns_frame.c new file mode 100644 index 000000000..b374222af --- /dev/null +++ b/app/src/main/jni/netguard/dns_frame.c @@ -0,0 +1,57 @@ +/* + This file is part of NetGuard. + + NetGuard is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + NetGuard is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with NetGuard. If not, see . + + Copyright 2015-2019 by Marcel Bokhorst (M66B) +*/ + +#include "dns_frame.h" + +struct dns_frame_decision dns_frame_decide(size_t bytes, size_t frame_len) { + struct dns_frame_decision d; + d.frame_len = frame_len; + + if (frame_len == 0) { + // A zero-length declared frame is not something the parser can + // act on; skip it entirely, matching the original "if (frame_len + // > 0)" guard. + d.should_parse = 0; + d.isolated = 0; + d.dlen = 0; + return d; + } + + d.should_parse = 1; + d.isolated = (frame_len + 2 == bytes) ? 1 : 0; + + // avail is the payload available in this recv() after the 2-byte + // prefix. The caller guarantees bytes > 2, so avail >= 1. + size_t avail = bytes - 2; + // isolated implies frame_len == avail. + d.dlen = (frame_len < avail) ? frame_len : avail; + + return d; +} + +void dns_frame_apply_rewrite(uint8_t *buffer, + const struct dns_frame_decision *decision, + size_t post_parse_dlen, + ssize_t *bytes) { + if (decision->isolated && post_parse_dlen != decision->frame_len) { + buffer[0] = (uint8_t) (post_parse_dlen >> 8); + buffer[1] = (uint8_t) post_parse_dlen; + *bytes = (ssize_t) post_parse_dlen + 2; + } +} diff --git a/app/src/main/jni/netguard/dns_frame.h b/app/src/main/jni/netguard/dns_frame.h new file mode 100644 index 000000000..d86d37e18 --- /dev/null +++ b/app/src/main/jni/netguard/dns_frame.h @@ -0,0 +1,93 @@ +/* + This file is part of NetGuard. + + NetGuard is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + NetGuard is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with NetGuard. If not, see . + + Copyright 2015-2019 by Marcel Bokhorst (M66B) +*/ + +#ifndef DNS_FRAME_H +#define DNS_FRAME_H + +/* + * Pure decision logic for DNS-over-TCP framing, 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. + * + * A single recv() on a DNS-over-TCP (port 53) socket may contain: + * - an isolated complete frame (2-byte length prefix + exactly one + * DNS message, with nothing left over); + * - a coalesced read (that frame plus additional bytes -- the start of + * the next frame, or more); + * - a split/partial frame (fewer bytes than the prefix declares). + * + * Policy: every frame's DNS payload is handed to the DNS parser so header + * blanking/policy enforcement always applies, but only an isolated + * complete frame may be shortened afterward (with its 2-byte length + * prefix rewritten to match). Shortening a coalesced or split read would + * discard bytes that still belong to the TCP stream. + */ + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +struct dns_frame_decision { + /* Whether the DNS parser should be invoked at all for this recv(). */ + int should_parse; + /* Whether this recv() held exactly one complete frame (frame_len + 2 + * == bytes). Only an isolated frame's prefix may be rewritten. */ + int isolated; + /* The frame length taken verbatim from the 2-byte prefix. */ + size_t frame_len; + /* The payload length to hand to the DNS parser: at most bytes - 2, + * so a split/partial read is never over-read. */ + size_t dlen; +}; + +/* + * Computes the framing decision for one recv() on a DNS-over-TCP socket. + * + * bytes: total bytes read by recv(); the caller guarantees bytes > 2 + * (there is at least a full 2-byte length prefix plus one byte). + * frame_len: the 16-bit frame length taken from the first two bytes of + * the buffer (buffer[0] << 8 | buffer[1]), before this call. + */ +struct dns_frame_decision dns_frame_decide(size_t bytes, size_t frame_len); + +/* + * Applies the post-parse rewrite to buffer, if any. buffer must point at + * the full recv() buffer, including the 2-byte length prefix, and must be + * at least *bytes long. post_parse_dlen is the (possibly shrunk) payload + * length the DNS parser reported. *bytes is updated in place when a + * rewrite happens; otherwise it is left untouched. + * + * No-op unless decision->isolated and post_parse_dlen differs from + * decision->frame_len -- matching the original inline check. + */ +void dns_frame_apply_rewrite(uint8_t *buffer, + const struct dns_frame_decision *decision, + size_t post_parse_dlen, + ssize_t *bytes); + +#ifdef __cplusplus +} +#endif + +#endif /* DNS_FRAME_H */ diff --git a/app/src/main/jni/netguard/netguard.c b/app/src/main/jni/netguard/netguard.c index 1f2c246b2..2a42dd9e7 100644 --- a/app/src/main/jni/netguard/netguard.c +++ b/app/src/main/jni/netguard/netguard.c @@ -79,6 +79,12 @@ jclass clsUsage; jint JNI_OnLoad(JavaVM *vm, void *reserved) { log_android(ANDROID_LOG_INFO, "JNI load"); + if (tcdns_abi_version() != TCDNS_ABI_VERSION) { + log_android(ANDROID_LOG_ERROR, "tc-dns ABI mismatch: native %u, header %u", + tcdns_abi_version(), TCDNS_ABI_VERSION); + return -1; + } + JNIEnv *env; if ((*vm)->GetEnv(vm, (void **) &env, JNI_VERSION_1_6) != JNI_OK) { log_android(ANDROID_LOG_INFO, "JNI load GetEnv failed"); diff --git a/app/src/main/jni/netguard/netguard.h b/app/src/main/jni/netguard/netguard.h index 7ec615db1..d60ba58f8 100644 --- a/app/src/main/jni/netguard/netguard.h +++ b/app/src/main/jni/netguard/netguard.h @@ -33,6 +33,8 @@ #include #include +#include "tcdns.h" + #define TAG "TrackerControl.JNI" // #define PROFILE_JNI 5 @@ -403,10 +405,8 @@ void check_icmp_socket(const struct arguments *args, const struct epoll_event *e void check_udp_socket(const struct arguments *args, const struct epoll_event *ev); -int32_t get_qname(const uint8_t *data, const size_t datalen, uint16_t off, char *qname); - void parse_dns_response(const struct arguments *args, const struct ng_session *session, - const uint8_t *data, size_t *datalen); + uint8_t *data, size_t *datalen); uint32_t get_send_window(const struct tcp_session *cur); diff --git a/app/src/main/jni/netguard/tcdns.h b/app/src/main/jni/netguard/tcdns.h new file mode 100644 index 000000000..c21aa59aa --- /dev/null +++ b/app/src/main/jni/netguard/tcdns.h @@ -0,0 +1,59 @@ +#ifndef TCDNS_H +#define TCDNS_H + +#include +#include + +#define TCDNS_ABI_VERSION 1u +#define TCDNS_UNCHANGED ((size_t) -1) + +/* + * Callbacks used by the dependency-free tc-dns message rewriter. + * + * tcdns_process_response receives one bare DNS message: it must not include + * an IP/UDP header or a DNS-over-TCP two-byte length prefix. The data pointer + * is valid for reads and writes for len bytes and is never retained. + * + * Every string passed to a callback is NUL-terminated, valid UTF-8 and + * modified-UTF-8 compatible, owned by the library, and valid only for the + * duration of that callback. Invalid UTF-8, embedded NUL bytes and + * supplementary scalar values are replaced with U+FFFD. ctx is opaque and is + * passed back verbatim. Callbacks must not re-enter + * tcdns_process_response or longjmp. + */ +typedef struct tcdns_callbacks { + uint32_t abi_version; + void (*record_answer)(void *ctx, const char *qname, const char *aname, + const char *resource, int32_t ttl); + int (*is_domain_blocked)(void *ctx, const char *qname); + uint8_t (*blocked_rcode)(void *ctx); + void (*on_blanked)(void *ctx, const char *qname, uint16_t qtype, uint8_t rcode); + /* Reserved for bounded malformed-input diagnostics; currently unused. */ + void (*log)(void *ctx, int32_t priority, const char *msg); /* nullable */ +} tcdns_callbacks; + +/* Android builds are 32- or 64-bit; catch accidental C ABI field drift. */ +_Static_assert(sizeof(tcdns_callbacks) == (sizeof(void *) == 8 ? 48 : 24), + "tcdns_callbacks ABI layout changed"); +_Static_assert(offsetof(tcdns_callbacks, record_answer) == (sizeof(void *) == 8 ? 8 : 4), + "tcdns_callbacks.record_answer offset changed"); +_Static_assert(offsetof(tcdns_callbacks, is_domain_blocked) == (sizeof(void *) == 8 ? 16 : 8), + "tcdns_callbacks.is_domain_blocked offset changed"); +_Static_assert(offsetof(tcdns_callbacks, blocked_rcode) == (sizeof(void *) == 8 ? 24 : 12), + "tcdns_callbacks.blocked_rcode offset changed"); +_Static_assert(offsetof(tcdns_callbacks, on_blanked) == (sizeof(void *) == 8 ? 32 : 16), + "tcdns_callbacks.on_blanked offset changed"); +_Static_assert(offsetof(tcdns_callbacks, log) == (sizeof(void *) == 8 ? 40 : 20), + "tcdns_callbacks.log offset changed"); + +/* + * Returns the new message length (end of the question section) when a policy + * hit blanks the response, or TCDNS_UNCHANGED otherwise. Bytes past the new + * length are left untouched. + */ +size_t tcdns_process_response(uint8_t *data, size_t len, + const tcdns_callbacks *cb, void *ctx); + +uint32_t tcdns_abi_version(void); + +#endif /* TCDNS_H */ diff --git a/app/src/main/jni/netguard/tcp.c b/app/src/main/jni/netguard/tcp.c index 770ec224d..d0bf405f0 100644 --- a/app/src/main/jni/netguard/tcp.c +++ b/app/src/main/jni/netguard/tcp.c @@ -18,6 +18,7 @@ */ #include "netguard.h" +#include "dns_frame.h" extern char socks5_addr[INET6_ADDRSTRLEN + 1]; extern int socks5_port; @@ -604,8 +605,20 @@ void check_tcp_socket(const struct arguments *args, // Process DNS response if (ntohs(s->tcp.dest) == 53 && bytes > 2) { - ssize_t dlen = bytes - 2; - parse_dns_response(args, s, buffer + 2, (size_t *) &dlen); + size_t frame_len = ((size_t) buffer[0] << 8) | buffer[1]; + // recv() may split or coalesce DNS frames. The + // header is blanked in place for every frame so + // policy still applies, but only an isolated + // complete frame can be shortened, because + // trimming a coalesced or split read would + // discard stream bytes. + struct dns_frame_decision decision = + dns_frame_decide((size_t) bytes, frame_len); + if (decision.should_parse) { + size_t dlen = decision.dlen; + parse_dns_response(args, s, buffer + 2, &dlen); + dns_frame_apply_rewrite(buffer, &decision, dlen, &bytes); + } } // Forward to tun diff --git a/app/src/main/jni/netguard/udp.c b/app/src/main/jni/netguard/udp.c index b070c0dcd..4e2d3a8a5 100644 --- a/app/src/main/jni/netguard/udp.c +++ b/app/src/main/jni/netguard/udp.c @@ -130,8 +130,11 @@ void check_udp_socket(const struct arguments *args, const struct epoll_event *ev s->udp.received += bytes; // Process DNS response - if (ntohs(s->udp.dest) == 53) - parse_dns_response(args, s, buffer, (size_t *) &bytes); + if (ntohs(s->udp.dest) == 53) { + size_t dlen = (size_t) bytes; + parse_dns_response(args, s, buffer, &dlen); + bytes = (ssize_t) dlen; + } // Forward to tun if (write_udp(args, &s->udp, buffer, (size_t) bytes) < 0) diff --git a/app/src/test/native/dns_frame_test.c b/app/src/test/native/dns_frame_test.c new file mode 100644 index 000000000..45c4bd0df --- /dev/null +++ b/app/src/test/native/dns_frame_test.c @@ -0,0 +1,258 @@ +/* + * Host unit tests for the DNS-over-TCP framing decision extracted into + * app/src/main/jni/netguard/dns_frame.{h,c}. + * + * This is a plain C test program with a tiny assert-based harness (no test + * framework dependency), so it can build and run with the system compiler + * on any host -- see .github/workflows/test.yml. It intentionally does not + * link netguard.h, JNI, or parse_dns_response: dns_frame_decide() and + * dns_frame_apply_rewrite() are pure, so a DNS-parse outcome is simulated + * inline (a "post_parse_dlen" value) instead of calling the real parser. + * + * Background: this framing logic shipped a real blocking bypass once + * already, fixed in commit 9c49cc09 ("Fix DNS filtering regressions from + * the tc-dns extraction") -- an earlier refactor skipped parse_dns_response + * entirely unless a recv() held exactly one complete frame, so coalesced or + * split reads went unfiltered. These tests pin the correct behavior down so + * that regression cannot silently return. + */ + +#include +#include +#include +#include + +#include "dns_frame.h" + +static int failures = 0; + +#define CHECK(cond, msg) \ + do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL: %s (%s:%d)\n", (msg), __FILE__, __LINE__); \ + failures++; \ + } \ + } while (0) + +/* Writes a big-endian 16-bit length prefix into buffer[0..1]. */ +static void set_prefix(uint8_t *buffer, size_t frame_len) { + buffer[0] = (uint8_t) (frame_len >> 8); + buffer[1] = (uint8_t) frame_len; +} + +static size_t read_prefix(const uint8_t *buffer) { + return ((size_t) buffer[0] << 8) | buffer[1]; +} + +/* A test double standing in for parse_dns_response(): optionally shrinks + * dlen (simulating a policy hit that blanks/truncates the DNS message), + * or leaves it unchanged. The extracted helpers never call this -- they + * only ever see its *result*, which is exactly the point of extracting + * them as pure functions. */ +static size_t stub_parse_unchanged(size_t dlen) { + return dlen; +} + +static size_t stub_parse_shrink_to(size_t new_len) { + return new_len; +} + +/* 1. Isolated complete frame: parser shortens it -> shorten + prefix rewritten. */ +static void test_isolated_frame_shortened(void) { + size_t frame_len = 50; + ssize_t bytes = (ssize_t) (frame_len + 2); + uint8_t buffer[2 + 50]; + set_prefix(buffer, frame_len); + memset(buffer + 2, 0xAA, frame_len); + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "isolated frame: should_parse"); + CHECK(d.isolated, "isolated frame: isolated"); + CHECK(d.dlen == frame_len, "isolated frame: dlen == frame_len"); + + size_t post_parse_dlen = stub_parse_shrink_to(20); + dns_frame_apply_rewrite(buffer, &d, post_parse_dlen, &bytes); + + CHECK(bytes == 22, "isolated frame shortened: bytes rewritten to 22"); + CHECK(read_prefix(buffer) == 20, "isolated frame shortened: prefix rewritten to 20"); +} + +/* 2. Isolated complete frame: parser does not shorten -> no rewrite. */ +static void test_isolated_frame_unchanged(void) { + size_t frame_len = 50; + ssize_t bytes = (ssize_t) (frame_len + 2); + uint8_t buffer[2 + 50]; + set_prefix(buffer, frame_len); + memset(buffer + 2, 0xBB, frame_len); + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "isolated frame unchanged: should_parse"); + CHECK(d.isolated, "isolated frame unchanged: isolated"); + CHECK(d.dlen == frame_len, "isolated frame unchanged: dlen == frame_len"); + + size_t post_parse_dlen = stub_parse_unchanged(d.dlen); + dns_frame_apply_rewrite(buffer, &d, post_parse_dlen, &bytes); + + CHECK(bytes == (ssize_t) (frame_len + 2), "isolated frame unchanged: bytes untouched"); + CHECK(read_prefix(buffer) == frame_len, "isolated frame unchanged: prefix untouched"); +} + +/* 3. Coalesced read: frame + extra bytes belonging to (the start of) the + * next frame. Only the first frame's payload is parsed; the read is never + * shortened and the prefix is never rewritten, because that would discard + * the extra bytes still owed to the stream. */ +static void test_coalesced_read(void) { + size_t frame_len = 50; + size_t extra = 30; + ssize_t bytes = (ssize_t) (frame_len + 2 + extra); + uint8_t buffer[2 + 50 + 30]; + set_prefix(buffer, frame_len); + memset(buffer + 2, 0xCC, frame_len); + memset(buffer + 2 + frame_len, 0xDD, extra); + + uint8_t snapshot[sizeof(buffer)]; + memcpy(snapshot, buffer, sizeof(buffer)); + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "coalesced read: should_parse (regression 9c49cc09)"); + CHECK(!d.isolated, "coalesced read: not isolated"); + CHECK(d.dlen == frame_len, "coalesced read: dlen == first frame's payload length only"); + + /* Even if the (simulated) parser wants to shrink the first frame, a + * coalesced read must never be shortened -- that would eat into the + * next frame's bytes. */ + size_t post_parse_dlen = stub_parse_shrink_to(10); + dns_frame_apply_rewrite(buffer, &d, post_parse_dlen, &bytes); + + CHECK(bytes == (ssize_t) (frame_len + 2 + extra), "coalesced read: bytes forwarded unchanged"); + CHECK(memcmp(buffer, snapshot, sizeof(buffer)) == 0, + "coalesced read: buffer contents forwarded unchanged (prefix + extra bytes)"); +} + +/* 4. Split/partial frame: recv() got fewer bytes than the prefix declares. + * Only the available bytes are parsed; never shortened/rewritten. */ +static void test_split_frame(void) { + size_t frame_len = 100; + size_t avail = 50; + ssize_t bytes = (ssize_t) (2 + avail); + uint8_t buffer[2 + 50]; + set_prefix(buffer, frame_len); + memset(buffer + 2, 0xEE, avail); + + uint8_t snapshot[sizeof(buffer)]; + memcpy(snapshot, buffer, sizeof(buffer)); + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "split frame: should_parse (regression 9c49cc09)"); + CHECK(!d.isolated, "split frame: not isolated"); + CHECK(d.dlen == avail, "split frame: dlen capped to bytes actually available"); + + size_t post_parse_dlen = stub_parse_unchanged(d.dlen); + dns_frame_apply_rewrite(buffer, &d, post_parse_dlen, &bytes); + + CHECK(bytes == (ssize_t) (2 + avail), "split frame: bytes untouched"); + CHECK(memcmp(buffer, snapshot, sizeof(buffer)) == 0, "split frame: buffer untouched"); +} + +/* 5. frame_len == 0: skipped entirely, no parse, no rewrite. */ +static void test_zero_frame_len_skipped(void) { + size_t frame_len = 0; + ssize_t bytes = 10; + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(!d.should_parse, "frame_len == 0: skipped (should_parse == 0)"); + CHECK(!d.isolated, "frame_len == 0: not isolated"); + CHECK(d.dlen == 0, "frame_len == 0: dlen == 0"); +} + +/* 6. bytes == 3 minimal edge: exactly one payload byte available. */ +static void test_bytes_equal_three_minimal(void) { + /* Isolated: a 1-byte DNS message, nothing else in the read. */ + { + size_t frame_len = 1; + ssize_t bytes = 3; + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "bytes==3 isolated: should_parse"); + CHECK(d.isolated, "bytes==3 isolated: isolated"); + CHECK(d.dlen == 1, "bytes==3 isolated: dlen == 1"); + } + /* Split: prefix declares more than the single available byte. */ + { + size_t frame_len = 5; + ssize_t bytes = 3; + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "bytes==3 split: should_parse"); + CHECK(!d.isolated, "bytes==3 split: not isolated"); + CHECK(d.dlen == 1, "bytes==3 split: dlen capped to the single available byte"); + } +} + +/* 7. frame_len at the maximum a 2-byte prefix can represent (0xFFFF): + * boundary check for the >>8 / mask arithmetic in the write-back path, and + * for a split read against that maximum. */ +static void test_frame_len_u16_boundary(void) { + size_t frame_len = 0xFFFF; /* 65535: max value a 2-byte prefix can hold */ + + /* Isolated at the boundary, with a shrink on rewrite. */ + { + ssize_t bytes = (ssize_t) (frame_len + 2); + uint8_t *buffer = malloc((size_t) bytes); + CHECK(buffer != NULL, "u16 boundary isolated: allocation"); + if (buffer != NULL) { + set_prefix(buffer, frame_len); + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "u16 boundary isolated: should_parse"); + CHECK(d.isolated, "u16 boundary isolated: isolated"); + CHECK(d.dlen == frame_len, "u16 boundary isolated: dlen == frame_len"); + + size_t post_parse_dlen = stub_parse_shrink_to(65435); /* still fits in 16 bits */ + dns_frame_apply_rewrite(buffer, &d, post_parse_dlen, &bytes); + + CHECK(bytes == (ssize_t) (65435 + 2), "u16 boundary isolated: bytes rewritten"); + CHECK(read_prefix(buffer) == 65435, "u16 boundary isolated: prefix rewritten correctly"); + free(buffer); + } + } + + /* Split: prefix claims the maximum 16-bit length, but recv() only got + * a small buffer's worth (realistic: the TCP read buffer is sized to + * the connection's MSS, far smaller than 65535). */ + { + size_t avail = 200; + ssize_t bytes = (ssize_t) (2 + avail); + uint8_t buffer[2 + 200]; + set_prefix(buffer, frame_len); + memset(buffer + 2, 0x11, avail); + + uint8_t snapshot[sizeof(buffer)]; + memcpy(snapshot, buffer, sizeof(buffer)); + + struct dns_frame_decision d = dns_frame_decide((size_t) bytes, frame_len); + CHECK(d.should_parse, "u16 boundary split: should_parse"); + CHECK(!d.isolated, "u16 boundary split: not isolated"); + CHECK(d.dlen == avail, "u16 boundary split: dlen capped to available bytes"); + + dns_frame_apply_rewrite(buffer, &d, stub_parse_unchanged(d.dlen), &bytes); + CHECK(bytes == (ssize_t) (2 + avail), "u16 boundary split: bytes untouched"); + CHECK(memcmp(buffer, snapshot, sizeof(buffer)) == 0, "u16 boundary split: buffer untouched"); + } +} + +int main(void) { + test_isolated_frame_shortened(); + test_isolated_frame_unchanged(); + test_coalesced_read(); + test_split_frame(); + test_zero_frame_len_skipped(); + test_bytes_equal_three_minimal(); + test_frame_len_u16_boundary(); + + if (failures == 0) { + printf("dns_frame_test: all tests passed\n"); + return 0; + } + + fprintf(stderr, "dns_frame_test: %d assertion(s) failed\n", failures); + return 1; +} diff --git a/wgbridge-rs/Cargo.lock b/wgbridge-rs/Cargo.lock index 0a14745f0..c0b695ae5 100644 --- a/wgbridge-rs/Cargo.lock +++ b/wgbridge-rs/Cargo.lock @@ -912,6 +912,10 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tc-dns" +version = "0.1.0" + [[package]] name = "thiserror" version = "1.0.69" @@ -1114,6 +1118,7 @@ dependencies = [ "jni", "libc", "log", + "tc-dns", "tokio", ] diff --git a/wgbridge-rs/Cargo.toml b/wgbridge-rs/Cargo.toml index aef388f46..e00289515 100644 --- a/wgbridge-rs/Cargo.toml +++ b/wgbridge-rs/Cargo.toml @@ -5,10 +5,15 @@ edition = "2021" license = "GPL-3.0-only" publish = false +[workspace] +members = ["tc-dns"] +resolver = "2" + [lib] crate-type = ["cdylib", "lib"] [dependencies] +tc-dns = { path = "tc-dns", features = ["capi"] } gotatun = { version = "=0.8.1", default-features = false, features = ["ring", "device"] } tokio = { version = "1.43", features = ["rt-multi-thread", "net", "sync", "time", "macros"] } base64 = "0.23" diff --git a/wgbridge-rs/README.md b/wgbridge-rs/README.md index f857fae26..8319cb803 100644 --- a/wgbridge-rs/README.md +++ b/wgbridge-rs/README.md @@ -1,5 +1,11 @@ # wgbridge-rs +This directory is a Cargo workspace with two members: + +- `wgbridge`, the Android WireGuard bridge described below; +- `tc-dns`, the dependency-free DNS message parser and response-policy core + shared by the WireGuard and NetGuard C paths. + A small Rust crate that lets TrackerControl run [gotatun] — Mullvad's WireGuard® implementation, a fork of Cloudflare's BoringTun — inside its own NetGuard-based VpnService. It replaces the earlier Go module that embedded @@ -36,17 +42,20 @@ encrypted side), so we plug in: ## Build -The `wgbridgeBuild` Gradle task runs `cargo ndk` automatically as part of -the Android build, so once prerequisites are in place you don't need to -invoke it directly: +The `wgbridgeBuild` Gradle task runs `cargo ndk` for `libwgbridge.so`, which also +exports the small C ABI used by the NetGuard engine to call `tc-dns`. It runs +automatically as part of an Android native build, so once prerequisites are in +place you don't need to invoke it directly: ```bash ./gradlew assembleGithubDebug # libwgbridge.so is built on demand ``` -The task tracks `src/**`, `Cargo.toml` and `Cargo.lock` as inputs and the -produced per-ABI libraries (`app/build/rustJniLibs//libwgbridge.so`) -as its output, so it's skipped when the Rust source hasn't changed. +The task tracks the workspace sources, manifests, lockfile and pinned Rust +toolchain. Its per-ABI outputs live under `app/build/rustJniLibs/` and are +skipped when inputs have not changed. `libnetguard.so` links this shared library +instead of embedding a second Rust static library, avoiding duplicate Rust +runtime code in the APK. ### Prerequisites @@ -79,11 +88,12 @@ Android 15+ devices with 16KB pages. ### Tests -The protocol-independent parts (UAPI config parsing, DNS answer parsing, +The protocol-independent parts (UAPI config parsing, DNS parsing and policy, key derivation) run on the host: ```bash -cargo test +cargo test --workspace +cargo test -p tc-dns --features capi ``` ## F-Droid build metadata diff --git a/wgbridge-rs/src/callbacks.rs b/wgbridge-rs/src/callbacks.rs index 7b559887f..3d2cf911e 100644 --- a/wgbridge-rs/src/callbacks.rs +++ b/wgbridge-rs/src/callbacks.rs @@ -10,6 +10,10 @@ pub trait SocketProtector: Send + Sync + 'static { /// Receives DNS answers observed on decrypted inbound packets and exposes the /// DNS policy used when the response is sent back to the app. pub trait DnsSink: Send + Sync + 'static { + /// Implementations must not panic. DNS processing runs on the packet + /// path, and the shared `tc-dns` core intentionally does not catch panics; + /// the Android JNI implementation must convert Java failures to its + /// existing `Result` handling before returning here. fn record_dns(&self, qname: &str, aname: &str, resource: &str, ttl: i32); /// Whether a response for `qname` should be returned without answers. diff --git a/wgbridge-rs/src/dns.rs b/wgbridge-rs/src/dns.rs index eb56d0a1e..41fd44c8e 100644 --- a/wgbridge-rs/src/dns.rs +++ b/wgbridge-rs/src/dns.rs @@ -6,17 +6,12 @@ use std::collections::HashMap; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::ops::Range; -use std::panic::{catch_unwind, AssertUnwindSafe}; use std::time::{Duration, Instant}; use crate::callbacks::DnsSink; - -const DNS_TYPE_A: u16 = 1; -const DNS_TYPE_AAAA: u16 = 28; -const DNS_CLASS_IN: u16 = 1; -const DNS_TYPE_SVCB: u16 = 64; -const DNS_TYPE_HTTPS: u16 = 65; -const DNS_HEADER_LEN: usize = 12; +use tcdns::{ + apply_policy, process_response, record_answers as record_dns_answers, DnsPolicy, Outcome, +}; const IP_PROTO_HOP_BY_HOP: u8 = 0; const IP_PROTO_TCP: u8 = 6; @@ -29,6 +24,22 @@ const MAX_TCP_DNS_FLOWS: usize = 64; const MAX_TCP_DNS_BUFFER: usize = u16::MAX as usize + 2; const TCP_DNS_IDLE_TIMEOUT: Duration = Duration::from_secs(60); +struct SinkPolicy<'a>(&'a dyn DnsSink); + +impl DnsPolicy for SinkPolicy<'_> { + fn record_answer(&self, qname: &str, aname: &str, resource: &str, ttl: i32) { + self.0.record_dns(qname, aname, resource, ttl); + } + + fn is_domain_blocked(&self, qname: &str) -> bool { + self.0.is_domain_blocked(qname) + } + + fn blocked_rcode(&self) -> u8 { + self.0.blocked_rcode() + } +} + #[derive(Clone, Debug, Hash, PartialEq, Eq)] struct TcpFlowKey { src: IpAddr, @@ -69,7 +80,7 @@ impl DnsInspector { match proto { IP_PROTO_UDP => { if let Some(msg) = udp_dns_payload(segment) { - record_answers(msg, recorder); + record_dns_answers(msg, &SinkPolicy(recorder)); } } IP_PROTO_TCP => { @@ -85,9 +96,7 @@ impl DnsInspector { tcp: &[u8], recorder: &dyn DnsSink, ) -> Option { - let Some(segment) = tcp_segment(packet, tcp) else { - return None; - }; + let segment = tcp_segment(packet, tcp)?; let mut context = TcpRewriteContext { frames: Vec::new() }; let now = Instant::now(); self.tcp_flows @@ -184,7 +193,7 @@ impl DnsInspector { } let msg = flow.buffer[2..2 + msg_len].to_vec(); flow.buffer.drain(..2 + msg_len); - record_answers(&msg, recorder); + record_dns_answers(&msg, &SinkPolicy(recorder)); } } @@ -269,9 +278,10 @@ pub fn inspect_dns_response(packet: &[u8], recorder: &dyn DnsSink) { impl DnsInspector { /// Inspects a decrypted packet and applies DNS policy before it reaches - /// the TUN. TCP rewriting is deliberately coupled to this inspector's - /// sequence/framing state; callers must not use the stateless UDP helper - /// for TCP segments. + /// the TUN. UDP responses are recorded and rewritten in a single parse. + /// TCP rewriting is deliberately coupled to this inspector's + /// sequence/framing state, so callers must route TCP segments through + /// this method rather than parsing them independently. pub fn inspect_and_rewrite( &mut self, packet: &mut [u8], @@ -282,14 +292,20 @@ impl DnsInspector { return None; }; if view.is_udp { - self.inspect(packet, policy); - return rewrite_dns_response(packet, policy); + let outcome = { + let msg = &mut packet[view.dns_start..view.dns_end]; + process_response(msg, &SinkPolicy(policy)) + }; + let Outcome::Blanked { new_len, .. } = outcome else { + return None; + }; + let new_total = view.dns_start + new_len; + repair_packet(packet, &view, new_total); + return Some(new_total); } let tcp = &packet[view.transport_offset..view.ip_total_len]; - let Some(context) = self.inspect_tcp(packet, tcp, policy) else { - return None; - }; + let context = self.inspect_tcp(packet, tcp, policy)?; let mut rewritten = false; for range in context.frames { let msg_start = view.transport_offset + range.start; @@ -297,15 +313,8 @@ impl DnsInspector { if msg_end > packet.len() { continue; } - let msg = &packet[msg_start..msg_end]; - let Some(layout) = parse_dns_layout(msg) else { - continue; - }; - if layout.contains_svcb || policy_is_domain_blocked(policy, &layout.qname) { - blank_dns_message( - &mut packet[msg_start..msg_end], - policy_blocked_rcode(policy), - ); + let outcome = apply_policy(&mut packet[msg_start..msg_end], &SinkPolicy(policy)); + if matches!(outcome, Outcome::Blanked { .. }) { rewritten = true; } } @@ -318,38 +327,6 @@ impl DnsInspector { } } -/// Applies the native DNS response policy to one decrypted IP packet. -/// -/// The caller must run [`DnsInspector::inspect`] first. That preserves the -/// native ordering where A/AAAA answers are recorded before a response is -/// blanked. UDP responses can be shortened because they are datagrams. TCP -/// packets are never shortened: changing a DNS-over-TCP payload length would -/// require sequence-number translation for every later segment. Instead, a -/// complete UDP response has its counts cleared and is trimmed to the question -/// section. TCP must go through [`DnsInspector::inspect_and_rewrite`], which -/// aligns complete frames to the tracked TCP sequence frontier. -/// -/// Returns the packet length to write when a response was rewritten. `None` -/// means that the packet was not a DNS response or policy left it unchanged. -pub fn rewrite_dns_response(packet: &mut [u8], policy: &dyn DnsSink) -> Option { - let view = dns_packet_view(packet)?; - if !view.is_udp { - return None; - } - - let msg = &packet[view.dns_start..view.dns_end]; - let layout = parse_dns_layout(msg)?; - if !layout.contains_svcb && !policy_is_domain_blocked(policy, &layout.qname) { - return None; - } - - let msg = &mut packet[view.dns_start..view.dns_end]; - blank_dns_message(msg, policy_blocked_rcode(policy)); - let new_total = view.dns_start + layout.question_end; - repair_packet(packet, &view, new_total); - Some(new_total) -} - #[derive(Clone, Copy, Debug)] struct DnsPacketView { ip_version: u8, @@ -503,82 +480,6 @@ fn dns_packet_view(packet: &[u8]) -> Option { } } -#[derive(Debug)] -struct DnsLayout { - qname: String, - question_end: usize, - contains_svcb: bool, -} - -fn parse_dns_layout(msg: &[u8]) -> Option { - if msg.len() < DNS_HEADER_LEN { - return None; - } - let flags = u16::from_be_bytes([msg[2], msg[3]]); - if flags & 0x8000 == 0 || flags & 0x7800 != 0 { - return None; - } - let qdcount = u16::from_be_bytes([msg[4], msg[5]]) as usize; - let ancount = u16::from_be_bytes([msg[6], msg[7]]) as usize; - if qdcount == 0 || ancount == 0 { - return None; - } - - let mut off = DNS_HEADER_LEN; - let mut qname = None; - for q in 0..qdcount { - let (name, next) = read_dns_name(msg, off, 0)?; - if next + 4 > msg.len() { - return None; - } - if q == 0 { - qname = Some(name); - } - off = next + 4; - } - let question_end = off; - let mut contains_svcb = false; - for _ in 0..ancount { - let (_name, next) = read_dns_name(msg, off, 0)?; - if next + 10 > msg.len() { - return None; - } - let typ = u16::from_be_bytes([msg[next], msg[next + 1]]); - let class = u16::from_be_bytes([msg[next + 2], msg[next + 3]]); - let rdlen = u16::from_be_bytes([msg[next + 8], msg[next + 9]]) as usize; - let rdata = next + 10; - if rdata + rdlen > msg.len() { - return None; - } - contains_svcb |= class == DNS_CLASS_IN && (typ == DNS_TYPE_SVCB || typ == DNS_TYPE_HTTPS); - off = rdata + rdlen; - } - Some(DnsLayout { - qname: qname?, - question_end, - contains_svcb, - }) -} - -fn blank_dns_message(msg: &mut [u8], rcode: u8) { - // Keep the ID and question section. The trailing answer bytes are left in - // place for TCP sequence safety; counts make them unreachable to DNS - // parsers. UDP callers trim the datagram at question_end afterwards. - let flags = 0x8000u16 | u16::from(rcode & 0x0f); - msg[2..4].copy_from_slice(&flags.to_be_bytes()); - msg[6..12].fill(0); -} - -fn policy_is_domain_blocked(policy: &dyn DnsSink, qname: &str) -> bool { - catch_unwind(AssertUnwindSafe(|| policy.is_domain_blocked(qname))).unwrap_or(false) -} - -fn policy_blocked_rcode(policy: &dyn DnsSink) -> u8 { - catch_unwind(AssertUnwindSafe(|| policy.blocked_rcode())) - .unwrap_or(3) - .min(15) -} - fn repair_packet(packet: &mut [u8], view: &DnsPacketView, new_total: usize) { let transport_len = new_total.saturating_sub(view.transport_offset); if view.is_udp { @@ -673,16 +574,6 @@ fn internet_checksum(data: &[u8]) -> u16 { !(sum as u16) } -fn record_answers(msg: &[u8], recorder: &dyn DnsSink) { - for rr in parse_dns_answers(msg) { - // The recorder crosses into Java; never let a failure there take - // down the packet path. - let _ = catch_unwind(AssertUnwindSafe(|| { - recorder.record_dns(&rr.qname, &rr.aname, &rr.resource, rr.ttl); - })); - } -} - fn tcp_segment<'a>(packet: &[u8], tcp: &'a [u8]) -> Option> { if tcp.len() < 20 || u16::from_be_bytes([tcp[0], tcp[1]]) != 53 { return None; @@ -834,142 +725,15 @@ fn is_ipv6_ext_header(next: u8) -> bool { next == IP_PROTO_HOP_BY_HOP || next == IP_PROTO_ROUTING || next == IP_PROTO_DST_OPTS } -#[derive(Debug, PartialEq, Eq)] -struct DnsAnswer { - qname: String, - aname: String, - resource: String, - ttl: i32, -} - -fn parse_dns_answers(msg: &[u8]) -> Vec { - let mut answers = Vec::new(); - if msg.len() < 12 { - return answers; - } - let flags = u16::from_be_bytes([msg[2], msg[3]]); - // Must be a response (QR=1) with opcode QUERY. - if flags & 0x8000 == 0 || flags & 0x7800 != 0 { - return answers; - } - - let qdcount = u16::from_be_bytes([msg[4], msg[5]]) as usize; - let ancount = u16::from_be_bytes([msg[6], msg[7]]) as usize; - if qdcount == 0 || ancount == 0 { - return answers; - } - - let mut off = 12usize; - let mut qname = String::new(); - for q in 0..qdcount { - let Some((name, next)) = read_dns_name(msg, off, 0) else { - return answers; - }; - if next + 4 > msg.len() { - return answers; - } - if q == 0 { - qname = name; - } - off = next + 4; - } - if qname.is_empty() { - return answers; - } - - for _ in 0..ancount { - let Some((aname, next)) = read_dns_name(msg, off, 0) else { - return answers; - }; - if next + 10 > msg.len() { - return answers; - } - let typ = u16::from_be_bytes([msg[next], msg[next + 1]]); - let class = u16::from_be_bytes([msg[next + 2], msg[next + 3]]); - let ttl = u32::from_be_bytes([msg[next + 4], msg[next + 5], msg[next + 6], msg[next + 7]]); - let rdlen = u16::from_be_bytes([msg[next + 8], msg[next + 9]]) as usize; - let rdata = next + 10; - if rdata + rdlen > msg.len() { - return answers; - } - - if class == DNS_CLASS_IN { - match typ { - DNS_TYPE_A if rdlen == 4 => { - let ip: [u8; 4] = msg[rdata..rdata + 4].try_into().unwrap(); - answers.push(DnsAnswer { - qname: qname.clone(), - aname, - resource: Ipv4Addr::from(ip).to_string(), - ttl: clamp_ttl(ttl), - }); - } - DNS_TYPE_AAAA if rdlen == 16 => { - let ip: [u8; 16] = msg[rdata..rdata + 16].try_into().unwrap(); - answers.push(DnsAnswer { - qname: qname.clone(), - aname, - resource: Ipv6Addr::from(ip).to_string(), - ttl: clamp_ttl(ttl), - }); - } - _ => {} - } - } - off = rdata + rdlen; - } - answers -} - -/// Reads a possibly-compressed DNS name. Returns the name and the offset of -/// the byte following it. Compression pointers are followed to a depth of 8. -fn read_dns_name(msg: &[u8], mut off: usize, depth: u32) -> Option<(String, usize)> { - if depth > 8 || off >= msg.len() { - return None; - } - let mut labels: Vec = Vec::new(); - loop { - if off >= msg.len() { - return None; - } - let l = msg[off] as usize; - match l & 0xc0 { - 0xc0 => { - if off + 1 >= msg.len() { - return None; - } - let ptr = ((l & 0x3f) << 8) | msg[off + 1] as usize; - let (name, _) = read_dns_name(msg, ptr, depth + 1)?; - if !name.is_empty() { - labels.extend(name.split('.').map(str::to_owned)); - } - return Some((labels.join("."), off + 2)); - } - 0x00 => { - if l == 0 { - return Some((labels.join("."), off + 1)); - } - off += 1; - if l > 63 || off + l > msg.len() { - return None; - } - labels.push(String::from_utf8_lossy(&msg[off..off + l]).into_owned()); - off += l; - } - _ => return None, - } - } -} - -fn clamp_ttl(ttl: u32) -> i32 { - ttl.min(i32::MAX as u32) as i32 -} - #[cfg(test)] mod tests { use super::*; use std::sync::Mutex; + const DNS_TYPE_A: u16 = 1; + const DNS_CLASS_IN: u16 = 1; + const DNS_TYPE_HTTPS: u16 = 65; + struct CollectingSink(Mutex>); impl DnsSink for CollectingSink { @@ -983,14 +747,6 @@ mod tests { } } - struct PanickingSink; - - impl DnsSink for PanickingSink { - fn record_dns(&self, _: &str, _: &str, _: &str, _: i32) { - panic!("recording failed"); - } - } - fn dns_message(parts: &[Vec]) -> Vec { let mut msg = vec![0u8; 12]; msg[0..2].copy_from_slice(&0x1234u16.to_be_bytes()); @@ -1030,6 +786,10 @@ mod tests { out } + fn question_end(name: &str) -> usize { + 12 + dns_name(name).len() + 4 + } + fn ipv4_udp(payload: &[u8]) -> Vec { let udp_len = 8 + payload.len(); let total = 20 + udp_len; @@ -1096,55 +856,6 @@ mod tests { packet } - #[test] - fn parse_dns_answers_records_a_and_aaaa() { - let msg = dns_message(&[ - dns_question("tracker.example", DNS_TYPE_A), - dns_answer_bytes("tracker.example", DNS_TYPE_A, 300, &[203, 0, 113, 7]), - dns_answer_bytes( - "tracker.example", - DNS_TYPE_AAAA, - 60, - &[0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], - ), - ]); - - let answers = parse_dns_answers(&msg); - assert_eq!(answers.len(), 2); - assert_eq!(answers[0].qname, "tracker.example"); - assert_eq!(answers[0].aname, "tracker.example"); - assert_eq!(answers[0].resource, "203.0.113.7"); - assert_eq!(answers[0].ttl, 300); - assert_eq!(answers[1].resource, "2001:db8::1"); - assert_eq!(answers[1].ttl, 60); - } - - #[test] - fn parse_dns_answers_ignores_queries() { - let mut msg = dns_message(&[dns_question("tracker.example", DNS_TYPE_A)]); - msg[2] = 0x01; - msg[3] = 0x00; - assert!(parse_dns_answers(&msg).is_empty()); - } - - #[test] - fn parse_dns_answers_follows_compression_pointers() { - // Question at offset 12; answer name is a pointer back to it. - let mut msg = dns_message(&[dns_question("tracker.example", DNS_TYPE_A)]); - msg[6..8].copy_from_slice(&1u16.to_be_bytes()); // ancount = 1 - msg.extend_from_slice(&[0xc0, 12]); // pointer to qname - msg.extend_from_slice(&DNS_TYPE_A.to_be_bytes()); - msg.extend_from_slice(&DNS_CLASS_IN.to_be_bytes()); - msg.extend_from_slice(&300u32.to_be_bytes()); - msg.extend_from_slice(&4u16.to_be_bytes()); - msg.extend_from_slice(&[203, 0, 113, 7]); - - let answers = parse_dns_answers(&msg); - assert_eq!(answers.len(), 1); - assert_eq!(answers[0].aname, "tracker.example"); - assert_eq!(answers[0].resource, "203.0.113.7"); - } - #[test] fn udp_payload_uses_udp_length() { let msg = dns_message(&[ @@ -1192,10 +903,12 @@ mod tests { let payload = tcp_dns_payload(segment).expect("TCP payload not recognized"); assert_eq!(payload, msg.as_slice()); - let answers = parse_dns_answers(payload); + let sink = CollectingSink(Mutex::new(Vec::new())); + inspect_dns_response(&packet, &sink); + let answers = sink.0.lock().unwrap(); assert_eq!(answers.len(), 1); - assert_eq!(answers[0].qname, "tracker.example"); - assert_eq!(answers[0].resource, "203.0.113.7"); + assert_eq!(answers[0].0, "tracker.example"); + assert_eq!(answers[0].2, "203.0.113.7"); } #[test] @@ -1285,15 +998,6 @@ mod tests { assert!(sink.0.lock().unwrap().is_empty()); } - #[test] - fn recorder_panic_does_not_propagate() { - let msg = dns_message(&[ - dns_question("tracker.example", DNS_TYPE_A), - dns_answer_bytes("tracker.example", DNS_TYPE_A, 300, &[203, 0, 113, 7]), - ]); - inspect_dns_response(&ipv4_udp(&msg), &PanickingSink); - } - #[test] fn inspect_records_through_sink() { let msg = dns_message(&[ @@ -1350,18 +1054,17 @@ mod tests { #[test] fn rewrite_svcb_ipv4_udp_trims_and_repairs_checksums() { let msg = svcb_response(); - let question_end = parse_dns_layout(&msg).unwrap().question_end; + let question_end = question_end("tracker.example"); let mut packet = ipv4_udp(&msg); let sink = CollectingSink(Mutex::new(Vec::new())); let mut inspector = DnsInspector::default(); - // The A record is recorded before the response is blanked. - inspector.inspect(&packet, &sink); - assert_eq!(sink.0.lock().unwrap().len(), 1); // A non-zero incoming checksum exercises the rewrite path. IPv4 UDP // packets with checksum zero deliberately retain zero. packet[26..28].copy_from_slice(&0x1234u16.to_be_bytes()); - let new_len = rewrite_dns_response(&mut packet, &sink).unwrap(); + let new_len = inspector.inspect_and_rewrite(&mut packet, &sink).unwrap(); + // The A record is recorded before the response is blanked. + assert_eq!(sink.0.lock().unwrap().len(), 1); assert_eq!(new_len, 20 + 8 + question_end); packet.truncate(new_len); @@ -1382,11 +1085,12 @@ mod tests { #[test] fn rewrite_svcb_ipv6_udp_updates_payload_and_checksum() { let msg = svcb_response(); - let question_end = parse_dns_layout(&msg).unwrap().question_end; + let question_end = question_end("tracker.example"); let mut packet = ipv6_udp_with_destination_options(&msg); let sink = CollectingSink(Mutex::new(Vec::new())); + let mut inspector = DnsInspector::default(); - let new_len = rewrite_dns_response(&mut packet, &sink).unwrap(); + let new_len = inspector.inspect_and_rewrite(&mut packet, &sink).unwrap(); assert_eq!(new_len, 48 + 8 + question_end); packet.truncate(new_len); assert_eq!( @@ -1404,11 +1108,14 @@ mod tests { dns_question("blocked.example", DNS_TYPE_A), dns_answer_bytes("blocked.example", DNS_TYPE_A, 300, &[203, 0, 113, 8]), ]); - let question_end = parse_dns_layout(&msg).unwrap().question_end; + let question_end = question_end("blocked.example"); let mut packet = ipv4_udp(&msg); packet[26..28].copy_from_slice(&0x1234u16.to_be_bytes()); + let mut inspector = DnsInspector::default(); - let new_len = rewrite_dns_response(&mut packet, &BlockingSink).unwrap(); + let new_len = inspector + .inspect_and_rewrite(&mut packet, &BlockingSink) + .unwrap(); assert_eq!(new_len, 20 + 8 + question_end); packet.truncate(new_len); let (_, segment) = transport_segment(&packet).unwrap(); @@ -1502,8 +1209,9 @@ mod tests { let mut packet = ipv4_udp(&msg); let before = packet.clone(); let sink = CollectingSink(Mutex::new(Vec::new())); + let mut inspector = DnsInspector::default(); - assert_eq!(rewrite_dns_response(&mut packet, &sink), None); + assert_eq!(inspector.inspect_and_rewrite(&mut packet, &sink), None); assert_eq!(packet, before); } @@ -1513,14 +1221,14 @@ mod tests { dns_question("ordinary.example", DNS_TYPE_A), dns_answer_bytes("ordinary.example", DNS_TYPE_HTTPS, 300, &[]), ]); - let layout = parse_dns_layout(&msg).unwrap(); - let (_, answer_name_end) = read_dns_name(&msg, layout.question_end, 0).unwrap(); + let answer_name_end = question_end("ordinary.example") + dns_name("ordinary.example").len(); msg[answer_name_end + 2..answer_name_end + 4].copy_from_slice(&2u16.to_be_bytes()); let mut packet = ipv4_udp(&msg); let before = packet.clone(); let sink = CollectingSink(Mutex::new(Vec::new())); + let mut inspector = DnsInspector::default(); - assert_eq!(rewrite_dns_response(&mut packet, &sink), None); + assert_eq!(inspector.inspect_and_rewrite(&mut packet, &sink), None); assert_eq!(packet, before); } } diff --git a/wgbridge-rs/tc-dns/Cargo.toml b/wgbridge-rs/tc-dns/Cargo.toml new file mode 100644 index 000000000..b0a00d693 --- /dev/null +++ b/wgbridge-rs/tc-dns/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "tc-dns" +version = "0.1.0" +edition = "2021" +license = "GPL-3.0-only" +publish = false + +[lib] +name = "tcdns" +crate-type = ["lib", "staticlib"] + +[features] +default = [] +capi = [] diff --git a/wgbridge-rs/tc-dns/src/capi.rs b/wgbridge-rs/tc-dns/src/capi.rs new file mode 100644 index 000000000..9b221018a --- /dev/null +++ b/wgbridge-rs/tc-dns/src/capi.rs @@ -0,0 +1,152 @@ +//! C ABI for applying the shared DNS response policy from the native packet +//! path. The caller owns the message buffer; callback strings are owned by +//! this library and remain valid only for the duration of their callback. + +use std::ffi::{c_char, c_void, CString}; + +use crate::{process_response, DnsPolicy, Outcome}; + +pub const TCDNS_ABI_VERSION: u32 = 1; +pub const TCDNS_UNCHANGED: usize = usize::MAX; + +pub type RecordAnswer = unsafe extern "C" fn( + ctx: *mut c_void, + qname: *const c_char, + aname: *const c_char, + resource: *const c_char, + ttl: i32, +); +pub type IsDomainBlocked = unsafe extern "C" fn(ctx: *mut c_void, qname: *const c_char) -> i32; +pub type BlockedRcode = unsafe extern "C" fn(ctx: *mut c_void) -> u8; +pub type OnBlanked = + unsafe extern "C" fn(ctx: *mut c_void, qname: *const c_char, qtype: u16, rcode: u8); +pub type Log = unsafe extern "C" fn(ctx: *mut c_void, priority: i32, msg: *const c_char); + +#[repr(C)] +pub struct TcdnsCallbacks { + pub abi_version: u32, + pub record_answer: Option, + pub is_domain_blocked: Option, + pub blocked_rcode: Option, + pub on_blanked: Option, + pub log: Option, +} + +impl TcdnsCallbacks { + fn is_valid(&self) -> bool { + self.abi_version == TCDNS_ABI_VERSION + && self.record_answer.is_some() + && self.is_domain_blocked.is_some() + && self.blocked_rcode.is_some() + && self.on_blanked.is_some() + } +} + +struct CapiPolicy<'a> { + callbacks: &'a TcdnsCallbacks, + ctx: *mut c_void, +} + +impl DnsPolicy for CapiPolicy<'_> { + fn record_answer(&self, qname: &str, aname: &str, resource: &str, ttl: i32) { + let Some(qname) = CString::new(qname).ok() else { + return; + }; + let Some(aname) = CString::new(aname).ok() else { + return; + }; + let Some(resource) = CString::new(resource).ok() else { + return; + }; + if let Some(record_answer) = self.callbacks.record_answer { + // SAFETY: callback validity is checked before processing; all + // CString pointers remain valid for this call only. + unsafe { + record_answer( + self.ctx, + qname.as_ptr(), + aname.as_ptr(), + resource.as_ptr(), + ttl, + ); + } + } + } + + fn is_domain_blocked(&self, qname: &str) -> bool { + let Some(qname) = CString::new(qname).ok() else { + return false; + }; + if let Some(is_domain_blocked) = self.callbacks.is_domain_blocked { + // SAFETY: callback validity is checked before processing and the + // CString pointer is valid for the duration of this call. + unsafe { is_domain_blocked(self.ctx, qname.as_ptr()) != 0 } + } else { + false + } + } + + fn blocked_rcode(&self) -> u8 { + if let Some(blocked_rcode) = self.callbacks.blocked_rcode { + // SAFETY: callback validity is checked before processing. + unsafe { blocked_rcode(self.ctx) } + } else { + 3 + } + } +} + +/// Returns the C ABI version supported by this library. +#[no_mangle] +pub extern "C" fn tcdns_abi_version() -> u32 { + TCDNS_ABI_VERSION +} + +/// Processes one bare DNS message in place. See `tcdns.h` for the complete +/// pointer and callback lifetime contract. +/// +/// # Safety +/// +/// `data` must be writable for `len` bytes when `len` is nonzero, `callbacks` +/// must point to a valid callback table for the duration of the call, and all +/// callbacks must obey the non-reentrancy and no-panic contract. +#[no_mangle] +pub unsafe extern "C" fn tcdns_process_response( + data: *mut u8, + len: usize, + callbacks: *const TcdnsCallbacks, + ctx: *mut c_void, +) -> usize { + let Some(callbacks) = callbacks.as_ref() else { + return TCDNS_UNCHANGED; + }; + if !callbacks.is_valid() || (len != 0 && data.is_null()) { + return TCDNS_UNCHANGED; + } + let message = if len == 0 { + &mut [] + } else { + // SAFETY: the C contract requires `data` to be writable for `len` + // bytes, and null was rejected above. + std::slice::from_raw_parts_mut(data, len) + }; + let policy = CapiPolicy { callbacks, ctx }; + match process_response(message, &policy) { + Outcome::Unchanged => TCDNS_UNCHANGED, + Outcome::Blanked { + new_len, + qname, + qtype, + rcode, + } => { + if let (Some(on_blanked), Some(qname)) = + (callbacks.on_blanked, CString::new(qname).ok()) + { + // SAFETY: callback validity is checked above and the CString + // pointer remains valid for this callback. + on_blanked(ctx, qname.as_ptr(), qtype, rcode); + } + new_len + } + } +} diff --git a/wgbridge-rs/tc-dns/src/lib.rs b/wgbridge-rs/tc-dns/src/lib.rs new file mode 100644 index 000000000..a1139dffe --- /dev/null +++ b/wgbridge-rs/tc-dns/src/lib.rs @@ -0,0 +1,76 @@ +#![cfg_attr( + not(test), + deny( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::indexing_slicing + ) +)] + +mod message; +pub mod policy; + +pub use policy::DnsPolicy; + +/// The result of applying the response policy to a bare DNS message. +#[derive(Debug, PartialEq, Eq)] +pub enum Outcome { + Unchanged, + Blanked { + new_len: usize, + qname: String, + qtype: u16, + rcode: u8, + }, +} + +/// Records valid A and AAAA answers from a DNS response. Malformed input is +/// ignored. A sink must not panic; this function deliberately does not catch +/// panics because the Android release profile uses `panic = "abort"`. +pub fn record_answers(msg: &[u8], policy: &dyn DnsPolicy) { + let _ = message::parse_answers_incrementally(msg, |answer| { + policy.record_answer(&answer.qname, &answer.aname, &answer.resource, answer.ttl); + }); +} + +/// Applies blanking policy to a bare DNS message without recording answers. +pub fn apply_policy(msg: &mut [u8], policy: &dyn DnsPolicy) -> Outcome { + let Some(parsed) = message::parse_message(msg) else { + return Outcome::Unchanged; + }; + apply_parsed(msg, parsed, policy) +} + +/// Records valid answers and applies blanking policy in one parse. Recording +/// happens before any blanking so address mappings survive policy rewrites. +pub fn process_response(msg: &mut [u8], policy: &dyn DnsPolicy) -> Outcome { + let Some(parsed) = message::parse_answers_incrementally(msg, |answer| { + policy.record_answer(&answer.qname, &answer.aname, &answer.resource, answer.ttl); + }) else { + return Outcome::Unchanged; + }; + apply_parsed(msg, parsed, policy) +} + +fn apply_parsed(msg: &mut [u8], parsed: message::DnsMessage, policy: &dyn DnsPolicy) -> Outcome { + // An empty root qname is valid DNS syntax but is not a policy subject. + // In particular, do not call a Java-backed policy with an empty string. + if parsed.qname.is_empty() { + return Outcome::Unchanged; + } + if !parsed.contains_svcb && !policy.is_domain_blocked(&parsed.qname) { + return Outcome::Unchanged; + } + let rcode = policy.blocked_rcode() & 0x0f; + message::blank_dns_message(msg, rcode); + Outcome::Blanked { + new_len: parsed.question_end, + qname: parsed.qname, + qtype: parsed.qtype, + rcode, + } +} + +#[cfg(feature = "capi")] +pub mod capi; diff --git a/wgbridge-rs/tc-dns/src/message.rs b/wgbridge-rs/tc-dns/src/message.rs new file mode 100644 index 000000000..f046d2e18 --- /dev/null +++ b/wgbridge-rs/tc-dns/src/message.rs @@ -0,0 +1,299 @@ +use std::net::{Ipv4Addr, Ipv6Addr}; + +pub(crate) const DNS_TYPE_A: u16 = 1; +pub(crate) const DNS_TYPE_AAAA: u16 = 28; +pub(crate) const DNS_CLASS_IN: u16 = 1; +pub(crate) const DNS_TYPE_SVCB: u16 = 64; +pub(crate) const DNS_TYPE_HTTPS: u16 = 65; +pub(crate) const DNS_HEADER_LEN: usize = 12; +const MAX_NAME_DEPTH: u32 = 8; +const MAX_NAME_LABELS: usize = 128; +const MAX_NAME_OCTETS: usize = 255; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct DnsAnswer { + pub(crate) qname: String, + pub(crate) aname: String, + pub(crate) resource: String, + pub(crate) ttl: i32, +} + +#[derive(Debug)] +pub(crate) struct DnsMessage { + pub(crate) qname: String, + pub(crate) qtype: u16, + pub(crate) question_end: usize, + pub(crate) contains_svcb: bool, +} + +#[derive(Debug)] +struct QuestionLayout { + qname: String, + qtype: u16, + question_end: usize, + ancount: usize, + answer_offset: usize, +} + +fn read_u16(msg: &[u8], offset: usize) -> Option { + let bytes = msg.get(offset..offset.checked_add(2)?)?; + Some(u16::from_be_bytes([*bytes.first()?, *bytes.get(1)?])) +} + +fn read_u32(msg: &[u8], offset: usize) -> Option { + let bytes = msg.get(offset..offset.checked_add(4)?)?; + Some(u32::from_be_bytes([ + *bytes.first()?, + *bytes.get(1)?, + *bytes.get(2)?, + *bytes.get(3)?, + ])) +} + +fn read_question_layout(msg: &[u8]) -> Option { + if msg.len() < DNS_HEADER_LEN { + return None; + } + let flags = read_u16(msg, 2)?; + if flags & 0x8000 == 0 || flags & 0x7800 != 0 { + return None; + } + let qdcount = usize::from(read_u16(msg, 4)?); + let ancount = usize::from(read_u16(msg, 6)?); + if qdcount == 0 || ancount == 0 { + return None; + } + + let mut offset = DNS_HEADER_LEN; + let mut qname = None; + let mut qtype = 0; + for question in 0..qdcount { + let (name, name_end) = read_dns_name(msg, offset, 0)?; + let question_end = name_end.checked_add(4)?; + if question_end > msg.len() { + return None; + } + if question == 0 { + qname = Some(name); + qtype = read_u16(msg, name_end)?; + } + offset = question_end; + } + Some(QuestionLayout { + qname: qname?, + qtype, + question_end: offset, + ancount, + answer_offset: offset, + }) +} + +pub(crate) fn parse_message(msg: &[u8]) -> Option { + let layout = read_question_layout(msg)?; + let mut offset = layout.answer_offset; + let mut contains_svcb = false; + for _ in 0..layout.ancount { + let (answer, answer_end, is_svcb) = parse_answer(msg, offset, &layout.qname)?; + contains_svcb |= is_svcb; + let _ = answer; + offset = answer_end; + } + Some(DnsMessage { + qname: layout.qname, + qtype: layout.qtype, + question_end: layout.question_end, + contains_svcb, + }) +} + +/// Parses the question section and then answers one at a time. A malformed +/// later answer returns `None`, so callers can retain records already emitted +/// from earlier answers while refusing to blank a partially validated message. +pub(crate) fn parse_answers_incrementally( + msg: &[u8], + mut on_answer: impl FnMut(&DnsAnswer), +) -> Option { + let layout = read_question_layout(msg)?; + if layout.qname.is_empty() { + return Some(DnsMessage { + qname: layout.qname, + qtype: layout.qtype, + question_end: layout.question_end, + contains_svcb: false, + }); + } + let mut offset = layout.answer_offset; + let mut contains_svcb = false; + for _ in 0..layout.ancount { + let (answer, answer_end, is_svcb) = parse_answer(msg, offset, &layout.qname)?; + contains_svcb |= is_svcb; + if let Some(answer) = answer { + on_answer(&answer); + } + offset = answer_end; + } + Some(DnsMessage { + qname: layout.qname, + qtype: layout.qtype, + question_end: layout.question_end, + contains_svcb, + }) +} + +fn parse_answer( + msg: &[u8], + offset: usize, + qname: &str, +) -> Option<(Option, usize, bool)> { + let (aname, name_end) = read_dns_name(msg, offset, 0)?; + let fixed_end = name_end.checked_add(10)?; + if fixed_end > msg.len() { + return None; + } + let typ = read_u16(msg, name_end)?; + let class = read_u16(msg, name_end.checked_add(2)?)?; + let ttl = read_u32(msg, name_end.checked_add(4)?)?; + let rdlen = usize::from(read_u16(msg, name_end.checked_add(8)?)?); + let rdata_end = fixed_end.checked_add(rdlen)?; + if rdata_end > msg.len() { + return None; + } + let rdata = msg.get(fixed_end..rdata_end)?; + let is_svcb = class == DNS_CLASS_IN && (typ == DNS_TYPE_SVCB || typ == DNS_TYPE_HTTPS); + let answer = if class != DNS_CLASS_IN { + None + } else { + match typ { + DNS_TYPE_A if rdlen == 4 => Some(DnsAnswer { + qname: qname.to_owned(), + aname, + resource: Ipv4Addr::new( + *rdata.first()?, + *rdata.get(1)?, + *rdata.get(2)?, + *rdata.get(3)?, + ) + .to_string(), + ttl: clamp_ttl(ttl), + }), + DNS_TYPE_AAAA if rdlen == 16 => { + let mut ip = [0u8; 16]; + ip.copy_from_slice(rdata); + Some(DnsAnswer { + qname: qname.to_owned(), + aname, + resource: Ipv6Addr::from(ip).to_string(), + ttl: clamp_ttl(ttl), + }) + } + _ => None, + } + }; + Some((answer, rdata_end, is_svcb)) +} + +fn clamp_ttl(ttl: u32) -> i32 { + ttl.min(i32::MAX as u32) as i32 +} + +/// Reads a possibly compressed DNS name. `next` is the byte after the encoded +/// name in the current message, so a label sequence followed by a pointer +/// resumes at the pointer's end (`off + 2`), not at the pointer target. +pub(crate) fn read_dns_name(msg: &[u8], start: usize, depth: u32) -> Option<(String, usize)> { + let mut state = NameState { + labels: 0, + encoded_octets: 0, + }; + read_dns_name_inner(msg, start, depth, &mut state) +} + +struct NameState { + labels: usize, + encoded_octets: usize, +} + +fn read_dns_name_inner( + msg: &[u8], + start: usize, + depth: u32, + state: &mut NameState, +) -> Option<(String, usize)> { + if depth > MAX_NAME_DEPTH || start >= msg.len() { + return None; + } + let mut labels = Vec::new(); + let mut offset = start; + loop { + let length = usize::from(*msg.get(offset)?); + match length & 0xc0 { + 0xc0 => { + let pointer_end = offset.checked_add(2)?; + if pointer_end > msg.len() { + return None; + } + // The pointer itself contributes nothing to the + // uncompressed-name length: RFC 1035's 255-octet cap + // applies to the expanded name, not the wire encoding, so + // only label octets (below, the 0x00 arm) count toward + // `MAX_NAME_OCTETS`. Pointer-chasing is bounded separately + // by `MAX_NAME_DEPTH` and the bounds check above. + let ptr = ((length & 0x3f) << 8) | usize::from(*msg.get(offset + 1)?); + let (name, _) = read_dns_name_inner(msg, ptr, depth + 1, state)?; + if !name.is_empty() { + labels.extend(name.split('.').map(str::to_owned)); + } + return Some((labels.join("."), pointer_end)); + } + 0x00 => { + state.encoded_octets = state.encoded_octets.checked_add(1)?; + if state.encoded_octets > MAX_NAME_OCTETS { + return None; + } + if length == 0 { + return Some((labels.join("."), offset.checked_add(1)?)); + } + if length > 63 { + return None; + } + let label_start = offset.checked_add(1)?; + let label_end = label_start.checked_add(length)?; + if label_end > msg.len() { + return None; + } + state.encoded_octets = state.encoded_octets.checked_add(length)?; + if state.encoded_octets > MAX_NAME_OCTETS || state.labels >= MAX_NAME_LABELS { + return None; + } + state.labels += 1; + labels.push(decode_label(msg.get(label_start..label_end)?)); + offset = label_end; + } + _ => return None, + } + } +} + +fn decode_label(label: &[u8]) -> String { + String::from_utf8_lossy(label) + .chars() + .map(|character| { + // JNI's NewStringUTF consumes modified UTF-8. Supplementary + // scalar values require surrogate-pair encoding there, so keep + // the C-ABI contract safe by replacing them alongside NUL. + if character == '\0' || character > '\u{ffff}' { + '\u{fffd}' + } else { + character + } + }) + .collect() +} + +pub(crate) fn blank_dns_message(msg: &mut [u8], rcode: u8) { + if let Some(flags) = msg.get_mut(2..4) { + flags.copy_from_slice(&(0x8000u16 | u16::from(rcode & 0x0f)).to_be_bytes()); + } + if let Some(counts) = msg.get_mut(6..12) { + counts.fill(0); + } +} diff --git a/wgbridge-rs/tc-dns/src/policy.rs b/wgbridge-rs/tc-dns/src/policy.rs new file mode 100644 index 000000000..73fff7da9 --- /dev/null +++ b/wgbridge-rs/tc-dns/src/policy.rs @@ -0,0 +1,17 @@ +/// Policy callbacks used by the DNS message parser and rewriter. +pub trait DnsPolicy { + /// Records one address answer before a response is potentially blanked. + fn record_answer(&self, qname: &str, aname: &str, resource: &str, ttl: i32); + + /// Returns whether a response for `qname` should be returned without + /// answers. The default leaves ordinary DNS responses unchanged. + fn is_domain_blocked(&self, _qname: &str) -> bool { + false + } + + /// RCODE used for a response blanked by policy. Only the low four bits + /// are meaningful and are masked by the message layer. + fn blocked_rcode(&self) -> u8 { + 3 + } +} diff --git a/wgbridge-rs/tc-dns/tests/capi.rs b/wgbridge-rs/tc-dns/tests/capi.rs new file mode 100644 index 000000000..8f515188d --- /dev/null +++ b/wgbridge-rs/tc-dns/tests/capi.rs @@ -0,0 +1,238 @@ +#![cfg(feature = "capi")] + +use std::ffi::{c_char, c_void, CStr}; +use std::mem::{offset_of, size_of}; + +use tcdns::capi::{ + tcdns_abi_version, tcdns_process_response, BlockedRcode, IsDomainBlocked, OnBlanked, + RecordAnswer, TcdnsCallbacks, TCDNS_ABI_VERSION, TCDNS_UNCHANGED, +}; + +const TYPE_A: u16 = 1; +const TYPE_HTTPS: u16 = 65; +const CLASS_IN: u16 = 1; + +#[derive(Default)] +struct Capture { + records: Vec<(String, String, String, i32)>, + blanked: Vec<(String, u16, u8)>, + blocked: bool, +} + +unsafe extern "C" fn record_answer( + ctx: *mut c_void, + qname: *const c_char, + aname: *const c_char, + resource: *const c_char, + ttl: i32, +) { + // SAFETY: tests pass a valid Capture pointer and callback-duration C + // strings from the library. + let capture = unsafe { &mut *(ctx.cast::()) }; + let qname = unsafe { CStr::from_ptr(qname) } + .to_string_lossy() + .into_owned(); + let aname = unsafe { CStr::from_ptr(aname) } + .to_string_lossy() + .into_owned(); + let resource = unsafe { CStr::from_ptr(resource) } + .to_string_lossy() + .into_owned(); + capture.records.push((qname, aname, resource, ttl)); +} + +unsafe extern "C" fn is_domain_blocked(ctx: *mut c_void, qname: *const c_char) -> i32 { + // SAFETY: tests pass a valid Capture pointer and callback-duration C string. + let capture = unsafe { &mut *(ctx.cast::()) }; + let qname = unsafe { CStr::from_ptr(qname) }; + assert!(!qname.to_bytes().contains(&0)); + i32::from(capture.blocked) +} + +unsafe extern "C" fn blocked_rcode(_ctx: *mut c_void) -> u8 { + 0x1f +} + +unsafe extern "C" fn on_blanked(ctx: *mut c_void, qname: *const c_char, qtype: u16, rcode: u8) { + // SAFETY: tests pass a valid Capture pointer and callback-duration C string. + let capture = unsafe { &mut *(ctx.cast::()) }; + let qname = unsafe { CStr::from_ptr(qname) } + .to_string_lossy() + .into_owned(); + capture.blanked.push((qname, qtype, rcode)); +} + +fn callbacks() -> TcdnsCallbacks { + TcdnsCallbacks { + abi_version: TCDNS_ABI_VERSION, + record_answer: Some(record_answer as RecordAnswer), + is_domain_blocked: Some(is_domain_blocked as IsDomainBlocked), + blocked_rcode: Some(blocked_rcode as BlockedRcode), + on_blanked: Some(on_blanked as OnBlanked), + log: None, + } +} + +fn question_name(name: &str) -> Vec { + let mut result = Vec::new(); + for label in name.split('.') { + result.push(label.len() as u8); + result.extend_from_slice(label.as_bytes()); + } + result.push(0); + result +} + +fn question(name: &[u8]) -> Vec { + let mut result = name.to_vec(); + result.extend_from_slice(&TYPE_A.to_be_bytes()); + result.extend_from_slice(&CLASS_IN.to_be_bytes()); + result +} + +fn answer(name: &[u8], qtype: u16, rdata: &[u8]) -> Vec { + let mut result = name.to_vec(); + result.extend_from_slice(&qtype.to_be_bytes()); + result.extend_from_slice(&CLASS_IN.to_be_bytes()); + result.extend_from_slice(&300u32.to_be_bytes()); + result.extend_from_slice(&(rdata.len() as u16).to_be_bytes()); + result.extend_from_slice(rdata); + result +} + +fn response(question_name: &[u8], answers: &[Vec]) -> Vec { + let mut result = vec![0u8; 12]; + result[2..4].copy_from_slice(&0x8180u16.to_be_bytes()); + result[4..6].copy_from_slice(&1u16.to_be_bytes()); + result[6..8].copy_from_slice(&(answers.len() as u16).to_be_bytes()); + result.extend_from_slice(&question(question_name)); + for answer in answers { + result.extend_from_slice(answer); + } + result +} + +fn svcb_response() -> Vec { + let name = question_name("tracker.example"); + response( + &name, + &[ + answer(&[0xc0, 12], TYPE_A, &[203, 0, 113, 7]), + answer(&[0xc0, 12], TYPE_HTTPS, &[]), + ], + ) +} + +#[test] +fn abi_layout_and_version_are_stable() { + assert_eq!(tcdns_abi_version(), TCDNS_ABI_VERSION); + assert_eq!(offset_of!(TcdnsCallbacks, abi_version), 0); + assert_eq!(offset_of!(TcdnsCallbacks, record_answer), 8); + assert_eq!(offset_of!(TcdnsCallbacks, is_domain_blocked), 16); + assert_eq!(offset_of!(TcdnsCallbacks, blocked_rcode), 24); + assert_eq!(offset_of!(TcdnsCallbacks, on_blanked), 32); + assert_eq!(offset_of!(TcdnsCallbacks, log), 40); + assert_eq!(size_of::(), 48); +} + +#[test] +fn null_and_invalid_callback_tables_are_unchanged() { + assert_eq!( + unsafe { + tcdns_process_response( + std::ptr::null_mut(), + 0, + std::ptr::null(), + std::ptr::null_mut(), + ) + }, + TCDNS_UNCHANGED + ); + assert_eq!( + unsafe { + tcdns_process_response( + std::ptr::null_mut(), + 1, + std::ptr::null(), + std::ptr::null_mut(), + ) + }, + TCDNS_UNCHANGED + ); + + let mut message = svcb_response(); + let original = message.clone(); + let mut capture = Capture::default(); + let mut invalid = callbacks(); + invalid.abi_version = TCDNS_ABI_VERSION + 1; + assert_eq!( + unsafe { + tcdns_process_response( + message.as_mut_ptr(), + message.len(), + &invalid, + (&mut capture as *mut Capture).cast(), + ) + }, + TCDNS_UNCHANGED + ); + assert_eq!(message, original); + + for missing in 0..4 { + let mut invalid = callbacks(); + match missing { + 0 => invalid.record_answer = None, + 1 => invalid.is_domain_blocked = None, + 2 => invalid.blocked_rcode = None, + _ => invalid.on_blanked = None, + } + let mut message = svcb_response(); + assert_eq!( + unsafe { + tcdns_process_response( + message.as_mut_ptr(), + message.len(), + &invalid, + (&mut capture as *mut Capture).cast(), + ) + }, + TCDNS_UNCHANGED + ); + } +} + +#[test] +fn callbacks_receive_terminated_strings_and_new_length() { + let raw_name = [3u8, 0xff, 0, b'a', 0]; + let mut message = vec![0u8; 12]; + message[2..4].copy_from_slice(&0x8180u16.to_be_bytes()); + message[4..6].copy_from_slice(&1u16.to_be_bytes()); + message[6..8].copy_from_slice(&2u16.to_be_bytes()); + message.extend_from_slice(&question(&raw_name)); + let question_end = message.len(); + message.extend_from_slice(&answer(&[0xc0, 12], TYPE_A, &[203, 0, 113, 7])); + message.extend_from_slice(&answer(&[0xc0, 12], TYPE_HTTPS, &[])); + + let mut capture = Capture { + blocked: false, + ..Capture::default() + }; + let callbacks = callbacks(); + let new_len = unsafe { + tcdns_process_response( + message.as_mut_ptr(), + message.len(), + &callbacks, + (&mut capture as *mut Capture).cast(), + ) + }; + assert_eq!(new_len, question_end); + assert_eq!(capture.records.len(), 1); + assert_eq!(capture.records[0].0, "��a"); + assert_eq!(capture.records[0].1, "��a"); + assert!(capture + .blanked + .iter() + .all(|(name, _, _)| !name.as_bytes().contains(&0))); + assert_eq!(capture.blanked, vec![("��a".to_owned(), TYPE_A, 0x0f)]); +} diff --git a/wgbridge-rs/tc-dns/tests/message.rs b/wgbridge-rs/tc-dns/tests/message.rs new file mode 100644 index 000000000..0241f40ff --- /dev/null +++ b/wgbridge-rs/tc-dns/tests/message.rs @@ -0,0 +1,713 @@ +use std::cell::{Cell, RefCell}; +use std::ffi::CString; + +use tcdns::{process_response, record_answers, DnsPolicy, Outcome}; + +const TYPE_A: u16 = 1; +const TYPE_AAAA: u16 = 28; +const TYPE_CNAME: u16 = 5; +const TYPE_HTTPS: u16 = 65; +const TYPE_RRSIG: u16 = 46; +const TYPE_OPT: u16 = 41; +const CLASS_IN: u16 = 1; + +#[derive(Default)] +struct TestPolicy { + records: RefCell>, + blocked: bool, + blocked_rcode: u8, + policy_calls: Cell, +} + +impl DnsPolicy for TestPolicy { + fn record_answer(&self, qname: &str, aname: &str, resource: &str, ttl: i32) { + self.records.borrow_mut().push(( + qname.to_owned(), + aname.to_owned(), + resource.to_owned(), + ttl, + )); + } + + fn is_domain_blocked(&self, _qname: &str) -> bool { + self.policy_calls.set(self.policy_calls.get() + 1); + self.blocked + } + + fn blocked_rcode(&self) -> u8 { + self.blocked_rcode + } +} + +struct NoopPolicy; + +impl DnsPolicy for NoopPolicy { + fn record_answer(&self, _qname: &str, _aname: &str, _resource: &str, _ttl: i32) {} +} + +fn name(name: &str) -> Vec { + let mut encoded = Vec::new(); + for label in name.split('.') { + encoded.push(label.len() as u8); + encoded.extend_from_slice(label.as_bytes()); + } + encoded.push(0); + encoded +} + +fn question(encoded_name: &[u8], qtype: u16) -> Vec { + let mut result = encoded_name.to_vec(); + result.extend_from_slice(&qtype.to_be_bytes()); + result.extend_from_slice(&CLASS_IN.to_be_bytes()); + result +} + +fn answer(encoded_name: &[u8], qtype: u16, ttl: u32, rdata: &[u8]) -> Vec { + let mut result = encoded_name.to_vec(); + result.extend_from_slice(&qtype.to_be_bytes()); + result.extend_from_slice(&CLASS_IN.to_be_bytes()); + result.extend_from_slice(&ttl.to_be_bytes()); + result.extend_from_slice(&(rdata.len() as u16).to_be_bytes()); + result.extend_from_slice(rdata); + result +} + +fn response(questions: &[Vec], answers: &[Vec]) -> Vec { + let mut result = vec![0u8; 12]; + result[2..4].copy_from_slice(&0x8180u16.to_be_bytes()); + result[4..6].copy_from_slice(&(questions.len() as u16).to_be_bytes()); + result[6..8].copy_from_slice(&(answers.len() as u16).to_be_bytes()); + for question in questions { + result.extend_from_slice(question); + } + for answer in answers { + result.extend_from_slice(answer); + } + result +} + +fn a_answer(encoded_name: &[u8]) -> Vec { + answer(encoded_name, TYPE_A, 300, &[203, 0, 113, 7]) +} + +/// Encodes a compression pointer to `offset` in the message. +fn ptr(offset: usize) -> [u8; 2] { + [0xc0 | ((offset >> 8) as u8), (offset & 0xff) as u8] +} + +#[test] +fn records_a_and_aaaa_answers() { + let qname = name("tracker.example"); + let message = response( + &[question(&qname, TYPE_A)], + &[ + answer(&[0xc0, 12], TYPE_A, 300, &[203, 0, 113, 7]), + answer( + &[0xc0, 12], + TYPE_AAAA, + 60, + &[0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], + ), + ], + ); + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 2); + assert_eq!(records[0].2, "203.0.113.7"); + assert_eq!(records[1].2, "2001:db8::1"); + assert_eq!(records[1].3, 60); +} + +#[test] +fn query_messages_are_ignored() { + let qname = name("tracker.example"); + let mut message = response(&[question(&qname, TYPE_A)], &[a_answer(&[0xc0, 12])]); + message[2..4].copy_from_slice(&0x0100u16.to_be_bytes()); + let policy = TestPolicy::default(); + record_answers(&message, &policy); + assert!(policy.records.borrow().is_empty()); +} + +#[test] +fn labels_then_pointer_resumes_after_pointer_and_records_answer() { + let qname = name("target.example"); + let mut answer_name = vec![3, b'w', b'w', b'w', 0xc0, 12]; + let mut message = response(&[question(&qname, TYPE_A)], &[]); + answer_name.extend_from_slice(&TYPE_A.to_be_bytes()); + answer_name.extend_from_slice(&CLASS_IN.to_be_bytes()); + answer_name.extend_from_slice(&300u32.to_be_bytes()); + answer_name.extend_from_slice(&4u16.to_be_bytes()); + answer_name.extend_from_slice(&[203, 0, 113, 7]); + message[6..8].copy_from_slice(&1u16.to_be_bytes()); + message.extend_from_slice(&answer_name); + + let policy = TestPolicy::default(); + record_answers(&message, &policy); + assert_eq!(policy.records.borrow().len(), 1); + assert_eq!(policy.records.borrow()[0].1, "www.target.example"); +} + +#[test] +fn reserved_name_length_bits_are_rejected() { + for reserved in [0x40u8, 0x80] { + let message = response(&[question(&[reserved, 0], TYPE_A)], &[a_answer(&[0])]); + let policy = TestPolicy::default(); + assert_eq!( + process_response(&mut message.clone(), &policy), + Outcome::Unchanged + ); + assert!(policy.records.borrow().is_empty()); + } +} + +fn answer_with_name(message: &mut Vec, encoded_name: &[u8]) { + let answer = a_answer(encoded_name); + message[6..8].copy_from_slice(&1u16.to_be_bytes()); + message.extend_from_slice(&answer); +} + +#[test] +fn name_depth_label_and_wire_octet_caps_reject_malformed_names() { + let qname = name("valid.example"); + + let mut deep = response(&[question(&qname, TYPE_A)], &[]); + let chain_offset = 12 + question(&qname, TYPE_A).len() + 16; + let mut pointer = vec![0xc0, chain_offset as u8]; + let chain_start = chain_offset; + for index in 0..9usize { + let target = chain_start + (index + 1) * 2; + pointer.extend_from_slice(&[0xc0, target as u8]); + } + pointer.push(0); + answer_with_name(&mut deep, &pointer[..2]); + deep.extend_from_slice(&pointer[2..]); + let policy = TestPolicy::default(); + assert_eq!(process_response(&mut deep, &policy), Outcome::Unchanged); + + let labels = vec![1u8; 130]; + let mut too_many_labels = vec![0u8; 12]; + too_many_labels[2..4].copy_from_slice(&0x8180u16.to_be_bytes()); + too_many_labels[4..6].copy_from_slice(&1u16.to_be_bytes()); + too_many_labels[6..8].copy_from_slice(&1u16.to_be_bytes()); + too_many_labels.extend_from_slice(&labels); + too_many_labels.push(0); + too_many_labels.extend_from_slice(&TYPE_A.to_be_bytes()); + too_many_labels.extend_from_slice(&CLASS_IN.to_be_bytes()); + too_many_labels.extend_from_slice(&a_answer(&[0])); + assert_eq!( + process_response(&mut too_many_labels, &NoopPolicy), + Outcome::Unchanged + ); + + let mut too_long = vec![0u8; 12]; + too_long[2..4].copy_from_slice(&0x8180u16.to_be_bytes()); + too_long[4..6].copy_from_slice(&1u16.to_be_bytes()); + too_long[6..8].copy_from_slice(&1u16.to_be_bytes()); + for _ in 0..4 { + too_long.push(63); + too_long.extend(std::iter::repeat_n(b'x', 63)); + } + too_long.push(0); + too_long.extend_from_slice(&TYPE_A.to_be_bytes()); + too_long.extend_from_slice(&CLASS_IN.to_be_bytes()); + too_long.extend_from_slice(&a_answer(&[0])); + assert_eq!( + process_response(&mut too_long, &NoopPolicy), + Outcome::Unchanged + ); + + let mut looped = response(&[question(&qname, TYPE_A)], &[]); + let offset = 12 + question(&qname, TYPE_A).len(); + answer_with_name(&mut looped, &[0xc0, offset as u8]); + assert_eq!( + process_response(&mut looped, &NoopPolicy), + Outcome::Unchanged + ); +} + +#[test] +fn invalid_utf8_and_nul_are_replaced_before_callbacks() { + let raw = [3, 0xff, 0, b'a', 0]; + let mut message = response(&[question(&raw, TYPE_A)], &[]); + let q_end = 12 + raw.len() + 4; + let pointer = [0xc0, 12]; + message[6..8].copy_from_slice(&1u16.to_be_bytes()); + message.extend_from_slice(&a_answer(&pointer)); + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].0, "��a"); + assert_eq!(records[0].1, "��a"); + assert!(CString::new(records[0].0.as_bytes()).is_ok()); + assert_eq!(q_end, 12 + raw.len() + 4); +} + +#[test] +fn supplementary_unicode_is_replaced_for_modified_utf8_callbacks() { + let raw = [4, 0xf0, 0x9f, 0x98, 0x80, 0]; + let mut message = response(&[question(&raw, TYPE_A)], &[]); + message[6..8].copy_from_slice(&1u16.to_be_bytes()); + message.extend_from_slice(&a_answer(&[0xc0, 12])); + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].0, "�"); + assert_eq!(records[0].1, "�"); +} + +#[test] +fn root_question_never_calls_policy_and_is_unchanged() { + let root = [0u8]; + let mut message = response( + &[question(&root, TYPE_A)], + &[answer(&[0], TYPE_HTTPS, 300, &[])], + ); + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + assert_eq!(process_response(&mut message, &policy), Outcome::Unchanged); + assert_eq!(policy.policy_calls.get(), 0); +} + +#[test] +fn ttl_is_saturated_and_malformed_address_lengths_are_ignored() { + let qname = name("tracker.example"); + let message = response( + &[question(&qname, TYPE_A)], + &[ + answer(&[0xc0, 12], TYPE_A, u32::MAX, &[203, 0, 113, 7]), + answer(&[0xc0, 12], TYPE_A, 300, &[203, 0, 113, 7, 99]), + ], + ); + let policy = TestPolicy::default(); + record_answers(&message, &policy); + assert_eq!(policy.records.borrow().len(), 1); + assert_eq!(policy.records.borrow()[0].3, i32::MAX); + + let malformed_length = response( + &[question(&qname, TYPE_A)], + &[answer(&[0xc0, 12], TYPE_A, 300, &[203, 0, 113, 7, 99])], + ); + let policy = TestPolicy::default(); + record_answers(&malformed_length, &policy); + assert!(policy.records.borrow().is_empty()); +} + +#[test] +fn multiple_questions_are_consumed_before_answers() { + let first = name("first.example"); + let second = name("second.example"); + let mut message = response( + &[question(&first, TYPE_A), question(&second, TYPE_A)], + &[answer(&[0xc0, 12], TYPE_A, 300, &[203, 0, 113, 7])], + ); + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + let outcome = process_response(&mut message, &policy); + assert_eq!(policy.records.borrow().len(), 1); + assert_eq!(policy.records.borrow()[0].0, "first.example"); + assert!( + matches!(outcome, Outcome::Blanked { new_len, qtype: TYPE_A, .. } if new_len == 12 + first.len() + 4 + second.len() + 4) + ); + assert_eq!(&message[6..12], &[0, 0, 0, 0, 0, 0]); +} + +#[test] +fn blocked_rcode_is_masked_to_four_bits() { + let qname = name("blocked.example"); + let mut message = response(&[question(&qname, TYPE_A)], &[a_answer(&[0xc0, 12])]); + let policy = TestPolicy { + blocked: true, + blocked_rcode: 0x1f, + ..TestPolicy::default() + }; + assert!(matches!( + process_response(&mut message, &policy), + Outcome::Blanked { rcode: 0x0f, .. } + )); + assert_eq!(message[3] & 0x0f, 0x0f); +} + +#[test] +fn malformed_answer_after_valid_a_records_a_but_does_not_blank() { + let qname = name("tracker.example"); + let mut malformed = answer(&[0xc0, 12], TYPE_HTTPS, 300, &[]); + let rdlen_offset = malformed.len() - 2; + malformed[rdlen_offset..].copy_from_slice(&100u16.to_be_bytes()); + let mut message = response( + &[question(&qname, TYPE_A)], + &[a_answer(&[0xc0, 12]), malformed], + ); + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + assert_eq!(process_response(&mut message, &policy), Outcome::Unchanged); + assert_eq!(policy.records.borrow().len(), 1); +} + +/// A pointer's own 2 bytes must not be charged against the 255-octet +/// uncompressed-name cap: that cap is RFC 1035's limit on the *expanded* +/// name, and a legal near-maximum name reached through a compression +/// pointer used to be wrongly rejected because the pointer bytes and the +/// fully expanded target shared one budget. +#[test] +fn near_maximum_length_name_reached_via_pointer_is_accepted_and_can_be_blocked() { + // Wire-encoded QNAME of exactly 254 octets: labels 63/63/63/60 plus the + // root byte, i.e. a 249-character domain (comfortably inside the + // 253-character legal limit). + let label_lens = [63usize, 63, 63, 60]; + let mut qname_encoded = Vec::new(); + let mut labels = Vec::new(); + for len in label_lens { + qname_encoded.push(len as u8); + let label = vec![b'a'; len]; + qname_encoded.extend_from_slice(&label); + labels.push(String::from_utf8(label).expect("ascii label")); + } + qname_encoded.push(0); + assert_eq!(qname_encoded.len(), 254); + let expected_qname = labels.join("."); + + let mut message = response( + &[question(&qname_encoded, TYPE_A)], + &[a_answer(&[0xc0, 12])], + ); + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + + assert!(matches!( + process_response(&mut message, &policy), + Outcome::Blanked { .. } + )); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].0, expected_qname); + assert_eq!(records[0].1, expected_qname); + assert_eq!(records[0].2, "203.0.113.7"); +} + +/// A genuinely over-long uncompressed name (more than 255 octets of labels) +/// must still be rejected — proving the cap on expanded-name label octets +/// stays active after the pointer-charge fix. The question name stays +/// short and valid; it is the answer's own (uncompressed) owner name that +/// exceeds the limit, exercising the same `read_dns_name_inner` label +/// accounting the fix touched. +#[test] +fn over_long_uncompressed_answer_owner_name_is_still_rejected() { + let qname = name("q.example"); + + // Four 63-octet labels alone already total 256 octets before the root + // byte, exceeding MAX_NAME_OCTETS regardless of compression. + let mut owner_name = Vec::new(); + for _ in 0..4 { + owner_name.push(63u8); + owner_name.extend(std::iter::repeat_n(b'a', 63)); + } + owner_name.push(0); + assert!(owner_name.len() > 255); + + let mut message = response(&[question(&qname, TYPE_A)], &[a_answer(&owner_name)]); + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + + assert_eq!(process_response(&mut message, &policy), Outcome::Unchanged); + assert!(policy.records.borrow().is_empty()); +} + +/// A CDN-fronted tracker's typical response shape: qname CNAME intermediate, +/// intermediate CNAME target, target A . Each owner name after the +/// question is a compression pointer into the previous record's RDATA, the +/// way real resolvers encode chains. Only the terminal A is recorded, and it +/// carries the *original question* qname (not the intermediate CNAME names) +/// alongside its own owner name as `aname`. +#[test] +fn cname_chain_records_final_a_with_question_qname_and_answer_owner_aname() { + let qname_encoded = name("chain.example"); + let question_bytes = question(&qname_encoded, TYPE_A); + let question_end = 12 + question_bytes.len(); + + // cname1's owner name is a 2-byte pointer to the question's qname. + let mid_encoded = name("mid.chain.example"); + let cname1 = answer(&ptr(12), TYPE_CNAME, 300, &mid_encoded); + // Every record here has a 2-byte pointer owner name, so the fixed + // header (name + type + class + ttl + rdlen) is always 12 bytes before + // RDATA starts. + let mid_rdata_offset = question_end + 12; + let cname1_end = question_end + cname1.len(); + + // cname2's owner name points at "mid.chain.example" inside cname1's own + // RDATA, exactly as a resolver compresses a chain it is building live. + let cdn_encoded = name("cdn.example"); + let cname2 = answer(&ptr(mid_rdata_offset), TYPE_CNAME, 300, &cdn_encoded); + let cdn_rdata_offset = cname1_end + 12; + + // The A record's owner name points at "cdn.example" inside cname2's + // RDATA. + let a_record = answer(&ptr(cdn_rdata_offset), TYPE_A, 300, &[203, 0, 113, 7]); + + let message = response( + std::slice::from_ref(&question_bytes), + &[cname1, cname2, a_record], + ); + + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1, "the two CNAMEs must not be recorded"); + assert_eq!(records[0].0, "chain.example", "qname is the question name"); + assert_eq!( + records[0].1, "cdn.example", + "aname is the A record's own owner" + ); + assert_eq!(records[0].2, "203.0.113.7"); + assert_eq!(records[0].3, 300); + drop(records); + + let mut blanked = message; + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + let outcome = process_response(&mut blanked, &policy); + assert!(matches!( + outcome, + Outcome::Blanked { new_len, qtype: TYPE_A, .. } if new_len == question_end + )); + assert_eq!(&blanked[6..12], &[0, 0, 0, 0, 0, 0]); +} + +/// Same CNAME-chain shape as above, but the chain terminates in an AAAA +/// record instead of A. +#[test] +fn cname_chain_ending_in_aaaa_records_final_answer_with_question_qname() { + let qname_encoded = name("chain6.example"); + let question_bytes = question(&qname_encoded, TYPE_AAAA); + let question_end = 12 + question_bytes.len(); + + let mid_encoded = name("mid.chain6.example"); + let cname1 = answer(&ptr(12), TYPE_CNAME, 300, &mid_encoded); + let mid_rdata_offset = question_end + 12; + let cname1_end = question_end + cname1.len(); + + let cdn_encoded = name("cdn6.example"); + let cname2 = answer(&ptr(mid_rdata_offset), TYPE_CNAME, 300, &cdn_encoded); + let cdn_rdata_offset = cname1_end + 12; + + let ip = [0x20, 0x01, 0x0d, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2]; + let aaaa_record = answer(&ptr(cdn_rdata_offset), TYPE_AAAA, 300, &ip); + + let message = response( + std::slice::from_ref(&question_bytes), + &[cname1, cname2, aaaa_record], + ); + + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1, "the two CNAMEs must not be recorded"); + assert_eq!(records[0].0, "chain6.example"); + assert_eq!(records[0].1, "cdn6.example"); + assert_eq!(records[0].2, "2001:db8::2"); + drop(records); + + let mut blanked = message; + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + let outcome = process_response(&mut blanked, &policy); + assert!(matches!( + outcome, + Outcome::Blanked { new_len, qtype: TYPE_AAAA, .. } if new_len == question_end + )); + assert_eq!(&blanked[6..12], &[0, 0, 0, 0, 0, 0]); +} + +/// The parser only ever walks `ancount` answers; it never inspects nscount +/// or arcount, so an EDNS(0) OPT pseudo-record in the additional section +/// (root owner, TYPE=41, arcount=1) must not disturb answer recording. This +/// documents that additional-section content is simply invisible to the +/// parser, and that blanking's unconditional zeroing of bytes 6..12 (the +/// ancount/nscount/arcount block) wipes arcount too even though it was +/// never validated. +#[test] +fn opt_pseudo_record_in_additional_section_does_not_disturb_parsing_or_blanking() { + let qname_encoded = name("opt.example"); + let question_bytes = question(&qname_encoded, TYPE_A); + let question_end = 12 + question_bytes.len(); + + let mut message = response(std::slice::from_ref(&question_bytes), &[a_answer(&ptr(12))]); + message[10..12].copy_from_slice(&1u16.to_be_bytes()); // arcount = 1 + message.push(0); // OPT owner name: root + message.extend_from_slice(&TYPE_OPT.to_be_bytes()); + message.extend_from_slice(&4096u16.to_be_bytes()); // requestor UDP payload size + message.extend_from_slice(&0u32.to_be_bytes()); // extended RCODE/version/flags + message.extend_from_slice(&0u16.to_be_bytes()); // RDLENGTH = 0 + + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].2, "203.0.113.7"); + drop(records); + + let mut blanked = message; + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + let outcome = process_response(&mut blanked, &policy); + assert!(matches!(outcome, Outcome::Blanked { new_len, .. } if new_len == question_end)); + assert_eq!(&blanked[6..12], &[0, 0, 0, 0, 0, 0]); +} + +/// An RRSIG (type 46) sitting in the answer section alongside a valid A: the +/// parser has no notion of DNSSEC, so RRSIG is simply an unrecognised type +/// whose RDATA is skipped over like any other non-A/AAAA record. +#[test] +fn rrsig_alongside_a_answer_is_skipped_but_a_is_recorded_and_blanking_works() { + let qname_encoded = name("sig.example"); + let question_bytes = question(&qname_encoded, TYPE_A); + let question_end = 12 + question_bytes.len(); + + // Stub RDATA: the parser only reads RDLENGTH and skips the bytes, it + // never interprets RRSIG's internal fields (type covered, algorithm, + // labels, expiration/inception, key tag, signer name, signature). + let rrsig_rdata = vec![0u8; 20]; + let rrsig = answer(&ptr(12), TYPE_RRSIG, 300, &rrsig_rdata); + + let mut message = response( + std::slice::from_ref(&question_bytes), + &[a_answer(&ptr(12)), rrsig], + ); + + let policy = TestPolicy::default(); + record_answers(&message, &policy); + let records = policy.records.borrow(); + assert_eq!(records.len(), 1, "RRSIG must not be recorded as an answer"); + assert_eq!(records[0].2, "203.0.113.7"); + drop(records); + + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + let outcome = process_response(&mut message, &policy); + assert!(matches!(outcome, Outcome::Blanked { new_len, .. } if new_len == question_end)); + assert_eq!(&message[6..12], &[0, 0, 0, 0, 0, 0]); +} + +/// After blanking, the truncated prefix (`msg[..new_len]`) must itself be a +/// self-consistent DNS message: this asserts exactly the header fields the +/// crate writes (`blank_dns_message`) and nothing more. In particular it +/// does not assert RD/RA, which the crate deliberately does not preserve — +/// `blank_dns_message` replaces the whole flags word with `0x8000 | rcode`, +/// so RD/RA/AA/TC/Opcode all read back as zero regardless of the original +/// request's flags. +#[test] +fn blanked_message_is_a_self_consistent_dns_message() { + let qname_encoded = name("blocked.example"); + let question_bytes = question(&qname_encoded, TYPE_A); + let mut message = response(std::slice::from_ref(&question_bytes), &[a_answer(&ptr(12))]); + + let policy = TestPolicy { + blocked: true, + blocked_rcode: 3, + ..TestPolicy::default() + }; + let outcome = process_response(&mut message, &policy); + let (new_len, rcode) = match outcome { + Outcome::Blanked { new_len, rcode, .. } => (new_len, rcode), + Outcome::Unchanged => panic!("expected Outcome::Blanked"), + }; + + // new_len is exactly the header plus the (untouched) question section. + assert_eq!(new_len, 12 + question_bytes.len()); + let truncated = &message[..new_len]; + + // Exact flags word: QR=1, everything else the crate never sets stays 0, + // RCODE is the masked policy rcode. + let flags = u16::from_be_bytes([truncated[2], truncated[3]]); + assert_eq!(flags, 0x8000u16 | u16::from(rcode)); + + // qdcount preserved; ancount/nscount/arcount all zeroed. + assert_eq!(&truncated[4..6], &1u16.to_be_bytes()); + assert_eq!(&truncated[6..12], &[0, 0, 0, 0, 0, 0]); + + // The question section is untouched byte-for-byte. + assert_eq!(&truncated[12..], question_bytes.as_slice()); +} + +/// A UDP-style response with TC (truncation) set whose second answer is cut +/// off mid-RDATA, as happens when a reply exceeds the path MTU. Per the +/// incremental-parse contract, answers validated before the cut are still +/// recorded, and the overall outcome fails open (`Unchanged`, no blanking) +/// rather than acting on a partially-parsed message. The parser does not +/// itself inspect the TC bit; the fail-open behaviour comes entirely from +/// the length check on the truncated second record. +#[test] +fn tc_bit_set_and_truncated_second_answer_fails_open_but_records_first_answer() { + let qname_encoded = name("tc.example"); + let question_bytes = question(&qname_encoded, TYPE_A); + let full = response( + std::slice::from_ref(&question_bytes), + &[ + a_answer(&ptr(12)), + answer(&ptr(12), TYPE_A, 300, &[198, 51, 100, 9]), + ], + ); + // Cut off the last 2 of the second answer's 4 RDATA bytes. + let cut_at = full.len() - 2; + let mut message = full[..cut_at].to_vec(); + // Set TC (bit 1 of the flags' high byte) on top of the standard + // QR|RD|RA flags `response` already set. + message[2] |= 0x02; + + let policy = TestPolicy { + blocked: true, + ..TestPolicy::default() + }; + let outcome = process_response(&mut message, &policy); + assert_eq!(outcome, Outcome::Unchanged); + let records = policy.records.borrow(); + assert_eq!( + records.len(), + 1, + "the answer before the cut is still recorded" + ); + assert_eq!(records[0].2, "203.0.113.7"); +} + +#[test] +fn process_response_fuzz_smoke_returns_for_fifty_thousand_inputs() { + let mut state = 0x9e37_79b9u32; + for index in 0..50_000usize { + state ^= state << 13; + state ^= state >> 17; + state ^= state << 5; + let length = (state as usize ^ index) % 601; + let mut message = vec![0u8; length]; + for byte in &mut message { + state ^= state << 13; + state ^= state >> 17; + state ^= state << 5; + *byte = state as u8; + } + if index % 2 == 0 && message.len() >= 12 { + message[2..4].copy_from_slice(&0x8180u16.to_be_bytes()); + message[4..6].copy_from_slice(&1u16.to_be_bytes()); + } + let _ = process_response(&mut message, &NoopPolicy); + } +}