diff options
25 files changed, 747 insertions, 157 deletions
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc index 2c4c5ab..158ac63 100644 --- a/chrome/browser/chromeos/login/login_utils.cc +++ b/chrome/browser/chromeos/login/login_utils.cc @@ -921,8 +921,11 @@ void LoginUtilsImpl::StartSignedInServices( // data encrypted with the user's GAIA password. ProfileSyncService* sync_service = ProfileSyncServiceFactory::GetInstance()->GetForProfile(user_profile); - if (sync_service) - sync_service->SetPassphrase(password_, false); + if (sync_service) { + sync_service->SetPassphrase(password_, + ProfileSyncService::IMPLICIT, + ProfileSyncService::INTERNAL); + } } password_.clear(); TokenService* token_service = user_profile->GetTokenService(); diff --git a/chrome/browser/chromeos/login/screen_locker.cc b/chrome/browser/chromeos/login/screen_locker.cc index 75aa7db4..a751ba5 100644 --- a/chrome/browser/chromeos/login/screen_locker.cc +++ b/chrome/browser/chromeos/login/screen_locker.cc @@ -271,7 +271,9 @@ void ScreenLocker::OnLoginSuccess( ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile)); if (service && !service->HasSyncSetupCompleted()) { // If sync has failed somehow, try setting the sync passphrase here. - service->SetPassphrase(password, false); + service->SetPassphrase(password, + ProfileSyncService::IMPLICIT, + ProfileSyncService::INTERNAL); } } DBusThreadManager::Get()->GetPowerManagerClient()-> diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 77bb4bc..9803dda 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -82,7 +82,8 @@ class SyncBackendHost::Core virtual void OnPassphraseRequired( sync_api::PassphraseRequiredReason reason, const sync_pb::EncryptedData& pending_keys) OVERRIDE; - virtual void OnPassphraseAccepted( + virtual void OnPassphraseAccepted() OVERRIDE; + virtual void OnBootstrapTokenUpdated( const std::string& bootstrap_token) OVERRIDE; virtual void OnStopSyncingPermanently() OVERRIDE; virtual void OnUpdatedToken(const std::string& token) OVERRIDE; @@ -125,7 +126,9 @@ class SyncBackendHost::Core // Called to set the passphrase on behalf of // SyncBackendHost::SupplyPassphrase. - void DoSetPassphrase(const std::string& passphrase, bool is_explicit); + void DoSetPassphrase(const std::string& passphrase, + bool is_explicit, + bool user_provided); // Called to turn on encryption of all sync data as well as // reencrypt everything. @@ -323,7 +326,8 @@ void SyncBackendHost::StartSyncingWithServer() { } void SyncBackendHost::SetPassphrase(const std::string& passphrase, - bool is_explicit) { + bool is_explicit, + bool user_provided) { if (!IsNigoriEnabled()) { SLOG(WARNING) << "Silently dropping SetPassphrase request."; return; @@ -335,7 +339,7 @@ void SyncBackendHost::SetPassphrase(const std::string& passphrase, // If encryption is enabled and we've got a SetPassphrase sync_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(&SyncBackendHost::Core::DoSetPassphrase, core_.get(), - passphrase, is_explicit)); + passphrase, is_explicit, user_provided)); } void SyncBackendHost::StopSyncManagerForShutdown( @@ -804,14 +808,23 @@ void SyncBackendHost::Core::OnPassphraseRequired( &SyncBackendHost::NotifyPassphraseRequired, reason, pending_keys); } -void SyncBackendHost::Core::OnPassphraseAccepted( +void SyncBackendHost::Core::OnPassphraseAccepted() { + if (!sync_loop_) + return; + DCHECK_EQ(MessageLoop::current(), sync_loop_); + host_.Call( + FROM_HERE, + &SyncBackendHost::NotifyPassphraseAccepted); +} + +void SyncBackendHost::Core::OnBootstrapTokenUpdated( const std::string& bootstrap_token) { if (!sync_loop_) return; DCHECK_EQ(MessageLoop::current(), sync_loop_); host_.Call( FROM_HERE, - &SyncBackendHost::NotifyPassphraseAccepted, bootstrap_token); + &SyncBackendHost::PersistEncryptionBootstrapToken, bootstrap_token); } void SyncBackendHost::Core::OnStopSyncingPermanently() { @@ -981,9 +994,10 @@ void SyncBackendHost::Core::DoRequestCleanupDisabledTypes() { } void SyncBackendHost::Core::DoSetPassphrase(const std::string& passphrase, - bool is_explicit) { + bool is_explicit, + bool user_provided) { DCHECK_EQ(MessageLoop::current(), sync_loop_); - sync_manager_->SetPassphrase(passphrase, is_explicit); + sync_manager_->SetPassphrase(passphrase, is_explicit, user_provided); } void SyncBackendHost::Core::DoEnableEncryptEverything() { @@ -1184,8 +1198,7 @@ void SyncBackendHost::NotifyPassphraseRequired( frontend_->OnPassphraseRequired(reason, pending_keys); } -void SyncBackendHost::NotifyPassphraseAccepted( - const std::string& bootstrap_token) { +void SyncBackendHost::NotifyPassphraseAccepted() { if (!frontend_) return; @@ -1193,8 +1206,6 @@ void SyncBackendHost::NotifyPassphraseAccepted( // Clear our cache of the cryptographer's pending keys. cached_pending_keys_.clear_blob(); - - PersistEncryptionBootstrapToken(bootstrap_token); frontend_->OnPassphraseAccepted(); } diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index bbb8bb7..6f508e5 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -179,11 +179,18 @@ class SyncBackendHost : public BackendDataTypeConfigurer { virtual void StartSyncingWithServer(); // Called on |frontend_loop_| to asynchronously set the passphrase. - // |is_explicit| is true if the call is in response to the user explicitly - // setting a passphrase as opposed to implicitly (from the users' perspective) - // using their Google Account password. An implicit SetPassphrase will *not* - // *not* override an explicit passphrase set previously. - void SetPassphrase(const std::string& passphrase, bool is_explicit); + // |is_explicit| is true if the call is in response to the user setting a + // custom explicit passphrase as opposed to implicitly (from the users' + // perspective) using their Google Account password. An implicit SetPassphrase + // will *not* override an explicit passphrase set previously. Note that + // if the data is encrypted with an old Google Account password, the user + // may still have to provide a "implicit" passphrase. + // |user_provided| corresponds to the user having manually provided this + // passphrase. It should only be false for passphrases intercepted from the + // Google Sign-in Success notification and true otherwise. + void SetPassphrase(const std::string& passphrase, + bool is_explicit, + bool user_provided); // Called on |frontend_loop_| to kick off shutdown procedure. After this, // no further sync activity will occur with the sync server and no further @@ -407,7 +414,7 @@ class SyncBackendHost : public BackendDataTypeConfigurer { sync_pb::EncryptedData pending_keys); // Invoked when the passphrase provided by the user has been accepted. - void NotifyPassphraseAccepted(const std::string& bootstrap_token); + void NotifyPassphraseAccepted(); // Invoked when an updated token is available from the sync server. void NotifyUpdatedToken(const std::string& token); diff --git a/chrome/browser/sync/internal_api/debug_info_event_listener.cc b/chrome/browser/sync/internal_api/debug_info_event_listener.cc index 239e42b..d783dee 100644 --- a/chrome/browser/sync/internal_api/debug_info_event_listener.cc +++ b/chrome/browser/sync/internal_api/debug_info_event_listener.cc @@ -48,11 +48,15 @@ void DebugInfoEventListener::OnPassphraseRequired( CreateAndAddEvent(sync_pb::DebugEventInfo::PASSPHRASE_REQUIRED); } -void DebugInfoEventListener::OnPassphraseAccepted( - const std::string& bootstrap_token) { +void DebugInfoEventListener::OnPassphraseAccepted() { CreateAndAddEvent(sync_pb::DebugEventInfo::PASSPHRASE_ACCEPTED); } +void DebugInfoEventListener::OnBootstrapTokenUpdated( + const std::string& bootstrap_token) { + CreateAndAddEvent(sync_pb::DebugEventInfo::BOOTSTRAP_TOKEN_UPDATED); +} + void DebugInfoEventListener::OnStopSyncingPermanently() { CreateAndAddEvent(sync_pb::DebugEventInfo::STOP_SYNCING_PERMANENTLY); } diff --git a/chrome/browser/sync/internal_api/debug_info_event_listener.h b/chrome/browser/sync/internal_api/debug_info_event_listener.h index 98ecfdf..a4daa18 100644 --- a/chrome/browser/sync/internal_api/debug_info_event_listener.h +++ b/chrome/browser/sync/internal_api/debug_info_event_listener.h @@ -40,7 +40,8 @@ class DebugInfoEventListener : public sync_api::SyncManager::Observer, virtual void OnPassphraseRequired( sync_api::PassphraseRequiredReason reason, const sync_pb::EncryptedData& pending_keys) OVERRIDE; - virtual void OnPassphraseAccepted( + virtual void OnPassphraseAccepted() OVERRIDE; + virtual void OnBootstrapTokenUpdated( const std::string& bootstrap_token) OVERRIDE; virtual void OnStopSyncingPermanently() OVERRIDE; virtual void OnUpdatedToken(const std::string& token) OVERRIDE; diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc index 09739d2..a93e256 100644 --- a/chrome/browser/sync/internal_api/sync_manager.cc +++ b/chrome/browser/sync/internal_api/sync_manager.cc @@ -241,7 +241,19 @@ class SyncManager::SyncInternal // Try to set the current passphrase to |passphrase|, and record whether // it is an explicit passphrase or implicitly using gaia in the Nigori // node. - void SetPassphrase(const std::string& passphrase, bool is_explicit); + // |is_explicit| is true if the call is in response to the user setting a + // custom explicit passphrase as opposed to implicitly (from the users' + // perspective) using their Google Account password. Once an explicit + // passphrase is set, it can never be overwritten (not even by another + // explicit passphrase). + // |user_provided| is true corresponds to the user having manually provided + // this passphrase. It should only be false for passphrases intercepted + // from the Google Sign-in Success notification. Note that if the data is + // encrypted with an old Google Account password, the user may still have to + // provide an "implicit" passphrase. + void SetPassphrase(const std::string& passphrase, + bool is_explicit, + bool user_provided); // Call periodically from a database-safe thread to persist recent changes // to the syncapi model. @@ -386,6 +398,25 @@ class SyncManager::SyncInternal // available. void RaiseAuthNeededEvent(); + // Helpers for SetPassphrase. TODO(rsimha): make these the public methods + // eventually and have them replace SetPassphrase(..). + // These correspond to setting a passphrase for decryption (when we have + // pending keys) or setting a passphrase for encryption (we do not have + // pending keys). + bool SetDecryptionPassphrase( + const KeyParams& key_params, + bool nigori_has_explicit_passphrase, + bool is_explicit, + bool user_provided, + Cryptographer* cryptographer, + std::string *new_bootstrap_token); + bool SetEncryptionPassphrase( + const KeyParams& key_params, + bool nigori_has_explicit_passphrase, + bool is_explicit, + Cryptographer* cryptographer, + std::string *new_bootstrap_token); + // Internal callback of UpdateCryptographerAndNigoriCallback. void UpdateCryptographerAndNigoriCallback( const base::Callback<void(bool)>& done_callback, @@ -742,9 +773,10 @@ void SyncManager::StartSyncingNormally() { } void SyncManager::SetPassphrase(const std::string& passphrase, - bool is_explicit) { + bool is_explicit, + bool user_provided) { DCHECK(thread_checker_.CalledOnValidThread()); - data_->SetPassphrase(passphrase, is_explicit); + data_->SetPassphrase(passphrase, is_explicit, user_provided); } void SyncManager::EnableEncryptEverything() { @@ -1168,7 +1200,8 @@ void SyncManager::SyncInternal::RaiseAuthNeededEvent() { } void SyncManager::SyncInternal::SetPassphrase( - const std::string& passphrase, bool is_explicit) { + const std::string& passphrase, bool is_explicit, bool user_provided) { + DCHECK(user_provided || !is_explicit); // We do not accept empty passphrases. if (passphrase.empty()) { DVLOG(1) << "Rejecting empty passphrase."; @@ -1186,7 +1219,7 @@ void SyncManager::SyncInternal::SetPassphrase( // All accesses to the cryptographer are protected by a transaction. WriteTransaction trans(FROM_HERE, GetUserShare()); Cryptographer* cryptographer = trans.GetCryptographer(); - KeyParams params = {"localhost", "dummy", passphrase}; + KeyParams key_params = {"localhost", "dummy", passphrase}; WriteNode node(&trans); if (!node.InitByTagLookup(kNigoriTag)) { @@ -1194,49 +1227,83 @@ void SyncManager::SyncInternal::SetPassphrase( NOTREACHED(); return; } - + bool nigori_has_explicit_passphrase = + node.GetNigoriSpecifics().using_explicit_passphrase(); + + // There are five cases to handle here: + // 1. The user has no pending keys and is setting their current GAIA password + // as the encryption passphrase. This happens either during first time sync + // with a clean profile, or after re-authenticating on a profile that was + // already signed in with the cryptographer ready. + // 2. The user is overwriting an (already provided) implicit passphrase with + // an explicit (custom) passphrase. There are no pending keys. + // 3. We're using the current GAIA password to decrypt the pending keys. This + // happens when signing in to an account with a previously set implicit + // passphrase, where the data is already encrypted with the newest GAIA + // password. + // 4. The user is providing an old GAIA password to decrypt the pending keys. + // In this case, the user is using an implicit passphrase, but has changed + // their password since they last encrypted their data, and therefore + // their current GAIA password was unable to decrypt the data. This will + // happen when the user is setting up a new profile with a previously + // encrypted account (after changing passwords). + // 5. The user is providing a previously set explicit passphrase to decrypt + // the pending keys. + // Furthermore, we enforce the following: The bootstrap encryption token will + // always be derived from the newest GAIA password if the account is using + // an implicit passphrase (even if the data is encrypted with an old GAIA + // password). If the account is using an explicit (custom) passphrase, the + // bootstrap token will be derived from the most recently provided explicit + // passphrase (that was able to decrypt the data). + // TODO(rsimha): Fix the plumbing so we call these two methods separately and + // directly from the PSS API. It may also make sense to embed this logic + // within the cryptographer itself. http://crbug.com/108718 + std::string bootstrap_token; + bool success = false; + sync_pb::EncryptedData pending_keys; if (cryptographer->has_pending_keys()) { - bool succeeded = false; - - // See if the explicit flag matches what is set in nigori. If not we dont - // even try the passphrase. Note: This could mean that we wont try setting - // the gaia password as passphrase if custom is elected by the user. Which - // is fine because nigori node has all the old passwords in it. - if (node.GetNigoriSpecifics().using_explicit_passphrase() == is_explicit) { - if (cryptographer->DecryptPendingKeys(params)) { - succeeded = true; - } else { - DVLOG(1) << "Passphrase failed to decrypt pending keys."; - } - } else { - DVLOG(1) << "Not trying the passphrase because the explicit flags dont " - << "match. Nigori node's explicit flag is " - << node.GetNigoriSpecifics().using_explicit_passphrase(); - } - - if (!succeeded) { - sync_pb::EncryptedData pending_keys; - if (cryptographer->has_pending_keys()) - pending_keys = cryptographer->GetPendingKeys(); - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnPassphraseRequired(sync_api::REASON_SET_PASSPHRASE_FAILED, - pending_keys)); - return; + pending_keys = cryptographer->GetPendingKeys(); + // Handles cases 3, 4, and 5. + success = SetDecryptionPassphrase(key_params, + nigori_has_explicit_passphrase, + is_explicit, + user_provided, + cryptographer, + &bootstrap_token); + if (success) { + // Nudge the syncer so that encrypted datatype updates that were waiting + // for this passphrase get applied as soon as possible. + RequestNudge(FROM_HERE); } - - // Nudge the syncer so that encrypted datatype updates that were waiting for - // this passphrase get applied as soon as possible. - RequestNudge(FROM_HERE); } else { - DVLOG(1) << "No pending keys, adding provided passphrase."; - - // Prevent an implicit SetPassphrase request from changing an explicitly - // set passphrase. - if (!is_explicit && node.GetNigoriSpecifics().using_explicit_passphrase()) - return; + // Handles cases 1 and 2. + success = SetEncryptionPassphrase(key_params, + nigori_has_explicit_passphrase, + is_explicit, + cryptographer, + &bootstrap_token); + } + + // It's possible we need to change the bootstrap token even if we failed to + // set the passphrase (for example if we need to preserve the new GAIA + // passphrase). + if (!bootstrap_token.empty()) { + DVLOG(1) << "Bootstrap token updated."; + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnBootstrapTokenUpdated(bootstrap_token)); + } - cryptographer->AddKey(params); + if (!success) { + DVLOG(1) << "SetPassphrase failure, notifying and returning."; + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnPassphraseRequired(sync_api::REASON_SET_PASSPHRASE_FAILED, + pending_keys)); + return; } + DVLOG(1) << "SetPassphrase success, updating nigori and reencrypting."; + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnPassphraseAccepted()); + DCHECK(cryptographer->is_ready()); // TODO(tim): Bug 58231. It would be nice if SetPassphrase didn't require // messing with the Nigori node, because we can't call SetPassphrase until @@ -1255,12 +1322,168 @@ void SyncManager::SyncInternal::SetPassphrase( // Does nothing if everything is already encrypted or the cryptographer has // pending keys. ReEncryptEverything(&trans); +} - DVLOG(1) << "Passphrase accepted, bootstrapping encryption."; - std::string bootstrap_token; - cryptographer->GetBootstrapToken(&bootstrap_token); - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnPassphraseAccepted(bootstrap_token)); +bool SyncManager::SyncInternal::SetEncryptionPassphrase( + const KeyParams& key_params, + bool nigori_has_explicit_passphrase, + bool is_explicit, + Cryptographer* cryptographer, + std::string *bootstrap_token) { + if (cryptographer->has_pending_keys()) { + LOG(ERROR) << "Attempt to set encryption passphrase failed because there " + << "were pending keys."; + return false; + } + if (!nigori_has_explicit_passphrase) { + // Case 1 and 2. Setting a new GAIA passphrase when there are no pending + // keys (1), or overwriting an implicit passphrase with a new explicit one + // (2) when there are no pending keys. + if (cryptographer->AddKey(key_params)) { + DVLOG(1) << "Setting " << (is_explicit ? "explicit" : "implicit" ) + << " passphrase for encryption."; + cryptographer->GetBootstrapToken(bootstrap_token); + return true; + } else { + NOTREACHED() << "Failed to add key to cryptographer."; + return false; + } + } else { // nigori_has_explicit_passphrase == true + NOTREACHED() << "Attempting to change explicit passphrase when one has " + << "already been set."; + return false; + } // nigori_has_explicit_passphrase + NOTREACHED(); + return false; +} + +bool SyncManager::SyncInternal::SetDecryptionPassphrase( + const KeyParams& key_params, + bool nigori_has_explicit_passphrase, + bool is_explicit, + bool user_provided, + Cryptographer* cryptographer, + std::string *bootstrap_token) { + if (!cryptographer->has_pending_keys()) { + NOTREACHED() << "Attempt to set decryption passphrase failed because there " + << "were no pending keys."; + return false; + + } + if (!nigori_has_explicit_passphrase) { + if (!is_explicit) { + if (!user_provided) { + // Case 3. + if (cryptographer->DecryptPendingKeys(key_params)) { + DVLOG(1) << "Implicit internal passphrase accepted for decryption."; + cryptographer->GetBootstrapToken(bootstrap_token); + return true; + } else { + DVLOG(1) << "Implicit internal passphrase failed to decrypt, adding " + << "anyways as default passphrase and persisting via " + << "bootstrap token."; + // Turns out we're encrypted with an old GAIA password, and we're + // actually in case 3. But, because this is the current GAIA + // password, we need to generate a new bootstrap token to preserve it. + // We build a temporary cryptographer to allow us to extract these + // params without polluting our current cryptographer. + Cryptographer temp_cryptographer; + temp_cryptographer.AddKey(key_params); + temp_cryptographer.GetBootstrapToken(bootstrap_token); + // We then set the new passphrase as the default passphrase of the + // real cryptographer, even though we have pending keys. This is safe, + // as although Cryptographer::is_initialized() will now be true, + // is_ready() will remain false due to having pending keys. + cryptographer->AddKey(key_params); + return false; + } + } else { // user_provided == true + if (cryptographer->is_initialized()) { + // We only want to change the default encryption key to the pending + // one if the pending keybag already contains the current default. + // This covers the case where a different client re-encrypted + // everything with a newer gaia passphrase (and hence the keybag + // contains keys from all previously used gaia passphrases). + // Otherwise, we're in a situation where the pending keys are + // encrypted with an old gaia passphrase, while the default is the + // current gaia passphrase. In that case, we preserve the default. + Cryptographer temp_cryptographer; + temp_cryptographer.SetPendingKeys(cryptographer->GetPendingKeys()); + if (temp_cryptographer.DecryptPendingKeys(key_params)) { + // Check to see if the pending bag of keys contains the current + // default key. + sync_pb::EncryptedData encrypted; + cryptographer->GetKeys(&encrypted); + if (temp_cryptographer.CanDecrypt(encrypted)) { + DVLOG(1) << "Implicit user provided passphrase accepted for " + << "decryption, overwriting default."; + // The pending keybag contains the current default. Go ahead + // and update the cryptographer, letting the default change. + // Case 3. + cryptographer->DecryptPendingKeys(key_params); + cryptographer->GetBootstrapToken(bootstrap_token); + return true; + } else { + // The pending keybag does not contain the current default + // encryption key. We want to restore the current default + // after decrypting the pending keys. + // Case 4. + DVLOG(1) << "Implicit user provided passphrase accepted for " + << "decryption, restoring implicit internal passphrase " + << "as default."; + std::string bootstrap_token_from_current_key; + cryptographer->GetBootstrapToken( + &bootstrap_token_from_current_key); + cryptographer->DecryptPendingKeys(key_params); + // Overwrite the default from the pending keys. + cryptographer->AddKeyFromBootstrapToken( + bootstrap_token_from_current_key); + return true; + } + } + } else if (cryptographer->DecryptPendingKeys(key_params)) { + // This can happpen if this is a client that has lost the credentials + // from the current gaia password, and has data encrypted with an old + // gaia password. We won't be able to re-encrypt to the most recent + // GAIA password, so for now just continue and initialize the + // cryptographer. + // This is a subset of case 4 that we don't handle properly yet. + // TODO(zea): trigger prompting for re-auth here. See part 2 of + // http://crbug.com/104508. + DVLOG(1) << "Implicit user provided passphrase accepted, initializing" + << " cryptographer."; + return true; + } else { + DVLOG(1) << "Implicit user provided passphrase failed to decrypt."; + return false; + } + } // user_provided + } else { // is_explicit == true + // This can happen if the client changes their password, re-authed on + // another machine, and we only just now received the updated nigori. + DVLOG(1) << "Explicit passphrase failed to decrypt because nigori had " + << "implicit passphrase."; + return false; + } // is_explicit + } else { // nigori_has_explicit_passphrase == true + if (!is_explicit) { + DVLOG(1) << "Implicit passphrase rejected because nigori had explicit " + << "passphrase."; + return false; + } else { // is_explicit == true + // Case 5. + if (cryptographer->DecryptPendingKeys(key_params)) { + DVLOG(1) << "Explicit passphrase accepted for decryption."; + cryptographer->GetBootstrapToken(bootstrap_token); + return true; + } else { + DVLOG(1) << "Explicit passphrase failed to decrypt."; + return false; + } + } // is_explicit + } // nigori_has_explicit_passphrase + NOTREACHED(); + return false; } bool SyncManager::SyncInternal::IsUsingExplicitPassphrase() { @@ -1389,8 +1612,7 @@ void SyncManager::SyncInternal::ReEncryptEverything(WriteTransaction* trans) { } // NOTE: We notify from within a transaction. - FOR_EACH_OBSERVER( - SyncManager::Observer, observers_, OnEncryptionComplete()); + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, OnEncryptionComplete()); } SyncManager::~SyncManager() { diff --git a/chrome/browser/sync/internal_api/sync_manager.h b/chrome/browser/sync/internal_api/sync_manager.h index f0ece70..cfb466c 100644 --- a/chrome/browser/sync/internal_api/sync_manager.h +++ b/chrome/browser/sync/internal_api/sync_manager.h @@ -285,11 +285,19 @@ class SyncManager { const sync_pb::EncryptedData& pending_keys) = 0; // Called when the passphrase provided by the user has been accepted and is - // now used to encrypt sync data. |bootstrap_token| is an opaque base64 - // encoded representation of the key generated by the accepted passphrase, - // and is provided to the observer for persistence purposes and use in a - // future initialization of sync (e.g. after restart). - virtual void OnPassphraseAccepted(const std::string& bootstrap_token) = 0; + // now used to encrypt sync data. + virtual void OnPassphraseAccepted() = 0; + + // |bootstrap_token| is an opaque base64 encoded representation of the key + // generated by the current passphrase, and is provided to the observer for + // persistence purposes and use in a future initialization of sync (e.g. + // after restart). The boostrap token will always be derived from the most + // recent GAIA password (for accounts with implicit passphrases), even if + // the data is still encrypted with an older GAIA password. For accounts + // with explicit passphrases, it will be the most recently seen custom + // passphrase. + virtual void OnBootstrapTokenUpdated( + const std::string& bootstrap_token) = 0; // Called when initialization is complete to the point that SyncManager can // process changes. This does not necessarily mean authentication succeeded @@ -479,11 +487,18 @@ class SyncManager { // If the passphrase in invalid, OnPassphraseRequired will be fired. // Calling this metdod again is the appropriate course of action to "retry" // with a new passphrase. - // |is_explicit| is true if the call is in response to the user explicitly - // setting a passphrase as opposed to implicitly (from the users' perspective) - // using their Google Account password. An implicit SetPassphrase will *not* - // *not* override an explicit passphrase set previously. - void SetPassphrase(const std::string& passphrase, bool is_explicit); + // |is_explicit| is true if the call is in response to the user setting a + // custom explicit passphrase as opposed to implicitly (from the users' + // perspective) using their Google Account password. An implicit SetPassphrase + // will *not* override an explicit passphrase set previously. Note that + // if the data is encrypted with an old Google Account password, the user + // may still have to provide a "implicit" passphrase. + // |user_provided| corresponds to the user having manually provided this + // passphrase. It should only be false for passphrases intercepted from the + // Google Sign-in Success notification and true otherwise. + void SetPassphrase(const std::string& passphrase, + bool is_explicit, + bool user_provided); // Puts the SyncScheduler into a mode where no normal nudge or poll traffic // will occur, but calls to RequestConfig will be supported. If |callback| diff --git a/chrome/browser/sync/internal_api/syncapi_unittest.cc b/chrome/browser/sync/internal_api/syncapi_unittest.cc index 0fb6e4a..c0b520d 100644 --- a/chrome/browser/sync/internal_api/syncapi_unittest.cc +++ b/chrome/browser/sync/internal_api/syncapi_unittest.cc @@ -66,6 +66,7 @@ using browser_sync::Cryptographer; using browser_sync::HasArgsAsList; using browser_sync::HasDetailsAsDictionary; using browser_sync::KeyParams; +using browser_sync::kNigoriTag; using browser_sync::JsArgList; using browser_sync::JsBackend; using browser_sync::JsEventHandler; @@ -667,7 +668,8 @@ class SyncManagerObserverMock : public SyncManager::Observer { MOCK_METHOD2(OnPassphraseRequired, void(sync_api::PassphraseRequiredReason, const sync_pb::EncryptedData&)); // NOLINT - MOCK_METHOD1(OnPassphraseAccepted, void(const std::string&)); // NOLINT + MOCK_METHOD0(OnPassphraseAccepted, void()); // NOLINT + MOCK_METHOD1(OnBootstrapTokenUpdated, void(const std::string&)); // NOLINT MOCK_METHOD0(OnStopSyncingPermanently, void()); // NOLINT MOCK_METHOD1(OnUpdatedToken, void(const std::string&)); // NOLINT MOCK_METHOD0(OnClearServerDataFailed, void()); // NOLINT @@ -707,6 +709,7 @@ class SyncManagerTest : public testing::Test, }; enum EncryptionStatus { + UNINITIALIZED, DEFAULT_ENCRYPTION, FULL_ENCRYPTION }; @@ -833,8 +836,12 @@ class SyncManagerTest : public testing::Test, Cryptographer* cryptographer = trans.GetCryptographer(); if (!cryptographer) return false; - KeyParams params = {"localhost", "dummy", "foobar"}; - cryptographer->AddKey(params); + if (encryption_status != UNINITIALIZED) { + KeyParams params = {"localhost", "dummy", "foobar"}; + cryptographer->AddKey(params); + } else { + DCHECK_NE(nigori_status, WRITE_TO_NIGORI); + } if (encryption_status == FULL_ENCRYPTION) cryptographer->set_encrypt_everything(); if (nigori_status == WRITE_TO_NIGORI) { @@ -1448,9 +1455,10 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) { // Trigger's a ReEncryptEverything with new passphrase. testing::Mock::VerifyAndClearExpectations(&observer_); - EXPECT_CALL(observer_, OnPassphraseAccepted(_)); + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); EXPECT_CALL(observer_, OnEncryptionComplete()); - sync_manager_.SetPassphrase("new_passphrase", true); + sync_manager_.SetPassphrase("new_passphrase", true, true); EXPECT_TRUE(sync_manager_.EncryptEverythingEnabledForTest()); { ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); @@ -1476,15 +1484,76 @@ TEST_F(SyncManagerTest, EncryptDataTypesWithData) { // a reencryption and should just notify immediately. // TODO(zea): add logic to ensure nothing was written. testing::Mock::VerifyAndClearExpectations(&observer_); - EXPECT_CALL(observer_, OnPassphraseAccepted(_)).Times(0); + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)).Times(0); + EXPECT_CALL(observer_, OnPassphraseAccepted()).Times(0); EXPECT_CALL(observer_, OnEncryptionComplete()); sync_manager_.EnableEncryptEverything(); } +// Test that when there are no pending keys and the cryptographer is not +// initialized, we add a key based on the current GAIA password. +// (case 1 in SyncManager::SyncInternalSetPassphrase) +TEST_F(SyncManagerTest, SetInitialGaiaPass) { + EXPECT_FALSE(SetUpEncryption(DONT_WRITE_NIGORI, UNINITIALIZED)); + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); + EXPECT_CALL(observer_, OnEncryptionComplete()); + sync_manager_.SetPassphrase("new_passphrase", false, false); + EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + ReadNode node(&trans); + EXPECT_TRUE(node.InitByTagLookup(kNigoriTag)); + sync_pb::NigoriSpecifics nigori = node.GetNigoriSpecifics(); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_ready()); + EXPECT_TRUE(cryptographer->CanDecrypt(nigori.encrypted())); + } +} + +// Test that when there are no pending keys and we have on the old GAIA +// password, we update and re-encrypt everything with the new GAIA password. +// (case 1 in SyncManager::SyncInternalSetPassphrase) +TEST_F(SyncManagerTest, UpdateGaiaPass) { + EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); + Cryptographer verifier; + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + std::string bootstrap_token; + cryptographer->GetBootstrapToken(&bootstrap_token); + verifier.Bootstrap(bootstrap_token); + } + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); + EXPECT_CALL(observer_, OnEncryptionComplete()); + sync_manager_.SetPassphrase("new_passphrase", false, false); + EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_ready()); + // Verify the default key has changed. + sync_pb::EncryptedData encrypted; + cryptographer->GetKeys(&encrypted); + EXPECT_FALSE(verifier.CanDecrypt(encrypted)); + } +} + +// Sets a new explicit passphrase. This should update the bootstrap token +// and re-encrypt everything. +// (case 2 in SyncManager::SyncInternalSetPassphrase) TEST_F(SyncManagerTest, SetPassphraseWithPassword) { + Cryptographer verifier; EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); { WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + // Store the default (soon to be old) key. + Cryptographer* cryptographer = trans.GetCryptographer(); + std::string bootstrap_token; + cryptographer->GetBootstrapToken(&bootstrap_token); + verifier.Bootstrap(bootstrap_token); + ReadNode root_node(&trans); root_node.InitByRootLookup(); @@ -1495,12 +1564,20 @@ TEST_F(SyncManagerTest, SetPassphraseWithPassword) { data.set_password_value("secret"); password_node.SetPasswordSpecifics(data); } - EXPECT_CALL(observer_, OnPassphraseAccepted(_)); + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); EXPECT_CALL(observer_, OnEncryptionComplete()); - sync_manager_.SetPassphrase("new_passphrase", true); + sync_manager_.SetPassphrase("new_passphrase", true, true); EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); { ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_ready()); + // Verify the default key has changed. + sync_pb::EncryptedData encrypted; + cryptographer->GetKeys(&encrypted); + EXPECT_FALSE(verifier.CanDecrypt(encrypted)); + ReadNode password_node(&trans); EXPECT_TRUE(password_node.InitByClientTagLookup(syncable::PASSWORDS, "foo")); @@ -1510,6 +1587,163 @@ TEST_F(SyncManagerTest, SetPassphraseWithPassword) { } } +// Manually set the pending keys in the cryptographer/nigori to reflect the data +// being encrypted with a new (unprovided) GAIA password, then supply the +// password. +// (case 3 in SyncManager::SyncInternalSetPassphrase) +TEST_F(SyncManagerTest, SupplyPendingGAIAPass) { + EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); + Cryptographer other_cryptographer; + { + WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + std::string bootstrap_token; + cryptographer->GetBootstrapToken(&bootstrap_token); + other_cryptographer.Bootstrap(bootstrap_token); + + // Now update the nigori to reflect the new keys, and update the + // cryptographer to have pending keys. + KeyParams params = {"localhost", "dummy", "passphrase2"}; + other_cryptographer.AddKey(params); + WriteNode node(&trans); + EXPECT_TRUE(node.InitByTagLookup(kNigoriTag)); + sync_pb::NigoriSpecifics nigori; + other_cryptographer.GetKeys(nigori.mutable_encrypted()); + cryptographer->Update(nigori); + EXPECT_TRUE(cryptographer->has_pending_keys()); + node.SetNigoriSpecifics(nigori); + } + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); + EXPECT_CALL(observer_, OnEncryptionComplete()); + sync_manager_.SetPassphrase("passphrase2", false, false); + EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_ready()); + // Verify we're encrypting with the new key. + sync_pb::EncryptedData encrypted; + cryptographer->GetKeys(&encrypted); + EXPECT_TRUE(other_cryptographer.CanDecrypt(encrypted)); + } +} + +// Manually set the pending keys in the cryptographer/nigori to reflect the data +// being encrypted with an old (unprovided) GAIA password. Attempt to supply +// the current GAIA password and verify the bootstrap token is updated. Then +// supply the old GAIA password, and verify we re-encrypt all data with the +// new GAIA password. +// (case 4 in SyncManager::SyncInternalSetPassphrase) +TEST_F(SyncManagerTest, SupplyPendingOldGAIAPass) { + EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); + Cryptographer other_cryptographer; + { + WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + std::string bootstrap_token; + cryptographer->GetBootstrapToken(&bootstrap_token); + other_cryptographer.Bootstrap(bootstrap_token); + + // Now update the nigori to reflect the new keys, and update the + // cryptographer to have pending keys. + KeyParams params = {"localhost", "dummy", "old_gaia"}; + other_cryptographer.AddKey(params); + WriteNode node(&trans); + EXPECT_TRUE(node.InitByTagLookup(kNigoriTag)); + sync_pb::NigoriSpecifics nigori; + other_cryptographer.GetKeys(nigori.mutable_encrypted()); + node.SetNigoriSpecifics(nigori); + cryptographer->Update(nigori); + + // other_cryptographer now contains all encryption keys, and is encrypting + // with the newest gaia. + KeyParams new_params = {"localhost", "dummy", "new_gaia"}; + other_cryptographer.AddKey(new_params); + } + // The bootstrap token should have been updated. Save it to ensure it's based + // on the new GAIA password. + std::string bootstrap_token; + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)) + .WillOnce(SaveArg<0>(&bootstrap_token)); + EXPECT_CALL(observer_, OnPassphraseRequired(_,_)); + sync_manager_.SetPassphrase("new_gaia", false, false); + EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); + testing::Mock::VerifyAndClearExpectations(&observer_); + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_initialized()); + EXPECT_FALSE(cryptographer->is_ready()); + // Verify we're encrypting with the new key, even though we have pending + // keys. + sync_pb::EncryptedData encrypted; + other_cryptographer.GetKeys(&encrypted); + EXPECT_TRUE(cryptographer->CanDecrypt(encrypted)); + } + EXPECT_CALL(observer_, OnPassphraseAccepted()); + EXPECT_CALL(observer_, OnEncryptionComplete()); + sync_manager_.SetPassphrase("old_gaia", false, true); + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_ready()); + + // Verify we're encrypting with the new key. + sync_pb::EncryptedData encrypted; + other_cryptographer.GetKeys(&encrypted); + EXPECT_TRUE(cryptographer->CanDecrypt(encrypted)); + + // Verify the saved bootstrap token is based on the new gaia password. + Cryptographer temp_cryptographer; + temp_cryptographer.Bootstrap(bootstrap_token); + EXPECT_TRUE(temp_cryptographer.CanDecrypt(encrypted)); + } +} + +// Manually set the pending keys in the cryptographer/nigori to reflect the data +// being encrypted with an explicit (unprovided) passphrase, then supply the +// passphrase. +// (case 5 in SyncManager::SyncInternalSetPassphrase) +TEST_F(SyncManagerTest, SupplyPendingExplicitPass) { + EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); + Cryptographer other_cryptographer; + { + WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + std::string bootstrap_token; + cryptographer->GetBootstrapToken(&bootstrap_token); + other_cryptographer.Bootstrap(bootstrap_token); + + // Now update the nigori to reflect the new keys, and update the + // cryptographer to have pending keys. + KeyParams params = {"localhost", "dummy", "explicit"}; + other_cryptographer.AddKey(params); + WriteNode node(&trans); + EXPECT_TRUE(node.InitByTagLookup(kNigoriTag)); + sync_pb::NigoriSpecifics nigori; + other_cryptographer.GetKeys(nigori.mutable_encrypted()); + cryptographer->Update(nigori); + EXPECT_TRUE(cryptographer->has_pending_keys()); + nigori.set_using_explicit_passphrase(true); + node.SetNigoriSpecifics(nigori); + } + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); + EXPECT_CALL(observer_, OnEncryptionComplete()); + sync_manager_.SetPassphrase("explicit", true, true); + EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); + { + ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); + Cryptographer* cryptographer = trans.GetCryptographer(); + EXPECT_TRUE(cryptographer->is_ready()); + // Verify we're encrypting with the new key. + sync_pb::EncryptedData encrypted; + cryptographer->GetKeys(&encrypted); + EXPECT_TRUE(other_cryptographer.CanDecrypt(encrypted)); + } +} + TEST_F(SyncManagerTest, SetPassphraseWithEmptyPasswordNode) { EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); int64 node_id = 0; @@ -1524,9 +1758,10 @@ TEST_F(SyncManagerTest, SetPassphraseWithEmptyPasswordNode) { root_node, tag)); node_id = password_node.GetId(); } - EXPECT_CALL(observer_, OnPassphraseAccepted(_)); + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); EXPECT_CALL(observer_, OnEncryptionComplete()); - sync_manager_.SetPassphrase("new_passphrase", true); + sync_manager_.SetPassphrase("new_passphrase", true, true); EXPECT_FALSE(sync_manager_.EncryptEverythingEnabledForTest()); { ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); @@ -1715,9 +1950,10 @@ TEST_F(SyncManagerTest, UpdateEntryWithEncryption) { // Set a new passphrase. Should set is_unsynced. testing::Mock::VerifyAndClearExpectations(&observer_); - EXPECT_CALL(observer_, OnPassphraseAccepted(_)); + EXPECT_CALL(observer_, OnBootstrapTokenUpdated(_)); + EXPECT_CALL(observer_, OnPassphraseAccepted()); EXPECT_CALL(observer_, OnEncryptionComplete()); - sync_manager_.SetPassphrase("new_passphrase", true); + sync_manager_.SetPassphrase("new_passphrase", true, true); { ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); ReadNode node(&trans); diff --git a/chrome/browser/sync/js/js_sync_manager_observer.cc b/chrome/browser/sync/js/js_sync_manager_observer.cc index ecb4304..843ac1d 100644 --- a/chrome/browser/sync/js/js_sync_manager_observer.cc +++ b/chrome/browser/sync/js/js_sync_manager_observer.cc @@ -71,16 +71,24 @@ void JsSyncManagerObserver::OnPassphraseRequired( HandleJsEvent(FROM_HERE, "onPassphraseRequired", JsEventDetails(&details)); } -void JsSyncManagerObserver::OnPassphraseAccepted( - const std::string& bootstrap_token) { +void JsSyncManagerObserver::OnPassphraseAccepted() { if (!event_handler_.IsInitialized()) { return; } DictionaryValue details; - details.SetString("bootstrapToken", "<redacted>"); HandleJsEvent(FROM_HERE, "onPassphraseAccepted", JsEventDetails(&details)); } +void JsSyncManagerObserver::OnBootstrapTokenUpdated( + const std::string& boostrap_token) { + if (!event_handler_.IsInitialized()) { + return; + } + DictionaryValue details; + details.SetString("bootstrapToken", "<redacted>"); + HandleJsEvent(FROM_HERE, "OnBootstrapTokenUpdated", JsEventDetails(&details)); +} + void JsSyncManagerObserver::OnEncryptedTypesChanged( syncable::ModelTypeSet encrypted_types, bool encrypt_everything) { diff --git a/chrome/browser/sync/js/js_sync_manager_observer.h b/chrome/browser/sync/js/js_sync_manager_observer.h index 44d2b5c..e3cddc7 100644 --- a/chrome/browser/sync/js/js_sync_manager_observer.h +++ b/chrome/browser/sync/js/js_sync_manager_observer.h @@ -39,7 +39,8 @@ class JsSyncManagerObserver : public sync_api::SyncManager::Observer { virtual void OnPassphraseRequired( sync_api::PassphraseRequiredReason reason, const sync_pb::EncryptedData& pending_keys) OVERRIDE; - virtual void OnPassphraseAccepted( + virtual void OnPassphraseAccepted() OVERRIDE; + virtual void OnBootstrapTokenUpdated( const std::string& bootstrap_token) OVERRIDE; virtual void OnEncryptedTypesChanged( syncable::ModelTypeSet encrypted_types, diff --git a/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc b/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc index be15219..1145458 100644 --- a/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc +++ b/chrome/browser/sync/js/js_sync_manager_observer_unittest.cc @@ -191,11 +191,11 @@ TEST_F(JsSyncManagerObserverTest, SensitiveNotifiations) { HasDetailsAsDictionary(redacted_token_details))); EXPECT_CALL(mock_js_event_handler_, HandleJsEvent( - "onPassphraseAccepted", + "OnBootstrapTokenUpdated", HasDetailsAsDictionary(redacted_bootstrap_token_details))); js_sync_manager_observer_.OnUpdatedToken("sensitive_token"); - js_sync_manager_observer_.OnPassphraseAccepted("sensitive_token"); + js_sync_manager_observer_.OnBootstrapTokenUpdated("sensitive_token"); PumpLoop(); } diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 111c72b..20d6572 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -810,7 +810,9 @@ void ProfileSyncService::OnPassphraseRequired( cached_passphrases_.gaia_passphrase.clear(); DVLOG(1) << "Attempting gaia passphrase."; // SetPassphrase will re-cache this passphrase if the syncer isn't ready. - SetPassphrase(gaia_passphrase, false); + SetPassphrase(gaia_passphrase, IMPLICIT, + (cached_passphrases_.user_provided_gaia ? + USER_PROVIDED : INTERNAL)); return; } @@ -821,7 +823,7 @@ void ProfileSyncService::OnPassphraseRequired( cached_passphrases_.explicit_passphrase.clear(); DVLOG(1) << "Attempting explicit passphrase."; // SetPassphrase will re-cache this passphrase if the syncer isn't ready. - SetPassphrase(explicit_passphrase, true); + SetPassphrase(explicit_passphrase, EXPLICIT, USER_PROVIDED); return; } @@ -1288,15 +1290,27 @@ void ProfileSyncService::DeactivateDataType(syncable::ModelType type) { } void ProfileSyncService::SetPassphrase(const std::string& passphrase, - bool is_explicit) { + PassphraseType type, + PassphraseSource source) { + DCHECK(source == USER_PROVIDED || type == IMPLICIT); if (ShouldPushChanges() || IsPassphraseRequired()) { - DVLOG(1) << "Setting " << (is_explicit ? "explicit" : "implicit"); - backend_->SetPassphrase(passphrase, is_explicit); + DVLOG(1) << "Setting " << (type == EXPLICIT ? "explicit" : "implicit") + << " passphrase."; + backend_->SetPassphrase(passphrase, + type == EXPLICIT, + source == USER_PROVIDED); } else { - if (is_explicit) { + if (type == EXPLICIT) { + NOTREACHED() << "SetPassphrase should only be called after the backend " + << "is initialized."; cached_passphrases_.explicit_passphrase = passphrase; } else { cached_passphrases_.gaia_passphrase = passphrase; + cached_passphrases_.user_provided_gaia = (source == USER_PROVIDED); + DVLOG(1) << "Caching " + << (cached_passphrases_.user_provided_gaia ? "user provided" : + "internal") + << " gaia passphrase."; } } } @@ -1437,13 +1451,12 @@ void ProfileSyncService::Observe(int type, // want to suppress start up anymore. sync_prefs_.SetStartSuppressed(false); - // We pass 'false' to SetPassphrase to denote that this is an implicit - // request and shouldn't override an explicit one. Thus, we either - // update the implicit passphrase (idempotent if the passphrase didn't - // actually change), or the user has an explicit passphrase set so this - // becomes a no-op. + // Because we specify IMPLICIT to SetPassphrase, we know it won't override + // an explicit one. Thus, we either update the implicit passphrase + // (idempotent if the passphrase didn't actually change), or the user has + // an explicit passphrase set so this becomes a no-op. if (!successful->password.empty()) - SetPassphrase(successful->password, false); + SetPassphrase(successful->password, IMPLICIT, INTERNAL); break; } case chrome::NOTIFICATION_TOKEN_REQUEST_FAILED: { diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index d9d8068..c05d70b 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -155,6 +155,16 @@ class ProfileSyncService : public browser_sync::SyncFrontend, MANUAL_START, }; + enum PassphraseType { + IMPLICIT, + EXPLICIT, + }; + + enum PassphraseSource { + INTERNAL, + USER_PROVIDED, + }; + // Default sync server URL. static const char* kSyncServerUrl; // Sync server URL for dev channel users @@ -448,12 +458,19 @@ class ProfileSyncService : public browser_sync::SyncFrontend, // This will check asynchronously whether the passphrase is valid and notify // ProfileSyncServiceObservers via the NotificationService when the outcome // is known. - // |is_explicit| is true if the call is in response to the user explicitly - // setting a passphrase as opposed to implicitly (from the users' perspective) - // using their Google Account password. An implicit SetPassphrase will - // *not* override an explicit passphrase set previously. + // |type| == EXPLICIT if the call is in response to the user setting a + // custom explicit passphrase as opposed to implicitly (from the users' + // perspective) using their Google Account password. Once an explicit + // passphrase is set, it can never be overwritten (not even by another + // explicit passphrase). + // |source| == USER_PROVIDED corresponds to the user having manually provided + // this passphrase. It should only be INTERNAL for passphrases intercepted + // from the Google Sign-in Success notification. Note that if the data is + // encrypted with an old Google Account password, the user may still have to + // provide an "implicit" passphrase. virtual void SetPassphrase(const std::string& passphrase, - bool is_explicit); + PassphraseType type, + PassphraseSource source); // Turns on encryption for all data. Callers must call OnUserChoseDatatypes() // after calling this to force the encryption to occur. @@ -672,6 +689,9 @@ class ProfileSyncService : public browser_sync::SyncFrontend, struct CachedPassphrases { std::string explicit_passphrase; std::string gaia_passphrase; + // This distinguishes from GAIA passphrases intercepted by the signin code + // (which will always be the most recent GAIA passphrase). + bool user_provided_gaia; }; CachedPassphrases cached_passphrases_; diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc index 7a999e7..59c039a 100644 --- a/chrome/browser/sync/profile_sync_service_harness.cc +++ b/chrome/browser/sync/profile_sync_service_harness.cc @@ -193,7 +193,9 @@ bool ProfileSyncServiceHarness::SetupSync( } // Set our implicit passphrase. - service_->SetPassphrase(password_, false); + service_->SetPassphrase(password_, + ProfileSyncService::IMPLICIT, + ProfileSyncService::USER_PROVIDED); // Wait for initial sync cycle to be completed. DCHECK_EQ(wait_state_, WAITING_FOR_INITIAL_SYNC); diff --git a/chrome/browser/sync/profile_sync_service_password_unittest.cc b/chrome/browser/sync/profile_sync_service_password_unittest.cc index 0ccfbba..0b2bbf0 100644 --- a/chrome/browser/sync/profile_sync_service_password_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_password_unittest.cc @@ -267,7 +267,9 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest { MessageLoop::current()->Run(); FlushLastDBTask(); - service_->SetPassphrase("foo", false); + service_->SetPassphrase("foo", + ProfileSyncService::IMPLICIT, + ProfileSyncService::INTERNAL); MessageLoop::current()->Run(); } } diff --git a/chrome/browser/sync/protocol/client_debug_info.proto b/chrome/browser/sync/protocol/client_debug_info.proto index af97839..6d67b72 100644 --- a/chrome/browser/sync/protocol/client_debug_info.proto +++ b/chrome/browser/sync/protocol/client_debug_info.proto @@ -38,6 +38,8 @@ message DebugEventInfo { ENCRYPTED_TYPES_CHANGED = 9; // Set of encrypted types has changed. ENCRYPTION_COMPLETE = 7; // Client has finished encrypting all data. ACTIONABLE_ERROR = 8; // Client received an actionable error. + BOOTSTRAP_TOKEN_UPDATED = 9; // A new cryptographer bootstrap token was + // generated. } optional EventType type = 1; optional SyncCycleCompletedEventInfo sync_cycle_completed_event_info = 2; diff --git a/chrome/browser/sync/sync_setup_flow.cc b/chrome/browser/sync/sync_setup_flow.cc index 3e2f1f2..c1abbee 100644 --- a/chrome/browser/sync/sync_setup_flow.cc +++ b/chrome/browser/sync/sync_setup_flow.cc @@ -453,7 +453,9 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) { // Caller passed a gaia passphrase. This is illegal if we are currently // using a secondary passphrase. DCHECK(!service_->IsUsingSecondaryPassphrase()); - service_->SetPassphrase(configuration.gaia_passphrase, false); + service_->SetPassphrase(configuration.gaia_passphrase, + ProfileSyncService::IMPLICIT, + ProfileSyncService::USER_PROVIDED); // Since the user entered the passphrase manually, set this flag so we can // report an error if the passphrase setting failed. user_tried_setting_passphrase_ = true; @@ -464,7 +466,9 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) { // as an attempt to encrypt the user's data using this new passphrase. if (configuration.set_secondary_passphrase && !configuration.secondary_passphrase.empty()) { - service_->SetPassphrase(configuration.secondary_passphrase, true); + service_->SetPassphrase(configuration.secondary_passphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); if (service_->IsUsingSecondaryPassphrase()) { user_tried_setting_passphrase_ = true; set_new_decryption_passphrase = true; @@ -494,7 +498,9 @@ void SyncSetupFlow::OnUserConfigured(const SyncConfiguration& configuration) { void SyncSetupFlow::OnPassphraseEntry(const std::string& passphrase) { Advance(SyncSetupWizard::SETTING_UP); - service_->SetPassphrase(passphrase, true); + service_->SetPassphrase(passphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); user_tried_setting_passphrase_ = true; } diff --git a/chrome/browser/sync/sync_setup_wizard_unittest.cc b/chrome/browser/sync/sync_setup_wizard_unittest.cc index 46b7cb0..5f0f98f 100644 --- a/chrome/browser/sync/sync_setup_wizard_unittest.cc +++ b/chrome/browser/sync/sync_setup_wizard_unittest.cc @@ -114,7 +114,8 @@ class ProfileSyncServiceForWizardTest : public ProfileSyncService { } virtual void SetPassphrase(const std::string& passphrase, - bool is_explicit) OVERRIDE { + PassphraseType type, + PassphraseSource source) OVERRIDE { passphrase_ = passphrase; } diff --git a/chrome/browser/sync/test/integration/passwords_helper.cc b/chrome/browser/sync/test/integration/passwords_helper.cc index 947ac47..f2cf8ab 100644 --- a/chrome/browser/sync/test/integration/passwords_helper.cc +++ b/chrome/browser/sync/test/integration/passwords_helper.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -107,7 +107,9 @@ void RemoveLogins(PasswordStore* store) { void SetPassphrase(int index, const std::string& passphrase) { test()->GetProfile(index)->GetProfileSyncService()->SetPassphrase( - passphrase, true); + passphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); } PasswordStore* GetPasswordStore(int index) { diff --git a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc index 450e6713..808b1a2 100644 --- a/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc +++ b/chrome/browser/sync/test/integration/two_client_bookmarks_sync_test.cc @@ -1804,7 +1804,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, // Set a passphrase and enable encryption on Client 0. Client 1 will not // understand the bookmark updates. - GetClient(0)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(0)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); ASSERT_TRUE(EnableEncryption(0, syncable::BOOKMARKS)); ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); @@ -1821,7 +1824,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientBookmarksSyncTest, EXPECT_FALSE(AllModelsMatch()); // Set the passphrase. Everything should resolve. - GetClient(1)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(1)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0))); EXPECT_TRUE(AllModelsMatchVerifier()); diff --git a/chrome/browser/sync/test/integration/two_client_passwords_sync_test.cc b/chrome/browser/sync/test/integration/two_client_passwords_sync_test.cc index 9b62e2f..e47084d 100644 --- a/chrome/browser/sync/test/integration/two_client_passwords_sync_test.cc +++ b/chrome/browser/sync/test/integration/two_client_passwords_sync_test.cc @@ -243,22 +243,6 @@ IN_PROC_BROWSER_TEST_F(TwoClientPasswordsSyncTest, ASSERT_EQ(browser_sync::GROUP_PASSWORD, routes[syncable::PASSWORDS]); } -IN_PROC_BROWSER_TEST_F(TwoClientPasswordsSyncTest, SetPassphraseTwice) { - ASSERT_TRUE(SetupSync()) << "SetupSync() failed."; - - SetPassphrase(0, kValidPassphrase); - ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); - ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); - - SetPassphrase(1, kValidPassphrase); - ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); - ASSERT_TRUE(GetClient(1)->AwaitFullSyncCompletion("Set passphrase.")); - - SetPassphrase(1, kValidPassphrase); - ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); - ASSERT_TRUE(GetClient(1)->AwaitFullSyncCompletion("Set passphrase again.")); -} - IN_PROC_BROWSER_TEST_F(TwoClientPasswordsSyncTest, SetDifferentPassphraseAndThenSetupSync) { ASSERT_TRUE(SetupClients()) << "SetupClients() failed."; diff --git a/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc b/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc index d5c9a23..0134830 100644 --- a/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc +++ b/chrome/browser/sync/test/integration/two_client_sessions_sync_test.cc @@ -160,7 +160,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, client0_windows.GetMutable())); ASSERT_TRUE(EnableEncryption(0, syncable::SESSIONS)); - GetClient(0)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(0)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired()); @@ -172,7 +175,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, GetClient(1)->GetLastSessionSnapshot()-> num_conflicting_updates); // The encrypted nodes. - GetClient(1)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(1)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(1)->WaitForTypeEncryption(syncable::SESSIONS)); @@ -204,7 +210,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, ASSERT_TRUE(CheckInitialState(1)); ASSERT_TRUE(EnableEncryption(0, syncable::SESSIONS)); - GetClient(0)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(0)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired()); @@ -226,7 +235,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, GetClient(1)->GetLastSessionSnapshot()-> num_conflicting_updates); // The encrypted nodes. - GetClient(1)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(1)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(1)->WaitForTypeEncryption(syncable::SESSIONS)); @@ -250,7 +262,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, ASSERT_TRUE(CheckInitialState(1)); ASSERT_TRUE(EnableEncryption(0, syncable::SESSIONS)); - GetClient(0)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(0)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired()); @@ -270,7 +285,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, // At this point we enter the passphrase, triggering a resync, in which the // local changes of client 1 get sent to client 0. - GetClient(1)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(1)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(1)->WaitForTypeEncryption(syncable::SESSIONS)); ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0))); @@ -309,7 +327,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, // Turn encryption on client 0. Client 1's foreign will be encrypted with the // new passphrase and synced back. It will be unable to decrypt it yet. ASSERT_TRUE(EnableEncryption(0, syncable::SESSIONS)); - GetClient(0)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(0)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); ASSERT_TRUE(AwaitQuiescence()); ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired()); @@ -323,7 +344,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, num_conflicting_updates); // The encrypted nodes. // At this point we enter the passphrase, triggering a resync. - GetClient(1)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(1)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(1)->WaitForTypeEncryption(syncable::SESSIONS)); @@ -354,7 +378,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, ASSERT_TRUE(CheckInitialState(0)); ASSERT_TRUE(CheckInitialState(1)); - GetClient(0)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(0)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(0)->AwaitPassphraseAccepted()); ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1))); ASSERT_TRUE(GetClient(1)->AwaitPassphraseRequired()); @@ -375,7 +402,10 @@ IN_PROC_BROWSER_TEST_F(TwoClientSessionsSyncTest, GetClient(1)->GetLastSessionSnapshot()-> num_conflicting_updates); // The encrypted nodes. - GetClient(1)->service()->SetPassphrase(kValidPassphrase, true); + GetClient(1)->service()->SetPassphrase( + kValidPassphrase, + ProfileSyncService::EXPLICIT, + ProfileSyncService::USER_PROVIDED); ASSERT_TRUE(GetClient(1)->AwaitPassphraseAccepted()); ASSERT_FALSE(GetClient(1)->service()->IsPassphraseRequired()); ASSERT_TRUE(GetClient(1)->WaitForTypeEncryption(syncable::SESSIONS)); diff --git a/chrome/browser/sync/util/cryptographer.cc b/chrome/browser/sync/util/cryptographer.cc index de83652..26fbc55 100644 --- a/chrome/browser/sync/util/cryptographer.cc +++ b/chrome/browser/sync/util/cryptographer.cc @@ -131,8 +131,6 @@ bool Cryptographer::GetKeys(sync_pb::EncryptedData* encrypted) const { } bool Cryptographer::AddKey(const KeyParams& params) { - DCHECK(NULL == pending_keys_.get()); - // Create the new Nigori and make it the default encryptor. scoped_ptr<Nigori> nigori(new Nigori); if (!nigori->InitByDerivation(params.hostname, @@ -144,6 +142,15 @@ bool Cryptographer::AddKey(const KeyParams& params) { return AddKeyImpl(nigori.release()); } +bool Cryptographer::AddKeyFromBootstrapToken( + const std::string restored_bootstrap_token) { + // Create the new Nigori and make it the default encryptor. + scoped_ptr<Nigori> nigori(UnpackBootstrapToken(restored_bootstrap_token)); + if (!nigori.get()) + return false; + return AddKeyImpl(nigori.release()); +} + bool Cryptographer::AddKeyImpl(Nigori* initialized_nigori) { scoped_ptr<Nigori> nigori(initialized_nigori); std::string name; diff --git a/chrome/browser/sync/util/cryptographer.h b/chrome/browser/sync/util/cryptographer.h index 2c319ab..d5e98f7 100644 --- a/chrome/browser/sync/util/cryptographer.h +++ b/chrome/browser/sync/util/cryptographer.h @@ -131,6 +131,11 @@ class Cryptographer { // Encrypt. bool AddKey(const KeyParams& params); + // Same as AddKey(..), but builds the new Nigori from a previously persisted + // bootstrap token. This can be useful when consuming a bootstrap token + // with a cryptographer that has already been initialized. + bool AddKeyFromBootstrapToken(const std::string restored_bootstrap_token); + // Decrypts |encrypted| and uses its contents to initialize Nigori instances. // Returns true unless decryption of |encrypted| fails. The caller is // responsible for checking that CanDecrypt(encrypted) == true. |