• Sreeram Ramachandran's avatar
    Fix fwmark handling for bypassable VPNs and DNS. · 1011b494
    Sreeram Ramachandran authored
    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
    1011b494
DnsProxyListener.cpp 14.4 KB