diff --git a/keep-core/src/entropy.rs b/keep-core/src/entropy.rs index dc32cffb..dac85ebd 100644 --- a/keep-core/src/entropy.rs +++ b/keep-core/src/entropy.rs @@ -6,6 +6,13 @@ use blake2::{Blake2b512, Digest}; use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; use std::sync::OnceLock; +use crate::frost::{frozen_positions, pair_looks_random, MAX_FROZEN_POSITIONS}; + +#[cfg(test)] +use crate::frost::{hamming_distance, MIN_HAMMING}; + +pub use crate::frost::EntropyHealthError; + const POOL_SIZE: usize = 128; static PROCESS_CONTEXT_COUNTER: AtomicU64 = AtomicU64::new(0); @@ -272,121 +279,14 @@ pub fn random_bytes() -> [u8; N] { try_random_bytes().expect("RNG health check failed: constant or zero output detected") } -/// Why a draw could not be trusted. -/// -/// The two arms are kept apart because they call for different responses. A -/// source that answered with constant output is a property of the machine and -/// will not improve on its own, so a co-signer should stop and stay stopped. -/// A call that failed to complete is a different statement, closer to "no file -/// descriptors" than to "the kernel is broken", and treating it as permanent -/// turns a transient fault into an outage that needs an operator to clear. -/// They also read differently in a log, which matters when someone is trying to -/// work out what happened. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum EntropyHealthError { - /// The source answered, and what it returned failed the health criteria. - Degraded, - /// The source could not be read at all. - Unavailable, -} - -impl std::fmt::Display for EntropyHealthError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Degraded => write!( - f, - "RNG health check failed: constant or zero output detected" - ), - Self::Unavailable => write!(f, "RNG unavailable: the OS refused to provide entropy"), - } - } -} - -impl std::error::Error for EntropyHealthError {} - -fn hamming_distance(a: &[u8; 32], b: &[u8; 32]) -> u32 { - a.iter().zip(b).map(|(x, y)| (x ^ y).count_ones()).sum() -} - -const MIN_HAMMING: u32 = 64; - -/// The most byte positions that may hold the same value in every sample before -/// the samples are refused. Only meaningful across three or more samples. -pub(crate) const MAX_FROZEN_POSITIONS: usize = 3; - -/// How many of the 32 byte positions hold the same value, whatever it is, in -/// every one of `samples`. -/// -/// This replaces a test that refused any position holding `0x00` or `0xff` in -/// every sample, which got both its rate and its coverage wrong. -/// -/// Its rate: that happens to healthy randomness about once in 260,000 -/// three-sample calls, measured at 12 in 3,000,000 checked draws against the -/// analytic 11.4. Every failure latches a co-signing node, and the periodic -/// check runs every 30 seconds, so a healthy always-on node took itself out of -/// service about once a month. -/// -/// Its coverage: it caught a frozen extreme byte, and any number of them, but -/// never a frozen byte of any other value. A source that fills part of a buffer -/// and leaves the rest as it was is the case that matters, because several -/// draws turn bytes straight into a scalar with no hash between: the dealer's -/// key, every polynomial coefficient, refresh and repair deltas, and the DKG -/// proof-of-knowledge nonce. Twelve frozen bytes there leave 160 bits in known -/// positions, about 2^80 work to recover, and the Hamming floor does not catch -/// it until roughly half the bytes are frozen. -/// -/// Counting positions frozen at any value, and refusing above -/// [`MAX_FROZEN_POSITIONS`], fixes both. A position matches across three -/// independent samples with probability 1 in 65,536, so four or more of 32 -/// doing so is a binomial tail of about 2e-15, the same order as the Hamming -/// floor, and no healthy node latches on it. What it still lets through, one to -/// three frozen positions, leaves at least 232 bits. -/// -/// Three samples is the floor. Across two, a position matches with probability -/// 1 in 256, and four of 32 matching becomes about 8e-6 per call, which is the -/// rate this exists to get away from. -pub(crate) fn frozen_positions(samples: &[[u8; 32]]) -> usize { - debug_assert!( - samples.len() >= 3, - "frozen_positions needs three samples; below that healthy input trips it" - ); - let Some((first, rest)) = samples.split_first() else { - return 0; - }; - (0..32) - .filter(|&pos| rest.iter().all(|s| s[pos] == first[pos])) - .count() -} - -/// Applies the health criteria to three samples: none all-zero, all distinct, -/// no more than [`MAX_FROZEN_POSITIONS`] byte positions holding the same value -/// across all three, and every pair at least [`MIN_HAMMING`] bits apart. +/// Applies the health criteria to three samples: every pair passes +/// [`pair_looks_random`], and no more than [`MAX_FROZEN_POSITIONS`] byte +/// positions hold the same value across all three. fn samples_look_random(samples: &[[u8; 32]; 3]) -> bool { - let all_nonzero = samples.iter().all(|s| s.iter().any(|&b| b != 0)); - let all_unique = - samples[0] != samples[1] && samples[1] != samples[2] && samples[0] != samples[2]; - - if !all_nonzero || !all_unique || frozen_positions(samples) > MAX_FROZEN_POSITIONS { - return false; - } - [(0, 1), (1, 2), (0, 2)] .iter() - .all(|&(i, j)| hamming_distance(&samples[i], &samples[j]) >= MIN_HAMMING) -} - -/// The health criteria for a pair of draws, for a caller judging the bytes it -/// was handed rather than a sample drawn beside them: neither draw all-zero, -/// the two distinct, and at least [`MIN_HAMMING`] bits apart, which a source -/// repeating itself with a handful of bits flipped still fails. -/// -/// These are the same criteria [`check_entropy_health`] applies to each of its -/// pairs. The frozen-position count is not among them, because across two -/// samples healthy randomness trips it often enough to latch a node; it needs -/// three, see [`frozen_positions`]. -pub(crate) fn pair_looks_random(a: &[u8; 32], b: &[u8; 32]) -> bool { - let nonzero = |s: &[u8; 32]| s.iter().any(|&b| b != 0); - nonzero(a) && nonzero(b) && a != b && hamming_distance(a, b) >= MIN_HAMMING + .all(|&(i, j)| pair_looks_random(&samples[i], &samples[j])) + && frozen_positions(samples) <= MAX_FROZEN_POSITIONS } /// Draws three samples straight from the OS RNG, with nothing mixed in. diff --git a/keep-core/src/frost/checked_rng.rs b/keep-core/src/frost/checked_rng.rs index b1eaf882..e8b9af79 100644 --- a/keep-core/src/frost/checked_rng.rs +++ b/keep-core/src/frost/checked_rng.rs @@ -3,7 +3,7 @@ //! Secret generation that checks the draw the secret is built from. //! -//! [`check_entropy_health`] certifies the source by drawing samples of its own. +//! The periodic entropy probe certifies the source by drawing samples of its own. //! A secret drawn afterwards comes from a different call, so a source that //! fails between the probe and the draw yields key material no check ever //! looked at. The window is narrow and periodic probing closes most of it, but @@ -44,15 +44,115 @@ //! do better. At a site that makes a single draw the only comparison is against //! bytes drawn after the secret, which catches a source that is still degraded //! when probed but not one that emits a single chosen value and then recovers. -//! -//! [`check_entropy_health`]: crate::entropy::check_entropy_health + +// The Ed25519 half is gated on keep-core's `ed25519` feature. This file is also +// compiled into the enclave, which does not declare that feature. +#![allow(unexpected_cfgs)] use frost_secp256k1_tr::rand_core::{impls, CryptoRng, Error as RngError, OsRng, RngCore}; use zeroize::{Zeroize, Zeroizing}; -use crate::entropy::{ - frozen_positions, pair_looks_random, EntropyHealthError, MAX_FROZEN_POSITIONS, -}; +/// Why a draw could not be trusted. +/// +/// The two arms are kept apart because they call for different responses. A +/// source that answered with constant output is a property of the machine and +/// will not improve on its own, so a co-signer should stop and stay stopped. +/// A call that failed to complete is a different statement, closer to "no file +/// descriptors" than to "the kernel is broken", and treating it as permanent +/// turns a transient fault into an outage that needs an operator to clear. +/// They also read differently in a log, which matters when someone is trying to +/// work out what happened. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EntropyHealthError { + /// The source answered, and what it returned failed the health criteria. + Degraded, + /// The source could not be read at all. + Unavailable, +} + +impl std::fmt::Display for EntropyHealthError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Degraded => write!( + f, + "RNG health check failed: constant or zero output detected" + ), + Self::Unavailable => write!(f, "RNG unavailable: the OS refused to provide entropy"), + } + } +} + +impl std::error::Error for EntropyHealthError {} + +/// How many bits differ between two 32-byte draws. +pub(crate) fn hamming_distance(a: &[u8; 32], b: &[u8; 32]) -> u32 { + a.iter().zip(b).map(|(x, y)| (x ^ y).count_ones()).sum() +} + +/// Bits two draws must differ by before they count as independent. +pub(crate) const MIN_HAMMING: u32 = 64; + +/// The most byte positions that may hold the same value in every sample before +/// the samples are refused. Only meaningful across three or more samples. +pub(crate) const MAX_FROZEN_POSITIONS: usize = 3; + +/// How many of the 32 byte positions hold the same value, whatever it is, in +/// every one of `samples`. +/// +/// This replaces a test that refused any position holding `0x00` or `0xff` in +/// every sample, which got both its rate and its coverage wrong. +/// +/// Its rate: that happens to healthy randomness about once in 260,000 +/// three-sample calls, measured at 12 in 3,000,000 checked draws against the +/// analytic 11.4. Every failure latches a co-signing node, and the periodic +/// check runs every 30 seconds, so a healthy always-on node took itself out of +/// service about once a month. +/// +/// Its coverage: it caught a frozen extreme byte, and any number of them, but +/// never a frozen byte of any other value. A source that fills part of a buffer +/// and leaves the rest as it was is the case that matters, because several +/// draws turn bytes straight into a scalar with no hash between: the dealer's +/// key, every polynomial coefficient, refresh and repair deltas, and the DKG +/// proof-of-knowledge nonce. Twelve frozen bytes there leave 160 bits in known +/// positions, about 2^80 work to recover, and the Hamming floor does not catch +/// it until roughly half the bytes are frozen. +/// +/// Counting positions frozen at any value, and refusing above +/// [`MAX_FROZEN_POSITIONS`], fixes both. A position matches across three +/// independent samples with probability 1 in 65,536, so four or more of 32 +/// doing so is a binomial tail of about 2e-15, the same order as the Hamming +/// floor, and no healthy node latches on it. What it still lets through, one to +/// three frozen positions, leaves at least 232 bits. +/// +/// Three samples is the floor. Across two, a position matches with probability +/// 1 in 256, and four of 32 matching becomes about 8e-6 per call, which is the +/// rate this exists to get away from. +pub(crate) fn frozen_positions(samples: &[[u8; 32]]) -> usize { + debug_assert!( + samples.len() >= 3, + "frozen_positions needs three samples; below that healthy input trips it" + ); + let Some((first, rest)) = samples.split_first() else { + return 0; + }; + (0..32) + .filter(|&pos| rest.iter().all(|s| s[pos] == first[pos])) + .count() +} + +/// The health criteria for a pair of draws, for a caller judging the bytes it +/// was handed rather than a sample drawn beside them: neither draw all-zero, +/// the two distinct, and at least [`MIN_HAMMING`] bits apart, which a source +/// repeating itself with a handful of bits flipped still fails. +/// +/// These are the same criteria the periodic entropy probe applies to each of +/// its pairs. The frozen-position count is not among them, because across two +/// samples healthy randomness trips it often enough to latch a node; it needs +/// three, see [`frozen_positions`]. +pub(crate) fn pair_looks_random(a: &[u8; 32], b: &[u8; 32]) -> bool { + let nonzero = |s: &[u8; 32]| s.iter().any(|&b| b != 0); + nonzero(a) && nonzero(b) && a != b && hamming_distance(a, b) >= MIN_HAMMING +} /// An RNG that holds every fill it hands out to the health criteria. /// diff --git a/keep-core/src/frost/mod.rs b/keep-core/src/frost/mod.rs index 7c05c049..fa6cadbc 100644 --- a/keep-core/src/frost/mod.rs +++ b/keep-core/src/frost/mod.rs @@ -22,7 +22,12 @@ mod verifying_set; #[cfg(feature = "ed25519")] pub mod ed25519; +#[doc(hidden)] +pub use checked_rng::EntropyHealthError; pub use checked_rng::{checked_draw, commit_checked, CheckedRng}; +pub(crate) use checked_rng::{frozen_positions, pair_looks_random, MAX_FROZEN_POSITIONS}; +#[cfg(test)] +pub(crate) use checked_rng::{hamming_distance, MIN_HAMMING}; pub use coordinator::Coordinator; pub use dealer::{ThresholdConfig, TrustedDealer}; pub use recover::recover_nsec; diff --git a/keep-enclave/enclave/src/signer.rs b/keep-enclave/enclave/src/signer.rs index ad14bdd5..275ce49f 100644 --- a/keep-enclave/enclave/src/signer.rs +++ b/keep-enclave/enclave/src/signer.rs @@ -11,12 +11,14 @@ use bitcoin::psbt::Psbt; use bitcoin::Network; use frost_secp256k1_tr as frost; use k256::schnorr::SigningKey; -use rand_core::OsRng; use sha2::{Digest, Sha256}; use signature::Signer; use std::collections::{BTreeMap, HashMap}; use zeroize::{Zeroize, ZeroizeOnDrop}; +#[path = "../../../keep-core/src/frost/checked_rng.rs"] +mod checked_rng; + #[derive(ZeroizeOnDrop)] pub struct EnclaveSigner { #[zeroize(skip)] @@ -196,7 +198,8 @@ impl EnclaveSigner { let signed_count = PsbtKeys::new(&entry.secret, network)?.sign(&mut psbt)?; let signed_bytes = psbt.serialize(); - self.audit_log.log_psbt_sign(key_id, psbt_bytes, signed_count); + self.audit_log + .log_psbt_sign(key_id, psbt_bytes, signed_count); Ok((signed_bytes, signed_count)) } @@ -225,7 +228,9 @@ impl EnclaveSigner { let key_package = frost::keys::KeyPackage::deserialize(&key_package_bytes) .map_err(|e| EnclaveError::InvalidKey(format!("Invalid FROST key package: {}", e)))?; - let pubkey = key_package.verifying_key().serialize() + let pubkey = key_package + .verifying_key() + .serialize() .map_err(|e| EnclaveError::InvalidKey(format!("Failed to serialize pubkey: {}", e)))?; let mut pubkey_arr = [0u8; 32]; pubkey_arr.copy_from_slice(&pubkey[1..33]); @@ -241,35 +246,57 @@ impl EnclaveSigner { Ok(pubkey_arr) } - pub fn frost_round1( - &mut self, - key_id: &str, - message: &[u8], - ) -> Result<(Vec, [u8; 32])> { - let entry = self.frost_keys.get(key_id) + pub fn frost_round1(&mut self, key_id: &str, message: &[u8]) -> Result<(Vec, [u8; 32])> { + let entry = self + .frost_keys + .get(key_id) .ok_or_else(|| EnclaveError::KeyNotFound(key_id.into()))?; let key_package = frost::keys::KeyPackage::deserialize(entry.key_package_bytes.as_slice()) .map_err(|e| EnclaveError::Signing(format!("Invalid key package: {}", e)))?; - let mut rng = OsRng; - let (nonces, our_commitment) = frost::round1::commit(key_package.signing_share(), &mut rng); + let identifier = *key_package.identifier(); + // commit_checked certifies the OsRng frost-core reads. The NSM-backed + // getrandom below only names the session. + let drawn = checked_rng::commit_checked(key_package.signing_share()) + .map_err(|e| EnclaveError::Signing(e.to_string())); + self.store_round1(key_id, message, identifier, drawn) + } + + fn store_round1( + &mut self, + key_id: &str, + message: &[u8], + identifier: frost::Identifier, + drawn: std::result::Result< + ( + frost::round1::SigningNonces, + frost::round1::SigningCommitments, + ), + EnclaveError, + >, + ) -> Result<(Vec, [u8; 32])> { + let (nonces, our_commitment) = drawn?; - let commitment_bytes = our_commitment.serialize() + let commitment_bytes = our_commitment + .serialize() .map_err(|e| EnclaveError::Signing(format!("Failed to serialize commitment: {}", e)))?; let mut session_id = [0u8; 32]; getrandom(&mut session_id)?; let mut commitments = BTreeMap::new(); - commitments.insert(*key_package.identifier(), our_commitment); - - self.frost_sessions.insert(session_id, FrostSession { - nonces, - commitments, - message: message.to_vec(), - key_id: key_id.to_string(), - }); + commitments.insert(identifier, our_commitment); + + self.frost_sessions.insert( + session_id, + FrostSession { + nonces, + commitments, + message: message.to_vec(), + key_id: key_id.to_string(), + }, + ); self.audit_log.log_frost_round1(key_id, message, session_id); @@ -282,7 +309,9 @@ impl EnclaveSigner { identifier_bytes: &[u8], commitment_bytes: &[u8], ) -> Result<()> { - let session = self.frost_sessions.get_mut(&session_id) + let session = self + .frost_sessions + .get_mut(&session_id) .ok_or_else(|| EnclaveError::Signing("Session not found".into()))?; let identifier = frost::Identifier::deserialize(identifier_bytes) @@ -294,29 +323,32 @@ impl EnclaveSigner { Ok(()) } - pub fn frost_round2( - &mut self, - session_id: [u8; 32], - ) -> Result> { - let session = self.frost_sessions.remove(&session_id) + pub fn frost_round2(&mut self, session_id: [u8; 32]) -> Result> { + let session = self + .frost_sessions + .remove(&session_id) .ok_or_else(|| EnclaveError::Signing("Session not found".into()))?; let key_id = session.key_id.clone(); - let entry = self.frost_keys.get(&key_id) + let entry = self + .frost_keys + .get(&key_id) .ok_or_else(|| EnclaveError::KeyNotFound(key_id.clone()))?; let key_package = frost::keys::KeyPackage::deserialize(entry.key_package_bytes.as_slice()) .map_err(|e| EnclaveError::Signing(format!("Invalid key package: {}", e)))?; - let signing_package = frost::SigningPackage::new(session.commitments.clone(), &session.message); + let signing_package = + frost::SigningPackage::new(session.commitments.clone(), &session.message); let signature_share = frost::round2::sign(&signing_package, &session.nonces, &key_package) .map_err(|e| EnclaveError::Signing(format!("FROST signing failed: {}", e)))?; let share_bytes = signature_share.serialize(); - self.audit_log.log_frost_round2(&key_id, session_id, &share_bytes); + self.audit_log + .log_frost_round2(&key_id, session_id, &share_bytes); Ok(share_bytes) } @@ -453,4 +485,110 @@ mod tests { let result = signer.sign("nonexistent", b"test"); assert!(result.is_err()); } + + #[test] + fn two_round1_draws_do_not_repeat_a_commitment() { + let mut signer = signer_with_share(); + let (first, _) = signer.frost_round1("group", b"one").unwrap(); + let (second, _) = signer.frost_round1("group", b"two").unwrap(); + assert_ne!(first, second); + assert_eq!(signer.frost_sessions.len(), 2); + } + + #[test] + fn a_refused_draw_stores_no_session() { + let mut signer = test_signer(); + let err = signer + .store_round1( + "group", + b"msg", + frost::Identifier::try_from(1u16).unwrap(), + Err(EnclaveError::Signing( + "RNG health check failed: constant or zero output detected".into(), + )), + ) + .unwrap_err(); + assert!(err.to_string().contains("RNG health check failed"), "{err}"); + assert!(signer.frost_sessions.is_empty()); + } + + #[test] + fn a_checked_round1_completes_a_verifying_signature() { + let (shares, pubkey_package) = frost::keys::generate_with_dealer( + 2, + 2, + frost::keys::IdentifierList::Default, + &mut rand_core::OsRng, + ) + .unwrap(); + let mut packages = shares + .into_iter() + .map(|(_, secret)| frost::keys::KeyPackage::try_from(secret).unwrap()); + let our_key = packages.next().unwrap(); + let their_key = packages.next().unwrap(); + + let mut signer = test_signer(); + signer + .import_frost_key( + "group", + our_key.serialize().unwrap(), + pubkey_package.serialize().unwrap(), + ) + .unwrap(); + + let message = b"pay the allowed destination"; + let (our_commitment_bytes, session_id) = signer.frost_round1("group", message).unwrap(); + let our_commitment = + frost::round1::SigningCommitments::deserialize(&our_commitment_bytes).unwrap(); + let (their_nonces, their_commitment) = + checked_rng::commit_checked(their_key.signing_share()).unwrap(); + signer + .frost_add_commitment( + session_id, + &their_key.identifier().serialize(), + &their_commitment.serialize().unwrap(), + ) + .unwrap(); + + let mut commitments = BTreeMap::new(); + commitments.insert(*our_key.identifier(), our_commitment); + commitments.insert(*their_key.identifier(), their_commitment); + let signing_package = frost::SigningPackage::new(commitments, message); + + let our_share_bytes = signer.frost_round2(session_id).unwrap(); + let our_share = frost::round2::SignatureShare::deserialize(&our_share_bytes).unwrap(); + let their_share = frost::round2::sign(&signing_package, &their_nonces, &their_key).unwrap(); + + let mut signature_shares = BTreeMap::new(); + signature_shares.insert(*our_key.identifier(), our_share); + signature_shares.insert(*their_key.identifier(), their_share); + let signature = + frost::aggregate(&signing_package, &signature_shares, &pubkey_package).unwrap(); + pubkey_package + .verifying_key() + .verify(message, &signature) + .unwrap(); + assert!(signer.frost_sessions.is_empty()); + } + + fn signer_with_share() -> EnclaveSigner { + let (shares, pubkey_package) = frost::keys::generate_with_dealer( + 2, + 2, + frost::keys::IdentifierList::Default, + &mut rand_core::OsRng, + ) + .unwrap(); + let (_, secret) = shares.into_iter().next().unwrap(); + let key = frost::keys::KeyPackage::try_from(secret).unwrap(); + let mut signer = test_signer(); + signer + .import_frost_key( + "group", + key.serialize().unwrap(), + pubkey_package.serialize().unwrap(), + ) + .unwrap(); + signer + } }