Revert "Sanitize Bundle from AbstractAccountAuthenticator." This reverts commit 09d381088952db904dbd0cc1c1c7362c033cf216. Reason for revert: b/342661425: fix filters obfuscated_gaia_id extra used for Cross profile account setup. A new version will be created. (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:2dc861d4ae98f4e18c82d3ad8f3b478dd2ddb7bb) Merged-In: I4da2ad09473f0155ebf11a45a48562b59ee5de37 Change-Id: I4da2ad09473f0155ebf11a45a48562b59ee5de37
diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index 88edb12..3499a3a 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -1773,8 +1773,7 @@ // Create a Session for the target user and pass in the bundle completeCloningAccount(response, result, account, toAccounts, userFrom); } else { - // Bundle format is not defined. - super.onResultSkipSanitization(result); + super.onResult(result); } } }.bind(); @@ -1861,8 +1860,7 @@ // account to avoid retries? // TODO: what we do with the visibility? - // Bundle format is not defined. - super.onResultSkipSanitization(result); + super.onResult(result); } @Override @@ -2108,7 +2106,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); IAccountManagerResponse response = getResponseAndClose(); if (response != null) { try { @@ -2462,7 +2459,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); if (result != null && result.containsKey(AccountManager.KEY_BOOLEAN_RESULT) && !result.containsKey(AccountManager.KEY_INTENT)) { final boolean removalAllowed = result.getBoolean(AccountManager.KEY_BOOLEAN_RESULT); @@ -2977,7 +2973,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); if (result != null) { String label = result.getString(AccountManager.KEY_AUTH_TOKEN_LABEL); Bundle bundle = new Bundle(); @@ -3155,7 +3150,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); if (result != null) { if (result.containsKey(AccountManager.KEY_AUTH_TOKEN_LABEL)) { Intent intent = newGrantCredentialsPermissionIntent( @@ -3627,12 +3621,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - Bundle sessionBundle = null; - if (result != null) { - // Session bundle will be removed from result. - sessionBundle = result.getBundle(AccountManager.KEY_ACCOUNT_SESSION_BUNDLE); - } - result = sanitizeBundle(result); mNumResults++; Intent intent = null; if (result != null) { @@ -3694,6 +3682,7 @@ // bundle contains data necessary for finishing the session // later. The session bundle will be encrypted here and // decrypted later when trying to finish the session. + Bundle sessionBundle = result.getBundle(AccountManager.KEY_ACCOUNT_SESSION_BUNDLE); if (sessionBundle != null) { String accountType = sessionBundle.getString(AccountManager.KEY_ACCOUNT_TYPE); if (TextUtils.isEmpty(accountType) @@ -4081,7 +4070,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); IAccountManagerResponse response = getResponseAndClose(); if (response == null) { return; @@ -4395,7 +4383,6 @@ @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); mNumResults++; if (result == null) { onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, "null bundle"); @@ -4952,68 +4939,6 @@ callback, resultReceiver); } - - // All keys for Strings passed from AbstractAccountAuthenticator using Bundle. - private static final String[] sStringBundleKeys = new String[] { - AccountManager.KEY_ACCOUNT_NAME, - AccountManager.KEY_ACCOUNT_TYPE, - AccountManager.KEY_AUTHTOKEN, - AccountManager.KEY_AUTH_TOKEN_LABEL, - AccountManager.KEY_ERROR_MESSAGE, - AccountManager.KEY_PASSWORD, - AccountManager.KEY_ACCOUNT_STATUS_TOKEN}; - - /** - * Keep only documented fields in a Bundle received from AbstractAccountAuthenticator. - */ - protected static Bundle sanitizeBundle(Bundle bundle) { - if (bundle == null) { - return null; - } - Bundle sanitizedBundle = new Bundle(); - Bundle.setDefusable(sanitizedBundle, true); - int updatedKeysCount = 0; - for (String stringKey : sStringBundleKeys) { - if (bundle.containsKey(stringKey)) { - String value = bundle.getString(stringKey); - sanitizedBundle.putString(stringKey, value); - updatedKeysCount++; - } - } - String key = AbstractAccountAuthenticator.KEY_CUSTOM_TOKEN_EXPIRY; - if (bundle.containsKey(key)) { - long expiryMillis = bundle.getLong(key, 0L); - sanitizedBundle.putLong(key, expiryMillis); - updatedKeysCount++; - } - key = AccountManager.KEY_BOOLEAN_RESULT; - if (bundle.containsKey(key)) { - boolean booleanResult = bundle.getBoolean(key, false); - sanitizedBundle.putBoolean(key, booleanResult); - updatedKeysCount++; - } - key = AccountManager.KEY_ERROR_CODE; - if (bundle.containsKey(key)) { - int errorCode = bundle.getInt(key, 0); - sanitizedBundle.putInt(key, errorCode); - updatedKeysCount++; - } - key = AccountManager.KEY_INTENT; - if (bundle.containsKey(key)) { - Intent intent = bundle.getParcelable(key, Intent.class); - sanitizedBundle.putParcelable(key, intent); - updatedKeysCount++; - } - if (bundle.containsKey(AccountManager.KEY_ACCOUNT_SESSION_BUNDLE)) { - // The field is not copied in sanitized bundle. - updatedKeysCount++; - } - if (updatedKeysCount != bundle.size()) { - Log.w(TAG, "Size mismatch after sanitizeBundle call."); - } - return sanitizedBundle; - } - private abstract class Session extends IAccountAuthenticatorResponse.Stub implements IBinder.DeathRecipient, ServiceConnection { private final Object mSessionLock = new Object(); @@ -5304,15 +5229,10 @@ } } } + @Override public void onResult(Bundle result) { Bundle.setDefusable(result, true); - result = sanitizeBundle(result); - onResultSkipSanitization(result); - } - - public void onResultSkipSanitization(Bundle result) { - Bundle.setDefusable(result, true); mNumResults++; Intent intent = null; if (result != null) {
diff --git a/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java index b9ce8ad..0c92abc 100644 --- a/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/accounts/AccountManagerServiceTest.java
@@ -1163,6 +1163,16 @@ verify(mMockAccountManagerResponse).onResult(mBundleCaptor.capture()); Bundle result = mBundleCaptor.getValue(); + Bundle sessionBundle = result.getBundle(AccountManager.KEY_ACCOUNT_SESSION_BUNDLE); + assertNotNull(sessionBundle); + // Assert that session bundle is decrypted and hence data is visible. + assertEquals(AccountManagerServiceTestFixtures.SESSION_DATA_VALUE_1, + sessionBundle.getString(AccountManagerServiceTestFixtures.SESSION_DATA_NAME_1)); + // Assert finishSessionAsUser added calling uid and pid into the sessionBundle + assertTrue(sessionBundle.containsKey(AccountManager.KEY_CALLER_UID)); + assertTrue(sessionBundle.containsKey(AccountManager.KEY_CALLER_PID)); + assertEquals(sessionBundle.getString( + AccountManager.KEY_ANDROID_PACKAGE_NAME), "APCT.package"); // Verify response data assertNull(result.getString(AccountManager.KEY_AUTHTOKEN, null)); @@ -2111,6 +2121,12 @@ result.getString(AccountManager.KEY_ACCOUNT_NAME)); assertEquals(AccountManagerServiceTestFixtures.ACCOUNT_TYPE_1, result.getString(AccountManager.KEY_ACCOUNT_TYPE)); + + Bundle optionBundle = result.getParcelable( + AccountManagerServiceTestFixtures.KEY_OPTIONS_BUNDLE); + // Assert addAccountAsUser added calling uid and pid into the option bundle + assertTrue(optionBundle.containsKey(AccountManager.KEY_CALLER_UID)); + assertTrue(optionBundle.containsKey(AccountManager.KEY_CALLER_PID)); } @SmallTest @@ -3441,52 +3457,6 @@ + (readTotalTime.doubleValue() / readerCount / loopSize)); } - @SmallTest - public void testSanitizeBundle_expectedFields() throws Exception { - Bundle bundle = new Bundle(); - bundle.putString(AccountManager.KEY_ACCOUNT_NAME, "name"); - bundle.putString(AccountManager.KEY_ACCOUNT_TYPE, "type"); - bundle.putString(AccountManager.KEY_AUTHTOKEN, "token"); - bundle.putString(AccountManager.KEY_AUTH_TOKEN_LABEL, "label"); - bundle.putString(AccountManager.KEY_ERROR_MESSAGE, "error message"); - bundle.putString(AccountManager.KEY_PASSWORD, "password"); - bundle.putString(AccountManager.KEY_ACCOUNT_STATUS_TOKEN, "status"); - - bundle.putLong(AbstractAccountAuthenticator.KEY_CUSTOM_TOKEN_EXPIRY, 123L); - bundle.putBoolean(AccountManager.KEY_BOOLEAN_RESULT, true); - bundle.putInt(AccountManager.KEY_ERROR_CODE, 456); - - Bundle sanitizedBundle = AccountManagerService.sanitizeBundle(bundle); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_ACCOUNT_NAME), "name"); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_ACCOUNT_TYPE), "type"); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_AUTHTOKEN), "token"); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_AUTH_TOKEN_LABEL), "label"); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_ERROR_MESSAGE), "error message"); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_PASSWORD), "password"); - assertEquals(sanitizedBundle.getString(AccountManager.KEY_ACCOUNT_STATUS_TOKEN), "status"); - - assertEquals(sanitizedBundle.getLong( - AbstractAccountAuthenticator.KEY_CUSTOM_TOKEN_EXPIRY, 0), 123L); - assertEquals(sanitizedBundle.getBoolean(AccountManager.KEY_BOOLEAN_RESULT, false), true); - assertEquals(sanitizedBundle.getInt(AccountManager.KEY_ERROR_CODE, 0), 456); - } - - @SmallTest - public void testSanitizeBundle_filtersUnexpectedFields() throws Exception { - Bundle bundle = new Bundle(); - bundle.putString(AccountManager.KEY_ACCOUNT_NAME, "name"); - bundle.putString("unknown_key", "value"); - Bundle sessionBundle = new Bundle(); - bundle.putBundle(AccountManager.KEY_ACCOUNT_SESSION_BUNDLE, sessionBundle); - - Bundle sanitizedBundle = AccountManagerService.sanitizeBundle(bundle); - - assertEquals(sanitizedBundle.getString(AccountManager.KEY_ACCOUNT_NAME), "name"); - assertFalse(sanitizedBundle.containsKey("unknown_key")); - // It is a valid response from Authenticator which will be accessed using original Bundle - assertFalse(sanitizedBundle.containsKey(AccountManager.KEY_ACCOUNT_SESSION_BUNDLE)); - } - private void waitForCyclicBarrier(CyclicBarrier cyclicBarrier) { try { cyclicBarrier.await(LATCH_TIMEOUT_MS, TimeUnit.MILLISECONDS);