commit c62368fd7ae1004acd60bc1de06118d8f5d879c9
author Lorenzo Colitti <lorenzo@google.com> Mon Mar 22 02:12:04 2021 +0900
committer Lorenzo Colitti <lorenzo@google.com> Mon Mar 22 02:29:29 2021 +0900
tree 3419b0e1e8b189cbdd3e920b28be6d84f5168d9b
parent 053b2a2737a8611eec64b004996b9ff1598403e8

Fix privileged apps calling registerDefaultNetworkCallback.

When registerDefaultNetworkCallback is called by an app that has
NETWORK_SETTINGS, the UID of the app is forgotten and the request
that is filed has an empty UID set. This results in that request
matching networks that have UID ranges that do not include it,
e.g., VPNs.

Fix this by ensuring that the UID ranges are properly set.

Bug: 165835257
Test: updated specific tests for this bug
Change-Id: I90bf79573342c144d1cfbc2f61a3155fdd5b1fa7

services/core/java/com/android/server/ConnectivityService.java

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 3923063..6fcb3f7 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -6116,10 +6116,15 @@
     private NetworkCapabilities copyDefaultNetworkCapabilitiesForUid(
             @NonNull final NetworkCapabilities netCapToCopy, @NonNull final int requestorUid,
             @NonNull final String requestorPackageName) {
+        // These capabilities are for a TRACK_DEFAULT callback, so:
+        // 1. Remove NET_CAPABILITY_VPN, because it's (currently!) the only difference between
+        //    mDefaultRequest and a per-UID default request.
+        //    TODO: stop depending on the fact that these two unrelated things happen to be the same
+        // 2. Always set the UIDs to mAsUid. restrictRequestUidsForCallerAndSetRequestorInfo will
+        //    not do this in the case of a privileged application.
         final NetworkCapabilities netCap = new NetworkCapabilities(netCapToCopy);
         netCap.removeCapability(NET_CAPABILITY_NOT_VPN);
         netCap.setSingleUid(requestorUid);
-        netCap.setUids(new ArraySet<>());
         restrictRequestUidsForCallerAndSetRequestorInfo(
                 netCap, requestorUid, requestorPackageName);
         return netCap;

tests/net/java/com/android/server/ConnectivityServiceTest.java

diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 2ec8707..ed9a44b 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -7641,8 +7641,7 @@
         assertUidRangesUpdatedForMyUid(true);
         defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn);
         vpnUidCallback.assertNoCallback();  // vpnUidCallback has NOT_VPN capability.
-        // TODO: this is a bug. The VPN does not apply to VPN_UID.
-        vpnUidDefaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn);
+        vpnUidDefaultCallback.assertNoCallback();  // VPN does not apply to VPN_UID
         assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork());
         assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID));
         assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED);
@@ -7654,7 +7653,7 @@
         defaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn);
         defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent);
         vpnUidCallback.assertNoCallback();
-        vpnUidDefaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn);  // BUG!
+        vpnUidDefaultCallback.assertNoCallback();
         assertNull(mCm.getActiveNetwork());
 
         mCm.unregisterNetworkCallback(callback);