Check CALL_PHONE and MANAGE_OWN_CALLS permission in placeCall
Staticaly check CALL_PHONE and MANAGE_OWN_CALLS depending on the
PhoneAccountHandle package name passed in instead of taking into
account whether or not the PhoneAccountHandle is associated with
a registered self-managed ConnectionService. This makes sure
we do not leak Telecom state with respect to which PhoneAccounts
are registered back to the caller.
Bug: 231988638
Test: atest TelecomUnitTests
Change-Id: Ie5eb859f79df22fc4710a0a745e8b6bd9b48aeb9
diff --git a/src/com/android/server/telecom/TelecomServiceImpl.java b/src/com/android/server/telecom/TelecomServiceImpl.java
index f5dd786..f2bd00f 100644
--- a/src/com/android/server/telecom/TelecomServiceImpl.java
+++ b/src/com/android/server/telecom/TelecomServiceImpl.java
@@ -1506,7 +1506,6 @@
enforceCallingPackage(callingPackage, "placeCall");
PhoneAccountHandle phoneAccountHandle = null;
- boolean clearPhoneAccountHandleExtra = false;
if (extras != null) {
phoneAccountHandle = extras.getParcelable(
TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE);
@@ -1515,32 +1514,28 @@
extras.remove(TelecomManager.EXTRA_IS_HANDOVER);
}
}
- boolean isSelfManaged = phoneAccountHandle != null &&
- isSelfManagedConnectionService(phoneAccountHandle);
- if (isSelfManaged) {
- try {
- mContext.enforceCallingOrSelfPermission(
- Manifest.permission.MANAGE_OWN_CALLS,
- "Self-managed ConnectionServices require "
- + "MANAGE_OWN_CALLS permission.");
- } catch (SecurityException e) {
- // Fallback to use mobile network to avoid disclosing phone account handle
- // package information
- clearPhoneAccountHandleExtra = true;
- }
+ ComponentName componentName = phoneAccountHandle != null
+ ? phoneAccountHandle.getComponentName() : null;
+ String packageName = componentName != null
+ ? componentName.getPackageName() : null;
- if (!clearPhoneAccountHandleExtra && !callingPackage.equals(
- phoneAccountHandle.getComponentName().getPackageName())
- && !canCallPhone(callingPackage, callingFeatureId,
- "CALL_PHONE permission required to place calls.")) {
- // The caller is not allowed to place calls, so fallback to use mobile
- // network.
- clearPhoneAccountHandleExtra = true;
- }
- } else if (!canCallPhone(callingPackage, callingFeatureId, "placeCall")) {
- throw new SecurityException("Package " + callingPackage
- + " is not allowed to place phone calls");
+ // Two cases here: the app calling this API is trying to place a call on another
+ // ConnectionService or the app calling this API implements a self-managed
+ // ConnectionService and is trying to place a call on their own ConnectionService.
+ // Case 1: If the app does not implement the ConnectionService they are requesting
+ // the call be placed on, ensure they have the correct CALL_PHONE permissions.
+ if (!callingPackage.equals(packageName) && !canCallPhone(callingPackage,
+ callingFeatureId, "CALL_PHONE permission required to place calls.")) {
+ throw new SecurityException("CALL_PHONE permission required to place calls.");
}
+ // Case 2: The package name of the caller matches the package name of the
+ // PhoneAccountHandle, so ensure the app has MANAGE_OWN_CALLS permission.
+ if (callingPackage.equals(packageName)) {
+ mContext.enforceCallingOrSelfPermission(Manifest.permission.MANAGE_OWN_CALLS,
+ "Self-managed ConnectionServices require MANAGE_OWN_CALLS permission.");
+ }
+
+ boolean isSelfManaged = isSelfManagedConnectionService(phoneAccountHandle);
// Note: we can still get here for the default/system dialer, even if the Phone
// permission is turned off. This is because the default/system dialer is always
@@ -1570,9 +1565,6 @@
final Intent intent = new Intent(hasCallPrivilegedPermission ?
Intent.ACTION_CALL_PRIVILEGED : Intent.ACTION_CALL, handle);
if (extras != null) {
- if (clearPhoneAccountHandleExtra) {
- extras.remove(TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE);
- }
extras.setDefusable(true);
intent.putExtras(extras);
}
diff --git a/tests/src/com/android/server/telecom/tests/TelecomServiceImplTest.java b/tests/src/com/android/server/telecom/tests/TelecomServiceImplTest.java
index 210f80e..21028cc 100644
--- a/tests/src/com/android/server/telecom/tests/TelecomServiceImplTest.java
+++ b/tests/src/com/android/server/telecom/tests/TelecomServiceImplTest.java
@@ -23,6 +23,7 @@
import static android.Manifest.permission.READ_PHONE_STATE;
import static android.Manifest.permission.READ_PRIVILEGED_PHONE_STATE;
+import android.Manifest;
import android.app.ActivityManager;
import android.app.AppOpsManager;
import android.content.ComponentName;
@@ -756,12 +757,139 @@
}
}
+
+ @SmallTest
+ @Test
+ public void testPlaceCallNoPermission_SelfManaged() throws Exception {
+ doReturn(false).when(mDefaultDialerCache).isDefaultOrSystemDialer(
+ eq(DEFAULT_DIALER_PACKAGE), anyInt());
+ when(mFakePhoneAccountRegistrar.getPhoneAccountUnchecked(TEL_PA_HANDLE_CURRENT)).thenReturn(
+ makeSelfManagedPhoneAccount(TEL_PA_HANDLE_CURRENT).build());
+ Uri handle = Uri.parse("tel:6505551234");
+ Bundle extras = createSampleExtras();
+ // callingPackage matches the PhoneAccountHandle, so this is an app with a self-managed
+ // ConnectionService.
+ extras.putParcelable(TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE, TEL_PA_HANDLE_CURRENT);
+
+ doThrow(new SecurityException()).when(mContext).enforceCallingOrSelfPermission(
+ eq(Manifest.permission.MANAGE_OWN_CALLS), anyString());
+
+ try {
+ mTSIBinder.placeCall(handle, extras, PACKAGE_NAME, null);
+ fail("Expected SecurityException because MANAGE_OWN_CALLS is not set");
+ } catch(SecurityException e) {
+ // expected
+ }
+ }
+
+ @SmallTest
+ @Test
+ public void testPlaceCallNoCallPhonePermission_Managed() throws Exception {
+ doReturn(false).when(mDefaultDialerCache).isDefaultOrSystemDialer(
+ eq(DEFAULT_DIALER_PACKAGE), anyInt());
+ when(mFakePhoneAccountRegistrar.getPhoneAccountUnchecked(TEL_PA_HANDLE_CURRENT)).thenReturn(
+ makeSelfManagedPhoneAccount(TEL_PA_HANDLE_CURRENT).build());
+ Uri handle = Uri.parse("tel:6505551234");
+ Bundle extras = createSampleExtras();
+ // callingPackage doesn't match the PhoneAccountHandle, so this app is not managing the
+ //ConnectionService
+ extras.putParcelable(TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE, TEL_PA_HANDLE_CURRENT);
+
+ // Since the packages do not match, the caller needs CALL_PHONE permission
+ doThrow(new SecurityException()).when(mContext).enforceCallingOrSelfPermission(
+ eq(CALL_PHONE), anyString());
+
+ try {
+ mTSIBinder.placeCall(handle, extras, DEFAULT_DIALER_PACKAGE, null);
+ fail("Expected SecurityException because CALL_PHONE is not set");
+ } catch(SecurityException e) {
+ // expected
+ }
+ }
+
+ @SmallTest
+ @Test
+ public void testPlaceCallNoCallPhoneAppOp_Managed() throws Exception {
+ doReturn(false).when(mDefaultDialerCache).isDefaultOrSystemDialer(
+ eq(DEFAULT_DIALER_PACKAGE), anyInt());
+ when(mFakePhoneAccountRegistrar.getPhoneAccountUnchecked(TEL_PA_HANDLE_CURRENT)).thenReturn(
+ makeSelfManagedPhoneAccount(TEL_PA_HANDLE_CURRENT).build());
+ Uri handle = Uri.parse("tel:6505551234");
+ Bundle extras = createSampleExtras();
+ // callingPackage doesn't match the PhoneAccountHandle, so this app is not managing the
+ //ConnectionService
+ extras.putParcelable(TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE, TEL_PA_HANDLE_CURRENT);
+
+ // Since the packages do not match, the caller needs CALL_PHONE app op
+ when(mAppOpsManager.noteOp(eq(AppOpsManager.OP_CALL_PHONE), anyInt(), anyString(),
+ nullable(String.class), nullable(String.class)))
+ .thenReturn(AppOpsManager.MODE_IGNORED);
+
+ try {
+ mTSIBinder.placeCall(handle, extras, DEFAULT_DIALER_PACKAGE, null);
+ fail("Expected SecurityException because CALL_PHONE is not set");
+ } catch(SecurityException e) {
+ // expected
+ }
+ }
+
+ @SmallTest
+ @Test
+ public void testPlaceCallWithNonEmergencyPermission_SelfManaged() throws Exception {
+ doReturn(false).when(mDefaultDialerCache).isDefaultOrSystemDialer(
+ eq(DEFAULT_DIALER_PACKAGE), anyInt());
+ when(mFakePhoneAccountRegistrar.getPhoneAccountUnchecked(TEL_PA_HANDLE_CURRENT)).thenReturn(
+ makeSelfManagedPhoneAccount(TEL_PA_HANDLE_CURRENT).build());
+ Uri handle = Uri.parse("tel:6505551234");
+ Bundle extras = createSampleExtras();
+ // callingPackage matches the PhoneAccountHandle, so this is an app with a self-managed
+ // ConnectionService.
+ extras.putParcelable(TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE, TEL_PA_HANDLE_CURRENT);
+
+ // enforceCallingOrSelfPermission is implicitly granted for MANAGE_OWN_CALLS here and
+ // CALL_PHONE is not required.
+ when(mAppOpsManager.noteOp(eq(AppOpsManager.OP_CALL_PHONE), anyInt(), anyString(),
+ nullable(String.class), nullable(String.class)))
+ .thenReturn(AppOpsManager.MODE_IGNORED);
+ doReturn(PackageManager.PERMISSION_DENIED)
+ .when(mContext).checkCallingPermission(CALL_PHONE);
+ doReturn(PackageManager.PERMISSION_DENIED)
+ .when(mContext).checkCallingPermission(CALL_PRIVILEGED);
+
+ mTSIBinder.placeCall(handle, extras, PACKAGE_NAME, null);
+ placeCallTestHelper(handle, extras, true);
+ }
+
+ @SmallTest
+ @Test
+ public void testPlaceCallWithNonEmergencyPermission_Managed() throws Exception {
+ doReturn(false).when(mDefaultDialerCache).isDefaultOrSystemDialer(
+ eq(DEFAULT_DIALER_PACKAGE), anyInt());
+ Uri handle = Uri.parse("tel:6505551234");
+ Bundle extras = createSampleExtras();
+ // callingPackage doesn't match the PhoneAccountHandle, so this app does not have a
+ // self-managed ConnectionService
+ extras.putParcelable(TelecomManager.EXTRA_PHONE_ACCOUNT_HANDLE, TEL_PA_HANDLE_CURRENT);
+
+ when(mAppOpsManager.noteOp(eq(AppOpsManager.OP_CALL_PHONE), anyInt(), anyString(),
+ nullable(String.class), nullable(String.class)))
+ .thenReturn(AppOpsManager.MODE_ALLOWED);
+ doReturn(PackageManager.PERMISSION_GRANTED)
+ .when(mContext).checkCallingPermission(CALL_PHONE);
+ doReturn(PackageManager.PERMISSION_DENIED)
+ .when(mContext).checkCallingPermission(CALL_PRIVILEGED);
+
+ mTSIBinder.placeCall(handle, extras, DEFAULT_DIALER_PACKAGE, null);
+ placeCallTestHelper(handle, extras, true);
+ }
+
@SmallTest
@Test
public void testPlaceCallWithNonEmergencyPermission() throws Exception {
Uri handle = Uri.parse("tel:6505551234");
Bundle extras = createSampleExtras();
+ // We are assumed to be default dialer in this test, so canCallPhone is always true.
when(mAppOpsManager.noteOp(eq(AppOpsManager.OP_CALL_PHONE), anyInt(), anyString(),
nullable(String.class), nullable(String.class)))
.thenReturn(AppOpsManager.MODE_ALLOWED);
@@ -780,6 +908,7 @@
Uri handle = Uri.parse("tel:6505551234");
Bundle extras = createSampleExtras();
+ // We are assumed to be default dialer in this test, so canCallPhone is always true.
when(mAppOpsManager.noteOp(eq(AppOpsManager.OP_CALL_PHONE), anyInt(), anyString(),
nullable(String.class), nullable(String.class)))
.thenReturn(AppOpsManager.MODE_IGNORED);
@@ -798,6 +927,7 @@
Uri handle = Uri.parse("tel:6505551234");
Bundle extras = createSampleExtras();
+ // We are assumed to be default dialer in this test, so canCallPhone is always true.
when(mAppOpsManager.noteOp(eq(AppOpsManager.OP_CALL_PHONE), anyInt(), anyString(),
nullable(String.class), nullable(String.class)))
.thenReturn(AppOpsManager.MODE_ALLOWED);
@@ -1284,6 +1414,12 @@
return paBuilder;
}
+ private PhoneAccount.Builder makeSelfManagedPhoneAccount(PhoneAccountHandle paHandle) {
+ PhoneAccount.Builder paBuilder = makePhoneAccount(paHandle);
+ paBuilder.setCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED);
+ return paBuilder;
+ }
+
private PhoneAccount.Builder makePhoneAccount(PhoneAccountHandle paHandle) {
return new PhoneAccount.Builder(paHandle, "testLabel");
}