diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java old mode 100644 new mode 100755 index 32e06e5ec6..99118accba --- a/service/src/com/android/server/connectivity/PermissionMonitor.java +++ b/service/src/com/android/server/connectivity/PermissionMonitor.java @@ -230,11 +230,11 @@ public class PermissionMonitor { boolean hasRestrictedPermission = hasRestrictedNetworkPermission(app); if (isNetwork || hasRestrictedPermission) { - Boolean permission = mApps.get(uid); + Boolean permission = mApps.get(UserHandle.getAppId(uid)); // If multiple packages share a UID (cf: android:sharedUserId) and ask for different // permissions, don't downgrade (i.e., if it's already SYSTEM, leave it as is). if (permission == null || permission == NETWORK) { - mApps.put(uid, hasRestrictedPermission); + mApps.put(UserHandle.getAppId(uid), hasRestrictedPermission); } } @@ -325,14 +325,14 @@ public class PermissionMonitor { // networks. mApps contains the result of checks for both hasNetworkPermission and // hasRestrictedNetworkPermission. If uid is in the mApps list that means uid has one of // permissions at least. - return mApps.containsKey(uid); + return mApps.containsKey(UserHandle.getAppId(uid)); } /** * Returns whether the given uid has permission to use restricted networks. */ public synchronized boolean hasRestrictedNetworksPermission(int uid) { - return Boolean.TRUE.equals(mApps.get(uid)); + return Boolean.TRUE.equals(mApps.get(UserHandle.getAppId(uid))); } private void update(Set users, Map apps, boolean add) { @@ -452,12 +452,13 @@ public class PermissionMonitor { // If multiple packages share a UID (cf: android:sharedUserId) and ask for different // permissions, don't downgrade (i.e., if it's already SYSTEM, leave it as is). - final Boolean permission = highestPermissionForUid(mApps.get(uid), packageName); - if (permission != mApps.get(uid)) { - mApps.put(uid, permission); + final int appId = UserHandle.getAppId(uid); + final Boolean permission = highestPermissionForUid(mApps.get(appId), packageName); + if (permission != mApps.get(appId)) { + mApps.put(appId, permission); Map apps = new HashMap<>(); - apps.put(uid, permission); + apps.put(appId, permission); update(mUsers, apps, true); } @@ -472,7 +473,7 @@ public class PermissionMonitor { updateVpnUids(vpn.getKey(), changedUids, true); } } - mAllApps.add(UserHandle.getAppId(uid)); + mAllApps.add(appId); } private Boolean highestUidNetworkPermission(int uid) { @@ -529,16 +530,17 @@ public class PermissionMonitor { return; } - if (permission == mApps.get(uid)) { + final int appId = UserHandle.getAppId(uid); + if (permission == mApps.get(appId)) { // The permissions of this UID have not changed. Nothing to do. return; } else if (permission != null) { - mApps.put(uid, permission); - apps.put(uid, permission); + mApps.put(appId, permission); + apps.put(appId, permission); update(mUsers, apps, true); } else { - mApps.remove(uid); - apps.put(uid, NETWORK); // doesn't matter which permission we pick here + mApps.remove(appId); + apps.put(appId, NETWORK); // doesn't matter which permission we pick here update(mUsers, apps, false); } } @@ -653,7 +655,7 @@ public class PermissionMonitor { */ private void removeBypassingUids(Set uids, int vpnAppUid) { uids.remove(vpnAppUid); - uids.removeIf(uid -> mApps.getOrDefault(uid, NETWORK) == SYSTEM); + uids.removeIf(uid -> mApps.getOrDefault(UserHandle.getAppId(uid), NETWORK) == SYSTEM); } /** @@ -795,12 +797,13 @@ public class PermissionMonitor { for (Integer uid : uidsToUpdate) { final Boolean permission = highestUidNetworkPermission(uid); + final int appId = UserHandle.getAppId(uid); if (null == permission) { - removedUids.put(uid, NETWORK); // Doesn't matter which permission is set here. - mApps.remove(uid); + removedUids.put(appId, NETWORK); // Doesn't matter which permission is set here. + mApps.remove(appId); } else { - updatedUids.put(uid, permission); - mApps.put(uid, permission); + updatedUids.put(appId, permission); + mApps.put(appId, permission); } } diff --git a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java index 7c733da4bd..9c7d9c005d 100644 --- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java @@ -528,13 +528,13 @@ public class PermissionMonitorTest { // MOCK_UID1: MOCK_PACKAGE1 only has network permission. // SYSTEM_UID: SYSTEM_PACKAGE1 has system permission. // SYSTEM_UID: SYSTEM_PACKAGE2 only has network permission. - doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(eq(SYSTEM), anyString()); doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(any(), eq(SYSTEM_PACKAGE1)); doReturn(NETWORK).when(mPermissionMonitor).highestPermissionForUid(any(), eq(SYSTEM_PACKAGE2)); doReturn(NETWORK).when(mPermissionMonitor).highestPermissionForUid(any(), eq(MOCK_PACKAGE1)); + doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(eq(SYSTEM), anyString()); // Add SYSTEM_PACKAGE2, expect only have network permission. mPermissionMonitor.onUserAdded(MOCK_USER1); @@ -549,6 +549,21 @@ public class PermissionMonitorTest { netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID}); + // Remove SYSTEM_PACKAGE2, expect keep system permission. + when(mPackageManager.getPackagesForUid(MOCK_USER1.getUid(SYSTEM_UID))) + .thenReturn(new String[]{SYSTEM_PACKAGE1}); + when(mPackageManager.getPackagesForUid(MOCK_USER2.getUid(SYSTEM_UID))) + .thenReturn(new String[]{SYSTEM_PACKAGE1}); + removePackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, + SYSTEM_PACKAGE2, SYSTEM_UID); + netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2}, + new int[]{SYSTEM_UID}); + + // Add SYSTEM_PACKAGE2, expect keep system permission. + addPackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, SYSTEM_PACKAGE2, SYSTEM_UID); + netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2}, + new int[]{SYSTEM_UID}); + addPackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1); netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2}, new int[]{SYSTEM_UID}); @@ -556,6 +571,10 @@ public class PermissionMonitorTest { new int[]{MOCK_UID1}); // Remove MOCK_UID1, expect no permission left for all user. + when(mPackageManager.getPackagesForUid(MOCK_USER1.getUid(MOCK_UID1))) + .thenReturn(new String[]{}); + when(mPackageManager.getPackagesForUid(MOCK_USER2.getUid(MOCK_UID1))) + .thenReturn(new String[]{}); mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1); removePackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1); netdMonitor.expectNoPermission(new UserHandle[]{MOCK_USER1, MOCK_USER2},