"Previous service signing keys" replace cert - #8477
Open
Max (maxtropets) wants to merge 1 commit into
Open
Max (maxtropets) wants to merge 1 commit into
Max (maxtropets) wants to merge 1 commit into
Conversation
3 of 8 tasks
Max (maxtropets)
force-pushed
the
f/prev-identity-becomes-keys
branch
3 times, most recently
from
October 5, 2026 18:39
b81e48d to
5cab32a
Compare
Max (maxtropets)
changed the title
Max (maxtropets)
changed the title
Max (maxtropets)
force-pushed
the
f/prev-identity-becomes-keys
branch
from
October 5, 2026 19:52
5cab32a to
62b8d46
Compare
Max (maxtropets)
marked this pull request as ready for review
October 5, 2026 19:53
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 1
Open (4)
transition_service_to_open_with_signing_keysonly whenremote_node.version is None… · NewX509_verifyreturns0(signature mismatch), there may be no new OpenSSL error on the error… · Newkey_files->at(SigningKeyType::CLASSICAL)will throwstd::out_of_rangeif the JSON object… · New
What changed in this PR
This PR adds support for opening/recovering a service using service signing public keys (instead of X.509 service certificates), including new configuration fields, governance actions, and test/doc updates.
Changes:
- Introduces
ccf::ServiceSigningKeys/ccf::SigningKeyTypeand outputs service signing key PEM files at startup. - Extends recovery/startup inputs and snapshot verification to accept previous service signing keys (with optional subject override for key-only recovery).
- Updates test infrastructure, sample constitutions/apps, schemas, and docs to exercise and describe the new behavior.
| File | Description |
|---|---|
| tests/start_network.py | Saves previous service signing key files alongside prior service identity during recovery start. |
| tests/recovery.py | Switches recovery opening to get_previous_service_identity, adds signing-keys-only recovery scenarios and checks. |
| tests/partitions_test.py | Uses consolidated previous-service identity/signing-keys helper when opening recovered service. |
| tests/infra/remote.py | Adds conditional handling for signing-key recovery config and file retrieval. |
| tests/infra/network.py | Adds helpers to derive/save signing public keys and plumbs them into recovery/open flows. |
| tests/infra/consortium.py | Extends transition_service_to_open to optionally emit signing-keys-based proposal action. |
| tests/e2e_operations.py | Uses save_service_identity(args) (now also capturing signing key files) during recovery-related flows. |
| tests/config.jinja | Adds recover config fields for subject override and previous signing key file map. |
| src/node/test/snapshotter.cpp | Adds unit test coverage for legacy JSON receipt verification using signing keys. |
| src/node/test/identity_types.cpp | Adds JSON round-trip tests for signing key file maps and signing key objects. |
| src/node/startup_inputs.h | Adds subject-name handling and previous signing key ingestion for key-only recovery. |
| src/node/snapshot_serdes.h | Extends snapshot receipt verification to use signing keys (COSE + JSON). |
| src/node/rpc/test/node_frontend_test.cpp | Updates node config/startup input tests for new optional fields and subject behavior. |
| src/node/rpc/gov_effects_interface.h | Extends governance identities to support either certs or signing keys. |
| src/node/node_state.h | Enforces “certs xor signing keys” and validates signing keys when opening recovered service. |
| src/node/identity.h | Adds helper to derive previous service signing key from either signing keys or certificate. |
| src/node/gov/extensions/node.h | Documents new governance extension transitionServiceToOpenWithSigningKeys. |
| src/node/gov/extensions/node.cpp | Implements JS extension to transition service to open using signing keys. |
| src/host/run.cpp | Outputs service signing public key files at startup (in addition to service cert). |
| src/enclave/main.cpp | Plumbs signing keys through enclave create-new-node call. |
| src/enclave/entry_points.h | Updates enclave entrypoint signature to return service signing keys. |
| src/enclave/enclave.h | Copies out generated service signing keys to the host. |
| src/crypto/test/crypto.cpp | Adds unit test for verifying certificate signature with a trusted public key. |
| src/crypto/openssl/verifier.h | Adds Verifier_OpenSSL::verify_certificate_signature. |
| src/crypto/openssl/verifier.cpp | Implements certificate-signature verification helper. |
| samples/minimal_ccf/app/actions.js | Adds governance action transition_service_to_open_with_signing_keys and validators. |
| samples/constitutions/default/actions.js | Mirrors signing-keys transition action and validation in default constitution. |
| samples/config/start_config.json | Adds command.service_signing_key_files output mapping. |
| samples/config/recover_config.json | Adds signing key outputs and previous signing-key recovery inputs (plus subject). |
| include/ccf/service_signing_keys.h | Adds public header defining signing key types and container. |
| include/ccf/node/configuration.h | Adds signing key output paths and new recover config options (optional identity, key files, subject). |
| doc/host_config_schema/host_config.json | Extends schema for signing key outputs and key-only recovery requirements. |
| doc/governance/accept_recovery.rst | Documents signing-keys-based recovery opening and configuration expectations. |
| CHANGELOG.md | Notes new proposal action and deprecations for previous-service certificate usage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+749
to
+757
| 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(), | ||
| } |
Comment on lines
+190
to
+203
| 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; |
Comment on lines
257
to
+270
| static void verify_cose_snapshot_receipt( | ||
| const SnapshotSegments& segments, | ||
| const std::optional<std::vector<uint8_t>>& prev_service_identity) | ||
| const std::optional<std::vector<uint8_t>>& prev_service_identity, | ||
| const std::optional<ServiceSigningKeys>& 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()); |
Comment on lines
+219
to
+230
| 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"))); | ||
| } |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Toggling another checkbox on #7848.
Is supposed to be purely config-inspired change
This doesn't yet affect KV or endorsements chains, they will come shortly in separate PR(s).