From c74502c2a705fa17d578e46f670d63c59dbf2914 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 09:28:05 +0000 Subject: [PATCH 01/35] Historical queries after rekey --- CMakeLists.txt | 15 ----------- src/kv/encryptor.h | 11 +++++--- src/kv/generic_serialise_wrapper.h | 5 ++-- src/kv/kv_types.h | 3 ++- src/kv/store.h | 8 +++--- src/node/historical_queries.h | 2 +- src/node/ledger_secrets.h | 42 ++++++++++++++++++++---------- src/node/node_state.h | 9 +++++++ tests/suite/test_suite.py | 16 +----------- 9 files changed, 56 insertions(+), 55 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cf857476229..f0eedf56262 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -564,21 +564,6 @@ if(BUILD_TESTS) 4000 ) - add_e2e_test( - NAME reconfiguration_rekey_test_suite - PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/e2e_suite.py - CONSENSUS cft - LABEL suite - ADDITIONAL_ARGS - --test-duration - 200 - --enforce-reqs - --test-suite - rekey_reconfiguration - --raft-election-timeout - 4000 - ) - add_e2e_test( NAME full_test_suite PYTHON_SCRIPT ${CMAKE_SOURCE_DIR}/tests/e2e_suite.py diff --git a/src/kv/encryptor.h b/src/kv/encryptor.h index 30fbf4d1aaa..e7bffbe81fa 100644 --- a/src/kv/encryptor.h +++ b/src/kv/encryptor.h @@ -91,6 +91,8 @@ namespace kv * @param[out] plain Decrypted plaintext * @param[in] version Version used to retrieve the corresponding * encryption key + * @param[in] is_historical If set, only consider encryption keys + * that have not been compacted away (used to decrypt historical queries) * * @return Boolean status indicating success of decryption. */ @@ -99,14 +101,17 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version) override + Version version, + bool is_historical = false) override { S hdr; hdr.deserialise(serialised_header); plain.resize(cipher.size()); - auto ret = ledger_secrets->get_encryption_key_for(version)->decrypt( - hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data()); + auto ret = + ledger_secrets->get_encryption_key_for(version, is_historical) + ->decrypt( + hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data()); if (!ret) { plain.resize(0); diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index 8a88d410d0c..ab07b563f72 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -313,7 +313,7 @@ namespace kv {} std::optional> init( - const uint8_t* data, size_t size) + const uint8_t* data, size_t size, bool is_historical = false) { current_reader = &public_reader; auto data_ = data; @@ -356,7 +356,8 @@ namespace kv {data_public, data_public + public_domain_length}, {data, data + crypto_util->get_header_length()}, decrypted_buffer, - version)) + version, + is_historical)) { return std::nullopt; } diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 37bf9f8e9d7..115916c76dd 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -437,7 +437,8 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version) = 0; + Version version, + bool is_historical = false) = 0; virtual void compact(Version version) = 0; virtual void rollback(Version version) = 0; diff --git a/src/kv/store.h b/src/kv/store.h index c5485741bd3..14710ae33b8 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -79,7 +79,7 @@ namespace kv // is used for historical queries, where it may deserialise arbitrary // transactions. In this case the Store is a useful container for a set of // Tables, but its versioning invariants are ignored. - const bool strict_versions = true; + const bool is_historical = false; DeserialiseSuccess commit_deserialised( OrderedChanges& changes, @@ -112,7 +112,7 @@ namespace kv } public: - Store(bool strict_versions_ = true) : strict_versions(strict_versions_) {} + Store(bool is_historical_ = false) : is_historical(is_historical_) {} Store( const ReplicateType& replicate_type_, @@ -629,7 +629,7 @@ namespace kv public_only ? kv::SecurityDomain::PUBLIC : std::optional()); - auto v_ = d.init(data.data(), data.size()); + auto v_ = d.init(data.data(), data.size(), is_historical); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); @@ -641,7 +641,7 @@ namespace kv // consensus. rollback(v - 1); - if (strict_versions) + if (!is_historical) { // Make sure this is the next transaction. auto cv = current_version(); diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index 6a2a2d9d3b2..44a9ce33276 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -196,7 +196,7 @@ namespace ccf::historical void deserialise_ledger_entry( consensus::Index idx, const LedgerEntry& entry) { - StorePtr store = std::make_shared(false); + StorePtr store = std::make_shared(true); store->set_encryptor(source_store.get_encryptor()); kv::ConsensusHookPtrs hooks; diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 02cbc41acaf..f5e8a55cc7f 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -54,16 +54,30 @@ namespace ccf SpinLock lock; LedgerSecretsMap ledger_secrets; - std::optional commit_key_it = std::nullopt; + std::optional first_non_compacted_it = + std::nullopt; - LedgerSecretsMap::iterator get_encryption_key_it(kv::Version version) + LedgerSecretsMap::iterator get_secret_it( + kv::Version version, bool is_historical = false) { - // Encryption key for a given version is the one with the highest version - // that is lower than the given version (e.g. if encryption_keys contains - // two keys for version 0 and 10 then the key associated with version 0 - // is used for version [0..9] and version 10 for versions 10+) - - auto search_begin = commit_key_it.value_or(ledger_secrets.begin()); + // The ledger secret used to encrypt/decrypt a transaction at a given + // version is the one with the highest version that is lower than the + // given version (e.g. if ledger_secrets contains two keys for version 0 + // and 10 then the key associated with version 0 is used for version + // [0..9] and version 10 for versions 10+) + + // Unless is_historical is true, only consider ledger secrets that + // have not been compacted. Otherwise, consider all ledger secrets since + // the start of time. + LedgerSecretsMap::iterator search_begin; + if (is_historical) + { + search_begin = ledger_secrets.begin(); + } + else + { + search_begin = first_non_compacted_it.value_or(ledger_secrets.begin()); + } auto search = std::upper_bound( search_begin, ledger_secrets.end(), version, [](auto a, const auto& b) { @@ -201,13 +215,13 @@ namespace ccf } ledger_secrets.merge(restored_ledger_secrets); - commit_key_it = ledger_secrets.begin(); + first_non_compacted_it = ledger_secrets.begin(); } - auto get_encryption_key_for(kv::Version version) + auto get_encryption_key_for(kv::Version version, bool is_historical = false) { std::lock_guard guard(lock); - return get_encryption_key_it(version)->second.key; + return get_secret_it(version, is_historical)->second.key; } void set_secret(kv::Version version, LedgerSecret&& secret) @@ -232,7 +246,7 @@ namespace ccf return; } - auto start = commit_key_it.value_or(ledger_secrets.begin()); + auto start = first_non_compacted_it.value_or(ledger_secrets.begin()); if (version < start->first) { LOG_FAIL_FMT( @@ -263,10 +277,10 @@ namespace ccf return; } - commit_key_it = get_encryption_key_it(version); + first_non_compacted_it = get_secret_it(version); LOG_TRACE_FMT( "First usable encryption key is now at seqno {}", - commit_key_it.value()->first); + first_non_compacted_it.value()->first); } }; } diff --git a/src/node/node_state.h b/src/node/node_state.h index 1985b1c635e..5e8dc3a624d 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -467,6 +467,15 @@ namespace ccf network.identity = std::make_unique(resp.network_info.identity); + LOG_FAIL_FMT( + "Start node with {} ledger secrets", + resp.network_info.ledger_secrets.size()); + + for (auto const& s : resp.network_info.ledger_secrets) + { + LOG_FAIL_FMT("LS from {}", s.first); + } + network.ledger_secrets = std::make_shared( self, std::move(resp.network_info.ledger_secrets)); diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 1ed5fffe24d..87742e047b7 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -43,12 +43,10 @@ # This suite tests that nodes addition, deletion and primary changes # can be interleaved -# Note: snapshot tests are not yet integrated in the main test suite -# as they test historical queries which do not yet work across rekey/recovery -# https://github.com/microsoft/CCF/issues/1648 suite_reconfiguration = [ reconfiguration.test_add_node_from_snapshot, reconfiguration.test_retire_primary, + rekey.test, reconfiguration.test_add_node, election.test_kill_primary, reconfiguration.test_add_node, @@ -60,18 +58,6 @@ ] suites["reconfiguration"] = suite_reconfiguration -# Slightly different reconfiguration suite that also interleaves rekeying -# Once https://github.com/microsoft/CCF/issues/1648 is complete, this can -# be merged with the main reconfiguration suite -suite_rekey_reconfiguration = [ - reconfiguration.test_add_node, - reconfiguration.test_retire_primary, - rekey.test, - reconfiguration.test_add_node, - election.test_kill_primary, - reconfiguration.test_add_node, -] -suites["rekey_reconfiguration"] = suite_rekey_reconfiguration all_tests_suite = [ # e2e_logging: From 8e4a2791144f1c3e573bc4bf3a334347293fbdd0 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 09:28:59 +0000 Subject: [PATCH 02/35] Cleanup PR --- src/node/node_state.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 5e8dc3a624d..1985b1c635e 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -467,15 +467,6 @@ namespace ccf network.identity = std::make_unique(resp.network_info.identity); - LOG_FAIL_FMT( - "Start node with {} ledger secrets", - resp.network_info.ledger_secrets.size()); - - for (auto const& s : resp.network_info.ledger_secrets) - { - LOG_FAIL_FMT("LS from {}", s.first); - } - network.ledger_secrets = std::make_shared( self, std::move(resp.network_info.ledger_secrets)); From d8ce3d12fdb84bfd36a2212526989d8459d72152 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 12:07:06 +0000 Subject: [PATCH 03/35] Fix null encryptor --- src/kv/test/null_encryptor.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/kv/test/null_encryptor.h b/src/kv/test/null_encryptor.h index 19ff2167db1..6cf0dbb62d9 100644 --- a/src/kv/test/null_encryptor.h +++ b/src/kv/test/null_encryptor.h @@ -26,7 +26,8 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version) override + Version version, + bool is_historical = false) override { plain = cipher; return true; From 010347272bf35ecbfc845653b04c29522948b013 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 15:31:03 +0000 Subject: [PATCH 04/35] Remove flag and use hot path --- src/kv/encryptor.h | 38 ++------------- src/kv/generic_serialise_wrapper.h | 5 +- src/kv/kv_types.h | 18 ++------ src/kv/store.h | 14 ++---- src/kv/test/null_encryptor.h | 5 +- src/node/historical_queries.h | 2 +- src/node/ledger_secrets.h | 74 ++++++++++++------------------ src/node/node_state.h | 12 ++--- tests/e2e_suite.py | 2 +- 9 files changed, 51 insertions(+), 119 deletions(-) diff --git a/src/kv/encryptor.h b/src/kv/encryptor.h index e7bffbe81fa..4050a1aef5b 100644 --- a/src/kv/encryptor.h +++ b/src/kv/encryptor.h @@ -14,7 +14,6 @@ namespace kv { private: std::shared_ptr ledger_secrets; - bool is_recovery = false; void set_iv(S& hdr, TxID tx_id, bool is_snapshot = false) { @@ -34,15 +33,7 @@ namespace kv } public: - TxEncryptor(std::shared_ptr secrets, bool is_recovery_ = false) : - ledger_secrets(secrets), - is_recovery(is_recovery_) - {} - - void disable_recovery() override - { - is_recovery = false; - } + TxEncryptor(std::shared_ptr secrets) : ledger_secrets(secrets) {} size_t get_header_length() override { @@ -91,8 +82,6 @@ namespace kv * @param[out] plain Decrypted plaintext * @param[in] version Version used to retrieve the corresponding * encryption key - * @param[in] is_historical If set, only consider encryption keys - * that have not been compacted away (used to decrypt historical queries) * * @return Boolean status indicating success of decryption. */ @@ -101,17 +90,14 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version, - bool is_historical = false) override + Version version) override { S hdr; hdr.deserialise(serialised_header); plain.resize(cipher.size()); - auto ret = - ledger_secrets->get_encryption_key_for(version, is_historical) - ->decrypt( - hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data()); + auto ret = ledger_secrets->get_encryption_key_for(version)->decrypt( + hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data()); if (!ret) { plain.resize(0); @@ -131,21 +117,5 @@ namespace kv // will fail. ledger_secrets->rollback(version); } - - void compact(Version version) override - { - // Advances the commit point to version, so that encryption keys that - // will no longer be used are not considered when selecting an encryption - // key. - // Note: Encryption keys are still kept in memory to be passed on to new - // nodes joining the service. - - // Do not compact ledger secrets on recovery, as all historical secrets - // are used to decrypt the historical ledger - if (!is_recovery) - { - ledger_secrets->compact(version); - } - } }; } \ No newline at end of file diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index ab07b563f72..8a88d410d0c 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -313,7 +313,7 @@ namespace kv {} std::optional> init( - const uint8_t* data, size_t size, bool is_historical = false) + const uint8_t* data, size_t size) { current_reader = &public_reader; auto data_ = data; @@ -356,8 +356,7 @@ namespace kv {data_public, data_public + public_domain_length}, {data, data + crypto_util->get_header_length()}, decrypted_buffer, - version, - is_historical)) + version)) { return std::nullopt; } diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index 115916c76dd..ada1203ef4d 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -202,14 +202,7 @@ namespace kv } }; - class Syncable - { - public: - virtual void rollback(Version v) = 0; - virtual void compact(Version v) = 0; - }; - - class TxHistory : public Syncable + class TxHistory { public: using RequestID = std::tuple< @@ -265,6 +258,8 @@ namespace kv const std::vector& request, uint8_t frame_format) = 0; virtual void append(const std::vector& replicated) = 0; + virtual void rollback(Version v) = 0; + virtual void compact(Version v) = 0; }; class Consensus : public ConfigurableConsensus @@ -420,7 +415,7 @@ namespace kv } }; - class AbstractTxEncryptor : public Syncable + class AbstractTxEncryptor { public: virtual ~AbstractTxEncryptor() {} @@ -437,14 +432,11 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version, - bool is_historical = false) = 0; + Version version) = 0; - virtual void compact(Version version) = 0; virtual void rollback(Version version) = 0; virtual size_t get_header_length() = 0; - virtual void disable_recovery() = 0; }; using EncryptorPtr = std::shared_ptr; diff --git a/src/kv/store.h b/src/kv/store.h index 14710ae33b8..6af89db3a5c 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -79,7 +79,7 @@ namespace kv // is used for historical queries, where it may deserialise arbitrary // transactions. In this case the Store is a useful container for a set of // Tables, but its versioning invariants are ignored. - const bool is_historical = false; + const bool strict_versions = true; DeserialiseSuccess commit_deserialised( OrderedChanges& changes, @@ -112,7 +112,7 @@ namespace kv } public: - Store(bool is_historical_ = false) : is_historical(is_historical_) {} + Store(bool strict_versions_ = true) : strict_versions(strict_versions_) {} Store( const ReplicateType& replicate_type_, @@ -510,12 +510,6 @@ namespace kv { h->compact(v); } - - auto e = get_encryptor(); - if (e) - { - e->compact(v); - } } for (auto& it : maps) @@ -629,7 +623,7 @@ namespace kv public_only ? kv::SecurityDomain::PUBLIC : std::optional()); - auto v_ = d.init(data.data(), data.size(), is_historical); + auto v_ = d.init(data.data(), data.size()); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); @@ -641,7 +635,7 @@ namespace kv // consensus. rollback(v - 1); - if (!is_historical) + if (strict_versions) { // Make sure this is the next transaction. auto cv = current_version(); diff --git a/src/kv/test/null_encryptor.h b/src/kv/test/null_encryptor.h index 6cf0dbb62d9..82331932d56 100644 --- a/src/kv/test/null_encryptor.h +++ b/src/kv/test/null_encryptor.h @@ -26,8 +26,7 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version, - bool is_historical = false) override + Version version) override { plain = cipher; return true; @@ -38,8 +37,6 @@ namespace kv return 0; } - void disable_recovery() override {} - void rollback(Version version) override {} void compact(Version version) override {} }; diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index 44a9ce33276..6a2a2d9d3b2 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -196,7 +196,7 @@ namespace ccf::historical void deserialise_ledger_entry( consensus::Index idx, const LedgerEntry& entry) { - StorePtr store = std::make_shared(true); + StorePtr store = std::make_shared(false); store->set_encryptor(source_store.get_encryptor()); kv::ConsensusHookPtrs hooks; diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index f5e8a55cc7f..f348e10f900 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -54,11 +54,10 @@ namespace ccf SpinLock lock; LedgerSecretsMap ledger_secrets; - std::optional first_non_compacted_it = - std::nullopt; + // std::optional first_non_compacted_it = + // std::nullopt; - LedgerSecretsMap::iterator get_secret_it( - kv::Version version, bool is_historical = false) + LedgerSecret get_secret_for_version(kv::Version version) { // The ledger secret used to encrypt/decrypt a transaction at a given // version is the one with the highest version that is lower than the @@ -66,30 +65,29 @@ namespace ccf // and 10 then the key associated with version 0 is used for version // [0..9] and version 10 for versions 10+) - // Unless is_historical is true, only consider ledger secrets that - // have not been compacted. Otherwise, consider all ledger secrets since - // the start of time. - LedgerSecretsMap::iterator search_begin; - if (is_historical) + if (ledger_secrets.empty()) { - search_begin = ledger_secrets.begin(); + throw std::logic_error("Ledger secrets map is empty"); } - else + + if (version >= ledger_secrets.rbegin()->first) { - search_begin = first_non_compacted_it.value_or(ledger_secrets.begin()); + // Hot path for encrypting/decrypting transactions since the latest + // rekey + return ledger_secrets.rbegin()->second; } auto search = std::upper_bound( - search_begin, ledger_secrets.end(), version, [](auto a, const auto& b) { - return b.first > a; - }); - if (search == search_begin) + ledger_secrets.begin(), + ledger_secrets.end(), + version, + [](auto a, const auto& b) { return b.first > a; }); + if (search == ledger_secrets.begin()) { - throw std::logic_error(fmt::format( - "kv::TxEncryptor: could not find ledger encryption key for seqno {}", - version)); + throw std::logic_error( + fmt::format("Could not find ledger secret for seqno {}", version)); } - return --search; + return (--search)->second; } void take_dependency_on_secrets(kv::ReadOnlyTx& tx) @@ -215,13 +213,13 @@ namespace ccf } ledger_secrets.merge(restored_ledger_secrets); - first_non_compacted_it = ledger_secrets.begin(); + // first_non_compacted_it = ledger_secrets.begin(); } - auto get_encryption_key_for(kv::Version version, bool is_historical = false) + auto get_encryption_key_for(kv::Version version) { std::lock_guard guard(lock); - return get_secret_it(version, is_historical)->second.key; + return get_secret_for_version(version).key; } void set_secret(kv::Version version, LedgerSecret&& secret) @@ -230,12 +228,12 @@ namespace ccf CCF_ASSERT_FMT( ledger_secrets.find(version) == ledger_secrets.end(), - "Encryption key at {} already exists", + "Ledger secret at seqno {} already exists", version); ledger_secrets.emplace(version, std::move(secret)); - LOG_INFO_FMT("Added new encryption key at seqno {}", version); + LOG_INFO_FMT("Added new ledger secret at seqno {}", version); } void rollback(kv::Version version) @@ -246,17 +244,17 @@ namespace ccf return; } - auto start = first_non_compacted_it.value_or(ledger_secrets.begin()); - if (version < start->first) + // auto start = first_non_compacted_it.value_or(ledger_secrets.begin()); + if (version < ledger_secrets.begin()->first) { LOG_FAIL_FMT( - "Cannot rollback encryptor at {}: committed key is at {}", + "Cannot rollback ledger secrets at {}: first secret is at {}", version, - start->first); + ledger_secrets.begin()->first); return; } - while (std::distance(start, ledger_secrets.end()) > 1) + while (ledger_secrets.size() > 1) { auto k = ledger_secrets.rbegin(); if (k->first <= version) @@ -264,23 +262,9 @@ namespace ccf break; } - LOG_TRACE_FMT("Rollback encryption key at seqno {}", k->first); + LOG_TRACE_FMT("Rollback ledger secrets at seqno {}", k->first); ledger_secrets.erase(k->first); } } - - void compact(kv::Version version) - { - std::lock_guard guard(lock); - if (ledger_secrets.empty()) - { - return; - } - - first_non_compacted_it = get_secret_it(version); - LOG_TRACE_FMT( - "First usable encryption key is now at seqno {}", - first_non_compacted_it.value()->first); - } }; } diff --git a/src/node/node_state.h b/src/node/node_state.h index 1985b1c635e..a2be62d4b7f 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -479,7 +479,7 @@ namespace ccf resp.network_info.consensus_type)); } - setup_encryptor(resp.network_info.public_only); + setup_encryptor(); setup_consensus(resp.network_info.public_only); setup_progress_tracker(); setup_history(); @@ -868,7 +868,7 @@ namespace ccf network.ledger_secrets->init(last_recovered_signed_idx + 1); network.ledger_secrets->set_node_id(self); - setup_encryptor(true); + setup_encryptor(); LOG_INFO_FMT("Deleted previous nodes and added self as {}", self); @@ -994,9 +994,6 @@ namespace ccf // Raft should deserialise all security domains when network is opened consensus->enable_all_domains(); - // Disable recovery so that only non-compacted keys are used - encryptor->disable_recovery(); - // Snapshots are only generated after recovery is complete snapshotter->set_tx_interval(recovery_snapshot_tx_interval); @@ -1728,7 +1725,7 @@ namespace ccf network.tables->set_history(history); } - void setup_encryptor(bool recovery = false) + void setup_encryptor() { // This function makes use of ledger secrets and should be called once // the node has joined the service (either via start_network() or @@ -1736,8 +1733,7 @@ namespace ccf #ifdef USE_NULL_ENCRYPTOR encryptor = std::make_shared(); #else - encryptor = - std::make_shared(network.ledger_secrets, recovery); + encryptor = std::make_shared(network.ledger_secrets); #endif network.tables->set_encryptor(encryptor); diff --git a/tests/e2e_suite.py b/tests/e2e_suite.py index a89246c3ea6..f1faa1e141c 100644 --- a/tests/e2e_suite.py +++ b/tests/e2e_suite.py @@ -107,7 +107,7 @@ def run(args): "name": s.test_name(test), "status": status.name, "elapsed (s)": round(test_elapsed, 2), - "memory": mem_stats(network), + "memory": mem_stats(new_network), } if reason is not None: From 9b37218c8968c2391e9fb87f885d9098b90374a9 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 15:35:49 +0000 Subject: [PATCH 05/35] Cleanup --- src/node/ledger_secrets.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index f348e10f900..e4a6aaad62a 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -54,8 +54,6 @@ namespace ccf SpinLock lock; LedgerSecretsMap ledger_secrets; - // std::optional first_non_compacted_it = - // std::nullopt; LedgerSecret get_secret_for_version(kv::Version version) { @@ -213,7 +211,6 @@ namespace ccf } ledger_secrets.merge(restored_ledger_secrets); - // first_non_compacted_it = ledger_secrets.begin(); } auto get_encryption_key_for(kv::Version version) @@ -244,7 +241,6 @@ namespace ccf return; } - // auto start = first_non_compacted_it.value_or(ledger_secrets.begin()); if (version < ledger_secrets.begin()->first) { LOG_FAIL_FMT( From bc12dca531d1c42d46a7de88eec2cf707e4844e4 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 15:51:26 +0000 Subject: [PATCH 06/35] Fix null encryptor --- src/kv/test/null_encryptor.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/kv/test/null_encryptor.h b/src/kv/test/null_encryptor.h index 82331932d56..1d6421fc29a 100644 --- a/src/kv/test/null_encryptor.h +++ b/src/kv/test/null_encryptor.h @@ -38,6 +38,5 @@ namespace kv } void rollback(Version version) override {} - void compact(Version version) override {} }; } \ No newline at end of file From 7177d70bc2f770d5411194ad1d0a4cfef5b94aa0 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 21 Jan 2021 17:26:03 +0000 Subject: [PATCH 07/35] Remove compaction --- src/node/test/encryptor.cpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index 01ae7d49100..c5b17361da5 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -321,7 +321,7 @@ TEST_CASE("KV integrity verification") kv::DeserialiseSuccess::FAILED); } -TEST_CASE("Encryptor compaction and rollback") +TEST_CASE("Encryptor rollback") { StringString map("map"); kv::Store store; @@ -360,18 +360,6 @@ TEST_CASE("Encryptor compaction and rollback") commit_one(store, map); ledger_secrets->set_secret(7, ccf::make_ledger_secret()); - store.compact(4); - encryptor->rollback(1); // No effect as rollback before commit point - - commit_one(store, map); - - encryptor->compact(7); - - commit_one(store, map); - commit_one(store, map); - - store.rollback(7); // No effect as rollback unique encryption key - commit_one(store, map); commit_one(store, map); } \ No newline at end of file From e75a280f7e8e3149b9601c507d85320a8d1f97d2 Mon Sep 17 00:00:00 2001 From: Julien Maffre <42961061+jumaffre@users.noreply.github.com> Date: Fri, 22 Jan 2021 09:32:42 +0000 Subject: [PATCH 08/35] Update src/node/ledger_secrets.h Co-authored-by: Eddy Ashton --- src/node/ledger_secrets.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index e4a6aaad62a..8976f362182 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -85,7 +85,7 @@ namespace ccf throw std::logic_error( fmt::format("Could not find ledger secret for seqno {}", version)); } - return (--search)->second; + return std::prev(search)->second; } void take_dependency_on_secrets(kv::ReadOnlyTx& tx) From 970baefd4cda1dea8bcb0f253e6c982daba128a9 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 11:11:17 +0000 Subject: [PATCH 09/35] Performance! --- src/node/ledger_secrets.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 8976f362182..92c04ccb6cf 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -55,7 +55,7 @@ namespace ccf SpinLock lock; LedgerSecretsMap ledger_secrets; - LedgerSecret get_secret_for_version(kv::Version version) + LedgerSecret& get_secret_for_version(kv::Version version) { // The ledger secret used to encrypt/decrypt a transaction at a given // version is the one with the highest version that is lower than the From f2a4b50c73a7a4dcb4668ad6acf54c8194a1b015 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 14:59:35 +0000 Subject: [PATCH 10/35] WIP: Faster on backup catchup --- src/kv/encryptor.h | 12 +++- src/kv/kv_types.h | 3 +- src/kv/test/stub_consensus.h | 15 +--- src/node/ledger_secrets.h | 69 ++++++++++++++---- src/node/test/encryptor.cpp | 135 ++++++++++++++++++++++++++--------- 5 files changed, 170 insertions(+), 64 deletions(-) diff --git a/src/kv/encryptor.h b/src/kv/encryptor.h index 4050a1aef5b..9d2abd8f074 100644 --- a/src/kv/encryptor.h +++ b/src/kv/encryptor.h @@ -82,6 +82,9 @@ namespace kv * @param[out] plain Decrypted plaintext * @param[in] version Version used to retrieve the corresponding * encryption key + * @param[in] historical_hint If true, considers all ledger secrets for + * decryption. Otherwise, try to use the latest used secret (defaults to + * false) * * @return Boolean status indicating success of decryption. */ @@ -90,14 +93,17 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version) override + Version version, + bool historical_hint = false) override { S hdr; hdr.deserialise(serialised_header); plain.resize(cipher.size()); - auto ret = ledger_secrets->get_encryption_key_for(version)->decrypt( - hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data()); + auto ret = + ledger_secrets->get_encryption_key_for(version, historical_hint) + ->decrypt( + hdr.get_iv(), hdr.tag, cipher, additional_data, plain.data()); if (!ret) { plain.resize(0); diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index ada1203ef4d..bad9a8ef59d 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -432,7 +432,8 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version) = 0; + Version version, + bool historical_hint = false) = 0; virtual void rollback(Version version) = 0; diff --git a/src/kv/test/stub_consensus.h b/src/kv/test/stub_consensus.h index ee4deacf571..a9faa85071d 100644 --- a/src/kv/test/stub_consensus.h +++ b/src/kv/test/stub_consensus.h @@ -8,6 +8,7 @@ #include #include +#include namespace kv { @@ -50,20 +51,6 @@ namespace kv } } - std::optional> pop_oldest_data() - { - if (!replica.empty()) - { - auto data = *std::get<1>(replica.front()); - replica.erase(replica.begin()); - return data; - } - else - { - return std::nullopt; - } - } - std::optional pop_oldest_entry() { if (!replica.empty()) diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 92c04ccb6cf..962620dbc9d 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -55,37 +55,73 @@ namespace ccf SpinLock lock; LedgerSecretsMap ledger_secrets; - LedgerSecret& get_secret_for_version(kv::Version version) - { - // The ledger secret used to encrypt/decrypt a transaction at a given - // version is the one with the highest version that is lower than the - // given version (e.g. if ledger_secrets contains two keys for version 0 - // and 10 then the key associated with version 0 is used for version - // [0..9] and version 10 for versions 10+) + std::optional last_used_secret_it = + std::nullopt; + const LedgerSecret& get_secret_for_version( + kv::Version version, bool historical_hint = false) + { if (ledger_secrets.empty()) { throw std::logic_error("Ledger secrets map is empty"); } - if (version >= ledger_secrets.rbegin()->first) + if (!historical_hint) { - // Hot path for encrypting/decrypting transactions since the latest - // rekey - return ledger_secrets.rbegin()->second; + if (last_used_secret_it.has_value()) + { + auto& last_used_secret_it_ = last_used_secret_it.value(); + if ( + std::next(last_used_secret_it_) == ledger_secrets.end() || + version < std::next(last_used_secret_it_)->first) + { + // Hot path on backups, when decrypting in sequence after having + // been given access to historical secrets + LOG_FAIL_FMT("Fast path backup"); + return last_used_secret_it_->second; + } + else + { + // After a rekey, the next ledger secret should be used + LOG_FAIL_FMT("Fast path update backup!"); + ++last_used_secret_it_; + return last_used_secret_it_->second; + } + } + else + { + auto latest = std::prev(ledger_secrets.end()); + if (version >= latest->first) + { + // Hot path when encrypting/decrypting latest entries + last_used_secret_it = latest; + return latest->second; + } + } } + // Slow path, e.g. historical queries or start of backup replay + LOG_FAIL_FMT("Slow path!"); + + // The ledger secret used to encrypt/decrypt a transaction at a given + // version is the one with the highest version that is lower than the + // given version (e.g. if ledger_secrets contains two keys for version 0 + // and 10 then the key associated with version 0 is used for version + // [0..9] and version 10 for versions 10+) auto search = std::upper_bound( ledger_secrets.begin(), ledger_secrets.end(), version, [](auto a, const auto& b) { return b.first > a; }); + if (search == ledger_secrets.begin()) { throw std::logic_error( fmt::format("Could not find ledger secret for seqno {}", version)); } - return std::prev(search)->second; + + last_used_secret_it = std::prev(search); + return last_used_secret_it.value()->second; } void take_dependency_on_secrets(kv::ReadOnlyTx& tx) @@ -213,10 +249,11 @@ namespace ccf ledger_secrets.merge(restored_ledger_secrets); } - auto get_encryption_key_for(kv::Version version) + auto get_encryption_key_for( + kv::Version version, bool historical_hint = false) { std::lock_guard guard(lock); - return get_secret_for_version(version).key; + return get_secret_for_version(version, historical_hint).key; } void set_secret(kv::Version version, LedgerSecret&& secret) @@ -261,6 +298,10 @@ namespace ccf LOG_TRACE_FMT("Rollback ledger secrets at seqno {}", k->first); ledger_secrets.erase(k->first); } + + // Optimistically assume that the next operation will use the first + // non-rollback secret + last_used_secret_it = std::prev(ledger_secrets.end()); } }; } diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index c5b17361da5..030cb08c378 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -3,13 +3,12 @@ #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN #include "kv/encryptor.h" - #include "kv/kv_types.h" +#include "kv/store.h" #include "kv/test/stub_consensus.h" #include "node/encryptor.h" #include "node/entities.h" #include "node/ledger_secrets.h" -#include "node/network_state.h" #include #include @@ -71,7 +70,6 @@ bool corrupt_serialised_tx( TEST_CASE("Simple encryption/decryption") { - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); @@ -102,7 +100,6 @@ TEST_CASE("Simple encryption/decryption") TEST_CASE("Subsequent ciphers from same plaintext are different") { - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); @@ -131,7 +128,6 @@ TEST_CASE("Subsequent ciphers from same plaintext are different") TEST_CASE("Ciphers at same seqno with different terms are different") { - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); @@ -159,7 +155,6 @@ TEST_CASE("Ciphers at same seqno with different terms are different") TEST_CASE("Ciphers at same seqno/term with and without snapshot are different") { - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); @@ -199,7 +194,6 @@ TEST_CASE("Ciphers at same seqno/term with and without snapshot are different") TEST_CASE("Additional data") { - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); @@ -239,47 +233,126 @@ TEST_CASE("KV encryption/decryption") kv::Store primary_store; kv::Store backup_store; - ccf::NetworkState network; - uint64_t node_id = 0; - auto ledger_secrets = std::make_shared(node_id); - ledger_secrets->init(); - ccf::NodeEncryptor encryptor(ledger_secrets); - - // Primary and backup stores have access to same ledger secrets - auto primary_encryptor = std::make_shared(ledger_secrets); - auto backup_encryptor = std::make_shared(ledger_secrets); + ccf::NodeId primary_id = 0; + ccf::NodeId backup_id = 1; + std::shared_ptr primary_ledger_secrets; + std::shared_ptr backup_ledger_secrets; - INFO("Setup stores"); + INFO("Initialise and communicate secrets to backup store"); { + // Initialise primary ledger secrets + primary_ledger_secrets = std::make_shared(primary_id); + primary_ledger_secrets->init(); + + // Initialise backup ledger secrets from primary + auto tx = primary_store.create_tx(); + auto secrets_so_far = primary_ledger_secrets->get(tx); + backup_ledger_secrets = std::make_shared( + backup_id, primary_ledger_secrets->get(tx)); + + auto primary_encryptor = + std::make_shared(primary_ledger_secrets); + auto backup_encryptor = + std::make_shared(backup_ledger_secrets); + primary_store.set_encryptor(primary_encryptor); primary_store.set_consensus(consensus); backup_store.set_encryptor(backup_encryptor); } - commit_one(primary_store, map); - INFO("Apply transaction to backup store"); { + commit_one(primary_store, map); REQUIRE( backup_store.deserialise(*consensus->get_latest_data(), hooks) == kv::DeserialiseSuccess::PASS); } - INFO("Simple rekey"); + INFO("Rekeys"); { - // In practice, rekey is done via local commit hooks - ledger_secrets->set_secret(2, ccf::make_ledger_secret()); - ledger_secrets->set_secret(3, ccf::make_ledger_secret()); - ledger_secrets->set_secret(4, ccf::make_ledger_secret()); - ledger_secrets->set_secret(5, ccf::make_ledger_secret()); + auto current_version = primary_store.current_version(); + for (size_t i = 1; i < 3; ++i) + { + // The primary and caught-up backup always encrypt/decrypt with the latest + // available ledger secret + auto new_ledger_secret = ccf::make_ledger_secret(); - commit_one(primary_store, map); + // In practice, rekey is done via local commit hooks on the secrets table. + auto ledger_secret_for_backup = new_ledger_secret; - auto serialised_tx = consensus->get_latest_data(); + primary_ledger_secrets->set_secret( + current_version + i, std::move(new_ledger_secret)); - REQUIRE( - backup_store.deserialise(*serialised_tx, hooks) == - kv::DeserialiseSuccess::PASS); + commit_one(primary_store, map); + + backup_ledger_secrets->set_secret( + current_version + i, std::move(ledger_secret_for_backup)); + + REQUIRE( + backup_store.deserialise(*consensus->get_latest_data(), hooks) == + kv::DeserialiseSuccess::PASS); + } + } +} + +TEST_CASE("Backup catchup from many ledger secrets") +{ + auto consensus = std::make_shared(); + StringString map("map"); + kv::Store primary_store; + kv::Store backup_store; + + ccf::NodeId primary_id = 0; + ccf::NodeId backup_id = 1; + std::shared_ptr primary_ledger_secrets; + std::shared_ptr backup_ledger_secrets; + + INFO("Initialise primary store and rekey ledger secrets a few times"); + { + // Initialise primary ledger secrets + primary_ledger_secrets = std::make_shared(primary_id); + primary_ledger_secrets->init(); + auto primary_encryptor = + std::make_shared(primary_ledger_secrets); + primary_store.set_encryptor(primary_encryptor); + primary_store.set_consensus(consensus); + + auto current_version = primary_store.current_version(); + for (size_t i = 2; i < 6; ++i) + { + commit_one(primary_store, map); + primary_ledger_secrets->set_secret( + current_version + i, ccf::make_ledger_secret()); + } + } + + INFO("Initialise backup from primary"); + { + // Just like in the join protocol, ledger secrets are passed to the joining + // node in advance of KV store catch up + auto tx = primary_store.create_tx(); + auto secrets_so_far = primary_ledger_secrets->get(tx); + backup_ledger_secrets = std::make_shared( + backup_id, primary_ledger_secrets->get(tx)); + + auto backup_encryptor = + std::make_shared(backup_ledger_secrets); + + backup_store.set_encryptor(backup_encryptor); + } + + // At this point, the backup has been given the ledger secrets but still + // needs to catch up (similar to join protocol) + INFO("Backup catch up over multiple rekeys"); + { + auto next_entry = consensus->pop_oldest_entry(); + while (next_entry.has_value()) + { + REQUIRE( + backup_store.deserialise(*std::get<1>(next_entry.value()), hooks) == + kv::DeserialiseSuccess::PASS); + next_entry = consensus->pop_oldest_entry(); + } } } @@ -290,7 +363,6 @@ TEST_CASE("KV integrity verification") kv::Store primary_store; kv::Store backup_store; - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); @@ -326,7 +398,6 @@ TEST_CASE("Encryptor rollback") StringString map("map"); kv::Store store; - ccf::NetworkState network; uint64_t node_id = 0; auto ledger_secrets = std::make_shared(node_id); ledger_secrets->init(); From 03cd273ad1b74f73aa02d7bae016659bdd8d4be8 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 15:10:48 +0000 Subject: [PATCH 11/35] Re-add hint for historical queries --- src/kv/generic_serialise_wrapper.h | 5 +++-- src/kv/store.h | 8 ++++---- src/kv/test/null_encryptor.h | 3 ++- src/node/historical_queries.h | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/kv/generic_serialise_wrapper.h b/src/kv/generic_serialise_wrapper.h index 8a88d410d0c..fe7b8c95c7e 100644 --- a/src/kv/generic_serialise_wrapper.h +++ b/src/kv/generic_serialise_wrapper.h @@ -313,7 +313,7 @@ namespace kv {} std::optional> init( - const uint8_t* data, size_t size) + const uint8_t* data, size_t size, bool historical_hint = false) { current_reader = &public_reader; auto data_ = data; @@ -356,7 +356,8 @@ namespace kv {data_public, data_public + public_domain_length}, {data, data + crypto_util->get_header_length()}, decrypted_buffer, - version)) + version, + historical_hint)) { return std::nullopt; } diff --git a/src/kv/store.h b/src/kv/store.h index 6af89db3a5c..1a432aabaec 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -79,7 +79,7 @@ namespace kv // is used for historical queries, where it may deserialise arbitrary // transactions. In this case the Store is a useful container for a set of // Tables, but its versioning invariants are ignored. - const bool strict_versions = true; + const bool is_historical = false; DeserialiseSuccess commit_deserialised( OrderedChanges& changes, @@ -112,7 +112,7 @@ namespace kv } public: - Store(bool strict_versions_ = true) : strict_versions(strict_versions_) {} + Store(bool is_historical_ = false) : is_historical(is_historical_) {} Store( const ReplicateType& replicate_type_, @@ -623,7 +623,7 @@ namespace kv public_only ? kv::SecurityDomain::PUBLIC : std::optional()); - auto v_ = d.init(data.data(), data.size()); + auto v_ = d.init(data.data(), data.size(), is_historical); if (!v_.has_value()) { LOG_FAIL_FMT("Initialisation of deserialise object failed"); @@ -635,7 +635,7 @@ namespace kv // consensus. rollback(v - 1); - if (strict_versions) + if (!is_historical) { // Make sure this is the next transaction. auto cv = current_version(); diff --git a/src/kv/test/null_encryptor.h b/src/kv/test/null_encryptor.h index 1d6421fc29a..7dc87e1caf5 100644 --- a/src/kv/test/null_encryptor.h +++ b/src/kv/test/null_encryptor.h @@ -26,7 +26,8 @@ namespace kv const std::vector& additional_data, const std::vector& serialised_header, std::vector& plain, - Version version) override + Version version, + bool historical_hint = false) override { plain = cipher; return true; diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index 6a2a2d9d3b2..44a9ce33276 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -196,7 +196,7 @@ namespace ccf::historical void deserialise_ledger_entry( consensus::Index idx, const LedgerEntry& entry) { - StorePtr store = std::make_shared(false); + StorePtr store = std::make_shared(true); store->set_encryptor(source_store.get_encryptor()); kv::ConsensusHookPtrs hooks; From e1bb80da145fd7938ed8c0a664855ebc1be91e54 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 15:27:33 +0000 Subject: [PATCH 12/35] Simplify logic --- src/kv/store.h | 2 +- src/node/ledger_secrets.h | 88 +++++++++++++++++---------------------- 2 files changed, 40 insertions(+), 50 deletions(-) diff --git a/src/kv/store.h b/src/kv/store.h index 1a432aabaec..3bb5f7e59c4 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -670,7 +670,7 @@ namespace kv map = new_map; new_maps[map_name] = new_map; LOG_DEBUG_FMT( - "Creating map {} while deserialising transaction at version {}", + "Creating map '{}' while deserialising transaction at version {}", map_name, v); } diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 962620dbc9d..b81f9c7870e 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -66,48 +66,30 @@ namespace ccf throw std::logic_error("Ledger secrets map is empty"); } - if (!historical_hint) + if (!historical_hint && last_used_secret_it.has_value()) { - if (last_used_secret_it.has_value()) - { - auto& last_used_secret_it_ = last_used_secret_it.value(); - if ( - std::next(last_used_secret_it_) == ledger_secrets.end() || - version < std::next(last_used_secret_it_)->first) - { - // Hot path on backups, when decrypting in sequence after having - // been given access to historical secrets - LOG_FAIL_FMT("Fast path backup"); - return last_used_secret_it_->second; - } - else - { - // After a rekey, the next ledger secret should be used - LOG_FAIL_FMT("Fast path update backup!"); - ++last_used_secret_it_; - return last_used_secret_it_->second; - } - } - else + // Fast path for non-historical queries as both primary and backup nodes + // encryt/decrypt entries in sequence, it is sufficient to keep an + // iterator on the last used secret to access ledger secrets in constant + // time. + auto& last_used_secret_it_ = last_used_secret_it.value(); + if ( + std::next(last_used_secret_it_) != ledger_secrets.end() && + version >= std::next(last_used_secret_it_)->first) { - auto latest = std::prev(ledger_secrets.end()); - if (version >= latest->first) - { - // Hot path when encrypting/decrypting latest entries - last_used_secret_it = latest; - return latest->second; - } + // Across a rekey, start using the next key + ++last_used_secret_it_; } - } - // Slow path, e.g. historical queries or start of backup replay - LOG_FAIL_FMT("Slow path!"); + return last_used_secret_it_->second; + } - // The ledger secret used to encrypt/decrypt a transaction at a given - // version is the one with the highest version that is lower than the - // given version (e.g. if ledger_secrets contains two keys for version 0 - // and 10 then the key associated with version 0 is used for version - // [0..9] and version 10 for versions 10+) + // Slow path, e.g. for historical queries. The ledger secret used to + // encrypt/decrypt a transaction at a given version is the one with the + // highest version that is lower than the given version (e.g. if + // ledger_secrets contains two keys for version 0 and 10 then the key + // associated with version 0 is used for version [0..9] and version 10 for + // versions 10+) auto search = std::upper_bound( ledger_secrets.begin(), ledger_secrets.end(), @@ -120,22 +102,30 @@ namespace ccf fmt::format("Could not find ledger secret for seqno {}", version)); } - last_used_secret_it = std::prev(search); - return last_used_secret_it.value()->second; + if (!historical_hint) + { + // Only update the last secret iterator on non-historical queries so + // that the fast path is always preserved for transactions on the main + // store + last_used_secret_it = std::prev(search); + } + + return std::prev(search)->second; } void take_dependency_on_secrets(kv::ReadOnlyTx& tx) { // Ledger secrets are not stored in the KV. Instead, they are - // cached in a unique LedgerSecrets instance that can be accessed without - // reading the KV. However, it is possible that the ledger secrets are - // updated (e.g. rekey tx) concurrently to their access by another tx. To - // prevent conflicts, accessing the ledger secrets require access to a tx - // object, which must take a dependency on the secrets table. + // cached in a unique LedgerSecrets instance that can be accessed + // without reading the KV. However, it is possible that the ledger + // secrets are updated (e.g. rekey tx) concurrently to their access by + // another tx. To prevent conflicts, accessing the ledger secrets + // require access to a tx object, which must take a dependency on the + // secrets table. auto v = tx.get_read_only_view(Tables::SECRETS); - // Taking a read dependency on the key at self, which would get updated on - // rekey + // Taking a read dependency on the key at self, which would get updated + // on rekey if (!self.has_value()) { throw std::logic_error( @@ -241,7 +231,8 @@ namespace ccf ledger_secrets.begin()->first) { throw std::logic_error(fmt::format( - "Last restored version {} is greater than first existing version {}", + "Last restored version {} is greater than first existing version " + "{}", restored_ledger_secrets.rbegin()->first, ledger_secrets.begin()->first)); } @@ -299,8 +290,7 @@ namespace ccf ledger_secrets.erase(k->first); } - // Optimistically assume that the next operation will use the first - // non-rollback secret + // Assume that the next operation will use the first non-rollbacked secret last_used_secret_it = std::prev(ledger_secrets.end()); } }; From 13e9793382fdebcf168d10e432c5aa42ec1265f8 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 15:30:37 +0000 Subject: [PATCH 13/35] Cleanup --- src/kv/test/stub_consensus.h | 1 - src/node/ledger_secrets.h | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/kv/test/stub_consensus.h b/src/kv/test/stub_consensus.h index a9faa85071d..6eac2c313cc 100644 --- a/src/kv/test/stub_consensus.h +++ b/src/kv/test/stub_consensus.h @@ -8,7 +8,6 @@ #include #include -#include namespace kv { diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index b81f9c7870e..d881bd2334d 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -69,7 +69,7 @@ namespace ccf if (!historical_hint && last_used_secret_it.has_value()) { // Fast path for non-historical queries as both primary and backup nodes - // encryt/decrypt entries in sequence, it is sufficient to keep an + // encryt/decrypt transactions in order, it is sufficient to keep an // iterator on the last used secret to access ledger secrets in constant // time. auto& last_used_secret_it_ = last_used_secret_it.value(); From 6f6c72c63277ced0915fbf3d85d847ca885cecec Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 15:56:31 +0000 Subject: [PATCH 14/35] fmt --- src/node/test/encryptor.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index 030cb08c378..5339173d819 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -3,6 +3,7 @@ #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN #include "kv/encryptor.h" + #include "kv/kv_types.h" #include "kv/store.h" #include "kv/test/stub_consensus.h" From 4d775aca2e2fb5c09dbd665db41487aa8bc321e7 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 22 Jan 2021 17:11:04 +0000 Subject: [PATCH 15/35] Decouple store's strict version from historical flag --- src/kv/store.h | 10 ++++++++-- src/node/historical_queries.h | 4 +++- src/node/node_state.h | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/kv/store.h b/src/kv/store.h index ab7be811aba..eaf3e6c1006 100644 --- a/src/kv/store.h +++ b/src/kv/store.h @@ -79,6 +79,9 @@ namespace kv // is used for historical queries, where it may deserialise arbitrary // transactions. In this case the Store is a useful container for a set of // Tables, but its versioning invariants are ignored. + const bool strict_versions = true; + + // If true, use historical ledger secrets to deserialise entries const bool is_historical = false; DeserialiseSuccess commit_deserialised( @@ -112,7 +115,10 @@ namespace kv } public: - Store(bool is_historical_ = false) : is_historical(is_historical_) {} + Store(bool strict_versions_ = true, bool is_historical_ = false) : + strict_versions(strict_versions_), + is_historical(is_historical_) + {} Store( const ReplicateType& replicate_type_, @@ -634,7 +640,7 @@ namespace kv // consensus. rollback(v - 1); - if (!is_historical) + if (strict_versions) { // Make sure this is the next transaction. auto cv = current_version(); diff --git a/src/node/historical_queries.h b/src/node/historical_queries.h index 44a9ce33276..b803520586f 100644 --- a/src/node/historical_queries.h +++ b/src/node/historical_queries.h @@ -196,7 +196,9 @@ namespace ccf::historical void deserialise_ledger_entry( consensus::Index idx, const LedgerEntry& entry) { - StorePtr store = std::make_shared(true); + StorePtr store = std::make_shared( + false /* Do not start from very first idx */, + true /* Make use of historical secrets */); store->set_encryptor(source_store.get_encryptor()); kv::ConsensusHookPtrs hooks; diff --git a/src/node/node_state.h b/src/node/node_state.h index 2efc2f0881e..278283f191f 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1052,7 +1052,9 @@ namespace ccf // void setup_private_recovery_store() { - recovery_store = std::make_shared(); + recovery_store = std::make_shared( + true /* Check transactions in order */, + true /* Make use of historical secrets */); recovery_history = std::make_shared( *recovery_store.get(), self, From ad79b696f428f23c5402313917e466cfa5598fa1 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sat, 23 Jan 2021 14:28:58 +0000 Subject: [PATCH 16/35] WIP --- CMakeLists.txt | 4 ++-- src/node/ledger_secrets.h | 33 +++++++++++++++++++++++++-------- src/node/node_state.h | 17 ++++++++++++++++- src/node/share_manager.h | 21 ++++++++++++++------- src/node/shares.h | 10 +++++----- tests/recovery.py | 8 ++++---- tests/suite/test_suite.py | 14 +++++++++----- 7 files changed, 75 insertions(+), 32 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f0eedf56262..5c639790e88 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -545,8 +545,8 @@ if(BUILD_TESTS) --enforce-reqs --test-suite rekey_recovery - --test-suite - membership_recovery + # --test-suite + # membership_recovery ) add_e2e_test( diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index d881bd2334d..067e5fba50f 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -46,6 +46,7 @@ namespace ccf } using LedgerSecretsMap = std::map; + using VersionedLedgerSecret = LedgerSecretsMap::value_type; class LedgerSecrets { @@ -146,7 +147,13 @@ namespace ccf { std::lock_guard guard(lock); - ledger_secrets.emplace(initial_version, make_ledger_secret()); + auto init_secret = make_ledger_secret(); + + LOG_INFO_FMT("Init ledger secrets at {}", initial_version); + LOG_FAIL_FMT( + "{}", tls::b64_from_raw(init_secret.raw_key)); // TODO: Delete + + ledger_secrets.emplace(initial_version, std::move(init_secret)); } void set_node_id(NodeId id) @@ -160,7 +167,7 @@ namespace ccf self = id; } - LedgerSecretsMap::value_type get_latest(kv::Tx& tx) + VersionedLedgerSecret get_latest(kv::Tx& tx) { std::lock_guard guard(lock); @@ -177,7 +184,7 @@ namespace ccf latest_ledger_secret->first, latest_ledger_secret->second); } - std::pair> + std::pair> get_latest_and_penultimate(kv::Tx& tx) { std::lock_guard guard(lock); @@ -193,11 +200,10 @@ namespace ccf const auto& latest_ledger_secret = ledger_secrets.rbegin(); if (ledger_secrets.size() < 2) { - return std::make_pair(latest_ledger_secret->second, std::nullopt); + return std::make_pair(latest_ledger_secret, std::nullopt); } return std::make_pair( - latest_ledger_secret->second, - std::next(ledger_secrets.rbegin())->second); + latest_ledger_secret, std::next(latest_ledger_secret)); } LedgerSecretsMap get( @@ -237,6 +243,16 @@ namespace ccf ledger_secrets.begin()->first)); } + LOG_FAIL_FMT("Ledger secrets before: {}", ledger_secrets.size()); + LOG_FAIL_FMT("LS before: {}", ledger_secrets.begin()->first); + + LOG_FAIL_FMT("Restored secrets: {}", restored_ledger_secrets.size()); + for (auto const& ls : restored_ledger_secrets) + { + LOG_FAIL_FMT( + "LS: {} - {}", ls.first, tls::b64_from_raw(ls.second.raw_key)); + } + ledger_secrets.merge(restored_ledger_secrets); } @@ -256,9 +272,10 @@ namespace ccf "Ledger secret at seqno {} already exists", version); - ledger_secrets.emplace(version, std::move(secret)); - LOG_INFO_FMT("Added new ledger secret at seqno {}", version); + LOG_FAIL_FMT("{}", tls::b64_from_raw(secret.raw_key)); // TODO: Delete + + ledger_secrets.emplace(version, std::move(secret)); } void rollback(kv::Version version) diff --git a/src/node/node_state.h b/src/node/node_state.h index 278283f191f..813c8ab6b7e 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -467,6 +467,14 @@ namespace ccf network.identity = std::make_unique(resp.network_info.identity); + LOG_FAIL_FMT( + "Joining with {} ledger secrets", + resp.network_info.ledger_secrets.size()); + for (auto const& s : resp.network_info.ledger_secrets) + { + LOG_FAIL_FMT("LS: {}", s.first); + } + network.ledger_secrets = std::make_shared( self, std::move(resp.network_info.ledger_secrets)); @@ -1600,7 +1608,7 @@ namespace ccf { // When recoverying from a snapshot, the first secret is valid from the // version at which it was recorded - static bool is_first_secret = !from_snapshot; + static bool is_first_secret = !from_snapshot; // false on snapshot! network.tables->set_map_hook( network.shares.get_name(), @@ -1616,6 +1624,10 @@ namespace ccf } const auto& v = opt_v.value(); + LOG_FAIL_FMT( + "Ledger secrets at KV seqno {} and wrapped latest seqno {}", + version, + v.wrapped_latest_ledger_secret.version); kv::Version ledger_secret_version; if (is_first_secret) @@ -1634,6 +1646,9 @@ namespace ccf v.wrapped_latest_ledger_secret.version == kv::NoVersion ? (version + 1) : v.wrapped_latest_ledger_secret.version; + + LOG_FAIL_FMT( + "Ledger secret version: {}", ledger_secret_version); } // No encrypted ledger secret are stored in the case of a pure diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 9e1ed5bb231..4c8758a5f6b 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -156,7 +156,8 @@ namespace ccf void set_recovery_shares_info( kv::Tx& tx, const LedgerSecret& latest_ledger_secret, - const std::optional& previous_ledger_secret = std::nullopt, + const std::optional& previous_ledger_secret = + std::nullopt, kv::Version latest_ls_version = kv::NoVersion) { // First, generate a fresh ledger secrets wrapping key and wrap the @@ -170,16 +171,19 @@ namespace ccf auto wrapped_latest_ls = ls_wrapping_key.wrap(latest_ledger_secret); std::vector encrypted_previous_secret = {}; + kv::Version version_previous_secret = kv::NoVersion; if (previous_ledger_secret.has_value()) { + version_previous_secret = previous_ledger_secret->first; + crypto::GcmCipher encrypted_previous_ls( - previous_ledger_secret->raw_key.size()); + previous_ledger_secret->second.raw_key.size()); auto iv = tls::create_entropy()->random(crypto::GCM_SIZE_IV); encrypted_previous_ls.hdr.set_iv(iv.data(), iv.size()); latest_ledger_secret.key->encrypt( encrypted_previous_ls.hdr.get_iv(), - previous_ledger_secret->raw_key, + previous_ledger_secret.second.->raw_key, nullb, encrypted_previous_ls.cipher.data(), encrypted_previous_ls.hdr.tag); @@ -188,9 +192,12 @@ namespace ccf } GenesisGenerator g(network, tx); - g.add_key_share_info({{latest_ls_version, wrapped_latest_ls}, - encrypted_previous_secret, - compute_encrypted_shares(tx, ls_wrapping_key)}); + + // TODO: Rename this + g.add_key_share_info( + {{latest_ls_version, wrapped_latest_ls}, + {version_previous_secret, encrypted_previous_secret}, + compute_encrypted_shares(tx, ls_wrapping_key)}); } std::vector encrypt_submitted_share( @@ -288,7 +295,7 @@ namespace ccf kv::Tx& tx, const LedgerSecret& new_ledger_secret) { set_recovery_shares_info( - tx, new_ledger_secret, network.ledger_secrets->get_latest(tx).second); + tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); } std::optional get_encrypted_share( diff --git a/src/node/shares.h b/src/node/shares.h index 39a8f1bd8d3..6fd070ffea1 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -14,7 +14,7 @@ namespace ccf using EncryptedShare = std::vector; using EncryptedSharesMap = std::map; - struct LatestLedgerSecret + struct EncryptedLedgerSecret { // In most cases (e.g. re-key, member retirement), this is unset // (kv::NoVersion), and the version at which the ledger secret is applicable @@ -30,8 +30,8 @@ namespace ccf MSGPACK_DEFINE(version, encrypted_data) }; - DECLARE_JSON_TYPE(LatestLedgerSecret) - DECLARE_JSON_REQUIRED_FIELDS(LatestLedgerSecret, version, encrypted_data) + DECLARE_JSON_TYPE(EncryptedLedgerSecret) + DECLARE_JSON_REQUIRED_FIELDS(EncryptedLedgerSecret, version, encrypted_data) struct RecoverySharesInfo { @@ -43,10 +43,10 @@ namespace ccf // re-assembled. // Latest ledger secret wrapped with the ledger secret wrapping key - LatestLedgerSecret wrapped_latest_ledger_secret; + EncryptedLedgerSecret wrapped_latest_ledger_secret; // Previous ledger secret encrypted with the latest ledger secret - std::vector encrypted_previous_ledger_secret; + EncryptedLedgerSecret encrypted_previous_ledger_secret; EncryptedSharesMap encrypted_shares; diff --git a/tests/recovery.py b/tests/recovery.py index acd9550ee8a..f52e426764b 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -11,7 +11,7 @@ @reqs.description("Recovering a network") @reqs.recover(number_txs=2) -def test(network, args, from_snapshot=False): +def test(network, args, from_snapshot=True): old_primary, _ = network.find_primary() snapshot_dir = None @@ -36,7 +36,7 @@ def test(network, args, from_snapshot=False): @reqs.description("Recovering a network, kill one node while submitting shares") @reqs.recover(number_txs=2) -def test_share_resilience(network, args, from_snapshot=False): +def test_share_resilience(network, args, from_snapshot=True): old_primary, _ = network.find_primary() snapshot_dir = None @@ -115,10 +115,10 @@ def run(args): # with and without snapshots if i % 2 == 0: recovered_network = test_share_resilience( - network, args, from_snapshot=True + network, args, from_snapshot=False ) else: - recovered_network = test(network, args, from_snapshot=False) + recovered_network = test(network, args, from_snapshot=True) network.stop_all_nodes() network = recovered_network LOG.success("Recovery complete on all nodes") diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 87742e047b7..c3eaa5fc3ee 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -18,13 +18,17 @@ # This suite tests that rekeying, network configuration changes # and recoveries can be interleaved suite_rekey_recovery = [ - recovery.test, - reconfiguration.test_add_node, + e2e_logging.test_historical_query, rekey.test, - reconfiguration.test_add_node, - recovery.test, rekey.test, - reconfiguration.test_add_node, + # reconfiguration.test_add_node_from_snapshot, + recovery.test, + # reconfiguration.test_add_node_from_snapshot, + # rekey.test, + # reconfiguration.test_add_node, + # recovery.test, + # rekey.test, + # reconfiguration.test_add_node, ] suites["rekey_recovery"] = suite_rekey_recovery From bc4b1f5f1727a53e5db45e1e121159ded15fd639 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 27 Jan 2021 14:00:08 +0000 Subject: [PATCH 17/35] WIP - recovering previous ledger secret version --- src/node/ledger_secrets.h | 8 +++----- src/node/node_state.h | 5 +++-- src/node/share_manager.h | 8 +++++--- src/node/shares.h | 12 +++++++----- tests/suite/test_suite.py | 2 +- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 067e5fba50f..276da9efc08 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -179,9 +179,7 @@ namespace ccf "Could not retrieve latest ledger secret: no secret set"); } - const auto& latest_ledger_secret = ledger_secrets.rbegin(); - return std::make_pair( - latest_ledger_secret->first, latest_ledger_secret->second); + return *ledger_secrets.rbegin(); } std::pair> @@ -200,10 +198,10 @@ namespace ccf const auto& latest_ledger_secret = ledger_secrets.rbegin(); if (ledger_secrets.size() < 2) { - return std::make_pair(latest_ledger_secret, std::nullopt); + return std::make_pair(*latest_ledger_secret, std::nullopt); } return std::make_pair( - latest_ledger_secret, std::next(latest_ledger_secret)); + *latest_ledger_secret, *std::next(latest_ledger_secret)); } LedgerSecretsMap get( diff --git a/src/node/node_state.h b/src/node/node_state.h index 813c8ab6b7e..29017a4a0c6 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1654,7 +1654,7 @@ namespace ccf // No encrypted ledger secret are stored in the case of a pure // re-share (i.e. no ledger rekey). if ( - !v.encrypted_previous_ledger_secret.empty() || + !v.encrypted_previous_ledger_secret.encrypted_data.empty() || ledger_secret_version == 1) { LOG_TRACE_FMT( @@ -1662,7 +1662,8 @@ namespace ccf ledger_secret_version); recovery_ledger_secrets.push_back( - {ledger_secret_version, v.encrypted_previous_ledger_secret}); + {ledger_secret_version, + v.encrypted_previous_ledger_secret.encrypted_data}); } } diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 4c8758a5f6b..3d77d837fe0 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -158,7 +158,8 @@ namespace ccf const LedgerSecret& latest_ledger_secret, const std::optional& previous_ledger_secret = std::nullopt, - kv::Version latest_ls_version = kv::NoVersion) + kv::Version latest_ls_version = + kv::NoVersion) // TODO: latest_ls_version should be optional { // First, generate a fresh ledger secrets wrapping key and wrap the // latest ledger secret with it. Then, encrypt the penultimate ledger @@ -183,7 +184,7 @@ namespace ccf latest_ledger_secret.key->encrypt( encrypted_previous_ls.hdr.get_iv(), - previous_ledger_secret.second.->raw_key, + previous_ledger_secret->second.raw_key, nullb, encrypted_previous_ls.cipher.data(), encrypted_previous_ls.hdr.tag); @@ -288,7 +289,8 @@ namespace ccf auto [latest, penultimate] = network.ledger_secrets->get_latest_and_penultimate(tx); - set_recovery_shares_info(tx, latest, penultimate, latest_ls_version); + set_recovery_shares_info( + tx, latest.second, penultimate, latest_ls_version); } void issue_shares_on_rekey( diff --git a/src/node/shares.h b/src/node/shares.h index 6fd070ffea1..1fe7b61bd87 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -14,7 +14,8 @@ namespace ccf using EncryptedShare = std::vector; using EncryptedSharesMap = std::map; - struct EncryptedLedgerSecret + // TODO: To unify with secrets.h encrypted ledger secret?? + struct SharesEncryptedLedgerSecret { // In most cases (e.g. re-key, member retirement), this is unset // (kv::NoVersion), and the version at which the ledger secret is applicable @@ -30,8 +31,9 @@ namespace ccf MSGPACK_DEFINE(version, encrypted_data) }; - DECLARE_JSON_TYPE(EncryptedLedgerSecret) - DECLARE_JSON_REQUIRED_FIELDS(EncryptedLedgerSecret, version, encrypted_data) + DECLARE_JSON_TYPE(SharesEncryptedLedgerSecret) + DECLARE_JSON_REQUIRED_FIELDS( + SharesEncryptedLedgerSecret, version, encrypted_data) struct RecoverySharesInfo { @@ -43,10 +45,10 @@ namespace ccf // re-assembled. // Latest ledger secret wrapped with the ledger secret wrapping key - EncryptedLedgerSecret wrapped_latest_ledger_secret; + SharesEncryptedLedgerSecret wrapped_latest_ledger_secret; // Previous ledger secret encrypted with the latest ledger secret - EncryptedLedgerSecret encrypted_previous_ledger_secret; + SharesEncryptedLedgerSecret encrypted_previous_ledger_secret; EncryptedSharesMap encrypted_shares; diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index c3eaa5fc3ee..680bb5434d7 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -20,7 +20,7 @@ suite_rekey_recovery = [ e2e_logging.test_historical_query, rekey.test, - rekey.test, + # rekey.test, # reconfiguration.test_add_node_from_snapshot, recovery.test, # reconfiguration.test_add_node_from_snapshot, From b84955239c459abd0a6e1f9490dac2966569aad9 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 28 Jan 2021 17:07:03 +0000 Subject: [PATCH 18/35] Cleanup --- src/crypto/symmetric_key.h | 3 +- src/node/node_state.h | 15 +-------- src/node/secret_broadcast.h | 2 +- src/node/share_manager.h | 56 ++++++++++++++++++++++---------- src/node/shares.h | 44 ++++++++++++++++--------- tests/e2e_suite.py | 3 +- tests/suite/test_requirements.py | 5 +++ tests/suite/test_suite.py | 3 +- 8 files changed, 79 insertions(+), 52 deletions(-) diff --git a/src/crypto/symmetric_key.h b/src/crypto/symmetric_key.h index 01827f373e8..d117ccb0eb1 100644 --- a/src/crypto/symmetric_key.h +++ b/src/crypto/symmetric_key.h @@ -124,10 +124,9 @@ namespace crypto return serial; } - void apply(const std::vector& serial) + void deserialise(const std::vector& serial) { auto size = serial.size(); - auto data_ = serial.data(); hdr = serialized::read(data_, size, GcmHeader<>::RAW_DATA_SIZE); cipher = serialized::read(data_, size, size); diff --git a/src/node/node_state.h b/src/node/node_state.h index 28c8b7381c3..d0799f5aa84 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1626,23 +1626,16 @@ namespace ccf } const auto& v = opt_v.value(); - LOG_FAIL_FMT( - "Ledger secrets at KV seqno {} and wrapped latest seqno {}", - version, - v.wrapped_latest_ledger_secret.version); // If the version is not set (rekeying), use the version // from the hook plus one. Otherwise (recovery), use the // version specified. auto ledger_secret_version = - v.wrapped_latest_ledger_secret.version == kv::NoVersion ? - (version + 1) : - v.wrapped_latest_ledger_secret.version; + v.wrapped_latest_ledger_secret.version.value_or(version + 1); // Do not recover the ledger secrets in case of a pure re-share // (e.g. recovery threshold update) as they are the same as in // the previous entry. - if ( recovery_ledger_secrets.empty() || recovery_ledger_secrets.back().next_version != @@ -1657,12 +1650,6 @@ namespace ccf recovery_ledger_secrets.push_back( {ledger_secret_version, v.encrypted_previous_ledger_secret}); } - else - { - LOG_FAIL_FMT( - "Skipping recovering ledger secret at {}", - ledger_secret_version); - } } return kv::ConsensusHookPtr(nullptr); diff --git a/src/node/secret_broadcast.h b/src/node/secret_broadcast.h index 1f39ea76246..500fc03d4c7 100644 --- a/src/node/secret_broadcast.h +++ b/src/node/secret_broadcast.h @@ -102,7 +102,7 @@ namespace ccf const std::vector& cipher) { crypto::GcmCipher gcmcipher; - gcmcipher.apply(cipher); + gcmcipher.deserialise(cipher); std::vector plain(gcmcipher.cipher.size()); crypto::KeyAesGcm primary_shared_key( diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 38e2b6b9415..a5396ef47dc 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -69,7 +69,7 @@ namespace ccf const std::vector& wrapped_latest_ledger_secret) { crypto::GcmCipher encrypted_ls; - encrypted_ls.apply(wrapped_latest_ledger_secret); + encrypted_ls.deserialise(wrapped_latest_ledger_secret); std::vector decrypted_ls(encrypted_ls.cipher.size()); if (!crypto::KeyAesGcm(data).decrypt( @@ -93,8 +93,13 @@ namespace ccf // Version at which the next ledger secret is applicable from kv::Version next_version; + PreviousEncryptedLedgerSecret encrypted_ledger_secret; + + // Version at which the ledger secret is applicable from + // kv::Version version; + // Previous ledger secret, encrypted with the current ledger secret - std::vector encrypted_ledger_secret; + // std::vector encrypted_ledger_secret; }; // The ShareManager class provides the interface between the ledger secrets, @@ -104,6 +109,10 @@ namespace ccf // membership updates) // - Re-assemble the ledger secrets on recovery, once a threshold of members // have successfully submitted their shares + + // TODO: Make this a map! + using RecoveredEncryptedLedgerSecrets = std::list; + class ShareManager { private: @@ -158,8 +167,7 @@ namespace ccf const LedgerSecret& latest_ledger_secret, const std::optional& previous_ledger_secret = std::nullopt, - kv::Version latest_ls_version = - kv::NoVersion) // TODO: latest_ls_version should be optional + std::optional latest_ls_version = std::nullopt) { // First, generate a fresh ledger secrets wrapping key and wrap the // latest ledger secret with it. Then, encrypt the penultimate ledger @@ -194,10 +202,15 @@ namespace ccf GenesisGenerator g(network, tx); + // LOG_FAIL_FMT( + // "Writing shares info: previous secret at {}, latest secret at {}", + // version_previous_secret, + // latest_ls_version); + // TODO: Rename this g.add_key_share_info( - {{latest_ls_version, wrapped_latest_ls}, - {version_previous_secret, encrypted_previous_secret}, + {{wrapped_latest_ls, latest_ls_version}, + {encrypted_previous_secret, version_previous_secret}, compute_encrypted_shares(tx, ls_wrapping_key)}); } @@ -226,7 +239,7 @@ namespace ccf LedgerSecret&& current_ledger_secret) { crypto::GcmCipher encrypted_share; - encrypted_share.apply(encrypted_submitted_share); + encrypted_share.deserialise(encrypted_submitted_share); std::vector decrypted_share(encrypted_share.cipher.size()); current_ledger_secret.key->decrypt( @@ -276,12 +289,14 @@ namespace ccf public: ShareManager(NetworkState& network_) : network(network_) {} + // TODO: Unify this with issue_shares_on_recovery() void issue_shares(kv::Tx& tx) { // Assumes that the ledger secrets have not been updated since the // last time shares have been issued (i.e. genesis or re-sharing only) - set_recovery_shares_info( - tx, network.ledger_secrets->get_latest(tx).second); + auto [latest, penultimate] = + network.ledger_secrets->get_latest_and_penultimate(tx); + set_recovery_shares_info(tx, latest.second, penultimate, latest.first); } void issue_shares_on_recovery(kv::Tx& tx, kv::Version latest_ls_version) @@ -296,6 +311,8 @@ namespace ccf void issue_shares_on_rekey( kv::Tx& tx, const LedgerSecret& new_ledger_secret) { + // No version here, on recovery, the version is derived from the hook at + // which the ledger secret is applied to the store set_recovery_shares_info( tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); } @@ -323,7 +340,7 @@ namespace ccf LedgerSecretsMap restore_recovery_shares_info( kv::Tx& tx, - const std::list& encrypted_recovery_secrets) + const RecoveredEncryptedLedgerSecrets& encrypted_recovery_secrets) { // First, re-assemble the ledger secret wrapping key from the submitted // encrypted shares. Then, unwrap the latest ledger secret and use it to @@ -344,21 +361,26 @@ namespace ccf recovery_shares_info->wrapped_latest_ledger_secret.encrypted_data); auto decryption_key = restored_ls.raw_key; + LOG_FAIL_FMT( + "Recovering {} encrypted ledger secrets", + encrypted_recovery_secrets.size()); + restored_ledger_secrets.emplace( encrypted_recovery_secrets.back().next_version, std::move(restored_ls)); - for (auto i = encrypted_recovery_secrets.rbegin(); - i != encrypted_recovery_secrets.rend(); - i++) + for (auto it = encrypted_recovery_secrets.rbegin(); + it != encrypted_recovery_secrets.rend(); + it++) { - if (i->encrypted_ledger_secret.empty()) + // TODO: Just ignore this instead?? + if (it->encrypted_ledger_secret.encrypted_data.empty()) { // First entry does not encrypt any other ledger secret (i.e. genesis) break; } crypto::GcmCipher encrypted_ls; - encrypted_ls.apply(i->encrypted_ledger_secret); + encrypted_ls.deserialise(it->encrypted_ledger_secret.encrypted_data); std::vector decrypted_ls(encrypted_ls.cipher.size()); if (!crypto::KeyAesGcm(decryption_key) @@ -371,12 +393,12 @@ namespace ccf { throw std::logic_error(fmt::format( "Decryption of ledger secret at {} failed", - std::next(i)->next_version)); + it->encrypted_ledger_secret.version)); } decryption_key = decrypted_ls; restored_ledger_secrets.emplace( - std::next(i)->next_version, std::move(decrypted_ls)); + it->encrypted_ledger_secret.version, std::move(decrypted_ls)); } return restored_ledger_secrets; diff --git a/src/node/shares.h b/src/node/shares.h index 1fe7b61bd87..d74f5c5f6ee 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -7,6 +7,7 @@ #include #include +#include #include namespace ccf @@ -14,26 +15,39 @@ namespace ccf using EncryptedShare = std::vector; using EncryptedSharesMap = std::map; - // TODO: To unify with secrets.h encrypted ledger secret?? - struct SharesEncryptedLedgerSecret + struct WrappedLedgerSecret { - // In most cases (e.g. re-key, member retirement), this is unset - // (kv::NoVersion), and the version at which the ledger secret is applicable - // from is derived from the local hook on recovery. - // In one case (i.e. after recovery of the public ledger), a new ledger - // secret is created to protect the integrity on the public-only - // transactions. However, the corresponding shares are only written at a - // later version, once the previous ledger secrets have been restored. - kv::Version version; + std::vector encrypted_data; + + // In most cases (e.g. re-key, member retirement), this is unset and the + // version at which the ledger secret is applicable from is derived from the + // version at which the recovery hook is triggered. In other cases (service + // open or in recovery), a new ledger secret is created to protect the + // integrity on the public-only transactions. However, the corresponding + // shares are only written at a later version, once the previous ledger + // secrets have been recovered. + std::optional version = std::nullopt; + + MSGPACK_DEFINE(encrypted_data, version) + }; + DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(WrappedLedgerSecret) + DECLARE_JSON_REQUIRED_FIELDS(WrappedLedgerSecret, encrypted_data) + DECLARE_JSON_OPTIONAL_FIELDS(WrappedLedgerSecret, version) + + // TODO: To unify with secrets.h encrypted ledger secret?? + struct PreviousEncryptedLedgerSecret + { std::vector encrypted_data; - MSGPACK_DEFINE(version, encrypted_data) + kv::Version version; + + MSGPACK_DEFINE(encrypted_data, version) }; - DECLARE_JSON_TYPE(SharesEncryptedLedgerSecret) + DECLARE_JSON_TYPE(PreviousEncryptedLedgerSecret) DECLARE_JSON_REQUIRED_FIELDS( - SharesEncryptedLedgerSecret, version, encrypted_data) + PreviousEncryptedLedgerSecret, encrypted_data, version) struct RecoverySharesInfo { @@ -45,10 +59,10 @@ namespace ccf // re-assembled. // Latest ledger secret wrapped with the ledger secret wrapping key - SharesEncryptedLedgerSecret wrapped_latest_ledger_secret; + WrappedLedgerSecret wrapped_latest_ledger_secret; // Previous ledger secret encrypted with the latest ledger secret - SharesEncryptedLedgerSecret encrypted_previous_ledger_secret; + PreviousEncryptedLedgerSecret encrypted_previous_ledger_secret; EncryptedSharesMap encrypted_shares; diff --git a/tests/e2e_suite.py b/tests/e2e_suite.py index 4de024d7aa9..594a3a2621f 100644 --- a/tests/e2e_suite.py +++ b/tests/e2e_suite.py @@ -117,8 +117,7 @@ def run(args): if new_network is None: raise ValueError(f"Network returned by {s.test_name(test)} is None") - # If the network was changed (e.g. recovery test), stop the previous network - # and use the new network from now on + # If the network was changed (e.g. recovery test), use the new network from now on if new_network != network: network = new_network diff --git a/tests/suite/test_requirements.py b/tests/suite/test_requirements.py index 66dbe2d5f52..147c17de5bb 100644 --- a/tests/suite/test_requirements.py +++ b/tests/suite/test_requirements.py @@ -153,6 +153,11 @@ def wrapper(*args, **kwargs): network=network, number_txs=infra.e2e_args.get("msgs_per_recovery") or number_txs, ) + network.txs.issue( + network=network, + number_txs=1, + repeat=True, + ) new_network = func(*args, **kwargs) new_network.txs.verify( network=new_network, diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 680bb5434d7..594f71ceba3 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -20,7 +20,8 @@ suite_rekey_recovery = [ e2e_logging.test_historical_query, rekey.test, - # rekey.test, + membership.test_set_recovery_threshold, + rekey.test, # reconfiguration.test_add_node_from_snapshot, recovery.test, # reconfiguration.test_add_node_from_snapshot, From 5b693f40683c2be52cb436ee65f784b3050bcd89 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 11:04:13 +0000 Subject: [PATCH 19/35] Simplify issue recovery share interface --- src/node/node_state.h | 5 ++--- src/node/rpc/member_frontend.h | 8 +++---- src/node/rpc/test/member_voting_test.cpp | 2 +- src/node/share_manager.h | 28 ++++++++---------------- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index d0799f5aa84..901a551c78e 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1019,8 +1019,7 @@ namespace ccf // Shares for the new ledger secret can only be issued now, once the // previous ledger secrets have been recovered - share_manager.issue_shares_on_recovery( - tx, last_recovered_signed_idx + 1); + share_manager.issue_recovery_shares(tx); GenesisGenerator g(network, tx); if (!g.open_service()) { @@ -1295,7 +1294,7 @@ namespace ccf // once the local hook on the secrets table has been triggered. auto new_ledger_secret = make_ledger_secret(); - share_manager.issue_shares_on_rekey(tx, new_ledger_secret); + share_manager.issue_recovery_shares(tx, new_ledger_secret); LedgerSecretsBroadcast::broadcast_new( network, node_encrypt_kp, tx, std::move(new_ledger_secret)); diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index ecff03a25d8..626048f8b5a 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -938,7 +938,7 @@ namespace ccf // allocated to each recovery member try { - share_manager.issue_shares(tx); + share_manager.issue_recovery_shares(tx); } catch (const std::logic_error& e) { @@ -977,7 +977,7 @@ namespace ccf const ProposalId& proposal_id, kv::Tx& tx, const nlohmann::json&) { try { - share_manager.issue_shares(tx); + share_manager.issue_recovery_shares(tx); } catch (const std::logic_error& e) { @@ -1013,7 +1013,7 @@ namespace ccf // Update recovery shares (same number of shares) try { - share_manager.issue_shares(tx); + share_manager.issue_recovery_shares(tx); } catch (const std::logic_error& e) { @@ -1670,7 +1670,7 @@ namespace ccf // member, all recovery members are allocated new recovery shares try { - share_manager.issue_shares(ctx.tx); + share_manager.issue_recovery_shares(ctx.tx); } catch (const std::logic_error& e) { diff --git a/src/node/rpc/test/member_voting_test.cpp b/src/node/rpc/test/member_voting_test.cpp index 59ebe6ecb2c..f4d2c45e94e 100644 --- a/src/node/rpc/test/member_voting_test.cpp +++ b/src/node/rpc/test/member_voting_test.cpp @@ -2022,7 +2022,7 @@ DOCTEST_TEST_CASE("Submit recovery shares") members[id] = {cert, enc_kp}; } gen.set_recovery_threshold(recovery_threshold); - share_manager.issue_shares(gen_tx); + share_manager.issue_recovery_shares(gen_tx); gen.finalize(); frontend.open(); } diff --git a/src/node/share_manager.h b/src/node/share_manager.h index a5396ef47dc..c4acb0f0230 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -172,9 +172,9 @@ namespace ccf // First, generate a fresh ledger secrets wrapping key and wrap the // latest ledger secret with it. Then, encrypt the penultimate ledger // secret with the latest ledger secret and split the ledger secret - // wrapping key, allocating a new share for each active member. Finally, - // encrypt each share with the public key of each member and record it in - // the shares table. + // wrapping key, allocating a new share for each active recovery member. + // Finally, encrypt each share with the public key of each member and + // record it in the shares table. auto ls_wrapping_key = LedgerSecretWrappingKey(); auto wrapped_latest_ls = ls_wrapping_key.wrap(latest_ledger_secret); @@ -289,30 +289,20 @@ namespace ccf public: ShareManager(NetworkState& network_) : network(network_) {} - // TODO: Unify this with issue_shares_on_recovery() - void issue_shares(kv::Tx& tx) - { - // Assumes that the ledger secrets have not been updated since the - // last time shares have been issued (i.e. genesis or re-sharing only) - auto [latest, penultimate] = - network.ledger_secrets->get_latest_and_penultimate(tx); - set_recovery_shares_info(tx, latest.second, penultimate, latest.first); - } - - void issue_shares_on_recovery(kv::Tx& tx, kv::Version latest_ls_version) + void issue_recovery_shares(kv::Tx& tx) { auto [latest, penultimate] = network.ledger_secrets->get_latest_and_penultimate(tx); - set_recovery_shares_info( - tx, latest.second, penultimate, latest_ls_version); + set_recovery_shares_info(tx, latest.second, penultimate, latest.first); } - void issue_shares_on_rekey( + void issue_recovery_shares( kv::Tx& tx, const LedgerSecret& new_ledger_secret) { - // No version here, on recovery, the version is derived from the hook at - // which the ledger secret is applied to the store + // The version at which the new ledger secret is applicable from is + // derived from the hook at which the ledger secret is applied to the + // store set_recovery_shares_info( tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); } From 9ae2bb358d360b9bd62c9acf0504665f6b09048a Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 11:51:02 +0000 Subject: [PATCH 20/35] WIP splitting recovery shares table --- src/node/entities.h | 2 ++ src/node/genesis_gen.h | 6 ---- src/node/network_tables.h | 4 ++- src/node/share_manager.h | 31 ++++++++--------- src/node/shares.h | 71 ++++++++++++++++++++------------------- 5 files changed, 56 insertions(+), 58 deletions(-) diff --git a/src/node/entities.h b/src/node/entities.h index c4298096eef..7a5ad6cdbc8 100644 --- a/src/node/entities.h +++ b/src/node/entities.h @@ -81,6 +81,8 @@ namespace ccf static constexpr auto GOV_HISTORY = "public:ccf.gov.governance.history"; static constexpr auto SERVICE = "public:ccf.gov.service"; static constexpr auto SHARES = "public:ccf.gov.shares"; + static constexpr auto ENCRYPTED_PAST_LEDGER_SECRET = + "public:ccf.gov.encrypted_past_ledger_secret"; static constexpr auto CONFIGURATION = "public:ccf.gov.config"; static constexpr auto SUBMITTED_SHARES = "public:ccf.gov.submitted_shares"; static constexpr auto SNAPSHOT_EVIDENCE = diff --git a/src/node/genesis_gen.h b/src/node/genesis_gen.h index 9e858860b78..b876c11238c 100644 --- a/src/node/genesis_gen.h +++ b/src/node/genesis_gen.h @@ -444,12 +444,6 @@ namespace ccf codeid_view->put(node_code_id, CodeStatus::ALLOWED_TO_JOIN); } - void add_key_share_info(const RecoverySharesInfo& key_share_info) - { - auto shares_view = tx.get_view(tables.shares); - shares_view->put(0, key_share_info); - } - bool set_recovery_threshold(size_t threshold, bool allow_zero = false) { auto config_view = tx.get_view(tables.config); diff --git a/src/node/network_tables.h b/src/node/network_tables.h index af263727072..9589bd040a5 100644 --- a/src/node/network_tables.h +++ b/src/node/network_tables.h @@ -54,7 +54,8 @@ namespace ccf CodeIDs node_code_ids; MemberAcks member_acks; GovernanceHistory governance_history; - Shares shares; + RecoveryShares shares; + EncryptedPastLedgerSecret encrypted_past_ledger_secret; SubmittedShares submitted_shares; Configuration config; @@ -118,6 +119,7 @@ namespace ccf member_acks(Tables::MEMBER_ACKS), governance_history(Tables::GOV_HISTORY), shares(Tables::SHARES), + encrypted_previous_ledger_secret(Tables::ENCRYPTED_PAST_LEDGER_SECRET), submitted_shares(Tables::SUBMITTED_SHARES), config(Tables::CONFIGURATION), ca_certs(Tables::CA_CERT_DERS), diff --git a/src/node/share_manager.h b/src/node/share_manager.h index c4acb0f0230..32f57b4d56e 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -200,18 +200,17 @@ namespace ccf encrypted_previous_secret = encrypted_previous_ls.serialise(); } - GenesisGenerator g(network, tx); - - // LOG_FAIL_FMT( - // "Writing shares info: previous secret at {}, latest secret at {}", - // version_previous_secret, - // latest_ls_version); - - // TODO: Rename this - g.add_key_share_info( + // TODO: We shouldn't have to update both on pure re-share!! + auto recovery_shares_handle = tx.get_view(network.shares); + recovery_shares_handle->put( + 0, {{wrapped_latest_ls, latest_ls_version}, - {encrypted_previous_secret, version_previous_secret}, compute_encrypted_shares(tx, ls_wrapping_key)}); + + auto encrypted_past_ls_handle = + tx.get_view(network.encrypted_past_ledger_secret); + encrypted_past_ls_handle->put( + 0, {encrypted_previous_secret, version_previous_secret}); } std::vector encrypt_submitted_share( @@ -310,7 +309,6 @@ namespace ccf std::optional get_encrypted_share( kv::Tx& tx, MemberId member_id) { - std::optional encrypted_share = std::nullopt; auto recovery_shares_info = tx.get_view(network.shares)->get(0); if (!recovery_shares_info.has_value()) { @@ -318,14 +316,13 @@ namespace ccf "Failed to retrieve current recovery shares info"); } - for (auto const& s : recovery_shares_info->encrypted_shares) + auto search = recovery_shares_info->encrypted_shares.find(member_id); + if (search == recovery_shares_info->encrypted_shares.end()) { - if (s.first == member_id) - { - encrypted_share = s.second; - } + return std::nullopt; } - return encrypted_share; + + return search->second; } LedgerSecretsMap restore_recovery_shares_info( diff --git a/src/node/shares.h b/src/node/shares.h index d74f5c5f6ee..95db04985f1 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -28,58 +28,61 @@ namespace ccf // secrets have been recovered. std::optional version = std::nullopt; - MSGPACK_DEFINE(encrypted_data, version) + MSGPACK_DEFINE(data, version) }; DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(WrappedLedgerSecret) DECLARE_JSON_REQUIRED_FIELDS(WrappedLedgerSecret, encrypted_data) DECLARE_JSON_OPTIONAL_FIELDS(WrappedLedgerSecret, version) - // TODO: To unify with secrets.h encrypted ledger secret?? - struct PreviousEncryptedLedgerSecret + struct RecoverySharesInfo { - std::vector encrypted_data; + // Latest ledger secret wrapped with the ledger secret wrapping key + WrappedLedgerSecret wrapped_latest_ledger_secret; - kv::Version version; + // Recovery shares encrypted with each active recovery member's public + // encryption key + EncryptedSharesMap encrypted_shares; - MSGPACK_DEFINE(encrypted_data, version) + MSGPACK_DEFINE(wrapped_latest_ledger_secret, encrypted_shares); }; - DECLARE_JSON_TYPE(PreviousEncryptedLedgerSecret) + DECLARE_JSON_TYPE(RecoverySharesInfo) DECLARE_JSON_REQUIRED_FIELDS( - PreviousEncryptedLedgerSecret, encrypted_data, version) + RecoverySharesInfo, wrapped_latest_ledger_secret, encrypted_shares) - struct RecoverySharesInfo + struct EncryptedPastLedgerSecretInfo { - // Keeping track of the latest and penultimate ledger secret allows the - // value of this table to remain at a constant size through the lifetime of - // the service. On recovery, a local hook on this table allows the service - // to reconstruct the history of encrypted ledger secrets which are - // decrypted in sequence once the ledger secret wrapping key is - // re-assembled. - - // Latest ledger secret wrapped with the ledger secret wrapping key - WrappedLedgerSecret wrapped_latest_ledger_secret; + // Past ledger secret encrypted with the latest ledger secret + std::vector encrypted_data; - // Previous ledger secret encrypted with the latest ledger secret - PreviousEncryptedLedgerSecret encrypted_previous_ledger_secret; + // Version at which the ledger secret is applicable from + kv::Version version; - EncryptedSharesMap encrypted_shares; + // Version at which the ledger secret was written to the store + kv::Version stored_version; - MSGPACK_DEFINE( - wrapped_latest_ledger_secret, - encrypted_previous_ledger_secret, - encrypted_shares); + MSGPACK_DEFINE(encrypted_data, version, stored_version) }; - DECLARE_JSON_TYPE(RecoverySharesInfo) + DECLARE_JSON_TYPE(PreviousEncryptedLedgerSecret) DECLARE_JSON_REQUIRED_FIELDS( - RecoverySharesInfo, - wrapped_latest_ledger_secret, - encrypted_previous_ledger_secret, - encrypted_shares) - - // The key for this table will always be 0 since a live service never needs to - // access historical recovery shares info. - using Shares = kv::Map; + PreviousEncryptedLedgerSecret, encrypted_data, version.stored_version) + + // The following two tables are distinct because some operations trigger a + // re-share without requiring the ledger secrets to be updated (e.g. updating + // the recovery threshold), and vice versa (e.g. ledger rekey). For historical + // queries, when recovering ledger secrets from the ledger, the version at + // which the previous ledger secret was _written_ to the store must be known + // and can be deduced to the version at which the + // EncryptedPastLedgerSecret map was updated. + + // The key for this table is always 0. It is updated every time the member + // recovery shares are updated, e.g. when the recovery threshold is modified + using RecoveryShares = kv::Map; + + // The key for this table is always 0. It is updated every time the ledger + // secrets are updated, e.g. at startup or on ledger rekey + using EncryptedPastLedgerSecret = + kv::Map; } \ No newline at end of file From 0b8e232b932be3db60a43b778c2a44962501dcf4 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 12:05:55 +0000 Subject: [PATCH 21/35] Compiles --- src/node/network_tables.h | 2 +- src/node/node_state.h | 80 ++++++++++++++++-------------- src/node/share_manager.h | 101 ++++++++++++++++++++------------------ src/node/shares.h | 9 ++-- 4 files changed, 101 insertions(+), 91 deletions(-) diff --git a/src/node/network_tables.h b/src/node/network_tables.h index 9589bd040a5..a93ddef163a 100644 --- a/src/node/network_tables.h +++ b/src/node/network_tables.h @@ -119,7 +119,7 @@ namespace ccf member_acks(Tables::MEMBER_ACKS), governance_history(Tables::GOV_HISTORY), shares(Tables::SHARES), - encrypted_previous_ledger_secret(Tables::ENCRYPTED_PAST_LEDGER_SECRET), + encrypted_past_ledger_secret(Tables::ENCRYPTED_PAST_LEDGER_SECRET), submitted_shares(Tables::SUBMITTED_SHARES), config(Tables::CONFIGURATION), ca_certs(Tables::CA_CERT_DERS), diff --git a/src/node/node_state.h b/src/node/node_state.h index da86fa93086..e6302794cb9 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1605,44 +1605,50 @@ namespace ccf void setup_recovery_hook() { network.tables->set_map_hook( - network.shares.get_name(), - network.shares.wrap_map_hook( - [this](kv::Version version, const Shares::Write& w) + network.encrypted_past_ledger_secret.get_name(), + network.encrypted_past_ledger_secret.wrap_map_hook( + [this](kv::Version version, const EncryptedPastLedgerSecret::Write& w) -> kv::ConsensusHookPtr { - for (const auto& [k, opt_v] : w) - { - if (!opt_v.has_value()) - { - throw std::logic_error( - fmt::format("Unexpected: removal from shares table ({})", k)); - } - - const auto& v = opt_v.value(); - - // If the version is not set (rekeying), use the version - // from the hook plus one. Otherwise (recovery), use the - // version specified. - auto ledger_secret_version = - v.wrapped_latest_ledger_secret.version.value_or(version + 1); - - // Do not recover the ledger secrets in case of a pure re-share - // (e.g. recovery threshold update) as they are the same as in - // the previous entry. - if ( - recovery_ledger_secrets.empty() || - recovery_ledger_secrets.back().next_version != - ledger_secret_version) - { - LOG_TRACE_FMT( - "Recovering one encrypted ledger secret at version {} and " - "next version {}", - v.encrypted_previous_ledger_secret.version, - ledger_secret_version); - - recovery_ledger_secrets.push_back( - {ledger_secret_version, v.encrypted_previous_ledger_secret}); - } - } + (void)version; + (void)w; + (void)this; + // TODO: Rewrite hook + // for (const auto& [k, opt_v] : w) + // { + // if (!opt_v.has_value()) + // { + // throw std::logic_error( + // fmt::format("Unexpected: removal from shares table ({})", + // k)); + // } + + // const auto& v = opt_v.value(); + + // // If the version is not set (rekeying), use the version + // // from the hook plus one. Otherwise (recovery), use the + // // version specified. + // auto ledger_secret_version = + // v.wrapped_latest_ledger_secret.version.value_or(version + 1); + + // // Do not recover the ledger secrets in case of a pure re-share + // // (e.g. recovery threshold update) as they are the same as in + // // the previous entry. + // if ( + // recovery_ledger_secrets.empty() || + // recovery_ledger_secrets.back().next_version != + // ledger_secret_version) + // { + // LOG_TRACE_FMT( + // "Recovering one encrypted ledger secret at version {} and " + // "next version {}", + // v.encrypted_previous_ledger_secret.version, + // ledger_secret_version); + + // recovery_ledger_secrets.push_back( + // {ledger_secret_version, + // v.encrypted_previous_ledger_secret}); + // } + // } return kv::ConsensusHookPtr(nullptr); })); diff --git a/src/node/share_manager.h b/src/node/share_manager.h index c2c59528178..8c801089d31 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -86,21 +86,21 @@ namespace ccf } }; - // During recovery, a list of RecoveredLedgerSecret is constructed from a - // local hook. - struct RecoveredLedgerSecret - { - // Version at which the next ledger secret is applicable from - kv::Version next_version; + // // During recovery, a list of RecoveredLedgerSecret is constructed from a + // // local hook. + // struct RecoveredLedgerSecret + // { + // // Version at which the next ledger secret is applicable from + // kv::Version next_version; - PreviousEncryptedLedgerSecret encrypted_ledger_secret; + // PreviousEncryptedLedgerSecret encrypted_ledger_secret; - // Version at which the ledger secret is applicable from - // kv::Version version; + // // Version at which the ledger secret is applicable from + // // kv::Version version; - // Previous ledger secret, encrypted with the current ledger secret - // std::vector encrypted_ledger_secret; - }; + // // Previous ledger secret, encrypted with the current ledger secret + // // std::vector encrypted_ledger_secret; + // }; // The ShareManager class provides the interface between the ledger secrets, // the ccf.shares and ccf.submitted_shares KV tables and the rest of the @@ -111,7 +111,8 @@ namespace ccf // have successfully submitted their shares // TODO: Make this a map! - using RecoveredEncryptedLedgerSecrets = std::list; + using RecoveredEncryptedLedgerSecrets = + std::list; class ShareManager { @@ -332,7 +333,7 @@ namespace ccf auto ls_wrapping_key = combine_from_submitted_shares(tx); - auto recovery_shares_info = tx.rw(network.shares)->get(0); + auto recovery_shares_info = tx.ro(network.shares)->get(0); if (!recovery_shares_info.has_value()) { throw std::logic_error( @@ -349,41 +350,43 @@ namespace ccf "Recovering {} encrypted ledger secrets", encrypted_recovery_secrets.size()); - restored_ledger_secrets.emplace( - encrypted_recovery_secrets.back().next_version, std::move(restored_ls)); - - for (auto it = encrypted_recovery_secrets.rbegin(); - it != encrypted_recovery_secrets.rend(); - it++) - { - // TODO: Just ignore this instead?? - if (it->encrypted_ledger_secret.encrypted_data.empty()) - { - // First entry does not encrypt any other ledger secret (i.e. genesis) - break; - } - - crypto::GcmCipher encrypted_ls; - encrypted_ls.deserialise(it->encrypted_ledger_secret.encrypted_data); - std::vector decrypted_ls(encrypted_ls.cipher.size()); - - if (!crypto::KeyAesGcm(decryption_key) - .decrypt( - encrypted_ls.hdr.get_iv(), - encrypted_ls.hdr.tag, - encrypted_ls.cipher, - nullb, - decrypted_ls.data())) - { - throw std::logic_error(fmt::format( - "Decryption of ledger secret at {} failed", - it->encrypted_ledger_secret.version)); - } - - decryption_key = decrypted_ls; - restored_ledger_secrets.emplace( - it->encrypted_ledger_secret.version, std::move(decrypted_ls)); - } + // TODO: Remove use of next_version here!! + // restored_ledger_secrets.emplace( + // encrypted_recovery_secrets.back().next_version, + // std::move(restored_ls)); + + // for (auto it = encrypted_recovery_secrets.rbegin(); + // it != encrypted_recovery_secrets.rend(); + // it++) + // { + // // TODO: Just ignore this instead?? + // if (it->encrypted_ledger_secret.encrypted_data.empty()) + // { + // // First entry does not encrypt any other ledger secret (i.e. + // genesis) break; + // } + + // crypto::GcmCipher encrypted_ls; + // encrypted_ls.deserialise(it->encrypted_ledger_secret.encrypted_data); + // std::vector decrypted_ls(encrypted_ls.cipher.size()); + + // if (!crypto::KeyAesGcm(decryption_key) + // .decrypt( + // encrypted_ls.hdr.get_iv(), + // encrypted_ls.hdr.tag, + // encrypted_ls.cipher, + // nullb, + // decrypted_ls.data())) + // { + // throw std::logic_error(fmt::format( + // "Decryption of ledger secret at {} failed", + // it->encrypted_ledger_secret.version)); + // } + + // decryption_key = decrypted_ls; + // restored_ledger_secrets.emplace( + // it->encrypted_ledger_secret.version, std::move(decrypted_ls)); + // } return restored_ledger_secrets; } diff --git a/src/node/shares.h b/src/node/shares.h index 95db04985f1..185f94fa6bc 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -28,7 +28,7 @@ namespace ccf // secrets have been recovered. std::optional version = std::nullopt; - MSGPACK_DEFINE(data, version) + MSGPACK_DEFINE(encrypted_data, version) }; DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(WrappedLedgerSecret) @@ -60,14 +60,15 @@ namespace ccf kv::Version version; // Version at which the ledger secret was written to the store - kv::Version stored_version; + // TODO: Unused for now + kv::Version stored_version = kv::NoVersion; MSGPACK_DEFINE(encrypted_data, version, stored_version) }; - DECLARE_JSON_TYPE(PreviousEncryptedLedgerSecret) + DECLARE_JSON_TYPE(EncryptedPastLedgerSecretInfo) DECLARE_JSON_REQUIRED_FIELDS( - PreviousEncryptedLedgerSecret, encrypted_data, version.stored_version) + EncryptedPastLedgerSecretInfo, encrypted_data, version, stored_version) // The following two tables are distinct because some operations trigger a // re-share without requiring the ledger secrets to be updated (e.g. updating From d096e82cb35e9361e40e741c02a1a394e9c4e441 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 15:28:12 +0000 Subject: [PATCH 22/35] Recovery works again --- src/node/node_state.h | 84 ++++++++++++++++++---------------- src/node/share_manager.h | 98 +++++++++++++++++++++------------------- src/node/shares.h | 24 ++++++---- tests/recovery.py | 2 +- 4 files changed, 114 insertions(+), 94 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index e6302794cb9..d823e43c3fd 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1609,45 +1609,53 @@ namespace ccf network.encrypted_past_ledger_secret.wrap_map_hook( [this](kv::Version version, const EncryptedPastLedgerSecret::Write& w) -> kv::ConsensusHookPtr { - (void)version; - (void)w; - (void)this; - // TODO: Rewrite hook - // for (const auto& [k, opt_v] : w) + if (w.size() > 1) + { + throw std::logic_error(fmt::format( + "Unexpected: multiple writes to {} table", + network.encrypted_past_ledger_secret.get_name())); + } + + auto& encrypted_past_ledger_secret = w.at(0); + if (!encrypted_past_ledger_secret.has_value()) + { + throw std::logic_error(fmt::format( + "Unexpected: removal from {} table", + network.encrypted_past_ledger_secret.get_name())); + } + + auto next_ledger_secret_version = + encrypted_past_ledger_secret->next_version.value_or(version); + + LOG_FAIL_FMT( + "Next ledger secret at: {}", next_ledger_secret_version); + + recovery_ledger_secrets.emplace( + next_ledger_secret_version, + std::move(encrypted_past_ledger_secret.value())); + + // // If the version is not set (rekeying), use the version + // // from the hook plus one. Otherwise (recovery), use the + // // version specified. + // auto ledger_secret_version = + // v.wrapped_latest_ledger_secret.version.value_or(version + 1); + + // // Do not recover the ledger secrets in case of a pure re-share + // // (e.g. recovery threshold update) as they are the same as in + // // the previous entry. + // if ( + // recovery_ledger_secrets.empty() || + // recovery_ledger_secrets.back().next_version != + // ledger_secret_version) // { - // if (!opt_v.has_value()) - // { - // throw std::logic_error( - // fmt::format("Unexpected: removal from shares table ({})", - // k)); - // } - - // const auto& v = opt_v.value(); - - // // If the version is not set (rekeying), use the version - // // from the hook plus one. Otherwise (recovery), use the - // // version specified. - // auto ledger_secret_version = - // v.wrapped_latest_ledger_secret.version.value_or(version + 1); - - // // Do not recover the ledger secrets in case of a pure re-share - // // (e.g. recovery threshold update) as they are the same as in - // // the previous entry. - // if ( - // recovery_ledger_secrets.empty() || - // recovery_ledger_secrets.back().next_version != - // ledger_secret_version) - // { - // LOG_TRACE_FMT( - // "Recovering one encrypted ledger secret at version {} and " - // "next version {}", - // v.encrypted_previous_ledger_secret.version, - // ledger_secret_version); - - // recovery_ledger_secrets.push_back( - // {ledger_secret_version, - // v.encrypted_previous_ledger_secret}); - // } + // LOG_TRACE_FMT( + // "Recovering one encrypted ledger secret at version {} and " + // "next version {}", + // v.encrypted_previous_ledger_secret.version, + // ledger_secret_version); + + // recovery_ledger_secrets.push_back( + // {ledger_secret_version, v.encrypted_previous_ledger_secret}); // } return kv::ConsensusHookPtr(nullptr); diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 8c801089d31..31d64280c4c 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -86,8 +86,6 @@ namespace ccf } }; - // // During recovery, a list of RecoveredLedgerSecret is constructed from a - // // local hook. // struct RecoveredLedgerSecret // { // // Version at which the next ledger secret is applicable from @@ -109,11 +107,6 @@ namespace ccf // membership updates) // - Re-assemble the ledger secrets on recovery, once a threshold of members // have successfully submitted their shares - - // TODO: Make this a map! - using RecoveredEncryptedLedgerSecrets = - std::list; - class ShareManager { private: @@ -205,12 +198,15 @@ namespace ccf auto recovery_shares = tx.rw(network.shares); recovery_shares->put( 0, - {{wrapped_latest_ls, latest_ls_version}, - compute_encrypted_shares(tx, ls_wrapping_key)}); + {{wrapped_latest_ls}, compute_encrypted_shares(tx, ls_wrapping_key)}); auto encrypted_past_ls = tx.rw(network.encrypted_past_ledger_secret); encrypted_past_ls->put( - 0, {encrypted_previous_secret, version_previous_secret}); + 0, + {encrypted_previous_secret, + version_previous_secret, + std::nullopt, + latest_ls_version}); } std::vector encrypt_submitted_share( @@ -323,6 +319,11 @@ namespace ccf return search->second; } + // During recovery, a list of RecoveredEncryptedLedgerSecrets is constructed + // from the local hook on the encrypted ledger secrets table + using RecoveredEncryptedLedgerSecrets = + std::map; + LedgerSecretsMap restore_recovery_shares_info( kv::Tx& tx, const RecoveredEncryptedLedgerSecrets& encrypted_recovery_secrets) @@ -331,6 +332,12 @@ namespace ccf // encrypted shares. Then, unwrap the latest ledger secret and use it to // decrypt the previous ledger secret and so on. + if (encrypted_recovery_secrets.empty()) + { + throw std::logic_error( + "There should be at least one encrypted ledger secret to recover"); + } + auto ls_wrapping_key = combine_from_submitted_shares(tx); auto recovery_shares_info = tx.ro(network.shares)->get(0); @@ -350,43 +357,40 @@ namespace ccf "Recovering {} encrypted ledger secrets", encrypted_recovery_secrets.size()); - // TODO: Remove use of next_version here!! - // restored_ledger_secrets.emplace( - // encrypted_recovery_secrets.back().next_version, - // std::move(restored_ls)); - - // for (auto it = encrypted_recovery_secrets.rbegin(); - // it != encrypted_recovery_secrets.rend(); - // it++) - // { - // // TODO: Just ignore this instead?? - // if (it->encrypted_ledger_secret.encrypted_data.empty()) - // { - // // First entry does not encrypt any other ledger secret (i.e. - // genesis) break; - // } - - // crypto::GcmCipher encrypted_ls; - // encrypted_ls.deserialise(it->encrypted_ledger_secret.encrypted_data); - // std::vector decrypted_ls(encrypted_ls.cipher.size()); - - // if (!crypto::KeyAesGcm(decryption_key) - // .decrypt( - // encrypted_ls.hdr.get_iv(), - // encrypted_ls.hdr.tag, - // encrypted_ls.cipher, - // nullb, - // decrypted_ls.data())) - // { - // throw std::logic_error(fmt::format( - // "Decryption of ledger secret at {} failed", - // it->encrypted_ledger_secret.version)); - // } - - // decryption_key = decrypted_ls; - // restored_ledger_secrets.emplace( - // it->encrypted_ledger_secret.version, std::move(decrypted_ls)); - // } + restored_ledger_secrets.emplace( + encrypted_recovery_secrets.rbegin()->first, std::move(restored_ls)); + + for (auto it = encrypted_recovery_secrets.rbegin(); + it != encrypted_recovery_secrets.rend(); + it++) + { + // TODO: Use optional here instead + if (it->second.encrypted_data.empty()) + { + // First entry does not encrypt any other ledger secret (i.e. genesis) + break; + } + + crypto::GcmCipher encrypted_ls; + encrypted_ls.deserialise(it->second.encrypted_data); + std::vector decrypted_ls(encrypted_ls.cipher.size()); + + if (!crypto::KeyAesGcm(decryption_key) + .decrypt( + encrypted_ls.hdr.get_iv(), + encrypted_ls.hdr.tag, + encrypted_ls.cipher, + nullb, + decrypted_ls.data())) + { + throw std::logic_error(fmt::format( + "Decryption of ledger secret at {} failed", it->second.version)); + } + + decryption_key = decrypted_ls; + restored_ledger_secrets.emplace( + it->second.version, std::move(decrypted_ls)); + } return restored_ledger_secrets; } diff --git a/src/node/shares.h b/src/node/shares.h index 185f94fa6bc..57790f39253 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -15,6 +15,7 @@ namespace ccf using EncryptedShare = std::vector; using EncryptedSharesMap = std::map; + // TODO: Delete this type struct WrappedLedgerSecret { std::vector encrypted_data; @@ -26,14 +27,14 @@ namespace ccf // integrity on the public-only transactions. However, the corresponding // shares are only written at a later version, once the previous ledger // secrets have been recovered. - std::optional version = std::nullopt; + // std::optional version = std::nullopt; - MSGPACK_DEFINE(encrypted_data, version) + MSGPACK_DEFINE(encrypted_data) }; - DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(WrappedLedgerSecret) + DECLARE_JSON_TYPE(WrappedLedgerSecret) DECLARE_JSON_REQUIRED_FIELDS(WrappedLedgerSecret, encrypted_data) - DECLARE_JSON_OPTIONAL_FIELDS(WrappedLedgerSecret, version) + // DECLARE_JSON_OPTIONAL_FIELDS(WrappedLedgerSecret, version) struct RecoverySharesInfo { @@ -51,6 +52,7 @@ namespace ccf DECLARE_JSON_REQUIRED_FIELDS( RecoverySharesInfo, wrapped_latest_ledger_secret, encrypted_shares) + // TODO: Perhaps rename this?? struct EncryptedPastLedgerSecretInfo { // Past ledger secret encrypted with the latest ledger secret @@ -61,14 +63,20 @@ namespace ccf // Version at which the ledger secret was written to the store // TODO: Unused for now - kv::Version stored_version = kv::NoVersion; + std::optional stored_version = std::nullopt; - MSGPACK_DEFINE(encrypted_data, version, stored_version) + // Version at which the _next_ ledger secret is applicable from + // TODO: Paste larger comment from the top of this file + std::optional next_version = std::nullopt; + + MSGPACK_DEFINE(encrypted_data, version, stored_version, next_version) }; - DECLARE_JSON_TYPE(EncryptedPastLedgerSecretInfo) + DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(EncryptedPastLedgerSecretInfo) DECLARE_JSON_REQUIRED_FIELDS( - EncryptedPastLedgerSecretInfo, encrypted_data, version, stored_version) + EncryptedPastLedgerSecretInfo, encrypted_data, version) + DECLARE_JSON_OPTIONAL_FIELDS( + EncryptedPastLedgerSecretInfo, stored_version, next_version) // The following two tables are distinct because some operations trigger a // re-share without requiring the ledger secrets to be updated (e.g. updating diff --git a/tests/recovery.py b/tests/recovery.py index 9876ce3dfef..3acfdd26aaf 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -122,7 +122,7 @@ def run(args): # network, args, from_snapshot=False # ) # else: - recovered_network = test(network, args, from_snapshot=True) + recovered_network = test(network, args, from_snapshot=False) network = recovered_network LOG.success("Recovery complete on all nodes") From 1dbe7214a431dfb01a364b542276dc1e0663857b Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 15:36:16 +0000 Subject: [PATCH 23/35] Simplify shares table --- src/node/share_manager.h | 36 +++++++++++------------------------- src/node/shares.h | 27 ++++----------------------- 2 files changed, 15 insertions(+), 48 deletions(-) diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 31d64280c4c..dcf09e71221 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -86,23 +86,15 @@ namespace ccf } }; - // struct RecoveredLedgerSecret - // { - // // Version at which the next ledger secret is applicable from - // kv::Version next_version; - - // PreviousEncryptedLedgerSecret encrypted_ledger_secret; - - // // Version at which the ledger secret is applicable from - // // kv::Version version; - - // // Previous ledger secret, encrypted with the current ledger secret - // // std::vector encrypted_ledger_secret; - // }; - - // The ShareManager class provides the interface between the ledger secrets, - // the ccf.shares and ccf.submitted_shares KV tables and the rest of the - // service. In particular, it is used to: + // During recovery, a list of RecoveredEncryptedLedgerSecrets is constructed + // from the local hook on the encrypted ledger secrets table. The key for each + // entry is the version at which the next ledger secret is applicable from. + using RecoveredEncryptedLedgerSecrets = + std::map; + + // The ShareManager class provides the interface between the ledger secrets + // object and the shares, ledger secrets and submitted shares KV tables. In + // particular, it is used to: // - Issue new recovery shares whenever required (e.g. on startup, rekey and // membership updates) // - Re-assemble the ledger secrets on recovery, once a threshold of members @@ -197,8 +189,7 @@ namespace ccf // TODO: We shouldn't have to update both on pure re-share!! auto recovery_shares = tx.rw(network.shares); recovery_shares->put( - 0, - {{wrapped_latest_ls}, compute_encrypted_shares(tx, ls_wrapping_key)}); + 0, {wrapped_latest_ls, compute_encrypted_shares(tx, ls_wrapping_key)}); auto encrypted_past_ls = tx.rw(network.encrypted_past_ledger_secret); encrypted_past_ls->put( @@ -319,11 +310,6 @@ namespace ccf return search->second; } - // During recovery, a list of RecoveredEncryptedLedgerSecrets is constructed - // from the local hook on the encrypted ledger secrets table - using RecoveredEncryptedLedgerSecrets = - std::map; - LedgerSecretsMap restore_recovery_shares_info( kv::Tx& tx, const RecoveredEncryptedLedgerSecrets& encrypted_recovery_secrets) @@ -350,7 +336,7 @@ namespace ccf LedgerSecretsMap restored_ledger_secrets; auto restored_ls = ls_wrapping_key.unwrap( - recovery_shares_info->wrapped_latest_ledger_secret.encrypted_data); + recovery_shares_info->wrapped_latest_ledger_secret); auto decryption_key = restored_ls.raw_key; LOG_FAIL_FMT( diff --git a/src/node/shares.h b/src/node/shares.h index 57790f39253..b637d2d7bc3 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -15,31 +15,10 @@ namespace ccf using EncryptedShare = std::vector; using EncryptedSharesMap = std::map; - // TODO: Delete this type - struct WrappedLedgerSecret - { - std::vector encrypted_data; - - // In most cases (e.g. re-key, member retirement), this is unset and the - // version at which the ledger secret is applicable from is derived from the - // version at which the recovery hook is triggered. In other cases (service - // open or in recovery), a new ledger secret is created to protect the - // integrity on the public-only transactions. However, the corresponding - // shares are only written at a later version, once the previous ledger - // secrets have been recovered. - // std::optional version = std::nullopt; - - MSGPACK_DEFINE(encrypted_data) - }; - - DECLARE_JSON_TYPE(WrappedLedgerSecret) - DECLARE_JSON_REQUIRED_FIELDS(WrappedLedgerSecret, encrypted_data) - // DECLARE_JSON_OPTIONAL_FIELDS(WrappedLedgerSecret, version) - struct RecoverySharesInfo { // Latest ledger secret wrapped with the ledger secret wrapping key - WrappedLedgerSecret wrapped_latest_ledger_secret; + std::vector wrapped_latest_ledger_secret; // Recovery shares encrypted with each active recovery member's public // encryption key @@ -88,10 +67,12 @@ namespace ccf // The key for this table is always 0. It is updated every time the member // recovery shares are updated, e.g. when the recovery threshold is modified + // and when the ledger secret is updated using RecoveryShares = kv::Map; // The key for this table is always 0. It is updated every time the ledger - // secrets are updated, e.g. at startup or on ledger rekey + // secret is updated, e.g. at startup or on ledger rekey. It is not updated on + // a pure re-share. using EncryptedPastLedgerSecret = kv::Map; } \ No newline at end of file From 57208fe3450a3b3fe32f5082a9fd40a7c783dff8 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 17:16:24 +0000 Subject: [PATCH 24/35] Recovery test works again! --- src/node/entities.h | 4 +-- src/node/network_tables.h | 4 +-- src/node/node_state.h | 51 ++++++++------------------- src/node/share_manager.h | 64 ++++++++++++++++++---------------- src/node/shares.h | 73 +++++++++++++++++++++++++++++++-------- tests/suite/test_suite.py | 2 +- 6 files changed, 112 insertions(+), 86 deletions(-) diff --git a/src/node/entities.h b/src/node/entities.h index 7a5ad6cdbc8..3ba216b0a3d 100644 --- a/src/node/entities.h +++ b/src/node/entities.h @@ -81,8 +81,8 @@ namespace ccf static constexpr auto GOV_HISTORY = "public:ccf.gov.governance.history"; static constexpr auto SERVICE = "public:ccf.gov.service"; static constexpr auto SHARES = "public:ccf.gov.shares"; - static constexpr auto ENCRYPTED_PAST_LEDGER_SECRET = - "public:ccf.gov.encrypted_past_ledger_secret"; + static constexpr auto ENCRYPTED_LEDGER_SECRETS = + "public:ccf.gov.encrypted_ledger_secrets"; static constexpr auto CONFIGURATION = "public:ccf.gov.config"; static constexpr auto SUBMITTED_SHARES = "public:ccf.gov.submitted_shares"; static constexpr auto SNAPSHOT_EVIDENCE = diff --git a/src/node/network_tables.h b/src/node/network_tables.h index a93ddef163a..09b10214a42 100644 --- a/src/node/network_tables.h +++ b/src/node/network_tables.h @@ -55,7 +55,7 @@ namespace ccf MemberAcks member_acks; GovernanceHistory governance_history; RecoveryShares shares; - EncryptedPastLedgerSecret encrypted_past_ledger_secret; + EncryptedLedgerSecretsInfo encrypted_ledger_secrets; SubmittedShares submitted_shares; Configuration config; @@ -119,7 +119,7 @@ namespace ccf member_acks(Tables::MEMBER_ACKS), governance_history(Tables::GOV_HISTORY), shares(Tables::SHARES), - encrypted_past_ledger_secret(Tables::ENCRYPTED_PAST_LEDGER_SECRET), + encrypted_ledger_secrets(Tables::ENCRYPTED_LEDGER_SECRETS), submitted_shares(Tables::SUBMITTED_SHARES), config(Tables::CONFIGURATION), ca_certs(Tables::CA_CERT_DERS), diff --git a/src/node/node_state.h b/src/node/node_state.h index d823e43c3fd..784f1bef738 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1123,8 +1123,8 @@ namespace ccf std::lock_guard guard(lock); sm.expect(State::partOfPublicNetwork); - auto restored_ledger_secrets = - share_manager.restore_recovery_shares_info(tx, recovery_ledger_secrets); + auto restored_ledger_secrets = share_manager.restore_recovery_shares_info( + tx, std::move(recovery_ledger_secrets)); // Broadcast decrypted ledger secrets to other nodes for them to initiate // private recovery too @@ -1146,7 +1146,6 @@ namespace ccf ledger_idx = recovery_store->current_version(); read_ledger_idx(++ledger_idx); - recovery_ledger_secrets.clear(); sm.advance(State::readingPrivateLedger); } @@ -1605,58 +1604,35 @@ namespace ccf void setup_recovery_hook() { network.tables->set_map_hook( - network.encrypted_past_ledger_secret.get_name(), - network.encrypted_past_ledger_secret.wrap_map_hook( - [this](kv::Version version, const EncryptedPastLedgerSecret::Write& w) + network.encrypted_ledger_secrets.get_name(), + network.encrypted_ledger_secrets.wrap_map_hook( + [this]( + kv::Version version, const EncryptedLedgerSecretsInfo::Write& w) -> kv::ConsensusHookPtr { if (w.size() > 1) { throw std::logic_error(fmt::format( "Unexpected: multiple writes to {} table", - network.encrypted_past_ledger_secret.get_name())); + network.encrypted_ledger_secrets.get_name())); } - auto& encrypted_past_ledger_secret = w.at(0); - if (!encrypted_past_ledger_secret.has_value()) + auto& encrypted_ledger_secret = w.at(0); + if (!encrypted_ledger_secret.has_value()) { throw std::logic_error(fmt::format( "Unexpected: removal from {} table", - network.encrypted_past_ledger_secret.get_name())); + network.encrypted_ledger_secrets.get_name())); } auto next_ledger_secret_version = - encrypted_past_ledger_secret->next_version.value_or(version); + encrypted_ledger_secret->next_version.value_or(version); LOG_FAIL_FMT( "Next ledger secret at: {}", next_ledger_secret_version); recovery_ledger_secrets.emplace( next_ledger_secret_version, - std::move(encrypted_past_ledger_secret.value())); - - // // If the version is not set (rekeying), use the version - // // from the hook plus one. Otherwise (recovery), use the - // // version specified. - // auto ledger_secret_version = - // v.wrapped_latest_ledger_secret.version.value_or(version + 1); - - // // Do not recover the ledger secrets in case of a pure re-share - // // (e.g. recovery threshold update) as they are the same as in - // // the previous entry. - // if ( - // recovery_ledger_secrets.empty() || - // recovery_ledger_secrets.back().next_version != - // ledger_secret_version) - // { - // LOG_TRACE_FMT( - // "Recovering one encrypted ledger secret at version {} and " - // "next version {}", - // v.encrypted_previous_ledger_secret.version, - // ledger_secret_version); - - // recovery_ledger_secrets.push_back( - // {ledger_secret_version, v.encrypted_previous_ledger_secret}); - // } + std::move(encrypted_ledger_secret->previous_ledger_secret)); return kv::ConsensusHookPtr(nullptr); })); @@ -1664,7 +1640,8 @@ namespace ccf void reset_recovery_hook() { - network.tables->unset_map_hook(network.shares.get_name()); + network.tables->unset_map_hook( + network.encrypted_ledger_secrets.get_name()); } void setup_n2n_channels() diff --git a/src/node/share_manager.h b/src/node/share_manager.h index dcf09e71221..790531219dd 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -90,7 +90,7 @@ namespace ccf // from the local hook on the encrypted ledger secrets table. The key for each // entry is the version at which the next ledger secret is applicable from. using RecoveredEncryptedLedgerSecrets = - std::map; + std::map>; // The ShareManager class provides the interface between the ledger secrets // object and the shares, ledger secrets and submitted shares KV tables. In @@ -162,8 +162,15 @@ namespace ccf // Finally, encrypt each share with the public key of each member and // record it in the shares table. + // TODO: We shouldn't have to update both on pure re-share!! + auto ls_wrapping_key = LedgerSecretWrappingKey(); auto wrapped_latest_ls = ls_wrapping_key.wrap(latest_ledger_secret); + auto recovery_shares = tx.rw(network.shares); + recovery_shares->put( + 0, {wrapped_latest_ls, compute_encrypted_shares(tx, ls_wrapping_key)}); + + auto encrypted_ls = tx.rw(network.encrypted_ledger_secrets); std::vector encrypted_previous_secret = {}; kv::Version version_previous_secret = kv::NoVersion; @@ -184,20 +191,18 @@ namespace ccf encrypted_previous_ls.hdr.tag); encrypted_previous_secret = encrypted_previous_ls.serialise(); + encrypted_ls->put( + 0, + {PreviousLedgerSecretInfo( + std::move(encrypted_previous_secret), + version_previous_secret, + std::nullopt), + latest_ls_version}); + } + else + { + encrypted_ls->put(0, {std::nullopt, latest_ls_version}); } - - // TODO: We shouldn't have to update both on pure re-share!! - auto recovery_shares = tx.rw(network.shares); - recovery_shares->put( - 0, {wrapped_latest_ls, compute_encrypted_shares(tx, ls_wrapping_key)}); - - auto encrypted_past_ls = tx.rw(network.encrypted_past_ledger_secret); - encrypted_past_ls->put( - 0, - {encrypted_previous_secret, - version_previous_secret, - std::nullopt, - latest_ls_version}); } std::vector encrypt_submitted_share( @@ -311,21 +316,18 @@ namespace ccf } LedgerSecretsMap restore_recovery_shares_info( - kv::Tx& tx, - const RecoveredEncryptedLedgerSecrets& encrypted_recovery_secrets) + kv::Tx& tx, RecoveredEncryptedLedgerSecrets&& recovery_ledger_secrets) { // First, re-assemble the ledger secret wrapping key from the submitted // encrypted shares. Then, unwrap the latest ledger secret and use it to // decrypt the previous ledger secret and so on. - if (encrypted_recovery_secrets.empty()) + if (recovery_ledger_secrets.empty()) { throw std::logic_error( "There should be at least one encrypted ledger secret to recover"); } - auto ls_wrapping_key = combine_from_submitted_shares(tx); - auto recovery_shares_info = tx.ro(network.shares)->get(0); if (!recovery_shares_info.has_value()) { @@ -335,30 +337,30 @@ namespace ccf LedgerSecretsMap restored_ledger_secrets; - auto restored_ls = ls_wrapping_key.unwrap( + auto restored_ls = combine_from_submitted_shares(tx).unwrap( recovery_shares_info->wrapped_latest_ledger_secret); auto decryption_key = restored_ls.raw_key; LOG_FAIL_FMT( "Recovering {} encrypted ledger secrets", - encrypted_recovery_secrets.size()); + recovery_ledger_secrets.size()); restored_ledger_secrets.emplace( - encrypted_recovery_secrets.rbegin()->first, std::move(restored_ls)); + recovery_ledger_secrets.rbegin()->first, std::move(restored_ls)); - for (auto it = encrypted_recovery_secrets.rbegin(); - it != encrypted_recovery_secrets.rend(); + for (auto it = recovery_ledger_secrets.rbegin(); + it != recovery_ledger_secrets.rend(); it++) { - // TODO: Use optional here instead - if (it->second.encrypted_data.empty()) + if (!it->second.has_value()) { - // First entry does not encrypt any other ledger secret (i.e. genesis) + // Very first entry does not encrypt any other ledger secret + LOG_FAIL_FMT("Skipping!"); // TODO: Remove break; } crypto::GcmCipher encrypted_ls; - encrypted_ls.deserialise(it->second.encrypted_data); + encrypted_ls.deserialise(it->second->encrypted_data); std::vector decrypted_ls(encrypted_ls.cipher.size()); if (!crypto::KeyAesGcm(decryption_key) @@ -370,14 +372,16 @@ namespace ccf decrypted_ls.data())) { throw std::logic_error(fmt::format( - "Decryption of ledger secret at {} failed", it->second.version)); + "Decryption of ledger secret at {} failed", it->second->version)); } decryption_key = decrypted_ls; restored_ledger_secrets.emplace( - it->second.version, std::move(decrypted_ls)); + std::next(it)->second->version, std::move(decrypted_ls)); } + recovery_ledger_secrets.clear(); + return restored_ledger_secrets; } diff --git a/src/node/shares.h b/src/node/shares.h index b637d2d7bc3..f9cd7063eea 100644 --- a/src/node/shares.h +++ b/src/node/shares.h @@ -31,31 +31,77 @@ namespace ccf DECLARE_JSON_REQUIRED_FIELDS( RecoverySharesInfo, wrapped_latest_ledger_secret, encrypted_shares) - // TODO: Perhaps rename this?? - struct EncryptedPastLedgerSecretInfo + struct PreviousLedgerSecretInfo { // Past ledger secret encrypted with the latest ledger secret - std::vector encrypted_data; + std::vector encrypted_data = {}; // Version at which the ledger secret is applicable from - kv::Version version; + kv::Version version = kv::NoVersion; - // Version at which the ledger secret was written to the store - // TODO: Unused for now + // Version at which the ledger secret was written to the store (unused for + // now) std::optional stored_version = std::nullopt; + PreviousLedgerSecretInfo() = default; + + PreviousLedgerSecretInfo( + std::vector&& encrypted_data_, + kv::Version version_, + std::optional stored_version_) : + encrypted_data(std::move(encrypted_data_)), + version(version_), + stored_version(stored_version_) + {} + + bool operator==(const PreviousLedgerSecretInfo& other) const + { + return encrypted_data == other.encrypted_data && + version == other.version && stored_version == other.stored_version; + } + + bool operator!=(const PreviousLedgerSecretInfo& other) const + { + return !(*this == other); + } + + MSGPACK_DEFINE(encrypted_data, version, stored_version) + }; + + DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(PreviousLedgerSecretInfo) + DECLARE_JSON_REQUIRED_FIELDS( + PreviousLedgerSecretInfo, encrypted_data, version) + DECLARE_JSON_OPTIONAL_FIELDS(PreviousLedgerSecretInfo, stored_version) + + struct EncryptedLedgerSecretInfo + { + // Previous ledger secret info, encrypted with the current ledger secret. + // Unset on service opening. + std::optional previous_ledger_secret = + std::nullopt; + // Version at which the _next_ ledger secret is applicable from - // TODO: Paste larger comment from the top of this file + // Note: In most cases (e.g. re-key, member retirement), this is unset and + // the version at which the next ledger secret is applicable from is + // derived from the local hook on recovery. In one case (i.e. after recovery + // of the public ledger), a new ledger secret is created to protect the + // integrity on the public-only transactions. However, the corresponding + // shares are only written at a later version, once the previous ledger + // secrets have been restored. std::optional next_version = std::nullopt; - MSGPACK_DEFINE(encrypted_data, version, stored_version, next_version) + MSGPACK_DEFINE(previous_ledger_secret, next_version) }; - DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(EncryptedPastLedgerSecretInfo) - DECLARE_JSON_REQUIRED_FIELDS( - EncryptedPastLedgerSecretInfo, encrypted_data, version) + // Note: Both fields are never empty at the same time + DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(EncryptedLedgerSecretInfo) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-parameter" +#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" + DECLARE_JSON_REQUIRED_FIELDS(EncryptedLedgerSecretInfo) +#pragma clang diagnostic pop DECLARE_JSON_OPTIONAL_FIELDS( - EncryptedPastLedgerSecretInfo, stored_version, next_version) + EncryptedLedgerSecretInfo, previous_ledger_secret, next_version) // The following two tables are distinct because some operations trigger a // re-share without requiring the ledger secrets to be updated (e.g. updating @@ -73,6 +119,5 @@ namespace ccf // The key for this table is always 0. It is updated every time the ledger // secret is updated, e.g. at startup or on ledger rekey. It is not updated on // a pure re-share. - using EncryptedPastLedgerSecret = - kv::Map; + using EncryptedLedgerSecretsInfo = kv::Map; } \ No newline at end of file diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 594f71ceba3..7820e98e0f4 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -20,7 +20,7 @@ suite_rekey_recovery = [ e2e_logging.test_historical_query, rekey.test, - membership.test_set_recovery_threshold, + # membership.test_set_recovery_threshold, rekey.test, # reconfiguration.test_add_node_from_snapshot, recovery.test, From e18f912a02f035494a0a84afdd0ac7295c9753f3 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 29 Jan 2021 17:44:44 +0000 Subject: [PATCH 25/35] Add pure reshuffle --- src/node/rpc/member_frontend.h | 5 ++--- src/node/share_manager.h | 39 +++++++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index 63f4307a54b..98df3a63b93 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -975,7 +975,7 @@ namespace ccf const ProposalId& proposal_id, kv::Tx& tx, const nlohmann::json&) { try { - share_manager.issue_recovery_shares(tx); + share_manager.reshuffle_recovery_shares(tx); } catch (const std::logic_error& e) { @@ -1008,10 +1008,9 @@ namespace ccf return false; } - // Update recovery shares (same number of shares) try { - share_manager.issue_recovery_shares(tx); + share_manager.reshuffle_recovery_shares(tx); } catch (const std::logic_error& e) { diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 790531219dd..5b26a32c7c9 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -148,6 +148,16 @@ namespace ccf return encrypted_shares; } + void shuffle_recovery_shares( + kv::Tx& tx, const LedgerSecret& latest_ledger_secret) + { + auto ls_wrapping_key = LedgerSecretWrappingKey(); + auto wrapped_latest_ls = ls_wrapping_key.wrap(latest_ledger_secret); + auto recovery_shares = tx.rw(network.shares); + recovery_shares->put( + 0, {wrapped_latest_ls, compute_encrypted_shares(tx, ls_wrapping_key)}); + } + void set_recovery_shares_info( kv::Tx& tx, const LedgerSecret& latest_ledger_secret, @@ -162,13 +172,7 @@ namespace ccf // Finally, encrypt each share with the public key of each member and // record it in the shares table. - // TODO: We shouldn't have to update both on pure re-share!! - - auto ls_wrapping_key = LedgerSecretWrappingKey(); - auto wrapped_latest_ls = ls_wrapping_key.wrap(latest_ledger_secret); - auto recovery_shares = tx.rw(network.shares); - recovery_shares->put( - 0, {wrapped_latest_ls, compute_encrypted_shares(tx, ls_wrapping_key)}); + shuffle_recovery_shares(tx, latest_ledger_secret); auto encrypted_ls = tx.rw(network.encrypted_ledger_secrets); @@ -280,6 +284,9 @@ namespace ccf void issue_recovery_shares(kv::Tx& tx) { + // Issue new recovery shares for the current ledger secret, recording the + // wrapped new ledger secret and encrypted previous ledger secret in the + // store. auto [latest, penultimate] = network.ledger_secrets->get_latest_and_penultimate(tx); @@ -289,13 +296,25 @@ namespace ccf void issue_recovery_shares( kv::Tx& tx, const LedgerSecret& new_ledger_secret) { - // The version at which the new ledger secret is applicable from is - // derived from the hook at which the ledger secret is applied to the - // store + // Issue new recovery shares of the new ledger secret, recording the + // wrapped new ledger secret and encrypted previous ledger secret in the + // store. + // Note: The version at which the new ledger secret is applicable from is + // derived from the hook at which the ledgersecret is applied to the + // store. set_recovery_shares_info( tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); } + void reshuffle_recovery_shares(kv::Tx& tx) + { + // Issue new recovery shares of the _same_ current ledger secret to all + // active recovery members. The encrypted ledger secrets recorded in the + // store are _not_ updated. + shuffle_recovery_shares( + tx, network.ledger_secrets->get_latest(tx).second); + } + std::optional get_encrypted_share( kv::Tx& tx, MemberId member_id) { From af8918eb8df2c102d70ea3eccbdbd1db9d0ff777 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sun, 31 Jan 2021 20:01:46 +0000 Subject: [PATCH 26/35] Snapshot fix --- src/node/ledger_secrets.h | 7 +++++++ src/node/node_state.h | 6 +++--- src/node/share_manager.h | 29 ++++++++++++++++++++++++++--- tests/recovery.py | 4 ++-- tests/suite/test_requirements.py | 10 +++++----- tests/suite/test_suite.py | 2 +- 6 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 8518de1de30..06709ef2337 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -63,6 +63,8 @@ namespace ccf const LedgerSecret& get_secret_for_version( kv::Version version, bool historical_hint = false) { + LOG_FAIL_FMT("Getting secret for version {}", version); + if (ledger_secrets.empty()) { throw std::logic_error("Ledger secrets map is empty"); @@ -112,6 +114,11 @@ namespace ccf last_used_secret_it = std::prev(search); } + LOG_FAIL_FMT( + "Using key at {}: {}", + std::prev(search)->first, + tls::b64_from_raw(std::prev(search)->second.raw_key)); + return std::prev(search)->second; } diff --git a/src/node/node_state.h b/src/node/node_state.h index 784f1bef738..cf543fad52b 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1123,8 +1123,8 @@ namespace ccf std::lock_guard guard(lock); sm.expect(State::partOfPublicNetwork); - auto restored_ledger_secrets = share_manager.restore_recovery_shares_info( - tx, std::move(recovery_ledger_secrets)); + auto restored_ledger_secrets = + share_manager.restore_recovery_shares_info(tx, recovery_ledger_secrets); // Broadcast decrypted ledger secrets to other nodes for them to initiate // private recovery too @@ -1625,7 +1625,7 @@ namespace ccf } auto next_ledger_secret_version = - encrypted_ledger_secret->next_version.value_or(version); + encrypted_ledger_secret->next_version.value_or(version + 1); LOG_FAIL_FMT( "Next ledger secret at: {}", next_ledger_secret_version); diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 5b26a32c7c9..31f69b2d52b 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -195,6 +195,11 @@ namespace ccf encrypted_previous_ls.hdr.tag); encrypted_previous_secret = encrypted_previous_ls.serialise(); + + LOG_FAIL_FMT( + "Setting recovery share info with previous secret at " + "{}", + version_previous_secret); encrypted_ls->put( 0, {PreviousLedgerSecretInfo( @@ -205,6 +210,7 @@ namespace ccf } else { + LOG_FAIL_FMT("Setting recovery share info with no previous secret"); encrypted_ls->put(0, {std::nullopt, latest_ls_version}); } } @@ -311,6 +317,7 @@ namespace ccf // Issue new recovery shares of the _same_ current ledger secret to all // active recovery members. The encrypted ledger secrets recorded in the // store are _not_ updated. + LOG_FAIL_FMT("Shuffling recovery shares"); shuffle_recovery_shares( tx, network.ledger_secrets->get_latest(tx).second); } @@ -334,8 +341,10 @@ namespace ccf return search->second; } + // TODO: rvalue recovery_ledger_secrets LedgerSecretsMap restore_recovery_shares_info( - kv::Tx& tx, RecoveredEncryptedLedgerSecrets&& recovery_ledger_secrets) + kv::Tx& tx, + const RecoveredEncryptedLedgerSecrets& recovery_ledger_secrets) { // First, re-assemble the ledger secret wrapping key from the submitted // encrypted shares. Then, unwrap the latest ledger secret and use it to @@ -367,10 +376,13 @@ namespace ccf restored_ledger_secrets.emplace( recovery_ledger_secrets.rbegin()->first, std::move(restored_ls)); + LOG_FAIL_FMT("Emplaced"); + for (auto it = recovery_ledger_secrets.rbegin(); it != recovery_ledger_secrets.rend(); it++) { + LOG_FAIL_FMT("First!"); if (!it->second.has_value()) { // Very first entry does not encrypt any other ledger secret @@ -394,12 +406,23 @@ namespace ccf "Decryption of ledger secret at {} failed", it->second->version)); } + LOG_FAIL_FMT("Recovering ledger secret at {}", it->second->version); decryption_key = decrypted_ls; restored_ledger_secrets.emplace( - std::next(it)->second->version, std::move(decrypted_ls)); + it->second->version, std::move(decrypted_ls)); + // if (std::next(it) == recovery_ledger_secrets.rend()) + // { + // restored_ledger_secrets.emplace(it-> , + // std::move(decrypted_ls)); + // } + // else + // { + // restored_ledger_secrets.emplace( + // std::next(it)->first, std::move(decrypted_ls)); + // } } - recovery_ledger_secrets.clear(); + // recovery_ledger_secrets.clear(); return restored_ledger_secrets; } diff --git a/tests/recovery.py b/tests/recovery.py index 3acfdd26aaf..06b76111a8c 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -38,7 +38,7 @@ def test(network, args, from_snapshot=False): @reqs.description("Recovering a network, kill one node while submitting shares") @reqs.recover(number_txs=2) -def test_share_resilience(network, args, from_snapshot=True): +def test_share_resilience(network, args, from_snapshot=False): old_primary, _ = network.find_primary() snapshot_dir = None @@ -122,7 +122,7 @@ def run(args): # network, args, from_snapshot=False # ) # else: - recovered_network = test(network, args, from_snapshot=False) + recovered_network = test(network, args, from_snapshot=True) network = recovered_network LOG.success("Recovery complete on all nodes") diff --git a/tests/suite/test_requirements.py b/tests/suite/test_requirements.py index 147c17de5bb..511849f4e48 100644 --- a/tests/suite/test_requirements.py +++ b/tests/suite/test_requirements.py @@ -153,11 +153,11 @@ def wrapper(*args, **kwargs): network=network, number_txs=infra.e2e_args.get("msgs_per_recovery") or number_txs, ) - network.txs.issue( - network=network, - number_txs=1, - repeat=True, - ) + # network.txs.issue( + # network=network, + # number_txs=1, + # repeat=True, + # ) new_network = func(*args, **kwargs) new_network.txs.verify( network=new_network, diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 7820e98e0f4..594f71ceba3 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -20,7 +20,7 @@ suite_rekey_recovery = [ e2e_logging.test_historical_query, rekey.test, - # membership.test_set_recovery_threshold, + membership.test_set_recovery_threshold, rekey.test, # reconfiguration.test_add_node_from_snapshot, recovery.test, From 83d354885e636ba359def77eabb66a4699e9bebc Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sun, 31 Jan 2021 20:58:12 +0000 Subject: [PATCH 27/35] Fixed unit test --- src/node/node_state.h | 25 ++++++++++--------- src/node/rpc/test/node_stub.h | 8 +++++- src/node/share_manager.h | 46 ++++++++++++++--------------------- 3 files changed, 39 insertions(+), 40 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index cf543fad52b..1ab8c7cb938 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1123,8 +1123,8 @@ namespace ccf std::lock_guard guard(lock); sm.expect(State::partOfPublicNetwork); - auto restored_ledger_secrets = - share_manager.restore_recovery_shares_info(tx, recovery_ledger_secrets); + auto restored_ledger_secrets = share_manager.restore_recovery_shares_info( + tx, std::move(recovery_ledger_secrets)); // Broadcast decrypted ledger secrets to other nodes for them to initiate // private recovery too @@ -1616,23 +1616,26 @@ namespace ccf network.encrypted_ledger_secrets.get_name())); } - auto& encrypted_ledger_secret = w.at(0); - if (!encrypted_ledger_secret.has_value()) + auto encrypted_ledger_secret_info = w.at(0); + if (!encrypted_ledger_secret_info.has_value()) { throw std::logic_error(fmt::format( "Unexpected: removal from {} table", network.encrypted_ledger_secrets.get_name())); } - auto next_ledger_secret_version = - encrypted_ledger_secret->next_version.value_or(version + 1); + // If the version of the next ledger secret is not set, deduce it + // from the hook version (i.e. ledger rekey) + if (!encrypted_ledger_secret_info->next_version.has_value()) + { + encrypted_ledger_secret_info->next_version = version + 1; + } - LOG_FAIL_FMT( - "Next ledger secret at: {}", next_ledger_secret_version); + LOG_DEBUG_FMT( + "Recovering encrypted ledger secret valid at seqno {}", version); - recovery_ledger_secrets.emplace( - next_ledger_secret_version, - std::move(encrypted_ledger_secret->previous_ledger_secret)); + recovery_ledger_secrets.emplace_back( + std::move(encrypted_ledger_secret_info.value())); return kv::ConsensusHookPtr(nullptr); })); diff --git a/src/node/rpc/test/node_stub.h b/src/node/rpc/test/node_stub.h index b847664122f..06f6a9d7430 100644 --- a/src/node/rpc/test/node_stub.h +++ b/src/node/rpc/test/node_stub.h @@ -91,7 +91,13 @@ namespace ccf void initiate_private_recovery(kv::Tx& tx) override { - share_manager.restore_recovery_shares_info(tx, {}); + kv::Version current_ledger_secret_version = 1; + RecoveredEncryptedLedgerSecrets recovered_secrets; + recovered_secrets.push_back( + EncryptedLedgerSecretInfo{std::nullopt, current_ledger_secret_version}); + + share_manager.restore_recovery_shares_info( + tx, std::move(recovered_secrets)); } }; } \ No newline at end of file diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 31f69b2d52b..0bd49ccb74a 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -89,8 +89,7 @@ namespace ccf // During recovery, a list of RecoveredEncryptedLedgerSecrets is constructed // from the local hook on the encrypted ledger secrets table. The key for each // entry is the version at which the next ledger secret is applicable from. - using RecoveredEncryptedLedgerSecrets = - std::map>; + using RecoveredEncryptedLedgerSecrets = std::list; // The ShareManager class provides the interface between the ledger secrets // object and the shares, ledger secrets and submitted shares KV tables. In @@ -341,14 +340,12 @@ namespace ccf return search->second; } - // TODO: rvalue recovery_ledger_secrets LedgerSecretsMap restore_recovery_shares_info( - kv::Tx& tx, - const RecoveredEncryptedLedgerSecrets& recovery_ledger_secrets) + kv::Tx& tx, RecoveredEncryptedLedgerSecrets&& recovery_ledger_secrets) { // First, re-assemble the ledger secret wrapping key from the submitted // encrypted shares. Then, unwrap the latest ledger secret and use it to - // decrypt the previous ledger secret and so on. + // decrypt the sequence of recovered ledger secrets, from the end. if (recovery_ledger_secrets.empty()) { @@ -369,29 +366,32 @@ namespace ccf recovery_shares_info->wrapped_latest_ledger_secret); auto decryption_key = restored_ls.raw_key; - LOG_FAIL_FMT( + LOG_DEBUG_FMT( "Recovering {} encrypted ledger secrets", recovery_ledger_secrets.size()); - restored_ledger_secrets.emplace( - recovery_ledger_secrets.rbegin()->first, std::move(restored_ls)); + auto& current_ledger_secret_version = + recovery_ledger_secrets.back().next_version; + if (!current_ledger_secret_version.has_value()) + { + throw std::logic_error("Current ledger secret version should be set"); + } - LOG_FAIL_FMT("Emplaced"); + restored_ledger_secrets.emplace( + current_ledger_secret_version.value(), std::move(restored_ls)); for (auto it = recovery_ledger_secrets.rbegin(); it != recovery_ledger_secrets.rend(); it++) { - LOG_FAIL_FMT("First!"); - if (!it->second.has_value()) + if (!it->previous_ledger_secret.has_value()) { // Very first entry does not encrypt any other ledger secret - LOG_FAIL_FMT("Skipping!"); // TODO: Remove break; } crypto::GcmCipher encrypted_ls; - encrypted_ls.deserialise(it->second->encrypted_data); + encrypted_ls.deserialise(it->previous_ledger_secret->encrypted_data); std::vector decrypted_ls(encrypted_ls.cipher.size()); if (!crypto::KeyAesGcm(decryption_key) @@ -403,26 +403,16 @@ namespace ccf decrypted_ls.data())) { throw std::logic_error(fmt::format( - "Decryption of ledger secret at {} failed", it->second->version)); + "Decryption of ledger secret at {} failed", + it->previous_ledger_secret->version)); } - LOG_FAIL_FMT("Recovering ledger secret at {}", it->second->version); decryption_key = decrypted_ls; restored_ledger_secrets.emplace( - it->second->version, std::move(decrypted_ls)); - // if (std::next(it) == recovery_ledger_secrets.rend()) - // { - // restored_ledger_secrets.emplace(it-> , - // std::move(decrypted_ls)); - // } - // else - // { - // restored_ledger_secrets.emplace( - // std::next(it)->first, std::move(decrypted_ls)); - // } + it->previous_ledger_secret->version, std::move(decrypted_ls)); } - // recovery_ledger_secrets.clear(); + recovery_ledger_secrets.clear(); return restored_ledger_secrets; } From d3ccde46631da47cb22d430badcdd9d57afc1227 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sun, 31 Jan 2021 21:07:17 +0000 Subject: [PATCH 28/35] Cleanup --- CMakeLists.txt | 6 +++--- src/node/ledger_secrets.h | 31 +++---------------------------- src/node/node_state.h | 8 -------- src/node/share_manager.h | 9 +-------- tests/recovery.py | 12 ++++++------ tests/suite/test_requirements.py | 5 ----- tests/suite/test_suite.py | 25 +++++++++++++++---------- 7 files changed, 28 insertions(+), 68 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d4ff4817a2e..701f41d222a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -519,7 +519,7 @@ if(BUILD_TESTS) CONSENSUS cft ADDITIONAL_ARGS --recovery - 2 + 3 # Shorten Raft election timeout to speed up test when it kills a node on # purpose to check that a recovery network is robust to a view change. --raft-election-timeout @@ -543,8 +543,8 @@ if(BUILD_TESTS) --enforce-reqs --test-suite rekey_recovery - # --test-suite - # membership_recovery + --test-suite + membership_recovery ) add_e2e_test( diff --git a/src/node/ledger_secrets.h b/src/node/ledger_secrets.h index 06709ef2337..924e46654b1 100644 --- a/src/node/ledger_secrets.h +++ b/src/node/ledger_secrets.h @@ -6,7 +6,6 @@ #include "kv/kv_types.h" #include "kv/tx.h" #include "secrets.h" -#include "tls/base64.h" #include "tls/entropy.h" #include @@ -63,8 +62,6 @@ namespace ccf const LedgerSecret& get_secret_for_version( kv::Version version, bool historical_hint = false) { - LOG_FAIL_FMT("Getting secret for version {}", version); - if (ledger_secrets.empty()) { throw std::logic_error("Ledger secrets map is empty"); @@ -114,11 +111,6 @@ namespace ccf last_used_secret_it = std::prev(search); } - LOG_FAIL_FMT( - "Using key at {}: {}", - std::prev(search)->first, - tls::b64_from_raw(std::prev(search)->second.raw_key)); - return std::prev(search)->second; } @@ -155,13 +147,7 @@ namespace ccf { std::lock_guard guard(lock); - auto init_secret = make_ledger_secret(); - - LOG_INFO_FMT("Init ledger secrets at {}", initial_version); - LOG_FAIL_FMT( - "{}", tls::b64_from_raw(init_secret.raw_key)); // TODO: Delete - - ledger_secrets.emplace(initial_version, std::move(init_secret)); + ledger_secrets.emplace(initial_version, make_ledger_secret()); } void set_node_id(NodeId id) @@ -249,16 +235,6 @@ namespace ccf ledger_secrets.begin()->first)); } - LOG_FAIL_FMT("Ledger secrets before: {}", ledger_secrets.size()); - LOG_FAIL_FMT("LS before: {}", ledger_secrets.begin()->first); - - LOG_FAIL_FMT("Restored secrets: {}", restored_ledger_secrets.size()); - for (auto const& ls : restored_ledger_secrets) - { - LOG_FAIL_FMT( - "LS: {} - {}", ls.first, tls::b64_from_raw(ls.second.raw_key)); - } - ledger_secrets.merge(restored_ledger_secrets); } @@ -278,10 +254,9 @@ namespace ccf "Ledger secret at seqno {} already exists", version); - LOG_INFO_FMT("Added new ledger secret at seqno {}", version); - LOG_FAIL_FMT("{}", tls::b64_from_raw(secret.raw_key)); // TODO: Delete - ledger_secrets.emplace(version, std::move(secret)); + + LOG_INFO_FMT("Added new ledger secret at seqno {}", version); } void rollback(kv::Version version) diff --git a/src/node/node_state.h b/src/node/node_state.h index 1ab8c7cb938..67ea52defb6 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -462,14 +462,6 @@ namespace ccf network.identity = std::make_unique(resp.network_info.identity); - LOG_FAIL_FMT( - "Joining with {} ledger secrets", - resp.network_info.ledger_secrets.size()); - for (auto const& s : resp.network_info.ledger_secrets) - { - LOG_FAIL_FMT("LS: {}", s.first); - } - network.ledger_secrets = std::make_shared( self, std::move(resp.network_info.ledger_secrets)); diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 0bd49ccb74a..328d0f995d5 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -194,11 +194,6 @@ namespace ccf encrypted_previous_ls.hdr.tag); encrypted_previous_secret = encrypted_previous_ls.serialise(); - - LOG_FAIL_FMT( - "Setting recovery share info with previous secret at " - "{}", - version_previous_secret); encrypted_ls->put( 0, {PreviousLedgerSecretInfo( @@ -209,7 +204,6 @@ namespace ccf } else { - LOG_FAIL_FMT("Setting recovery share info with no previous secret"); encrypted_ls->put(0, {std::nullopt, latest_ls_version}); } } @@ -305,7 +299,7 @@ namespace ccf // wrapped new ledger secret and encrypted previous ledger secret in the // store. // Note: The version at which the new ledger secret is applicable from is - // derived from the hook at which the ledgersecret is applied to the + // derived from the hook at which the ledger secret is applied to the // store. set_recovery_shares_info( tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); @@ -316,7 +310,6 @@ namespace ccf // Issue new recovery shares of the _same_ current ledger secret to all // active recovery members. The encrypted ledger secrets recorded in the // store are _not_ updated. - LOG_FAIL_FMT("Shuffling recovery shares"); shuffle_recovery_shares( tx, network.ledger_secrets->get_latest(tx).second); } diff --git a/tests/recovery.py b/tests/recovery.py index 06b76111a8c..a1f77b6cb74 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -117,12 +117,12 @@ def run(args): for i in range(args.recovery): # Alternate between recovery with primary change and stable primary-ship, # with and without snapshots - # if i % 2 == 0: - # recovered_network = test_share_resilience( - # network, args, from_snapshot=False - # ) - # else: - recovered_network = test(network, args, from_snapshot=True) + if i % 2 == 0: + recovered_network = test_share_resilience( + network, args, from_snapshot=True + ) + else: + recovered_network = test(network, args, from_snapshot=False) network = recovered_network LOG.success("Recovery complete on all nodes") diff --git a/tests/suite/test_requirements.py b/tests/suite/test_requirements.py index 511849f4e48..66dbe2d5f52 100644 --- a/tests/suite/test_requirements.py +++ b/tests/suite/test_requirements.py @@ -153,11 +153,6 @@ def wrapper(*args, **kwargs): network=network, number_txs=infra.e2e_args.get("msgs_per_recovery") or number_txs, ) - # network.txs.issue( - # network=network, - # number_txs=1, - # repeat=True, - # ) new_network = func(*args, **kwargs) new_network.txs.verify( network=new_network, diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 594f71ceba3..2c73868fbb2 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -15,21 +15,26 @@ suites = dict() +# This test suite currently fails and is not yet run by the CI +# https://github.com/microsoft/CCF/issues/1648 +historical_recovery_snapshot_failure = [ + e2e_logging.test_historical_query, + rekey.test, + rekey.test, + recovery.test, +] + + # This suite tests that rekeying, network configuration changes # and recoveries can be interleaved suite_rekey_recovery = [ - e2e_logging.test_historical_query, - rekey.test, - membership.test_set_recovery_threshold, + recovery.test, + reconfiguration.test_add_node, rekey.test, - # reconfiguration.test_add_node_from_snapshot, + reconfiguration.test_add_node, recovery.test, - # reconfiguration.test_add_node_from_snapshot, - # rekey.test, - # reconfiguration.test_add_node, - # recovery.test, - # rekey.test, - # reconfiguration.test_add_node, + rekey.test, + reconfiguration.test_add_node, ] suites["rekey_recovery"] = suite_rekey_recovery From f9f1a4209fdf0b3586101d7ce09da5366de4735a Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sun, 31 Jan 2021 21:13:15 +0000 Subject: [PATCH 29/35] Rename --- src/node/rpc/member_frontend.h | 4 ++-- src/node/share_manager.h | 15 +++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index 98df3a63b93..ad9dee24873 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -975,7 +975,7 @@ namespace ccf const ProposalId& proposal_id, kv::Tx& tx, const nlohmann::json&) { try { - share_manager.reshuffle_recovery_shares(tx); + share_manager.shuffle_recovery_shares(tx); } catch (const std::logic_error& e) { @@ -1010,7 +1010,7 @@ namespace ccf try { - share_manager.reshuffle_recovery_shares(tx); + share_manager.shuffle_recovery_shares(tx); } catch (const std::logic_error& e) { diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 328d0f995d5..4da53d125d3 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -86,9 +86,8 @@ namespace ccf } }; - // During recovery, a list of RecoveredEncryptedLedgerSecrets is constructed - // from the local hook on the encrypted ledger secrets table. The key for each - // entry is the version at which the next ledger secret is applicable from. + // During recovery, a list of EncryptedLedgerSecretInfo is constructed + // from the local hook on the encrypted ledger secrets table. using RecoveredEncryptedLedgerSecrets = std::list; // The ShareManager class provides the interface between the ledger secrets @@ -147,7 +146,7 @@ namespace ccf return encrypted_shares; } - void shuffle_recovery_shares( + void _shuffle_recovery_shares( kv::Tx& tx, const LedgerSecret& latest_ledger_secret) { auto ls_wrapping_key = LedgerSecretWrappingKey(); @@ -171,7 +170,7 @@ namespace ccf // Finally, encrypt each share with the public key of each member and // record it in the shares table. - shuffle_recovery_shares(tx, latest_ledger_secret); + _shuffle_recovery_shares(tx, latest_ledger_secret); auto encrypted_ls = tx.rw(network.encrypted_ledger_secrets); @@ -305,12 +304,12 @@ namespace ccf tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); } - void reshuffle_recovery_shares(kv::Tx& tx) + void shuffle_recovery_shares(kv::Tx& tx) { // Issue new recovery shares of the _same_ current ledger secret to all // active recovery members. The encrypted ledger secrets recorded in the // store are _not_ updated. - shuffle_recovery_shares( + _shuffle_recovery_shares( tx, network.ledger_secrets->get_latest(tx).second); } @@ -338,7 +337,7 @@ namespace ccf { // First, re-assemble the ledger secret wrapping key from the submitted // encrypted shares. Then, unwrap the latest ledger secret and use it to - // decrypt the sequence of recovered ledger secrets, from the end. + // decrypt the sequence of recovered ledger secrets, from the last one. if (recovery_ledger_secrets.empty()) { From 24760473d1c5a19dbb047a624281608dea0de902 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sun, 31 Jan 2021 21:13:37 +0000 Subject: [PATCH 30/35] Format --- src/node/test/encryptor.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/test/encryptor.cpp b/src/node/test/encryptor.cpp index e2304d39329..b7a48ef096a 100644 --- a/src/node/test/encryptor.cpp +++ b/src/node/test/encryptor.cpp @@ -3,6 +3,7 @@ #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN #include "kv/encryptor.h" + #include "kv/kv_types.h" #include "kv/store.h" #include "kv/test/stub_consensus.h" From 24fe8dd8da12f657b2e5cb058b478329d30a3cd4 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Sun, 31 Jan 2021 21:17:50 +0000 Subject: [PATCH 31/35] Shuffle instead of new shares --- src/node/rpc/member_frontend.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index ad9dee24873..9a80d78a898 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -1666,7 +1666,7 @@ namespace ccf // member, all recovery members are allocated new recovery shares try { - share_manager.issue_recovery_shares(ctx.tx); + share_manager.shuffle_recovery_shares(ctx.tx); } catch (const std::logic_error& e) { From 21c8b0809e48e8955b8d00d80be6dc5ca0147a0c Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Mon, 1 Feb 2021 09:25:44 +0000 Subject: [PATCH 32/35] Set recovery hook before deserialising snapshot --- src/node/node_state.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 67ea52defb6..ecf110d5bb7 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -480,6 +480,14 @@ namespace ccf setup_progress_tracker(); setup_history(); + if (resp.network_info.public_only) + { + last_recovered_signed_idx = + resp.network_info.last_recovered_signed_idx; + setup_recovery_hook(); + snapshotter->set_snapshot_generation(false); + } + if (startup_snapshot_info) { // It is only possible to deserialise the entire snapshot then, @@ -538,11 +546,6 @@ namespace ccf if (resp.network_info.public_only) { - last_recovered_signed_idx = - resp.network_info.last_recovered_signed_idx; - setup_recovery_hook(); - snapshotter->set_snapshot_generation(false); - sm.advance(State::partOfPublicNetwork); } else From ae306d980d2a478fc2f4e114ac9c9f14e3dfb8db Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 3 Feb 2021 16:05:08 +0000 Subject: [PATCH 33/35] PR comments --- src/node/entities.h | 2 +- src/node/node_state.h | 5 +++-- src/node/share_manager.h | 43 +++++++++++++++++++++++++--------------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/node/entities.h b/src/node/entities.h index 3ba216b0a3d..310e1a18e35 100644 --- a/src/node/entities.h +++ b/src/node/entities.h @@ -82,7 +82,7 @@ namespace ccf static constexpr auto SERVICE = "public:ccf.gov.service"; static constexpr auto SHARES = "public:ccf.gov.shares"; static constexpr auto ENCRYPTED_LEDGER_SECRETS = - "public:ccf.gov.encrypted_ledger_secrets"; + "public:ccf.internal.encrypted_ledger_secrets"; static constexpr auto CONFIGURATION = "public:ccf.gov.config"; static constexpr auto SUBMITTED_SHARES = "public:ccf.gov.submitted_shares"; static constexpr auto SNAPSHOT_EVIDENCE = diff --git a/src/node/node_state.h b/src/node/node_state.h index ecf110d5bb7..80177b0018a 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1607,7 +1607,8 @@ namespace ccf if (w.size() > 1) { throw std::logic_error(fmt::format( - "Unexpected: multiple writes to {} table", + "Transaction contains {} writes to map {}, expected one", + w.size(), network.encrypted_ledger_secrets.get_name())); } @@ -1615,7 +1616,7 @@ namespace ccf if (!encrypted_ledger_secret_info.has_value()) { throw std::logic_error(fmt::format( - "Unexpected: removal from {} table", + "Removal from {} table", network.encrypted_ledger_secrets.get_name())); } diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 4da53d125d3..746505b44d7 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -146,7 +146,7 @@ namespace ccf return encrypted_shares; } - void _shuffle_recovery_shares( + void shuffle_recovery_shares( kv::Tx& tx, const LedgerSecret& latest_ledger_secret) { auto ls_wrapping_key = LedgerSecretWrappingKey(); @@ -170,7 +170,7 @@ namespace ccf // Finally, encrypt each share with the public key of each member and // record it in the shares table. - _shuffle_recovery_shares(tx, latest_ledger_secret); + shuffle_recovery_shares(tx, latest_ledger_secret); auto encrypted_ls = tx.rw(network.encrypted_ledger_secrets); @@ -280,36 +280,47 @@ namespace ccf public: ShareManager(NetworkState& network_) : network(network_) {} + /** Issue new recovery shares for the current ledger secret, recording the + * wrapped new ledger secret and encrypted previous ledger secret in the + * store. + * + * @param tx Store transaction object + */ void issue_recovery_shares(kv::Tx& tx) { - // Issue new recovery shares for the current ledger secret, recording the - // wrapped new ledger secret and encrypted previous ledger secret in the - // store. auto [latest, penultimate] = network.ledger_secrets->get_latest_and_penultimate(tx); set_recovery_shares_info(tx, latest.second, penultimate, latest.first); } + /** Issue new recovery shares of the new ledger secret, recording the + /* wrapped new ledger secret and encrypted current (now previous) ledger + /* secret in the store. + /* + /* @param tx Store transaction object + /* @param new_ledger_secret New ledger secret + /* + /* Note: The version at which the new ledger secret is applicable from is + /* derived from the hook at which the ledger secret is applied to the + /* store. + */ void issue_recovery_shares( kv::Tx& tx, const LedgerSecret& new_ledger_secret) { - // Issue new recovery shares of the new ledger secret, recording the - // wrapped new ledger secret and encrypted previous ledger secret in the - // store. - // Note: The version at which the new ledger secret is applicable from is - // derived from the hook at which the ledger secret is applied to the - // store. set_recovery_shares_info( tx, new_ledger_secret, network.ledger_secrets->get_latest(tx)); } + /** Issue new recovery shares of the same current ledger secret to all + /* active recovery members. The encrypted ledger secrets recorded in the + /* store are not updated. + /* + /* @param tx Store transaction object + */ void shuffle_recovery_shares(kv::Tx& tx) { - // Issue new recovery shares of the _same_ current ledger secret to all - // active recovery members. The encrypted ledger secrets recorded in the - // store are _not_ updated. - _shuffle_recovery_shares( + shuffle_recovery_shares( tx, network.ledger_secrets->get_latest(tx).second); } @@ -342,7 +353,7 @@ namespace ccf if (recovery_ledger_secrets.empty()) { throw std::logic_error( - "There should be at least one encrypted ledger secret to recover"); + "Could not find any ledger secrets in the ledger"); } auto recovery_shares_info = tx.ro(network.shares)->get(0); From 6d1743c5771640673013aca9225560a825055902 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 3 Feb 2021 16:07:55 +0000 Subject: [PATCH 34/35] d'oh --- src/node/share_manager.h | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 746505b44d7..d2d09cf1d32 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -295,16 +295,16 @@ namespace ccf } /** Issue new recovery shares of the new ledger secret, recording the - /* wrapped new ledger secret and encrypted current (now previous) ledger - /* secret in the store. - /* - /* @param tx Store transaction object - /* @param new_ledger_secret New ledger secret - /* - /* Note: The version at which the new ledger secret is applicable from is - /* derived from the hook at which the ledger secret is applied to the - /* store. - */ + * wrapped new ledger secret and encrypted current (now previous) ledger + * secret in the store. + * + * @param tx Store transaction object + * @param new_ledger_secret New ledger secret + * + * Note: The version at which the new ledger secret is applicable from is + * derived from the hook at which the ledger secret is applied to the + * store. + */ void issue_recovery_shares( kv::Tx& tx, const LedgerSecret& new_ledger_secret) { @@ -313,11 +313,11 @@ namespace ccf } /** Issue new recovery shares of the same current ledger secret to all - /* active recovery members. The encrypted ledger secrets recorded in the - /* store are not updated. - /* - /* @param tx Store transaction object - */ + * active recovery members. The encrypted ledger secrets recorded in the + * store are not updated. + * + * @param tx Store transaction object + */ void shuffle_recovery_shares(kv::Tx& tx) { shuffle_recovery_shares( From 2f22cd689af98ac2b12539b351ad6738066a2eb5 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 4 Feb 2021 11:41:46 +0000 Subject: [PATCH 35/35] Added comment to clarify optional value being always set --- src/node/share_manager.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node/share_manager.h b/src/node/share_manager.h index d2d09cf1d32..3b023a3f65f 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -377,6 +377,8 @@ namespace ccf recovery_ledger_secrets.back().next_version; if (!current_ledger_secret_version.has_value()) { + // This should always be set by the recovery hook, which sets this to + // the version at which it is called if unset in the store throw std::logic_error("Current ledger secret version should be set"); }