Use appId instead of uid
Multiple user's ares of mApps are not initialized in startMonitoring(),
so mApps.get(uid) of multi-user's app returns null in onPackageAdded().
As the result, permission of system uid is updated to "Network" and
any system application cannot use dedicated apn like IMS.
Using appId avoids this problem.
Bug: 168932048
Test: atest FrameworksNetTests
Signed-off-by: Sangcheol Lee <goodsc.lee@samsung.com>
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1733212
Merged-In: Ib8ea2185d8056bddb2ca5a8006f83afb3cffc9f4
Change-Id: Ib8ea2185d8056bddb2ca5a8006f83afb3cffc9f4
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 3328846..e98a638
--- a/service/src/com/android/server/connectivity/PermissionMonitor.java
+++ b/service/src/com/android/server/connectivity/PermissionMonitor.java
@@ -230,11 +230,11 @@
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 @@
// 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<UserHandle> users, Map<Integer, Boolean> apps, boolean add) {
@@ -452,12 +452,13 @@
// 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<Integer, Boolean> apps = new HashMap<>();
- apps.put(uid, permission);
+ apps.put(appId, permission);
update(mUsers, apps, true);
}
@@ -472,7 +473,7 @@
updateVpnUids(vpn.getKey(), changedUids, true);
}
}
- mAllApps.add(UserHandle.getAppId(uid));
+ mAllApps.add(appId);
}
private Boolean highestUidNetworkPermission(int uid) {
@@ -529,16 +530,17 @@
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 @@
*/
private void removeBypassingUids(Set<Integer> 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 @@
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 e98f5db..db63495 100644
--- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
+++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
@@ -521,13 +521,13 @@
// 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);
@@ -542,6 +542,21 @@
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});
@@ -549,6 +564,10 @@
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},