From c37ee22714ddec1104ba3a2189cf77924ac27812 Mon Sep 17 00:00:00 2001 From: Carlos Valdivia Date: Wed, 17 Jun 2015 20:17:37 -0700 Subject: [PATCH] Tweak GET_ACCOUNTS behavior and improve memory. Related to recent permissions and system health changes. This change will make it so that calls to AccountManager#getAccountsByType will work for the owning account authenticator even if they don't have permissions. This is pretty fundamental to having a working authenticator and it doesn't make sense to have it be disabled (or have authenticators hack around the framework). Also changed how TokenCache works so that memory usage is still predictable (no more than 64kb) but token caching won't be at the mercy of garbage collection. This is important for writing stable cts tests. Change-Id: Ib31b550616b266ee5a04eb26b04ba0023ca0cb83 --- core/java/android/accounts/AccountManager.java | 6 +- .../server/accounts/AccountManagerService.java | 279 ++++++++++++--------- .../com/android/server/accounts/TokenCache.java | 163 ++++++++---- 3 files changed, 277 insertions(+), 171 deletions(-) diff --git a/core/java/android/accounts/AccountManager.java b/core/java/android/accounts/AccountManager.java index 3001c2c9ef0f..aa7692b1097f 100644 --- a/core/java/android/accounts/AccountManager.java +++ b/core/java/android/accounts/AccountManager.java @@ -489,7 +489,8 @@ public class AccountManager { *

It is safe to call this method from the main thread. * *

