Loading flags/messaging.aconfig +8 −1 Original line number Original line Diff line number Diff line package: "com.android.internal.telephony.flags" package: "com.android.internal.telephony.flags" flag { name: "reject_bad_sub_id_interaction" namespace: "telephony" description: "Previously, the DB allows insertion of a random sub Id, but doesn't allow query it. This change rejects such interaction." bug: "294125411" } No newline at end of file src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java +14 −23 Original line number Original line Diff line number Diff line Loading @@ -3646,7 +3646,7 @@ public class SubscriptionManagerService extends ISub.Stub { * else {@code false} if subscription is not associated with user. * else {@code false} if subscription is not associated with user. * * * @throws SecurityException if the caller doesn't have permissions required. * @throws SecurityException if the caller doesn't have permissions required. * * @throws IllegalArgumentException if the subscription has no records on device. */ */ @Override @Override public boolean isSubscriptionAssociatedWithUser(int subscriptionId, public boolean isSubscriptionAssociatedWithUser(int subscriptionId, Loading @@ -3656,18 +3656,11 @@ public class SubscriptionManagerService extends ISub.Stub { long token = Binder.clearCallingIdentity(); long token = Binder.clearCallingIdentity(); try { try { // Return true if there are no subscriptions on the device. // Throw IAE if no record of the sub's association state. List<SubscriptionInfo> subInfoList = getAllSubInfoList( if (mSubscriptionDatabaseManager.getSubscriptionInfoInternal(subscriptionId) == null) { mContext.getOpPackageName(), mContext.getAttributionTag()); throw new IllegalArgumentException( if (subInfoList == null || subInfoList.isEmpty()) { "[isSubscriptionAssociatedWithUser]: Subscription doesn't exist: " return true; + subscriptionId); } List<Integer> subIdList = subInfoList.stream().map(SubscriptionInfo::getSubscriptionId) .collect(Collectors.toList()); if (!subIdList.contains(subscriptionId)) { // Return true as this subscription is not available on the device. return true; } } // Get list of subscriptions associated with this user. // Get list of subscriptions associated with this user. Loading Loading @@ -3709,23 +3702,21 @@ public class SubscriptionManagerService extends ISub.Stub { long token = Binder.clearCallingIdentity(); long token = Binder.clearCallingIdentity(); try { try { List<SubscriptionInfo> subInfoList = getAllSubInfoList( List<SubscriptionInfoInternal> subInfoList = mSubscriptionDatabaseManager mContext.getOpPackageName(), mContext.getAttributionTag()); .getAllSubscriptions(); if (subInfoList == null || subInfoList.isEmpty()) { if (subInfoList.isEmpty()) { return new ArrayList<>(); return new ArrayList<>(); } } List<SubscriptionInfo> subscriptionsAssociatedWithUser = new ArrayList<>(); List<SubscriptionInfo> subscriptionsAssociatedWithUser = new ArrayList<>(); List<SubscriptionInfo> subscriptionsWithNoAssociation = new ArrayList<>(); List<SubscriptionInfo> subscriptionsWithNoAssociation = new ArrayList<>(); for (SubscriptionInfo subInfo : subInfoList) { for (SubscriptionInfoInternal subInfo : subInfoList) { int subId = subInfo.getSubscriptionId(); if (subInfo.getUserId() == userHandle.getIdentifier()) { UserHandle subIdUserHandle = getSubscriptionUserHandle(subId); if (userHandle.equals(subIdUserHandle)) { // Store subscriptions whose user handle matches with required user handle. // Store subscriptions whose user handle matches with required user handle. subscriptionsAssociatedWithUser.add(subInfo); subscriptionsAssociatedWithUser.add(subInfo.toSubscriptionInfo()); } else if (subIdUserHandle == null) { } else if (subInfo.getUserId() == UserHandle.USER_NULL) { // Store subscriptions whose user handle is set to null. // Store subscriptions whose user handle is set to null. subscriptionsWithNoAssociation.add(subInfo); subscriptionsWithNoAssociation.add(subInfo.toSubscriptionInfo()); } } } } Loading tests/telephonytests/src/com/android/internal/telephony/TelephonyPermissionsTest.java +41 −2 Original line number Original line Diff line number Diff line Loading @@ -48,6 +48,7 @@ import android.test.mock.MockContentProvider; import android.test.mock.MockContentResolver; import android.test.mock.MockContentResolver; import android.test.suitebuilder.annotation.SmallTest; import android.test.suitebuilder.annotation.SmallTest; import com.android.internal.telephony.flags.FeatureFlags; import com.android.internal.util.test.FakeSettingsProvider; import com.android.internal.util.test.FakeSettingsProvider; import com.android.server.pm.permission.LegacyPermissionManagerService; import com.android.server.pm.permission.LegacyPermissionManagerService; Loading @@ -57,7 +58,6 @@ import org.junit.Test; import java.lang.reflect.Field; import java.lang.reflect.Field; import java.util.Map; import java.util.Map; @SmallTest @SmallTest public class TelephonyPermissionsTest { public class TelephonyPermissionsTest { Loading @@ -71,6 +71,7 @@ public class TelephonyPermissionsTest { // Mocked classes // Mocked classes private Context mMockContext; private Context mMockContext; private FeatureFlags mMockFeatureFlag; private AppOpsManager mMockAppOps; private AppOpsManager mMockAppOps; private SubscriptionManager mMockSubscriptionManager; private SubscriptionManager mMockSubscriptionManager; private ITelephony mMockTelephony; private ITelephony mMockTelephony; Loading @@ -84,10 +85,12 @@ public class TelephonyPermissionsTest { private MockContentResolver mMockContentResolver; private MockContentResolver mMockContentResolver; private FakeSettingsConfigProvider mFakeSettingsConfigProvider; private FakeSettingsConfigProvider mFakeSettingsConfigProvider; private FeatureFlags mRealFeatureFlagToBeRestored; @Before @Before public void setUp() throws Exception { public void setUp() throws Exception { mMockContext = mock(Context.class); mMockContext = mock(Context.class); mMockFeatureFlag = mock(FeatureFlags.class); mMockAppOps = mock(AppOpsManager.class); mMockAppOps = mock(AppOpsManager.class); mMockSubscriptionManager = mock(SubscriptionManager.class); mMockSubscriptionManager = mock(SubscriptionManager.class); mMockTelephony = mock(ITelephony.class); mMockTelephony = mock(ITelephony.class); Loading Loading @@ -129,13 +132,17 @@ public class TelephonyPermissionsTest { .thenReturn(TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS); .thenReturn(TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS); when(mMockContext.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, when(mMockContext.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, PID, UID)).thenReturn(PackageManager.PERMISSION_DENIED); PID, UID)).thenReturn(PackageManager.PERMISSION_DENIED); replaceFeatureFlag(mMockFeatureFlag); setTelephonyMockAsService(); setTelephonyMockAsService(); } } @After @After public void tearDown() { public void tearDown() throws Exception { replaceFeatureFlag(mRealFeatureFlagToBeRestored); mMockContentResolver = null; mMockContentResolver = null; mFakeSettingsConfigProvider = null; mFakeSettingsConfigProvider = null; mRealFeatureFlagToBeRestored = null; } } @Test @Test Loading Loading @@ -540,6 +547,30 @@ public class TelephonyPermissionsTest { UserHandle.SYSTEM, "911")); UserHandle.SYSTEM, "911")); } } @Test public void testCheckSubscriptionAssociatedWithUser_badSub_flag_enabled() { doReturn(true).when(mMockFeatureFlag).rejectBadSubIdInteraction(); doThrow(new IllegalArgumentException("has no records on device")) .when(mMockSubscriptionManager).isSubscriptionAssociatedWithUser(SUB_ID, UserHandle.SYSTEM); assertFalse(TelephonyPermissions.checkSubscriptionAssociatedWithUser(mMockContext, SUB_ID, UserHandle.SYSTEM)); } @Test public void testCheckSubscriptionAssociatedWithUser_badSub_flag_disabled() { doReturn(false).when(mMockFeatureFlag).rejectBadSubIdInteraction(); doThrow(new IllegalArgumentException("No records found for sub")) .when(mMockSubscriptionManager).isSubscriptionAssociatedWithUser(SUB_ID, UserHandle.SYSTEM); assertTrue(TelephonyPermissions.checkSubscriptionAssociatedWithUser(mMockContext, SUB_ID, UserHandle.SYSTEM)); assertTrue(TelephonyPermissions.checkSubscriptionAssociatedWithUser(mMockContext, SubscriptionManager.INVALID_SUBSCRIPTION_ID, UserHandle.SYSTEM)); } // Put mMockTelephony into service cache so that TELEPHONY_SUPPLIER will get it. // Put mMockTelephony into service cache so that TELEPHONY_SUPPLIER will get it. private void setTelephonyMockAsService() throws Exception { private void setTelephonyMockAsService() throws Exception { when(mMockTelephonyBinder.queryLocalInterface(anyString())).thenReturn(mMockTelephony); when(mMockTelephonyBinder.queryLocalInterface(anyString())).thenReturn(mMockTelephony); Loading Loading @@ -630,4 +661,12 @@ public class TelephonyPermissionsTest { field.set(providerHolder, iContentProvider); field.set(providerHolder, iContentProvider); } } private synchronized void replaceFeatureFlag(final FeatureFlags newValue) throws Exception { Field field = TelephonyPermissions.class.getDeclaredField("sFeatureFlag"); field.setAccessible(true); mRealFeatureFlagToBeRestored = (FeatureFlags) field.get(null); field.set(null, newValue); } } } tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java +6 −2 Original line number Original line Diff line number Diff line Loading @@ -1113,8 +1113,6 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { @Test @Test public void testIsSubscriptionAssociatedWithUser() { public void testIsSubscriptionAssociatedWithUser() { insertSubscription(FAKE_SUBSCRIPTION_INFO1); // Should fail without MANAGE_SUBSCRIPTION_USER_ASSOCIATION // Should fail without MANAGE_SUBSCRIPTION_USER_ASSOCIATION assertThrows(SecurityException.class, () -> mSubscriptionManagerServiceUT assertThrows(SecurityException.class, () -> mSubscriptionManagerServiceUT .isSubscriptionAssociatedWithUser(1, FAKE_USER_HANDLE)); .isSubscriptionAssociatedWithUser(1, FAKE_USER_HANDLE)); Loading @@ -1125,6 +1123,12 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { Manifest.permission.MANAGE_SUBSCRIPTION_USER_ASSOCIATION); Manifest.permission.MANAGE_SUBSCRIPTION_USER_ASSOCIATION); mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE); mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE); // Should fail for non-existent sub Id assertThrows(IllegalArgumentException.class, () -> mSubscriptionManagerServiceUT .isSubscriptionAssociatedWithUser(1, FAKE_USER_HANDLE)); insertSubscription(FAKE_SUBSCRIPTION_INFO1); mSubscriptionManagerServiceUT.setSubscriptionUserHandle(FAKE_USER_HANDLE, 1); mSubscriptionManagerServiceUT.setSubscriptionUserHandle(FAKE_USER_HANDLE, 1); processAllMessages(); processAllMessages(); verify(mMockedSubscriptionManagerServiceCallback).onSubscriptionChanged(eq(1)); verify(mMockedSubscriptionManagerServiceCallback).onSubscriptionChanged(eq(1)); Loading Loading
flags/messaging.aconfig +8 −1 Original line number Original line Diff line number Diff line package: "com.android.internal.telephony.flags" package: "com.android.internal.telephony.flags" flag { name: "reject_bad_sub_id_interaction" namespace: "telephony" description: "Previously, the DB allows insertion of a random sub Id, but doesn't allow query it. This change rejects such interaction." bug: "294125411" } No newline at end of file
src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java +14 −23 Original line number Original line Diff line number Diff line Loading @@ -3646,7 +3646,7 @@ public class SubscriptionManagerService extends ISub.Stub { * else {@code false} if subscription is not associated with user. * else {@code false} if subscription is not associated with user. * * * @throws SecurityException if the caller doesn't have permissions required. * @throws SecurityException if the caller doesn't have permissions required. * * @throws IllegalArgumentException if the subscription has no records on device. */ */ @Override @Override public boolean isSubscriptionAssociatedWithUser(int subscriptionId, public boolean isSubscriptionAssociatedWithUser(int subscriptionId, Loading @@ -3656,18 +3656,11 @@ public class SubscriptionManagerService extends ISub.Stub { long token = Binder.clearCallingIdentity(); long token = Binder.clearCallingIdentity(); try { try { // Return true if there are no subscriptions on the device. // Throw IAE if no record of the sub's association state. List<SubscriptionInfo> subInfoList = getAllSubInfoList( if (mSubscriptionDatabaseManager.getSubscriptionInfoInternal(subscriptionId) == null) { mContext.getOpPackageName(), mContext.getAttributionTag()); throw new IllegalArgumentException( if (subInfoList == null || subInfoList.isEmpty()) { "[isSubscriptionAssociatedWithUser]: Subscription doesn't exist: " return true; + subscriptionId); } List<Integer> subIdList = subInfoList.stream().map(SubscriptionInfo::getSubscriptionId) .collect(Collectors.toList()); if (!subIdList.contains(subscriptionId)) { // Return true as this subscription is not available on the device. return true; } } // Get list of subscriptions associated with this user. // Get list of subscriptions associated with this user. Loading Loading @@ -3709,23 +3702,21 @@ public class SubscriptionManagerService extends ISub.Stub { long token = Binder.clearCallingIdentity(); long token = Binder.clearCallingIdentity(); try { try { List<SubscriptionInfo> subInfoList = getAllSubInfoList( List<SubscriptionInfoInternal> subInfoList = mSubscriptionDatabaseManager mContext.getOpPackageName(), mContext.getAttributionTag()); .getAllSubscriptions(); if (subInfoList == null || subInfoList.isEmpty()) { if (subInfoList.isEmpty()) { return new ArrayList<>(); return new ArrayList<>(); } } List<SubscriptionInfo> subscriptionsAssociatedWithUser = new ArrayList<>(); List<SubscriptionInfo> subscriptionsAssociatedWithUser = new ArrayList<>(); List<SubscriptionInfo> subscriptionsWithNoAssociation = new ArrayList<>(); List<SubscriptionInfo> subscriptionsWithNoAssociation = new ArrayList<>(); for (SubscriptionInfo subInfo : subInfoList) { for (SubscriptionInfoInternal subInfo : subInfoList) { int subId = subInfo.getSubscriptionId(); if (subInfo.getUserId() == userHandle.getIdentifier()) { UserHandle subIdUserHandle = getSubscriptionUserHandle(subId); if (userHandle.equals(subIdUserHandle)) { // Store subscriptions whose user handle matches with required user handle. // Store subscriptions whose user handle matches with required user handle. subscriptionsAssociatedWithUser.add(subInfo); subscriptionsAssociatedWithUser.add(subInfo.toSubscriptionInfo()); } else if (subIdUserHandle == null) { } else if (subInfo.getUserId() == UserHandle.USER_NULL) { // Store subscriptions whose user handle is set to null. // Store subscriptions whose user handle is set to null. subscriptionsWithNoAssociation.add(subInfo); subscriptionsWithNoAssociation.add(subInfo.toSubscriptionInfo()); } } } } Loading
tests/telephonytests/src/com/android/internal/telephony/TelephonyPermissionsTest.java +41 −2 Original line number Original line Diff line number Diff line Loading @@ -48,6 +48,7 @@ import android.test.mock.MockContentProvider; import android.test.mock.MockContentResolver; import android.test.mock.MockContentResolver; import android.test.suitebuilder.annotation.SmallTest; import android.test.suitebuilder.annotation.SmallTest; import com.android.internal.telephony.flags.FeatureFlags; import com.android.internal.util.test.FakeSettingsProvider; import com.android.internal.util.test.FakeSettingsProvider; import com.android.server.pm.permission.LegacyPermissionManagerService; import com.android.server.pm.permission.LegacyPermissionManagerService; Loading @@ -57,7 +58,6 @@ import org.junit.Test; import java.lang.reflect.Field; import java.lang.reflect.Field; import java.util.Map; import java.util.Map; @SmallTest @SmallTest public class TelephonyPermissionsTest { public class TelephonyPermissionsTest { Loading @@ -71,6 +71,7 @@ public class TelephonyPermissionsTest { // Mocked classes // Mocked classes private Context mMockContext; private Context mMockContext; private FeatureFlags mMockFeatureFlag; private AppOpsManager mMockAppOps; private AppOpsManager mMockAppOps; private SubscriptionManager mMockSubscriptionManager; private SubscriptionManager mMockSubscriptionManager; private ITelephony mMockTelephony; private ITelephony mMockTelephony; Loading @@ -84,10 +85,12 @@ public class TelephonyPermissionsTest { private MockContentResolver mMockContentResolver; private MockContentResolver mMockContentResolver; private FakeSettingsConfigProvider mFakeSettingsConfigProvider; private FakeSettingsConfigProvider mFakeSettingsConfigProvider; private FeatureFlags mRealFeatureFlagToBeRestored; @Before @Before public void setUp() throws Exception { public void setUp() throws Exception { mMockContext = mock(Context.class); mMockContext = mock(Context.class); mMockFeatureFlag = mock(FeatureFlags.class); mMockAppOps = mock(AppOpsManager.class); mMockAppOps = mock(AppOpsManager.class); mMockSubscriptionManager = mock(SubscriptionManager.class); mMockSubscriptionManager = mock(SubscriptionManager.class); mMockTelephony = mock(ITelephony.class); mMockTelephony = mock(ITelephony.class); Loading Loading @@ -129,13 +132,17 @@ public class TelephonyPermissionsTest { .thenReturn(TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS); .thenReturn(TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS); when(mMockContext.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, when(mMockContext.checkPermission(android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE, PID, UID)).thenReturn(PackageManager.PERMISSION_DENIED); PID, UID)).thenReturn(PackageManager.PERMISSION_DENIED); replaceFeatureFlag(mMockFeatureFlag); setTelephonyMockAsService(); setTelephonyMockAsService(); } } @After @After public void tearDown() { public void tearDown() throws Exception { replaceFeatureFlag(mRealFeatureFlagToBeRestored); mMockContentResolver = null; mMockContentResolver = null; mFakeSettingsConfigProvider = null; mFakeSettingsConfigProvider = null; mRealFeatureFlagToBeRestored = null; } } @Test @Test Loading Loading @@ -540,6 +547,30 @@ public class TelephonyPermissionsTest { UserHandle.SYSTEM, "911")); UserHandle.SYSTEM, "911")); } } @Test public void testCheckSubscriptionAssociatedWithUser_badSub_flag_enabled() { doReturn(true).when(mMockFeatureFlag).rejectBadSubIdInteraction(); doThrow(new IllegalArgumentException("has no records on device")) .when(mMockSubscriptionManager).isSubscriptionAssociatedWithUser(SUB_ID, UserHandle.SYSTEM); assertFalse(TelephonyPermissions.checkSubscriptionAssociatedWithUser(mMockContext, SUB_ID, UserHandle.SYSTEM)); } @Test public void testCheckSubscriptionAssociatedWithUser_badSub_flag_disabled() { doReturn(false).when(mMockFeatureFlag).rejectBadSubIdInteraction(); doThrow(new IllegalArgumentException("No records found for sub")) .when(mMockSubscriptionManager).isSubscriptionAssociatedWithUser(SUB_ID, UserHandle.SYSTEM); assertTrue(TelephonyPermissions.checkSubscriptionAssociatedWithUser(mMockContext, SUB_ID, UserHandle.SYSTEM)); assertTrue(TelephonyPermissions.checkSubscriptionAssociatedWithUser(mMockContext, SubscriptionManager.INVALID_SUBSCRIPTION_ID, UserHandle.SYSTEM)); } // Put mMockTelephony into service cache so that TELEPHONY_SUPPLIER will get it. // Put mMockTelephony into service cache so that TELEPHONY_SUPPLIER will get it. private void setTelephonyMockAsService() throws Exception { private void setTelephonyMockAsService() throws Exception { when(mMockTelephonyBinder.queryLocalInterface(anyString())).thenReturn(mMockTelephony); when(mMockTelephonyBinder.queryLocalInterface(anyString())).thenReturn(mMockTelephony); Loading Loading @@ -630,4 +661,12 @@ public class TelephonyPermissionsTest { field.set(providerHolder, iContentProvider); field.set(providerHolder, iContentProvider); } } private synchronized void replaceFeatureFlag(final FeatureFlags newValue) throws Exception { Field field = TelephonyPermissions.class.getDeclaredField("sFeatureFlag"); field.setAccessible(true); mRealFeatureFlagToBeRestored = (FeatureFlags) field.get(null); field.set(null, newValue); } } }
tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java +6 −2 Original line number Original line Diff line number Diff line Loading @@ -1113,8 +1113,6 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { @Test @Test public void testIsSubscriptionAssociatedWithUser() { public void testIsSubscriptionAssociatedWithUser() { insertSubscription(FAKE_SUBSCRIPTION_INFO1); // Should fail without MANAGE_SUBSCRIPTION_USER_ASSOCIATION // Should fail without MANAGE_SUBSCRIPTION_USER_ASSOCIATION assertThrows(SecurityException.class, () -> mSubscriptionManagerServiceUT assertThrows(SecurityException.class, () -> mSubscriptionManagerServiceUT .isSubscriptionAssociatedWithUser(1, FAKE_USER_HANDLE)); .isSubscriptionAssociatedWithUser(1, FAKE_USER_HANDLE)); Loading @@ -1125,6 +1123,12 @@ public class SubscriptionManagerServiceTest extends TelephonyTest { Manifest.permission.MANAGE_SUBSCRIPTION_USER_ASSOCIATION); Manifest.permission.MANAGE_SUBSCRIPTION_USER_ASSOCIATION); mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE); mContextFixture.addCallingOrSelfPermission(Manifest.permission.READ_PRIVILEGED_PHONE_STATE); // Should fail for non-existent sub Id assertThrows(IllegalArgumentException.class, () -> mSubscriptionManagerServiceUT .isSubscriptionAssociatedWithUser(1, FAKE_USER_HANDLE)); insertSubscription(FAKE_SUBSCRIPTION_INFO1); mSubscriptionManagerServiceUT.setSubscriptionUserHandle(FAKE_USER_HANDLE, 1); mSubscriptionManagerServiceUT.setSubscriptionUserHandle(FAKE_USER_HANDLE, 1); processAllMessages(); processAllMessages(); verify(mMockedSubscriptionManagerServiceCallback).onSubscriptionChanged(eq(1)); verify(mMockedSubscriptionManagerServiceCallback).onSubscriptionChanged(eq(1)); Loading