Revert "Revert "Permissions: GET_ACCOUNTS permission cleanup""
This reverts commit 845d14db9066c3262f270237b52e315aa71508b2.
Bug: 22942480
Bug: 22902898
Change-Id: If2d0e11594284414953c3dd45475fc7ba3366295
diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java
index 32fd56a..83e8db0 100644
--- a/services/core/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -527,14 +527,14 @@
+ ", pid " + Binder.getCallingPid());
}
if (account == null) throw new IllegalArgumentException("account is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot get secrets for accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -627,14 +627,14 @@
}
if (account == null) throw new IllegalArgumentException("account is null");
if (key == null) throw new IllegalArgumentException("key is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot get user data for accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -664,22 +664,32 @@
final long identityToken = clearCallingIdentity();
try {
- Collection<AccountAuthenticatorCache.ServiceInfo<AuthenticatorDescription>>
- authenticatorCollection = mAuthenticatorCache.getAllServices(userId);
- AuthenticatorDescription[] types =
- new AuthenticatorDescription[authenticatorCollection.size()];
- int i = 0;
- for (AccountAuthenticatorCache.ServiceInfo<AuthenticatorDescription> authenticator
- : authenticatorCollection) {
- types[i] = authenticator.type;
- i++;
- }
- return types;
+ return getAuthenticatorTypesInternal(userId);
+
} finally {
restoreCallingIdentity(identityToken);
}
}
+ /**
+ * Should only be called inside of a clearCallingIdentity block.
+ */
+ private AuthenticatorDescription[] getAuthenticatorTypesInternal(int userId) {
+ Collection<AccountAuthenticatorCache.ServiceInfo<AuthenticatorDescription>>
+ authenticatorCollection = mAuthenticatorCache.getAllServices(userId);
+ AuthenticatorDescription[] types =
+ new AuthenticatorDescription[authenticatorCollection.size()];
+ int i = 0;
+ for (AccountAuthenticatorCache.ServiceInfo<AuthenticatorDescription> authenticator
+ : authenticatorCollection) {
+ types[i] = authenticator.type;
+ i++;
+ }
+ return types;
+ }
+
+
+
private boolean isCrossUser(int callingUid, int userId) {
return (userId != UserHandle.getCallingUserId()
&& callingUid != Process.myUid()
@@ -697,7 +707,8 @@
+ ", pid " + Binder.getCallingPid());
}
if (account == null) throw new IllegalArgumentException("account is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot explicitly add accounts of type: %s",
callingUid,
@@ -713,12 +724,10 @@
*/
// fails if the account already exists
- int uid = getCallingUid();
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
- return addAccountInternal(accounts, account, password, extras, false, uid);
+ return addAccountInternal(accounts, account, password, extras, false, callingUid);
} finally {
restoreCallingIdentity(identityToken);
}
@@ -794,25 +803,26 @@
if (account == null) {
throw new IllegalArgumentException("account is null");
}
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot notify authentication for accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = Binder.getCallingUserHandle().getIdentifier();
+
if (!canUserModifyAccounts(userId) || !canUserModifyAccountsForType(userId, account.type)) {
return false;
}
- int user = UserHandle.getCallingUserId();
+
long identityToken = clearCallingIdentity();
try {
- UserAccounts accounts = getUserAccounts(user);
+ UserAccounts accounts = getUserAccounts(userId);
+ return updateLastAuthenticatedTime(account);
} finally {
restoreCallingIdentity(identityToken);
}
- return updateLastAuthenticatedTime(account);
}
private boolean updateLastAuthenticatedTime(Account account) {
@@ -985,8 +995,9 @@
if (response == null) throw new IllegalArgumentException("response is null");
if (account == null) throw new IllegalArgumentException("account is null");
if (features == null) throw new IllegalArgumentException("features is null");
- checkReadAccountsPermitted(callingUid, account.type);
int userId = UserHandle.getCallingUserId();
+ checkReadAccountsPermitted(callingUid, account.type, userId);
+
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1062,14 +1073,14 @@
+ ", pid " + Binder.getCallingPid());
}
if (accountToRename == null) throw new IllegalArgumentException("account is null");
- if (!isAccountManagedByCaller(accountToRename.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(accountToRename.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot rename accounts of type: %s",
callingUid,
accountToRename.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1211,14 +1222,15 @@
* authenticator. This will let users remove accounts (via Settings in the system) but not
* arbitrary applications (like competing authenticators).
*/
- if (!isAccountManagedByCaller(account.type, callingUid) && !isSystemUid(callingUid)) {
+ UserHandle user = new UserHandle(userId);
+ if (!isAccountManagedByCaller(account.type, callingUid, user.getIdentifier())
+ && !isSystemUid(callingUid)) {
String msg = String.format(
"uid %s cannot remove accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
-
if (!canUserModifyAccounts(userId)) {
try {
response.onError(AccountManager.ERROR_CODE_USER_RESTRICTED,
@@ -1235,10 +1247,7 @@
}
return;
}
-
- UserHandle user = new UserHandle(userId);
long identityToken = clearCallingIdentity();
-
UserAccounts accounts = getUserAccounts(userId);
cancelNotification(getSigninRequiredNotificationId(accounts, account), user);
synchronized(accounts.credentialsPermissionNotificationIds) {
@@ -1268,6 +1277,7 @@
+ ", caller's uid " + callingUid
+ ", pid " + Binder.getCallingPid());
}
+ int userId = Binder.getCallingUserHandle().getIdentifier();
if (account == null) {
/*
* Null accounts should result in returning false, as per
@@ -1275,22 +1285,18 @@
*/
Log.e(TAG, "account is null");
return false;
- } else if (!isAccountManagedByCaller(account.type, callingUid)) {
+ } else if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot explicitly add accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
-
UserAccounts accounts = getUserAccountsForCaller();
- int userId = Binder.getCallingUserHandle().getIdentifier();
if (!canUserModifyAccounts(userId) || !canUserModifyAccountsForType(userId, account.type)) {
return false;
}
-
logRecord(accounts, DebugDbHelper.ACTION_CALLED_ACCOUNT_REMOVE, TABLE_ACCOUNTS);
-
long identityToken = clearCallingIdentity();
try {
return removeAccountInternal(accounts, account);
@@ -1524,14 +1530,14 @@
}
if (account == null) throw new IllegalArgumentException("account is null");
if (authTokenType == null) throw new IllegalArgumentException("authTokenType is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot peek the authtokens associated with accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1552,14 +1558,14 @@
}
if (account == null) throw new IllegalArgumentException("account is null");
if (authTokenType == null) throw new IllegalArgumentException("authTokenType is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot set auth tokens associated with accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1578,14 +1584,14 @@
+ ", pid " + Binder.getCallingPid());
}
if (account == null) throw new IllegalArgumentException("account is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot set secrets for accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1642,14 +1648,14 @@
+ ", pid " + Binder.getCallingPid());
}
if (account == null) throw new IllegalArgumentException("account is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot clear passwords for accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1670,14 +1676,14 @@
}
if (key == null) throw new IllegalArgumentException("key is null");
if (account == null) throw new IllegalArgumentException("account is null");
- if (!isAccountManagedByCaller(account.type, callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(account.type, callingUid, userId)) {
String msg = String.format(
"uid %s cannot set user data for accounts of type: %s",
callingUid,
account.type);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -1840,8 +1846,8 @@
// skip the check if customTokens
final int callerUid = Binder.getCallingUid();
- final boolean permissionGranted = customTokens ||
- permissionIsGranted(account, authTokenType, callerUid);
+ final boolean permissionGranted =
+ customTokens || permissionIsGranted(account, authTokenType, callerUid, userId);
// Get the calling package. We will use it for the purpose of caching.
final String callerPkg = loginOptions.getString(AccountManager.KEY_ANDROID_PACKAGE_NAME);
@@ -2363,14 +2369,14 @@
}
if (response == null) throw new IllegalArgumentException("response is null");
if (accountType == null) throw new IllegalArgumentException("accountType is null");
- if (!isAccountManagedByCaller(accountType, callingUid) && !isSystemUid(callingUid)) {
+ int userId = UserHandle.getCallingUserId();
+ if (!isAccountManagedByCaller(accountType, callingUid, userId) && !isSystemUid(callingUid)) {
String msg = String.format(
"uid %s cannot edit authenticator properites for account type: %s",
callingUid,
accountType);
throw new SecurityException(msg);
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts accounts = getUserAccounts(userId);
@@ -2493,20 +2499,22 @@
}
/**
- * Returns the accounts for a specific user
+ * Returns the accounts visible to the client within the context of a specific user
* @hide
*/
public Account[] getAccounts(int userId) {
int callingUid = Binder.getCallingUid();
- if (!isReadAccountsPermitted(callingUid, null)) {
+ List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId);
+ if (visibleAccountTypes.isEmpty()) {
return new Account[0];
}
long identityToken = clearCallingIdentity();
try {
- UserAccounts accounts = getUserAccounts(userId);
- synchronized (accounts.cacheLock) {
- return getAccountsFromCacheLocked(accounts, null, callingUid, null);
- }
+ return getAccountsInternal(
+ userId,
+ callingUid,
+ null, // packageName
+ visibleAccountTypes);
} finally {
restoreCallingIdentity(identityToken);
}
@@ -2588,22 +2596,52 @@
callingUid = packageUid;
}
- // Authenticators should be able to see their own accounts regardless of permissions.
- if (TextUtils.isEmpty(type) && !isReadAccountsPermitted(callingUid, type)) {
+ List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId);
+ if (visibleAccountTypes.isEmpty()
+ || (type != null && !visibleAccountTypes.contains(type))) {
return new Account[0];
- }
+ } else if (visibleAccountTypes.contains(type)) {
+ // Prune the list down to just the requested type.
+ visibleAccountTypes = new ArrayList<>();
+ visibleAccountTypes.add(type);
+ } // else aggregate all the visible accounts (it won't matter if the list is empty).
long identityToken = clearCallingIdentity();
try {
- UserAccounts accounts = getUserAccounts(userId);
- synchronized (accounts.cacheLock) {
- return getAccountsFromCacheLocked(accounts, type, callingUid, callingPackage);
- }
+ return getAccountsInternal(
+ userId,
+ callingUid,
+ callingPackage,
+ visibleAccountTypes);
} finally {
restoreCallingIdentity(identityToken);
}
}
+ private Account[] getAccountsInternal(
+ int userId,
+ int callingUid,
+ String callingPackage,
+ List<String> visibleAccountTypes) {
+ UserAccounts accounts = getUserAccounts(userId);
+ synchronized (accounts.cacheLock) {
+ UserAccounts userAccounts = getUserAccounts(userId);
+ ArrayList<Account> visibleAccounts = new ArrayList<>();
+ for (String visibleType : visibleAccountTypes) {
+ Account[] accountsForType = getAccountsFromCacheLocked(
+ userAccounts, visibleType, callingUid, callingPackage);
+ if (accountsForType != null) {
+ visibleAccounts.addAll(Arrays.asList(accountsForType));
+ }
+ }
+ Account[] result = new Account[visibleAccounts.size()];
+ for (int i = 0; i < visibleAccounts.size(); i++) {
+ result[i] = visibleAccounts.get(i);
+ }
+ return result;
+ }
+ }
+
@Override
public boolean addSharedAccountAsUser(Account account, int userId) {
userId = handleIncomingUser(userId);
@@ -2739,8 +2777,12 @@
}
if (response == null) throw new IllegalArgumentException("response is null");
if (type == null) throw new IllegalArgumentException("accountType is null");
- if (!isReadAccountsPermitted(callingUid, type)) {
+ int userId = UserHandle.getCallingUserId();
+
+ List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId);
+ if (!visibleAccountTypes.contains(type)) {
Bundle result = new Bundle();
+ // Need to return just the accounts that are from matching signatures.
result.putParcelableArray(AccountManager.KEY_ACCOUNTS, new Account[0]);
try {
response.onResult(result);
@@ -2749,7 +2791,6 @@
}
return;
}
- int userId = UserHandle.getCallingUserId();
long identityToken = clearCallingIdentity();
try {
UserAccounts userAccounts = getUserAccounts(userId);
@@ -2763,7 +2804,11 @@
onResult(response, result);
return;
}
- new GetAccountsByTypeAndFeatureSession(userAccounts, response, type, features,
+ new GetAccountsByTypeAndFeatureSession(
+ userAccounts,
+ response,
+ type,
+ features,
callingUid).bind();
} finally {
restoreCallingIdentity(identityToken);
@@ -3696,10 +3741,11 @@
return false;
}
- private boolean permissionIsGranted(Account account, String authTokenType, int callerUid) {
+ private boolean permissionIsGranted(
+ Account account, String authTokenType, int callerUid, int userId) {
final boolean isPrivileged = isPrivileged(callerUid);
final boolean fromAuthenticator = account != null
- && isAccountManagedByCaller(account.type, callerUid);
+ && isAccountManagedByCaller(account.type, callerUid, userId);
final boolean hasExplicitGrants = account != null
&& hasExplicitlyGrantedPermission(account, authTokenType, callerUid);
if (Log.isLoggable(TAG, Log.VERBOSE)) {
@@ -3711,23 +3757,45 @@
return fromAuthenticator || hasExplicitGrants || isPrivileged;
}
- private boolean isAccountManagedByCaller(String accountType, int callingUid) {
+ private boolean isAccountVisibleToCaller(String accountType, int callingUid, int userId) {
if (accountType == null) {
return false;
+ } else {
+ return getTypesVisibleToCaller(callingUid, userId).contains(accountType);
}
- final int callingUserId = UserHandle.getUserId(callingUid);
+ }
+
+ private boolean isAccountManagedByCaller(String accountType, int callingUid, int userId) {
+ if (accountType == null) {
+ return false;
+ } else {
+ return getTypesManagedByCaller(callingUid, userId).contains(accountType);
+ }
+ }
+
+ private List<String> getTypesVisibleToCaller(int callingUid, int userId) {
+ boolean isPermitted =
+ isPermitted(callingUid, Manifest.permission.GET_ACCOUNTS,
+ Manifest.permission.GET_ACCOUNTS_PRIVILEGED);
+ Log.i(TAG, String.format("getTypesVisibleToCaller: isPermitted? %s", isPermitted));
+ return getTypesForCaller(callingUid, userId, isPermitted);
+ }
+
+ private List<String> getTypesManagedByCaller(int callingUid, int userId) {
+ return getTypesForCaller(callingUid, userId, false);
+ }
+
+ private List<String> getTypesForCaller(
+ int callingUid, int userId, boolean isOtherwisePermitted) {
+ List<String> managedAccountTypes = new ArrayList<>();
for (RegisteredServicesCache.ServiceInfo<AuthenticatorDescription> serviceInfo :
- mAuthenticatorCache.getAllServices(callingUserId)) {
- if (serviceInfo.type.type.equals(accountType)) {
- /*
- * We can't simply compare uids because uids can be recycled before the
- * authenticator cache is updated.
- */
- final int sigChk = mPackageManager.checkSignatures(serviceInfo.uid, callingUid);
- return sigChk == PackageManager.SIGNATURE_MATCH;
+ mAuthenticatorCache.getAllServices(userId)) {
+ final int sigChk = mPackageManager.checkSignatures(serviceInfo.uid, callingUid);
+ if (isOtherwisePermitted || sigChk == PackageManager.SIGNATURE_MATCH) {
+ managedAccountTypes.add(serviceInfo.type.type);
}
}
- return false;
+ return managedAccountTypes;
}
private boolean isAccountPresentForCaller(String accountName, String accountType) {
@@ -3792,28 +3860,12 @@
return false;
}
- private boolean isReadAccountsPermitted(int callingUid, String accountType) {
- /*
- * Settings app (which is in the same uid as AcocuntManagerService), apps with the
- * GET_ACCOUNTS permission or authenticators that own the account type should be able to
- * access accounts of the specified account.
- */
- boolean isPermitted =
- isPermitted(callingUid, Manifest.permission.GET_ACCOUNTS,
- Manifest.permission.GET_ACCOUNTS_PRIVILEGED);
- boolean isAccountManagedByCaller = isAccountManagedByCaller(accountType, callingUid);
- Log.w(TAG, String.format(
- "isReadAccountPermitted: isPermitted: %s, isAM: %s",
- isPermitted,
- isAccountManagedByCaller));
- return isPermitted || isAccountManagedByCaller;
- }
-
/** Succeeds if any of the specified permissions are granted. */
private void checkReadAccountsPermitted(
int callingUid,
- String accountType) {
- if (!isReadAccountsPermitted(callingUid, accountType)) {
+ String accountType,
+ int userId) {
+ if (!isAccountVisibleToCaller(accountType, callingUid, userId)) {
String msg = String.format(
"caller uid %s cannot access %s accounts",
callingUid,