[crypto] Add scrypt key derivation to SymmetricKey

Add a method for key derivation using the scrypt algorithm to crypto::SymmetricKey. Rename the pre-existing method for key derivation (which uses PBKDF2) to clearly disambiguate between the two. TBRing because all the changes to files not under crypto/ are a trivial library method name change. TBR=mnissler@chromium.org,droger@chromium.org,alemate@chromium.org,cfroussios@chromium.org,tbarzic@chromium.org Bug: 875931 Change-Id: Iaa0b2bb0fc3ae2481733072363718cffd8811b97 Reviewed-on: https://chromium-review.googlesource.com/1181881 Commit-Queue: David Davidović <davidovic@google.com> Reviewed-by: David Benjamin <davidben@chromium.org> Reviewed-by: vitaliii <vitaliii@chromium.org> Cr-Commit-Position: refs/heads/master@{#586247}
qweri0p · Aug 27, 2018 · f8cd6a0 · f8cd6a0
1 parent c7ddd4a
commit f8cd6a0
Show file tree

Hide file tree

Showing 16 changed files with 237 additions and 166 deletions.
diff --git a/chrome/browser/chromeos/settings/token_encryptor.cc b/chrome/browser/chromeos/settings/token_encryptor.cc
@@ -70,7 +70,7 @@ std::string CryptohomeTokenEncryptor::DecryptWithSystemSalt(
 std::unique_ptr<crypto::SymmetricKey> CryptohomeTokenEncryptor::PassphraseToKey(
     const std::string& passphrase,
     const std::string& salt) {
-  return crypto::SymmetricKey::DeriveKeyFromPassword(
+  return crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
       crypto::SymmetricKey::AES, passphrase, salt, 1000, 256);
 }
 

diff --git a/chrome/browser/signin/local_auth.cc b/chrome/browser/signin/local_auth.cc
@@ -105,7 +105,7 @@ std::string CreateSecurePasswordHash(const std::string& salt,
 
   // Library call to create secure password hash as SymmetricKey (uses PBKDF2).
   std::unique_ptr<crypto::SymmetricKey> password_key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, password, salt, encoding.iteration_count,
           encoding.hash_bits));
   std::string password_hash = password_key->key();

diff --git a/chromeos/login/auth/authpolicy_login_helper.cc b/chromeos/login/auth/authpolicy_login_helper.cc
@@ -88,7 +88,7 @@ std::string DoDecrypt(const std::string& encrypted_data,
   const size_t kHmacKeySize = 32;
   const size_t kKeySize = kAesKeySize + kAesIvSize + kHmacKeySize;
   std::unique_ptr<crypto::SymmetricKey> key =
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::HMAC_SHA1, password, salt, 10000, kKeySize * 8);
   if (!key) {
     LOG(ERROR) << error_msg;

diff --git a/chromeos/login/auth/key.cc b/chromeos/login/auth/key.cc
@@ -85,7 +85,7 @@ void Key::Transform(KeyType target_key_type, const std::string& salt) {
     }
     case KEY_TYPE_SALTED_PBKDF2_AES256_1234: {
       std::unique_ptr<crypto::SymmetricKey> key(
-          crypto::SymmetricKey::DeriveKeyFromPassword(
+          crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
               crypto::SymmetricKey::AES, secret_, salt, kNumIterations,
               kKeySizeInBits));
       base::Base64Encode(key->key(), &secret_);

diff --git a/chromeos/network/onc/onc_utils.cc b/chromeos/network/onc/onc_utils.cc
@@ -721,9 +721,9 @@ std::unique_ptr<base::Value> Decrypt(const std::string& passphrase,
   }
 
   std::unique_ptr<crypto::SymmetricKey> key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(crypto::SymmetricKey::AES,
-                                                  passphrase, salt, iterations,
-                                                  kKeySizeInBits));
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
+          crypto::SymmetricKey::AES, passphrase, salt, iterations,
+          kKeySizeInBits));
 
   if (!base::Base64Decode(initial_vector, &initial_vector)) {
     NET_LOG(ERROR) << kUnableToDecode;

diff --git a/components/os_crypt/os_crypt_linux.cc b/components/os_crypt/os_crypt_linux.cc
@@ -121,7 +121,7 @@ std::unique_ptr<crypto::SymmetricKey> GetEncryptionKey(Version version) {
 
   // Create an encryption key from our password and salt.
   std::unique_ptr<crypto::SymmetricKey> encryption_key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, *password, salt, kEncryptionIterations,
           kDerivedKeySizeInBits));
   DCHECK(encryption_key);

