From 623bf162d451fc3d6da79efda638e471e773d624 Mon Sep 17 00:00:00 2001
From: Julien Maffre <42961061+jumaffre@users.noreply.github.com>
Date: Mon, 12 Jun 2023 11:23:19 +0100
Subject: [PATCH] Update OpenSSL SHA digest API (#5336)

---
 .daily_canary                      |  2 +-
 include/ccf/crypto/hash_provider.h |  2 +-
 src/crypto/hash.cpp                |  2 --
 src/crypto/openssl/hash.cpp        | 40 ++++++++++++++++++++-------
 src/crypto/openssl/hash.h          |  3 +-
 src/crypto/test/crypto.cpp         | 44 ++++++++++++++++++++++++++++++
 6 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/.daily_canary b/.daily_canary
index 38cdbf6283c1..6959c02cd672 100644
--- a/.daily_canary
+++ b/.daily_canary
@@ -1,4 +1,4 @@
    ---     ___           ___
   (- -)   (= =)   |     Y   &  +-- 
- (  V  ) >  x  >  O    +---=---'
+ (  V  ) >  .  >  O    +---=---'
 /--x-m- /--n-n---xXx--/--yY--------
diff --git a/include/ccf/crypto/hash_provider.h b/include/ccf/crypto/hash_provider.h
index f93a2c064bce..55c5f425e0c3 100644
--- a/include/ccf/crypto/hash_provider.h
+++ b/include/ccf/crypto/hash_provider.h
@@ -47,7 +47,7 @@ namespace crypto
     }
 
     template <>
-    void update<std::vector<uint8_t>>(const std::vector<uint8_t>& d)
+    void update(const std::vector<uint8_t>& d)
     {
       update_hash(d);
     }
diff --git a/src/crypto/hash.cpp b/src/crypto/hash.cpp
index 879c07175770..44ffe11b1e2f 100644
--- a/src/crypto/hash.cpp
+++ b/src/crypto/hash.cpp
@@ -6,8 +6,6 @@
 #include "ccf/crypto/hkdf.h"
 #include "ccf/crypto/sha256.h"
 
-#include <openssl/sha.h>
-
 namespace crypto
 {
   void default_sha256(const std::span<const uint8_t>& data, uint8_t* h)
diff --git a/src/crypto/openssl/hash.cpp b/src/crypto/openssl/hash.cpp
index 36e01feba430..ae93518385f6 100644
--- a/src/crypto/openssl/hash.cpp
+++ b/src/crypto/openssl/hash.cpp
@@ -3,6 +3,7 @@
 
 #include "crypto/openssl/hash.h"
 
+#include <openssl/evp.h>
 #include <openssl/sha.h>
 #include <stdexcept>
 
@@ -38,21 +39,31 @@ namespace crypto
 
   void openssl_sha256(const std::span<const uint8_t>& data, uint8_t* h)
   {
-    SHA256_CTX ctx;
-    SHA256_Init(&ctx);
-    SHA256_Update(&ctx, data.data(), data.size());
-    SHA256_Final(h, &ctx);
+    const EVP_MD* md = EVP_sha256();
+    int rc = EVP_Digest(data.data(), data.size(), h, nullptr, md, nullptr);
+    if (rc != 1)
+    {
+      throw std::logic_error(fmt::format("EVP_Digest failed: {}", rc));
+    }
   }
 
   ISha256OpenSSL::ISha256OpenSSL()
   {
-    ctx = new SHA256_CTX;
-    SHA256_Init((SHA256_CTX*)ctx);
+    const EVP_MD* md = EVP_sha256();
+    ctx = EVP_MD_CTX_new();
+    int rc = EVP_DigestInit(ctx, md);
+    if (rc != 1)
+    {
+      throw std::logic_error(fmt::format("EVP_DigestInit failed: {}", rc));
+    }
   }
 
   ISha256OpenSSL::~ISha256OpenSSL()
   {
-    delete (SHA256_CTX*)ctx;
+    if (ctx)
+    {
+      EVP_MD_CTX_free(ctx);
+    }
   }
 
   void ISha256OpenSSL::update_hash(std::span<const uint8_t> data)
@@ -62,7 +73,11 @@ namespace crypto
       throw std::logic_error("Attempting to use hash after it was finalised");
     }
 
