Repository: randombit/botan
Pull request #2720 (2021-04-19 23:48:12)

Run scrypt and bcrypt-pbkdf through PasswordHash
The old top level fns like scrypt are now deprecated, and we can now mark the algorithm headers as future-internal.


Patch:
diff --git a/src/cli/pbkdf.cpp b/src/cli/pbkdf.cpp
index d17c49220..079e121e7 100644
--- a/src/cli/pbkdf.cpp
+++ b/src/cli/pbkdf.cpp
@@ -6,14 +6,14 @@
 
 #include "cli.h"
 
-#if defined(BOTAN_HAS_PBKDF)
+#if defined(BOTAN_HAS_PASSWORD_HASHING)
    #include <botan/pwdhash.h>
    #include <botan/internal/os_utils.h>
 #endif
 
 namespace Botan_CLI {
 
-#if defined(BOTAN_HAS_PBKDF)
+#if defined(BOTAN_HAS_PASSWORD_HASHING)
 
 class PBKDF_Tune final : public Command
    {
diff --git a/src/cli/speed.cpp b/src/cli/speed.cpp
index 500c5aea5..cc9cfb09a 100644
--- a/src/cli/speed.cpp
+++ b/src/cli/speed.cpp
@@ -119,10 +119,6 @@
    #include <botan/newhope.h>
 #endif
 
-#if defined(BOTAN_HAS_SCRYPT)
-   #include <botan/scrypt.h>
-#endif
-
 #if defined(BOTAN_HAS_BCRYPT)
    #include <botan/bcrypt.h>
 #endif
@@ -131,6 +127,10 @@
    #include <botan/passhash9.h>
 #endif
 
+#if defined(BOTAN_HAS_PASSWORD_HASHING)
+  #include <botan/pwdhash.h>
+#endif
+
 namespace Botan_CLI {
 
 using Botan::Timer;
@@ -2213,17 +2213,20 @@ class Speed final : public Command
       void bench_scrypt(const std::string& /*provider*/,
                         std::chrono::milliseconds msec)
          {
+         auto pwdhash_fam = Botan::PasswordHashFamily::create_or_throw("Scrypt");
 
          for(size_t N : { 8192, 16384, 32768, 65536 })
             {
             for(size_t r : { 1, 8, 16 })
                {
-               for(size_t p : { 1, 4 })
+               for(size_t p : { 1 })
                   {
+                  auto pwdhash = pwdhash_fam->from_params(N, r, p);
+
                   auto scrypt_timer = make_timer(
                      "scrypt-" + std::to_string(N) + "-" +
                      std::to_string(r) + "-" + std::to_string(p) +
-                     " (" + std::to_string(Botan::scrypt_memory_usage(N, r, p) / (1024*1024)) + " MiB)");
+                     " (" + std::to_string(pwdhash->total_memory_usage() / (1024*1024)) + " MiB)");
 
                   uint8_t out[64];
                   uint8_t salt[8];
@@ -2232,8 +2235,11 @@ class Speed final : public Command
                   while(scrypt_timer->under(msec))
                      {
                      scrypt_timer->run([&] {
-                        Botan::scrypt(out, sizeof(out), "password",
-                                      salt, sizeof(salt), N, r, p);
+                        pwdhash->derive_key(out, sizeof(out),
+                                            "password", 8,
+                                            salt, sizeof(salt));
+
+                        Botan::copy_mem(salt, out, 8);
                         });
                      }
 
diff --git a/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp b/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp
index 13fc12e7d..da0dd791b 100644
--- a/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp
+++ b/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp
@@ -12,14 +12,10 @@
 
 namespace Botan {
 
-void Bcrypt_PBKDF::derive_key(uint8_t output[], size_t output_len,
-                              const char* password, size_t password_len,
-                              const uint8_t salt[], size_t salt_len) const
+Bcrypt_PBKDF::Bcrypt_PBKDF(size_t iterations) :
+   m_iterations(iterations)
    {
-   bcrypt_pbkdf(output, output_len,
-                password, password_len,
-                salt, salt_len,
-                m_iterations);
+   BOTAN_ARG_CHECK(m_iterations > 0, "Invalid Bcrypt-PBKDF iterations");
    }
 
 std::string Bcrypt_PBKDF::to_string() const
@@ -46,9 +42,13 @@ std::unique_ptr<PasswordHash> Bcrypt_PBKDF_Family::tune(size_t output_length,
 
    const size_t starting_iter = 2;
 
+   auto pwhash = this->from_iterations(starting_iter);
+
    timer.run_until_elapsed(tune_time, [&]() {
       uint8_t output[32] = { 0 };
-      bcrypt_pbkdf(output, sizeof(output), "test", 4, nullptr, 0, starting_iter);
+      pwhash->derive_key(output, sizeof(output),
+                         "test", 4,
+                         nullptr, 0);
       });
 
    if(timer.events() < blocks || timer.value() == 0)
@@ -127,13 +127,10 @@ void bcrypt_round(Blowfish& blowfish,
 
 }
 
-void bcrypt_pbkdf(uint8_t output[], size_t output_len,
-                  const char* pass, size_t pass_len,
-                  const uint8_t salt[], size_t salt_len,
-                  size_t rounds)
+void Bcrypt_PBKDF::derive_key(uint8_t output[], size_t output_len,
+                              const char* password, size_t password_len,
+                              const uint8_t salt[], size_t salt_len) const
    {
-   BOTAN_ARG_CHECK(rounds >= 1, "Invalid rounds for Bcrypt PBKDF");
-
    // No output desired, so we are all done already...
    if(output_len == 0)
       return;
@@ -143,8 +140,9 @@ void bcrypt_pbkdf(uint8_t output[], size_t output_len,
    const size_t BCRYPT_BLOCK_SIZE = 32;
    const size_t blocks = (output_len + BCRYPT_BLOCK_SIZE - 1) / BCRYPT_BLOCK_SIZE;
 
-   std::unique_ptr<HashFunction> sha512 = HashFunction::create_or_throw("SHA-512");
-   const secure_vector<uint8_t> pass_hash = sha512->process(reinterpret_cast<const uint8_t*>(pass), pass_len);
+   auto sha512 = HashFunction::create_or_throw("SHA-512");
+   const auto pass_hash =
+      sha512->process(reinterpret_cast<const uint8_t*>(password), password_len);
 
    secure_vector<uint8_t> salt_hash(sha512->output_length());
 
@@ -162,7 +160,7 @@ void bcrypt_pbkdf(uint8_t output[], size_t output_len,
 
       bcrypt_round(blowfish, pass_hash, salt_hash, out, tmp);
 
-      for(size_t r = 1; r != rounds; ++r)
+      for(size_t r = 1; r < m_iterations; ++r)
          {
          // Next salt is H(prev_output)
          sha512->update(tmp);
diff --git a/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.h b/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.h
index ab5e62288..ce6a3c718 100644
--- a/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.h
+++ b/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.h
@@ -9,7 +9,7 @@
 
 #include <botan/pwdhash.h>
 
-//BOTAN_FUTURE_INTERNAL_HEADER(bcrypt_pbkdf.h)
+BOTAN_FUTURE_INTERNAL_HEADER(bcrypt_pbkdf.h)
 
 namespace Botan {
 
@@ -19,7 +19,7 @@ namespace Botan {
 class BOTAN_PUBLIC_API(2,11) Bcrypt_PBKDF final : public PasswordHash
    {
    public:
-      Bcrypt_PBKDF(size_t iterations) : m_iterations(iterations) {}
+      Bcrypt_PBKDF(size_t iterations);
 
       Bcrypt_PBKDF(const Bcrypt_PBKDF& other) = default;
       Bcrypt_PBKDF& operator=(const Bcrypt_PBKDF&) = default;
@@ -67,10 +67,18 @@ class BOTAN_PUBLIC_API(2,11) Bcrypt_PBKDF_Family final : public PasswordHashFami
 /**
 * Bcrypt PBKDF compatible with OpenBSD bcrypt_pbkdf
 */
-void BOTAN_UNSTABLE_API bcrypt_pbkdf(uint8_t output[], size_t output_len,
-                                     const char* pass, size_t pass_len,
-                                     const uint8_t salt[], size_t salt_len,
-                                     size_t rounds);
+BOTAN_DEPRECATED("Use PasswordHashFamily+PasswordHash")
+inline void bcrypt_pbkdf(uint8_t output[], size_t output_len,
+                         const char* password, size_t password_len,
+                         const uint8_t salt[], size_t salt_len,
+                         size_t rounds)
+   {
+   auto pwdhash_fam = PasswordHashFamily::create_or_throw("Bcrypt-PBKDF");
+   auto pwdhash = pwdhash_fam->from_params(rounds);
+   pwdhash->derive_key(output, output_len,
+                       password, password_len,
+                       salt, salt_len);
+   }
 
 }
 
diff --git a/src/lib/pbkdf/info.txt b/src/lib/pbkdf/info.txt
index 650414f41..d9fd59304 100644
--- a/src/lib/pbkdf/info.txt
+++ b/src/lib/pbkdf/info.txt
@@ -1,5 +1,6 @@
 <defines>
 PBKDF -> 20180902
+PASSWORD_HASHING -> 20210419
 </defines>
 
 <requires>
diff --git a/src/lib/pbkdf/scrypt/scrypt.cpp b/src/lib/pbkdf/scrypt/scrypt.cpp
index 63fa61601..3c3eba95e 100644
--- a/src/lib/pbkdf/scrypt/scrypt.cpp
+++ b/src/lib/pbkdf/scrypt/scrypt.cpp
@@ -7,15 +7,24 @@
 
 #include <botan/scrypt.h>
 #include <botan/pbkdf2.h>
+#include <botan/exceptn.h>
 #include <botan/internal/salsa20.h>
 #include <botan/internal/loadstor.h>
-#include <botan/exceptn.h>
 #include <botan/internal/bit_ops.h>
 #include <botan/internal/timer.h>
 #include <sstream>
 
 namespace Botan {
 
+namespace {
+
+size_t scrypt_memory_usage(size_t N, size_t r, size_t p)
+   {
+   return 128 * r * (N + p);
+   }
+
+}
+
 std::string Scrypt_Family::name() const
    {
    return "Scrypt";
@@ -53,9 +62,13 @@ std::unique_ptr<PasswordHash> Scrypt_Family::tune(size_t output_length,
    Timer timer("Scrypt");
    const auto tune_time = BOTAN_PBKDF_TUNING_TIME;
 
+   auto pwdhash = this->from_params(N, r, p);
+
    timer.run_until_elapsed(tune_time, [&]() {
       uint8_t output[32] = { 0 };
-      scrypt(output, sizeof(output), "test", 4, nullptr, 0, N, r, p);
+      pwdhash->derive_key(output, sizeof(output),
+                          "test", 4,
+                          nullptr, 0);
       });
 
    // No timer events seems strange, perhaps something is wrong - give
@@ -147,17 +160,11 @@ std::string Scrypt::to_string() const
 
 size_t Scrypt::total_memory_usage() const
    {
-   return scrypt_memory_usage(m_N, m_r, m_p);
-   }
+   const size_t N = memory_param();
+   const size_t p = parallelism();
+   const size_t r = iterations();
 
-void Scrypt::derive_key(uint8_t output[], size_t output_len,
-                        const char* password, size_t password_len,
-                        const uint8_t salt[], size_t salt_len) const
-   {
-   scrypt(output, output_len,
-          password, password_len,
-          salt, salt_len,
-          N(), r(), p());
+   return scrypt_memory_usage(N, r, p);
    }
 
 namespace {
@@ -208,11 +215,14 @@ void scryptROMmix(size_t r, size_t N, uint8_t* B, secure_vector<uint8_t>& V)
 
 }
 
-void scrypt(uint8_t output[], size_t output_len,
-            const char* password, size_t password_len,
-            const uint8_t salt[], size_t salt_len,
-            size_t N, size_t r, size_t p)
+void Scrypt::derive_key(uint8_t output[], size_t output_len,
+                        const char* password, size_t password_len,
+                        const uint8_t salt[], size_t salt_len) const
    {
+   const size_t N = memory_param();
+   const size_t p = parallelism();
+   const size_t r = iterations();
+
    const size_t S = 128 * r;
    secure_vector<uint8_t> B(p * S);
    // temp space
diff --git a/src/lib/pbkdf/scrypt/scrypt.h b/src/lib/pbkdf/scrypt/scrypt.h
index 60cdbd046..7a5b3478c 100644
--- a/src/lib/pbkdf/scrypt/scrypt.h
+++ b/src/lib/pbkdf/scrypt/scrypt.h
@@ -10,7 +10,7 @@
 
 #include <botan/pwdhash.h>
 
-//BOTAN_FUTURE_INTERNAL_HEADER(scrypt.h)
+BOTAN_FUTURE_INTERNAL_HEADER(scrypt.h)
 
 namespace Botan {
 
@@ -34,15 +34,11 @@ class BOTAN_PUBLIC_API(2,8) Scrypt final : public PasswordHash
 
       std::string to_string() const override;
 
-      size_t N() const { return m_N; }
-      size_t r() const { return m_r; }
-      size_t p() const { return m_p; }
+      size_t iterations() const override { return m_r; }
 
-      size_t iterations() const override { return r(); }
+      size_t parallelism() const override { return m_p; }
 
-      size_t parallelism() const override { return p(); }
-
-      size_t memory_param() const override { return N(); }
+      size_t memory_param() const override { return m_N; }
 
       size_t total_memory_usage() const override;
 
@@ -84,10 +80,18 @@ class BOTAN_PUBLIC_API(2,8) Scrypt_Family final : public PasswordHashFamily
 *
 * Scrypt uses approximately (p + N + 1) * 128 * r bytes of memory
 */
-void BOTAN_PUBLIC_API(2,8) scrypt(uint8_t output[], size_t output_len,
-                                  const char* password, size_t password_len,
-                                  const uint8_t salt[], size_t salt_len,
-                                  size_t N, size_t r, size_t p);
+BOTAN_DEPRECATED("Use PasswordHashFamily+PasswordHash")
+inline void scrypt(uint8_t output[], size_t output_len,
+                   const char* password, size_t password_len,
+                   const uint8_t salt[], size_t salt_len,
+                   size_t N, size_t r, size_t p)
+   {
+   auto pwdhash_fam = PasswordHashFamily::create_or_throw("Scrypt");
+   auto pwdhash = pwdhash_fam->from_params(N, r, p);
+   pwdhash->derive_key(output, output_len,
+                       password, password_len,
+                       salt, salt_len);
+   }
 
 /**
 * Scrypt key derivation function (RFC 7914)
@@ -106,20 +110,17 @@ void BOTAN_PUBLIC_API(2,8) scrypt(uint8_t output[], size_t output_len,
 *
 * Scrypt uses approximately (p + N + 1) * 128 * r bytes of memory
 */
+BOTAN_DEPRECATED("Use PasswordHashFamily+PasswordHash")
 inline void scrypt(uint8_t output[], size_t output_len,
                    const std::string& password,
                    const uint8_t salt[], size_t salt_len,
                    size_t N, size_t r, size_t p)
    {
-   return scrypt(output, output_len,
-                 password.c_str(), password.size(),
-                 salt, salt_len,
-                 N, r, p);
-   }
-
-inline size_t scrypt_memory_usage(size_t N, size_t r, size_t p)
-   {
-   return 128 * r * (N + p);
+   auto pwdhash_fam = PasswordHashFamily::create_or_throw("Scrypt");
+   auto pwdhash = pwdhash_fam->from_params(N, r, p);
+   pwdhash->derive_key(output, output_len,
+                       password.c_str(), password.size(),
+                       salt, salt_len);
    }
 
 }
diff --git a/src/lib/pubkey/pbes2/pbes2.cpp b/src/lib/pubkey/pbes2/pbes2.cpp
index 47f301415..0120cb026 100644
--- a/src/lib/pubkey/pbes2/pbes2.cpp
+++ b/src/lib/pubkey/pbes2/pbes2.cpp
@@ -16,10 +16,6 @@
 #include <botan/oids.h>
 #include <botan/rng.h>
 
-#if defined(BOTAN_HAS_SCRYPT)
-   #include <botan/scrypt.h>
-#endif
-
 namespace Botan {
 
 namespace {
@@ -65,7 +61,6 @@ secure_vector<uint8_t> derive_key(const std::string& passphrase,
                         salt.data(), salt.size());
       return derived_key;
       }
-#if defined(BOTAN_HAS_SCRYPT)
    else if(kdf_algo.get_oid() == OID::from_string("Scrypt"))
       {
       secure_vector<uint8_t> salt;
@@ -86,12 +81,15 @@ secure_vector<uint8_t> derive_key(const std::string& passphrase,
          key_length = default_key_size;
 
       secure_vector<uint8_t> derived_key(key_length);
-      scrypt(derived_key.data(), derived_key.size(), passphrase,
-             salt.data(), salt.size(), N, r, p);
+
+      auto pwdhash_fam = PasswordHashFamily::create_or_throw("Scrypt");
+      auto pwdhash = pwdhash_fam->from_params(N, r, p);
+      pwdhash->derive_key(derived_key.data(), derived_key.size(),
+                          passphrase.data(), passphrase.size(),
+                          salt.data(), salt.size());
 
       return derived_key;
       }
-#endif
    else
       throw Decoding_Error("PBE-PKCS5 v2.0: Unknown KDF algorithm " +
                            kdf_algo.get_oid().to_string());
@@ -109,9 +107,7 @@ secure_vector<uint8_t> derive_key(const std::string& passphrase,
 
    if(digest == "Scrypt")
       {
-#if defined(BOTAN_HAS_SCRYPT)
-
-      std::unique_ptr<PasswordHashFamily> pwhash_fam = PasswordHashFamily::create_or_throw("Scrypt");
+      auto pwhash_fam = PasswordHashFamily::create_or_throw("Scrypt");
 
       std::unique_ptr<PasswordHash> pwhash;
 
@@ -149,9 +145,6 @@ secure_vector<uint8_t> derive_key(const std::string& passphrase,
 
       kdf_algo = AlgorithmIdentifier(OID::from_string("Scrypt"), scrypt_params);
       return key;
-#else
-      throw Not_Implemented("Scrypt is not available in this build");
-#endif
       }
    else
       {


Test patch:
diff --git a/src/tests/test_pbkdf.cpp b/src/tests/test_pbkdf.cpp
index ecdf2afe7..541aefac4 100644
--- a/src/tests/test_pbkdf.cpp
+++ b/src/tests/test_pbkdf.cpp
@@ -12,16 +12,8 @@
    #include <botan/pwdhash.h>
 #endif
 
-#if defined(BOTAN_HAS_PGP_S2K)
-   #include <botan/pgp_s2k.h>
-#endif
-
-#if defined(BOTAN_HAS_SCRYPT)
-   #include <botan/scrypt.h>
-#endif
-
-#if defined(BOTAN_HAS_PBKDF_BCRYPT)
-   #include <botan/bcrypt_pbkdf.h>
+#if defined(BOTAN_HAS_RFC4880)
+   #include <botan/rfc4880.h>
 #endif
 
 namespace Botan_Tests {
@@ -176,13 +168,23 @@ class Bcrypt_PBKDF_KAT_Tests final : public Text_Based_Test
 
          Test::Result result("bcrypt PBKDF");
 
-         std::vector<uint8_t> output(expected.size());
-         Botan::bcrypt_pbkdf(output.data(), output.size(),
-                             passphrase.data(), passphrase.size(),
-                             salt.data(), salt.size(),
-                             rounds);
+         auto pwdhash_fam = Botan::PasswordHashFamily::create("Bcrypt-PBKDF");
 
-         result.test_eq("derived key", output, expected);
+         if(!pwdhash_fam)
+            {
+            result.test_failure("Bcrypt-PBKDF is missing PasswordHashFamily");
+            return result;
+            }
+
+         auto pwdhash = pwdhash_fam->from_iterations(rounds);
+
+         std::vector<uint8_t> derived(expected.size());
+         pwdhash->derive_key(derived.data(), derived.size(),
+                             reinterpret_cast<const char*>(passphrase.data()),
+                             passphrase.size(),
+                             salt.data(), salt.size());
+
+         result.test_eq("derived key", derived, expected);
 
          return result;
          }
@@ -213,13 +215,6 @@ class Scrypt_KAT_Tests final : public Text_Based_Test
          if(N >= 1048576 && Test::run_long_tests() == false)
             return result;
 
-         std::vector<uint8_t> output(expected.size());
-         Botan::scrypt(output.data(), output.size(),
-                       passphrase, salt.data(), salt.size(),
-                       N, R, P);
-
-         result.test_eq("derived key", output, expected);
-
          auto pwdhash_fam = Botan::PasswordHashFamily::create("Scrypt");
 
          if(!pwdhash_fam)