From de5b46c10b599a8e15d0b9b038f15313e798522c Mon Sep 17 00:00:00 2001 From: paulhu Date: Mon, 1 Jun 2020 23:36:16 +0800 Subject: [PATCH] [RFPM04] Adjust hasRestrictedNetworkPermission method There are two parts in hasRestrictedNetworkPermission method. One is carryover package check and one is uid permission check. Seperate carryover package check part to another method to be clearer the usage of hasRestrictedNetworkPermission method. Bug: 132784544 Test: atest FrameworksNetTests Change-Id: Iab0609fcaa52acb0dfcb31ca957f22bfe8d62392 --- .../connectivity/PermissionMonitor.java | 33 ++--- .../connectivity/PermissionMonitorTest.java | 122 +++++++++--------- 2 files changed, 75 insertions(+), 80 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index f8774b1b00..7202f0f401 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java @@ -171,8 +171,8 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse mAllApps.add(UserHandle.getAppId(uid)); final boolean isNetwork = hasPermission(CHANGE_NETWORK_STATE, uid); - final boolean hasRestrictedPermission = - hasRestrictedNetworkPermission(app.applicationInfo); + final boolean hasRestrictedPermission = hasRestrictedNetworkPermission(uid) + || isCarryoverPackage(app.applicationInfo); if (isNetwork || hasRestrictedPermission) { Boolean permission = mApps.get(uid); @@ -200,7 +200,7 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse for (int i = 0; i < systemPermission.size(); i++) { ArraySet perms = systemPermission.valueAt(i); int uid = systemPermission.keyAt(i); - int netdPermission = 0; + int netdPermission = PERMISSION_NONE; // Get the uids of native services that have UPDATE_DEVICE_STATS or INTERNET permission. if (perms != null) { netdPermission |= perms.contains(UPDATE_DEVICE_STATS) @@ -225,20 +225,21 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse } @VisibleForTesting - boolean hasRestrictedNetworkPermission(@Nullable final ApplicationInfo appInfo) { - if (appInfo == null) return false; - // TODO : remove this check in the future(b/162295056). All apps should just - // request the appropriate permission for their use case since android Q. - if ((appInfo.targetSdkVersion < VERSION_Q && isVendorApp(appInfo)) + // TODO : remove this check in the future(b/162295056). All apps should just request the + // appropriate permission for their use case since android Q. + boolean isCarryoverPackage(@Nullable final ApplicationInfo appInfo) { + if (appInfo == null) return false; + return (appInfo.targetSdkVersion < VERSION_Q && isVendorApp(appInfo)) // Backward compatibility for b/114245686, on devices that launched before Q daemons // and apps running as the system UID are exempted from this check. - || (appInfo.uid == SYSTEM_UID && mDeps.getDeviceFirstSdkInt() < VERSION_Q)) { - return true; - } + || (appInfo.uid == SYSTEM_UID && mDeps.getDeviceFirstSdkInt() < VERSION_Q); + } - return hasPermission(PERMISSION_MAINLINE_NETWORK_STACK, appInfo.uid) - || hasPermission(NETWORK_STACK, appInfo.uid) - || hasPermission(CONNECTIVITY_USE_RESTRICTED_NETWORKS, appInfo.uid); + @VisibleForTesting + boolean hasRestrictedNetworkPermission(final int uid) { + return hasPermission(CONNECTIVITY_USE_RESTRICTED_NETWORKS, uid) + || hasPermission(PERMISSION_MAINLINE_NETWORK_STACK, uid) + || hasPermission(NETWORK_STACK, uid); } /** Returns whether the given uid has using background network permission. */ @@ -328,8 +329,8 @@ public class PermissionMonitor implements PackageManagerInternal.PackageListObse try { final PackageInfo app = mPackageManager.getPackageInfo(name, GET_PERMISSIONS); final boolean isNetwork = hasPermission(CHANGE_NETWORK_STATE, uid); - final boolean hasRestrictedPermission = - hasRestrictedNetworkPermission(app.applicationInfo); + final boolean hasRestrictedPermission = hasRestrictedNetworkPermission(uid) + || isCarryoverPackage(app.applicationInfo); if (isNetwork || hasRestrictedPermission) { currentPermission = hasRestrictedPermission; } diff --git a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java index 6633c9d69c..79343de3b3 100644 --- a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java @@ -28,6 +28,7 @@ import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_PRODUCT; import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_VENDOR; import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; +import static android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK; import static android.os.Process.SYSTEM_UID; import static com.android.server.connectivity.PermissionMonitor.NETWORK; @@ -138,17 +139,10 @@ public class PermissionMonitorTest { verify(mMockPmi).getPackageList(mPermissionMonitor); } - /** - * Remove all permissions from the uid then build new package info and setup permissions to uid - * for checking restricted network permission. - */ - private boolean hasRestrictedNetworkPermission(String partition, int targetSdkVersion, int uid, - String... permissions) { + private boolean wouldBeCarryoverPackage(String partition, int targetSdkVersion, int uid) { final PackageInfo packageInfo = buildPackageInfo(partition, uid, MOCK_USER1); packageInfo.applicationInfo.targetSdkVersion = targetSdkVersion; - removeAllPermissions(uid); - addPermissions(uid, permissions); - return mPermissionMonitor.hasRestrictedNetworkPermission(packageInfo.applicationInfo); + return mPermissionMonitor.isCarryoverPackage(packageInfo.applicationInfo); } private static PackageInfo packageInfoWithPartition(String partition) { @@ -228,61 +222,57 @@ public class PermissionMonitorTest { assertTrue(mPermissionMonitor.isVendorApp(app.applicationInfo)); } + /** + * Remove all permissions from the uid then setup permissions to uid for checking restricted + * network permission. + */ + private void assertRestrictedNetworkPermission(boolean hasPermission, int uid, + String... permissions) { + removeAllPermissions(uid); + addPermissions(uid, permissions); + assertEquals(hasPermission, mPermissionMonitor.hasRestrictedNetworkPermission(uid)); + } + @Test public void testHasRestrictedNetworkPermission() { - assertFalse(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CHANGE_NETWORK_STATE)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, MOCK_UID1, NETWORK_STACK)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CONNECTIVITY_INTERNAL)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CHANGE_WIFI_STATE)); + assertRestrictedNetworkPermission(false, MOCK_UID1); + assertRestrictedNetworkPermission(false, MOCK_UID1, CHANGE_NETWORK_STATE); + assertRestrictedNetworkPermission(true, MOCK_UID1, NETWORK_STACK); + assertRestrictedNetworkPermission(false, MOCK_UID1, CONNECTIVITY_INTERNAL); + assertRestrictedNetworkPermission(true, MOCK_UID1, CONNECTIVITY_USE_RESTRICTED_NETWORKS); + assertRestrictedNetworkPermission(false, MOCK_UID1, CHANGE_WIFI_STATE); + assertRestrictedNetworkPermission(true, MOCK_UID1, PERMISSION_MAINLINE_NETWORK_STACK); - assertFalse(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_Q, MOCK_UID1)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_Q, MOCK_UID1, CONNECTIVITY_INTERNAL)); + assertFalse(mPermissionMonitor.hasRestrictedNetworkPermission(MOCK_UID2)); + assertFalse(mPermissionMonitor.hasRestrictedNetworkPermission(SYSTEM_UID)); } @Test - public void testHasRestrictedNetworkPermissionSystemUid() { + public void testIsCarryoverPackage() { doReturn(VERSION_P).when(mDeps).getDeviceFirstSdkInt(); - assertTrue(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, SYSTEM_UID, CONNECTIVITY_INTERNAL)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_P, SYSTEM_UID, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); + assertTrue(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID)); + assertTrue(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_P, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_P, MOCK_UID1)); + assertTrue(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_P, MOCK_UID1)); + assertTrue(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID)); + assertTrue(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_Q, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_Q, MOCK_UID1)); + assertFalse(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_Q, MOCK_UID1)); doReturn(VERSION_Q).when(mDeps).getDeviceFirstSdkInt(); - assertFalse(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID, CONNECTIVITY_INTERNAL)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - } + assertFalse(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID)); + assertTrue(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_P, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_P, MOCK_UID1)); + assertTrue(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_P, MOCK_UID1)); + assertFalse(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_Q, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_SYSTEM, VERSION_Q, MOCK_UID1)); + assertFalse(wouldBeCarryoverPackage(PARTITION_VENDOR, VERSION_Q, MOCK_UID1)); - @Test - public void testHasRestrictedNetworkPermissionVendorApp() { - assertTrue(hasRestrictedNetworkPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_P, MOCK_UID1, CHANGE_NETWORK_STATE)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_P, MOCK_UID1, NETWORK_STACK)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_P, MOCK_UID1, CONNECTIVITY_INTERNAL)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_P, MOCK_UID1, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - assertTrue(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_P, MOCK_UID1, CHANGE_WIFI_STATE)); - - assertFalse(hasRestrictedNetworkPermission(PARTITION_VENDOR, VERSION_Q, MOCK_UID1)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_Q, MOCK_UID1, CONNECTIVITY_INTERNAL)); - assertFalse(hasRestrictedNetworkPermission( - PARTITION_VENDOR, VERSION_Q, MOCK_UID1, CHANGE_NETWORK_STATE)); + assertFalse(wouldBeCarryoverPackage(PARTITION_OEM, VERSION_Q, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_PRODUCT, VERSION_Q, SYSTEM_UID)); + assertFalse(wouldBeCarryoverPackage(PARTITION_OEM, VERSION_Q, MOCK_UID1)); + assertFalse(wouldBeCarryoverPackage(PARTITION_PRODUCT, VERSION_Q, MOCK_UID1)); } private void assertBackgroundPermission(boolean hasPermission, String name, int uid, @@ -296,19 +286,23 @@ public class PermissionMonitorTest { @Test public void testHasUseBackgroundNetworksPermission() throws Exception { + assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(MOCK_UID1)); + assertBackgroundPermission(false, "mock1", MOCK_UID1); + assertBackgroundPermission(false, "mock2", MOCK_UID1, CONNECTIVITY_INTERNAL); + assertBackgroundPermission(true, "mock3", MOCK_UID1, NETWORK_STACK); + + assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(MOCK_UID2)); + assertBackgroundPermission(false, "mock4", MOCK_UID2); + assertBackgroundPermission(true, "mock5", MOCK_UID2, + CONNECTIVITY_USE_RESTRICTED_NETWORKS); + doReturn(VERSION_Q).when(mDeps).getDeviceFirstSdkInt(); assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(SYSTEM_UID)); assertBackgroundPermission(false, "system1", SYSTEM_UID); - assertBackgroundPermission(false, "system2", SYSTEM_UID, CONNECTIVITY_INTERNAL); - assertBackgroundPermission(true, "system3", SYSTEM_UID, CHANGE_NETWORK_STATE); - - assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(MOCK_UID1)); - assertBackgroundPermission(false, "mock1", MOCK_UID1); - assertBackgroundPermission(true, "mock2", MOCK_UID1, CONNECTIVITY_USE_RESTRICTED_NETWORKS); - - assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(MOCK_UID2)); - assertBackgroundPermission(false, "mock3", MOCK_UID2, CONNECTIVITY_INTERNAL); - assertBackgroundPermission(true, "mock4", MOCK_UID2, NETWORK_STACK); + assertBackgroundPermission(true, "system2", SYSTEM_UID, CHANGE_NETWORK_STATE); + doReturn(VERSION_P).when(mDeps).getDeviceFirstSdkInt(); + removeAllPermissions(SYSTEM_UID); + assertBackgroundPermission(true, "system3", SYSTEM_UID); } private class NetdMonitor {