From c5711be2ed8d9dde2471947e818cd83463195932 Mon Sep 17 00:00:00 2001 From: achamayou Date: Thu, 3 Sep 2026 21:32:12 +0100 Subject: [PATCH 1/3] Fix endorsed certificate data race Publish immutable endorsed certificate snapshots atomically and let pending signature transactions retain the snapshot they were created with. This prevents certificate renewal from racing signature construction and changing an already queued signature. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0adcbd2f-a656-4fe9-a2fc-2b5f66c3d778 --- src/node/history.h | 22 +++++++----- src/node/test/history.cpp | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 8 deletions(-) diff --git a/src/node/history.h b/src/node/history.h index 6ebfccf01d8..362c1d6c418 100644 --- a/src/node/history.h +++ b/src/node/history.h @@ -26,7 +26,9 @@ #include "tasks/task_system.h" #include +#include #include +#include #include #define HAVE_OPENSSL @@ -313,7 +315,7 @@ namespace ccf NodeId id; ccf::crypto::ECKeyPair& node_kp; ccf::crypto::ECKeyPair_OpenSSL& service_kp; - ccf::crypto::Pem& endorsed_cert; + std::shared_ptr endorsed_cert; const ccf::COSESignaturesConfig& cose_signatures_config; const ccf::LedgerSignMode ledger_sign_mode; std::unordered_map& cose_key_cache; @@ -326,7 +328,7 @@ namespace ccf NodeId id_, ccf::crypto::ECKeyPair& node_kp_, ccf::crypto::ECKeyPair_OpenSSL& service_kp_, - ccf::crypto::Pem& endorsed_cert_, + std::shared_ptr endorsed_cert_, const ccf::COSESignaturesConfig& cose_signatures_config_, ccf::LedgerSignMode ledger_sign_mode_, std::unordered_map& cose_key_cache_) : @@ -336,7 +338,7 @@ namespace ccf id(std::move(id_)), node_kp(node_kp_), service_kp(service_kp_), - endorsed_cert(endorsed_cert_), + endorsed_cert(std::move(endorsed_cert_)), cose_signatures_config(cose_signatures_config_), ledger_sign_mode(ledger_sign_mode_), cose_key_cache(cose_key_cache_) @@ -364,7 +366,7 @@ namespace ccf root, {}, // Nonce is currently empty primary_sig, - endorsed_cert); + *endorsed_cert); signatures->put(sig_value); } @@ -577,7 +579,8 @@ namespace ccf ccf::kv::Term term_of_last_version = 0; ccf::kv::Term term_of_next_version{}; - std::optional endorsed_cert = std::nullopt; + std::atomic> endorsed_cert = + nullptr; struct ServiceSigningIdentity { @@ -943,7 +946,8 @@ namespace ccf return; } - if (!endorsed_cert.has_value()) + auto endorsed_cert_ = endorsed_cert.load(std::memory_order_acquire); + if (endorsed_cert_ == nullptr) { throw std::logic_error( fmt::format("No endorsed certificate set to emit signature")); @@ -968,7 +972,7 @@ namespace ccf id, node_kp, *signing_identity->service_kp, - endorsed_cert.value(), + std::move(endorsed_cert_), signing_identity->cose_signatures_config, signing_identity->ledger_sign_mode, cose_key_cache), @@ -1022,7 +1026,9 @@ namespace ccf void set_endorsed_certificate(const ccf::crypto::Pem& cert) override { - endorsed_cert = cert; + endorsed_cert.store( + std::make_shared(cert), + std::memory_order_release); } private: diff --git a/src/node/test/history.cpp b/src/node/test/history.cpp index 83d46c1a520..8d40fbc7cba 100644 --- a/src/node/test/history.cpp +++ b/src/node/test/history.cpp @@ -19,6 +19,7 @@ #include #undef FAIL +#include #include #include @@ -314,6 +315,76 @@ class TestPendingTx : public ccf::kv::PendingTx } }; +TEST_CASE("Pending signatures retain their endorsed certificate") +{ + auto encryptor = std::make_shared(); + auto consensus = std::make_shared(); + auto node_kp = ccf::crypto::make_ec_key_pair(); + auto service_kp = std::dynamic_pointer_cast( + ccf::crypto::make_ec_key_pair()); + + const auto first_cert = + node_kp->self_sign("CN=First Node", valid_from, valid_to); + const auto second_cert = + node_kp->self_sign("CN=Second Node", valid_from, valid_to); + + ccf::kv::Store store; + store.set_encryptor(encryptor); + store.set_consensus(consensus); + + auto history = std::make_shared( + store, ccf::kv::test::PrimaryNodeId, *node_kp); + history->set_endorsed_certificate(first_cert); + history->set_service_signing_identity( + service_kp, ccf::COSESignaturesConfig{}); + store.set_history(history); + + constexpr auto store_term = 2; + store.initialise_term(store_term); + + MapT table("public:table"); + const auto gap_txid = store.next_txid(); + + history->emit_signature(); + REQUIRE(consensus->number_of_replicas() == 0); + + history->set_endorsed_certificate(second_cert); + REQUIRE( + store.commit( + gap_txid, + std::make_unique(gap_txid, store, table), + false) == ccf::kv::CommitResult::SUCCESS); + + auto tx = store.create_read_only_tx(); + auto signatures = tx.ro(ccf::Tables::SIGNATURES); + const auto signature = signatures->get(); + REQUIRE(signature.has_value()); + REQUIRE(signature->cert == first_cert); + + std::atomic updater_started = false; + std::jthread updater([&](std::stop_token stop_token) { + updater_started.store(true, std::memory_order_release); + while (!stop_token.stop_requested()) + { + history->set_endorsed_certificate(first_cert); + history->set_endorsed_certificate(second_cert); + } + }); + + while (!updater_started.load(std::memory_order_acquire)) + { + std::this_thread::yield(); + } + + for (size_t i = 0; i < 32; ++i) + { + history->emit_signature(); + } + + updater.request_stop(); + updater.join(); +} + struct PausedSignatureCommit { ccf::ds::Mutex lock; From d5aeaf55bb14da526164f2aa65cfb6b7e877bb64 Mon Sep 17 00:00:00 2001 From: achamayou Date: Thu, 3 Sep 2026 22:10:50 +0100 Subject: [PATCH 2/3] Include stop token explicitly Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0adcbd2f-a656-4fe9-a2fc-2b5f66c3d778 --- src/node/test/history.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/test/history.cpp b/src/node/test/history.cpp index 8d40fbc7cba..4bdd1e80c0a 100644 --- a/src/node/test/history.cpp +++ b/src/node/test/history.cpp @@ -21,6 +21,7 @@ #include #include +#include #include using MapT = ccf::kv::Map; From b0eb6e8ab35f388a6bd93b51e4df59dc2f0217bf Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 4 Sep 2026 10:10:00 +0100 Subject: [PATCH 3/3] Add thread yield after setting endorsed certificates Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/node/test/history.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/test/history.cpp b/src/node/test/history.cpp index 4bdd1e80c0a..71d0d7abdf0 100644 --- a/src/node/test/history.cpp +++ b/src/node/test/history.cpp @@ -369,6 +369,7 @@ TEST_CASE("Pending signatures retain their endorsed certificate") { history->set_endorsed_certificate(first_cert); history->set_endorsed_certificate(second_cert); + std::this_thread::yield(); } });