LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use appId instead of uid

Browse Source 
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>
Change-Id: Ib8ea2185d8056bddb2ca5a8006f83afb3cffc9f4
This commit is contained in:
Sangcheol Lee
2021-06-10 16:06:07 +09:00
parent 45f922a2ff
commit 1e5d742596
2 changed files with 42 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
service/src/com/android/server/connectivity/PermissionMonitor.java Normal file → Executable file
View File

@@ -230,11 +230,11 @@ public class PermissionMonitor {
boolean hasRestrictedPermission = hasRestrictedNetworkPermission(app); boolean hasRestrictedPermission = hasRestrictedNetworkPermission(app);
if (isNetwork || hasRestrictedPermission) { 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 // 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). // permissions, don't downgrade (i.e., if it's already SYSTEM, leave it as is).
if (permission == null || permission == NETWORK) { 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 // 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 // hasRestrictedNetworkPermission. If uid is in the mApps list that means uid has one of
// permissions at least. // permissions at least.
return mApps.containsKey(uid); return mApps.containsKey(UserHandle.getAppId(uid));
} }
/** /**
* Returns whether the given uid has permission to use restricted networks. * Returns whether the given uid has permission to use restricted networks.
*/ */
public synchronized boolean hasRestrictedNetworksPermission(int uid) { 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) { private void update(Set<UserHandle> users, Map<Integer, Boolean> apps, boolean add) {
@@ -452,12 +452,13 @@ public class PermissionMonitor {
// If multiple packages share a UID (cf: android:sharedUserId) and ask for different // 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). // permissions, don't downgrade (i.e., if it's already SYSTEM, leave it as is).
final Boolean permission = highestPermissionForUid(mApps.get(uid), packageName); final int appId = UserHandle.getAppId(uid);
if (permission != mApps.get(uid)) { final Boolean permission = highestPermissionForUid(mApps.get(appId), packageName);
mApps.put(uid, permission); if (permission != mApps.get(appId)) {
mApps.put(appId, permission);
Map<Integer, Boolean> apps = new HashMap<>(); Map<Integer, Boolean> apps = new HashMap<>();
apps.put(uid, permission); apps.put(appId, permission);
update(mUsers, apps, true); update(mUsers, apps, true);
} }
@@ -472,7 +473,7 @@ public class PermissionMonitor {
updateVpnUids(vpn.getKey(), changedUids, true); updateVpnUids(vpn.getKey(), changedUids, true);
} }
} }
mAllApps.add(UserHandle.getAppId(uid)); mAllApps.add(appId);
} }
private Boolean highestUidNetworkPermission(int uid) { private Boolean highestUidNetworkPermission(int uid) {
@@ -529,16 +530,17 @@ public class PermissionMonitor {
return; 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. // The permissions of this UID have not changed. Nothing to do.
return; return;
} else if (permission != null) { } else if (permission != null) {
mApps.put(uid, permission); mApps.put(appId, permission);
apps.put(uid, permission); apps.put(appId, permission);
update(mUsers, apps, true); update(mUsers, apps, true);
} else { } else {
mApps.remove(uid); mApps.remove(appId);
apps.put(uid, NETWORK); // doesn't matter which permission we pick here apps.put(appId, NETWORK); // doesn't matter which permission we pick here
update(mUsers, apps, false); update(mUsers, apps, false);
} }
} }
@@ -653,7 +655,7 @@ public class PermissionMonitor {
*/ */
private void removeBypassingUids(Set<Integer> uids, int vpnAppUid) { private void removeBypassingUids(Set<Integer> uids, int vpnAppUid) {
uids.remove(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) { for (Integer uid : uidsToUpdate) {
final Boolean permission = highestUidNetworkPermission(uid); final Boolean permission = highestUidNetworkPermission(uid);
final int appId = UserHandle.getAppId(uid);
if (null == permission) { if (null == permission) {
removedUids.put(uid, NETWORK); // Doesn't matter which permission is set here. removedUids.put(appId, NETWORK); // Doesn't matter which permission is set here.
mApps.remove(uid); mApps.remove(appId);
} else { } else {
updatedUids.put(uid, permission); updatedUids.put(appId, permission);
mApps.put(uid, permission); mApps.put(appId, permission);
} }
} }

21
tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
View File

@@ -522,13 +522,13 @@ public class PermissionMonitorTest {
// MOCK_UID1: MOCK_PACKAGE1 only has network permission. // MOCK_UID1: MOCK_PACKAGE1 only has network permission.
// SYSTEM_UID: SYSTEM_PACKAGE1 has system permission. // SYSTEM_UID: SYSTEM_PACKAGE1 has system permission.
// SYSTEM_UID: SYSTEM_PACKAGE2 only has network permission. // SYSTEM_UID: SYSTEM_PACKAGE2 only has network permission.
doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(eq(SYSTEM), anyString());
doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(any(), doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(any(),
eq(SYSTEM_PACKAGE1)); eq(SYSTEM_PACKAGE1));
doReturn(NETWORK).when(mPermissionMonitor).highestPermissionForUid(any(), doReturn(NETWORK).when(mPermissionMonitor).highestPermissionForUid(any(),
eq(SYSTEM_PACKAGE2)); eq(SYSTEM_PACKAGE2));
doReturn(NETWORK).when(mPermissionMonitor).highestPermissionForUid(any(), doReturn(NETWORK).when(mPermissionMonitor).highestPermissionForUid(any(),
eq(MOCK_PACKAGE1)); eq(MOCK_PACKAGE1));
doReturn(SYSTEM).when(mPermissionMonitor).highestPermissionForUid(eq(SYSTEM), anyString());
// Add SYSTEM_PACKAGE2, expect only have network permission. // Add SYSTEM_PACKAGE2, expect only have network permission.
mPermissionMonitor.onUserAdded(MOCK_USER1); mPermissionMonitor.onUserAdded(MOCK_USER1);
@@ -543,6 +543,21 @@ public class PermissionMonitorTest {
netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2}, netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2},
new int[]{SYSTEM_UID}); 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); addPackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1);
netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2}, netdMonitor.expectPermission(SYSTEM, new UserHandle[]{MOCK_USER1, MOCK_USER2},
new int[]{SYSTEM_UID}); new int[]{SYSTEM_UID});
@@ -550,6 +565,10 @@ public class PermissionMonitorTest {
new int[]{MOCK_UID1}); new int[]{MOCK_UID1});
// Remove MOCK_UID1, expect no permission left for all user. // 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); mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID1);
removePackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1); removePackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_UID1);
netdMonitor.expectNoPermission(new UserHandle[]{MOCK_USER1, MOCK_USER2}, netdMonitor.expectNoPermission(new UserHandle[]{MOCK_USER1, MOCK_USER2},