fix(security): nine CLI security/hardening fixes — release v0.2.1#61
Merged
Conversation
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 <glenn.g@affinidi.com>
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 <glenn.g@affinidi.com>
… 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 <glenn.g@affinidi.com>
The --unlock-code flag passes the config-encryption passphrase via argv, which is readable by any local user through `ps` or /proc/<pid>/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 <glenn.g@affinidi.com>
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 <glenn.g@affinidi.com>
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 <glenn.g@affinidi.com>
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 <glenn.g@affinidi.com>
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 <glenn.g@affinidi.com>
…atch `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 <glenn.g@affinidi.com>
Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
Security release containing nine fixes to the openvtc CLI; see CHANGELOG for the full list. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
7 tasks
stormer78
added a commit
that referenced
this pull request
May 24, 2026
vta-sdk 0.7 moves `access_token`, `access_expires_at` and `refresh_token` off `WebvhServerRecord` and into a separate service-internal `WebvhServerAuthRecord` (keyspace prefix `server-auth:`). Public-surface records no longer carry token material so they don't leak into REST list responses, DIDComm `webvh.servers.list` results, or backup exports. Update the navigation test fixture to construct the new shape. Detected by default-features CI on PR #61 / #62 (`(bin "openvtc" test)` target) with E0560. Masked locally because earlier verification piped `cargo test --workspace` through `tail`, dropping cargo's non-zero exit code. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
stormer78
added a commit
that referenced
this pull request
May 24, 2026
* fix(build): import arboard::Clipboard in did_keys_export_show The clipboard-clear logic added in 56b3642 references `Clipboard::new()` and `clipboard.get_text()` without importing `arboard::Clipboard`. The adjacent [C] copy handler goes through `crate::clipboard::copy_to_clipboard` (an OSC 52 / arboard abstraction) which is why the file had no direct arboard import, and why the regression slipped past local `cargo check --all-features` but failed default-features CI on PR #61 with E0433. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * chore(deps): bump vta-sdk 0.6 -> 0.7 Picks up the 0.7.0 release of the Verifiable Trust Infrastructure SDK (github.com/OpenVTC/verifiable-trust-infrastructure). No API changes required at the openvtc call sites — workspace builds clean under default and --no-default-features, tests and clippy pass. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * chore(deps): cargo update Pulls the latest compatible patch versions of the Affinidi / VTA / WASM dependency stacks: affinidi-crypto 0.1.5 -> 0.1.6 affinidi-messaging-mediator 0.15.3 -> 0.15.4 affinidi-messaging-test-mediator 0.2.2 -> 0.2.3 didwebvh-rs 0.5.2 -> 0.5.3 vta-sdk 0.5.0 -> 0.7.0 (lockfile resync) serde_json 1.0.149 -> 1.0.150 autocfg, bumpalo, js-sys, wasm-bindgen*, web-sys No Cargo.toml ranges changed beyond the explicit vta-sdk minor bump in the previous commit. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * fix(test): drop removed token fields from WebvhServerRecord fixture vta-sdk 0.7 moves `access_token`, `access_expires_at` and `refresh_token` off `WebvhServerRecord` and into a separate service-internal `WebvhServerAuthRecord` (keyspace prefix `server-auth:`). Public-surface records no longer carry token material so they don't leak into REST list responses, DIDComm `webvh.servers.list` results, or backup exports. Update the navigation test fixture to construct the new shape. Detected by default-features CI on PR #61 / #62 (`(bin "openvtc" test)` target) with E0560. Masked locally because earlier verification piped `cargo test --workspace` through `tail`, dropping cargo's non-zero exit code. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * chore(ci): bump MSRV pin 1.94.0 -> 1.95.0 The workspace declares `rust-version = "1.95.0"` and `did-git-sign` depends on language features (e.g. let-chains) that require it, but the dedicated MSRV CI job still pinned `dtolnay/rust-toolchain@1.94.0`. That job has been red since the workspace rust-version was bumped; align the pin with the declared floor so MSRV verifies the version we actually support. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * chore(clippy): apply Rust 1.95 stable lint suggestions CI runs `cargo clippy --workspace --all-targets -- -D warnings`. Several lints that were warning-only in 1.94 became enforced after the workspace rust-version bumped to 1.95, leaving clippy red on main: did-git-sign/src/main.rs collapsible_else_if (uninstall scope) openvtc-core/tests/relationship_e2e.rs explicit_auto_deref on `*type_url` openvtc/src/state_handler/message_dispatch.rs items_after_test_module — move `mod tests` to file end (matches sibling modules' convention) openvtc/src/state_handler/mod.rs collapsible_if on the VTA-context lookup; collapse into a single chained `if matches!(...) && let ...` openvtc/src/state_handler/settings_actions.rs collapsible_match — hoist `field == 0` into the pattern guard openvtc/src/state_handler/setup_wizard.rs collapsible_match — `VtaCreateKeys` hoists cleanly; the two arms that bind owned values (`server_id`, `custom_path`, `webvh_address`) can't move into a match guard (E0507), so keep the inner-if form and `#[allow]` the lint locally openvtc/src/ui/pages/main/mod.rs collapsible_match on Up / Down arms openvtc/src/ui/pages/setup_flow/mod.rs default_constructed_unit_structs (`DidGitSignSetup` is a unit struct) No behaviour changes; pure refactors. Verified with `cargo clippy --workspace --all-targets -- -D warnings` clean and `cargo build --workspace [--no-default-features]` passing. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * style: cargo fmt Signed-off-by: Glenn Gore <glenn.g@affinidi.com> --------- Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle of nine independent security and hardening fixes to
openvtc/CLI, plus release bump to v0.2.1. Each patch is its own commit with the original author preserved and DCO sign-off.High severity
c0d0a70Argon2id KDF bypassed in non-openpgp build — non-openpgp-cardbuilds were passing the raw passphrase directly toSecuredConfig::save(). This skipped Argon2id entirely (no memory-hard stretching) and produced a key that load-timeUnlockCode::from_string()could never match, so the saved config was unrecoverable. Fix: route throughderive_passphrase_key(..., b"openvtc-unlock-code-v1")to match the openpgp branch.Medium severity
f29e8e0Path traversal via--profile/OPENVTC_CONFIG_PROFILE— profile name was spliced verbatim into lock-file paths, config paths, and OS keyring account IDs. Restrict to[A-Za-z0-9._-]with no..at the CLI entry.9bbac54Armored PGP private key inDebug—DIDKeysExportStatederivedDebugwhile holding the private-key block; reachable fromStatevia every broadcast clone. Replace derive with a manualDebugimpl that redactsexported.f4c5ba2Exported key armor lingered in broadcastState—state.setup.did_keys_export.exportedwas cloned on every state-channel tick for the rest of the wizard. Clear it onAction::SetupCompleted.47732e3OOB panic on stale token-list index —tokens[self.selected]panicked the TUI when the list shrank (unplug / re-enumerate). Switch to.get()with graceful fallthrough on both Enter handler and admin-PIN render path.56b3642Private key left on OS clipboard indefinitely —[C]copied the armored key butEnternever cleared it. Clear only if the clipboard still contains exactly what we put there.f3277c1ConfigImportplaintext passphrase retained intui_inputbuffers — both passphrase inputs are now reset afterImportConfigis dispatched, matching the sibling secret-input pages (which already did this since 9bfde1c6ccce).Low severity / hygiene
0d4640d--unlock-codevisible in process list — argv can't be portably scrubbed; document in--helpand emit a runtime warning when the flag is used.2fb6d3dTerminal left in raw mode on panic — install a panic hook afterenable_raw_mode()that disables raw mode and leaves the alternate screen before chaining to the original hook.Other
f8499fd—cargo fmtoncli.rsafter applying fix: Correcting incorrect git paths #4.ca213d0— workspace version0.2.0 → 0.2.1, CHANGELOG entry.Test plan
cargo check --workspace --all-featurespassescargo fmt --checkpassescargo test --workspace(run in CI)openvtc setup) on bothopenpgp-cardand default buildsopenvtc --profile '../evil',openvtc --profile '',openvtc --profile 'a/b'