diff --git a/.gitignore b/.gitignore index ccb959f..3267c9b 100644 --- a/.gitignore +++ b/.gitignore @@ -10,9 +10,14 @@ target/ .vscode/ mina-frost-client/tests/assets/ +mina-frost-client/tests/assets-duplicate/ # GraphQL schemas *.graphql -# AI generated folders +# AI generated folders and files .claude/ +.codex + +# Playground +playground/ diff --git a/Cargo.lock b/Cargo.lock index 7f1a1ed..0a3d4d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -275,7 +275,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "246a225cc6131e9ee4f24619af0f19d67761fff15d7ccc22e42b80846e69449a" dependencies = [ "num-traits", - "rand 0.8.5", + "rand 0.8.6", "rayon", ] @@ -1785,7 +1785,7 @@ dependencies = [ "mina-signer", "mina-tx", "postcard", - "rand 0.8.5", + "rand 0.8.6", "rand_core 0.6.4", "regex", "reqwest", @@ -1828,7 +1828,7 @@ dependencies = [ "mina-curves", "o1-utils", "once_cell", - "rand 0.8.5", + "rand 0.8.6", "rayon", "serde", "serde_with", @@ -1849,7 +1849,7 @@ dependencies = [ "mina-hasher", "num-bigint", "o1-utils", - "rand 0.8.5", + "rand 0.8.6", "serde", "sha2", "thiserror 2.0.18", @@ -1910,7 +1910,7 @@ checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" dependencies = [ "num-integer", "num-traits", - "rand 0.8.5", + "rand 0.8.6", "serde", ] @@ -1951,7 +1951,7 @@ dependencies = [ "hex", "num-bigint", "num-integer", - "rand 0.8.5", + "rand 0.8.6", "rand_core 0.6.4", "rayon", "rmp-serde", @@ -2182,7 +2182,7 @@ dependencies = [ "bit-vec", "bitflags", "num-traits", - "rand 0.9.2", + "rand 0.9.4", "rand_chacha 0.9.0", "rand_xorshift", "regex-syntax", @@ -2226,7 +2226,7 @@ dependencies = [ "bytes", "getrandom 0.3.4", "lru-slab", - "rand 0.9.2", + "rand 0.9.4", "ring", "rustc-hash", "rustls", @@ -2281,9 +2281,9 @@ checksum = "dc33ff2d4973d518d823d61aa239014831e521c75da58e3df4840d3f47749d09" [[package]] name = "rand" -version = "0.8.5" +version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +checksum = "5ca0ecfa931c29007047d1bc58e623ab12e5590e8c7cc53200d5202b69266d8a" dependencies = [ "libc", "rand_chacha 0.3.1", @@ -2292,9 +2292,9 @@ dependencies = [ [[package]] name = "rand" -version = "0.9.2" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6db2770f06117d490610c7488547d543617b21bfa07796d7a12f6f1bd53850d1" +checksum = "44c5af06bb1b7d3216d91932aed5265164bf384dc89cd6ba05cf59a35f5f76ea" dependencies = [ "rand_chacha 0.9.0", "rand_core 0.9.5", @@ -2302,9 +2302,9 @@ dependencies = [ [[package]] name = "rand" -version = "0.10.0" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc266eb313df6c5c09c1c7b1fbe2510961e5bcd3add930c1e31f7ed9da0feff8" +checksum = "d2e8e8bcc7961af1fdac401278c6a831614941f6164ee3bf4ce61b7edb162207" dependencies = [ "chacha20 0.10.0", "getrandom 0.4.2", @@ -2599,9 +2599,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.10" +version = "0.103.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df33b2b81ac578cabaf06b89b0631153a3f416b0a886e8a7a1707fb51abbd1ef" +checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" dependencies = [ "ring", "rustls-pki-types", @@ -2996,7 +2996,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.2", + "getrandom 0.3.4", "once_cell", "rustix", "windows-sys 0.61.2", @@ -3327,7 +3327,7 @@ checksum = "a68d3c8f01c0cfa54a75291d83601161799e4a89a39e0929f4b0354d88757a37" dependencies = [ "getrandom 0.4.2", "js-sys", - "rand 0.10.0", + "rand 0.10.1", "serde_core", "wasm-bindgen", ] @@ -3887,7 +3887,7 @@ dependencies = [ "derive_more", "ed25519", "ed25519-dalek", - "rand 0.8.5", + "rand 0.8.6", "sha2", "x25519-dalek", "zeroize", diff --git a/SIGNING-WORKFLOW.md b/SIGNING-WORKFLOW.md index b7bd6be..6762358 100644 --- a/SIGNING-WORKFLOW.md +++ b/SIGNING-WORKFLOW.md @@ -19,7 +19,7 @@ This guide walks you through signing Mina transactions using FROST threshold sig | Join DKG (participant) | `mina-frost-client dkg -d -s -t -c ` | | List groups | `mina-frost-client groups -c ` | | Coordinate signing | `mina-frost-client coordinator -g -S -m -o -n -c ` | -| Join signing | `mina-frost-client participant -g -c ` | +| Join signing | `mina-frost-client participant -g -S -c ` | | Build GraphQL | `mina-frost-client graphql-build -i -o ` | | Broadcast | `mina-frost-client graphql-broadcast -g -e ` | @@ -601,7 +601,7 @@ mina-frost-client participant \ | `-c` | Path to participant's config file | | `-s` | Server URL (optional if stored in group config) | | `-g` | Group public key | -| `-S` | Session ID (optional if only one active session) | +| `-S` | Session ID (use `sessions` command to list if coordinator output was lost) | | `-y` | Auto-approve signing (skip confirmation prompt) | **Example:** @@ -611,6 +611,7 @@ mina-frost-client participant \ -c ~/.frost/bob.toml \ -s localhost:2744 \ -g \ + -S \ -y # Eve joins @@ -618,6 +619,7 @@ mina-frost-client participant \ -c ~/.frost/eve.toml \ -s localhost:2744 \ -g \ + -S \ -y ``` diff --git a/mina-frost-client/examples/signing_example/signing_example.sh b/mina-frost-client/examples/signing_example/signing_example.sh index a5a2843..ea0d98e 100755 --- a/mina-frost-client/examples/signing_example/signing_example.sh +++ b/mina-frost-client/examples/signing_example/signing_example.sh @@ -102,22 +102,41 @@ use_frost_client coordinator \ -o "$GENERATED_DIR/signature.json" & COORDINATOR_PID=$! -# Give coordinator time to create the signing session +# Wait for the coordinator to create the signing session and get its ID echo "Waiting for coordinator to create signing session..." -sleep 5 +SESSION_ID="" +for _i in $(seq 1 20); do + SESSION_ID=$(use_frost_client sessions \ + -c "$GENERATED_DIR/alice.toml" \ + --server-url "$SERVER_URL" \ + --group "$GROUP_PUBLIC_KEY" 2>&1 \ + | sed -n 's/^Session with ID //p' | head -n1) + [ -n "$SESSION_ID" ] && break + sleep 0.5 +done + +if [ -z "$SESSION_ID" ]; then + echo "ERROR: Could not get session ID from coordinator" + exit 1 +fi +echo "Session ID: $SESSION_ID" echo "Starting participant (Bob)..." -echo "y" | use_frost_client participant \ +use_frost_client participant \ -c "$GENERATED_DIR/bob.toml" \ --server-url "$SERVER_URL" \ - --group "$GROUP_PUBLIC_KEY" & + --group "$GROUP_PUBLIC_KEY" \ + --session "$SESSION_ID" \ + -y & BOB_PID=$! echo "Starting participant (Eve)..." -echo "y" | use_frost_client participant \ +use_frost_client participant \ -c "$GENERATED_DIR/eve.toml" \ --server-url "$SERVER_URL" \ - --group "$GROUP_PUBLIC_KEY" & + --group "$GROUP_PUBLIC_KEY" \ + --session "$SESSION_ID" \ + -y & EVE_PID=$! # Wait for completion diff --git a/mina-frost-client/src/api.rs b/mina-frost-client/src/api.rs index b414d94..479c915 100644 --- a/mina-frost-client/src/api.rs +++ b/mina-frost-client/src/api.rs @@ -76,6 +76,12 @@ impl std::fmt::Debug for PublicKey { } } +impl std::fmt::Display for PublicKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", hex::encode(&self.0)) + } +} + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct SendArgs { pub session_id: Uuid, diff --git a/mina-frost-client/src/cli/args.rs b/mina-frost-client/src/cli/args.rs index ce92bc0..34fee89 100644 --- a/mina-frost-client/src/cli/args.rs +++ b/mina-frost-client/src/cli/args.rs @@ -204,10 +204,9 @@ pub enum Command { /// to list) #[arg(short, long)] group: String, - /// The session ID to use (use `sessions` to list). Can be omitted in - /// case there is a single active session. + /// The session ID to use (use `sessions` to list). #[arg(short = 'S', long)] - session: Option, + session: String, /// Automatically answer yes to signing any package. #[arg(short = 'y', long, default_value_t = false)] yes: bool, diff --git a/mina-frost-client/src/cli/participant.rs b/mina-frost-client/src/cli/participant.rs index 1950dae..aa8b27e 100644 --- a/mina-frost-client/src/cli/participant.rs +++ b/mina-frost-client/src/cli/participant.rs @@ -41,7 +41,7 @@ pub async fn run_bluepallas(args: &Command) -> Result<(), Box> { &group_config, key_package, server_url, - session, + &session, )?; // Execute signing @@ -84,7 +84,7 @@ fn setup_participant_config( group_config: &Group, key_package: KeyPackage, server_url: Option, - session: Option, + session: &str, ) -> Result, Box> { // Determine server URL let server_url = if let Some(server_url) = server_url { @@ -113,7 +113,7 @@ fn setup_participant_config( port: server_url_parsed .port_or_known_default() .expect("always works for https"), - session_id: session.unwrap_or_default(), + session_id: session.to_owned(), comm_privkey: Some( user_config .communication_key diff --git a/mina-frost-client/src/coordinator/comms/http.rs b/mina-frost-client/src/coordinator/comms/http.rs index 6149629..98b89b6 100644 --- a/mina-frost-client/src/coordinator/comms/http.rs +++ b/mina-frost-client/src/coordinator/comms/http.rs @@ -1,7 +1,7 @@ //! HTTP implementation of the Comms trait. use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, HashMap, HashSet}, error::Error, io::{BufRead, Write}, marker::PhantomData, @@ -98,12 +98,10 @@ impl Comms for HTTPComms { }) .await?; - if self.config.signers.is_empty() { - eprintln!( - "Send the following session ID to participants: {}", - r.session_id - ); - } + eprintln!( + "Send the following session ID to participants: {}", + r.session_id + ); self.session_id = Some(r.session_id); let Some(comm_privkey) = &self.config.comm_privkey else { @@ -119,6 +117,7 @@ impl Comms for HTTPComms { eprint!("Waiting for participants to send their commitments..."); + let mut commitment_senders: HashSet = HashSet::new(); loop { let r = self .client @@ -128,8 +127,37 @@ impl Comms for HTTPComms { }) .await?; for msg in r.msgs { - let msg = cipher.decrypt(msg)?; - self.state.recv(msg)?; + // A participant may rejoin with a fresh Noise context; warn and skip to avoid DoS. + if commitment_senders.contains(&msg.sender) { + eprintln!( + "Warning: participant {} attempted to rejoin the session; ignoring", + msg.sender + ); + continue; + } + let sender = msg.sender.clone(); + // A malicious or broken participant must not be able to kill the coordinator. + let msg = match cipher.decrypt(msg) { + Ok(msg) => msg, + Err(_) => { + eprintln!( + "Warning: failed to decrypt message from {}; ignoring", + sender + ); + continue; + } + }; + match self.state.recv(msg) { + Ok(()) => { + commitment_senders.insert(sender); + } + Err(e) => { + eprintln!( + "Warning: ignoring invalid commitment from {}: {}", + sender, e + ); + } + } } tokio::time::sleep(Duration::from_secs(2)).await; eprint!("."); @@ -185,6 +213,7 @@ impl Comms for HTTPComms { eprintln!("Waiting for participants to send their SignatureShares..."); + let mut seen_share_senders: HashSet = HashSet::new(); loop { let r = self .client @@ -194,8 +223,37 @@ impl Comms for HTTPComms { }) .await?; for msg in r.msgs { - let msg = cipher.decrypt(msg)?; - self.state.recv(msg)?; + // A participant may rejoin with a fresh Noise context; warn and skip to avoid DoS. + if seen_share_senders.contains(&msg.sender) { + eprintln!( + "Warning: participant {} attempted to rejoin the session; ignoring", + msg.sender + ); + continue; + } + let sender = msg.sender.clone(); + // A malicious or broken participant must not be able to kill the coordinator. + let msg = match cipher.decrypt(msg) { + Ok(msg) => msg, + Err(_) => { + eprintln!( + "Warning: failed to decrypt message from {}; ignoring", + sender + ); + continue; + } + }; + match self.state.recv(msg) { + Ok(()) => { + seen_share_senders.insert(sender); + } + Err(e) => { + eprintln!( + "Warning: ignoring invalid signature share from {}: {}", + sender, e + ); + } + } } tokio::time::sleep(Duration::from_secs(2)).await; eprint!("."); diff --git a/mina-frost-client/src/participant/comms/http.rs b/mina-frost-client/src/participant/comms/http.rs index cf5c834..c56a15b 100644 --- a/mina-frost-client/src/participant/comms/http.rs +++ b/mina-frost-client/src/participant/comms/http.rs @@ -168,19 +168,9 @@ where ); eprintln!("Joining signing session..."); - let session_id = match self.session_id { - Some(s) => s, - None => { - // Get session ID from server - let r = self.client.list_sessions().await?; - if r.session_ids.len() > 1 { - return Err(eyre!("user has more than one FROST session active; use `mina-frost-client sessions` to list them and specify the session ID with `-S`").into()); - } else if r.session_ids.is_empty() { - return Err(eyre!("User has no current sessions active. The Coordinator should either specify your username, or manually share the session ID which you can specify with --session_id").into()); - } - r.session_ids[0] - } - }; + let session_id = self + .session_id + .ok_or_else(|| eyre!("session ID is required; use `-S` to specify it"))?; self.session_id = Some(session_id); let (Some(comm_privkey), Some(comm_coordinator_pubkey_getter)) = ( diff --git a/mina-frost-client/tests/duplicate_participant.rs b/mina-frost-client/tests/duplicate_participant.rs new file mode 100644 index 0000000..cc8ffbc --- /dev/null +++ b/mina-frost-client/tests/duplicate_participant.rs @@ -0,0 +1,196 @@ +mod helpers; + +use helpers::{ + binary_name, build_client_binary, form_group_with_dkg, get_session_id, greet_participants, + group_keys_from_config, introduce_participant, participant_args, run_cli_spawn_piped, + start_frostd, ChildGuard, CliParticipant, SigningParticipant, +}; +use lazy_static::lazy_static; +use std::fs; +use std::io::Result; +use std::path::{Path, PathBuf}; +use std::process::{Child, Output}; +use std::thread; +use std::time::Duration; + +lazy_static! { + static ref binary_path: PathBuf = PathBuf::from(format!( + "{}/../target/release/{}", + env!("CARGO_MANIFEST_DIR"), + binary_name() + )); + static ref working_dir: PathBuf = PathBuf::from(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/assets-duplicate" + )); + static ref message_path: PathBuf = PathBuf::from(concat!( + env!("CARGO_MANIFEST_DIR"), + "/examples/signing_example/message.json" + )); +} + +const SIG_FILE: &str = "signature.json"; +const NETWORK_ID: &str = "testnet"; + +fn setup() -> Result { + if working_dir.exists() { + fs::remove_dir_all(working_dir.clone())?; + } + fs::create_dir_all(working_dir.clone())?; + + let built_binary = build_client_binary( + env!("CARGO_MANIFEST_DIR"), + None, + mina_tx::zkapp_tx::IS_MESA_HARDFORK, + ); + assert!( + built_binary.exists(), + "release client binary does not exist at {}", + built_binary.display() + ); + + start_frostd(&working_dir).map(ChildGuard) +} + +fn sign_with_duplicate_participant( + binary: &Path, + cwd: &Path, + group_pk_hex: &str, + msg_path: &Path, + sig_file: &str, + network_id: &str, + server_url: &str, + threshold: usize, + participants: &[SigningParticipant], +) -> Output { + assert!(participants.len() >= threshold && threshold > 0); + + let mut coord_args = vec![ + "coordinator".to_string(), + "-c".to_string(), + participants[0].config_path.clone(), + "-s".to_string(), + server_url.to_string(), + "--group".to_string(), + group_pk_hex.to_string(), + "-m".to_string(), + msg_path.to_string_lossy().to_string(), + "-o".to_string(), + sig_file.to_string(), + "-n".to_string(), + network_id.to_string(), + ]; + for participant in participants.iter().take(threshold) { + coord_args.push("-S".to_string()); + coord_args.push(participant.pubkey_hex.clone()); + } + + let coordinator = run_cli_spawn_piped(binary, cwd, &coord_args); + + let session_id = get_session_id( + binary, + cwd, + &participants[0].config_path, + server_url, + group_pk_hex, + ) + .expect("no signing session appeared after coordinator started"); + + let participant_children: Vec = participants + .iter() + .take(threshold) + .map(|p| { + let args = participant_args(p, server_url, group_pk_hex, &session_id); + run_cli_spawn_piped(binary, cwd, &args) + }) + .collect(); + + // Give them time to complete the Noise handshake and send their commitments + thread::sleep(Duration::from_millis(1500)); + + // Spawn a duplicate of the first participant (fresh Noise context, same session) + let dupe_args = participant_args(&participants[0], server_url, group_pk_hex, &session_id); + let mut dupe = run_cli_spawn_piped(binary, cwd, &dupe_args); + thread::sleep(Duration::from_secs(2)); + let _ = dupe.kill(); + let _ = dupe.wait(); + + for child in participant_children { + let _ = child.wait_with_output(); + } + + coordinator + .wait_with_output() + .expect("coordinator subprocess did not exit") +} + +/// Verifies that when a participant accidentally re-joins a session with a fresh +/// Noise context, the coordinator warns and continues rather than crashing, and +/// the signing session completes successfully. +#[test] +fn duplicate_participant_handled_gracefully() -> Result<()> { + let server_process = setup()?; + + let participants = (0..3) + .map(|x| introduce_participant(&binary_path, &working_dir, &x.to_string())) + .collect::>(); + + greet_participants(&binary_path, &working_dir, &participants); + + form_group_with_dkg( + &binary_path, + &working_dir, + &participants, + 2, + "localhost:2744", + "Duplicate Test Group", + )?; + + let (group_pk_hex, _) = + group_keys_from_config(&binary_path, &working_dir, &participants[0].toml); + + let signing_participants: Vec = participants + .iter() + .map(|p| SigningParticipant { + config_path: p.toml.clone(), + pubkey_hex: p.pubkey_hex.clone(), + }) + .collect(); + + let output = sign_with_duplicate_participant( + &binary_path, + &working_dir, + &group_pk_hex, + &message_path, + SIG_FILE, + NETWORK_ID, + "localhost:2744", + 2, + &signing_participants, + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!( + output.status.success(), + "coordinator failed unexpectedly\nstdout={stdout}\nstderr={stderr}" + ); + assert!( + stdout.contains("Signature saved"), + "coordinator did not save a signature\nstdout={stdout}\nstderr={stderr}" + ); + let warned = stderr.contains("attempted to rejoin the session; ignoring") + || stderr.contains("failed to decrypt message from"); + assert!( + warned, + "coordinator did not warn about the duplicate participant\nstderr={stderr}" + ); + assert!( + !stderr.contains("SnowError"), + "coordinator crashed with SnowError (regression)\nstderr={stderr}" + ); + + drop(server_process); + Ok(()) +} diff --git a/mina-frost-client/tests/helpers.rs b/mina-frost-client/tests/helpers.rs index af3ac3c..d2e9379 100644 --- a/mina-frost-client/tests/helpers.rs +++ b/mina-frost-client/tests/helpers.rs @@ -269,6 +269,59 @@ pub fn run_cli_spawn_piped(binary: &Path, cwd: &Path, args: &[String]) -> Child .expect("failed to spawn CLI command") } +/// Poll the server until a signing session appears, then return its ID. +pub fn get_session_id( + binary: &Path, + cwd: &Path, + config: &str, + server_url: &str, + group_pk_hex: &str, +) -> Option { + let re = Regex::new(r"Session with ID ([0-9a-f-]{36})").unwrap(); + for _ in 0..20 { + if let Ok(output) = Command::new(binary) + .args([ + "sessions", + "-c", + config, + "-s", + server_url, + "--group", + group_pk_hex, + ]) + .current_dir(cwd) + .output() + { + let stderr = String::from_utf8_lossy(&output.stderr); + if let Some(caps) = re.captures(&stderr) { + return Some(caps[1].to_string()); + } + } + thread::sleep(Duration::from_millis(500)); + } + None +} + +pub fn participant_args( + participant: &SigningParticipant, + server_url: &str, + group_pk_hex: &str, + session_id: &str, +) -> Vec { + vec![ + "participant".to_string(), + "-c".to_string(), + participant.config_path.clone(), + "-s".to_string(), + server_url.to_string(), + "--group".to_string(), + group_pk_hex.to_string(), + "-S".to_string(), + session_id.to_string(), + "-y".to_string(), + ] +} + pub fn sign_with_binary( binary: &Path, cwd: &Path, @@ -292,7 +345,7 @@ pub fn sign_with_binary( ); assert!(threshold > 0, "threshold must be > 0"); - let mut args = vec![ + let mut coord_args = vec![ "coordinator".to_string(), "-c".to_string(), participants[0].config_path.clone(), @@ -307,33 +360,29 @@ pub fn sign_with_binary( "-n".to_string(), network_id.to_string(), ]; - for participant in participants.iter().take(threshold) { - args.push("-S".to_string()); - args.push(participant.pubkey_hex.clone()); + coord_args.push("-S".to_string()); + coord_args.push(participant.pubkey_hex.clone()); } let mut children: Vec<(Vec, Child)> = Vec::new(); - children.push((args.clone(), run_cli_spawn_piped(binary, cwd, &args))); + children.push(( + coord_args.clone(), + run_cli_spawn_piped(binary, cwd, &coord_args), + )); - thread::sleep(Duration::from_secs(1)); + let session_id = get_session_id( + binary, + cwd, + &participants[0].config_path, + server_url, + group_pk_hex, + ) + .expect("no signing session appeared after coordinator started"); for participant in participants.iter().take(threshold) { - let participant_args = vec![ - "participant".to_string(), - "-c".to_string(), - participant.config_path.clone(), - "-s".to_string(), - server_url.to_string(), - "--group".to_string(), - group_pk_hex.to_string(), - "-y".to_string(), - ]; - - children.push(( - participant_args.clone(), - run_cli_spawn_piped(binary, cwd, &participant_args), - )); + let args = participant_args(participant, server_url, group_pk_hex, &session_id); + children.push((args.clone(), run_cli_spawn_piped(binary, cwd, &args))); } for (child_args, child) in children {