While-list apps to access account if already saw it
Sync adapters without an account access cannot run until the
user approves the account access (for the case the account
access is not allowed by other policy such as being singed
with the same cert as the authenticator). However, if the
sync adapter package already got the account from another
app which means it already saw the account we white-list
the sync adapter app to access the account as it already
saw it - the bird is out of the cage.
bug:31162498
Change-Id: I2b72f3b0d6307561ed68db2f2e9c900b15e8d098
diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java
index b152372..a275773 100644
--- a/services/core/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -26,12 +26,14 @@
import android.accounts.AuthenticatorDescription;
import android.accounts.CantAddAccountActivity;
import android.accounts.GrantCredentialsPermissionActivity;
+import android.accounts.IAccountAccessTracker;
import android.accounts.IAccountAuthenticator;
import android.accounts.IAccountAuthenticatorResponse;
import android.accounts.IAccountManager;
import android.accounts.IAccountManagerResponse;
import android.annotation.IntRange;
import android.annotation.NonNull;
+import android.annotation.Nullable;
import android.app.ActivityManager;
import android.app.ActivityManagerNative;
import android.app.ActivityThread;
@@ -120,11 +122,11 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
@@ -285,7 +287,7 @@
private final Object cacheLock = new Object();
/** protected by the {@link #cacheLock} */
private final HashMap<String, Account[]> accountCache =
- new LinkedHashMap<String, Account[]>();
+ new LinkedHashMap<>();
/** protected by the {@link #cacheLock} */
private final Map<Account, Map<String, String>> userDataCache = new HashMap<>();
/** protected by the {@link #cacheLock} */
@@ -337,6 +339,8 @@
private final SparseArray<UserAccounts> mUsers = new SparseArray<>();
private final SparseBooleanArray mLocalUnlockedUsers = new SparseBooleanArray();
+ private CopyOnWriteArrayList<AccountManagerInternal.OnAppPermissionChangeListener>
+ mAppPermissionChangeListeners = new CopyOnWriteArrayList<>();
private static AtomicReference<AccountManagerService> sThis = new AtomicReference<>();
private static final Account[] EMPTY_ACCOUNT_ARRAY = new Account[]{};
@@ -560,7 +564,7 @@
if (!checkAccess || hasAccountAccess(account, packageName,
UserHandle.getUserHandleForUid(uid))) {
cancelNotification(getCredentialPermissionNotificationId(account,
- AccountManager.ACCOUNT_ACCESS_TOKEN, uid), packageName,
+ AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE, uid), packageName,
UserHandle.getUserHandleForUid(uid));
}
}
@@ -1196,7 +1200,8 @@
final ArrayList<String> accountNames = cur.getValue();
final Account[] accountsForType = new Account[accountNames.size()];
for (int i = 0; i < accountsForType.length; i++) {
- accountsForType[i] = new Account(accountNames.get(i), accountType);
+ accountsForType[i] = new Account(accountNames.get(i), accountType,
+ new AccountAccessTracker());
}
accounts.accountCache.put(accountType, accountsForType);
}
@@ -1977,6 +1982,8 @@
Bundle result = new Bundle();
result.putString(AccountManager.KEY_ACCOUNT_NAME, resultingAccount.name);
result.putString(AccountManager.KEY_ACCOUNT_TYPE, resultingAccount.type);
+ result.putBinder(AccountManager.KEY_ACCOUNT_ACCESS_TRACKER,
+ resultingAccount.getAccessTracker().asBinder());
try {
response.onResult(result);
} catch (RemoteException e) {
@@ -2324,7 +2331,7 @@
for (Pair<Pair<Account, String>, Integer> key
: accounts.credentialsPermissionNotificationIds.keySet()) {
if (account.equals(key.first.first)
- && AccountManager.ACCOUNT_ACCESS_TOKEN.equals(key.first.second)) {
+ && AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE.equals(key.first.second)) {
final int uid = (Integer) key.second;
mHandler.post(() -> cancelAccountAccessRequestNotificationIfNeeded(
account, uid, false));
@@ -3887,22 +3894,36 @@
Preconditions.checkArgumentInRange(userId, 0, Integer.MAX_VALUE, "user must be concrete");
try {
-
final int uid = mPackageManager.getPackageUidAsUser(packageName, userId);
- // Use null token which means any token. Having a token means the package
- // is trusted by the authenticator, hence it is fine to access the account.
- if (permissionIsGranted(account, null, uid, userId)) {
- return true;
- }
- // In addition to the permissions required to get an auth token we also allow
- // the account to be accessed by holders of the get accounts permissions.
- return checkUidPermission(Manifest.permission.GET_ACCOUNTS_PRIVILEGED, uid, packageName)
- || checkUidPermission(Manifest.permission.GET_ACCOUNTS, uid, packageName);
+ return hasAccountAccess(account, packageName, uid);
} catch (NameNotFoundException e) {
return false;
}
}
+ private boolean hasAccountAccess(@NonNull Account account, @Nullable String packageName,
+ int uid) {
+ if (packageName == null) {
+ String[] packageNames = mPackageManager.getPackagesForUid(uid);
+ if (ArrayUtils.isEmpty(packageNames)) {
+ return false;
+ }
+ // For app op checks related to permissions all packages in the UID
+ // have the same app op state, so doesn't matter which one we pick.
+ packageName = packageNames[0];
+ }
+
+ // Use null token which means any token. Having a token means the package
+ // is trusted by the authenticator, hence it is fine to access the account.
+ if (permissionIsGranted(account, null, uid, UserHandle.getUserId(uid))) {
+ return true;
+ }
+ // In addition to the permissions required to get an auth token we also allow
+ // the account to be accessed by holders of the get accounts permissions.
+ return checkUidPermission(Manifest.permission.GET_ACCOUNTS_PRIVILEGED, uid, packageName)
+ || checkUidPermission(Manifest.permission.GET_ACCOUNTS, uid, packageName);
+ }
+
private boolean checkUidPermission(String permission, int uid, String opPackageName) {
final long identity = Binder.clearCallingIdentity();
try {
@@ -3978,7 +3999,7 @@
private void handleAuthenticatorResponse(boolean accessGranted) throws RemoteException {
cancelNotification(getCredentialPermissionNotificationId(account,
- AccountManager.ACCOUNT_ACCESS_TOKEN, uid), packageName,
+ AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE, uid), packageName,
UserHandle.getUserHandleForUid(uid));
if (callback != null) {
Bundle result = new Bundle();
@@ -3986,7 +4007,7 @@
callback.sendResult(result);
}
}
- }), AccountManager.ACCOUNT_ACCESS_TOKEN, false);
+ }), AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE, false);
}
@Override
@@ -5939,6 +5960,12 @@
cancelAccountAccessRequestNotificationIfNeeded(account, uid, true);
}
+
+ // Listeners are a final CopyOnWriteArrayList, hence no lock needed.
+ for (AccountManagerInternal.OnAppPermissionChangeListener listener
+ : mAppPermissionChangeListeners) {
+ mHandler.post(() -> listener.onAppPermissionChanged(account, uid));
+ }
}
/**
@@ -5968,9 +5995,16 @@
} finally {
db.endTransaction();
}
+
cancelNotification(getCredentialPermissionNotificationId(account, authTokenType, uid),
new UserHandle(accounts.userId));
}
+
+ // Listeners are a final CopyOnWriteArrayList, hence no lock needed.
+ for (AccountManagerInternal.OnAppPermissionChangeListener listener
+ : mAppPermissionChangeListeners) {
+ mHandler.post(() -> listener.onAppPermissionChanged(account, uid));
+ }
}
static final private String stringArrayToString(String[] value) {
@@ -6009,7 +6043,7 @@
if (accountsForType != null) {
System.arraycopy(accountsForType, 0, newAccountsForType, 0, oldLength);
}
- newAccountsForType[oldLength] = account;
+ newAccountsForType[oldLength] = new Account(account, new AccountAccessTracker());
accounts.accountCache.put(account.type, newAccountsForType);
}
@@ -6626,6 +6660,33 @@
}
}
+ private final class AccountAccessTracker extends IAccountAccessTracker.Stub {
+ @Override
+ public void onAccountAccessed() throws RemoteException {
+ final int uid = Binder.getCallingUid();
+ if (UserHandle.getAppId(uid) == Process.SYSTEM_UID) {
+ return;
+ }
+ final int userId = UserHandle.getCallingUserId();
+ final long identity = Binder.clearCallingIdentity();
+ try {
+ for (Account account : getAccounts(userId, mContext.getOpPackageName())) {
+ IAccountAccessTracker accountTracker = account.getAccessTracker();
+ if (accountTracker != null && asBinder() == accountTracker.asBinder()) {
+ // An app just accessed the account. At this point it knows about
+ // it and there is not need to hide this account from the app.
+ if (!hasAccountAccess(account, null, uid)) {
+ updateAppPermission(account, AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE,
+ uid, true);
+ }
+ }
+ }
+ } finally {
+ Binder.restoreCallingIdentity(identity);
+ }
+ }
+ }
+
private final class AccountManagerInternalImpl extends AccountManagerInternal {
@Override
public void requestAccountAccess(@NonNull Account account, @NonNull String packageName,
@@ -6647,7 +6708,8 @@
return;
}
- if (hasAccountAccess(account, packageName, new UserHandle(userId))) {
+ if (AccountManagerService.this.hasAccountAccess(account, packageName,
+ new UserHandle(userId))) {
Bundle result = new Bundle();
result.putBoolean(AccountManager.KEY_BOOLEAN_RESULT, true);
callback.sendResult(result);
@@ -6663,7 +6725,22 @@
}
Intent intent = newRequestAccountAccessIntent(account, packageName, uid, callback);
- doNotification(mUsers.get(userId), account, null, intent, packageName, userId);
+ final UserAccounts userAccounts;
+ synchronized (mUsers) {
+ userAccounts = mUsers.get(userId);
+ }
+ doNotification(userAccounts, account, null, intent, packageName, userId);
+ }
+
+ @Override
+ public void addOnAppPermissionChangeListener(OnAppPermissionChangeListener listener) {
+ // Listeners are a final CopyOnWriteArrayList, hence no lock needed.
+ mAppPermissionChangeListeners.add(listener);
+ }
+
+ @Override
+ public boolean hasAccountAccess(@NonNull Account account, @IntRange(from = 0) int uid) {
+ return AccountManagerService.this.hasAccountAccess(account, null, uid);
}
}
}
diff --git a/services/core/java/com/android/server/content/ContentService.java b/services/core/java/com/android/server/content/ContentService.java
index 12955f5..4e236d1 100644
--- a/services/core/java/com/android/server/content/ContentService.java
+++ b/services/core/java/com/android/server/content/ContentService.java
@@ -482,7 +482,7 @@
SyncManager syncManager = getSyncManager();
if (syncManager != null) {
syncManager.scheduleSync(account, userId, uId, authority, extras,
- false /* onlyThoseWithUnkownSyncableState */);
+ SyncStorageEngine.AuthorityInfo.UNDEFINED);
}
} finally {
restoreCallingIdentity(identityToken);
@@ -548,7 +548,7 @@
} else {
syncManager.scheduleSync(
request.getAccount(), userId, callerUid, request.getProvider(), extras,
- false /* onlyThoseWithUnknownSyncableState */);
+ SyncStorageEngine.AuthorityInfo.UNDEFINED);
}
} finally {
restoreCallingIdentity(identityToken);
diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java
index ef3c4b2..22bc60d 100644
--- a/services/core/java/com/android/server/content/SyncManager.java
+++ b/services/core/java/com/android/server/content/SyncManager.java
@@ -504,7 +504,7 @@
@Override
public void onSyncRequest(SyncStorageEngine.EndPoint info, int reason, Bundle extras) {
scheduleSync(info.account, info.userId, reason, info.provider, extras,
- false);
+ AuthorityInfo.UNDEFINED);
}
});
@@ -534,7 +534,7 @@
if (!removed) {
scheduleSync(null, UserHandle.USER_ALL,
SyncOperation.REASON_SERVICE_CHANGED,
- type.authority, null, false /* onlyThoseWithUnkownSyncableState */);
+ type.authority, null, AuthorityInfo.UNDEFINED);
}
}
}, mSyncHandler);
@@ -576,6 +576,15 @@
mAccountManager = (AccountManager) mContext.getSystemService(Context.ACCOUNT_SERVICE);
mAccountManagerInternal = LocalServices.getService(AccountManagerInternal.class);
+ mAccountManagerInternal.addOnAppPermissionChangeListener((Account account, int uid) -> {
+ // If the UID gained access to the account kick-off syncs lacking account access
+ if (mAccountManagerInternal.hasAccountAccess(account, uid)) {
+ scheduleSync(account, UserHandle.getUserId(uid),
+ SyncOperation.REASON_ACCOUNTS_UPDATED,
+ null, null, AuthorityInfo.SYNCABLE_NO_ACCOUNT_ACCESS);
+ }
+ });
+
mBatteryStats = IBatteryStats.Stub.asInterface(ServiceManager.getService(
BatteryStats.SERVICE_NAME));
@@ -671,7 +680,7 @@
service.type.accountType, userHandle)) {
if (!canAccessAccount(account, packageName, userId)) {
mAccountManager.updateAppPermission(account,
- AccountManager.ACCOUNT_ACCESS_TOKEN, service.uid, true);
+ AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE, service.uid, true);
}
}
}
@@ -778,10 +787,11 @@
* @param extras a Map of SyncAdapter-specific information to control
* syncs of a specific provider. Can be null. Is ignored
* if the url is null.
- * @param onlyThoseWithUnkownSyncableState Only sync authorities that have unknown state.
+ * @param targetSyncState Only sync authorities that have the specified sync state.
+ * Use {@link AuthorityInfo#UNDEFINED} to sync all authorities.
*/
public void scheduleSync(Account requestedAccount, int userId, int reason,
- String requestedAuthority, Bundle extras, boolean onlyThoseWithUnkownSyncableState) {
+ String requestedAuthority, Bundle extras, int targetSyncState) {
final boolean isLoggable = Log.isLoggable(TAG, Log.VERBOSE);
if (extras == null) {
extras = new Bundle();
@@ -888,7 +898,7 @@
if (result != null
&& result.getBoolean(AccountManager.KEY_BOOLEAN_RESULT)) {
scheduleSync(account.account, userId, reason, authority,
- finalExtras, onlyThoseWithUnkownSyncableState);
+ finalExtras, targetSyncState);
}
}
));
@@ -903,9 +913,10 @@
isSyncable = AuthorityInfo.SYNCABLE;
}
- if (onlyThoseWithUnkownSyncableState && isSyncable >= 0) {
+ if (targetSyncState != AuthorityInfo.UNDEFINED && targetSyncState != isSyncable) {
continue;
}
+
if (!syncAdapterInfo.type.supportsUploading() && uploadOnly) {
continue;
}
@@ -931,7 +942,7 @@
final String owningPackage = syncAdapterInfo.componentName.getPackageName();
- if (isSyncable < 0) {
+ if (isSyncable == AuthorityInfo.NOT_INITIALIZED) {
// Initialisation sync.
Bundle newExtras = new Bundle();
newExtras.putBoolean(ContentResolver.SYNC_EXTRAS_INITIALIZE, true);
@@ -950,8 +961,8 @@
owningUid, owningPackage, reason, source,
authority, newExtras, allowParallelSyncs)
);
- }
- if (!onlyThoseWithUnkownSyncableState) {
+ } else if (targetSyncState == AuthorityInfo.UNDEFINED
+ || targetSyncState == isSyncable) {
if (isLoggable) {
Slog.v(TAG, "scheduleSync:"
+ " delay until " + delayUntil
@@ -1076,7 +1087,7 @@
final Bundle extras = new Bundle();
extras.putBoolean(ContentResolver.SYNC_EXTRAS_UPLOAD, true);
scheduleSync(account, userId, reason, authority, extras,
- false /* onlyThoseWithUnkownSyncableState */);
+ AuthorityInfo.UNDEFINED);
}
public SyncAdapterType[] getSyncAdapterTypes(int userId) {
@@ -1535,7 +1546,7 @@
mContext.getOpPackageName());
for (Account account : accounts) {
scheduleSync(account, userId, SyncOperation.REASON_USER_START, null, null,
- true /* onlyThoseWithUnknownSyncableState */);
+ AuthorityInfo.NOT_INITIALIZED);
}
}
@@ -2714,7 +2725,8 @@
if (syncTargets != null) {
scheduleSync(syncTargets.account, syncTargets.userId,
- SyncOperation.REASON_ACCOUNTS_UPDATED, syncTargets.provider, null, true);
+ SyncOperation.REASON_ACCOUNTS_UPDATED, syncTargets.provider,
+ null, AuthorityInfo.NOT_INITIALIZED);
}
}
diff --git a/services/core/java/com/android/server/content/SyncStorageEngine.java b/services/core/java/com/android/server/content/SyncStorageEngine.java
index 8289bae..069ae73 100644
--- a/services/core/java/com/android/server/content/SyncStorageEngine.java
+++ b/services/core/java/com/android/server/content/SyncStorageEngine.java
@@ -211,6 +211,12 @@
public static class AuthorityInfo {
// Legal values of getIsSyncable
+
+ /**
+ * The syncable state is undefined.
+ */
+ public static final int UNDEFINED = -2;
+
/**
* Default state for a newly installed adapter. An uninitialized adapter will receive an
* initialization sync which are governed by a different set of rules to that of regular