Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Skip to content
Commit 1011b494 authored by Sreeram Ramachandran's avatar Sreeram Ramachandran
Browse files

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
parent 95684ba1
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment