[Sync] Move keystore key handling to SyncEncryptionHandlerImpl

sync/util/cryptographer.h

Index: sync/util/cryptographer.h
diff --git a/sync/util/cryptographer.h b/sync/util/cryptographer.h
index 77a164f75a7e508237c4320b04e336ca1dbcf9a9..a9351bc5e6235761460a89aa7cf8681e0160abee 100644
--- a/sync/util/cryptographer.h
+++ b/sync/util/cryptographer.h
@@ -63,10 +63,6 @@ class Cryptographer {
   // never call Bootstrap at all.
   void Bootstrap(const std::string& restored_bootstrap_token);
 
-  // Bootstrap the keystore key.
-  void BootstrapKeystoreKey(
-      const std::string& restored_keystore_bootstrap_token);
-
   // Returns whether we can decrypt |encrypted| using the keys we currently know
   // about.
   bool CanDecrypt(const sync_pb::EncryptedData& encrypted) const;
@@ -135,11 +131,14 @@ class Cryptographer {
   // correspond to a nigori that has already been installed into the keybag.
   void SetDefaultKey(const std::string& key_name);
 
-  bool is_initialized() const { return !nigoris_.empty() && default_nigori_; }
+  bool is_initialized() const {
+    return !nigoris_.empty() && default_nigori_.get();
+  }
 
   // Returns whether this Cryptographer is ready to encrypt and decrypt data.
-  bool is_ready() const { return is_initialized() &&
-                          has_pending_keys() == false; }
+  bool is_ready() const {
+    return is_initialized() && has_pending_keys() == false;

[akalin, 2012/08/27 22:06:55]

has_pending_keys() == false -> !has_pending_keys()

[Nicolas Zea, 2012/08/28 01:18:50]

Done.

+  }
 
   // Returns whether there is a pending set of keys that needs to be decrypted.
   bool has_pending_keys() const { return NULL != pending_keys_.get(); }
@@ -149,18 +148,6 @@ class Cryptographer {
   // can't be created (i.e. if this Cryptograhper doesn't have valid keys).
   bool GetBootstrapToken(std::string* token) const;
 
-  // Obtain the bootstrap token based on the keystore encryption key.
-  bool GetKeystoreKeyBootstrapToken(std::string* token) const;
-
-  // Set the keystore-derived nigori from the provided key.
-  // Returns true if we succesfully create the keystore derived nigori from the
-  // provided key, false otherwise.
-  bool SetKeystoreKey(const std::string& keystore_key);
-
-  // Returns true if we currently have a keystore-derived nigori, false
-  // otherwise.
-  bool HasKeystoreKey() const;
-
   Encryptor* encryptor() const { return encryptor_; }
 
  private:
@@ -173,9 +160,8 @@ class Cryptographer {
   // Does not update the default nigori.
   void InstallKeyBag(const sync_pb::NigoriKeyBag& bag);
 
-  // Helper method to add a nigori as either the new default nigori or the new
-  // keystore nigori.
-  bool AddKeyImpl(Nigori* nigori, bool is_keystore_key);
+  // Helper method to add a nigori as the default key.
+  bool AddKeyImpl(scoped_ptr<Nigori> nigori);
 
   // Functions to serialize + encrypt a Nigori object in an opaque format for
   // persistence by sync infrastructure.
@@ -184,9 +170,12 @@ class Cryptographer {
 
   Encryptor* const encryptor_;
 
-  NigoriMap nigoris_;  // The Nigoris we know about, mapped by key name.
-  NigoriMap::value_type* default_nigori_;  // The Nigori used for encryption.
-  NigoriMap::value_type* keystore_nigori_; // Nigori generated from keystore.
+  // The Nigoris we know about, mapped by key name.
+  NigoriMap nigoris_;
+  // The Nigori used for encryption.
+  NigoriMap::value_type::second_type default_nigori_;

[akalin, 2012/08/27 22:06:55]

i think it's clearer to just specify linked_ptr<const Nigori> as the type here. 

[Nicolas Zea, 2012/08/28 01:18:50]

Done.

+  // The key name associated with |default_nigori_|.

[akalin, 2012/08/27 22:06:55]

Maybe I'm just OCD, but I prefer default_nigori_name_ to come first, since it's
the key.

[Nicolas Zea, 2012/08/28 01:18:50]

Done.

+  std::string default_nigori_name_;

[akalin, 2012/08/27 22:06:55]

Add a comment saying that the pair (default_nigori_name_, default_nigori_) must
exist in nigoris_.  But that the worst case if that invariant is broken is that
we encrypt with an old nigori.

Also, we cache |default_nigori_| for speed, right?  May want to mention that in
the comment.

[Nicolas Zea, 2012/08/28 01:18:50]

Actually, I realize the worst case is we encrypt with a nigori and set an
improper name (which will never be decryptable). So I'm making the condition
check in Encrypt non-debug-only.

 
   scoped_ptr<sync_pb::EncryptedData> pending_keys_;