commit d2b004b55f40bfc682510d57c72686a67f4cf913
author Rambo Wang <rambowang@google.com> Tue Mar 30 10:10:23 2021 -0700
committer Rambo Wang <rambowang@google.com> Wed Apr 07 15:25:31 2021 -0700
tree b3370aa04c4f65b8e921fa797fbd9f3f4c93eecb
parent 7e3bc126a66b440d181ac7c84586c12f5387b29d

CarrierConfigLoader: IPC call with invalid subId will silently fail

When invalid subId/phoneId is provided to IPC call methods below,
the methods direct return, with a line of log. This will make the
failure silently ignored to the client side:
- overrideConfig
- notifyConfigChangedForSubId
- updateConfigForPhoneId

To fix them, IAE or ISE are thrown to notify the client the failure.

Bug: 184001819
Test: atest com.android.phone.CarrierConfigLoaderTest
Test: atest CtsVcnTestCases: android.net.vcn.cts.VcnManagerTest -- --abi
x86_64

Change-Id: I64202ace126e1b6537fed96ab1ccaf487d29ac1f

src/com/android/phone/CarrierConfigLoader.java

diff --git a/src/com/android/phone/CarrierConfigLoader.java b/src/com/android/phone/CarrierConfigLoader.java
index 3d9e0b0..76a16d3 100644
--- a/src/com/android/phone/CarrierConfigLoader.java
+++ b/src/com/android/phone/CarrierConfigLoader.java
@@ -1178,20 +1178,20 @@
 
     @Override
     @NonNull
-    public PersistableBundle getConfigForSubId(int subId, String callingPackage) {
-        return getConfigForSubIdWithFeature(subId, callingPackage, null);
+    public PersistableBundle getConfigForSubId(int subscriptionId, String callingPackage) {
+        return getConfigForSubIdWithFeature(subscriptionId, callingPackage, null);
     }
 
     @Override
     @NonNull
-    public PersistableBundle getConfigForSubIdWithFeature(int subId, String callingPackage,
+    public PersistableBundle getConfigForSubIdWithFeature(int subscriptionId, String callingPackage,
             String callingFeatureId) {
-        if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(mContext, subId, callingPackage,
-                callingFeatureId, "getCarrierConfig")) {
+        if (!TelephonyPermissions.checkCallingOrSelfReadPhoneState(mContext, subscriptionId,
+                callingPackage, callingFeatureId, "getCarrierConfig")) {
             return new PersistableBundle();
         }
 
-        int phoneId = SubscriptionManager.getPhoneId(subId);
+        int phoneId = SubscriptionManager.getPhoneId(subscriptionId);
         PersistableBundle retConfig = CarrierConfigManager.getDefaultConfig();
         if (SubscriptionManager.isValidPhoneId(phoneId)) {
             PersistableBundle config = mConfigFromDefaultApp[phoneId];
@@ -1235,7 +1235,8 @@
         int phoneId = SubscriptionManager.getPhoneId(subscriptionId);
         if (!SubscriptionManager.isValidPhoneId(phoneId)) {
             logd("Ignore invalid phoneId: " + phoneId + " for subId: " + subscriptionId);
-            return;
+            throw new IllegalArgumentException(
+                    "Invalid phoneId " + phoneId + " for subId " + subscriptionId);
         }
         // Post to run on handler thread on which all states should be confined.
         mHandler.post(() -> {
@@ -1274,17 +1275,18 @@
     }
 
     @Override
-    public void notifyConfigChangedForSubId(int subId) {
-        int phoneId = SubscriptionManager.getPhoneId(subId);
-        if (!SubscriptionManager.isValidPhoneId(phoneId)) {
-            logd("Ignore invalid phoneId: " + phoneId + " for subId: " + subId);
-            return;
-        }
-
+    public void notifyConfigChangedForSubId(int subscriptionId) {
         // Requires the calling app to be either a carrier privileged app for this subId or
         // system privileged app with MODIFY_PHONE_STATE permission.
-        TelephonyPermissions.enforceCallingOrSelfModifyPermissionOrCarrierPrivilege(mContext, subId,
-                "Require carrier privileges or MODIFY_PHONE_STATE permission.");
+        TelephonyPermissions.enforceCallingOrSelfModifyPermissionOrCarrierPrivilege(mContext,
+                subscriptionId, "Require carrier privileges or MODIFY_PHONE_STATE permission.");
+
+        int phoneId = SubscriptionManager.getPhoneId(subscriptionId);
+        if (!SubscriptionManager.isValidPhoneId(phoneId)) {
+            logd("Ignore invalid phoneId: " + phoneId + " for subId: " + subscriptionId);
+            throw new IllegalArgumentException(
+                    "Invalid phoneId " + phoneId + " for subId " + subscriptionId);
+        }
 
         // This method should block until deleting has completed, so that an error which prevents us
         // from clearing the cache is passed back to the carrier app. With the files successfully
@@ -1301,7 +1303,7 @@
                 android.Manifest.permission.MODIFY_PHONE_STATE, null);
         logdWithLocalLog("Update config for phoneId: " + phoneId + " simState: " + simState);
         if (!SubscriptionManager.isValidPhoneId(phoneId)) {
-            return;
+            throw new IllegalArgumentException("Invalid phoneId: " + phoneId);
         }
         // requires Java 7 for switch on string.
         switch (simState) {