Merge "Support strict mode private DNS on VPNs that provide Internet." am: a56eab41a6
am: 549439cc92

Change-Id: I28cc679def7cdd4dc5187e65f4524c3998ebcafc
diff --git a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java
index 7bdf396..8e9350d 100644
--- a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java
@@ -520,6 +520,9 @@
         return NetworkMonitorUtils.isValidationRequired(mNetworkCapabilities);
     }
 
+    private boolean isPrivateDnsValidationRequired() {
+        return NetworkMonitorUtils.isPrivateDnsValidationRequired(mNetworkCapabilities);
+    }
 
     private void notifyNetworkTested(int result, @Nullable String redirectUrl) {
         try {
@@ -607,7 +610,7 @@
                     return HANDLED;
                 case CMD_PRIVATE_DNS_SETTINGS_CHANGED: {
                     final PrivateDnsConfig cfg = (PrivateDnsConfig) message.obj;
-                    if (!isValidationRequired() || cfg == null || !cfg.inStrictMode()) {
+                    if (!isPrivateDnsValidationRequired() || cfg == null || !cfg.inStrictMode()) {
                         // No DNS resolution required.
                         //
                         // We don't force any validation in opportunistic mode
@@ -843,9 +846,20 @@
                     //    the network so don't bother validating here.  Furthermore sending HTTP
                     //    packets over the network may be undesirable, for example an extremely
                     //    expensive metered network, or unwanted leaking of the User Agent string.
+                    //
+                    // On networks that need to support private DNS in strict mode (e.g., VPNs, but
+                    // not networks that don't provide Internet access), we still need to perform
+                    // private DNS server resolution.
                     if (!isValidationRequired()) {
-                        validationLog("Network would not satisfy default request, not validating");
-                        transitionTo(mValidatedState);
+                        if (isPrivateDnsValidationRequired()) {
+                            validationLog("Network would not satisfy default request, "
+                                    + "resolving private DNS");
+                            transitionTo(mEvaluatingPrivateDnsState);
+                        } else {
+                            validationLog("Network would not satisfy default request, "
+                                    + "not validating");
+                            transitionTo(mValidatedState);
+                        }
                         return HANDLED;
                     }
                     mEvaluateAttempts++;
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 5f7bcc2..d9b541a 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -40,7 +40,7 @@
 import static android.net.NetworkCapabilities.TRANSPORT_VPN;
 import static android.net.NetworkPolicyManager.RULE_NONE;
 import static android.net.NetworkPolicyManager.uidRulesToString;
-import static android.net.shared.NetworkMonitorUtils.isValidationRequired;
+import static android.net.shared.NetworkMonitorUtils.isPrivateDnsValidationRequired;
 import static android.os.Process.INVALID_UID;
 import static android.system.OsConstants.IPPROTO_TCP;
 import static android.system.OsConstants.IPPROTO_UDP;
@@ -2824,8 +2824,8 @@
         }
     }
 
-    private boolean networkRequiresValidation(NetworkAgentInfo nai) {
-        return isValidationRequired(nai.networkCapabilities);
+    private boolean networkRequiresPrivateDnsValidation(NetworkAgentInfo nai) {
+        return isPrivateDnsValidationRequired(nai.networkCapabilities);
     }
 
     private void handleFreshlyValidatedNetwork(NetworkAgentInfo nai) {
@@ -2843,7 +2843,7 @@
 
         for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
             handlePerNetworkPrivateDnsConfig(nai, cfg);
-            if (networkRequiresValidation(nai)) {
+            if (networkRequiresPrivateDnsValidation(nai)) {
                 handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties));
             }
         }
@@ -2852,7 +2852,7 @@
     private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) {
         // Private DNS only ever applies to networks that might provide
         // Internet access and therefore also require validation.
-        if (!networkRequiresValidation(nai)) return;
+        if (!networkRequiresPrivateDnsValidation(nai)) return;
 
         // Notify the NetworkAgentInfo/NetworkMonitor in case NetworkMonitor needs to cancel or
         // schedule DNS resolutions. If a DNS resolution is required the
diff --git a/services/core/java/com/android/server/connectivity/ConnectivityConstants.java b/services/core/java/com/android/server/connectivity/ConnectivityConstants.java
index 6fa98b8..0fb6fec 100644
--- a/services/core/java/com/android/server/connectivity/ConnectivityConstants.java
+++ b/services/core/java/com/android/server/connectivity/ConnectivityConstants.java
@@ -29,7 +29,7 @@
     //
     // This ensures that a) the explicitly selected network is never trumped by anything else, and
     // b) the explicitly selected network is never torn down.
-    public static final int MAXIMUM_NETWORK_SCORE = 100;
+    public static final int EXPLICITLY_SELECTED_NETWORK_SCORE = 100;
     // VPNs typically have priority over other networks. Give them a score that will
     // let them win every single time.
     public static final int VPN_DEFAULT_SCORE = 101;
diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
index cfa9131..34772d0 100644
--- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java
@@ -483,11 +483,11 @@
         // down an explicitly selected network before the user gets a chance to prefer it when
         // a higher-scoring network (e.g., Ethernet) is available.
         if (networkMisc.explicitlySelected && (networkMisc.acceptUnvalidated || pretendValidated)) {
-            return ConnectivityConstants.MAXIMUM_NETWORK_SCORE;
+            return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE;
         }
 
         int score = currentScore;
-        if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty()) {
+        if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) {
             score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY;
         }
         if (score < 0) score = 0;
diff --git a/services/net/java/android/net/shared/NetworkMonitorUtils.java b/services/net/java/android/net/shared/NetworkMonitorUtils.java
index bb4a603..46e9c73 100644
--- a/services/net/java/android/net/shared/NetworkMonitorUtils.java
+++ b/services/net/java/android/net/shared/NetworkMonitorUtils.java
@@ -43,16 +43,23 @@
             "android.permission.ACCESS_NETWORK_CONDITIONS";
 
     /**
-     * Return whether validation is required for a network.
-     * @param dfltNetCap Default requested network capabilities.
+     * Return whether validation is required for private DNS in strict mode.
      * @param nc Network capabilities of the network to test.
      */
-    public static boolean isValidationRequired(NetworkCapabilities nc) {
+    public static boolean isPrivateDnsValidationRequired(NetworkCapabilities nc) {
         // TODO: Consider requiring validation for DUN networks.
         return nc != null
                 && nc.hasCapability(NET_CAPABILITY_INTERNET)
                 && nc.hasCapability(NET_CAPABILITY_NOT_RESTRICTED)
-                && nc.hasCapability(NET_CAPABILITY_TRUSTED)
-                && nc.hasCapability(NET_CAPABILITY_NOT_VPN);
+                && nc.hasCapability(NET_CAPABILITY_TRUSTED);
+    }
+
+    /**
+     * Return whether validation is required for a network.
+     * @param nc Network capabilities of the network to test.
+     */
+    public static boolean isValidationRequired(NetworkCapabilities nc) {
+        // TODO: Consider requiring validation for DUN networks.
+        return isPrivateDnsValidationRequired(nc) && nc.hasCapability(NET_CAPABILITY_NOT_VPN);
     }
 }
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 0079d5f..b0cc207 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -28,6 +28,7 @@
 import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA;
 import static android.net.ConnectivityManager.TYPE_MOBILE_MMS;
 import static android.net.ConnectivityManager.TYPE_NONE;
+import static android.net.ConnectivityManager.TYPE_VPN;
 import static android.net.ConnectivityManager.TYPE_WIFI;
 import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_INVALID;
 import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY;
@@ -489,7 +490,7 @@
 
         MockNetworkAgent(int transport, LinkProperties linkProperties) {
             final int type = transportToLegacyType(transport);
-            final String typeName = ConnectivityManager.getNetworkTypeName(transport);
+            final String typeName = ConnectivityManager.getNetworkTypeName(type);
             mNetworkInfo = new NetworkInfo(type, 0, typeName, "Mock");
             mNetworkCapabilities = new NetworkCapabilities();
             mNetworkCapabilities.addTransportType(transport);
@@ -619,6 +620,10 @@
             mNetworkAgent.sendNetworkScore(mScore);
         }
 
+        public int getScore() {
+            return mScore;
+        }
+
         public void explicitlySelected(boolean acceptUnvalidated) {
             mNetworkAgent.explicitlySelected(acceptUnvalidated);
         }
@@ -1330,6 +1335,8 @@
                 return TYPE_WIFI;
             case TRANSPORT_CELLULAR:
                 return TYPE_MOBILE;
+            case TRANSPORT_VPN:
+                return TYPE_VPN;
             default:
                 return TYPE_NONE;
         }
@@ -5370,6 +5377,58 @@
     }
 
     @Test
