Cryptobox code modernization

Only encrypt so it's easy to verify everything still works.

6 files changed, 82 insertions, 66 deletions

diff --git a/doc/manual/cryptobox.rst b/doc/manual/cryptobox.rst
index ea77eee5a..a3a0d02b0 100644
--- a/doc/manual/cryptobox.rst
+++ b/doc/manual/cryptobox.rst
@@ -11,6 +11,10 @@ This is a set of simple routines that encrypt some data using a
 passphrase. There are defined in the header `cryptobox.h`, inside
 namespace `Botan::CryptoBox`.
 
+It generates cipher and MAC keys using 8192 iterations of PBKDF2 with
+HMAC(SHA-512), then encrypts using Serpent in CTR mode and authenticates using a
+HMAC(SHA-512) mac of the ciphertext, truncated to 160 bits.
+
  .. cpp:function:: std::string encrypt(const byte input[], size_t input_len, \
                                        const std::string& passphrase, \
                                        RandomNumberGenerator& rng)
diff --git a/src/lib/kdf/kdf.cpp b/src/lib/kdf/kdf.cpp
index f3d962c14..a1b0b91a9 100644
--- a/src/lib/kdf/kdf.cpp
+++ b/src/lib/kdf/kdf.cpp
@@ -221,9 +221,9 @@ std::unique_ptr<KDF>
 KDF::create_or_throw(const std::string& algo,
                              const std::string& provider)
    {
-   if(auto bc = KDF::create(algo, provider))
+   if(auto kdf = KDF::create(algo, provider))
       {
-      return bc;
+      return kdf;
       }
    throw Lookup_Error("KDF", algo, provider);
    }
diff --git a/src/lib/misc/cryptobox/cryptobox.cpp b/src/lib/misc/cryptobox/cryptobox.cpp
index 0ff6fe8f5..7551e21c1 100644
--- a/src/lib/misc/cryptobox/cryptobox.cpp
+++ b/src/lib/misc/cryptobox/cryptobox.cpp
@@ -35,7 +35,8 @@ const size_t MAC_OUTPUT_LEN = 20;
 const size_t PBKDF_SALT_LEN = 10;
 const size_t PBKDF_ITERATIONS = 8 * 1024;
 
-const size_t PBKDF_OUTPUT_LEN = CIPHER_KEY_LEN + CIPHER_IV_LEN + MAC_KEY_LEN;
+const size_t PBKDF_OUTPUT_LEN = CIPHER_KEY_LEN + MAC_KEY_LEN + CIPHER_IV_LEN;
+const size_t HEADER_LEN = VERSION_CODE_LEN + PBKDF_SALT_LEN + MAC_OUTPUT_LEN;
 
 }
 
