Fix multiple DNS leaks

framework/api/module-lib-current.txt

@@ -32,6 +32,7 @@ package android.net {
     method @Deprecated @RequiresPermission(android.Manifest.permission.NETWORK_STACK) public void setProfileNetworkPreference(@NonNull android.os.UserHandle, int, @Nullable java.util.concurrent.Executor, @Nullable Runnable);
     method @RequiresPermission(android.Manifest.permission.NETWORK_STACK) public void setProfileNetworkPreferences(@NonNull android.os.UserHandle, @NonNull java.util.List<android.net.ProfileNetworkPreference>, @Nullable java.util.concurrent.Executor, @Nullable Runnable);
     method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void setRequireVpnForUids(boolean, @NonNull java.util.Collection<android.util.Range<java.lang.Integer>>);
+    method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void setRequireVpnForUids2(boolean, @NonNull java.util.Collection<android.util.Range<java.lang.Integer>>, @NonNull java.util.Collection<android.util.Range<java.lang.Integer>>);
     method @RequiresPermission(anyOf={android.Manifest.permission.NETWORK_SETTINGS, android.Manifest.permission.NETWORK_STACK, android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK}) public void setUidFirewallRule(int, int, int);
     method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_STACK, android.Manifest.permission.NETWORK_SETTINGS}) public void setVpnDefaultForUids(@NonNull String, @NonNull java.util.Collection<android.util.Range<java.lang.Integer>>);
     method @RequiresPermission(anyOf={android.Manifest.permission.MANAGE_TEST_NETWORKS, android.Manifest.permission.NETWORK_STACK}) public void simulateDataStall(int, long, @NonNull android.net.Network, @NonNull android.os.PersistableBundle);

framework/src/android/net/ConnectivityManager.java

@@ -1594,6 +1594,22 @@ private static UidRange[] getUidRangeArray(@NonNull Collection<Range<Integer>> r
         return rangesArray;
     }
 
