creating gsk when splitting dealer secret (#4788)

Problem: The trusted dealer provides the spending key to create the package. This means that the trusted dealer has the capability to sign the transaction on their own terms without the participants being involved.

Solution: We generate a separate key that is discarded while partially creating the package (it generates the nullifier key and outgoing view key). This means that the trusted dealer no longer will be able to sign the transaction and only all the participants can do so.

ironfish-rust/src/frost_utils/split_secret.rs

@@ -14,26 +14,19 @@ use ironfish_frost::{
 use rand::{CryptoRng, RngCore};
 use std::collections::HashMap;
 
-use crate::errors::{IronfishError, IronfishErrorKind};
+use crate::errors::IronfishError;
+use crate::SaplingKey;
 
 pub struct SecretShareConfig {
     pub min_signers: u16,
     pub identities: Vec<Identity>,
-    pub secret: Vec<u8>,
+    pub spender_key: SaplingKey,
 }
 
 pub(crate) fn split_secret<R: RngCore + CryptoRng>(
     config: &SecretShareConfig,
     mut rng: R,
 ) -> Result<(HashMap<Identity, KeyPackage>, PublicKeyPackage), IronfishError> {
-    let secret_bytes: [u8; 32] = config
-        .secret
-        .clone()
-        .try_into()
-        .map_err(|_| IronfishError::new(IronfishErrorKind::InvalidSecret))?;
-
-    let secret_key = SigningKey::deserialize(secret_bytes)?;
-
     let mut frost_id_map = config
         .identities
         .iter()
@@ -43,13 +36,12 @@ pub(crate) fn split_secret<R: RngCore + CryptoRng>(
     let frost_ids = frost_id_map.keys().cloned().collect::<Vec<_>>();
     let identifier_list = IdentifierList::Custom(&frost_ids[..]);
 
+    let secret_key = SigningKey::deserialize(config.spender_key.spend_authorizing_key.to_bytes())?;
+    let max_signers: u16 = config.identities.len().try_into()?;
+
     let (shares, pubkeys) = split(
         &secret_key,
-        config
-            .identities
-            .len()
-            .try_into()
-            .expect("too many identities"),
+        max_signers,
         config.min_signers,
         identifier_list,
         &mut rng,
@@ -77,39 +69,18 @@ mod test {
     use crate::{keys::SaplingKey, test_util::create_multisig_identities};
     use ironfish_frost::frost::{frost::keys::reconstruct, JubjubBlake2b512};
 
-    #[test]
-    fn test_invalid_secret() {
-        let identities = create_multisig_identities(10);
-
-        let vec = vec![1; 31];
-        let config = SecretShareConfig {
-            min_signers: 2,
-            identities,
-            secret: vec,
-        };
-
-        let rng = rand::thread_rng();
-        let result = split_secret(&config, rng);
-        assert!(result.is_err());
-        assert!(
-            matches!(result.unwrap_err().kind, IronfishErrorKind::InvalidSecret),
-            "expected InvalidSecret error"
-        );
-    }
-
     #[test]
     fn test_split_secret() {
         let identities = create_multisig_identities(10);
         let identities_length = identities.len();
 
         let rng = rand::thread_rng();
-
-        let key = SaplingKey::generate_key().spend_authorizing_key.to_bytes();
+        let key = SaplingKey::generate_key();
 
         let config = SecretShareConfig {
             min_signers: 2,
             identities,
-            secret: key.to_vec(),
+            spender_key: key,
         };
 
         let (key_packages, _) = split_secret(&config, rng).unwrap();
@@ -122,6 +93,9 @@ mod test {
 
         let scalar = signing_key.to_scalar();
 
-        assert_eq!(scalar.to_bytes(), key);
+        assert_eq!(
+            scalar.to_bytes(),
+            config.spender_key.spend_authorizing_key.to_bytes()
+        );
     }
 }

ironfish-rust/src/frost_utils/split_spender_key.rs

@@ -32,30 +32,31 @@ pub fn split_spender_key(
     min_signers: u16,
     identities: Vec<Identity>,
 ) -> Result<TrustedDealerKeyPackages, IronfishError> {
-    let secret = spender_key.spend_authorizing_key.to_bytes().to_vec();
+    let group_secret_key = SaplingKey::generate_key();
 
     let secret_config = SecretShareConfig {
         min_signers,
         identities,
-        secret,
+        spender_key: spender_key.clone(),
     };
 
     let (key_packages, public_key_package) = split_secret(&secret_config, thread_rng())?;
 
-    let proof_authorizing_key = spender_key.sapling_proof_generation_key().nsk;
+    let proof_authorizing_key = secret_config.spender_key.sapling_proof_generation_key().nsk;
 
     let authorizing_key = public_key_package.verifying_key().serialize();
     let authorizing_key = Option::from(SubgroupPoint::from_bytes(&authorizing_key))
         .ok_or_else(|| IronfishError::new(IronfishErrorKind::InvalidAuthorizingKey))?;
-    let nullifier_deriving_key =
-        *PROOF_GENERATION_KEY_GENERATOR * spender_key.sapling_proof_generation_key().nsk;
+
+    let nullifier_deriving_key = *PROOF_GENERATION_KEY_GENERATOR * proof_authorizing_key;
+
     let view_key = ViewKey {
         authorizing_key,
         nullifier_deriving_key,
     };
 
-    let incoming_view_key = spender_key.incoming_view_key().clone();
-    let outgoing_view_key: OutgoingViewKey = spender_key.outgoing_view_key().clone();
+    let incoming_view_key = secret_config.spender_key.incoming_view_key().clone();
+    let outgoing_view_key: OutgoingViewKey = group_secret_key.outgoing_view_key().clone();
 
     let public_address = incoming_view_key.public_address();
 
@@ -100,10 +101,11 @@ mod test {
             sapling_key.view_key.to_bytes(),
             "should have the same incoming viewing key"
         );
+        let sapling_key_clone = sapling_key.clone();
 
         assert_eq!(
             trusted_dealer_key_packages.public_address,
-            sapling_key.public_address(),
+            sapling_key_clone.public_address(),
             "should have the same public address"
         );