Track (and use) UserHandle to make connections
When binding to services, use the UserHandle which registered the
PhoneAccount in order to ensure that the correct user receives the
Intent.
Also make sure that TelecomManager does not leak PhoneAccounts across
profiles, unless the caller is a system-privileged app. PhoneAccounts
will not leak across users, even for system-privileged apps.
Bug: 17421902
Bug: 17421044
Change-Id: I673904a7ae038c88688bba62642e93174e55c308
diff --git a/src/com/android/server/telecom/PhoneAccountRegistrar.java b/src/com/android/server/telecom/PhoneAccountRegistrar.java
index dc92129..eb3248e 100644
--- a/src/com/android/server/telecom/PhoneAccountRegistrar.java
+++ b/src/com/android/server/telecom/PhoneAccountRegistrar.java
@@ -17,21 +17,25 @@
package com.android.server.telecom;
import android.Manifest;
+import android.content.ComponentName;
+import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo;
import android.content.pm.ServiceInfo;
+import android.content.pm.UserInfo;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
+import android.net.Uri;
+import android.os.Process;
+import android.os.UserHandle;
+import android.os.UserManager;
import android.provider.Settings;
import android.telecom.ConnectionService;
import android.telecom.PhoneAccount;
import android.telecom.PhoneAccountHandle;
import android.telephony.PhoneNumberUtils;
import android.telephony.SubscriptionManager;
-import android.content.ComponentName;
-import android.content.Context;
-import android.net.Uri;
import android.text.TextUtils;
import android.util.AtomicFile;
import android.util.Base64;
@@ -92,8 +96,10 @@
private final List<Listener> mListeners = new CopyOnWriteArrayList<>();
private final AtomicFile mAtomicFile;
private final Context mContext;
+ private final UserManager mUserManager;
private final SubscriptionManager mSubscriptionManager;
private State mState;
+ private UserHandle mCurrentUserHandle;
@VisibleForTesting
public PhoneAccountRegistrar(Context context) {
@@ -114,7 +120,9 @@
mState = new State();
mContext = context;
+ mUserManager = UserManager.get(context);
mSubscriptionManager = SubscriptionManager.from(mContext);
+ mCurrentUserHandle = Process.myUserHandle();
read();
}
@@ -127,9 +135,11 @@
* @return The value of the subscription id or -1 if it does not exist or is not valid.
*/
public int getSubscriptionIdForPhoneAccount(PhoneAccountHandle accountHandle) {
- PhoneAccount account = getPhoneAccount(accountHandle);
- if (account == null || !account.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION) ||
- !TextUtils.isDigitsOnly(accountHandle.getId())) {
+ PhoneAccount account = getPhoneAccountInternal(accountHandle);
+ if (account == null
+ || !account.hasCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)
+ || !TextUtils.isDigitsOnly(accountHandle.getId())
+ || !isVisibleForUser(accountHandle)) {
// Since no decimals or negative numbers can be valid subscription ids, only a string of
// numbers can be subscription id
return SubscriptionManager.INVALID_SUBSCRIPTION_ID;
@@ -138,7 +148,10 @@
}
/**
- * Retrieves the default outgoing phone account supporting the specified uriScheme.
+ * Retrieves the default outgoing phone account supporting the specified uriScheme. Note that if
+ * {@link #mCurrentUserHandle} does not have visibility into the current default, {@code null}
+ * will be returned.
+ *
* @param uriScheme The URI scheme for the outgoing call.
* @return The {@link PhoneAccountHandle} to use.
*/
@@ -148,8 +161,9 @@
if (userSelected != null) {
// If there is a default PhoneAccount, ensure it supports calls to handles with the
// specified uriScheme.
- final PhoneAccount userSelectedAccount = getPhoneAccount(userSelected);
- if (userSelectedAccount.supportsUriScheme(uriScheme)) {
+ final PhoneAccount userSelectedAccount = getPhoneAccountInternal(userSelected);
+ if (userSelectedAccount.supportsUriScheme(uriScheme)
+ && isVisibleForUser(userSelected)) {
return userSelected;
}
}
@@ -160,20 +174,29 @@
// There are no accounts, so there can be no default
return null;
case 1:
- // There is only one account, which is by definition the default
- return outgoing.get(0);
+ // There is only one account, which is by definition the default.
+ PhoneAccountHandle onlyHandle = outgoing.get(0);
+ if (isVisibleForUser(onlyHandle)) {
+ return outgoing.get(0);
+ }
+ return null;
default:
// There are multiple accounts with no selected default
return null;
}
}
+ /**
+ * @return The user-selected outgoing {@link PhoneAccount}, or null if it hasn't been set (or
+ * if it was set by another user).
+ */
PhoneAccountHandle getUserSelectedOutgoingPhoneAccount() {
if (mState.defaultOutgoing != null) {
// Return the registered outgoing default iff it still exists (we keep a sticky
// default to survive account deletion and re-addition)
for (int i = 0; i < mState.accounts.size(); i++) {
- if (mState.accounts.get(i).getAccountHandle().equals(mState.defaultOutgoing)) {
+ if (mState.accounts.get(i).getAccountHandle().equals(mState.defaultOutgoing)
+ && isVisibleForUser(mState.defaultOutgoing)) {
return mState.defaultOutgoing;
}
}
@@ -202,14 +225,14 @@
return;
}
- if (!getPhoneAccount(accountHandle).hasCapabilities(
+ if (!getPhoneAccountInternal(accountHandle).hasCapabilities(
PhoneAccount.CAPABILITY_CALL_PROVIDER)) {
Log.w(this, "Trying to set non-call-provider default outgoing %s",
accountHandle);
return;
}
- if (getPhoneAccount(accountHandle).hasCapabilities(
+ if (getPhoneAccountInternal(accountHandle).hasCapabilities(
PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION)) {
// If the account selected is a SIM account, propagate down to the subscription
// record.
@@ -231,7 +254,7 @@
public void setSimCallManager(PhoneAccountHandle callManager) {
if (callManager != null) {
- PhoneAccount callManagerAccount = getPhoneAccount(callManager);
+ PhoneAccount callManagerAccount = getPhoneAccountInternal(callManager);
if (callManagerAccount == null) {
Log.d(this, "setSimCallManager: Nonexistent call manager: %s", callManager);
return;
@@ -249,6 +272,9 @@
fireSimCallManagerChanged();
}
+ /**
+ * @return The {@link PhoneAccount}s which are visible to {@link #mCurrentUserHandle}.
+ */
public PhoneAccountHandle getSimCallManager() {
if (mState.simCallManager != null) {
if (NO_ACCOUNT_SELECTED.equals(mState.simCallManager)) {
@@ -258,7 +284,8 @@
// setting to survive account deletion and re-addition)
for (int i = 0; i < mState.accounts.size(); i++) {
if (mState.accounts.get(i).getAccountHandle().equals(mState.simCallManager)
- && !resolveComponent(mState.simCallManager.getComponentName()).isEmpty()) {
+ && !resolveComponent(mState.simCallManager).isEmpty()
+ && isVisibleForUser(mState.simCallManager)) {
return mState.simCallManager;
}
}
@@ -270,12 +297,13 @@
if (!TextUtils.isEmpty(defaultConnectionMgr)) {
ComponentName componentName = ComponentName.unflattenFromString(defaultConnectionMgr);
// Make sure that the component can be resolved.
- List<ResolveInfo> resolveInfos = resolveComponent(componentName);
+ List<ResolveInfo> resolveInfos = resolveComponent(componentName, null);
if (!resolveInfos.isEmpty()) {
// See if there is registered PhoneAccount by this component.
List<PhoneAccountHandle> handles = getAllPhoneAccountHandles();
for (PhoneAccountHandle handle : handles) {
- if (componentName.equals(handle.getComponentName())) {
+ if (componentName.equals(handle.getComponentName())
+ && isVisibleForUser(handle)) {
return handle;
}
}
@@ -290,11 +318,95 @@
return null;
}
- private List<ResolveInfo> resolveComponent(ComponentName componentName) {
+ /**
+ * A version of {@link #getPhoneAccount} which does not guard for the current user.
+ *
+ * @param handle
+ * @return
+ */
+ PhoneAccount getPhoneAccountInternal(PhoneAccountHandle handle) {
+ for (PhoneAccount m : mState.accounts) {
+ if (Objects.equals(handle, m.getAccountHandle())) {
+ return m;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Update the current UserHandle to track when users are switched. This will allow the
+ * PhoneAccountRegistar to self-filter the PhoneAccounts to make sure we don't leak anything
+ * across users.
+ *
+ * @param userHandle The {@link UserHandle}, as delivered by
+ * {@link Intent#ACTION_USER_SWITCHED}.
+ */
+ public void setCurrentUserHandle(UserHandle userHandle) {
+ if (userHandle == null) {
+ Log.d(this, "setCurrentUserHandle, userHandle = null");
+ userHandle = Process.myUserHandle();
+ }
+ Log.d(this, "setCurrentUserHandle, %s", userHandle);
+ mCurrentUserHandle = userHandle;
+ }
+
+ private boolean isVisibleForUser(PhoneAccountHandle accountHandle) {
+ if (accountHandle == null) {
+ return false;
+ }
+
+ return isVisibleForUser(getPhoneAccountInternal(accountHandle));
+ }
+
+ private boolean isVisibleForUser(PhoneAccount account) {
+ if (account == null) {
+ return false;
+ }
+
+ // If this PhoneAccount has CAPABILITY_MULTI_USER, it should be visible to all users and
+ // all profiles. Only Telephony and SIP accounts should have this capability.
+ if (account.hasCapabilities(PhoneAccount.CAPABILITY_MULTI_USER)) {
+ return true;
+ }
+
+ UserHandle phoneAccountUserHandle = account.getAccountHandle().getUserHandle();
+ if (phoneAccountUserHandle == null) {
+ return false;
+ }
+
+ if (mCurrentUserHandle == null) {
+ Log.d(this, "Current user is null; assuming true");
+ return true;
+ }
+
+ // Unlike in TelecomServiceImpl, we only care about *profiles* here. We want to make sure
+ // that we don't resolve PhoneAccount across *users*, but resolving across *profiles* is
+ // fine.
+ List<UserInfo> profileUsers = mUserManager.getProfiles(mCurrentUserHandle.getIdentifier());
+
+ for (UserInfo profileInfo : profileUsers) {
+ if (profileInfo.getUserHandle().equals(phoneAccountUserHandle)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private List<ResolveInfo> resolveComponent(PhoneAccountHandle phoneAccountHandle) {
+ return resolveComponent(phoneAccountHandle.getComponentName(),
+ phoneAccountHandle.getUserHandle());
+ }
+
+ private List<ResolveInfo> resolveComponent(ComponentName componentName,
+ UserHandle userHandle) {
PackageManager pm = mContext.getPackageManager();
Intent intent = new Intent(ConnectionService.SERVICE_INTERFACE);
intent.setComponent(componentName);
- return pm.queryIntentServices(intent, 0);
+ if (userHandle != null) {
+ return pm.queryIntentServicesAsUser(intent, 0, userHandle.getIdentifier());
+ } else {
+ return pm.queryIntentServices(intent, 0);
+ }
}
/**
@@ -305,22 +417,21 @@
public List<PhoneAccountHandle> getAllPhoneAccountHandles() {
List<PhoneAccountHandle> accountHandles = new ArrayList<>();
for (PhoneAccount m : mState.accounts) {
- accountHandles.add(m.getAccountHandle());
+ if (isVisibleForUser(m)) {
+ accountHandles.add(m.getAccountHandle());
+ }
}
return accountHandles;
}
public List<PhoneAccount> getAllPhoneAccounts() {
- return new ArrayList<>(mState.accounts);
- }
-
- /**
- * Determines the number of all {@link PhoneAccount}s.
- *
- * @return The total number {@link PhoneAccount}s.
- */
- public int getAllPhoneAccountsCount() {
- return mState.accounts.size();
+ List<PhoneAccount> accounts = new ArrayList<>(mState.accounts.size());
+ for (PhoneAccount account : mState.accounts) {
+ if (isVisibleForUser(account)) {
+ accounts.add(account);
+ }
+ }
+ return accounts;
}
/**
@@ -354,7 +465,8 @@
for (PhoneAccount m : mState.accounts) {
if (Objects.equals(
packageName,
- m.getAccountHandle().getComponentName().getPackageName())) {
+ m.getAccountHandle().getComponentName().getPackageName())
+ && isVisibleForUser(m)) {
accountHandles.add(m.getAccountHandle());
}
}
@@ -373,7 +485,8 @@
public PhoneAccount getPhoneAccount(PhoneAccountHandle handle) {
for (PhoneAccount m : mState.accounts) {
- if (Objects.equals(handle, m.getAccountHandle())) {
+ if (Objects.equals(handle, m.getAccountHandle())
+ && isVisibleForUser(m)) {
return m;
}
}
@@ -421,7 +534,8 @@
public void unregisterPhoneAccount(PhoneAccountHandle accountHandle) {
for (int i = 0; i < mState.accounts.size(); i++) {
- if (Objects.equals(accountHandle, mState.accounts.get(i).getAccountHandle())) {
+ PhoneAccountHandle handle = mState.accounts.get(i).getAccountHandle();
+ if (Objects.equals(accountHandle, handle)) {
mState.accounts.remove(i);
break;
}
@@ -435,15 +549,16 @@
* Un-registers all phone accounts associated with a specified package.
*
* @param packageName The package for which phone accounts will be removed.
+ * @param userHandle The {@link UserHandle} the package is running under.
*/
- public void clearAccounts(String packageName) {
+ public void clearAccounts(String packageName, UserHandle userHandle) {
boolean accountsRemoved = false;
Iterator<PhoneAccount> it = mState.accounts.iterator();
while (it.hasNext()) {
PhoneAccount phoneAccount = it.next();
- if (Objects.equals(
- packageName,
- phoneAccount.getAccountHandle().getComponentName().getPackageName())) {
+ PhoneAccountHandle handle = phoneAccount.getAccountHandle();
+ if (Objects.equals(packageName, handle.getComponentName().getPackageName())
+ && Objects.equals(userHandle, handle.getUserHandle())) {
Log.i(this, "Removing phone account " + phoneAccount.getLabel());
it.remove();
accountsRemoved = true;
@@ -532,12 +647,23 @@
private List<PhoneAccountHandle> getPhoneAccountHandles(int flags, String uriScheme) {
List<PhoneAccountHandle> accountHandles = new ArrayList<>();
for (PhoneAccount m : mState.accounts) {
- if (m.hasCapabilities(flags) && (uriScheme == null || m.supportsUriScheme(uriScheme))) {
- // Also filter out unresolveable accounts
- if (!resolveComponent(m.getAccountHandle().getComponentName()).isEmpty()) {
- accountHandles.add(m.getAccountHandle());
- }
+ if (!m.hasCapabilities(flags)) {
+ // Account doesn't have the right capabilities; skip this one.
+ continue;
}
+ if (uriScheme != null && !m.supportsUriScheme(uriScheme)) {
+ // Account doesn't support this URI scheme; skip this one.
+ continue;
+ }
+ if (resolveComponent(m.getAccountHandle()).isEmpty()) {
+ // This component cannot be resolved anymore; skip this one.
+ continue;
+ }
+ if (!isVisibleForUser(m)) {
+ // Account is not visible for the current user; skip this one.
+ continue;
+ }
+ accountHandles.add(m.getAccountHandle());
}
return accountHandles;
}
@@ -604,7 +730,7 @@
try {
XmlSerializer serializer = new FastXmlSerializer();
serializer.setOutput(new BufferedOutputStream(os), "utf-8");
- writeToXml(mState, serializer);
+ writeToXml(mState, serializer, mContext);
serializer.flush();
success = true;
} finally {
@@ -648,15 +774,29 @@
}
}
+ // Verify all of the UserHandles.
+ List<PhoneAccount> badAccounts = new ArrayList<>();
+ for (PhoneAccount phoneAccount : mState.accounts) {
+ UserHandle userHandle = phoneAccount.getAccountHandle().getUserHandle();
+ if (userHandle == null) {
+ Log.w(this, "Missing UserHandle for %s", phoneAccount);
+ badAccounts.add(phoneAccount);
+ } else if (mUserManager.getSerialNumberForUser(userHandle) == -1) {
+ Log.w(this, "User does not exist for %s", phoneAccount);
+ badAccounts.add(phoneAccount);
+ }
+ }
+ mState.accounts.removeAll(badAccounts);
+
// If an upgrade occurred, write out the changed data.
- if (versionChanged) {
+ if (versionChanged || !badAccounts.isEmpty()) {
write();
}
}
- private static void writeToXml(State state, XmlSerializer serializer)
+ private static void writeToXml(State state, XmlSerializer serializer, Context context)
throws IOException {
- sStateXml.writeToXml(state, serializer);
+ sStateXml.writeToXml(state, serializer, context);
}
private static State readFromXml(XmlPullParser parser, Context context)
@@ -678,7 +818,7 @@
/**
* Write the supplied object to XML
*/
- public abstract void writeToXml(T o, XmlSerializer serializer)
+ public abstract void writeToXml(T o, XmlSerializer serializer, Context context)
throws IOException;
/**
@@ -742,6 +882,13 @@
}
}
+ protected void writeLong(String tagName, long value, XmlSerializer serializer)
+ throws IOException {
+ serializer.startTag(null, tagName);
+ serializer.text(Long.valueOf(value).toString());
+ serializer.endTag(null, tagName);
+ }
+
/**
* Reads a string array from the XML parser.
*
@@ -790,7 +937,7 @@
private static final String VERSION = "version";
@Override
- public void writeToXml(State o, XmlSerializer serializer)
+ public void writeToXml(State o, XmlSerializer serializer, Context context)
throws IOException {
if (o != null) {
serializer.startTag(null, CLASS_STATE);
@@ -798,19 +945,19 @@
if (o.defaultOutgoing != null) {
serializer.startTag(null, DEFAULT_OUTGOING);
- sPhoneAccountHandleXml.writeToXml(o.defaultOutgoing, serializer);
+ sPhoneAccountHandleXml.writeToXml(o.defaultOutgoing, serializer, context);
serializer.endTag(null, DEFAULT_OUTGOING);
}
if (o.simCallManager != null) {
serializer.startTag(null, SIM_CALL_MANAGER);
- sPhoneAccountHandleXml.writeToXml(o.simCallManager, serializer);
+ sPhoneAccountHandleXml.writeToXml(o.simCallManager, serializer, context);
serializer.endTag(null, SIM_CALL_MANAGER);
}
serializer.startTag(null, ACCOUNTS);
for (PhoneAccount m : o.accounts) {
- sPhoneAccountXml.writeToXml(m, serializer);
+ sPhoneAccountXml.writeToXml(m, serializer, context);
}
serializer.endTag(null, ACCOUNTS);
@@ -838,6 +985,13 @@
parser.nextTag();
s.simCallManager = sPhoneAccountHandleXml.readFromXml(parser,
s.versionNumber, context);
+ if (s.simCallManager.getUserHandle() == null) {
+ // This should never happen, but handle the upgrade case.
+ s.simCallManager = new PhoneAccountHandle(
+ s.simCallManager.getComponentName(),
+ s.simCallManager.getId(),
+ Process.myUserHandle());
+ }
} else if (parser.getName().equals(ACCOUNTS)) {
int accountsDepth = parser.getDepth();
while (XmlUtils.nextElementWithin(parser, accountsDepth)) {
@@ -874,14 +1028,14 @@
private static final String SUPPORTED_URI_SCHEMES = "supported_uri_schemes";
@Override
- public void writeToXml(PhoneAccount o, XmlSerializer serializer)
+ public void writeToXml(PhoneAccount o, XmlSerializer serializer, Context context)
throws IOException {
if (o != null) {
serializer.startTag(null, CLASS_PHONE_ACCOUNT);
if (o.getAccountHandle() != null) {
serializer.startTag(null, ACCOUNT_HANDLE);
- sPhoneAccountHandleXml.writeToXml(o.getAccountHandle(), serializer);
+ sPhoneAccountHandleXml.writeToXml(o.getAccountHandle(), serializer, context);
serializer.endTag(null, ACCOUNT_HANDLE);
}
@@ -1027,9 +1181,10 @@
private static final String CLASS_PHONE_ACCOUNT_HANDLE = "phone_account_handle";
private static final String COMPONENT_NAME = "component_name";
private static final String ID = "id";
+ private static final String USER_SERIAL_NUMBER = "user_serial_number";
@Override
- public void writeToXml(PhoneAccountHandle o, XmlSerializer serializer)
+ public void writeToXml(PhoneAccountHandle o, XmlSerializer serializer, Context context)
throws IOException {
if (o != null) {
serializer.startTag(null, CLASS_PHONE_ACCOUNT_HANDLE);
@@ -1041,6 +1196,12 @@
writeTextIfNonNull(ID, o.getId(), serializer);
+ if (o.getUserHandle() != null && context != null) {
+ UserManager userManager = UserManager.get(context);
+ writeLong(USER_SERIAL_NUMBER,
+ userManager.getSerialNumberForUser(o.getUserHandle()), serializer);
+ }
+
serializer.endTag(null, CLASS_PHONE_ACCOUNT_HANDLE);
}
}
@@ -1051,7 +1212,11 @@
if (parser.getName().equals(CLASS_PHONE_ACCOUNT_HANDLE)) {
String componentNameString = null;
String idString = null;
+ String userSerialNumberString = null;
int outerDepth = parser.getDepth();
+
+ UserManager userManager = UserManager.get(context);
+
while (XmlUtils.nextElementWithin(parser, outerDepth)) {
if (parser.getName().equals(COMPONENT_NAME)) {
parser.next();
@@ -1059,12 +1224,25 @@
} else if (parser.getName().equals(ID)) {
parser.next();
idString = parser.getText();
+ } else if (parser.getName().equals(USER_SERIAL_NUMBER)) {
+ parser.next();
+ userSerialNumberString = parser.getText();
}
}
if (componentNameString != null) {
+ UserHandle userHandle = null;
+ if (userSerialNumberString != null) {
+ try {
+ long serialNumber = Long.parseLong(userSerialNumberString);
+ userHandle = userManager.getUserForSerialNumber(serialNumber);
+ } catch (NumberFormatException e) {
+ Log.e(this, e, "Could not parse UserHandle " + userSerialNumberString);
+ }
+ }
return new PhoneAccountHandle(
ComponentName.unflattenFromString(componentNameString),
- idString);
+ idString,
+ userHandle);
}
}
return null;