fix: bind checkpoint state position to the verified terminal header#402
fix: bind checkpoint state position to the verified terminal header#402matthias-wright wants to merge 3 commits into
Conversation
matthias-wright
force-pushed
the
m/checkpoint-header-chain
branch
from
9d32aa7 to
1d0d4e9
Compare
|
I think this is a good partial fix for the positional part of #215. The new checks correctly bind the decoded checkpoint state to the verified terminal header for:
That matches the honest checkpoint flow, where the checkpoint is created from the penultimate block and committed by the terminal block. However, I do not think this fully fixes #215 as written. The issue also calls out other decoded Those fields are still controlled by whoever constructs the checkpoint artifact. A malicious checkpoint can satisfy the new height/epoch/head checks while still choosing a forged The added test covers the Small defensive note: |
|
#422 (which builds on this PR) prevents loading a single-checkpoint file without verification (unless an unsafe flag is set), and also adds verification, such as checking that |
|
That makes sense to me. Since #422 builds on this branch and includes the checkpoint-position binding commit from #402, I think #402 can be closed as superseded once #422 is merged. #422 also covers the follow-up concerns here by requiring verified checkpoint startup by default, binding |
…acts to verified chain (#422) Builds on #402. Addresses #214. Changes: -Require verification by default. run_node_inner now panics when a checkpoint is supplied without a finalized-headers chain to verify against, instead of warning and continuing. -Add --unsafe-skip-checkpoint-verification (RunFlags, requires --checkpoint-path) as the explicit opt-out for importing a checkpoint without verification artifacts; logs a loud UNSAFE warning when used. -Bind last_block to the verified chain. On the verified path, require last_block.digest() == terminal.finalization().proposal.payload (the verified chain terminal), panicking on mismatch. -Use the signature-verified terminal as the completion finalization. Replace the separately-loaded, unverified top-level finalized_header with the chain terminal FinalizedHeader (whose certificate was verified by -verify_checkpoint_chain) before handing it to the syncer. -Cover checkpoint startup policy and harden artifact binding.
|
Superseded by #422. |
#423) Builds on #422 (which builds on #402) Addresses #216. Changes: -types/src/checkpoint.rs: add Step 5 to verify_checkpoint_chain_with_weak_subjectivity binding the decoded queues to the terminal header's committed deltas — require checkpoint_state.get_added_validators(next_epoch) to equal terminal.added_validators() and checkpoint_state.get_removed_validators() to equal terminal.removed_validators() (order-normalized), rejecting mismatches with a new CheckpointVerificationError::CheckpointTransitionQueueMismatch. -Document the inherent residual: pending additions for epochs beyond next_epoch (possible when validator warm-up spans more than one epoch) are committed by finalized headers after the terminal and cannot be authenticated by a chain ending at this epoch; they are bound only once the next epoch's boundary header is verified. -Test: add test_checkpoint_verifier_binds_transition_queues_to_terminal_header (honest empty-queue checkpoint verifies; a checkpoint carrying a pending removed_validators entry while the terminal header commits an empty set is rejected, with hash/membership/position checks still passing). Parameterize checkpoint_verification_fixture with the checkpoint's removed_validators queue and update its callers.
…425) Builds on #423 (which builds on #422 and #402). Addresses #217. Changes: -Step 3 now accumulates a node-key to BLS-key map and requires each checkpoint account's consensus_public_key to equal the BLS key accumulated for that node from the verified history. Mismatch returns the new CheckpointConsensusKeyMismatch error variant. The reverse membership check uses the same map. -Add test_checkpoint_verifier_binds_consensus_keys_to_terminal_header covering both directions: the honest case (accounts carry the real, accumulated consensus keys and verify) and a swapped consensus key (rejected with CheckpointConsensusKeyMismatch). The shared checkpoint_verification_fixture gains a tamper_consensus_key parameter that swaps one active account's stored consensus key while leaving its node key, status, and the accumulated set intact. -Bind checkpoint consensus keys to the verified terminal header.
Builds on #426 (which builds on #425, #423, #422, and #402). Addresses #311. This only adds test coverage, the issue is not reachable. The decoded ConsensusState, including every validator_accounts entry, is committed by checkpoint.data, which verify_checkpoint_chain binds to the terminal finalized header via checkpoint_hash == sha256(checkpoint.data). Appending a Joining account changes the digest, so the tampered checkpoint no longer matches the honest terminal header and is rejected at Step 2. Changes: -types/src/checkpoint.rs: test_checkpoint_verifier_rejects_extra_joining_account — reproduces the attack directly. It decodes the honest checkpoint, injects an extra Joining account (leaving the active signing set untouched), re-encodes, and asserts verify_checkpoint_chain rejects it with CheckpointHashMismatch. -application/src/actor.rs: rejects_block_with_mismatched_checkpoint_hash — the consensus-layer half. It mirrors accepts_ordinary_child_inside_epoch, changing only the block's checkpoint_hash, and asserts handle_verify returns false, isolating the checkpoint_hash check as the sole cause.
2 participants
Addresses #215.
Changes: