diff options
4 files changed, 41 insertions, 18 deletions
diff --git a/core/java/android/net/vcn/IVcnManagementService.aidl b/core/java/android/net/vcn/IVcnManagementService.aidl index 6a3cb42ed75d..5b79f7311b6d 100644 --- a/core/java/android/net/vcn/IVcnManagementService.aidl +++ b/core/java/android/net/vcn/IVcnManagementService.aidl @@ -29,7 +29,7 @@ import android.os.ParcelUuid; */ interface IVcnManagementService { void setVcnConfig(in ParcelUuid subscriptionGroup, in VcnConfig config, in String opPkgName); - void clearVcnConfig(in ParcelUuid subscriptionGroup); + void clearVcnConfig(in ParcelUuid subscriptionGroup, in String opPkgName); void addVcnUnderlyingNetworkPolicyListener(in IVcnUnderlyingNetworkPolicyListener listener); void removeVcnUnderlyingNetworkPolicyListener(in IVcnUnderlyingNetworkPolicyListener listener); diff --git a/core/java/android/net/vcn/VcnManager.java b/core/java/android/net/vcn/VcnManager.java index c0189e2202d2..abd41dacdeb6 100644 --- a/core/java/android/net/vcn/VcnManager.java +++ b/core/java/android/net/vcn/VcnManager.java @@ -154,7 +154,7 @@ public class VcnManager { requireNonNull(subscriptionGroup, "subscriptionGroup was null"); try { - mService.clearVcnConfig(subscriptionGroup); + mService.clearVcnConfig(subscriptionGroup, mContext.getOpPackageName()); } catch (ServiceSpecificException e) { throw new IOException(e); } catch (RemoteException e) { diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index d561ab96c365..642fc6bad578 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -349,7 +349,8 @@ public class VcnManagementService extends IVcnManagementService.Stub { } } - private void enforceCallingUserAndCarrierPrivilege(ParcelUuid subscriptionGroup) { + private void enforceCallingUserAndCarrierPrivilege( + ParcelUuid subscriptionGroup, String pkgName) { // 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. @@ -363,12 +364,15 @@ public class VcnManagementService extends IVcnManagementService.Stub { subscriptionInfos.addAll(subMgr.getSubscriptionsInGroup(subscriptionGroup)); }); - final TelephonyManager telMgr = mContext.getSystemService(TelephonyManager.class); for (SubscriptionInfo info : subscriptionInfos) { + final TelephonyManager telMgr = mContext.getSystemService(TelephonyManager.class) + .createForSubscriptionId(info.getSubscriptionId()); + // Check subscription is active first; much cheaper/faster check, and an app (currently) // cannot be carrier privileged for inactive subscriptions. if (subMgr.isValidSlotIndex(info.getSimSlotIndex()) - && telMgr.hasCarrierPrivileges(info.getSubscriptionId())) { + && telMgr.checkCarrierPrivilegesForPackage(pkgName) + == TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS) { // TODO (b/173717728): Allow configuration for inactive, but manageable // subscriptions. // TODO (b/173718661): Check for whole subscription groups at a time. @@ -536,7 +540,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { mContext.getSystemService(AppOpsManager.class) .checkPackage(mDeps.getBinderCallingUid(), config.getProvisioningPackageName()); - enforceCallingUserAndCarrierPrivilege(subscriptionGroup); + enforceCallingUserAndCarrierPrivilege(subscriptionGroup, opPkgName); Binder.withCleanCallingIdentity(() -> { synchronized (mLock) { @@ -554,11 +558,14 @@ public class VcnManagementService extends IVcnManagementService.Stub { * <p>Implements the IVcnManagementService Binder interface. */ @Override - public void clearVcnConfig(@NonNull ParcelUuid subscriptionGroup) { + public void clearVcnConfig(@NonNull ParcelUuid subscriptionGroup, @NonNull String opPkgName) { requireNonNull(subscriptionGroup, "subscriptionGroup was null"); + requireNonNull(opPkgName, "opPkgName was null"); Slog.v(TAG, "VCN config cleared for subGrp: " + subscriptionGroup); - enforceCallingUserAndCarrierPrivilege(subscriptionGroup); + mContext.getSystemService(AppOpsManager.class) + .checkPackage(mDeps.getBinderCallingUid(), opPkgName); + enforceCallingUserAndCarrierPrivilege(subscriptionGroup, opPkgName); Binder.withCleanCallingIdentity(() -> { synchronized (mLock) { diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index a9d5822be226..babea364edbe 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -19,6 +19,8 @@ package com.android.server; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; +import static android.telephony.TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS; +import static android.telephony.TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS; import static com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import static com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionTrackerCallback; @@ -238,9 +240,14 @@ public class VcnManagementServiceTest { doReturn(Collections.singletonList(TEST_SUBSCRIPTION_INFO)) .when(mSubMgr) .getSubscriptionsInGroup(any()); - doReturn(isPrivileged) + doReturn(mTelMgr) .when(mTelMgr) - .hasCarrierPrivileges(eq(TEST_SUBSCRIPTION_INFO.getSubscriptionId())); + .createForSubscriptionId(eq(TEST_SUBSCRIPTION_INFO.getSubscriptionId())); + doReturn(isPrivileged + ? CARRIER_PRIVILEGE_STATUS_HAS_ACCESS + : CARRIER_PRIVILEGE_STATUS_NO_ACCESS) + .when(mTelMgr) + .checkCarrierPrivilegesForPackage(eq(TEST_PACKAGE_NAME)); } @Test @@ -391,7 +398,7 @@ public class VcnManagementServiceTest { mTestLooper.moveTimeForward( VcnManagementService.CARRIER_PRIVILEGES_LOST_TEARDOWN_DELAY_MS / 2); mTestLooper.dispatchAll(); - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2, TEST_PACKAGE_NAME); final Vcn newInstance = startAndGetVcnInstance(TEST_UUID_2); // Verify that new instance was different, and the old one was torn down @@ -492,7 +499,7 @@ public class VcnManagementServiceTest { doReturn(Process.SYSTEM_UID).when(mMockDeps).getBinderCallingUid(); try { - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME); fail("Expected IllegalStateException exception for system server"); } catch (IllegalStateException expected) { } @@ -505,7 +512,7 @@ public class VcnManagementServiceTest { .getBinderCallingUid(); try { - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME); fail("Expected security exception for non system user"); } catch (SecurityException expected) { } @@ -516,15 +523,24 @@ public class VcnManagementServiceTest { setupMockedCarrierPrivilege(false); try { - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME); fail("Expected security exception for missing carrier privileges"); } catch (SecurityException expected) { } } @Test + public void testClearVcnConfigMismatchedPackages() throws Exception { + try { + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, "IncorrectPackage"); + fail("Expected security exception due to mismatched packages"); + } catch (SecurityException expected) { + } + } + + @Test public void testClearVcnConfig() throws Exception { - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_1, TEST_PACKAGE_NAME); assertTrue(mVcnMgmtSvc.getConfigs().isEmpty()); verify(mConfigReadWriteHelper).writeToDisk(any(PersistableBundle.class)); } @@ -535,7 +551,7 @@ public class VcnManagementServiceTest { mVcnMgmtSvc.registerVcnStatusCallback(TEST_UUID_2, mMockStatusCallback, TEST_PACKAGE_NAME); verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_ACTIVE); - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2, TEST_PACKAGE_NAME); verify(mMockStatusCallback).onVcnStatusChanged(VcnManager.VCN_STATUS_CODE_NOT_CONFIGURED); } @@ -564,7 +580,7 @@ public class VcnManagementServiceTest { verify(vcnInstance).updateConfig(TEST_VCN_CONFIG); // Verify Vcn is stopped if it was already started - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2, TEST_PACKAGE_NAME); verify(vcnInstance).teardownAsynchronously(); } @@ -781,7 +797,7 @@ public class VcnManagementServiceTest { mVcnMgmtSvc.setVcnConfig(TEST_UUID_2, TEST_VCN_CONFIG, TEST_PACKAGE_NAME); mVcnMgmtSvc.addVcnUnderlyingNetworkPolicyListener(mMockPolicyListener); - mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2); + mVcnMgmtSvc.clearVcnConfig(TEST_UUID_2, TEST_PACKAGE_NAME); verify(mMockPolicyListener).onPolicyChanged(); } |