diff --git a/crates/cli/src/commands/test/peers.rs b/crates/cli/src/commands/test/peers.rs index 454e52af..1eb982ae 100644 --- a/crates/cli/src/commands/test/peers.rs +++ b/crates/cli/src/commands/test/peers.rs @@ -424,17 +424,23 @@ fn parse_peers(enr_strings: &[String]) -> Result> { .collect() } -// enr must be ASCII-only +/// Shortens an ENR to `...` for display. +/// +/// ENRs are base64 so in practice ASCII, but the string comes from `--enrs` +/// config input: `str::get` returns `None` mid-code-point, so walk inwards to +/// the nearest boundary rather than slicing bytes and panicking. fn format_enr(enr: &str) -> String { if enr.len() <= 17 { return enr.to_string(); } - let bytes = enr.as_bytes(); - format!( - "{}...{}", - std::str::from_utf8(&bytes[..13]).expect("ENR must be ASCII"), - std::str::from_utf8(&bytes[enr.len().saturating_sub(4)..]).expect("ENR must be ASCII"), - ) + let head = (0..=13) + .rev() + .find_map(|i| enr.get(..i)) + .unwrap_or_default(); + let tail = (enr.len().saturating_sub(4)..=enr.len()) + .find_map(|i| enr.get(i..)) + .unwrap_or_default(); + format!("{head}...{tail}") } fn peer_target_name(peer: &Peer, enr_str: &str) -> String { @@ -1429,4 +1435,29 @@ mod tests { results.keys().collect::>() ); } + + #[test] + fn format_enr_pins_ascii_output() { + assert_eq!(format_enr("enr:short"), "enr:short"); + // 17 bytes is still returned verbatim. + assert_eq!(format_enr("enr:-abcdefghijkl"), "enr:-abcdefghijkl"); + assert_eq!( + format_enr("enr:-Ku4QHqVeJ8PPzcvW1234567890"), + "enr:-Ku4QHqVe...7890" + ); + } + + #[test] + fn format_enr_truncates_on_char_boundaries() { + // A 2-byte code point straddles byte 13 (the head cut). + let head = format!("{}{}", "a".repeat(12), "é".repeat(6)); + assert_eq!(format_enr(&head), format!("{}...éé", "a".repeat(12))); + + // A 2-byte code point straddles the tail cut (len - 4). + let tail = format!("{}{}", "a".repeat(17), "é".repeat(3)); + assert_eq!(format_enr(&tail), format!("{}...éé", "a".repeat(13))); + + // All multi-byte, both cuts land mid-code-point. + assert_eq!(format_enr(&"€".repeat(10)), "€€€€...€"); + } } diff --git a/crates/frost/src/frost_core.rs b/crates/frost/src/frost_core.rs index 7a9783fc..d15d4c3a 100644 --- a/crates/frost/src/frost_core.rs +++ b/crates/frost/src/frost_core.rs @@ -34,6 +34,9 @@ pub enum FrostCoreError { /// The commitment has no coefficients. #[error("incorrect commitment")] IncorrectCommitment, + /// The polynomial has no coefficients. + #[error("empty polynomial")] + EmptyPolynomial, } /// A participant identifier wrapping a non-zero scalar. @@ -162,8 +165,13 @@ impl SigningShare { } /// Evaluate the polynomial defined by `coefficients` at `peer`. - pub fn from_coefficients(coefficients: &[Scalar], peer: Identifier) -> Self { - Self::new(evaluate_polynomial(peer, coefficients)) + /// + /// Returns [`FrostCoreError::EmptyPolynomial`] if `coefficients` is empty. + pub fn from_coefficients( + coefficients: &[Scalar], + peer: Identifier, + ) -> Result { + Ok(Self::new(evaluate_polynomial(peer, coefficients)?)) } } @@ -412,7 +420,13 @@ impl PublicKeyPackage { /// /// See: #[allow(clippy::arithmetic_side_effects)] -fn evaluate_polynomial(identifier: Identifier, coefficients: &[Scalar]) -> Scalar { +fn evaluate_polynomial( + identifier: Identifier, + coefficients: &[Scalar], +) -> Result { + let a0 = *coefficients + .first() + .ok_or(FrostCoreError::EmptyPolynomial)?; let mut value = Scalar::ZERO; let x = identifier.to_scalar(); @@ -420,11 +434,7 @@ fn evaluate_polynomial(identifier: Identifier, coefficients: &[Scalar]) -> Scala value = value + *coeff; value = value * x; } - value = value - + *coefficients - .first() - .expect("coefficients must have at least one element"); - value + Ok(value + a0) } /// Evaluate the VSS verification equation at `identifier`. @@ -587,4 +597,13 @@ mod tests { Err(FrostCoreError::IncorrectNumberOfCommitments) )); } + + #[test] + fn from_coefficients_rejects_empty_polynomial() { + let peer = Identifier::from_u32(1).expect("identifier"); + assert!(matches!( + SigningShare::from_coefficients(&[], peer), + Err(FrostCoreError::EmptyPolynomial) + )); + } } diff --git a/crates/frost/src/kryptology.rs b/crates/frost/src/kryptology.rs index 858b597a..cdfe2d21 100644 --- a/crates/frost/src/kryptology.rs +++ b/crates/frost/src/kryptology.rs @@ -359,7 +359,7 @@ pub fn round1( continue; } let j_id = Identifier::from_u32(j)?; - let mut share_scalar = SigningShare::from_coefficients(&coefficients, j_id).to_scalar(); + let mut share_scalar = SigningShare::from_coefficients(&coefficients, j_id)?.to_scalar(); shares.insert( j, ShamirShare { @@ -431,7 +431,7 @@ pub fn round2( let own_identifier = Identifier::from_u32(secret.id)?; let mut own_share_scalar = - SigningShare::from_coefficients(&secret.coefficients, own_identifier).to_scalar(); + SigningShare::from_coefficients(&secret.coefficients, own_identifier)?.to_scalar(); let mut peer_commitments: BTreeMap = BTreeMap::new(); diff --git a/crates/p2p/src/k1.rs b/crates/p2p/src/k1.rs index 9665b9f3..cad5e6e9 100644 --- a/crates/p2p/src/k1.rs +++ b/crates/p2p/src/k1.rs @@ -20,6 +20,10 @@ pub enum K1Error { /// IOError. #[error("IO error: {0}")] IoError(#[from] std::io::Error), + + /// The backup path already exists as a directory. + #[error("backup path is a directory: {0}")] + BackupPathIsDir(PathBuf), } /// Returns the charon-enr-private-key path relative to the data dir. @@ -58,19 +62,21 @@ fn backup_priv_key(data_dir: &Path) -> Result<()> { let current_time = chrono::Utc::now(); let nonce = OsRng.next_u64(); - let backup_path = data_dir.join(KEY_BACKUP_DIR).join(format!( + let backup_dir = data_dir.join(KEY_BACKUP_DIR); + let backup_path = backup_dir.join(format!( "{}_{}", current_time.format("%Y-%m-%d_%H-%M-%S_%f"), nonce )); - std::fs::create_dir_all( - backup_path - .parent() - .expect("Backup path parent should exist"), - ) - .map_err(K1Error::IoError)?; + std::fs::create_dir_all(&backup_dir).map_err(K1Error::IoError)?; + copy_backup(&key_path, &backup_path) +} + +/// Copies the private key to `backup_path`, which must not already exist as a +/// directory. +fn copy_backup(key_path: &Path, backup_path: &Path) -> Result<()> { if backup_path.is_dir() { - panic!("Backup path is a directory: {:?}", backup_path); + return Err(K1Error::BackupPathIsDir(backup_path.to_path_buf())); } std::fs::copy(key_path, backup_path).map_err(K1Error::IoError)?; Ok(()) @@ -225,4 +231,18 @@ mod tests { Ok(()) } + + #[test] + fn copy_backup_rejects_existing_directory() -> Result<()> { + let temp_dir = setup_temp_dir(); + let key = key_path(temp_dir.path()); + fs::write(&key, "key")?; + let backup_path = temp_dir.path().join("backup"); + fs::create_dir(&backup_path)?; + + let err = copy_backup(&key, &backup_path).expect_err("directory must be rejected"); + assert!(matches!(err, K1Error::BackupPathIsDir(_))); + + Ok(()) + } }