diff --git a/components/os_crypt/os_crypt_mac.mm b/components/os_crypt/os_crypt_mac.mm
@@ -87,10 +87,11 @@
 
   // Create an encryption key from our password and salt. The key is
   // intentionally leaked.
-  cached_encryption_key = crypto::SymmetricKey::DeriveKeyFromPassword(
-                              crypto::SymmetricKey::AES, password, salt,
-                              kEncryptionIterations, kDerivedKeySizeInBits)
-                              .release();
+  cached_encryption_key =
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
+          crypto::SymmetricKey::AES, password, salt, kEncryptionIterations,
+          kDerivedKeySizeInBits)
+          .release();
   ANNOTATE_LEAKING_OBJECT_PTR(cached_encryption_key);
   DCHECK(cached_encryption_key);
   return cached_encryption_key;

diff --git a/components/os_crypt/os_crypt_posix.cc b/components/os_crypt/os_crypt_posix.cc
@@ -46,11 +46,9 @@ crypto::SymmetricKey* GetEncryptionKey() {
 
   // Create an encryption key from our password and salt.
   std::unique_ptr<crypto::SymmetricKey> encryption_key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(crypto::SymmetricKey::AES,
-                                                  password,
-                                                  salt,
-                                                  kEncryptionIterations,
-                                                  kDerivedKeySizeInBits));
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
+          crypto::SymmetricKey::AES, password, salt, kEncryptionIterations,
+          kDerivedKeySizeInBits));
   DCHECK(encryption_key.get());
 
   return encryption_key.release();

diff --git a/components/sync/base/nigori.cc b/components/sync/base/nigori.cc
@@ -69,25 +69,26 @@ bool Nigori::InitByDerivation(const std::string& hostname,
   salt_password << username << hostname;
 
   // Suser = PBKDF2(Username || Servername, "saltsalt", Nsalt, 8)
-  std::unique_ptr<SymmetricKey> user_salt(SymmetricKey::DeriveKeyFromPassword(
-      SymmetricKey::HMAC_SHA1, salt_password.str(), kSaltSalt, kSaltIterations,
-      kSaltKeySizeInBits));
+  std::unique_ptr<SymmetricKey> user_salt(
+      SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
+          SymmetricKey::HMAC_SHA1, salt_password.str(), kSaltSalt,
+          kSaltIterations, kSaltKeySizeInBits));
   DCHECK(user_salt);
 
   // Kuser = PBKDF2(P, Suser, Nuser, 16)
-  user_key_ = SymmetricKey::DeriveKeyFromPassword(
+  user_key_ = SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
       SymmetricKey::AES, password, user_salt->key(), kUserIterations,
       kDerivedKeySizeInBits);
   DCHECK(user_key_);
 
   // Kenc = PBKDF2(P, Suser, Nenc, 16)
-  encryption_key_ = SymmetricKey::DeriveKeyFromPassword(
+  encryption_key_ = SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
       SymmetricKey::AES, password, user_salt->key(), kEncryptionIterations,
       kDerivedKeySizeInBits);
   DCHECK(encryption_key_);
 
   // Kmac = PBKDF2(P, Suser, Nmac, 16)
-  mac_key_ = SymmetricKey::DeriveKeyFromPassword(
+  mac_key_ = SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
       SymmetricKey::HMAC_SHA1, password, user_salt->key(), kSigningIterations,
       kDerivedKeySizeInBits);
   DCHECK(mac_key_);

