[DO NOT MERGE] Use tokens instead of account access trackers

We keep track which process saw and account to whitelist
the app for future access as an optimization to avoid
prompting the user for account access approval. Some apps
use SefeParcelable where the parcels are marshalled
which does not allow the parcel to contain IBinders.
To avoid this we are switching from account tracker remote
objects to unforgeable tokens.

bug:31162498

Change-Id: I3b52bff720655f695ad0c58d420eb35ef93161b9

Android.mk

@@ -63,7 +63,6 @@ LOCAL_SRC_FILES += \
 	core/java/android/accessibilityservice/IAccessibilityServiceClient.aidl \
 	core/java/android/accounts/IAccountManager.aidl \
 	core/java/android/accounts/IAccountManagerResponse.aidl \
-	core/java/android/accounts/IAccountAccessTracker.aidl \
 	core/java/android/accounts/IAccountAuthenticator.aidl \
 	core/java/android/accounts/IAccountAuthenticatorResponse.aidl \
 	core/java/android/app/IActivityContainer.aidl \

core/java/android/accounts/Account.java

@@ -18,9 +18,11 @@ package android.accounts;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
+import android.content.Context;
 import android.os.Parcelable;
 import android.os.Parcel;
 import android.os.RemoteException;
+import android.os.ServiceManager;
 import android.text.TextUtils;
 import android.util.ArraySet;
 import android.util.Log;
