Restore DNS policy parity under WireGuard

app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java

@@ -1464,7 +1464,15 @@ private static List<InetAddress> getWireGuardDns(net.kollnig.missioncontrol.wg.W
         return listDns;
     }
 
-    private static boolean hasActiveWireGuardDns(SharedPreferences prefs) {
+    /**
+     * Whether the configured WireGuard egress will own DNS routing.
+     *
+     * The tunnel supplies a protected public fallback when the config has no
+     * usable DNS entry, so Secure DNS must be paused for that case too. The
+     * parser check keeps a malformed or incomplete preference from suppressing
+     * DoH when no tunnel can actually be started.
+     */
+    static boolean hasActiveWireGuard(SharedPreferences prefs) {
         if (!prefs.getBoolean("wg_enabled", false))
             return false;
 
@@ -1473,9 +1481,8 @@ private static boolean hasActiveWireGuardDns(SharedPreferences prefs) {
             return false;
 
         try {
-            net.kollnig.missioncontrol.wg.WgConfig config =
-                    net.kollnig.missioncontrol.wg.WgConfigParser.INSTANCE.parse(wgConfigText);
-            return !getWireGuardDns(config).isEmpty();
+            net.kollnig.missioncontrol.wg.WgConfigParser.INSTANCE.parse(wgConfigText);
+            return true;
         } catch (Throwable ignored) {
             return false;
         }
@@ -1486,8 +1493,8 @@ private void updateDnsProxyState() {
         net.kollnig.missioncontrol.dns.DnsProxyServer proxy =
                 net.kollnig.missioncontrol.dns.DnsProxyServer.getInstance(this);
 
-        if (prefs.getBoolean("doh_enabled", false) && hasActiveWireGuardDns(prefs)) {
-            Log.i(TAG, "Secure DNS proxy disabled while WireGuard DNS is active");
+        if (prefs.getBoolean("doh_enabled", false) && hasActiveWireGuard(prefs)) {
+            Log.i(TAG, "Secure DNS proxy disabled while WireGuard egress is active");
             proxy.stop();
         } else {
             proxy.checkAndUpdateState();
@@ -2149,8 +2156,11 @@ private void prepareForwarding() {
             }
         }
 
-        // Add DoH DNS forwarding when enabled and not superseded by WireGuard DNS.
-        if (prefs.getBoolean("doh_enabled", false) && !hasActiveWireGuardDns(prefs)) {
+        // Add DoH DNS forwarding only when WireGuard is not owning DNS. This
+        // includes configs without a DNS line: getBuilder() installs a
+        // protected public fallback for those configs, and the DoH client is
+        // excluded from this VPN so it cannot be safely chained through WG.
+        if (prefs.getBoolean("doh_enabled", false) && !hasActiveWireGuard(prefs)) {
             Forward dnsFwd = new Forward();
             dnsFwd.protocol = 17; // UDP
             dnsFwd.dport = 53;

app/src/main/java/net/kollnig/missioncontrol/wgbridge/DnsRecorder.java

@@ -1,11 +1,23 @@
 package net.kollnig.missioncontrol.wgbridge;
 
 /**
- * Receives DNS answers observed on decrypted inbound packets. Passive:
- * TrackerControl uses this mapping later when deciding on app connections,
- * but the DNS response is not blocked or rewritten. Called from native
+ * Receives DNS answers observed on decrypted inbound packets and exposes the
+ * DNS policy used when a response is sent back to the app. Called from native
  * tunnel threads.
  */
 public interface DnsRecorder {
     void recordDns(String qname, String aname, String resource, int ttl);
+
+    /**
+     * Returns whether a response for {@code qname} should be returned without
+     * answers. The current app policy is intentionally a no-op.
+     */
+    default boolean isDomainBlocked(String qname) {
+        return false;
+    }
+
+    /** RCODE for a response blanked by the DNS policy (NXDOMAIN by default). */
+    default int blockedRcode() {
+        return 3;
+    }
 }

app/src/main/res/values/strings.xml

@@ -166,7 +166,7 @@
 
     <!-- Secure DNS (DoH) -->
     <string name="setting_doh_enabled">Secure DNS (DoH)</string>
-    <string name="summary_doh_enabled">Encrypt DNS queries using DNS-over-HTTPS. Automatically paused when WireGuard provides DNS, because those queries use the WireGuard tunnel instead.</string>
+    <string name="summary_doh_enabled">Encrypt DNS queries using DNS-over-HTTPS. Automatically paused when WireGuard is active, because those queries use the WireGuard tunnel instead.</string>
     <string name="warning_beta">Beta feature. May not work as expected.</string>
     <string name="setting_doh_endpoint">DoH Endpoint URL</string>
     <string name="summary_doh_endpoint">HTTPS URL for DNS-over-HTTPS queries</string>

app/src/test/java/eu/faircode/netguard/ServiceSinkholeSecureDnsTest.java

@@ -0,0 +1,89 @@
+/*
+ * This file is part of TrackerControl.
+ *
+ * TrackerControl 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.
+ */
+
+package eu.faircode.netguard;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import android.content.SharedPreferences;
+
+import androidx.preference.PreferenceManager;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+
+/**
+ * Secure DNS must not be started alongside the userspace WireGuard egress.
+ * The egress supplies the VPN DNS path, including its public fallback when a
+ * config omits {@code DNS =}; the app process itself is excluded from the VPN.
+ */
+@RunWith(RobolectricTestRunner.class)
+public class ServiceSinkholeSecureDnsTest {
+    private static final String KEY = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
+    private static final String WG_CONFIG =
+            "[Interface]\n" +
+                    "PrivateKey = " + KEY + "\n" +
+                    "Address = 10.64.0.2/32\n" +
+                    "%s" +
+                    "\n[Peer]\n" +
+                    "PublicKey = " + KEY + "\n" +
+                    "AllowedIPs = 0.0.0.0/0\n" +
+                    "Endpoint = 198.51.100.1:51820\n";
+
+    private SharedPreferences prefs;
+
+    @Before
+    public void setUp() {
+        prefs = PreferenceManager.getDefaultSharedPreferences(RuntimeEnvironment.getApplication());
+        prefs.edit().clear().commit();
+    }
+
+    @Test
+    public void wireGuardWithoutDnsStillOwnsDnsPath() {
+        prefs.edit()
+                .putBoolean("wg_enabled", true)
+                .putString("wg_config", String.format(WG_CONFIG, ""))
+                .commit();
+
+        assertTrue(ServiceSinkhole.hasActiveWireGuard(prefs));
+    }
+
+    @Test
+    public void wireGuardWithDnsOwnsDnsPath() {
+        prefs.edit()
+                .putBoolean("wg_enabled", true)
+                .putString("wg_config", String.format(WG_CONFIG, "DNS = 10.64.0.1\n"))
+                .commit();
+
+        assertTrue(ServiceSinkhole.hasActiveWireGuard(prefs));
+    }
+
+    @Test
+    public void disabledOrIncompleteWireGuardDoesNotSuppressDoh() {
+        prefs.edit().putString("wg_config", String.format(WG_CONFIG, "")).commit();
+        assertFalse(ServiceSinkhole.hasActiveWireGuard(prefs));
+
+        prefs.edit().putBoolean("wg_enabled", true).putString("wg_config", "").commit();
+        assertFalse(ServiceSinkhole.hasActiveWireGuard(prefs));
+    }
+
+    @Test
+    public void malformedWireGuardDoesNotSuppressDoh() {
+        prefs.edit()
+                .putBoolean("wg_enabled", true)
+                .putString("wg_config", "not a WireGuard config")
+                .commit();
+
+        assertFalse(ServiceSinkhole.hasActiveWireGuard(prefs));
+    }
+}

wgbridge-rs/README.md

@@ -26,8 +26,9 @@ encrypted side), so we plug in:
 - `SocketpairRecv` — reads outbound raw IP packets from the socketpair fd
   written by `jni/netguard/ip.c` (batched, via tokio's `AsyncFd`);
 - `TunFdSend` — writes decrypted inbound packets to the VpnService TUN fd,
-  running passive DNS inspection (A/AAAA answers feed TrackerControl's
-  tracker mapping) on the way through;
+  recording A/AAAA answers for TrackerControl's mapping and applying its DNS
+  response policy (SVCB/HTTPS blanking and the explicit domain-policy hook) on
+  the way through;
 - `ProtectedUdpFactory` — binds the outer UDP sockets and protects them via
   the Java `Protector` callback. Because gotatun re-invokes the factory on
   every reconfigure, `Tunnel.rebind()` doubles as "move the encrypted
@@ -128,7 +129,11 @@ class Wgbridge {
 
 interface Protector   { boolean protect(int fd); }
 interface Logger      { void verbosef(String s); void errorf(String s); }
-interface DnsRecorder { void recordDns(String qname, String aname, String resource, int ttl); }
+interface DnsRecorder {
+    void recordDns(String qname, String aname, String resource, int ttl);
+    default boolean isDomainBlocked(String qname) { return false; }
+    default int blockedRcode() { return 3; }
+}
 
 class Tunnel {
     void setConfig(String uapiConfig);
@@ -148,11 +153,8 @@ hostnames, and re-resolves them on network changes via `updateEndpoint`).
 
 ## Potential improvements
 
-- **DNS upstream privacy**: app DNS packets to port 53 currently stay on the
-  local NetGuard path so DNS forwarding, tracker lookup, and local resolvers
-  keep working. That is fine for interception, but the upstream resolver path
-  should be revisited when WireGuard is enabled. Ideally, TrackerControl would
-  still intercept app DNS locally while sending DoH/plain-DNS fallback upstream
-  through WireGuard, except for deliberately local-network DNS such as a router
-  or Pi-hole. This is not urgent, but it matters for a complete IP-privacy
-  story because TrackerControl itself is excluded from the VPN route.
+- **Split DNS-over-TCP rewriting**: inbound TCP DNS is recorded with bounded
+  reassembly, but response rewriting is limited to complete frames that begin
+  and end in one sequence-aligned segment. Rewriting a frame split across
+  packets needs buffering plus TCP sequence translation; continuation segments
+  are deliberately forwarded unchanged until that exists.

wgbridge-rs/src/callbacks.rs

@@ -7,11 +7,26 @@ pub trait SocketProtector: Send + Sync + 'static {
     fn protect(&self, fd: i32) -> bool;
 }
 
-/// Receives DNS answers observed on decrypted inbound packets. Passive:
-/// TrackerControl uses this mapping later when deciding on app connections,
-/// but the DNS response is not blocked or rewritten here.
+/// 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 {
     fn record_dns(&self, qname: &str, aname: &str, resource: &str, ttl: i32);
+
+    /// Whether a response for `qname` should be returned without answers.
+    ///
+    /// The current Android callback deliberately returns `false`, matching
+    /// ServiceSinkhole.isDomainBlocked on the master branch. Keeping this as
+    /// an explicit policy hook lets the packet rewriter preserve the native
+    /// path's semantics without inventing a second blocklist in Rust.
+    fn is_domain_blocked(&self, _qname: &str) -> bool {
+        false
+    }
+
+    /// RCODE used for a response blanked by the DNS policy. DNS RCODE is four
+    /// bits; invalid callback values are clamped by the packet rewriter.
+    fn blocked_rcode(&self) -> u8 {
+        3 // NXDOMAIN, matching the native default preference.
+    }
 }
 
 /// Bridge-level log lines destined for the Java side.