diff --git a/crypto/encryptor_unittest.cc b/crypto/encryptor_unittest.cc
@@ -16,7 +16,7 @@
 
 TEST(EncryptorTest, EncryptDecrypt) {
   std::unique_ptr<crypto::SymmetricKey> key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, "password", "saltiest", 1000, 256));
   EXPECT_TRUE(key.get());
 
@@ -40,27 +40,27 @@ TEST(EncryptorTest, EncryptDecrypt) {
 
 TEST(EncryptorTest, DecryptWrongKey) {
   std::unique_ptr<crypto::SymmetricKey> key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, "password", "saltiest", 1000, 256));
   EXPECT_TRUE(key.get());
 
   // A wrong key that can be detected by implementations that validate every
   // byte in the padding.
   std::unique_ptr<crypto::SymmetricKey> wrong_key(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, "wrongword", "sweetest", 1000, 256));
   EXPECT_TRUE(wrong_key.get());
 
   // A wrong key that can't be detected by any implementation.  The password
   // "wrongword;" would also work.
   std::unique_ptr<crypto::SymmetricKey> wrong_key2(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, "wrongword+", "sweetest", 1000, 256));
   EXPECT_TRUE(wrong_key2.get());
 
   // A wrong key that can be detected by all implementations.
   std::unique_ptr<crypto::SymmetricKey> wrong_key3(
-      crypto::SymmetricKey::DeriveKeyFromPassword(
+      crypto::SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
           crypto::SymmetricKey::AES, "wrongwordx", "sweetest", 1000, 256));
   EXPECT_TRUE(wrong_key3.get());
 

