Misc HKDF follow-ups

src/core/crypto/crypto_hkdf.cc

@@ -7,6 +7,7 @@
 #include <cstddef>     // std::size_t
 #include <cstdint>     // std::uint8_t
 #include <optional>    // std::optional, std::nullopt
+#include <stdexcept>   // std::runtime_error
 #include <string>      // std::string
 #include <string_view> // std::string_view
 
@@ -27,7 +28,15 @@ auto extract(const sourcemeta::core::KDFHash hash, const std::string_view salt,
              const std::string_view input_key_material)
     -> std::array<std::uint8_t, Size> {
   std::array<std::uint8_t, Size> output{};
-  sourcemeta::core::hkdf_extract(hash, salt, input_key_material, output.data());
+  // A refused extraction would otherwise leave the zero-filled buffer standing
+  // in for a pseudorandom key, which is both wrong and predictable, so it
+  // throws as the HMAC underneath does rather than reporting a derivation that
+  // never happened
+  if (!sourcemeta::core::hkdf_extract(hash, salt, input_key_material,
+                                      output.data())) {
+    throw std::runtime_error("Could not extract an HKDF pseudorandom key");
+  }
+
   return output;
 }
 

src/core/crypto/crypto_hkdf_loop.h

@@ -21,25 +21,45 @@ namespace sourcemeta::core {
 // RFC 5869 builds entirely on HMAC, so a backend without a dedicated key
 // derivation primitive composes one from the HMAC it already provides
 
+// The counterpart of the string scope for a fixed digest buffer, since the
+// HMAC underneath throws on two of the backends and an unwind would otherwise
+// leave a block of derived key material behind
+struct SecureDigestScope {
+  explicit SecureDigestScope(
+      std::array<std::uint8_t, KDF_MAXIMUM_DIGEST_BYTES> &value) noexcept
+      : target{value} {}
+  SecureDigestScope(const SecureDigestScope &) = delete;
+  auto operator=(const SecureDigestScope &) -> SecureDigestScope & = delete;
+  SecureDigestScope(SecureDigestScope &&) = delete;
+  auto operator=(SecureDigestScope &&) -> SecureDigestScope & = delete;
+  ~SecureDigestScope() {
+    secure_zero(this->target.data(), this->target.size());
+  }
+  std::array<std::uint8_t, KDF_MAXIMUM_DIGEST_BYTES> &target;
+};
+
 inline auto
 hkdf_loop_hmac(const KDFHash hash, const std::string_view key,
                const std::string_view message,
                std::array<std::uint8_t, KDF_MAXIMUM_DIGEST_BYTES> &output)
     -> std::size_t {
   switch (hash) {
     case KDFHash::SHA256: {
-      const auto digest{hmac_sha256_digest(key, message)};
+      auto digest{hmac_sha256_digest(key, message)};
       std::copy_n(digest.begin(), digest.size(), output.begin());
+      secure_zero(digest.data(), digest.size());
       return digest.size();
     }
     case KDFHash::SHA384: {
-      const auto digest{hmac_sha384_digest(key, message)};
+      auto digest{hmac_sha384_digest(key, message)};
       std::copy_n(digest.begin(), digest.size(), output.begin());
+      secure_zero(digest.data(), digest.size());
       return digest.size();
     }
     case KDFHash::SHA512: {
-      const auto digest{hmac_sha512_digest(key, message)};
+      auto digest{hmac_sha512_digest(key, message)};
       std::copy_n(digest.begin(), digest.size(), output.begin());
+      secure_zero(digest.data(), digest.size());
       return digest.size();
     }
   }
@@ -53,9 +73,9 @@ inline auto hkdf_extract_loop(const KDFHash hash, const std::string_view salt,
                               const std::string_view input_key_material,
                               unsigned char *const output) -> bool {
   std::array<std::uint8_t, KDF_MAXIMUM_DIGEST_BYTES> digest{};
+  const SecureDigestScope scope{digest};
   const auto size{hkdf_loop_hmac(hash, salt, input_key_material, digest)};
   std::copy_n(digest.begin(), size, output);
-  secure_zero(digest.data(), digest.size());
   return true;
 }
 
@@ -67,11 +87,13 @@ inline auto hkdf_expand_loop(const KDFHash hash,
                              unsigned char *const output,
                              const std::size_t length) -> bool {
   std::array<std::uint8_t, KDF_MAXIMUM_DIGEST_BYTES> block{};
+  const SecureDigestScope block_scope{block};
   std::size_t block_size{0};
   std::size_t produced{0};
   std::uint8_t counter{0};
 
   std::string message;
+  const SecureStringScope message_scope{message};
   message.reserve(kdf_digest_bytes(hash) + info.size() + 1);
 
   while (produced < length) {
@@ -87,9 +109,6 @@ inline auto hkdf_expand_loop(const KDFHash hash,
     produced += usable;
   }
 
-  // Both the working buffer and the last block carry derived key material
-  secure_zero(message);
-  secure_zero(block.data(), block.size());
   return true;
 }
 
@@ -101,18 +120,17 @@ hkdf_derive_loop(const KDFHash hash, const std::string_view input_key_material,
                  unsigned char *const output, const std::size_t length)
     -> bool {
   std::array<std::uint8_t, KDF_MAXIMUM_DIGEST_BYTES> pseudorandom_key{};
+  const SecureDigestScope scope{pseudorandom_key};
   if (!hkdf_extract_loop(hash, salt, input_key_material,
                          pseudorandom_key.data())) {
     return false;
   }
 
-  const auto result{hkdf_expand_loop(
+  return hkdf_expand_loop(
       hash,
       std::string_view{reinterpret_cast<const char *>(pseudorandom_key.data()),
                        kdf_digest_bytes(hash)},
-      info, output, length)};
-  secure_zero(pseudorandom_key.data(), pseudorandom_key.size());
-  return result;
+      info, output, length);
 }
 
 } // namespace sourcemeta::core

src/core/crypto/include/sourcemeta/core/crypto_hkdf.h

@@ -19,7 +19,8 @@ namespace sourcemeta::core {
 /// the first step of RFC 5869 Section 2.2. The salt is optional and need not
 /// be secret, and an empty one stands for the string of digest-length zeros
 /// that section substitutes when none is provided. RFC 5869 Section 3.1
-/// recommends supplying one. For example:
+/// recommends supplying one. Throws when the underlying provider refuses the
+/// extraction, since there is no pseudorandom key to report. For example:
 ///
 /// ```cpp
 /// #include <sourcemeta/core/crypto.h>