diff options
author | maxbogue <maxbogue@chromium.org> | 2015-11-18 08:26:58 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-11-18 16:28:05 +0000 |
commit | 774f31729ab739d01f63aa3f10b18140b8ea1df2 (patch) | |
tree | 421d5491f1bef42ea175328bfd069e2bb7fa36f3 | |
parent | 41f59ba2f0d7c3eceff177a15a17905969ef29b0 (diff) | |
download | chromium_src-774f31729ab739d01f63aa3f10b18140b8ea1df2.zip chromium_src-774f31729ab739d01f63aa3f10b18140b8ea1df2.tar.gz chromium_src-774f31729ab739d01f63aa3f10b18140b8ea1df2.tar.bz2 |
[Sync] Introduce GmsCoreSyncListener.
This change introduces a framework for sharing the user's custom
passphrase encryption key with GmsCore. This is to prevent users from
seeing the custom passphrase dialog pop up in GmsCore after they have
already entered it in Chrome.
BUG=552106
Review URL: https://codereview.chromium.org/1414203016
Cr-Commit-Position: refs/heads/master@{#360344}
16 files changed, 289 insertions, 10 deletions
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java index 4ae346a..92bcd1c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java @@ -84,6 +84,7 @@ import org.chromium.chrome.browser.services.GoogleServicesManager; import org.chromium.chrome.browser.share.ShareHelper; import org.chromium.chrome.browser.smartcard.EmptyPKCS11AuthenticationManager; import org.chromium.chrome.browser.smartcard.PKCS11AuthenticationManager; +import org.chromium.chrome.browser.sync.GmsCoreSyncListener; import org.chromium.chrome.browser.sync.SyncController; import org.chromium.chrome.browser.tab.AuthenticatorNavigationInterceptor; import org.chromium.chrome.browser.tab.Tab; @@ -789,6 +790,14 @@ public class ChromeApplication extends ContentApplication { } /** + * @return An instance of GmsCoreSyncListener to notify GmsCore of sync encryption key changes. + * Will be null if one is unavailable. + */ + public GmsCoreSyncListener createGmsCoreSyncListener() { + return null; + } + + /** * @return An instance of AppDetailsDelegate that can be queried about app information for the * App Banner feature. Will be null if one is unavailable. */ diff --git a/chrome/android/java/src/org/chromium/chrome/browser/sync/GmsCoreSyncListener.java b/chrome/android/java/src/org/chromium/chrome/browser/sync/GmsCoreSyncListener.java new file mode 100644 index 0000000..4d3642a --- /dev/null +++ b/chrome/android/java/src/org/chromium/chrome/browser/sync/GmsCoreSyncListener.java @@ -0,0 +1,42 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.sync; + +import org.chromium.base.VisibleForTesting; + +/** + * A listener that will share sync's custom passphrase encryption key with GmsCore. + * + * This is to prevent users from seeing the custom passphrase dialog pop up in GmsCore after they + * have already entered it in Chrome. + */ +public abstract class GmsCoreSyncListener implements ProfileSyncService.SyncStateChangedListener { + private boolean mGmsCoreInformed; + + /** + * Inform GMSCore of a new custom passphrase encryption key. + * + * @param key The serialized NigoriKey proto. + */ + public abstract void updateEncryptionKey(byte[] key); + + @Override + @VisibleForTesting + public void syncStateChanged() { + ProfileSyncService syncService = ProfileSyncService.get(); + boolean passphraseSet = syncService.isBackendInitialized() + && syncService.isUsingSecondaryPassphrase() && syncService.isCryptographerReady(); + if (passphraseSet && !mGmsCoreInformed) { + byte[] key = syncService.getCustomPassphraseKey(); + if (key.length > 0) { + updateEncryptionKey(key); + mGmsCoreInformed = true; + } + } else if (!passphraseSet && mGmsCoreInformed) { + // Prepare to inform GmsCore if a passphrase is set again. + mGmsCoreInformed = false; + } + } +} diff --git a/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java b/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java index 31f6d5b..3daecc5 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java @@ -195,6 +195,11 @@ public class ProfileSyncService { return nativeIsUsingSecondaryPassphrase(mNativeProfileSyncServiceAndroid); } + public byte[] getCustomPassphraseKey() { + assert isUsingSecondaryPassphrase(); + return nativeGetCustomPassphraseKey(mNativeProfileSyncServiceAndroid); + } + /** * Checks if we need a passphrase to decrypt a currently-enabled data type. This returns false * if a passphrase is needed for a type that is not currently enabled. @@ -496,6 +501,7 @@ public class ProfileSyncService { private native boolean nativeIsPassphraseRequiredForDecryption( long nativeProfileSyncServiceAndroid); private native boolean nativeIsUsingSecondaryPassphrase(long nativeProfileSyncServiceAndroid); + private native byte[] nativeGetCustomPassphraseKey(long nativeProfileSyncServiceAndroid); private native boolean nativeSetDecryptionPassphrase( long nativeProfileSyncServiceAndroid, String passphrase); private native void nativeSetEncryptionPassphrase( diff --git a/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java b/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java index 2e4ca57..ee25f4b 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java @@ -14,6 +14,7 @@ import org.chromium.base.ApplicationStatus.ActivityStateListener; import org.chromium.base.Log; import org.chromium.base.ThreadUtils; import org.chromium.base.VisibleForTesting; +import org.chromium.chrome.browser.ChromeApplication; import org.chromium.chrome.browser.identity.UniqueIdentificationGenerator; import org.chromium.chrome.browser.identity.UniqueIdentificationGeneratorFactory; import org.chromium.chrome.browser.invalidation.InvalidationController; @@ -90,6 +91,12 @@ public class SyncController implements ProfileSyncService.SyncStateChangedListen } } }); + + GmsCoreSyncListener gmsCoreSyncListener = + ((ChromeApplication) context.getApplicationContext()).createGmsCoreSyncListener(); + if (gmsCoreSyncListener != null) { + mProfileSyncService.addSyncStateChangedListener(gmsCoreSyncListener); + } } /** diff --git a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java index a2b1df3..18dd573 100644 --- a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java +++ b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java @@ -361,6 +361,19 @@ public class FakeServerHelper { }); } + /** + * Clear the server data (perform dashboard stop and clear). + */ + public void clearServerData() { + checkFakeServerInitialized("useFakeServer must be called before clearing data"); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + nativeClearServerData(mNativeFakeServerHelperAndroid, sNativeFakeServer); + } + }); + } + private static void checkFakeServerInitialized(String failureMessage) { if (sNativeFakeServer == 0L) { throw new IllegalStateException(failureMessage); @@ -399,4 +412,6 @@ public class FakeServerHelper { long nativeFakeServerHelperAndroid, long nativeFakeServer); private native void nativeDeleteEntity( long nativeFakeServerHelperAndroid, long nativeFakeServer, String id); + private native void nativeClearServerData( + long nativeFakeServerHelperAndroid, long nativeFakeServer); } diff --git a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/GmsCoreSyncListenerTest.java b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/GmsCoreSyncListenerTest.java new file mode 100644 index 0000000..8713add --- /dev/null +++ b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/GmsCoreSyncListenerTest.java @@ -0,0 +1,129 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.chrome.browser.sync; + +import android.accounts.Account; +import android.test.suitebuilder.annotation.MediumTest; + +import org.chromium.base.ThreadUtils; +import org.chromium.base.test.util.Feature; +import org.chromium.chrome.test.util.browser.sync.SyncTestUtil; +import org.chromium.content.browser.test.util.Criteria; +import org.chromium.content.browser.test.util.CriteriaHelper; + +/** + * Test suite for the GmsCoreSyncListener. + */ +public class GmsCoreSyncListenerTest extends SyncTestBase { + private static final String PASSPHRASE = "passphrase"; + + static class CountingGmsCoreSyncListener extends GmsCoreSyncListener { + private int mCallCount = 0; + + @Override + public void updateEncryptionKey(byte[] key) { + mCallCount++; + } + + public int callCount() { + return mCallCount; + } + } + + private CountingGmsCoreSyncListener mListener; + + @Override + protected void setUp() throws Exception { + super.setUp(); + mListener = new CountingGmsCoreSyncListener(); + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + ProfileSyncService.get().addSyncStateChangedListener(mListener); + } + }); + } + + @Override + protected void tearDown() throws Exception { + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + ProfileSyncService.get().removeSyncStateChangedListener(mListener); + } + }); + super.tearDown(); + } + + @MediumTest + @Feature({"Sync"}) + public void testGetsKey() throws Throwable { + Account account = setUpTestAccountAndSignInToSync(); + assertEquals(0, mListener.callCount()); + encryptWithPassphrase(PASSPHRASE); + waitForCallCount(1); + signOut(); + signIn(account); + assertEquals(1, mListener.callCount()); + decryptWithPassphrase(PASSPHRASE); + waitForCallCount(2); + } + + @MediumTest + @Feature({"Sync"}) + public void testClearData() throws Throwable { + setUpTestAccountAndSignInToSync(); + assertEquals(0, mListener.callCount()); + encryptWithPassphrase(PASSPHRASE); + waitForCallCount(1); + clearServerData(); + startSync(); + encryptWithPassphrase(PASSPHRASE); + waitForCallCount(2); + } + + private void encryptWithPassphrase(final String passphrase) throws InterruptedException { + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + ProfileSyncService.get().setEncryptionPassphrase(passphrase); + } + }); + waitForCryptographer(); + // Make sure the new encryption settings make it to the server. + SyncTestUtil.triggerSyncAndWaitForCompletion(mContext); + } + + private void decryptWithPassphrase(final String passphrase) throws InterruptedException { + ThreadUtils.runOnUiThreadBlocking(new Runnable() { + @Override + public void run() { + ProfileSyncService.get().setDecryptionPassphrase(passphrase); + } + }); + } + + private void waitForCryptographer() throws InterruptedException { + boolean isReady = CriteriaHelper.pollForUIThreadCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + ProfileSyncService syncService = ProfileSyncService.get(); + return syncService.isUsingSecondaryPassphrase() + && syncService.isCryptographerReady(); + } + }); + assertTrue("Timed out waiting for cryptographer to be ready.", isReady); + } + + private void waitForCallCount(final int count) throws InterruptedException { + CriteriaHelper.pollForUIThreadCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + return mListener.callCount() == count; + } + }); + assertEquals(count, mListener.callCount()); + } +} diff --git a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTestBase.java b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTestBase.java index 8fe66d8..4360d4e 100644 --- a/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTestBase.java +++ b/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/SyncTestBase.java @@ -16,6 +16,8 @@ import org.chromium.chrome.browser.signin.SigninManager; import org.chromium.chrome.test.ChromeActivityTestCaseBase; import org.chromium.chrome.test.util.browser.signin.SigninTestUtil; import org.chromium.chrome.test.util.browser.sync.SyncTestUtil; +import org.chromium.content.browser.test.util.Criteria; +import org.chromium.content.browser.test.util.CriteriaHelper; import org.chromium.sync.AndroidSyncSettings; import org.chromium.sync.ModelType; import org.chromium.sync.test.util.MockSyncContentResolverDelegate; @@ -23,6 +25,8 @@ import org.chromium.sync.test.util.MockSyncContentResolverDelegate; import java.util.Arrays; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; /** * Base class for common functionality between sync tests. @@ -125,7 +129,6 @@ public class SyncTestBase extends ChromeActivityTestCaseBase<ChromeActivity> { protected Account setUpTestAccountAndSignInToSync() throws InterruptedException { Account account = setUpTestAccount(); signIn(account); - SyncTestUtil.verifySyncIsActiveForAccount(mContext, account); assertTrue("Sync everything should be enabled", SyncTestUtil.isSyncEverythingEnabled(mContext)); return account; @@ -151,22 +154,43 @@ public class SyncTestBase extends ChromeActivityTestCaseBase<ChromeActivity> { getInstrumentation().waitForIdleSync(); } - protected void signIn(final Account account) { + protected void signIn(final Account account) throws InterruptedException { ThreadUtils.runOnUiThreadBlocking(new Runnable() { @Override public void run() { mSyncController.signIn(getActivity(), account.name); } }); + SyncTestUtil.verifySyncIsActiveForAccount(mContext, account); } protected void signOut() throws InterruptedException { + final Semaphore s = new Semaphore(0); ThreadUtils.runOnUiThreadBlocking(new Runnable() { @Override public void run() { - SigninManager.get(mContext).signOut(getActivity(), null); + SigninManager.get(mContext).signOut(getActivity(), new Runnable() { + @Override + public void run() { + s.release(); + } + }); } }); + assertTrue(s.tryAcquire(SyncTestUtil.UI_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + SyncTestUtil.verifySyncIsSignedOut(mContext); + } + + protected void clearServerData() throws InterruptedException { + mFakeServerHelper.clearServerData(); + SyncTestUtil.triggerSync(); + boolean syncStopped = CriteriaHelper.pollForUIThreadCriteria(new Criteria() { + @Override + public boolean isSatisfied() { + return !ProfileSyncService.get().isSyncRequested(); + } + }, SyncTestUtil.UI_TIMEOUT_MS, SyncTestUtil.CHECK_INTERVAL_MS); + assertTrue("Timed out waiting for sync to stop.", syncStopped); } protected void disableDataType(final int modelType) { diff --git a/chrome/browser/sync/profile_sync_service_android.cc b/chrome/browser/sync/profile_sync_service_android.cc index b4c8083..e0312c6 100644 --- a/chrome/browser/sync/profile_sync_service_android.cc +++ b/chrome/browser/sync/profile_sync_service_android.cc @@ -241,6 +241,13 @@ jboolean ProfileSyncServiceAndroid::IsUsingSecondaryPassphrase( return sync_service_->IsUsingSecondaryPassphrase(); } +ScopedJavaLocalRef<jbyteArray> +ProfileSyncServiceAndroid::GetCustomPassphraseKey(JNIEnv* env, jobject obj) { + std::string key = sync_service_->GetCustomPassphraseKey(); + return base::android::ToJavaByteArray( + env, reinterpret_cast<const uint8_t*>(key.data()), key.size()); +} + jint ProfileSyncServiceAndroid::GetPassphraseType(JNIEnv* env, jobject) { DCHECK_CURRENTLY_ON(BrowserThread::UI); return sync_service_->GetPassphraseType(); diff --git a/chrome/browser/sync/profile_sync_service_android.h b/chrome/browser/sync/profile_sync_service_android.h index c035a78..a3d067c 100644 --- a/chrome/browser/sync/profile_sync_service_android.h +++ b/chrome/browser/sync/profile_sync_service_android.h @@ -64,6 +64,9 @@ class ProfileSyncServiceAndroid : public sync_driver::SyncServiceObserver { jboolean IsPassphraseRequired(JNIEnv* env, jobject obj); jboolean IsPassphraseRequiredForDecryption(JNIEnv* env, jobject obj); jboolean IsUsingSecondaryPassphrase(JNIEnv* env, jobject obj); + base::android::ScopedJavaLocalRef<jbyteArray> GetCustomPassphraseKey( + JNIEnv* env, + jobject obj); jint GetPassphraseType(JNIEnv* env, jobject obj); void SetEncryptionPassphrase(JNIEnv* env, jobject obj, diff --git a/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/sync/SyncTestUtil.java b/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/sync/SyncTestUtil.java index c50ee4a..9b48687 100644 --- a/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/sync/SyncTestUtil.java +++ b/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/sync/SyncTestUtil.java @@ -186,12 +186,9 @@ public final class SyncTestUtil { } /** - * Triggers a sync and waits till it is complete. + * Triggers a sync cycle. */ - public static void triggerSyncAndWaitForCompletion(final Context context) - throws InterruptedException { - final long oldSyncTime = getCurrentSyncTime(context); - // Request sync. + public static void triggerSync() throws InterruptedException { ThreadUtils.runOnUiThreadBlocking(new Runnable() { @Override public void run() { @@ -199,6 +196,16 @@ public final class SyncTestUtil { .requestSyncFromNativeChromeForAllTypes(); } }); + } + + /** + * Triggers a sync and waits till it is complete. + */ + public static void triggerSyncAndWaitForCompletion(final Context context) + throws InterruptedException { + final long oldSyncTime = getCurrentSyncTime(context); + + triggerSync(); // Wait till lastSyncedTime > oldSyncTime. Assert.assertTrue("Timed out waiting for syncing to complete.", diff --git a/components/browser_sync/browser/profile_sync_service.cc b/components/browser_sync/browser/profile_sync_service.cc index dd6636b..4ace523 100644 --- a/components/browser_sync/browser/profile_sync_service.cc +++ b/components/browser_sync/browser/profile_sync_service.cc @@ -1863,6 +1863,13 @@ bool ProfileSyncService::IsUsingSecondaryPassphrase() const { passphrase_type == syncer::CUSTOM_PASSPHRASE; } +std::string ProfileSyncService::GetCustomPassphraseKey() const { + sync_driver::SystemEncryptor encryptor; + syncer::Cryptographer cryptographer(&encryptor); + cryptographer.Bootstrap(sync_prefs_.GetEncryptionBootstrapToken()); + return cryptographer.GetDefaultNigoriKeyData(); +} + syncer::PassphraseType ProfileSyncService::GetPassphraseType() const { return backend_->GetPassphraseType(); } diff --git a/components/browser_sync/browser/profile_sync_service.h b/components/browser_sync/browser/profile_sync_service.h index 32c998e..6f6a9345 100644 --- a/components/browser_sync/browser/profile_sync_service.h +++ b/components/browser_sync/browser/profile_sync_service.h @@ -565,6 +565,10 @@ class ProfileSyncService : public sync_driver::SyncService, // killed in the near future. void FlushDirectory() const; + // Returns a serialized NigoriKey proto generated from the bootstrap token in + // SyncPrefs. Will return the empty string if no bootstrap token exists. + std::string GetCustomPassphraseKey() const; + // Needed to test whether the directory is deleted properly. base::FilePath GetDirectoryPathForTest() const; diff --git a/sync/test/fake_server/android/fake_server_helper_android.cc b/sync/test/fake_server/android/fake_server_helper_android.cc index 4254295..ed5d639 100644 --- a/sync/test/fake_server/android/fake_server_helper_android.cc +++ b/sync/test/fake_server/android/fake_server_helper_android.cc @@ -294,6 +294,14 @@ void FakeServerHelperAndroid::DeleteEntity( fake_server::TombstoneEntity::Create(native_id)); } +void FakeServerHelperAndroid::ClearServerData(JNIEnv* env, + jobject obj, + jlong fake_server) { + fake_server::FakeServer* fake_server_ptr = + reinterpret_cast<fake_server::FakeServer*>(fake_server); + fake_server_ptr->ClearServerData(); +} + // static bool FakeServerHelperAndroid::Register(JNIEnv* env) { return RegisterNativesImpl(env); diff --git a/sync/test/fake_server/android/fake_server_helper_android.h b/sync/test/fake_server/android/fake_server_helper_android.h index a0418ed..fb95af1 100644 --- a/sync/test/fake_server/android/fake_server_helper_android.h +++ b/sync/test/fake_server/android/fake_server_helper_android.h @@ -114,6 +114,9 @@ class FakeServerHelperAndroid { jlong fake_server, jstring id); + // Simulates a dashboard stop and clear. + void ClearServerData(JNIEnv* env, jobject obj, jlong fake_server); + private: virtual ~FakeServerHelperAndroid(); diff --git a/sync/test/fake_server/fake_server.cc b/sync/test/fake_server/fake_server.cc index 7849cd8..1b2f16d 100644 --- a/sync/test/fake_server/fake_server.cc +++ b/sync/test/fake_server/fake_server.cc @@ -186,14 +186,18 @@ FakeServer::FakeServer() : version_(0), network_enabled_(true), enable_implicit_permanent_folder_creation_(false), weak_ptr_factory_(this) { + Init(); +} + +FakeServer::~FakeServer() {} + +void FakeServer::Init() { keystore_keys_.push_back(kDefaultKeystoreKey); const bool create_result = CreateDefaultPermanentItems(); DCHECK(create_result) << "Permanent items were not created successfully."; } -FakeServer::~FakeServer() {} - bool FakeServer::CreatePermanentBookmarkFolder(const std::string& server_tag, const std::string& name) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -619,6 +623,7 @@ void FakeServer::ClearServerData() { entities_.clear(); keystore_keys_.clear(); ++store_birthday_; + Init(); } void FakeServer::SetAuthenticated() { diff --git a/sync/test/fake_server/fake_server.h b/sync/test/fake_server/fake_server.h index 7bcd935..7a55fbe 100644 --- a/sync/test/fake_server/fake_server.h +++ b/sync/test/fake_server/fake_server.h @@ -153,6 +153,9 @@ class FakeServer { typedef base::ScopedPtrMap<std::string, scoped_ptr<FakeServerEntity>> EntityMap; + // Gets FakeServer ready for syncing. + void Init(); + // Processes a GetUpdates call. bool HandleGetUpdatesRequest(const sync_pb::GetUpdatesMessage& get_updates, sync_pb::GetUpdatesResponse* response); |