From acee543e827fde482eae76db3efeeac6cd3950cb Mon Sep 17 00:00:00 2001 From: dimka90 Date: Tue, 13 Oct 2026 09:34:14 +0100 Subject: [PATCH 1/2] Check validator keys against the registry and reject one key for both roles KeyManager loaded each validator's attestation and proposal secret keys without checking them, and annotated_validators.yaml entries naming neither an attester nor a proposer file were loaded as one key for both roles. leanSpec's ValidatorRegistry rejects one key for both roles: a validator signs a proposal and an attestation in the same slot, and XMSS is a stateful one-time signature. A key that differs from the registry also makes every signature fail verification at peers. SecretKey now derives its public key, and loading checks each validator's two keys differ and equal its entry in the anchor state's registry. A single-key manifest entry is rejected. --- lean_client/Cargo.lock | 2 + lean_client/src/main.rs | 2 +- lean_client/validator/Cargo.toml | 4 + lean_client/validator/src/keys.rs | 102 ++++++++++++++++++++++ lean_client/validator/src/lib.rs | 27 ++++-- lean_client/validator/tests/unit_tests.rs | 21 +++++ lean_client/xmss/src/secret_key.rs | 5 ++ 7 files changed, 154 insertions(+), 9 deletions(-) diff --git a/lean_client/Cargo.lock b/lean_client/Cargo.lock index 077f2b77..a87d63d4 100644 --- a/lean_client/Cargo.lock +++ b/lean_client/Cargo.lock @@ -6118,6 +6118,8 @@ dependencies = [ "fork_choice", "futures", "metrics", + "rand 0.10.1", + "rand_chacha 0.10.0", "rayon", "serde", "serde_yaml", diff --git a/lean_client/src/main.rs b/lean_client/src/main.rs index 4055511e..af2789c9 100644 --- a/lean_client/src/main.rs +++ b/lean_client/src/main.rs @@ -723,7 +723,7 @@ async fn main() -> Result<()> { if keys_path.exists() { match ValidatorService::new_with_keys_and_aggregator( config.clone(), - num_validators, + &anchor_state.validators, keys_path, cpu_normal_executor.clone(), cpu_verify_executor.clone(), diff --git a/lean_client/validator/Cargo.toml b/lean_client/validator/Cargo.toml index 01471cbf..3f0a660d 100644 --- a/lean_client/validator/Cargo.toml +++ b/lean_client/validator/Cargo.toml @@ -20,3 +20,7 @@ typenum = { workspace = true } xmss = { workspace = true } zeroize = { workspace = true } rayon = { workspace = true } + +[dev-dependencies] +rand = { workspace = true } +rand_chacha = { workspace = true } diff --git a/lean_client/validator/src/keys.rs b/lean_client/validator/src/keys.rs index 299c052c..904f69b3 100644 --- a/lean_client/validator/src/keys.rs +++ b/lean_client/validator/src/keys.rs @@ -5,6 +5,7 @@ use tracing::info; use anyhow::anyhow; use anyhow::{Context, Result, ensure}; +use containers::Validator; use xmss::{SecretKey, Signature}; /// Manages XMSS secret keys for validators (attestation + proposal keys per validator) @@ -99,6 +100,39 @@ impl KeyManager { Ok(()) } + /// Check a validator's loaded keys against its registry entry. + /// + /// The attestation and proposal keys must differ: a validator signs a proposal and an + /// attestation in the same slot, and XMSS is a stateful one-time signature, so one key + /// shared by both roles would be reused (leanSpec `ValidatorRegistry`). Each key must + /// also equal the registry's public key, or every signature fails verification at peers. + pub fn check_keys(&self, validator_index: u64, validator: &Validator) -> Result<()> { + let attestation_key = self + .attestation_keys + .get(&validator_index) + .ok_or_else(|| anyhow!("No attestation key loaded for validator {validator_index}"))? + .public_key(); + let proposal_key = self + .proposal_keys + .get(&validator_index) + .ok_or_else(|| anyhow!("No proposal key loaded for validator {validator_index}"))? + .public_key(); + + ensure!( + attestation_key != proposal_key, + "Attestation and proposal keys must differ for validator {validator_index}, but the same key is loaded for both" + ); + ensure!( + attestation_key == validator.attestation_pubkey, + "Attestation key for validator {validator_index} does not match the validator registry" + ); + ensure!( + proposal_key == validator.proposal_pubkey, + "Proposal key for validator {validator_index} does not match the validator registry" + ); + Ok(()) + } + /// Sign an attestation message with the validator's attestation secret key. pub fn sign_attestation( &self, @@ -140,6 +174,10 @@ impl KeyManager { #[cfg(test)] mod tests { + use rand::SeedableRng; + use rand_chacha::ChaCha8Rng; + use xmss::PublicKey; + use super::*; #[test] @@ -148,4 +186,68 @@ mod tests { let result = KeyManager::new("/nonexistent/path"); assert!(result.is_err()); } + + fn key_manager(attestation: SecretKey, proposal: SecretKey) -> KeyManager { + KeyManager { + attestation_keys: HashMap::from([(0, attestation)]), + proposal_keys: HashMap::from([(0, proposal)]), + keys_dir: PathBuf::new(), + } + } + + fn validator(attestation_pubkey: &PublicKey, proposal_pubkey: &PublicKey) -> Validator { + Validator { + attestation_pubkey: attestation_pubkey.clone(), + proposal_pubkey: proposal_pubkey.clone(), + index: 0, + } + } + + #[test] + fn check_keys_matches_registry_and_distinct_roles() { + let mut rng = ChaCha8Rng::seed_from_u64(7); + let (attestation_pk, attestation_sk) = SecretKey::generate_key_pair(&mut rng, 0, 4); + let (proposal_pk, proposal_sk) = SecretKey::generate_key_pair(&mut rng, 0, 4); + let keys = key_manager(attestation_sk, proposal_sk); + + keys.check_keys(0, &validator(&attestation_pk, &proposal_pk)) + .expect("matching keys"); + + let cases = [ + ( + "swapped roles", + validator(&proposal_pk, &attestation_pk), + "Attestation key for validator 0 does not match", + ), + ( + "proposal key not in registry", + validator(&attestation_pk, &attestation_pk), + "Proposal key for validator 0 does not match", + ), + ]; + for (name, registry_entry, want) in cases { + let err = keys + .check_keys(0, ®istry_entry) + .expect_err(name) + .to_string(); + assert!( + err.contains(want), + "{name}: error {err:?} does not contain {want:?}" + ); + } + } + + #[test] + fn check_keys_rejects_one_key_for_both_roles() { + let mut rng = ChaCha8Rng::seed_from_u64(7); + let (public_key, secret_key) = SecretKey::generate_key_pair(&mut rng, 0, 4); + let mut rng = ChaCha8Rng::seed_from_u64(7); + let (_, same_secret_key) = SecretKey::generate_key_pair(&mut rng, 0, 4); + + let err = key_manager(secret_key, same_secret_key) + .check_keys(0, &validator(&public_key, &public_key)) + .expect_err("one key for both roles") + .to_string(); + assert!(err.contains("must differ"), "error {err:?}"); + } } diff --git a/lean_client/validator/src/lib.rs b/lean_client/validator/src/lib.rs index 332fee8f..cbb8882b 100644 --- a/lean_client/validator/src/lib.rs +++ b/lean_client/validator/src/lib.rs @@ -181,8 +181,6 @@ pub fn snapshot_aggregation_inputs(store: &Store) -> Option /// Entry in an annotated_validators.yaml file. /// Each validator index has two entries: one for the attester key and one for the proposer key, /// distinguished by the filename containing "attester" or "proposer". -/// Single-key format (e.g. validator_N_sk.ssz with neither keyword) is also accepted and -/// uses the same file for both attestation and proposal. #[derive(Debug, Clone, Deserialize)] pub struct AnnotatedValidatorEntry { pub index: u64, @@ -219,9 +217,12 @@ impl ValidatorConfig { } else if entry.privkey_file.contains("proposer") { slot.1 = Some(entry.privkey_file.clone()); } else { - // Single-key format: same file for both keys. - slot.0 = Some(entry.privkey_file.clone()); - slot.1 = Some(entry.privkey_file.clone()); + // One key for both roles would reuse XMSS one-time state; leanSpec rejects it. + bail!( + "privkey_file {} for validator {} names neither an attester nor a proposer key", + entry.privkey_file, + entry.index + ); } } @@ -372,7 +373,7 @@ impl ValidatorService { pub fn new_with_keys( config: ValidatorConfig, - num_validators: u64, + validators: &Validators, keys_dir: impl AsRef, cpu_normal_executor: Arc, cpu_verify_executor: Arc, @@ -380,7 +381,7 @@ impl ValidatorService { ) -> Result> { Self::new_with_keys_and_aggregator( config, - num_validators, + validators, keys_dir, cpu_normal_executor, cpu_verify_executor, @@ -389,9 +390,11 @@ impl ValidatorService { ) } + /// Loads each assigned validator's keys and checks them against `validators`, the + /// anchor state's registry; see `KeyManager::check_keys`. pub fn new_with_keys_and_aggregator( config: ValidatorConfig, - num_validators: u64, + validators: &Validators, keys_dir: impl AsRef, cpu_normal_executor: Arc, cpu_verify_executor: Arc, @@ -411,7 +414,15 @@ impl ValidatorService { } else { key_manager.load_keys(idx)?; } + let validator = validators.get(idx).map_err(|_| { + format!( + "Validator {idx} is not in the registry of {} validators", + validators.len_u64() + ) + })?; + key_manager.check_keys(idx, validator)?; } + let num_validators = validators.len_u64(); info!( node_id = %config.node_id, diff --git a/lean_client/validator/tests/unit_tests.rs b/lean_client/validator/tests/unit_tests.rs index 4bef9827..4e29bebc 100644 --- a/lean_client/validator/tests/unit_tests.rs +++ b/lean_client/validator/tests/unit_tests.rs @@ -43,3 +43,24 @@ fn test_is_assigned() { assert!(!config.is_assigned(1)); assert!(!config.is_assigned(3)); } + +#[test] +fn test_load_from_file_rejects_single_key_entry() { + let path = std::env::temp_dir().join(format!( + "annotated_validators_single_key_{}.yaml", + std::process::id() + )); + std::fs::write( + &path, + "test_0:\n - index: 0\n pubkey_hex: \"0x00\"\n privkey_file: validator_0_sk.ssz\n", + ) + .unwrap(); + + let result = ValidatorConfig::load_from_file(&path, "test_0"); + std::fs::remove_file(&path).unwrap(); + let err = result.expect_err("single-key entry").to_string(); + assert!( + err.contains("names neither an attester nor a proposer key"), + "error {err:?}" + ); +} diff --git a/lean_client/xmss/src/secret_key.rs b/lean_client/xmss/src/secret_key.rs index a26b9dc9..96b25ab8 100644 --- a/lean_client/xmss/src/secret_key.rs +++ b/lean_client/xmss/src/secret_key.rs @@ -15,6 +15,11 @@ use crate::{PublicKey, Signature}; pub struct SecretKey(XmssSecretKey); impl SecretKey { + /// Returns the public key this secret key signs for. + pub fn public_key(&self) -> PublicKey { + PublicKey::from_lean(self.0.public_key()) + } + pub fn sign(&self, message: H256, epoch: u32) -> Result { if !self.0.activation_slots().contains(&epoch) { return Err(anyhow!("epoch {epoch} outside key activation window")); From ed2595d833c22dfea77822e5a7ae64584b16482b Mon Sep 17 00:00:00 2001 From: dimka90 Date: Tue, 13 Oct 2026 09:39:29 +0100 Subject: [PATCH 2/2] Refuse to start when the given XMSS keys cannot be loaded With --hash-sig-key-dir set, a key that failed to load or did not match the validator registry, or a missing key directory, only logged a warning and the node ran with zero signatures, so a misconfigured validator kept running and produced nothing peers accept. Exit with the error instead. Without --hash-sig-key-dir the node still runs with zero signatures, as before. --- lean_client/src/main.rs | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/lean_client/src/main.rs b/lean_client/src/main.rs index af2789c9..a240f090 100644 --- a/lean_client/src/main.rs +++ b/lean_client/src/main.rs @@ -741,33 +741,18 @@ async fn main() -> Result<()> { Some(service) } Err(e) => { - warn!( - "Failed to load XMSS keys: {}, falling back to zero signatures", - e - ); - Some(ValidatorService::new_with_aggregator( - config, - num_validators, - cpu_normal_executor.clone(), - cpu_verify_executor.clone(), - cpu_snark_executor.clone(), - args.is_aggregator, - )) + error!("Failed to load XMSS keys: {}. Refusing to start.", e); + return Err(anyhow::anyhow!("failed to load XMSS keys: {e}")); } } } else { - warn!( - "Hash-sig key directory not found: {:?}, using zero signatures", + error!( + "Hash-sig key directory not found: {:?}. Refusing to start.", keys_path ); - Some(ValidatorService::new_with_aggregator( - config, - num_validators, - cpu_normal_executor.clone(), - cpu_verify_executor.clone(), - cpu_snark_executor.clone(), - args.is_aggregator, - )) + return Err(anyhow::anyhow!( + "hash-sig key directory not found: {keys_path:?}" + )); } } else { info!(