Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5b6355a0 authored by Cody Kesting's avatar Cody Kesting
Browse files

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
parent dea99007

services/core/java/com/android/server/VcnManagementService.java

+0 −11
Original line number Original line Diff line number Diff line
@@ -168,9 +168,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { 
    @NonNull

    @NonNull

    private final TrackingNetworkCallback mTrackingNetworkCallback = new TrackingNetworkCallback();

    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")

    @GuardedBy("mLock")

    @NonNull

    @NonNull

    private final Map<ParcelUuid, VcnConfig> mConfigs = new ArrayMap<>();

    private final Map<ParcelUuid, VcnConfig> mConfigs = new ArrayMap<>();
@@ -369,7 +366,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { 
                        new NetworkRequest.Builder().clearCapabilities().build(),

                        new NetworkRequest.Builder().clearCapabilities().build(),

                        mTrackingNetworkCallback);

                        mTrackingNetworkCallback);

        mTelephonySubscriptionTracker.register();

        mTelephonySubscriptionTracker.register();

        mLocationPermissionChecker = mDeps.newLocationPermissionChecker(mVcnContext.getContext());

    }

    }





    private void enforcePrimaryUser() {

    private void enforcePrimaryUser() {
@@ -836,13 +832,6 @@ public class VcnManagementService extends IVcnManagementService.Stub { 
            return false;

            return false;

        }

        }





        if (!mLocationPermissionChecker.checkLocationPermission(

                cbInfo.mPkgName,

                "VcnStatusCallback" /* featureId */,

                cbInfo.mUid,

                null /* message */)) {

            return false;

        }

        return true;

        return true;

    }

    }

tests/vcn/java/com/android/server/VcnManagementServiceTest.java

+3 −47
Original line number Original line Diff line number Diff line
@@ -81,7 +81,6 @@ import android.telephony.TelephonyManager; 
import androidx.test.filters.SmallTest;

import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;

import androidx.test.runner.AndroidJUnit4;





import com.android.net.module.util.LocationPermissionChecker;

import com.android.server.VcnManagementService.VcnCallback;

import com.android.server.VcnManagementService.VcnCallback;

import com.android.server.VcnManagementService.VcnStatusCallbackInfo;

import com.android.server.VcnManagementService.VcnStatusCallbackInfo;

import com.android.server.vcn.TelephonySubscriptionTracker;

