Skip to content

Conversation

@cjen1-msft
Copy link
Contributor

@cjen1-msft cjen1-msft commented Jan 6, 2026

The current local sealing approach can end up with skew between what the latest ledger secret and the sealed secret.

The idea is that we can force these to be in sync by sealing to the ledger and unsealing from it.

The complication is that these changes must occur in the same transaction as any ledger secret updates, and so the primary must be able to seal for other nodes.
Hence nodes include a sealed private key, and corresponding public key in their network info, and the primary uses the public key to seal the ledger secret.
Thus a recovering node can derive the sealing key, read the ledger to unseal the private key and decrypt the sealed ledger secret.

This process is summarised below:

    flowchart TB
        subgraph SNP["SNP PSP"]
            DK["DERIVED_KEY"]
            VCEK
            Measurement
            Policy["UserData (Policy)"]
            TCB

            VCEK --> DK
            Measurement --> DK
            Policy --> DK            
            TCB --> DK
        end

        subgraph KG["Key Generation"]
            subgraph Sealing Key
                SK["Sealing Key<br/>(HKDF)"]
                Label["Label: <br/>CCF_LOCAL_SEALING_KEY"]
                DK -->|ikm| SK
                Label -->|info| SK
            end
            
            RSA["Recovery Key<br/>(RSA Key Pair)"]
            PubKey["Public Key"]
            PrivKey["Private Key"]
            RSA --> PubKey
            RSA --> PrivKey

            LS["Ledger secret"]
            LSWK["Ledger secret wrapping key"]
        end

        subgraph Sealed["Store: nodes.sealed_recovery_keys"]
            SPK["Sealed Private Key<br/>(AES-GCM encrypted)"]
            SK -->|key| SPK
            PrivKey --> SPK

            StoredPubKey["Public Key (plaintext)"]
            PubKey --> StoredPubKey
        end


        subgraph Shares["Store: gov.sealed_shares table"]
            WLS["Wrapped Ledger Secret<br/>(AES-GCM encrypted)"]
            LSWK -->|key| WLS
            LS --> WLS

            EWK["Encrypted Wrapping Key<br/>(per-node, RSA-OAEP encrypted)"]
            StoredPubKey -->|key| EWK
            LSWK --> EWK
        end

        subgraph Recovery["Recovery Process"]
            UPK["Unsealed Private Key"]
            SK -->|key| UPK
            SPK --> UPK

            UWK["Unsealed Wrapping Key"]
            UPK -->|key| UWK
            EWK --> UWK

            ULS["Unsealed Ledger Secret"]
            UWK -->|key| ULS
            WLS --> ULS            
        end
Loading

);
if (ccf.node.shuffleSealedShares !== undefined) {
ccf.node.shuffleSealedShares();
}
Copy link
Contributor Author

@cjen1-msft cjen1-msft Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunate but required as 6.X does not have shuffleSealedShares.
Without this call, nodes which join the network must wait until the next rekey before they are recoverable.
To be clear this PR always ensures that if you can read recovery shares from the ledger for yourself, you have the latest ledger secret, its just that these nodes would not be able to get recovery shares and complete recovery until the rekey on the original network.

The alternative is to use a hook on the nodes table, in which case a joining node would be recoverable after the hook's transaction is committed, which should only be after a few seconds, but this change shuffles the shares in the same transaction, and so means that there is desync risk. (modulo during migration)

Comment on lines +105 to +108

// Do we also need to cleanse the recovery_key_pair or will openssl handle
// that in the EVP_KEY destructor?
OPENSSL_cleanse(derived_key.data(), derived_key.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is necessary, and I can't figure it out from the openssl docs.

Suggested change
// Do we also need to cleanse the recovery_key_pair or will openssl handle
// that in the EVP_KEY destructor?
OPENSSL_cleanse(derived_key.data(), derived_key.size());
OPENSSL_cleanse(derived_key.data(), derived_key.size());


auto* sealed_recovery_keys =
args.tx.template rw<SealedRecoveryKeys>(Tables::SEALED_RECOVERY_KEYS);
sealed_recovery_keys->remove(node_id);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the only place where nodes get cleaned up? I couldn't find anywhere else obvious but I may be missing something :)

@cjen1-msft cjen1-msft added the run-long-test Run Long Test job label Jan 19, 2026
@cjen1-msft cjen1-msft marked this pull request as ready for review January 20, 2026 15:13
@cjen1-msft cjen1-msft requested a review from a team as a code owner January 20, 2026 15:13
Copilot AI review requested due to automatic review settings January 20, 2026 15:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request significantly improves the local sealing mechanism for CCF by moving from a disk-based approach to a ledger-based approach. The key changes include:

Purpose:
The PR addresses potential skew issues between the latest ledger secret and sealed secrets by ensuring synchronization through ledger storage. Nodes now seal recovery keys that allow the primary to seal ledger secrets for all nodes, enabling robust disaster recovery.

Changes:

  • Replaced disk-based sealed secret storage with ledger-based storage in two new tables
  • Introduced RSA key pairs for recovery, with private keys sealed using SNP-derived keys
  • Enabled the primary to seal ledger secrets for other nodes using their public keys
  • Updated configuration to use enable_local_sealing and previous_local_sealing_identity instead of file paths

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/node/local_sealing.cpp New implementation of ledger-based sealing with RSA key pairs and AES-GCM encryption
src/node/local_sealing.h Simplified header exposing public API for sealing operations
src/service/tables/local_sealing.h New table definitions for sealed recovery keys and sealed shares
include/ccf/service/local_sealing.h Public interface defining SealedRecoveryKey structure
src/node/node_state.h Integration of sealing during node join and recovery
src/node/share_manager.h Share management refactored to support sealed shares alongside recovery shares
src/node/rpc/node_frontend.h RPC handlers updated to store/remove sealed recovery keys and shuffle sealed shares
src/node/rpc/node_call_types.h Added sealed_recovery_key field to join/create node structures
src/js/extensions/ccf/node.cpp Added JS API for shuffling sealed shares
samples/constitutions/default/actions.js Constitution updated to shuffle sealed shares when nodes become trusted
tests/infra/remote.py Updated test infrastructure to use new configuration parameters
tests/infra/node.py Removed disk-based sealed secret management code
tests/e2e_operations.py Updated tests to use ledger-based approach, removed disk corruption test
doc/operations/recovery.rst Comprehensive documentation of new local sealing approach with diagrams
doc/audit/builtin_maps.rst Documentation of new sealed_shares and sealed_recovery_keys tables
CMakeLists.txt Added local_sealing.cpp to build

Copy link
Contributor

@fournet fournet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks!

To be on the safe side, we could also document that the authenticity of the key materials encrypted to the node public key stems from its inclusion in the ledger (not just successful decryption using the sealed private key).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants