From b0abebacec8280e5efc2f3ea6a17d3c90c9604f3 Mon Sep 17 00:00:00 2001 From: Marcella Hastings Date: Fri, 25 Aug 2023 14:54:45 -0400 Subject: [PATCH] add keygen output constructor and tests #376 * add keygen output constructor and tests * add extra check on PIDs in presign input --- src/auxinfo/output.rs | 65 +++++++--- src/keygen/mod.rs | 4 +- src/keygen/output.rs | 250 ++++++++++++++++++++++++++++++++++++++ src/keygen/participant.rs | 120 ++---------------- src/presign/input.rs | 57 +++------ 5 files changed, 330 insertions(+), 166 deletions(-) create mode 100644 src/keygen/output.rs diff --git a/src/auxinfo/output.rs b/src/auxinfo/output.rs index 7da3bd9a..35031e9a 100644 --- a/src/auxinfo/output.rs +++ b/src/auxinfo/output.rs @@ -4,7 +4,7 @@ use std::collections::HashSet; use crate::{ auxinfo::info::{AuxInfoPrivate, AuxInfoPublic}, - errors::{CallerError, Result}, + errors::{CallerError, InternalError, Result}, protocol::ParticipantIdentifier, }; use tracing::error; @@ -36,7 +36,7 @@ impl Output { .map(AuxInfoPublic::participant) .collect::>(); if pids.len() != public_auxinfo.len() { - error!("Tried to create a keygen output using a set of public material from non-unique participants"); + error!("Tried to create an auxinfo output using a set of public material from non-unique participants"); Err(CallerError::BadInput)? } @@ -73,6 +73,21 @@ impl Output { &self.public_auxinfo } + pub(crate) fn private_pid(&self) -> Result { + let expected_public_key = self.private_auxinfo.encryption_key(); + match self + .public_auxinfo + .iter() + .find(|auxinfo| &expected_public_key == auxinfo.pk()) + { + Some(public_auxinfo) => Ok(public_auxinfo.participant()), + None => { + error!("Didn't find public aux info corresponding to the private aux info, but there should be one by construction"); + Err(InternalError::InternalInvariantFailed) + } + } + } + pub(crate) fn find_public(&self, pid: ParticipantIdentifier) -> Option<&AuxInfoPublic> { self.public_auxinfo .iter() @@ -93,10 +108,11 @@ mod tests { }; impl Output { - /// Simulate the output of an auxinfo run with the given participants. + /// Simulate the valid output of an auxinfo run with the given + /// participants. /// - /// This should __never__ be called outside of tests! It does not - /// validate the PID input. + /// This should __never__ be called outside of tests! The given `pids` + /// must not contain duplicates. pub(crate) fn simulate( pids: &[ParticipantIdentifier], rng: &mut (impl CryptoRng + RngCore), @@ -118,27 +134,24 @@ mod tests { }) .unzip(); - Self { - private_auxinfo: private_auxinfo.pop().unwrap(), - public_auxinfo, - } + Self::from_parts(public_auxinfo, private_auxinfo.pop().unwrap()).unwrap() } - /// Simulate a consistent output of an auxinfo run with the given + /// Simulate a consistent, valid output of an auxinfo run with the given /// participants. /// /// This produces output for every config in the provided set. The - /// config must have a non-zero length. + /// config must have a non-zero length and the given `pids` must not + /// contain duplicates. pub(crate) fn simulate_set( configs: &[ParticipantConfig], rng: &mut (impl CryptoRng + RngCore), ) -> Vec { - let (private_auxinfo, public_auxinfo): (Vec<_>, Vec<_>) = configs + let (public_auxinfo, private_auxinfo): (Vec<_>, Vec<_>) = configs .iter() .map(|config| { let (key, _, _) = DecryptionKey::new(rng).unwrap(); ( - AuxInfoPrivate::from(key.clone()), AuxInfoPublic::new( &(), config.id(), @@ -146,15 +159,15 @@ mod tests { VerifiedRingPedersen::extract(&key, &(), rng).unwrap(), ) .unwrap(), + AuxInfoPrivate::from(key), ) }) .unzip(); private_auxinfo .into_iter() - .map(|private_auxinfo| Self { - private_auxinfo, - public_auxinfo: public_auxinfo.clone(), + .map(|private_auxinfo| { + Self::from_parts(public_auxinfo.clone(), private_auxinfo).unwrap() }) .collect() } @@ -181,10 +194,24 @@ mod tests { // Duplicate one of the PIDs pids.push(pids[4]); - let output = Output::simulate(&pids, rng); + let (mut private_auxinfo, public_auxinfo): (Vec<_>, Vec<_>) = pids + .iter() + .map(|&pid| { + let (key, _, _) = DecryptionKey::new(rng).unwrap(); + ( + AuxInfoPrivate::from(key.clone()), + AuxInfoPublic::new( + &(), + pid, + key.encryption_key(), + VerifiedRingPedersen::extract(&key, &(), rng).unwrap(), + ) + .unwrap(), + ) + }) + .unzip(); - let (public, private) = output.into_parts(); - assert!(Output::from_parts(public, private).is_err()); + assert!(Output::from_parts(public_auxinfo, private_auxinfo.pop().unwrap()).is_err()); } #[test] diff --git a/src/keygen/mod.rs b/src/keygen/mod.rs index 4c46b057..121bfd82 100644 --- a/src/keygen/mod.rs +++ b/src/keygen/mod.rs @@ -9,7 +9,9 @@ mod keygen_commit; mod keyshare; +mod output; mod participant; pub use keyshare::{KeySharePrivate, KeySharePublic}; -pub use participant::{KeygenParticipant, Output}; +pub use output::Output; +pub use participant::KeygenParticipant; diff --git a/src/keygen/output.rs b/src/keygen/output.rs new file mode 100644 index 00000000..985f7457 --- /dev/null +++ b/src/keygen/output.rs @@ -0,0 +1,250 @@ +// Copyright (c) 2022-2023 Bolt Labs Holdings, Inc +// +// This source code is licensed under both the MIT license found in the +// LICENSE-MIT file in the root directory of this source tree and the Apache +// License, Version 2.0 found in the LICENSE-APACHE file in the root directory +// of this source tree. + +use std::collections::HashSet; + +use crate::{ + errors::{CallerError, InternalError, Result}, + keygen::keyshare::{KeySharePrivate, KeySharePublic}, + utils::CurvePoint, + ParticipantIdentifier, +}; + +use k256::ecdsa::VerifyingKey; +use tracing::error; + +/// Output type from key generation, including all parties' public key shares, +/// this party's private key share, and a bit of global randomness. +#[derive(Debug, Clone)] +pub struct Output { + public_key_shares: Vec, + private_key_share: KeySharePrivate, + rid: [u8; 32], +} + +impl Output { + /// Construct the generated public key. + pub fn public_key(&self) -> Result { + // Add up all the key shares + let public_key_point = self + .public_key_shares + .iter() + .fold(CurvePoint::IDENTITY, |sum, share| sum + *share.as_ref()); + + VerifyingKey::from_encoded_point(&public_key_point.0.to_affine().into()).map_err(|_| { + error!("Keygen output does not produce a valid public key."); + InternalError::InternalInvariantFailed + }) + } + + pub(crate) fn public_key_shares(&self) -> &[KeySharePublic] { + &self.public_key_shares + } + + pub(crate) fn private_key_share(&self) -> &KeySharePrivate { + &self.private_key_share + } + + /// Get the [`ParticipantIdentifier`] corresponding to the + /// [`KeySharePrivate`]. + pub(crate) fn private_pid(&self) -> Result { + let expected_public_share = self.private_key_share.public_share()?; + match self + .public_key_shares + .iter() + .find(|share| share.as_ref() == &expected_public_share) + { + Some(public_key_share) => Ok(public_key_share.participant()), + None => { + error!("Didn't find a public key share corresponding to the private key share, but there should be one by construction"); + Err(InternalError::InternalInvariantFailed) + } + } + } + + /// This could be made public if appropriate + #[cfg(test)] + pub(crate) fn rid(&self) -> &[u8; 32] { + &self.rid + } + + /// Create a new `Output` from its constitutent parts. + /// + /// This method should only be used with components that were previously + /// derived via the [`Output::into_parts()`] method; the calling application + /// should not try to form public and private key shares independently. + /// + /// The provided components must satisfy the following properties: + /// - There is a valid key pair -- that is, the public key corresponding to + /// the private key share must be contained in the list of public shares. + /// - The public key shares must be from a unique set of participants + pub fn from_parts( + public_key_shares: Vec, + private_key_share: KeySharePrivate, + rid: [u8; 32], + ) -> Result { + let pids = public_key_shares + .iter() + .map(KeySharePublic::participant) + .collect::>(); + if pids.len() != public_key_shares.len() { + error!("Tried to create a keygen output using a set of public material from non-unique participants"); + Err(CallerError::BadInput)? + } + + let expected_public_share = private_key_share.public_share()?; + if !public_key_shares + .iter() + .any(|share| share.as_ref() == &expected_public_share) + { + error!("Tried to create a keygen output using a private share with no corresponding public share"); + Err(CallerError::BadInput)? + } + + Ok(Self { + public_key_shares, + private_key_share, + rid, + }) + } + + /// Decompose the `Output` into its constituent parts. + /// + /// # 🔒 Storage requirements + /// The [`KeySharePrivate`] must be stored securely by the calling + /// application, and a best effort should be made to drop it from memory + /// after it's securely stored. + /// + /// The public components (including the byte array and the public key + /// shares) can be stored in the clear. + pub fn into_parts(self) -> (Vec, KeySharePrivate, [u8; 32]) { + (self.public_key_shares, self.private_key_share, self.rid) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{utils::testing::init_testing, ParticipantConfig, ParticipantIdentifier}; + use rand::{CryptoRng, Rng, RngCore}; + + impl Output { + /// Simulate the valid output of a keygen run with the given + /// participants. + /// + /// This should __never__ be called outside of tests! The given `pids` + /// must not contain duplicates. + pub(crate) fn simulate( + pids: &[ParticipantIdentifier], + rng: &mut (impl CryptoRng + RngCore), + ) -> Self { + let (mut private_key_shares, public_key_shares): (Vec<_>, Vec<_>) = pids + .iter() + .map(|&pid| { + // TODO #340: Replace with KeyShare methods once they exist. + let secret = KeySharePrivate::random(rng); + let public = secret.public_share().unwrap(); + (secret, KeySharePublic::new(pid, public)) + }) + .unzip(); + + let rid = rng.gen(); + + Self::from_parts(public_key_shares, private_key_shares.pop().unwrap(), rid).unwrap() + } + + /// Simulate a consistent, valid output of a keygen run with the given + /// participants. + /// + /// This produces output for every config in the provided set. The + /// config must have a non-zero length, and the given `pids` must not + /// contain duplicates. + pub(crate) fn simulate_set( + configs: &[ParticipantConfig], + rng: &mut (impl CryptoRng + RngCore), + ) -> Vec { + let (private_key_shares, public_key_shares): (Vec<_>, Vec<_>) = configs + .iter() + .map(|config| { + // TODO #340: Replace with KeyShare methods once they exist. + let secret = KeySharePrivate::random(rng); + let public = secret.public_share().unwrap(); + (secret, KeySharePublic::new(config.id(), public)) + }) + .unzip(); + + let rid = rng.gen(); + + private_key_shares + .into_iter() + .map(|private_key_share| { + Self::from_parts(public_key_shares.clone(), private_key_share, rid).unwrap() + }) + .collect() + } + } + + #[test] + fn from_into_parts_works() { + let rng = &mut init_testing(); + let pids = std::iter::repeat_with(|| ParticipantIdentifier::random(rng)) + .take(5) + .collect::>(); + let output = Output::simulate(&pids, rng); + + let (public, private, rid) = output.into_parts(); + assert!(Output::from_parts(public, private, rid).is_ok()); + } + + #[test] + fn private_field_must_correspond_to_a_public() { + let rng = &mut init_testing(); + let pids = std::iter::repeat_with(|| ParticipantIdentifier::random(rng)) + .take(5) + .collect::>(); + + // Use the simulate function to get a set of valid public components + let output = Output::simulate(&pids, rng); + + // Create a random private share. It's legally possible for this to match one of + // the public keys but it's so unlikely that we won't check it. + let bad_private_key_share = KeySharePrivate::random(rng); + + assert!( + Output::from_parts(output.public_key_shares, bad_private_key_share, output.rid) + .is_err() + ) + } + + #[test] + fn public_shares_must_not_have_duplicate_pids() { + let rng = &mut init_testing(); + let mut pids = std::iter::repeat_with(|| ParticipantIdentifier::random(rng)) + .take(5) + .collect::>(); + + // Duplicate one of the PIDs + pids.push(pids[4]); + + // Form output with the duplicated PID + let (mut private_key_shares, public_key_shares): (Vec<_>, Vec<_>) = pids + .iter() + .map(|&pid| { + // TODO #340: Replace with KeyShare methods once they exist. + let secret = KeySharePrivate::random(rng); + let public = secret.public_share().unwrap(); + (secret, KeySharePublic::new(pid, public)) + }) + .unzip(); + + let rid = rng.gen(); + + assert!( + Output::from_parts(public_key_shares, private_key_shares.pop().unwrap(), rid).is_err() + ); + } +} diff --git a/src/keygen/participant.rs b/src/keygen/participant.rs index 0771a1c8..b191a7f9 100644 --- a/src/keygen/participant.rs +++ b/src/keygen/participant.rs @@ -14,13 +14,15 @@ use crate::{ keygen::{ keygen_commit::{KeygenCommit, KeygenDecommit}, keyshare::{KeySharePrivate, KeySharePublic}, + output::Output, }, local_storage::LocalStorage, messages::{KeygenMessageType, Message, MessageType}, - participant::{Broadcast, InnerProtocolParticipant, ProcessOutcome, ProtocolParticipant}, + participant::{ + Broadcast, InnerProtocolParticipant, ProcessOutcome, ProtocolParticipant, Status, + }, protocol::{ParticipantIdentifier, ProtocolType, SharedContext}, run_only_once, - utils::CurvePoint, zkp::{ pisch::{CommonInput, PiSchPrecommit, PiSchProof, ProverSecret}, Proof2, @@ -28,8 +30,6 @@ use crate::{ Identifier, }; -use crate::participant::Status; -use k256::ecdsa::VerifyingKey; use merlin::Transcript; use rand::{CryptoRng, RngCore}; use tracing::{error, info, instrument, warn}; @@ -119,40 +119,6 @@ pub struct KeygenParticipant { status: Status, } -/// Output type from key generation, including all parties' public key shares, -/// this party's private key share, and a bit of global randomness. -#[derive(Debug, Clone)] -pub struct Output { - public_key_shares: Vec, - private_key_share: KeySharePrivate, - #[allow(unused)] - rid: [u8; 32], -} - -impl Output { - /// Construct the generated public key. - pub fn public_key(&self) -> Result { - // Add up all the key shares - let public_key_point = self - .public_key_shares - .iter() - .fold(CurvePoint::IDENTITY, |sum, share| sum + *share.as_ref()); - - VerifyingKey::from_encoded_point(&public_key_point.0.to_affine().into()).map_err(|_| { - error!("Keygen output does not produce a valid public key."); - InternalError::InternalInvariantFailed - }) - } - - pub(crate) fn public_key_shares(&self) -> &[KeySharePublic] { - &self.public_key_shares - } - - pub(crate) fn private_key_share(&self) -> &KeySharePrivate { - &self.private_key_share - } -} - impl ProtocolParticipant for KeygenParticipant { type Input = (); type Output = Output; @@ -605,11 +571,7 @@ impl KeygenParticipant { .remove::(self.id)?; self.status = Status::TerminatedSuccessfully; - let output = Output { - public_key_shares, - private_key_share, - rid: global_rid, - }; + let output = Output::from_parts(public_key_shares, private_key_share, global_rid)?; Ok(ProcessOutcome::Terminated(output)) } else { // Otherwise, we'll have to wait for more round three messages. @@ -628,70 +590,14 @@ fn schnorr_proof_transcript(global_rid: &[u8; 32]) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::{utils::testing::init_testing, Identifier, ParticipantConfig}; + use crate::{ + utils::{testing::init_testing, CurvePoint}, + Identifier, ParticipantConfig, + }; use rand::{CryptoRng, Rng, RngCore}; use std::collections::HashMap; use tracing::debug; - impl Output { - /// Simulate the output of a keygen run with the given participants. - /// - /// This should __never__ be called outside of tests! - pub(crate) fn simulate( - pids: &[ParticipantIdentifier], - rng: &mut (impl CryptoRng + RngCore), - ) -> Self { - let (mut private_key_shares, public_key_shares): (Vec<_>, Vec<_>) = pids - .iter() - .map(|&pid| { - // TODO #340: Replace with KeyShare methods once they exist. - let secret = KeySharePrivate::random(rng); - let public = secret.public_share().unwrap(); - (secret, KeySharePublic::new(pid, public)) - }) - .unzip(); - - let rid = rng.gen(); - - Self { - private_key_share: private_key_shares.pop().unwrap(), - public_key_shares, - rid, - } - } - - /// Simulate a consistent output of a keygen run with the given - /// participants. - /// - /// This produces output for every config in the provided set. The - /// config must have a non-zero length. - pub(crate) fn simulate_set( - configs: &[ParticipantConfig], - rng: &mut (impl CryptoRng + RngCore), - ) -> Vec { - let (private_key_shares, public_key_shares): (Vec<_>, Vec<_>) = configs - .iter() - .map(|config| { - // TODO #340: Replace with KeyShare methods once they exist. - let secret = KeySharePrivate::random(rng); - let public = secret.public_share().unwrap(); - (secret, KeySharePublic::new(config.id(), public)) - }) - .unzip(); - - let rid = rng.gen(); - - private_key_shares - .into_iter() - .map(|private_key_share| Self { - private_key_share, - public_key_shares: public_key_shares.clone(), - rid, - }) - .collect() - } - } - impl KeygenParticipant { pub fn new_quorum( sid: Identifier, @@ -826,7 +732,7 @@ mod tests { let mut publics_for_pid = vec![]; for output in &outputs { let key_share = output - .public_key_shares + .public_key_shares() .iter() .find(|key_share| key_share.participant() == pid); @@ -855,20 +761,20 @@ mod tests { .zip(quorum.iter().map(ProtocolParticipant::id)) { let public_share = output - .public_key_shares + .public_key_shares() .iter() .find(|public_share| public_share.participant() == pid); assert!(public_share.is_some()); let expected_public_share = - CurvePoint::GENERATOR.multiply_by_bignum(output.private_key_share.as_ref())?; + CurvePoint::GENERATOR.multiply_by_bignum(output.private_key_share().as_ref())?; assert_eq!(public_share.unwrap().as_ref(), &expected_public_share); } // Check that every participant has the same `rid` value assert!(outputs .windows(2) - .all(|outputs| outputs[0].rid == outputs[1].rid)); + .all(|outputs| outputs[0].rid() == outputs[1].rid())); Ok(()) } diff --git a/src/presign/input.rs b/src/presign/input.rs index 09c20bb8..5d08dadf 100644 --- a/src/presign/input.rs +++ b/src/presign/input.rs @@ -1,3 +1,9 @@ +// Copyright (c) 2022-2023 Bolt Labs Holdings, Inc +// +// This source code is licensed under both the MIT license found in the +// LICENSE-MIT file in the root directory of this source tree and the Apache +// License, Version 2.0 found in the LICENSE-APACHE file in the root directory +// of this source tree. use std::collections::HashSet; use tracing::error; @@ -49,24 +55,23 @@ impl Input { Err(CallerError::BadInput)? } - // There shouldn't be duplicates. Since we checked equality of the sets and the - // lengths already, this also applies to auxinfo (also, the `auxinfo::Output` - // type checks this at construction already). + // There shouldn't be duplicates. + // This check is redundant, since it's also checked in the `auxinfo::Output` and + // `keygen::Output` constructors, so we actually don't test it below. + // Also, since we checked equality of the sets and the lengths already, checking + // for keygen also validates it for auxinfo if key_pids.len() != keygen_output.public_key_shares().len() { error!("Duplicate participant IDs appeared in AuxInfo and KeyShare public input."); Err(CallerError::BadInput)? } - // The private key share should map to one of the public values. - // NB: The corresponding check for `auxinfo::Output` is handled in that type's - // constructor. - let expected_public_share = keygen_output.private_key_share().public_share()?; - if !keygen_output - .public_key_shares() - .iter() - .any(|public_share| expected_public_share == *public_share.as_ref()) - { - error!("Keygen private share did not correspond to any of the provided keygen public shares."); + // The constructors for keygen and auxinfo output already check other important + // properties, like that the private component maps to one of public + // components for each one. + + // The participant IDs for the private components of each output should match + if keygen_output.private_pid() != auxinfo_output.private_pid() { + error!("Expected private keygen and auxinfo outputs to correspond to the same participant, but they didn't"); Err(CallerError::BadInput)? } @@ -186,32 +191,6 @@ mod test { ); } - #[test] - fn inputs_must_not_have_duplicates() { - let rng = &mut init_testing(); - - let mut pids = std::iter::repeat_with(|| ParticipantIdentifier::random(rng)) - .take(5) - .collect::>(); - - // This test doesn't bother adding a duplicate in only one of the inputs because - // that would fail the length checks tested in `inputs_must_be_same_length` - // Instead, duplicate one of the PIDs... - pids.push(pids[4]); - - // ...and simulate the outputs. The public material will be different for the - // two duplicated PIDs. - let keygen_output = keygen::Output::simulate(&pids, rng); - let auxinfo_output = auxinfo::Output::simulate(&pids, rng); - - let result = Input::new(auxinfo_output, keygen_output); - assert!(result.is_err()); - assert_eq!( - result.unwrap_err(), - InternalError::CallingApplicationMistake(CallerError::BadInput) - ); - } - #[test] fn inputs_must_have_same_participant_sets() { let rng = &mut init_testing();