From 7546e41d8a25cb168599a53b6e0137540ec0feb9 Mon Sep 17 00:00:00 2001 From: Gaurav Sinha <39692417+gaurav137@users.noreply.github.com> Date: Sun, 15 Dec 2024 13:51:00 +0000 Subject: [PATCH] Update --- doc/host_config_schema/cchost_config.json | 8 ++-- doc/schemas/gov_openapi.json | 12 ++++- include/ccf/service/tables/members.h | 38 +++++++++------ samples/constitutions/default/actions.js | 40 +++++++++------- src/host/configuration.h | 4 +- src/host/main.cpp | 24 ++++------ src/node/gov/handlers/service_state.h | 19 ++++++++ src/service/internal_tables_access.h | 57 ++++++++++++++++------- tests/infra/consortium.py | 2 +- tests/infra/member.py | 4 +- 10 files changed, 133 insertions(+), 75 deletions(-) diff --git a/doc/host_config_schema/cchost_config.json b/doc/host_config_schema/cchost_config.json index 61d71d2385cc..b4356fef495c 100644 --- a/doc/host_config_schema/cchost_config.json +++ b/doc/host_config_schema/cchost_config.json @@ -326,10 +326,10 @@ "type": ["string", "null"], "description": "Path to member data file (JSON)" }, - "recovery_owner": { - "type": "boolean", - "default": false, - "description": "Whether the member acts as a recovery owner and gets assigned the full recovery share" + "recovery_role": { + "type": "string", + "enum": ["NonParticipant", "Participant", "Owner"], + "description": "Whether the member acts as a recovery participant and gets assigned a single share or as an owner and gets assigned the full recovery share" } }, "required": ["certificate_file"], diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index 70ca5ad390f6..f6880c01e9df 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -344,8 +344,8 @@ "member_data": { "$ref": "#/components/schemas/json" }, - "recovery_owner": { - "$ref": "#/components/schemas/boolean" + "recovery_role": { + "$ref": "#/components/schemas/MemberRecoveryRole" }, "status": { "$ref": "#/components/schemas/MemberStatus" @@ -415,6 +415,14 @@ }, "type": "object" }, + "MemberRecoveryRole": { + "enum": [ + "NonParticipant", + "Participant", + "Owner" + ], + "type": "string" + }, "MemberStatus": { "enum": [ "Accepted", diff --git a/include/ccf/service/tables/members.h b/include/ccf/service/tables/members.h index c8fe91adb870..b3c86d45eed7 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, + Participant, + + /** If set then the member is to receive a full share ("super-share") + 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 @@ -36,10 +52,7 @@ namespace ccf std::optional encryption_pub_key = std::nullopt; nlohmann::json member_data = nullptr; - /** If set then the member is to receive a full share ("super-share") - allowing it to single-handedly recover the network without - requiring any other recovery member to submit their shares. */ - std::optional recovery_owner = std::nullopt; + std::optional recovery_role = std::nullopt; NewMember() {} @@ -47,23 +60,23 @@ namespace ccf const ccf::crypto::Pem& cert_, const std::optional& encryption_pub_key_ = std::nullopt, const nlohmann::json& member_data_ = nullptr, - const std::optional& recovery_owner_ = std::nullopt) : + const std::optional& recovery_role_ = std::nullopt) : cert(cert_), encryption_pub_key(encryption_pub_key_), member_data(member_data_), - recovery_owner(recovery_owner_) + 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 && recovery_owner == rhs.recovery_owner; + 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, recovery_owner) + NewMember, encryption_pub_key, member_data, recovery_role) struct MemberDetails { @@ -73,20 +86,17 @@ namespace ccf members for example. */ nlohmann::json member_data = nullptr; - /** If set then the member is to receive a full share ("super-share") - allowing it to single-handedly recover the network without - requiring any other recovery member to submit their shares. */ - std::optional recovery_owner = std::nullopt; + std::optional recovery_role = std::nullopt; bool operator==(const MemberDetails& rhs) const { return status == rhs.status && member_data == rhs.member_data && - recovery_owner == rhs.recovery_owner; + 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, recovery_owner) + 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 85f00b949ae2..fd260f6c28b4 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -166,8 +166,9 @@ function invalidateOtherOpenProposals(proposalIdToRetain) { } function setServiceCertificateValidityPeriod(validFrom, validityPeriodDays) { - const rawConfig = - ccf.kv["public:ccf.gov.service.config"].get(getSingletonKvKey()); + const rawConfig = ccf.kv["public:ccf.gov.service.config"].get( + getSingletonKvKey(), + ); if (rawConfig === undefined) { throw new Error("Service configuration could not be found"); } @@ -216,8 +217,9 @@ function setNodeCertificateValidityPeriod( throw new Error(`Node ${nodeId} has no certificate signing request`); } - const rawConfig = - ccf.kv["public:ccf.gov.service.config"].get(getSingletonKvKey()); + const rawConfig = ccf.kv["public:ccf.gov.service.config"].get( + getSingletonKvKey(), + ); if (rawConfig === undefined) { throw new Error("Service configuration could not be found"); } @@ -359,16 +361,16 @@ const actions = new Map([ function (args) { checkX509CertBundle(args.cert, "cert"); checkType(args.member_data, "object?", "member_data"); - checkType(args.recovery_owner, "boolean?", "recovery_owner"); + checkType(args.recovery_role, "string?", "recovery_role"); if ( args.encryption_pub_key == null && args.encryption_pub_key == undefined && - args.recovery_owner !== null && - args.recovery_owner !== undefined + args.recovery_role !== null && + args.recovery_role !== undefined ) { throw new Error( - "Cannot specify a recovery_owner value when encryption_pub_key is not specified", + "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 @@ -400,15 +402,16 @@ const actions = new Map([ let member_info = {}; member_info.member_data = args.member_data; - member_info.recovery_owner = args.recovery_owner; + member_info.recovery_role = args.recovery_role; member_info.status = "Accepted"; ccf.kv["public:ccf.gov.members.info"].set( rawMemberId, ccf.jsonCompatibleToBuf(member_info), ); - const rawSignature = - ccf.kv["public:ccf.internal.signatures"].get(getSingletonKvKey()); + const rawSignature = ccf.kv["public:ccf.internal.signatures"].get( + getSingletonKvKey(), + ); if (rawSignature === undefined) { ccf.kv["public:ccf.gov.members.acks"].set(rawMemberId); } else { @@ -450,8 +453,9 @@ const actions = new Map([ // would still be a sufficient number of recovery members left // to recover the service if (isActiveMember && isRecoveryMember) { - const rawConfig = - ccf.kv["public:ccf.gov.service.config"].get(getSingletonKvKey()); + const rawConfig = ccf.kv["public:ccf.gov.service.config"].get( + getSingletonKvKey(), + ); if (rawConfig === undefined) { throw new Error("Service configuration could not be found"); } @@ -1189,8 +1193,9 @@ const actions = new Map([ } }, function (args) { - const rawConfig = - ccf.kv["public:ccf.gov.service.config"].get(getSingletonKvKey()); + const rawConfig = ccf.kv["public:ccf.gov.service.config"].get( + getSingletonKvKey(), + ); if (rawConfig === undefined) { throw new Error("Service configuration could not be found"); } @@ -1273,8 +1278,9 @@ const actions = new Map([ checkEntityId(args.node_id, "node_id"); }, function (args) { - const rawConfig = - ccf.kv["public:ccf.gov.service.config"].get(getSingletonKvKey()); + const rawConfig = ccf.kv["public:ccf.gov.service.config"].get( + getSingletonKvKey(), + ); if (rawConfig === undefined) { throw new Error("Service configuration could not be found"); } diff --git a/src/host/configuration.h b/src/host/configuration.h index 26b3cec1a5a9..1aaac10a9c2d 100644 --- a/src/host/configuration.h +++ b/src/host/configuration.h @@ -48,7 +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_owner = std::nullopt; + std::optional recovery_role = std::nullopt; bool operator==(const ParsedMemberInfo& other) const = default; }; @@ -59,7 +59,7 @@ namespace host ParsedMemberInfo, encryption_public_key_file, data_json_file, - recovery_owner); + recovery_role); struct CCHostConfig : public ccf::CCFConfig { diff --git a/src/host/main.cpp b/src/host/main.cpp index c1290ee9f76d..339d7291776f 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -216,18 +216,6 @@ int main(int argc, char** argv) config.ledger.directory)); } - for (auto const& m : config.command.start.members) - { - if ( - !m.encryption_public_key_file.has_value() && - m.recovery_owner.has_value()) - { - throw std::logic_error(fmt::format( - "No public encryption key has been specified but recovery owner " - "value has been set for a member")); - } - } - // Count members with public encryption key who are not recovery // owners as only these members will be handed a recovery share // that accrues towards the recovery threshold. @@ -240,7 +228,8 @@ int main(int argc, char** argv) { if ( m.encryption_public_key_file.has_value() && - (!m.recovery_owner.has_value() || !m.recovery_owner.value())) + m.recovery_role.value_or(ccf::MemberRecoveryRole::Participant) != + ccf::MemberRecoveryRole::Owner) { members_with_pubk_count++; } @@ -620,14 +609,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_owner = 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())); - recovery_owner = m.recovery_owner; + if (m.recovery_role.has_value()) + { + recovery_role = m.recovery_role.value(); + } } nlohmann::json md = nullptr; @@ -640,7 +632,7 @@ int main(int argc, char** argv) ccf::crypto::Pem(files::slurp(m.certificate_file)), public_encryption_key, md, - recovery_owner); + recovery_role); } startup_config.start.constitution = ""; for (const auto& constitution_path : diff --git a/src/node/gov/handlers/service_state.h b/src/node/gov/handlers/service_state.h index c941e14f960a..b1338fc9befe 100644 --- a/src/node/gov/handlers/service_state.h +++ b/src/node/gov/handlers/service_state.h @@ -35,6 +35,25 @@ namespace ccf::gov::endpoints member["publicEncryptionKey"] = enc_key.value().str(); } + ccf::MemberRecoveryRole recovery_role; + 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; + } + else + { + recovery_role = ccf::MemberRecoveryRole::NonParticipant; + } + } + + member["recoveryRole"] = recovery_role; + return member; } diff --git a/src/service/internal_tables_access.h b/src/service/internal_tables_access.h index afde15579268..300cb992d432 100644 --- a/src/service/internal_tables_access.h +++ b/src/service/internal_tables_access.h @@ -96,7 +96,8 @@ namespace ccf return false; } - return mi->recovery_owner.value_or(false); + return mi->recovery_role.has_value() && + mi->recovery_role.value() == MemberRecoveryRole::Owner; } static bool is_active_member( @@ -134,7 +135,8 @@ namespace ccf if ( info->status == MemberStatus::ACTIVE && - !info->recovery_owner.value_or(false)) + info->recovery_role.value_or(MemberRecoveryRole::Participant) != + MemberRecoveryRole::Owner) { active_recovery_members[mid] = pem; } @@ -165,7 +167,8 @@ namespace ccf if ( info->status == MemberStatus::ACTIVE && - info->recovery_owner.value_or(false)) + info->recovery_role.value_or(MemberRecoveryRole::Participant) == + MemberRecoveryRole::Owner) { active_recovery_owners[mid] = pem; } @@ -193,14 +196,34 @@ namespace ccf return id; } - if ( - !member_pub_info.encryption_pub_key.has_value() && - member_pub_info.recovery_owner.has_value()) + if (member_pub_info.recovery_role.has_value()) { - throw std::logic_error(fmt::format( - "Member {} cannot be added as recovery_owner has a value set but no " - "encryption public key is specified", - id)); + 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); @@ -208,7 +231,7 @@ namespace ccf id, {MemberStatus::ACCEPTED, member_pub_info.member_data, - member_pub_info.recovery_owner}); + member_pub_info.recovery_role}); if (member_pub_info.encryption_pub_key.has_value()) { @@ -288,8 +311,8 @@ namespace ccf 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) + // 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); @@ -384,8 +407,8 @@ namespace ccf } else if (ni.status == ccf::NodeStatus::RETIRED) { - // If a node is retired, but knowledge of their retirement has not yet - // been globally committed, they are still considered active. + // If a node is retired, but knowledge of their retirement has not + // yet been globally committed, they are still considered active. auto cni = nodes->get_globally_committed(nid); if (cni.has_value() && !cni->retired_committed) { @@ -760,8 +783,8 @@ namespace ccf if (service_status.value() == ServiceStatus::WAITING_FOR_RECOVERY_SHARES) { // While waiting for recovery shares, the recovery threshold cannot be - // modified. Otherwise, the threshold could be passed without triggering - // the end of recovery procedure + // modified. Otherwise, the threshold could be passed without + // triggering the end of recovery procedure LOG_FAIL_FMT( "Cannot set recovery threshold: service is currently waiting for " "recovery shares"); diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index e3a96194fe70..ff4fd9ccbc4a 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -234,7 +234,7 @@ def generate_and_propose_new_member( else None ), member_data=member_data, - recovery_owner=recovery_owner if recovery_member else None, + recovery_role="Owner" if recovery_owner and recovery_member else None, ) proposal = self.get_any_active_member().propose(remote_node, proposal_body) diff --git a/tests/infra/member.py b/tests/infra/member.py index 5cb5331f681b..25dbf70adb12 100644 --- a/tests/infra/member.py +++ b/tests/infra/member.py @@ -305,8 +305,8 @@ def __init__( self.member_info["data_json_file"] = ( f"{self.local_id}_data.json" if member_data else None ) - if is_recovery_owner is not None: - self.member_info["recovery_owner"] = is_recovery_owner + if is_recovery_owner is True: + self.member_info["recovery_role"] = "Owner" if key_generator is not None: key_generator_args = [