+    public void testVpnUnvalidated() throws Exception {
+        final TestNetworkCallback callback = new TestNetworkCallback();
+        mCm.registerDefaultNetworkCallback(callback);
+
+        // Bring up Ethernet.
+        mEthernetNetworkAgent = new MockNetworkAgent(TRANSPORT_ETHERNET);
+        mEthernetNetworkAgent.connect(true);
+        callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent);
+        callback.assertNoCallback();
+
+        // Bring up a VPN that has the INTERNET capability, initially unvalidated.
+        final int uid = Process.myUid();
+        final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN);
+        final ArraySet<UidRange> ranges = new ArraySet<>();
+        ranges.add(new UidRange(uid, uid));
+        mMockVpn.setNetworkAgent(vpnNetworkAgent);
+        mMockVpn.setUids(ranges);
+        vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */);
+        mMockVpn.connect();
+
+        // Even though the VPN is unvalidated, it becomes the default network for our app.
+        callback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent);
+        // TODO: this looks like a spurious callback.
+        callback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent);
+        callback.assertNoCallback();
+
+        assertTrue(vpnNetworkAgent.getScore() > mEthernetNetworkAgent.getScore());
+        assertEquals(ConnectivityConstants.VPN_DEFAULT_SCORE, vpnNetworkAgent.getScore());
+        assertEquals(vpnNetworkAgent.getNetwork(), mCm.getActiveNetwork());
+
+        NetworkCapabilities nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork());
+        assertFalse(nc.hasCapability(NET_CAPABILITY_VALIDATED));
+        assertTrue(nc.hasCapability(NET_CAPABILITY_INTERNET));
+
+        assertFalse(NetworkMonitorUtils.isValidationRequired(vpnNetworkAgent.mNetworkCapabilities));
+        assertTrue(NetworkMonitorUtils.isPrivateDnsValidationRequired(
+                vpnNetworkAgent.mNetworkCapabilities));
+
+        // Pretend that the VPN network validates.
+        vpnNetworkAgent.setNetworkValid();
+        vpnNetworkAgent.mNetworkMonitor.forceReevaluation(Process.myUid());
+        // Expect to see the validated capability, but no other changes, because the VPN is already
+        // the default network for the app.
+        callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, vpnNetworkAgent);
+        callback.assertNoCallback();
+
+        vpnNetworkAgent.disconnect();
+        callback.expectCallback(CallbackState.LOST, vpnNetworkAgent);
+        callback.expectAvailableCallbacksValidated(mEthernetNetworkAgent);
+    }
+
+    @Test
     public void testVpnSetUnderlyingNetworks() {
         final int uid = Process.myUid();