This method requires the caller to hold the permission - * {@link android.Manifest.permission#GET_ACCOUNTS}. + * {@link android.Manifest.permission#GET_ACCOUNTS} or share a uid with the + * authenticator that owns the account type. * * @param type The type of accounts to return, null to retrieve all accounts * @return An array of {@link Account}, one per matching account. Empty @@ -615,7 +616,8 @@ public class AccountManager { * {@link AccountManagerFuture} must not be used on the main thread. * *

This method requires the caller to hold the permission - * {@link android.Manifest.permission#GET_ACCOUNTS}. + * {@link android.Manifest.permission#GET_ACCOUNTS} or share a uid with the + * authenticator that owns the account type. * * @param type The type of accounts to return, must not be null * @param features An array of the account features to require, diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index 50d311ff2748..a27097468405 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -86,7 +86,6 @@ import com.google.android.collect.Sets; import java.io.File; import java.io.FileDescriptor; import java.io.PrintWriter; -import java.lang.ref.WeakReference; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.SimpleDateFormat; @@ -115,7 +114,6 @@ public class AccountManagerService implements RegisteredServicesCacheListener { private static final String TAG = "AccountManagerService"; - private static final int TIMEOUT_DELAY_MS = 1000 * 60; private static final String DATABASE_NAME = "accounts.db"; private static final int DATABASE_VERSION = 8; @@ -216,7 +214,7 @@ public class AccountManagerService new HashMap>(); /** protected by the {@link #cacheLock} */ - private final HashMap> accountTokenCaches = new HashMap<>(); + private final TokenCache accountTokenCaches = new TokenCache(); /** * protected by the {@link #cacheLock} @@ -529,7 +527,7 @@ public class AccountManagerService + ", pid " + Binder.getCallingPid()); } if (account == null) throw new IllegalArgumentException("account is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot get secrets for accounts of type: %s", callingUid, @@ -627,7 +625,7 @@ public class AccountManagerService } if (account == null) throw new IllegalArgumentException("account is null"); if (key == null) throw new IllegalArgumentException("key is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot get user data for accounts of type: %s", callingUid, @@ -645,15 +643,22 @@ public class AccountManagerService @Override public AuthenticatorDescription[] getAuthenticatorTypes(int userId) { + int callingUid = Binder.getCallingUid(); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "getAuthenticatorTypes: " + "for user id " + userId - + "caller's uid " + Binder.getCallingUid() + + "caller's uid " + callingUid + ", pid " + Binder.getCallingPid()); } // Only allow the system process to read accounts of other users - enforceCrossUserPermission(userId, "User " + UserHandle.getCallingUserId() - + " trying get authenticator types for " + userId); + if (isCrossUser(callingUid, userId)) { + throw new SecurityException( + String.format( + "User %s tying to get authenticator types for %s" , + UserHandle.getCallingUserId(), + userId)); + } + final long identityToken = clearCallingIdentity(); try { Collection> @@ -672,14 +677,12 @@ public class AccountManagerService } } - private void enforceCrossUserPermission(int userId, String errorMessage) { - if (userId != UserHandle.getCallingUserId() - && Binder.getCallingUid() != Process.myUid() + private boolean isCrossUser(int callingUid, int userId) { + return (userId != UserHandle.getCallingUserId() + && callingUid != Process.myUid() && mContext.checkCallingOrSelfPermission( - android.Manifest.permission.INTERACT_ACROSS_USERS_FULL) - != PackageManager.PERMISSION_GRANTED) { - throw new SecurityException(errorMessage); - } + android.Manifest.permission.INTERACT_ACROSS_USERS_FULL) + != PackageManager.PERMISSION_GRANTED); } @Override @@ -691,7 +694,7 @@ public class AccountManagerService + ", pid " + Binder.getCallingPid()); } if (account == null) throw new IllegalArgumentException("account is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot explicitly add accounts of type: %s", callingUid, @@ -720,8 +723,11 @@ public class AccountManagerService @Override public void copyAccountToUser(final IAccountManagerResponse response, final Account account, int userFrom, int userTo) { - enforceCrossUserPermission(UserHandle.USER_ALL, "Calling copyAccountToUser requires " + int callingUid = Binder.getCallingUid(); + if (isCrossUser(callingUid, UserHandle.USER_ALL)) { + throw new SecurityException("Calling copyAccountToUser requires " + android.Manifest.permission.INTERACT_ACROSS_USERS_FULL); + } final UserAccounts fromAccounts = getUserAccounts(userFrom); final UserAccounts toAccounts = getUserAccounts(userTo); if (fromAccounts == null || toAccounts == null) { @@ -784,7 +790,7 @@ public class AccountManagerService if (account == null) { throw new IllegalArgumentException("account is null"); } - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot notify authentication for accounts of type: %s", callingUid, @@ -957,17 +963,18 @@ public class AccountManagerService @Override public void hasFeatures(IAccountManagerResponse response, Account account, String[] features) { + int callingUid = Binder.getCallingUid(); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "hasFeatures: " + account + ", response " + response + ", features " + stringArrayToString(features) - + ", caller's uid " + Binder.getCallingUid() + + ", caller's uid " + callingUid + ", pid " + Binder.getCallingPid()); } 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"); - checkReadAccountsPermission(); + checkReadAccountsPermitted(callingUid, account.type); UserAccounts accounts = getUserAccountsForCaller(); long identityToken = clearCallingIdentity(); try { @@ -1043,7 +1050,7 @@ public class AccountManagerService + ", pid " + Binder.getCallingPid()); } if (accountToRename == null) throw new IllegalArgumentException("account is null"); - if (!isAccountOwnedByCallingUid(accountToRename.type, callingUid)) { + if (!isAccountManagedByCaller(accountToRename.type, callingUid)) { String msg = String.format( "uid %s cannot rename accounts of type: %s", callingUid, @@ -1053,8 +1060,7 @@ public class AccountManagerService UserAccounts accounts = getUserAccountsForCaller(); long identityToken = clearCallingIdentity(); try { - Account resultingAccount = renameAccountInternal(accounts, accountToRename, newName, - callingUid); + Account resultingAccount = renameAccountInternal(accounts, accountToRename, newName); Bundle result = new Bundle(); result.putString(AccountManager.KEY_ACCOUNT_NAME, resultingAccount.name); result.putString(AccountManager.KEY_ACCOUNT_TYPE, resultingAccount.type); @@ -1069,7 +1075,7 @@ public class AccountManagerService } private Account renameAccountInternal( - UserAccounts accounts, Account accountToRename, String newName, int callingUid) { + UserAccounts accounts, Account accountToRename, String newName) { Account resultAccount = null; /* * Cancel existing notifications. Let authenticators @@ -1179,16 +1185,20 @@ public class AccountManagerService } if (response == null) throw new IllegalArgumentException("response is null"); if (account == null) throw new IllegalArgumentException("account is null"); - // Only allow the system process to modify accounts of other users - enforceCrossUserPermission(userId, "User " + UserHandle.getCallingUserId() - + " trying to remove account for " + userId); + if (isCrossUser(callingUid, userId)) { + throw new SecurityException( + String.format( + "User %s tying remove account for %s" , + UserHandle.getCallingUserId(), + userId)); + } /* * Only the system or authenticator should be allowed to remove accounts for that * authenticator. This will let users remove accounts (via Settings in the system) but not * arbitrary applications (like competing authenticators). */ - if (!isAccountOwnedByCallingUid(account.type, callingUid) && !isSystemUid(callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid) && !isSystemUid(callingUid)) { String msg = String.format( "uid %s cannot remove accounts of type: %s", callingUid, @@ -1252,7 +1262,7 @@ public class AccountManagerService */ Log.e(TAG, "account is null"); return false; - } else if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + } else if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot explicitly add accounts of type: %s", callingUid, @@ -1398,16 +1408,7 @@ public class AccountManagerService return; } // Also wipe out cached token in memory. - for (Account a : accounts.accountTokenCaches.keySet()) { - if (a.type.equals(accountType)) { - WeakReference tokenCacheRef = - accounts.accountTokenCaches.get(a); - TokenCache cache = null; - if (tokenCacheRef != null && (cache = tokenCacheRef.get()) != null) { - cache.remove(authToken); - } - } - } + accounts.accountTokenCaches.remove(accountType, authToken); } private void invalidateAuthTokenLocked(UserAccounts accounts, SQLiteDatabase db, @@ -1459,11 +1460,8 @@ public class AccountManagerService cancelNotification(getSigninRequiredNotificationId(accounts, account), new UserHandle(accounts.userId)); synchronized (accounts.cacheLock) { - TokenCache cache = getTokenCacheForAccountLocked(accounts, account); - if (cache != null) { - cache.put(token, tokenType, callerPkg, callerSigDigest, expiryMillis); - } - return; + accounts.accountTokenCaches.put( + account, token, tokenType, callerPkg, callerSigDigest, expiryMillis); } } @@ -1512,7 +1510,7 @@ public class AccountManagerService } if (account == null) throw new IllegalArgumentException("account is null"); if (authTokenType == null) throw new IllegalArgumentException("authTokenType is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot peek the authtokens associated with accounts of type: %s", callingUid, @@ -1539,7 +1537,7 @@ public class AccountManagerService } if (account == null) throw new IllegalArgumentException("account is null"); if (authTokenType == null) throw new IllegalArgumentException("authTokenType is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot set auth tokens associated with accounts of type: %s", callingUid, @@ -1564,7 +1562,7 @@ public class AccountManagerService + ", pid " + Binder.getCallingPid()); } if (account == null) throw new IllegalArgumentException("account is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot set secrets for accounts of type: %s", callingUid, @@ -1627,7 +1625,7 @@ public class AccountManagerService + ", pid " + Binder.getCallingPid()); } if (account == null) throw new IllegalArgumentException("account is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot clear passwords for accounts of type: %s", callingUid, @@ -1654,7 +1652,7 @@ public class AccountManagerService } if (key == null) throw new IllegalArgumentException("key is null"); if (account == null) throw new IllegalArgumentException("account is null"); - if (!isAccountOwnedByCallingUid(account.type, callingUid)) { + if (!isAccountManagedByCaller(account.type, callingUid)) { String msg = String.format( "uid %s cannot set user data for accounts of type: %s", callingUid, @@ -1780,7 +1778,6 @@ public class AccountManagerService final boolean notifyOnAuthFailure, final boolean expectActivityLaunch, final Bundle loginOptions) { - if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "getAuthToken: " + account + ", response " + response @@ -1877,6 +1874,9 @@ public class AccountManagerService callerPkg, callerPkgSigDigest); if (token != null) { + if (Log.isLoggable(TAG, Log.VERBOSE)) { + Log.v(TAG, "getAuthToken: cache hit ofr custom token authenticator."); + } Bundle result = new Bundle(); result.putString(AccountManager.KEY_AUTHTOKEN, token); result.putString(AccountManager.KEY_ACCOUNT_NAME, account.name); @@ -1914,10 +1914,11 @@ public class AccountManagerService public void onResult(Bundle result) { if (result != null) { if (result.containsKey(AccountManager.KEY_AUTH_TOKEN_LABEL)) { - Intent intent = newGrantCredentialsPermissionIntent(account, callerUid, + Intent intent = newGrantCredentialsPermissionIntent( + account, + callerUid, new AccountAuthenticatorResponse(this), - authTokenType, - result.getString(AccountManager.KEY_AUTH_TOKEN_LABEL)); + authTokenType); Bundle bundle = new Bundle(); bundle.putParcelable(AccountManager.KEY_INTENT, intent); onResult(bundle); @@ -1995,9 +1996,6 @@ public class AccountManagerService GrantCredentialsPermissionActivity.EXTRAS_REQUESTING_UID, -1); String authTokenType = intent.getStringExtra( GrantCredentialsPermissionActivity.EXTRAS_AUTH_TOKEN_TYPE); - String authTokenLabel = intent.getStringExtra( - GrantCredentialsPermissionActivity.EXTRAS_AUTH_TOKEN_LABEL); - final String titleAndSubtitle = mContext.getString(R.string.permission_request_notification_with_subtitle, account.name); @@ -2025,7 +2023,7 @@ public class AccountManagerService } private Intent newGrantCredentialsPermissionIntent(Account account, int uid, - AccountAuthenticatorResponse response, String authTokenType, String authTokenLabel) { + AccountAuthenticatorResponse response, String authTokenType) { Intent intent = new Intent(mContext, GrantCredentialsPermissionActivity.class); // See FLAG_ACTIVITY_NEW_TASK docs for limitations and benefits of the flag. @@ -2149,6 +2147,7 @@ public class AccountManagerService public void addAccountAsUser(final IAccountManagerResponse response, final String accountType, final String authTokenType, final String[] requiredFeatures, final boolean expectActivityLaunch, final Bundle optionsIn, int userId) { + int callingUid = Binder.getCallingUid(); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "addAccount: accountType " + accountType + ", response " + response @@ -2161,10 +2160,14 @@ public class AccountManagerService } if (response == null) throw new IllegalArgumentException("response is null"); if (accountType == null) throw new IllegalArgumentException("accountType is null"); - // Only allow the system process to add accounts of other users - enforceCrossUserPermission(userId, "User " + UserHandle.getCallingUserId() - + " trying to add account for " + userId); + if (isCrossUser(callingUid, userId)) { + throw new SecurityException( + String.format( + "User %s trying to add account for %s" , + UserHandle.getCallingUserId(), + userId)); + } // Is user disallowed from modifying accounts? if (!canUserModifyAccounts(userId)) { @@ -2235,20 +2238,28 @@ public class AccountManagerService } @Override - public void confirmCredentialsAsUser(IAccountManagerResponse response, - final Account account, final Bundle options, final boolean expectActivityLaunch, + public void confirmCredentialsAsUser( + IAccountManagerResponse response, + final Account account, + final Bundle options, + final boolean expectActivityLaunch, int userId) { - // Only allow the system process to read accounts of other users - enforceCrossUserPermission(userId, "User " + UserHandle.getCallingUserId() - + " trying to confirm account credentials for " + userId); - + int callingUid = Binder.getCallingUid(); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "confirmCredentials: " + account + ", response " + response + ", expectActivityLaunch " + expectActivityLaunch - + ", caller's uid " + Binder.getCallingUid() + + ", caller's uid " + callingUid + ", pid " + Binder.getCallingPid()); } + // Only allow the system process to read accounts of other users + if (isCrossUser(callingUid, userId)) { + throw new SecurityException( + String.format( + "User %s trying to confirm account credentials for %s" , + UserHandle.getCallingUserId(), + userId)); + } if (response == null) throw new IllegalArgumentException("response is null"); if (account == null) throw new IllegalArgumentException("account is null"); UserAccounts accounts = getUserAccounts(userId); @@ -2324,7 +2335,7 @@ public class AccountManagerService } if (response == null) throw new IllegalArgumentException("response is null"); if (accountType == null) throw new IllegalArgumentException("accountType is null"); - if (!isAccountOwnedByCallingUid(accountType, callingUid) && !isSystemUid(callingUid)) { + if (!isAccountManagedByCaller(accountType, callingUid) && !isSystemUid(callingUid)) { String msg = String.format( "uid %s cannot edit authenticator properites for account type: %s", callingUid, @@ -2457,9 +2468,11 @@ public class AccountManagerService * @hide */ public Account[] getAccounts(int userId) { - checkReadAccountsPermission(); UserAccounts accounts = getUserAccounts(userId); int callingUid = Binder.getCallingUid(); + if (!isReadAccountsPermitted(callingUid, null)) { + return new Account[0]; + } long identityToken = clearCallingIdentity(); try { synchronized (accounts.cacheLock) { @@ -2519,7 +2532,10 @@ public class AccountManagerService return getAccountsAsUser(type, userId, null, -1); } - private Account[] getAccountsAsUser(String type, int userId, String callingPackage, + private Account[] getAccountsAsUser( + String type, + int userId, + String callingPackage, int packageUid) { int callingUid = Binder.getCallingUid(); // Only allow the system process to read accounts of other users @@ -2542,7 +2558,12 @@ public class AccountManagerService if (packageUid != -1 && UserHandle.isSameApp(callingUid, Process.myUid())) { callingUid = packageUid; } - checkReadAccountsPermission(); + + // Authenticators should be able to see their own accounts regardless of permissions. + if (TextUtils.isEmpty(type) && !isReadAccountsPermitted(callingUid, type)) { + return new Account[0]; + } + long identityToken = clearCallingIdentity(); try { UserAccounts accounts = getUserAccounts(userId); @@ -2593,7 +2614,7 @@ public class AccountManagerService logRecord(db, DebugDbHelper.ACTION_ACCOUNT_RENAME, TABLE_SHARED_ACCOUNTS, sharedTableAccountId, accounts, callingUid); // Recursively rename the account. - renameAccountInternal(accounts, account, newName, callingUid); + renameAccountInternal(accounts, account, newName); } return r > 0; } @@ -2663,7 +2684,6 @@ public class AccountManagerService @Override public Account[] getAccountsByTypeForPackage(String type, String packageName) { - checkBinderPermission(android.Manifest.permission.INTERACT_ACROSS_USERS); int packageUid = -1; try { packageUid = AppGlobals.getPackageManager().getPackageUid( @@ -2676,20 +2696,31 @@ public class AccountManagerService } @Override - public void getAccountsByFeatures(IAccountManagerResponse response, - String type, String[] features) { + public void getAccountsByFeatures( + IAccountManagerResponse response, + String type, + String[] features) { + int callingUid = Binder.getCallingUid(); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, "getAccounts: accountType " + type + ", response " + response + ", features " + stringArrayToString(features) - + ", caller's uid " + Binder.getCallingUid() + + ", caller's uid " + callingUid + ", pid " + Binder.getCallingPid()); } if (response == null) throw new IllegalArgumentException("response is null"); if (type == null) throw new IllegalArgumentException("accountType is null"); - checkReadAccountsPermission(); + if (!isReadAccountsPermitted(callingUid, type)) { + Bundle result = new Bundle(); + result.putParcelableArray(AccountManager.KEY_ACCOUNTS, new Account[0]); + try { + response.onResult(result); + } catch (RemoteException e) { + Log.e(TAG, "Cannot respond to caller do to exception." , e); + } + return; + } UserAccounts userAccounts = getUserAccountsForCaller(); - int callingUid = Binder.getCallingUid(); long identityToken = clearCallingIdentity(); try { if (features == null || features.length == 0) { @@ -2872,11 +2903,6 @@ public class AccountManagerService } } - public void scheduleTimeout() { - mMessageHandler.sendMessageDelayed( - mMessageHandler.obtainMessage(MESSAGE_TIMED_OUT, this), TIMEOUT_DELAY_MS); - } - public void cancelTimeout() { mMessageHandler.removeMessages(MESSAGE_TIMED_OUT, this); } @@ -3181,12 +3207,9 @@ public class AccountManagerService // who called. private static String ACTION_CALLED_ACCOUNT_ADD = "action_called_account_add"; private static String ACTION_CALLED_ACCOUNT_REMOVE = "action_called_account_remove"; - private static String ACTION_CALLED_ACCOUNT_RENAME = "action_called_account_rename"; private static SimpleDateFormat dateFromat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); - private static String UPDATE_WHERE_CLAUSE = KEY + "=?"; - private static void createDebugTable(SQLiteDatabase db) { db.execSQL("CREATE TABLE " + TABLE_DEBUG + " ( " + ACCOUNTS_ID + " INTEGER," @@ -3421,7 +3444,7 @@ public class AccountManagerService } } - public IBinder onBind(Intent intent) { + public IBinder onBind(@SuppressWarnings("unused") Intent intent) { return asBinder(); } @@ -3576,21 +3599,29 @@ public class AccountManagerService } } - /** Succeeds if any of the specified permissions are granted. */ - private void checkBinderPermission(String... permissions) { - final int uid = Binder.getCallingUid(); - + private boolean isPermitted(int callingUid, String... permissions) { for (String perm : permissions) { if (mContext.checkCallingOrSelfPermission(perm) == PackageManager.PERMISSION_GRANTED) { if (Log.isLoggable(TAG, Log.VERBOSE)) { - Log.v(TAG, " caller uid " + uid + " has " + perm); + Log.v(TAG, " caller uid " + callingUid + " has " + perm); } - return; + return true; } } - String msg = "caller uid " + uid + " lacks any of " + TextUtils.join(",", permissions); - Log.w(TAG, " " + msg); - throw new SecurityException(msg); + return false; + } + + /** Succeeds if any of the specified permissions are granted. */ + private void checkBinderPermission(String... permissions) { + final int callingUid = Binder.getCallingUid(); + if (isPermitted(callingUid, permissions)) { + String msg = String.format( + "caller uid %s lacks any of %s", + callingUid, + TextUtils.join(",", permissions)); + Log.w(TAG, " " + msg); + throw new SecurityException(msg); + } } private int handleIncomingUser(int userId) { @@ -3646,6 +3677,9 @@ public class AccountManagerService } private boolean isAccountManagedByCaller(String accountType, int callingUid) { + if (accountType == null) { + return false; + } final int callingUserId = UserHandle.getUserId(callingUid); for (RegisteredServicesCache.ServiceInfo serviceInfo : mAuthenticatorCache.getAllServices(callingUserId)) { @@ -3723,20 +3757,34 @@ public class AccountManagerService return false; } - private boolean isAccountOwnedByCallingUid(String accountType, int callingUid) { - if (!isAccountManagedByCaller(accountType, callingUid)) { - String msg = "caller uid " + callingUid + " is different than the authenticator's uid"; - Log.w(TAG, msg); - return false; - } - if (Log.isLoggable(TAG, Log.VERBOSE)) { - Log.v(TAG, "caller uid " + callingUid + " is the same as the authenticator's uid"); - } - return true; + 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); + boolean isAccountManagedByCaller = isAccountManagedByCaller(accountType, callingUid); + Log.w(TAG, String.format( + "isReadAccountPermitted: isPermitted: %s, isAM: %s", + isPermitted, + isAccountManagedByCaller)); + return isPermitted || isAccountManagedByCaller; } - private void checkReadAccountsPermission() { - checkBinderPermission(Manifest.permission.GET_ACCOUNTS); + /** Succeeds if any of the specified permissions are granted. */ + private void checkReadAccountsPermitted( + int callingUid, + String accountType) { + if (!isReadAccountsPermitted(callingUid, accountType)) { + String msg = String.format( + "caller uid %s cannot access %s accounts", + callingUid, + accountType); + Log.w(TAG, " " + msg); + throw new SecurityException(msg); + } } private boolean canUserModifyAccounts(int userId) { @@ -4008,8 +4056,8 @@ public class AccountManagerService String callingPackage, byte[] pkgSigDigest) { synchronized (accounts.cacheLock) { - TokenCache cache = getTokenCacheForAccountLocked(accounts, account); - return cache.get(tokenType, callingPackage, pkgSigDigest); + return accounts.accountTokenCaches.get( + account, tokenType, callingPackage, pkgSigDigest); } } @@ -4094,17 +4142,6 @@ public class AccountManagerService return authTokensForAccount; } - protected TokenCache getTokenCacheForAccountLocked(UserAccounts accounts, Account account) { - WeakReference cacheRef = accounts.accountTokenCaches.get(account); - TokenCache cache; - if (cacheRef == null || (cache = cacheRef.get()) == null) { - cache = new TokenCache(); - cacheRef = new WeakReference<>(cache); - accounts.accountTokenCaches.put(account, cacheRef); - } - return cache; - } - private Context getContextForUser(UserHandle user) { try { return mContext.createPackageContextAsUser(mContext.getPackageName(), 0, user); diff --git a/services/core/java/com/android/server/accounts/TokenCache.java b/services/core/java/com/android/server/accounts/TokenCache.java index 70a7010bd00f..be91f9831113 100644 --- a/services/core/java/com/android/server/accounts/TokenCache.java +++ b/services/core/java/com/android/server/accounts/TokenCache.java @@ -16,6 +16,12 @@ package com.android.server.accounts; +import android.accounts.Account; +import android.util.LruCache; +import android.util.Pair; + +import com.android.internal.util.Preconditions; + import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -23,10 +29,12 @@ import java.util.List; import java.util.Objects; /** - * TokenCaches manage tokens associated with an account in memory. + * TokenCaches manage time limited authentication tokens in memory. */ /* default */ class TokenCache { + private static final int MAX_CACHE_CHARS = 64000; + private static class Value { public final String token; public final long expiryEpochMillis; @@ -38,11 +46,13 @@ import java.util.Objects; } private static class Key { + public final Account account; public final String packageName; public final String tokenType; public final byte[] sigDigest; - public Key(String tokenType, String packageName, byte[] sigDigest) { + public Key(Account account, String tokenType, String packageName, byte[] sigDigest) { + this.account = account; this.tokenType = tokenType; this.packageName = packageName; this.sigDigest = sigDigest; @@ -52,7 +62,8 @@ import java.util.Objects; public boolean equals(Object o) { if (o != null && o instanceof Key) { Key cacheKey = (Key) o; - return Objects.equals(packageName, cacheKey.packageName) + return Objects.equals(account, cacheKey.account) + && Objects.equals(packageName, cacheKey.packageName) && Objects.equals(tokenType, cacheKey.tokenType) && Arrays.equals(sigDigest, cacheKey.sigDigest); } else { @@ -62,46 +73,109 @@ import java.util.Objects; @Override public int hashCode() { - return packageName.hashCode() ^ tokenType.hashCode() ^ Arrays.hashCode(sigDigest); + return account.hashCode() + ^ packageName.hashCode() + ^ tokenType.hashCode() + ^ Arrays.hashCode(sigDigest); } } - /** - * Map associating basic token lookup information with with actual tokens (and optionally their - * expiration times). - */ - private HashMap mCachedTokens = new HashMap<>(); + private static class TokenLruCache extends LruCache { - /** - * Map associated tokens with an Evictor that will manage evicting the token from the cache. - * This reverse lookup is needed because very little information is given at token invalidation - * time. - */ - private HashMap mTokenEvictors = new HashMap<>(); + private class Evictor { + private final List mKeys; + + public Evictor() { + mKeys = new ArrayList<>(); + } + + public void add(Key k) { + mKeys.add(k); + } + + public void evict() { + for (Key k : mKeys) { + TokenLruCache.this.remove(k); + } + } + } + + /** + * Map associated tokens with an Evictor that will manage evicting the token from the + * cache. This reverse lookup is needed because very little information is given at token + * invalidation time. + */ + private HashMap, Evictor> mTokenEvictors = new HashMap<>(); + private HashMap mAccountEvictors = new HashMap<>(); + + public TokenLruCache() { + super(MAX_CACHE_CHARS); + } + + @Override + protected int sizeOf(Key k, Value v) { + return v.token.length(); + } + + @Override + protected void entryRemoved(boolean evicted, Key k, Value oldVal, Value newVal) { + // When a token has been removed, clean up the associated Evictor. + if (oldVal != null && newVal == null) { + /* + * This is recursive, but it won't spiral out of control because LruCache is + * thread safe and the Evictor can only be removed once. + */ + Evictor evictor = mTokenEvictors.remove(oldVal.token); + if (evictor != null) { + evictor.evict(); + } + } + } - private class Evictor { - private final String mToken; - private final List mKeys; + public void putToken(Key k, Value v) { + // Prepare for removal by token string. + Evictor tokenEvictor = mTokenEvictors.get(v.token); + if (tokenEvictor == null) { + tokenEvictor = new Evictor(); + } + tokenEvictor.add(k); + mTokenEvictors.put(new Pair<>(k.account.type, v.token), tokenEvictor); - public Evictor(String token) { - mKeys = new ArrayList<>(); - mToken = token; + // Prepare for removal by associated account. + Evictor accountEvictor = mAccountEvictors.get(k.account); + if (accountEvictor == null) { + accountEvictor = new Evictor(); + } + accountEvictor.add(k); + mAccountEvictors.put(k.account, tokenEvictor); + + // Only cache the token once we can remove it directly or by account. + put(k, v); } - public void add(Key k) { - mKeys.add(k); + public void evict(String accountType, String token) { + Evictor evictor = mTokenEvictors.get(new Pair<>(accountType, token)); + if (evictor != null) { + evictor.evict(); + } + } - public void evict() { - for (Key k : mKeys) { - mCachedTokens.remove(k); + public void evict(Account account) { + Evictor evictor = mAccountEvictors.get(account); + if (evictor != null) { + evictor.evict(); } - // Clear out the evictor reference. - mTokenEvictors.remove(mToken); } } /** + * Map associating basic token lookup information with with actual tokens (and optionally their + * expiration times). + */ + private TokenLruCache mCachedTokens = new TokenLruCache(); + + /** * Caches the specified token until the specified expiryMillis. The token will be associated * with the given token type, package name, and digest of signatures. * @@ -112,51 +186,44 @@ import java.util.Objects; * @param expiryMillis */ public void put( + Account account, String token, String tokenType, String packageName, byte[] sigDigest, long expiryMillis) { + Preconditions.checkNotNull(account); if (token == null || System.currentTimeMillis() > expiryMillis) { return; } - Key k = new Key(tokenType, packageName, sigDigest); - // Prep evictor. No token should be cached without a corresponding evictor. - Evictor evictor = mTokenEvictors.get(token); - if (evictor == null) { - evictor = new Evictor(token); - } - evictor.add(k); - mTokenEvictors.put(token, evictor); - // Then cache values. + Key k = new Key(account, tokenType, packageName, sigDigest); Value v = new Value(token, expiryMillis); - mCachedTokens.put(k, v); + mCachedTokens.putToken(k, v); } /** * Evicts the specified token from the cache. This should be called as part of a token * invalidation workflow. */ - public void remove(String token) { - Evictor evictor = mTokenEvictors.get(token); - if (evictor == null) { - // This condition is expected if the token isn't cached. - return; - } - evictor.evict(); + public void remove(String accountType, String token) { + mCachedTokens.evict(accountType, token); + } + + public void remove(Account account) { + mCachedTokens.evict(account); } /** * Gets a token from the cache if possible. */ - public String get(String tokenType, String packageName, byte[] sigDigest) { - Key k = new Key(tokenType, packageName, sigDigest); + public String get(Account account, String tokenType, String packageName, byte[] sigDigest) { + Key k = new Key(account, tokenType, packageName, sigDigest); Value v = mCachedTokens.get(k); long currentTime = System.currentTimeMillis(); if (v != null && currentTime < v.expiryEpochMillis) { return v.token; } else if (v != null) { - remove(v.token); + remove(account.type, v.token); } return null; } -- 2.11.0