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
diff --git a/core/java/android/accounts/AccountManager.java b/core/java/android/accounts/AccountManager.java
index 3001c2c..aa7692b 100644
--- a/core/java/android/accounts/AccountManager.java
+++ b/core/java/android/accounts/AccountManager.java
@@ -489,7 +489,8 @@
* <p>It is safe to call this method from the main thread.
*
* <p>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 @@
* {@link AccountManagerFuture} must not be used on the main thread.
*
* <p>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 50d311f..a270974 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 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 @@
implements RegisteredServicesCacheListener<AuthenticatorDescription> {
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 @@
new HashMap<Account, HashMap<String, String>>();
/** protected by the {@link #cacheLock} */
- private final HashMap<Account, WeakReference<TokenCache>> accountTokenCaches = new HashMap<>();
+ private final TokenCache accountTokenCaches = new TokenCache();
/**
* protected by the {@link #cacheLock}
@@ -529,7 +527,7 @@
+ ", 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 @@
}
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 @@
@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<AccountAuthenticatorCache.ServiceInfo<AuthenticatorDescription>>
@@ -672,14 +677,12 @@
}
}
- 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 @@
+ ", 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 @@
@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 @@
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 @@
@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 @@
+ ", 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 @@
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 @@
}
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 @@
}
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 @@
*/
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 @@
return;
}
// Also wipe out cached token in memory.
- for (Account a : accounts.accountTokenCaches.keySet()) {
- if (a.type.equals(accountType)) {
- WeakReference<TokenCache> 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 @@
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 @@
}
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 @@
}
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 @@
+ ", 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 @@
+ ", 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 @@
}
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 @@
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 @@
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 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 @@
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 @@
}
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 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 @@
}
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 @@
}
@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 @@
}
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 @@
* @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 @@
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 @@
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 @@
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 @@
@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 @@
}
@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 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 @@
// 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 IBinder onBind(Intent intent) {
+ public IBinder onBind(@SuppressWarnings("unused") Intent intent) {
return asBinder();
}
@@ -3576,21 +3599,29 @@
}
}
- /** 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 @@
}
private boolean isAccountManagedByCaller(String accountType, int callingUid) {
+ if (accountType == null) {
+ return false;
+ }
final int callingUserId = UserHandle.getUserId(callingUid);
for (RegisteredServicesCache.ServiceInfo<AuthenticatorDescription> serviceInfo :
mAuthenticatorCache.getAllServices(callingUserId)) {
@@ -3723,20 +3757,34 @@
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 @@
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 @@
return authTokensForAccount;
}
- protected TokenCache getTokenCacheForAccountLocked(UserAccounts accounts, Account account) {
- WeakReference<TokenCache> 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 70a7010..be91f98 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.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 @@
}
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 @@
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,7 +73,99 @@
@Override
public int hashCode() {
- return packageName.hashCode() ^ tokenType.hashCode() ^ Arrays.hashCode(sigDigest);
+ return account.hashCode()
+ ^ packageName.hashCode()
+ ^ tokenType.hashCode()
+ ^ Arrays.hashCode(sigDigest);
+ }
+ }
+
+ private static class TokenLruCache extends LruCache<Key, Value> {
+
+ private class Evictor {
+ private final List<Key> 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<Pair<String, String>, Evictor> mTokenEvictors = new HashMap<>();
+ private HashMap<Account, Evictor> 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();
+ }
+ }
+ }
+
+ 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);
+
+ // 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 evict(String accountType, String token) {
+ Evictor evictor = mTokenEvictors.get(new Pair<>(accountType, token));
+ if (evictor != null) {
+ evictor.evict();
+ }
+
+ }
+
+ public void evict(Account account) {
+ Evictor evictor = mAccountEvictors.get(account);
+ if (evictor != null) {
+ evictor.evict();
+ }
}
}
@@ -70,36 +173,7 @@
* Map associating basic token lookup information with with actual tokens (and optionally their
* expiration times).
*/
- private HashMap<Key, Value> mCachedTokens = new HashMap<>();
-
- /**
- * 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<String, Evictor> mTokenEvictors = new HashMap<>();
-
- private class Evictor {
- private final String mToken;
- private final List<Key> mKeys;
-
- public Evictor(String token) {
- mKeys = new ArrayList<>();
- mToken = token;
- }
-
- public void add(Key k) {
- mKeys.add(k);
- }
-
- public void evict() {
- for (Key k : mKeys) {
- mCachedTokens.remove(k);
- }
- // Clear out the evictor reference.
- mTokenEvictors.remove(mToken);
- }
- }
+ private TokenLruCache mCachedTokens = new TokenLruCache();
/**
* Caches the specified token until the specified expiryMillis. The token will be associated
@@ -112,51 +186,44 @@
* @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;
}
