From c0d0a7019767f2c7160eeaf9351fe32803f121a2 Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 06:42:33 +0200 Subject: [PATCH 01/11] fix(security): derive unlock key via Argon2id in non-openpgp import path When openvtc-cli2 is built without the openpgp-card feature, the config import path passed the raw passphrase bytes directly to SecuredConfig::save() as the AES-256 key. This bypasses the Argon2id KDF entirely, meaning: - the on-disk SecuredConfig is encrypted with a key that has no memory-hard stretching, making offline brute-force trivial - the saved config cannot be decrypted afterwards because UnlockCode::from_string() (used at load time) always applies Argon2id, so the keys never match - save() fails outright unless the passphrase happens to be exactly 32 bytes long (first_chunk::<32>() check) The openpgp-card path already calls derive_passphrase_key() with the "openvtc-unlock-code-v1" domain label; bring the non-openpgp path into line so both builds produce the same 32-byte Argon2id-derived key. Signed-off-by: Glenn Gore --- openvtc/src/state_handler/setup_sequence/config.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/openvtc/src/state_handler/setup_sequence/config.rs b/openvtc/src/state_handler/setup_sequence/config.rs index 4c1c050..dfea7d0 100644 --- a/openvtc/src/state_handler/setup_sequence/config.rs +++ b/openvtc/src/state_handler/setup_sequence/config.rs @@ -189,7 +189,13 @@ impl ConfigExtension for Config { } else { None }, - Some(&new_unlock_passphrase.expose_secret().as_bytes().to_vec()), + Some( + &derive_passphrase_key( + new_unlock_passphrase.expose_secret().as_bytes(), + b"openvtc-unlock-code-v1", + )? + .to_vec(), + ), ) .map_err(|e| anyhow::anyhow!("Couldn't save Secured Config: {e}"))?; From f29e8e0686b05c43439a5b7d93b92e7354db21db Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 06:50:11 +0200 Subject: [PATCH 02/11] fix(security): reject path-traversal characters in profile name The profile name comes from --profile or OPENVTC_CONFIG_PROFILE and is spliced verbatim into filesystem paths by process_lock::get_lock_file() ("{path}config-{profile}.lock") and the public/secured config writers, and is also used as the OS keyring account name. A value such as "../../../../tmp/evil" lets the lock-file writer create or clobber a file outside ~/.config/openvtc with attacker-chosen contents (the current PID), and lets a hostile environment steer key material into an unexpected keyring entry. Restrict profile names to [A-Za-z0-9._-] with no ".." component at the CLI entry point, before any path is constructed. Signed-off-by: Glenn Gore --- openvtc/src/main.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/openvtc/src/main.rs b/openvtc/src/main.rs index ece2bba..20b9df3 100644 --- a/openvtc/src/main.rs +++ b/openvtc/src/main.rs @@ -126,6 +126,23 @@ async fn main() -> Result<()> { .unwrap_or_else(|| "default".to_string()) }; + // The profile name is interpolated into lock-file and config paths and + // used as the OS keyring account identifier; reject path separators and + // traversal sequences before it reaches the filesystem. + if profile.is_empty() + || !profile + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.') + || profile.contains("..") + { + eprintln!( + "{} {}", + style("ERROR: Invalid profile name:").color256(CLI_RED), + style(&profile).color256(CLI_ORANGE) + ); + bail!("Profile name may only contain [A-Za-z0-9._-] and must not contain '..'"); + } + // Check if profile is currently active elsewhere? let lock_file = check_duplicate_instance(&profile)?; From 9bbac5444f43ba37ae936ca4644a19b9d30828a3 Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 08:22:28 +0200 Subject: [PATCH 03/11] fix(security): redact armored private key block in DIDKeysExportState Debug DIDKeysExportState derived Debug while holding the PGP-armored private key export in `exported`. The top-level `State` struct also derives Debug and is cloned through the state channel on every UI tick, so any `{:?}` of State (panic backtrace, tracing, ad-hoc debug print) would dump the full private key block. Replace the derive with a manual Debug impl that redacts `exported`. Signed-off-by: Glenn Gore --- openvtc/src/state_handler/setup_sequence/mod.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/openvtc/src/state_handler/setup_sequence/mod.rs b/openvtc/src/state_handler/setup_sequence/mod.rs index cd13201..215892a 100644 --- a/openvtc/src/state_handler/setup_sequence/mod.rs +++ b/openvtc/src/state_handler/setup_sequence/mod.rs @@ -272,12 +272,22 @@ pub struct DidGitSignSetupState { } /// Update messages as the Key export works through -#[derive(Clone, Debug, Default)] +#[derive(Clone, Default)] pub struct DIDKeysExportState { pub messages: Vec, + /// PGP-armored private key block — must never appear in Debug output pub exported: Option, } +impl fmt::Debug for DIDKeysExportState { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("DIDKeysExportState") + .field("messages", &self.messages) + .field("exported", &self.exported.as_ref().map(|_| "[REDACTED]")) + .finish() + } +} + /// State relating to detecting attached hardware tokens #[cfg(feature = "openpgp-card")] #[derive(Clone, Default)] From 0d4640d39c89687209cf5e12be9342b1b331f06c Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 08:26:48 +0200 Subject: [PATCH 04/11] fix(security): warn that --unlock-code is visible in the process list The --unlock-code flag passes the config-encryption passphrase via argv, which is readable by any local user through `ps` or /proc//cmdline and is typically captured in shell history. We can't retroactively scrub argv portably, so document the exposure in the --help text and emit a runtime warning when the flag is used so that users on multi-tenant hosts are nudged toward the interactive prompt instead. Signed-off-by: Glenn Gore --- openvtc/src/cli.rs | 7 ++++++- openvtc/src/main.rs | 8 ++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/openvtc/src/cli.rs b/openvtc/src/cli.rs index 848f2cf..06ef306 100644 --- a/openvtc/src/cli.rs +++ b/openvtc/src/cli.rs @@ -18,7 +18,12 @@ pub fn cli() -> Command { Arg::new("unlock-code") .short('u') .long("unlock-code") - .help("If using unlock codes, can specify it here"), + .help( + "Unlock passphrase for the encrypted config. \ + WARNING: command-line arguments are visible to other \ + local users via the process list (`ps`, /proc); prefer \ + the interactive prompt on shared systems.", + ), Arg::new("profile") .short('p') .long("profile") diff --git a/openvtc/src/main.rs b/openvtc/src/main.rs index 20b9df3..3d98de1 100644 --- a/openvtc/src/main.rs +++ b/openvtc/src/main.rs @@ -302,6 +302,14 @@ fn load_fast(profile: &str) -> Result { ConfigProtectionType::Token { .. } => None, ConfigProtectionType::Encrypted => { if let Some(passphrase) = cli().get_matches().get_one::("unlock-code") { + eprintln!( + "{}", + style( + "WARNING: --unlock-code exposes the passphrase in the process list; \ + prefer the interactive prompt on shared systems." + ) + .color256(CLI_ORANGE) + ); Some(UnlockCode::from_string(passphrase)?) } else { let mut result = None; From 2fb6d3dc6ce7f5dcbbdc159b8a680550d92a2ddc Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 09:23:52 +0200 Subject: [PATCH 05/11] fix(security): restore terminal on panic via panic hook setup_terminal() puts the TTY into raw mode and switches to the alternate screen, but the matching restore only runs on the normal and error return paths of UiManager::main_loop(). Any panic inside the render loop, a key handler, or a spawned task therefore leaves the user's terminal in raw mode on the alternate screen with no cursor, forcing a blind `reset`. Install a panic hook after entering raw mode that disables raw mode and leaves the alternate screen before chaining to the original hook, so the panic message is visible and the shell is usable. Signed-off-by: Glenn Gore --- openvtc/src/ui/mod.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/openvtc/src/ui/mod.rs b/openvtc/src/ui/mod.rs index d17b25c..9bda5bd 100644 --- a/openvtc/src/ui/mod.rs +++ b/openvtc/src/ui/mod.rs @@ -99,6 +99,16 @@ fn setup_terminal() -> anyhow::Result>> { EnableBracketedPaste )?; + // Ensure a panic anywhere in the render loop or a spawned task still + // returns the terminal to a usable state instead of leaving it in raw + // mode on the alternate screen. + let original_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + let _ = disable_raw_mode(); + let _ = execute!(io::stdout(), LeaveAlternateScreen); + original_hook(info); + })); + let mut terminal = Terminal::new(CrosstermBackend::new(stdout))?; terminal.clear()?; From f4c5ba28b495fc84e99b9016539cf97666a8ff06 Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 09:24:43 +0200 Subject: [PATCH 06/11] fix(security): drop exported private-key armor from State after use DIDKeysExportState::exported holds the ASCII-armored PGP private key block so the export page can render it and copy it to the clipboard. Once the wizard advances to the final page that data is no longer needed, but it was left in SetupState and therefore cloned and sent over the state broadcast channel on every subsequent tick for the remainder of the wizard. Clear the field when handling Action::SetupCompleted so the secret key material does not linger in additional heap copies of State. Signed-off-by: Glenn Gore --- openvtc/src/state_handler/setup_wizard.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openvtc/src/state_handler/setup_wizard.rs b/openvtc/src/state_handler/setup_wizard.rs index 7d48eb0..13533c0 100644 --- a/openvtc/src/state_handler/setup_wizard.rs +++ b/openvtc/src/state_handler/setup_wizard.rs @@ -159,6 +159,10 @@ impl StateHandler { }, Action::SetupCompleted(setup_flow) => { state.setup.active_page = SetupPage::FinalPage; + // The armored private-key block is no longer needed once we + // leave the export page; drop it so it stops being cloned + // out on every state broadcast. + state.setup.did_keys_export.exported = None; state.setup.final_page.messages.push(MessageType::Info("Generating your profile configuration...".to_string())); state.setup.final_page.messages.push(MessageType::Info("Securing sensitive data for storage...".to_string())); state.setup.final_page.messages.push(MessageType::Info("Your device may prompt for authentication to access OS secure storage.".to_string())); From 47732e384805aa1494c0fefc61b4cbc548c2550e Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 10:16:04 +0200 Subject: [PATCH 07/11] fix(security): avoid OOB panic on stale token-list index TokenSelect::selected is local widget state preserved across move_with_state(), while state.tokens.tokens is refreshed from the broadcast State on every tick. If the token list shrinks between when the user navigated the list and the next access -- e.g. a token is unplugged, or [R] re-enumerates and finds fewer cards -- the retained index can exceed the new bounds. Both the Enter handler and the admin-PIN render path indexed tokens[self.selected] directly, panicking the TUI on the next keypress or frame respectively. Use .get() in both places and fall through to the existing no-selection / early-return behaviour instead. Signed-off-by: Glenn Gore --- .../setup_flow/pgp_token/token_select.rs | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/openvtc/src/ui/pages/setup_flow/pgp_token/token_select.rs b/openvtc/src/ui/pages/setup_flow/pgp_token/token_select.rs index dbfc81b..08bdda3 100644 --- a/openvtc/src/ui/pages/setup_flow/pgp_token/token_select.rs +++ b/openvtc/src/ui/pages/setup_flow/pgp_token/token_select.rs @@ -108,16 +108,24 @@ impl TokenSelect { let _ = state.action_tx.send(Action::SetAdminPin(token, admin_pin)); } else { // Selected Token - Now get Admin PIN - if state.token_select.selected == state.props.state.tokens.tokens.len() { - // No token selected - state.token_select.selected_token = None; - let result = navigate(SetupEvent::TokenNoSelection, &state.props.state); - handle_nav_result(result, state); - } else { - state.token_select.selected_token = Some( - state.props.state.tokens.tokens[state.token_select.selected].clone(), - ); - state.token_select.ask_admin_pin = true; + // `selected` persists across state updates while the token + // list can shrink (unplug / refresh), so look it up safely. + match state + .props + .state + .tokens + .tokens + .get(state.token_select.selected) + { + Some(token) => { + state.token_select.selected_token = Some(token.clone()); + state.token_select.ask_admin_pin = true; + } + None => { + state.token_select.selected_token = None; + let result = navigate(SetupEvent::TokenNoSelection, &state.props.state); + handle_nav_result(result, state); + } } } } @@ -139,7 +147,10 @@ impl TokenSelect { if self.ask_admin_pin { // Need to get ADMIN Pin from the user - let mut lock = match state.tokens.tokens[self.selected].try_lock() { + let Some(token) = state.tokens.tokens.get(self.selected) else { + return; + }; + let mut lock = match token.try_lock() { Ok(lock) => lock, Err(_) => return, }; From 56b3642cb363b4adadcaf8915f92f8def3656483 Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 10:25:33 +0200 Subject: [PATCH 08/11] fix(security): clear private-key clipboard when leaving export page The export page offers [C] to copy the ASCII-armored PGP private key block to the OS clipboard. Nothing ever cleared it, so the key material remained in the clipboard -- and in any clipboard manager / history daemon -- indefinitely after the wizard moved on. When the user presses Enter to continue, check whether the clipboard still contains exactly what we put there and, if so, clear it. If the user has copied something else in the meantime we leave the clipboard untouched. Signed-off-by: Glenn Gore --- .../src/ui/pages/setup_flow/did_keys_export_show.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/openvtc/src/ui/pages/setup_flow/did_keys_export_show.rs b/openvtc/src/ui/pages/setup_flow/did_keys_export_show.rs index a314ed1..3bb5a01 100644 --- a/openvtc/src/ui/pages/setup_flow/did_keys_export_show.rs +++ b/openvtc/src/ui/pages/setup_flow/did_keys_export_show.rs @@ -49,6 +49,17 @@ impl DIDKeysExportShow { let _ = state.action_tx.send(Action::Exit); } KeyCode::Enter => { + // If we put the private-key block on the clipboard and it is + // still there, clear it before leaving so it does not linger + // in clipboard history / managers. Don't clobber anything the + // user has copied since. + if state.did_keys_export_show.clipboard_copy + && let Some(exported) = &state.props.state.did_keys_export.exported + && let Ok(mut clipboard) = Clipboard::new() + && clipboard.get_text().ok().as_deref() == Some(exported.as_str()) + { + let _ = clipboard.clear(); + } let result = navigate(SetupEvent::ExportComplete, &state.props.state); handle_nav_result(result, state); } From f3277c1197a1891deed3f92a036f2424f89def71 Mon Sep 17 00:00:00 2001 From: gkh_clankerbot_2000 Date: Fri, 24 Apr 2026 13:58:53 +0200 Subject: [PATCH 09/11] fix(security): clear ConfigImport passphrase Input buffers after dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `unlock_code_set.rs`, `did_keys_export_inputs.rs`, `vta_credential.rs` and `token_select.rs` all `.reset()` their secret-bearing `tui_input` fields once the value has been wrapped in a `SecretString` and sent to the state handler (added in 9bfde1c6ccce). The config-import page was missed, so both the source-config unlock passphrase and the new profile passphrase remained in the UI-side `Input` buffers — outside `secrecy`'s zeroize-on-drop — and were re-touched on every render frame for the rest of the wizard. Reset both passphrase inputs immediately after the `ImportConfig` action is dispatched. Signed-off-by: Glenn Gore --- openvtc/src/ui/pages/setup_flow/config_import.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openvtc/src/ui/pages/setup_flow/config_import.rs b/openvtc/src/ui/pages/setup_flow/config_import.rs index 6561980..dac65c3 100644 --- a/openvtc/src/ui/pages/setup_flow/config_import.rs +++ b/openvtc/src/ui/pages/setup_flow/config_import.rs @@ -108,6 +108,11 @@ impl ConfigImport { .value() .to_string(), )); + // The action carries SecretString copies; drop the + // plaintext from the tui_input buffers so it isn't + // re-rendered (masked) every frame or left in heap memory. + state.config_import.config_unlock_passphrase.reset(); + state.config_import.new_unlock_passphrase.reset(); } } KeyCode::Esc if !completed_ok && !locked => match state.config_import.active_input { From f8499fdabe756883817baff85295d2eff056c29c Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Sun, 24 May 2026 17:26:02 +0800 Subject: [PATCH 10/11] style: cargo fmt Signed-off-by: Glenn Gore --- openvtc/src/cli.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/openvtc/src/cli.rs b/openvtc/src/cli.rs index 06ef306..c95deca 100644 --- a/openvtc/src/cli.rs +++ b/openvtc/src/cli.rs @@ -15,15 +15,12 @@ pub fn cli() -> Command { .arg_required_else_help(false) .allow_external_subcommands(true) .args([ - Arg::new("unlock-code") - .short('u') - .long("unlock-code") - .help( - "Unlock passphrase for the encrypted config. \ + Arg::new("unlock-code").short('u').long("unlock-code").help( + "Unlock passphrase for the encrypted config. \ WARNING: command-line arguments are visible to other \ local users via the process list (`ps`, /proc); prefer \ the interactive prompt on shared systems.", - ), + ), Arg::new("profile") .short('p') .long("profile") From ca213d091e85ec8fb9537c69e9406ae35db41dfa Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Sun, 24 May 2026 17:29:08 +0800 Subject: [PATCH 11/11] chore: release v0.2.1 Security release containing nine fixes to the openvtc CLI; see CHANGELOG for the full list. Signed-off-by: Glenn Gore --- CHANGELOG.md | 14 ++++++++++++++ Cargo.toml | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81aeaf9..6d08cfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +## [0.2.1] - 2026-05-24 + +### Security + +- **Derive unlock key via Argon2id in non-openpgp import path** — the non-`openpgp-card` build was passing the raw passphrase to `SecuredConfig::save()` as the AES-256 key, bypassing the Argon2id KDF and making the saved config unrecoverable since `UnlockCode::from_string()` always applies Argon2id at load time +- **Reject path-traversal characters in profile name** — `--profile` and `OPENVTC_CONFIG_PROFILE` were spliced verbatim into lock-file paths, config paths, and OS keyring account names; now restricted to `[A-Za-z0-9._-]` with no `..` component +- **Redact armored private key block in `DIDKeysExportState` `Debug`** — the derived `Debug` impl could dump the full PGP-armored private key through any `{:?}` of `State` (panic backtrace, tracing, debug print) +- **Warn that `--unlock-code` is visible in the process list** — the flag exposes the passphrase via `ps`/`/proc//cmdline` and shell history; help text now documents this and a runtime warning nudges users toward the interactive prompt +- **Restore terminal on panic via panic hook** — panics inside the render loop, key handlers, or spawned tasks no longer leave the TTY in raw mode on the alternate screen +- **Drop exported private-key armor from `State` after use** — the armored PGP private key block was cloned through the state broadcast channel on every tick for the remainder of the setup wizard +- **Avoid OOB panic on stale token-list index** — unplugging or re-enumerating tokens no longer panics the TUI when a retained selection index exceeds the new bounds +- **Clear private-key clipboard when leaving export page** — the ASCII-armored PGP private key block placed on the OS clipboard by `[C]` is now cleared on continue (unless the user has copied something else in the meantime) +- **Clear `ConfigImport` passphrase `Input` buffers after dispatch** — both passphrase inputs are now reset once wrapped in `SecretString` and dispatched, matching the other secret-input pages + ## [0.2.0] - 2026-05-05 ### Added diff --git a/Cargo.toml b/Cargo.toml index 96b93e7..6a53ef6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ resolver = "3" [workspace.package] description = "Open Verifiable Trust Community (OpenVTC) - First Person Protocol" -version = "0.2.0" +version = "0.2.1" edition = "2024" publish = false authors = ["Glenn Gore "]