+    /**
+     * @hide
+     */
+    @RequiresPermission(anyOf = {
+            NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
+            android.Manifest.permission.NETWORK_STACK,
+            android.Manifest.permission.NETWORK_SETTINGS})
+    @SystemApi(client = MODULE_LIBRARIES)
+    public void setRequireVpnForUids(boolean requireVpn,
+            @NonNull Collection<Range<Integer>> ranges) {
+        // All existing known calls to this have been removed. Any new or unknown calls will have
+        // ranges set to a value that can result in leaks, so it's an illegal argument.
+        throw new IllegalArgumentException(
+                "ranges is not strict, call setRequireVpnForUids2 instead");
+    }
+
     /**
      * Adds or removes a requirement for given UID ranges to use the VPN.
      *
@@ -1622,6 +1638,8 @@ private static UidRange[] getUidRangeArray(@NonNull Collection<Range<Integer>> r
      * This method should be called only by the VPN code.
      *
      * @param ranges the UID ranges to restrict
+     * @param strictRanges the UID ranges to restrict, which include the VPN app itself (when not
+     *                     using the legacy VPN)
      * @param requireVpn whether the specified UID ranges must use a VPN
      *
      * @hide
@@ -1631,16 +1649,19 @@ private static UidRange[] getUidRangeArray(@NonNull Collection<Range<Integer>> r
             android.Manifest.permission.NETWORK_STACK,
             android.Manifest.permission.NETWORK_SETTINGS})
     @SystemApi(client = MODULE_LIBRARIES)
-    public void setRequireVpnForUids(boolean requireVpn,
-            @NonNull Collection<Range<Integer>> ranges) {
+    public void setRequireVpnForUids2(boolean requireVpn,
+            @NonNull Collection<Range<Integer>> ranges,
+            @NonNull Collection<Range<Integer>> strictRanges) {
         Objects.requireNonNull(ranges);
+        Objects.requireNonNull(strictRanges);
         // The Range class is not parcelable. Convert to UidRange, which is what is used internally.
         // This method is not necessarily expected to be used outside the system server, so
         // parceling may not be necessary, but it could be used out-of-process, e.g., by the network
         // stack process, or by tests.
         final UidRange[] rangesArray = getUidRangeArray(ranges);
+        final UidRange[] strictRangesArray = getUidRangeArray(strictRanges);
         try {
-            mService.setRequireVpnForUids(requireVpn, rangesArray);
+            mService.setRequireVpnForUids(requireVpn, rangesArray, strictRangesArray);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         }

framework/src/android/net/IConnectivityManager.aidl

@@ -133,7 +133,7 @@ interface IConnectivityManager
 
     ProxyInfo getProxyForNetwork(in Network nework);
 
-    void setRequireVpnForUids(boolean requireVpn, in UidRange[] ranges);
+    void setRequireVpnForUids(boolean requireVpn, in UidRange[] ranges, in UidRange[] strictRanges);
     void setLegacyLockdownVpnEnabled(boolean enabled);
 
     void setProvisioningNotificationVisible(boolean visible, int networkType, in String action);

service/src/com/android/server/ConnectivityService.java

@@ -7455,9 +7455,12 @@ public void handleMessage(Message msg) {
                 case EVENT_BLOCKED_REASONS_CHANGED:
                     handleBlockedReasonsChanged((List) msg.obj);
                     break;
-                case EVENT_SET_REQUIRE_VPN_FOR_UIDS:
-                    handleSetRequireVpnForUids(toBool(msg.arg1), (UidRange[]) msg.obj);
+                case EVENT_SET_REQUIRE_VPN_FOR_UIDS: {
+                    final Pair<UidRange[], UidRange[]> arg =
+                            (Pair<UidRange[], UidRange[]>) msg.obj;
+                    handleSetRequireVpnForUids(toBool(msg.arg1), arg.first, arg.second);
                     break;
+                }
                 case EVENT_SET_OEM_NETWORK_PREFERENCE: {
                     final Pair<OemNetworkPreferences, IOnCompleteListener> arg =
                             (Pair<OemNetworkPreferences, IOnCompleteListener>) msg.obj;
@@ -8047,45 +8050,56 @@ private boolean isUidBlockedByVpn(int uid, List<UidRange> blockedUidRanges) {
     }
 
     @Override
-    public void setRequireVpnForUids(boolean requireVpn, UidRange[] ranges) {
+    public void setRequireVpnForUids(boolean requireVpn, UidRange[] ranges,
+            UidRange[] strictRanges) {
         enforceNetworkStackOrSettingsPermission();
         mHandler.sendMessage(mHandler.obtainMessage(EVENT_SET_REQUIRE_VPN_FOR_UIDS,
-                encodeBool(requireVpn), 0 /* arg2 */, ranges));
+                encodeBool(requireVpn), 0 /* arg2 */, new Pair<>(ranges, strictRanges)));
     }
 
