diff --git a/CHANGELOG.md b/CHANGELOG.md index 37063c3848b..4842e2920cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. [7.0.19]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.19 +### Added + +- The `transition_service_to_open_with_signing_keys` proposal action opens a service with `previous_service_signing_keys` and `next_service_signing_keys`, maps of identity types to PEM public keys, instead of certificates. The public header `ccf/service_signing_keys.h` declares `ccf::ServiceSigningKeys` and `ccf::SigningKeyType`. See [accepting recovery](https://microsoft.github.io/CCF/main/governance/accept_recovery.html) (#8477). + +### Deprecated + +- The `previous_service_identity` argument of the `transition_service_to_open` proposal and the `command.recover.previous_service_identity_file` configuration option are deprecated in favour of the `transition_service_to_open_with_signing_keys` proposal and `command.recover.previous_service_signing_key_files` respectively. In C++, `ccf::CCFConfig::Command::Recover::previous_service_identity_file` is now `std::optional` (#8477). + ### Fixed - Paused RPC reads now resume when another interface releases the inbound budget, even if older libuv versions coalesce the notification. Previously, reads could remain paused until an unrelated event triggered a recheck (#8498). diff --git a/doc/governance/accept_recovery.rst b/doc/governance/accept_recovery.rst index 5ee0e6ce09d..a69ee943c56 100644 --- a/doc/governance/accept_recovery.rst +++ b/doc/governance/accept_recovery.rst @@ -84,9 +84,33 @@ A member proposes to recover the network and other members can vote on the propo Once the proposal to recover the network has passed under the rules of the :term:`Constitution`, the recovered service is ready for members to submit their recovery shares. -Note that the ``transition_service_to_open`` proposal takes two parameters: the previous and the next :term:`Service Identity` (X.509 certificates in PEM format). The previous identity must match the identity supplied to the recovery node at startup, while the next identity must match the recovered service's newly generated identity. Snapshot validation is performed earlier at node startup using the configured previous service identity. Since both identities are recorded on the ledger with the proposal, it is always clear at which point the identity changed. +Members can open the recovered service with either of two proposals. ``transition_service_to_open`` takes ``previous_service_identity`` and ``next_service_identity``, PEM certificates. ``transition_service_to_open_with_signing_keys`` takes ``previous_service_signing_keys`` and ``next_service_signing_keys``, JSON objects mapping identity types to PEM public keys, and does not accept certificates. The previous and next values must match the previous service identity recorded in the ledger and the recovered service, respectively. Each key map must contain a ``CLASSICAL`` key, and only ``CLASSICAL`` keys are compared. -.. note:: The ``previous_service_identity`` argument to the ``transition_service_to_open`` proposal is required for recovery, but must not be provided when opening a new service as there is no previous identity. +The ``previous_service_identity`` argument is deprecated, so recovery proposals should use ``transition_service_to_open_with_signing_keys``. These identities are recorded on the ledger with the proposal. + +Each service writes its signing keys to the files configured by ``command.service_signing_key_files``, by default ``service_signing_key_classical.pem``. A ``transition_service_to_open_with_signing_keys`` proposal uses the previous service's file and the recovered service's file: + +.. code-block:: json + + { + "actions": [ + { + "name": "transition_service_to_open_with_signing_keys", + "args": { + "previous_service_signing_keys": { + "CLASSICAL": "-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----\n" + }, + "next_service_signing_keys": { + "CLASSICAL": "-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----\n" + } + } + } + ] + } + +Snapshot validation happens earlier at node startup. Operators must supply ``command.recover.previous_service_signing_key_files`` or the deprecated ``command.recover.previous_service_identity_file``. When key files are supplied, they are used for verification even if a certificate is also supplied, with no fallback to the certificate. The recovered service certificate inherits the subject of the previous certificate when ``command.recover.previous_service_identity_file`` is supplied. Otherwise, ``command.recover.service_cert_subject_name`` is required and sets the subject. See :doc:`/operations/configuration`. + +.. note:: Recovery proposals require previous signing keys or a previous service certificate. Neither previous-identity argument is needed when opening a new service. Submitting Recovery Shares -------------------------- diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 58b132b6bde..d43c5f16395 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -169,6 +169,20 @@ "type": "string", "default": "service_cert.pem", "description": "For ``Start`` and ``Recover`` nodes, path to which service certificate will be written to on startup. For ``Join`` nodes, path to the certificate of the existing service to join" + }, + "service_signing_key_files": { + "type": "object", + "default": { "CLASSICAL": "service_signing_key_classical.pem" }, + "properties": { + "CLASSICAL": { + "type": "string", + "minLength": 1, + "description": "Output path for the classical service signing public key (PEM)" + } + }, + "required": ["CLASSICAL"], + "additionalProperties": false, + "description": "For ``Start`` and ``Recover`` nodes, paths to which service signing public keys will be written on startup" } }, "allOf": [ @@ -364,15 +378,43 @@ "description": "Initial validity period (days) for service certificate", "minimum": 1 }, + "service_cert_subject_name": { + "type": "string", + "minLength": 1, + "description": "Subject name for the recovered service certificate when ``previous_service_identity_file`` is not set; otherwise the previous certificate's subject is inherited" + }, "previous_service_identity_file": { "type": "string", - "description": "Path to the previous service certificate (PEM) file" + "minLength": 1, + "description": "Deprecated. Path to the previous service certificate (PEM) file. Use ``previous_service_signing_key_files`` instead" + }, + "previous_service_signing_key_files": { + "type": "object", + "properties": { + "CLASSICAL": { + "type": "string", + "minLength": 1, + "description": "Path to the previous classical service signing public key (PEM)" + } + }, + "required": ["CLASSICAL"], + "additionalProperties": false, + "description": "Paths to the previous service signing public keys" } }, - "required": ["previous_service_identity_file"], + "anyOf": [ + { "required": ["previous_service_identity_file"] }, + { + "required": [ + "previous_service_signing_key_files", + "service_cert_subject_name" + ] + } + ], "additionalProperties": false } - } + }, + "required": ["recover"] } } ], diff --git a/include/ccf/node/configuration.h b/include/ccf/node/configuration.h index 4101d35c2ae..134ceccde47 100644 --- a/include/ccf/node/configuration.h +++ b/include/ccf/node/configuration.h @@ -15,7 +15,9 @@ #include "ccf/service/tables/host_data.h" #include "ccf/service/tables/members.h" #include "ccf/service/tables/self_healing_open.h" +#include "ccf/service_signing_keys.h" +#include #include #include #include @@ -225,6 +227,8 @@ namespace ccf { StartType type = StartType::Start; std::string service_certificate_file = "service_cert.pem"; + std::map service_signing_key_files = { + {SigningKeyType::CLASSICAL, "service_signing_key_classical.pem"}}; struct Start { @@ -258,7 +262,11 @@ namespace ccf struct Recover { size_t initial_service_certificate_validity_days = 1; - std::string previous_service_identity_file; + std::optional service_cert_subject_name = std::nullopt; + std::optional previous_service_identity_file = + std::nullopt; + std::optional> + previous_service_signing_key_files = std::nullopt; bool operator==(const Recover&) const = default; }; Recover recover = {}; @@ -412,12 +420,19 @@ namespace ccf DECLARE_JSON_OPTIONAL_FIELDS( CCFConfig::Command::Recover, initial_service_certificate_validity_days, - previous_service_identity_file); + service_cert_subject_name, + previous_service_identity_file, + previous_service_signing_key_files); DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::Command); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::Command, type); DECLARE_JSON_OPTIONAL_FIELDS( - CCFConfig::Command, service_certificate_file, start, join, recover); + CCFConfig::Command, + service_certificate_file, + service_signing_key_files, + start, + join, + recover); DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig, network, command); diff --git a/include/ccf/service_signing_keys.h b/include/ccf/service_signing_keys.h new file mode 100644 index 00000000000..b2342ba93dd --- /dev/null +++ b/include/ccf/service_signing_keys.h @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "ccf/crypto/pem.h" + +#include +#include + +namespace ccf +{ + struct SigningKeyType + { + static constexpr auto CLASSICAL = "CLASSICAL"; + }; + + using ServiceSigningKeys = std::map; +} diff --git a/samples/config/recover_config.json b/samples/config/recover_config.json index 7af4249c41c..a291059db90 100644 --- a/samples/config/recover_config.json +++ b/samples/config/recover_config.json @@ -19,9 +19,16 @@ "command": { "type": "Recover", "service_certificate_file": "service_cert.pem", + "service_signing_key_files": { + "CLASSICAL": "service_signing_key_classical.pem" + }, "recover": { "initial_service_certificate_validity_days": 1, - "previous_service_identity_file": "previous_service_cert.pem" + "service_cert_subject_name": "CN=A Sample CCF Service", + "previous_service_identity_file": "previous_service_cert.pem", + "previous_service_signing_key_files": { + "CLASSICAL": "previous_service_signing_key_classical.pem" + } } }, "ledger": { diff --git a/samples/config/start_config.json b/samples/config/start_config.json index 5ad54cd839f..b7bba712c98 100644 --- a/samples/config/start_config.json +++ b/samples/config/start_config.json @@ -28,6 +28,9 @@ "command": { "type": "Start", "service_certificate_file": "service_cert.pem", + "service_signing_key_files": { + "CLASSICAL": "service_signing_key_classical.pem" + }, "start": { "constitution_files": [ "validate.js", diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 521cde0bcc8..4c80785cd18 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -414,6 +414,17 @@ function checkX509CertBundle(value, field) { } } +function checkServiceSigningKeys(value, field) { + if (value === null || Array.isArray(value)) { + throw new Error(`${field} must be an object`); + } + checkType(value, "object", field); + checkType(value.CLASSICAL, "string", `${field}.CLASSICAL (PEM public key)`); + for (const [identityType, key] of Object.entries(value)) { + checkType(key, "string", `${field}.${identityType} (PEM public key)`); + } +} + function invalidateOtherOpenProposals(proposalIdToRetain) { const proposalsMap = ccf.kv["public:ccf.gov.proposals_info"]; proposalsMap.forEach((v, k) => { @@ -935,6 +946,63 @@ const actions = new Map([ }, ), ], + [ + "transition_service_to_open_with_signing_keys", + new Action( + function (args) { + if ( + args.previous_service_identity !== undefined || + args.next_service_identity !== undefined + ) { + throw new Error( + "Service certificates are not accepted, use transition_service_to_open instead", + ); + } + checkServiceSigningKeys( + args.next_service_signing_keys, + "next_service_signing_keys", + ); + if (args.previous_service_signing_keys !== undefined) { + checkServiceSigningKeys( + args.previous_service_signing_keys, + "previous_service_signing_keys", + ); + } + }, + + function (args) { + const service_info = "public:ccf.gov.service.info"; + const rawService = ccf.kv[service_info].get(getSingletonKvKey()); + if (rawService === undefined) { + throw new Error("Service information could not be found"); + } + + const service = ccf.bufToJsonCompatible(rawService); + + if ( + service.status === "Recovering" && + (args.previous_service_signing_keys === undefined || + args.next_service_signing_keys === undefined) + ) { + throw new Error( + `Opening a recovering network requires both, the previous and the next service signing keys`, + ); + } + + const previous_keys = + args.previous_service_signing_keys !== undefined + ? ccf.jsonCompatibleToBuf(args.previous_service_signing_keys) + : undefined; + const next_keys = ccf.jsonCompatibleToBuf( + args.next_service_signing_keys, + ); + ccf.node.transitionServiceToOpenWithSigningKeys( + previous_keys, + next_keys, + ); + }, + ), + ], [ "set_js_app", new Action( diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 6a0d563776d..e2688a3e619 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -394,6 +394,17 @@ function checkX509CertBundle(value, field) { } } +function checkServiceSigningKeys(value, field) { + if (value === null || Array.isArray(value)) { + throw new Error(`${field} must be an object`); + } + checkType(value, "object", field); + checkType(value.CLASSICAL, "string", `${field}.CLASSICAL (PEM public key)`); + for (const [identityType, key] of Object.entries(value)) { + checkType(key, "string", `${field}.${identityType} (PEM public key)`); + } +} + function invalidateOtherOpenProposals(proposalIdToRetain) { const proposalsMap = ccf.kv["public:ccf.gov.proposals_info"]; proposalsMap.forEach((v, k) => { @@ -914,6 +925,63 @@ const actions = new Map([ }, ), ], + [ + "transition_service_to_open_with_signing_keys", + new Action( + function (args) { + if ( + args.previous_service_identity !== undefined || + args.next_service_identity !== undefined + ) { + throw new Error( + "Service certificates are not accepted, use transition_service_to_open instead", + ); + } + checkServiceSigningKeys( + args.next_service_signing_keys, + "next_service_signing_keys", + ); + if (args.previous_service_signing_keys !== undefined) { + checkServiceSigningKeys( + args.previous_service_signing_keys, + "previous_service_signing_keys", + ); + } + }, + + function (args) { + const service_info = "public:ccf.gov.service.info"; + const rawService = ccf.kv[service_info].get(getSingletonKvKey()); + if (rawService === undefined) { + throw new Error("Service information could not be found"); + } + + const service = ccf.bufToJsonCompatible(rawService); + + if ( + service.status === "Recovering" && + (args.previous_service_signing_keys === undefined || + args.next_service_signing_keys === undefined) + ) { + throw new Error( + `Opening a recovering network requires both, the previous and the next service signing keys`, + ); + } + + const previous_keys = + args.previous_service_signing_keys !== undefined + ? ccf.jsonCompatibleToBuf(args.previous_service_signing_keys) + : undefined; + const next_keys = ccf.jsonCompatibleToBuf( + args.next_service_signing_keys, + ); + ccf.node.transitionServiceToOpenWithSigningKeys( + previous_keys, + next_keys, + ); + }, + ), + ], [ "set_js_app", new Action( diff --git a/src/crypto/openssl/verifier.cpp b/src/crypto/openssl/verifier.cpp index 8b6a893a924..5a5562d8345 100644 --- a/src/crypto/openssl/verifier.cpp +++ b/src/crypto/openssl/verifier.cpp @@ -182,6 +182,27 @@ namespace ccf::crypto return valid; } + bool Verifier_OpenSSL::verify_certificate_signature( + const Pem& signing_key) const + { + Unique_BIO key_bio(signing_key); + Unique_PKEY key(key_bio); + const auto rc = X509_verify(cert, key); + if (rc < 0) + { + throw std::runtime_error(fmt::format( + "OpenSSL certificate signature verification error: {}", + OpenSSL::first_error())); + } + if (rc == 0) + { + LOG_DEBUG_FMT( + "Certificate signature does not match the trusted public key: {}", + OpenSSL::first_error()); + } + return rc == 1; + } + bool Verifier_OpenSSL::is_self_signed() const { return (X509_get_extension_flags(cert) & EXFLAG_SS) != 0U; diff --git a/src/crypto/openssl/verifier.h b/src/crypto/openssl/verifier.h index 5941b642284..e8c0b0591a6 100644 --- a/src/crypto/openssl/verifier.h +++ b/src/crypto/openssl/verifier.h @@ -29,6 +29,10 @@ namespace ccf::crypto const std::vector& chain = {}, bool ignore_time = false) override; + // Verifies only the certificate signature, with a trusted public key + [[nodiscard]] bool verify_certificate_signature( + const Pem& signing_key) const; + bool is_self_signed() const override; std::string serial_number() const override; diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index c8cb8a7d902..09da19464d3 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -277,6 +277,26 @@ TEST_CASE("Verifier rejects unsupported public key type") make_verifier(cert_pem), "unsupported public key type", std::logic_error); } +TEST_CASE("Verifier checks a certificate signature with a trusted public key") +{ + const auto issuer = make_ec_key_pair(); + const auto issuer_cert = generate_self_signed_cert(issuer, "CN=issuer"); + const auto subject = make_ec_key_pair(); + const auto cert = create_endorsed_cert( + subject->public_key_pem(), + "CN=subject", + {}, + make_verifier(issuer_cert)->validity_period(), + issuer->private_key_pem(), + issuer_cert); + const Verifier_OpenSSL verifier(cert.raw()); + + CHECK(verifier.verify_certificate_signature(issuer->public_key_pem())); + CHECK_FALSE(verifier.verify_certificate_signature(subject->public_key_pem())); + CHECK_THROWS( + std::ignore = verifier.verify_certificate_signature(issuer_cert)); +} + TEST_CASE("Private PEM imports enforce key family") { const auto ec = make_ec_key_pair(); diff --git a/src/enclave/enclave.h b/src/enclave/enclave.h index 300912d9abb..572652756ff 100644 --- a/src/enclave/enclave.h +++ b/src/enclave/enclave.h @@ -212,6 +212,7 @@ namespace ccf ccf::CCFConfig ccf_config_, std::vector& node_cert, std::vector& service_cert, + ServiceSigningKeys& service_signing_keys, std::vector& rpc_addresses) { start_type = start_type_; @@ -353,6 +354,7 @@ namespace ccf // When starting a node in start or recover modes, fresh network secrets // are created and the associated certificate can be passed to the host service_cert = create_info.service_cert.raw(); + service_signing_keys = std::move(create_info.service_signing_keys); } return CreateNodeStatus::OK; diff --git a/src/enclave/entry_points.h b/src/enclave/entry_points.h index d0309ddb219..c3905c489fc 100644 --- a/src/enclave/entry_points.h +++ b/src/enclave/entry_points.h @@ -4,6 +4,7 @@ #include "ccf/node/configuration.h" #include "ccf/node/start_type.h" +#include "ccf/service_signing_keys.h" #include "common/configuration.h" #include "common/enclave_interface_types.h" #include "ds/work_beacon.h" @@ -22,6 +23,7 @@ namespace ccf const ccf::CCFConfig& ccf_config, std::vector& node_cert, std::vector& service_cert, + ServiceSigningKeys& service_signing_keys, std::vector& rpc_addresses, StartType start_type, ccf::LoggerLevel log_level, diff --git a/src/enclave/main.cpp b/src/enclave/main.cpp index 2cc9e1fd811..df94beb5297 100644 --- a/src/enclave/main.cpp +++ b/src/enclave/main.cpp @@ -30,6 +30,7 @@ namespace ccf const ccf::CCFConfig& ccf_config, std::vector& node_cert, std::vector& service_cert, + ServiceSigningKeys& service_signing_keys, std::vector& rpc_addresses, StartType start_type, ccf::LoggerLevel log_level, @@ -145,7 +146,12 @@ namespace ccf try { status = enclave->create_new_node( - start_type, ccf_config, node_cert, service_cert, rpc_addresses); + start_type, + ccf_config, + node_cert, + service_cert, + service_signing_keys, + rpc_addresses); } catch (...) { diff --git a/src/host/run.cpp b/src/host/run.cpp index 433b65ecf52..23ad548a9eb 100644 --- a/src/host/run.cpp +++ b/src/host/run.cpp @@ -226,6 +226,7 @@ namespace ccf EnclaveConfig& enclave_config, std::vector& node_cert, std::vector& service_cert, + ServiceSigningKeys& service_signing_keys, std::vector& rpc_addresses, ccf::LoggerLevel log_level, ringbuffer::NotifyingWriterFactory& notifying_factory, @@ -249,6 +250,7 @@ namespace ccf config, node_cert, service_cert, + service_signing_keys, rpc_addresses, config.command.type, log_level, @@ -291,10 +293,11 @@ namespace ccf return std::nullopt; } - void write_certificates_to_disk( + void write_identity_files_to_disk( const ccf::CCFConfig& config, const std::vector& node_cert, - const std::vector& service_cert) + const std::vector& service_cert, + const ServiceSigningKeys& service_signing_keys) { // Write the node and service certs to disk. files::dump(node_cert, config.output_files.node_certificate_file); @@ -310,6 +313,14 @@ namespace ccf LOG_INFO_FMT( "Output service certificate to {}", config.command.service_certificate_file); + for (const auto& [identity_type, public_key] : service_signing_keys) + { + const auto& path = + config.command.service_signing_key_files.at(identity_type); + files::dump(public_key.raw(), path); + LOG_INFO_FMT( + "Output {} service signing public key to {}", identity_type, path); + } } } @@ -498,6 +509,7 @@ namespace ccf const size_t certificate_size = 4096; std::vector node_cert(certificate_size); std::vector service_cert(certificate_size); + ServiceSigningKeys service_signing_keys; std::vector rpc_addresses; if (ccf::pal::platform == ccf::pal::Platform::Virtual) @@ -552,6 +564,7 @@ namespace ccf enclave_config, node_cert, service_cert, + service_signing_keys, rpc_addresses, log_level, factories.notifying_factory, @@ -563,8 +576,8 @@ namespace ccf return enclave_creation_result; } - // Output certificates to disk - write_certificates_to_disk(config, node_cert, service_cert); + write_identity_files_to_disk( + config, node_cert, service_cert, service_signing_keys); // Run enclave threads and event loop run_enclave_threads(config, *runtime_control); diff --git a/src/node/gov/extensions/node.cpp b/src/node/gov/extensions/node.cpp index 52f3b2bba3e..4ea6b6ecc41 100644 --- a/src/node/gov/extensions/node.cpp +++ b/src/node/gov/extensions/node.cpp @@ -78,6 +78,27 @@ namespace ccf::js::extensions return ccf::js::core::constants::Undefined; } + // Returns false if value is neither undefined nor an array buffer + bool get_service_signing_keys( + JSContext* ctx, + JSValueConst value, + std::optional& keys) + { + if (JS_IsUndefined(value) != 0) + { + return true; + } + size_t size = 0; + const auto* bytes = JS_GetArrayBuffer(ctx, &size, value); + if (bytes == nullptr) + { + return false; + } + keys = + ccf::parse_json_safe(bytes, bytes + size).get(); + return true; + } + JSValue js_node_transition_service_to_open( JSContext* ctx, [[maybe_unused]] JSValueConst this_val, @@ -147,7 +168,78 @@ namespace ccf::js::extensions } identities.next = ccf::crypto::Pem(next_bytes, next_bytes_sz); - GOV_DEBUG_FMT("next service identity: {}", identities.next.str()); + GOV_DEBUG_FMT("next service identity: {}", identities.next->str()); + + gov_effects->transition_service_to_open(*tx_ptr, identities); + } + catch (const std::exception& e) + { + GOV_FAIL_FMT("Unable to open service: {}", e.what()); + return JS_ThrowInternalError( + ctx, "Unable to open service: %s", e.what()); + } + + return ccf::js::core::constants::Undefined; + } + + JSValue js_node_transition_service_to_open_with_signing_keys( + JSContext* ctx, + [[maybe_unused]] JSValueConst this_val, + int argc, + [[maybe_unused]] JSValueConst* argv) + { + js::core::Context& jsctx = + *reinterpret_cast(JS_GetContextOpaque(ctx)); + + if (argc != 2) + { + return JS_ThrowTypeError( + ctx, "Passed %d arguments but expected two", argc); + } + + auto* extension = jsctx.get_extension(); + if (extension == nullptr) + { + return JS_ThrowInternalError(ctx, "Failed to get extension object"); + } + + auto* gov_effects = extension->gov_effects; + if (gov_effects == nullptr) + { + return JS_ThrowInternalError( + ctx, "Failed to get governance effects object"); + } + + auto* tx_ptr = extension->tx; + if (tx_ptr == nullptr) + { + return JS_ThrowInternalError(ctx, "Failed to get tx object"); + } + + try + { + AbstractGovernanceEffects::ServiceIdentities identities; + + if (!get_service_signing_keys( + ctx, argv[0], identities.previous_signing_keys)) + { + return JS_ThrowTypeError( + ctx, + "Previous service signing keys argument is not an array buffer"); + } + + if (JS_IsUndefined(argv[1]) != 0) + { + return JS_ThrowInternalError( + ctx, "Proposal requires the next service signing keys"); + } + + if (!get_service_signing_keys( + ctx, argv[1], identities.next_signing_keys)) + { + return JS_ThrowTypeError( + ctx, "Next service signing keys argument is not an array buffer"); + } gov_effects->transition_service_to_open(*tx_ptr, identities); } @@ -362,6 +454,12 @@ namespace ccf::js::extensions "transitionServiceToOpen", ctx.new_c_function( js_node_transition_service_to_open, "transitionServiceToOpen", 2))); + JS_CHECK_OR_THROW(node.set( + "transitionServiceToOpenWithSigningKeys", + ctx.new_c_function( + js_node_transition_service_to_open_with_signing_keys, + "transitionServiceToOpenWithSigningKeys", + 2))); JS_CHECK_OR_THROW(node.set( "triggerRecoverySharesRefresh", ctx.new_c_function( diff --git a/src/node/gov/extensions/node.h b/src/node/gov/extensions/node.h index 9f6099053f1..4e71c58d31a 100644 --- a/src/node/gov/extensions/node.h +++ b/src/node/gov/extensions/node.h @@ -12,6 +12,7 @@ namespace ccf::js::extensions * * - ccf.node.triggerLedgerRekey * - ccf.node.transitionServiceToOpen + * - ccf.node.transitionServiceToOpenWithSigningKeys * - ccf.node.triggerRecoverySharesRefresh * - ccf.node.triggerLedgerChunk * - ccf.node.triggerSnapshot diff --git a/src/node/identity.h b/src/node/identity.h index 54fca5ae1aa..803f40a972e 100644 --- a/src/node/identity.h +++ b/src/node/identity.h @@ -5,15 +5,43 @@ #include "ccf/cose_signatures_config.h" #include "ccf/crypto/curve.h" #include "ccf/crypto/verifier.h" +#include "ccf/service_signing_keys.h" #include "crypto/certs.h" #include "crypto/openssl/ec_key_pair.h" +#include #include +#include +#include #include #include namespace ccf { + inline ccf::crypto::ECPublicKeyPtr get_previous_service_classical_signing_key( + const std::optional& keys, + const std::optional>& certificate) + { + if (keys.has_value()) + { + if (!keys->contains(SigningKeyType::CLASSICAL)) + { + throw std::logic_error(fmt::format( + "Missing {} previous service signing public key", + SigningKeyType::CLASSICAL)); + } + return ccf::crypto::make_ec_public_key( + keys->at(SigningKeyType::CLASSICAL)); + } + + if (!certificate.has_value()) + { + throw std::logic_error("No previous service identity is configured"); + } + return ccf::crypto::make_ec_public_key( + ccf::crypto::make_unique_verifier(*certificate)->public_key_der()); + } + struct NetworkIdentity { ccf::crypto::Pem priv_key; diff --git a/src/node/node_state.h b/src/node/node_state.h index c8f388c6bbb..087a4f75272 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -22,6 +22,7 @@ #include "ccf/service/node_info_network.h" #include "ccf/service/tables/self_healing_open.h" #include "ccf/service/tables/service.h" +#include "ccf/service_signing_keys.h" #include "ccf/tx.h" #include "consensus/aft/raft.h" #include "consensus/ledger_enclave.h" @@ -97,6 +98,7 @@ namespace ccf { ccf::crypto::Pem self_signed_node_cert; ccf::crypto::Pem service_cert; + ServiceSigningKeys service_signing_keys; }; inline void reset_data(std::vector& data) @@ -564,13 +566,10 @@ namespace ccf void verify_recovery_snapshot_candidate_unsafe( const SnapshotSegments& segments, ccf::kv::Version snapshot_seqno) { - if (!startup_inputs.previous_service_identity.has_value()) - { - throw std::logic_error("No previous service identity is configured"); - } - - const ccf::crypto::Pem target_identity( - *startup_inputs.previous_service_identity); + const auto target_key = get_previous_service_classical_signing_key( + startup_inputs.previous_service_signing_keys, + startup_inputs.previous_service_identity) + ->public_key_der(); verify_snapshot_seqno( segments, network.tables->get_encryptor(), snapshot_seqno); @@ -578,7 +577,10 @@ namespace ccf { try { - verify_snapshot(segments, target_identity.raw()); + verify_snapshot( + segments, + startup_inputs.previous_service_identity, + startup_inputs.previous_service_signing_keys); LOG_INFO_FMT( "Recovery snapshot at {} is directly signed by the configured " "previous service identity", @@ -599,7 +601,7 @@ namespace ccf try { const auto verifier = - ccf::crypto::make_cose_verifier_from_pem_cert(target_identity); + ccf::crypto::make_cose_verifier_from_key(target_key); if (verifier->verify_detached(segments.receipt, receipt.merkle_root)) { LOG_INFO_FMT( @@ -626,8 +628,6 @@ namespace ccf const auto scan = scan_recovery_snapshot_ledger_files( config.ledger, network.tables->get_encryptor(), snapshot_seqno); - const auto target_key = ccf::crypto::public_key_der_from_cert( - ccf::crypto::cert_pem_to_der(target_identity)); const auto snapshot_signer_key = validate_recovery_snapshot_endorsement_chain( scan.endorsements, target_key, snapshot_seqno); @@ -1324,7 +1324,7 @@ namespace ccf initiate_quote_generation(); LOG_INFO_FMT("Created new node {}", self); - return {new_self_signed_node_cert, network.identity->cert}; + break; } case StartType::Join: { @@ -1339,24 +1339,17 @@ namespace ccf initiate_quote_generation(); LOG_INFO_FMT("Created join node {}", self); - return {new_self_signed_node_cert, {}}; + return {new_self_signed_node_cert, {}, {}}; } case StartType::Recover: { LOG_INFO_FMT("Creating new node - recover"); - // Already enforced by resolve_startup_inputs(); kept as a guard for - // the dereference below, with the same message. - if (!startup_inputs.previous_service_identity.has_value()) - { - throw std::logic_error( - "Recovery requires the certificate of the previous service " - "identity"); - } - ccf::crypto::Pem previous_service_identity_cert( - startup_inputs.previous_service_identity.value()); + get_previous_service_classical_signing_key( + startup_inputs.previous_service_signing_keys, + startup_inputs.previous_service_identity); network.identity = std::make_unique( - ccf::crypto::get_subject_name(previous_service_identity_cert), + startup_inputs.service_cert_subject_name, curve_id, startup_time, config.command.recover.initial_service_certificate_validity_days); @@ -1364,7 +1357,7 @@ namespace ccf initiate_quote_generation(); LOG_INFO_FMT("Created recovery node {}", self); - return {new_self_signed_node_cert, network.identity->cert}; + break; } default: { @@ -1372,6 +1365,14 @@ namespace ccf fmt::format("Node was started in unknown mode {}", start_type)); } } + + ServiceSigningKeys service_signing_keys{ + {SigningKeyType::CLASSICAL, + network.identity->get_key_pair()->public_key_pem()}}; + return { + new_self_signed_node_cert, + network.identity->cert, + std::move(service_signing_keys)}; } // @@ -2634,6 +2635,18 @@ namespace ccf // independently synchronised (share_manager, via LedgerSecrets), or // copied out under recovery_secrets_lock. + const bool with_signing_keys = + identities.previous_signing_keys.has_value() || + identities.next_signing_keys.has_value(); + if ( + with_signing_keys && + (identities.previous.has_value() || identities.next.has_value())) + { + throw std::logic_error( + "transition_service_to_open accepts either service certificates or " + "service signing keys, not a mix of both"); + } + auto* service = tx.rw(Tables::SERVICE); auto service_info = service->get(); if (!service_info.has_value()) @@ -2654,40 +2667,13 @@ namespace ccf return; } - if (service_info->status == ServiceStatus::RECOVERING) + if (with_signing_keys) { - const auto prev_ident = - tx.ro(Tables::PREVIOUS_SERVICE_IDENTITY) - ->get(); - if (!prev_ident.has_value() || !identities.previous.has_value()) - { - throw std::logic_error( - "Recovery with service certificates requires both, a previous " - "service identity written to the KV during recovery genesis and a " - "transition_service_to_open proposal that contains previous and " - "next service certificates"); - } - - const ccf::crypto::Pem from_proposal( - identities.previous->data(), identities.previous->size()); - if (prev_ident.value() != from_proposal) - { - throw std::logic_error(fmt::format( - "Previous service identity does not match.\nActual:\n{}\nIn " - "proposal:\n{}", - prev_ident->str(), - from_proposal.str())); - } + check_signing_keys_to_open(tx, service_info.value(), identities); } - - if (identities.next != service_info->cert) + else { - throw std::logic_error(fmt::format( - "Service identity mismatch: the next service identity in the " - "transition_service_to_open proposal does not match the current " - "service identity:\nNext:\n{}\nCurrent:\n{}", - identities.next.str(), - service_info->cert.str())); + check_certificates_to_open(tx, service_info.value(), identities); } if (is_part_of_public_network()) @@ -2758,6 +2744,127 @@ namespace ccf } private: + // Checks the identities in a transition_service_to_open proposal + static void check_certificates_to_open( + ccf::kv::Tx& tx, + const ServiceInfo& service_info, + const AbstractGovernanceEffects::ServiceIdentities& identities) + { + if (service_info.status == ServiceStatus::RECOVERING) + { + const auto prev_ident = + tx.ro(Tables::PREVIOUS_SERVICE_IDENTITY) + ->get(); + if (!prev_ident.has_value() || !identities.previous.has_value()) + { + throw std::logic_error( + "Recovery with service certificates requires both, a previous " + "service identity written to the KV during recovery genesis and a " + "transition_service_to_open proposal that contains previous and " + "next service certificates"); + } + + const ccf::crypto::Pem from_proposal( + identities.previous->data(), identities.previous->size()); + if (prev_ident.value() != from_proposal) + { + throw std::logic_error(fmt::format( + "Previous service identity does not match.\nActual:\n{}\nIn " + "proposal:\n{}", + prev_ident->str(), + from_proposal.str())); + } + } + + if (!identities.next.has_value()) + { + throw std::logic_error( + "transition_service_to_open requires the next service certificate"); + } + if (identities.next.value() != service_info.cert) + { + throw std::logic_error(fmt::format( + "Service identity mismatch: the next service identity in the " + "transition_service_to_open proposal does not match the current " + "service identity:\nNext:\n{}\nCurrent:\n{}", + identities.next->str(), + service_info.cert.str())); + } + } + + static std::vector classical_signing_key_der( + const ServiceSigningKeys& keys, const char* name) + { + const auto key = keys.find(SigningKeyType::CLASSICAL); + if (key == keys.end()) + { + throw std::logic_error(fmt::format( + "Missing {} {} service signing key", + SigningKeyType::CLASSICAL, + name)); + } + return ccf::crypto::make_ec_public_key(key->second)->public_key_der(); + } + + // Checks the identities in a transition_service_to_open_with_signing_keys + // proposal. Only CLASSICAL keys are compared. + static void check_signing_keys_to_open( + ccf::kv::Tx& tx, + const ServiceInfo& service_info, + const AbstractGovernanceEffects::ServiceIdentities& identities) + { + if (service_info.status == ServiceStatus::RECOVERING) + { + const auto prev_ident = + tx.ro(Tables::PREVIOUS_SERVICE_IDENTITY) + ->get(); + if ( + !prev_ident.has_value() || + !identities.previous_signing_keys.has_value()) + { + throw std::logic_error( + "Recovery with service signing keys requires both, a previous " + "service identity written to the KV during recovery genesis and a " + "transition_service_to_open_with_signing_keys proposal that " + "contains previous and next service signing keys"); + } + + const auto expected_key = ccf::crypto::public_key_der_from_cert( + ccf::crypto::cert_pem_to_der(prev_ident.value())); + if ( + classical_signing_key_der( + identities.previous_signing_keys.value(), "previous") != + expected_key) + { + throw std::logic_error(fmt::format( + "Previous service identity does not match.\nActual:\n{}\nIn " + "proposal:\n{}", + prev_ident->str(), + nlohmann::json(identities.previous_signing_keys.value()).dump())); + } + } + + if (!identities.next_signing_keys.has_value()) + { + throw std::logic_error( + "transition_service_to_open_with_signing_keys requires the next " + "service signing keys"); + } + const auto& next_keys = identities.next_signing_keys.value(); + const auto current_key = + get_service_signing_identity(tx, IdentityType::CLASSICAL); + if ( + !current_key.has_value() || + classical_signing_key_der(next_keys, "next") != current_key->value) + { + throw std::logic_error(fmt::format( + "Service identity mismatch: the next service signing keys in the " + "transition_service_to_open_with_signing_keys proposal do not " + "match the current service signing keys:\nNext:\n{}", + nlohmann::json(next_keys).dump())); + } + } + // Copies of the recovery state protected by recovery_secrets_lock. These // return by value so that callers never hold the mutex while touching the // KV store, which would invert the KV locks -> recovery_secrets_lock order diff --git a/src/node/rpc/gov_effects_interface.h b/src/node/rpc/gov_effects_interface.h index f940d9b540f..60d350ef69c 100644 --- a/src/node/rpc/gov_effects_interface.h +++ b/src/node/rpc/gov_effects_interface.h @@ -4,6 +4,7 @@ #include "ccf/crypto/pem.h" #include "ccf/node_subsystem_interface.h" +#include "ccf/service_signing_keys.h" #include "ccf/tx.h" namespace ccf @@ -18,10 +19,13 @@ namespace ccf return "GovernanceEffects"; } + // Either certificates or signing keys, never a mix of both struct ServiceIdentities { std::optional previous; - ccf::crypto::Pem next; + std::optional next; + std::optional previous_signing_keys = std::nullopt; + std::optional next_signing_keys = std::nullopt; }; virtual void transition_service_to_open( diff --git a/src/node/rpc/test/node_frontend_test.cpp b/src/node/rpc/test/node_frontend_test.cpp index b08da3b78ec..3bc0a2634c9 100644 --- a/src/node/rpc/test/node_frontend_test.cpp +++ b/src/node/rpc/test/node_frontend_test.cpp @@ -135,6 +135,7 @@ TEST_CASE("Node configuration retains operator file paths") {"host_data_transparent_statement_path", "not-loaded/statement.cose"}}}, {"recover", {{"previous_service_identity_file", "not-loaded/previous.pem"}, + {"service_cert_subject_name", "CN=Recovered Service"}, {"initial_service_certificate_validity_days", 13}}}}}}; auto config = input.get(); @@ -189,6 +190,8 @@ TEST_CASE("Node configuration retains operator file paths") config.command.recover.previous_service_identity_file == "not-loaded/previous.pem"); CHECK(config.command.recover.initial_service_certificate_validity_days == 13); + CHECK( + config.command.recover.service_cert_subject_name == "CN=Recovered Service"); const auto defaults = json{ {"network", CCFConfig{}.network}, @@ -199,6 +202,7 @@ TEST_CASE("Node configuration retains operator file paths") CHECK(defaults.command.join.fetch_recent_snapshot); CHECK( defaults.command.recover.initial_service_certificate_validity_days == 1); + CHECK_FALSE(defaults.command.recover.service_cert_subject_name.has_value()); } TEST_CASE("Genesis request retains resolved data on the wire") @@ -345,6 +349,7 @@ TEST_CASE("Startup inputs are read from files") TEST_CASE("Startup inputs are resolved by start type") { const ScopedTempDir dir; + const auto previous_subject = ccf::crypto::get_subject_name(member_cert); const auto missing_file = (dir.path / "missing").string(); const auto bytes = [](const std::string& s) { return std::vector(s.begin(), s.end()); @@ -369,7 +374,7 @@ TEST_CASE("Startup inputs are resolved by start type") config.command.start.constitution_files = { write_test_file(dir, "constitution.js", "constitution")}; config.command.recover.previous_service_identity_file = - write_test_file(dir, "previous_identity.pem", "previous identity"); + write_test_file(dir, "previous_identity.pem", member_cert.str()); { INFO("Start reads node data, service data and genesis inputs"); @@ -390,7 +395,40 @@ TEST_CASE("Startup inputs are resolved by start type") CHECK(inputs.service_data == json{{"service", 2}}); CHECK_FALSE(inputs.genesis_info.has_value()); CHECK(inputs.join_service_cert.empty()); - CHECK(inputs.previous_service_identity == bytes("previous identity")); + CHECK(inputs.previous_service_identity == member_cert.raw()); + CHECK(inputs.service_cert_subject_name == previous_subject); + } + + { + INFO("A supplied certificate provides the recovery subject"); + auto with_subject = config; + with_subject.command.recover.service_cert_subject_name = + "CN=Different Service"; + CHECK( + resolve(with_subject, StartType::Recover).service_cert_subject_name == + previous_subject); + } + + { + INFO("Key-only recovery requires the configured subject"); + auto keys_only = config; + keys_only.command.recover.previous_service_identity_file.reset(); + keys_only.command.recover.previous_service_signing_key_files = + std::map{ + {SigningKeyType::CLASSICAL, + write_test_file(dir, "previous_key.pem", kp->public_key_pem().str())}}; + keys_only.command.recover.service_cert_subject_name = + "CN=Recovered Service"; + const auto inputs = resolve(keys_only, StartType::Recover); + CHECK_FALSE(inputs.previous_service_identity.has_value()); + CHECK(inputs.service_cert_subject_name == "CN=Recovered Service"); + + keys_only.command.recover.service_cert_subject_name.reset(); + CHECK( + logic_error_message( + [&]() { resolve_startup_inputs(keys_only, StartType::Recover); }) == + "Recovery without command.recover.previous_service_identity_file " + "requires command.recover.service_cert_subject_name"); } { @@ -410,11 +448,12 @@ TEST_CASE("Startup inputs are resolved by start type") { INFO("Inputs required by the start type must be readable"); auto no_identity = config; - no_identity.command.recover.previous_service_identity_file = ""; + no_identity.command.recover.previous_service_identity_file.reset(); CHECK( logic_error_message( [&]() { resolve_startup_inputs(no_identity, StartType::Recover); }) == - "Recovery requires the certificate of the previous service identity"); + "Recovery requires previous service signing keys or a previous service " + "certificate"); auto no_service_cert = config; no_service_cert.command.service_certificate_file = missing_file; diff --git a/src/node/snapshot_serdes.h b/src/node/snapshot_serdes.h index 9a032e483c9..43401cb86a0 100644 --- a/src/node/snapshot_serdes.h +++ b/src/node/snapshot_serdes.h @@ -9,12 +9,14 @@ #include "ccf/historical_queries_adapter.h" #include "ccf/service/tables/nodes.h" #include "crypto/cose.h" +#include "crypto/openssl/verifier.h" #include "ds/internal_logger.h" #include "ds/serialized.h" #include "kv/kv_types.h" #include "kv/serialised_entry_format.h" #include "node/cose_common.h" #include "node/history.h" +#include "node/identity.h" #include "node/rpc/network_identity_chain_helpers.h" #include "node/tx_receipt_impl.h" @@ -254,14 +256,18 @@ namespace ccf static void verify_cose_snapshot_receipt( const SnapshotSegments& segments, - const std::optional>& prev_service_identity) + const std::optional>& prev_service_identity, + const std::optional& prev_service_signing_keys = + std::nullopt) { const auto receipt = decode_and_verify_cose_snapshot_receipt(segments); - if (prev_service_identity) + if (prev_service_signing_keys || prev_service_identity) { - auto verifier = ccf::crypto::make_cose_verifier_from_pem_cert( - ccf::crypto::Pem(*prev_service_identity)); + const auto key = get_previous_service_classical_signing_key( + prev_service_signing_keys, prev_service_identity); + auto verifier = + ccf::crypto::make_cose_verifier_from_key(key->public_key_der()); if (!verifier->verify_detached(segments.receipt, receipt.merkle_root)) { throw std::logic_error( @@ -274,7 +280,9 @@ namespace ccf static void verify_json_snapshot_receipt( const SnapshotSegments& segments, - const std::optional>& prev_service_identity) + const std::optional>& prev_service_identity, + const std::optional& prev_service_signing_keys = + std::nullopt) { auto j = ccf::parse_json_safe(segments.receipt.begin(), segments.receipt.end()); @@ -299,7 +307,8 @@ namespace ccf auto root = receipt->calculate_root(); - auto v = ccf::crypto::make_unique_verifier(receipt->cert); + auto v = + std::make_unique(receipt->cert.raw()); if (!v->verify_hash( root.h.data(), root.h.size(), @@ -311,7 +320,19 @@ namespace ccf "Signature verification failed for snapshot receipt"); } - if (prev_service_identity) + if (prev_service_signing_keys) + { + const auto key = get_previous_service_classical_signing_key( + prev_service_signing_keys, prev_service_identity); + if (!v->verify_certificate_signature(key->public_key_pem())) + { + throw std::logic_error( + "Previous service identity does not endorse the node identity " + "that signed the snapshot"); + } + LOG_DEBUG_FMT("Previous service signing key endorses snapshot signer"); + } + else if (prev_service_identity) { ccf::crypto::Pem prev_pem(*prev_service_identity); if (!v->verify_certificate( @@ -328,7 +349,9 @@ namespace ccf static void verify_snapshot( const SnapshotSegments& segments, - std::optional> prev_service_identity = std::nullopt) + std::optional> prev_service_identity = std::nullopt, + const std::optional& prev_service_signing_keys = + std::nullopt) { LOG_INFO_FMT( "Deserialising snapshot receipt (size: {}).", segments.receipt.size()); @@ -359,12 +382,14 @@ namespace ccf if (first_byte == ENCODED_COSE_SIGN1_TAG) { LOG_DEBUG_FMT("Snapshot with COSE receipt detected"); - verify_cose_snapshot_receipt(segments, prev_service_identity); + verify_cose_snapshot_receipt( + segments, prev_service_identity, prev_service_signing_keys); } else if (first_byte == '{') { LOG_DEBUG_FMT("Snapshot with JSON receipt detected"); - verify_json_snapshot_receipt(segments, prev_service_identity); + verify_json_snapshot_receipt( + segments, prev_service_identity, prev_service_signing_keys); } else { diff --git a/src/node/startup_inputs.h b/src/node/startup_inputs.h index e628a343239..e22bf62b7c5 100644 --- a/src/node/startup_inputs.h +++ b/src/node/startup_inputs.h @@ -3,9 +3,11 @@ #pragma once #include "ccf/crypto/pem.h" +#include "ccf/crypto/verifier.h" #include "ccf/ds/json.h" #include "ccf/node/configuration.h" #include "ccf/node/start_type.h" +#include "ccf/service_signing_keys.h" #include "ds/internal_logger.h" #include "node/rpc/node_call_types.h" @@ -119,7 +121,7 @@ namespace ccf return genesis; } - // File-backed inputs from the operator configuration, other than the SNP + // Resolved startup inputs from the operator configuration, other than the SNP // attestation files (read during quote generation) and the join transparent // statement (read on each join attempt). struct StartupInputs @@ -133,8 +135,11 @@ namespace ccf // Join only std::vector join_service_cert; // Recover only + std::string service_cert_subject_name; std::optional> previous_service_identity = std::nullopt; + std::optional previous_service_signing_keys = + std::nullopt; }; // Reads each input required by start_type exactly once, throwing if any @@ -180,17 +185,49 @@ namespace ccf { const auto& identity_file = config.command.recover.previous_service_identity_file; - if (identity_file.empty()) + const auto& key_files = + config.command.recover.previous_service_signing_key_files; + if (!identity_file.has_value() && !key_files.has_value()) { throw std::logic_error( - "Recovery requires the certificate of the previous service " - "identity"); + "Recovery requires previous service signing keys or a previous " + "service certificate"); } - LOG_INFO_FMT( - "Reading previous service identity from {}", identity_file); - inputs.previous_service_identity = - read_startup_file(identity_file, "previous service identity"); + if (identity_file.has_value()) + { + LOG_INFO_FMT( + "Reading previous service identity from {}", *identity_file); + inputs.previous_service_identity = + read_startup_file(*identity_file, "previous service identity"); + // The recovered service certificate inherits the previous subject + inputs.service_cert_subject_name = ccf::crypto::get_subject_name( + ccf::crypto::Pem(*inputs.previous_service_identity)); + } + else + { + const auto& configured_subject = + config.command.recover.service_cert_subject_name; + if (!configured_subject.has_value()) + { + throw std::logic_error( + "Recovery without command.recover.previous_service_identity_file " + "requires command.recover.service_cert_subject_name"); + } + inputs.service_cert_subject_name = configured_subject.value(); + } + if (key_files.has_value()) + { + auto& keys = inputs.previous_service_signing_keys.emplace(); + const auto& path = key_files->at(SigningKeyType::CLASSICAL); + LOG_INFO_FMT( + "Reading previous CLASSICAL service signing public key from {}", + path); + keys.emplace( + SigningKeyType::CLASSICAL, + ccf::crypto::Pem(read_startup_file( + path, "previous CLASSICAL service signing public key"))); + } break; } default: diff --git a/src/node/test/identity_types.cpp b/src/node/test/identity_types.cpp index 74f3d96e5b7..0bf9bbaa3c0 100644 --- a/src/node/test/identity_types.cpp +++ b/src/node/test/identity_types.cpp @@ -4,6 +4,9 @@ #include "service/tables/identity_types.h" #include "ccf/kv/unit.h" +#include "ccf/node/configuration.h" +#include "ccf/service_signing_keys.h" +#include "crypto/openssl/ec_key_pair.h" #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN #include @@ -93,3 +96,38 @@ TEST_CASE("Identities round-trips through JSON") const nlohmann::json j = identities; REQUIRE(j.get() == identities); } + +TEST_CASE("Service signing key files are a JSON object keyed by identity name") +{ + const ccf::CCFConfig::Command command; + const nlohmann::json paths = command.service_signing_key_files; + REQUIRE( + paths == + nlohmann::json{{"CLASSICAL", "service_signing_key_classical.pem"}}); + + const nlohmann::json custom = { + {"type", "Start"}, + {"service_signing_key_files", {{"CLASSICAL", "custom_signing_key.pem"}}}}; + const auto parsed = custom.get(); + REQUIRE( + parsed.service_signing_key_files.at(ccf::SigningKeyType::CLASSICAL) == + "custom_signing_key.pem"); + const nlohmann::json round_trip = parsed; + REQUIRE( + round_trip.at("service_signing_key_files") == + custom.at("service_signing_key_files")); +} + +TEST_CASE( + "Service signing public keys round-trip as PEM strings in a JSON object") +{ + const ccf::crypto::ECKeyPair_OpenSSL key_pair( + ccf::crypto::CurveID::SECP384R1); + const auto public_key = key_pair.public_key_pem(); + const ccf::ServiceSigningKeys keys{ + {ccf::SigningKeyType::CLASSICAL, public_key}}; + + const nlohmann::json j = keys; + REQUIRE(j == nlohmann::json{{"CLASSICAL", public_key.str()}}); + REQUIRE(j.get() == keys); +} diff --git a/src/node/test/snapshotter.cpp b/src/node/test/snapshotter.cpp index f4761a4483b..929d052edb5 100644 --- a/src/node/test/snapshotter.cpp +++ b/src/node/test/snapshotter.cpp @@ -3,6 +3,10 @@ #include "node/snapshotter.h" +#include "ccf/ds/x509_time_fmt.h" +#include "ccf/receipt.h" +#include "ccf/service/tables/nodes.h" +#include "ccf/service_signing_keys.h" #include "crypto/openssl/hash.h" #include "ds/files.h" #include "ds/internal_logger.h" @@ -153,6 +157,54 @@ TEST_CASE("Recovery snapshot endorsement scan reads ledger files directly") scan.endorsements, target_key, 1)); } +TEST_CASE("Legacy JSON snapshot receipts are verified with signing keys") +{ + using namespace std::literals; + const auto valid_from = + ccf::ds::to_x509_time_string(std::chrono::system_clock::now() - 1h); + const auto valid_to = + ccf::ds::to_x509_time_string(std::chrono::system_clock::now() + 1h); + + const auto service_kp = ccf::crypto::make_ec_key_pair(); + const auto service_cert = + service_kp->self_sign("CN=service", valid_from, valid_to); + const auto signer_kp = ccf::crypto::make_ec_key_pair(); + const std::vector snapshot = {1, 2, 3}; + + auto receipt = std::make_shared(); + receipt->cert = service_kp->sign_csr( + service_cert, signer_kp->create_csr("CN=node"), valid_from, valid_to); + receipt->node_id = ccf::compute_node_id_from_kp(signer_kp); + receipt->leaf_components.write_set_digest = + ccf::crypto::Sha256Hash(std::string("write set")); + receipt->leaf_components.commit_evidence = "ce:2.4:abcd"; + receipt->leaf_components.claims_digest.set( + ccf::crypto::Sha256Hash(snapshot.data(), snapshot.size())); + const auto root = receipt->calculate_root(); + receipt->signature = signer_kp->sign_hash(root.h.data(), root.h.size()); + + const auto receipt_str = nlohmann::json(ccf::ReceiptPtr(receipt)).dump(); + const std::vector receipt_bytes( + receipt_str.begin(), receipt_str.end()); + const ccf::SnapshotSegments segments{snapshot, receipt_bytes}; + + const ccf::ServiceSigningKeys service_keys{ + {ccf::SigningKeyType::CLASSICAL, service_kp->public_key_pem()}}; + const ccf::ServiceSigningKeys other_keys{ + {ccf::SigningKeyType::CLASSICAL, + ccf::crypto::make_ec_key_pair()->public_key_pem()}}; + + REQUIRE_NOTHROW(ccf::verify_snapshot(segments, std::nullopt, service_keys)); + REQUIRE_THROWS_WITH( + ccf::verify_snapshot(segments, std::nullopt, other_keys), + "Previous service identity does not endorse the node identity that " + "signed the snapshot"); + + INFO("Mismatching keys do not fall back to a matching certificate"); + REQUIRE_THROWS( + ccf::verify_snapshot(segments, service_cert.raw(), other_keys)); +} + TEST_CASE("Recovery snapshot endorsement scan bounds candidate endorsements") { ScopedSnapshotDir ledger_dir; diff --git a/tests/config.jinja b/tests/config.jinja index d4bba84898a..654c3591e0b 100644 --- a/tests/config.jinja +++ b/tests/config.jinja @@ -49,8 +49,10 @@ "host_data_transparent_statement_path": {{ host_data_transparent_statement_path|tojson }}{% endif %} }, "recover": { - "initial_service_certificate_validity_days": {{ initial_service_cert_validity_days }}, - "previous_service_identity_file": "{{ previous_service_identity_file }}" + "initial_service_certificate_validity_days": {{ initial_service_cert_validity_days }}{% if recovery_service_cert_subject_name is defined and recovery_service_cert_subject_name is not none %}, + "service_cert_subject_name": {{ recovery_service_cert_subject_name|tojson }}{% endif %}{% if previous_service_identity_file is defined and previous_service_identity_file is not none %}, + "previous_service_identity_file": {{ previous_service_identity_file|tojson }}{% endif %}{% if previous_service_signing_key_files is defined and previous_service_signing_key_files is not none %}, + "previous_service_signing_key_files": {{ previous_service_signing_key_files|tojson }}{% endif %} } }, "ledger": diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index 0c61e507660..1cae2f4f2fe 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -2561,7 +2561,8 @@ def run_initial_uvm_descriptor_checks(const_args): ) network.consortium.add_snp_uvm_endorsement(primary, did, feed, bumped_svn) - network_service_identity_file, _ = network.save_service_identity_to_file() + network.save_service_identity(args) + network_service_identity_file = args.previous_service_identity_file snapshots_dir = network.get_committed_snapshots(primary) network.stop_all_nodes() LOG.info("Check that the a UVM descriptor is present") @@ -2680,7 +2681,8 @@ def get_min_tcb_versions(node): tcb_versions_before_recovery[cpuid]["hexstring"] == tcb_hex_before_recovery ), tcb_versions_before_recovery - network_service_identity_file, _ = network.save_service_identity_to_file() + network.save_service_identity(args) + network_service_identity_file = args.previous_service_identity_file snapshots_dir = network.get_committed_snapshots(primary) network.stop_all_nodes() diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index b4ef3549179..17c4a5eef6d 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -497,6 +497,13 @@ def add_user(self, remote_node, user_id, user_data=None): def get_service_identity(self): return slurp_file(os.path.join(self.common_dir, "service_cert.pem")) + def get_service_signing_keys(self): + return { + "CLASSICAL": slurp_file( + os.path.join(self.common_dir, "service_signing_key_classical.pem") + ) + } + def add_users_and_transition_service_to_open(self, remote_node, users): proposal = {"actions": []} for user_id in users: @@ -719,7 +726,12 @@ def remove_ca_cert_bundle(self, remote_node, cert_name): proposal = self.get_any_active_member().propose(remote_node, proposal_body) return self.vote_using_majority(remote_node, proposal, careful_vote) - def transition_service_to_open(self, remote_node, previous_service_identity=None): + def transition_service_to_open( + self, + remote_node, + previous_service_identity=None, + previous_service_signing_keys=None, + ): """ Assuming a network in state OPENING, this functions creates a new proposal and make members vote to transition the network to state @@ -731,16 +743,25 @@ def transition_service_to_open(self, remote_node, previous_service_identity=None if r.body.json()["state"] == infra.node.State.PART_OF_NETWORK.value: is_recovery = False + action = "transition_service_to_open" args = {} if CCFVersion(remote_node.version) > CCFVersion("ccf-2.0.0-rc3"): - args = { - "previous_service_identity": previous_service_identity, - "next_service_identity": self.get_service_identity(), - } + if ( + remote_node.version is None + and previous_service_signing_keys is not None + ): + action = "transition_service_to_open_with_signing_keys" + args = { + "previous_service_signing_keys": previous_service_signing_keys, + "next_service_signing_keys": self.get_service_signing_keys(), + } + else: + args = { + "previous_service_identity": previous_service_identity, + "next_service_identity": self.get_service_identity(), + } - proposal_body, careful_vote = self.make_proposal( - "transition_service_to_open", **args - ) + proposal_body, careful_vote = self.make_proposal(action, **args) proposal = self.get_any_active_member().propose(remote_node, proposal_body) self.vote_using_majority( diff --git a/tests/infra/network.py b/tests/infra/network.py index b6ebf175001..f9bf5cdfd2d 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -20,6 +20,7 @@ import ccf.ledger from ccf.tx_id import TxID from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.serialization import Encoding, PublicFormat from cryptography.x509 import load_pem_x509_certificate from loguru import logger as LOG @@ -31,6 +32,7 @@ import infra.openapi import infra.path import infra.proc +import infra.remote from infra.clients import CCFConnectionException, CCFIOException, flush_info from infra.consortium import slurp_file from infra.node import CCFVersion @@ -45,6 +47,56 @@ COMMON_FOLDER = "common" +def get_previous_service_identity(args): + certificate_file = getattr(args, "previous_service_identity_file", None) + key_files = getattr(args, "previous_service_signing_key_files", None) + return { + "previous_service_identity": ( + slurp_file(certificate_file) if certificate_file else None + ), + "previous_service_signing_keys": ( + { + identity_type: slurp_file(path) + for identity_type, path in key_files.items() + } + if key_files is not None + else None + ), + } + + +def service_signing_key_from_certificate(certificate): + return ( + load_pem_x509_certificate(certificate.encode("ascii"), default_backend()) + .public_key() + .public_bytes(Encoding.PEM, PublicFormat.SubjectPublicKeyInfo) + .decode("ascii") + ) + + +def save_service_signing_keys(certificate_file, directory, key_files=None): + if key_files is None: + # Historical services did not export signing public keys separately. + keys = { + "CLASSICAL": service_signing_key_from_certificate( + slurp_file(certificate_file) + ) + } + else: + keys = { + identity_type: slurp_file(path) for identity_type, path in key_files.items() + } + + stem = os.path.splitext(os.path.basename(certificate_file))[0] + paths = {} + for identity_type, key in keys.items(): + path = os.path.join(directory, f"{stem}_{identity_type}_pubk.pem") + with open(path, "w", encoding="utf-8") as key_file: + key_file.write(key) + paths[identity_type] = path + return paths + + class NodeRole(Enum): ANY = auto() PRIMARY = auto() @@ -216,6 +268,8 @@ class Network: "config_file", "ubsan_options", "previous_service_identity_file", + "previous_service_signing_key_files", + "recovery_service_cert_subject_name", "snp_endorsements_servers", "node_to_node_message_limit", "historical_cache_soft_limit", @@ -982,16 +1036,9 @@ def recover( # so we make sure that we're running the right one. self.consortium.set_constitution(random_node, args.constitution) - prev_service_identity = None - if ( - args.previous_service_identity_file is not None - and args.previous_service_identity_file != "" - ): - prev_service_identity = slurp_file(args.previous_service_identity_file) - self.consortium.transition_service_to_open( self.find_random_node(), - previous_service_identity=prev_service_identity, + **get_previous_service_identity(args), ) if via_local_sealing: @@ -2500,7 +2547,10 @@ def verify_service_certificate_validity_period(self, expected_validity_days): def refresh_service_identity_file(self, args): """ Refresh service_cert.pem from the current primary node, so that future client - connections pick up the new service certificate. + connections pick up the new service certificate. The service signing key files + are copied from the joined node that started or recovered the service, as the + files fetched on startup may come from a recovery node whose identity was not + retained. """ primary = self.find_random_node() with primary.client(verify_ca=False) as c: @@ -2516,6 +2566,20 @@ def refresh_service_identity_file(self, args): with open(identity_filepath, "w", encoding="utf-8") as f: f.write(new_service_identity) + exporters = [ + node + for node in self.get_joined_nodes() + if node.remote.start_type + in {infra.remote.StartType.start, infra.remote.StartType.recover} + ] + assert len(exporters) == 1, [node.local_node_id for node in exporters] + (exporter,) = exporters + if exporter.remote.supports_service_signing_keys: + exporter.remote.get_service_signing_key_files(self.common_dir) + LOG.info( + f"Refreshed service signing key files from node {exporter.local_node_id}" + ) + def get_service_identity(self): n = self.find_random_node() with n.client() as c: @@ -2542,6 +2606,18 @@ def save_service_identity_to_file(self): def save_service_identity(self, args): path, identity = self.save_service_identity_to_file() args.previous_service_identity_file = path + signing_key_file = os.path.join( + self.common_dir, "service_signing_key_classical.pem" + ) + args.previous_service_signing_key_files = save_service_signing_keys( + path, + self.common_dir, + ( + {"CLASSICAL": signing_key_file} + if os.path.exists(signing_key_file) + else None + ), + ) return identity def identity(self, name=None): diff --git a/tests/infra/remote.py b/tests/infra/remote.py index 6aaaacfbd5a..80ba40f2c55 100644 --- a/tests/infra/remote.py +++ b/tests/infra/remote.py @@ -552,6 +552,7 @@ def __init__( self.name = f"{label}_{local_node_id}" self.start_type = start_type + self.supports_service_signing_keys = version is None self.local_node_id = local_node_id self.pem = f"{local_node_id}.pem" self.node_address_file = f"{local_node_id}.node_address" @@ -741,6 +742,14 @@ def __init__( # This will also ensure the render produced valid JSON j = json.loads(output) + if not self.supports_service_signing_keys: + j["command"].get("recover", {}).pop( + "previous_service_signing_key_files", None + ) + j["command"].get("recover", {}).pop( + "service_cert_subject_name", None + ) + # Releases before 7.0.16 reject this unknown HTTP configuration field. if v is not None and v < Version("7.0.16"): for interface in j["network"]["rpc_interfaces"].values(): @@ -860,6 +869,11 @@ def get_startup_files(self, dst_path, timeout=FILE_TIMEOUT_S): self.remote.get(self.rpc_addresses_file, dst_path, timeout=timeout) if self.start_type in {StartType.start, StartType.recover}: self.remote.get("service_cert.pem", dst_path, timeout=timeout) + if self.supports_service_signing_keys: + self.get_service_signing_key_files(dst_path, timeout=timeout) + + def get_service_signing_key_files(self, dst_path, timeout=FILE_TIMEOUT_S): + self.remote.get("service_signing_key_classical.pem", dst_path, timeout=timeout) def debug_node_cmd(self): return self.remote.debug_node_cmd() diff --git a/tests/partitions_test.py b/tests/partitions_test.py index 8e789e99739..766893fb10e 100644 --- a/tests/partitions_test.py +++ b/tests/partitions_test.py @@ -1016,7 +1016,7 @@ def test_recovery_elections(orig_network, args): r = c.get("/node/network") assert r.status_code == 200, r - previous_identity = orig_network.save_service_identity(args) + orig_network.save_service_identity(args) c.wait_for_commit( orig_network.consortium.set_recovery_threshold(old_primary, 1) ) @@ -1041,11 +1041,11 @@ def test_recovery_elections(orig_network, args): ) new_primary, new_backups = network.find_nodes() network.consortium.transition_service_to_open( - new_primary, previous_service_identity=previous_identity + new_primary, **infra.network.get_previous_service_identity(args) ) with new_primary.client("user0") as c: - previous_identity = network.save_service_identity(args) + network.save_service_identity(args) member = network.consortium.get_active_recovery_participants()[0] diff --git a/tests/recovery.py b/tests/recovery.py index 7602b12c811..44c16391918 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -41,7 +41,6 @@ test_cose_receipt_schema, verify_receipt, ) -from infra.consortium import slurp_file from infra.runner import ConcurrentRunner from loguru import logger as LOG from reconfiguration import assert_no_ipv4_in_node_configs @@ -256,14 +255,9 @@ def recover_with_primary_dying(args, recovered_network, after_backups_recovered= recovered_network.find_random_node() ) - prev_service_identity = None - if args.previous_service_identity_file: - prev_service_identity = slurp_file(args.previous_service_identity_file) - LOG.info(f"Prev identity: {prev_service_identity}") - recovered_network.consortium.transition_service_to_open( recovered_network.find_random_node(), - previous_service_identity=prev_service_identity, + **infra.network.get_previous_service_identity(args), ) retired_primary, initial_view = recovered_network.find_primary() @@ -443,7 +437,7 @@ def test_recovery_member_changes_rejected_during_recovery(network, args): primary, _ = recovered_network.find_primary() recovered_network.consortium.transition_service_to_open( primary, - previous_service_identity=slurp_file(args.previous_service_identity_file), + **infra.network.get_previous_service_identity(args), ) recovered_network.consortium.check_for_service( primary, @@ -556,9 +550,7 @@ def run_reconfiguration_before_recovery_shares(args): primary, _ = recovered_network.find_primary() recovered_network.consortium.transition_service_to_open( primary, - previous_service_identity=slurp_file( - args.previous_service_identity_file - ), + **infra.network.get_previous_service_identity(args), ) recovered_network.consortium.check_for_service( primary, infra.network.ServiceStatus.WAITING_FOR_RECOVERY_SHARES @@ -638,6 +630,7 @@ def test_recover_service( isolate_latest_snapshot=False, election_after_backups_recovered=False, recovered_networks=None, + signing_keys_only=False, ): if not from_snapshot and snapshots_dir is not None: raise ValueError("snapshots_dir requires from_snapshot=True") @@ -680,6 +673,7 @@ def test_recover_service( snapshots_dir=isolated_snapshots_dir, election_after_backups_recovered=election_after_backups_recovered, recovered_networks=recovered_networks, + signing_keys_only=signing_keys_only, ) return _recover_service( @@ -692,6 +686,7 @@ def test_recover_service( snapshots_dir=snapshots_dir, election_after_backups_recovered=election_after_backups_recovered, recovered_networks=recovered_networks, + signing_keys_only=signing_keys_only, ) @@ -711,6 +706,70 @@ def test_recover_service_with_ledger_after_snapshot(network, args): return test_recover_service(network, args, snapshots_dir=snapshots_dir) +def check_signing_keys_proposal_rejections(network, args, previous_identity): + """ + Signing key proposals carrying certificates are not created, and mismatching + signing keys fail when applied, leaving the service recovering. + """ + primary, _ = network.find_primary() + consortium = network.consortium + action = "transition_service_to_open_with_signing_keys" + previous_keys = infra.network.get_previous_service_identity(args)[ + "previous_service_signing_keys" + ] + next_keys = consortium.get_service_signing_keys() + + for certificate_args in ( + {"next_service_identity": consortium.get_service_identity()}, + {"previous_service_identity": previous_identity}, + ): + body, _ = consortium.make_proposal( + action, + previous_service_signing_keys=previous_keys, + next_service_signing_keys=next_keys, + **certificate_args, + ) + try: + consortium.get_any_active_member().propose(primary, body) + assert False, f"Proposal should not be created: {list(certificate_args)}" + except infra.proposal.ProposalNotCreated as e: + assert e.response.status_code == http.HTTPStatus.BAD_REQUEST, e.response + + for mismatching_args, expected_error in ( + ( + { + "previous_service_signing_keys": next_keys, + "next_service_signing_keys": next_keys, + }, + "Previous service identity does not match", + ), + ( + { + "previous_service_signing_keys": previous_keys, + "next_service_signing_keys": previous_keys, + }, + ( + "the next service signing keys in the " + "transition_service_to_open_with_signing_keys proposal do not match" + ), + ), + ): + body, ballot = consortium.make_proposal(action, **mismatching_args) + proposal = consortium.get_any_active_member().propose(primary, body) + try: + consortium.vote_using_majority(primary, proposal, ballot) + assert False, "Mismatching proposal should not be accepted" + except infra.proposal.ProposalNotAccepted as e: + assert ( + e.response.status_code == http.HTTPStatus.INTERNAL_SERVER_ERROR + ), e.response + assert ( + expected_error in e.response.body.json()["error"]["message"] + ), e.response + + consortium.check_for_service(primary, infra.network.ServiceStatus.RECOVERING) + + def _recover_service( network, args, @@ -721,6 +780,7 @@ def _recover_service( snapshots_dir=None, election_after_backups_recovered=False, recovered_networks=None, + signing_keys_only=False, ): network.save_service_identity(args) old_node_ids = {node.node_id for node in network.get_joined_nodes()} @@ -770,6 +830,14 @@ def _recover_service( else: current_ledger_dir, committed_ledger_dirs = old_primary.get_ledger() + if signing_keys_only: + # Without the previous certificate, the recovered service certificate + # subject must be configured explicitly + args.previous_service_identity_file = None + args.recovery_service_cert_subject_name = load_pem_x509_certificate( + prev_ident.encode("ascii"), default_backend() + ).subject.rfc4514_string() + with tempfile.NamedTemporaryFile(mode="w+") as node_data_tf: start_node_data = {"this is a": "recovery node"} json.dump(start_node_data, node_data_tf) @@ -834,6 +902,9 @@ def _recover_service( r = c.get("/node/ready/app") assert r.status_code == http.HTTPStatus.SERVICE_UNAVAILABLE.value, r + if signing_keys_only: + check_signing_keys_proposal_rejections(recovered_network, args, prev_ident) + if force_election: recover_with_primary_dying( args, @@ -905,6 +976,17 @@ def _recover_service( unexpected_removable_node_ids = old_node_ids & removable_node_ids assert not unexpected_removable_node_ids, unexpected_removable_node_ids + if signing_keys_only: + recovered_cert = load_pem_x509_certificate( + current_network_info["service_certificate"].encode("ascii"), + default_backend(), + ) + assert ( + recovered_cert.subject.rfc4514_string() + == args.recovery_service_cert_subject_name + ), recovered_cert.subject + args.recovery_service_cert_subject_name = None + return recovered_network @@ -1008,51 +1090,61 @@ def test_recover_service_with_wrong_identity(network, args): current_ledger_dir, committed_ledger_dirs = old_primary.get_ledger() - # Attempt a recovery with the wrong previous service certificate - # The mismatch results in all snapshots being ignored - - args.previous_service_identity_file = network.consortium.user_cert_path("user0") - - broken_network = infra.network.Network( - args.nodes, - args.binary_dir, - args.debug_nodes, - existing_network=network, - ) + # Each wrong previous identity results in all snapshots being ignored, and + # the mismatch is only fatal when used in a transition proposal. Invalid + # signing keys must not fall back to the correct previous certificate. + wrong_identity_file = network.consortium.user_cert_path("user0") + for identity_file, signing_key_files in ( + (wrong_identity_file, None), + ( + first_service_identity_file, + infra.network.save_service_signing_keys( + wrong_identity_file, network.common_dir + ), + ), + ): + args.previous_service_identity_file = identity_file + args.previous_service_signing_key_files = signing_key_files - broken_network.start_in_recovery( - args, - ledger_dir=current_ledger_dir, - committed_ledger_dirs=committed_ledger_dirs, - snapshots_dir=snapshots_dir, - ) + broken_network = infra.network.Network( + args.nodes, + args.binary_dir, + args.debug_nodes, + existing_network=network, + ) - # The mismatch is only fatal when used in a transition proposal - exception = None - try: - broken_network.recover(args) - except Exception as ex: - exception = ex + broken_network.start_in_recovery( + args, + ledger_dir=current_ledger_dir, + committed_ledger_dirs=committed_ledger_dirs, + snapshots_dir=snapshots_dir, + ) - broken_network.ignoring_shutdown_errors = True - broken_network.stop_all_nodes(skip_verification=True) + exception = None + try: + broken_network.recover(args) + except Exception as ex: + exception = ex - if exception is None: - raise ValueError("Recovery should have failed") + broken_network.ignoring_shutdown_errors = True + broken_network.stop_all_nodes(skip_verification=True) - if not broken_network.nodes[0].check_log_for_error_message( - "Previous service identity does not match the service identity that signed the snapshot" - ): - raise ValueError("Node log does not contain the expected error message") + if exception is None: + raise ValueError("Recovery should have failed") - if not broken_network.nodes[0].check_log_for_error_message( - "Unable to open service: Previous service identity does not match." - ): - raise ValueError("Node log does not contain the expected error message") + for message in ( + "Previous service identity does not match the service identity that signed the snapshot", + "Unable to open service: Previous service identity does not match.", + ): + if not broken_network.nodes[0].check_log_for_error_message(message): + raise ValueError( + f"Node log does not contain the expected error message: {message}" + ) - # Recover, now with the correct service identity + # Recover, now with the correct previous service certificate only args.previous_service_identity_file = first_service_identity_file + args.previous_service_signing_key_files = None recovered_network = infra.network.Network( args.nodes, @@ -1298,6 +1390,11 @@ def run_recover_service_from_files( args.previous_service_identity_file = os.path.join( old_common, "service_cert.pem" ) + args.previous_service_signing_key_files = ( + infra.network.save_service_signing_keys( + args.previous_service_identity_file, new_common + ) + ) network.start_in_recovery( args, @@ -1558,7 +1655,7 @@ def test_share_resilience(network, args, from_snapshot=False): primary, _ = recovered_network.find_primary() recovered_network.consortium.transition_service_to_open( primary, - previous_service_identity=slurp_file(args.previous_service_identity_file), + **infra.network.get_previous_service_identity(args), ) # Submit all required recovery shares minus one. Last recovery share is @@ -1849,9 +1946,12 @@ def run(args, ipv6=False): network, args, from_snapshot=False ) else: - # Vary nodes certificate elliptic curve + # Vary nodes certificate elliptic curve, and recover from the + # previous service signing keys only args.curve_id = infra.network.EllipticCurve.secp256r1 - network = test_recover_service(network, args, from_snapshot=False) + network = test_recover_service( + network, args, from_snapshot=False, signing_keys_only=True + ) for node in network.get_joined_nodes(): node.verify_certificate_validity_period() @@ -2128,6 +2228,122 @@ def run_recover_snapshot_alone(args): return network +def run_recovery_with_signing_keys_only(args): + """ + Open a new service with transition_service_to_open_with_signing_keys, after + malformed signing key maps are rejected. Then recover it with previous + signing keys only: a wrong key ignores the snapshot and cannot open the + service, while the right key uses the snapshot and opens it. + """ + with infra.network.network( + args.nodes, args.binary_dir, args.debug_nodes, pdb=args.pdb + ) as network: + network.start(args) + primary, _ = network.find_primary() + consortium = network.consortium + consortium.activate(primary) + member = consortium.get_any_active_member() + action = "transition_service_to_open_with_signing_keys" + next_keys = consortium.get_service_signing_keys() + + for malformed_keys in ( + None, + "not an object", + [next_keys["CLASSICAL"]], + {}, + {"CLASSICAL": 42}, + ): + body = { + "actions": [ + { + "name": action, + "args": {"next_service_signing_keys": malformed_keys}, + } + ] + } + try: + member.propose(primary, body) + assert False, f"Proposal should not be created: {malformed_keys}" + except infra.proposal.ProposalNotCreated as e: + assert e.response.status_code == http.HTTPStatus.BAD_REQUEST, e.response + + body, ballot = consortium.make_proposal( + action, next_service_signing_keys={"CLASSICAL": "not a PEM key"} + ) + proposal = member.propose(primary, body) + try: + consortium.vote_using_majority(primary, proposal, ballot) + assert False, "Proposal with a non-PEM key should not be accepted" + except infra.proposal.ProposalNotAccepted as e: + assert ( + "PEM constructed with non-PEM data" + in e.response.body.json()["error"]["message"] + ), e.response + + body, ballot = consortium.make_proposal( + action, next_service_signing_keys=next_keys + ) + proposal = member.propose(primary, body) + consortium.vote_using_majority(primary, proposal, ballot) + consortium.check_for_service(primary, infra.network.ServiceStatus.OPEN) + + snapshots_dir = network.get_committed_snapshots(primary) + previous_identity = network.save_service_identity(args) + signing_key_files = args.previous_service_signing_key_files + args.previous_service_identity_file = None + args.recovery_service_cert_subject_name = load_pem_x509_certificate( + previous_identity.encode("ascii"), default_backend() + ).subject.rfc4514_string() + network.stop_all_nodes() + ledger_dir, committed_ledger_dirs = primary.get_ledger() + + args.previous_service_signing_key_files = ( + infra.network.save_service_signing_keys( + os.path.join(network.common_dir, f"{member.local_id}_cert.pem"), + network.common_dir, + ) + ) + broken_network = infra.network.Network( + args.nodes, args.binary_dir, args.debug_nodes, existing_network=network + ) + try: + broken_network.start_in_recovery( + args, + ledger_dir=ledger_dir, + committed_ledger_dirs=committed_ledger_dirs, + snapshots_dir=snapshots_dir, + ) + broken_network.recover(args) + assert False, "Recovery with a wrong previous signing key should fail" + except infra.proposal.ProposalNotAccepted: + pass + finally: + broken_network.ignoring_shutdown_errors = True + broken_network.stop_all_nodes(skip_verification=True) + for message in ( + "Previous service identity does not match the service identity that signed the snapshot", + "Unable to open service: Previous service identity does not match.", + ): + assert broken_network.nodes[0].check_log_for_error_message(message), message + + args.previous_service_signing_key_files = signing_key_files + recovered_network = infra.network.Network( + args.nodes, args.binary_dir, args.debug_nodes, existing_network=network + ) + with infra.network.close_on_error(recovered_network): + recovered_network.start_in_recovery( + args, + ledger_dir=ledger_dir, + committed_ledger_dirs=committed_ledger_dirs, + snapshots_dir=snapshots_dir, + ) + assert recovered_network.nodes[0].check_log_for_error_message( + "is directly signed by the configured previous service identity" + ) + recovered_network.recover(args) + recovered_network.stop_all_nodes() + + def run_recovery_with_missing_service_data(args): """ Recovery nodes read their file-backed inputs when they are created, so a @@ -2518,6 +2734,9 @@ def run_recovery_after_cose_upgrade(args): strict_args.previous_service_identity_file = ( recovered_args.previous_service_identity_file ) + strict_args.previous_service_signing_key_files = ( + recovered_args.previous_service_signing_key_files + ) strict_network = infra.network.Network( args.nodes, args.binary_dir, @@ -3363,6 +3582,14 @@ def add(parser): nodes=infra.e2e_args.min_nodes(cr.args, f=0), # 1 node suffices for recovery ) + cr.add( + "recovery_signing_keys_only", + run_recovery_with_signing_keys_only, + package="samples/apps/logging/logging", + nodes=infra.e2e_args.min_nodes(cr.args, f=0), # 1 node suffices for recovery + snapshot_tx_interval=10, + ) + cr.add( "recovery_expired_node_certificate_snapshot", run_recover_snapshot_from_expired_node_certificate, diff --git a/tests/start_network.py b/tests/start_network.py index a1ddc71068f..8a1ea147c2b 100644 --- a/tests/start_network.py +++ b/tests/start_network.py @@ -128,6 +128,20 @@ def run(args): LOG.warning(f"Storing previous service's cert at {backup_location}") shutil.copy(previous_service_cert, backup_location) args.previous_service_identity_file = backup_location + signing_key_file = os.path.join( + args.common_dir, "service_signing_key_classical.pem" + ) + args.previous_service_signing_key_files = ( + infra.network.save_service_signing_keys( + backup_location, + args.common_dir, + ( + {"CLASSICAL": signing_key_file} + if os.path.exists(signing_key_file) + else None + ), + ) + ) network.start_in_recovery( args,