Add a slew of clearCallingIdentity() invocations to telecom.
We used to use Handlers to move things onto the telecom main thread when
they came in through Binder calls. This also had the effect of
switching to telecom's calling identity.
Since we no longer do that, we need to manually reset the calling
identity to avoid the permissions failures that we've been seeing
lately. This change adds all the checks that are necessary today and
checks which would help down the road if any code changes.
Bug: 20830275
Change-Id: I73e3e2d310a505ac96abe3aed7f2118346c7be1f
diff --git a/src/com/android/server/telecom/BluetoothPhoneServiceImpl.java b/src/com/android/server/telecom/BluetoothPhoneServiceImpl.java
index dd2e5dd..ad3e10d 100644
--- a/src/com/android/server/telecom/BluetoothPhoneServiceImpl.java
+++ b/src/com/android/server/telecom/BluetoothPhoneServiceImpl.java
@@ -25,6 +25,7 @@
import android.content.Intent;
import android.content.IntentFilter;
import android.net.Uri;
+import android.os.Binder;
import android.os.IBinder;
import android.os.RemoteException;
import android.telecom.Connection;
@@ -83,13 +84,20 @@
public boolean answerCall() throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "BT - answering call");
- Call call = mCallsManager.getRingingCall();
- if (call != null) {
- mCallsManager.answerCall(call, 0);
- return true;
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "BT - answering call");
+ Call call = mCallsManager.getRingingCall();
+ if (call != null) {
+ mCallsManager.answerCall(call, 0);
+ return true;
+ }
+ return false;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
- return false;
+
}
}
@@ -97,13 +105,19 @@
public boolean hangupCall() throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "BT - hanging up call");
- Call call = mCallsManager.getForegroundCall();
- if (call != null) {
- mCallsManager.disconnectCall(call);
- return true;
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "BT - hanging up call");
+ Call call = mCallsManager.getForegroundCall();
+ if (call != null) {
+ mCallsManager.disconnectCall(call);
+ return true;
+ }
+ return false;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
- return false;
}
}
@@ -111,16 +125,22 @@
public boolean sendDtmf(int dtmf) throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "BT - sendDtmf %c", Log.DEBUG ? dtmf : '.');
- Call call = mCallsManager.getForegroundCall();
- if (call != null) {
- // TODO: Consider making this a queue instead of starting/stopping
- // in quick succession.
- mCallsManager.playDtmfTone(call, (char) dtmf);
- mCallsManager.stopDtmfTone(call);
- return true;
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "BT - sendDtmf %c", Log.DEBUG ? dtmf : '.');
+ Call call = mCallsManager.getForegroundCall();
+ if (call != null) {
+ // TODO: Consider making this a queue instead of starting/stopping
+ // in quick succession.
+ mCallsManager.playDtmfTone(call, (char) dtmf);
+ mCallsManager.stopDtmfTone(call);
+ return true;
+ }
+ return false;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
- return false;
}
}
@@ -128,14 +148,20 @@
public String getNetworkOperator() throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "getNetworkOperator");
- PhoneAccount account = getBestPhoneAccount();
- if (account != null) {
- return account.getLabel().toString();
- } else {
- // Finally, just get the network name from telephony.
- return TelephonyManager.from(mContext)
- .getNetworkOperatorName();
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "getNetworkOperator");
+ PhoneAccount account = getBestPhoneAccount();
+ if (account != null) {
+ return account.getLabel().toString();
+ } else {
+ // Finally, just get the network name from telephony.
+ return TelephonyManager.from(mContext)
+ .getNetworkOperatorName();
+ }
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
}
}
@@ -144,19 +170,25 @@
public String getSubscriberNumber() throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "getSubscriberNumber");
- String address = null;
- PhoneAccount account = getBestPhoneAccount();
- if (account != null) {
- Uri addressUri = account.getAddress();
- if (addressUri != null) {
- address = addressUri.getSchemeSpecificPart();
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "getSubscriberNumber");
+ String address = null;
+ PhoneAccount account = getBestPhoneAccount();
+ if (account != null) {
+ Uri addressUri = account.getAddress();
+ if (addressUri != null) {
+ address = addressUri.getSchemeSpecificPart();
+ }
}
+ if (TextUtils.isEmpty(address)) {
+ address = TelephonyManager.from(mContext).getLine1Number();
+ }
+ return address;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
- if (TextUtils.isEmpty(address)) {
- address = TelephonyManager.from(mContext).getLine1Number();
- }
- return address;
}
}
@@ -164,17 +196,23 @@
public boolean listCurrentCalls() throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- // only log if it is after we recently updated the headset state or else it can clog
- // the android log since this can be queried every second.
- boolean logQuery = mHeadsetUpdatedRecently;
- mHeadsetUpdatedRecently = false;
- if (logQuery) {
- Log.i(TAG, "listcurrentCalls");
+ long token = Binder.clearCallingIdentity();
+ try {
+ // only log if it is after we recently updated the headset state or else it can
+ // clog the android log since this can be queried every second.
+ boolean logQuery = mHeadsetUpdatedRecently;
+ mHeadsetUpdatedRecently = false;
+
+ if (logQuery) {
+ Log.i(TAG, "listcurrentCalls");
+ }
+
+ sendListOfCalls(logQuery);
+ return true;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
-
- sendListOfCalls(logQuery);
- return true;
}
}
@@ -182,9 +220,15 @@
public boolean queryPhoneState() throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "queryPhoneState");
- updateHeadsetWithCallState(true /* force */);
- return true;
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "queryPhoneState");
+ updateHeadsetWithCallState(true /* force */);
+ return true;
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
@@ -192,8 +236,14 @@
public boolean processChld(int chld) throws RemoteException {
synchronized (mLock) {
enforceModifyPermission();
- Log.i(TAG, "processChld %d", chld);
- return BluetoothPhoneServiceImpl.this.processChld(chld);
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ Log.i(TAG, "processChld %d", chld);
+ return BluetoothPhoneServiceImpl.this.processChld(chld);
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
diff --git a/src/com/android/server/telecom/TelecomServiceImpl.java b/src/com/android/server/telecom/TelecomServiceImpl.java
index 82b50fd..b219358 100644
--- a/src/com/android/server/telecom/TelecomServiceImpl.java
+++ b/src/com/android/server/telecom/TelecomServiceImpl.java
@@ -70,6 +70,8 @@
PhoneAccountHandle defaultOutgoingPhoneAccount =
mPhoneAccountRegistrar.getOutgoingPhoneAccountForScheme(uriScheme);
// Make sure that the calling user can see this phone account.
+ // TODO: Does this isVisible check actually work considering we are clearing
+ // the calling identity?
if (defaultOutgoingPhoneAccount != null
&& !isVisibleToCaller(defaultOutgoingPhoneAccount)) {
Log.w(this, "No account found for the calling user");
@@ -108,11 +110,15 @@
public void setUserSelectedOutgoingPhoneAccount(PhoneAccountHandle accountHandle) {
synchronized (mLock) {
enforceModifyPermission();
+
+ long token = Binder.clearCallingIdentity();
try {
mPhoneAccountRegistrar.setUserSelectedOutgoingPhoneAccount(accountHandle);
} catch (Exception e) {
Log.e(this, e, "setUserSelectedOutgoingPhoneAccount");
throw e;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
}
}
@@ -126,6 +132,8 @@
synchronized (mLock) {
long token = Binder.clearCallingIdentity();
try {
+ // TODO: Does this isVisible check actually work considering we are clearing
+ // the calling identity?
return filterForAccountsVisibleToCaller(
mPhoneAccountRegistrar.getCallCapablePhoneAccounts(null));
} catch (Exception e) {
@@ -147,6 +155,8 @@
long token = Binder.clearCallingIdentity();
try {
+ // TODO: Does this isVisible check actually work considering we are clearing
+ // the calling identity?
return filterForAccountsVisibleToCaller(
mPhoneAccountRegistrar.getCallCapablePhoneAccounts(uriScheme));
} catch (Exception e) {
@@ -274,6 +284,8 @@
long token = Binder.clearCallingIdentity();
try {
+ // TODO: Does this isVisible check actually work considering we are clearing
+ // the calling identity?
return filterForAccountsVisibleToCaller(
mPhoneAccountRegistrar.getConnectionManagerPhoneAccounts());
} catch (Exception e) {
@@ -367,15 +379,19 @@
return false;
}
+ if (!isVisibleToCaller(accountHandle)) {
+ Log.w(this, "%s is not visible for the calling user", accountHandle);
+ return false;
+ }
+
+ long token = Binder.clearCallingIdentity();
try {
- if (!isVisibleToCaller(accountHandle)) {
- Log.w(this, "%s is not visible for the calling user", accountHandle);
- return false;
- }
return mPhoneAccountRegistrar.isVoiceMailNumber(accountHandle, number);
} catch (Exception e) {
Log.e(this, e, "getSubscriptionIdForPhoneAccount");
throw e;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
}
}
@@ -421,17 +437,21 @@
}
synchronized (mLock) {
+ if (!isVisibleToCaller(accountHandle)) {
+ Log.w(this, "%s is not visible for the calling user", accountHandle);
+ return null;
+ }
+
+ long token = Binder.clearCallingIdentity();
try {
- if (!isVisibleToCaller(accountHandle)) {
- Log.w(this, "%s is not visible for the calling user", accountHandle);
- return null;
- }
int subId =
mPhoneAccountRegistrar.getSubscriptionIdForPhoneAccount(accountHandle);
return getTelephonyManager().getLine1NumberForSubscriber(subId);
} catch (Exception e) {
Log.e(this, e, "getSubscriptionIdForPhoneAccount");
throw e;
+ } finally {
+ Binder.restoreCallingIdentity(token);
}
}
}
@@ -443,7 +463,13 @@
public void silenceRinger(String callingPackage) {
synchronized (mLock) {
enforceModifyPermissionOrPrivilegedDialer(callingPackage);
- mCallsManager.getRinger().silence();
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ mCallsManager.getRinger().silence();
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
@@ -532,7 +558,13 @@
public boolean endCall() {
synchronized (mLock) {
enforceModifyPermission();
- return endCallInternal();
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ return endCallInternal();
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
@@ -543,7 +575,13 @@
public void acceptRingingCall() {
synchronized (mLock) {
enforceModifyPermission();
- acceptRingingCallInternal();
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ acceptRingingCallInternal();
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
@@ -558,7 +596,13 @@
}
synchronized (mLock) {
- mCallsManager.getInCallController().bringToForeground(showDialpad);
+
+ long token = Binder.clearCallingIdentity();
+ try {
+ mCallsManager.getInCallController().bringToForeground(showDialpad);
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
@@ -569,7 +613,12 @@
public void cancelMissedCallsNotification(String callingPackage) {
synchronized (mLock) {
enforceModifyPermissionOrPrivilegedDialer(callingPackage);
- mCallsManager.getMissedCallNotifier().clearMissedCalls();
+ long token = Binder.clearCallingIdentity();
+ try {
+ mCallsManager.getMissedCallNotifier().clearMissedCalls();
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
}
}
