summaryrefslogtreecommitdiffstats
path: root/sync/engine
diff options
context:
space:
mode:
authorrsimha@chromium.org <rsimha@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-20 02:54:56 +0000
committerrsimha@chromium.org <rsimha@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-20 02:54:56 +0000
commit25cf2a0e92f4de4dd8a36c4dff25cba032fdbab4 (patch)
treef5edacd5357f2e8ea7d2a0bcef8924347aa0caaa /sync/engine
parent91e20036b8f64ca6fdf5f5c039f7e44f5aa39cbf (diff)
downloadchromium_src-25cf2a0e92f4de4dd8a36c4dff25cba032fdbab4.zip
chromium_src-25cf2a0e92f4de4dd8a36c4dff25cba032fdbab4.tar.gz
chromium_src-25cf2a0e92f4de4dd8a36c4dff25cba032fdbab4.tar.bz2
Separate SetPassphrase into encryption and decryption APIs
As of today, there is just one SetPassphrase API in ProfileSyncService, SyncBackendHost, SyncManager and SyncInternal, and it is used as a general purpose API for setting a passphrase during encryption and decryption. This is bad because the UI is aware of its intent when SetPassphrase is called, and yet, we discard that intent and recompute it via internal state deep inside SyncInternal. Ideally, we should allow the UI to declare its intent by specifying whether it is trying to encrypt or decrypt by calling the appropriate API. The end goal of this change is to make the UI more responsive to the user when a passphrase is entered for decryption, because we can now test the entered passphrase against the UI layer's local cache of the cryptographer's keys, and give the user immediate feedback without having to display a spinner while the call is sent down to the syncer thread. This patch does a bunch of things: - Separates the SetPassphrase API into SetEncryptionPassphrase and SetDecryptionPassphrase. - Plumbs these two APIs all the way up the stack, from SyncInternal, SyncManager, SyncbackendHost and ProfileSyncService. - Adds a synchronous check in SetDecryptionPassphrase to see if the user-entered passphrase works on the cached pending keys, and thereby eliminates the need to show a spinner in the UI while decrypting. - Updates calls from the UI layer and from unit and integration tests to the correct variant of the API. - Updates the code in sync_setup_flow.cc to immediately show an error when an incorrect passphrase is entered. BUG=108718, 95269 TEST=unit_tests, sync_unit_tests, sync_integration_tests, manually run through sign in with / without passphrases, try incorrect passphrases (we now expect instant errors) Review URL: https://chromiumcodereview.appspot.com/9584040 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127630 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync/engine')
-rw-r--r--sync/engine/conflict_resolver.cc6
-rw-r--r--sync/engine/syncer_unittest.cc4
-rw-r--r--sync/engine/syncer_util.cc6
3 files changed, 8 insertions, 8 deletions
diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index 548613f..17f76a4 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -100,8 +100,8 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
// a) We can decrypt both the local and server data (else we'd be in
// conflict encryption and not attempting to resolve).
// b) All unsynced changes have been re-encrypted with the default key (
- // occurs either in AttemptToUpdateEntry, SetPassphrase, or
- // RefreshEncryption).
+ // occurs either in AttemptToUpdateEntry, SetEncryptionPassphrase,
+ // SetDecryptionPassphrase, or RefreshEncryption).
// c) Base_server_specifics having a valid datatype means that we received
// an undecryptable update that only changed specifics, and since then have
// not received any further non-specifics-only or decryptable updates.
@@ -235,7 +235,7 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
// the cryptographer. If we don't have pending keys we can store the
// merged set back immediately. Else we preserve the server keys and will
// update the nigori when the user provides the pending passphrase via
- // SetPassphrase(..).
+ // SetDecryptionPassphrase(..).
if (cryptographer->is_ready()) {
cryptographer->GetKeys(server_nigori->mutable_encrypted());
}
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 9a66d98..72ec057 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -1027,8 +1027,8 @@ TEST_F(SyncerTest, NigoriConflicts) {
sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
cryptographer(&wtrans)->GetKeys(nigori->mutable_encrypted());
cryptographer(&wtrans)->UpdateNigoriFromEncryptedTypes(nigori);
- // Normally this would be written as part of SetPassphrase, but we do it
- // manually for the test.
+ // Normally this would be written as part of SetDecryptionPassphrase, but we
+ // do it manually for the test.
nigori_entry.Put(SPECIFICS, specifics);
nigori_entry.Put(IS_UNSYNCED, true);
}
diff --git a/sync/engine/syncer_util.cc b/sync/engine/syncer_util.cc
index f003bcd..8526109 100644
--- a/sync/engine/syncer_util.cc
+++ b/sync/engine/syncer_util.cc
@@ -286,9 +286,9 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry(
// Make sure any unsynced changes are properly encrypted as necessary.
// We only perform this if the cryptographer is ready. If not, these are
- // re-encrypted at SetPassphrase time (via ReEncryptEverything). This logic
- // covers the case where the nigori updated marked new datatypes for
- // encryption, but didn't change the passphrase.
+ // re-encrypted at SetDecryptionPassphrase time (via ReEncryptEverything).
+ // This logic covers the case where the nigori update marked new datatypes
+ // for encryption, but didn't change the passphrase.
if (cryptographer->is_ready()) {
// Note that we don't bother to encrypt any data for which IS_UNSYNCED
// == false here. The machine that turned on encryption should know about