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 "] diff --git a/openvtc/src/cli.rs b/openvtc/src/cli.rs index 848f2cf..c95deca 100644 --- a/openvtc/src/cli.rs +++ b/openvtc/src/cli.rs @@ -15,10 +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("If using unlock codes, can specify it here"), + 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") diff --git a/openvtc/src/main.rs b/openvtc/src/main.rs index ece2bba..3d98de1 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)?; @@ -285,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; 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}"))?; 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)] 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())); 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()?; 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 { 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); } 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, };