change reconstruct() to take KeyPackages; validate size (#523)

change reconstruct() to take KeyPackages; validate size

frost-core/CHANGELOG.md

@@ -11,6 +11,8 @@ Entries are listed in reverse chronological order.
   running previous versions.
 * A new `min_signers` field was added to `KeyPackage`, which changes its
   `new()` method and its serde serialization.
+* `reconstruct()` was changed to take a slice of `KeyPackage`s instead of
+  `SecretShare`s since users are expect to store the former and not the latter.
 
 ## Released
 

frost-core/src/frost/keys.rs

@@ -787,8 +787,8 @@ pub(crate) fn generate_secret_shares<C: Ciphersuite>(
     Ok(secret_shares)
 }
 
-/// Recompute the secret from at least `min_signers` secret shares
+/// Recompute the secret from at least `min_signers` secret shares (inside
-/// using Lagrange interpolation.
+/// [`KeyPackage`]s) using Lagrange interpolation.
 ///
 /// This can be used if for some reason the original key must be restored; e.g.
 /// if threshold signing is not required anymore.
@@ -797,34 +797,46 @@ pub(crate) fn generate_secret_shares<C: Ciphersuite>(
 /// able to generate signatures only using the shares, without having to
 /// reconstruct the original key.
 ///
-/// The caller is responsible for providing at least `min_signers` shares;
+/// The caller is responsible for providing at least `min_signers` packages;
 /// if less than that is provided, a different key will be returned.
 pub fn reconstruct<C: Ciphersuite>(
-    secret_shares: &[SecretShare<C>],
+    key_packages: &[KeyPackage<C>],
 ) -> Result<SigningKey<C>, Error<C>> {
-    if secret_shares.is_empty() {
+    if key_packages.is_empty() {
+        return Err(Error::IncorrectNumberOfShares);
+    }
+    // There is no obvious way to get `min_signers` in order to validate the
+    // size of `secret_shares`. Since that is just a best-effort validation,
+    // we don't need to worry too much about adversarial situations where people
+    // lie about min_signers, so just get the minimum value out of all of them.
+    let min_signers = key_packages
+        .iter()
+        .map(|k| k.min_signers)
+        .min()
+        .expect("should not be empty since that was just tested");
+    if key_packages.len() < min_signers as usize {
         return Err(Error::IncorrectNumberOfShares);
     }
 
     let mut secret = <<C::Group as Group>::Field>::zero();
 
-    let identifiers: BTreeSet<_> = secret_shares
+    let identifiers: BTreeSet<_> = key_packages
         .iter()
         .map(|s| s.identifier())
         .cloned()
         .collect();
 
-    if identifiers.len() != secret_shares.len() {
+    if identifiers.len() != key_packages.len() {
         return Err(Error::DuplicatedIdentifier);
     }
 
     // Compute the Lagrange coefficients
-    for secret_share in secret_shares.iter() {
+    for secret_share in key_packages.iter() {
         let lagrange_coefficient =
             compute_lagrange_coefficient(&identifiers, None, secret_share.identifier)?;
 
         // Compute y = f(0) via polynomial interpolation of these t-of-n solutions ('points) of f
-        secret = secret + (lagrange_coefficient * secret_share.value.0);
+        secret = secret + (lagrange_coefficient * secret_share.secret_share().0);
     }
 
     Ok(SigningKey { scalar: secret })

frost-core/src/tests/ciphersuite_generic.rs

@@ -41,12 +41,14 @@ pub fn check_share_generation<C: Ciphersuite, R: RngCore + CryptoRng>(mut rng: R
     )
     .unwrap();
 
-    for secret_share in secret_shares.iter() {
+    let key_packages: Vec<frost::keys::KeyPackage<C>> = secret_shares
-        assert!(secret_share.verify().is_ok());
+        .iter()
-    }
+        .cloned()
+        .map(|s| s.try_into().unwrap())
+        .collect();
 
     assert_eq!(
-        frost::keys::reconstruct::<C>(&secret_shares)
+        frost::keys::reconstruct::<C>(&key_packages)
             .unwrap()
             .serialize()
             .as_ref(),
@@ -60,11 +62,16 @@ pub fn check_share_generation<C: Ciphersuite, R: RngCore + CryptoRng>(mut rng: R
         Error::IncorrectNumberOfShares
     );
 
-    let mut secret_shares = secret_shares;
+    assert_eq!(
-    secret_shares[0] = secret_shares[1].clone();
+        frost::keys::reconstruct::<C>(&key_packages[0..1]).unwrap_err(),
+        Error::IncorrectNumberOfShares
+    );
+
+    let mut key_packages = key_packages;
+    key_packages[0] = key_packages[1].clone();
 
     assert_eq!(
-        frost::keys::reconstruct::<C>(&secret_shares).unwrap_err(),
+        frost::keys::reconstruct::<C>(&key_packages).unwrap_err(),
         Error::DuplicatedIdentifier
     );
 }

frost-ed25519/src/lib.rs

@@ -266,7 +266,7 @@ pub mod keys {
     ///
     /// The caller is responsible for providing at least `min_signers` shares;
     /// if less than that is provided, a different key will be returned.
-    pub fn reconstruct(secret_shares: &[SecretShare]) -> Result<SigningKey, Error> {
+    pub fn reconstruct(secret_shares: &[KeyPackage]) -> Result<SigningKey, Error> {
         frost::keys::reconstruct(secret_shares)
     }
 

frost-ed448/src/lib.rs

@@ -260,7 +260,7 @@ pub mod keys {
     ///
     /// The caller is responsible for providing at least `min_signers` shares;
     /// if less than that is provided, a different key will be returned.
-    pub fn reconstruct(secret_shares: &[SecretShare]) -> Result<SigningKey, Error> {
+    pub fn reconstruct(secret_shares: &[KeyPackage]) -> Result<SigningKey, Error> {
         frost::keys::reconstruct(secret_shares)
     }
 

frost-p256/src/lib.rs

@@ -292,7 +292,7 @@ pub mod keys {
     ///
     /// The caller is responsible for providing at least `min_signers` shares;
     /// if less than that is provided, a different key will be returned.
-    pub fn reconstruct(secret_shares: &[SecretShare]) -> Result<SigningKey, Error> {
+    pub fn reconstruct(secret_shares: &[KeyPackage]) -> Result<SigningKey, Error> {
         frost::keys::reconstruct(secret_shares)
     }
 

frost-ristretto255/src/lib.rs

@@ -251,7 +251,7 @@ pub mod keys {
     ///
     /// The caller is responsible for providing at least `min_signers` shares;
     /// if less than that is provided, a different key will be returned.
-    pub fn reconstruct(secret_shares: &[SecretShare]) -> Result<SigningKey, Error> {
+    pub fn reconstruct(secret_shares: &[KeyPackage]) -> Result<SigningKey, Error> {
         frost::keys::reconstruct(secret_shares)
     }
 

frost-secp256k1/src/lib.rs

@@ -291,7 +291,7 @@ pub mod keys {
     ///
     /// The caller is responsible for providing at least `min_signers` shares;
     /// if less than that is provided, a different key will be returned.
-    pub fn reconstruct(secret_shares: &[SecretShare]) -> Result<SigningKey, Error> {
+    pub fn reconstruct(secret_shares: &[KeyPackage]) -> Result<SigningKey, Error> {
         frost::keys::reconstruct(secret_shares)
     }