-    SHA256_Update((SHA256_CTX*)ctx, data.data(), data.size());
+    int rc = EVP_DigestUpdate(ctx, data.data(), data.size());
+    if (rc != 1)
+    {
+      throw std::logic_error(fmt::format("EVP_DigestUpdate failed: {}", rc));
+    }
   }
 
   Sha256Hash ISha256OpenSSL::finalise()
@@ -73,8 +88,13 @@ namespace crypto
     }
 
     Sha256Hash r;
-    SHA256_Final(r.h.data(), (SHA256_CTX*)ctx);
-    delete (SHA256_CTX*)ctx;
+    int rc = EVP_DigestFinal(ctx, r.h.data(), nullptr);
+    if (rc != 1)
+    {
+      EVP_MD_CTX_free(ctx);
+      throw std::logic_error(fmt::format("EVP_DigestFinal failed: {}", rc));
+    }
+    EVP_MD_CTX_free(ctx);
     ctx = nullptr;
     return r;
   }
diff --git a/src/crypto/openssl/hash.h b/src/crypto/openssl/hash.h
index 643561df7c2f..c02ea7112462 100644
--- a/src/crypto/openssl/hash.h
+++ b/src/crypto/openssl/hash.h
@@ -7,7 +7,6 @@
 
 #include <openssl/evp.h>
 #include <openssl/kdf.h>
-#include <openssl/sha.h>
 
 #define FMT_HEADER_ONLY
 #include <fmt/format.h>
@@ -75,7 +74,7 @@ namespace crypto
     virtual Sha256Hash finalise();
 
   protected:
-    void* ctx;
+    EVP_MD_CTX* ctx = nullptr;
   };
 
   void openssl_sha256(const std::span<const uint8_t>& data, uint8_t* h);
diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp
index 0f762417ba2f..2bbf7e0d8568 100644
--- a/src/crypto/test/crypto.cpp
+++ b/src/crypto/test/crypto.cpp
@@ -877,4 +877,48 @@ TEST_CASE("PEM to JWK and back")
       REQUIRE(jwk == jwk2);
     }
   }
+}
+
+TEST_CASE("Incremental hash")
+{
+  auto simple_hash = crypto::Sha256Hash(contents);
+
+  INFO("Incremental hash");
+  {
+    INFO("Finalise before any update");
+    {
+      auto ihash = make_incremental_sha256();
+      auto final_hash = ihash->finalise();
+      REQUIRE(final_hash != simple_hash);
+    }
+
+    INFO("Update one by one");
+    {
+      auto ihash = make_incremental_sha256();
+      for (auto const& c : contents)
+      {
+        ihash->update(c);
+      }
+      auto final_hash = ihash->finalise();
+      REQUIRE(final_hash == simple_hash);
+
+      REQUIRE_THROWS_AS(ihash->finalise(), std::logic_error);
+    }
+
+    INFO("Update in large chunks");
+    {
+      constexpr size_t chunk_size = 10;
+      auto ihash = make_incremental_sha256();
+      for (auto it = contents.begin(); it < contents.end(); it += chunk_size)
+      {
+        auto end =
+          it + chunk_size > contents.end() ? contents.end() : it + chunk_size;
+        ihash->update(std::vector<uint8_t>{it, end});
+      }
+      auto final_hash = ihash->finalise();
+      REQUIRE(final_hash == simple_hash);
+
+      REQUIRE_THROWS_AS(ihash->finalise(), std::logic_error);
+    }
+  }
 }
\ No newline at end of file