Fix fwmark handling for bypassable VPNs and DNS.
This is a significant change to the way fwmarks are handled for two purposes:
1. Bypassable VPN.
This was introduced in http://ag/510058 and had an issue that if there's a
default network, it would always be used in connect(), so the bypassable VPN
wouldn't get any traffic. This CL fixes that issue by using the bypassable
VPN's NetId in connect(). See the comments in the code for more details.
2. DNS.
The previous DNS code (specifically, getNetworkForUser()) had two problems:
+ Even if a user asks for a NetId they have permission for, we'd always use
the user's VPN if they were subject to one. So, for example, a system IMS
app that brings up the mobile network in the presence of a VPN would still
have its DNS queries sent over the VPN, instead of mobile as desired.
+ Any user could perform DNS over any valid network, even one they didn't
have permissions for, as long as they weren't subject to a VPN. So, for
example, an app could use the DNS servers of a different profile's VPN.
This CL fixes those problems. See getNetworkForDns() for more details.
The two pieces above are inter-related. Previously, we never set the explicit
bit from the DNS code. But we need to do that if the user asks for a network
explicitly, for two reasons:
o So that the DNS query is really restricted to that network and doesn't
fallthrough to the default network.
o So that the heuristic described in ON_CONNECT works in all cases. I.e., if the
DNS proxy's connect() request comes in with the explicit bit NOT set, we know
that the NetId can only be either the default network or a VPN.
This CL is not intended to be robust against race conditions. In general, very
little of the netd code is resilient. A separate effort needs to be undertaken
to carefully audit all the code and logic to guard against things like:
* A VPN being established between calls to getNetworkForDns() and connect().
* State changes between multiple calls to NetworkController from clients such as
FwmarkServer and DnsProxyListener.
* Routing rules / iptables rules being set up in a less-than-ideal order.
* ... etc.
Bug: 15347374
Change-Id: I5baad9168c4f4f3ef4129e07234b4bf24b0d8ba2
diff --git a/server/NetworkController.h b/server/NetworkController.h
index f065ba5..59497fa 100644
--- a/server/NetworkController.h
+++ b/server/NetworkController.h
@@ -47,11 +47,12 @@
unsigned getDefaultNetwork() const;
int setDefaultNetwork(unsigned netId) WARN_UNUSED_RESULT;
- // Order of preference: UID-specific, requestedNetId, default.
- // Specify NETID_UNSET for requestedNetId if the default network is preferred.
- // forDns indicates if we're querying the netId for a DNS request. This avoids sending DNS
- // requests to VPNs without DNS servers.
- unsigned getNetworkForUser(uid_t uid, unsigned requestedNetId, bool forDns) const;
+ // Sets |*netId| to an appropriate NetId to use for DNS for the given user. Call with |*netId|
+ // set to a non-NETID_UNSET value if the user already has indicated a preference. Returns the
+ // fwmark value to set on the socket when performing the DNS request.
+ uint32_t getNetworkForDns(unsigned* netId, uid_t uid) const;
+ unsigned getNetworkForUser(uid_t uid) const;
+ unsigned getNetworkForConnect(uid_t uid) const;
unsigned getNetworkForInterface(const char* interface) const;
bool isVirtualNetwork(unsigned netId) const;
@@ -87,6 +88,7 @@
Network* getNetworkLocked(unsigned netId) const;
VirtualNetwork* getVirtualNetworkForUserLocked(uid_t uid) const;
Permission getPermissionForUserLocked(uid_t uid) const;
+ bool canUserSelectNetworkLocked(uid_t uid, unsigned netId) const;
int modifyRoute(unsigned netId, const char* interface, const char* destination,
const char* nexthop, bool add, bool legacy, uid_t uid) WARN_UNUSED_RESULT;