From 7692fba5c1246f69bd78eb4b41ef2dc7bd442f0b Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Wed, 13 Jan 2021 13:33:16 -0800 Subject: [PATCH] LocationPermissionChecker: Exempt privileged components from location check This is a port of the exemption that exists in WifiPermissionsUtil. Settings, sysui, network stack needs to be able to access all network state regardless of location toggle. If we want to move sysui, etc to retrieve WifiInfo via NetworkCapabilities (which is the current plan), this exemption is essential since UI should reflect wifi state regardless of location toggle state. Bug: 162602799 Test: atest LocationPermissionCheckerTest Change-Id: I49ce465eccce27bb7a860d882360436fd9ec19c6 --- .../util/LocationPermissionChecker.java | 41 +++++++++++++++++++ .../util/LocationPermissionCheckerTest.java | 21 ++++++++++ .../server/ConnectivityServiceTest.java | 28 +++++++++---- 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/core/java/com/android/internal/util/LocationPermissionChecker.java b/core/java/com/android/internal/util/LocationPermissionChecker.java index cd8fc350362d0..c583d5a5be372 100644 --- a/core/java/com/android/internal/util/LocationPermissionChecker.java +++ b/core/java/com/android/internal/util/LocationPermissionChecker.java @@ -24,6 +24,7 @@ import android.app.AppOpsManager; import android.content.Context; import android.content.pm.PackageManager; import android.location.LocationManager; +import android.net.NetworkStack; import android.os.Binder; import android.os.Build; import android.os.UserHandle; @@ -147,6 +148,13 @@ public class LocationPermissionChecker { int uid, @Nullable String message) { checkPackage(uid, pkgName); + // Apps with NETWORK_SETTINGS, NETWORK_SETUP_WIZARD, NETWORK_STACK & MAINLINE_NETWORK_STACK + // are granted a bypass. + if (checkNetworkSettingsPermission(uid) || checkNetworkSetupWizardPermission(uid) + || checkNetworkStackPermission(uid) || checkMainlineNetworkStackPermission(uid)) { + return SUCCEEDED; + } + // Location mode must be enabled if (!isLocationModeEnabled()) { return ERROR_LOCATION_MODE_OFF; @@ -259,4 +267,37 @@ public class LocationPermissionChecker { // We don't care about pid, pass in -1 return mContext.checkPermission(permissionType, -1, uid); } + + /** + * Returns true if the |uid| holds NETWORK_SETTINGS permission. + */ + public boolean checkNetworkSettingsPermission(int uid) { + return getUidPermission(android.Manifest.permission.NETWORK_SETTINGS, uid) + == PackageManager.PERMISSION_GRANTED; + } + + /** + * Returns true if the |uid| holds NETWORK_SETUP_WIZARD permission. + */ + public boolean checkNetworkSetupWizardPermission(int uid) { + return getUidPermission(android.Manifest.permission.NETWORK_SETUP_WIZARD, uid) + == PackageManager.PERMISSION_GRANTED; + } + + /** + * Returns true if the |uid| holds NETWORK_STACK permission. + */ + public boolean checkNetworkStackPermission(int uid) { + return getUidPermission(android.Manifest.permission.NETWORK_STACK, uid) + == PackageManager.PERMISSION_GRANTED; + } + + /** + * Returns true if the |uid| holds MAINLINE_NETWORK_STACK permission. + */ + public boolean checkMainlineNetworkStackPermission(int uid) { + return getUidPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, uid) + == PackageManager.PERMISSION_GRANTED; + } + } diff --git a/core/tests/utiltests/src/com/android/internal/util/LocationPermissionCheckerTest.java b/core/tests/utiltests/src/com/android/internal/util/LocationPermissionCheckerTest.java index 4c3eaeb1730bf..7175f562d7eff 100644 --- a/core/tests/utiltests/src/com/android/internal/util/LocationPermissionCheckerTest.java +++ b/core/tests/utiltests/src/com/android/internal/util/LocationPermissionCheckerTest.java @@ -15,6 +15,8 @@ */ package com.android.internal.util; +import static android.Manifest.permission.NETWORK_SETTINGS; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -82,6 +84,7 @@ public class LocationPermissionCheckerTest { private int mAllowCoarseLocationApps; private int mFineLocationPermission; private int mAllowFineLocationApps; + private int mNetworkSettingsPermission; private int mCurrentUser; private boolean mIsLocationEnabled; private boolean mThrowSecurityException; @@ -138,6 +141,7 @@ public class LocationPermissionCheckerTest { mFineLocationPermission = PackageManager.PERMISSION_DENIED; mAllowCoarseLocationApps = AppOpsManager.MODE_ERRORED; mAllowFineLocationApps = AppOpsManager.MODE_ERRORED; + mNetworkSettingsPermission = PackageManager.PERMISSION_DENIED; } private void setupMockInterface() { @@ -151,6 +155,8 @@ public class LocationPermissionCheckerTest { .thenReturn(mCoarseLocationPermission); when(mMockContext.checkPermission(mManifestStringFine, -1, mUid)) .thenReturn(mFineLocationPermission); + when(mMockContext.checkPermission(NETWORK_SETTINGS, -1, mUid)) + .thenReturn(mNetworkSettingsPermission); when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(mIsLocationEnabled); } @@ -264,6 +270,21 @@ public class LocationPermissionCheckerTest { assertEquals(LocationPermissionChecker.ERROR_LOCATION_MODE_OFF, result); } + @Test + public void testenforceCanAccessScanResults_LocationModeDisabledHasNetworkSettings() + throws Exception { + mThrowSecurityException = false; + mIsLocationEnabled = false; + mNetworkSettingsPermission = PackageManager.PERMISSION_GRANTED; + setupTestCase(); + + final int result = + mChecker.checkLocationPermissionWithDetailInfo( + TEST_PKG_NAME, TEST_FEATURE_ID, mUid, null); + assertEquals(LocationPermissionChecker.SUCCEEDED, result); + } + + private static void assertThrows(Class exceptionClass, Runnable r) { try { r.run(); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index b6e6816ca5313..efc27b35260b8 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -2074,10 +2074,6 @@ public class ConnectivityServiceTest { @Test public void testOwnerUidCannotChange() throws Exception { - // Owner UIDs are not visible without location permission. - setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, - Manifest.permission.ACCESS_FINE_LOCATION); - final NetworkCapabilities ncTemplate = new NetworkCapabilities(); final int originalOwnerUid = Process.myUid(); ncTemplate.setOwnerUid(originalOwnerUid); @@ -2097,6 +2093,10 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true); waitForIdle(); + // Owner UIDs are not visible without location permission. + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + // Check that the capability change has been applied but the owner UID is not modified. NetworkCapabilities nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork()); assertEquals(originalOwnerUid, nc.getOwnerUid()); @@ -7781,8 +7781,22 @@ public class ConnectivityServiceTest { naExtraInfo.unregister(); } + // To avoid granting location permission bypass. + private void denyAllLocationPrivilegedPermissions() { + mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + PERMISSION_DENIED); + mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, + PERMISSION_DENIED); + mServiceContext.setPermission(Manifest.permission.NETWORK_STACK, + PERMISSION_DENIED); + mServiceContext.setPermission(Manifest.permission.NETWORK_SETUP_WIZARD, + PERMISSION_DENIED); + } + private void setupLocationPermissions( int targetSdk, boolean locationToggle, String op, String perm) throws Exception { + denyAllLocationPrivilegedPermissions(); + final ApplicationInfo applicationInfo = new ApplicationInfo(); applicationInfo.targetSdkVersion = targetSdk; when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any())) @@ -8156,15 +8170,13 @@ public class ConnectivityServiceTest { new NetworkAgentInfo(null, network, null, null, new NetworkCapabilities(), 0, mServiceContext, null, null, mService, null, null, null, 0, INVALID_UID); - setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, - Manifest.permission.ACCESS_FINE_LOCATION); - mMockVpn.establishForMyUid(); assertUidRangesUpdatedForMyUid(true); // Wait for networks to connect and broadcasts to be sent before removing permissions. waitForIdle(); - mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {network})); waitForIdle();