@@ -43,32 +44,6 @@ std::string encrypt(const uint8_t input[], size_t input_len,
                     const std::string& passphrase,
                     RandomNumberGenerator& rng)
    {
-   secure_vector<uint8_t> pbkdf_salt(PBKDF_SALT_LEN);
-   rng.randomize(pbkdf_salt.data(), pbkdf_salt.size());
-
-   PKCS5_PBKDF2 pbkdf(new HMAC(new SHA_512));
-
-   OctetString master_key = pbkdf.derive_key(
-      PBKDF_OUTPUT_LEN,
-      passphrase,
-      pbkdf_salt.data(),
-      pbkdf_salt.size(),
-      PBKDF_ITERATIONS);
-
-   const uint8_t* mk = master_key.begin();
-
-   SymmetricKey cipher_key(mk, CIPHER_KEY_LEN);
-   SymmetricKey mac_key(&mk[CIPHER_KEY_LEN], MAC_KEY_LEN);
-   InitializationVector iv(&mk[CIPHER_KEY_LEN + MAC_KEY_LEN], CIPHER_IV_LEN);
-
-   Pipe pipe(get_cipher("Serpent/CTR-BE", cipher_key, iv, ENCRYPTION),
-             new Fork(
-                nullptr,
-                new MAC_Filter(new HMAC(new SHA_512),
-                               mac_key, MAC_OUTPUT_LEN)));
-
-   pipe.process_msg(input, input_len);
-
    /*
    Output format is:
       version # (4 bytes)
@@ -76,25 +51,43 @@ std::string encrypt(const uint8_t input[], size_t input_len,
       mac (20 bytes)
       ciphertext
    */
-   const size_t ciphertext_len = pipe.remaining(0);
-
-   std::vector<uint8_t> out_buf(VERSION_CODE_LEN +
-                             PBKDF_SALT_LEN +
-                             MAC_OUTPUT_LEN +
-                             ciphertext_len);
-
+   secure_vector<uint8_t> out_buf(HEADER_LEN + input_len);
    for(size_t i = 0; i != VERSION_CODE_LEN; ++i)
      out_buf[i] = get_byte(i, CRYPTOBOX_VERSION_CODE);
+   rng.randomize(&out_buf[VERSION_CODE_LEN], PBKDF_SALT_LEN);
+   // space left for MAC here
+   copy_mem(&out_buf[HEADER_LEN], input, input_len);
 
-   copy_mem(&out_buf[VERSION_CODE_LEN], pbkdf_salt.data(), PBKDF_SALT_LEN);
+   // Generate the keys and IV
 
-   BOTAN_ASSERT_EQUAL(
-      pipe.read(&out_buf[VERSION_CODE_LEN + PBKDF_SALT_LEN], MAC_OUTPUT_LEN, 1),
-      MAC_OUTPUT_LEN, "MAC output");
-   BOTAN_ASSERT_EQUAL(
-      pipe.read(&out_buf[VERSION_CODE_LEN + PBKDF_SALT_LEN + MAC_OUTPUT_LEN],
-                ciphertext_len, 0),
-      ciphertext_len, "Ciphertext size");
+   std::unique_ptr<PBKDF> pbkdf(PBKDF::create_or_throw("PBKDF2(HMAC(SHA-512))"));
+
+   OctetString master_key = pbkdf->derive_key(
+      CIPHER_KEY_LEN + MAC_KEY_LEN + CIPHER_IV_LEN,
+      passphrase,
+      &out_buf[VERSION_CODE_LEN],
+      PBKDF_SALT_LEN,
+      PBKDF_ITERATIONS);
+
+   const uint8_t* mk = master_key.begin();
+   const uint8_t* cipher_key = mk;
+   const uint8_t* mac_key = mk + CIPHER_KEY_LEN;
+   const uint8_t* iv = mk + CIPHER_KEY_LEN + MAC_KEY_LEN;
+
+   // Now encrypt and authenticate
+   std::unique_ptr<Cipher_Mode> ctr(get_cipher_mode("Serpent/CTR-BE", ENCRYPTION));
+   ctr->set_key(cipher_key, CIPHER_KEY_LEN);
+   ctr->start(iv, CIPHER_IV_LEN);
+   ctr->finish(out_buf, HEADER_LEN);
+
+   std::unique_ptr<MessageAuthenticationCode> hmac =
+      MessageAuthenticationCode::create_or_throw("HMAC(SHA-512)");
+   hmac->set_key(mac_key, MAC_KEY_LEN);
+   hmac->update(&out_buf[HEADER_LEN], input_len);
+
+   // Can't write directly because of MAC truncation
+   secure_vector<uint8_t> mac = hmac->final();
+   copy_mem(&out_buf[VERSION_CODE_LEN + PBKDF_SALT_LEN], mac.data(), MAC_OUTPUT_LEN);
 
    return PEM_Code::encode(out_buf, "BOTAN CRYPTOBOX MESSAGE");
    }