@@ -41,7 +43,7 @@ public class Account implements Parcelable {
 
     public final String name;
     public final String type;
-    private final @Nullable IAccountAccessTracker mAccessTracker;
+    private final @Nullable String accessId;
 
     public boolean equals(Object o) {
         if (o == this) return true;
@@ -64,14 +66,14 @@ public class Account implements Parcelable {
     /**
      * @hide
      */
-    public Account(@NonNull Account other, @Nullable IAccountAccessTracker accessTracker) {
+    public Account(@NonNull Account other, @NonNull String accessId) {
-        this(other.name, other.type, accessTracker);
+        this(other.name, other.type, accessId);
     }
 
     /**
      * @hide
      */
-    public Account(String name, String type, IAccountAccessTracker accessTracker) {
+    public Account(String name, String type, String accessId) {
         if (TextUtils.isEmpty(name)) {
             throw new IllegalArgumentException("the name must not be empty: " + name);
         }
@@ -80,18 +82,20 @@ public class Account implements Parcelable {
         }
         this.name = name;
         this.type = type;
-        this.mAccessTracker = accessTracker;
+        this.accessId = accessId;
     }
 
     public Account(Parcel in) {
         this.name = in.readString();
         this.type = in.readString();
-        this.mAccessTracker = IAccountAccessTracker.Stub.asInterface(in.readStrongBinder());
+        this.accessId = in.readString();
-        if (mAccessTracker != null) {
+        if (accessId != null) {
             synchronized (sAccessedAccounts) {
                 if (sAccessedAccounts.add(this)) {
                     try {
-                        mAccessTracker.onAccountAccessed();
+                        IAccountManager accountManager = IAccountManager.Stub.asInterface(
+                                ServiceManager.getService(Context.ACCOUNT_SERVICE));
+                        accountManager.onAccountAccessed(accessId);
                     } catch (RemoteException e) {
                         Log.e(TAG, "Error noting account access", e);
                     }
@@ -101,8 +105,8 @@ public class Account implements Parcelable {
     }
 
     /** @hide */
-    public IAccountAccessTracker getAccessTracker() {
+    public String getAccessId() {
-        return mAccessTracker;
+        return accessId;
     }
 
     public int describeContents() {
@@ -112,7 +116,7 @@ public class Account implements Parcelable {
     public void writeToParcel(Parcel dest, int flags) {
         dest.writeString(name);
         dest.writeString(type);
-        dest.writeStrongInterface(mAccessTracker);
+        dest.writeString(accessId);
     }
 
     public static final Creator<Account> CREATOR = new Creator<Account>() {

core/java/android/accounts/AccountManager.java

@@ -179,12 +179,12 @@ public class AccountManager {
     public static final String KEY_ACCOUNT_TYPE = "accountType";
 
     /**
-     * Bundle key used for the {@link IAccountAccessTracker} account access tracker
+     * Bundle key used for the account access id used for noting the
-     * used for noting the account was accessed when unmarshalled from a parcel.
+     * account was accessed when unmarshalled from a parcel.
      *
      * @hide
      */
-    public static final String KEY_ACCOUNT_ACCESS_TRACKER = "accountAccessTracker";
+    public static final String KEY_ACCOUNT_ACCESS_ID = "accountAccessId";
 
     /**
      * Bundle key used for the auth token value in results
@@ -821,9 +821,8 @@ public class AccountManager {
             public Account bundleToResult(Bundle bundle) throws AuthenticatorException {
                 String name = bundle.getString(KEY_ACCOUNT_NAME);
                 String type = bundle.getString(KEY_ACCOUNT_TYPE);
-                IAccountAccessTracker tracker = IAccountAccessTracker.Stub.asInterface(
+                String accessId = bundle.getString(KEY_ACCOUNT_ACCESS_ID);
-                        bundle.getBinder(KEY_ACCOUNT_ACCESS_TRACKER));
+                return new Account(name, type, accessId);
-                return new Account(name, type, tracker);
             }
         }.start();
     }
@@ -2279,7 +2278,7 @@ public class AccountManager {
                                     result.putString(KEY_ACCOUNT_NAME, null);
                                     result.putString(KEY_ACCOUNT_TYPE, null);
                                     result.putString(KEY_AUTHTOKEN, null);
-                                    result.putBinder(KEY_ACCOUNT_ACCESS_TRACKER, null);
+                                    result.putBinder(KEY_ACCOUNT_ACCESS_ID, null);
                                     try {
                                         mResponse.onResult(result);
                                     } catch (RemoteException e) {
@@ -2306,9 +2305,7 @@ public class AccountManager {
                                             Account account = new Account(
                                                     value.getString(KEY_ACCOUNT_NAME),
                                                     value.getString(KEY_ACCOUNT_TYPE),
-                                                    IAccountAccessTracker.Stub.asInterface(
+                                                    value.getString(KEY_ACCOUNT_ACCESS_ID));
-                                                            value.getBinder(
-                                                                    KEY_ACCOUNT_ACCESS_TRACKER)));
                                             mFuture = getAuthToken(account, mAuthTokenType,
                                                     mLoginOptions,  mActivity, mMyCallback,
                                                     mHandler);
@@ -2358,9 +2355,8 @@ public class AccountManager {
                         setException(new AuthenticatorException("account not in result"));
                         return;
                     }
-                    final IAccountAccessTracker tracker = IAccountAccessTracker.Stub.asInterface(
+                    final String accessId = result.getString(KEY_ACCOUNT_ACCESS_ID);
-                            result.getBinder(KEY_ACCOUNT_ACCESS_TRACKER));
+                    final Account account = new Account(accountName, accountType, accessId);
-                    final Account account = new Account(accountName, accountType, tracker);
                     mNumAccounts = 1;
                     getAuthToken(account, mAuthTokenType, null /* options */, mActivity,
                             mMyCallback, mHandler);

core/java/android/accounts/IAccountAccessTracker.aidl

@@ -1,26 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package android.accounts;
-
-/**
- * Interface to track which apps accessed an account
- *
- * @hide
- */
-oneway interface IAccountAccessTracker {
-    void onAccountAccessed();
-}

core/java/android/accounts/IAccountManager.aidl

@@ -110,4 +110,6 @@ interface IAccountManager {
     /* Crate an intent to request account access for package and a given user id */
     IntentSender createRequestAccountAccessIntentSenderAsUser(in Account account,
         String packageName, in UserHandle userHandle);
+
+    void onAccountAccessed(String token);
 }

services/core/java/com/android/server/accounts/AccountManagerService.java

@@ -26,7 +26,6 @@ import android.accounts.AccountManagerInternal;
 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;
@@ -89,7 +88,6 @@ import android.os.UserManager;
 import android.os.storage.StorageManager;
 import android.text.TextUtils;
 import android.util.Log;
-import android.util.PackageUtils;
 import android.util.Pair;
 import android.util.Slog;
 import android.util.SparseArray;
@@ -128,6 +126,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
+import java.util.UUID;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -692,7 +692,7 @@ public class AccountManagerService
                     final Account[] accountsForType = new Account[accountNames.size()];
                     for (int i = 0; i < accountsForType.length; i++) {
                         accountsForType[i] = new Account(accountNames.get(i), accountType,
-                                new AccountAccessTracker());
+                                UUID.randomUUID().toString());
                     }
                     accounts.accountCache.put(accountType, accountsForType);
                 }
@@ -1502,8 +1502,8 @@ public class AccountManagerService
             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,
+            result.putString(AccountManager.KEY_ACCOUNT_ACCESS_ID,
-                    resultingAccount.getAccessTracker().asBinder());
+                    resultingAccount.getAccessId());
             try {
                 response.onResult(result);
             } catch (RemoteException e) {
@@ -4079,6 +4079,30 @@ public class AccountManagerService
         }
     }
 
+    @Override
+    public void onAccountAccessed(String token) 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())) {
+                if (Objects.equals(account.getAccessId(), token)) {
+                    // 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 abstract class Session extends IAccountAuthenticatorResponse.Stub
             implements IBinder.DeathRecipient, ServiceConnection {
         IAccountManagerResponse mResponse;
@@ -5714,9 +5738,9 @@ public class AccountManagerService
         if (accountsForType != null) {
             System.arraycopy(accountsForType, 0, newAccountsForType, 0, oldLength);
         }
-        IAccountAccessTracker accessTracker = account.getAccessTracker() != null
+        String token = account.getAccessId() != null ? account.getAccessId()
-                ? account.getAccessTracker() : new AccountAccessTracker();
+                : UUID.randomUUID().toString();
-        newAccountsForType[oldLength] = new Account(account, accessTracker);
+        newAccountsForType[oldLength] = new Account(account, token);
         accounts.accountCache.put(account.type, newAccountsForType);
         return newAccountsForType[oldLength];
     }
@@ -5961,33 +5985,6 @@ public class AccountManagerService
         }
     }
 
-    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 {
         private final Object mLock = new Object();