From 16b4306fb619cf3c909b3505406be3c10df77cb5 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 8 Jan 2025 19:22:13 -0300 Subject: [PATCH] coordinator, participant: don't send identifier; get it from channel authentication --- Cargo.lock | 1 - coordinator/Cargo.toml | 1 - coordinator/src/args.rs | 28 ++-------- coordinator/src/comms/http.rs | 90 +++++++++++++++---------------- frost-client/src/config.rs | 18 +++++++ frost-client/src/coordinator.rs | 17 +++--- frostd/src/types.rs | 20 +------ frostd/tests/integration_tests.rs | 35 ++++++------ participant/src/comms/http.rs | 20 +++---- 9 files changed, 98 insertions(+), 132 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cb21781..3f1bc18 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -674,7 +674,6 @@ version = "0.1.0" dependencies = [ "async-trait", "clap", - "derivative", "exitcode", "eyre", "frost-core", diff --git a/coordinator/Cargo.toml b/coordinator/Cargo.toml index 0f28396..bb559d4 100644 --- a/coordinator/Cargo.toml +++ b/coordinator/Cargo.toml @@ -7,7 +7,6 @@ edition = "2021" [dependencies] async-trait = "0.1.83" -derivative = "2.2.0" eyre = "0.6.12" frost-core = { version = "2.0.0", features = ["serde"] } frost-rerandomized = { version = "2.0.0-rc.0", features = ["serde"] } diff --git a/coordinator/src/args.rs b/coordinator/src/args.rs index f7485b6..a3825a9 100644 --- a/coordinator/src/args.rs +++ b/coordinator/src/args.rs @@ -1,15 +1,15 @@ use std::{ + collections::HashMap, env, error::Error, fs, io::{BufRead, Write}, - rc::Rc, }; use clap::Parser; use eyre::eyre; -use frost_core::{keys::PublicKeyPackage, Ciphersuite}; +use frost_core::{keys::PublicKeyPackage, Ciphersuite, Identifier}; use frost_rerandomized::Randomizer; use crate::input::read_from_file_or_stdin; @@ -85,10 +85,8 @@ pub struct ProcessedArgs { /// FROST server. pub http: bool, - /// The comma-separated keys of the signers to use in - /// HTTP mode. If HTTP mode is enabled and this is empty, then the session - /// ID will be printed and will have to be shared manually. - pub signers: Vec>, + /// Signers to use in HTTP mode, as a map of public keys to identifiers. + pub signers: HashMap, Identifier>, /// The number of participants. pub num_signers: u16, @@ -119,15 +117,6 @@ pub struct ProcessedArgs { /// The coordinator's communication public key for HTTP mode. pub comm_pubkey: Option>, - - /// A function that confirms if the public key of a participant is in the - /// user's contact book, returning the same public key, or None if not. For - /// HTTP mode. - // It is a `Rc` to make it easier to use; - // using `fn()` would preclude using closures and using generics would - // require a lot of code change for something simple. - #[allow(clippy::type_complexity)] - pub comm_participant_pubkey_getter: Option) -> Option>>>, } impl ProcessedArgs { @@ -158,12 +147,6 @@ impl ProcessedArgs { &args.public_key_package, )?; - let signers = args - .signers - .iter() - .map(|s| Ok(hex::decode(s)?.to_vec())) - .collect::>>()?; - let public_key_package: PublicKeyPackage = serde_json::from_str(&out)?; let messages = read_messages(&args.message, output, input)?; @@ -174,7 +157,7 @@ impl ProcessedArgs { Ok(ProcessedArgs { cli: args.cli, http: false, - signers, + signers: HashMap::new(), num_signers, public_key_package, messages, @@ -184,7 +167,6 @@ impl ProcessedArgs { port: args.port, comm_privkey: None, comm_pubkey: None, - comm_participant_pubkey_getter: None, }) } } diff --git a/coordinator/src/comms/http.rs b/coordinator/src/comms/http.rs index 263261d..6d7d7d3 100644 --- a/coordinator/src/comms/http.rs +++ b/coordinator/src/comms/http.rs @@ -16,9 +16,7 @@ use frost_core::{ Identifier, SigningPackage, }; -use frostd::{ - Msg, PublicKey, SendCommitmentsArgs, SendSignatureSharesArgs, SendSigningPackageArgs, Uuid, -}; +use frostd::{Msg, PublicKey, SendSigningPackageArgs, Uuid}; use participant::comms::http::Noise; use rand::thread_rng; use xeddsa::{xed25519, Sign as _}; @@ -36,8 +34,7 @@ pub struct SessionStateArgs { /// /// This can be used by a Coordinator to help maitain state and handle /// messages from the Participants. -#[derive(derivative::Derivative)] -#[derivative(Debug)] +#[derive(Debug)] pub enum SessionState { /// Waiting for participants to send their commitments. WaitingForCommitments { @@ -74,7 +71,11 @@ pub enum SessionState { impl SessionState { /// Create a new SessionState for the given number of messages and signers. - pub fn new(num_messages: usize, num_signers: usize) -> Self { + pub fn new( + num_messages: usize, + num_signers: usize, + pubkeys: HashMap, Identifier>, + ) -> Self { let args = SessionStateArgs { num_messages, num_signers, @@ -82,7 +83,7 @@ impl SessionState { Self::WaitingForCommitments { args, commitments: Default::default(), - pubkeys: Default::default(), + pubkeys, } } @@ -95,12 +96,12 @@ impl SessionState { pub fn recv(&mut self, msg: Msg) -> Result<(), Box> { match self { SessionState::WaitingForCommitments { .. } => { - let send_commitments_args: SendCommitmentsArgs = + let send_commitments_args: Vec> = serde_json::from_slice(&msg.msg)?; self.handle_commitments(msg.sender, send_commitments_args)?; } SessionState::WaitingForSignatureShares { .. } => { - let send_signature_shares_args: SendSignatureSharesArgs = + let send_signature_shares_args: Vec> = serde_json::from_slice(&msg.msg)?; self.handle_signature_share(msg.sender, send_signature_shares_args)?; } @@ -113,34 +114,31 @@ impl SessionState { fn handle_commitments( &mut self, pubkey: Vec, - send_commitments_args: SendCommitmentsArgs, + commitments: Vec>, ) -> Result<(), Box> { if let SessionState::WaitingForCommitments { args, - commitments, - pubkeys: usernames, + commitments: commitments_map, + pubkeys, } = self { - if send_commitments_args.commitments.len() != args.num_messages { + if commitments.len() != args.num_messages { return Err(eyre!("wrong number of commitments").into()); } + let identifier = *pubkeys.get(&pubkey).ok_or(eyre!("unknown participant"))?; // Add commitment to map. // Currently ignores the possibility of overwriting previous values // (it seems better to ignore overwrites, which could be caused by // poor networking connectivity leading to retries) - commitments.insert( - send_commitments_args.identifier, - send_commitments_args.commitments, - ); - usernames.insert(pubkey, send_commitments_args.identifier); + commitments_map.insert(identifier, commitments); // If complete, advance to next state - if commitments.len() == args.num_signers { + if commitments_map.len() == args.num_signers { *self = SessionState::WaitingForSignatureShares { args: args.clone(), - commitments: commitments.clone(), - pubkeys: usernames.clone(), + commitments: commitments_map.clone(), + pubkeys: pubkeys.clone(), signature_shares: Default::default(), } } @@ -199,37 +197,35 @@ impl SessionState { /// Handle signature share sent by a participant. fn handle_signature_share( &mut self, - _username: Vec, - send_signature_shares_args: SendSignatureSharesArgs, + pubkey: Vec, + signature_shares: Vec>, ) -> Result<(), Box> { if let SessionState::WaitingForSignatureShares { args, commitments, - signature_shares, - .. + signature_shares: signature_shares_map, + pubkeys, } = self { - if send_signature_shares_args.signature_share.len() != args.num_messages { + if signature_shares.len() != args.num_messages { return Err(eyre!("wrong number of signature shares").into()); } - if !commitments.contains_key(&send_signature_shares_args.identifier) { + let identifier = *pubkeys.get(&pubkey).ok_or(eyre!("unknown participant"))?; + if !commitments.contains_key(&identifier) { return Err(eyre!("invalid identifier").into()); } // Currently ignoring the possibility of overwriting previous values // (it seems better to ignore overwrites, which could be caused by // poor networking connectivity leading to retries) - signature_shares.insert( - send_signature_shares_args.identifier, - send_signature_shares_args.signature_share, - ); + signature_shares_map.insert(identifier, signature_shares); // If complete, advance to next state - if signature_shares.keys().cloned().collect::>() + if signature_shares_map.keys().cloned().collect::>() == commitments.keys().cloned().collect::>() { *self = SessionState::SignatureSharesReady { args: args.clone(), - signature_shares: signature_shares.clone(), + signature_shares: signature_shares_map.clone(), } } Ok(()) @@ -286,7 +282,11 @@ impl HTTPComms { session_id: None, access_token: None, args: args.clone(), - state: SessionState::new(args.messages.len(), args.num_signers as usize), + state: SessionState::new( + args.messages.len(), + args.num_signers as usize, + args.signers.clone(), + ), pubkeys: Default::default(), send_noise: None, recv_noise: None, @@ -387,7 +387,7 @@ impl Comms for HTTPComms { .post(format!("{}/create_new_session", self.host_port)) .bearer_auth(self.access_token.as_ref().expect("was just set")) .json(&frostd::CreateNewSessionArgs { - pubkeys: self.args.signers.iter().cloned().map(PublicKey).collect(), + pubkeys: self.args.signers.keys().cloned().map(PublicKey).collect(), message_count: 1, }) .send() @@ -403,21 +403,15 @@ impl Comms for HTTPComms { } self.session_id = Some(r.session_id); - let (Some(comm_privkey), Some(comm_participant_pubkey_getter)) = ( - &self.args.comm_privkey, - &self.args.comm_participant_pubkey_getter, - ) else { - return Err( - eyre!("comm_privkey and comm_participant_pubkey_getter must be specified").into(), - ); + let Some(comm_privkey) = &self.args.comm_privkey else { + return Err(eyre!("comm_privkey must be specified").into()); }; // If encryption is enabled, create the Noise objects let mut send_noise_map = HashMap::new(); let mut recv_noise_map = HashMap::new(); - for pubkey in &self.args.signers { - let comm_participant_pubkey = comm_participant_pubkey_getter(pubkey).ok_or_eyre("A participant in specified FROST session is not registered in the coordinator's address book")?; + for comm_participant_pubkey in self.args.signers.keys() { let builder = snow::Builder::new( "Noise_K_25519_ChaChaPoly_BLAKE2s" .parse() @@ -426,7 +420,7 @@ impl Comms for HTTPComms { let send_noise = Noise::new( builder .local_private_key(comm_privkey) - .remote_public_key(&comm_participant_pubkey) + .remote_public_key(comm_participant_pubkey) .build_initiator()?, ); let builder = snow::Builder::new( @@ -437,11 +431,11 @@ impl Comms for HTTPComms { let recv_noise = Noise::new( builder .local_private_key(comm_privkey) - .remote_public_key(&comm_participant_pubkey) + .remote_public_key(comm_participant_pubkey) .build_responder()?, ); - send_noise_map.insert(pubkey.clone(), send_noise); - recv_noise_map.insert(pubkey.clone(), recv_noise); + send_noise_map.insert(comm_participant_pubkey.clone(), send_noise); + recv_noise_map.insert(comm_participant_pubkey.clone(), recv_noise); } self.send_noise = Some(send_noise_map); self.recv_noise = Some(recv_noise_map); diff --git a/frost-client/src/config.rs b/frost-client/src/config.rs index 9fce935..a2743a0 100644 --- a/frost-client/src/config.rs +++ b/frost-client/src/config.rs @@ -7,6 +7,7 @@ use std::{ }; use eyre::{eyre, OptionExt}; +use frost_core::{Ciphersuite, Identifier}; use serde::{Deserialize, Serialize}; use crate::{ciphersuite_helper::ciphersuite_helper, contact::Contact, write_atomic}; @@ -102,6 +103,16 @@ impl Group { } Ok(s) } + + /// Get a group participant by their pubkey. + pub fn participant_by_pubkey(&self, pubkey: &[u8]) -> Result> { + Ok(self + .participant + .values() + .find(|p| p.pubkey == pubkey) + .cloned() + .ok_or_eyre("Participant not found")?) + } } /// A FROST group participant. @@ -121,6 +132,13 @@ pub struct Participant { pub pubkey: Vec, } +impl Participant { + /// Return the parsed identifier for the participant. + pub fn identifier(&self) -> Result, Box> { + Ok(Identifier::::deserialize(&self.identifier)?) + } +} + impl Config { /// Returns the default path of the config /// ($HOME/.config/frost/credentials.toml in Linux) if `path` is None, diff --git a/frost-client/src/coordinator.rs b/frost-client/src/coordinator.rs index 45db248..3acf7b5 100644 --- a/frost-client/src/coordinator.rs +++ b/frost-client/src/coordinator.rs @@ -1,5 +1,5 @@ +use std::collections::HashMap; use std::error::Error; -use std::rc::Rc; use coordinator::cli::cli_for_processed_args; use eyre::eyre; @@ -68,11 +68,14 @@ pub(crate) async fn run_for_ciphersuite( let signers = signers .iter() - .map(|s| Ok(hex::decode(s)?.to_vec())) - .collect::, Box>>()?; + .map(|s| { + let pubkey = hex::decode(s)?.to_vec(); + let contact = group.participant_by_pubkey(&pubkey)?; + Ok((pubkey, contact.identifier()?)) + }) + .collect::, Box>>()?; let num_signers = signers.len() as u16; - let group_participants = group.participant.clone(); let pargs = coordinator::args::ProcessedArgs { cli: false, http: true, @@ -102,12 +105,6 @@ pub(crate) async fn run_for_ciphersuite( .pubkey .clone(), ), - comm_participant_pubkey_getter: Some(Rc::new(move |participant_pubkey| { - group_participants - .values() - .find(|p| p.pubkey == *participant_pubkey) - .map(|p| p.pubkey.clone()) - })), }; cli_for_processed_args(pargs, &mut input, &mut output).await?; diff --git a/frostd/src/types.rs b/frostd/src/types.rs index 2920b5e..d042c37 100644 --- a/frostd/src/types.rs +++ b/frostd/src/types.rs @@ -1,6 +1,4 @@ -use frost_core::{ - round1::SigningCommitments, round2::SignatureShare, Ciphersuite, Identifier, SigningPackage, -}; +use frost_core::{Ciphersuite, SigningPackage}; use frost_rerandomized::Randomizer; use serde::{Deserialize, Serialize}; pub use uuid::Uuid; @@ -144,15 +142,7 @@ pub struct CloseSessionArgs { pub session_id: Uuid, } -#[derive(Clone, Debug, Serialize, Deserialize)] -#[serde(bound = "C: Ciphersuite")] -pub struct SendCommitmentsArgs { - pub identifier: Identifier, - pub commitments: Vec>, -} - #[derive(Serialize, Deserialize, derivative::Derivative)] -#[derivative(Debug)] #[serde(bound = "C: Ciphersuite")] pub struct SendSigningPackageArgs { pub signing_package: Vec>, @@ -161,13 +151,5 @@ pub struct SendSigningPackageArgs { deserialize_with = "serdect::slice::deserialize_hex_or_bin_vec" )] pub aux_msg: Vec, - #[derivative(Debug = "ignore")] pub randomizer: Vec>, } - -#[derive(Clone, Debug, Serialize, Deserialize)] -#[serde(bound = "C: Ciphersuite")] -pub struct SendSignatureSharesArgs { - pub identifier: Identifier, - pub signature_share: Vec>, -} diff --git a/frostd/tests/integration_tests.rs b/frostd/tests/integration_tests.rs index 3e17908..54e2caa 100644 --- a/frostd/tests/integration_tests.rs +++ b/frostd/tests/integration_tests.rs @@ -1,12 +1,13 @@ use core::str; -use std::{collections::BTreeMap, error::Error, time::Duration}; +use std::{ + collections::{BTreeMap, HashMap}, + error::Error, + time::Duration, +}; use axum_test::TestServer; use coordinator::comms::http::SessionState; -use frostd::{ - args::Args, router, AppState, SendCommitmentsArgs, SendSignatureSharesArgs, - SendSigningPackageArgs, -}; +use frostd::{args::Args, router, AppState, SendSigningPackageArgs}; use rand::thread_rng; use reqwest::Certificate; @@ -163,10 +164,6 @@ async fn test_main_router< nonces_map.insert(*identifier, nonces_vec); // Send commitments to server - let send_commitments_args = SendCommitmentsArgs { - identifier: *identifier, - commitments: commitments_vec, - }; let res = server .post("/send") .authorization_bearer(token) @@ -174,7 +171,7 @@ async fn test_main_router< session_id, // Empty recipients: Coordinator recipients: vec![], - msg: serde_json::to_vec(&send_commitments_args)?, + msg: serde_json::to_vec(&commitments_vec)?, }) .await; if res.status_code() != 200 { @@ -183,7 +180,17 @@ async fn test_main_router< } // As the coordinator, get the commitments - let mut coordinator_state = SessionState::::new(2, 2); + let pubkey_identifier_map = HashMap::from([ + ( + alice_keypair.public.clone(), + *key_packages.first_key_value().unwrap().0, + ), + ( + bob_keypair.public.clone(), + *key_packages.last_key_value().unwrap().0, + ), + ]); + let mut coordinator_state = SessionState::::new(2, 2, pubkey_identifier_map); loop { let res = server .post("/receive") @@ -296,10 +303,6 @@ async fn test_main_router< }; // Send SignatureShares to the server - let send_signature_shares_args = SendSignatureSharesArgs { - identifier: *identifier, - signature_share: signature_shares, - }; let res = server .post("/send") .authorization_bearer(token) @@ -307,7 +310,7 @@ async fn test_main_router< session_id, // Empty recipients: Coordinator recipients: vec![], - msg: serde_json::to_vec(&send_signature_shares_args)?, + msg: serde_json::to_vec(&signature_shares)?, }) .await; res.assert_status_ok(); diff --git a/participant/src/comms/http.rs b/participant/src/comms/http.rs index f0a076e..ea84935 100644 --- a/participant/src/comms/http.rs +++ b/participant/src/comms/http.rs @@ -111,7 +111,7 @@ pub struct HTTPComms { _phantom: PhantomData, } -use frostd::{SendCommitmentsArgs, SendSignatureSharesArgs, SendSigningPackageArgs, Uuid}; +use frostd::{SendSigningPackageArgs, Uuid}; // TODO: Improve error handling for invalid session id impl HTTPComms @@ -168,7 +168,7 @@ where _input: &mut dyn BufRead, _output: &mut dyn Write, commitments: SigningCommitments, - identifier: Identifier, + _identifier: Identifier, rerandomized: bool, ) -> Result< ( @@ -250,9 +250,7 @@ where ); }; - // If encryption is enabled, create the Noise objects - - // We need to know what is the username of the coordinator in order + // We need to know what is the pubkey of the coordinator in order // to encrypt message to them. let session_info = self .client @@ -291,10 +289,7 @@ where self.recv_noise = Some(recv_noise); // Send Commitments to Server - let send_commitments_args = SendCommitmentsArgs { - identifier, - commitments: vec![commitments], - }; + let send_commitments_args = vec![commitments]; let msg = self.encrypt(serde_json::to_vec(&send_commitments_args)?)?; self.client .post(format!("{}/send", self.host_port)) @@ -354,17 +349,14 @@ where async fn send_signature_share( &mut self, - identifier: Identifier, + _identifier: Identifier, signature_share: SignatureShare, ) -> Result<(), Box> { // Send signature share to Coordinator eprintln!("Sending signature share to coordinator..."); - let send_signature_shares_args = SendSignatureSharesArgs { - identifier, - signature_share: vec![signature_share], - }; + let send_signature_shares_args = vec![signature_share]; let msg = self.encrypt(serde_json::to_vec(&send_signature_shares_args)?)?;