From 85736370a15bbb44cbe8e7746fce2a0777398074 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:17:07 +0000 Subject: [PATCH 1/4] Initial plan From 6686bf494db5985d947290ceedb7e87e374ef169 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:57:33 +0000 Subject: [PATCH 2/4] Reuse pre-keyed AES-GCM contexts Co-authored-by: eddyashton <6000239+eddyashton@users.noreply.github.com> --- include/ccf/crypto/openssl/openssl_wrappers.h | 8 + src/crypto/openssl/symmetric_key.cpp | 178 +++++++++++++++--- src/crypto/openssl/symmetric_key.h | 12 +- src/crypto/test/bench.cpp | 33 ++++ src/crypto/test/crypto.cpp | 160 ++++++++++++++++ src/node/test/encryptor.cpp | 6 +- 6 files changed, 367 insertions(+), 30 deletions(-) diff --git a/include/ccf/crypto/openssl/openssl_wrappers.h b/include/ccf/crypto/openssl/openssl_wrappers.h index ff375f54b32b..cafd51b1e62a 100644 --- a/include/ccf/crypto/openssl/openssl_wrappers.h +++ b/include/ccf/crypto/openssl/openssl_wrappers.h @@ -354,6 +354,14 @@ namespace ccf::crypto::OpenSSL using Unique_SSL_OBJECT::Unique_SSL_OBJECT; }; + struct Unique_EVP_CIPHER + : public Unique_SSL_OBJECT + { + Unique_EVP_CIPHER(EVP_CIPHER* cipher) : + Unique_SSL_OBJECT(cipher, EVP_CIPHER_free) + {} + }; + struct Unique_STACK_OF_X509 : public Unique_SSL_OBJECT { diff --git a/src/crypto/openssl/symmetric_key.cpp b/src/crypto/openssl/symmetric_key.cpp index 14d8d997fc0d..479615499091 100644 --- a/src/crypto/openssl/symmetric_key.cpp +++ b/src/crypto/openssl/symmetric_key.cpp @@ -7,9 +7,12 @@ #include "ccf/crypto/symmetric_key.h" #include "ds/internal_logger.h" +#include #include +#include #include #include +#include namespace ccf::crypto { @@ -19,30 +22,158 @@ namespace ccf::crypto static constexpr size_t KEY_SIZE_192 = 192; static constexpr size_t KEY_SIZE_128 = 128; - KeyAesGcm_OpenSSL::KeyAesGcm_OpenSSL(std::span rawKey) : - key(std::vector(rawKey.data(), rawKey.data() + rawKey.size())) + namespace { - const auto n = static_cast(rawKey.size() * CHAR_BIT); - if (n >= KEY_SIZE_256) + static constexpr size_t MAX_CACHED_CONTEXTS = 16; + + const char* get_gcm_cipher_name(std::span raw_key) { - evp_cipher = EVP_aes_256_gcm(); - evp_cipher_wrap_pad = EVP_aes_256_wrap_pad(); + const auto n = static_cast(raw_key.size() * CHAR_BIT); + if (n >= KEY_SIZE_256) + { + return "AES-256-GCM"; + } + if (n >= KEY_SIZE_192) + { + return "AES-192-GCM"; + } + if (n >= KEY_SIZE_128) + { + return "AES-128-GCM"; + } + throw std::logic_error( + fmt::format("Need at least {} bits, only have {}", KEY_SIZE_128, n)); } - else if (n >= KEY_SIZE_192) + + const EVP_CIPHER* get_wrap_pad_cipher(std::span raw_key) { - evp_cipher = EVP_aes_192_gcm(); - evp_cipher_wrap_pad = EVP_aes_192_wrap_pad(); + const auto n = static_cast(raw_key.size() * CHAR_BIT); + if (n >= KEY_SIZE_256) + { + return EVP_aes_256_wrap_pad(); + } + if (n >= KEY_SIZE_192) + { + return EVP_aes_192_wrap_pad(); + } + return EVP_aes_128_wrap_pad(); } - else if (n >= KEY_SIZE_128) + + struct CachedContext { - evp_cipher = EVP_aes_128_gcm(); - evp_cipher_wrap_pad = EVP_aes_128_wrap_pad(); - } - else + std::mutex lock; + std::optional context = std::nullopt; + bool keyed = false; + }; + + class ContextLease { - throw std::logic_error( - fmt::format("Need at least {} bits, only have {}", KEY_SIZE_128, n)); - } + private: + CachedContext* cached = nullptr; + [[maybe_unused]] std::unique_lock lock; + std::optional uncached = std::nullopt; + EVP_CIPHER_CTX* context = nullptr; + + public: + ContextLease( + CachedContext& cached_, std::unique_lock&& lock_) : + cached(&cached_), + lock(std::move(lock_)) + { + if (!cached->context.has_value()) + { + cached->context.emplace(); + } + context = cached->context.value(); + } + + ContextLease() : uncached(std::in_place), context(uncached.value()) {} + + EVP_CIPHER_CTX* get() + { + return context; + } + + void initialise( + bool encrypt, const EVP_CIPHER* cipher, std::span key) + { + if (cached != nullptr && cached->keyed) + { + return; + } + + if (encrypt) + { + CHECK1( + EVP_EncryptInit_ex2(context, cipher, key.data(), nullptr, nullptr)); + } + else + { + CHECK1( + EVP_DecryptInit_ex2(context, cipher, key.data(), nullptr, nullptr)); + } + + if (cached != nullptr) + { + cached->keyed = true; + } + } + }; + + class ContextPool + { + private: + const bool encrypt; + std::array contexts; + + public: + ContextPool(bool encrypt_) : encrypt(encrypt_) {} + + ContextLease acquire( + const EVP_CIPHER* cipher, std::span key) + { + for (auto& cached : contexts) + { + std::unique_lock lock(cached.lock, std::try_to_lock); + if (lock.owns_lock()) + { + ContextLease lease(cached, std::move(lock)); + lease.initialise(encrypt, cipher, key); + return lease; + } + } + + ContextLease lease; + lease.initialise(encrypt, cipher, key); + return lease; + } + }; + } + + struct KeyAesGcm_OpenSSL::ContextPools + { + ContextPool encrypt{true}; + ContextPool decrypt{false}; + }; + + KeyAesGcm_OpenSSL::KeyAesGcm_OpenSSL(std::span rawKey) : + key(std::vector(rawKey.data(), rawKey.data() + rawKey.size())), + evp_cipher(EVP_CIPHER_fetch(nullptr, get_gcm_cipher_name(rawKey), nullptr)), + evp_cipher_wrap_pad(get_wrap_pad_cipher(rawKey)), + context_pools(std::make_unique()) + {} + + KeyAesGcm_OpenSSL::KeyAesGcm_OpenSSL(KeyAesGcm_OpenSSL&& that) noexcept : + key(std::move(that.key)), + evp_cipher(std::move(that.evp_cipher)), + evp_cipher_wrap_pad(that.evp_cipher_wrap_pad), + context_pools(std::move(that.context_pools)) + {} + + KeyAesGcm_OpenSSL::~KeyAesGcm_OpenSSL() + { + context_pools.reset(); + OPENSSL_cleanse(const_cast(key.data()), key.size()); } size_t KeyAesGcm_OpenSSL::key_size() const @@ -62,12 +193,12 @@ namespace ccf::crypto throw std::logic_error("aad and plain cannot both be empty"); } - Unique_EVP_CIPHER_CTX ctx; - CHECK1(EVP_EncryptInit_ex(ctx, evp_cipher, nullptr, key.data(), nullptr)); + auto lease = context_pools->encrypt.acquire(evp_cipher, key); + auto* ctx = lease.get(); CHECK1( EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), nullptr)); - CHECK1(EVP_EncryptInit_ex(ctx, nullptr, nullptr, key.data(), iv.data())); + CHECK1(EVP_EncryptInit_ex2(ctx, nullptr, nullptr, iv.data(), nullptr)); if (!aad.empty()) { @@ -113,12 +244,13 @@ namespace ccf::crypto std::span aad, std::vector& plain) const { - Unique_EVP_CIPHER_CTX ctx; - CHECK1(EVP_DecryptInit_ex(ctx, evp_cipher, nullptr, nullptr, nullptr)); + auto lease = context_pools->decrypt.acquire(evp_cipher, key); + auto* ctx = lease.get(); + CHECK1( EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), nullptr)); - CHECK1(EVP_DecryptInit_ex(ctx, nullptr, nullptr, key.data(), iv.data())); + CHECK1(EVP_DecryptInit_ex2(ctx, nullptr, nullptr, iv.data(), nullptr)); if (!aad.empty()) { int aad_outl{0}; diff --git a/src/crypto/openssl/symmetric_key.h b/src/crypto/openssl/symmetric_key.h index 3729cdb7c9b7..86c99958fce1 100644 --- a/src/crypto/openssl/symmetric_key.h +++ b/src/crypto/openssl/symmetric_key.h @@ -12,18 +12,18 @@ namespace ccf::crypto class KeyAesGcm_OpenSSL : public KeyAesGcm { private: - const std::vector key; - const EVP_CIPHER* evp_cipher = nullptr; + struct ContextPools; + + std::vector key; + OpenSSL::Unique_EVP_CIPHER evp_cipher; const EVP_CIPHER* evp_cipher_wrap_pad; + std::unique_ptr context_pools; public: KeyAesGcm_OpenSSL(std::span rawKey); KeyAesGcm_OpenSSL(const KeyAesGcm_OpenSSL& that) = delete; KeyAesGcm_OpenSSL(KeyAesGcm_OpenSSL&& that) noexcept; - ~KeyAesGcm_OpenSSL() override - { - OPENSSL_cleanse(const_cast(key.data()), key.size()); - } + ~KeyAesGcm_OpenSSL() override; [[nodiscard]] size_t key_size() const override; diff --git a/src/crypto/test/bench.cpp b/src/crypto/test/bench.cpp index 675b8690fec5..748e3cbc6a9a 100644 --- a/src/crypto/test/bench.cpp +++ b/src/crypto/test/bench.cpp @@ -112,6 +112,29 @@ static void benchmark_hmac(picobench::state& s) s.stop_timer(); } +template +static void benchmark_aes_gcm_encrypt(picobench::state& s) +{ + const std::vector key(GCM_DEFAULT_KEY_SIZE, 0x42); + const auto contents = make_contents(); + auto aes_gcm_key = make_key_aes_gcm(key); + StandardGcmHeader header; + std::vector cipher; + uint64_t iv = 0; + + s.start_timer(); + for (auto _ : s) + { + (void)_; + memcpy(header.iv.data(), &iv, sizeof(iv)); + ++iv; + aes_gcm_key->encrypt(header.get_iv(), contents, {}, cipher, header.tag); + do_not_optimize(cipher); + clobber_memory(); + } + s.stop_timer(); +} + template static void benchmark_hash(picobench::state& s) { @@ -465,6 +488,16 @@ namespace HMAC_bench PICOBENCH(openssl_hmac_sha256_64).PICO_HASH_SUFFIX(); } +PICOBENCH_SUITE("aes gcm"); +namespace AES_GCM_bench +{ + auto aes_gcm_encrypt_64 = benchmark_aes_gcm_encrypt<64>; + PICOBENCH(aes_gcm_encrypt_64).iterations({100000}); + + auto aes_gcm_encrypt_1024 = benchmark_aes_gcm_encrypt<1024>; + PICOBENCH(aes_gcm_encrypt_1024).iterations({100000}); +} + std::vector shares; PICOBENCH_SUITE("share"); diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index 94dcb2bb8678..d9ec1437082f 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -25,12 +25,16 @@ #include "crypto/openssl/verifier.h" #include "crypto/openssl/x509_time.h" +#include +#include +#include #include #include #include #include #include #include +#include using namespace std; using namespace ccf::crypto; @@ -809,6 +813,162 @@ static const vector& get_raw_key() return v; } +TEST_CASE("AES-GCM context reuse") +{ + const std::vector key(16, 0); + const std::vector iv(12, 0); + const std::vector plain(16, 0); + const std::vector expected_cipher = { + 0x03, + 0x88, + 0xda, + 0xce, + 0x60, + 0xb6, + 0xa3, + 0x92, + 0xf3, + 0x28, + 0xc2, + 0xb9, + 0x71, + 0xb2, + 0xfe, + 0x78}; + const uint8_t expected_tag[GCM_SIZE_TAG] = { + 0xab, + 0x6e, + 0x47, + 0xd4, + 0x2c, + 0xec, + 0x13, + 0xbd, + 0xf5, + 0x3a, + 0x67, + 0xb2, + 0x12, + 0x57, + 0xbd, + 0xdf}; + auto aes_gcm_key = make_key_aes_gcm(key); + + std::vector cipher; + uint8_t tag[GCM_SIZE_TAG] = {}; + aes_gcm_key->encrypt(iv, plain, {}, cipher, tag); + + REQUIRE(cipher == expected_cipher); + REQUIRE(std::equal(std::begin(tag), std::end(tag), std::begin(expected_tag))); + + std::vector decrypted; + std::array invalid_tag; + std::copy(std::begin(tag), std::end(tag), invalid_tag.begin()); + invalid_tag[0] ^= 1; + REQUIRE_FALSE( + aes_gcm_key->decrypt(iv, invalid_tag.data(), cipher, {}, decrypted)); + REQUIRE(decrypted.empty()); + + REQUIRE(aes_gcm_key->decrypt(iv, tag, cipher, {}, decrypted)); + REQUIRE(decrypted == plain); +} + +TEST_CASE("AES-GCM empty inputs") +{ + auto aes_gcm_key = make_key_aes_gcm(get_raw_key()); + const std::vector iv(12, 0); + const std::vector aad(8, 0x42); + const std::vector plain(8, 0x24); + uint8_t tag[GCM_SIZE_TAG] = {}; + std::vector cipher; + std::vector decrypted; + + aes_gcm_key->encrypt(iv, {}, aad, cipher, tag); + REQUIRE(cipher.empty()); + REQUIRE(aes_gcm_key->decrypt(iv, tag, cipher, aad, decrypted)); + REQUIRE(decrypted.empty()); + + aes_gcm_key->encrypt(iv, plain, {}, cipher, tag); + REQUIRE(aes_gcm_key->decrypt(iv, tag, cipher, {}, decrypted)); + REQUIRE(decrypted == plain); + + REQUIRE_THROWS_AS( + aes_gcm_key->encrypt(iv, {}, {}, cipher, tag), std::logic_error); + + const std::vector empty_key(16, 0); + auto empty_aes_gcm_key = make_key_aes_gcm(empty_key); + const uint8_t empty_tag[GCM_SIZE_TAG] = { + 0x58, + 0xe2, + 0xfc, + 0xce, + 0xfa, + 0x7e, + 0x30, + 0x61, + 0x36, + 0x7f, + 0x1d, + 0x57, + 0xa4, + 0xe7, + 0x45, + 0x5a}; + decrypted.clear(); + REQUIRE(empty_aes_gcm_key->decrypt(iv, empty_tag, {}, {}, decrypted)); + REQUIRE(decrypted.empty()); +} + +TEST_CASE("Concurrent AES-GCM context reuse") +{ + constexpr size_t thread_count = 24; + constexpr size_t iteration_count = 128; + auto aes_gcm_key = make_key_aes_gcm(get_raw_key()); + std::barrier start(thread_count); + std::atomic success = true; + std::vector threads; + + for (size_t thread_index = 0; thread_index < thread_count; ++thread_index) + { + threads.emplace_back([&, thread_index]() { + try + { + start.arrive_and_wait(); + for (size_t i = 0; i < iteration_count; ++i) + { + const uint64_t nonce = (thread_index * iteration_count) + i + 1; + std::vector iv(12, 0); + memcpy(iv.data(), &nonce, sizeof(nonce)); + const std::vector plain(64, thread_index); + const std::vector aad(16, i); + std::vector cipher; + uint8_t tag[GCM_SIZE_TAG] = {}; + + aes_gcm_key->encrypt(iv, plain, aad, cipher, tag); + std::vector decrypted; + if ( + !aes_gcm_key->decrypt(iv, tag, cipher, aad, decrypted) || + decrypted != plain) + { + success = false; + } + } + } + catch (...) + { + success = false; + } + }); + } + + for (auto& thread : threads) + { + thread.join(); + } + + REQUIRE(success); +} + TEST_CASE("ExtendedIv0") { auto k = ccf::crypto::make_key_aes_gcm(get_raw_key()); diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index 19b819d803a5..8ecce071936a 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -414,18 +414,22 @@ TEST_CASE("Encryptor rollback") ledger_secrets->init(); auto encryptor = std::make_shared(ledger_secrets); store.set_encryptor(encryptor); + std::weak_ptr rolled_back_key; commit_one(store, map); // Assumes tx at seqno 2 rekeys. Txs from seqno 3 will be encrypted with new // secret commit_one(store, map); - ledger_secrets->set_secret(3, ccf::make_ledger_secret()); + auto rolled_back_secret = ccf::make_ledger_secret(); + rolled_back_key = rolled_back_secret->key; + ledger_secrets->set_secret(3, std::move(rolled_back_secret)); commit_one(store, map); // Rollback store at seqno 1, discarding encryption key at 3 store.rollback({store_term, 1}, store.commit_view()); + REQUIRE(rolled_back_key.expired()); commit_one(store, map); From 1163f593e8e48314919359d5d97dc86dc53a5482 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Wed, 19 Aug 2026 12:12:00 +0000 Subject: [PATCH 3/4] Agent Host changes for agents/context-object-ownership-implementation --- CHANGELOG.md | 1 + include/ccf/crypto/openssl/openssl_wrappers.h | 8 - include/ccf/crypto/symmetric_key.h | 29 ++ src/crypto/openssl/symmetric_key.cpp | 329 ++++++++---------- src/crypto/openssl/symmetric_key.h | 7 +- src/crypto/test/bench.cpp | 3 +- src/crypto/test/crypto.cpp | 10 +- src/kv/encryptor.h | 15 +- src/node/ledger_secret.h | 29 +- src/node/test/encryptor.cpp | 38 ++ 10 files changed, 261 insertions(+), 208 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 126fdff584c8..0b805ca13a27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). - New `ledger.max_transaction_size` node configuration option (default `32MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is now rejected with `413 Payload Too Large` and error code `TransactionTooLarge`, and subsequent transactions are unaffected, where previously an excessively large transaction could terminate the node. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. It must be smaller than `memory.max_msg_size` by at least the ring-buffer range response overhead, which is validated at node startup and by `--check` (#7992). +- C++ callers can use `ccf::crypto::KeyAesGcm::make_context()` to explicitly own and reuse a pre-keyed AES-GCM context when they can ensure it is not accessed concurrently (#8170). ### Changed diff --git a/include/ccf/crypto/openssl/openssl_wrappers.h b/include/ccf/crypto/openssl/openssl_wrappers.h index cafd51b1e62a..ff375f54b32b 100644 --- a/include/ccf/crypto/openssl/openssl_wrappers.h +++ b/include/ccf/crypto/openssl/openssl_wrappers.h @@ -354,14 +354,6 @@ namespace ccf::crypto::OpenSSL using Unique_SSL_OBJECT::Unique_SSL_OBJECT; }; - struct Unique_EVP_CIPHER - : public Unique_SSL_OBJECT - { - Unique_EVP_CIPHER(EVP_CIPHER* cipher) : - Unique_SSL_OBJECT(cipher, EVP_CIPHER_free) - {} - }; - struct Unique_STACK_OF_X509 : public Unique_SSL_OBJECT { diff --git a/include/ccf/crypto/symmetric_key.h b/include/ccf/crypto/symmetric_key.h index 4eccdc5d368b..383159ba0dcb 100644 --- a/include/ccf/crypto/symmetric_key.h +++ b/include/ccf/crypto/symmetric_key.h @@ -70,9 +70,38 @@ namespace ccf::crypto class KeyAesGcm { public: + class Context + { + public: + Context() = default; + virtual ~Context() = default; + + Context(const Context&) = delete; + Context& operator=(const Context&) = delete; + Context(Context&&) = delete; + Context& operator=(Context&&) = delete; + + // Contexts are reusable, but are not safe for concurrent use. + virtual void encrypt( + std::span iv, + std::span plain, + std::span aad, + std::vector& cipher, + uint8_t tag[GCM_SIZE_TAG]) = 0; + + virtual bool decrypt( + std::span iv, + const uint8_t tag[GCM_SIZE_TAG], + std::span cipher, + std::span aad, + std::vector& plain) = 0; + }; + KeyAesGcm() = default; virtual ~KeyAesGcm() = default; + virtual std::unique_ptr make_context() = 0; + // AES-GCM encryption virtual void encrypt( std::span iv, diff --git a/src/crypto/openssl/symmetric_key.cpp b/src/crypto/openssl/symmetric_key.cpp index 479615499091..6f75cf71ce03 100644 --- a/src/crypto/openssl/symmetric_key.cpp +++ b/src/crypto/openssl/symmetric_key.cpp @@ -7,12 +7,9 @@ #include "ccf/crypto/symmetric_key.h" #include "ds/internal_logger.h" -#include #include -#include #include #include -#include namespace ccf::crypto { @@ -24,22 +21,20 @@ namespace ccf::crypto namespace { - static constexpr size_t MAX_CACHED_CONTEXTS = 16; - - const char* get_gcm_cipher_name(std::span raw_key) + const EVP_CIPHER* get_gcm_cipher(std::span raw_key) { const auto n = static_cast(raw_key.size() * CHAR_BIT); if (n >= KEY_SIZE_256) { - return "AES-256-GCM"; + return EVP_aes_256_gcm(); } if (n >= KEY_SIZE_192) { - return "AES-192-GCM"; + return EVP_aes_192_gcm(); } if (n >= KEY_SIZE_128) { - return "AES-128-GCM"; + return EVP_aes_128_gcm(); } throw std::logic_error( fmt::format("Need at least {} bits, only have {}", KEY_SIZE_128, n)); @@ -59,120 +54,172 @@ namespace ccf::crypto return EVP_aes_128_wrap_pad(); } - struct CachedContext + void encrypt_with_context( + EVP_CIPHER_CTX* ctx, + std::span iv, + std::span plain, + std::span aad, + std::vector& cipher, + uint8_t tag[GCM_SIZE_TAG]) { - std::mutex lock; - std::optional context = std::nullopt; - bool keyed = false; - }; + if (aad.empty() && plain.empty()) + { + throw std::logic_error("aad and plain cannot both be empty"); + } + + CHECK1( + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), nullptr)); + CHECK1(EVP_EncryptInit_ex2(ctx, nullptr, nullptr, iv.data(), nullptr)); + + if (!aad.empty()) + { + int aad_outl{0}; + CHECK1( + EVP_EncryptUpdate(ctx, nullptr, &aad_outl, aad.data(), aad.size())); + } + + std::vector ciphertext(plain.size()); + if (!plain.empty()) + { + int cypher_outl{0}; + CHECK1(EVP_EncryptUpdate( + ctx, ciphertext.data(), &cypher_outl, plain.data(), plain.size())); + + // As we use no padding, we expect the input and output lengths to + // match. + assert(static_cast(cypher_outl) == plain.size()); + } + + int final_outl{0}; + CHECK1(EVP_EncryptFinal_ex(ctx, nullptr, &final_outl)); + + // As long a we use GSM cipher, the final outl must be 0, because there's + // no padding and the block size is equal to 1, so EncryptUpdate() always + // does the whole thing. Final is still a must to finalize and check the + // error. + // + // See https://docs.openssl.org/3.3/man3/EVP_EncryptInit/#aead-interface. + assert(final_outl == 0); + + CHECK1( + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, GCM_SIZE_TAG, &tag[0])); + + if (!plain.empty()) + { + cipher = std::move(ciphertext); + } + } - class ContextLease + bool decrypt_with_context( + EVP_CIPHER_CTX* ctx, + std::span iv, + const uint8_t tag[GCM_SIZE_TAG], + std::span cipher, + std::span aad, + std::vector& plain) { - private: - CachedContext* cached = nullptr; - [[maybe_unused]] std::unique_lock lock; - std::optional uncached = std::nullopt; - EVP_CIPHER_CTX* context = nullptr; + CHECK1( + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), nullptr)); - public: - ContextLease( - CachedContext& cached_, std::unique_lock&& lock_) : - cached(&cached_), - lock(std::move(lock_)) + CHECK1(EVP_DecryptInit_ex2(ctx, nullptr, nullptr, iv.data(), nullptr)); + if (!aad.empty()) { - if (!cached->context.has_value()) - { - cached->context.emplace(); - } - context = cached->context.value(); + int aad_outl{0}; + CHECK1( + EVP_DecryptUpdate(ctx, nullptr, &aad_outl, aad.data(), aad.size())); } - ContextLease() : uncached(std::in_place), context(uncached.value()) {} + std::vector plaintext(cipher.size()); + if (!cipher.empty()) + { + int plain_outl{0}; + CHECK1(EVP_DecryptUpdate( + ctx, plaintext.data(), &plain_outl, cipher.data(), cipher.size())); + + // As we use no padding, we expect the input and output lengths to + // match. + assert(static_cast(plain_outl) == cipher.size()); + } - EVP_CIPHER_CTX* get() + void* tag_ptr = const_cast(static_cast(tag)); + CHECK1( + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, GCM_SIZE_TAG, tag_ptr)); + + int final_outl{0}; + if (EVP_DecryptFinal_ex(ctx, nullptr, &final_outl) != 1) { - return context; + return false; } - void initialise( - bool encrypt, const EVP_CIPHER* cipher, std::span key) + // As long a we use GSM cipher, the final outl must be 0, because there's + // no padding and the block size is equal to 1, so EncryptUpdate() always + // does the whole thing. Final is still a must to finalize and check the + // error. + // + // See https://docs.openssl.org/3.3/man3/EVP_EncryptInit/#aead-interface. + assert(final_outl == 0); + + if (!cipher.empty()) { - if (cached != nullptr && cached->keyed) - { - return; - } - - if (encrypt) - { - CHECK1( - EVP_EncryptInit_ex2(context, cipher, key.data(), nullptr, nullptr)); - } - else - { - CHECK1( - EVP_DecryptInit_ex2(context, cipher, key.data(), nullptr, nullptr)); - } - - if (cached != nullptr) - { - cached->keyed = true; - } + plain = std::move(plaintext); } - }; - class ContextPool + return true; + } + + class AesGcmContext_OpenSSL : public KeyAesGcm::Context { private: - const bool encrypt; - std::array contexts; + Unique_EVP_CIPHER_CTX encrypt_context; + Unique_EVP_CIPHER_CTX decrypt_context; public: - ContextPool(bool encrypt_) : encrypt(encrypt_) {} - - ContextLease acquire( + AesGcmContext_OpenSSL( const EVP_CIPHER* cipher, std::span key) { - for (auto& cached : contexts) - { - std::unique_lock lock(cached.lock, std::try_to_lock); - if (lock.owns_lock()) - { - ContextLease lease(cached, std::move(lock)); - lease.initialise(encrypt, cipher, key); - return lease; - } - } - - ContextLease lease; - lease.initialise(encrypt, cipher, key); - return lease; + CHECK1(EVP_EncryptInit_ex2( + encrypt_context, cipher, key.data(), nullptr, nullptr)); + CHECK1(EVP_DecryptInit_ex2( + decrypt_context, cipher, key.data(), nullptr, nullptr)); + } + + void encrypt( + std::span iv, + std::span plain, + std::span aad, + std::vector& cipher, + uint8_t tag[GCM_SIZE_TAG]) override + { + encrypt_with_context(encrypt_context, iv, plain, aad, cipher, tag); + } + + bool decrypt( + std::span iv, + const uint8_t tag[GCM_SIZE_TAG], + std::span cipher, + std::span aad, + std::vector& plain) override + { + return decrypt_with_context( + decrypt_context, iv, tag, cipher, aad, plain); } }; } - struct KeyAesGcm_OpenSSL::ContextPools - { - ContextPool encrypt{true}; - ContextPool decrypt{false}; - }; - KeyAesGcm_OpenSSL::KeyAesGcm_OpenSSL(std::span rawKey) : key(std::vector(rawKey.data(), rawKey.data() + rawKey.size())), - evp_cipher(EVP_CIPHER_fetch(nullptr, get_gcm_cipher_name(rawKey), nullptr)), - evp_cipher_wrap_pad(get_wrap_pad_cipher(rawKey)), - context_pools(std::make_unique()) + evp_cipher(get_gcm_cipher(rawKey)), + evp_cipher_wrap_pad(get_wrap_pad_cipher(rawKey)) {} KeyAesGcm_OpenSSL::KeyAesGcm_OpenSSL(KeyAesGcm_OpenSSL&& that) noexcept : key(std::move(that.key)), - evp_cipher(std::move(that.evp_cipher)), - evp_cipher_wrap_pad(that.evp_cipher_wrap_pad), - context_pools(std::move(that.context_pools)) + evp_cipher(that.evp_cipher), + evp_cipher_wrap_pad(that.evp_cipher_wrap_pad) {} KeyAesGcm_OpenSSL::~KeyAesGcm_OpenSSL() { - context_pools.reset(); OPENSSL_cleanse(const_cast(key.data()), key.size()); } @@ -181,6 +228,11 @@ namespace ccf::crypto return key.size() * CHAR_BIT; } + std::unique_ptr KeyAesGcm_OpenSSL::make_context() + { + return std::make_unique(evp_cipher, key); + } + void KeyAesGcm_OpenSSL::encrypt( std::span iv, std::span plain, @@ -188,53 +240,10 @@ namespace ccf::crypto std::vector& cipher, uint8_t tag[GCM_SIZE_TAG]) const { - if (aad.empty() && plain.empty()) - { - throw std::logic_error("aad and plain cannot both be empty"); - } - - auto lease = context_pools->encrypt.acquire(evp_cipher, key); - auto* ctx = lease.get(); - + Unique_EVP_CIPHER_CTX context; CHECK1( - EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), nullptr)); - CHECK1(EVP_EncryptInit_ex2(ctx, nullptr, nullptr, iv.data(), nullptr)); - - if (!aad.empty()) - { - int aad_outl{0}; - CHECK1( - EVP_EncryptUpdate(ctx, nullptr, &aad_outl, aad.data(), aad.size())); - } - - std::vector ciphertext(plain.size()); - if (!plain.empty()) - { - int cypher_outl{0}; - CHECK1(EVP_EncryptUpdate( - ctx, ciphertext.data(), &cypher_outl, plain.data(), plain.size())); - - // As we use no padding, we expect the input and output lengths to match. - assert(static_cast(cypher_outl) == plain.size()); - } - - int final_outl{0}; - CHECK1(EVP_EncryptFinal_ex(ctx, nullptr, &final_outl)); - - // As long a we use GSM cipher, the final outl must be 0, because there's no - // padding and the block size is equal to 1, so EncryptUpdate() always does - // the whole thing. Final is still a must to finalize and check the error. - // - // See https://docs.openssl.org/3.3/man3/EVP_EncryptInit/#aead-interface. - assert(final_outl == 0); - - CHECK1( - EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, GCM_SIZE_TAG, &tag[0])); - - if (!plain.empty()) - { - cipher = std::move(ciphertext); - } + EVP_EncryptInit_ex2(context, evp_cipher, key.data(), nullptr, nullptr)); + encrypt_with_context(context, iv, plain, aad, cipher, tag); } bool KeyAesGcm_OpenSSL::decrypt( @@ -244,54 +253,10 @@ namespace ccf::crypto std::span aad, std::vector& plain) const { - auto lease = context_pools->decrypt.acquire(evp_cipher, key); - auto* ctx = lease.get(); - - CHECK1( - EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, iv.size(), nullptr)); - - CHECK1(EVP_DecryptInit_ex2(ctx, nullptr, nullptr, iv.data(), nullptr)); - if (!aad.empty()) - { - int aad_outl{0}; - CHECK1( - EVP_DecryptUpdate(ctx, nullptr, &aad_outl, aad.data(), aad.size())); - } - - std::vector plaintext(cipher.size()); - if (!cipher.empty()) - { - int plain_outl{0}; - CHECK1(EVP_DecryptUpdate( - ctx, plaintext.data(), &plain_outl, cipher.data(), cipher.size())); - - // As we use no padding, we expect the input and output lengths to match. - assert(static_cast(plain_outl) == cipher.size()); - } - - void* tag_ptr = const_cast(static_cast(tag)); + Unique_EVP_CIPHER_CTX context; CHECK1( - EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, GCM_SIZE_TAG, tag_ptr)); - - int final_outl{0}; - if (EVP_DecryptFinal_ex(ctx, nullptr, &final_outl) != 1) - { - return false; - } - - // As long a we use GSM cipher, the final outl must be 0, because there's no - // padding and the block size is equal to 1, so EncryptUpdate() always does - // the whole thing. Final is still a must to finalize and check the error. - // - // See https://docs.openssl.org/3.3/man3/EVP_EncryptInit/#aead-interface. - assert(final_outl == 0); - - if (!cipher.empty()) - { - plain = std::move(plaintext); - } - - return true; + EVP_DecryptInit_ex2(context, evp_cipher, key.data(), nullptr, nullptr)); + return decrypt_with_context(context, iv, tag, cipher, aad, plain); } std::vector KeyAesGcm_OpenSSL::ckm_aes_key_wrap_pad( diff --git a/src/crypto/openssl/symmetric_key.h b/src/crypto/openssl/symmetric_key.h index 86c99958fce1..49cb756219d3 100644 --- a/src/crypto/openssl/symmetric_key.h +++ b/src/crypto/openssl/symmetric_key.h @@ -12,12 +12,9 @@ namespace ccf::crypto class KeyAesGcm_OpenSSL : public KeyAesGcm { private: - struct ContextPools; - std::vector key; - OpenSSL::Unique_EVP_CIPHER evp_cipher; + const EVP_CIPHER* evp_cipher; const EVP_CIPHER* evp_cipher_wrap_pad; - std::unique_ptr context_pools; public: KeyAesGcm_OpenSSL(std::span rawKey); @@ -27,6 +24,8 @@ namespace ccf::crypto [[nodiscard]] size_t key_size() const override; + std::unique_ptr make_context() override; + void encrypt( std::span iv, std::span plain, diff --git a/src/crypto/test/bench.cpp b/src/crypto/test/bench.cpp index 748e3cbc6a9a..fca169f642d0 100644 --- a/src/crypto/test/bench.cpp +++ b/src/crypto/test/bench.cpp @@ -118,6 +118,7 @@ static void benchmark_aes_gcm_encrypt(picobench::state& s) const std::vector key(GCM_DEFAULT_KEY_SIZE, 0x42); const auto contents = make_contents(); auto aes_gcm_key = make_key_aes_gcm(key); + auto context = aes_gcm_key->make_context(); StandardGcmHeader header; std::vector cipher; uint64_t iv = 0; @@ -128,7 +129,7 @@ static void benchmark_aes_gcm_encrypt(picobench::state& s) (void)_; memcpy(header.iv.data(), &iv, sizeof(iv)); ++iv; - aes_gcm_key->encrypt(header.get_iv(), contents, {}, cipher, header.tag); + context->encrypt(header.get_iv(), contents, {}, cipher, header.tag); do_not_optimize(cipher); clobber_memory(); } diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index d9ec1437082f..730060a3fff0 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -853,10 +853,12 @@ TEST_CASE("AES-GCM context reuse") 0xbd, 0xdf}; auto aes_gcm_key = make_key_aes_gcm(key); + auto context = aes_gcm_key->make_context(); + aes_gcm_key.reset(); std::vector cipher; uint8_t tag[GCM_SIZE_TAG] = {}; - aes_gcm_key->encrypt(iv, plain, {}, cipher, tag); + context->encrypt(iv, plain, {}, cipher, tag); REQUIRE(cipher == expected_cipher); REQUIRE(std::equal(std::begin(tag), std::end(tag), std::begin(expected_tag))); @@ -866,10 +868,10 @@ TEST_CASE("AES-GCM context reuse") std::copy(std::begin(tag), std::end(tag), invalid_tag.begin()); invalid_tag[0] ^= 1; REQUIRE_FALSE( - aes_gcm_key->decrypt(iv, invalid_tag.data(), cipher, {}, decrypted)); + context->decrypt(iv, invalid_tag.data(), cipher, {}, decrypted)); REQUIRE(decrypted.empty()); - REQUIRE(aes_gcm_key->decrypt(iv, tag, cipher, {}, decrypted)); + REQUIRE(context->decrypt(iv, tag, cipher, {}, decrypted)); REQUIRE(decrypted == plain); } @@ -919,7 +921,7 @@ TEST_CASE("AES-GCM empty inputs") REQUIRE(decrypted.empty()); } -TEST_CASE("Concurrent AES-GCM context reuse") +TEST_CASE("Concurrent AES-GCM convenience calls") { constexpr size_t thread_count = 24; constexpr size_t iteration_count = 128; diff --git a/src/kv/encryptor.h b/src/kv/encryptor.h index 0e2cdfcf8559..a19a496b3901 100644 --- a/src/kv/encryptor.h +++ b/src/kv/encryptor.h @@ -82,14 +82,14 @@ namespace ccf::kv set_iv(hdr, tx_id, entry_type); - auto key = - ledger_secrets->get_encryption_key_for(tx_id.seqno, historical_hint); - if (key == nullptr) + auto secret = + ledger_secrets->get_secret_for(tx_id.seqno, historical_hint); + if (secret == nullptr) { return false; } - key->encrypt(hdr.get_iv(), plain, additional_data, cipher, hdr.tag); + secret->encrypt(hdr.get_iv(), plain, additional_data, cipher, hdr.tag); serialised_header = hdr.serialise(); @@ -125,15 +125,14 @@ namespace ccf::kv hdr.deserialise(serialised_header); term = hdr.get_term(); - auto key = - ledger_secrets->get_encryption_key_for(version, historical_hint); - if (key == nullptr) + auto secret = ledger_secrets->get_secret_for(version, historical_hint); + if (secret == nullptr) { return false; } auto ret = - key->decrypt(hdr.get_iv(), hdr.tag, cipher, additional_data, plain); + secret->decrypt(hdr.get_iv(), hdr.tag, cipher, additional_data, plain); if (!ret) { plain.resize(0); diff --git a/src/node/ledger_secret.h b/src/node/ledger_secret.h index 49355e24407b..6dd72121a5a3 100644 --- a/src/node/ledger_secret.h +++ b/src/node/ledger_secret.h @@ -5,6 +5,7 @@ #include "ccf/crypto/entropy.h" #include "ccf/crypto/hmac.h" #include "ccf/crypto/symmetric_key.h" +#include "ccf/pal/locking.h" #include "kv/kv_types.h" #include "service/tables/secrets.h" #include "service/tables/shares.h" @@ -26,6 +27,8 @@ namespace ccf { std::vector raw_key; std::shared_ptr key; + std::unique_ptr context; + ccf::pal::Mutex context_lock; std::optional previous_secret_stored_version = std::nullopt; ccf::crypto::HashBytes commit_secret; @@ -42,6 +45,28 @@ namespace ccf return commit_secret; } + void encrypt( + std::span iv, + std::span plain, + std::span aad, + std::vector& cipher, + uint8_t tag[ccf::crypto::GCM_SIZE_TAG]) + { + std::lock_guard guard(context_lock); + context->encrypt(iv, plain, aad, cipher, tag); + } + + bool decrypt( + std::span iv, + const uint8_t tag[ccf::crypto::GCM_SIZE_TAG], + std::span cipher, + std::span aad, + std::vector& plain) + { + std::lock_guard guard(context_lock); + return context->decrypt(iv, tag, cipher, aad, plain); + } + bool operator==(const LedgerSecret& other) const { return raw_key == other.raw_key && @@ -62,6 +87,7 @@ namespace ccf LedgerSecret(const LedgerSecret& other) : raw_key(other.raw_key), key(ccf::crypto::make_key_aes_gcm(other.raw_key)), + context(key->make_context()), previous_secret_stored_version(other.previous_secret_stored_version), commit_secret(derive_commit_secret(raw_key)) {} @@ -72,6 +98,7 @@ namespace ccf std::nullopt) : raw_key(raw_key_), key(ccf::crypto::make_key_aes_gcm(std::move(raw_key_))), + context(key->make_context()), previous_secret_stored_version(previous_secret_stored_version_), commit_secret(derive_commit_secret(raw_key)) {} @@ -97,7 +124,7 @@ namespace ccf encrypted_ls.deserialise(encrypted_previous_secret_raw); std::vector decrypted_ls_raw; - if (!ledger_secret->key->decrypt( + if (!ledger_secret->decrypt( encrypted_ls.hdr.get_iv(), encrypted_ls.hdr.tag, encrypted_ls.cipher, diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index 8ecce071936a..498dcb1aa042 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -13,8 +13,11 @@ #include #undef FAIL +#include +#include #include #include +#include ccf::kv::ConsensusHookPtrs hooks; using StringString = ccf::kv::Map; @@ -102,6 +105,41 @@ TEST_CASE("Simple encryption/decryption") REQUIRE(encrypt_round_trip(encryptor, plain, 6)); } +TEST_CASE("Concurrent encryption/decryption") +{ + constexpr size_t thread_count = 16; + constexpr size_t iteration_count = 64; + auto ledger_secrets = std::make_shared(); + ledger_secrets->init(); + ccf::NodeEncryptor encryptor(ledger_secrets); + std::barrier start(thread_count); + std::atomic success = true; + std::vector threads; + + for (size_t thread_index = 0; thread_index < thread_count; ++thread_index) + { + threads.emplace_back([&, thread_index]() { + start.arrive_and_wait(); + for (size_t i = 0; i < iteration_count; ++i) + { + std::vector plain(64, thread_index); + const auto version = (thread_index * iteration_count) + i + 1; + if (!encrypt_round_trip(encryptor, plain, version)) + { + success = false; + } + } + }); + } + + for (auto& thread : threads) + { + thread.join(); + } + + REQUIRE(success); +} + TEST_CASE("Subsequent ciphers from same plaintext are different") { auto ledger_secrets = std::make_shared(); From 9d7860593f9b30ee79bd93041f2bbd6c3df2b98d Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 27 Aug 2026 10:16:49 +0000 Subject: [PATCH 4/4] Update changelog and refactor OpenSSL symmetric key implementation --- CHANGELOG.md | 2 +- src/crypto/openssl/symmetric_key.cpp | 22 ++++++---------------- src/crypto/openssl/symmetric_key.h | 3 +-- src/crypto/test/crypto.cpp | 3 ++- src/node/test/encryptor.cpp | 19 +++++++++++++------ 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b805ca13a27..09473c32e6d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - C++ endpoints can now use `ccf::endpoints::Endpoint::add_openapi_response()` to document additional HTTP responses in their generated OpenAPI schema without changing the endpoint's primary success response (#8115). - New `ledger.max_transaction_size` node configuration option (default `32MB`), which caps the total serialised size of transactions written to the ledger. The limit covers the whole ledger entry: the fixed 8-byte ledger entry header, the ledger encryption header, public domain size field, public domain and encrypted private domain. It is checked before a transaction is applied, so an oversized transaction is now rejected with `413 Payload Too Large` and error code `TransactionTooLarge`, and subsequent transactions are unaffected, where previously an excessively large transaction could terminate the node. Reserved internal signature transactions are exempt because they must fill their reserved ledger version. The limit applies only to newly serialised non-reserved transactions; deserialising existing entries (including during recovery), historical queries and snapshots are unaffected, so entries written under a larger or unset limit remain readable. It must be smaller than `memory.max_msg_size` by at least the ring-buffer range response overhead, which is validated at node startup and by `--check` (#7992). -- C++ callers can use `ccf::crypto::KeyAesGcm::make_context()` to explicitly own and reuse a pre-keyed AES-GCM context when they can ensure it is not accessed concurrently (#8170). +- C++ callers can use `ccf::crypto::KeyAesGcm::make_context()` to explicitly own and reuse a pre-keyed AES-GCM context when they can ensure it is not accessed concurrently (#8178). ### Changed diff --git a/src/crypto/openssl/symmetric_key.cpp b/src/crypto/openssl/symmetric_key.cpp index 6f75cf71ce03..2926488fd510 100644 --- a/src/crypto/openssl/symmetric_key.cpp +++ b/src/crypto/openssl/symmetric_key.cpp @@ -93,7 +93,7 @@ namespace ccf::crypto int final_outl{0}; CHECK1(EVP_EncryptFinal_ex(ctx, nullptr, &final_outl)); - // As long a we use GSM cipher, the final outl must be 0, because there's + // As long as we use GCM cipher, the final outl must be 0, because there's // no padding and the block size is equal to 1, so EncryptUpdate() always // does the whole thing. Final is still a must to finalize and check the // error. @@ -104,10 +104,7 @@ namespace ccf::crypto CHECK1( EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, GCM_SIZE_TAG, &tag[0])); - if (!plain.empty()) - { - cipher = std::move(ciphertext); - } + cipher = std::move(ciphertext); } bool decrypt_with_context( @@ -145,13 +142,15 @@ namespace ccf::crypto CHECK1( EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, GCM_SIZE_TAG, tag_ptr)); + plain.clear(); + int final_outl{0}; if (EVP_DecryptFinal_ex(ctx, nullptr, &final_outl) != 1) { return false; } - // As long a we use GSM cipher, the final outl must be 0, because there's + // As long as we use GCM cipher, the final outl must be 0, because there's // no padding and the block size is equal to 1, so EncryptUpdate() always // does the whole thing. Final is still a must to finalize and check the // error. @@ -159,10 +158,7 @@ namespace ccf::crypto // See https://docs.openssl.org/3.3/man3/EVP_EncryptInit/#aead-interface. assert(final_outl == 0); - if (!cipher.empty()) - { - plain = std::move(plaintext); - } + plain = std::move(plaintext); return true; } @@ -212,12 +208,6 @@ namespace ccf::crypto evp_cipher_wrap_pad(get_wrap_pad_cipher(rawKey)) {} - KeyAesGcm_OpenSSL::KeyAesGcm_OpenSSL(KeyAesGcm_OpenSSL&& that) noexcept : - key(std::move(that.key)), - evp_cipher(that.evp_cipher), - evp_cipher_wrap_pad(that.evp_cipher_wrap_pad) - {} - KeyAesGcm_OpenSSL::~KeyAesGcm_OpenSSL() { OPENSSL_cleanse(const_cast(key.data()), key.size()); diff --git a/src/crypto/openssl/symmetric_key.h b/src/crypto/openssl/symmetric_key.h index 49cb756219d3..0425f92cd52b 100644 --- a/src/crypto/openssl/symmetric_key.h +++ b/src/crypto/openssl/symmetric_key.h @@ -12,14 +12,13 @@ namespace ccf::crypto class KeyAesGcm_OpenSSL : public KeyAesGcm { private: - std::vector key; + const std::vector key; const EVP_CIPHER* evp_cipher; const EVP_CIPHER* evp_cipher_wrap_pad; public: KeyAesGcm_OpenSSL(std::span rawKey); KeyAesGcm_OpenSSL(const KeyAesGcm_OpenSSL& that) = delete; - KeyAesGcm_OpenSSL(KeyAesGcm_OpenSSL&& that) noexcept; ~KeyAesGcm_OpenSSL() override; [[nodiscard]] size_t key_size() const override; diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index 730060a3fff0..7b987804c83f 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -887,6 +887,7 @@ TEST_CASE("AES-GCM empty inputs") aes_gcm_key->encrypt(iv, {}, aad, cipher, tag); REQUIRE(cipher.empty()); + decrypted.assign(8, 0xAB); REQUIRE(aes_gcm_key->decrypt(iv, tag, cipher, aad, decrypted)); REQUIRE(decrypted.empty()); @@ -916,7 +917,7 @@ TEST_CASE("AES-GCM empty inputs") 0xe7, 0x45, 0x5a}; - decrypted.clear(); + decrypted.assign(8, 0xAB); REQUIRE(empty_aes_gcm_key->decrypt(iv, empty_tag, {}, {}, decrypted)); REQUIRE(decrypted.empty()); } diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index 498dcb1aa042..fd46575b0e7b 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -119,16 +119,23 @@ TEST_CASE("Concurrent encryption/decryption") for (size_t thread_index = 0; thread_index < thread_count; ++thread_index) { threads.emplace_back([&, thread_index]() { - start.arrive_and_wait(); - for (size_t i = 0; i < iteration_count; ++i) + try { - std::vector plain(64, thread_index); - const auto version = (thread_index * iteration_count) + i + 1; - if (!encrypt_round_trip(encryptor, plain, version)) + start.arrive_and_wait(); + for (size_t i = 0; i < iteration_count; ++i) { - success = false; + std::vector plain(64, thread_index); + const auto version = (thread_index * iteration_count) + i + 1; + if (!encrypt_round_trip(encryptor, plain, version)) + { + success = false; + } } } + catch (...) + { + success = false; + } }); }