diff --git a/src/lib/pbkdf/pbkdf.cpp b/src/lib/pbkdf/pbkdf.cpp
index 40610998b..d2dfc3d41 100644
--- a/src/lib/pbkdf/pbkdf.cpp
+++ b/src/lib/pbkdf/pbkdf.cpp
@@ -68,6 +68,18 @@ std::unique_ptr<PBKDF> PBKDF::create(const std::string& algo_spec,
    return nullptr;
    }
 
+//static
+std::unique_ptr<PBKDF>
+PBKDF::create_or_throw(const std::string& algo,
+                             const std::string& provider)
+   {
+   if(auto pbkdf = PBKDF::create(algo, provider))
+      {
+      return pbkdf;
+      }
+   throw Lookup_Error("PBKDF", algo, provider);
+   }
+
 std::vector<std::string> PBKDF::providers(const std::string& algo_spec)
    {
    return probe_providers_of<PBKDF>(algo_spec, { "base", "openssl" });
diff --git a/src/lib/pbkdf/pbkdf.h b/src/lib/pbkdf/pbkdf.h
index a86f4d5c1..7c397da22 100644
--- a/src/lib/pbkdf/pbkdf.h
+++ b/src/lib/pbkdf/pbkdf.h
@@ -33,6 +33,15 @@ class BOTAN_PUBLIC_API(2,0) PBKDF
                                            const std::string& provider = "");
 
       /**
+      * Create an instance based on a name, or throw if the
+      * algo/provider combination cannot be found. If provider is
+      * empty then best available is chosen.
+      */
+      static std::unique_ptr<PBKDF>
+         create_or_throw(const std::string& algo_spec,
+                         const std::string& provider = "");
+
+      /**
       * @return list of available providers for this algorithm, empty if not available
       */
       static std::vector<std::string> providers(const std::string& algo_spec);
diff --git a/src/tests/test_cryptobox.cpp b/src/tests/test_cryptobox.cpp
index a45c2cfef..4e2ba83c2 100644
--- a/src/tests/test_cryptobox.cpp
+++ b/src/tests/test_cryptobox.cpp
@@ -22,33 +22,31 @@ class Cryptobox_Tests final : public Test
    public:
       std::vector<Test::Result> run() override
          {
-         std::vector<Test::Result> results;
          Test::Result result("cryptobox");
 
-         const std::vector<uint8_t> msg = Botan::hex_decode("AABBCC");
-         const std::string password = "secret";
-
-         std::string ciphertext = Botan::CryptoBox::encrypt(msg.data(), msg.size(),
-                                  password,
-                                  Test::rng());
-
-         try
-            {
-            std::string plaintext = Botan::CryptoBox::decrypt(ciphertext, password);
-
-            const uint8_t* pt_b = reinterpret_cast<const uint8_t*>(plaintext.data());
-
-            std::vector<uint8_t> pt_vec(pt_b, pt_b + plaintext.size());
-
-            result.test_eq("decrypt", pt_vec, msg);
-            }
-         catch(std::exception& e)
+         for(size_t i = 0; i <= 128; i += 7)
             {
-            result.test_failure("cryptobox decrypt", e.what());
+            const std::string password = Test::random_password();
+            const std::vector<uint8_t> input = unlock(Test::rng().random_vec(i));
+
+            const std::string ciphertext =
+               Botan::CryptoBox::encrypt(input.data(), input.size(), password, Test::rng());
+
+            try
+               {
+               const std::string decrypted = Botan::CryptoBox::decrypt(ciphertext, password);
+
+               const uint8_t* pt_b = reinterpret_cast<const uint8_t*>(decrypted.data());
+               std::vector<uint8_t> pt_vec(pt_b, pt_b + decrypted.size());
+               result.test_eq("decrypt", pt_vec, input);
+               }
+            catch(std::exception& e)
+               {
+               result.test_failure("cryptobox decrypt", e.what());
+               }
             }
 
-         results.push_back(result);
-         return results;
+         return {result};
          }
    };