import com.android.server.vcn.TelephonySubscriptionTracker;
@@ -162,8 +161,6 @@ public class VcnManagementServiceTest { 
            mock(PersistableBundleUtils.LockingReadWriteHelper.class);

            mock(PersistableBundleUtils.LockingReadWriteHelper.class);

    private final TelephonySubscriptionTracker mSubscriptionTracker =

    private final TelephonySubscriptionTracker mSubscriptionTracker =

            mock(TelephonySubscriptionTracker.class);

            mock(TelephonySubscriptionTracker.class);

    private final LocationPermissionChecker mLocationPermissionChecker =

            mock(LocationPermissionChecker.class);





    private final ArgumentCaptor<VcnCallback> mVcnCallbackCaptor =

    private final ArgumentCaptor<VcnCallback> mVcnCallbackCaptor =

            ArgumentCaptor.forClass(VcnCallback.class);

            ArgumentCaptor.forClass(VcnCallback.class);
@@ -207,9 +204,6 @@ public class VcnManagementServiceTest { 
        doReturn(mConfigReadWriteHelper)

        doReturn(mConfigReadWriteHelper)

                .when(mMockDeps)

                .when(mMockDeps)

                .newPersistableBundleLockingReadWriteHelper(any());

                .newPersistableBundleLockingReadWriteHelper(any());

        doReturn(mLocationPermissionChecker)

                .when(mMockDeps)

                .newLocationPermissionChecker(eq(mMockContext));





        // Setup VCN instance generation

        // Setup VCN instance generation

        doAnswer((invocation) -> {

        doAnswer((invocation) -> {
@@ -521,10 +515,6 @@ public class VcnManagementServiceTest { 




    @Test

    @Test

    public void testSetVcnConfigNotifiesStatusCallback() throws Exception {

    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));

        triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(TEST_UUID_2));





        mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME);

        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)

        doReturn(isVcnActive ? VCN_STATUS_CODE_ACTIVE : VCN_STATUS_CODE_SAFE_MODE)

                .when(vcn)

                .when(vcn)

                .getStatus();

                .getStatus();



        doReturn(true)

                .when(mLocationPermissionChecker)

                .checkLocationPermission(eq(TEST_PACKAGE_NAME), any(), eq(TEST_UID), any());

    }

    }





    private NetworkCapabilities.Builder getNetworkCapabilitiesBuilderForTransport(

    private NetworkCapabilities.Builder getNetworkCapabilitiesBuilderForTransport(
@@ -933,8 +919,7 @@ public class VcnManagementServiceTest { 
            @NonNull ParcelUuid subGroup,

            @NonNull ParcelUuid subGroup,

            @NonNull String pkgName,

            @NonNull String pkgName,

            int uid,

            int uid,

            boolean hasPermissionsforSubGroup,

            boolean hasPermissionsforSubGroup)

            boolean hasLocationPermission)

            throws Exception {

            throws Exception {

        TelephonySubscriptionSnapshot snapshot =

        TelephonySubscriptionSnapshot snapshot =

                triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(subGroup));

                triggerSubscriptionTrackerCbAndGetSnapshot(Collections.singleton(subGroup));
@@ -946,10 +931,6 @@ public class VcnManagementServiceTest { 
                .when(snapshot)

                .when(snapshot)

                .packageHasPermissionsForSubscriptionGroup(eq(subGroup), eq(pkgName));

                .packageHasPermissionsForSubscriptionGroup(eq(subGroup), eq(pkgName));





        doReturn(hasLocationPermission)

                .when(mLocationPermissionChecker)

                .checkLocationPermission(eq(pkgName), any(), eq(uid), any());



        mVcnMgmtSvc.registerVcnStatusCallback(subGroup, mMockStatusCallback, pkgName);

        mVcnMgmtSvc.registerVcnStatusCallback(subGroup, mMockStatusCallback, pkgName);





        triggerVcnSafeMode(subGroup, snapshot, true /* enterSafeMode */);

        triggerVcnSafeMode(subGroup, snapshot, true /* enterSafeMode */);
@@ -959,11 +940,7 @@ public class VcnManagementServiceTest { 
    public void testVcnStatusCallbackOnSafeModeStatusChangedWithCarrierPrivileges()

    public void testVcnStatusCallbackOnSafeModeStatusChangedWithCarrierPrivileges()

            throws Exception {

            throws Exception {

        triggerVcnStatusCallbackOnSafeModeStatusChanged(

        triggerVcnStatusCallbackOnSafeModeStatusChanged(

                TEST_UUID_1,

                TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, true /* hasPermissionsforSubGroup */);

                TEST_PACKAGE_NAME,

                TEST_UID,

                true /* hasPermissionsforSubGroup */,

                true /* hasLocationPermission */);





        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE);

        verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE);

    }

    }
@@ -972,25 +949,7 @@ public class VcnManagementServiceTest { 
    public void testVcnStatusCallbackOnSafeModeStatusChangedWithoutCarrierPrivileges()

    public void testVcnStatusCallbackOnSafeModeStatusChangedWithoutCarrierPrivileges()

            throws Exception {

            throws Exception {

        triggerVcnStatusCallbackOnSafeModeStatusChanged(

        triggerVcnStatusCallbackOnSafeModeStatusChanged(

                TEST_UUID_1,

                TEST_UUID_1, TEST_PACKAGE_NAME, TEST_UID, false /* hasPermissionsforSubGroup */);

                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 */);





        verify(mMockStatusCallback, never())

        verify(mMockStatusCallback, never())

                .onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE);

                .onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_SAFE_MODE);
@@ -1052,9 +1011,6 @@ public class VcnManagementServiceTest { 
                .when(snapshot)

                .when(snapshot)

                .packageHasPermissionsForSubscriptionGroup(

                .packageHasPermissionsForSubscriptionGroup(

                        eq(TEST_UUID_1), eq(TEST_CB_PACKAGE_NAME));

                        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(

        mVcnMgmtSvc.registerVcnStatusCallback(

                TEST_UUID_1, mMockStatusCallback, TEST_CB_PACKAGE_NAME);

                TEST_UUID_1, mMockStatusCallback, TEST_CB_PACKAGE_NAME);