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
diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java
index f7ae58c..8796516 100644
--- a/services/core/java/com/android/server/VcnManagementService.java
+++ b/services/core/java/com/android/server/VcnManagementService.java
@@ -168,9 +168,6 @@
@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 @@
new NetworkRequest.Builder().clearCapabilities().build(),
mTrackingNetworkCallback);
mTelephonySubscriptionTracker.register();
- mLocationPermissionChecker = mDeps.newLocationPermissionChecker(mVcnContext.getContext());
}
private void enforcePrimaryUser() {
@@ -836,13 +832,6 @@
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 43e6676..bbc9bb6 100644
--- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java
+++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java
@@ -81,7 +81,6 @@
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 @@
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 @@
doReturn(mConfigReadWriteHelper)
.when(mMockDeps)
.newPersistableBundleLockingReadWriteHelper(any());
- doReturn(mLocationPermissionChecker)
- .when(mMockDeps)
- .newLocationPermissionChecker(eq(mMockContext));
// Setup VCN instance generation
doAnswer((invocation) -> {
@@ -521,10 +515,6 @@
@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 @@
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 @@
@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 @@
.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 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 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 @@
.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);