From 4cbd624d73fcc879f9bf5553cdade1e95400152b Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:05:45 +0100 Subject: [PATCH 01/18] test(softsign): pin what keygen and import print and return --- tests/cli/mod.rs | 19 ++++ tests/cli/softsign.rs | 147 +++++++++++++++++++++++++++++++ tests/cli/yubihsm/keys/import.rs | 19 +--- 3 files changed, 167 insertions(+), 18 deletions(-) create mode 100644 tests/cli/softsign.rs diff --git a/tests/cli/mod.rs b/tests/cli/mod.rs index 628c58ac..29b88fc0 100644 --- a/tests/cli/mod.rs +++ b/tests/cli/mod.rs @@ -8,6 +8,8 @@ use std::{ use super::KMS_EXE_PATH; mod init; +#[cfg(feature = "softsign")] +mod softsign; mod version; #[cfg(feature = "yubihsm")] @@ -61,6 +63,23 @@ fn a_failed_command_is_reported_with_both_streams() { run_successfully(["not-a-real-subcommand"]); } +/// `text` without the ANSI colour codes `tmkms` writes even when not on a terminal +pub fn without_colour_codes(text: &str) -> String { + let mut plain = String::new(); + let mut chars = text.chars(); + + while let Some(c) = chars.next() { + if c == '\u{1b}' { + // A colour code runs from the escape character to the next `m` + chars.by_ref().find(|&c| c == 'm'); + } else { + plain.push(c); + } + } + + plain +} + #[test] fn test_usage() { let status_code = run(&[] as &[&OsStr]).status.code().unwrap(); diff --git a/tests/cli/softsign.rs b/tests/cli/softsign.rs new file mode 100644 index 00000000..a06685e4 --- /dev/null +++ b/tests/cli/softsign.rs @@ -0,0 +1,147 @@ +//! Integration tests for the `softsign` subcommands + +use crate::cli; +use std::{fs, os::unix::fs::PermissionsExt, str}; +use tmkms::key_utils; + +/// A priv_validator key file holding an Ed25519 key +const ED25519_PRIV_VALIDATOR_PATH: &str = "tests/support/priv_validator_mock.json"; + +/// A priv_validator key file holding a secp256k1 key +const SECP256K1_PRIV_VALIDATOR_PATH: &str = "tests/support/priv_validator_secp256k1_mock.json"; + +/// Run `tmkms softsign` with `args`, and return its exit code and what it wrote to +/// stdout and stderr, with the colour codes removed +fn softsign(args: &[&str]) -> (Option, String, String) { + let out = cli::run(["softsign"].iter().chain(args)); + let plain = |stream: &[u8]| cli::without_colour_codes(str::from_utf8(stream).unwrap()); + (out.status.code(), plain(&out.stdout), plain(&out.stderr)) +} + +/// The permission bits of the file at `path` +fn mode(path: &str) -> u32 { + fs::metadata(path).unwrap().permissions().mode() & 0o777 +} + +/// Every outcome of `softsign keygen`: the exit code and every line it prints +#[test] +fn keygen_reports_every_outcome() { + let dir = tempfile::tempdir().unwrap(); + let path = |name: &str| dir.path().join(name).display().to_string(); + let (consensus_key, account_key) = (path("consensus.key"), path("account.key")); + let unwritable = path("missing/key"); + let write_error = format!( + "error: I/O error: couldn't write `{unwritable}`: No such file or directory (os error 2)\n" + ); + + let cases: Vec<(Vec<&str>, Option, String)> = vec![ + ( + vec!["keygen"], + Some(1), + "Usage: tmkms softsign keygen [-t account,consensus] PATH\n".into(), + ), + ( + vec!["keygen", "-t", "bogus", &consensus_key], + Some(1), + "error: unknown key type: bogus (must be 'account' or 'consensus')\n".into(), + ), + (vec!["keygen", &unwritable], Some(1), write_error.clone()), + ( + vec!["keygen", "-t", "account", &unwritable], + Some(1), + write_error, + ), + ( + vec!["keygen", &consensus_key], + Some(0), + format!(" Generated consensus (Ed25519) private key at: {consensus_key}\n"), + ), + ( + vec!["keygen", "-t", "account", &account_key], + Some(0), + format!(" Generated account (secp256k1) private key at: {account_key}\n"), + ), + ]; + + for (args, code, stderr) in cases { + assert_eq!(softsign(&args), (code, String::new(), stderr), "{args:?}"); + } + + assert_eq!((mode(&consensus_key), mode(&account_key)), (0o600, 0o600)); + key_utils::load_signing_key(&consensus_key).unwrap(); + key_utils::load_base64_secp256k1_key(&account_key).unwrap(); +} + +/// Every outcome of `softsign import`: the exit code and every line it prints +#[test] +fn import_reports_every_outcome() { + let dir = tempfile::tempdir().unwrap(); + let path = |name: &str| dir.path().join(name).display().to_string(); + let (ed25519_json, secp256k1_json) = + (ED25519_PRIV_VALIDATOR_PATH, SECP256K1_PRIV_VALIDATOR_PATH); + let (missing_json, output, unwritable) = ( + path("missing.json"), + path("output.key"), + path("missing/key"), + ); + let write_error = format!( + "error: I/O error: couldn't write `{unwritable}`: No such file or directory (os error 2)\n" + ); + + let failures: Vec<(Vec<&str>, String)> = vec![ + ( + vec!["import", ed25519_json], + "error: expected 2 arguments, got 1\n\n\ + Usage: tmkms softsign import [priv_validator.json] [output.key]\n" + .into(), + ), + ( + vec!["import", "-f", "bogus", ed25519_json, &output], + "error: config error: invalid key format: bogus (must be 'json' or 'raw')\n".into(), + ), + ( + vec!["import", "-f", "raw", ed25519_json, &output], + "error: config error: invalid key format: raw (must be 'json' or 'raw')\n".into(), + ), + ( + vec!["import", &missing_json, &output], + format!( + "error: couldn't load {missing_json}: StringTracer: failed to open file: \ + {missing_json}: No such file or directory (os error 2)\n" + ), + ), + ( + vec!["import", ed25519_json, &unwritable], + write_error.clone(), + ), + (vec!["import", secp256k1_json, &unwritable], write_error), + ]; + + for (args, stderr) in failures { + assert_eq!( + softsign(&args), + (Some(1), String::new(), stderr), + "{args:?}" + ); + } + + for (json, key, algorithm) in [ + (ed25519_json, path("ed25519.key"), "Ed25519"), + (secp256k1_json, path("secp256k1.key"), "Secp256k1"), + ] { + let (code, stdout, stderr) = softsign(&["import", json, &key]); + let logged = format!( + " INFO tmkms::commands::softsign::import: Imported {algorithm} private key to {key}\n" + ); + + assert_eq!((code, stderr.as_str()), (Some(0), ""), "{json}"); + assert!( + stdout.ends_with(&logged) && stdout.lines().count() == 1, + "{stdout}" + ); + assert_eq!(mode(&key), 0o600); + } + + key_utils::load_signing_key(path("ed25519.key")).unwrap(); + key_utils::load_base64_secp256k1_key(path("secp256k1.key")).unwrap(); +} diff --git a/tests/cli/yubihsm/keys/import.rs b/tests/cli/yubihsm/keys/import.rs index 8b994b04..a06b184a 100644 --- a/tests/cli/yubihsm/keys/import.rs +++ b/tests/cli/yubihsm/keys/import.rs @@ -38,24 +38,7 @@ fn run_writing_only_stderr(args: &[&str]) -> (Option, String) { ); let stderr = str::from_utf8(&out.stderr).unwrap(); - (out.status.code(), without_colour_codes(stderr)) -} - -/// `text` without the ANSI colour codes `tmkms` writes even when not on a terminal -fn without_colour_codes(text: &str) -> String { - let mut plain = String::new(); - let mut chars = text.chars(); - - while let Some(c) = chars.next() { - if c == '\u{1b}' { - // A colour code runs from the escape character to the next `m` - chars.by_ref().find(|&c| c == 'm'); - } else { - plain.push(c); - } - } - - plain + (out.status.code(), cli::without_colour_codes(stderr)) } /// Write `contents` to a file named `name` in `dir` and return its path From 4b012c72131976d426508def575b9c2b73904121 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:06:24 +0100 Subject: [PATCH 02/18] test(yubihsm): pin what setup prints and returns --- tests/cli/mod.rs | 27 +++++++++- tests/cli/yubihsm.rs | 2 + tests/cli/yubihsm/setup.rs | 107 +++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/cli/yubihsm/setup.rs diff --git a/tests/cli/mod.rs b/tests/cli/mod.rs index 29b88fc0..166ff61e 100644 --- a/tests/cli/mod.rs +++ b/tests/cli/mod.rs @@ -2,7 +2,8 @@ use std::{ ffi::OsStr, - process::{Command, Output}, + io::Write, + process::{Command, Output, Stdio}, }; use super::KMS_EXE_PATH; @@ -24,6 +25,30 @@ where Command::new(KMS_EXE_PATH).args(args).output().unwrap() } +/// Run the `tmkms` CLI command with the given arguments, typing `stdin` into it +pub fn run_with_stdin(args: I, stdin: &str) -> Output +where + I: IntoIterator, + S: AsRef, +{ + let mut child = Command::new(KMS_EXE_PATH) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + + // Dropping the handle closes stdin, as an operator ending the input would + child + .stdin + .take() + .unwrap() + .write_all(stdin.as_bytes()) + .unwrap(); + child.wait_with_output().unwrap() +} + /// Run the `tmkms` CLI command with the expectation that it will exit successfully, /// panicking with its stdout and stderr if it does not pub fn run_successfully(args: I) -> Output diff --git a/tests/cli/yubihsm.rs b/tests/cli/yubihsm.rs index 9a459256..5f2e80da 100644 --- a/tests/cli/yubihsm.rs +++ b/tests/cli/yubihsm.rs @@ -18,3 +18,5 @@ pub const SECP256K1_PRIV_VALIDATOR_PATH: &str = "tests/support/priv_validator_se mod detect; mod help; mod keys; +#[cfg(feature = "yubihsm-mock")] +mod setup; diff --git a/tests/cli/yubihsm/setup.rs b/tests/cli/yubihsm/setup.rs new file mode 100644 index 00000000..e537f3da --- /dev/null +++ b/tests/cli/yubihsm/setup.rs @@ -0,0 +1,107 @@ +//! Integration tests for the `yubihsm setup` subcommand, against the mock HSM + +use crate::cli; +use std::str; + +/// A valid 24-word admin mnemonic, so that everything derived from it is known +const MNEMONIC: &str = "abandon amount liar amount expire adjust cage candy arch gather drum \ + bullet absurd math era live bid rhythm alien crouch range attend journey \ + unaware"; + +/// What setup prints before it reads the mnemonic to restore +const RESTORE_PROMPT: &str = "Restoring and reprovisioning YubiHSM from existing 24-word \ + mnemonic phrase.\n\n\ + *** Enter mnemonic (separate words with spaces): "; + +/// What setup prints once it has read a valid mnemonic +const MNEMONIC_DECODED: &str = "\nMnemonic phrase decoded/checksummed successfully!\n\n"; + +/// What setup prints before it lists the keys it would install +const ERASE_WARNING: &str = "This process will *ERASE* the configured YubiHSM2 and \ + reinitialize it:\n\n\ + - YubiHSM serial: 0123456789\n\n\ + Authentication keys with the following IDs and passwords will be \ + created:\n\n"; + +/// The mnemonic and the keys derived from `MNEMONIC`, as setup lists them +const DERIVED_KEYS: &str = "- key 0x0001: admin:\n\n \ + abandon amount liar amount expire adjust cage candy arch gather drum bullet\n \ + absurd math era live bid rhythm alien crouch range attend journey unaware\n\n\ + - authkey 0x0002 [operator]: kms-operator-password-1ztuhkpagag6tp9tptl9qma2wcuzsvyff\n\ + - authkey 0x0003 [auditor]: kms-auditor-password-16zukfrl5lcs4tg9udcvv98fzfsx9zluq\n\ + - authkey 0x0004 [validator]: kms-validator-password-168w265r9rr9gjr3wu4j0dtzhkyadyy04\n\ + - wrapkey 0x0001 [primary]: \ + b55c675289521f334d544ff62f1860af42d62814a5233a16174faff5d46fbe7e\n"; + +/// The question setup asks before it erases the HSM +const CONFIRMATION: &str = "\n*** Are you SURE you want erase and reinitialize this HSM? (y/N): "; + +/// Run `yubihsm setup` against the mock HSM with `options`, typing `stdin` into it, and +/// return its exit code and what it wrote to stdout and stderr, with the colour codes +/// removed +fn setup(options: &[&str], stdin: &str) -> (Option, String, String) { + let args = ["yubihsm", "setup", "-c", super::KMS_CONFIG_PATH]; + let out = cli::run_with_stdin(args.iter().chain(options), stdin); + let plain = |stream: &[u8]| cli::without_colour_codes(str::from_utf8(stream).unwrap()); + (out.status.code(), plain(&out.stdout), plain(&out.stderr)) +} + +#[test] +fn setup_prints_the_keys_it_derives_from_a_restored_mnemonic() { + assert_eq!( + setup(&["-r", "-p"], &format!("{MNEMONIC}\n")), + ( + Some(0), + format!( + "{RESTORE_PROMPT}{MNEMONIC_DECODED}Below are all keys/passwords derived from \ + your 24-word admin mnemonic:\n\n{DERIVED_KEYS}" + ), + String::new() + ) + ); +} + +#[test] +fn setup_refuses_a_mnemonic_it_cannot_decode() { + assert_eq!( + setup(&["-r"], "not a mnemonic\n"), + ( + Some(1), + RESTORE_PROMPT.to_owned(), + "*** ERROR: Couldn't decode mnemonic\n".to_owned() + ) + ); +} + +#[test] +fn setup_stops_when_the_erase_is_not_confirmed() { + assert_eq!( + setup(&["-r"], &format!("{MNEMONIC}\nn\n")), + ( + Some(1), + format!( + "{RESTORE_PROMPT}{MNEMONIC_DECODED}{ERASE_WARNING}{DERIVED_KEYS}{CONFIRMATION}\ + Aborting\n" + ), + String::new() + ) + ); +} + +#[test] +fn setup_reinitializes_the_hsm_when_the_erase_is_confirmed() { + let (code, stdout, stderr) = setup(&["-r"], &format!("{MNEMONIC}\ny\n")); + let before_erase = + format!("{RESTORE_PROMPT}{MNEMONIC_DECODED}{ERASE_WARNING}{DERIVED_KEYS}{CONFIRMATION}"); + + assert_eq!( + (code, stderr.as_str()), + ( + Some(0), + " Success reinitialized YubiHSM (serial: 0123456789)\n" + ), + "{stdout}" + ); + assert!(stdout.starts_with(&before_erase), "{stdout}"); + assert!(stdout.contains("installing wrap key: primary:"), "{stdout}"); +} From 0a98cb9ec6dd10f39441c30446a8a7c292defac7 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:07:23 +0100 Subject: [PATCH 03/18] fix(softsign): wipe the key before keygen exits on a failed write --- src/commands/softsign/keygen.rs | 51 +++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/src/commands/softsign/keygen.rs b/src/commands/softsign/keygen.rs index 780a5927..34fb5127 100644 --- a/src/commands/softsign/keygen.rs +++ b/src/commands/softsign/keygen.rs @@ -1,6 +1,6 @@ //! `tmkms softsign keygen` subcommand -use crate::{key_utils, prelude::*}; +use crate::{error::Error, key_utils, prelude::*}; use abscissa_core::Command; use clap::Parser; use k256::ecdsa; @@ -32,7 +32,7 @@ impl Runnable for KeygenCommand { let output_path = &self.output_paths[0]; - match self + let generated = match self .key_type .as_ref() .map(AsRef::as_ref) @@ -47,30 +47,35 @@ impl Runnable for KeygenCommand { ); process::exit(1); } + }; + + // Exit only here, once the generator has returned and dropped the key: + // `process::exit` runs no destructors, so nothing would wipe it + if let Err(e) = generated { + status_err!("{}", e); + process::exit(1); } } } /// Randomly generate a Base64-encoded secp256k1 key and store it at the given path -fn generate_secp256k1_key(output_path: &Path) { +fn generate_secp256k1_key(output_path: &Path) -> Result<(), Error> { let signing_key = ecdsa::SigningKey::random(&mut OsRng); // `to_bytes` hands back a copy of the private scalar let secret_bytes = Zeroizing::new(signing_key.to_bytes()); - key_utils::write_base64_secret(output_path, &secret_bytes).unwrap_or_else(|e| { - status_err!("{}", e); - process::exit(1); - }); + key_utils::write_base64_secret(output_path, &secret_bytes)?; status_ok!( "Generated", "account (secp256k1) private key at: {}", output_path.display() ); + Ok(()) } /// Randomly generate a Base64-encoded Ed25519 key and store it at the given path -fn generate_ed25519_key(output_path: &Path) { +fn generate_ed25519_key(output_path: &Path) -> Result<(), Error> { // `Zeroizing` wipes the seed when this scope ends; `SigningKey` zeroizes // itself on drop, but the buffer it was built from would otherwise be left // in place @@ -80,14 +85,36 @@ fn generate_ed25519_key(output_path: &Path) { // the stack and leave that copy unwiped let sk = ed25519_dalek::SigningKey::from_bytes(&sk_bytes); - key_utils::write_base64_secret(output_path, sk.as_bytes()).unwrap_or_else(|e| { - status_err!("{}", e); - process::exit(1); - }); + key_utils::write_base64_secret(output_path, sk.as_bytes())?; status_ok!( "Generated", "consensus (Ed25519) private key at: {}", output_path.display() ); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `process::exit` runs no destructors, so a write failure that ended the process + /// where it happened would skip wiping the key just generated. It must come back to + /// `run` instead, after the key has been dropped. + #[test] + fn a_failed_write_comes_back_to_run() { + let dir = tempfile::tempdir().unwrap(); + let unwritable = dir.path().join("missing/key"); + + for generate in [generate_secp256k1_key, generate_ed25519_key] { + assert_eq!( + generate(&unwritable).unwrap_err().to_string(), + format!( + "I/O error: couldn't write `{}`: No such file or directory (os error 2)", + unwritable.display() + ) + ); + } + } } From cac6f7d6a08366c6d4c8564bc406a082403a2147 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:08:13 +0100 Subject: [PATCH 04/18] fix(softsign): wipe the key before import exits on a failed write --- src/commands/softsign/import.rs | 88 +++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 20 deletions(-) diff --git a/src/commands/softsign/import.rs b/src/commands/softsign/import.rs index c79eef9f..24571807 100644 --- a/src/commands/softsign/import.rs +++ b/src/commands/softsign/import.rs @@ -1,11 +1,15 @@ //! `tmkms softsign import` command -use crate::{config::provider::softsign::KeyFormat, key_utils, prelude::*}; +use crate::{config::provider::softsign::KeyFormat, error::Error, key_utils, prelude::*}; use abscissa_core::Command; use clap::Parser; use cometbft::PrivateKey; use cometbft_config::PrivValidatorKey; -use std::{path::PathBuf, process}; +use std::{ + path::{Path, PathBuf}, + process, +}; +use zeroize::Zeroizing; /// `import` command: import a `priv_validator.json` formatted key and convert /// it into the raw format used by the softsign backend (by default) @@ -54,25 +58,69 @@ impl Runnable for ImportCommand { }) .priv_key; - match secret_key { - PrivateKey::Ed25519(sk) => { - key_utils::write_base64_secret(output_path, sk.as_bytes()).unwrap_or_else(|e| { - status_err!("{}", e); - process::exit(1); - }); - info!("Imported Ed25519 private key to {}", output_path.display()); - } - PrivateKey::Secp256k1(sk) => { - key_utils::write_base64_secret(output_path, &sk.to_bytes()).unwrap_or_else(|e| { - status_err!("{}", e); - process::exit(1); - }); - info!( - "Imported Secp256k1 private key to {}", - output_path.display() - ); + // Exit only here, once `write_secret_key` has returned and dropped the key: + // `process::exit` runs no destructors, so nothing would wipe it + match write_secret_key(secret_key, output_path) { + Ok(algorithm) => info!( + "Imported {} private key to {}", + algorithm, + output_path.display() + ), + Err(e) => { + status_err!("{}", e); + process::exit(1); } - _ => unreachable!("unsupported priv_validator.json algorithm"), + } + } +} + +/// Write `secret_key` to `output_path` in the softsign format, and return the name of +/// its algorithm +fn write_secret_key(secret_key: PrivateKey, output_path: &Path) -> Result<&'static str, Error> { + match secret_key { + PrivateKey::Ed25519(sk) => { + key_utils::write_base64_secret(output_path, sk.as_bytes())?; + Ok("Ed25519") + } + PrivateKey::Secp256k1(sk) => { + // `to_bytes` hands back a copy of the private scalar + let secret_bytes = Zeroizing::new(sk.to_bytes()); + key_utils::write_base64_secret(output_path, &secret_bytes)?; + Ok("Secp256k1") + } + _ => unreachable!("unsupported priv_validator.json algorithm"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `process::exit` runs no destructors, so a write failure that ended the process + /// where it happened would skip wiping the imported key. It must come back to `run` + /// instead, after the key has been dropped. + #[test] + fn a_failed_write_comes_back_to_run() { + let dir = tempfile::tempdir().unwrap(); + let unwritable = dir.path().join("missing/key"); + + for key_file in [ + "tests/support/priv_validator_mock.json", + "tests/support/priv_validator_secp256k1_mock.json", + ] { + let secret_key = PrivValidatorKey::load_json_file(&key_file) + .unwrap() + .priv_key; + + assert_eq!( + write_secret_key(secret_key, &unwritable) + .unwrap_err() + .to_string(), + format!( + "I/O error: couldn't write `{}`: No such file or directory (os error 2)", + unwritable.display() + ) + ); } } } From 7f6b8f33893cb2544ce501007c703d2d1384f1f5 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:11:28 +0100 Subject: [PATCH 05/18] fix(yubihsm): wipe the mnemonic and derived keys on every setup exit --- src/commands/yubihsm/setup.rs | 153 +++++++++++++++++++++++++--------- 1 file changed, 114 insertions(+), 39 deletions(-) diff --git a/src/commands/yubihsm/setup.rs b/src/commands/yubihsm/setup.rs index bc2d1f73..c1fd9a6c 100644 --- a/src/commands/yubihsm/setup.rs +++ b/src/commands/yubihsm/setup.rs @@ -10,7 +10,7 @@ use hkdf::Hkdf; use sha2::Sha512; use std::{ fs::File, - io::{self, Write}, + io::{self, BufRead, Write}, path::PathBuf, process::exit, }; @@ -66,11 +66,55 @@ pub struct SetupCommand { pub write_report: Option, } +/// Why setup stopped before reinitializing the HSM +#[derive(Debug)] +enum Stop { + /// The mnemonic typed in to restore couldn't be decoded + UndecodableMnemonic, + + /// The operator didn't confirm erasing the HSM + EraseNotConfirmed, + + /// The HSM failed a command + Hsm(String), +} + +impl Stop { + /// Stop because the HSM failed a command + fn hsm(e: impl std::error::Error) -> Self { + Stop::Hsm(e.to_string()) + } +} + impl Runnable for SetupCommand { /// Perform initial YubiHSM dervice provisioning fn run(&self) { + // Exit only here, once `setup` has returned and dropped every secret it derived: + // `process::exit` runs no destructors, so nothing would wipe them + match self.setup() { + Ok(()) => (), + Err(Stop::UndecodableMnemonic) => { + eprintln!("*** ERROR: Couldn't decode mnemonic"); + exit(1); + } + Err(Stop::EraseNotConfirmed) => { + println!("Aborting"); + exit(1); + } + Err(Stop::Hsm(e)) => { + status_err!("HSM error: {}", e); + exit(1); + } + } + } +} + +impl SetupCommand { + /// Derive the admin mnemonic and every key from it, print them, and unless only + /// printing, erase and reinitialize the HSM with them + fn setup(&self) -> Result<(), Stop> { let hsm_connector = crate::yubihsm::connector(); - let hsm_serial_number = get_hsm_client(hsm_connector) + let hsm_serial_number = get_hsm_client(hsm_connector)? .device_info() .expect("error getting device info") .serial_number; @@ -79,9 +123,12 @@ impl Runnable for SetupCommand { println!("Restoring and reprovisioning YubiHSM from existing 24-word mnemonic phrase."); println!(); - read_mnemonic_from_stdin("*** Enter mnemonic (separate words with spaces): ") + read_mnemonic( + &mut io::stdin().lock(), + "*** Enter mnemonic (separate words with spaces): ", + )? } else { - generate_mnemonic_from_hsm_and_os_csprngs(hsm_connector) + generate_mnemonic_from_hsm_and_os_csprngs(hsm_connector)? }; let roles = derive_roles_from_mnemonic(&mnemonic); @@ -142,17 +189,22 @@ impl Runnable for SetupCommand { println!("- wrapkey 0x0001 [primary]: {}", wrapkey_hex.as_str()); if self.print_only { - exit(0); + return Ok(()); } - prompt_for_user_approval("Are you SURE you want erase and reinitialize this HSM?"); + if !erase_confirmed( + &mut io::stdin().lock(), + "Are you SURE you want erase and reinitialize this HSM?", + ) { + return Err(Stop::EraseNotConfirmed); + } let report = yubihsm::setup::erase_device_and_init_with_profile( hsm_connector.clone(), crate::yubihsm::config().auth.credentials(), profile, ) - .unwrap_or_else(|e| hsm_error(&e)); + .map_err(Stop::hsm)?; status_ok!( "Success", @@ -177,16 +229,18 @@ impl Runnable for SetupCommand { panic!("error writing report: {e}"); }) } + + Ok(()) } } -/// Read the mnemonic phrase from STDIN -fn read_mnemonic_from_stdin(prompt: &str) -> mnemonic::Phrase { +/// Read the mnemonic phrase from `input`, after printing `prompt` +fn read_mnemonic(input: &mut impl BufRead, prompt: &str) -> Result { print!("{prompt}"); io::stdout().flush().unwrap(); let mut input_string = String::new(); - io::stdin() + input .read_line(&mut input_string) .expect("error reading mnemonic from STDIN!"); @@ -194,14 +248,12 @@ fn read_mnemonic_from_stdin(prompt: &str) -> mnemonic::Phrase { let input_phrase = input_words.join(" "); input_string.zeroize(); - let result = mnemonic::Phrase::new(input_phrase, BIP39_LANGUAGE).unwrap_or_else(|_| { - eprintln!("*** ERROR: Couldn't decode mnemonic"); - exit(1); - }); + let result = mnemonic::Phrase::new(input_phrase, BIP39_LANGUAGE) + .map_err(|_| Stop::UndecodableMnemonic)?; println!("\nMnemonic phrase decoded/checksummed successfully!\n"); - result + Ok(result) } /// Display the mnemonic as two groups of 12 words @@ -221,13 +273,13 @@ fn print_mnemonic(mnemonic: &mnemonic::Phrase) { /// We need to create our own client here since the global one maintains /// a persistent connection, and we need to close this one before we can /// reprovision the HSM -fn get_hsm_client(hsm_connector: &Connector) -> yubihsm::Client { +fn get_hsm_client(hsm_connector: &Connector) -> Result { yubihsm::Client::open( hsm_connector.clone(), crate::yubihsm::config().auth.credentials(), false, ) - .unwrap_or_else(|e| hsm_error(&e)) + .map_err(Stop::hsm) } /// Generate entropy by combining entropy both from the host OS and from the @@ -237,13 +289,13 @@ fn get_hsm_client(hsm_connector: &Connector) -> yubihsm::Client { /// function (HKDF) in order to derive the recovery passphrase, which ideally /// ensures that the passphrase will be securely random so long as at least /// one of the two inputs is secure. -fn generate_mnemonic_from_hsm_and_os_csprngs(hsm_connector: &Connector) -> mnemonic::Phrase { - let hsm_client = get_hsm_client(hsm_connector); +fn generate_mnemonic_from_hsm_and_os_csprngs( + hsm_connector: &Connector, +) -> Result { + let hsm_client = get_hsm_client(hsm_connector)?; // Obtain half of the IKM from the YubiHSM (256-bits) - let mut ikm = hsm_client - .get_pseudo_random(KEY_SIZE) - .unwrap_or_else(|e| hsm_error(&e)); + let mut ikm = hsm_client.get_pseudo_random(KEY_SIZE).map_err(Stop::hsm)?; // Obtain another half of the IKM from the host OS (256-bits) // for a total of 512-bits IKM. This ensures we still get 256-bits @@ -261,7 +313,7 @@ fn generate_mnemonic_from_hsm_and_os_csprngs(hsm_connector: &Connector) -> mnemo let result = mnemonic::Phrase::from_entropy(okm, BIP39_LANGUAGE); okm.zeroize(); - result + Ok(result) } /// Derive the default roles form the given BIP39 `mnemonic::Phrase` @@ -470,30 +522,18 @@ fn create_object_label(label_prefix: &str) -> object::Label { object::Label::from([label_prefix, ×tamp].join(":").as_ref()) } -/// Prompt the user to ensure they want to proceed -fn prompt_for_user_approval(prompt: &str) { +/// Ask, after printing `prompt`, whether to proceed, and read the answer from `input` +fn erase_confirmed(input: &mut impl BufRead, prompt: &str) -> bool { print!("\n*** {prompt} (y/N): "); io::stdout().flush().unwrap(); let mut choice_in = String::new(); - io::stdin() + input .read_line(&mut choice_in) .expect("Failed to read user input"); let choice = choice_in.trim(); - - if choice != "y" && choice != "Y" { - println!("Aborting"); - exit(1); - } -} - -/// Handler for HSM errors -fn hsm_error(e: &dyn std::error::Error) -> ! { - status_err!("HSM error: {}", e); - - // TODO: handle exits via abscissa - exit(1); + choice == "y" || choice == "Y" } #[cfg(test)] @@ -515,6 +555,41 @@ mod tests { mnemonic::Phrase::from_entropy(TEST_KEY, BIP39_LANGUAGE) } + /// `process::exit` runs no destructors, so a mnemonic that can't be decoded must come + /// back to `run`, which exits only once every secret has been dropped + #[test] + fn an_undecodable_mnemonic_comes_back_to_run() { + assert!(matches!( + read_mnemonic(&mut "not a mnemonic\n".as_bytes(), "mnemonic: "), + Err(Stop::UndecodableMnemonic) + )); + } + + #[test] + fn a_typed_mnemonic_is_decoded() { + let typed = format!("{TEST_BIP39_PHRASE}\n"); + let mnemonic = read_mnemonic(&mut typed.as_bytes(), "mnemonic: ").unwrap(); + assert_eq!(mnemonic.entropy(), &TEST_KEY); + } + + /// Declining comes back to `run` for the same reason: the caller holds every secret + #[test] + fn only_y_confirms_the_erase() { + for (answer, confirmed) in [ + ("y\n", true), + ("Y\n", true), + ("n\n", false), + ("\n", false), + ("yes\n", false), + ] { + assert_eq!( + erase_confirmed(&mut answer.as_bytes(), "erase?"), + confirmed, + "{answer:?}" + ); + } + } + #[test] fn derive_test_key_from_test_mnemonic() { assert_eq!(test_mnemonic().entropy(), &TEST_KEY); From 8ab6d0a5a138aae5be9fbb6b0b24a8441ae66472 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:13:48 +0100 Subject: [PATCH 06/18] test: give each test KMS a port and socket nothing else can hold --- Cargo.lock | 51 -------------------------------------------- Cargo.toml | 1 - tests/integration.rs | 20 +++++++---------- 3 files changed, 8 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ae6a991..14342c00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1678,15 +1678,6 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" -[[package]] -name = "ppv-lite86" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9" -dependencies = [ - "zerocopy", -] - [[package]] name = "primeorder" version = "0.13.6" @@ -1749,27 +1740,6 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" -[[package]] -name = "rand" -version = "0.8.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22f6172bdec972074665ed81ed53b71da00bfc44b65a753cfde883ec4c702a1a" -dependencies = [ - "libc", - "rand_chacha", - "rand_core", -] - -[[package]] -name = "rand_chacha" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" -dependencies = [ - "ppv-lite86", - "rand_core", -] - [[package]] name = "rand_core" version = "0.6.4" @@ -2366,7 +2336,6 @@ dependencies = [ "ledger", "once_cell", "prost", - "rand", "rand_core", "rpassword", "sdkms", @@ -2943,26 +2912,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "zerocopy" -version = "0.8.56" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "556764e583adb45a9f8d413c2a147fa7e8d821e48e12b14fd560b607998b75eb" -dependencies = [ - "zerocopy-derive", -] - -[[package]] -name = "zerocopy-derive" -version = "0.8.56" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2ab42fc20575779bd240faa45f94a74256f755c0fa9e89f0ede20d91d0cdfc1" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "zerofrom" version = "0.1.8" diff --git a/Cargo.toml b/Cargo.toml index f05d3fb2..94a0b3e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -55,7 +55,6 @@ zeroize = "1" [dev-dependencies] abscissa_core = { version = "0.9", features = ["testing"] } byteorder = "1" -rand = "0.8" toml = "0.9" [features] diff --git a/tests/integration.rs b/tests/integration.rs index 0f7d5b9f..74827e84 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -2,7 +2,6 @@ use chrono::{DateTime, Utc}; use cometbft_p2p::{self as p2p, IdentitySecret, PublicKey, ReadMsg, SecretConnection, WriteMsg}; -use rand::Rng; use signature::Verifier; use std::{ fs::{self, File}, @@ -99,15 +98,14 @@ struct KmsProcess { impl KmsProcess { /// Spawn the KMS process and wait for an incoming TCP connection pub fn create_tcp(key_type: &KeyType) -> Self { - // Generate a random port and a config file - let port: u16 = rand::thread_rng().gen_range(60000..=65535); + // Listen on a port the OS picks, which no other process can hold + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let port = listener.local_addr().unwrap().port(); + let kms_dir = tempfile::tempdir().unwrap(); let config = KmsProcess::create_tcp_config(port, key_type, &kms_dir); - // Listen on a random port - let listener = TcpListener::bind(format!("{}:{}", "127.0.0.1", port)).unwrap(); - listener.set_nonblocking(true).unwrap(); - let mut process = KmsProcess::spawn(&config, &kms_dir); let socket = wait_for_connection(&mut process, &kms_dir, || { @@ -129,12 +127,10 @@ impl KmsProcess { /// Like `create_unix`, with `prepare` changing the KMS's directory before the KMS starts fn create_unix_with(key_type: &KeyType, prepare: impl FnOnce(&Path)) -> Self { - // Create a random socket path and a config file - let mut rng = rand::thread_rng(); - let letter: char = rng.gen_range(b'a'..=b'z') as char; - let number: u32 = rng.gen_range(0..=999999); - let socket_path = format!("/tmp/tmkms-{letter}{number:06}.sock"); + // A socket in the KMS's own directory, which nothing else uses and which is + // removed with it let kms_dir = tempfile::tempdir().unwrap(); + let socket_path = kms_dir.path().join("kms.sock").display().to_string(); let config = KmsProcess::create_unix_config(&socket_path, key_type, &kms_dir); prepare(kms_dir.path()); From 522ce0a317115c99fe732cb39da7ce99aa307fdf Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:16:15 +0100 Subject: [PATCH 07/18] test: build each CLI test helper only where it is used --- tests/cli/mod.rs | 10 +++++++--- tests/cli/yubihsm/keys.rs | 4 +++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/cli/mod.rs b/tests/cli/mod.rs index 166ff61e..7f7269a9 100644 --- a/tests/cli/mod.rs +++ b/tests/cli/mod.rs @@ -2,8 +2,7 @@ use std::{ ffi::OsStr, - io::Write, - process::{Command, Output, Stdio}, + process::{Command, Output}, }; use super::KMS_EXE_PATH; @@ -25,12 +24,16 @@ where Command::new(KMS_EXE_PATH).args(args).output().unwrap() } -/// Run the `tmkms` CLI command with the given arguments, typing `stdin` into it +/// Run the `tmkms` CLI command with the given arguments, typing `stdin` into it. Only +/// the `yubihsm setup` tests, which need the mock HSM, type into a command. +#[cfg(feature = "yubihsm-mock")] pub fn run_with_stdin(args: I, stdin: &str) -> Output where I: IntoIterator, S: AsRef, { + use std::{io::Write, process::Stdio}; + let mut child = Command::new(KMS_EXE_PATH) .args(args) .stdin(Stdio::piped()) @@ -89,6 +92,7 @@ fn a_failed_command_is_reported_with_both_streams() { } /// `text` without the ANSI colour codes `tmkms` writes even when not on a terminal +#[cfg(any(feature = "softsign", feature = "yubihsm"))] pub fn without_colour_codes(text: &str) -> String { let mut plain = String::new(); let mut chars = text.chars(); diff --git a/tests/cli/yubihsm/keys.rs b/tests/cli/yubihsm/keys.rs index 21dc0d12..747d0907 100644 --- a/tests/cli/yubihsm/keys.rs +++ b/tests/cli/yubihsm/keys.rs @@ -4,7 +4,9 @@ mod generate; mod import; mod list; -pub use super::{KMS_CONFIG_PATH, PRIV_VALIDATOR_CONFIG_PATH, SECP256K1_PRIV_VALIDATOR_PATH}; +#[cfg(feature = "yubihsm-mock")] +pub use super::KMS_CONFIG_PATH; +pub use super::{PRIV_VALIDATOR_CONFIG_PATH, SECP256K1_PRIV_VALIDATOR_PATH}; use crate::cli; #[test] From 159b1d8f229f292fc9926a7d0d23f71dcae55aa9 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:16:59 +0100 Subject: [PATCH 08/18] docs: cover setup and import in the zeroization notes --- FORK.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/FORK.md b/FORK.md index 6752d74f..3aa80b61 100644 --- a/FORK.md +++ b/FORK.md @@ -43,7 +43,7 @@ liveness/DoS > hygiene. | 11 | `tmkms init` will not destroy an existing identity key without `-f` | **Medium** | Data loss | | 12 | Disk failures are no longer reported as "attempted double sign" | **Low** | Observability | | 13 | Bounded TCP connect, Unix-socket timeouts, no peer payloads in logs | **Low** | Liveness/DoS | -| 14 | Zeroization gaps closed in key load, keygen, `yubihsm keys import` (see below) | **Low** | Hygiene | +| 14 | Zeroization gaps closed in key load, keygen, key import and setup (see below) | **Low** | Hygiene | | 15 | Dependencies refreshed, CI hardened, dev image off EOL centos:7 | **Low** | Supply chain | | 16 | `tmkms ledger init` can succeed at all, and honours the double-sign check | **Low** | Upstream bug (dead) | | 17 | A `gno` key format prints the public key gno.land accepts | **Low** | gno.land addition | @@ -184,8 +184,7 @@ Signing is unaffected either way. `fortanixdsm` feature, which the release image does not build, and even there the Fortanix DSM client speaks HTTP/1.1, so it never runs. The fix needs hyper 1, which `sdkms` does not use. -- **Key copies held inside two dependencies are not wiped (#14).** tmkms cannot - reach them: +- **Some key copies are out of tmkms's reach, and not wiped (#14):** - `iq-cometbft` parses every `priv_validator_key.json`, and its Ed25519 key type is a plain byte array that never wipes itself and cannot be wiped from outside. Each parse leaves the seed in freed memory: `yubihsm keys import`, `softsign import`, @@ -194,9 +193,11 @@ Signing is unaffected either way. wiped either, whatever the key type. - `yubihsm` copies a key into command buffers it frees without wiping, which upstream marks as a TODO. So every key `yubihsm keys import` stores, Base64 or - JSON, leaves copies in freed memory. + JSON, and every key `yubihsm setup` installs leave copies in freed memory. + - A mnemonic typed into `yubihsm setup --restore` stays in the standard library's + stdin buffer, which nothing can wipe. - Revisit when these crates wipe their key buffers. + Revisit the first two when these crates wipe their key buffers. - **The container runs as root**, matching the previously published image, because key and state files arrive on a host-mounted volume whose ownership is managed outside the image. From be1865d7d27c13ed8a37096953a7a2178c04c92c Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:16:59 +0100 Subject: [PATCH 09/18] docs: update the verification counts --- FORK.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FORK.md b/FORK.md index 3aa80b61..7e26bec7 100644 --- a/FORK.md +++ b/FORK.md @@ -206,7 +206,7 @@ Signing is unaffected either way. - `cargo fmt --all -- --check` — clean. - `cargo clippy --all-features --all-targets` — clean. -- `cargo test --all-features -- --test-threads 1` — 70 unit + 31 integration tests +- `cargo test --all-features -- --test-threads 1` — 75 unit + 37 integration tests pass (10 ignored, all hardware-dependent), printing nothing but their results. - The published image is pulled and checked on every release: it reports its own version, keeps `tmkms` as the entrypoint, and links the USB libraries the From ed35a6910e957f32d6c276f105c6763608bcc19b Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:49:47 +0100 Subject: [PATCH 10/18] fix(yubihsm): read setup's credentials before deriving any secret --- src/commands/yubihsm/setup.rs | 24 +++++++------ tests/cli/yubihsm/setup.rs | 65 ++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/commands/yubihsm/setup.rs b/src/commands/yubihsm/setup.rs index c1fd9a6c..0512eb41 100644 --- a/src/commands/yubihsm/setup.rs +++ b/src/commands/yubihsm/setup.rs @@ -113,8 +113,11 @@ impl SetupCommand { /// Derive the admin mnemonic and every key from it, print them, and unless only /// printing, erase and reinitialize the HSM with them fn setup(&self) -> Result<(), Stop> { + // Reading a password file ends the process when it fails, so read the + // credentials once, before any secret exists + let credentials = crate::yubihsm::config().auth.credentials(); let hsm_connector = crate::yubihsm::connector(); - let hsm_serial_number = get_hsm_client(hsm_connector)? + let hsm_serial_number = get_hsm_client(hsm_connector, &credentials)? .device_info() .expect("error getting device info") .serial_number; @@ -128,7 +131,7 @@ impl SetupCommand { "*** Enter mnemonic (separate words with spaces): ", )? } else { - generate_mnemonic_from_hsm_and_os_csprngs(hsm_connector)? + generate_mnemonic_from_hsm_and_os_csprngs(hsm_connector, &credentials)? }; let roles = derive_roles_from_mnemonic(&mnemonic); @@ -201,7 +204,7 @@ impl SetupCommand { let report = yubihsm::setup::erase_device_and_init_with_profile( hsm_connector.clone(), - crate::yubihsm::config().auth.credentials(), + credentials, profile, ) .map_err(Stop::hsm)?; @@ -273,13 +276,11 @@ fn print_mnemonic(mnemonic: &mnemonic::Phrase) { /// We need to create our own client here since the global one maintains /// a persistent connection, and we need to close this one before we can /// reprovision the HSM -fn get_hsm_client(hsm_connector: &Connector) -> Result { - yubihsm::Client::open( - hsm_connector.clone(), - crate::yubihsm::config().auth.credentials(), - false, - ) - .map_err(Stop::hsm) +fn get_hsm_client( + hsm_connector: &Connector, + credentials: &Credentials, +) -> Result { + yubihsm::Client::open(hsm_connector.clone(), credentials.clone(), false).map_err(Stop::hsm) } /// Generate entropy by combining entropy both from the host OS and from the @@ -291,8 +292,9 @@ fn get_hsm_client(hsm_connector: &Connector) -> Result { /// one of the two inputs is secure. fn generate_mnemonic_from_hsm_and_os_csprngs( hsm_connector: &Connector, + credentials: &Credentials, ) -> Result { - let hsm_client = get_hsm_client(hsm_connector)?; + let hsm_client = get_hsm_client(hsm_connector, credentials)?; // Obtain half of the IKM from the YubiHSM (256-bits) let mut ikm = hsm_client.get_pseudo_random(KEY_SIZE).map_err(Stop::hsm)?; diff --git a/tests/cli/yubihsm/setup.rs b/tests/cli/yubihsm/setup.rs index e537f3da..f0bbb2c5 100644 --- a/tests/cli/yubihsm/setup.rs +++ b/tests/cli/yubihsm/setup.rs @@ -1,7 +1,12 @@ //! Integration tests for the `yubihsm setup` subcommand, against the mock HSM use crate::cli; -use std::str; +use std::{ + fs, + io::{Read, Write}, + process::{Command, Stdio}, + str, +}; /// A valid 24-word admin mnemonic, so that everything derived from it is known const MNEMONIC: &str = "abandon amount liar amount expire adjust cage candy arch gather drum \ @@ -105,3 +110,61 @@ fn setup_reinitializes_the_hsm_when_the_erase_is_confirmed() { assert!(stdout.starts_with(&before_erase), "{stdout}"); assert!(stdout.contains("installing wrap key: primary:"), "{stdout}"); } + +/// Setup reads the HSM password file before it derives any secret. Reading it fails by +/// ending the process, so a read after that point would skip wiping every secret. +#[test] +fn setup_reads_its_password_file_before_deriving_secrets() { + let dir = tempfile::tempdir().unwrap(); + let password_file = dir.path().join("password"); + fs::write(&password_file, "password\n").unwrap(); + let config = dir.path().join("tmkms.toml"); + let mock_config = fs::read_to_string(super::KMS_CONFIG_PATH).unwrap(); + let password_line = r#"auth = { key = 1, password = "password" }"#; + assert!(mock_config.contains(password_line)); + fs::write( + &config, + mock_config.replace( + password_line, + &format!( + r#"auth = {{ key = 1, password_file = "{}" }}"#, + password_file.display() + ), + ), + ) + .unwrap(); + + let mut setup = Command::new(crate::KMS_EXE_PATH) + .args(["yubihsm", "setup", "-c", config.to_str().unwrap(), "-r"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let mut stdin = setup.stdin.take().unwrap(); + let mut stdout = setup.stdout.take().unwrap(); + stdin.write_all(format!("{MNEMONIC}\n").as_bytes()).unwrap(); + + // Once setup asks to confirm, every secret exists: take the password file away + let mut printed = Vec::new(); + let mut byte = [0]; + while !printed.ends_with(CONFIRMATION.as_bytes()) && stdout.read(&mut byte).unwrap() == 1 { + printed.push(byte[0]); + } + fs::remove_file(&password_file).unwrap(); + stdin.write_all(b"y\n").unwrap(); + drop(stdin); + stdout.read_to_end(&mut printed).unwrap(); + + let out = setup.wait_with_output().unwrap(); + let stderr = cli::without_colour_codes(str::from_utf8(&out.stderr).unwrap()); + assert_eq!( + (out.status.code(), stderr.as_str()), + ( + Some(0), + " Success reinitialized YubiHSM (serial: 0123456789)\n" + ), + "{}", + String::from_utf8_lossy(&printed) + ); +} From bf5356e08b050ac811a0970213c529719dd800c3 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:51:09 +0100 Subject: [PATCH 11/18] fix(yubihsm): wipe the mnemonic typed into setup --- src/commands/yubihsm.rs | 2 +- src/commands/yubihsm/setup.rs | 10 +++++---- tests/secret_decoding.rs | 39 +++++++++++++++++++++++++++++++---- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/commands/yubihsm.rs b/src/commands/yubihsm.rs index d1ef4573..4f6e0f51 100644 --- a/src/commands/yubihsm.rs +++ b/src/commands/yubihsm.rs @@ -2,7 +2,7 @@ mod detect; pub mod keys; -mod setup; +pub mod setup; mod test; pub use self::{detect::DetectCommand, keys::KeysCommand, setup::SetupCommand, test::TestCommand}; diff --git a/src/commands/yubihsm/setup.rs b/src/commands/yubihsm/setup.rs index 0512eb41..98b3c532 100644 --- a/src/commands/yubihsm/setup.rs +++ b/src/commands/yubihsm/setup.rs @@ -68,7 +68,7 @@ pub struct SetupCommand { /// Why setup stopped before reinitializing the HSM #[derive(Debug)] -enum Stop { +pub enum Stop { /// The mnemonic typed in to restore couldn't be decoded UndecodableMnemonic, @@ -238,7 +238,7 @@ impl SetupCommand { } /// Read the mnemonic phrase from `input`, after printing `prompt` -fn read_mnemonic(input: &mut impl BufRead, prompt: &str) -> Result { +pub fn read_mnemonic(input: &mut impl BufRead, prompt: &str) -> Result { print!("{prompt}"); io::stdout().flush().unwrap(); @@ -248,10 +248,12 @@ fn read_mnemonic(input: &mut impl BufRead, prompt: &str) -> Result = input_string.split_whitespace().collect(); - let input_phrase = input_words.join(" "); + let input_phrase = Zeroizing::new(input_words.join(" ")); input_string.zeroize(); - let result = mnemonic::Phrase::new(input_phrase, BIP39_LANGUAGE) + // `Phrase::new` keeps its own copy, which it wipes, but drops what it is handed + // without wiping it, so it only borrows the phrase + let result = mnemonic::Phrase::new(input_phrase.as_str(), BIP39_LANGUAGE) .map_err(|_| Stop::UndecodableMnemonic)?; println!("\nMnemonic phrase decoded/checksummed successfully!\n"); diff --git a/tests/secret_decoding.rs b/tests/secret_decoding.rs index 9a4d8cf1..001ed5db 100644 --- a/tests/secret_decoding.rs +++ b/tests/secret_decoding.rs @@ -1,6 +1,7 @@ //! A Base64 key must not leave its decoded bytes in freed memory, even when it fails -//! to decode partway. This is its own test binary because checking that needs a -//! global allocator that inspects every buffer as it is freed. +//! to decode partway, nor a typed mnemonic its words. This is its own test binary +//! because checking that needs a global allocator that inspects every buffer as it is +//! freed. use std::{ alloc::{GlobalAlloc, Layout, System}, @@ -24,7 +25,19 @@ fn seed_middle() -> &'static [u8] { /// Set when a freed buffer still held `seed_middle()` static FREED_SEED: AtomicBool = AtomicBool::new(false); -/// The system allocator, recording whether a freed buffer still held the seed +/// An admin mnemonic as an operator types it into `yubihsm setup --restore` +#[cfg(feature = "yubihsm")] +const TYPED_MNEMONIC: &str = "abandon amount liar amount expire adjust cage candy arch gather \ + drum bullet absurd math era live bid rhythm alien crouch range \ + attend journey unaware\n"; + +/// Words of `TYPED_MNEMONIC` that no other buffer in this process holds +const MNEMONIC_WORDS: &[u8] = b"liar amount expire adjust cage candy"; + +/// Set when a freed buffer still held `MNEMONIC_WORDS` +static FREED_MNEMONIC: AtomicBool = AtomicBool::new(false); + +/// The system allocator, recording whether a freed buffer still held a secret struct SeedScanner; unsafe impl GlobalAlloc for SeedScanner { @@ -40,6 +53,12 @@ unsafe impl GlobalAlloc for SeedScanner { { FREED_SEED.store(true, Ordering::SeqCst); } + if freed + .windows(MNEMONIC_WORDS.len()) + .any(|w| w == MNEMONIC_WORDS) + { + FREED_MNEMONIC.store(true, Ordering::SeqCst); + } unsafe { System.dealloc(ptr, layout) } } } @@ -48,7 +67,8 @@ unsafe impl GlobalAlloc for SeedScanner { static ALLOCATOR: SeedScanner = SeedScanner; /// One test, because the allocator's record is shared by everything in this process. -/// It covers both readers of a Base64 key: key load and `yubihsm keys import`. +/// It covers both readers of a Base64 key, key load and `yubihsm keys import`, and the +/// mnemonic reader of `yubihsm setup`. #[test] fn a_base64_key_leaves_no_decoded_bytes_in_freed_memory() { let dir = tempfile::tempdir().unwrap(); @@ -100,4 +120,15 @@ fn a_base64_key_leaves_no_decoded_bytes_in_freed_memory() { "yubihsm keys import left a key that fails to decode in freed memory" ); } + + #[cfg(feature = "yubihsm")] + { + use tmkms::commands::yubihsm::setup::read_mnemonic; + + drop(read_mnemonic(&mut TYPED_MNEMONIC.as_bytes(), "").unwrap()); + assert!( + !FREED_MNEMONIC.load(Ordering::SeqCst), + "yubihsm setup left a typed mnemonic in freed memory" + ); + } } From ef91566ac5c0a1169009df2f469b9e2dd3d8e11f Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:52:33 +0100 Subject: [PATCH 12/18] fix(yubihsm): keep setup's working secrets in buffers that wipe themselves --- src/commands/yubihsm/setup.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/commands/yubihsm/setup.rs b/src/commands/yubihsm/setup.rs index 98b3c532..a7a323f2 100644 --- a/src/commands/yubihsm/setup.rs +++ b/src/commands/yubihsm/setup.rs @@ -242,14 +242,13 @@ pub fn read_mnemonic(input: &mut impl BufRead, prompt: &str) -> Result = input_string.split_whitespace().collect(); let input_phrase = Zeroizing::new(input_words.join(" ")); - input_string.zeroize(); // `Phrase::new` keeps its own copy, which it wipes, but drops what it is handed // without wiping it, so it only borrows the phrase @@ -267,9 +266,8 @@ fn print_mnemonic(mnemonic: &mnemonic::Phrase) { let words_len = words.len(); for word_group in &[&words[..(words_len / 2)], &words[(words_len / 2)..]] { - let mut word_group_joined = word_group.join(" "); - println!(" {word_group_joined}"); - word_group_joined.zeroize(); + let word_group_joined = Zeroizing::new(word_group.join(" ")); + println!(" {}", word_group_joined.as_str()); } } @@ -299,25 +297,24 @@ fn generate_mnemonic_from_hsm_and_os_csprngs( let hsm_client = get_hsm_client(hsm_connector, credentials)?; // Obtain half of the IKM from the YubiHSM (256-bits) - let mut ikm = hsm_client.get_pseudo_random(KEY_SIZE).map_err(Stop::hsm)?; + let hsm_entropy = Zeroizing::new(hsm_client.get_pseudo_random(KEY_SIZE).map_err(Stop::hsm)?); // Obtain another half of the IKM from the host OS (256-bits) // for a total of 512-bits IKM. This ensures we still get 256-bits - // of good IKM even in the event one of the RNGs fails. + // of good IKM even in the event one of the RNGs fails. The capacity is + // reserved up front, since growing the buffer would free the old one unwiped. + let mut ikm = Zeroizing::new(Vec::with_capacity(2 * KEY_SIZE)); + ikm.extend_from_slice(&hsm_entropy); ikm.extend_from_slice(&[0u8; KEY_SIZE]); getrandom(&mut ikm[KEY_SIZE..]).expect("RNG failure!"); let kdf = Hkdf::::new(None, &ikm); // 32-bytes (256-bits) -> 24 BIP32 words - let mut okm = [0u8; KEY_SIZE]; - kdf.expand(HKDF_MNEMONIC_INFO, &mut okm).unwrap(); - ikm.zeroize(); + let mut okm = Zeroizing::new([0u8; KEY_SIZE]); + kdf.expand(HKDF_MNEMONIC_INFO, okm.as_mut()).unwrap(); - let result = mnemonic::Phrase::from_entropy(okm, BIP39_LANGUAGE); - okm.zeroize(); - - Ok(result) + Ok(mnemonic::Phrase::from_entropy(*okm, BIP39_LANGUAGE)) } /// Derive the default roles form the given BIP39 `mnemonic::Phrase` From 22faab3d3221983097383084e98e7e74bb76f88b Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:53:35 +0100 Subject: [PATCH 13/18] test(yubihsm): pin setup's random mnemonic and its HSM failure report --- tests/cli/yubihsm/setup.rs | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/cli/yubihsm/setup.rs b/tests/cli/yubihsm/setup.rs index f0bbb2c5..2e3302fc 100644 --- a/tests/cli/yubihsm/setup.rs +++ b/tests/cli/yubihsm/setup.rs @@ -66,6 +66,69 @@ fn setup_prints_the_keys_it_derives_from_a_restored_mnemonic() { ); } +#[test] +fn setup_prints_a_random_mnemonic_and_the_keys_it_derives() { + let (code, stdout, stderr) = setup(&["-p"], ""); + assert_eq!((code, stderr.as_str()), (Some(0), ""), "{stdout}"); + + let lines: Vec<&str> = stdout.lines().collect(); + let expected_shape = [ + "Below is a randomly generated 24-word admin mnemonic and derived keys/passwords:", + "", + "- key 0x0001: admin:", + "", + " ", + " ", + "", + "- authkey 0x0002 [operator]: kms-operator-password-1", + "- authkey 0x0003 [auditor]: kms-auditor-password-1", + "- authkey 0x0004 [validator]: kms-validator-password-1", + "- wrapkey 0x0001 [primary]: ", + ]; + assert_eq!(lines.len(), expected_shape.len(), "{stdout}"); + for (line, start) in lines.iter().zip(expected_shape) { + assert!(line.starts_with(start), "{line:?} in {stdout}"); + } + + let words = format!("{} {}", lines[4].trim(), lines[5].trim()); + assert_eq!(words.split(' ').count(), 24, "{stdout}"); + hkd32::mnemonic::Phrase::new(&words, Default::default()).unwrap(); + + let wrap_key = lines[10].trim_start_matches(expected_shape[10]); + assert!( + wrap_key.len() == 64 && wrap_key.bytes().all(|b| b.is_ascii_hexdigit()), + "{stdout}" + ); +} + +#[test] +fn setup_reports_an_hsm_failure() { + let dir = tempfile::tempdir().unwrap(); + let config = dir.path().join("tmkms.toml"); + let mock_config = fs::read_to_string(super::KMS_CONFIG_PATH).unwrap(); + let password = r#"password = "password""#; + assert!(mock_config.contains(password)); + fs::write( + &config, + mock_config.replace(password, r#"password = "wrong""#), + ) + .unwrap(); + + let out = cli::run(["yubihsm", "setup", "-c", config.to_str().unwrap()]); + let stderr = cli::without_colour_codes(str::from_utf8(&out.stderr).unwrap()); + + assert_eq!( + (out.status.code(), str::from_utf8(&out.stdout).unwrap()), + (Some(1), "") + ); + assert!( + stderr.starts_with("error: HSM error: authentication failed: ") + && stderr + .ends_with("invalid credentials for authentication key #1 (cryptogram mismatch)\n"), + "{stderr}" + ); +} + #[test] fn setup_refuses_a_mnemonic_it_cannot_decode() { assert_eq!( From d565c707ad0a1b267b59dea1b7744e507638bc32 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:55:40 +0100 Subject: [PATCH 14/18] test(softsign): pin import's refusal of the base64 format --- tests/cli/softsign.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/cli/softsign.rs b/tests/cli/softsign.rs index a06685e4..1e24a947 100644 --- a/tests/cli/softsign.rs +++ b/tests/cli/softsign.rs @@ -103,6 +103,10 @@ fn import_reports_every_outcome() { vec!["import", "-f", "raw", ed25519_json, &output], "error: config error: invalid key format: raw (must be 'json' or 'raw')\n".into(), ), + ( + vec!["import", "-f", "base64", ed25519_json, &output], + "error: invalid format: Base64 (must be 'json')\n".into(), + ), ( vec!["import", &missing_json, &output], format!( From 8e740b920da6c4ec90ae5115c44bb9ba0e6b3252 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:55:40 +0100 Subject: [PATCH 15/18] docs(yubihsm): say setup can stop after erasing the HSM --- src/commands/yubihsm/setup.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/yubihsm/setup.rs b/src/commands/yubihsm/setup.rs index a7a323f2..1f1fc134 100644 --- a/src/commands/yubihsm/setup.rs +++ b/src/commands/yubihsm/setup.rs @@ -66,7 +66,8 @@ pub struct SetupCommand { pub write_report: Option, } -/// Why setup stopped before reinitializing the HSM +/// Why setup stopped without reinitializing the HSM. An HSM failure can come after the +/// erase, leaving the HSM erased but not set up. #[derive(Debug)] pub enum Stop { /// The mnemonic typed in to restore couldn't be decoded From 4a6e71f226b5034b7bf617b5d5767c10d99e9faf Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:55:40 +0100 Subject: [PATCH 16/18] test: keep each test KMS socket path short whatever TMPDIR is --- tests/integration.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index 74827e84..b8935162 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -128,8 +128,9 @@ impl KmsProcess { /// Like `create_unix`, with `prepare` changing the KMS's directory before the KMS starts fn create_unix_with(key_type: &KeyType, prepare: impl FnOnce(&Path)) -> Self { // A socket in the KMS's own directory, which nothing else uses and which is - // removed with it - let kms_dir = tempfile::tempdir().unwrap(); + // removed with it. The directory is under `/tmp` because a socket path is + // limited to about 100 bytes, and `$TMPDIR` can be long. + let kms_dir = tempfile::tempdir_in("/tmp").unwrap(); let socket_path = kms_dir.path().join("kms.sock").display().to_string(); let config = KmsProcess::create_unix_config(&socket_path, key_type, &kms_dir); prepare(kms_dir.path()); From a3476870e28b75caf341c4bc1aa53544f5bcf475 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:55:55 +0100 Subject: [PATCH 17/18] docs: name the stdout buffer setup's printed keys pass through --- FORK.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/FORK.md b/FORK.md index 7e26bec7..0f5983fc 100644 --- a/FORK.md +++ b/FORK.md @@ -193,9 +193,9 @@ Signing is unaffected either way. wiped either, whatever the key type. - `yubihsm` copies a key into command buffers it frees without wiping, which upstream marks as a TODO. So every key `yubihsm keys import` stores, Base64 or - JSON, and every key `yubihsm setup` installs leave copies in freed memory. - - A mnemonic typed into `yubihsm setup --restore` stays in the standard library's - stdin buffer, which nothing can wipe. + JSON, and every key `yubihsm setup` installs leaves copies in freed memory. + - The standard library's stdin and stdout buffers, which nothing can wipe, keep a + mnemonic typed into `yubihsm setup --restore` and parts of the keys setup prints. Revisit the first two when these crates wipe their key buffers. - **The container runs as root**, matching the previously published image, because From 9525dd4abed1b9d205dc519590a2fbbf2aa36d86 Mon Sep 17 00:00:00 2001 From: aeddi Date: Wed, 4 Nov 2026 19:56:40 +0100 Subject: [PATCH 18/18] docs: update the verification counts --- FORK.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FORK.md b/FORK.md index 0f5983fc..6516417c 100644 --- a/FORK.md +++ b/FORK.md @@ -206,7 +206,7 @@ Signing is unaffected either way. - `cargo fmt --all -- --check` — clean. - `cargo clippy --all-features --all-targets` — clean. -- `cargo test --all-features -- --test-threads 1` — 75 unit + 37 integration tests +- `cargo test --all-features -- --test-threads 1` — 75 unit + 40 integration tests pass (10 ignored, all hardware-dependent), printing nothing but their results. - The published image is pulled and checked on every release: it reports its own version, keeps `tmkms` as the entrypoint, and links the USB libraries the