diff --git a/doc/host_config_schema/cchost_config.json b/doc/host_config_schema/cchost_config.json index 4b06681c18b..80a34845c4a 100644 --- a/doc/host_config_schema/cchost_config.json +++ b/doc/host_config_schema/cchost_config.json @@ -325,6 +325,11 @@ "data_json_file": { "type": ["string", "null"], "description": "Path to member data file (JSON)" + }, + "recovery_role": { + "type": "string", + "enum": ["NonParticipant", "Participant", "Owner"], + "description": "Whether the member acts as a recovery participant and gets assigned a share that can contribute towards a recovery threshold or as an owner and gets assigned a full recovery key" } }, "required": ["certificate_file"], diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index 857b3f5ec25..d8d5ae3ad7c 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -344,6 +344,9 @@ "member_data": { "$ref": "#/components/schemas/json" }, + "recovery_role": { + "$ref": "#/components/schemas/MemberRecoveryRole" + }, "status": { "$ref": "#/components/schemas/MemberStatus" } @@ -412,6 +415,14 @@ }, "type": "object" }, + "MemberRecoveryRole": { + "enum": [ + "NonParticipant", + "Participant", + "Owner" + ], + "type": "string" + }, "MemberStatus": { "enum": [ "Accepted", @@ -1337,7 +1348,7 @@ "info": { "description": "This API is used to submit and query proposals which affect CCF's public governance tables.", "title": "CCF Governance API", - "version": "4.5.1" + "version": "4.6.0" }, "openapi": "3.0.0", "paths": { diff --git a/include/ccf/service/tables/members.h b/include/ccf/service/tables/members.h index 6e9d5144f18..ed0e135657b 100644 --- a/include/ccf/service/tables/members.h +++ b/include/ccf/service/tables/members.h @@ -21,6 +21,22 @@ namespace ccf DECLARE_JSON_ENUM( MemberStatus, {{MemberStatus::ACCEPTED, "Accepted"}, {MemberStatus::ACTIVE, "Active"}}); + + enum class MemberRecoveryRole + { + NonParticipant = 0, + Participant, + + /** If set then the member is to receive a key allowing it + to single-handedly recover the network without requiring + any other recovery member to submit their shares. */ + Owner + }; + DECLARE_JSON_ENUM( + MemberRecoveryRole, + {{MemberRecoveryRole::NonParticipant, "NonParticipant"}, + {MemberRecoveryRole::Participant, "Participant"}, + {MemberRecoveryRole::Owner, "Owner"}}); } namespace ccf @@ -33,26 +49,31 @@ namespace ccf std::optional encryption_pub_key = std::nullopt; nlohmann::json member_data = nullptr; + std::optional recovery_role = std::nullopt; + NewMember() {} NewMember( const ccf::crypto::Pem& cert_, const std::optional& encryption_pub_key_ = std::nullopt, - const nlohmann::json& member_data_ = nullptr) : + const nlohmann::json& member_data_ = nullptr, + const std::optional& recovery_role_ = std::nullopt) : cert(cert_), encryption_pub_key(encryption_pub_key_), - member_data(member_data_) + member_data(member_data_), + recovery_role(recovery_role_) {} bool operator==(const NewMember& rhs) const { return cert == rhs.cert && encryption_pub_key == rhs.encryption_pub_key && - member_data == rhs.member_data; + member_data == rhs.member_data && recovery_role == rhs.recovery_role; } }; DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(NewMember) DECLARE_JSON_REQUIRED_FIELDS(NewMember, cert) - DECLARE_JSON_OPTIONAL_FIELDS(NewMember, encryption_pub_key, member_data) + DECLARE_JSON_OPTIONAL_FIELDS( + NewMember, encryption_pub_key, member_data, recovery_role) struct MemberDetails { @@ -62,14 +83,17 @@ namespace ccf members for example. */ nlohmann::json member_data = nullptr; + std::optional recovery_role = std::nullopt; + bool operator==(const MemberDetails& rhs) const { - return status == rhs.status && member_data == rhs.member_data; + return status == rhs.status && member_data == rhs.member_data && + recovery_role == rhs.recovery_role; } }; DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(MemberDetails) DECLARE_JSON_REQUIRED_FIELDS(MemberDetails, status) - DECLARE_JSON_OPTIONAL_FIELDS(MemberDetails, member_data) + DECLARE_JSON_OPTIONAL_FIELDS(MemberDetails, member_data, recovery_role) using MemberInfo = ServiceMap; diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 7a41bc0a8e0..0dd2e8ada4c 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -372,11 +372,25 @@ const actions = new Map([ function (args) { checkX509CertBundle(args.cert, "cert"); checkType(args.member_data, "object?", "member_data"); - // Also check that public encryption key is well formed, if it exists + const recovery_role = args.recovery_role; + if (recovery_role !== undefined) { + checkEnum( + recovery_role, + ["NonParticipant", "Participant", "Owner"], + "recovery_role", + ); + } - // Check if member exists - // if not, check there is no enc pub key - // if it does, check it doesn't have an enc pub key in ledger + if ( + args.encryption_pub_key == null && + args.recovery_role !== null && + args.recovery_role !== undefined + ) { + throw new Error( + "Cannot specify a recovery_role value when encryption_pub_key is not specified", + ); + } + // Also check that public encryption key is well formed, if it exists }, function (args) { @@ -401,6 +415,7 @@ const actions = new Map([ let member_info = {}; member_info.member_data = args.member_data; + member_info.recovery_role = args.recovery_role; member_info.status = "Accepted"; ccf.kv["public:ccf.gov.members.info"].set( rawMemberId, diff --git a/src/crypto/sharing.h b/src/crypto/sharing.h index 75af6754bda..cedb4cb8d7f 100644 --- a/src/crypto/sharing.h +++ b/src/crypto/sharing.h @@ -56,17 +56,20 @@ namespace ccf::crypto return k; } - std::vector serialise() const + void serialise(std::vector& serialised) const { auto size = serialised_size; - std::vector serialised(size); + if (serialised.size() != size) + { + throw std::invalid_argument("Invalid serialised share size"); + } + auto data = serialised.data(); serialized::write(data, size, x); for (size_t i = 0; i < LIMBS; ++i) { serialized::write(data, size, y[i]); } - return serialised; } Share(const std::span& serialised) diff --git a/src/crypto/test/secret_sharing.cpp b/src/crypto/test/secret_sharing.cpp index a0f72d754d1..51441859f5e 100644 --- a/src/crypto/test/secret_sharing.cpp +++ b/src/crypto/test/secret_sharing.cpp @@ -172,7 +172,9 @@ TEST_CASE("Serialisation") share.y[7] = 6; share.y[8] = 7; share.y[9] = 56; - Share new_share(share.serialise()); + std::vector serialised(share.serialised_size); + share.serialise(serialised); + Share new_share(serialised); INFO(share.to_str()); INFO(new_share.to_str()); diff --git a/src/host/configuration.h b/src/host/configuration.h index d6b9765b4fc..54edf8e4be7 100644 --- a/src/host/configuration.h +++ b/src/host/configuration.h @@ -48,6 +48,7 @@ namespace host std::string certificate_file; std::optional encryption_public_key_file = std::nullopt; std::optional data_json_file = std::nullopt; + std::optional recovery_role = std::nullopt; bool operator==(const ParsedMemberInfo& other) const = default; }; @@ -55,7 +56,10 @@ namespace host DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(ParsedMemberInfo); DECLARE_JSON_REQUIRED_FIELDS(ParsedMemberInfo, certificate_file); DECLARE_JSON_OPTIONAL_FIELDS( - ParsedMemberInfo, encryption_public_key_file, data_json_file); + ParsedMemberInfo, + encryption_public_key_file, + data_json_file, + recovery_role); struct CCHostConfig : public ccf::CCFConfig { diff --git a/src/host/main.cpp b/src/host/main.cpp index f20873b41c3..86f2db1962e 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -221,17 +221,29 @@ int main(int argc, char** argv) "On start, ledger directory should not exist ({})", config.ledger.directory)); } + // Count members with public encryption key as only these members will be // handed a recovery share. - // Note that it is acceptable to start a network without any member having - // a recovery share. The service will check that at least one recovery - // member is added before the service can be opened. - size_t members_with_pubk_count = 0; + // Note that it is acceptable to start a network without any member + // having a recovery share. The service will check that at least one + // recovery member (participant or owner) is added before the + // service can be opened. + size_t recovery_participants_count = 0; + size_t recovery_owners_count = 0; for (auto const& m : config.command.start.members) { if (m.encryption_public_key_file.has_value()) { - members_with_pubk_count++; + auto role = + m.recovery_role.value_or(ccf::MemberRecoveryRole::Participant); + if (role == ccf::MemberRecoveryRole::Participant) + { + recovery_participants_count++; + } + else if (role == ccf::MemberRecoveryRole::Owner) + { + recovery_owners_count++; + } } } @@ -239,20 +251,46 @@ int main(int argc, char** argv) config.command.start.service_configuration.recovery_threshold; if (recovery_threshold == 0) { - LOG_INFO_FMT( - "Recovery threshold unset. Defaulting to number of initial " - "consortium members with a public encryption key ({}).", - members_with_pubk_count); - recovery_threshold = members_with_pubk_count; + if (recovery_participants_count == 0 && recovery_owners_count != 0) + { + LOG_INFO_FMT( + "Recovery threshold unset. Defaulting to 1 as only consortium " + "members that are recovery owners ({}) are specified.", + recovery_owners_count); + recovery_threshold = 1; + } + else + { + LOG_INFO_FMT( + "Recovery threshold unset. Defaulting to number of initial " + "consortium members with a public encryption key ({}).", + recovery_participants_count); + recovery_threshold = recovery_participants_count; + } } - else if (recovery_threshold > members_with_pubk_count) + else { - throw std::logic_error(fmt::format( - "Recovery threshold ({}) cannot be greater than total number ({})" - "of initial consortium members with a public encryption " - "key (specified via --member-info options)", - recovery_threshold, - members_with_pubk_count)); + if (recovery_participants_count == 0 && recovery_owners_count != 0) + { + if (recovery_threshold > 1) + { + throw std::logic_error(fmt::format( + "Recovery threshold ({}) cannot be greater than 1 when all " + "initial consortium members ({}) are of type recovery owner " + "(specified via --member-info options)", + recovery_threshold, + recovery_participants_count)); + } + } + else if (recovery_threshold > recovery_participants_count) + { + throw std::logic_error(fmt::format( + "Recovery threshold ({}) cannot be greater than total number ({})" + "of initial consortium members with a public encryption " + "key (specified via --member-info options)", + recovery_threshold, + recovery_participants_count)); + } } } } @@ -612,12 +650,17 @@ int main(int argc, char** argv) for (auto const& m : config.command.start.members) { std::optional public_encryption_key = std::nullopt; + std::optional recovery_role = std::nullopt; if ( m.encryption_public_key_file.has_value() && !m.encryption_public_key_file.value().empty()) { public_encryption_key = ccf::crypto::Pem( files::slurp(m.encryption_public_key_file.value())); + if (m.recovery_role.has_value()) + { + recovery_role = m.recovery_role.value(); + } } nlohmann::json md = nullptr; @@ -629,7 +672,8 @@ int main(int argc, char** argv) startup_config.start.members.emplace_back( ccf::crypto::Pem(files::slurp(m.certificate_file)), public_encryption_key, - md); + md, + recovery_role); } startup_config.start.constitution = ""; for (const auto& constitution_path : diff --git a/src/node/gov/handlers/acks.h b/src/node/gov/handlers/acks.h index 7e50f1af290..1d91a5948d7 100644 --- a/src/node/gov/handlers/acks.h +++ b/src/node/gov/handlers/acks.h @@ -266,11 +266,12 @@ namespace ccf::gov::endpoints return; } - // If this is a newly-active recovery member in an open service, - // allocate them a recovery share immediately + // If this is a newly-active recovery participant/owner in an open + // service, allocate them a recovery share immediately if ( newly_active && - InternalTablesAccess::is_recovery_member(ctx.tx, member_id)) + InternalTablesAccess::is_recovery_participant_or_owner( + ctx.tx, member_id)) { auto service_status = InternalTablesAccess::get_service_status(ctx.tx); diff --git a/src/node/gov/handlers/recovery.h b/src/node/gov/handlers/recovery.h index 52d64284bbd..c1d82aed3ff 100644 --- a/src/node/gov/handlers/recovery.h +++ b/src/node/gov/handlers/recovery.h @@ -130,11 +130,14 @@ namespace ccf::gov::endpoints params["share"].template get()); size_t submitted_shares_count = 0; + bool full_key_submitted = false; try { submitted_shares_count = share_manager.submit_recovery_share( ctx.tx, member_id, raw_recovery_share); + full_key_submitted = ShareManager::is_full_key(raw_recovery_share); + OPENSSL_cleanse( raw_recovery_share.data(), raw_recovery_share.size()); } @@ -157,14 +160,22 @@ namespace ccf::gov::endpoints const auto threshold = InternalTablesAccess::get_recovery_threshold(ctx.tx); - // Same format of message, whether this is sufficient to trigger - // recovery or not - std::string message = fmt::format( - "{}/{} recovery shares successfully submitted", - submitted_shares_count, - threshold); + std::string message; + if (full_key_submitted) + { + message = "Full recovery key successfully submitted"; + } + else + { + // Same format of message, whether this is sufficient to trigger + // recovery or not + message = fmt::format( + "{}/{} recovery shares successfully submitted", + submitted_shares_count, + threshold); + } - if (submitted_shares_count >= threshold) + if (submitted_shares_count >= threshold || full_key_submitted) { message += "\nEnd of recovery procedure initiated"; GOV_INFO_FMT("{} - initiating recovery", message); @@ -196,6 +207,7 @@ namespace ccf::gov::endpoints response_body["message"] = message; response_body["submittedCount"] = submitted_shares_count; response_body["recoveryThreshold"] = threshold; + response_body["fullKeySubmitted"] = full_key_submitted; ctx.rpc_ctx->set_response_json(response_body, HTTP_STATUS_OK); return; diff --git a/src/node/gov/handlers/service_state.h b/src/node/gov/handlers/service_state.h index 6b010334e66..31b77c69e04 100644 --- a/src/node/gov/handlers/service_state.h +++ b/src/node/gov/handlers/service_state.h @@ -35,6 +35,19 @@ namespace ccf::gov::endpoints member["publicEncryptionKey"] = enc_key.value().str(); } + ccf::MemberRecoveryRole recovery_role = + ccf::MemberRecoveryRole::NonParticipant; + if (member_details.recovery_role.has_value()) + { + recovery_role = member_details.recovery_role.value(); + } + else if (enc_key.has_value()) + { + recovery_role = ccf::MemberRecoveryRole::Participant; + } + + member["recoveryRole"] = recovery_role; + return member; } diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index c2498f1f8b9..1fc8cefecb2 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -610,7 +610,7 @@ namespace ccf openapi_info.description = "This API is used to submit and query proposals which affect CCF's " "public governance tables."; - openapi_info.document_version = "4.5.1"; + openapi_info.document_version = "4.6.0"; } static std::optional get_caller_member_id( @@ -770,10 +770,12 @@ namespace ccf auto member_info = members->get(member_id.value()); if ( service_status.value() == ServiceStatus::OPEN && - InternalTablesAccess::is_recovery_member(ctx.tx, member_id.value())) + InternalTablesAccess::is_recovery_participant_or_owner( + ctx.tx, member_id.value())) { // When the service is OPEN and the new active member is a recovery - // member, all recovery members are allocated new recovery shares + // participant/owner, all recovery members are allocated new recovery + // shares try { share_manager.shuffle_recovery_shares(ctx.tx); diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 4e26c1386f1..4632295e4e4 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -18,43 +18,25 @@ namespace ccf { - class LedgerSecretWrappingKey + class SharedLedgerSecretWrappingKey { private: static constexpr auto KZ_KEY_SIZE = ccf::crypto::GCM_DEFAULT_KEY_SIZE; bool has_wrapped = false; size_t num_shares; size_t recovery_threshold; - std::vector data; // Referred to as "kz" in TR + ccf::crypto::sharing::Share secret; std::vector shares; public: - LedgerSecretWrappingKey(size_t num_shares_, size_t recovery_threshold_) : + SharedLedgerSecretWrappingKey( + size_t num_shares_, size_t recovery_threshold_) : num_shares(num_shares_), recovery_threshold(recovery_threshold_) { shares.resize(num_shares); - ccf::crypto::sharing::Share secret; ccf::crypto::sharing::sample_secret_and_shares( secret, shares, recovery_threshold); - data = secret.key(KZ_KEY_SIZE); - } - - LedgerSecretWrappingKey( - std::vector&& shares_, - size_t recovery_threshold_) : - recovery_threshold(recovery_threshold_) - { - shares = shares_; - ccf::crypto::sharing::Share secret; - ccf::crypto::sharing::recover_unauthenticated_secret( - secret, shares, recovery_threshold); - data = secret.key(KZ_KEY_SIZE); - } - - ~LedgerSecretWrappingKey() - { - OPENSSL_cleanse(data.data(), data.size()); } size_t get_num_shares() const @@ -72,17 +54,16 @@ namespace ccf std::vector> shares_; for (const ccf::crypto::sharing::Share& share : shares) { - shares_.emplace_back(share.serialise()); + std::vector share_serialised(share.serialised_size); + share.serialise(share_serialised); + shares_.emplace_back(share_serialised); } return shares_; } - template - T get_raw_data() const + void get_full_share_serialised(std::vector& serialised) const { - T ret; - std::copy_n(data.begin(), data.size(), ret.begin()); - return ret; + secret.serialise(serialised); } std::vector wrap(const LedgerSecretPtr& ledger_secret) @@ -95,18 +76,50 @@ namespace ccf ccf::crypto::GcmCipher encrypted_ls(ledger_secret->raw_key.size()); - ccf::crypto::make_key_aes_gcm(data)->encrypt( - encrypted_ls.hdr.get_iv(), // iv is always 0 here as the share wrapping - // key is never re-used for encryption - ledger_secret->raw_key, - {}, - encrypted_ls.cipher, - encrypted_ls.hdr.tag); + std::vector data = secret.key(KZ_KEY_SIZE); + try + { + ccf::crypto::make_key_aes_gcm(data)->encrypt( + encrypted_ls.hdr + .get_iv(), // iv is always 0 here as the share wrapping + // key is never re-used for encryption + ledger_secret->raw_key, + {}, + encrypted_ls.cipher, + encrypted_ls.hdr.tag); + } + catch (...) + { + OPENSSL_cleanse(data.data(), data.size()); + throw; + } has_wrapped = true; return encrypted_ls.serialise(); } + }; + + class ReconstructedLedgerSecretWrappingKey + { + private: + static constexpr auto KZ_KEY_SIZE = ccf::crypto::GCM_DEFAULT_KEY_SIZE; + ccf::crypto::sharing::Share secret; + + public: + ReconstructedLedgerSecretWrappingKey( + std::vector&& shares_, + size_t recovery_threshold_) + { + ccf::crypto::sharing::recover_unauthenticated_secret( + secret, shares_, recovery_threshold_); + } + + ReconstructedLedgerSecretWrappingKey( + const ccf::crypto::sharing::Share& secret_) + { + secret = secret_; + } LedgerSecretPtr unwrap( const std::vector& wrapped_latest_ledger_secret) @@ -115,14 +128,23 @@ namespace ccf encrypted_ls.deserialise(wrapped_latest_ledger_secret); std::vector decrypted_ls; - if (!ccf::crypto::make_key_aes_gcm(data)->decrypt( - encrypted_ls.hdr.get_iv(), - encrypted_ls.hdr.tag, - encrypted_ls.cipher, - {}, - decrypted_ls)) + std::vector data = secret.key(KZ_KEY_SIZE); + try + { + if (!ccf::crypto::make_key_aes_gcm(data)->decrypt( + encrypted_ls.hdr.get_iv(), + encrypted_ls.hdr.tag, + encrypted_ls.cipher, + {}, + decrypted_ls)) + { + throw std::logic_error("Unwrapping latest ledger secret failed"); + } + } + catch (...) { - throw std::logic_error("Unwrapping latest ledger secret failed"); + OPENSSL_cleanse(data.data(), data.size()); + throw; } return std::make_shared(std::move(decrypted_ls)); @@ -146,16 +168,17 @@ namespace ccf std::shared_ptr ledger_secrets; EncryptedSharesMap compute_encrypted_shares( - ccf::kv::Tx& tx, const LedgerSecretWrappingKey& ls_wrapping_key) + ccf::kv::Tx& tx, const SharedLedgerSecretWrappingKey& ls_wrapping_key) { EncryptedSharesMap encrypted_shares; auto shares = ls_wrapping_key.get_shares(); - auto active_recovery_members_info = - InternalTablesAccess::get_active_recovery_members(tx); + auto active_recovery_participants_info = + InternalTablesAccess::get_active_recovery_participants(tx); size_t share_index = 0; - for (auto const& [member_id, enc_pub_key] : active_recovery_members_info) + for (auto const& [member_id, enc_pub_key] : + active_recovery_participants_info) { auto member_enc_pubk = ccf::crypto::make_rsa_public_key(enc_pub_key); auto raw_share = std::vector( @@ -166,18 +189,41 @@ namespace ccf share_index++; } + auto active_recovery_owners_info = + InternalTablesAccess::get_active_recovery_owners(tx); + if (active_recovery_owners_info.size() > 0) + { + std::vector full_share_serialised( + ccf::crypto::sharing::Share::serialised_size); + ls_wrapping_key.get_full_share_serialised(full_share_serialised); + + for (auto const& [member_id, enc_pub_key] : active_recovery_owners_info) + { + auto member_enc_pubk = ccf::crypto::make_rsa_public_key(enc_pub_key); + encrypted_shares[member_id] = + member_enc_pubk->rsa_oaep_wrap(full_share_serialised); + } + + OPENSSL_cleanse( + full_share_serialised.data(), full_share_serialised.size()); + } + return encrypted_shares; } void shuffle_recovery_shares( ccf::kv::Tx& tx, const LedgerSecretPtr& latest_ledger_secret) { - auto active_recovery_members_info = - InternalTablesAccess::get_active_recovery_members(tx); + auto active_recovery_participants_info = + InternalTablesAccess::get_active_recovery_participants(tx); + auto active_recovery_owners_info = + InternalTablesAccess::get_active_recovery_owners(tx); size_t recovery_threshold = InternalTablesAccess::get_recovery_threshold(tx); - if (active_recovery_members_info.empty()) + if ( + active_recovery_participants_info.empty() && + active_recovery_owners_info.empty()) { throw std::logic_error( "There should be at least one active recovery member to issue " @@ -191,18 +237,36 @@ namespace ccf "shares are computed"); } - if (recovery_threshold > active_recovery_members_info.size()) + size_t num_shares; + if (!active_recovery_participants_info.empty()) { - throw std::logic_error(fmt::format( - "Recovery threshold {} should be equal to or less than the number of " - "active recovery members {}", - recovery_threshold, - active_recovery_members_info.size())); + if (recovery_threshold > active_recovery_participants_info.size()) + { + throw std::logic_error(fmt::format( + "Recovery threshold {} should be equal to or less than the number " + "of active recovery members {}", + recovery_threshold, + active_recovery_participants_info.size())); + } + + num_shares = active_recovery_participants_info.size(); + } + else + { + if (recovery_threshold > 1) + { + throw std::logic_error(fmt::format( + "Recovery threshold {} cannot be greater than 1 when the " + "consortium consists of only active recovery owner members ({})", + recovery_threshold, + active_recovery_owners_info.size())); + } + + num_shares = 1; } - const auto num_shares = active_recovery_members_info.size(); auto ls_wrapping_key = - LedgerSecretWrappingKey(num_shares, recovery_threshold); + SharedLedgerSecretWrappingKey(num_shares, recovery_threshold); auto wrapped_latest_ls = ls_wrapping_key.wrap(latest_ledger_secret); auto recovery_shares = tx.rw(Tables::SHARES); @@ -299,16 +363,17 @@ namespace ccf return decrypted_share; } - LedgerSecretWrappingKey combine_from_encrypted_submitted_shares( - ccf::kv::Tx& tx) + ReconstructedLedgerSecretWrappingKey + combine_from_encrypted_submitted_shares(ccf::kv::Tx& tx) { auto encrypted_submitted_shares = tx.rw( Tables::ENCRYPTED_SUBMITTED_SHARES); auto config = tx.rw(Tables::CONFIGURATION); + std::optional full_share; std::vector new_shares = {}; encrypted_submitted_shares->foreach( - [&new_shares, &tx, this]( + [&new_shares, &full_share, &tx, this]( const MemberId, const EncryptedSubmittedShare& encrypted_share) { auto decrypted_share = decrypt_submitted_share( encrypted_share, ledger_secrets->get_latest(tx).second); @@ -316,7 +381,20 @@ namespace ccf { case ccf::crypto::sharing::Share::serialised_size: { - new_shares.emplace_back(decrypted_share); + // For a new share, we can check the index and decide if it's + // a full share or just a partial share (compare to zero). + // If it is a full share, we can short-circuit and return a + // ReconstructedLedgerSecretWrappingKey directly, otherwise we + // follow the existing flow. + auto share = ccf::crypto::sharing::Share(decrypted_share); + if (share.x == 0) + { + full_share = share; + } + else + { + new_shares.emplace_back(decrypted_share); + } break; } default: @@ -330,9 +408,19 @@ namespace ccf } } OPENSSL_cleanse(decrypted_share.data(), decrypted_share.size()); + if (full_share.has_value()) + { + return false; + } + return true; }); + if (full_share.has_value()) + { + return ReconstructedLedgerSecretWrappingKey(full_share.value()); + } + auto num_shares = new_shares.size(); auto recovery_threshold = config->get()->recovery_threshold; @@ -345,7 +433,8 @@ namespace ccf recovery_threshold)); } - return LedgerSecretWrappingKey(std::move(new_shares), recovery_threshold); + return ReconstructedLedgerSecretWrappingKey( + std::move(new_shares), recovery_threshold); } public: @@ -489,6 +578,24 @@ namespace ccf return restored_ledger_secrets; } + static bool is_full_key( + const std::vector& submitted_recovery_share) + { + if ( + submitted_recovery_share.size() == + ccf::crypto::sharing::Share::serialised_size) + { + auto share = ccf::crypto::sharing::Share(submitted_recovery_share); + if (share.x == 0) + { + // Index value of 0 indicates a full key. + return true; + } + } + + return false; + } + size_t submit_recovery_share( ccf::kv::Tx& tx, MemberId member_id, diff --git a/src/service/internal_tables_access.h b/src/service/internal_tables_access.h index a4733f18ddd..72dbd998edd 100644 --- a/src/service/internal_tables_access.h +++ b/src/service/internal_tables_access.h @@ -70,7 +70,7 @@ namespace ccf } } - static bool is_recovery_member( + static bool is_recovery_participant_or_owner( ccf::kv::ReadOnlyTx& tx, const MemberId& member_id) { auto member_encryption_public_keys = @@ -80,6 +80,27 @@ namespace ccf return member_encryption_public_keys->get(member_id).has_value(); } + static bool is_recovery_participant( + ccf::kv::ReadOnlyTx& tx, const MemberId& member_id) + { + return is_recovery_participant_or_owner(tx, member_id) && + !is_recovery_owner(tx, member_id); + } + + static bool is_recovery_owner( + ccf::kv::ReadOnlyTx& tx, const MemberId& member_id) + { + auto member_info = tx.ro(Tables::MEMBER_INFO); + auto mi = member_info->get(member_id); + if (!mi.has_value()) + { + return false; + } + + return mi->recovery_role.has_value() && + mi->recovery_role.value() == MemberRecoveryRole::Owner; + } + static bool is_active_member( ccf::kv::ReadOnlyTx& tx, const MemberId& member_id) { @@ -93,7 +114,39 @@ namespace ccf return mi->status == MemberStatus::ACTIVE; } - static std::map get_active_recovery_members( + static std::map + get_active_recovery_participants(ccf::kv::ReadOnlyTx& tx) + { + auto member_info = tx.ro(Tables::MEMBER_INFO); + auto member_encryption_public_keys = + tx.ro( + Tables::MEMBER_ENCRYPTION_PUBLIC_KEYS); + + std::map active_recovery_participants; + + member_encryption_public_keys->foreach( + [&active_recovery_participants, + &member_info](const auto& mid, const auto& pem) { + auto info = member_info->get(mid); + if (!info.has_value()) + { + throw std::logic_error( + fmt::format("Recovery member {} has no member info", mid)); + } + + if ( + info->status == MemberStatus::ACTIVE && + info->recovery_role.value_or(MemberRecoveryRole::Participant) == + MemberRecoveryRole::Participant) + { + active_recovery_participants[mid] = pem; + } + return true; + }); + return active_recovery_participants; + } + + static std::map get_active_recovery_owners( ccf::kv::ReadOnlyTx& tx) { auto member_info = tx.ro(Tables::MEMBER_INFO); @@ -101,10 +154,10 @@ namespace ccf tx.ro( Tables::MEMBER_ENCRYPTION_PUBLIC_KEYS); - std::map active_recovery_members; + std::map active_recovery_owners; member_encryption_public_keys->foreach( - [&active_recovery_members, + [&active_recovery_owners, &member_info](const auto& mid, const auto& pem) { auto info = member_info->get(mid); if (!info.has_value()) @@ -113,13 +166,16 @@ namespace ccf fmt::format("Recovery member {} has no member info", mid)); } - if (info->status == MemberStatus::ACTIVE) + if ( + info->status == MemberStatus::ACTIVE && + info->recovery_role.value_or(MemberRecoveryRole::Participant) == + MemberRecoveryRole::Owner) { - active_recovery_members[mid] = pem; + active_recovery_owners[mid] = pem; } return true; }); - return active_recovery_members; + return active_recovery_owners; } static MemberId add_member( @@ -141,9 +197,42 @@ namespace ccf return id; } + if (member_pub_info.recovery_role.has_value()) + { + auto member_recovery_role = member_pub_info.recovery_role.value(); + if (!member_pub_info.encryption_pub_key.has_value()) + { + if (member_recovery_role != ccf::MemberRecoveryRole::NonParticipant) + { + throw std::logic_error(fmt::format( + "Member {} cannot be added as recovery_role has a value set but " + "no " + "encryption public key is specified", + id)); + } + } + else + { + if ( + member_recovery_role != ccf::MemberRecoveryRole::Participant && + member_recovery_role != ccf::MemberRecoveryRole::Owner) + { + throw std::logic_error(fmt::format( + "Recovery member {} cannot be added as with recovery role value " + "of " + "{}", + id, + member_recovery_role)); + } + } + } + member_certs->put(id, member_pub_info.cert); member_info->put( - id, {MemberStatus::ACCEPTED, member_pub_info.member_data}); + id, + {MemberStatus::ACCEPTED, + member_pub_info.member_data, + member_pub_info.recovery_role}); if (member_pub_info.encryption_pub_key.has_value()) { @@ -209,24 +298,76 @@ namespace ccf // If the member was active and had a recovery share, check that // the new number of active members is still sufficient for // recovery - if ( - member_to_remove->status == MemberStatus::ACTIVE && - is_recovery_member(tx, member_id)) - { - // Because the member to remove is active, there is at least one active - // member (i.e. get_active_recovery_members_count_after >= 0) - size_t get_active_recovery_members_count_after = - get_active_recovery_members(tx).size() - 1; - auto recovery_threshold = get_recovery_threshold(tx); - if (get_active_recovery_members_count_after < recovery_threshold) + if (member_to_remove->status == MemberStatus::ACTIVE) + { + if (is_recovery_participant(tx, member_id)) { - LOG_FAIL_FMT( - "Failed to remove recovery member {}: number of active recovery " - "members ({}) would be less than recovery threshold ({})", - member_id, - get_active_recovery_members_count_after, - recovery_threshold); - return false; + size_t active_recovery_participants_count_after = + get_active_recovery_participants(tx).size() - 1; + auto recovery_threshold = get_recovery_threshold(tx); + auto active_recovery_owners_count = + get_active_recovery_owners(tx).size(); + if ( + active_recovery_participants_count_after == 0 && + active_recovery_owners_count > 0 && recovery_threshold == 1) + { + // Its fine to remove all active recovery particiants as long as + // recover owner(s) exist with a threshold of 1. + LOG_INFO_FMT( + "Allowing last active recovery participant member {}: to " + "be removed as active recovery owner members ({}) are present " + "with recovery threshold ({}).", + member_id, + active_recovery_owners_count, + recovery_threshold); + } + else if ( + active_recovery_participants_count_after < recovery_threshold) + { + // Because the member to remove is active, there is at least one + // active member (i.e. active_recovery_participants_count_after >= + // 0) + LOG_FAIL_FMT( + "Failed to remove recovery member {}: number of active recovery " + "participant members ({}) would be less than recovery threshold " + "({})", + member_id, + active_recovery_participants_count_after, + recovery_threshold); + return false; + } + } + else if (is_recovery_owner(tx, member_id)) + { + size_t active_recovery_owners_count_after = + get_active_recovery_owners(tx).size() - 1; + auto recovery_threshold = get_recovery_threshold(tx); + auto active_recovery_participants_count = + get_active_recovery_participants(tx).size(); + if (active_recovery_owners_count_after == 0) + { + if (active_recovery_participants_count > 0) + { + LOG_INFO_FMT( + "Allowing last active recovery owner member {}: to " + "be removed as active recovery owner participants ({}) are " + "present with recovery threshold ({}).", + member_id, + active_recovery_participants_count, + recovery_threshold); + } + else + { + LOG_FAIL_FMT( + "Failed to remove last active recovery owner member {}: number " + "of active recovery participant members ({}) would be less " + "than recovery threshold ({})", + member_id, + active_recovery_participants_count, + recovery_threshold); + return false; + } + } } } @@ -493,14 +634,30 @@ namespace ccf { auto service = tx.rw(Tables::SERVICE); - auto active_recovery_members_count = - get_active_recovery_members(tx).size(); - if (active_recovery_members_count < get_recovery_threshold(tx)) + auto active_recovery_participants_count = + get_active_recovery_participants(tx).size(); + auto active_recovery_owners_count = get_active_recovery_owners(tx).size(); + if ( + active_recovery_participants_count == 0 && + active_recovery_owners_count != 0) + { + if (get_recovery_threshold(tx) > 1) + { + LOG_FAIL_FMT( + "Cannot open network as a network with only active recovery owners " + "({}) can have " + "a recovery threshold of 1 but current recovery threshold value is " + "({})", + active_recovery_owners_count, + get_recovery_threshold(tx)); + } + } + else if (active_recovery_participants_count < get_recovery_threshold(tx)) { LOG_FAIL_FMT( "Cannot open network as number of active recovery members ({}) is " "less than recovery threshold ({})", - active_recovery_members_count, + active_recovery_participants_count, get_recovery_threshold(tx)); return false; } @@ -708,15 +865,33 @@ namespace ccf } else if (service_status.value() == ServiceStatus::OPEN) { - auto get_active_recovery_members_count = - get_active_recovery_members(tx).size(); - if (threshold > get_active_recovery_members_count) + auto active_recovery_participants_count = + get_active_recovery_participants(tx).size(); + auto active_recovery_owners_count = + get_active_recovery_owners(tx).size(); + + if ( + active_recovery_owners_count != 0 && + active_recovery_participants_count == 0) + { + if (threshold > 1) + { + LOG_FAIL_FMT( + "Cannot set recovery threshold to {} when only " + "active consortium members ({}) that are of type recovery owner " + "exist.", + threshold, + active_recovery_owners_count); + return false; + } + } + else if (threshold > active_recovery_participants_count) { LOG_FAIL_FMT( "Cannot set recovery threshold to {} as it is greater than the " - "number of active recovery members ({})", + "number of active recovery participant members ({})", threshold, - get_active_recovery_members_count); + active_recovery_participants_count); return false; } } diff --git a/tests/governance.py b/tests/governance.py index e778d9a3f4f..65218ad06be 100644 --- a/tests/governance.py +++ b/tests/governance.py @@ -2,6 +2,7 @@ # Licensed under the Apache 2.0 License. import os import http +import infra.member import infra.network import infra.path import infra.proc @@ -255,7 +256,7 @@ def run_test_all_members(network): assert response_data == member.member_data response_pub_enc_key = response_member.get("publicEncryptionKey") - if member.is_recovery_member: + if member.recovery_role != infra.member.RecoveryRole.NonParticipant: enc_pub_key_file = os.path.join( primary.common_dir, member.member_info["encryption_public_key_file"] ) @@ -264,6 +265,14 @@ def run_test_all_members(network): else: assert response_pub_enc_key is None + response_recovery_role = response_member.get("recoveryRole") + if member.recovery_role == infra.member.RecoveryRole.Owner: + assert response_recovery_role == "Owner" + elif member.recovery_role == infra.member.RecoveryRole.Participant: + assert response_recovery_role == "Participant" + else: + assert response_recovery_role == "NonParticipant" + # Test on current network run_test_all_members(network) diff --git a/tests/governance_js.py b/tests/governance_js.py index b413c09828f..b4b24ebaecc 100644 --- a/tests/governance_js.py +++ b/tests/governance_js.py @@ -728,7 +728,7 @@ def propose_and_assert_accepted(signer_id, proposal): args.participants_curve, network.consortium.common_dir, network.consortium.share_script, - is_recovery_member=False, + recovery_role=infra.member.RecoveryRole.NonParticipant, key_generator=network.consortium.key_generator, authenticate_session=network.consortium.authenticate_session, gov_api_impl=network.consortium.gov_api_impl, @@ -855,7 +855,9 @@ def test_actions(network, args): try: network.consortium.set_recovery_threshold( node, - recovery_threshold=len(network.consortium.get_active_recovery_members()) + recovery_threshold=len( + network.consortium.get_active_recovery_participants() + ) + 1, ) assert ( diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index 14fafffea66..19c4c2dccda 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -71,33 +71,38 @@ def __init__( # and the state of the service if members_info is not None: self.recovery_threshold = 0 - for m_local_id, has_share, m_data in members_info: + for m_local_id, recovery_role, m_data in members_info: new_member = infra.member.Member( f"member{m_local_id}", curve, common_dir, share_script, - has_share, + recovery_role, key_generator, m_data, authenticate_session=authenticate_session, gov_api_impl=self.gov_api_impl, ) - if has_share: + if recovery_role == infra.member.RecoveryRole.Participant: self.recovery_threshold += 1 self.members.append(new_member) else: for f in os.listdir(self.common_dir): if re.search("member(.*)_cert.pem", f) is not None: local_id = f.split("_")[0] + recovery_role = ( + infra.member.RecoveryRole.Participant + if os.path.isfile( + os.path.join(self.common_dir, f"{local_id}_enc_privk.pem") + ) + else infra.member.RecoveryRole.NonParticipant + ) new_member = infra.member.Member( local_id, curve, self.common_dir, share_script, - is_recovery_member=os.path.isfile( - os.path.join(self.common_dir, f"{local_id}_enc_privk.pem") - ), + recovery_role, authenticate_session=authenticate_session, gov_api_impl=self.gov_api_impl, ) @@ -198,7 +203,11 @@ def activate(self, remote_node): m.ack(remote_node) def generate_and_propose_new_member( - self, remote_node, curve, recovery_member=True, member_data=None + self, + remote_node, + curve, + recovery_role=infra.member.RecoveryRole.Participant, + member_data=None, ): # The Member returned by this function is in state ACCEPTED. The new Member # should ACK to become active. @@ -208,7 +217,7 @@ def generate_and_propose_new_member( curve, self.common_dir, self.share_script, - is_recovery_member=recovery_member, + recovery_role=recovery_role, key_generator=self.key_generator, authenticate_session=self.authenticate_session, gov_api_impl=self.gov_api_impl, @@ -223,10 +232,13 @@ def generate_and_propose_new_member( slurp_file( os.path.join(self.common_dir, f"{new_member_local_id}_enc_pubk.pem") ) - if recovery_member + if recovery_role != infra.member.RecoveryRole.NonParticipant else None ), member_data=member_data, + recovery_role=( + "Owner" if recovery_role == infra.member.RecoveryRole.Owner else None + ), ) proposal = self.get_any_active_member().propose(remote_node, proposal_body) @@ -235,10 +247,14 @@ def generate_and_propose_new_member( return (proposal, new_member, careful_vote) def generate_and_add_new_member( - self, remote_node, curve, recovery_member=True, member_data=None + self, + remote_node, + curve, + recovery_role=infra.member.RecoveryRole.Participant, + member_data=None, ): proposal, new_member, careful_vote = self.generate_and_propose_new_member( - remote_node, curve, recovery_member, member_data + remote_node, curve, recovery_role, member_data ) self.vote_using_majority(remote_node, proposal, careful_vote) @@ -256,25 +272,43 @@ def get_members_info(self): def get_active_members(self): return [member for member in self.members if member.is_active()] - def get_active_recovery_members(self): + def get_active_recovery_participants(self): return [ member for member in self.members - if (member.is_active() and member.is_recovery_member) + if ( + member.is_active() + and member.recovery_role == infra.member.RecoveryRole.Participant + ) + ] + + def get_active_recovery_owners(self): + return [ + member + for member in self.members + if ( + member.is_active() + and member.recovery_role == infra.member.RecoveryRole.Owner + ) ] def get_active_non_recovery_members(self): return [ member for member in self.members - if (member.is_active() and not member.is_recovery_member) + if ( + member.is_active() + and member.recovery_role == infra.member.RecoveryRole.NonParticipant + ) ] - def get_any_active_member(self, recovery_member=None): - if recovery_member is not None: - if recovery_member is True: - return random.choice(self.get_active_recovery_members()) - elif recovery_member is False: + def get_any_active_member(self, recovery_role=None): + if recovery_role is not None: + if recovery_role is infra.member.RecoveryRole.Owner: + return random.choice(self.get_active_recovery_owners()) + elif recovery_role is infra.member.RecoveryRole.Participant: + return random.choice(self.get_active_recovery_participants()) + else: return random.choice(self.get_active_non_recovery_members()) else: return random.choice(self.get_active_members()) @@ -706,7 +740,7 @@ def recover_with_shares(self, remote_node): with remote_node.client() as nc: check_commit = infra.checker.Checker(nc) - for m in self.get_active_recovery_members(): + for m in self.get_active_recovery_participants(): r = m.get_and_submit_recovery_share(remote_node) submitted_shares_count += 1 check_commit(r) @@ -721,6 +755,20 @@ def recover_with_shares(self, remote_node): else: assert "End of recovery procedure initiated" not in r.body.text() + def recover_with_owner_share(self, remote_node): + submitted_shares_count = 0 + with remote_node.client() as nc: + check_commit = infra.checker.Checker(nc) + + m = self.get_any_active_member( + recovery_role=infra.member.RecoveryRole.Owner + ) + r = m.get_and_submit_recovery_share(remote_node) + submitted_shares_count += 1 + check_commit(r) + assert "Full recovery key successfully submitted" in r.body.text() + assert "End of recovery procedure initiated" in r.body.text() + def set_recovery_threshold(self, remote_node, recovery_threshold): proposal_body, careful_vote = self.make_proposal( "set_recovery_threshold", recovery_threshold=recovery_threshold diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index b6123217c8b..23d4e157291 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -274,11 +274,17 @@ def cli_args( default=1, ) parser.add_argument( - "--initial-recovery-member-count", - help="Number of initial members that are handed recovery shares", + "--initial-recovery-participant-count", + help="Number of initial members that are handed partial recovery shares", type=int, default=int(os.getenv("INITIAL_MEMBER_COUNT", "3")), ) + parser.add_argument( + "--initial-recovery-owner-count", + help="Number of initial members that are handed full recovery shares", + type=int, + default=int(os.getenv("INITIAL_RECOVERY_OWNER_COUNT", "0")), + ) parser.add_argument( "--ledger-recovery-timeout", help="On recovery, maximum timeout (s) while reading the ledger", diff --git a/tests/infra/member.py b/tests/infra/member.py index d40a8a8b083..728f4108fe9 100644 --- a/tests/infra/member.py +++ b/tests/infra/member.py @@ -37,6 +37,12 @@ class MemberStatus(Enum): ACTIVE = "Active" +class RecoveryRole(Enum): + NonParticipant = "NonParticipant" + Participant = "Participant" + Owner = "Owner" + + class MemberAPI: class v1_Base: def propose(self, member, remote_node, proposal): @@ -273,7 +279,7 @@ def __init__( curve, common_dir, share_script, - is_recovery_member=True, + recovery_role=RecoveryRole.Participant, key_generator=None, member_data=None, authenticate_session=True, @@ -284,7 +290,7 @@ def __init__( self.status = MemberStatus.ACCEPTED self.share_script = share_script self.member_data = member_data - self.is_recovery_member = is_recovery_member + self.recovery_role = recovery_role self.is_retired = False self.authenticate_session = authenticate_session assert self.authenticate_session == "COSE", self.authenticate_session @@ -298,11 +304,15 @@ def __init__( self.member_info = {} self.member_info["certificate_file"] = f"{self.local_id}_cert.pem" self.member_info["encryption_public_key_file"] = ( - f"{self.local_id}_enc_pubk.pem" if is_recovery_member else None + f"{self.local_id}_enc_pubk.pem" + if recovery_role != RecoveryRole.NonParticipant + else None ) self.member_info["data_json_file"] = ( f"{self.local_id}_data.json" if member_data else None ) + if recovery_role == RecoveryRole.Owner: + self.member_info["recovery_role"] = "Owner" if key_generator is not None: key_generator_args = [ @@ -312,7 +322,7 @@ def __init__( f"{curve.name}", ] - if is_recovery_member: + if recovery_role != RecoveryRole.NonParticipant: key_generator_args += [ "--gen-enc-key", ] @@ -418,7 +428,7 @@ def get_and_decrypt_recovery_share(self, remote_node): ) def get_and_submit_recovery_share(self, remote_node): - if not self.is_recovery_member: + if self.recovery_role == RecoveryRole.NonParticipant: raise ValueError(f"Member {self.local_id} does not have a recovery share") help_res = infra.proc.ccall(self.share_script, "--help", log_output=False) diff --git a/tests/infra/network.py b/tests/infra/network.py index fcb9244ac12..ab325dd1a8c 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -6,6 +6,7 @@ from contextlib import contextmanager from enum import Enum, IntEnum, auto from infra.clients import flush_info +import infra.member import infra.path import infra.proc import infra.service_load @@ -558,6 +559,13 @@ def start(self, args, **kwargs): mc >= args.initial_operator_provisioner_count + args.initial_operator_count ), f"Not enough members ({mc}) for the set amount of operator provisioners and operators" + if args.initial_recovery_owner_count > 0: + assert ( + mc + >= args.initial_recovery_participant_count + + args.initial_recovery_owner_count + ), f"Not enough members ({mc}) for the set amount of recovery participants and owners ({args.initial_recovery_participant_count + args.initial_recovery_owner_count})" + initial_members_info = [] for i in range(mc): member_data = None @@ -568,10 +576,20 @@ def start(self, args, **kwargs): < args.initial_operator_provisioner_count + args.initial_operator_count ): member_data = {"is_operator": True} + recovery_role = infra.member.RecoveryRole.NonParticipant + if i < args.initial_recovery_participant_count: + recovery_role = infra.member.RecoveryRole.Participant + elif ( + i + < args.initial_recovery_participant_count + + args.initial_recovery_owner_count + ): + recovery_role = infra.member.RecoveryRole.Owner + initial_members_info += [ ( i, - (i < args.initial_recovery_member_count), + recovery_role, member_data, ) ] @@ -698,7 +716,7 @@ def start_in_recovery( self.wait_for_all_nodes_to_commit(primary=primary, timeout=20) LOG.success("All nodes joined public network") - def recover(self, args, expected_recovery_count=None): + def recover(self, args, expected_recovery_count=None, via_recovery_owner=False): """ Recovers a CCF network previously started in recovery mode. :param args: command line arguments to configure the CCF nodes. @@ -727,7 +745,11 @@ def recover(self, args, expected_recovery_count=None): self.find_random_node(), previous_service_identity=prev_service_identity, ) - self.consortium.recover_with_shares(self.find_random_node()) + + if via_recovery_owner: + self.consortium.recover_with_owner_share(self.find_random_node()) + else: + self.consortium.recover_with_shares(self.find_random_node()) for node in self.get_joined_nodes(): self.wait_for_state( diff --git a/tests/membership.py b/tests/membership.py index 299f6fe661d..30e63274966 100644 --- a/tests/membership.py +++ b/tests/membership.py @@ -15,7 +15,7 @@ @reqs.description("Add and activate a new member to the consortium") -def test_add_member(network, args, recovery_member=True): +def test_add_member(network, args, recovery_role=infra.member.RecoveryRole.Participant): primary, _ = network.find_primary() member_data = { @@ -27,7 +27,7 @@ def test_add_member(network, args, recovery_member=True): primary, curve=infra.network.EllipticCurve(args.participants_curve).next(), member_data=member_data, - recovery_member=recovery_member, + recovery_role=recovery_role, ) r = new_member.ack(primary) @@ -39,11 +39,14 @@ def test_add_member(network, args, recovery_member=True): @reqs.description("Retire existing member") def test_remove_member_no_reqs( - network, args, member_to_remove=None, recovery_member=True + network, + args, + member_to_remove=None, + recovery_role=infra.member.RecoveryRole.Participant, ): primary, _ = network.find_primary() if member_to_remove is None: - member_to_remove = network.consortium.get_any_active_member(recovery_member) + member_to_remove = network.consortium.get_any_active_member(recovery_role) network.consortium.remove_member(primary, member_to_remove) # Check that remove member cannot be authenticated by the service @@ -59,8 +62,13 @@ def test_remove_member_no_reqs( # Called by test suite. membership test deliberately attempts to remove recovery member. @reqs.sufficient_recovery_member_count() -def test_remove_member(network, args, member_to_remove=None, recovery_member=True): - return test_remove_member_no_reqs(network, args, member_to_remove, recovery_member) +def test_remove_member( + network, + args, + member_to_remove=None, + recovery_role=infra.member.RecoveryRole.Participant, +): + return test_remove_member_no_reqs(network, args, member_to_remove, recovery_role) @reqs.description("Issue new recovery shares (without re-key)") @@ -78,7 +86,7 @@ def test_set_recovery_threshold(network, args, recovery_threshold=None): # The new recovery threshold is guaranteed to be different from the # previous one. list_recovery_threshold = list( - range(1, len(network.consortium.get_active_recovery_members()) + 1) + range(1, len(network.consortium.get_active_recovery_participants()) + 1) ) list_recovery_threshold.remove(network.consortium.recovery_threshold) recovery_threshold = random.choice(list_recovery_threshold) @@ -88,29 +96,40 @@ def test_set_recovery_threshold(network, args, recovery_threshold=None): return network -def assert_recovery_shares_update(are_shared_updated, func, network, args, **kwargs): +def assert_recovery_shares_update(are_shares_updated, func, network, args, **kwargs): primary, _ = network.find_primary() - saved_recovery_shares = {} - for m in network.consortium.get_active_recovery_members(): - saved_recovery_shares[m] = m.get_and_decrypt_recovery_share(primary) + saved_recovery_member_shares = {} + saved_recovery_owner_shares = {} + for m in network.consortium.get_active_recovery_participants(): + saved_recovery_member_shares[m] = m.get_and_decrypt_recovery_share(primary) + for m in network.consortium.get_active_recovery_owners(): + saved_recovery_owner_shares[m] = m.get_and_decrypt_recovery_share(primary) if func is test_remove_member: - recovery_member = kwargs.pop("recovery_member") + recovery_role = kwargs.pop("recovery_role") member_to_remove = network.consortium.get_any_active_member( - recovery_member=recovery_member + recovery_role=recovery_role ) - if recovery_member: - saved_recovery_shares.pop(member_to_remove) + if recovery_role == infra.member.RecoveryRole.Owner: + saved_recovery_owner_shares.pop(member_to_remove) + elif recovery_role == infra.member.RecoveryRole.Participant: + saved_recovery_member_shares.pop(member_to_remove) - func(network, args, member_to_remove) + func(network, args, member_to_remove, recovery_role) elif func is test_set_recovery_threshold and "recovery_threshold" in kwargs: func(network, args, recovery_threshold=kwargs["recovery_threshold"]) else: func(network, args, **kwargs) - for m, share_before in saved_recovery_shares.items(): - if are_shared_updated: + for m, share_before in saved_recovery_member_shares.items(): + if are_shares_updated: + assert share_before != m.get_and_decrypt_recovery_share(primary) + else: + assert share_before == m.get_and_decrypt_recovery_share(primary) + + for m, share_before in saved_recovery_owner_shares.items(): + if are_shares_updated: assert share_before != m.get_and_decrypt_recovery_share(primary) else: assert share_before == m.get_and_decrypt_recovery_share(primary) @@ -119,7 +138,8 @@ def assert_recovery_shares_update(are_shared_updated, func, network, args, **kwa def service_startups(args): LOG.info("Starting service with insufficient number of recovery members") args.initial_member_count = 2 - args.initial_recovery_member_count = 0 + args.initial_recovery_participant_count = 0 + args.initial_recovery_owner_count = 0 args.initial_operator_count = 1 args.ledger_recovery_timeout = 5 with infra.network.network(args.nodes, args.binary_dir, pdb=args.pdb) as network: @@ -139,25 +159,47 @@ def service_startups(args): "Starting service with a recovery operator member, a non-recovery operator member and a non-recovery non-operator member" ) args.initial_member_count = 3 - args.initial_recovery_member_count = 1 + args.initial_recovery_participant_count = 1 + args.initial_recovery_owner_count = 0 args.initial_operator_count = 2 with infra.network.network(args.nodes, args.binary_dir, pdb=args.pdb) as network: network.start_and_open(args) LOG.info( - "Starting service with a recovery operator member, a recovery non-operator member and a non-recovery non-operator member" + "Starting service with a recovery operator member, a recovery non-operator member, a non-recovery non-operator member and a recovery owner member" ) - args.initial_member_count = 3 - args.initial_recovery_member_count = 2 + args.initial_member_count = 4 + args.initial_recovery_participant_count = 2 + args.initial_recovery_owner_count = 1 args.initial_operator_count = 1 with infra.network.network(args.nodes, args.binary_dir, pdb=args.pdb) as network: network.start_and_open(args) + LOG.info("Starting service with a recovery member number of recovery members") + args.initial_member_count = 2 + args.initial_recovery_participant_count = 0 + args.initial_recovery_owner_count = 0 + args.initial_operator_count = 1 + args.ledger_recovery_timeout = 5 + with infra.network.network(args.nodes, args.binary_dir, pdb=args.pdb) as network: + try: + network.start_and_open(args) + assert False, "Service cannot be opened with no recovery members" + except infra.proposal.ProposalNotAccepted: + primary, _ = network.find_primary() + network.consortium.check_for_service( + primary, infra.network.ServiceStatus.OPENING + ) + LOG.success( + "Service could not be opened with insufficient number of recovery members" + ) + def recovery_shares_scenario(args): # Members 0 and 1 are recovery members, member 2 isn't args.initial_member_count = 3 - args.initial_recovery_member_count = 2 + args.initial_recovery_participant_count = 2 + args.initial_recovery_owner_count = 0 non_recovery_member_id = "member2" # Recovery threshold is initially set to number of recovery members (2) @@ -187,7 +229,9 @@ def recovery_shares_scenario(args): # members would be under recovery threshold (2) LOG.info("Removing a recovery member should not be possible") try: - test_remove_member_no_reqs(network, args, recovery_member=True) + test_remove_member_no_reqs( + network, args, recovery_role=infra.member.RecoveryRole.Participant + ) assert False, "Removing a recovery member should not be possible" except infra.proposal.ProposalNotAccepted as e: # This is an apply() time failure, so the proposal remains Open @@ -200,29 +244,67 @@ def recovery_shares_scenario(args): non_recovery_member_id ) test_remove_member( - network, args, member_to_remove=member_to_remove, recovery_member=False + network, + args, + member_to_remove=member_to_remove, + recovery_role=infra.member.RecoveryRole.NonParticipant, ) LOG.info("Removing an already-removed member succeeds with no effect") test_remove_member( - network, args, member_to_remove=member_to_remove, recovery_member=False + network, + args, + member_to_remove=member_to_remove, + recovery_role=infra.member.RecoveryRole.NonParticipant, ) LOG.info("Adding one non-recovery member") assert_recovery_shares_update( - False, test_add_member, network, args, recovery_member=False + False, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.NonParticipant, ) LOG.info("Adding one recovery member") assert_recovery_shares_update( - True, test_add_member, network, args, recovery_member=True + True, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, + ) + LOG.info("Adding one recovery owner") + assert_recovery_shares_update( + True, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Owner, ) LOG.info("Removing one non-recovery member") assert_recovery_shares_update( - False, test_remove_member, network, args, recovery_member=False + False, + test_remove_member, + network, + args, + recovery_role=infra.member.RecoveryRole.NonParticipant, ) LOG.info("Removing one recovery member") assert_recovery_shares_update( - True, test_remove_member, network, args, recovery_member=True + True, + test_remove_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, + ) + LOG.info("Removing one recovery owner") + assert_recovery_shares_update( + True, + test_remove_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Owner, ) LOG.info("Reduce recovery threshold") @@ -237,7 +319,11 @@ def recovery_shares_scenario(args): # Removing a recovery member now succeeds LOG.info("Removing one recovery member") assert_recovery_shares_update( - True, test_remove_member, network, args, recovery_member=True + True, + test_remove_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, ) LOG.info("Set recovery threshold to 0 is impossible") @@ -259,7 +345,9 @@ def recovery_shares_scenario(args): test_set_recovery_threshold( network, args, - recovery_threshold=len(network.consortium.get_active_recovery_members()) + recovery_threshold=len( + network.consortium.get_active_recovery_participants() + ) + 1, ) assert ( @@ -302,9 +390,253 @@ def recovery_shares_scenario(args): ) +def recovery_shares_with_owners_scenario(args): + # Members 0 and 1 are recovery participants, member 2 is recovery owner and member 3 is non-recovery member + args.initial_member_count = 4 + args.initial_recovery_participant_count = 2 + args.initial_recovery_owner_count = 1 + recovery_owner_id = "member2" + non_recovery_member_id = "member3" + + # Recovery threshold is initially set to number of recovery participants (2) + with infra.network.network( + args.nodes, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb + ) as network: + network.start_and_open(args) + + assert ( + len(network.consortium.get_active_recovery_participants()) == 2 + ), f"Unexpected recovery members count: {len(network.consortium.get_active_recovery_participants())}" + + assert ( + len(network.consortium.get_active_recovery_owners()) == 1 + ), f"Unexpected recovery owners count: {len(network.consortium.get_active_recovery_owners())}" + + # Removing the only recovery owner is allowed as recovery participant members exist. + LOG.info( + "Removing the recovery owner is still possible as recovery members exist" + ) + member_to_remove = network.consortium.get_member_by_local_id(recovery_owner_id) + test_remove_member( + network, + args, + member_to_remove=member_to_remove, + recovery_role=infra.member.RecoveryRole.Owner, + ) + + # Recovery owner count should now be 0. + assert ( + len(network.consortium.get_active_recovery_owners()) == 0 + ), f"Unexpected recovery owners count: {len(network.consortium.get_active_recovery_owners())}" + + # Removing a recovery member is not possible as the number of recovery + # participant members (2) would be under recovery threshold (2) + LOG.info("Removing a recovery member should not be possible") + try: + test_remove_member_no_reqs( + network, args, recovery_role=infra.member.RecoveryRole.Participant + ) + assert False, "Removing a recovery member should not be possible" + except infra.proposal.ProposalNotAccepted as e: + # This is an apply() time failure, so the proposal remains Open + # since the last vote is effectively discarded + assert e.proposal.state == infra.proposal.ProposalState.OPEN + + # However, removing a non-recovery member is allowed + LOG.info("Removing a non-recovery member is still possible") + member_to_remove = network.consortium.get_member_by_local_id( + non_recovery_member_id + ) + test_remove_member( + network, + args, + member_to_remove=member_to_remove, + recovery_role=infra.member.RecoveryRole.NonParticipant, + ) + + LOG.info("Removing an already-removed member succeeds with no effect") + test_remove_member( + network, + args, + member_to_remove=member_to_remove, + recovery_role=infra.member.RecoveryRole.NonParticipant, + ) + + LOG.info("Adding one non-recovery member") + assert_recovery_shares_update( + False, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.NonParticipant, + ) + + LOG.info("Adding one recovery owner") + assert_recovery_shares_update( + True, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Owner, + ) + assert ( + len(network.consortium.get_active_recovery_owners()) == 1 + ), f"Unexpected recovery owners count: {len(network.consortium.get_active_recovery_owners())}" + + assert ( + len(network.consortium.get_active_recovery_participants()) == 2 + ), f"Unexpected recovery members count: {len(network.consortium.get_active_recovery_participants())}" + + LOG.info("Reduce recovery threshold") + assert_recovery_shares_update( + True, + test_set_recovery_threshold, + network, + args, + recovery_threshold=network.consortium.recovery_threshold - 1, + ) + + # Removing a recovery member now succeeds as threshold was reduced by 1 + LOG.info("Removing one recovery member") + assert_recovery_shares_update( + True, + test_remove_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, + ) + + # Removing the last recovery member also succeeds as there are owners and threshold is 1 + LOG.info("Removing the last recovery member when a recovery owner is present") + assert_recovery_shares_update( + True, + test_remove_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, + ) + + # Removing the only recovery owner when no other owner/participants exist should be impossible + LOG.info( + "Removing the only recovery owner when no other owner/participants exist should not be possible" + ) + try: + test_remove_member_no_reqs( + network, args, recovery_role=infra.member.RecoveryRole.Owner + ) + assert False, "Removing the recovery owner should not be possible" + except infra.proposal.ProposalNotAccepted as e: + # This is an apply() time failure, so the proposal remains Open + # since the last vote is effectively discarded + assert e.proposal.state == infra.proposal.ProposalState.OPEN + + assert ( + len(network.consortium.get_active_recovery_participants()) == 0 + ), f"Unexpected recovery members count: {len(network.consortium.get_active_recovery_participants())}" + + assert ( + len(network.consortium.get_active_recovery_owners()) == 1 + ), f"Unexpected recovery owners count: {len(network.consortium.get_active_recovery_owners())}" + + LOG.info("Set recovery threshold to 0 is impossible") + exception = infra.proposal.ProposalNotCreated + try: + test_set_recovery_threshold(network, args, recovery_threshold=0) + assert False, "Setting recovery threshold to 0 should not be possible" + except exception as e: + assert ( + e.response.status_code == 400 + and e.response.body.json()["error"]["code"] + == "ProposalFailedToValidate" + ), e.response.body.text() + + LOG.info( + "Set recovery threshold to more than 1 when only active recovery owners exist is impossible" + ) + try: + test_set_recovery_threshold( + network, + args, + recovery_threshold=2, + ) + assert ( + False + ), "Setting recovery threshold to more than 1 when only active recovery owners exist should not be possible" + except infra.proposal.ProposalNotAccepted as e: + # This is an apply() time failure, so the proposal remains Open + # since the last vote is effectively discarded + assert e.proposal.state == infra.proposal.ProposalState.OPEN + + try: + test_set_recovery_threshold(network, args, recovery_threshold=256) + assert False, "Recovery threshold cannot be set to > 255" + except exception as e: + assert ( + e.response.status_code == 400 + and e.response.body.json()["error"]["code"] + == "ProposalFailedToValidate" + ), e.response.body.text() + + primary, _ = network.find_primary() + try: + network.consortium.set_recovery_threshold(primary, recovery_threshold=None) + assert False, "Recovery threshold value must be passed as proposal argument" + except exception as e: + assert ( + e.response.status_code == 400 + and e.response.body.json()["error"]["code"] + == "ProposalFailedToValidate" + ), e.response.body.text() + + LOG.info( + "Setting recovery threshold to current threshold of 1 does not update shares" + ) + assert_recovery_shares_update( + False, + test_set_recovery_threshold, + network, + args, + recovery_threshold=1, + ) + + LOG.info("Adding two recovery participant members") + assert_recovery_shares_update( + True, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, + ) + assert_recovery_shares_update( + True, + test_add_member, + network, + args, + recovery_role=infra.member.RecoveryRole.Participant, + ) + + assert ( + len(network.consortium.get_active_recovery_participants()) == 2 + ), f"Unexpected recovery members count: {len(network.consortium.get_active_recovery_participants())}" + + assert ( + len(network.consortium.get_active_recovery_owners()) == 1 + ), f"Unexpected recovery owners count: {len(network.consortium.get_active_recovery_owners())}" + + LOG.info("Setting recovery threshold to 2 should now be possible") + assert_recovery_shares_update( + True, + test_set_recovery_threshold, + network, + args, + recovery_threshold=2, + ) + + def run(args): service_startups(args) recovery_shares_scenario(args) + recovery_shares_with_owners_scenario(args) if __name__ == "__main__": diff --git a/tests/recovery.py b/tests/recovery.py index 48ec2b01312..d59a7a371c0 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the Apache 2.0 License. import infra.e2e_args +import infra.member import infra.network import infra.node import infra.logging_app as app @@ -108,7 +109,9 @@ def verify_endorsements_chain(primary, endorsements, pubkey): @reqs.description("Recover a service") @reqs.recover(number_txs=2) -def test_recover_service(network, args, from_snapshot=True, no_ledger=False): +def test_recover_service( + network, args, from_snapshot=True, no_ledger=False, via_recovery_owner=False +): network.save_service_identity(args) old_primary, _ = network.find_primary() @@ -199,7 +202,7 @@ def test_recover_service(network, args, from_snapshot=True, no_ledger=False): r = c.get("/node/ready/app") assert r.status_code == http.HTTPStatus.SERVICE_UNAVAILABLE.value, r - recovered_network.recover(args) + recovered_network.recover(args, via_recovery_owner=via_recovery_owner) LOG.info("Check that new service view is as expected") new_primary, _ = recovered_network.find_primary() @@ -996,6 +999,67 @@ def run_recover_snapshot_alone(args): return network +def run_recover_via_initial_recovery_owner(args): + """ + Recover a service using the recovery owner added as part of service creation, without requiring any other recovery members to participate. + """ + txs = app.LoggingTxs("user0") + args.initial_member_count = 4 + args.initial_recovery_participant_count = 3 + args.initial_recovery_owner_count = 1 + with infra.network.network( + args.nodes, + args.binary_dir, + args.debug_nodes, + args.perf_nodes, + pdb=args.pdb, + txs=txs, + ) as network: + network.start_and_open(args) + # Recover service using recovery owner and participants + network = test_recover_service( + network, args, from_snapshot=True, via_recovery_owner=True + ) + network = test_recover_service(network, args, from_snapshot=True) + return network + + +def run_recover_via_added_recovery_owner(args): + """ + Recover a service using the recovery owner added after opening the service, without requiring any other recovery members to participate. + """ + txs = app.LoggingTxs("user0") + args.initial_recovery_participant_count = 2 + args.initial_recovery_owner_count = 0 + with infra.network.network( + args.nodes, + args.binary_dir, + args.debug_nodes, + args.perf_nodes, + pdb=args.pdb, + txs=txs, + ) as network: + network.start_and_open(args) + primary, _ = network.find_primary() + + # Add a recovery owner after opening the network + recovery_owner = network.consortium.generate_and_add_new_member( + primary, + curve=args.participants_curve, + recovery_role=infra.member.RecoveryRole.Owner, + ) + r = recovery_owner.ack(primary) + with primary.client() as nc: + nc.wait_for_commit(r) + + # Recover service using recovery owner and participants + network = test_recover_service( + network, args, from_snapshot=True, via_recovery_owner=True + ) + network = test_recover_service(network, args, from_snapshot=True) + return network + + if __name__ == "__main__": def add(parser): @@ -1052,4 +1116,18 @@ def add(parser): nodes=infra.e2e_args.min_nodes(cr.args, f=0), # 1 node suffices for recovery ) + cr.add( + "recovery_via_initial_recovery_owner", + run_recover_via_initial_recovery_owner, + package="samples/apps/logging/liblogging", + nodes=infra.e2e_args.min_nodes(cr.args, f=0), # 1 node suffices for recovery + ) + + cr.add( + "recovery_via_added_recovery_owner", + run_recover_via_added_recovery_owner, + package="samples/apps/logging/liblogging", + nodes=infra.e2e_args.min_nodes(cr.args, f=0), # 1 node suffices for recovery + ) + cr.run() diff --git a/tests/suite/test_requirements.py b/tests/suite/test_requirements.py index b7c3f21391d..01e7d3ab93f 100644 --- a/tests/suite/test_requirements.py +++ b/tests/suite/test_requirements.py @@ -5,6 +5,7 @@ from infra.snp import IS_SNP from loguru import logger as LOG +from infra.member import RecoveryRole class TestRequirementsNotMet(Exception): @@ -97,14 +98,20 @@ def check(network, args, *nargs, **kwargs): def sufficient_recovery_member_count(): - def check(network, args, recovery_member=True, *nargs, **kwargs): - if recovery_member and ( - len(network.consortium.get_active_recovery_members()) + def check( + network, + args, + recovery_role=RecoveryRole.Participant, + *nargs, + **kwargs, + ): + if recovery_role == RecoveryRole.Participant and ( + len(network.consortium.get_active_recovery_participants()) <= network.consortium.recovery_threshold ): raise TestRequirementsNotMet( "Cannot remove recovery member since number of active recovery members" - f" ({len(network.consortium.get_active_recovery_members()) - 1}) would be less than" + f" ({len(network.consortium.get_active_recovery_participants()) - 1}) would be less than" f" the recovery threshold ({network.consortium.recovery_threshold})" )