From 1a8f3e0a14ecb88f8aebdea10fc3fa94b1ee9b69 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Tue, 29 Mar 2022 01:01:03 +0000 Subject: [PATCH] Allow provisioning package to retrieve subGrp, clear it's own config This changes the VCN to allow a VCN provisioning package to retrieve its listing of configured subgroups and clear its own configurations, regardless of whether it is the active subscription group. Safety is guaranteed based on the VCN's clearing of packages when app data is cleared, and when the app is uninstalled. In addition to the configurations not being retrievable, the clearing of the configs will ensure that sideloading of an app with the same package name provides no ability to otherwise impact settings. Bug: 227248744 Test: atest FrameworksVcnTests Change-Id: I2c774c00373942f895169a761d4e9a1d5b0b2edb --- core/java/android/net/vcn/VcnManager.java | 15 +- .../android/server/VcnManagementService.java | 159 ++++++++++++++---- .../server/VcnManagementServiceTest.java | 107 ++++++++++-- 3 files changed, 228 insertions(+), 53 deletions(-) diff --git a/core/java/android/net/vcn/VcnManager.java b/core/java/android/net/vcn/VcnManager.java index 390c3b9453c2c..f1b110ab29c8c 100644 --- a/core/java/android/net/vcn/VcnManager.java +++ b/core/java/android/net/vcn/VcnManager.java @@ -172,11 +172,11 @@ public class VcnManager { * *

An app that has carrier privileges for any of the subscriptions in the given group may * clear a VCN configuration. This API is ONLY permitted for callers running as the primary - * user. Any active VCN will be torn down. + * user. Any active VCN associated with this configuration will be torn down. * * @param subscriptionGroup the subscription group that the configuration should be applied to - * @throws SecurityException if the caller does not have carrier privileges, or is not running - * as the primary user + * @throws SecurityException if the caller does not have carrier privileges, is not the owner of + * the associated configuration, or is not running as the primary user * @throws IOException if the configuration failed to be cleared from disk. This may occur due * to temporary disk errors, or more permanent conditions such as a full disk. */ @@ -196,8 +196,13 @@ public class VcnManager { /** * Retrieves the list of Subscription Groups for which a VCN Configuration has been set. * - *

The returned list will include only subscription groups for which the carrier app is - * privileged, and which have an associated {@link VcnConfig}. + *

The returned list will include only subscription groups for which an associated {@link + * VcnConfig} exists, and the app is either: + * + *

* * @throws SecurityException if the caller is not running as the primary user */ diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index 2d328d8b09496..fa5e93729a011 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -87,6 +87,7 @@ import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -172,7 +173,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { @NonNull private final VcnNetworkProvider mNetworkProvider; @NonNull private final TelephonySubscriptionTrackerCallback mTelephonySubscriptionTrackerCb; @NonNull private final TelephonySubscriptionTracker mTelephonySubscriptionTracker; - @NonNull private final BroadcastReceiver mPkgChangeReceiver; + @NonNull private final BroadcastReceiver mVcnBroadcastReceiver; @NonNull private final TrackingNetworkCallback mTrackingNetworkCallback = new TrackingNetworkCallback(); @@ -217,28 +218,17 @@ public class VcnManagementService extends IVcnManagementService.Stub { mConfigDiskRwHelper = mDeps.newPersistableBundleLockingReadWriteHelper(VCN_CONFIG_FILE); - mPkgChangeReceiver = new BroadcastReceiver() { - @Override - public void onReceive(Context context, Intent intent) { - final String action = intent.getAction(); - - if (Intent.ACTION_PACKAGE_ADDED.equals(action) - || Intent.ACTION_PACKAGE_REPLACED.equals(action) - || Intent.ACTION_PACKAGE_REMOVED.equals(action)) { - mTelephonySubscriptionTracker.handleSubscriptionsChanged(); - } else { - Log.wtf(TAG, "received unexpected intent: " + action); - } - } - }; + mVcnBroadcastReceiver = new VcnBroadcastReceiver(); final IntentFilter intentFilter = new IntentFilter(); intentFilter.addAction(Intent.ACTION_PACKAGE_ADDED); intentFilter.addAction(Intent.ACTION_PACKAGE_REPLACED); intentFilter.addAction(Intent.ACTION_PACKAGE_REMOVED); + intentFilter.addAction(Intent.ACTION_PACKAGE_DATA_CLEARED); + intentFilter.addAction(Intent.ACTION_PACKAGE_FULLY_REMOVED); intentFilter.addDataScheme("package"); mContext.registerReceiver( - mPkgChangeReceiver, intentFilter, null /* broadcastPermission */, mHandler); + mVcnBroadcastReceiver, intentFilter, null /* broadcastPermission */, mHandler); // Run on handler to ensure I/O does not block system server startup mHandler.post(() -> { @@ -443,6 +433,53 @@ public class VcnManagementService extends IVcnManagementService.Stub { return Objects.equals(subGrp, snapshot.getActiveDataSubscriptionGroup()); } + private class VcnBroadcastReceiver extends BroadcastReceiver { + @Override + public void onReceive(Context context, Intent intent) { + final String action = intent.getAction(); + + switch (action) { + case Intent.ACTION_PACKAGE_ADDED: // Fallthrough + case Intent.ACTION_PACKAGE_REPLACED: // Fallthrough + case Intent.ACTION_PACKAGE_REMOVED: + // Reevaluate subscriptions + mTelephonySubscriptionTracker.handleSubscriptionsChanged(); + + break; + case Intent.ACTION_PACKAGE_FULLY_REMOVED: + case Intent.ACTION_PACKAGE_DATA_CLEARED: + final String pkgName = intent.getData().getSchemeSpecificPart(); + + if (pkgName == null || pkgName.isEmpty()) { + logWtf("Package name was empty or null for intent with action" + action); + return; + } + + // Clear configs for the packages that had data cleared, or removed. + synchronized (mLock) { + final List toRemove = new ArrayList<>(); + for (Entry entry : mConfigs.entrySet()) { + if (pkgName.equals(entry.getValue().getProvisioningPackageName())) { + toRemove.add(entry.getKey()); + } + } + + for (ParcelUuid subGrp : toRemove) { + stopAndClearVcnConfigInternalLocked(subGrp); + } + + if (!toRemove.isEmpty()) { + writeConfigsToDiskLocked(); + } + } + + break; + default: + Slog.wtf(TAG, "received unexpected intent: " + action); + } + } + } + private class VcnSubscriptionTrackerCallback implements TelephonySubscriptionTrackerCallback { /** * Handles subscription group changes, as notified by {@link TelephonySubscriptionTracker} @@ -504,6 +541,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { final Map> currSubGrpMappings = getSubGroupToSubIdMappings(mLastSnapshot); if (!currSubGrpMappings.equals(oldSubGrpMappings)) { + garbageCollectAndWriteVcnConfigsLocked(); notifyAllPolicyListenersLocked(); } } @@ -643,6 +681,39 @@ public class VcnManagementService extends IVcnManagementService.Stub { }); } + private void enforceCarrierPrivilegeOrProvisioningPackage( + @NonNull ParcelUuid subscriptionGroup, @NonNull String pkg) { + // Only apps running in the primary (system) user are allowed to configure the VCN. This is + // in line with Telephony's behavior with regards to binding to a Carrier App provided + // CarrierConfigService. + enforcePrimaryUser(); + + if (isProvisioningPackageForConfig(subscriptionGroup, pkg)) { + return; + } + + // Must NOT be called from cleared binder identity, since this checks user calling identity + enforceCallingUserAndCarrierPrivilege(subscriptionGroup, pkg); + } + + private boolean isProvisioningPackageForConfig( + @NonNull ParcelUuid subscriptionGroup, @NonNull String pkg) { + // Try-finally to return early if matching owned subscription found. + final long identity = Binder.clearCallingIdentity(); + try { + synchronized (mLock) { + final VcnConfig config = mConfigs.get(subscriptionGroup); + if (config != null && pkg.equals(config.getProvisioningPackageName())) { + return true; + } + } + } finally { + Binder.restoreCallingIdentity(identity); + } + + return false; + } + /** * Clears the VcnManagementService for a given subscription group. * @@ -656,31 +727,56 @@ public class VcnManagementService extends IVcnManagementService.Stub { mContext.getSystemService(AppOpsManager.class) .checkPackage(mDeps.getBinderCallingUid(), opPkgName); - enforceCallingUserAndCarrierPrivilege(subscriptionGroup, opPkgName); + enforceCarrierPrivilegeOrProvisioningPackage(subscriptionGroup, opPkgName); Binder.withCleanCallingIdentity(() -> { synchronized (mLock) { - mConfigs.remove(subscriptionGroup); - final boolean vcnExists = mVcns.containsKey(subscriptionGroup); - - stopVcnLocked(subscriptionGroup); - - if (vcnExists) { - // TODO(b/181789060): invoke asynchronously after Vcn notifies through - // VcnCallback - notifyAllPermissionedStatusCallbacksLocked( - subscriptionGroup, VCN_STATUS_CODE_NOT_CONFIGURED); - } - + stopAndClearVcnConfigInternalLocked(subscriptionGroup); writeConfigsToDiskLocked(); } }); } + private void stopAndClearVcnConfigInternalLocked(@NonNull ParcelUuid subscriptionGroup) { + mConfigs.remove(subscriptionGroup); + final boolean vcnExists = mVcns.containsKey(subscriptionGroup); + + stopVcnLocked(subscriptionGroup); + + if (vcnExists) { + // TODO(b/181789060): invoke asynchronously after Vcn notifies through + // VcnCallback + notifyAllPermissionedStatusCallbacksLocked( + subscriptionGroup, VCN_STATUS_CODE_NOT_CONFIGURED); + } + } + + private void garbageCollectAndWriteVcnConfigsLocked() { + final SubscriptionManager subMgr = mContext.getSystemService(SubscriptionManager.class); + + boolean shouldWrite = false; + + final Iterator configsIterator = mConfigs.keySet().iterator(); + while (configsIterator.hasNext()) { + final ParcelUuid subGrp = configsIterator.next(); + + final List subscriptions = subMgr.getSubscriptionsInGroup(subGrp); + if (subscriptions == null || subscriptions.isEmpty()) { + // Trim subGrps with no more subscriptions; must have moved to another subGrp + configsIterator.remove(); + shouldWrite = true; + } + } + + if (shouldWrite) { + writeConfigsToDiskLocked(); + } + } + /** * Retrieves the list of subscription groups with configured VcnConfigs * - *

Limited to subscription groups for which the caller is carrier privileged. + *

Limited to subscription groups for which the caller had configured. * *

Implements the IVcnManagementService Binder interface. */ @@ -696,7 +792,8 @@ public class VcnManagementService extends IVcnManagementService.Stub { final List result = new ArrayList<>(); synchronized (mLock) { for (ParcelUuid subGrp : mConfigs.keySet()) { - if (mLastSnapshot.packageHasPermissionsForSubscriptionGroup(subGrp, opPkgName)) { + if (mLastSnapshot.packageHasPermissionsForSubscriptionGroup(subGrp, opPkgName) + || isProvisioningPackageForConfig(subGrp, opPkgName)) { result.add(subGrp); } } diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index bb98bc0bab536..54b3c400af4f1 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -65,6 +65,7 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkRequest; import android.net.TelephonyNetworkSpecifier; +import android.net.Uri; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; import android.net.vcn.VcnConfig; @@ -114,18 +115,24 @@ import java.util.UUID; public class VcnManagementServiceTest { private static final String TEST_PACKAGE_NAME = VcnManagementServiceTest.class.getPackage().getName(); + private static final String TEST_PACKAGE_NAME_2 = "TEST_PKG_2"; private static final String TEST_CB_PACKAGE_NAME = VcnManagementServiceTest.class.getPackage().getName() + ".callback"; private static final ParcelUuid TEST_UUID_1 = new ParcelUuid(new UUID(0, 0)); private static final ParcelUuid TEST_UUID_2 = new ParcelUuid(new UUID(1, 1)); + private static final ParcelUuid TEST_UUID_3 = new ParcelUuid(new UUID(2, 2)); private static final VcnConfig TEST_VCN_CONFIG; + private static final VcnConfig TEST_VCN_CONFIG_PKG_2; private static final int TEST_UID = Process.FIRST_APPLICATION_UID; static { final Context mockConfigContext = mock(Context.class); - doReturn(TEST_PACKAGE_NAME).when(mockConfigContext).getOpPackageName(); + doReturn(TEST_PACKAGE_NAME).when(mockConfigContext).getOpPackageName(); TEST_VCN_CONFIG = VcnConfigTest.buildTestConfig(mockConfigContext); + + doReturn(TEST_PACKAGE_NAME_2).when(mockConfigContext).getOpPackageName(); + TEST_VCN_CONFIG_PKG_2 = VcnConfigTest.buildTestConfig(mockConfigContext); } private static final Map TEST_VCN_CONFIG_MAP = @@ -246,18 +253,24 @@ public class VcnManagementServiceTest { eq(android.Manifest.permission.NETWORK_FACTORY), any()); } + private void setupMockedCarrierPrivilege(boolean isPrivileged) { + setupMockedCarrierPrivilege(isPrivileged, TEST_PACKAGE_NAME); + } + + private void setupMockedCarrierPrivilege(boolean isPrivileged, String pkg) { doReturn(Collections.singletonList(TEST_SUBSCRIPTION_INFO)) .when(mSubMgr) .getSubscriptionsInGroup(any()); doReturn(mTelMgr) .when(mTelMgr) .createForSubscriptionId(eq(TEST_SUBSCRIPTION_INFO.getSubscriptionId())); - doReturn(isPrivileged - ? CARRIER_PRIVILEGE_STATUS_HAS_ACCESS - : CARRIER_PRIVILEGE_STATUS_NO_ACCESS) + doReturn( + isPrivileged + ? CARRIER_PRIVILEGE_STATUS_HAS_ACCESS + : CARRIER_PRIVILEGE_STATUS_NO_ACCESS) .when(mTelMgr) - .checkCarrierPrivilegesForPackage(eq(TEST_PACKAGE_NAME)); + .checkCarrierPrivilegesForPackage(eq(pkg)); } @Test @@ -414,7 +427,13 @@ public class VcnManagementServiceTest { private BroadcastReceiver getPackageChangeReceiver() { final ArgumentCaptor captor = ArgumentCaptor.forClass(BroadcastReceiver.class); - verify(mMockContext).registerReceiver(captor.capture(), any(), any(), any()); + verify(mMockContext).registerReceiver(captor.capture(), argThat(filter -> { + return filter.hasAction(Intent.ACTION_PACKAGE_ADDED) + && filter.hasAction(Intent.ACTION_PACKAGE_REPLACED) + && filter.hasAction(Intent.ACTION_PACKAGE_REMOVED) + && filter.hasAction(Intent.ACTION_PACKAGE_DATA_CLEARED) + && filter.hasAction(Intent.ACTION_PACKAGE_FULLY_REMOVED); + }), any(), any()); return captor.getValue(); } @@ -538,6 +557,44 @@ public class VcnManagementServiceTest { verify(mSubscriptionTracker).handleSubscriptionsChanged(); } + @Test + public void testPackageChangeListener_packageDataCleared() throws Exception { + triggerSubscriptionTrackerCbAndGetSnapshot(TEST_UUID_1, Collections.singleton(TEST_UUID_1)); + final Vcn vcn = mVcnMgmtSvc.getAllVcns().get(TEST_UUID_1); + + final BroadcastReceiver receiver = getPackageChangeReceiver(); + assertEquals(TEST_VCN_CONFIG_MAP, mVcnMgmtSvc.getConfigs()); + + final Intent intent = new Intent(Intent.ACTION_PACKAGE_DATA_CLEARED); + intent.setData(Uri.parse("package:" + TEST_PACKAGE_NAME)); + intent.putExtra(Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(TEST_UID)); + + receiver.onReceive(mMockContext, intent); + mTestLooper.dispatchAll(); + verify(vcn).teardownAsynchronously(); + assertTrue(mVcnMgmtSvc.getConfigs().isEmpty()); + verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class)); + } + + @Test + public void testPackageChangeListener_packageFullyRemoved() throws Exception { + triggerSubscriptionTrackerCbAndGetSnapshot(TEST_UUID_1, Collections.singleton(TEST_UUID_1)); + final Vcn vcn = mVcnMgmtSvc.getAllVcns().get(TEST_UUID_1); + + final BroadcastReceiver receiver = getPackageChangeReceiver(); + assertEquals(TEST_VCN_CONFIG_MAP, mVcnMgmtSvc.getConfigs()); + + final Intent intent = new Intent(Intent.ACTION_PACKAGE_FULLY_REMOVED); + intent.setData(Uri.parse("package:" + TEST_PACKAGE_NAME)); + intent.putExtra(Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(TEST_UID)); + + receiver.onReceive(mMockContext, intent); + mTestLooper.dispatchAll(); + verify(vcn).teardownAsynchronously(); + assertTrue(mVcnMgmtSvc.getConfigs().isEmpty()); + verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class)); + } + @Test public void testSetVcnConfigRequiresNonSystemServer() throws Exception { doReturn(Process.SYSTEM_UID).when(mMockDeps).getBinderCallingUid(); @@ -578,7 +635,7 @@ public class VcnManagementServiceTest { @Test public void testSetVcnConfigMismatchedPackages() throws Exception { try { - mVcnMgmtSvc.setVcnConfig(TEST_UUID_1, TEST_VCN_CONFIG, "IncorrectPackage"); + mVcnMgmtSvc.setVcnConfig(TEST_UUID_1, TEST_VCN_CONFIG, TEST_PACKAGE_NAME_2); fail("Expected exception due to mismatched packages in config and method call"); } catch (IllegalArgumentException expected) { verify(mMockPolicyListener, never()).onPolicyChanged(); @@ -678,11 +735,12 @@ public class VcnManagementServiceTest { } @Test - public void testClearVcnConfigRequiresCarrierPrivileges() throws Exception { + public void testClearVcnConfigRequiresCarrierPrivilegesOrProvisioningPackage() + throws Exception { setupMockedCarrierPrivilege(false); try { - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME_2); fail("Expected security exception for missing carrier privileges"); } catch (SecurityException expected) { } @@ -691,19 +749,31 @@ public class VcnManagementServiceTest { @Test public void testClearVcnConfigMismatchedPackages() throws Exception { try { - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, "IncorrectPackage"); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME_2); fail("Expected security exception due to mismatched packages"); } catch (SecurityException expected) { } } @Test - public void testClearVcnConfig() throws Exception { + public void testClearVcnConfig_callerIsProvisioningPackage() throws Exception { + // Lose carrier privileges to test that provisioning package is sufficient. + setupMockedCarrierPrivilege(false); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME); assertTrue(mVcnMgmtSvc.getConfigs().isEmpty()); verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class)); } + @Test + public void testClearVcnConfig_callerIsCarrierPrivileged() throws Exception { + setupMockedCarrierPrivilege(true, TEST_PACKAGE_NAME_2); + + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME_2); + assertTrue(mVcnMgmtSvc.getConfigs().isEmpty()); + verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class)); + } + @Test public void testClearVcnConfigNotifiesStatusCallback() throws Exception { setupSubscriptionAndStartVcn(TEST_SUBSCRIPTION_ID, TEST_UUID_2, true /* isActive */); @@ -755,11 +825,12 @@ public class VcnManagementServiceTest { @Test public void testGetConfiguredSubscriptionGroupsMismatchedPackages() throws Exception { - final String badPackage = "IncorrectPackage"; - doThrow(new SecurityException()).when(mAppOpsMgr).checkPackage(TEST_UID, badPackage); + doThrow(new SecurityException()) + .when(mAppOpsMgr) + .checkPackage(TEST_UID, TEST_PACKAGE_NAME_2); try { - mVcnMgmtSvc.getConfiguredSubscriptionGroups(badPackage); + mVcnMgmtSvc.getConfiguredSubscriptionGroups(TEST_PACKAGE_NAME_2); fail("Expected security exception due to mismatched packages"); } catch (SecurityException expected) { } @@ -767,14 +838,16 @@ public class VcnManagementServiceTest { @Test public void testGetConfiguredSubscriptionGroups() throws Exception { + setupMockedCarrierPrivilege(true, TEST_PACKAGE_NAME_2); mVcnMgmtSvc.setVcnConfig(TEST_UUID_2, TEST_VCN_CONFIG, TEST_PACKAGE_NAME); + mVcnMgmtSvc.setVcnConfig(TEST_UUID_3, TEST_VCN_CONFIG_PKG_2, TEST_PACKAGE_NAME_2); - // Assert that if both UUID 1 and 2 are provisioned, the caller only gets ones that they are - // privileged for. + // Assert that if UUIDs 1, 2 and 3 are provisioned, the caller only gets ones that they are + // privileged for, or are the provisioning package of. triggerSubscriptionTrackerCbAndGetSnapshot(TEST_UUID_1, Collections.singleton(TEST_UUID_1)); final List subGrps = mVcnMgmtSvc.getConfiguredSubscriptionGroups(TEST_PACKAGE_NAME); - assertEquals(Collections.singletonList(TEST_UUID_1), subGrps); + assertEquals(Arrays.asList(new ParcelUuid[] {TEST_UUID_1, TEST_UUID_2}), subGrps); } @Test