diff --git a/crypto/symmetric_key.cc b/crypto/symmetric_key.cc
@@ -18,6 +18,26 @@
 
 namespace crypto {
 
+namespace {
+
+bool CheckDerivationParameters(SymmetricKey::Algorithm algorithm,
+                               size_t key_size_in_bits) {
+  switch (algorithm) {
+    case SymmetricKey::AES:
+      // Whitelist supported key sizes to avoid accidentally relying on
+      // algorithms available in NSS but not BoringSSL and vice
+      // versa. Note that BoringSSL does not support AES-192.
+      return key_size_in_bits == 128 || key_size_in_bits == 256;
+    case SymmetricKey::HMAC_SHA1:
+      return key_size_in_bits % 8 == 0 && key_size_in_bits != 0;
+  }
+
+  NOTREACHED();
+  return false;
+}
+
+}  // namespace
+
 SymmetricKey::~SymmetricKey() {
   std::fill(key_.begin(), key_.end(), '\0');  // Zero out the confidential key.
 }
@@ -50,32 +70,22 @@ std::unique_ptr<SymmetricKey> SymmetricKey::GenerateRandomKey(
 }
 
 // static
-std::unique_ptr<SymmetricKey> SymmetricKey::DeriveKeyFromPassword(
+std::unique_ptr<SymmetricKey> SymmetricKey::DeriveKeyFromPasswordUsingPbkdf2(
     Algorithm algorithm,
     const std::string& password,
     const std::string& salt,
     size_t iterations,
     size_t key_size_in_bits) {
-  DCHECK(algorithm == AES || algorithm == HMAC_SHA1);
-
-  if (algorithm == AES) {
-    // Whitelist supported key sizes to avoid accidentaly relying on
-    // algorithms available in NSS but not BoringSSL and vice
-    // versa. Note that BoringSSL does not support AES-192.
-    if (key_size_in_bits != 128 && key_size_in_bits != 256)
-      return nullptr;
-  }
+  if (!CheckDerivationParameters(algorithm, key_size_in_bits))
+    return nullptr;
 
   size_t key_size_in_bytes = key_size_in_bits / 8;
-  DCHECK_EQ(key_size_in_bits, key_size_in_bytes * 8);
-
-  if (key_size_in_bytes == 0)
-    return nullptr;
 
   OpenSSLErrStackTracer err_tracer(FROM_HERE);
   std::unique_ptr<SymmetricKey> key(new SymmetricKey);
   uint8_t* key_data = reinterpret_cast<uint8_t*>(
       base::WriteInto(&key->key_, key_size_in_bytes + 1));
+
   int rv = PKCS5_PBKDF2_HMAC_SHA1(
       password.data(), password.length(),
       reinterpret_cast<const uint8_t*>(salt.data()), salt.length(),
@@ -84,6 +94,34 @@ std::unique_ptr<SymmetricKey> SymmetricKey::DeriveKeyFromPassword(
   return rv == 1 ? std::move(key) : nullptr;
 }
 
+// static
+std::unique_ptr<SymmetricKey> SymmetricKey::DeriveKeyFromPasswordUsingScrypt(
+    Algorithm algorithm,
+    const std::string& password,
+    const std::string& salt,
+    size_t cost_parameter,
+    size_t block_size,
+    size_t parallelization_parameter,
+    size_t max_memory_bytes,
+    size_t key_size_in_bits) {
+  if (!CheckDerivationParameters(algorithm, key_size_in_bits))
+    return nullptr;
+
+  size_t key_size_in_bytes = key_size_in_bits / 8;
+
+  OpenSSLErrStackTracer err_tracer(FROM_HERE);
+  std::unique_ptr<SymmetricKey> key(new SymmetricKey);
+  uint8_t* key_data = reinterpret_cast<uint8_t*>(
+      base::WriteInto(&key->key_, key_size_in_bytes + 1));
+
+  int rv = EVP_PBE_scrypt(password.data(), password.length(),
+                          reinterpret_cast<const uint8_t*>(salt.data()),
+                          salt.length(), cost_parameter, block_size,
+                          parallelization_parameter, max_memory_bytes, key_data,
+                          key_size_in_bytes);
+  return rv == 1 ? std::move(key) : nullptr;
+}
+
 // static
 std::unique_ptr<SymmetricKey> SymmetricKey::Import(Algorithm algorithm,
                                                    const std::string& raw_key) {

diff --git a/crypto/symmetric_key.h b/crypto/symmetric_key.h
@@ -41,17 +41,34 @@ class CRYPTO_EXPORT SymmetricKey {
   // used to derive the key from the password. |key_size_in_bits| must be a
   // multiple of 8. The caller is responsible for deleting the returned
   // SymmetricKey.
-  static std::unique_ptr<SymmetricKey> DeriveKeyFromPassword(
+  static std::unique_ptr<SymmetricKey> DeriveKeyFromPasswordUsingPbkdf2(
       Algorithm algorithm,
       const std::string& password,
       const std::string& salt,
       size_t iterations,
       size_t key_size_in_bits);
 
+  // Derives a key from the supplied password and salt using scrypt, suitable
+  // for use with specified |algorithm|. Note |algorithm| is not the algorithm
+  // used to derive the key from the password. |cost_parameter|, |block_size|,
+  // and |parallelization_parameter| correspond to the parameters |N|, |r|, and
+  // |p| from the scrypt specification (see RFC 7914). |key_size_in_bits| must
+  // be a multiple of 8. The caller is responsible for deleting the returned
+  // SymmetricKey.
+  static std::unique_ptr<SymmetricKey> DeriveKeyFromPasswordUsingScrypt(
+      Algorithm algorithm,
+      const std::string& password,
+      const std::string& salt,
+      size_t cost_parameter,
+      size_t block_size,
+      size_t parallelization_parameter,
+      size_t max_memory_bytes,
+      size_t key_size_in_bits);
+
   // Imports an array of key bytes in |raw_key|. This key may have been
-  // generated by GenerateRandomKey or DeriveKeyFromPassword and exported with
-  // key(). The key must be of suitable size for use with |algorithm|.
-  // The caller owns the returned SymmetricKey.
+  // generated by GenerateRandomKey or DeriveKeyFromPassword{Pbkdf2,Scrypt} and
+  // exported with key(). The key must be of suitable size for use with
+  // |algorithm|. The caller owns the returned SymmetricKey.
   static std::unique_ptr<SymmetricKey> Import(Algorithm algorithm,
                                               const std::string& raw_key);