diff options
author | 2021-04-07 15:11:49 -0700 | |
---|---|---|
committer | 2021-04-07 15:27:45 -0700 | |
commit | 5b6355a09fa375348750b24b47a059a44ac2bf1b (patch) | |
tree | c248baaf43c77062c973bbaab6c6340fb2f15451 | |
parent | dea9900796d5dece1d72e3742462ac9241f03d68 (diff) |
Remove location-permission check from VcnStatusCallbacks.
This CL updates VcnManagementService to not require location permissions
for receiving VcnStatusCallback invocations. This change is safe to make
because these callbacks are not capable of leaking any location-related
information.
Bug: 180556279
Test: atest FrameworksVcnTests CtsVcnTestCases
Change-Id: I38600aeb2489f139b3479b07de77facc2ae838c5
-rw-r--r-- | services/core/java/com/android/server/VcnManagementService.java | 11 | ||||
-rw-r--r-- | tests/vcn/java/com/android/server/VcnManagementServiceTest.java | 50 |
2 files changed, 3 insertions, 58 deletions
diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index f7ae58ca0eb4..8796516c1064 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -168,9 +168,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { @NonNull private final TrackingNetworkCallback mTrackingNetworkCallback = new TrackingNetworkCallback(); - /** Can only be assigned when {@link #systemReady()} is called, since it uses AppOpsManager. */ - @Nullable private LocationPermissionChecker mLocationPermissionChecker; - @GuardedBy("mLock") @NonNull private final Map<ParcelUuid, VcnConfig> mConfigs = new ArrayMap<>(); @@ -369,7 +366,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { new NetworkRequest.Builder().clearCapabilities().build(), mTrackingNetworkCallback); mTelephonySubscriptionTracker.register(); - mLocationPermissionChecker = mDeps.newLocationPermissionChecker(mVcnContext.getContext()); } private void enforcePrimaryUser() { @@ -836,13 +832,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { return false; } - if (!mLocationPermissionChecker.checkLocationPermission( - cbInfo.mPkgName, - "VcnStatusCallback" /* featureId */, - cbInfo.mUid, - null /* message */)) { - return false; - } return true; } diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index 43e6676e1d4c..bbc9bb600f6d 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -81,7 +81,6 @@ import android.telephony.TelephonyManager; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; -import com.android.net.module.util.LocationPermissionChecker; import com.android.server.VcnManagementService.VcnCallback; import com.android.server.VcnManagementService.VcnStatusCallbackInfo; import com.android.server.vcn.TelephonySubscriptionTracker; @@ -162,8 +161,6 @@ public class VcnManagementServiceTest { mock(PersistableBundleUtils.LockingReadWriteHelper.class); private final TelephonySubscriptionTracker mSubscriptionTracker = mock(TelephonySubscriptionTracker.class); - private final LocationPermissionChecker mLocationPermissionChecker = - mock(LocationPermissionChecker.class); private final ArgumentCaptor<VcnCallback> mVcnCallbackCaptor = ArgumentCaptor.forClass(VcnCallback.class); @@ -207,9 +204,6 @@ public class VcnManagementServiceTest { doReturn(mConfigReadWriteHelper) .when(mMockDeps) .newPersistableBundleLockingReadWriteHelper(any()); - doReturn(mLocationPermissionChecker) - .when(mMockDeps) - .newLocationPermissionChecker(eq(mMockContext)); // Setup VCN instance generation doAnswer((invocation) -> { @@ -521,10 +515,6 @@ public class VcnManagementServiceTest { @Test public void testSetVcnConfigNotifiesStatusCallback() throws Exception { - mVcnMgmtSvc.systemReady(); - doReturn(true) - .when(mLocationPermissionChecker) - .checkLocationPermission(eq(TEST_PACKAGE_NAME), any(), eq(TEST_UID), any()); triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(TEST_UUID_2)); mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME); @@ -697,10 +687,6 @@ public class VcnManagementServiceTest { doReturn(isVcnActive ? VCN_STATUS_CODE_ACTIVE : VCN_STATUS_CODE_SAFE_MODE) .when(vcn) .getStatus(); - - doReturn(true) - .when(mLocationPermissionChecker) - .checkLocationPermission(eq(TEST_PACKAGE_NAME), any(), eq(TEST_UID), any()); } private NetworkCapabilities.Builder getNetworkCapabilitiesBuilderForTransport( @@ -933,8 +919,7 @@ public class VcnManagementServiceTest { @NonNull ParcelUuid subGroup, @NonNull String pkgName, int uid, - boolean hasPermissionsforSubGroup, - boolean hasLocationPermission) + boolean hasPermissionsforSubGroup) throws Exception { TelephonySubscriptionSnapshot snapshot = triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(subGroup)); @@ -946,10 +931,6 @@ public class VcnManagementServiceTest { .when(snapshot) .packageHasPermissionsForSubscriptionGroup(eq(subGroup), eq(pkgName)); - doReturn(hasLocationPermission) - .when(mLocationPermissionChecker) - .checkLocationPermission(eq(pkgName), any(), eq(uid), any()); - mVcnMgmtSvc.registerVcnStatusCallback(subGroup, mMockStatusCallback, pkgName); triggerVcnSafeMode(subGroup, snapshot, true /* enterSafeMode */); @@ -959,11 +940,7 @@ public class VcnManagementServiceTest { public void testVcnStatusCallbackOnSafeModeStatusChangedWithCarrierPrivileges() throws Exception { triggerVcnStatusCallbackOnSafeModeStatusChanged( - TEST_UUID_1, - TEST_PACKAGE_NAME, - TEST_UID, - true /* hasPermissionsforSubGroup */, - true /* hasLocationPermission */); + TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, true /* hasPermissionsforSubGroup */); verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE); } @@ -972,25 +949,7 @@ public class VcnManagementServiceTest { public void testVcnStatusCallbackOnSafeModeStatusChangedWithoutCarrierPrivileges() throws Exception { triggerVcnStatusCallbackOnSafeModeStatusChanged( - TEST_UUID_1, - TEST_PACKAGE_NAME, - TEST_UID, - false /* hasPermissionsforSubGroup */, - true /* hasLocationPermission */); - - verify(mMockStatusCallback, never()) - .onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE); - } - - @Test - public void testVcnStatusCallbackOnSafeModeStatusChangedWithoutLocationPermission() - throws Exception { - triggerVcnStatusCallbackOnSafeModeStatusChanged( - TEST_UUID_1, - TEST_PACKAGE_NAME, - TEST_UID, - true /* hasPermissionsforSubGroup */, - false /* hasLocationPermission */); + TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, false /* hasPermissionsforSubGroup */); verify(mMockStatusCallback, never()) .onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE); @@ -1052,9 +1011,6 @@ public class VcnManagementServiceTest { .when(snapshot) .packageHasPermissionsForSubscriptionGroup( eq(TEST_UUID_1), eq(TEST_CB_PACKAGE_NAME)); - doReturn(true) - .when(mLocationPermissionChecker) - .checkLocationPermission(eq(TEST_CB_PACKAGE_NAME), any(), eq(TEST_UID), any()); mVcnMgmtSvc.registerVcnStatusCallback( TEST_UUID_1, mMockStatusCallback, TEST_CB_PACKAGE_NAME); |