From 82ed01b49a4cd659982916979a0c1fc00f5efd60 Mon Sep 17 00:00:00 2001 From: liamsmith827 Date: Wed, 4 Mar 2026 02:29:59 +0000 Subject: [PATCH 1/4] fixup! fix DNS leak in VPN lockdown mode when VPN is down When a VPN app is doing DNS with its own tunnel servers, protecting that traffic can result in the tunnel and tunnel DNS server IP addresses leaking. Any VPN app that is setting tunnel DNS servers that aren't reachable through the tunnel is already broken, as apps would be unable to do DNS when leak blocking is enabled. --- server/NetworkController.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp index b3648364..1b0fd5fe 100644 --- a/server/NetworkController.cpp +++ b/server/NetworkController.cpp @@ -242,8 +242,12 @@ uint32_t NetworkController::getNetworkForDnsLocked(unsigned* netId, uid_t uid) c // servers (through the default network). Otherwise, the query is guaranteed to fail. // http://b/29498052 Network *network = getNetworkLocked(*netId); - if (network && network->isVirtual() && !resolv_has_nameservers(*netId)) { - *netId = defaultNetId; + if (network && network->isVirtual()) { + if (!resolv_has_nameservers(*netId)) { + *netId = defaultNetId; + } else { + fwmark.protectedFromVpn = false; + } } } else { // If the user is subject to a VPN and the VPN provides DNS servers, use those servers @@ -253,7 +257,7 @@ uint32_t NetworkController::getNetworkForDnsLocked(unsigned* netId, uid_t uid) c VirtualNetwork* virtualNetwork = getVirtualNetworkForUserLocked(uid); if (virtualNetwork && resolv_has_nameservers(virtualNetwork->getNetId())) { *netId = virtualNetwork->getNetId(); - fwmark.explicitlySelected = true; + fwmark.protectedFromVpn = false; } else { // TODO: return an error instead of silently doing the DNS lookup on the wrong network. // http://b/27560555 From 68d35410379d363e5624c4d2f2bca380b0a3bf32 Mon Sep 17 00:00:00 2001 From: liamsmith827 Date: Sat, 7 Mar 2026 01:21:49 +0000 Subject: [PATCH 2/4] track lockdown VPN uids in NetworkController --- server/NetdNativeService.cpp | 5 +++++ server/NetworkController.cpp | 17 +++++++++++++++++ server/NetworkController.h | 3 +++ 3 files changed, 25 insertions(+) diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp index 957261a4..33c8ddd1 100644 --- a/server/NetdNativeService.cpp +++ b/server/NetdNativeService.cpp @@ -372,6 +372,11 @@ binder::Status NetdNativeService::networkRejectNonSecureVpn( } else { err = RouteController::removeUsersFromRejectNonSecureNetworkRule(uidRanges); } + + if (!err) { + err = gCtls->netCtrl.updateLockdownVpnUids(add, uidRanges); + } + return statusFromErrcode(err); } diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp index 1b0fd5fe..d511ffe2 100644 --- a/server/NetworkController.cpp +++ b/server/NetworkController.cpp @@ -664,6 +664,23 @@ int NetworkController::removeUsersFromNetwork(unsigned netId, const UidRanges& u return network->removeUsers(uidRanges, subPriority); } +int NetworkController::updateLockdownVpnUids(bool add, const UidRanges& uidRanges) { + ScopedWLock lock(mRWLock); + if (add) { + mVpnLockdownUids.add(uidRanges); + // The caller should not send overlapping ranges, and if they do then hasUid won't work + // properly and a leak could occur. + if (mVpnLockdownUids.overlapsSelf()) { + ALOGE("Lockdown VPN uid ranges are overlapping: %s", + mVpnLockdownUids.toString().c_str()); + return -EINVAL; + } + } else { + mVpnLockdownUids.remove(uidRanges); + } + return 0; +} + int NetworkController::addRoute(unsigned netId, const char* interface, const char* destination, const char* nexthop, bool legacy, uid_t uid, int mtu, bool isLocalRoute) { diff --git a/server/NetworkController.h b/server/NetworkController.h index fe72518e..46759b59 100644 --- a/server/NetworkController.h +++ b/server/NetworkController.h @@ -124,6 +124,7 @@ class NetworkController { int32_t subPriority); [[nodiscard]] int removeUsersFromNetwork(unsigned netId, const UidRanges& uidRanges, int32_t subPriority); + [[nodiscard]] int updateLockdownVpnUids(bool add, const UidRanges& uidRanges); // |nexthop| can be NULL (to indicate a directly-connected route), "unreachable" (to indicate a // route that's blocked), "throw" (to indicate the lack of a match), or a regular IP address. @@ -211,6 +212,8 @@ class NetworkController { // we should fix it. // This map is deprecated, if flag connectivityServiceDestroySocket is enabled. std::unordered_map> mAddressToIfindices; + + UidRanges mVpnLockdownUids; }; } // namespace android::net From 83c8d9b3529fea6032f9f008b313fad2dd7e9a3b Mon Sep 17 00:00:00 2001 From: liamsmith827 Date: Sat, 7 Mar 2026 01:24:35 +0000 Subject: [PATCH 3/4] stop using default DNS for lockdown VPN uids --- server/NetworkController.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp index d511ffe2..801e1b6e 100644 --- a/server/NetworkController.cpp +++ b/server/NetworkController.cpp @@ -219,6 +219,14 @@ uint32_t NetworkController::getNetworkForDnsLocked(unsigned* netId, uid_t uid) c Network* appDefaultNetwork = getPhysicalOrUnreachableNetworkForUserLocked(uid); unsigned defaultNetId = appDefaultNetwork ? appDefaultNetwork->getNetId() : mDefaultNetId; + if (!fwmark.protectedFromVpn && mVpnLockdownUids.hasUid(uid)) { + // If a uid is under a lockdown VPN, using the default network DNS servers is a DNS leak. + // + // The servers to be used for a resolution are set in resolv_populate_res_for_net(). There + // is no NetConfig for NETID_UNSET, so no servers will get set. Requests are terminated in + // res_nsend() where -ESRCH is returned due to the lack of servers. + defaultNetId = NETID_UNSET; + } // Common case: there is no VPN that applies to the user, and the query did not specify a netId. // Therefore, it is safe to set the explicit bit on this query and skip all the complex logic @@ -244,6 +252,8 @@ uint32_t NetworkController::getNetworkForDnsLocked(unsigned* netId, uid_t uid) c Network *network = getNetworkLocked(*netId); if (network && network->isVirtual()) { if (!resolv_has_nameservers(*netId)) { + // If the VPN app itself gets here then it has made a mistake. If it wouldn't cause + // compatibility issues, we could save it from leaking. *netId = defaultNetId; } else { fwmark.protectedFromVpn = false; From a0fdafa3abafda13b8249c181cc301e8e7e0266b Mon Sep 17 00:00:00 2001 From: liamsmith827 Date: Sat, 7 Mar 2026 01:28:46 +0000 Subject: [PATCH 4/4] allow checking if DNS blocked by lockdown VPN --- server/NetworkController.cpp | 20 ++++++++++++++++++++ server/NetworkController.h | 1 + server/main.cpp | 5 +++++ 3 files changed, 26 insertions(+) diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp index 801e1b6e..98496207 100644 --- a/server/NetworkController.cpp +++ b/server/NetworkController.cpp @@ -50,6 +50,7 @@ #define DBG 0 #include +#include namespace netflags = android::net::platform::flags; @@ -212,6 +213,25 @@ int NetworkController::setDefaultNetwork(unsigned netId) { return 0; } +// This complements the default DNS server leak solution added in getNetworkForDnsLocked(). This +// gets called much earlier in the DNS request execution. Both solutions do currently suffice on +// their own. +bool NetworkController::checkLockdownVpnBlockingDns(android_net_context* netcontext) const { + ScopedRLock lock(mRWLock); + + uid_t uid = netcontext->uid; + + Fwmark dnsMark; + dnsMark.intValue = netcontext->dns_mark; + + if (dnsMark.protectedFromVpn || !mVpnLockdownUids.hasUid(uid)) { + return false; + } + + VirtualNetwork* userVirtualNetwork = getVirtualNetworkForUserLocked(uid); + return userVirtualNetwork == nullptr || userVirtualNetwork->getNetId() != netcontext->dns_netid; +} + uint32_t NetworkController::getNetworkForDnsLocked(unsigned* netId, uid_t uid) const { Fwmark fwmark; fwmark.protectedFromVpn = canProtectLocked(uid, *netId); diff --git a/server/NetworkController.h b/server/NetworkController.h index 46759b59..89558abc 100644 --- a/server/NetworkController.h +++ b/server/NetworkController.h @@ -101,6 +101,7 @@ class NetworkController { unsigned getNetworkForUser(uid_t uid) const; unsigned getNetworkForConnect(uid_t uid) const; void getNetworkContext(unsigned netId, uid_t uid, struct android_net_context* netcontext) const; + bool checkLockdownVpnBlockingDns(android_net_context* netcontext) const; unsigned getNetworkForInterface(const char* interface) const; unsigned getNetworkForInterface(const int ifIndex) const; bool isVirtualNetwork(unsigned netId) const; diff --git a/server/main.cpp b/server/main.cpp index d27fd76a..6c7d181c 100644 --- a/server/main.cpp +++ b/server/main.cpp @@ -107,6 +107,10 @@ bool evaluateDomainNameCallback(const android_net_context& netcontext, const cha return true; } +bool checkLockdownVpnBlockingDnsCallback(android_net_context* netcontext) { + return gCtls->netCtrl.checkLockdownVpnBlockingDns(netcontext); +} + bool initDnsResolver() { ResolverNetdCallbacks callbacks = { .check_calling_permission = &checkCallingPermissionCallback, @@ -114,6 +118,7 @@ bool initDnsResolver() { .log = &logCallback, .tagSocket = &tagSocketCallback, .evaluate_domain_name = &evaluateDomainNameCallback, + .check_lockdown_vpn_blocking_dns = &checkLockdownVpnBlockingDnsCallback, }; return resolv_init(&callbacks); }