-    private void handleSetRequireVpnForUids(boolean requireVpn, UidRange[] ranges) {
+    private void handleSetRequireVpnForUids(boolean requireVpn, UidRange[] ranges,
+            UidRange[] strictRanges) {
         if (DBG) {
             Log.d(TAG, "Setting VPN " + (requireVpn ? "" : "not ") + "required for UIDs: "
-                    + Arrays.toString(ranges));
+                    + Arrays.toString(ranges) + ", and strict UIDs: " + Arrays.toString(
+                            strictRanges));
         }
-        // Cannot use a Set since the list of UID ranges might contain duplicates.
-        final List<UidRange> newVpnBlockedUidRanges = new ArrayList(mVpnBlockedUidRanges);
-        for (int i = 0; i < ranges.length; i++) {
-            if (requireVpn) {
-                newVpnBlockedUidRanges.add(ranges[i]);
-            } else {
-                newVpnBlockedUidRanges.remove(ranges[i]);
+
+        // Keep calling conditions consistent with AOSP. This was originally checked by caller.
+        if (strictRanges.length > 0) {
+            try {
+                mNetd.networkRejectNonSecureVpn(requireVpn, toUidRangeStableParcels(strictRanges));
+            } catch (RemoteException | ServiceSpecificException e) {
+                Log.e(TAG, "setRequireVpnForUids(" + requireVpn + ", "
+                        + Arrays.toString(strictRanges) + "): netd command failed: " + e);
+                throw new IllegalStateException("lockdown VPN firewall in invalid state");
             }
         }
 
-        try {
-            mNetd.networkRejectNonSecureVpn(requireVpn, toUidRangeStableParcels(ranges));
-        } catch (RemoteException | ServiceSpecificException e) {
-            Log.e(TAG, "setRequireVpnForUids(" + requireVpn + ", "
-                    + Arrays.toString(ranges) + "): netd command failed: " + e);
-        }
+        // Keep calling conditions consistent with AOSP. This was originally checked by caller.
+        if (ranges.length > 0) {
+            // Cannot use a Set since the list of UID ranges might contain duplicates.
+            final List<UidRange> newVpnBlockedUidRanges = new ArrayList(mVpnBlockedUidRanges);
+            for (int i = 0; i < ranges.length; i++) {
+                if (requireVpn) {
+                    newVpnBlockedUidRanges.add(ranges[i]);
+                } else {
+                    newVpnBlockedUidRanges.remove(ranges[i]);
+                }
+            }
 
-        if (mDeps.isAtLeastT()) {
-            mPermissionMonitor.updateVpnLockdownUidRanges(requireVpn, ranges);
-        }
+            if (mDeps.isAtLeastT()) {
+                mPermissionMonitor.updateVpnLockdownUidRanges(requireVpn, ranges);
+            }
 
-        forEachNetworkAgentInfo(nai -> {
-            final boolean curMetered = nai.networkCapabilities.isMetered();
-            maybeNotifyNetworkBlocked(nai, curMetered, curMetered,
-                    mVpnBlockedUidRanges, newVpnBlockedUidRanges);
-        });
+            forEachNetworkAgentInfo(nai -> {
+                final boolean curMetered = nai.networkCapabilities.isMetered();
+                maybeNotifyNetworkBlocked(nai, curMetered, curMetered,
+                        mVpnBlockedUidRanges, newVpnBlockedUidRanges);
+            });
 
-        mVpnBlockedUidRanges = newVpnBlockedUidRanges;
+            mVpnBlockedUidRanges = newVpnBlockedUidRanges;
+        }
     }
 
     @Override

service/src/com/android/server/NetIdManager.java

@@ -68,7 +68,11 @@ private int getNextAvailableNetIdLocked(
             int lastId, @NonNull SparseBooleanArray netIdInUse) {
         int netId = lastId;
         for (int i = MIN_NET_ID; i <= mMaxNetId; i++) {
-            netId = netId < mMaxNetId ? netId + 1 : MIN_NET_ID;
+            if (netId < mMaxNetId) {
+                netId += 1;
+            } else {
+                break;
+            }
             if (!netIdInUse.get(netId)) {
                 return netId;
             }

tests/cts/hostside/app/src/com/android/cts/net/hostside/VpnTest.java

@@ -1876,7 +1876,7 @@ public void testBlockIncomingPackets() throws Exception {
 
             // Lockdown uid that has the remote UDP socket
             runWithShellPermissionIdentity(() -> {
-                mCM.setRequireVpnForUids(true /* requireVpn */, lockdownRange);
+                mCM.setRequireVpnForUids2(true /* requireVpn */, lockdownRange, lockdownRange);
             }, NETWORK_SETTINGS);
 
             // setRequireVpnForUids setup a lockdown rule asynchronously. So it needs to wait for
@@ -1907,7 +1907,7 @@ public void testBlockIncomingPackets() throws Exception {
                 Os.close(remoteUdpFd);
             }, /* cleanup */ () -> {
                 runWithShellPermissionIdentity(() -> {
-                    mCM.setRequireVpnForUids(false /* requireVpn */, lockdownRange);
+                    mCM.setRequireVpnForUids2(false /* requireVpn */, lockdownRange, lockdownRange);
                 }, NETWORK_SETTINGS);
             });
     }