diff --git a/Cargo.lock b/Cargo.lock index 5000ce4eb..7a3fc5ee8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12498,6 +12498,7 @@ dependencies = [ "url", "utoipa", "uuid", + "zeroize", ] [[package]] diff --git a/crates/core/tests/e2e_authenticator_insert_update_remove.rs b/crates/core/tests/e2e_authenticator_insert_update_remove.rs index 2fe57c4d0..159225593 100644 --- a/crates/core/tests/e2e_authenticator_insert_update_remove.rs +++ b/crates/core/tests/e2e_authenticator_insert_update_remove.rs @@ -5,7 +5,6 @@ use std::time::Duration; use alloy::{ primitives::{Address, U256}, providers::ProviderBuilder, - signers::local::PrivateKeySigner, }; use eddsa_babyjubjub::{EdDSAPrivateKey, EdDSAPublicKey}; use reqwest::Client; @@ -16,7 +15,7 @@ use world_id_core::{ use world_id_gateway::{ BatchPolicyConfig, GatewayConfig, SignerArgs, defaults, spawn_gateway_for_tests, }; -use world_id_primitives::{Config, TREE_DEPTH, merkle::AccountInclusionProof}; +use world_id_primitives::{Config, Signer, TREE_DEPTH, merkle::AccountInclusionProof}; use world_id_test_utils::{ anvil::{TestAnvil, WorldIDRegistry}, fixtures::{MerkleFixture, single_leaf_merkle_fixture}, @@ -64,11 +63,15 @@ fn make_inclusion_proof( AccountInclusionProof::<{ TREE_DEPTH }>::new(inclusion_proof, key_set) } -// Derives keys from seed using same logic as Authenticator's internal Signer +// Derives keys from a master seed using the same domain-separated KDF as the +// Authenticator's internal `Signer`, so the test-side pubkey/address match +// what `Authenticator::init` derives from the same seed. fn derive_keys_from_seed(seed: [u8; 32]) -> (EdDSAPublicKey, Address) { - let onchain = PrivateKeySigner::from_bytes(&seed.into()).unwrap(); - let offchain = EdDSAPrivateKey::from_bytes(seed); - (offchain.public(), onchain.address()) + let signer = Signer::from_seed_bytes(&seed).unwrap(); + ( + signer.offchain_signer_pubkey(), + signer.onchain_signer_address(), + ) } fn make_config( diff --git a/crates/core/tests/issuer_registration.rs b/crates/core/tests/issuer_registration.rs index 434fd9794..a95b94f6a 100644 --- a/crates/core/tests/issuer_registration.rs +++ b/crates/core/tests/issuer_registration.rs @@ -1,8 +1,10 @@ #![cfg(feature = "issuer")] +use alloy::{primitives::U256, providers::Provider as _}; use eyre::Result; use taceo_oprf_test_utils::PEER_ADDRESSES; use world_id_core::Issuer; +use world_id_primitives::Signer; use world_id_test_utils::anvil::{CredentialSchemaIssuerRegistry, TestAnvil}; /// Complete test for registering an issuer schema @@ -10,33 +12,38 @@ use world_id_test_utils::anvil::{CredentialSchemaIssuerRegistry, TestAnvil}; async fn test_register_issuer_schema() -> Result<()> { let anvil = TestAnvil::spawn()?; - let issuer_signer = anvil.signer(0)?; - let issuer_seed_bytes: [u8; 32] = issuer_signer.to_bytes().into(); + let deployer = anvil.signer(0)?; - let oprf_key_registry = anvil - .deploy_oprf_key_registry(issuer_signer.clone()) - .await?; + let oprf_key_registry = anvil.deploy_oprf_key_registry(deployer.clone()).await?; // Register OPRF nodes (required before initKeyGen can be called) anvil - .register_oprf_nodes( - oprf_key_registry, - issuer_signer.clone(), - PEER_ADDRESSES.to_vec(), - ) + .register_oprf_nodes(oprf_key_registry, deployer.clone(), PEER_ADDRESSES.to_vec()) .await?; let issuer_registry_address = anvil - .deploy_credential_schema_issuer_registry(issuer_signer.clone(), oprf_key_registry) + .deploy_credential_schema_issuer_registry(deployer.clone(), oprf_key_registry) .await?; // Add CredentialSchemaIssuerRegistry as OprfKeyRegistry admin so it can call initKeyGen anvil - .add_oprf_key_registry_admin( - oprf_key_registry, - issuer_signer.clone(), - issuer_registry_address, - ) + .add_oprf_key_registry_admin(oprf_key_registry, deployer.clone(), issuer_registry_address) + .await?; + + // The issuer's on-chain SECP256K1 key is derived from `issuer_seed_bytes` via a + // domain-separated KDF (see `Signer::from_seed_bytes`), so the resulting on-chain + // address is **not** equal to any of anvil's pre-funded mnemonic accounts. We + // therefore pre-fund the derived address with `anvil_setBalance` so the issuer + // can pay for its own gas when calling `register`. + let issuer_seed_bytes: [u8; 32] = [42u8; 32]; + let issuer_signer_address = + Signer::from_seed_bytes(&issuer_seed_bytes)?.onchain_signer_address(); + + let provider = anvil.provider()?; + let one_eth = U256::from(10).pow(U256::from(18)); + let _: () = provider + .client() + .request("anvil_setBalance", (issuer_signer_address, one_eth)) .await?; let mut issuer = Issuer::new( @@ -48,16 +55,13 @@ async fn test_register_issuer_schema() -> Result<()> { let issuer_schema_id = 1u64; issuer.register_schema(issuer_schema_id).await?; - let provider = anvil.provider()?; let registry = CredentialSchemaIssuerRegistry::new(issuer_registry_address, provider); - let signer_address = issuer_signer.address(); - let registered_signer = registry .getSignerForIssuerSchemaId(issuer_schema_id) .call() .await?; - assert_eq!(registered_signer, signer_address); + assert_eq!(registered_signer, issuer_signer_address); Ok(()) } diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index c1efcf31f..417635803 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -44,6 +44,7 @@ strum = { workspace = true, features = ["derive"] } utoipa = { workspace = true, optional = true } uuid = { workspace = true, features = ["js"] } sha2 = { workspace = true } +zeroize = { workspace = true } embed-doc-image = "0.1.4" # WASM support: getrandom needs 'js' feature in browsers diff --git a/crates/primitives/src/signer.rs b/crates/primitives/src/signer.rs index ade1f3a2f..492b1500b 100644 --- a/crates/primitives/src/signer.rs +++ b/crates/primitives/src/signer.rs @@ -2,9 +2,21 @@ use crate::PrimitiveError; use alloy::{primitives::Address, signers::local::PrivateKeySigner}; use eddsa_babyjubjub::{EdDSAPrivateKey, EdDSAPublicKey}; use secrecy::{ExposeSecret, SecretBox}; +use sha2::{Digest as _, Sha256}; +use zeroize::Zeroizing; + +/// Domain-separation tag used to derive the on-chain `SECP256K1` signing key from the master seed. +const ONCHAIN_KEY_DERIVATION_TAG: &[u8] = b"world-id-protocol/signer/onchain/v1"; +/// Domain-separation tag used to derive the off-chain `EdDSA` signing key from the master seed. +const OFFCHAIN_KEY_DERIVATION_TAG: &[u8] = b"world-id-protocol/signer/offchain/v1"; /// The inner signer which can sign requests for both on-chain and off-chain operations. Both issuers and authenticators use this. /// +/// The on-chain and off-chain keys are derived from the same master seed via a +/// domain-separated SHA-256 KDF, so leaking either signing key does not let an +/// attacker recover the master seed (one-way hash) and therefore does not allow +/// recovery of the other key. +/// /// Both keys are zeroized on drop. #[derive(Debug)] pub struct Signer { @@ -15,28 +27,36 @@ pub struct Signer { } impl Signer { - /// Initializes a new signer from an input seed. + /// Initializes a new signer from a 32-byte master seed. + /// + /// The on-chain (`SECP256K1`) and off-chain (`EdDSA` on `BabyJubJub`) signing keys + /// are derived independently from `master_seed` via `SHA-256(domain_tag || master_seed)`, + /// so the two keys are not linkable: compromising one does not reveal the master seed + /// and therefore does not let an attacker derive the other key. /// /// # Errors - /// Returns `PrimitiveError::InvalidInput` if the seed is not exactly 32 bytes. - pub fn from_seed_bytes(seed: &[u8]) -> Result { - if seed.len() != 32 { + /// Returns `PrimitiveError::InvalidInput` if `master_seed` is not exactly 32 bytes, + /// or if the derived on-chain private key is rejected by `SECP256K1` (negligibly + /// probable for a uniformly random seed). + pub fn from_seed_bytes(master_seed: &[u8]) -> Result { + if master_seed.len() != 32 { return Err(PrimitiveError::InvalidInput { attribute: "seed".to_string(), - reason: format!("must be 32 bytes, got {} bytes", seed.len()), + reason: format!("must be 32 bytes, got {} bytes", master_seed.len()), }); } - let bytes: [u8; 32] = seed.try_into().map_err(|_| PrimitiveError::InvalidInput { - attribute: "seed".to_string(), - reason: "failed to convert to [u8; 32]".to_string(), - })?; - let onchain_signer = PrivateKeySigner::from_bytes(&bytes.into()).map_err(|e| { - PrimitiveError::InvalidInput { - attribute: "seed".to_string(), - reason: format!("invalid private key: {e}"), - } - })?; - let offchain_signer = SecretBox::new(Box::new(EdDSAPrivateKey::from_bytes(bytes))); + + let onchain_seed = derive_subkey(ONCHAIN_KEY_DERIVATION_TAG, master_seed); + let offchain_seed = derive_subkey(OFFCHAIN_KEY_DERIVATION_TAG, master_seed); + + let onchain_signer = + PrivateKeySigner::from_bytes(&(*onchain_seed).into()).map_err(|e| { + PrimitiveError::InvalidInput { + attribute: "seed".to_string(), + reason: format!("invalid derived on-chain private key: {e}"), + } + })?; + let offchain_signer = SecretBox::new(Box::new(EdDSAPrivateKey::from_bytes(*offchain_seed))); Ok(Self { onchain_signer, @@ -68,3 +88,90 @@ impl Signer { self.offchain_signer.expose_secret().public() } } + +/// Derives a 32-byte subkey from `master_seed` under a domain-separation `tag` using +/// `SHA-256(tag || master_seed)`. +/// +/// The output is wrapped in [`Zeroizing`] so the derived secret bytes are wiped +/// from memory when the value is dropped. +fn derive_subkey(tag: &[u8], master_seed: &[u8]) -> Zeroizing<[u8; 32]> { + let mut hasher = Sha256::new(); + hasher.update(tag); + hasher.update(master_seed); + let digest = hasher.finalize(); + + let mut out = Zeroizing::new([0u8; 32]); + out.copy_from_slice(&digest); + out +} + +#[cfg(test)] +mod tests { + use super::*; + + const TEST_SEED_A: [u8; 32] = [0x11; 32]; + const TEST_SEED_B: [u8; 32] = [0x22; 32]; + + #[test] + fn from_seed_bytes_rejects_wrong_length() { + let err = Signer::from_seed_bytes(&[0u8; 31]).unwrap_err(); + assert!(matches!(err, PrimitiveError::InvalidInput { .. })); + + let err = Signer::from_seed_bytes(&[0u8; 33]).unwrap_err(); + assert!(matches!(err, PrimitiveError::InvalidInput { .. })); + } + + #[test] + fn from_seed_bytes_is_deterministic() { + let s1 = Signer::from_seed_bytes(&TEST_SEED_A).unwrap(); + let s2 = Signer::from_seed_bytes(&TEST_SEED_A).unwrap(); + + assert_eq!(s1.onchain_signer_address(), s2.onchain_signer_address()); + assert_eq!( + s1.offchain_signer_pubkey().pk, + s2.offchain_signer_pubkey().pk + ); + } + + #[test] + fn different_master_seeds_produce_different_keys() { + let s1 = Signer::from_seed_bytes(&TEST_SEED_A).unwrap(); + let s2 = Signer::from_seed_bytes(&TEST_SEED_B).unwrap(); + + assert_ne!(s1.onchain_signer_address(), s2.onchain_signer_address()); + assert_ne!( + s1.offchain_signer_pubkey().pk, + s2.offchain_signer_pubkey().pk + ); + } + + /// Regression test for the linkability finding: the raw 32-byte master seed must + /// not be reused as either signing key. The bytes fed into the on-chain signer + /// and the bytes fed into the off-chain signer must both differ from the master + /// seed and from each other. + #[test] + fn onchain_and_offchain_keys_are_unlinked_from_master_seed() { + let master = TEST_SEED_A; + + let onchain_seed = derive_subkey(ONCHAIN_KEY_DERIVATION_TAG, &master); + let offchain_seed = derive_subkey(OFFCHAIN_KEY_DERIVATION_TAG, &master); + + assert_ne!(*onchain_seed, master); + assert_ne!(*offchain_seed, master); + assert_ne!(*onchain_seed, *offchain_seed); + + let signer = Signer::from_seed_bytes(&master).unwrap(); + let onchain_from_derived = PrivateKeySigner::from_bytes(&(*onchain_seed).into()).unwrap(); + let offchain_signer_derived = EdDSAPrivateKey::from_bytes(*offchain_seed); + assert_eq!( + signer.onchain_signer_address(), + onchain_from_derived.address(), + "on-chain key must come from the derived sub-seed, not the master seed", + ); + assert_eq!( + signer.offchain_signer_pubkey(), + offchain_signer_derived.public(), + "off-chain key must come from the derived sub-seed, not the master seed", + ) + } +} diff --git a/services/gateway/tests/test_inflight.rs b/services/gateway/tests/test_inflight.rs index 7010f57e2..a2bc34820 100644 --- a/services/gateway/tests/test_inflight.rs +++ b/services/gateway/tests/test_inflight.rs @@ -2,11 +2,11 @@ use alloy::{ primitives::{Address, U256}, signers::local::PrivateKeySigner, }; -use eddsa_babyjubjub::{EdDSAPrivateKey, EdDSAPublicKey}; +use eddsa_babyjubjub::EdDSAPublicKey; use reqwest::StatusCode; use world_id_authenticator::{Authenticator, AuthenticatorError, OnchainKeyRepresentable}; use world_id_primitives::{ - Config, TREE_DEPTH, + Config, Signer, TREE_DEPTH, api_types::{ GatewayRequestId, GatewayRequestKind, GatewayStatusResponse, RecoverAccountRequest, }, @@ -65,10 +65,15 @@ fn make_inclusion_proof( AccountInclusionProof::<{ TREE_DEPTH }>::new(inclusion_proof, key_set) } +// Derives keys from a master seed using the same domain-separated KDF as the +// Authenticator's internal `Signer`, so the test-side pubkey/address match +// what `Authenticator::init` derives from the same seed. fn derive_keys_from_seed(seed: [u8; 32]) -> (EdDSAPublicKey, Address) { - let onchain = PrivateKeySigner::from_bytes(&seed.into()).unwrap(); - let offchain = EdDSAPrivateKey::from_bytes(seed); - (offchain.public(), onchain.address()) + let signer = Signer::from_seed_bytes(&seed).unwrap(); + ( + signer.offchain_signer_pubkey(), + signer.onchain_signer_address(), + ) } /// Register a new account and wait for on-chain finalization. diff --git a/services/oprf-dev-client/src/lib.rs b/services/oprf-dev-client/src/lib.rs index 45bf605d2..27c3f7e2a 100644 --- a/services/oprf-dev-client/src/lib.rs +++ b/services/oprf-dev-client/src/lib.rs @@ -14,7 +14,9 @@ use world_id_core::{ Authenticator, AuthenticatorError, EdDSAPrivateKey, EdDSASignature, FieldElement, proof::{CircomGroth16Material, errors}, }; -use world_id_primitives::{AuthenticatorPublicKeySet, TREE_DEPTH, merkle::MerkleInclusionProof}; +use world_id_primitives::{ + AuthenticatorPublicKeySet, Signer, TREE_DEPTH, merkle::MerkleInclusionProof, +}; use world_id_proof::circuit_inputs::QueryProofCircuitInput; const HARDCODED_RP_SIGNER: &str = @@ -201,6 +203,13 @@ pub async fn init_authenticator( let authenticator = Authenticator::init_or_register(&seed, world_config.into(), None) .await? .with_proof_materials(query_material, Arc::new(nullifier_material)); - let authenticator_private_key = EdDSAPrivateKey::from_bytes(seed); + // Mirror the Authenticator's internal off-chain key derivation so the + // returned private key matches the registered off-chain pubkey. The raw + // `seed` bytes are no longer used directly as an EdDSA secret since + // `Signer::from_seed_bytes` derives sub-keys via a domain-separated KDF. + let authenticator_private_key = Signer::from_seed_bytes(&seed)? + .offchain_signer_private_key() + .expose_secret() + .clone(); Ok((authenticator, authenticator_private_key)) }