fix: bind checkpoint consensus keys to the verified terminal header#425
Conversation
|
I think this is a good partial fix for the current active-validator key-swap case, but I think there is still a bypass for validators that are queued to join. The new check binds That matters because epoch activation later loads the account by node key and only changes: account.status = ValidatorStatus::Active;It does not overwrite So a malicious checkpoint can make I think the verifier should also enforce: validator_accounts[added.node_key].consensus_public_key == added.consensus_keyfor every validator in |
9639565 to
bfc4fc9
Compare
|
Update(bfc4fc9):
|
|
This looks good to me now. |
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.
Builds on #423 (which builds on #422 and #402).
Addresses #217.
Changes: