From 72a4ecafe8dd850b8c50c62152e1d7df3f37b4d0 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:03:29 +0200 Subject: [PATCH 01/16] =?UTF-8?q?feat(platform-wallet-storage):=20SecretSt?= =?UTF-8?q?ore=20foundation=20=E2=80=94=20zeroizing=20wrappers,=20error,?= =?UTF-8?q?=20validation,=20MemoryStore?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group A (Tasks 1–3) of the secret-storage feature. All gated behind the opt-in `secrets` Cargo feature (never enabled by `default`). Task 1 — `secrets::secret`: `SecretString` (trimmed MIT fork of dash-evo-tool `Secret`, the egui `TextBuffer`/`take()` leak path deleted by construction — SEC-REQ-3.8.1/3.8.2) + net-new byte-oriented `SecretBytes`. Redacting `Debug`, no `Display`/`Deref`/`Serialize`, full-capacity zeroize on drop, best-effort `region` mlock, `subtle::ConstantTimeEq` on `SecretBytes`. The only `unsafe` is the forked full-capacity wipe in `Drop`, confined behind a narrow `#[allow(unsafe_code)]` + `// SAFETY:` proof — `#![deny(unsafe_code)]` stays crate-wide (SEC-REQ-4.8). Task 2 — `secrets::error::SecretStoreError`: concrete `thiserror` enum, no boxed dyn error (SEC-REQ-4.4 / TC-082), no `#[non_exhaustive]`, no secret/passphrase/plaintext/source in any variant, static `#[error]` strings. `secrets::validate`: 32-byte `WalletId` newtype + `^[A-Za-z0-9._-]{1,64}$` label allowlist, reject-not-sanitize (SEC-REQ-4.3, CWE-22/20). Task 3 — `secrets::store::SecretStore` trait (`get` returns `Option`, never bare `Vec` — SEC-REQ-4.1) + `MemoryStore` test double, gated by `__secrets-test-helpers` so it is unreachable from production builds (SEC-REQ-2.3.1/2.3.2). `src/lib.rs` slot activated; `secrets` feature wires only the RustSec-clean pinned crypto (argon2=0.5.3, chacha20poly1305=0.10.1, zeroize=1.8.2, subtle=2.6.1, region=3.0.2, getrandom; keyring-core 4.x split). MSRV 1.92 verified to compile the full dep set (`aes-gcm` omitted). `Send + Sync` / object-safety compile-asserts added. Satisfies SEC-REQ 3.1, 3.2, 3.3, 3.5, 3.6, 3.8.1, 3.8.2, 4.1, 4.2, 4.3, 4.4, 4.5, 4.6, 4.8, 2.0.3, 2.3.1, 2.3.2. Co-Authored-By: Claudius the Magnificent (1M context) --- Cargo.lock | 177 ++++++++ .../rs-platform-wallet-storage/Cargo.toml | 54 ++- .../rs-platform-wallet-storage/src/lib.rs | 20 +- .../src/secrets/error.rs | 78 ++++ .../src/secrets/memory.rs | 127 ++++++ .../src/secrets/mod.rs | 31 ++ .../src/secrets/secret.rs | 388 ++++++++++++++++++ .../src/secrets/store.rs | 55 +++ .../src/secrets/validate.rs | 100 +++++ 9 files changed, 1025 insertions(+), 5 deletions(-) create mode 100644 packages/rs-platform-wallet-storage/src/secrets/error.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/memory.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/mod.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/secret.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/store.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/validate.rs diff --git a/Cargo.lock b/Cargo.lock index 499ea6a1be..b081d4c383 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -140,6 +140,16 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "apple-native-keyring-store" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7be2f067ccd8d4b4d4a66ddafe0f32a5dff31732f32dbff85fefc40929b1f72" +dependencies = [ + "keyring-core", + "log", +] + [[package]] name = "arbitrary" version = "1.4.2" @@ -158,6 +168,18 @@ dependencies = [ "rustversion", ] +[[package]] +name = "argon2" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072" +dependencies = [ + "base64ct", + "blake2", + "cpufeatures 0.2.17", + "password-hash", +] + [[package]] name = "arrayref" version = "0.3.9" @@ -643,6 +665,15 @@ dependencies = [ "wyz", ] +[[package]] +name = "blake2" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" +dependencies = [ + "digest", +] + [[package]] name = "blake2b_simd" version = "1.0.4" @@ -1446,6 +1477,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" dependencies = [ "generic-array 0.14.7", + "rand_core 0.6.4", "typenum", ] @@ -1802,6 +1834,46 @@ version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4ae5f15dda3c708c0ade84bfee31ccab44a3da4f88015ed22f63732abe300c8" +[[package]] +name = "dbus" +version = "0.9.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b942602992bb7acfd1f51c49811c58a610ef9181b6e66f3e519d79b540a3bf73" +dependencies = [ + "libc", + "libdbus-sys", + "windows-sys 0.61.2", +] + +[[package]] +name = "dbus-secret-service" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "708b509edf7889e53d7efb0ffadd994cc6c2345ccb62f55cfd6b0682165e4fa6" +dependencies = [ + "aes", + "block-padding", + "cbc", + "dbus", + "fastrand", + "hkdf", + "num", + "once_cell", + "openssl", + "sha2", + "zeroize", +] + +[[package]] +name = "dbus-secret-service-keyring-store" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21d8f54da401bb5eb2a4d873ac4b359f4a599df2ca8634bb5b8c045e5ee78757" +dependencies = [ + "dbus-secret-service", + "keyring-core", +] + [[package]] name = "delegate" version = "0.13.5" @@ -3901,6 +3973,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "keyring-core" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb1e621458ca9c51aa110bd0339d4751a056b9576bf1253aee1aa560dda0fc9d" +dependencies = [ + "log", +] + [[package]] name = "keyword-search-contract" version = "3.1.0-dev.1" @@ -3945,6 +4026,16 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libdbus-sys" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "328c4789d42200f1eeec05bd86c9c13c7f091d2ba9a6ea35acdf51f31bc0f043" +dependencies = [ + "cc", + "pkg-config", +] + [[package]] name = "libloading" version = "0.8.9" @@ -3997,6 +4088,26 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "linux-keyutils" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83270a18e9f90d0707c41e9f35efada77b64c0e6f3f1810e71c8368a864d5590" +dependencies = [ + "bitflags 2.11.1", + "libc", +] + +[[package]] +name = "linux-keyutils-keyring-store" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39fbed79f71dc21eb21d3d07c0e908a3c58ff9a1fdbf5cf44230fb3deb6d994b" +dependencies = [ + "keyring-core", + "linux-keyutils", +] + [[package]] name = "linux-raw-sys" version = "0.4.15" @@ -4065,6 +4176,15 @@ dependencies = [ "sha2", ] +[[package]] +name = "mach2" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d640282b302c0bb0a2a8e0233ead9035e3bed871f0b7e81fe4a1ec829765db44" +dependencies = [ + "libc", +] + [[package]] name = "masternode-reward-shares-contract" version = "3.1.0-dev.1" @@ -4584,6 +4704,15 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" +[[package]] +name = "openssl-src" +version = "300.6.0+3.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8e8cbfd3a4a8c8f089147fd7aaa33cf8c7450c4d09f8f80698a0cf093abeff4" +dependencies = [ + "cc", +] + [[package]] name = "openssl-sys" version = "0.9.116" @@ -4592,6 +4721,7 @@ checksum = "f28a22dc7140cda5f096e5e7724a6962ca81a7f8bfd2979f9b18c11af56318c4" dependencies = [ "cc", "libc", + "openssl-src", "pkg-config", "vcpkg", ] @@ -4678,6 +4808,17 @@ dependencies = [ "regex", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "pasta_curves" version = "0.5.1" @@ -4949,36 +5090,47 @@ dependencies = [ name = "platform-wallet-storage" version = "3.1.0-dev.1" dependencies = [ + "apple-native-keyring-store", + "argon2", "assert_cmd", "barrel", "bincode", + "chacha20poly1305", "chrono", "clap", "dash-sdk", "dashcore", + "dbus-secret-service-keyring-store", "dpp", "filetime", "fs2", + "getrandom 0.2.17", "hex", "humantime", "key-wallet", "key-wallet-manager", + "keyring-core", + "linux-keyutils-keyring-store", "platform-wallet", "platform-wallet-storage", "predicates", "proptest", "refinery", + "region", "rusqlite", "serde", "serde_json", "serial_test", "sha2", "static_assertions", + "subtle", "tempfile", "thiserror 1.0.69", "tracing", "tracing-subscriber", "tracing-test", + "windows-native-keyring-store", + "zeroize", ] [[package]] @@ -5731,6 +5883,18 @@ version = "0.8.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" +[[package]] +name = "region" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6b6ebd13bc009aef9cd476c1310d49ac354d36e240cf1bd753290f3dc7199a7" +dependencies = [ + "bitflags 1.3.2", + "libc", + "mach2", + "windows-sys 0.52.0", +] + [[package]] name = "rend" version = "0.4.2" @@ -8627,6 +8791,19 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-native-keyring-store" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5fd986f648459dd29aa252ed3a5ad11a60c0b1251bf81625fb03a86c69d274e" +dependencies = [ + "byteorder", + "keyring-core", + "regex", + "windows-sys 0.61.2", + "zeroize", +] + [[package]] name = "windows-registry" version = "0.6.1" diff --git a/packages/rs-platform-wallet-storage/Cargo.toml b/packages/rs-platform-wallet-storage/Cargo.toml index 6009b2af1d..137763d1ab 100644 --- a/packages/rs-platform-wallet-storage/Cargo.toml +++ b/packages/rs-platform-wallet-storage/Cargo.toml @@ -59,6 +59,18 @@ chrono = { version = "0.4", default-features = false, features = [ ], optional = true } sha2 = { version = "0.10", optional = true } +# Secret-storage deps (gated by the `secrets` feature). RustSec-clean +# pins (Smythe §7); `aes-gcm` is deliberately omitted. `keyring`'s +# library is `keyring-core` + per-platform store crates (the `keyring` +# crate itself is sample/CLI). Verified to build under MSRV 1.92. +argon2 = { version = "=0.5.3", optional = true } +chacha20poly1305 = { version = "=0.10.1", optional = true } +zeroize = { version = "=1.8.2", features = ["derive"], optional = true } +subtle = { version = "=2.6.1", optional = true } +getrandom = { version = "0.2", optional = true } +region = { version = "=3.0.2", optional = true } +keyring-core = { version = "=1.0.0", optional = true } + # CLI deps (gated by the `cli` feature) clap = { version = "4", features = ["derive"], optional = true } humantime = { version = "2", optional = true } @@ -67,6 +79,23 @@ tracing-subscriber = { version = "0.3", features = [ "env-filter", ], optional = true } +# Per-platform OS-keyring credential stores (the keyring-core 4.x split: +# `keyring-core` is the API, these provide the backends). Gated by +# `secrets` via `dep:`. Target-specific tables MUST follow all +# `[dependencies]` entries. +[target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies] +linux-keyutils-keyring-store = { version = "=1.0.0", optional = true } +dbus-secret-service-keyring-store = { version = "=1.0.0", features = [ + "crypto-rust", + "vendored", +], optional = true } + +[target.'cfg(target_os = "macos")'.dependencies] +apple-native-keyring-store = { version = "=1.0.0", optional = true } + +[target.'cfg(target_os = "windows")'.dependencies] +windows-native-keyring-store = { version = "=1.0.0", optional = true } + [dev-dependencies] proptest = "1" assert_cmd = "2" @@ -76,6 +105,7 @@ filetime = "0.2" tracing-test = { version = "0.2", features = ["no-env-filter"] } serial_test = "3" platform-wallet-storage = { path = ".", features = ["sqlite", "cli", "__test-helpers"] } +tempfile = "3" [features] default = ["sqlite", "cli"] @@ -104,10 +134,26 @@ cli = [ "dep:serde_json", "dep:tracing-subscriber", ] -# Future `SecretStore` submodule. Slot is reserved; the module is not -# implemented in this build — enabling the feature today is a no-op -# beyond a `// pub mod secrets;` marker in `src/lib.rs`. -secrets = [] +# `SecretStore` submodule (`platform_wallet_storage::secrets`): +# zeroizing secret wrappers + Keyring / EncryptedFile backends. Opt-in; +# never enabled by `default`. Pulls only RustSec-clean pinned crypto. +secrets = [ + "dep:argon2", + "dep:chacha20poly1305", + "dep:zeroize", + "dep:subtle", + "dep:getrandom", + "dep:region", + "dep:keyring-core", + "dep:linux-keyutils-keyring-store", + "dep:dbus-secret-service-keyring-store", + "dep:apple-native-keyring-store", + "dep:windows-native-keyring-store", +] +# Exposes `secrets::MemoryStore` (in-RAM test double). Double-underscore +# prefix = Cargo's "MUST NOT enable from downstream" convention; keeps +# the test store unreachable from production builds (SEC-REQ-2.3.1). +__secrets-test-helpers = ["secrets"] # Exposes `lock_conn_for_test` / `config_for_test` accessors on # `SqlitePersister` so this crate's own integration tests can probe # the write connection. The double-underscore prefix follows Cargo's diff --git a/packages/rs-platform-wallet-storage/src/lib.rs b/packages/rs-platform-wallet-storage/src/lib.rs index c50e546b3c..1a40d38588 100644 --- a/packages/rs-platform-wallet-storage/src/lib.rs +++ b/packages/rs-platform-wallet-storage/src/lib.rs @@ -23,7 +23,9 @@ #[cfg(feature = "sqlite")] pub mod sqlite; -// pub mod secrets; // reserved — future SecretStore submodule. + +#[cfg(feature = "secrets")] +pub mod secrets; // Convenience re-exports kept under the crate root so embedders don't // have to spell out the `::sqlite::` middle segment for the common @@ -54,3 +56,19 @@ fn _object_safety_check(persister: SqlitePersister) { let _: std::sync::Arc = std::sync::Arc::new(persister); } + +// `SecretStore` must be object-safe and its error `Send + Sync`, so a +// backend can be held behind `Arc` and its errors +// crossed between threads / FFI. +#[cfg(feature = "secrets")] +#[allow(dead_code)] +const fn _secrets_send_sync_check() {} +#[cfg(feature = "secrets")] +const _: () = { + _secrets_send_sync_check::(); +}; +#[cfg(all(feature = "secrets", any(test, feature = "__secrets-test-helpers")))] +#[allow(dead_code)] +fn _secret_store_object_safety_check(store: secrets::MemoryStore) { + let _: std::sync::Arc = std::sync::Arc::new(store); +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/error.rs b/packages/rs-platform-wallet-storage/src/secrets/error.rs new file mode 100644 index 0000000000..a06b454a31 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/error.rs @@ -0,0 +1,78 @@ +//! Typed errors for the `SecretStore` backends. +//! +//! Concrete `thiserror` enum — no boxed dynamic error trait object +//! (SEC-REQ-4.4 / TC-082), no `#[non_exhaustive]` (prior project +//! decision), and **no** secret byte, passphrase, plaintext, or +//! stringified source that could carry one in any variant. +//! `#[error("...")]` strings are static and structural; only +//! non-secret diagnostics (a permission `mode`, a format `found` +//! version) are carried as typed fields (SEC-REQ-2.0.1 / 2.2.8, +//! CWE-209/CWE-532). + +/// Errors returned by [`SecretStore`](super::SecretStore) backends. +/// +/// Variant taxonomy lets a caller distinguish "no secure backend, ask +/// the operator" from "wrong passphrase, re-prompt" without ever +/// inspecting a secret. +#[derive(Debug, thiserror::Error)] +pub enum SecretStoreError { + /// No secure OS keyring is reachable (headless / no Secret Service / + /// no D-Bus session). Fail closed — never degrade to plaintext. + #[error("secret backend unavailable")] + BackendUnavailable, + + /// The OS keyring exists but its collection is locked. + #[error("keyring is locked")] + KeyringLocked, + + /// No secret stored under the requested `(wallet_id, label)`. + #[error("secret not found")] + NotFound, + + /// AEAD tag verification failed. Carries **no** decrypted-but- + /// unverified bytes and no source (SEC-REQ-2.2.8, CWE-347). + #[error("decryption/integrity check failed")] + Decrypt, + + /// The supplied passphrase did not unlock the vault. + #[error("wrong passphrase")] + WrongPassphrase, + + /// `label` failed the `^[A-Za-z0-9._-]{1,64}$` allowlist + /// (SEC-REQ-4.3, CWE-22/CWE-20). + #[error("invalid label")] + InvalidLabel, + + /// Filesystem error (open / write / rename / fsync). The inner + /// `io::Error` carries an OS code and a path *the caller supplied*, + /// never a secret. + #[error("io error")] + Io(#[from] std::io::Error), + + /// Argon2 key derivation failed. The upstream error carries no + /// useful non-secret diagnostic, so it is intentionally not + /// embedded (SEC-REQ-2.2.8). + #[error("key derivation failed")] + KdfFailure, + + /// The vault header declared a `format_version` this build does not + /// understand. Refuse, fail closed (SEC-REQ-2.2.9). + #[error("unsupported vault format version {found}")] + VersionUnsupported { + /// The version byte read from the (authenticated) header. + found: u32, + }, + + /// The vault file was malformed (bad magic, truncated header, bad + /// record framing) — no plaintext was produced. + #[error("malformed vault file")] + MalformedVault, + + /// A pre-existing vault file had permissions looser than `0600`. + /// Refuse rather than tighten-and-trust (SEC-REQ-2.2.10). + #[error("vault file has insecure permissions")] + InsecurePermissions { + /// The offending POSIX mode bits (not secret). + mode: u32, + }, +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/memory.rs b/packages/rs-platform-wallet-storage/src/secrets/memory.rs new file mode 100644 index 0000000000..4030140996 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/memory.rs @@ -0,0 +1,127 @@ +//! In-RAM [`SecretStore`] test double. +//! +//! Gated behind `__secrets-test-helpers` (Cargo's "MUST NOT enable from +//! downstream" convention) so it is unreachable from production builds +//! and can never be a silent fallback for a failed real backend +//! (SEC-REQ-2.3.1). Values sit in [`SecretBytes`] so even test memory +//! is wiped and the type contract is exercised uniformly +//! (SEC-REQ-2.3.2). +//! +//! ## Threat coverage +//! +//! Covers **nothing at rest** — process RAM only, by design. Never use +//! outside tests. + +use std::collections::HashMap; +use std::sync::Mutex; + +use super::error::SecretStoreError; +use super::secret::SecretBytes; +use super::store::SecretStore; +use super::validate::{validated_label, WalletId}; + +/// A `HashMap`-backed [`SecretStore`] for tests. No persistence, no +/// encryption. +#[derive(Default)] +pub struct MemoryStore { + map: Mutex>>, +} + +impl MemoryStore { + /// A fresh empty store. + pub fn new() -> Self { + Self::default() + } +} + +impl SecretStore for MemoryStore { + fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { + let label = validated_label(label)?; + let mut map = self.map.lock().expect("MemoryStore mutex poisoned"); + map.insert((wallet_id, label.to_string()), bytes.to_vec()); + Ok(()) + } + + fn get( + &self, + wallet_id: WalletId, + label: &str, + ) -> Result, SecretStoreError> { + let label = validated_label(label)?; + let map = self.map.lock().expect("MemoryStore mutex poisoned"); + Ok(map + .get(&(wallet_id, label.to_string())) + .map(|v| SecretBytes::from_slice(v))) + } + + fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { + let label = validated_label(label)?; + let mut map = self.map.lock().expect("MemoryStore mutex poisoned"); + map.remove(&(wallet_id, label.to_string())); + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn wid(b: u8) -> WalletId { + WalletId::from([b; 32]) + } + + #[test] + fn roundtrip_and_overwrite() { + let s = MemoryStore::new(); + assert!(s.get(wid(1), "bip39_mnemonic").unwrap().is_none()); + s.put(wid(1), "bip39_mnemonic", &[1, 2, 3]).unwrap(); + assert_eq!( + s.get(wid(1), "bip39_mnemonic") + .unwrap() + .unwrap() + .expose_secret(), + &[1, 2, 3] + ); + s.put(wid(1), "bip39_mnemonic", &[4, 5]).unwrap(); + assert_eq!( + s.get(wid(1), "bip39_mnemonic") + .unwrap() + .unwrap() + .expose_secret(), + &[4, 5] + ); + } + + #[test] + fn idempotent_delete_and_namespacing() { + let s = MemoryStore::new(); + s.put(wid(1), "seed", &[7]).unwrap(); + s.delete(wid(1), "seed").unwrap(); + s.delete(wid(1), "seed").unwrap(); // idempotent + assert!(s.get(wid(1), "seed").unwrap().is_none()); + + s.put(wid(1), "seed", &[1]).unwrap(); + s.put(wid(2), "seed", &[2]).unwrap(); + assert_eq!( + s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), + &[1] + ); + assert_eq!( + s.get(wid(2), "seed").unwrap().unwrap().expose_secret(), + &[2] + ); + } + + #[test] + fn rejects_invalid_label() { + let s = MemoryStore::new(); + assert!(matches!( + s.put(wid(1), "../escape", &[0]), + Err(SecretStoreError::InvalidLabel) + )); + assert!(matches!( + s.get(wid(1), ""), + Err(SecretStoreError::InvalidLabel) + )); + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs new file mode 100644 index 0000000000..5c768f478c --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -0,0 +1,31 @@ +//! Out-of-band storage for wallet secret material (mnemonic / seed / +//! xpriv), kept entirely off the SQLite persister's data path. +//! +//! Enabled by the opt-in `secrets` feature (never on by `default`). +//! Everything secret-bearing lives under this `src/secrets/` tree by +//! design: `tests/secrets_scan.rs` scans only `src/sqlite/schema/` + +//! `migrations/` and exempts this module, so this module owns its own +//! review discipline (`tests/secrets_guard.rs`, SEC-REQ-4.5/4.5.1). +//! +//! # Memory hygiene +//! +//! Secrets cross every boundary inside [`SecretBytes`] / [`SecretString`] +//! (zeroize-on-drop, redacting `Debug`, no `Display`/`Serialize`, +//! best-effort `mlock`). Errors are a concrete enum with no secret in +//! any variant. + +mod error; +mod secret; +mod store; +mod validate; + +#[cfg(any(test, feature = "__secrets-test-helpers"))] +mod memory; + +pub use error::SecretStoreError; +pub use secret::{SecretBytes, SecretString}; +pub use store::SecretStore; +pub use validate::WalletId; + +#[cfg(any(test, feature = "__secrets-test-helpers"))] +pub use memory::MemoryStore; diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs new file mode 100644 index 0000000000..e3d33ad1de --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -0,0 +1,388 @@ +//! Zeroizing secret wrappers. +//! +//! [`SecretString`] is a trimmed fork of dash-evo-tool's `Secret` +//! (`src/model/secret.rs`, MIT) with the `egui::TextBuffer` impl — +//! including its SEC-003 `take()` plaintext-leak path — **removed by +//! construction**: this crate has no egui, so the leak vector cannot +//! exist (SEC-REQ-3.8.1 / 3.8.2, CWE-316). +//! +//! [`SecretBytes`] is net-new: the byte-oriented wrapper for seeds, +//! xprivs, KDF output, AEAD keys and decrypted plaintext (SEC-REQ-3.8.1 +//! / 4.1). +//! +//! Both: redacting `Debug`, no `Display`/`Deref`/`Serialize`, full +//! buffer wipe on drop, best-effort `region` mlock. +//! +//! --- +//! Portions Copyright (c) Dash Core Group, originating from +//! dash-evo-tool (`src/model/secret.rs`), MIT License: +//! +//! Permission is hereby granted, free of charge, to any person +//! obtaining a copy of this software and associated documentation +//! files (the "Software"), to deal in the Software without +//! restriction, including without limitation the rights to use, copy, +//! modify, merge, publish, distribute, sublicense, and/or sell copies +//! of the Software, and to permit persons to whom the Software is +//! furnished to do so, subject to the following conditions: +//! +//! The above copyright notice and this permission notice shall be +//! included in all copies or substantial portions of the Software. +//! +//! THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND. + +use std::fmt; + +use subtle::ConstantTimeEq; +use zeroize::{Zeroize, Zeroizing}; + +/// Pre-allocation capacity for [`SecretString`] buffers. +/// +/// `mlock` is page-granular, so a sub-page buffer locks a whole page +/// regardless; 4096 bytes also makes `String` reallocation (which +/// leaves an un-zeroed freed buffer the allocator owns) virtually +/// impossible for any human-entered passphrase or mnemonic. +const DEFAULT_CAPACITY: usize = 4096; + +/// Zeroize-on-drop wrapper for secret UTF-8 strings (BIP-39 mnemonic, +/// `EncryptedFileStore` passphrase). +/// +/// `Display`, `Deref`, `DerefMut`, `Serialize` are intentionally **not** +/// implemented; read access is the explicit [`expose_secret`] only. +/// `Debug` is redacted. The backing buffer is wiped over its full +/// capacity on drop and best-effort `mlock`ed against swap. +/// +/// [`expose_secret`]: SecretString::expose_secret +pub struct SecretString { + inner: Zeroizing, + _lock: Option, +} + +impl SecretString { + /// Wrap a string, copying it into a capacity-padded buffer, + /// zeroizing the source, and best-effort `mlock`ing the buffer. + pub fn new(s: impl Into) -> Self { + let mut source: String = s.into(); + let cap = source.len().max(DEFAULT_CAPACITY); + let mut buf = String::with_capacity(cap); + buf.push_str(&source); + source.zeroize(); + let lock = region::lock(buf.as_ptr(), buf.capacity()) + .map_err(|e| { + tracing::debug!("mlock failed for SecretString: {e}"); + e + }) + .ok(); + Self { + inner: Zeroizing::new(buf), + _lock: lock, + } + } + + /// An empty, capacity-padded, locked buffer. + pub fn empty() -> Self { + Self::default() + } + + /// Borrow the plaintext. The only read path. + pub fn expose_secret(&self) -> &str { + &self.inner + } + + /// Secret length in bytes. + pub fn len(&self) -> usize { + self.inner.len() + } + + /// Whether the secret is empty. + pub fn is_empty(&self) -> bool { + self.inner.is_empty() + } + + /// A new `SecretString` holding the whitespace-trimmed content, + /// keeping the trimmed copy inside the wrapper. + pub fn trimmed(&self) -> Self { + Self::new(self.inner.trim().to_string()) + } +} + +impl Drop for SecretString { + fn drop(&mut self) { + let ptr = self.inner.as_mut_ptr(); + let cap = self.inner.capacity(); + if cap > 0 { + // SAFETY: `ptr` is the `String`'s allocation, valid and + // uniquely borrowed for `cap` bytes during drop. We only + // write zeros within `[0, cap)`. This wipes the bytes in + // `[len, cap)` that `Zeroizing` (which clears only + // `0..len`) would miss. + #[allow(unsafe_code)] + let slice = unsafe { std::slice::from_raw_parts_mut(ptr, cap) }; + slice.zeroize(); + } + } +} + +impl Default for SecretString { + fn default() -> Self { + let s = String::with_capacity(DEFAULT_CAPACITY); + let lock = region::lock(s.as_ptr(), s.capacity()) + .map_err(|e| { + tracing::debug!("mlock failed for SecretString: {e}"); + e + }) + .ok(); + Self { + inner: Zeroizing::new(s), + _lock: lock, + } + } +} + +impl fmt::Debug for SecretString { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("SecretString(***)") + } +} + +impl PartialEq for SecretString { + /// Best-effort timing-resistant passphrase **UX** equality only. + /// Length differences early-return, leaking length through timing; + /// this is never used for a security decision (the wrong-seed gate + /// uses [`SecretBytes`]' fixed-width `subtle` compare instead) — + /// SEC-REQ-3.8.2. + fn eq(&self, other: &Self) -> bool { + let a = self.expose_secret().as_bytes(); + let b = other.expose_secret().as_bytes(); + if a.len() != b.len() { + return false; + } + a.ct_eq(b).into() + } +} + +impl Eq for SecretString {} + +impl From for SecretString { + fn from(s: String) -> Self { + Self::new(s) + } +} + +impl From<&str> for SecretString { + fn from(s: &str) -> Self { + Self::new(s.to_string()) + } +} + +/// Zeroize-on-drop wrapper for secret **bytes**: BIP-32 seed +/// (`[u8; 64]`), xpriv, Argon2 output, AEAD key, decrypted plaintext, +/// ciphertext-in-flight (SEC-REQ-3.8.1 / 4.1). +/// +/// Not `Copy`; `Clone` is intentionally absent to enforce copy +/// minimization (SEC-REQ-3.5) — move it, or `expose_secret()` and copy +/// deliberately into another wrapper. `Display`, `Deref`, `Serialize` +/// are intentionally **not** implemented; `Debug` is redacted; the +/// buffer is wiped on drop and best-effort `mlock`ed. +pub struct SecretBytes { + inner: Zeroizing>, + _lock: Option, +} + +impl SecretBytes { + /// Wrap a byte vector, zeroizing the source, best-effort `mlock`ing + /// the wrapped buffer. + pub fn new(mut bytes: Vec) -> Self { + let lock = region::lock(bytes.as_ptr(), bytes.capacity().max(1)) + .map_err(|e| { + tracing::debug!("mlock failed for SecretBytes: {e}"); + e + }) + .ok(); + let inner = Zeroizing::new(std::mem::take(&mut bytes)); + bytes.zeroize(); + Self { inner, _lock: lock } + } + + /// A zeroed buffer of `len` bytes, best-effort `mlock`ed — for + /// in-place fills (KDF output, decrypt target). + pub fn zeroed(len: usize) -> Self { + Self::new(vec![0u8; len]) + } + + /// Copy a borrowed slice into a fresh wrapper. Deliberate, explicit + /// copy (SEC-REQ-3.5) — the only way to duplicate secret bytes. + pub fn from_slice(bytes: &[u8]) -> Self { + Self::new(bytes.to_vec()) + } + + /// Borrow the plaintext bytes. The only read path. + pub fn expose_secret(&self) -> &[u8] { + &self.inner + } + + /// Mutably borrow the plaintext bytes (in-place KDF/decrypt fill). + pub fn expose_secret_mut(&mut self) -> &mut [u8] { + &mut self.inner + } + + /// Secret length in bytes. + pub fn len(&self) -> usize { + self.inner.len() + } + + /// Whether the secret is empty. + pub fn is_empty(&self) -> bool { + self.inner.is_empty() + } +} + +impl ConstantTimeEq for SecretBytes { + /// Fixed-width constant-time compare over the byte region — no + /// length early-return (SEC-REQ-3.6). `subtle::ConstantTimeEq` on + /// unequal-length slices yields `0` without leaking *where* they + /// differ; the only observable is the (non-secret) length. + fn ct_eq(&self, other: &Self) -> subtle::Choice { + self.inner.as_slice().ct_eq(other.inner.as_slice()) + } +} + +impl PartialEq for SecretBytes { + fn eq(&self, other: &Self) -> bool { + self.ct_eq(other).into() + } +} + +impl Eq for SecretBytes {} + +impl fmt::Debug for SecretBytes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "SecretBytes([REDACTED; {}])", self.inner.len()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn secret_string_debug_redacted() { + let s = SecretString::new("correct horse battery staple"); + let dbg = format!("{s:?}"); + assert_eq!(dbg, "SecretString(***)"); + assert!(!dbg.contains("horse")); + } + + #[test] + fn secret_string_expose_and_trim() { + let s = SecretString::new(" abandon ability "); + assert_eq!(s.expose_secret(), " abandon ability "); + assert_eq!(s.trimmed().expose_secret(), "abandon ability"); + } + + #[test] + fn secret_string_eq_is_value_based() { + assert_eq!(SecretString::new("pw"), SecretString::new("pw")); + assert_ne!(SecretString::new("pw"), SecretString::new("px")); + assert_ne!(SecretString::new("pw"), SecretString::new("pww")); + } + + #[test] + fn secret_string_empty_default() { + assert!(SecretString::empty().is_empty()); + assert_eq!(SecretString::default().len(), 0); + } + + #[test] + fn secret_bytes_debug_redacted() { + let b = SecretBytes::from_slice(&[1, 2, 3, 4, 5]); + let dbg = format!("{b:?}"); + assert_eq!(dbg, "SecretBytes([REDACTED; 5])"); + assert!(!dbg.contains('1')); + } + + #[test] + fn secret_bytes_roundtrip_and_zeroed() { + let b = SecretBytes::from_slice(&[9, 8, 7]); + assert_eq!(b.expose_secret(), &[9, 8, 7]); + assert_eq!(b.len(), 3); + let z = SecretBytes::zeroed(4); + assert_eq!(z.expose_secret(), &[0, 0, 0, 0]); + } + + #[test] + fn secret_bytes_constant_time_eq() { + let a = SecretBytes::from_slice(&[1, 2, 3, 4]); + let b = SecretBytes::from_slice(&[1, 2, 3, 4]); + let c = SecretBytes::from_slice(&[1, 2, 3, 5]); + let d = SecretBytes::from_slice(&[1, 2, 3]); + assert!(bool::from(a.ct_eq(&b))); + assert!(!bool::from(a.ct_eq(&c))); + assert!(!bool::from(a.ct_eq(&d))); + assert_eq!(a, b); + assert_ne!(a, c); + } + + #[test] + fn secret_bytes_expose_mut_fills_in_place() { + let mut b = SecretBytes::zeroed(3); + b.expose_secret_mut().copy_from_slice(&[7, 7, 7]); + assert_eq!(b.expose_secret(), &[7, 7, 7]); + } + + // `SecretBytes`/`SecretString` must run `Drop` (zeroize), so they + // cannot be trivially droppable. + const _: () = { + assert!(std::mem::needs_drop::()); + assert!(std::mem::needs_drop::()); + }; + + /// Best-effort runtime check that `Drop` wipes the full `SecretString` + /// capacity. Reads freed memory — UB in the strict sense, flaky under + /// parallelism; run single-threaded: + /// `cargo test --features secrets -- secret_string_drop_zeroes --ignored --test-threads=1` + #[test] + #[ignore] + fn secret_string_drop_zeroes_full_capacity() { + let ptr: *const u8; + let cap: usize; + { + let s = SecretString::new("sensitive_seed_material"); + ptr = s.inner.as_ptr(); + cap = s.inner.capacity(); + // SAFETY: live allocation, read for `cap` bytes pre-drop. + #[allow(unsafe_code)] + let pre = unsafe { std::slice::from_raw_parts(ptr, cap) }; + assert!(pre.iter().any(|&b| b != 0)); + } + // SAFETY: best-effort post-free read; single-thread makes page + // reuse before this read unlikely. + #[allow(unsafe_code)] + let post = unsafe { std::slice::from_raw_parts(ptr, cap) }; + assert!(post.iter().all(|&b| b == 0), "buffer not zeroed on drop"); + } + + /// Best-effort runtime check that `Drop` wipes `SecretBytes`. Same + /// caveat as above; run single-threaded with `--ignored`. A + /// page-sized buffer is used so the allocator is unlikely to reuse + /// the freed page before the post-drop read (a tiny `Vec` would be + /// recycled immediately, making the check meaningless). + #[test] + #[ignore] + fn secret_bytes_drop_zeroes() { + let ptr: *const u8; + let cap: usize; + { + let b = SecretBytes::from_slice(&[0xAB; 4096]); + ptr = b.inner.as_ptr(); + cap = b.inner.capacity(); + // SAFETY: live allocation, read for `cap` bytes pre-drop. + #[allow(unsafe_code)] + let pre = unsafe { std::slice::from_raw_parts(ptr, cap) }; + assert!(pre.iter().any(|&x| x != 0)); + } + // SAFETY: best-effort post-free read; see note above. + #[allow(unsafe_code)] + let post = unsafe { std::slice::from_raw_parts(ptr, cap) }; + assert!(post.iter().all(|&x| x == 0), "buffer not zeroed on drop"); + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs new file mode 100644 index 0000000000..6f60d4d00a --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -0,0 +1,55 @@ +//! The [`SecretStore`] port. + +use super::error::SecretStoreError; +use super::secret::SecretBytes; +use super::validate::WalletId; + +/// Stores wallet secret material out-of-band of the SQLite persister. +/// +/// Implementations MUST NOT write any secret byte to the database, its +/// WAL, backups, `tracing` events, `Debug`/`Display`, error payloads, +/// panic messages, or temp files outside their own controlled path +/// (the SECRETS.md invariant, SEC-REQ-2.0.1). +/// +/// All three methods validate `label` against the +/// `^[A-Za-z0-9._-]{1,64}$` allowlist before touching a backing store, +/// returning [`SecretStoreError::InvalidLabel`] on violation rather +/// than sanitizing. +pub trait SecretStore: Send + Sync { + /// Store `bytes` under `(wallet_id, label)`, overwrite-safe: an + /// existing label is atomically replaced or the call fails closed — + /// both old and new plaintext are never simultaneously recoverable + /// (SEC-REQ-2.0.2). + /// + /// The caller owns and must zeroize the source buffer; prefer + /// [`put_secret`](SecretStore::put_secret) so the source is a + /// `&SecretBytes`. The implementation MUST NOT copy `bytes` into a + /// long-lived unwrapped buffer. + fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError>; + + /// Retrieve the secret. `Ok(None)` for a missing label — idempotent + /// and non-secret-leaking (SEC-REQ-2.0.3). The returned buffer + /// zeroizes on drop (SEC-REQ-4.1); a bare `Vec` is never + /// returned. + fn get( + &self, + wallet_id: WalletId, + label: &str, + ) -> Result, SecretStoreError>; + + /// Idempotent delete. `Ok(())` whether or not the label existed; no + /// secret-bearing error distinguishes the two cases. + fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError>; + + /// Ergonomic [`put`](SecretStore::put) over an already-wrapped + /// secret. Default impl forwards the exposed bytes; no extra + /// long-lived copy is made. + fn put_secret( + &self, + wallet_id: WalletId, + label: &str, + secret: &SecretBytes, + ) -> Result<(), SecretStoreError> { + self.put(wallet_id, label, secret.expose_secret()) + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/validate.rs b/packages/rs-platform-wallet-storage/src/secrets/validate.rs new file mode 100644 index 0000000000..2ecfac5464 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/validate.rs @@ -0,0 +1,100 @@ +//! Input validation for the `SecretStore` key space (SEC-REQ-4.3). +//! +//! `wallet_id` is fixed-width 32 bytes — enforced by the [`WalletId`] +//! type, not at runtime. `label` is reject-not-sanitize against a +//! strict allowlist before any backend maps it to a filename or a +//! keyring attribute (CWE-22 path traversal, CWE-20 improper input). + +use super::error::SecretStoreError; + +/// A 32-byte wallet identifier — the `SecretStore` namespace key. +/// +/// Public correlation material, **not** a secret (Smythe §1.1): it is +/// derived from public wallet state, never from the seed's private +/// bytes. Fixed width is a type invariant, so no runtime length check +/// is needed. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct WalletId(pub [u8; 32]); + +impl WalletId { + /// The raw 32 id bytes. + pub fn as_bytes(&self) -> &[u8; 32] { + &self.0 + } + + /// Lowercase hex form, for filesystem / keyring namespacing. + pub fn to_hex(&self) -> String { + hex::encode(self.0) + } +} + +impl From<[u8; 32]> for WalletId { + fn from(bytes: [u8; 32]) -> Self { + Self(bytes) + } +} + +/// Maximum `label` length, matching the allowlist's `{1,64}` bound. +const MAX_LABEL_LEN: usize = 64; + +/// Validate a `label` against `^[A-Za-z0-9._-]{1,64}$` and return it +/// unchanged on success. Rejects (never sanitizes) so a traversal / +/// attribute-injection attempt is a hard error, not silently rewritten. +pub(crate) fn validated_label(label: &str) -> Result<&str, SecretStoreError> { + let ok = (1..=MAX_LABEL_LEN).contains(&label.len()) + && label + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'_' | b'-')); + if ok { + Ok(label) + } else { + Err(SecretStoreError::InvalidLabel) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_allowlisted_labels() { + for ok in [ + "bip39_mnemonic", + "bip32-seed", + "x.priv.0", + "A", + &"a".repeat(64), + ] { + assert!(validated_label(ok).is_ok(), "should accept {ok:?}"); + } + } + + #[test] + fn rejects_traversal_and_injection() { + for bad in [ + "", // empty + &"a".repeat(65), // too long + "../etc/passwd", // path traversal + "a/b", // separator + "a\\b", // windows separator + "a b", // space + "lab\0el", // NUL + "lab\nel", // newline + "café", // non-ASCII + "a:b", // keyring attribute delimiter + "a;DROP TABLE", // sql-ish + ] { + assert!( + matches!(validated_label(bad), Err(SecretStoreError::InvalidLabel)), + "should reject {bad:?}" + ); + } + } + + #[test] + fn wallet_id_hex_is_fixed_width() { + let id = WalletId::from([0xAB; 32]); + assert_eq!(id.to_hex().len(), 64); + assert_eq!(id.as_bytes().len(), 32); + } +} From 183c9f303770e998085c7e7563e8611301a8f466 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:04:24 +0200 Subject: [PATCH 02/16] =?UTF-8?q?feat(platform-wallet-storage):=20Encrypte?= =?UTF-8?q?dFileStore=20=E2=80=94=20Argon2id=20+=20XChaCha20-Poly1305=20va?= =?UTF-8?q?ult?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group B Task 4. `secrets::file::{mod,format,crypto}`: - Argon2id KDF (`argon2 0.5.3`): floors m≥19456 KiB / t≥2 / p=1 enforced before any derivation; shipped default 64 MiB / t=3; params + 32-byte CSPRNG salt stored in the versioned header (SEC-REQ-2.2.1/.2/.3/.4). - XChaCha20-Poly1305 (`chacha20poly1305 0.10.1`): fresh random 24-byte nonce per `put` (counter forbidden); combined decrypt so no unverified plaintext is ever materialized (SEC-REQ-2.2.5/.6/.8). - AAD = canonical length-prefixed `format_version‖wallet_id‖label`, defeating blob-swap / version-rollback (SEC-REQ-2.2.7). - Self-describing magic+version header; unknown version refused, fail closed (SEC-REQ-2.2.9). - 0600 at creation via O_EXCL + fchmod before any ciphertext byte; pre-existing loose perms refused; atomic temp→fsync→rename→dir-fsync; temp holds only ciphertext, removed on failure (SEC-REQ-2.2.10/.11). - Atomic rekey: fresh salt + fresh per-entry nonces, no `.bak` (SEC-REQ-2.2.12). Passphrase held in `SecretString`, never persisted, zeroized on drop; derived key recomputed per op, never retained (SEC-REQ-2.2.13). Satisfies SEC-REQ 2.0.1, 2.0.2, 2.0.4, 2.2.1–2.2.13, 4.1. Co-Authored-By: Claudius the Magnificent (1M context) --- .../rs-platform-wallet-storage/src/lib.rs | 4 +- .../src/secrets/file/crypto.rs | 224 +++++++++ .../src/secrets/file/format.rs | 242 ++++++++++ .../src/secrets/file/mod.rs | 427 ++++++++++++++++++ .../src/secrets/mod.rs | 12 + 5 files changed, 907 insertions(+), 2 deletions(-) create mode 100644 packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/file/format.rs create mode 100644 packages/rs-platform-wallet-storage/src/secrets/file/mod.rs diff --git a/packages/rs-platform-wallet-storage/src/lib.rs b/packages/rs-platform-wallet-storage/src/lib.rs index 1a40d38588..ae40acbc90 100644 --- a/packages/rs-platform-wallet-storage/src/lib.rs +++ b/packages/rs-platform-wallet-storage/src/lib.rs @@ -67,8 +67,8 @@ const fn _secrets_send_sync_check() {} const _: () = { _secrets_send_sync_check::(); }; -#[cfg(all(feature = "secrets", any(test, feature = "__secrets-test-helpers")))] +#[cfg(feature = "secrets")] #[allow(dead_code)] -fn _secret_store_object_safety_check(store: secrets::MemoryStore) { +fn _secret_store_object_safety_check(store: secrets::EncryptedFileStore) { let _: std::sync::Arc = std::sync::Arc::new(store); } diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs new file mode 100644 index 0000000000..5858369b6b --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs @@ -0,0 +1,224 @@ +//! Argon2id KDF + XChaCha20-Poly1305 AEAD (SEC-REQ-2.2.1–2.2.8). +//! +//! `pub(crate)` only — no crypto primitive escapes the `secrets` tree. + +use argon2::{Algorithm, Argon2, Params, Version}; +use chacha20poly1305::aead::Aead; +use chacha20poly1305::{KeyInit, XChaCha20Poly1305, XNonce}; +use getrandom::getrandom; + +use super::super::error::SecretStoreError; +use super::super::secret::SecretBytes; + +/// Argon2 parameter floors (SEC-REQ-2.2.2) — derivation MUST NOT use +/// anything weaker; a header declaring less is refused. +pub(crate) const ARGON2_MIN_M_KIB: u32 = 19_456; +pub(crate) const ARGON2_MIN_T: u32 = 2; +pub(crate) const ARGON2_P: u32 = 1; + +/// Shipped defaults for new vaults (SEC-REQ-2.2.2 SHOULD target: +/// 64 MiB, t≥3). +pub(crate) const ARGON2_DEFAULT_M_KIB: u32 = 65_536; +pub(crate) const ARGON2_DEFAULT_T: u32 = 3; + +/// CSPRNG salt width (≥16 required; we use 32 — SEC-REQ-2.2.3). +pub(crate) const SALT_LEN: usize = 32; +/// XChaCha20-Poly1305 nonce width (SEC-REQ-2.2.6). +pub(crate) const NONCE_LEN: usize = 24; +/// Derived AEAD key width. +pub(crate) const KEY_LEN: usize = 32; + +/// Fill `buf` with CSPRNG bytes (`OsRng` via `getrandom`). +pub(crate) fn random_bytes(buf: &mut [u8]) -> Result<(), SecretStoreError> { + getrandom(buf).map_err(|_| SecretStoreError::KdfFailure) +} + +/// Argon2id parameters as stored in / read from a vault header. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct KdfParams { + pub m_kib: u32, + pub t: u32, + pub p: u32, +} + +impl KdfParams { + /// The shipped default for new vaults. + pub(crate) fn default_target() -> Self { + Self { + m_kib: ARGON2_DEFAULT_M_KIB, + t: ARGON2_DEFAULT_T, + p: ARGON2_P, + } + } + + /// Reject params below the floors (a downgraded header) before any + /// derivation runs (SEC-REQ-2.2.2). + pub(crate) fn enforce_floors(&self) -> Result<(), SecretStoreError> { + if self.m_kib < ARGON2_MIN_M_KIB || self.t < ARGON2_MIN_T || self.p != ARGON2_P { + return Err(SecretStoreError::KdfFailure); + } + Ok(()) + } +} + +/// Derive a 32-byte AEAD key from `passphrase` + `salt` with Argon2id. +/// Output lands directly in a [`SecretBytes`] (SEC-REQ-2.2.4). +pub(crate) fn derive_key( + passphrase: &[u8], + salt: &[u8], + params: KdfParams, +) -> Result { + params.enforce_floors()?; + let argon_params = Params::new(params.m_kib, params.t, params.p, Some(KEY_LEN)) + .map_err(|_| SecretStoreError::KdfFailure)?; + let argon = Argon2::new(Algorithm::Argon2id, Version::V0x13, argon_params); + let mut key = SecretBytes::zeroed(KEY_LEN); + argon + .hash_password_into(passphrase, salt, key.expose_secret_mut()) + .map_err(|_| SecretStoreError::KdfFailure)?; + Ok(key) +} + +/// Encrypt `plaintext` under `key` with a fresh random nonce, binding +/// `aad`. Returns `(nonce, ciphertext_with_tag)` (SEC-REQ-2.2.5/.6/.7). +pub(crate) fn seal( + key: &SecretBytes, + aad: &[u8], + plaintext: &[u8], +) -> Result<([u8; NONCE_LEN], Vec), SecretStoreError> { + let cipher = XChaCha20Poly1305::new_from_slice(key.expose_secret()) + .map_err(|_| SecretStoreError::KdfFailure)?; + let mut nonce_bytes = [0u8; NONCE_LEN]; + random_bytes(&mut nonce_bytes)?; + let nonce = XNonce::from_slice(&nonce_bytes); + let ct = cipher + .encrypt( + nonce, + chacha20poly1305::aead::Payload { + msg: plaintext, + aad, + }, + ) + .map_err(|_| SecretStoreError::Decrypt)?; + Ok((nonce_bytes, ct)) +} + +/// Decrypt `ciphertext` under `key`/`nonce`/`aad`. On tag failure +/// returns [`SecretStoreError::Decrypt`] and **no** plaintext — the +/// combined (non-detached) API never materializes unverified bytes at +/// our boundary (SEC-REQ-2.2.8, CWE-347, the RUSTSEC-2023-0096 lesson). +pub(crate) fn open( + key: &SecretBytes, + nonce: &[u8; NONCE_LEN], + aad: &[u8], + ciphertext: &[u8], +) -> Result { + let cipher = XChaCha20Poly1305::new_from_slice(key.expose_secret()) + .map_err(|_| SecretStoreError::KdfFailure)?; + let nonce = XNonce::from_slice(nonce); + let pt = cipher + .decrypt( + nonce, + chacha20poly1305::aead::Payload { + msg: ciphertext, + aad, + }, + ) + .map_err(|_| SecretStoreError::Decrypt)?; + Ok(SecretBytes::new(pt)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn floors_reject_weak_params() { + assert!(KdfParams { + m_kib: 1024, + t: 2, + p: 1 + } + .enforce_floors() + .is_err()); + assert!(KdfParams { + m_kib: ARGON2_MIN_M_KIB, + t: 1, + p: 1 + } + .enforce_floors() + .is_err()); + assert!(KdfParams { + m_kib: ARGON2_MIN_M_KIB, + t: 2, + p: 2 + } + .enforce_floors() + .is_err()); + assert!(KdfParams::default_target().enforce_floors().is_ok()); + } + + #[test] + fn seal_open_roundtrip_with_floor_params() { + // Floor params keep the test fast; production uses the default + // target (64 MiB) which is too slow for a unit test. + let params = KdfParams { + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + }; + let mut salt = [0u8; SALT_LEN]; + random_bytes(&mut salt).unwrap(); + let key = derive_key(b"correct horse", &salt, params).unwrap(); + let aad = b"v1|wallet|label"; + let (nonce, ct) = seal(&key, aad, b"top secret seed").unwrap(); + let pt = open(&key, &nonce, aad, &ct).unwrap(); + assert_eq!(pt.expose_secret(), b"top secret seed"); + } + + #[test] + fn wrong_aad_fails_with_no_plaintext() { + let params = KdfParams { + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + }; + let salt = [9u8; SALT_LEN]; + let key = derive_key(b"pw", &salt, params).unwrap(); + let (nonce, ct) = seal(&key, b"slot-A", b"seed").unwrap(); + let err = open(&key, &nonce, b"slot-B", &ct).unwrap_err(); + assert!(matches!(err, SecretStoreError::Decrypt)); + } + + #[test] + fn wrong_key_fails() { + let params = KdfParams { + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + }; + let salt = [1u8; SALT_LEN]; + let k1 = derive_key(b"right", &salt, params).unwrap(); + let k2 = derive_key(b"wrong", &salt, params).unwrap(); + let (nonce, ct) = seal(&k1, b"aad", b"seed").unwrap(); + assert!(matches!( + open(&k2, &nonce, b"aad", &ct), + Err(SecretStoreError::Decrypt) + )); + } + + #[test] + fn nonces_are_unique_across_seals() { + let params = KdfParams { + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + }; + let key = derive_key(b"pw", &[2u8; SALT_LEN], params).unwrap(); + let mut seen = std::collections::HashSet::new(); + for _ in 0..256 { + let (nonce, _) = seal(&key, b"aad", b"x").unwrap(); + assert!(seen.insert(nonce), "nonce reuse across put"); + } + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs new file mode 100644 index 0000000000..2f1d2bcd44 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs @@ -0,0 +1,242 @@ +//! Versioned, self-describing vault format + canonical AAD +//! (SEC-REQ-2.2.7 / 2.2.9). +//! +//! ```text +//! MAGIC 9 b"PWSVAULT1" +//! format_version u32 LE (= 1) +//! kdf_id u8 (1 = Argon2id) +//! m_kib u32 LE +//! t u32 LE +//! p u32 LE +//! salt_len u8 (= 32) +//! salt 32 +//! ── header ends ── +//! entries, each: label_len u16 LE | label | nonce 24 | ct_len u32 LE | ct+tag +//! ``` +//! +//! The whole file is one logical map for a single `wallet_id`; KDF +//! params/salt are therefore per-wallet. + +use super::super::error::SecretStoreError; +use super::crypto::{KdfParams, NONCE_LEN, SALT_LEN}; + +pub(crate) const MAGIC: &[u8; 9] = b"PWSVAULT1"; +pub(crate) const FORMAT_VERSION: u32 = 1; +pub(crate) const KDF_ID_ARGON2ID: u8 = 1; + +/// Parsed header (KDF params + salt). +#[derive(Debug, Clone)] +pub(crate) struct Header { + pub params: KdfParams, + pub salt: [u8; SALT_LEN], +} + +/// One decrypted-on-demand vault entry. +#[derive(Debug, Clone)] +pub(crate) struct Entry { + pub label: String, + pub nonce: [u8; NONCE_LEN], + pub ciphertext: Vec, +} + +/// Canonical length-prefixed AAD binding ciphertext to its slot +/// (SEC-REQ-2.2.7): `format_version ‖ wallet_id ‖ label`. A blob moved +/// to another slot, or a rolled-back `format_version`, fails the tag. +pub(crate) fn aad(format_version: u32, wallet_id: &[u8; 32], label: &str) -> Vec { + let lb = label.as_bytes(); + let mut v = Vec::with_capacity(4 + 4 + 32 + 4 + lb.len()); + v.extend_from_slice(&format_version.to_le_bytes()); + v.extend_from_slice(&(wallet_id.len() as u32).to_le_bytes()); + v.extend_from_slice(wallet_id); + v.extend_from_slice(&(lb.len() as u32).to_le_bytes()); + v.extend_from_slice(lb); + v +} + +/// Serialize a full vault (header + entries) to bytes. Contains only +/// salt/params (non-secret) + ciphertext — never plaintext. +pub(crate) fn serialize(header: &Header, entries: &[Entry]) -> Vec { + let mut out = Vec::new(); + out.extend_from_slice(MAGIC); + out.extend_from_slice(&FORMAT_VERSION.to_le_bytes()); + out.push(KDF_ID_ARGON2ID); + out.extend_from_slice(&header.params.m_kib.to_le_bytes()); + out.extend_from_slice(&header.params.t.to_le_bytes()); + out.extend_from_slice(&header.params.p.to_le_bytes()); + out.push(SALT_LEN as u8); + out.extend_from_slice(&header.salt); + for e in entries { + let lb = e.label.as_bytes(); + out.extend_from_slice(&(lb.len() as u16).to_le_bytes()); + out.extend_from_slice(lb); + out.extend_from_slice(&e.nonce); + out.extend_from_slice(&(e.ciphertext.len() as u32).to_le_bytes()); + out.extend_from_slice(&e.ciphertext); + } + out +} + +struct Reader<'a> { + buf: &'a [u8], + pos: usize, +} + +impl<'a> Reader<'a> { + fn take(&mut self, n: usize) -> Result<&'a [u8], SecretStoreError> { + let end = self + .pos + .checked_add(n) + .ok_or(SecretStoreError::MalformedVault)?; + let s = self + .buf + .get(self.pos..end) + .ok_or(SecretStoreError::MalformedVault)?; + self.pos = end; + Ok(s) + } + + fn u8(&mut self) -> Result { + Ok(self.take(1)?[0]) + } + + fn u16(&mut self) -> Result { + let b = self.take(2)?; + Ok(u16::from_le_bytes([b[0], b[1]])) + } + + fn u32(&mut self) -> Result { + let b = self.take(4)?; + Ok(u32::from_le_bytes([b[0], b[1], b[2], b[3]])) + } +} + +/// Parse a vault. Refuses unknown magic/version (fail closed, +/// SEC-REQ-2.2.9); parameter floors are enforced later at derive time. +pub(crate) fn deserialize(buf: &[u8]) -> Result<(Header, Vec), SecretStoreError> { + let mut r = Reader { buf, pos: 0 }; + if r.take(MAGIC.len())? != MAGIC { + return Err(SecretStoreError::MalformedVault); + } + let version = r.u32()?; + if version != FORMAT_VERSION { + return Err(SecretStoreError::VersionUnsupported { found: version }); + } + if r.u8()? != KDF_ID_ARGON2ID { + return Err(SecretStoreError::MalformedVault); + } + let m_kib = r.u32()?; + let t = r.u32()?; + let p = r.u32()?; + let salt_len = r.u8()? as usize; + if salt_len != SALT_LEN { + return Err(SecretStoreError::MalformedVault); + } + let mut salt = [0u8; SALT_LEN]; + salt.copy_from_slice(r.take(SALT_LEN)?); + + let mut entries = Vec::new(); + while r.pos < buf.len() { + let label_len = r.u16()? as usize; + let label = std::str::from_utf8(r.take(label_len)?) + .map_err(|_| SecretStoreError::MalformedVault)? + .to_string(); + let mut nonce = [0u8; NONCE_LEN]; + nonce.copy_from_slice(r.take(NONCE_LEN)?); + let ct_len = r.u32()? as usize; + let ciphertext = r.take(ct_len)?.to_vec(); + entries.push(Entry { + label, + nonce, + ciphertext, + }); + } + Ok(( + Header { + params: KdfParams { m_kib, t, p }, + salt, + }, + entries, + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn aad_binds_slot() { + let w = [1u8; 32]; + assert_ne!(aad(1, &w, "a"), aad(1, &w, "b")); + assert_ne!(aad(1, &w, "a"), aad(2, &w, "a")); + assert_ne!(aad(1, &w, "a"), aad(1, &[2u8; 32], "a")); + // Length-prefix defeats `"a"+"bc"` vs `"ab"+"c"` ambiguity. + assert_ne!(aad(1, &w, "ab"), { + let mut v = aad(1, &w, "a"); + v.extend_from_slice(b"b"); + v + }); + } + + #[test] + fn serialize_deserialize_roundtrip() { + let header = Header { + params: KdfParams::default_target(), + salt: [7u8; SALT_LEN], + }; + let entries = vec![ + Entry { + label: "bip39_mnemonic".into(), + nonce: [3u8; NONCE_LEN], + ciphertext: vec![1, 2, 3, 4], + }, + Entry { + label: "bip32-seed".into(), + nonce: [9u8; NONCE_LEN], + ciphertext: vec![5, 6], + }, + ]; + let bytes = serialize(&header, &entries); + let (h2, e2) = deserialize(&bytes).unwrap(); + assert_eq!(h2.params, header.params); + assert_eq!(h2.salt, header.salt); + assert_eq!(e2.len(), 2); + assert_eq!(e2[0].label, "bip39_mnemonic"); + assert_eq!(e2[1].ciphertext, vec![5, 6]); + } + + #[test] + fn rejects_bad_magic_and_unknown_version() { + assert!(matches!( + deserialize(b"NOPENOPE...."), + Err(SecretStoreError::MalformedVault) + )); + let mut bytes = serialize( + &Header { + params: KdfParams::default_target(), + salt: [0u8; SALT_LEN], + }, + &[], + ); + let v = MAGIC.len(); + bytes[v..v + 4].copy_from_slice(&999u32.to_le_bytes()); + assert!(matches!( + deserialize(&bytes), + Err(SecretStoreError::VersionUnsupported { found: 999 }) + )); + } + + #[test] + fn rejects_truncated() { + let bytes = serialize( + &Header { + params: KdfParams::default_target(), + salt: [0u8; SALT_LEN], + }, + &[], + ); + assert!(matches!( + deserialize(&bytes[..bytes.len() - 5]), + Err(SecretStoreError::MalformedVault) + )); + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs new file mode 100644 index 0000000000..c091c9ebe7 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -0,0 +1,427 @@ +//! [`EncryptedFileStore`] — passphrase-encrypted on-disk vault. +//! +//! One vault file per `wallet_id` (path namespaced by `wallet_id` +//! hex). Argon2id KDF + XChaCha20-Poly1305 AEAD, AAD-bound to +//! `(format_version, wallet_id, label)`, written atomically at mode +//! 0600. +//! +//! ## Threat coverage +//! +//! Covers **A1** (other local user), **A4** (lost laptop / cold +//! backup), **A6** (synced backup of the vault file): the at-rest file +//! is Argon2id + AEAD, useless without the passphrase. Does **not** +//! cover **A3** (passphrase / derived key resident while unlocked), a +//! weak operator passphrase (KDF raises cost, does not eliminate the +//! risk — accepted, AR-2), or **A5** if the derived key / plaintext is +//! swapped or core-dumped while unlocked (best-effort mitigated by +//! zeroize + mlock, not eliminated). + +mod crypto; +mod format; + +use std::fs::{self, OpenOptions}; +use std::io::Write; +use std::path::{Path, PathBuf}; + +use crypto::{KdfParams, SALT_LEN}; +use format::{Entry, Header}; + +use super::error::SecretStoreError; +use super::secret::{SecretBytes, SecretString}; +use super::store::SecretStore; +use super::validate::{validated_label, WalletId}; + +/// A passphrase-encrypted file-backed [`SecretStore`]. +/// +/// The passphrase is held in a [`SecretString`] for the store's +/// lifetime so each operation can re-derive the per-vault key; it is +/// never written anywhere and is zeroized when the store drops +/// (SEC-REQ-2.2.13). The derived AEAD key is recomputed per operation +/// and dropped (zeroized) immediately after use — it is never retained +/// on the struct. +pub struct EncryptedFileStore { + dir: PathBuf, + passphrase: SecretString, +} + +impl EncryptedFileStore { + /// Open (or prepare to create) a vault store rooted at `dir`, + /// unlocked by `passphrase`. `dir` is created if missing. + pub fn open(dir: impl AsRef, passphrase: SecretString) -> Result { + let dir = dir.as_ref().to_path_buf(); + fs::create_dir_all(&dir)?; + Ok(Self { dir, passphrase }) + } + + fn vault_path(&self, wallet_id: &WalletId) -> PathBuf { + self.dir.join(format!("{}.pwsvault", wallet_id.to_hex())) + } + + /// Read + parse a vault file, or `None` if it does not exist. + /// Refuses a pre-existing file with looser-than-0600 perms + /// (SEC-REQ-2.2.10). + fn read_vault(&self, path: &Path) -> Result)>, SecretStoreError> { + match fs::metadata(path) { + Ok(meta) => { + check_perms(&meta)?; + let bytes = fs::read(path)?; + Ok(Some(format::deserialize(&bytes)?)) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(e.into()), + } + } + + /// Atomically (temp → fsync → rename → dir-fsync) write the vault, + /// creating the temp at 0600 via `O_EXCL`+`fchmod` before any + /// ciphertext byte is written (SEC-REQ-2.2.10/.11). The temp holds + /// only ciphertext+header — never plaintext. + fn write_vault( + &self, + path: &Path, + header: &Header, + entries: &[Entry], + ) -> Result<(), SecretStoreError> { + let serialized = format::serialize(header, entries); + let tmp = path.with_extension("pwsvault.tmp"); + // Remove a stale temp so O_EXCL can take a clean lock. + let _ = fs::remove_file(&tmp); + let result = (|| -> Result<(), SecretStoreError> { + let mut opts = OpenOptions::new(); + opts.write(true).create_new(true); + set_create_mode(&mut opts); + let mut f = opts.open(&tmp)?; + enforce_mode_0600(&f)?; + f.write_all(&serialized)?; + f.sync_all()?; + fs::rename(&tmp, path)?; + if let Some(parent) = path.parent() { + if let Ok(d) = fs::File::open(parent) { + let _ = d.sync_all(); + } + } + Ok(()) + })(); + if result.is_err() { + let _ = fs::remove_file(&tmp); + } + result + } + + /// Re-encrypt every entry under a fresh salt + fresh per-entry + /// nonces with the current default Argon2 params and atomically + /// replace the vault — no `.bak` retains old key material + /// (SEC-REQ-2.2.12). + pub fn rekey( + &mut self, + wallet_id: WalletId, + new_passphrase: SecretString, + ) -> Result<(), SecretStoreError> { + let path = self.vault_path(&wallet_id); + let Some((old_header, old_entries)) = self.read_vault(&path)? else { + self.passphrase = new_passphrase; + return Ok(()); + }; + let old_key = crypto::derive_key( + self.passphrase.expose_secret().as_bytes(), + &old_header.salt, + old_header.params, + )?; + + let mut new_salt = [0u8; SALT_LEN]; + crypto::random_bytes(&mut new_salt)?; + let new_params = KdfParams::default_target(); + let new_key = crypto::derive_key( + new_passphrase.expose_secret().as_bytes(), + &new_salt, + new_params, + )?; + + let mut new_entries = Vec::with_capacity(old_entries.len()); + for e in &old_entries { + let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), &e.label); + let pt = crypto::open(&old_key, &e.nonce, &aad, &e.ciphertext) + .map_err(|_| SecretStoreError::WrongPassphrase)?; + let (nonce, ct) = crypto::seal(&new_key, &aad, pt.expose_secret())?; + new_entries.push(Entry { + label: e.label.clone(), + nonce, + ciphertext: ct, + }); + } + let new_header = Header { + params: new_params, + salt: new_salt, + }; + self.write_vault(&path, &new_header, &new_entries)?; + self.passphrase = new_passphrase; + Ok(()) + } +} + +impl SecretStore for EncryptedFileStore { + fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { + let label = validated_label(label)?.to_string(); + let path = self.vault_path(&wallet_id); + let (header, mut entries) = match self.read_vault(&path)? { + Some(v) => v, + None => { + let mut salt = [0u8; SALT_LEN]; + crypto::random_bytes(&mut salt)?; + ( + Header { + params: KdfParams::default_target(), + salt, + }, + Vec::new(), + ) + } + }; + let key = crypto::derive_key( + self.passphrase.expose_secret().as_bytes(), + &header.salt, + header.params, + )?; + let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), &label); + let (nonce, ciphertext) = crypto::seal(&key, &aad, bytes)?; + entries.retain(|e| e.label != label); + entries.push(Entry { + label, + nonce, + ciphertext, + }); + self.write_vault(&path, &header, &entries) + } + + fn get( + &self, + wallet_id: WalletId, + label: &str, + ) -> Result, SecretStoreError> { + let label = validated_label(label)?; + let path = self.vault_path(&wallet_id); + let Some((header, entries)) = self.read_vault(&path)? else { + return Ok(None); + }; + let Some(entry) = entries.iter().find(|e| e.label == label) else { + return Ok(None); + }; + let key = crypto::derive_key( + self.passphrase.expose_secret().as_bytes(), + &header.salt, + header.params, + )?; + let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), label); + match crypto::open(&key, &entry.nonce, &aad, &entry.ciphertext) { + Ok(pt) => Ok(Some(pt)), + Err(SecretStoreError::Decrypt) => Err(SecretStoreError::WrongPassphrase), + Err(e) => Err(e), + } + } + + fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { + let label = validated_label(label)?; + let path = self.vault_path(&wallet_id); + let Some((header, mut entries)) = self.read_vault(&path)? else { + return Ok(()); + }; + let before = entries.len(); + entries.retain(|e| e.label != label); + if entries.len() == before { + return Ok(()); + } + self.write_vault(&path, &header, &entries) + } +} + +#[cfg(unix)] +fn check_perms(meta: &fs::Metadata) -> Result<(), SecretStoreError> { + use std::os::unix::fs::MetadataExt; + let mode = meta.mode() & 0o777; + if mode & 0o077 != 0 { + return Err(SecretStoreError::InsecurePermissions { mode }); + } + Ok(()) +} + +#[cfg(not(unix))] +fn check_perms(_meta: &fs::Metadata) -> Result<(), SecretStoreError> { + Ok(()) +} + +#[cfg(unix)] +fn set_create_mode(opts: &mut OpenOptions) { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); +} + +#[cfg(not(unix))] +fn set_create_mode(_opts: &mut OpenOptions) {} + +#[cfg(unix)] +fn enforce_mode_0600(f: &fs::File) -> Result<(), SecretStoreError> { + use std::os::unix::fs::PermissionsExt; + f.set_permissions(fs::Permissions::from_mode(0o600))?; + Ok(()) +} + +#[cfg(not(unix))] +fn enforce_mode_0600(_f: &fs::File) -> Result<(), SecretStoreError> { + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn store(dir: &Path) -> EncryptedFileStore { + EncryptedFileStore::open(dir, SecretString::new("pw-correct")).unwrap() + } + + fn wid(b: u8) -> WalletId { + WalletId::from([b; 32]) + } + + #[test] + fn roundtrip_persists_across_reopen() { + let dir = tempfile::tempdir().unwrap(); + { + let s = store(dir.path()); + s.put(wid(1), "bip39_mnemonic", b"abandon abandon").unwrap(); + } + let s2 = store(dir.path()); + let got = s2.get(wid(1), "bip39_mnemonic").unwrap().unwrap(); + assert_eq!(got.expose_secret(), b"abandon abandon"); + assert!(s2.get(wid(1), "missing").unwrap().is_none()); + } + + #[test] + fn wrong_passphrase_fails_no_plaintext() { + let dir = tempfile::tempdir().unwrap(); + store(dir.path()) + .put(wid(1), "seed", b"super secret") + .unwrap(); + let bad = EncryptedFileStore::open(dir.path(), SecretString::new("pw-wrong")).unwrap(); + let err = bad.get(wid(1), "seed").unwrap_err(); + assert!(matches!(err, SecretStoreError::WrongPassphrase)); + // The error renders without any plaintext. + assert!(!format!("{err}").contains("super secret")); + } + + #[test] + fn idempotent_delete_and_overwrite() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + s.delete(wid(1), "seed").unwrap(); // no vault yet + s.put(wid(1), "seed", b"v1").unwrap(); + s.put(wid(1), "seed", b"v2").unwrap(); + assert_eq!( + s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), + b"v2" + ); + s.delete(wid(1), "seed").unwrap(); + s.delete(wid(1), "seed").unwrap(); // idempotent + assert!(s.get(wid(1), "seed").unwrap().is_none()); + } + + #[test] + fn blob_swap_across_label_is_rejected() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + s.put(wid(1), "labelA", b"secretA").unwrap(); + s.put(wid(1), "labelB", b"secretB").unwrap(); + let path = s.vault_path(&wid(1)); + let (header, mut entries) = s.read_vault(&path).unwrap().unwrap(); + // Move A's ciphertext+nonce into B's slot. + let a = entries + .iter() + .find(|e| e.label == "labelA") + .unwrap() + .clone(); + for e in entries.iter_mut() { + if e.label == "labelB" { + e.nonce = a.nonce; + e.ciphertext = a.ciphertext.clone(); + } + } + s.write_vault(&path, &header, &entries).unwrap(); + assert!(matches!( + s.get(wid(1), "labelB"), + Err(SecretStoreError::WrongPassphrase) | Err(SecretStoreError::Decrypt) + )); + } + + #[cfg(unix)] + #[test] + fn vault_created_0600() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + s.put(wid(1), "seed", b"x").unwrap(); + let mode = fs::metadata(s.vault_path(&wid(1))) + .unwrap() + .permissions() + .mode() + & 0o777; + assert_eq!(mode, 0o600); + } + + #[cfg(unix)] + #[test] + fn loose_perms_preexisting_file_refused() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + s.put(wid(1), "seed", b"x").unwrap(); + let path = s.vault_path(&wid(1)); + fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); + assert!(matches!( + s.get(wid(1), "seed"), + Err(SecretStoreError::InsecurePermissions { mode: 0o644 }) + )); + } + + #[test] + fn rekey_reencrypts_and_old_passphrase_fails() { + let dir = tempfile::tempdir().unwrap(); + let mut s = store(dir.path()); + s.put(wid(1), "seed", b"value").unwrap(); + let old_bytes = fs::read(s.vault_path(&wid(1))).unwrap(); + s.rekey(wid(1), SecretString::new("pw-new")).unwrap(); + // New passphrase reads; ciphertext changed; no .bak left. + assert_eq!( + s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), + b"value" + ); + let new_bytes = fs::read(s.vault_path(&wid(1))).unwrap(); + assert_ne!(old_bytes, new_bytes); + let stale: Vec<_> = fs::read_dir(dir.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| { + let n = e.file_name(); + let n = n.to_string_lossy(); + n.ends_with(".bak") || n.ends_with(".tmp") + }) + .collect(); + assert!(stale.is_empty(), "rekey left stale files: {stale:?}"); + let old = EncryptedFileStore::open(dir.path(), SecretString::new("pw-correct")).unwrap(); + assert!(matches!( + old.get(wid(1), "seed"), + Err(SecretStoreError::WrongPassphrase) + )); + } + + #[test] + fn no_plaintext_in_vault_file() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + s.put(wid(1), "seed", b"PLAINTEXTNEEDLE").unwrap(); + let raw = fs::read(s.vault_path(&wid(1))).unwrap(); + assert!( + raw.windows(b"PLAINTEXTNEEDLE".len()) + .all(|w| w != b"PLAINTEXTNEEDLE"), + "plaintext leaked into vault file" + ); + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index 5c768f478c..0168c2fa17 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -7,6 +7,16 @@ //! `migrations/` and exempts this module, so this module owns its own //! review discipline (`tests/secrets_guard.rs`, SEC-REQ-4.5/4.5.1). //! +//! # Backends & selection +//! +//! [`EncryptedFileStore`] (Argon2id + XChaCha20-Poly1305 vault file) is +//! fully self-contained — the recommended default on **headless / +//! server** hosts. The OS-keyring backend (recommended on desktop) +//! lands alongside it; **backend selection is an explicit operator +//! decision — there is no silent fallback between backends** +//! (SEC-REQ-2.1.3 / AR-4). [`MemoryStore`] is test-only and gated so it +//! is unreachable from production builds. +//! //! # Memory hygiene //! //! Secrets cross every boundary inside [`SecretBytes`] / [`SecretString`] @@ -15,6 +25,7 @@ //! any variant. mod error; +mod file; mod secret; mod store; mod validate; @@ -23,6 +34,7 @@ mod validate; mod memory; pub use error::SecretStoreError; +pub use file::EncryptedFileStore; pub use secret::{SecretBytes, SecretString}; pub use store::SecretStore; pub use validate::WalletId; From bfbb55177353efab5000330da73099f14b1ee518 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:05:38 +0200 Subject: [PATCH 03/16] =?UTF-8?q?feat(platform-wallet-storage):=20KeyringS?= =?UTF-8?q?tore=20=E2=80=94=20OS=20keyring=20backend=20(keyring-core=204.x?= =?UTF-8?q?=20split)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group B Task 5. `secrets::keyring::KeyringStore` over the keyring 4.x split: `keyring-core 1.0.0` API + per-platform store crates (linux-keyutils / dbus-secret-service / apple-native / windows-native), all exact-pinned, RustSec-clean, MSRV-1.92-verified. - Namespacing: service `dash.platform-wallet-storage`, account `{wallet_id_hex}:{label}` — two wallets cannot collide, a different app cannot silently read; only the non-secret index appears in keyring attributes (SEC-REQ-2.1.2, CWE-312). - Fail-closed: headless / no Secret Service / no D-Bus → typed `BackendUnavailable`; locked → typed error. Never `unwrap`, never a silent plaintext / weaker-store fallback (SEC-REQ-2.1.3/.4 / AR-4). - keyring-core's bare `Vec` from `get_secret` is wrapped into `SecretBytes` and the intermediate zeroized immediately (SEC-REQ-3.1/4.1). - Per-OS threat-coverage rustdoc on the type (SEC-REQ-2.0.4 / 2.1.3). Backend selection is an explicit operator decision — no auto-fallback between KeyringStore and EncryptedFileStore (SEC-REQ-2.1.3 / AR-4). Satisfies SEC-REQ 2.0.1, 2.0.4, 2.1.1, 2.1.2, 2.1.3, 2.1.4. Co-Authored-By: Claudius the Magnificent (1M context) --- .../src/secrets/keyring.rs | 205 ++++++++++++++++++ .../src/secrets/mod.rs | 23 +- 2 files changed, 221 insertions(+), 7 deletions(-) create mode 100644 packages/rs-platform-wallet-storage/src/secrets/keyring.rs diff --git a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs new file mode 100644 index 0000000000..efa500ac35 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs @@ -0,0 +1,205 @@ +//! [`KeyringStore`] — OS keyring backend (keyring-core 4.x split). +//! +//! Delegates at-rest protection to the OS credential store. Its +//! security *is* the OS keyring's security. +//! +//! ## Threat coverage +//! +//! Covers **A1** (other local user) and **A4** (lost laptop) where the +//! platform encrypts keyring items at rest and scopes them to the user. +//! Does **not** cover **A2/A3** same-user malware (most OS keyrings +//! hand the secret to any same-user process that asks), **A5** if the +//! keyring daemon itself is scraped, or **headless Linux** with no +//! Secret Service — that fails closed (`BackendUnavailable`), never +//! degrades to plaintext. +//! +//! ### Per-OS reality +//! +//! - **Linux/FreeBSD:** Secret Service (gnome-keyring / KWallet) needs +//! a D-Bus session + unlocked collection. Headless / SSH / CI boxes +//! frequently lack it → fail closed; the operator selects +//! [`EncryptedFileStore`](super::EncryptedFileStore) explicitly. +//! - **macOS:** Keychain ACL — a re-signed binary with the same +//! code-signing identity is A3 (accepted, AR-5). +//! - **Windows:** Credential Manager / DPAPI is user-profile scoped; a +//! same-user process can unprotect it. DPAPI is **not** a defense +//! against same-user malware, only A1/A4. + +use std::sync::Arc; + +use keyring_core::api::CredentialStore; +use keyring_core::{Entry, Error as KeyringError}; + +use super::error::SecretStoreError; +use super::secret::SecretBytes; +use super::store::SecretStore; +use super::validate::{validated_label, WalletId}; + +/// Keyring `service` namespace — application-scoped so a different app +/// cannot silently read the entry (SEC-REQ-2.1.2). +const SERVICE: &str = "dash.platform-wallet-storage"; + +/// An OS-keyring-backed [`SecretStore`]. +/// +/// The `account` is `"{wallet_id_hex}:{label}"`, so two wallets cannot +/// collide. Only that non-secret index appears in keyring attributes — +/// never a secret byte (SEC-REQ-2.1.2, CWE-312). +pub struct KeyringStore { + store: Arc, +} + +impl KeyringStore { + /// Open the platform's default credential store, failing closed + /// (typed [`SecretStoreError::BackendUnavailable`]) when none is + /// reachable (headless / no Secret Service / no D-Bus). Never + /// panics, never falls back to a weaker store (SEC-REQ-2.1.3). + pub fn new() -> Result { + let store = default_store()?; + Ok(Self { store }) + } + + fn entry(&self, wallet_id: &WalletId, label: &str) -> Result { + let account = format!("{}:{}", wallet_id.to_hex(), label); + self.store + .build(SERVICE, &account, None) + .map_err(map_keyring_err) + } +} + +#[cfg(any(target_os = "linux", target_os = "freebsd"))] +fn default_store() -> Result, SecretStoreError> { + // Prefer the kernel keyutils store; fall back to Secret Service. + // Both failing (headless, no session keyring, no D-Bus) is + // fail-closed by design (SEC-REQ-2.1.3 / AR-4). + if let Ok(s) = linux_keyutils_keyring_store::Store::new() { + return Ok(s as Arc); + } + dbus_secret_service_keyring_store::Store::new() + .map(|s| s as Arc) + .map_err(map_keyring_err) +} + +#[cfg(target_os = "macos")] +fn default_store() -> Result, SecretStoreError> { + apple_native_keyring_store::Store::new() + .map(|s| s as Arc) + .map_err(map_keyring_err) +} + +#[cfg(target_os = "windows")] +fn default_store() -> Result, SecretStoreError> { + windows_native_keyring_store::Store::new() + .map(|s| s as Arc) + .map_err(map_keyring_err) +} + +#[cfg(not(any( + target_os = "linux", + target_os = "freebsd", + target_os = "macos", + target_os = "windows" +)))] +fn default_store() -> Result, SecretStoreError> { + Err(SecretStoreError::BackendUnavailable) +} + +/// Map keyring-core errors to the typed taxonomy. `NoEntry` is *not* +/// mapped here — callers translate it to `Ok(None)`/`Ok(())`. No +/// keyring error string is embedded (it could echo the `account`, +/// which is non-secret, but the taxonomy stays clean — SEC-REQ-2.0.1). +fn map_keyring_err(e: KeyringError) -> SecretStoreError { + match e { + KeyringError::NoEntry => SecretStoreError::NotFound, + KeyringError::NoStorageAccess(_) + | KeyringError::NoDefaultStore + | KeyringError::PlatformFailure(_) => SecretStoreError::BackendUnavailable, + _ => SecretStoreError::BackendUnavailable, + } +} + +impl SecretStore for KeyringStore { + fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { + let label = validated_label(label)?; + let entry = self.entry(&wallet_id, label)?; + entry.set_secret(bytes).map_err(map_keyring_err) + } + + fn get( + &self, + wallet_id: WalletId, + label: &str, + ) -> Result, SecretStoreError> { + let label = validated_label(label)?; + let entry = self.entry(&wallet_id, label)?; + match entry.get_secret() { + Ok(mut v) => { + let secret = SecretBytes::from_slice(&v); + // keyring-core returns a bare `Vec`; wipe the + // intermediate now that it is wrapped (SEC-REQ-3.1). + use zeroize::Zeroize; + v.zeroize(); + Ok(Some(secret)) + } + Err(KeyringError::NoEntry) => Ok(None), + Err(e) => Err(map_keyring_err(e)), + } + } + + fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { + let label = validated_label(label)?; + let entry = self.entry(&wallet_id, label)?; + match entry.delete_credential() { + Ok(()) | Err(KeyringError::NoEntry) => Ok(()), + Err(e) => Err(map_keyring_err(e)), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn invalid_label_rejected_before_backend() { + // Label validation must precede any keyring access, so this + // is deterministic even on headless CI with no keyring. + if let Ok(s) = KeyringStore::new() { + assert!(matches!( + s.put(WalletId::from([0; 32]), "../escape", b"x"), + Err(SecretStoreError::InvalidLabel) + )); + } + } + + #[test] + fn headless_fails_closed_not_panic() { + // On headless CI `new()` returns `BackendUnavailable`; where a + // keyring exists it succeeds. Either way: typed, no panic, no + // plaintext fallback. + match KeyringStore::new() { + Ok(_) | Err(SecretStoreError::BackendUnavailable) => {} + Err(other) => panic!("unexpected: {other}"), + } + } + + /// Round-trip needs a live keyring; `#[ignore]` so headless CI does + /// not fail. Run locally on a desktop with an unlocked keyring: + /// `cargo test --features secrets keyring_roundtrip -- --ignored` + #[test] + #[ignore] + fn keyring_roundtrip_and_namespacing() { + let s = KeyringStore::new().expect("keyring available"); + let w1 = WalletId::from([1; 32]); + let w2 = WalletId::from([2; 32]); + s.put(w1, "seed", b"alpha").unwrap(); + s.put(w2, "seed", b"beta").unwrap(); + assert_eq!( + s.get(w1, "seed").unwrap().unwrap().expose_secret(), + b"alpha" + ); + assert_eq!(s.get(w2, "seed").unwrap().unwrap().expose_secret(), b"beta"); + s.delete(w1, "seed").unwrap(); + s.delete(w2, "seed").unwrap(); + assert!(s.get(w1, "seed").unwrap().is_none()); + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index 0168c2fa17..202e3be4dd 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -9,13 +9,20 @@ //! //! # Backends & selection //! -//! [`EncryptedFileStore`] (Argon2id + XChaCha20-Poly1305 vault file) is -//! fully self-contained — the recommended default on **headless / -//! server** hosts. The OS-keyring backend (recommended on desktop) -//! lands alongside it; **backend selection is an explicit operator -//! decision — there is no silent fallback between backends** -//! (SEC-REQ-2.1.3 / AR-4). [`MemoryStore`] is test-only and gated so it -//! is unreachable from production builds. +//! Two production backends ship; **selection is an explicit operator +//! decision — there is no silent fallback between them** (SEC-REQ-2.1.3 +//! / AR-4): +//! +//! - [`KeyringStore`] — OS keyring. Recommended default on **desktop** +//! OSes. Fails closed on headless Linux (no Secret Service) with a +//! typed [`SecretStoreError::BackendUnavailable`], never a degraded +//! plaintext store. +//! - [`EncryptedFileStore`] — Argon2id + XChaCha20-Poly1305 vault file. +//! Recommended default on **headless / server** hosts; fully +//! self-contained, no environment caveat. +//! +//! [`MemoryStore`] is test-only and gated so it is unreachable from +//! production builds. //! //! # Memory hygiene //! @@ -26,6 +33,7 @@ mod error; mod file; +mod keyring; mod secret; mod store; mod validate; @@ -35,6 +43,7 @@ mod memory; pub use error::SecretStoreError; pub use file::EncryptedFileStore; +pub use keyring::KeyringStore; pub use secret::{SecretBytes, SecretString}; pub use store::SecretStore; pub use validate::WalletId; From e3ac1a6f6f5c0f5d21e15187db5e06840b730dae Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:06:01 +0200 Subject: [PATCH 04/16] test(platform-wallet-storage): positive secrets guard + API-shape integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group B Task 6. `tests/secrets_guard.rs` (SEC-REQ-4.5.1): positive string-level scan of `src/secrets/` asserting no logging/formatting sink (`tracing::*`/`println!`/`format!`/`panic!`/…) is paired with an `expose_secret()` result — the guard `tests/secrets_scan.rs` deliberately does NOT cover this tree. Green on the clean tree; fails the moment a secret is routed to a sink. `tests/secrets_api.rs`: `get` returns `Option` (type binding, never `Vec` — SEC-REQ-4.1); `dyn SecretStore` object-safety / positive build guard (SEC-REQ-4.5); no boxed dyn error in `src/secrets/` (TC-082 parity, comment-aware); error `Display` is static and secret-free (SEC-REQ-2.0.1/3.3, CWE-209); wrapper `Debug` redacted at the boundary (SEC-REQ-3.3). `MemoryStore` intentionally unreachable from this external test crate (SEC-REQ-2.3.1). Satisfies SEC-REQ 4.5, 4.5.1. Co-Authored-By: Claudius the Magnificent (1M context) --- .../tests/secrets_api.rs | 118 ++++++++++++++++++ .../tests/secrets_guard.rs | 96 ++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 packages/rs-platform-wallet-storage/tests/secrets_api.rs create mode 100644 packages/rs-platform-wallet-storage/tests/secrets_guard.rs diff --git a/packages/rs-platform-wallet-storage/tests/secrets_api.rs b/packages/rs-platform-wallet-storage/tests/secrets_api.rs new file mode 100644 index 0000000000..509114621e --- /dev/null +++ b/packages/rs-platform-wallet-storage/tests/secrets_api.rs @@ -0,0 +1,118 @@ +//! Type-shape + boundary guards for the `secrets` API +//! (SEC-REQ-4.1 / 4.4 / 4.5, TC-082 parity). +//! +//! Compiled only with `--features secrets`. Uses `EncryptedFileStore` +//! (always available under `secrets`); `MemoryStore` is intentionally +//! unreachable here (SEC-REQ-2.3.1) — it is exercised only by the +//! crate's in-module unit tests. + +#![cfg(feature = "secrets")] + +use std::path::Path; + +use platform_wallet_storage::secrets::{ + EncryptedFileStore, SecretBytes, SecretStore, SecretStoreError, SecretString, WalletId, +}; + +fn open(dir: &Path) -> EncryptedFileStore { + EncryptedFileStore::open(dir, SecretString::new("test-pass")).unwrap() +} + +/// `SecretStore::get` returns `Option`, never a bare +/// `Vec` (SEC-REQ-4.1). This binding only compiles if the type is +/// exactly that. +#[test] +fn get_returns_zeroizing_wrapper_not_vec() { + let dir = tempfile::tempdir().unwrap(); + let s = open(dir.path()); + let w = WalletId::from([1; 32]); + s.put(w, "seed", b"abc").unwrap(); + let got: Option = s.get(w, "seed").unwrap(); + assert_eq!(got.unwrap().expose_secret(), b"abc"); +} + +/// The secrets module is reachable, compiles, and round-trips through +/// `dyn SecretStore` (SEC-REQ-4.5 positive build guard). +#[test] +fn secrets_tree_builds_and_is_object_safe() { + let dir = tempfile::tempdir().unwrap(); + let s: std::sync::Arc = std::sync::Arc::new(open(dir.path())); + let w = WalletId::from([9; 32]); + s.put(w, "bip39_mnemonic", b"x").unwrap(); + assert!(s.get(w, "bip39_mnemonic").unwrap().is_some()); +} + +/// No `Box` in the `secrets` tree's public surface — TC-082 +/// parity for the module the schema scanner does not cover. +#[test] +fn no_box_dyn_error_in_secrets_src() { + let dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/secrets"); + let mut offenders = Vec::new(); + walk(&dir, &mut offenders); + assert!( + offenders.is_empty(), + "Box found in secrets src:\n{}", + offenders.join("\n") + ); + + fn walk(dir: &Path, out: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for e in entries.flatten() { + let p = e.path(); + if p.is_dir() { + walk(&p, out); + continue; + } + if p.extension().and_then(|x| x.to_str()) != Some("rs") { + continue; + } + let Ok(body) = std::fs::read_to_string(&p) else { + continue; + }; + for (i, line) in body.lines().enumerate() { + // The rule bans the *type* in code; prose explaining + // the rule (doc/line comments) is not a violation. + let trimmed = line.trim_start(); + if trimmed.starts_with("//") || trimmed.starts_with("*") { + continue; + } + let s = line.replace(' ', ""); + if s.contains("Box) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let p = entry.path(); + if p.is_dir() { + scan(&p, offenders); + continue; + } + if p.extension().and_then(|e| e.to_str()) != Some("rs") { + continue; + } + let Ok(body) = std::fs::read_to_string(&p) else { + continue; + }; + // Join continuations: a leaking call may wrap across lines. + for (idx, window) in body.lines().collect::>().windows(2).enumerate() { + let joined = format!("{} {}", window[0], window[1]); + if !joined.contains("expose_secret") { + continue; + } + // The `expose_secret` definitions/doc lines in `secret.rs` + // and intentional debug-redaction tests are not sinks. + if window.iter().any(|l| { + let t = l.trim_start(); + t.starts_with("//") || t.starts_with("///") || t.starts_with("*") + }) && !SINKS.iter().any(|s| joined.contains(s)) + { + continue; + } + for sink in SINKS { + if joined.contains(sink) && joined.contains("expose_secret") { + offenders.push(format!( + "{}:{}: `{sink}` paired with `expose_secret` — {}", + p.display(), + idx + 1, + window[0].trim() + )); + } + } + } + } +} + +#[test] +fn no_secret_sink_in_secrets_module() { + let manifest = Path::new(env!("CARGO_MANIFEST_DIR")); + let mut offenders = Vec::new(); + scan(&manifest.join("src/secrets"), &mut offenders); + assert!( + offenders.is_empty(), + "secret material may be reaching a log/format sink:\n{}", + offenders.join("\n") + ); +} From 029753f44da81c76f059f0272dcca830d3bd5834 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:06:13 +0200 Subject: [PATCH 05/16] ci(platform-wallet-storage): cargo-deny advisories gate covering the secrets crypto deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group B Task 8 (SEC-REQ-4.7). The existing `rustsec/audit-check` already audits the full `Cargo.lock` — which now pins the `secrets`-gated crypto (argon2/chacha20poly1305/zeroize/subtle/region/ keyring-core + per-platform stores), so they are advisory-checked even though `default` does not enable `secrets`. This adds a `cargo-deny check advisories --all-features` job so the feature-conditional dependency graph is exercised explicitly, plus a workspace `deny.toml` (advisory ignore kept in sync with `.cargo/audit.toml`). Locally verified: `cargo audit` exits 0; none of the secrets crypto pins carry any RustSec advisory (confirms Smythe §7 first-hand). The only flagged item, RUSTSEC-2025-0141 (bincode unmaintained), is a pre-existing unrelated wasm-sdk/dpp dependency, not in the secrets path. Satisfies SEC-REQ 4.7. Co-Authored-By: Claudius the Magnificent (1M context) --- .github/workflows/security-audit-rust.yml | 19 +++++++++++ deny.toml | 39 +++++++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 deny.toml diff --git a/.github/workflows/security-audit-rust.yml b/.github/workflows/security-audit-rust.yml index 518fa4ae06..00e44ce4fe 100644 --- a/.github/workflows/security-audit-rust.yml +++ b/.github/workflows/security-audit-rust.yml @@ -17,3 +17,22 @@ jobs: uses: rustsec/audit-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} + + rs-crates-deny-advisories: + name: cargo-deny advisories (incl. secrets feature) + runs-on: ubuntu-24.04 + steps: + - name: Check out repo + uses: actions/checkout@v4 + + # `cargo audit` reads `Cargo.lock`, which already contains the + # `secrets`-gated crypto pins; `cargo deny` with `--all-features` + # additionally exercises the feature-conditional dependency graph + # so `platform-wallet-storage`'s `secrets` deps are advisory- + # checked even though `default` does not enable them + # (SEC-REQ-4.7). + - name: cargo-deny advisories + uses: EmbarkStudios/cargo-deny-action@v2 + with: + command: check advisories + arguments: --all-features diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000000..2b826cb899 --- /dev/null +++ b/deny.toml @@ -0,0 +1,39 @@ +# cargo-deny configuration — advisory gate for the workspace, +# including the `secrets`-gated crypto dependencies of +# `platform-wallet-storage` (SEC-REQ-4.7). +# +# `cargo deny` resolves the full `Cargo.lock`, so the pinned +# `argon2` / `chacha20poly1305` / `zeroize` / `subtle` / `region` / +# `keyring-core` + per-platform store crates are in scope regardless of +# which features a given build enables. The CI invocation additionally +# passes `--all-features` so feature-gated graphs are exercised too. + +[advisories] +version = 2 +# Keep in sync with `.cargo/audit.toml`. +ignore = ["RUSTSEC-2020-0071"] + +[bans] +multiple-versions = "allow" + +[licenses] +version = 2 +allow = [ + "MIT", + "Apache-2.0", + "Apache-2.0 WITH LLVM-exception", + "BSD-2-Clause", + "BSD-3-Clause", + "ISC", + "Unicode-DFS-2016", + "Unicode-3.0", + "Zlib", + "MPL-2.0", + "CC0-1.0", + "0BSD", +] +confidence-threshold = 0.8 + +[sources] +unknown-registry = "deny" +unknown-git = "allow" From 1c55f892d86d990881875111d1b15ba0241527a2 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:32:19 +0200 Subject: [PATCH 06/16] fix(platform-wallet-storage): passphrase-verification token + hardened atomic vault write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1 (HIGH, Marvin QA-001): a `put`/`get`/`delete`/`rekey` against an EXISTING vault with a passphrase deriving a DIFFERENT key than the vault was created with previously wrote a mismatched-key entry and returned Ok, producing an unreadable mixed-key vault. The header now carries a passphrase-verification token: an XChaCha20-Poly1305 seal of a fixed constant under the header-Argon2id-derived key, AAD-bound to `(format_version, wallet_id, "\0verify")` (the leading-NUL label is disjoint from every allowlisted entry label, so the token can never alias a real slot). Every operation on an existing vault derives the key from the supplied passphrase and verifies the token FIRST; a mismatch fails the Poly1305 tag (constant-time, no extra compare, no plaintext on failure) and returns `SecretStoreError::WrongPassphrase` before any entry is read, written, or deleted. New vaults write the token at creation; `rekey` verifies the old token and writes a fresh one. `format_version` bumped 1→2; v1/v2 cross-reads fail closed via the existing `VersionUnsupported` path. C6 (LOW, Smythe SEC-RA-001): `write_vault` no longer swallows the directory-fsync result — it is propagated as a typed error so the atomic temp→fsync→rename→dir-fsync chain (SEC-REQ-2.2.11) is fully enforced. C7 (LOW, Marvin QA-004): the temp file now uses a unique name (`pid` + monotonic counter) created with `O_EXCL` and the destination is never pre-removed, so a crash can never leave the vault absent and concurrent writers cannot collide on a fixed temp name. The atomic rename + fsync ordering is unchanged. Tests (red→green, file/mod.rs): wrong-pass `put` to existing vault ⇒ `Err(WrongPassphrase)` + vault still readable with the correct pass + rejected slot never written; wrong-pass `get`/`delete` ⇒ `Err(WrongPassphrase)` + vault unmutated; correct pass round-trips unchanged. The two wrong-pass tests were FAILED before this fix and pass after; format (de)serialize round-trips the token fields. Co-Authored-By: Claudius the Magnificent (1M context) --- .../src/secrets/file/format.rs | 75 +++++--- .../src/secrets/file/mod.rs | 178 +++++++++++++----- 2 files changed, 185 insertions(+), 68 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs index 2f1d2bcd44..8dfaaacd7d 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs @@ -3,32 +3,50 @@ //! //! ```text //! MAGIC 9 b"PWSVAULT1" -//! format_version u32 LE (= 1) +//! format_version u32 LE (= 2) //! kdf_id u8 (1 = Argon2id) //! m_kib u32 LE //! t u32 LE //! p u32 LE //! salt_len u8 (= 32) //! salt 32 +//! verify_nonce 24 XNonce for the passphrase-verification token +//! verify_ct_len u32 LE +//! verify_ct AEAD(VERIFY_CONSTANT) under the header key //! ── header ends ── //! entries, each: label_len u16 LE | label | nonce 24 | ct_len u32 LE | ct+tag //! ``` //! //! The whole file is one logical map for a single `wallet_id`; KDF -//! params/salt are therefore per-wallet. +//! params/salt are therefore per-wallet. `verify_ct` is an AEAD seal of +//! a fixed constant under the header-derived key — a wrong passphrase +//! fails its tag, so a mismatched key is rejected before any entry is +//! written or read (no mixed-key corruption). use super::super::error::SecretStoreError; use super::crypto::{KdfParams, NONCE_LEN, SALT_LEN}; pub(crate) const MAGIC: &[u8; 9] = b"PWSVAULT1"; -pub(crate) const FORMAT_VERSION: u32 = 1; +pub(crate) const FORMAT_VERSION: u32 = 2; pub(crate) const KDF_ID_ARGON2ID: u8 = 1; -/// Parsed header (KDF params + salt). +/// Fixed plaintext sealed under the header key to form the passphrase- +/// verification token. Its only purpose is the AEAD tag check; the +/// value itself is not secret. +pub(crate) const VERIFY_CONSTANT: &[u8] = b"PWSVAULT-VERIFY-v1"; + +/// AAD slot label for the verification token. The leading NUL keeps it +/// disjoint from every allowlisted entry label (SEC-REQ-4.3), so the +/// token can never alias a real entry's AAD. +pub(crate) const VERIFY_LABEL: &str = "\0verify"; + +/// Parsed header (KDF params + salt + passphrase-verification token). #[derive(Debug, Clone)] pub(crate) struct Header { pub params: KdfParams, pub salt: [u8; SALT_LEN], + pub verify_nonce: [u8; NONCE_LEN], + pub verify_ct: Vec, } /// One decrypted-on-demand vault entry. @@ -53,6 +71,14 @@ pub(crate) fn aad(format_version: u32, wallet_id: &[u8; 32], label: &str) -> Vec v } +/// AAD for the passphrase-verification token — the same canonical +/// construction as entry AAD but bound to [`VERIFY_LABEL`], so the +/// token is cryptographically tied to this `(version, wallet_id)` and +/// cannot be replayed into an entry slot. +pub(crate) fn verify_aad(format_version: u32, wallet_id: &[u8; 32]) -> Vec { + aad(format_version, wallet_id, VERIFY_LABEL) +} + /// Serialize a full vault (header + entries) to bytes. Contains only /// salt/params (non-secret) + ciphertext — never plaintext. pub(crate) fn serialize(header: &Header, entries: &[Entry]) -> Vec { @@ -65,6 +91,9 @@ pub(crate) fn serialize(header: &Header, entries: &[Entry]) -> Vec { out.extend_from_slice(&header.params.p.to_le_bytes()); out.push(SALT_LEN as u8); out.extend_from_slice(&header.salt); + out.extend_from_slice(&header.verify_nonce); + out.extend_from_slice(&(header.verify_ct.len() as u32).to_le_bytes()); + out.extend_from_slice(&header.verify_ct); for e in entries { let lb = e.label.as_bytes(); out.extend_from_slice(&(lb.len() as u16).to_le_bytes()); @@ -133,6 +162,10 @@ pub(crate) fn deserialize(buf: &[u8]) -> Result<(Header, Vec), SecretStor } let mut salt = [0u8; SALT_LEN]; salt.copy_from_slice(r.take(SALT_LEN)?); + let mut verify_nonce = [0u8; NONCE_LEN]; + verify_nonce.copy_from_slice(r.take(NONCE_LEN)?); + let verify_ct_len = r.u32()? as usize; + let verify_ct = r.take(verify_ct_len)?.to_vec(); let mut entries = Vec::new(); while r.pos < buf.len() { @@ -154,6 +187,8 @@ pub(crate) fn deserialize(buf: &[u8]) -> Result<(Header, Vec), SecretStor Header { params: KdfParams { m_kib, t, p }, salt, + verify_nonce, + verify_ct, }, entries, )) @@ -177,12 +212,18 @@ mod tests { }); } - #[test] - fn serialize_deserialize_roundtrip() { - let header = Header { + fn test_header() -> Header { + Header { params: KdfParams::default_target(), salt: [7u8; SALT_LEN], - }; + verify_nonce: [5u8; NONCE_LEN], + verify_ct: vec![0xCC; 34], + } + } + + #[test] + fn serialize_deserialize_roundtrip() { + let header = test_header(); let entries = vec![ Entry { label: "bip39_mnemonic".into(), @@ -199,6 +240,8 @@ mod tests { let (h2, e2) = deserialize(&bytes).unwrap(); assert_eq!(h2.params, header.params); assert_eq!(h2.salt, header.salt); + assert_eq!(h2.verify_nonce, header.verify_nonce); + assert_eq!(h2.verify_ct, header.verify_ct); assert_eq!(e2.len(), 2); assert_eq!(e2[0].label, "bip39_mnemonic"); assert_eq!(e2[1].ciphertext, vec![5, 6]); @@ -210,13 +253,7 @@ mod tests { deserialize(b"NOPENOPE...."), Err(SecretStoreError::MalformedVault) )); - let mut bytes = serialize( - &Header { - params: KdfParams::default_target(), - salt: [0u8; SALT_LEN], - }, - &[], - ); + let mut bytes = serialize(&test_header(), &[]); let v = MAGIC.len(); bytes[v..v + 4].copy_from_slice(&999u32.to_le_bytes()); assert!(matches!( @@ -227,13 +264,7 @@ mod tests { #[test] fn rejects_truncated() { - let bytes = serialize( - &Header { - params: KdfParams::default_target(), - salt: [0u8; SALT_LEN], - }, - &[], - ); + let bytes = serialize(&test_header(), &[]); assert!(matches!( deserialize(&bytes[..bytes.len() - 5]), Err(SecretStoreError::MalformedVault) diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs index c091c9ebe7..15e8aaf4d8 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -22,10 +22,14 @@ mod format; use std::fs::{self, OpenOptions}; use std::io::Write; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; use crypto::{KdfParams, SALT_LEN}; use format::{Entry, Header}; +/// Process-local counter for unique temp-file names (C7). +static COUNTER: AtomicU64 = AtomicU64::new(0); + use super::error::SecretStoreError; use super::secret::{SecretBytes, SecretString}; use super::store::SecretStore; @@ -57,6 +61,55 @@ impl EncryptedFileStore { self.dir.join(format!("{}.pwsvault", wallet_id.to_hex())) } + /// Build a fresh header for a brand-new vault: random salt, default + /// Argon2 params, and a passphrase-verification token sealed under + /// the freshly derived key (SEC-REQ-2.2.x; the token is the + /// mixed-key-corruption guard). + fn new_header( + &self, + wallet_id: &WalletId, + passphrase: &SecretString, + ) -> Result<(Header, SecretBytes), SecretStoreError> { + let mut salt = [0u8; SALT_LEN]; + crypto::random_bytes(&mut salt)?; + let params = KdfParams::default_target(); + let key = crypto::derive_key(passphrase.expose_secret().as_bytes(), &salt, params)?; + let v_aad = format::verify_aad(format::FORMAT_VERSION, wallet_id.as_bytes()); + let (verify_nonce, verify_ct) = crypto::seal(&key, &v_aad, format::VERIFY_CONSTANT)?; + Ok(( + Header { + params, + salt, + verify_nonce, + verify_ct, + }, + key, + )) + } + + /// Derive the key from the supplied passphrase and verify it + /// against the header's token *before* any entry is touched. A + /// wrong passphrase fails the token's AEAD tag (constant-time) and + /// yields `WrongPassphrase` with no plaintext — defeating the + /// mixed-key-corruption defect (Marvin QA-001 / SEC-REQ-2.2.x). + fn derive_and_verify( + &self, + wallet_id: &WalletId, + header: &Header, + ) -> Result { + let key = crypto::derive_key( + self.passphrase.expose_secret().as_bytes(), + &header.salt, + header.params, + )?; + let v_aad = format::verify_aad(format::FORMAT_VERSION, wallet_id.as_bytes()); + match crypto::open(&key, &header.verify_nonce, &v_aad, &header.verify_ct) { + Ok(_) => Ok(key), + Err(SecretStoreError::Decrypt) => Err(SecretStoreError::WrongPassphrase), + Err(e) => Err(e), + } + } + /// Read + parse a vault file, or `None` if it does not exist. /// Refuses a pre-existing file with looser-than-0600 perms /// (SEC-REQ-2.2.10). @@ -83,9 +136,12 @@ impl EncryptedFileStore { entries: &[Entry], ) -> Result<(), SecretStoreError> { let serialized = format::serialize(header, entries); - let tmp = path.with_extension("pwsvault.tmp"); - // Remove a stale temp so O_EXCL can take a clean lock. - let _ = fs::remove_file(&tmp); + // Unique temp name (pid + monotonic counter) created with + // O_EXCL — no fixed name and no destination pre-remove, so a + // crash can never leave the vault absent and two writers can't + // collide on the temp (Marvin QA-004). + let unique = COUNTER.fetch_add(1, Ordering::Relaxed); + let tmp = path.with_extension(format!("pwsvault.tmp.{}.{unique}", std::process::id())); let result = (|| -> Result<(), SecretStoreError> { let mut opts = OpenOptions::new(); opts.write(true).create_new(true); @@ -95,10 +151,11 @@ impl EncryptedFileStore { f.write_all(&serialized)?; f.sync_all()?; fs::rename(&tmp, path)?; + // The directory entry must be fsync'd too, or a crash can + // lose the rename (SEC-REQ-2.2.11). if let Some(parent) = path.parent() { - if let Ok(d) = fs::File::open(parent) { - let _ = d.sync_all(); - } + let d = fs::File::open(parent)?; + d.sync_all()?; } Ok(()) })(); @@ -122,20 +179,8 @@ impl EncryptedFileStore { self.passphrase = new_passphrase; return Ok(()); }; - let old_key = crypto::derive_key( - self.passphrase.expose_secret().as_bytes(), - &old_header.salt, - old_header.params, - )?; - - let mut new_salt = [0u8; SALT_LEN]; - crypto::random_bytes(&mut new_salt)?; - let new_params = KdfParams::default_target(); - let new_key = crypto::derive_key( - new_passphrase.expose_secret().as_bytes(), - &new_salt, - new_params, - )?; + let old_key = self.derive_and_verify(&wallet_id, &old_header)?; + let (new_header, new_key) = self.new_header(&wallet_id, &new_passphrase)?; let mut new_entries = Vec::with_capacity(old_entries.len()); for e in &old_entries { @@ -149,10 +194,6 @@ impl EncryptedFileStore { ciphertext: ct, }); } - let new_header = Header { - params: new_params, - salt: new_salt, - }; self.write_vault(&path, &new_header, &new_entries)?; self.passphrase = new_passphrase; Ok(()) @@ -163,25 +204,16 @@ impl SecretStore for EncryptedFileStore { fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { let label = validated_label(label)?.to_string(); let path = self.vault_path(&wallet_id); - let (header, mut entries) = match self.read_vault(&path)? { - Some(v) => v, + let (header, key, mut entries) = match self.read_vault(&path)? { + Some((header, entries)) => { + let key = self.derive_and_verify(&wallet_id, &header)?; + (header, key, entries) + } None => { - let mut salt = [0u8; SALT_LEN]; - crypto::random_bytes(&mut salt)?; - ( - Header { - params: KdfParams::default_target(), - salt, - }, - Vec::new(), - ) + let (header, key) = self.new_header(&wallet_id, &self.passphrase)?; + (header, key, Vec::new()) } }; - let key = crypto::derive_key( - self.passphrase.expose_secret().as_bytes(), - &header.salt, - header.params, - )?; let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), &label); let (nonce, ciphertext) = crypto::seal(&key, &aad, bytes)?; entries.retain(|e| e.label != label); @@ -203,14 +235,10 @@ impl SecretStore for EncryptedFileStore { let Some((header, entries)) = self.read_vault(&path)? else { return Ok(None); }; + let key = self.derive_and_verify(&wallet_id, &header)?; let Some(entry) = entries.iter().find(|e| e.label == label) else { return Ok(None); }; - let key = crypto::derive_key( - self.passphrase.expose_secret().as_bytes(), - &header.salt, - header.params, - )?; let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), label); match crypto::open(&key, &entry.nonce, &aad, &entry.ciphertext) { Ok(pt) => Ok(Some(pt)), @@ -225,6 +253,9 @@ impl SecretStore for EncryptedFileStore { let Some((header, mut entries)) = self.read_vault(&path)? else { return Ok(()); }; + // Verify the passphrase before mutating, so a wrong pass can + // neither delete an entry nor rewrite the vault. + self.derive_and_verify(&wallet_id, &header)?; let before = entries.len(); entries.retain(|e| e.label != label); if entries.len() == before { @@ -401,7 +432,7 @@ mod tests { .filter(|e| { let n = e.file_name(); let n = n.to_string_lossy(); - n.ends_with(".bak") || n.ends_with(".tmp") + n.ends_with(".bak") || n.contains(".tmp") }) .collect(); assert!(stale.is_empty(), "rekey left stale files: {stale:?}"); @@ -412,6 +443,61 @@ mod tests { )); } + #[test] + fn put_with_wrong_passphrase_to_existing_vault_is_rejected() { + let dir = tempfile::tempdir().unwrap(); + store(dir.path()).put(wid(1), "seed", b"orig").unwrap(); + let wrong = EncryptedFileStore::open(dir.path(), SecretString::new("pw-wrong")).unwrap(); + // The defect: this used to write a mixed-key entry and return Ok. + let err = wrong.put(wid(1), "seed2", b"intruder").unwrap_err(); + assert!(matches!(err, SecretStoreError::WrongPassphrase)); + // Original vault still fully readable with the correct pass. + let ok = store(dir.path()); + assert_eq!( + ok.get(wid(1), "seed").unwrap().unwrap().expose_secret(), + b"orig" + ); + // The rejected slot was never written. + assert!(ok.get(wid(1), "seed2").unwrap().is_none()); + } + + #[test] + fn get_and_delete_with_wrong_passphrase_are_rejected() { + let dir = tempfile::tempdir().unwrap(); + store(dir.path()).put(wid(1), "seed", b"orig").unwrap(); + let wrong = EncryptedFileStore::open(dir.path(), SecretString::new("pw-wrong")).unwrap(); + assert!(matches!( + wrong.get(wid(1), "seed"), + Err(SecretStoreError::WrongPassphrase) + )); + assert!(matches!( + wrong.delete(wid(1), "seed"), + Err(SecretStoreError::WrongPassphrase) + )); + // delete must not have mutated the vault. + let ok = store(dir.path()); + assert_eq!( + ok.get(wid(1), "seed").unwrap().unwrap().expose_secret(), + b"orig" + ); + } + + #[test] + fn correct_passphrase_round_trips_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + s.put(wid(1), "seed", b"orig").unwrap(); + s.put(wid(1), "seed2", b"second").unwrap(); + assert_eq!( + s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), + b"orig" + ); + assert_eq!( + s.get(wid(1), "seed2").unwrap().unwrap().expose_secret(), + b"second" + ); + } + #[test] fn no_plaintext_in_vault_file() { let dir = tempfile::tempdir().unwrap(); From 0a7c3f050fb4c039b824004bfc18a8246e864326 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:32:31 +0200 Subject: [PATCH 07/16] fix(platform-wallet-storage): map keyring-core NoStorageAccess to KeyringLocked; correct keyring-core attribution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C3 (MED, Adams PROJ-002 / Marvin QA-003): `map_keyring_err` collapsed keyring-core's `NoStorageAccess` into `BackendUnavailable`, leaving `SecretStoreError::KeyringLocked` dead. Per keyring-core 1.0.0 docs, `NoStorageAccess` covers the locked-collection case ("it might be that the credential store is locked"), so it now maps to `KeyringLocked`, enabling the unlock-retry UX (SEC-REQ-2.1.4). Genuinely-absent backends (`NoDefaultStore` / `PlatformFailure`) stay `BackendUnavailable`. Added `locked_keyring_maps_to_keyring_locked` asserting the locked, absent, and not-found mappings. C5 (LOW, Adams PROJ-003 / Marvin QA-004): the module header said "keyring-core 4.x split" — inaccurate. Reworded to state the lib is `keyring-core 1.0.0` plus the per-platform store crates; the `keyring` 4.x crate is the sample CLI and is not a dependency. No dependency change. Co-Authored-By: Claudius the Magnificent (1M context) --- .../src/secrets/keyring.rs | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs index efa500ac35..53c34e00ff 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs @@ -1,4 +1,8 @@ -//! [`KeyringStore`] — OS keyring backend (keyring-core 4.x split). +//! [`KeyringStore`] — OS keyring backend. +//! +//! Built on `keyring-core 1.0.0` (the split-architecture library) plus +//! the per-platform credential-store crates; the `keyring` 4.x crate +//! itself is the sample CLI and is not a dependency here. //! //! Delegates at-rest protection to the OS credential store. Its //! security *is* the OS keyring's security. @@ -107,12 +111,20 @@ fn default_store() -> Result, SecretStoreError> { /// mapped here — callers translate it to `Ok(None)`/`Ok(())`. No /// keyring error string is embedded (it could echo the `account`, /// which is non-secret, but the taxonomy stays clean — SEC-REQ-2.0.1). +/// +/// Per keyring-core 1.0.0, `NoStorageAccess` covers the *locked* +/// collection case ("it might be that the credential store is +/// locked"), so it maps to [`SecretStoreError::KeyringLocked`] to +/// drive the unlock-retry UX (SEC-REQ-2.1.4). A genuinely absent +/// backend (`NoDefaultStore` / `PlatformFailure`) is +/// [`SecretStoreError::BackendUnavailable`]. fn map_keyring_err(e: KeyringError) -> SecretStoreError { match e { KeyringError::NoEntry => SecretStoreError::NotFound, - KeyringError::NoStorageAccess(_) - | KeyringError::NoDefaultStore - | KeyringError::PlatformFailure(_) => SecretStoreError::BackendUnavailable, + KeyringError::NoStorageAccess(_) => SecretStoreError::KeyringLocked, + KeyringError::NoDefaultStore | KeyringError::PlatformFailure(_) => { + SecretStoreError::BackendUnavailable + } _ => SecretStoreError::BackendUnavailable, } } @@ -171,6 +183,28 @@ mod tests { } } + #[test] + fn locked_keyring_maps_to_keyring_locked() { + // keyring-core's `NoStorageAccess` covers the locked-collection + // case; it must surface as `KeyringLocked` so the caller can + // prompt for unlock (SEC-REQ-2.1.4), not as `BackendUnavailable`. + let locked = + KeyringError::NoStorageAccess(std::io::Error::other("collection is locked").into()); + assert!(matches!( + map_keyring_err(locked), + SecretStoreError::KeyringLocked + )); + // A genuinely absent backend stays `BackendUnavailable`. + assert!(matches!( + map_keyring_err(KeyringError::NoDefaultStore), + SecretStoreError::BackendUnavailable + )); + assert!(matches!( + map_keyring_err(KeyringError::NoEntry), + SecretStoreError::NotFound + )); + } + #[test] fn headless_fails_closed_not_panic() { // On headless CI `new()` returns `BackendUnavailable`; where a From 884d4707c92d30be033e37271f0a30af866e462a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:32:40 +0200 Subject: [PATCH 08/16] fix(platform-wallet-storage): MemoryStore stores SecretBytes so it zeroizes on drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C4 (MED, Smythe SEC-RA-002 / Adams PROJ-004 / Marvin QA-002): the rustdoc claimed stored values sit in `SecretBytes`, but the map held a bare `Vec` that never zeroized — code contradicted the doc. Fixed the code (not the doc): the backing map is now `HashMap<(WalletId,String), SecretBytes>`, closing SEC-REQ-2.3.2 so even test memory is wiped on drop. Added `stored_value_is_zeroizing_ wrapper` (type-binding assertion) + a `needs_drop::()` compile-time guard. Co-Authored-By: Claudius the Magnificent (1M context) --- .../src/secrets/memory.rs | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/memory.rs b/packages/rs-platform-wallet-storage/src/secrets/memory.rs index 4030140996..d4d0a8f3ae 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/memory.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/memory.rs @@ -21,10 +21,11 @@ use super::store::SecretStore; use super::validate::{validated_label, WalletId}; /// A `HashMap`-backed [`SecretStore`] for tests. No persistence, no -/// encryption. +/// encryption. Stored values sit in [`SecretBytes`] so even test +/// memory zeroizes on drop (SEC-REQ-2.3.2). #[derive(Default)] pub struct MemoryStore { - map: Mutex>>, + map: Mutex>, } impl MemoryStore { @@ -38,7 +39,10 @@ impl SecretStore for MemoryStore { fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { let label = validated_label(label)?; let mut map = self.map.lock().expect("MemoryStore mutex poisoned"); - map.insert((wallet_id, label.to_string()), bytes.to_vec()); + map.insert( + (wallet_id, label.to_string()), + SecretBytes::from_slice(bytes), + ); Ok(()) } @@ -51,7 +55,7 @@ impl SecretStore for MemoryStore { let map = self.map.lock().expect("MemoryStore mutex poisoned"); Ok(map .get(&(wallet_id, label.to_string())) - .map(|v| SecretBytes::from_slice(v))) + .map(|v| SecretBytes::from_slice(v.expose_secret()))) } fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { @@ -112,6 +116,23 @@ mod tests { ); } + // The store must hold a zeroize-on-drop wrapper, not a bare + // `Vec` (SEC-REQ-2.3.2 / Marvin QA-002): the value type must + // run `Drop`. + const _: () = { + assert!(std::mem::needs_drop::()); + }; + + #[test] + fn stored_value_is_zeroizing_wrapper() { + let s = MemoryStore::new(); + s.put(wid(1), "seed", &[0xAB; 32]).unwrap(); + let map = s.map.lock().unwrap(); + // This binding only compiles if the value type is `SecretBytes`. + let v: &SecretBytes = map.get(&(wid(1), "seed".to_string())).unwrap(); + assert_eq!(v.expose_secret(), &[0xAB; 32]); + } + #[test] fn rejects_invalid_label() { let s = MemoryStore::new(); From 1256cb8adba09d6abf471986b4ab63e1be27b8c5 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:32:47 +0200 Subject: [PATCH 09/16] docs(platform-wallet-storage): correct keyring-core attribution in Cargo.toml comment C5 (LOW, Adams PROJ-003 / Marvin QA-004): the per-platform-store dependency comment said "keyring-core 4.x split". Reworded to state accurately that `keyring-core 1.0.0` is the API and the per-platform crates provide the backends (the `keyring` 4.x crate is the sample CLI and is intentionally not depended on). No dependency change. Co-Authored-By: Claudius the Magnificent (1M context) --- packages/rs-platform-wallet-storage/Cargo.toml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-wallet-storage/Cargo.toml b/packages/rs-platform-wallet-storage/Cargo.toml index 137763d1ab..e553357555 100644 --- a/packages/rs-platform-wallet-storage/Cargo.toml +++ b/packages/rs-platform-wallet-storage/Cargo.toml @@ -79,9 +79,10 @@ tracing-subscriber = { version = "0.3", features = [ "env-filter", ], optional = true } -# Per-platform OS-keyring credential stores (the keyring-core 4.x split: -# `keyring-core` is the API, these provide the backends). Gated by -# `secrets` via `dep:`. Target-specific tables MUST follow all +# Per-platform OS-keyring credential stores. `keyring-core 1.0.0` is +# the API; these crates provide the platform backends (the `keyring` +# 4.x crate is the sample CLI and is intentionally not depended on). +# Gated by `secrets` via `dep:`. Target-specific tables MUST follow all # `[dependencies]` entries. [target.'cfg(any(target_os = "linux", target_os = "freebsd"))'.dependencies] linux-keyutils-keyring-store = { version = "=1.0.0", optional = true } From 1c296989903a9099eea9988428855bbe913aac51 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 19 May 2026 16:32:57 +0200 Subject: [PATCH 10/16] docs(platform-wallet-storage): SECRETS.md reflects the delivered SecretStore API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C2 (MED, Adams PROJ-001): the trait sketch was stale/dangerous — `get -> Option>` (the exact CRITICAL leak SEC-REQ-4.1 forbids) and the false "feature flag exists today but flips no code" line. Rewritten to the delivered API: `get -> Result, SecretStoreError>`, accurate `put`/`delete` signatures, the real backends (KeyringStore/EncryptedFileStore/MemoryStore with their fail-closed / gating semantics), and the now-true statement that enabling `secrets` activates the module. Present-state only, no history narration; no forbidden token introduced into `src/sqlite/schema/` or `migrations/`. Co-Authored-By: Claudius the Magnificent (1M context) --- .../rs-platform-wallet-storage/SECRETS.md | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index 8871f0f396..da99971f58 100644 --- a/packages/rs-platform-wallet-storage/SECRETS.md +++ b/packages/rs-platform-wallet-storage/SECRETS.md @@ -12,29 +12,45 @@ Keystore, OS keyring, encrypted file vault). They are re-derived as needed via the wallet's BIP-32/BIP-39 plumbing and never touch the SQLite file the persister writes. -## Future `secrets` submodule sketch +## The `secrets` submodule -This crate is structured so the `SecretStore` trait can land as a -submodule (`platform_wallet_storage::secrets`) gated behind a `secrets` -Cargo feature, sharing the crate-level error type and config -conventions. The module slot is reserved in `src/lib.rs` with a -commented-out `pub mod secrets;` line; the feature flag exists today -but flips no code. +`platform_wallet_storage::secrets` is gated behind the opt-in `secrets` +Cargo feature (never enabled by `default`). Enabling the feature +activates the module: it pulls the pinned crypto/keyring dependencies +and compiles `src/secrets/`. Secrets reach a backend only through this +trait — never through the SQLite persister DTO. ```rust -trait SecretStore: Send + Sync { - fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<()>; - fn get(&self, wallet_id: WalletId, label: &str) -> Result>>; - fn delete(&self, wallet_id: WalletId, label: &str) -> Result<()>; +pub trait SecretStore: Send + Sync { + fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) + -> Result<(), SecretStoreError>; + fn get(&self, wallet_id: WalletId, label: &str) + -> Result, SecretStoreError>; + fn delete(&self, wallet_id: WalletId, label: &str) + -> Result<(), SecretStoreError>; } ``` -Reference backends to plan for: +`get` returns `Option` — a zeroize-on-drop wrapper, never +a bare `Vec`. `label` is validated against +`^[A-Za-z0-9._-]{1,64}$`; `wallet_id` is a fixed 32-byte newtype. +`SecretStoreError` is a concrete `thiserror` enum carrying no secret +bytes. -- `KeyringStore` (default) — OS-native keyring; recoverable across - reinstalls when the keyring is. -- `EncryptedFileStore` — Argon2id + XChaCha20-Poly1305 over a passphrase. -- `MemoryStore` — tests only. +Backends: + +- `KeyringStore` — OS-native keyring (`keyring-core 1.0.0` + the + per-platform store crates). Recommended default on desktop OSes; + fails closed (`BackendUnavailable`) on headless Linux with no Secret + Service — never a silent plaintext fallback. +- `EncryptedFileStore` — Argon2id + XChaCha20-Poly1305 vault file with + a header-stored passphrase-verification token. Recommended default + on headless / server hosts. +- `MemoryStore` — tests only, gated behind `__secrets-test-helpers` so + it is unreachable from production builds. + +Backend selection is an explicit operator decision; there is no +automatic fallback between backends. ## What the SQLite backend WILL refuse to store From 2c7927b4f5627988d6d7ce28c3be14b74ac783ee Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 20 May 2026 14:40:29 +0200 Subject: [PATCH 11/16] chore(platform-wallet-storage,ci): drop cargo-deny, flip secrets default-on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the cargo-deny advisories CI job and its `deny.toml` config in favour of the existing `rustsec/audit-check` job. Once `secrets` is in the default feature set, `Cargo.lock` unconditionally pins the RustSec-clean crypto stack (`argon2`/`chacha20poly1305`/`zeroize`/ `subtle`/`region`/`keyring-core` + per-platform store crates) so a single audit run covers them all (SEC-REQ-4.7). `secrets` joins `sqlite`+`cli` as a default feature. Dev-dependency on self adds `default-features = false` so the off-state CI invocation (`--no-default-features --features sqlite,cli`) actually exercises the secrets-disabled graph — otherwise the dev-dep view would silently re-enable defaults for every integration test. New `tests/secrets_off_state.rs` is the runtime D4 guard: gated `#[cfg(not(feature = "secrets"))]`, it builds against the persister surface only and asserts the off-state graph stays consumable. T1+T2 land atomically — cargo-deny removal coincides with secrets going default-on so crypto pins never drop out of audit scope between commits. Co-Authored-By: Claudius the Magnificent (1M context) --- .github/workflows/security-audit-rust.yml | 19 --------- deny.toml | 39 ------------------- .../rs-platform-wallet-storage/Cargo.toml | 17 +++++--- packages/rs-platform-wallet-storage/README.md | 12 +++--- .../tests/secrets_off_state.rs | 31 +++++++++++++++ 5 files changed, 49 insertions(+), 69 deletions(-) delete mode 100644 deny.toml create mode 100644 packages/rs-platform-wallet-storage/tests/secrets_off_state.rs diff --git a/.github/workflows/security-audit-rust.yml b/.github/workflows/security-audit-rust.yml index 00e44ce4fe..518fa4ae06 100644 --- a/.github/workflows/security-audit-rust.yml +++ b/.github/workflows/security-audit-rust.yml @@ -17,22 +17,3 @@ jobs: uses: rustsec/audit-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - - rs-crates-deny-advisories: - name: cargo-deny advisories (incl. secrets feature) - runs-on: ubuntu-24.04 - steps: - - name: Check out repo - uses: actions/checkout@v4 - - # `cargo audit` reads `Cargo.lock`, which already contains the - # `secrets`-gated crypto pins; `cargo deny` with `--all-features` - # additionally exercises the feature-conditional dependency graph - # so `platform-wallet-storage`'s `secrets` deps are advisory- - # checked even though `default` does not enable them - # (SEC-REQ-4.7). - - name: cargo-deny advisories - uses: EmbarkStudios/cargo-deny-action@v2 - with: - command: check advisories - arguments: --all-features diff --git a/deny.toml b/deny.toml deleted file mode 100644 index 2b826cb899..0000000000 --- a/deny.toml +++ /dev/null @@ -1,39 +0,0 @@ -# cargo-deny configuration — advisory gate for the workspace, -# including the `secrets`-gated crypto dependencies of -# `platform-wallet-storage` (SEC-REQ-4.7). -# -# `cargo deny` resolves the full `Cargo.lock`, so the pinned -# `argon2` / `chacha20poly1305` / `zeroize` / `subtle` / `region` / -# `keyring-core` + per-platform store crates are in scope regardless of -# which features a given build enables. The CI invocation additionally -# passes `--all-features` so feature-gated graphs are exercised too. - -[advisories] -version = 2 -# Keep in sync with `.cargo/audit.toml`. -ignore = ["RUSTSEC-2020-0071"] - -[bans] -multiple-versions = "allow" - -[licenses] -version = 2 -allow = [ - "MIT", - "Apache-2.0", - "Apache-2.0 WITH LLVM-exception", - "BSD-2-Clause", - "BSD-3-Clause", - "ISC", - "Unicode-DFS-2016", - "Unicode-3.0", - "Zlib", - "MPL-2.0", - "CC0-1.0", - "0BSD", -] -confidence-threshold = 0.8 - -[sources] -unknown-registry = "deny" -unknown-git = "allow" diff --git a/packages/rs-platform-wallet-storage/Cargo.toml b/packages/rs-platform-wallet-storage/Cargo.toml index e553357555..c4ee479a08 100644 --- a/packages/rs-platform-wallet-storage/Cargo.toml +++ b/packages/rs-platform-wallet-storage/Cargo.toml @@ -105,11 +105,15 @@ static_assertions = "1" filetime = "0.2" tracing-test = { version = "0.2", features = ["no-env-filter"] } serial_test = "3" -platform-wallet-storage = { path = ".", features = ["sqlite", "cli", "__test-helpers"] } +# `default-features = false` so the off-state CI invocation +# (`--no-default-features --features sqlite,cli`) actually exercises a +# build with `secrets` disabled — otherwise the dev-dep view would +# silently re-enable the default feature set for every integration test. +platform-wallet-storage = { path = ".", default-features = false, features = ["__test-helpers"] } tempfile = "3" [features] -default = ["sqlite", "cli"] +default = ["sqlite", "cli", "secrets"] # SQLite-backed persister (`platform_wallet_storage::sqlite`). sqlite = [ "dep:key-wallet", @@ -135,9 +139,12 @@ cli = [ "dep:serde_json", "dep:tracing-subscriber", ] -# `SecretStore` submodule (`platform_wallet_storage::secrets`): -# zeroizing secret wrappers + Keyring / EncryptedFile backends. Opt-in; -# never enabled by `default`. Pulls only RustSec-clean pinned crypto. +# `secrets` submodule (`platform_wallet_storage::secrets`): zeroizing +# secret wrappers + EncryptedFile backend + OS-keyring construction +# helper, all built on the upstream `keyring_core::api` SPI. Default-on +# so `Cargo.lock` unconditionally pins the RustSec-clean crypto stack +# (SEC-REQ-4.7). Disable explicitly via `--no-default-features` to +# build the storage crate without the crypto graph. secrets = [ "dep:argon2", "dep:chacha20poly1305", diff --git a/packages/rs-platform-wallet-storage/README.md b/packages/rs-platform-wallet-storage/README.md index c3c6fc4a32..9e97e44ae3 100644 --- a/packages/rs-platform-wallet-storage/README.md +++ b/packages/rs-platform-wallet-storage/README.md @@ -115,14 +115,14 @@ validation failure (e.g. corrupt backup source). |---|---|---| | `sqlite` | yes | SQLite persister (`platform_wallet_storage::sqlite`) and all of its native deps (`rusqlite`, `refinery`, `dpp`, `dash-sdk`, `key-wallet`, etc.) | | `cli` | yes | Maintenance binary `platform-wallet-storage`. Implies `sqlite`. | -| `secrets` | no | Reserved for the future `SecretStore` submodule. No code lands today. | +| `secrets` | yes | `platform_wallet_storage::secrets` submodule — zeroizing secret wrappers (`SecretBytes`, `SecretString`), the `EncryptedFileStore` Argon2id + XChaCha20-Poly1305 vault backend, and the `default_credential_store()` OS-keyring constructor. Implements the upstream `keyring_core::api::{CredentialApi, CredentialStoreApi}` SPI. | | `__test-helpers` | no | Crate-private `lock_conn_for_test` / `config_for_test` accessors. The double-underscore prefix follows Cargo's "do not enable from downstream" convention; the methods are also `#[doc(hidden)]`. | +| `__secrets-test-helpers` | no | Exposes `secrets::MemoryCredentialStore`, the in-RAM test double. Double-underscore = unreachable from production builds. | -`cargo build -p platform-wallet-storage --no-default-features` builds -the crate with neither the SQLite backend nor the CLI compiled in. -The resulting library has no public surface today; the build mode -exists to support a future split where one cargo target wants only -the secrets feature. +`cargo build -p platform-wallet-storage --no-default-features` builds a +minimal core with neither the SQLite backend, the CLI, nor the secrets +submodule. `--no-default-features --features sqlite,cli` is the +"persister-only" build mode (no crypto dependencies). ## Schema diff --git a/packages/rs-platform-wallet-storage/tests/secrets_off_state.rs b/packages/rs-platform-wallet-storage/tests/secrets_off_state.rs new file mode 100644 index 0000000000..fb0fdb76ea --- /dev/null +++ b/packages/rs-platform-wallet-storage/tests/secrets_off_state.rs @@ -0,0 +1,31 @@ +//! Runtime guard that the `secrets` feature is genuinely optional +//! (D4): with `--no-default-features --features sqlite,cli` the +//! `secrets` module compiles out of the public surface and the SQLite +//! persister still links cleanly. +//! +//! Invocation: +//! `cargo test -p platform-wallet-storage --no-default-features \ +//! --features sqlite,cli --test secrets_off_state` +//! +//! Under the default build (`secrets` enabled) this file's only test is +//! the `cfg`-disabled body below — a deliberate no-op so the same file +//! satisfies both build modes. + +#[cfg(not(feature = "secrets"))] +#[test] +fn secrets_module_absent_when_feature_off() { + // The persister side of the crate is still reachable. + let _ = std::any::type_name::(); + + // Building this file at all without the `secrets` cfg-gate + // satisfying its imports is the proof: every secrets-only symbol + // lives behind `#[cfg(feature = "secrets")]`, so the crate's + // public namespace contains no `secrets::*` re-exports here. +} + +#[cfg(feature = "secrets")] +#[test] +fn secrets_off_state_test_runs_under_no_default_features() { + // No-op under default features; the meaningful assertion runs only + // when the off-state CI invocation flips `secrets` off. +} From 123f9087d63d0ab366f700277a93df1b9bef6fde Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 20 May 2026 14:49:52 +0200 Subject: [PATCH 12/16] refactor(platform-wallet-storage): adopt keyring_core SPI for secret backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retires the crate-local `SecretStore` trait + `SecretStoreError` enum and rebuilds the `secrets` submodule on `keyring_core::api::{CredentialApi, CredentialStoreApi}` — the upstream SPI shipped by `keyring-core 1.0.0`. The `EncryptedFileStore`'s security construction (Argon2id + XChaCha20-Poly1305 + AAD verify token + 0600 + atomic temp→rename + dir-fsync + zeroize) is preserved byte-for-byte; only the trait surface changes. API-shape mapping (Nagatha §1, variant A — the `:` delimiter is rejected by the label allowlist): service = "dash.platform-wallet-storage/" + hex(wallet_id) user = label Per-task content: - **T3** `src/secrets/file/error.rs` — new `FileStoreError` enum (`Decrypt`, `WrongPassphrase`, `KdfFailure`, `VersionUnsupported`, `MalformedVault`, `InvalidLabel`, `InsecurePermissions`, `Io`). Static `#[error]` strings only; no secret in any variant. `src/secrets/file/error_bridge.rs` — `FileStoreFailure` unit-only marker (Smythe EDIT-3: no `String`/`Vec`/`Path` fields permitted, enforced via a compile-time `Copy` assertion) boxed inside `keyring_core::Error::NoStorageAccess` (WrongPassphrase) or rendered into `BadStoreFormat`'s static `String` payload. The `downcast_failure` helper recovers the marker for D1(b). - **T4** `src/secrets/file/mod.rs` — `EncryptedFileStore` implements `CredentialStoreApi`; per-`(service, user)` entries implement `CredentialApi`. The store is held behind an internal `Arc` so long-lived credentials can outlive the public handle. `delete` honors upstream's `NoEntry`-if-absent contract (D3). `service` parsing rejects mismatch with `Invalid("service", _)`; `validated_label` runs at `build` time AND every `CredentialApi` op (defence in depth, M-2). All twelve in-module security tests port one-for-one through the SPI (NoEntry for absence, downcast for typed-error checks). - **T5** `src/secrets/keyring.rs` — `KeyringStore` wrapper retired in favour of the bare `default_credential_store() -> Result, keyring_core::Error>` constructor. Headless / unknown OS / D-Bus-less Linux → `NoDefaultStore` per D2 (typed, single SPI error). Never panics, never falls back. - **T7** `src/secrets/memory.rs` — `MemoryStore` → `MemoryCredentialStore` implementing `CredentialStoreApi`. Internal map keys on `(service, user)` strings, values remain `SecretBytes` (SEC-REQ-2.3.2). Still gated behind `__secrets-test-helpers`. - **T8** `src/lib.rs` — object-safety + `Send + Sync` assertions now target `keyring_core::Error` and `dyn CredentialStoreApi + Send + Sync`. `src/secrets/mod.rs` re-exports the new surface; `pub use SecretStore` / `SecretStoreError` retired. - **Tests** — `tests/secrets_api.rs` rewritten against the SPI; the `Vec → SecretBytes::new` consumer-seam pattern (Smythe EDIT-1: no named intermediate `Vec` binding) is the type-shape assertion. `tests/secrets_guard.rs` extended with the EDIT-2 EDIT-2 guard: no `{{:?}}`-debug-format paired with `keyring_core::Error` in `src/secrets/` (since `BadEncoding`/`BadDataFormat` embed raw `Vec`). All twelve `EncryptedFileStore` security invariants pass on the new API. `tests/secrets_seed_provider_adapter.rs` and the `seed_provider_adapter.rs` source file are NOT landed on this branch: the `SeedProvider`/`WalletSecret`/`SeedUnavailable` types they consume live in `rs-platform-wallet` on PR #3692, not on this base. The rewritten adapter will land on PR #3692's rebase onto this tip — see the rework report. Co-Authored-By: Claudius the Magnificent (1M context) --- .../rs-platform-wallet-storage/src/lib.rs | 14 +- .../src/secrets/file/crypto.rs | 34 +- .../src/secrets/{ => file}/error.rs | 75 ++- .../src/secrets/file/error_bridge.rs | 198 +++++++ .../src/secrets/file/format.rs | 32 +- .../src/secrets/file/mod.rs | 551 ++++++++++++++---- .../src/secrets/keyring.rs | 229 ++------ .../src/secrets/memory.rs | 238 +++++--- .../src/secrets/mod.rs | 64 +- .../src/secrets/store.rs | 55 -- .../src/secrets/validate.rs | 39 +- .../tests/secrets_api.rs | 76 ++- .../tests/secrets_guard.rs | 55 ++ 13 files changed, 1069 insertions(+), 591 deletions(-) rename packages/rs-platform-wallet-storage/src/secrets/{ => file}/error.rs (55%) create mode 100644 packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs delete mode 100644 packages/rs-platform-wallet-storage/src/secrets/store.rs diff --git a/packages/rs-platform-wallet-storage/src/lib.rs b/packages/rs-platform-wallet-storage/src/lib.rs index ae40acbc90..c4d2ab779c 100644 --- a/packages/rs-platform-wallet-storage/src/lib.rs +++ b/packages/rs-platform-wallet-storage/src/lib.rs @@ -57,18 +57,20 @@ fn _object_safety_check(persister: SqlitePersister) { std::sync::Arc::new(persister); } -// `SecretStore` must be object-safe and its error `Send + Sync`, so a -// backend can be held behind `Arc` and its errors -// crossed between threads / FFI. +// The keyring SPI must be object-safe and its error `Send + Sync`, so +// a backend can be held behind `Arc` and its errors crossed between threads / FFI. #[cfg(feature = "secrets")] #[allow(dead_code)] const fn _secrets_send_sync_check() {} #[cfg(feature = "secrets")] const _: () = { - _secrets_send_sync_check::(); + _secrets_send_sync_check::(); + _secrets_send_sync_check::(); }; #[cfg(feature = "secrets")] #[allow(dead_code)] -fn _secret_store_object_safety_check(store: secrets::EncryptedFileStore) { - let _: std::sync::Arc = std::sync::Arc::new(store); +fn _credential_store_object_safety_check(store: secrets::EncryptedFileStore) { + let _: std::sync::Arc = + std::sync::Arc::new(store); } diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs index 5858369b6b..8d94cce6cc 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs @@ -7,7 +7,7 @@ use chacha20poly1305::aead::Aead; use chacha20poly1305::{KeyInit, XChaCha20Poly1305, XNonce}; use getrandom::getrandom; -use super::super::error::SecretStoreError; +use super::error::FileStoreError; use super::super::secret::SecretBytes; /// Argon2 parameter floors (SEC-REQ-2.2.2) — derivation MUST NOT use @@ -29,8 +29,8 @@ pub(crate) const NONCE_LEN: usize = 24; pub(crate) const KEY_LEN: usize = 32; /// Fill `buf` with CSPRNG bytes (`OsRng` via `getrandom`). -pub(crate) fn random_bytes(buf: &mut [u8]) -> Result<(), SecretStoreError> { - getrandom(buf).map_err(|_| SecretStoreError::KdfFailure) +pub(crate) fn random_bytes(buf: &mut [u8]) -> Result<(), FileStoreError> { + getrandom(buf).map_err(|_| FileStoreError::KdfFailure) } /// Argon2id parameters as stored in / read from a vault header. @@ -53,9 +53,9 @@ impl KdfParams { /// Reject params below the floors (a downgraded header) before any /// derivation runs (SEC-REQ-2.2.2). - pub(crate) fn enforce_floors(&self) -> Result<(), SecretStoreError> { + pub(crate) fn enforce_floors(&self) -> Result<(), FileStoreError> { if self.m_kib < ARGON2_MIN_M_KIB || self.t < ARGON2_MIN_T || self.p != ARGON2_P { - return Err(SecretStoreError::KdfFailure); + return Err(FileStoreError::KdfFailure); } Ok(()) } @@ -67,15 +67,15 @@ pub(crate) fn derive_key( passphrase: &[u8], salt: &[u8], params: KdfParams, -) -> Result { +) -> Result { params.enforce_floors()?; let argon_params = Params::new(params.m_kib, params.t, params.p, Some(KEY_LEN)) - .map_err(|_| SecretStoreError::KdfFailure)?; + .map_err(|_| FileStoreError::KdfFailure)?; let argon = Argon2::new(Algorithm::Argon2id, Version::V0x13, argon_params); let mut key = SecretBytes::zeroed(KEY_LEN); argon .hash_password_into(passphrase, salt, key.expose_secret_mut()) - .map_err(|_| SecretStoreError::KdfFailure)?; + .map_err(|_| FileStoreError::KdfFailure)?; Ok(key) } @@ -85,9 +85,9 @@ pub(crate) fn seal( key: &SecretBytes, aad: &[u8], plaintext: &[u8], -) -> Result<([u8; NONCE_LEN], Vec), SecretStoreError> { +) -> Result<([u8; NONCE_LEN], Vec), FileStoreError> { let cipher = XChaCha20Poly1305::new_from_slice(key.expose_secret()) - .map_err(|_| SecretStoreError::KdfFailure)?; + .map_err(|_| FileStoreError::KdfFailure)?; let mut nonce_bytes = [0u8; NONCE_LEN]; random_bytes(&mut nonce_bytes)?; let nonce = XNonce::from_slice(&nonce_bytes); @@ -99,12 +99,12 @@ pub(crate) fn seal( aad, }, ) - .map_err(|_| SecretStoreError::Decrypt)?; + .map_err(|_| FileStoreError::Decrypt)?; Ok((nonce_bytes, ct)) } /// Decrypt `ciphertext` under `key`/`nonce`/`aad`. On tag failure -/// returns [`SecretStoreError::Decrypt`] and **no** plaintext — the +/// returns [`FileStoreError::Decrypt`] and **no** plaintext — the /// combined (non-detached) API never materializes unverified bytes at /// our boundary (SEC-REQ-2.2.8, CWE-347, the RUSTSEC-2023-0096 lesson). pub(crate) fn open( @@ -112,9 +112,9 @@ pub(crate) fn open( nonce: &[u8; NONCE_LEN], aad: &[u8], ciphertext: &[u8], -) -> Result { +) -> Result { let cipher = XChaCha20Poly1305::new_from_slice(key.expose_secret()) - .map_err(|_| SecretStoreError::KdfFailure)?; + .map_err(|_| FileStoreError::KdfFailure)?; let nonce = XNonce::from_slice(nonce); let pt = cipher .decrypt( @@ -124,7 +124,7 @@ pub(crate) fn open( aad, }, ) - .map_err(|_| SecretStoreError::Decrypt)?; + .map_err(|_| FileStoreError::Decrypt)?; Ok(SecretBytes::new(pt)) } @@ -187,7 +187,7 @@ mod tests { let key = derive_key(b"pw", &salt, params).unwrap(); let (nonce, ct) = seal(&key, b"slot-A", b"seed").unwrap(); let err = open(&key, &nonce, b"slot-B", &ct).unwrap_err(); - assert!(matches!(err, SecretStoreError::Decrypt)); + assert!(matches!(err, FileStoreError::Decrypt)); } #[test] @@ -203,7 +203,7 @@ mod tests { let (nonce, ct) = seal(&k1, b"aad", b"seed").unwrap(); assert!(matches!( open(&k2, &nonce, b"aad", &ct), - Err(SecretStoreError::Decrypt) + Err(FileStoreError::Decrypt) )); } diff --git a/packages/rs-platform-wallet-storage/src/secrets/error.rs b/packages/rs-platform-wallet-storage/src/secrets/file/error.rs similarity index 55% rename from packages/rs-platform-wallet-storage/src/secrets/error.rs rename to packages/rs-platform-wallet-storage/src/secrets/file/error.rs index a06b454a31..3ff29cd5fd 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/error.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/error.rs @@ -1,34 +1,21 @@ -//! Typed errors for the `SecretStore` backends. +//! File-backend-unique error taxonomy. //! -//! Concrete `thiserror` enum — no boxed dynamic error trait object -//! (SEC-REQ-4.4 / TC-082), no `#[non_exhaustive]` (prior project -//! decision), and **no** secret byte, passphrase, plaintext, or -//! stringified source that could carry one in any variant. -//! `#[error("...")]` strings are static and structural; only -//! non-secret diagnostics (a permission `mode`, a format `found` -//! version) are carried as typed fields (SEC-REQ-2.0.1 / 2.2.8, -//! CWE-209/CWE-532). +//! Concrete `thiserror` enum (SEC-REQ-4.4 / TC-082), no +//! `#[non_exhaustive]`, **no** secret byte, passphrase, plaintext, or +//! stringified source that could carry one in any variant. `#[error]` +//! strings are static + structural; only non-secret diagnostics (POSIX +//! mode bits, header version int) are carried as typed fields +//! (SEC-REQ-2.0.1 / 2.2.8, CWE-209/CWE-532). +//! +//! The `EncryptedFileStore` surfaces this enum at its construction / +//! `rekey` API; its `keyring_core::api::CredentialApi` / +//! `CredentialStoreApi` impls bridge it through +//! [`into_keyring`](super::error_bridge::into_keyring) so SPI callers +//! see a uniform `keyring_core::Error`. -/// Errors returned by [`SecretStore`](super::SecretStore) backends. -/// -/// Variant taxonomy lets a caller distinguish "no secure backend, ask -/// the operator" from "wrong passphrase, re-prompt" without ever -/// inspecting a secret. +/// Errors produced by the `EncryptedFileStore` vault backend. #[derive(Debug, thiserror::Error)] -pub enum SecretStoreError { - /// No secure OS keyring is reachable (headless / no Secret Service / - /// no D-Bus session). Fail closed — never degrade to plaintext. - #[error("secret backend unavailable")] - BackendUnavailable, - - /// The OS keyring exists but its collection is locked. - #[error("keyring is locked")] - KeyringLocked, - - /// No secret stored under the requested `(wallet_id, label)`. - #[error("secret not found")] - NotFound, - +pub enum FileStoreError { /// AEAD tag verification failed. Carries **no** decrypted-but- /// unverified bytes and no source (SEC-REQ-2.2.8, CWE-347). #[error("decryption/integrity check failed")] @@ -38,25 +25,14 @@ pub enum SecretStoreError { #[error("wrong passphrase")] WrongPassphrase, - /// `label` failed the `^[A-Za-z0-9._-]{1,64}$` allowlist - /// (SEC-REQ-4.3, CWE-22/CWE-20). - #[error("invalid label")] - InvalidLabel, - - /// Filesystem error (open / write / rename / fsync). The inner - /// `io::Error` carries an OS code and a path *the caller supplied*, - /// never a secret. - #[error("io error")] - Io(#[from] std::io::Error), - /// Argon2 key derivation failed. The upstream error carries no /// useful non-secret diagnostic, so it is intentionally not - /// embedded (SEC-REQ-2.2.8). + /// embedded. #[error("key derivation failed")] KdfFailure, /// The vault header declared a `format_version` this build does not - /// understand. Refuse, fail closed (SEC-REQ-2.2.9). + /// understand (SEC-REQ-2.2.9). #[error("unsupported vault format version {found}")] VersionUnsupported { /// The version byte read from the (authenticated) header. @@ -68,6 +44,11 @@ pub enum SecretStoreError { #[error("malformed vault file")] MalformedVault, + /// `label` failed the `^[A-Za-z0-9._-]{1,64}$` allowlist + /// (SEC-REQ-4.3, CWE-22/CWE-20). + #[error("invalid label")] + InvalidLabel, + /// A pre-existing vault file had permissions looser than `0600`. /// Refuse rather than tighten-and-trust (SEC-REQ-2.2.10). #[error("vault file has insecure permissions")] @@ -75,4 +56,16 @@ pub enum SecretStoreError { /// The offending POSIX mode bits (not secret). mode: u32, }, + + /// Filesystem error (open / write / rename / fsync). The inner + /// `io::Error` carries an OS code and a path *the caller supplied*, + /// never a secret. + #[error("io error")] + Io(#[from] std::io::Error), +} + +impl From for FileStoreError { + fn from(_: super::super::validate::InvalidLabel) -> Self { + Self::InvalidLabel + } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs b/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs new file mode 100644 index 0000000000..cff4f89e94 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs @@ -0,0 +1,198 @@ +//! Bridge between [`FileStoreError`] and `keyring_core::Error`. +//! +//! The file backend's failure modes (wrong passphrase, malformed +//! vault, insecure permissions, KDF failure) are unique to a local +//! AEAD vault — `keyring_core::Error` does not name them. To stay on a +//! single SPI error type without losing the structural distinction we +//! box a unit-only [`FileStoreFailure`] marker inside +//! `keyring_core::Error::{NoStorageAccess, BadStoreFormat}`'s payload +//! (D1). Consumers (notably the seed-provider adapter) recover the +//! marker via `Error::source()` + downcast — see +//! [`downcast_failure`]. +//! +//! Per Smythe EDIT-3, [`FileStoreFailure`] is **unit-variants only** +//! and never carries user-supplied or secret data; the cross-SPI +//! bridge is secret-free by construction. + +use std::error::Error as StdError; + +use keyring_core::Error as KeyringError; + +use super::error::FileStoreError; + +/// File-backend failure marker boxed across the +/// `keyring_core::Error::{NoStorageAccess, BadStoreFormat}` seam. +/// +/// **Unit variants only** (Smythe EDIT-3): no field may carry a +/// user-supplied path, a secret byte, a passphrase, a label, or +/// stringified data. Numeric correlation fields are acceptable; this +/// taxonomy currently needs none. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FileStoreFailure { + /// Wrong passphrase rejected at the header verify-token tag check. + WrongPassphrase, + /// AEAD decryption / integrity check failed on a stored entry. + Decrypt, + /// Argon2 key derivation failed. + KdfFailure, + /// Vault header declared an unsupported `format_version`. + VersionUnsupported, + /// Vault file framing was malformed. + MalformedVault, + /// Pre-existing vault file held looser-than-0600 permissions. + InsecurePermissions, +} + +impl std::fmt::Display for FileStoreFailure { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Static, parameter-free strings — no user / secret data may + // ever enter this Display (Smythe EDIT-3). + f.write_str(match self { + Self::WrongPassphrase => "wrong passphrase", + Self::Decrypt => "decryption/integrity check failed", + Self::KdfFailure => "key derivation failed", + Self::VersionUnsupported => "unsupported vault format version", + Self::MalformedVault => "malformed vault file", + Self::InsecurePermissions => "vault file has insecure permissions", + }) + } +} + +impl StdError for FileStoreFailure {} + +/// Lift a [`FileStoreError`] into a `keyring_core::Error` for the +/// `CredentialApi` / `CredentialStoreApi` seam. +/// +/// - `WrongPassphrase` rides inside +/// [`KeyringError::NoStorageAccess`] (operator UX: "ask the operator +/// to unlock" — same family as today's `KeyringLocked` mapping). +/// - `Decrypt`/`KdfFailure`/`VersionUnsupported`/`MalformedVault`/ +/// `InsecurePermissions` ride inside [`KeyringError::BadStoreFormat`] +/// with a static `String` — the structural marker is recovered by +/// downcasting the source. Per Smythe EDIT-2 we never put secret +/// data in `BadDataFormat`/`BadEncoding`. +/// - `InvalidLabel` becomes +/// `KeyringError::Invalid("user", "")`. +/// - `Io` becomes `KeyringError::PlatformFailure(io_err)`. +pub fn into_keyring(e: FileStoreError) -> KeyringError { + match e { + FileStoreError::WrongPassphrase => { + KeyringError::NoStorageAccess(Box::new(FileStoreFailure::WrongPassphrase)) + } + FileStoreError::Decrypt => bad_format(FileStoreFailure::Decrypt), + FileStoreError::KdfFailure => bad_format(FileStoreFailure::KdfFailure), + FileStoreError::VersionUnsupported { .. } => bad_format(FileStoreFailure::VersionUnsupported), + FileStoreError::MalformedVault => bad_format(FileStoreFailure::MalformedVault), + FileStoreError::InsecurePermissions { .. } => bad_format(FileStoreFailure::InsecurePermissions), + FileStoreError::InvalidLabel => { + KeyringError::Invalid("user".to_string(), "label allowlist violation".to_string()) + } + FileStoreError::Io(io) => KeyringError::PlatformFailure(Box::new(io)), + } +} + +/// `BadStoreFormat` with the marker both in the boxed `source()` chain +/// and as the rendered string — keeps Display informative while letting +/// downcast recover the structural variant. +fn bad_format(failure: FileStoreFailure) -> KeyringError { + KeyringError::BadStoreFormat(failure.to_string()) +} + +/// Recover a [`FileStoreFailure`] from a `keyring_core::Error`, if +/// the error was produced by the file backend's [`into_keyring`]. +/// Returns `None` for non-file-backend errors and for variants the +/// bridge does not carry a marker on (e.g. `BadStoreFormat`'s +/// `String`-only variant — see callers' fallback handling). +pub fn downcast_failure(e: &KeyringError) -> Option { + let src: &(dyn StdError + 'static) = match e { + KeyringError::NoStorageAccess(inner) => inner.as_ref(), + // `BadStoreFormat` carries only a `String` payload, so its + // structural marker is read off the rendered text below. + KeyringError::BadStoreFormat(s) => return marker_from_message(s), + _ => return None, + }; + src.downcast_ref::().copied() +} + +fn marker_from_message(s: &str) -> Option { + for f in [ + FileStoreFailure::Decrypt, + FileStoreFailure::KdfFailure, + FileStoreFailure::VersionUnsupported, + FileStoreFailure::MalformedVault, + FileStoreFailure::InsecurePermissions, + FileStoreFailure::WrongPassphrase, + ] { + if s == f.to_string() { + return Some(f); + } + } + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn wrong_passphrase_round_trips_via_no_storage_access() { + let k = into_keyring(FileStoreError::WrongPassphrase); + assert!(matches!(k, KeyringError::NoStorageAccess(_))); + assert_eq!( + downcast_failure(&k), + Some(FileStoreFailure::WrongPassphrase) + ); + } + + #[test] + fn bad_store_format_markers_round_trip() { + for (err, expected) in [ + (FileStoreError::Decrypt, FileStoreFailure::Decrypt), + (FileStoreError::KdfFailure, FileStoreFailure::KdfFailure), + ( + FileStoreError::VersionUnsupported { found: 999 }, + FileStoreFailure::VersionUnsupported, + ), + (FileStoreError::MalformedVault, FileStoreFailure::MalformedVault), + ( + FileStoreError::InsecurePermissions { mode: 0o644 }, + FileStoreFailure::InsecurePermissions, + ), + ] { + let k = into_keyring(err); + assert!(matches!(k, KeyringError::BadStoreFormat(_))); + assert_eq!(downcast_failure(&k), Some(expected)); + } + } + + #[test] + fn invalid_label_maps_to_invalid_user() { + let k = into_keyring(FileStoreError::InvalidLabel); + match k { + KeyringError::Invalid(attr, _) => assert_eq!(attr, "user"), + other => panic!("expected Invalid, got {other:?}"), + } + } + + #[test] + fn io_maps_to_platform_failure() { + let io = std::io::Error::other("boom"); + let k = into_keyring(FileStoreError::Io(io)); + assert!(matches!(k, KeyringError::PlatformFailure(_))); + } + + #[test] + fn downcast_returns_none_for_unrelated_errors() { + assert!(downcast_failure(&KeyringError::NoEntry).is_none()); + assert!(downcast_failure(&KeyringError::NoDefaultStore).is_none()); + } + + /// `FileStoreFailure` is unit-variants only (Smythe EDIT-3): no + /// field may carry user-supplied or secret data. The `Copy` bound + /// is the structural witness — only enums whose variants hold + /// `Copy` data can derive it. + const _: () = { + const fn _assert_copy() {} + _assert_copy::(); + }; +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs index 8dfaaacd7d..fd20a95a33 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs @@ -23,7 +23,7 @@ //! fails its tag, so a mismatched key is rejected before any entry is //! written or read (no mixed-key corruption). -use super::super::error::SecretStoreError; +use super::error::FileStoreError; use super::crypto::{KdfParams, NONCE_LEN, SALT_LEN}; pub(crate) const MAGIC: &[u8; 9] = b"PWSVAULT1"; @@ -111,29 +111,29 @@ struct Reader<'a> { } impl<'a> Reader<'a> { - fn take(&mut self, n: usize) -> Result<&'a [u8], SecretStoreError> { + fn take(&mut self, n: usize) -> Result<&'a [u8], FileStoreError> { let end = self .pos .checked_add(n) - .ok_or(SecretStoreError::MalformedVault)?; + .ok_or(FileStoreError::MalformedVault)?; let s = self .buf .get(self.pos..end) - .ok_or(SecretStoreError::MalformedVault)?; + .ok_or(FileStoreError::MalformedVault)?; self.pos = end; Ok(s) } - fn u8(&mut self) -> Result { + fn u8(&mut self) -> Result { Ok(self.take(1)?[0]) } - fn u16(&mut self) -> Result { + fn u16(&mut self) -> Result { let b = self.take(2)?; Ok(u16::from_le_bytes([b[0], b[1]])) } - fn u32(&mut self) -> Result { + fn u32(&mut self) -> Result { let b = self.take(4)?; Ok(u32::from_le_bytes([b[0], b[1], b[2], b[3]])) } @@ -141,24 +141,24 @@ impl<'a> Reader<'a> { /// Parse a vault. Refuses unknown magic/version (fail closed, /// SEC-REQ-2.2.9); parameter floors are enforced later at derive time. -pub(crate) fn deserialize(buf: &[u8]) -> Result<(Header, Vec), SecretStoreError> { +pub(crate) fn deserialize(buf: &[u8]) -> Result<(Header, Vec), FileStoreError> { let mut r = Reader { buf, pos: 0 }; if r.take(MAGIC.len())? != MAGIC { - return Err(SecretStoreError::MalformedVault); + return Err(FileStoreError::MalformedVault); } let version = r.u32()?; if version != FORMAT_VERSION { - return Err(SecretStoreError::VersionUnsupported { found: version }); + return Err(FileStoreError::VersionUnsupported { found: version }); } if r.u8()? != KDF_ID_ARGON2ID { - return Err(SecretStoreError::MalformedVault); + return Err(FileStoreError::MalformedVault); } let m_kib = r.u32()?; let t = r.u32()?; let p = r.u32()?; let salt_len = r.u8()? as usize; if salt_len != SALT_LEN { - return Err(SecretStoreError::MalformedVault); + return Err(FileStoreError::MalformedVault); } let mut salt = [0u8; SALT_LEN]; salt.copy_from_slice(r.take(SALT_LEN)?); @@ -171,7 +171,7 @@ pub(crate) fn deserialize(buf: &[u8]) -> Result<(Header, Vec), SecretStor while r.pos < buf.len() { let label_len = r.u16()? as usize; let label = std::str::from_utf8(r.take(label_len)?) - .map_err(|_| SecretStoreError::MalformedVault)? + .map_err(|_| FileStoreError::MalformedVault)? .to_string(); let mut nonce = [0u8; NONCE_LEN]; nonce.copy_from_slice(r.take(NONCE_LEN)?); @@ -251,14 +251,14 @@ mod tests { fn rejects_bad_magic_and_unknown_version() { assert!(matches!( deserialize(b"NOPENOPE...."), - Err(SecretStoreError::MalformedVault) + Err(FileStoreError::MalformedVault) )); let mut bytes = serialize(&test_header(), &[]); let v = MAGIC.len(); bytes[v..v + 4].copy_from_slice(&999u32.to_le_bytes()); assert!(matches!( deserialize(&bytes), - Err(SecretStoreError::VersionUnsupported { found: 999 }) + Err(FileStoreError::VersionUnsupported { found: 999 }) )); } @@ -267,7 +267,7 @@ mod tests { let bytes = serialize(&test_header(), &[]); assert!(matches!( deserialize(&bytes[..bytes.len() - 5]), - Err(SecretStoreError::MalformedVault) + Err(FileStoreError::MalformedVault) )); } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs index 15e8aaf4d8..b67dc3e2c3 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -3,7 +3,8 @@ //! One vault file per `wallet_id` (path namespaced by `wallet_id` //! hex). Argon2id KDF + XChaCha20-Poly1305 AEAD, AAD-bound to //! `(format_version, wallet_id, label)`, written atomically at mode -//! 0600. +//! 0600. Implements the upstream `keyring_core::api::CredentialStoreApi` +//! SPI; per-`(service, user)` credentials implement `CredentialApi`. //! //! ## Threat coverage //! @@ -17,25 +18,42 @@ //! zeroize + mlock, not eliminated). mod crypto; +pub(crate) mod error; +pub(crate) mod error_bridge; mod format; +use std::any::Any; +use std::collections::HashMap; use std::fs::{self, OpenOptions}; use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Arc; + +use keyring_core::api::{Credential, CredentialApi, CredentialPersistence, CredentialStoreApi}; +use keyring_core::{Entry, Error as KeyringError, Result as KeyringResult}; use crypto::{KdfParams, SALT_LEN}; -use format::{Entry, Header}; +use error::FileStoreError; +use error_bridge::into_keyring; +use format::{Entry as VaultEntry, Header}; + +use super::secret::{SecretBytes, SecretString}; +use super::validate::{validated_label, WalletId}; /// Process-local counter for unique temp-file names (C7). static COUNTER: AtomicU64 = AtomicU64::new(0); -use super::error::SecretStoreError; -use super::secret::{SecretBytes, SecretString}; -use super::store::SecretStore; -use super::validate::{validated_label, WalletId}; +/// Upstream service-prefix for vault entries. The full `service` +/// string is `SERVICE_PREFIX + hex(wallet_id)`, mapping each wallet +/// to its own keyring "service" namespace. +pub const SERVICE_PREFIX: &str = "dash.platform-wallet-storage/"; -/// A passphrase-encrypted file-backed [`SecretStore`]. +/// Vendor / id tags published through `CredentialStoreApi`. +const VENDOR: &str = "dash.platform-wallet-storage"; +const STORE_ID: &str = "encrypted-file-store-v1"; + +/// A passphrase-encrypted file-backed credential store. /// /// The passphrase is held in a [`SecretString`] for the store's /// lifetime so each operation can re-derive the per-vault key; it is @@ -44,6 +62,13 @@ use super::validate::{validated_label, WalletId}; /// and dropped (zeroized) immediately after use — it is never retained /// on the struct. pub struct EncryptedFileStore { + inner: Arc, +} + +/// Reference-counted backing so credentials returned from +/// [`CredentialStoreApi::build`] hold a clone of the store without +/// keeping the public handle alive. +struct EncryptedFileStoreInner { dir: PathBuf, passphrase: SecretString, } @@ -51,12 +76,58 @@ pub struct EncryptedFileStore { impl EncryptedFileStore { /// Open (or prepare to create) a vault store rooted at `dir`, /// unlocked by `passphrase`. `dir` is created if missing. - pub fn open(dir: impl AsRef, passphrase: SecretString) -> Result { + pub fn open( + dir: impl AsRef, + passphrase: SecretString, + ) -> Result { let dir = dir.as_ref().to_path_buf(); fs::create_dir_all(&dir)?; - Ok(Self { dir, passphrase }) + Ok(Self { + inner: Arc::new(EncryptedFileStoreInner { dir, passphrase }), + }) } + /// Re-encrypt every entry for `wallet_id` under a fresh salt + + /// fresh per-entry nonces, then atomically replace the vault. No + /// `.bak` retains old key material (SEC-REQ-2.2.12). Replaces this + /// store's passphrase atomically on success. + pub fn rekey( + &mut self, + wallet_id: WalletId, + new_passphrase: SecretString, + ) -> Result<(), FileStoreError> { + // The store must hold a unique reference so the swap is + // observable to every outstanding credential consistently. + let inner = Arc::get_mut(&mut self.inner) + .expect("rekey requires exclusive access to the store"); + inner.rekey(wallet_id, new_passphrase) + } + + #[cfg(test)] + fn vault_path(&self, wallet_id: &WalletId) -> PathBuf { + self.inner.vault_path(wallet_id) + } + + #[cfg(test)] + fn read_vault( + &self, + path: &Path, + ) -> Result)>, FileStoreError> { + self.inner.read_vault(path) + } + + #[cfg(test)] + fn write_vault( + &self, + path: &Path, + header: &Header, + entries: &[VaultEntry], + ) -> Result<(), FileStoreError> { + self.inner.write_vault(path, header, entries) + } +} + +impl EncryptedFileStoreInner { fn vault_path(&self, wallet_id: &WalletId) -> PathBuf { self.dir.join(format!("{}.pwsvault", wallet_id.to_hex())) } @@ -69,7 +140,7 @@ impl EncryptedFileStore { &self, wallet_id: &WalletId, passphrase: &SecretString, - ) -> Result<(Header, SecretBytes), SecretStoreError> { + ) -> Result<(Header, SecretBytes), FileStoreError> { let mut salt = [0u8; SALT_LEN]; crypto::random_bytes(&mut salt)?; let params = KdfParams::default_target(); @@ -96,7 +167,7 @@ impl EncryptedFileStore { &self, wallet_id: &WalletId, header: &Header, - ) -> Result { + ) -> Result { let key = crypto::derive_key( self.passphrase.expose_secret().as_bytes(), &header.salt, @@ -105,7 +176,7 @@ impl EncryptedFileStore { let v_aad = format::verify_aad(format::FORMAT_VERSION, wallet_id.as_bytes()); match crypto::open(&key, &header.verify_nonce, &v_aad, &header.verify_ct) { Ok(_) => Ok(key), - Err(SecretStoreError::Decrypt) => Err(SecretStoreError::WrongPassphrase), + Err(FileStoreError::Decrypt) => Err(FileStoreError::WrongPassphrase), Err(e) => Err(e), } } @@ -113,7 +184,10 @@ impl EncryptedFileStore { /// Read + parse a vault file, or `None` if it does not exist. /// Refuses a pre-existing file with looser-than-0600 perms /// (SEC-REQ-2.2.10). - fn read_vault(&self, path: &Path) -> Result)>, SecretStoreError> { + fn read_vault( + &self, + path: &Path, + ) -> Result)>, FileStoreError> { match fs::metadata(path) { Ok(meta) => { check_perms(&meta)?; @@ -133,8 +207,8 @@ impl EncryptedFileStore { &self, path: &Path, header: &Header, - entries: &[Entry], - ) -> Result<(), SecretStoreError> { + entries: &[VaultEntry], + ) -> Result<(), FileStoreError> { let serialized = format::serialize(header, entries); // Unique temp name (pid + monotonic counter) created with // O_EXCL — no fixed name and no destination pre-remove, so a @@ -142,7 +216,7 @@ impl EncryptedFileStore { // collide on the temp (Marvin QA-004). let unique = COUNTER.fetch_add(1, Ordering::Relaxed); let tmp = path.with_extension(format!("pwsvault.tmp.{}.{unique}", std::process::id())); - let result = (|| -> Result<(), SecretStoreError> { + let result = (|| -> Result<(), FileStoreError> { let mut opts = OpenOptions::new(); opts.write(true).create_new(true); set_create_mode(&mut opts); @@ -165,15 +239,11 @@ impl EncryptedFileStore { result } - /// Re-encrypt every entry under a fresh salt + fresh per-entry - /// nonces with the current default Argon2 params and atomically - /// replace the vault — no `.bak` retains old key material - /// (SEC-REQ-2.2.12). - pub fn rekey( + fn rekey( &mut self, wallet_id: WalletId, new_passphrase: SecretString, - ) -> Result<(), SecretStoreError> { + ) -> Result<(), FileStoreError> { let path = self.vault_path(&wallet_id); let Some((old_header, old_entries)) = self.read_vault(&path)? else { self.passphrase = new_passphrase; @@ -186,9 +256,9 @@ impl EncryptedFileStore { for e in &old_entries { let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), &e.label); let pt = crypto::open(&old_key, &e.nonce, &aad, &e.ciphertext) - .map_err(|_| SecretStoreError::WrongPassphrase)?; + .map_err(|_| FileStoreError::WrongPassphrase)?; let (nonce, ct) = crypto::seal(&new_key, &aad, pt.expose_secret())?; - new_entries.push(Entry { + new_entries.push(VaultEntry { label: e.label.clone(), nonce, ciphertext: ct, @@ -198,26 +268,30 @@ impl EncryptedFileStore { self.passphrase = new_passphrase; Ok(()) } -} -impl SecretStore for EncryptedFileStore { - fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { + /// `put` — overwrite-safe atomic seal under `(wallet_id, label)`. + fn put( + &self, + wallet_id: &WalletId, + label: &str, + bytes: &[u8], + ) -> Result<(), FileStoreError> { let label = validated_label(label)?.to_string(); - let path = self.vault_path(&wallet_id); + let path = self.vault_path(wallet_id); let (header, key, mut entries) = match self.read_vault(&path)? { Some((header, entries)) => { - let key = self.derive_and_verify(&wallet_id, &header)?; + let key = self.derive_and_verify(wallet_id, &header)?; (header, key, entries) } None => { - let (header, key) = self.new_header(&wallet_id, &self.passphrase)?; + let (header, key) = self.new_header(wallet_id, &self.passphrase)?; (header, key, Vec::new()) } }; let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), &label); let (nonce, ciphertext) = crypto::seal(&key, &aad, bytes)?; entries.retain(|e| e.label != label); - entries.push(Entry { + entries.push(VaultEntry { label, nonce, ciphertext, @@ -225,58 +299,207 @@ impl SecretStore for EncryptedFileStore { self.write_vault(&path, &header, &entries) } + /// `get` — returns the raw plaintext as `Vec` (the upstream + /// SPI contract). Callers wrap into [`SecretBytes`] at the seam. + /// `NoEntry`-shaped absence rides as `Ok(None)`. fn get( &self, - wallet_id: WalletId, + wallet_id: &WalletId, label: &str, - ) -> Result, SecretStoreError> { + ) -> Result>, FileStoreError> { let label = validated_label(label)?; - let path = self.vault_path(&wallet_id); + let path = self.vault_path(wallet_id); let Some((header, entries)) = self.read_vault(&path)? else { return Ok(None); }; - let key = self.derive_and_verify(&wallet_id, &header)?; + let key = self.derive_and_verify(wallet_id, &header)?; let Some(entry) = entries.iter().find(|e| e.label == label) else { return Ok(None); }; let aad = format::aad(format::FORMAT_VERSION, wallet_id.as_bytes(), label); match crypto::open(&key, &entry.nonce, &aad, &entry.ciphertext) { - Ok(pt) => Ok(Some(pt)), - Err(SecretStoreError::Decrypt) => Err(SecretStoreError::WrongPassphrase), + Ok(pt) => Ok(Some(pt.expose_secret().to_vec())), + Err(FileStoreError::Decrypt) => Err(FileStoreError::WrongPassphrase), Err(e) => Err(e), } } - fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { + /// `delete` — upstream-compliant: returns whether an entry was + /// removed so the SPI seam can surface `NoEntry` (D3, per the + /// `CredentialApi::delete_credential` contract). + fn delete(&self, wallet_id: &WalletId, label: &str) -> Result { let label = validated_label(label)?; - let path = self.vault_path(&wallet_id); + let path = self.vault_path(wallet_id); let Some((header, mut entries)) = self.read_vault(&path)? else { - return Ok(()); + return Ok(false); }; // Verify the passphrase before mutating, so a wrong pass can // neither delete an entry nor rewrite the vault. - self.derive_and_verify(&wallet_id, &header)?; + self.derive_and_verify(wallet_id, &header)?; let before = entries.len(); entries.retain(|e| e.label != label); if entries.len() == before { - return Ok(()); + return Ok(false); + } + self.write_vault(&path, &header, &entries)?; + Ok(true) + } +} + +/// Parse a `service` string into a [`WalletId`]. The slash-prefixed +/// allowlist-disjoint shape (`label` never contains `/`) means an +/// attacker-controlled label cannot smuggle a bogus wallet id. +fn parse_service(service: &str) -> Result { + let Some(hex) = service.strip_prefix(SERVICE_PREFIX) else { + return Err(KeyringError::Invalid( + "service".to_string(), + "expected dash.platform-wallet-storage/".to_string(), + )); + }; + if hex.len() != 64 { + return Err(KeyringError::Invalid( + "service".to_string(), + "wallet id hex must be 64 chars".to_string(), + )); + } + let mut bytes = [0u8; 32]; + hex::decode_to_slice(hex, &mut bytes).map_err(|_| { + KeyringError::Invalid( + "service".to_string(), + "wallet id hex is not lowercase hex".to_string(), + ) + })?; + Ok(WalletId::from(bytes)) +} + +/// A `(wallet_id, label)` row in an [`EncryptedFileStore`]. +/// +/// All four operations re-validate `user` (label) and re-derive the +/// per-vault key (so a wrong passphrase fails closed at every call) — +/// defence in depth; the credential is long-lived and the cached +/// fields are reachable through `get_specifiers`. +pub struct EncryptedFileCredential { + store: Arc, + wallet_id: WalletId, + label: String, +} + +impl std::fmt::Debug for EncryptedFileCredential { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("EncryptedFileCredential") + .field("wallet_id", &self.wallet_id.to_hex()) + .field("label", &self.label) + .finish_non_exhaustive() + } +} + +impl CredentialApi for EncryptedFileCredential { + fn set_secret(&self, secret: &[u8]) -> KeyringResult<()> { + // Re-validate at every op (defence in depth, M-2 / SEC-REQ-4.3). + let _ = validated_label(&self.label) + .map_err(FileStoreError::from) + .map_err(into_keyring)?; + self.store + .put(&self.wallet_id, &self.label, secret) + .map_err(into_keyring) + } + + fn get_secret(&self) -> KeyringResult> { + let _ = validated_label(&self.label) + .map_err(FileStoreError::from) + .map_err(into_keyring)?; + match self.store.get(&self.wallet_id, &self.label) { + Ok(Some(v)) => Ok(v), + Ok(None) => Err(KeyringError::NoEntry), + Err(e) => Err(into_keyring(e)), } - self.write_vault(&path, &header, &entries) + } + + fn delete_credential(&self) -> KeyringResult<()> { + let _ = validated_label(&self.label) + .map_err(FileStoreError::from) + .map_err(into_keyring)?; + match self.store.delete(&self.wallet_id, &self.label) { + Ok(true) => Ok(()), + Ok(false) => Err(KeyringError::NoEntry), + Err(e) => Err(into_keyring(e)), + } + } + + fn get_credential(&self) -> KeyringResult>> { + // Every entry is already a specifier — no wrapper layer. + Ok(None) + } + + fn get_specifiers(&self) -> Option<(String, String)> { + Some(( + format!("{SERVICE_PREFIX}{}", self.wallet_id.to_hex()), + self.label.clone(), + )) + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +impl CredentialStoreApi for EncryptedFileStore { + fn vendor(&self) -> String { + VENDOR.to_string() + } + + fn id(&self) -> String { + STORE_ID.to_string() + } + + fn build( + &self, + service: &str, + user: &str, + _modifiers: Option<&HashMap<&str, &str>>, + ) -> KeyringResult { + let wallet_id = parse_service(service)?; + let label = validated_label(user) + .map_err(FileStoreError::from) + .map_err(into_keyring)? + .to_string(); + let cred = EncryptedFileCredential { + store: self.inner.clone(), + wallet_id, + label, + }; + Ok(Entry::new_with_credential(Arc::new(cred))) + } + + fn as_any(&self) -> &dyn Any { + self + } + + fn persistence(&self) -> CredentialPersistence { + CredentialPersistence::UntilDelete + } +} + +impl std::fmt::Debug for EncryptedFileStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("EncryptedFileStore") + .field("dir", &self.inner.dir) + .finish_non_exhaustive() } } #[cfg(unix)] -fn check_perms(meta: &fs::Metadata) -> Result<(), SecretStoreError> { +fn check_perms(meta: &fs::Metadata) -> Result<(), FileStoreError> { use std::os::unix::fs::MetadataExt; let mode = meta.mode() & 0o777; if mode & 0o077 != 0 { - return Err(SecretStoreError::InsecurePermissions { mode }); + return Err(FileStoreError::InsecurePermissions { mode }); } Ok(()) } #[cfg(not(unix))] -fn check_perms(_meta: &fs::Metadata) -> Result<(), SecretStoreError> { +fn check_perms(_meta: &fs::Metadata) -> Result<(), FileStoreError> { Ok(()) } @@ -290,14 +513,14 @@ fn set_create_mode(opts: &mut OpenOptions) { fn set_create_mode(_opts: &mut OpenOptions) {} #[cfg(unix)] -fn enforce_mode_0600(f: &fs::File) -> Result<(), SecretStoreError> { +fn enforce_mode_0600(f: &fs::File) -> Result<(), FileStoreError> { use std::os::unix::fs::PermissionsExt; f.set_permissions(fs::Permissions::from_mode(0o600))?; Ok(()) } #[cfg(not(unix))] -fn enforce_mode_0600(_f: &fs::File) -> Result<(), SecretStoreError> { +fn enforce_mode_0600(_f: &fs::File) -> Result<(), FileStoreError> { Ok(()) } @@ -313,57 +536,77 @@ mod tests { WalletId::from([b; 32]) } + fn entry(s: &EncryptedFileStore, w: WalletId, label: &str) -> Entry { + let service = format!("{SERVICE_PREFIX}{}", w.to_hex()); + s.build(&service, label, None).expect("build") + } + #[test] fn roundtrip_persists_across_reopen() { let dir = tempfile::tempdir().unwrap(); { let s = store(dir.path()); - s.put(wid(1), "bip39_mnemonic", b"abandon abandon").unwrap(); + entry(&s, wid(1), "bip39_mnemonic") + .set_secret(b"abandon abandon") + .unwrap(); } let s2 = store(dir.path()); - let got = s2.get(wid(1), "bip39_mnemonic").unwrap().unwrap(); - assert_eq!(got.expose_secret(), b"abandon abandon"); - assert!(s2.get(wid(1), "missing").unwrap().is_none()); + let got = entry(&s2, wid(1), "bip39_mnemonic").get_secret().unwrap(); + assert_eq!(got, b"abandon abandon"); + let missing = entry(&s2, wid(1), "missing").get_secret().unwrap_err(); + assert!(matches!(missing, KeyringError::NoEntry)); } #[test] fn wrong_passphrase_fails_no_plaintext() { let dir = tempfile::tempdir().unwrap(); - store(dir.path()) - .put(wid(1), "seed", b"super secret") + entry(&store(dir.path()), wid(1), "seed") + .set_secret(b"super secret") .unwrap(); let bad = EncryptedFileStore::open(dir.path(), SecretString::new("pw-wrong")).unwrap(); - let err = bad.get(wid(1), "seed").unwrap_err(); - assert!(matches!(err, SecretStoreError::WrongPassphrase)); + let err = entry(&bad, wid(1), "seed").get_secret().unwrap_err(); + // The boxed `FileStoreFailure::WrongPassphrase` rides in + // `NoStorageAccess` per the bridge (D1). + assert_eq!( + error_bridge::downcast_failure(&err), + Some(error_bridge::FileStoreFailure::WrongPassphrase) + ); // The error renders without any plaintext. assert!(!format!("{err}").contains("super secret")); } #[test] - fn idempotent_delete_and_overwrite() { + fn delete_returns_no_entry_when_absent() { let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - s.delete(wid(1), "seed").unwrap(); // no vault yet - s.put(wid(1), "seed", b"v1").unwrap(); - s.put(wid(1), "seed", b"v2").unwrap(); - assert_eq!( - s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), - b"v2" - ); - s.delete(wid(1), "seed").unwrap(); - s.delete(wid(1), "seed").unwrap(); // idempotent - assert!(s.get(wid(1), "seed").unwrap().is_none()); + // No vault file at all → NoEntry per D3. + assert!(matches!( + entry(&s, wid(1), "seed").delete_credential(), + Err(KeyringError::NoEntry) + )); + entry(&s, wid(1), "seed").set_secret(b"v1").unwrap(); + entry(&s, wid(1), "seed").set_secret(b"v2").unwrap(); + assert_eq!(entry(&s, wid(1), "seed").get_secret().unwrap(), b"v2"); + entry(&s, wid(1), "seed").delete_credential().unwrap(); + // Second delete on the now-absent entry: NoEntry per D3. + assert!(matches!( + entry(&s, wid(1), "seed").delete_credential(), + Err(KeyringError::NoEntry) + )); + assert!(matches!( + entry(&s, wid(1), "seed").get_secret(), + Err(KeyringError::NoEntry) + )); } #[test] fn blob_swap_across_label_is_rejected() { let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - s.put(wid(1), "labelA", b"secretA").unwrap(); - s.put(wid(1), "labelB", b"secretB").unwrap(); + entry(&s, wid(1), "labelA").set_secret(b"secretA").unwrap(); + entry(&s, wid(1), "labelB").set_secret(b"secretB").unwrap(); let path = s.vault_path(&wid(1)); let (header, mut entries) = s.read_vault(&path).unwrap().unwrap(); - // Move A's ciphertext+nonce into B's slot. let a = entries .iter() .find(|e| e.label == "labelA") @@ -376,10 +619,18 @@ mod tests { } } s.write_vault(&path, &header, &entries).unwrap(); - assert!(matches!( - s.get(wid(1), "labelB"), - Err(SecretStoreError::WrongPassphrase) | Err(SecretStoreError::Decrypt) - )); + let err = entry(&s, wid(1), "labelB").get_secret().unwrap_err(); + // Either WrongPassphrase (via header verify) or Decrypt — both + // signal a tampered ciphertext. + let downcast = error_bridge::downcast_failure(&err); + assert!( + matches!( + downcast, + Some(error_bridge::FileStoreFailure::WrongPassphrase) + | Some(error_bridge::FileStoreFailure::Decrypt) + ), + "unexpected error: {err:?}" + ); } #[cfg(unix)] @@ -388,7 +639,7 @@ mod tests { use std::os::unix::fs::PermissionsExt; let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - s.put(wid(1), "seed", b"x").unwrap(); + entry(&s, wid(1), "seed").set_secret(b"x").unwrap(); let mode = fs::metadata(s.vault_path(&wid(1))) .unwrap() .permissions() @@ -403,27 +654,25 @@ mod tests { use std::os::unix::fs::PermissionsExt; let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - s.put(wid(1), "seed", b"x").unwrap(); + entry(&s, wid(1), "seed").set_secret(b"x").unwrap(); let path = s.vault_path(&wid(1)); fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap(); - assert!(matches!( - s.get(wid(1), "seed"), - Err(SecretStoreError::InsecurePermissions { mode: 0o644 }) - )); + let err = entry(&s, wid(1), "seed").get_secret().unwrap_err(); + assert_eq!( + error_bridge::downcast_failure(&err), + Some(error_bridge::FileStoreFailure::InsecurePermissions) + ); } #[test] fn rekey_reencrypts_and_old_passphrase_fails() { let dir = tempfile::tempdir().unwrap(); let mut s = store(dir.path()); - s.put(wid(1), "seed", b"value").unwrap(); + entry(&s, wid(1), "seed").set_secret(b"value").unwrap(); let old_bytes = fs::read(s.vault_path(&wid(1))).unwrap(); s.rekey(wid(1), SecretString::new("pw-new")).unwrap(); // New passphrase reads; ciphertext changed; no .bak left. - assert_eq!( - s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), - b"value" - ); + assert_eq!(entry(&s, wid(1), "seed").get_secret().unwrap(), b"value"); let new_bytes = fs::read(s.vault_path(&wid(1))).unwrap(); assert_ne!(old_bytes, new_bytes); let stale: Vec<_> = fs::read_dir(dir.path()) @@ -437,72 +686,79 @@ mod tests { .collect(); assert!(stale.is_empty(), "rekey left stale files: {stale:?}"); let old = EncryptedFileStore::open(dir.path(), SecretString::new("pw-correct")).unwrap(); - assert!(matches!( - old.get(wid(1), "seed"), - Err(SecretStoreError::WrongPassphrase) - )); + let err = entry(&old, wid(1), "seed").get_secret().unwrap_err(); + assert_eq!( + error_bridge::downcast_failure(&err), + Some(error_bridge::FileStoreFailure::WrongPassphrase) + ); } #[test] fn put_with_wrong_passphrase_to_existing_vault_is_rejected() { let dir = tempfile::tempdir().unwrap(); - store(dir.path()).put(wid(1), "seed", b"orig").unwrap(); + entry(&store(dir.path()), wid(1), "seed") + .set_secret(b"orig") + .unwrap(); let wrong = EncryptedFileStore::open(dir.path(), SecretString::new("pw-wrong")).unwrap(); // The defect: this used to write a mixed-key entry and return Ok. - let err = wrong.put(wid(1), "seed2", b"intruder").unwrap_err(); - assert!(matches!(err, SecretStoreError::WrongPassphrase)); - // Original vault still fully readable with the correct pass. - let ok = store(dir.path()); + let err = entry(&wrong, wid(1), "seed2") + .set_secret(b"intruder") + .unwrap_err(); assert_eq!( - ok.get(wid(1), "seed").unwrap().unwrap().expose_secret(), - b"orig" + error_bridge::downcast_failure(&err), + Some(error_bridge::FileStoreFailure::WrongPassphrase) ); + // Original vault still fully readable with the correct pass. + let ok = store(dir.path()); + assert_eq!(entry(&ok, wid(1), "seed").get_secret().unwrap(), b"orig"); // The rejected slot was never written. - assert!(ok.get(wid(1), "seed2").unwrap().is_none()); + assert!(matches!( + entry(&ok, wid(1), "seed2").get_secret(), + Err(KeyringError::NoEntry) + )); } #[test] fn get_and_delete_with_wrong_passphrase_are_rejected() { let dir = tempfile::tempdir().unwrap(); - store(dir.path()).put(wid(1), "seed", b"orig").unwrap(); + entry(&store(dir.path()), wid(1), "seed") + .set_secret(b"orig") + .unwrap(); let wrong = EncryptedFileStore::open(dir.path(), SecretString::new("pw-wrong")).unwrap(); - assert!(matches!( - wrong.get(wid(1), "seed"), - Err(SecretStoreError::WrongPassphrase) - )); - assert!(matches!( - wrong.delete(wid(1), "seed"), - Err(SecretStoreError::WrongPassphrase) - )); - // delete must not have mutated the vault. - let ok = store(dir.path()); + let get_err = entry(&wrong, wid(1), "seed").get_secret().unwrap_err(); assert_eq!( - ok.get(wid(1), "seed").unwrap().unwrap().expose_secret(), - b"orig" + error_bridge::downcast_failure(&get_err), + Some(error_bridge::FileStoreFailure::WrongPassphrase) ); + let del_err = entry(&wrong, wid(1), "seed") + .delete_credential() + .unwrap_err(); + assert_eq!( + error_bridge::downcast_failure(&del_err), + Some(error_bridge::FileStoreFailure::WrongPassphrase) + ); + // delete must not have mutated the vault. + let ok = store(dir.path()); + assert_eq!(entry(&ok, wid(1), "seed").get_secret().unwrap(), b"orig"); } #[test] fn correct_passphrase_round_trips_unchanged() { let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - s.put(wid(1), "seed", b"orig").unwrap(); - s.put(wid(1), "seed2", b"second").unwrap(); - assert_eq!( - s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), - b"orig" - ); - assert_eq!( - s.get(wid(1), "seed2").unwrap().unwrap().expose_secret(), - b"second" - ); + entry(&s, wid(1), "seed").set_secret(b"orig").unwrap(); + entry(&s, wid(1), "seed2").set_secret(b"second").unwrap(); + assert_eq!(entry(&s, wid(1), "seed").get_secret().unwrap(), b"orig"); + assert_eq!(entry(&s, wid(1), "seed2").get_secret().unwrap(), b"second"); } #[test] fn no_plaintext_in_vault_file() { let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - s.put(wid(1), "seed", b"PLAINTEXTNEEDLE").unwrap(); + entry(&s, wid(1), "seed") + .set_secret(b"PLAINTEXTNEEDLE") + .unwrap(); let raw = fs::read(s.vault_path(&wid(1))).unwrap(); assert!( raw.windows(b"PLAINTEXTNEEDLE".len()) @@ -510,4 +766,57 @@ mod tests { "plaintext leaked into vault file" ); } + + #[test] + fn build_rejects_malformed_service() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + for bad in [ + "no-prefix", + "dash.platform-wallet-storage/short", + // wrong prefix + "wrong-app/0000000000000000000000000000000000000000000000000000000000000000", + // non-hex in expected slot + "dash.platform-wallet-storage/zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", + ] { + let err = s.build(bad, "seed", None).unwrap_err(); + match err { + KeyringError::Invalid(attr, _) => assert_eq!(attr, "service"), + other => panic!("expected Invalid(\"service\"), got {other:?}"), + } + } + } + + #[test] + fn build_rejects_invalid_label() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + let service = format!("{SERVICE_PREFIX}{}", wid(1).to_hex()); + for bad in ["../escape", "", "lab el", "a:b"] { + let err = s.build(&service, bad, None).unwrap_err(); + match err { + KeyringError::Invalid(attr, _) => assert_eq!(attr, "user"), + other => panic!("expected Invalid(\"user\"), got {other:?}"), + } + } + } + + #[test] + fn get_specifiers_round_trip_the_pair() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + let e = entry(&s, wid(1), "seed"); + let (service, user) = e.get_specifiers().unwrap(); + assert_eq!(service, format!("{SERVICE_PREFIX}{}", wid(1).to_hex())); + assert_eq!(user, "seed"); + } + + #[test] + fn persistence_is_until_delete() { + let dir = tempfile::tempdir().unwrap(); + let s = store(dir.path()); + assert!(matches!(s.persistence(), CredentialPersistence::UntilDelete)); + assert_eq!(s.vendor(), VENDOR); + assert_eq!(s.id(), STORE_ID); + } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs index 53c34e00ff..ada73fe45d 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs @@ -1,11 +1,14 @@ -//! [`KeyringStore`] — OS keyring backend. +//! OS-keyring construction helper. //! -//! Built on `keyring-core 1.0.0` (the split-architecture library) plus -//! the per-platform credential-store crates; the `keyring` 4.x crate -//! itself is the sample CLI and is not a dependency here. +//! Built on `keyring-core 1.0.0` (the SPI library) plus the +//! per-platform credential-store crates; the `keyring` 4.x sample CLI +//! crate itself is intentionally not a dependency. //! -//! Delegates at-rest protection to the OS credential store. Its -//! security *is* the OS keyring's security. +//! There is no crate-local wrapper around the per-platform store: a +//! caller takes [`default_credential_store`]'s return value and either +//! uses it directly via [`keyring_core::api::CredentialStoreApi`] or +//! installs it as the process default via +//! [`keyring_core::set_default_store`]. //! //! ## Threat coverage //! @@ -14,8 +17,9 @@ //! Does **not** cover **A2/A3** same-user malware (most OS keyrings //! hand the secret to any same-user process that asks), **A5** if the //! keyring daemon itself is scraped, or **headless Linux** with no -//! Secret Service — that fails closed (`BackendUnavailable`), never -//! degrades to plaintext. +//! Secret Service — that fails closed +//! ([`keyring_core::Error::NoDefaultStore`]), never degrades to +//! plaintext. //! //! ### Per-OS reality //! @@ -31,70 +35,53 @@ use std::sync::Arc; -use keyring_core::api::CredentialStore; -use keyring_core::{Entry, Error as KeyringError}; +use keyring_core::api::CredentialStoreApi; +use keyring_core::Error as KeyringError; -use super::error::SecretStoreError; -use super::secret::SecretBytes; -use super::store::SecretStore; -use super::validate::{validated_label, WalletId}; - -/// Keyring `service` namespace — application-scoped so a different app -/// cannot silently read the entry (SEC-REQ-2.1.2). -const SERVICE: &str = "dash.platform-wallet-storage"; - -/// An OS-keyring-backed [`SecretStore`]. +/// Open the platform's default credential store, failing closed +/// (typed [`KeyringError::NoDefaultStore`]) when none is reachable +/// (headless / no Secret Service / no D-Bus). Never panics, never +/// falls back to a weaker store (SEC-REQ-2.1.3 / D2). /// -/// The `account` is `"{wallet_id_hex}:{label}"`, so two wallets cannot -/// collide. Only that non-secret index appears in keyring attributes — -/// never a secret byte (SEC-REQ-2.1.2, CWE-312). -pub struct KeyringStore { - store: Arc, -} - -impl KeyringStore { - /// Open the platform's default credential store, failing closed - /// (typed [`SecretStoreError::BackendUnavailable`]) when none is - /// reachable (headless / no Secret Service / no D-Bus). Never - /// panics, never falls back to a weaker store (SEC-REQ-2.1.3). - pub fn new() -> Result { - let store = default_store()?; - Ok(Self { store }) - } - - fn entry(&self, wallet_id: &WalletId, label: &str) -> Result { - let account = format!("{}:{}", wallet_id.to_hex(), label); - self.store - .build(SERVICE, &account, None) - .map_err(map_keyring_err) - } +/// The returned `Arc` may be passed straight to +/// [`keyring_core::set_default_store`] or used directly to build +/// entries. +pub fn default_credential_store( +) -> Result, KeyringError> { + platform_default_store() } #[cfg(any(target_os = "linux", target_os = "freebsd"))] -fn default_store() -> Result, SecretStoreError> { +fn platform_default_store( +) -> Result, KeyringError> { // Prefer the kernel keyutils store; fall back to Secret Service. // Both failing (headless, no session keyring, no D-Bus) is // fail-closed by design (SEC-REQ-2.1.3 / AR-4). if let Ok(s) = linux_keyutils_keyring_store::Store::new() { - return Ok(s as Arc); + return Ok(s); + } + match dbus_secret_service_keyring_store::Store::new() { + Ok(s) => Ok(s), + Err(_) => Err(KeyringError::NoDefaultStore), } - dbus_secret_service_keyring_store::Store::new() - .map(|s| s as Arc) - .map_err(map_keyring_err) } #[cfg(target_os = "macos")] -fn default_store() -> Result, SecretStoreError> { - apple_native_keyring_store::Store::new() - .map(|s| s as Arc) - .map_err(map_keyring_err) +fn platform_default_store( +) -> Result, KeyringError> { + match apple_native_keyring_store::Store::new() { + Ok(s) => Ok(s), + Err(_) => Err(KeyringError::NoDefaultStore), + } } #[cfg(target_os = "windows")] -fn default_store() -> Result, SecretStoreError> { - windows_native_keyring_store::Store::new() - .map(|s| s as Arc) - .map_err(map_keyring_err) +fn platform_default_store( +) -> Result, KeyringError> { + match windows_native_keyring_store::Store::new() { + Ok(s) => Ok(s), + Err(_) => Err(KeyringError::NoDefaultStore), + } } #[cfg(not(any( @@ -103,137 +90,23 @@ fn default_store() -> Result, SecretStoreError> { target_os = "macos", target_os = "windows" )))] -fn default_store() -> Result, SecretStoreError> { - Err(SecretStoreError::BackendUnavailable) -} - -/// Map keyring-core errors to the typed taxonomy. `NoEntry` is *not* -/// mapped here — callers translate it to `Ok(None)`/`Ok(())`. No -/// keyring error string is embedded (it could echo the `account`, -/// which is non-secret, but the taxonomy stays clean — SEC-REQ-2.0.1). -/// -/// Per keyring-core 1.0.0, `NoStorageAccess` covers the *locked* -/// collection case ("it might be that the credential store is -/// locked"), so it maps to [`SecretStoreError::KeyringLocked`] to -/// drive the unlock-retry UX (SEC-REQ-2.1.4). A genuinely absent -/// backend (`NoDefaultStore` / `PlatformFailure`) is -/// [`SecretStoreError::BackendUnavailable`]. -fn map_keyring_err(e: KeyringError) -> SecretStoreError { - match e { - KeyringError::NoEntry => SecretStoreError::NotFound, - KeyringError::NoStorageAccess(_) => SecretStoreError::KeyringLocked, - KeyringError::NoDefaultStore | KeyringError::PlatformFailure(_) => { - SecretStoreError::BackendUnavailable - } - _ => SecretStoreError::BackendUnavailable, - } -} - -impl SecretStore for KeyringStore { - fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { - let label = validated_label(label)?; - let entry = self.entry(&wallet_id, label)?; - entry.set_secret(bytes).map_err(map_keyring_err) - } - - fn get( - &self, - wallet_id: WalletId, - label: &str, - ) -> Result, SecretStoreError> { - let label = validated_label(label)?; - let entry = self.entry(&wallet_id, label)?; - match entry.get_secret() { - Ok(mut v) => { - let secret = SecretBytes::from_slice(&v); - // keyring-core returns a bare `Vec`; wipe the - // intermediate now that it is wrapped (SEC-REQ-3.1). - use zeroize::Zeroize; - v.zeroize(); - Ok(Some(secret)) - } - Err(KeyringError::NoEntry) => Ok(None), - Err(e) => Err(map_keyring_err(e)), - } - } - - fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { - let label = validated_label(label)?; - let entry = self.entry(&wallet_id, label)?; - match entry.delete_credential() { - Ok(()) | Err(KeyringError::NoEntry) => Ok(()), - Err(e) => Err(map_keyring_err(e)), - } - } +fn platform_default_store( +) -> Result, KeyringError> { + Err(KeyringError::NoDefaultStore) } #[cfg(test)] mod tests { use super::*; - #[test] - fn invalid_label_rejected_before_backend() { - // Label validation must precede any keyring access, so this - // is deterministic even on headless CI with no keyring. - if let Ok(s) = KeyringStore::new() { - assert!(matches!( - s.put(WalletId::from([0; 32]), "../escape", b"x"), - Err(SecretStoreError::InvalidLabel) - )); - } - } - - #[test] - fn locked_keyring_maps_to_keyring_locked() { - // keyring-core's `NoStorageAccess` covers the locked-collection - // case; it must surface as `KeyringLocked` so the caller can - // prompt for unlock (SEC-REQ-2.1.4), not as `BackendUnavailable`. - let locked = - KeyringError::NoStorageAccess(std::io::Error::other("collection is locked").into()); - assert!(matches!( - map_keyring_err(locked), - SecretStoreError::KeyringLocked - )); - // A genuinely absent backend stays `BackendUnavailable`. - assert!(matches!( - map_keyring_err(KeyringError::NoDefaultStore), - SecretStoreError::BackendUnavailable - )); - assert!(matches!( - map_keyring_err(KeyringError::NoEntry), - SecretStoreError::NotFound - )); - } - #[test] fn headless_fails_closed_not_panic() { - // On headless CI `new()` returns `BackendUnavailable`; where a - // keyring exists it succeeds. Either way: typed, no panic, no - // plaintext fallback. - match KeyringStore::new() { - Ok(_) | Err(SecretStoreError::BackendUnavailable) => {} + // On headless CI the constructor returns `NoDefaultStore`; + // where a keyring exists it succeeds. Either way: typed, no + // panic, no plaintext fallback (SEC-REQ-2.1.3 / D2). + match default_credential_store() { + Ok(_) | Err(KeyringError::NoDefaultStore) => {} Err(other) => panic!("unexpected: {other}"), } } - - /// Round-trip needs a live keyring; `#[ignore]` so headless CI does - /// not fail. Run locally on a desktop with an unlocked keyring: - /// `cargo test --features secrets keyring_roundtrip -- --ignored` - #[test] - #[ignore] - fn keyring_roundtrip_and_namespacing() { - let s = KeyringStore::new().expect("keyring available"); - let w1 = WalletId::from([1; 32]); - let w2 = WalletId::from([2; 32]); - s.put(w1, "seed", b"alpha").unwrap(); - s.put(w2, "seed", b"beta").unwrap(); - assert_eq!( - s.get(w1, "seed").unwrap().unwrap().expose_secret(), - b"alpha" - ); - assert_eq!(s.get(w2, "seed").unwrap().unwrap().expose_secret(), b"beta"); - s.delete(w1, "seed").unwrap(); - s.delete(w2, "seed").unwrap(); - assert!(s.get(w1, "seed").unwrap().is_none()); - } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/memory.rs b/packages/rs-platform-wallet-storage/src/secrets/memory.rs index d4d0a8f3ae..2cf2ee5b5f 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/memory.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/memory.rs @@ -1,4 +1,4 @@ -//! In-RAM [`SecretStore`] test double. +//! In-RAM [`CredentialStoreApi`] test double. //! //! Gated behind `__secrets-test-helpers` (Cargo's "MUST NOT enable from //! downstream" convention) so it is unreachable from production builds @@ -12,137 +12,205 @@ //! Covers **nothing at rest** — process RAM only, by design. Never use //! outside tests. +use std::any::Any; use std::collections::HashMap; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; + +use keyring_core::api::{Credential, CredentialApi, CredentialPersistence, CredentialStoreApi}; +use keyring_core::{Entry, Error as KeyringError, Result as KeyringResult}; -use super::error::SecretStoreError; use super::secret::SecretBytes; -use super::store::SecretStore; -use super::validate::{validated_label, WalletId}; +use super::validate::validated_label; + +const VENDOR: &str = "dash.platform-wallet-storage.memory"; +const STORE_ID: &str = "memory-credential-store-v1"; + +type StoreMap = HashMap<(String, String), SecretBytes>; -/// A `HashMap`-backed [`SecretStore`] for tests. No persistence, no +/// A `HashMap`-backed credential store for tests. No persistence, no /// encryption. Stored values sit in [`SecretBytes`] so even test /// memory zeroizes on drop (SEC-REQ-2.3.2). #[derive(Default)] -pub struct MemoryStore { - map: Mutex>, +pub struct MemoryCredentialStore { + map: Arc>, } -impl MemoryStore { +impl MemoryCredentialStore { /// A fresh empty store. pub fn new() -> Self { Self::default() } + + /// Convenience constructor returning the store as an + /// `Arc` for installation as + /// the keyring default or for handing to adapters. + pub fn new_arc() -> Arc { + Arc::new(Self::new()) + } } -impl SecretStore for MemoryStore { - fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError> { - let label = validated_label(label)?; - let mut map = self.map.lock().expect("MemoryStore mutex poisoned"); - map.insert( - (wallet_id, label.to_string()), - SecretBytes::from_slice(bytes), - ); - Ok(()) +impl std::fmt::Debug for MemoryCredentialStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MemoryCredentialStore").finish_non_exhaustive() } +} - fn get( +impl CredentialStoreApi for MemoryCredentialStore { + fn vendor(&self) -> String { + VENDOR.to_string() + } + + fn id(&self) -> String { + STORE_ID.to_string() + } + + fn build( &self, - wallet_id: WalletId, - label: &str, - ) -> Result, SecretStoreError> { - let label = validated_label(label)?; - let map = self.map.lock().expect("MemoryStore mutex poisoned"); - Ok(map - .get(&(wallet_id, label.to_string())) - .map(|v| SecretBytes::from_slice(v.expose_secret()))) - } - - fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError> { - let label = validated_label(label)?; - let mut map = self.map.lock().expect("MemoryStore mutex poisoned"); - map.remove(&(wallet_id, label.to_string())); + service: &str, + user: &str, + _modifiers: Option<&HashMap<&str, &str>>, + ) -> KeyringResult { + let label = validated_label(user).map_err(|_| { + KeyringError::Invalid("user".to_string(), "label allowlist violation".to_string()) + })?; + let cred = MemoryCredential { + map: self.map.clone(), + service: service.to_string(), + user: label.to_string(), + }; + Ok(Entry::new_with_credential(Arc::new(cred))) + } + + fn as_any(&self) -> &dyn Any { + self + } + + fn persistence(&self) -> CredentialPersistence { + CredentialPersistence::ProcessOnly + } +} + +/// One row in a [`MemoryCredentialStore`]. +pub struct MemoryCredential { + map: Arc>, + service: String, + user: String, +} + +impl std::fmt::Debug for MemoryCredential { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MemoryCredential") + .field("service", &self.service) + .field("user", &self.user) + .finish_non_exhaustive() + } +} + +impl CredentialApi for MemoryCredential { + fn set_secret(&self, secret: &[u8]) -> KeyringResult<()> { + let mut m = self.map.lock().expect("MemoryCredentialStore mutex poisoned"); + m.insert( + (self.service.clone(), self.user.clone()), + SecretBytes::from_slice(secret), + ); Ok(()) } + + fn get_secret(&self) -> KeyringResult> { + let m = self.map.lock().expect("MemoryCredentialStore mutex poisoned"); + match m.get(&(self.service.clone(), self.user.clone())) { + Some(v) => Ok(v.expose_secret().to_vec()), + None => Err(KeyringError::NoEntry), + } + } + + fn delete_credential(&self) -> KeyringResult<()> { + let mut m = self.map.lock().expect("MemoryCredentialStore mutex poisoned"); + match m.remove(&(self.service.clone(), self.user.clone())) { + Some(_) => Ok(()), + None => Err(KeyringError::NoEntry), + } + } + + fn get_credential(&self) -> KeyringResult>> { + Ok(None) + } + + fn get_specifiers(&self) -> Option<(String, String)> { + Some((self.service.clone(), self.user.clone())) + } + + fn as_any(&self) -> &dyn Any { + self + } } #[cfg(test)] mod tests { use super::*; - fn wid(b: u8) -> WalletId { - WalletId::from([b; 32]) + fn build(s: &MemoryCredentialStore, service: &str, user: &str) -> Entry { + s.build(service, user, None).expect("build") } #[test] fn roundtrip_and_overwrite() { - let s = MemoryStore::new(); - assert!(s.get(wid(1), "bip39_mnemonic").unwrap().is_none()); - s.put(wid(1), "bip39_mnemonic", &[1, 2, 3]).unwrap(); - assert_eq!( - s.get(wid(1), "bip39_mnemonic") - .unwrap() - .unwrap() - .expose_secret(), - &[1, 2, 3] - ); - s.put(wid(1), "bip39_mnemonic", &[4, 5]).unwrap(); - assert_eq!( - s.get(wid(1), "bip39_mnemonic") - .unwrap() - .unwrap() - .expose_secret(), - &[4, 5] - ); + let s = MemoryCredentialStore::new(); + let e = build(&s, "svc", "bip39_mnemonic"); + assert!(matches!(e.get_secret(), Err(KeyringError::NoEntry))); + e.set_secret(&[1, 2, 3]).unwrap(); + assert_eq!(e.get_secret().unwrap(), vec![1, 2, 3]); + e.set_secret(&[4, 5]).unwrap(); + assert_eq!(e.get_secret().unwrap(), vec![4, 5]); } #[test] - fn idempotent_delete_and_namespacing() { - let s = MemoryStore::new(); - s.put(wid(1), "seed", &[7]).unwrap(); - s.delete(wid(1), "seed").unwrap(); - s.delete(wid(1), "seed").unwrap(); // idempotent - assert!(s.get(wid(1), "seed").unwrap().is_none()); - - s.put(wid(1), "seed", &[1]).unwrap(); - s.put(wid(2), "seed", &[2]).unwrap(); - assert_eq!( - s.get(wid(1), "seed").unwrap().unwrap().expose_secret(), - &[1] - ); - assert_eq!( - s.get(wid(2), "seed").unwrap().unwrap().expose_secret(), - &[2] - ); + fn delete_returns_no_entry_when_absent_and_after_delete() { + let s = MemoryCredentialStore::new(); + let e = build(&s, "svc", "seed"); + assert!(matches!(e.delete_credential(), Err(KeyringError::NoEntry))); + e.set_secret(&[7]).unwrap(); + e.delete_credential().unwrap(); + assert!(matches!(e.delete_credential(), Err(KeyringError::NoEntry))); + assert!(matches!(e.get_secret(), Err(KeyringError::NoEntry))); + } + + #[test] + fn namespacing_across_service() { + let s = MemoryCredentialStore::new(); + let a = build(&s, "svc-a", "seed"); + let b = build(&s, "svc-b", "seed"); + a.set_secret(&[1]).unwrap(); + b.set_secret(&[2]).unwrap(); + assert_eq!(a.get_secret().unwrap(), vec![1]); + assert_eq!(b.get_secret().unwrap(), vec![2]); } - // The store must hold a zeroize-on-drop wrapper, not a bare - // `Vec` (SEC-REQ-2.3.2 / Marvin QA-002): the value type must - // run `Drop`. + // The map's value type must be a zeroize-on-drop wrapper, never a + // bare `Vec` (SEC-REQ-2.3.2). The compile-time witness: const _: () = { assert!(std::mem::needs_drop::()); }; #[test] fn stored_value_is_zeroizing_wrapper() { - let s = MemoryStore::new(); - s.put(wid(1), "seed", &[0xAB; 32]).unwrap(); + let s = MemoryCredentialStore::new(); + build(&s, "svc", "seed").set_secret(&[0xAB; 32]).unwrap(); let map = s.map.lock().unwrap(); // This binding only compiles if the value type is `SecretBytes`. - let v: &SecretBytes = map.get(&(wid(1), "seed".to_string())).unwrap(); + let v: &SecretBytes = map.get(&("svc".to_string(), "seed".to_string())).unwrap(); assert_eq!(v.expose_secret(), &[0xAB; 32]); } #[test] fn rejects_invalid_label() { - let s = MemoryStore::new(); - assert!(matches!( - s.put(wid(1), "../escape", &[0]), - Err(SecretStoreError::InvalidLabel) - )); - assert!(matches!( - s.get(wid(1), ""), - Err(SecretStoreError::InvalidLabel) - )); + let s = MemoryCredentialStore::new(); + for bad in ["../escape", "", "a b"] { + let err = s.build("svc", bad, None).unwrap_err(); + match err { + KeyringError::Invalid(attr, _) => assert_eq!(attr, "user"), + other => panic!("expected Invalid, got {other:?}"), + } + } } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index 202e3be4dd..f41872d21e 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -1,52 +1,60 @@ //! Out-of-band storage for wallet secret material (mnemonic / seed / //! xpriv), kept entirely off the SQLite persister's data path. //! -//! Enabled by the opt-in `secrets` feature (never on by `default`). +//! The SPI is upstream's +//! [`keyring_core::api::CredentialStoreApi`] / [`CredentialApi`]. +//! This crate contributes: +//! +//! - [`EncryptedFileStore`] — Argon2id + XChaCha20-Poly1305 vault file +//! `CredentialStoreApi` implementation. Recommended on **headless / +//! server** hosts; fully self-contained, no environment caveat. +//! - [`default_credential_store`] — opens the platform OS keyring as a +//! `CredentialStoreApi`, fail-closed with +//! [`keyring_core::Error::NoDefaultStore`] on headless Linux +//! (SEC-REQ-2.1.3 / AR-4). Recommended on **desktop** OSes. +//! - [`SecretBytes`] / [`SecretString`] — zeroize-on-drop wrappers +//! applied at the consumer seam (the upstream SPI returns bare +//! `Vec` from `get_secret`; we re-wrap immediately). +//! - [`FileStoreError`] / [`FileStoreFailure`] — file-backend +//! construction errors + the unit-only marker bridged into +//! `keyring_core::Error` for the `CredentialApi` seam. +//! +//! [`CredentialApi`]: keyring_core::api::CredentialApi +//! [`CredentialStoreApi`]: keyring_core::api::CredentialStoreApi +//! //! Everything secret-bearing lives under this `src/secrets/` tree by //! design: `tests/secrets_scan.rs` scans only `src/sqlite/schema/` + //! `migrations/` and exempts this module, so this module owns its own //! review discipline (`tests/secrets_guard.rs`, SEC-REQ-4.5/4.5.1). //! -//! # Backends & selection -//! -//! Two production backends ship; **selection is an explicit operator -//! decision — there is no silent fallback between them** (SEC-REQ-2.1.3 -//! / AR-4): -//! -//! - [`KeyringStore`] — OS keyring. Recommended default on **desktop** -//! OSes. Fails closed on headless Linux (no Secret Service) with a -//! typed [`SecretStoreError::BackendUnavailable`], never a degraded -//! plaintext store. -//! - [`EncryptedFileStore`] — Argon2id + XChaCha20-Poly1305 vault file. -//! Recommended default on **headless / server** hosts; fully -//! self-contained, no environment caveat. +//! # Memory hygiene //! -//! [`MemoryStore`] is test-only and gated so it is unreachable from -//! production builds. +//! The upstream SPI returns `Vec` from `get_secret`. Consumers +//! MUST wrap it via [`SecretBytes::new`] **immediately** (no named +//! intermediate `Vec` binding) so the bare buffer's window is zero +//! statements (Smythe EDIT-1): `SecretBytes::new` `std::mem::take`s +//! the `Vec` into a `Zeroizing>` without copying. //! -//! # Memory hygiene +//! # Backend selection //! -//! Secrets cross every boundary inside [`SecretBytes`] / [`SecretString`] -//! (zeroize-on-drop, redacting `Debug`, no `Display`/`Serialize`, -//! best-effort `mlock`). Errors are a concrete enum with no secret in -//! any variant. +//! Selection is an explicit operator decision — there is no silent +//! fallback between [`EncryptedFileStore`] and the OS keyring +//! (SEC-REQ-2.1.3 / AR-4). -mod error; mod file; mod keyring; mod secret; -mod store; mod validate; #[cfg(any(test, feature = "__secrets-test-helpers"))] mod memory; -pub use error::SecretStoreError; -pub use file::EncryptedFileStore; -pub use keyring::KeyringStore; +pub use file::error::FileStoreError; +pub use file::error_bridge::{downcast_failure, FileStoreFailure}; +pub use file::{EncryptedFileCredential, EncryptedFileStore, SERVICE_PREFIX}; +pub use keyring::default_credential_store; pub use secret::{SecretBytes, SecretString}; -pub use store::SecretStore; pub use validate::WalletId; #[cfg(any(test, feature = "__secrets-test-helpers"))] -pub use memory::MemoryStore; +pub use memory::{MemoryCredential, MemoryCredentialStore}; diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs deleted file mode 100644 index 6f60d4d00a..0000000000 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ /dev/null @@ -1,55 +0,0 @@ -//! The [`SecretStore`] port. - -use super::error::SecretStoreError; -use super::secret::SecretBytes; -use super::validate::WalletId; - -/// Stores wallet secret material out-of-band of the SQLite persister. -/// -/// Implementations MUST NOT write any secret byte to the database, its -/// WAL, backups, `tracing` events, `Debug`/`Display`, error payloads, -/// panic messages, or temp files outside their own controlled path -/// (the SECRETS.md invariant, SEC-REQ-2.0.1). -/// -/// All three methods validate `label` against the -/// `^[A-Za-z0-9._-]{1,64}$` allowlist before touching a backing store, -/// returning [`SecretStoreError::InvalidLabel`] on violation rather -/// than sanitizing. -pub trait SecretStore: Send + Sync { - /// Store `bytes` under `(wallet_id, label)`, overwrite-safe: an - /// existing label is atomically replaced or the call fails closed — - /// both old and new plaintext are never simultaneously recoverable - /// (SEC-REQ-2.0.2). - /// - /// The caller owns and must zeroize the source buffer; prefer - /// [`put_secret`](SecretStore::put_secret) so the source is a - /// `&SecretBytes`. The implementation MUST NOT copy `bytes` into a - /// long-lived unwrapped buffer. - fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) -> Result<(), SecretStoreError>; - - /// Retrieve the secret. `Ok(None)` for a missing label — idempotent - /// and non-secret-leaking (SEC-REQ-2.0.3). The returned buffer - /// zeroizes on drop (SEC-REQ-4.1); a bare `Vec` is never - /// returned. - fn get( - &self, - wallet_id: WalletId, - label: &str, - ) -> Result, SecretStoreError>; - - /// Idempotent delete. `Ok(())` whether or not the label existed; no - /// secret-bearing error distinguishes the two cases. - fn delete(&self, wallet_id: WalletId, label: &str) -> Result<(), SecretStoreError>; - - /// Ergonomic [`put`](SecretStore::put) over an already-wrapped - /// secret. Default impl forwards the exposed bytes; no extra - /// long-lived copy is made. - fn put_secret( - &self, - wallet_id: WalletId, - label: &str, - secret: &SecretBytes, - ) -> Result<(), SecretStoreError> { - self.put(wallet_id, label, secret.expose_secret()) - } -} diff --git a/packages/rs-platform-wallet-storage/src/secrets/validate.rs b/packages/rs-platform-wallet-storage/src/secrets/validate.rs index 2ecfac5464..b6a7ffff0b 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/validate.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/validate.rs @@ -1,13 +1,11 @@ -//! Input validation for the `SecretStore` key space (SEC-REQ-4.3). +//! Input validation for the `secrets` key space (SEC-REQ-4.3). //! //! `wallet_id` is fixed-width 32 bytes — enforced by the [`WalletId`] //! type, not at runtime. `label` is reject-not-sanitize against a //! strict allowlist before any backend maps it to a filename or a //! keyring attribute (CWE-22 path traversal, CWE-20 improper input). -use super::error::SecretStoreError; - -/// A 32-byte wallet identifier — the `SecretStore` namespace key. +/// A 32-byte wallet identifier — the per-vault namespace key. /// /// Public correlation material, **not** a secret (Smythe §1.1): it is /// derived from public wallet state, never from the seed's private @@ -37,10 +35,15 @@ impl From<[u8; 32]> for WalletId { /// Maximum `label` length, matching the allowlist's `{1,64}` bound. const MAX_LABEL_LEN: usize = 64; +/// Marker returned by [`validated_label`] on rejection. Backend +/// adapters lift this into their own typed error. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct InvalidLabel; + /// Validate a `label` against `^[A-Za-z0-9._-]{1,64}$` and return it /// unchanged on success. Rejects (never sanitizes) so a traversal / /// attribute-injection attempt is a hard error, not silently rewritten. -pub(crate) fn validated_label(label: &str) -> Result<&str, SecretStoreError> { +pub(crate) fn validated_label(label: &str) -> Result<&str, InvalidLabel> { let ok = (1..=MAX_LABEL_LEN).contains(&label.len()) && label .bytes() @@ -48,7 +51,7 @@ pub(crate) fn validated_label(label: &str) -> Result<&str, SecretStoreError> { if ok { Ok(label) } else { - Err(SecretStoreError::InvalidLabel) + Err(InvalidLabel) } } @@ -72,20 +75,20 @@ mod tests { #[test] fn rejects_traversal_and_injection() { for bad in [ - "", // empty - &"a".repeat(65), // too long - "../etc/passwd", // path traversal - "a/b", // separator - "a\\b", // windows separator - "a b", // space - "lab\0el", // NUL - "lab\nel", // newline - "café", // non-ASCII - "a:b", // keyring attribute delimiter - "a;DROP TABLE", // sql-ish + "", + &"a".repeat(65), + "../etc/passwd", + "a/b", + "a\\b", + "a b", + "lab\0el", + "lab\nel", + "café", + "a:b", + "a;DROP TABLE", ] { assert!( - matches!(validated_label(bad), Err(SecretStoreError::InvalidLabel)), + validated_label(bad).is_err(), "should reject {bad:?}" ); } diff --git a/packages/rs-platform-wallet-storage/tests/secrets_api.rs b/packages/rs-platform-wallet-storage/tests/secrets_api.rs index 509114621e..9e408e0f02 100644 --- a/packages/rs-platform-wallet-storage/tests/secrets_api.rs +++ b/packages/rs-platform-wallet-storage/tests/secrets_api.rs @@ -2,44 +2,58 @@ //! (SEC-REQ-4.1 / 4.4 / 4.5, TC-082 parity). //! //! Compiled only with `--features secrets`. Uses `EncryptedFileStore` -//! (always available under `secrets`); `MemoryStore` is intentionally -//! unreachable here (SEC-REQ-2.3.1) — it is exercised only by the -//! crate's in-module unit tests. +//! (always available under `secrets`); `MemoryCredentialStore` is +//! intentionally unreachable here (SEC-REQ-2.3.1) — it is exercised +//! only by the crate's own in-module unit tests behind +//! `__secrets-test-helpers`. #![cfg(feature = "secrets")] use std::path::Path; +use std::sync::Arc; +use keyring_core::api::CredentialStoreApi; +use keyring_core::{Error as KeyringError, Result as KeyringResult}; use platform_wallet_storage::secrets::{ - EncryptedFileStore, SecretBytes, SecretStore, SecretStoreError, SecretString, WalletId, + downcast_failure, EncryptedFileStore, FileStoreFailure, SecretBytes, SecretString, WalletId, + SERVICE_PREFIX, }; fn open(dir: &Path) -> EncryptedFileStore { EncryptedFileStore::open(dir, SecretString::new("test-pass")).unwrap() } -/// `SecretStore::get` returns `Option`, never a bare -/// `Vec` (SEC-REQ-4.1). This binding only compiles if the type is -/// exactly that. +fn service(w: WalletId) -> String { + format!("{SERVICE_PREFIX}{}", w.to_hex()) +} + +/// `CredentialApi::get_secret` returns `Vec` per upstream — we +/// re-wrap it via `SecretBytes::new` at the consumer seam (no named +/// intermediate `Vec` binding, Smythe EDIT-1). This binding only +/// compiles when the re-wrap type is exactly `SecretBytes`. #[test] -fn get_returns_zeroizing_wrapper_not_vec() { +fn get_secret_rewraps_into_zeroizing_at_consumer_seam() { let dir = tempfile::tempdir().unwrap(); let s = open(dir.path()); let w = WalletId::from([1; 32]); - s.put(w, "seed", b"abc").unwrap(); - let got: Option = s.get(w, "seed").unwrap(); - assert_eq!(got.unwrap().expose_secret(), b"abc"); + let entry = s.build(&service(w), "seed", None).unwrap(); + entry.set_secret(b"abc").unwrap(); + let wrapped: SecretBytes = SecretBytes::new(entry.get_secret().unwrap()); + assert_eq!(wrapped.expose_secret(), b"abc"); } -/// The secrets module is reachable, compiles, and round-trips through -/// `dyn SecretStore` (SEC-REQ-4.5 positive build guard). +/// The secrets module is reachable and the store is object-safe +/// behind `Arc` (SEC-REQ-4.5 +/// positive build guard). #[test] fn secrets_tree_builds_and_is_object_safe() { let dir = tempfile::tempdir().unwrap(); - let s: std::sync::Arc = std::sync::Arc::new(open(dir.path())); + let s: Arc = Arc::new(open(dir.path())); let w = WalletId::from([9; 32]); - s.put(w, "bip39_mnemonic", b"x").unwrap(); - assert!(s.get(w, "bip39_mnemonic").unwrap().is_some()); + let entry: KeyringResult<_> = s.build(&service(w), "bip39_mnemonic", None); + entry.unwrap().set_secret(b"x").unwrap(); + let e2 = s.build(&service(w), "bip39_mnemonic", None).unwrap(); + assert_eq!(e2.get_secret().unwrap(), b"x"); } /// No `Box` in the `secrets` tree's public surface — TC-082 @@ -72,8 +86,6 @@ fn no_box_dyn_error_in_secrets_src() { continue; }; for (i, line) in body.lines().enumerate() { - // The rule bans the *type* in code; prose explaining - // the rule (doc/line comments) is not a violation. let trimmed = line.trim_start(); if trimmed.starts_with("//") || trimmed.starts_with("*") { continue; @@ -87,24 +99,36 @@ fn no_box_dyn_error_in_secrets_src() { } } -/// The error enum carries no secret in `Display` (SEC-REQ-2.0.1 / -/// 3.3 / CWE-209). +/// The bridged `keyring_core::Error` carries no secret in `Display` +/// (SEC-REQ-2.0.1 / 3.3 / CWE-209). Per Smythe EDIT-2, `{:?}` is the +/// dangerous shape (it can echo `BadEncoding(Vec)` / +/// `BadDataFormat(Vec, _)`); the file backend never constructs +/// those variants with secret bytes, and our consumers must not +/// `{:?}`-print `keyring_core::Error` either (see `secrets_guard`). #[test] fn error_display_is_static_and_secret_free() { let dir = tempfile::tempdir().unwrap(); let store = open(dir.path()); let w = WalletId::from([4; 32]); - store.put(w, "seed", b"PLAINTEXTNEEDLE").unwrap(); + let entry = store.build(&service(w), "seed", None).unwrap(); + entry.set_secret(b"PLAINTEXTNEEDLE").unwrap(); + let bad = EncryptedFileStore::open(dir.path(), SecretString::new("wrong-pass")).unwrap(); - let err = bad.get(w, "seed").unwrap_err(); + let err = bad + .build(&service(w), "seed", None) + .unwrap() + .get_secret() + .unwrap_err(); let rendered = format!("{err}"); assert!(!rendered.contains("PLAINTEXTNEEDLE")); assert!(!rendered.contains("wrong-pass")); - assert_eq!(rendered, "wrong passphrase"); + assert_eq!(downcast_failure(&err), Some(FileStoreFailure::WrongPassphrase)); - let inv = store.put(w, "../bad", b"x").unwrap_err(); - assert!(matches!(inv, SecretStoreError::InvalidLabel)); - assert_eq!(format!("{inv}"), "invalid label"); + let inv = store.build(&service(w), "../bad", None).unwrap_err(); + match inv { + KeyringError::Invalid(attr, _) => assert_eq!(attr, "user"), + other => panic!("expected Invalid, got {other:?}"), + } } /// `SecretBytes`/`SecretString` `Debug` is redacted at the API diff --git a/packages/rs-platform-wallet-storage/tests/secrets_guard.rs b/packages/rs-platform-wallet-storage/tests/secrets_guard.rs index 67a0e145e8..5fddaaa6cb 100644 --- a/packages/rs-platform-wallet-storage/tests/secrets_guard.rs +++ b/packages/rs-platform-wallet-storage/tests/secrets_guard.rs @@ -94,3 +94,58 @@ fn no_secret_sink_in_secrets_module() { offenders.join("\n") ); } + +/// Smythe EDIT-2 — `keyring_core::Error` embeds raw `Vec` in +/// `BadEncoding` / `BadDataFormat`; `Display` is safe but `{:?}` is +/// dangerous. Forbid `{:?}` debug-formatting of any binding the seam +/// code holds as a `keyring_core::Error` inside `src/secrets/`. +/// +/// String-level scan: it flags `{:?}` paired with `KeyringError` / +/// `keyring_core::Error` on the same source line. The unit-test files +/// for the bridge necessarily print the error in assert messages — +/// those tests live in this `tests/` tree, not under `src/secrets/`. +#[test] +fn no_debug_format_of_keyring_error_in_secrets_module() { + const DEBUG_TOKENS: &[&str] = &["{:?}", "{e:?}", "{err:?}", "{:#?}"]; + const ERROR_NAMES: &[&str] = &["KeyringError", "keyring_core::Error"]; + + let manifest = Path::new(env!("CARGO_MANIFEST_DIR")); + let mut offenders = Vec::new(); + visit(&manifest.join("src/secrets"), &mut offenders); + assert!( + offenders.is_empty(), + "Smythe EDIT-2: `{{:?}}` debug-format paired with `keyring_core::Error` \ + in src/secrets/ (BadEncoding/BadDataFormat embed raw Vec):\n{}", + offenders.join("\n") + ); + + fn visit(dir: &Path, out: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let p = entry.path(); + if p.is_dir() { + visit(&p, out); + continue; + } + if p.extension().and_then(|e| e.to_str()) != Some("rs") { + continue; + } + let Ok(body) = std::fs::read_to_string(&p) else { + continue; + }; + for (idx, line) in body.lines().enumerate() { + let trimmed = line.trim_start(); + if trimmed.starts_with("//") || trimmed.starts_with("*") { + continue; + } + let has_dbg = DEBUG_TOKENS.iter().any(|t| line.contains(t)); + let has_err = ERROR_NAMES.iter().any(|n| line.contains(n)); + if has_dbg && has_err { + out.push(format!("{}:{}: {}", p.display(), idx + 1, line.trim())); + } + } + } + } +} From f733d3867a04e26fe93ed16ede778f9551b4c53e Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 20 May 2026 14:57:01 +0200 Subject: [PATCH 13/16] docs(platform-wallet-storage): SECRETS.md + lib root reflect keyring_core SPI Rewrites SECRETS.md as the present-state spec for the secrets submodule on the upstream `keyring_core::api` SPI: - Drops the retired `SecretStore` trait listing. - Documents the `service = "dash.platform-wallet-storage/" + hex(wid)`, `user = label` key shape with the allowlist precondition. - Memory hygiene section codifies Smythe EDIT-1: `SecretBytes::new(...)` is the consumer-seam wrapper, no named intermediate `Vec` binding. - Backends section: `EncryptedFileStore` + `default_credential_store()` + test-only `MemoryCredentialStore`. - Cross-SPI error bridge: `FileStoreFailure` unit-only marker (EDIT-3 constraint stated as load-bearing), `downcast_failure` recovery path, EDIT-2 `{:?}`-format ban on `keyring_core::Error` documented with its enforcement test. - Audit hooks section adds `secrets_off_state` (D4) and rephrases `secrets_guard` to cover both leak sinks. - Cargo features paragraph notes `secrets` is default-on; cargo-deny removal is noted via the lockfile-is-audit-coverage rationale. `src/lib.rs` crate-level doc retouched to point at the new SPI and backend names (the prior "SecretStore reserved" phrasing retired). `tests/secrets_scan.rs` exemption comment rephrased to describe the present state. Co-Authored-By: Claudius the Magnificent (1M context) --- .../rs-platform-wallet-storage/SECRETS.md | 147 ++++++++++++++---- .../rs-platform-wallet-storage/src/lib.rs | 17 +- .../src/secrets/file/crypto.rs | 2 +- .../src/secrets/file/error_bridge.rs | 24 +-- .../src/secrets/file/format.rs | 2 +- .../src/secrets/file/mod.rs | 37 ++--- .../src/secrets/keyring.rs | 16 +- .../src/secrets/memory.rs | 18 ++- .../src/secrets/validate.rs | 5 +- .../tests/secrets_api.rs | 5 +- .../tests/secrets_scan.rs | 5 +- 11 files changed, 181 insertions(+), 97 deletions(-) diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index da99971f58..9ffb319703 100644 --- a/packages/rs-platform-wallet-storage/SECRETS.md +++ b/packages/rs-platform-wallet-storage/SECRETS.md @@ -14,44 +14,106 @@ SQLite file the persister writes. ## The `secrets` submodule -`platform_wallet_storage::secrets` is gated behind the opt-in `secrets` -Cargo feature (never enabled by `default`). Enabling the feature -activates the module: it pulls the pinned crypto/keyring dependencies -and compiles `src/secrets/`. Secrets reach a backend only through this -trait — never through the SQLite persister DTO. +`platform_wallet_storage::secrets` is part of the crate's default +feature set. The SPI is upstream's +`keyring_core::api::{CredentialApi, CredentialStoreApi}` shipped by +`keyring-core 1.0.0`; this crate contributes backends and zeroizing +wrappers, not the trait surface. ```rust -pub trait SecretStore: Send + Sync { - fn put(&self, wallet_id: WalletId, label: &str, bytes: &[u8]) - -> Result<(), SecretStoreError>; - fn get(&self, wallet_id: WalletId, label: &str) - -> Result, SecretStoreError>; - fn delete(&self, wallet_id: WalletId, label: &str) - -> Result<(), SecretStoreError>; -} +use keyring_core::api::{CredentialApi, CredentialStoreApi}; +use platform_wallet_storage::secrets::{ + EncryptedFileStore, SecretBytes, SecretString, WalletId, SERVICE_PREFIX, +}; + +let store = EncryptedFileStore::open("/var/lib/wallet/vault", SecretString::new("pw"))?; +let service = format!("{SERVICE_PREFIX}{}", WalletId::from(wallet_id).to_hex()); +let entry = store.build(&service, "mnemonic", None)?; +entry.set_secret(b"abandon ability ...")?; +let plaintext = SecretBytes::new(entry.get_secret()?); // re-wrap at the consumer seam ``` -`get` returns `Option` — a zeroize-on-drop wrapper, never -a bare `Vec`. `label` is validated against -`^[A-Za-z0-9._-]{1,64}$`; `wallet_id` is a fixed 32-byte newtype. -`SecretStoreError` is a concrete `thiserror` enum carrying no secret -bytes. +### Key shape + +| upstream field | this crate's mapping | +|---|---| +| `service` | `"dash.platform-wallet-storage/" + hex(wallet_id)` (`SERVICE_PREFIX` + 64 hex chars) — one keyring "service" namespace per wallet | +| `user` | `label`, validated against `^[A-Za-z0-9._-]{1,64}$` (SEC-REQ-4.3) before reaching the SPI; allowlist excludes `/`, `:`, space, NUL, non-ASCII | + +`WalletId` is a fixed 32-byte newtype. `validated_label` runs at +`CredentialStoreApi::build` time AND at every `CredentialApi` +operation (defence in depth — credentials are long-lived). + +### Memory hygiene at the seam + +The upstream SPI returns plaintext as `Vec` from +`CredentialApi::get_secret`. The contract: callers MUST wrap that +result into [`SecretBytes::new(...)`] **immediately**, with no named +intermediate `Vec` binding (Smythe EDIT-1). `SecretBytes::new` takes +the `Vec` by value and `std::mem::take`s it into a +`Zeroizing>` — no copy of the bare buffer ever survives past +the constructor expression, so the bare-`Vec` exposure window is zero +statements. The wrapper is also best-effort `mlock`ed and `Debug` is +redacted. + +`CredentialApi::set_secret` accepts `&[u8]` (a borrow); no long-lived +unwrapped copy is allocated. -Backends: +### Backends -- `KeyringStore` — OS-native keyring (`keyring-core 1.0.0` + the - per-platform store crates). Recommended default on desktop OSes; - fails closed (`BackendUnavailable`) on headless Linux with no Secret - Service — never a silent plaintext fallback. -- `EncryptedFileStore` — Argon2id + XChaCha20-Poly1305 vault file with - a header-stored passphrase-verification token. Recommended default - on headless / server hosts. -- `MemoryStore` — tests only, gated behind `__secrets-test-helpers` so - it is unreachable from production builds. +- **`EncryptedFileStore`** — Argon2id (memory ≥ 19 MiB, t ≥ 2, defaults + 64 MiB / t=3) + XChaCha20-Poly1305 AEAD with random 24-byte XNonce + per entry. AAD binds ciphertext to + `format_version ‖ wallet_id ‖ label` so a blob moved between slots + fails the tag. A header-stored passphrase-verification token is + unsealed before any entry is touched (mixed-key-corruption guard). + Vault file created at mode 0600 via `O_EXCL`+`fchmod`; writes + temp→fsync→rename→dir-fsync; rekey replaces atomically with no + `.bak` (SEC-REQ-2.2.x). Construction errors surface as + [`FileStoreError`]; the `CredentialApi` seam bridges them through + the unit-only [`FileStoreFailure`] marker boxed inside + `keyring_core::Error::{NoStorageAccess, BadStoreFormat}` payloads. + Consumers recover the marker via `secrets::downcast_failure(&err)`. +- **OS keyring** — `secrets::default_credential_store()` returns an + `Arc` over the platform's + default credential store (`linux-keyutils-keyring-store` → + `dbus-secret-service-keyring-store` on Linux/FreeBSD; + `apple-native-keyring-store` on macOS; `windows-native-keyring-store` + on Windows). Fail-closed with `keyring_core::Error::NoDefaultStore` + on headless / unknown OS (SEC-REQ-2.1.3 / AR-4) — never a silent + plaintext fallback. The returned `Arc` is suitable for + `keyring_core::set_default_store(...)`. +- **`MemoryCredentialStore`** — gated behind `__secrets-test-helpers`; + unreachable from production builds. Backend selection is an explicit operator decision; there is no automatic fallback between backends. +### The cross-SPI error bridge + +`keyring_core::Error` does not name file-backend-unique failure modes +(wrong passphrase, malformed vault, insecure permissions, KDF +failure). The file backend boxes a unit-only [`FileStoreFailure`] +inside `keyring_core::Error::NoStorageAccess` (for `WrongPassphrase`, +matching the operator UX of `KeyringLocked`) or renders it into +`BadStoreFormat`'s static `String` payload (for `Decrypt`, +`KdfFailure`, `VersionUnsupported`, `MalformedVault`, +`InsecurePermissions`). `secrets::downcast_failure(&err)` recovers the +typed variant; the bridge is the single recovery path consumers use. + +[`FileStoreFailure`] is **unit-variants only** (Smythe EDIT-3): no +field may carry a user-supplied path, secret byte, passphrase, label, +or stringified payload. Numeric correlation fields are acceptable; the +current taxonomy needs none. The constraint is enforced via a +compile-time `Copy` assertion in the bridge module. + +Per Smythe EDIT-2, `keyring_core::Error` is safe to `Display` +(`{ }`-format), but `{:?}`-format embeds `BadEncoding(Vec)` / +`BadDataFormat(Vec, _)` payloads — those variants are NEVER +constructed by our backends with secret bytes, and +`tests/secrets_guard.rs` enforces that no debug-format pairs with +`keyring_core::Error` inside `src/secrets/`. + ## What the SQLite backend WILL refuse to store The `identity_keys` table is for **public** material only — DPP @@ -67,13 +129,32 @@ secret-free. `mnemonic`, `seed`, `xpriv`, `secret`. A new column, blob field, or comment that uses any of those words breaks the test — forcing the author to either rename, or add their phrase to the file's - allow-list with a rationale. The future `src/secrets/` directory is - exempt by design. -- NFR-4 / TC-082 (`tests/sqlite_persist_roundtrip.rs::tc082_no_box_dyn_error_in_src`): + allow-list with a rationale. The `src/secrets/` directory is exempt + by design (its own positive guard below covers it). +- **`tests/secrets_guard.rs`**: positive secret-leak guard for + `src/secrets/`. Forbids logging/formatting sinks that pair with + `expose_secret(...)` on the same logical statement (SEC-REQ-4.5.1), + AND forbids `{:?}`-debug-format paired with `keyring_core::Error` + (Smythe EDIT-2). +- **`tests/secrets_api.rs`**: shape guards — `CredentialApi::get_secret` + re-wraps through `SecretBytes::new` (EDIT-1), redacting `Debug` on + `SecretBytes`/`SecretString`, no `Box` in `src/secrets/` + (TC-082 parity). +- **`tests/secrets_off_state.rs`**: runtime guard that + `--no-default-features --features sqlite,cli` builds the persister + without pulling in the `secrets` module (D4). +- **NFR-4 / TC-082** (`tests/sqlite_persist_roundtrip.rs::tc082_no_box_dyn_error_in_src`): all public method signatures use concrete error types (`SqlitePersisterError`, `PersistenceError`) — never `Box` — so a future leak is caught by `grep`. +The CI advisory check runs `rustsec/audit-check` over `Cargo.lock`; +because `secrets` is in the default feature set, the pinned +`argon2` / `chacha20poly1305` / `zeroize` / `subtle` / `region` / +`keyring-core` / per-platform store crate versions are +unconditionally in the lockfile and therefore unconditionally in +audit scope (SEC-REQ-4.7). + ## Backup retention and secrets Manual / auto backups are byte-for-byte copies of the live DB. They @@ -81,3 +162,7 @@ inherit the same "no secrets in the file" invariant. Operators may still want to encrypt backups at rest using a file-system level tool (GnuPG, age, encfs); this crate does not do that for them and never ships SQLCipher. + +[`SecretBytes::new(...)`]: ./src/secrets/secret.rs +[`FileStoreError`]: ./src/secrets/file/error.rs +[`FileStoreFailure`]: ./src/secrets/file/error_bridge.rs diff --git a/packages/rs-platform-wallet-storage/src/lib.rs b/packages/rs-platform-wallet-storage/src/lib.rs index c4d2ab779c..b468915f70 100644 --- a/packages/rs-platform-wallet-storage/src/lib.rs +++ b/packages/rs-platform-wallet-storage/src/lib.rs @@ -1,12 +1,15 @@ //! Storage backends for the `platform-wallet` crate. //! -//! Today this crate ships the SQLite-backed -//! [`sqlite::SqlitePersister`] implementation of -//! [`PlatformWalletPersistence`](platform_wallet::changeset::PlatformWalletPersistence). -//! The crate is structured so a future `secrets` submodule — a -//! `SecretStore` for mnemonic / private-key material, sketched in -//! [`SECRETS.md`](../SECRETS.md) — can ship alongside it without a -//! crate split. +//! The SQLite-backed [`sqlite::SqlitePersister`] implements +//! [`PlatformWalletPersistence`](platform_wallet::changeset::PlatformWalletPersistence) +//! for the persister DTO (public wallet state — no secrets). +//! +//! The [`secrets`] submodule implements +//! `keyring_core::api::CredentialStoreApi` for an Argon2id + +//! XChaCha20-Poly1305 vault ([`secrets::EncryptedFileStore`]) and +//! exposes [`secrets::default_credential_store`] for the platform OS +//! keyring. See [`SECRETS.md`](../SECRETS.md) for the full key shape, +//! memory-hygiene contract, and audit hooks. //! //! ## Canonical type paths //! diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs index 8d94cce6cc..3ab83c31ae 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs @@ -7,8 +7,8 @@ use chacha20poly1305::aead::Aead; use chacha20poly1305::{KeyInit, XChaCha20Poly1305, XNonce}; use getrandom::getrandom; -use super::error::FileStoreError; use super::super::secret::SecretBytes; +use super::error::FileStoreError; /// Argon2 parameter floors (SEC-REQ-2.2.2) — derivation MUST NOT use /// anything weaker; a header declaring less is refused. diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs b/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs index cff4f89e94..34c45f9fa9 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs @@ -81,9 +81,13 @@ pub fn into_keyring(e: FileStoreError) -> KeyringError { } FileStoreError::Decrypt => bad_format(FileStoreFailure::Decrypt), FileStoreError::KdfFailure => bad_format(FileStoreFailure::KdfFailure), - FileStoreError::VersionUnsupported { .. } => bad_format(FileStoreFailure::VersionUnsupported), + FileStoreError::VersionUnsupported { .. } => { + bad_format(FileStoreFailure::VersionUnsupported) + } FileStoreError::MalformedVault => bad_format(FileStoreFailure::MalformedVault), - FileStoreError::InsecurePermissions { .. } => bad_format(FileStoreFailure::InsecurePermissions), + FileStoreError::InsecurePermissions { .. } => { + bad_format(FileStoreFailure::InsecurePermissions) + } FileStoreError::InvalidLabel => { KeyringError::Invalid("user".to_string(), "label allowlist violation".to_string()) } @@ -115,19 +119,16 @@ pub fn downcast_failure(e: &KeyringError) -> Option { } fn marker_from_message(s: &str) -> Option { - for f in [ + [ FileStoreFailure::Decrypt, FileStoreFailure::KdfFailure, FileStoreFailure::VersionUnsupported, FileStoreFailure::MalformedVault, FileStoreFailure::InsecurePermissions, FileStoreFailure::WrongPassphrase, - ] { - if s == f.to_string() { - return Some(f); - } - } - None + ] + .into_iter() + .find(|f| s == f.to_string()) } #[cfg(test)] @@ -153,7 +154,10 @@ mod tests { FileStoreError::VersionUnsupported { found: 999 }, FileStoreFailure::VersionUnsupported, ), - (FileStoreError::MalformedVault, FileStoreFailure::MalformedVault), + ( + FileStoreError::MalformedVault, + FileStoreFailure::MalformedVault, + ), ( FileStoreError::InsecurePermissions { mode: 0o644 }, FileStoreFailure::InsecurePermissions, diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs index fd20a95a33..40ec0da1f5 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs @@ -23,8 +23,8 @@ //! fails its tag, so a mismatched key is rejected before any entry is //! written or read (no mixed-key corruption). -use super::error::FileStoreError; use super::crypto::{KdfParams, NONCE_LEN, SALT_LEN}; +use super::error::FileStoreError; pub(crate) const MAGIC: &[u8; 9] = b"PWSVAULT1"; pub(crate) const FORMAT_VERSION: u32 = 2; diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs index b67dc3e2c3..3cead48993 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -76,10 +76,7 @@ struct EncryptedFileStoreInner { impl EncryptedFileStore { /// Open (or prepare to create) a vault store rooted at `dir`, /// unlocked by `passphrase`. `dir` is created if missing. - pub fn open( - dir: impl AsRef, - passphrase: SecretString, - ) -> Result { + pub fn open(dir: impl AsRef, passphrase: SecretString) -> Result { let dir = dir.as_ref().to_path_buf(); fs::create_dir_all(&dir)?; Ok(Self { @@ -98,8 +95,8 @@ impl EncryptedFileStore { ) -> Result<(), FileStoreError> { // The store must hold a unique reference so the swap is // observable to every outstanding credential consistently. - let inner = Arc::get_mut(&mut self.inner) - .expect("rekey requires exclusive access to the store"); + let inner = + Arc::get_mut(&mut self.inner).expect("rekey requires exclusive access to the store"); inner.rekey(wallet_id, new_passphrase) } @@ -109,10 +106,7 @@ impl EncryptedFileStore { } #[cfg(test)] - fn read_vault( - &self, - path: &Path, - ) -> Result)>, FileStoreError> { + fn read_vault(&self, path: &Path) -> Result)>, FileStoreError> { self.inner.read_vault(path) } @@ -184,10 +178,7 @@ impl EncryptedFileStoreInner { /// Read + parse a vault file, or `None` if it does not exist. /// Refuses a pre-existing file with looser-than-0600 perms /// (SEC-REQ-2.2.10). - fn read_vault( - &self, - path: &Path, - ) -> Result)>, FileStoreError> { + fn read_vault(&self, path: &Path) -> Result)>, FileStoreError> { match fs::metadata(path) { Ok(meta) => { check_perms(&meta)?; @@ -270,12 +261,7 @@ impl EncryptedFileStoreInner { } /// `put` — overwrite-safe atomic seal under `(wallet_id, label)`. - fn put( - &self, - wallet_id: &WalletId, - label: &str, - bytes: &[u8], - ) -> Result<(), FileStoreError> { + fn put(&self, wallet_id: &WalletId, label: &str, bytes: &[u8]) -> Result<(), FileStoreError> { let label = validated_label(label)?.to_string(); let path = self.vault_path(wallet_id); let (header, key, mut entries) = match self.read_vault(&path)? { @@ -302,11 +288,7 @@ impl EncryptedFileStoreInner { /// `get` — returns the raw plaintext as `Vec` (the upstream /// SPI contract). Callers wrap into [`SecretBytes`] at the seam. /// `NoEntry`-shaped absence rides as `Ok(None)`. - fn get( - &self, - wallet_id: &WalletId, - label: &str, - ) -> Result>, FileStoreError> { + fn get(&self, wallet_id: &WalletId, label: &str) -> Result>, FileStoreError> { let label = validated_label(label)?; let path = self.vault_path(wallet_id); let Some((header, entries)) = self.read_vault(&path)? else { @@ -815,7 +797,10 @@ mod tests { fn persistence_is_until_delete() { let dir = tempfile::tempdir().unwrap(); let s = store(dir.path()); - assert!(matches!(s.persistence(), CredentialPersistence::UntilDelete)); + assert!(matches!( + s.persistence(), + CredentialPersistence::UntilDelete + )); assert_eq!(s.vendor(), VENDOR); assert_eq!(s.id(), STORE_ID); } diff --git a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs index ada73fe45d..fb5dfe8b21 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/keyring.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/keyring.rs @@ -46,14 +46,13 @@ use keyring_core::Error as KeyringError; /// The returned `Arc` may be passed straight to /// [`keyring_core::set_default_store`] or used directly to build /// entries. -pub fn default_credential_store( -) -> Result, KeyringError> { +pub fn default_credential_store() -> Result, KeyringError> +{ platform_default_store() } #[cfg(any(target_os = "linux", target_os = "freebsd"))] -fn platform_default_store( -) -> Result, KeyringError> { +fn platform_default_store() -> Result, KeyringError> { // Prefer the kernel keyutils store; fall back to Secret Service. // Both failing (headless, no session keyring, no D-Bus) is // fail-closed by design (SEC-REQ-2.1.3 / AR-4). @@ -67,8 +66,7 @@ fn platform_default_store( } #[cfg(target_os = "macos")] -fn platform_default_store( -) -> Result, KeyringError> { +fn platform_default_store() -> Result, KeyringError> { match apple_native_keyring_store::Store::new() { Ok(s) => Ok(s), Err(_) => Err(KeyringError::NoDefaultStore), @@ -76,8 +74,7 @@ fn platform_default_store( } #[cfg(target_os = "windows")] -fn platform_default_store( -) -> Result, KeyringError> { +fn platform_default_store() -> Result, KeyringError> { match windows_native_keyring_store::Store::new() { Ok(s) => Ok(s), Err(_) => Err(KeyringError::NoDefaultStore), @@ -90,8 +87,7 @@ fn platform_default_store( target_os = "macos", target_os = "windows" )))] -fn platform_default_store( -) -> Result, KeyringError> { +fn platform_default_store() -> Result, KeyringError> { Err(KeyringError::NoDefaultStore) } diff --git a/packages/rs-platform-wallet-storage/src/secrets/memory.rs b/packages/rs-platform-wallet-storage/src/secrets/memory.rs index 2cf2ee5b5f..84136d62b9 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/memory.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/memory.rs @@ -51,7 +51,8 @@ impl MemoryCredentialStore { impl std::fmt::Debug for MemoryCredentialStore { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("MemoryCredentialStore").finish_non_exhaustive() + f.debug_struct("MemoryCredentialStore") + .finish_non_exhaustive() } } @@ -108,7 +109,10 @@ impl std::fmt::Debug for MemoryCredential { impl CredentialApi for MemoryCredential { fn set_secret(&self, secret: &[u8]) -> KeyringResult<()> { - let mut m = self.map.lock().expect("MemoryCredentialStore mutex poisoned"); + let mut m = self + .map + .lock() + .expect("MemoryCredentialStore mutex poisoned"); m.insert( (self.service.clone(), self.user.clone()), SecretBytes::from_slice(secret), @@ -117,7 +121,10 @@ impl CredentialApi for MemoryCredential { } fn get_secret(&self) -> KeyringResult> { - let m = self.map.lock().expect("MemoryCredentialStore mutex poisoned"); + let m = self + .map + .lock() + .expect("MemoryCredentialStore mutex poisoned"); match m.get(&(self.service.clone(), self.user.clone())) { Some(v) => Ok(v.expose_secret().to_vec()), None => Err(KeyringError::NoEntry), @@ -125,7 +132,10 @@ impl CredentialApi for MemoryCredential { } fn delete_credential(&self) -> KeyringResult<()> { - let mut m = self.map.lock().expect("MemoryCredentialStore mutex poisoned"); + let mut m = self + .map + .lock() + .expect("MemoryCredentialStore mutex poisoned"); match m.remove(&(self.service.clone(), self.user.clone())) { Some(_) => Ok(()), None => Err(KeyringError::NoEntry), diff --git a/packages/rs-platform-wallet-storage/src/secrets/validate.rs b/packages/rs-platform-wallet-storage/src/secrets/validate.rs index b6a7ffff0b..090536060c 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/validate.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/validate.rs @@ -87,10 +87,7 @@ mod tests { "a:b", "a;DROP TABLE", ] { - assert!( - validated_label(bad).is_err(), - "should reject {bad:?}" - ); + assert!(validated_label(bad).is_err(), "should reject {bad:?}"); } } diff --git a/packages/rs-platform-wallet-storage/tests/secrets_api.rs b/packages/rs-platform-wallet-storage/tests/secrets_api.rs index 9e408e0f02..b118ecee2f 100644 --- a/packages/rs-platform-wallet-storage/tests/secrets_api.rs +++ b/packages/rs-platform-wallet-storage/tests/secrets_api.rs @@ -122,7 +122,10 @@ fn error_display_is_static_and_secret_free() { let rendered = format!("{err}"); assert!(!rendered.contains("PLAINTEXTNEEDLE")); assert!(!rendered.contains("wrong-pass")); - assert_eq!(downcast_failure(&err), Some(FileStoreFailure::WrongPassphrase)); + assert_eq!( + downcast_failure(&err), + Some(FileStoreFailure::WrongPassphrase) + ); let inv = store.build(&service(w), "../bad", None).unwrap_err(); match inv { diff --git a/packages/rs-platform-wallet-storage/tests/secrets_scan.rs b/packages/rs-platform-wallet-storage/tests/secrets_scan.rs index a2248b35d2..9ae2f59087 100644 --- a/packages/rs-platform-wallet-storage/tests/secrets_scan.rs +++ b/packages/rs-platform-wallet-storage/tests/secrets_scan.rs @@ -96,8 +96,9 @@ fn no_secret_substrings_in_schema_or_migrations() { // `src/sqlite/schema` (SQLite-backend column definitions and blob // encoders) and `migrations/` (refinery DDL) are the entire // persistence surface for non-secret material. `src/secrets/` is - // exempt by design — that submodule WILL legitimately mention - // `private`, `mnemonic`, `seed` once the SecretStore lands. + // exempt by design — that submodule legitimately mentions + // `private`, `mnemonic`, `seed`; its own `secrets_guard.rs` test + // covers it. scan_dir(&manifest.join("src/sqlite/schema"), &mut offenders); scan_dir(&manifest.join("migrations"), &mut offenders); assert!( From 18a06558182ba0ae129f7e8504213cfd49f67a1e Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 20 May 2026 14:58:04 +0200 Subject: [PATCH 14/16] test(platform-wallet-storage): default-build proof for the secrets surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/secrets_default_on_compiles.rs` (M-S4) — a build-only assertion that the default feature set (`secrets` in) re-exports every public type/function in the `secrets` submodule. Names: `EncryptedFileStore`, `SecretBytes`, `SecretString`, `WalletId`, `FileStoreError`, `FileStoreFailure`, `SERVICE_PREFIX`, `default_credential_store`, `keyring_core::Error`. Compiling the test target is the assertion; the body never exercises a backend. Pairs with `tests/secrets_off_state.rs` (D4 — runtime proof under `--no-default-features --features sqlite,cli` that the surface compiles out and the persister still links). Co-Authored-By: Claudius the Magnificent (1M context) --- .../tests/secrets_default_on_compiles.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs diff --git a/packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs b/packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs new file mode 100644 index 0000000000..e4713f962c --- /dev/null +++ b/packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs @@ -0,0 +1,30 @@ +//! Build-only proof (M-S4) that the default build (no flag passed) +//! reaches `EncryptedFileStore` as a public type. +//! +//! With `secrets` in the default feature set, importing the type from +//! the crate root without enabling any feature flag is the assertion. +//! The test body never exercises a backend — it only compiles. + +#![cfg(feature = "secrets")] + +use platform_wallet_storage::secrets::{ + default_credential_store, EncryptedFileStore, FileStoreError, FileStoreFailure, SecretBytes, + SecretString, WalletId, SERVICE_PREFIX, +}; + +#[test] +fn default_build_exposes_secrets_surface() { + // Type-only proof: name every public re-export. + fn _accepts_path( + p: &std::path::Path, + pw: SecretString, + ) -> Result { + EncryptedFileStore::open(p, pw) + } + let _ = _accepts_path as fn(_, _) -> _; + let _ = SERVICE_PREFIX.len(); + let _ = std::mem::size_of::(); + let _ = std::mem::size_of::(); + let _ = std::mem::size_of::(); + let _: fn() -> Result<_, keyring_core::Error> = default_credential_store; +} From 3eefec2174d4aa4f53a7d91fe4711228ce26cf52 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 20 May 2026 15:47:41 +0200 Subject: [PATCH 15/16] fix(platform-wallet-storage): forbid == on SecretBytes/SecretString (EDIT-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QA-501 (MEDIUM, EDIT-4 forward-compat): `SecretBytes`/`SecretString` retained `impl PartialEq`/`Eq` despite EDIT-4's binding intent. The impls delegated to constant-time compares so today's behaviour is safe, but leaving `==` reachable means future bridge code could inherit a non-constant-time path or a length-leaking shortcut without review noticing. EDIT-4 says: no `==` on secret bytes, no exception. Strip the impls and let `subtle::ConstantTimeEq::ct_eq` be the only equality path. - `secret.rs` — removed `impl PartialEq for SecretBytes` / `impl Eq for SecretBytes` and `impl PartialEq for SecretString` / `impl Eq for SecretString`. `SecretString` gains an `impl ConstantTimeEq` so callers keep a constant-time-safe equivalence path (was previously implicit inside `PartialEq::eq`). - Public rustdoc on both types names `PartialEq`/`Eq` in the "not implemented" list and points callers at `ConstantTimeEq::ct_eq`. - `compile_fail` doc-test on each type asserts that `a == b` does NOT compile — durable forward-compat guard. If a future change adds `PartialEq` back, the doc-test starts compiling and the test fails. - Test callers migrated: - `secret_string_eq_is_value_based` → `secret_string_ct_eq_is_value_based`, asserts via `bool::from(a.ct_eq(&b))`. - `secret_bytes_constant_time_eq` drops its trailing `assert_eq!(a, b)` / `assert_ne!(a, c)` lines (the prior ct_eq-based assertions above them already covered the same invariant). Workspace-wide grep confirmed no other `==`/`assert_eq!` callers on `SecretBytes`/`SecretString` exist. Co-Authored-By: Claudius the Magnificent (1M context) --- .../src/secrets/secret.rs | 80 +++++++++++-------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index e3d33ad1de..ebdf96ad45 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/secret.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -46,12 +46,23 @@ const DEFAULT_CAPACITY: usize = 4096; /// Zeroize-on-drop wrapper for secret UTF-8 strings (BIP-39 mnemonic, /// `EncryptedFileStore` passphrase). /// -/// `Display`, `Deref`, `DerefMut`, `Serialize` are intentionally **not** -/// implemented; read access is the explicit [`expose_secret`] only. -/// `Debug` is redacted. The backing buffer is wiped over its full -/// capacity on drop and best-effort `mlock`ed against swap. +/// `Display`, `Deref`, `DerefMut`, `Serialize`, `PartialEq`, `Eq` are +/// intentionally **not** implemented; read access is the explicit +/// [`expose_secret`] only, and equality goes through +/// [`subtle::ConstantTimeEq`] (Smythe EDIT-4 — `==` on secret bytes is +/// forbidden, no exception, so future bridge code cannot inherit a +/// non-constant-time path). `Debug` is redacted. The backing buffer is +/// wiped over its full capacity on drop and best-effort `mlock`ed +/// against swap. /// /// [`expose_secret`]: SecretString::expose_secret +/// +/// ```compile_fail +/// use platform_wallet_storage::secrets::SecretString; +/// let a = SecretString::new("pw"); +/// let b = SecretString::new("pw"); +/// let _ = a == b; // EDIT-4: `==` on SecretString is forbidden; use ConstantTimeEq::ct_eq +/// ``` pub struct SecretString { inner: Zeroizing, _lock: Option, @@ -144,24 +155,19 @@ impl fmt::Debug for SecretString { } } -impl PartialEq for SecretString { - /// Best-effort timing-resistant passphrase **UX** equality only. - /// Length differences early-return, leaking length through timing; - /// this is never used for a security decision (the wrong-seed gate - /// uses [`SecretBytes`]' fixed-width `subtle` compare instead) — - /// SEC-REQ-3.8.2. - fn eq(&self, other: &Self) -> bool { - let a = self.expose_secret().as_bytes(); - let b = other.expose_secret().as_bytes(); - if a.len() != b.len() { - return false; - } - a.ct_eq(b).into() +impl ConstantTimeEq for SecretString { + /// Constant-time compare over the equal-length region. Unequal + /// lengths return `0` without revealing where they differ; the + /// only observable is the (non-secret) length difference — + /// SEC-REQ-3.8.2, the documented `PartialEq` length-leak caveat + /// from the upstream `Secret` fork. + fn ct_eq(&self, other: &Self) -> subtle::Choice { + self.expose_secret() + .as_bytes() + .ct_eq(other.expose_secret().as_bytes()) } } -impl Eq for SecretString {} - impl From for SecretString { fn from(s: String) -> Self { Self::new(s) @@ -180,9 +186,19 @@ impl From<&str> for SecretString { /// /// Not `Copy`; `Clone` is intentionally absent to enforce copy /// minimization (SEC-REQ-3.5) — move it, or `expose_secret()` and copy -/// deliberately into another wrapper. `Display`, `Deref`, `Serialize` -/// are intentionally **not** implemented; `Debug` is redacted; the +/// deliberately into another wrapper. `Display`, `Deref`, `Serialize`, +/// `PartialEq`, `Eq` are intentionally **not** implemented; equality +/// goes through [`subtle::ConstantTimeEq`] only (Smythe EDIT-4 — `==` +/// on secret bytes is forbidden, no exception, so future bridge code +/// cannot inherit a non-constant-time path). `Debug` is redacted; the /// buffer is wiped on drop and best-effort `mlock`ed. +/// +/// ```compile_fail +/// use platform_wallet_storage::secrets::SecretBytes; +/// let a = SecretBytes::new(vec![0u8; 32]); +/// let b = SecretBytes::new(vec![0u8; 32]); +/// let _ = a == b; // EDIT-4: `==` on SecretBytes is forbidden; use ConstantTimeEq::ct_eq +/// ``` pub struct SecretBytes { inner: Zeroizing>, _lock: Option, @@ -246,14 +262,6 @@ impl ConstantTimeEq for SecretBytes { } } -impl PartialEq for SecretBytes { - fn eq(&self, other: &Self) -> bool { - self.ct_eq(other).into() - } -} - -impl Eq for SecretBytes {} - impl fmt::Debug for SecretBytes { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "SecretBytes([REDACTED; {}])", self.inner.len()) @@ -280,10 +288,14 @@ mod tests { } #[test] - fn secret_string_eq_is_value_based() { - assert_eq!(SecretString::new("pw"), SecretString::new("pw")); - assert_ne!(SecretString::new("pw"), SecretString::new("px")); - assert_ne!(SecretString::new("pw"), SecretString::new("pww")); + fn secret_string_ct_eq_is_value_based() { + // EDIT-4: equality goes through `ConstantTimeEq` only. + let same = SecretString::new("pw").ct_eq(&SecretString::new("pw")); + let diff = SecretString::new("pw").ct_eq(&SecretString::new("px")); + let len_diff = SecretString::new("pw").ct_eq(&SecretString::new("pww")); + assert!(bool::from(same)); + assert!(!bool::from(diff)); + assert!(!bool::from(len_diff)); } #[test] @@ -318,8 +330,6 @@ mod tests { assert!(bool::from(a.ct_eq(&b))); assert!(!bool::from(a.ct_eq(&c))); assert!(!bool::from(a.ct_eq(&d))); - assert_eq!(a, b); - assert_ne!(a, c); } #[test] From 8a5ef7aabd5d35a7e0fe9dfcdd121f7c11a744f5 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Fri, 22 May 2026 13:31:41 +0200 Subject: [PATCH 16/16] fix(platform-wallet-storage): rekey returns FileStoreError::Busy instead of panicking; doc cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `EncryptedFileStore::rekey` panicked via `Arc::get_mut(...).expect(...)` whenever an outstanding `EncryptedFileCredential` (which clones the inner `Arc` in `build()`) was still alive — a caller-reachable runtime state, not a logic bug. Swap the `expect` for a recoverable typed `FileStoreError::Busy`, preserving the fail-loud property (still no silent stale-handle rekey). Wire a parity `FileStoreFailure::Busy` unit variant through the SPI bridge (`into_keyring` -> NoStorageAccess, Display, marker_from_message) keeping the enum unit-variants-only + Copy. Add a focused rekey-busy test plus bridge round-trip coverage. Docs: present-state lede + package description (drop "future SecretStore"), fix `__secrets-test-helpers` to name `MemoryCredentialStore`, add `getrandom` to the SECRETS.md audit-scope enumeration, document the load-bearing FileStoreFailure Display text, and note why SecretBytes keeps `.max(1)` on region::lock. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rs-platform-wallet-storage/Cargo.toml | 4 +- packages/rs-platform-wallet-storage/README.md | 14 +++--- .../rs-platform-wallet-storage/SECRETS.md | 10 +++-- .../src/secrets/file/error.rs | 8 ++++ .../src/secrets/file/error_bridge.rs | 45 ++++++++++++++++--- .../src/secrets/file/mod.rs | 29 ++++++++++-- .../src/secrets/secret.rs | 3 ++ 7 files changed, 91 insertions(+), 22 deletions(-) diff --git a/packages/rs-platform-wallet-storage/Cargo.toml b/packages/rs-platform-wallet-storage/Cargo.toml index c4ee479a08..7fc9024ac2 100644 --- a/packages/rs-platform-wallet-storage/Cargo.toml +++ b/packages/rs-platform-wallet-storage/Cargo.toml @@ -5,7 +5,7 @@ rust-version.workspace = true edition = "2021" authors = ["Dash Core Team"] license = "MIT" -description = "Storage backends for platform-wallet: SQLite persistence (today) and a future SecretStore submodule" +description = "Storage backends for platform-wallet: SQLite persistence and keyring_core secret backends (encrypted-file + OS keyring)." [lib] path = "src/lib.rs" @@ -158,7 +158,7 @@ secrets = [ "dep:apple-native-keyring-store", "dep:windows-native-keyring-store", ] -# Exposes `secrets::MemoryStore` (in-RAM test double). Double-underscore +# Exposes `secrets::MemoryCredentialStore` (in-RAM test double). Double-underscore # prefix = Cargo's "MUST NOT enable from downstream" convention; keeps # the test store unreachable from production builds (SEC-REQ-2.3.1). __secrets-test-helpers = ["secrets"] diff --git a/packages/rs-platform-wallet-storage/README.md b/packages/rs-platform-wallet-storage/README.md index 9e97e44ae3..0d69786c2f 100644 --- a/packages/rs-platform-wallet-storage/README.md +++ b/packages/rs-platform-wallet-storage/README.md @@ -1,12 +1,14 @@ # platform-wallet-storage Storage backends for the -[`platform-wallet`](../rs-platform-wallet) crate. Today this crate -ships a SQLite-backed implementation of `PlatformWalletPersistence` -under [`sqlite`](src/sqlite/) plus a maintenance CLI; the crate is -structured so a future `SecretStore` (currently sketched in -[`SECRETS.md`](./SECRETS.md)) can land as a sibling submodule under -[`secrets`](src/) without a crate split. +[`platform-wallet`](../rs-platform-wallet) crate. This crate ships a +SQLite-backed implementation of `PlatformWalletPersistence` under +[`sqlite`](src/sqlite/), a maintenance CLI, and the +[`secrets`](src/secrets/) submodule — a `keyring_core` SPI +implementation pairing the in-house `EncryptedFileStore` +(Argon2id + XChaCha20-Poly1305 on-disk vault) with the OS keyring +backends. All three are on by default; see [`SECRETS.md`](./SECRETS.md) +for the secret-storage threat model and design. ## At a glance diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index 9ffb319703..ec0e8bf180 100644 --- a/packages/rs-platform-wallet-storage/SECRETS.md +++ b/packages/rs-platform-wallet-storage/SECRETS.md @@ -150,10 +150,12 @@ secret-free. The CI advisory check runs `rustsec/audit-check` over `Cargo.lock`; because `secrets` is in the default feature set, the pinned -`argon2` / `chacha20poly1305` / `zeroize` / `subtle` / `region` / -`keyring-core` / per-platform store crate versions are -unconditionally in the lockfile and therefore unconditionally in -audit scope (SEC-REQ-4.7). +`argon2` / `chacha20poly1305` / `zeroize` / `subtle` / `getrandom` +(the `OsRng` source for the salt + per-entry nonces, specified as the +semver range `getrandom = "0.2"` and lock-pinned to 0.2.17 by +lock-file convention) / `region` / `keyring-core` / per-platform store +crate versions are unconditionally in the lockfile and therefore +unconditionally in audit scope (SEC-REQ-4.7). ## Backup retention and secrets diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/error.rs b/packages/rs-platform-wallet-storage/src/secrets/file/error.rs index 3ff29cd5fd..f832e60d19 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/error.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/error.rs @@ -57,6 +57,14 @@ pub enum FileStoreError { mode: u32, }, + /// `rekey` was called while an `EncryptedFileCredential` (built via + /// `CredentialStoreApi::build`) still holds a clone of the inner + /// `Arc`, so the store lacks the exclusive reference the atomic + /// passphrase swap requires. A recoverable runtime state — drop the + /// outstanding credentials and retry — not a logic bug. + #[error("store is busy: outstanding credentials prevent rekey")] + Busy, + /// Filesystem error (open / write / rename / fsync). The inner /// `io::Error` carries an OS code and a path *the caller supplied*, /// never a secret. diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs b/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs index 34c45f9fa9..2e8ff6d4b9 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/error_bridge.rs @@ -41,9 +41,15 @@ pub enum FileStoreFailure { MalformedVault, /// Pre-existing vault file held looser-than-0600 permissions. InsecurePermissions, + /// `rekey` ran while an outstanding credential held the inner `Arc`. + Busy, } impl std::fmt::Display for FileStoreFailure { + /// **Load-bearing text.** [`marker_from_message`] recovers the + /// variant from a `BadStoreFormat` `String` by exact match against + /// these strings, so editing any arm here requires updating + /// `marker_from_message` in lockstep (and vice versa). fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // Static, parameter-free strings — no user / secret data may // ever enter this Display (Smythe EDIT-3). @@ -54,6 +60,7 @@ impl std::fmt::Display for FileStoreFailure { Self::VersionUnsupported => "unsupported vault format version", Self::MalformedVault => "malformed vault file", Self::InsecurePermissions => "vault file has insecure permissions", + Self::Busy => "store is busy: outstanding credentials prevent rekey", }) } } @@ -79,6 +86,7 @@ pub fn into_keyring(e: FileStoreError) -> KeyringError { FileStoreError::WrongPassphrase => { KeyringError::NoStorageAccess(Box::new(FileStoreFailure::WrongPassphrase)) } + FileStoreError::Busy => KeyringError::NoStorageAccess(Box::new(FileStoreFailure::Busy)), FileStoreError::Decrypt => bad_format(FileStoreFailure::Decrypt), FileStoreError::KdfFailure => bad_format(FileStoreFailure::KdfFailure), FileStoreError::VersionUnsupported { .. } => { @@ -126,6 +134,7 @@ fn marker_from_message(s: &str) -> Option { FileStoreFailure::MalformedVault, FileStoreFailure::InsecurePermissions, FileStoreFailure::WrongPassphrase, + FileStoreFailure::Busy, ] .into_iter() .find(|f| s == f.to_string()) @@ -136,13 +145,18 @@ mod tests { use super::*; #[test] - fn wrong_passphrase_round_trips_via_no_storage_access() { - let k = into_keyring(FileStoreError::WrongPassphrase); - assert!(matches!(k, KeyringError::NoStorageAccess(_))); - assert_eq!( - downcast_failure(&k), - Some(FileStoreFailure::WrongPassphrase) - ); + fn no_storage_access_markers_round_trip() { + for (err, expected) in [ + ( + FileStoreError::WrongPassphrase, + FileStoreFailure::WrongPassphrase, + ), + (FileStoreError::Busy, FileStoreFailure::Busy), + ] { + let k = into_keyring(err); + assert!(matches!(k, KeyringError::NoStorageAccess(_))); + assert_eq!(downcast_failure(&k), Some(expected)); + } } #[test] @@ -185,6 +199,23 @@ mod tests { assert!(matches!(k, KeyringError::PlatformFailure(_))); } + #[test] + fn marker_from_message_round_trips_every_variant() { + // Display text is load-bearing: every variant must recover from + // its own rendered string, or the BadStoreFormat seam loses it. + for f in [ + FileStoreFailure::WrongPassphrase, + FileStoreFailure::Decrypt, + FileStoreFailure::KdfFailure, + FileStoreFailure::VersionUnsupported, + FileStoreFailure::MalformedVault, + FileStoreFailure::InsecurePermissions, + FileStoreFailure::Busy, + ] { + assert_eq!(marker_from_message(&f.to_string()), Some(f)); + } + } + #[test] fn downcast_returns_none_for_unrelated_errors() { assert!(downcast_failure(&KeyringError::NoEntry).is_none()); diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs index 3cead48993..8e71081e82 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -94,9 +94,14 @@ impl EncryptedFileStore { new_passphrase: SecretString, ) -> Result<(), FileStoreError> { // The store must hold a unique reference so the swap is - // observable to every outstanding credential consistently. - let inner = - Arc::get_mut(&mut self.inner).expect("rekey requires exclusive access to the store"); + // observable to every outstanding credential consistently. A + // live credential clones the inner `Arc` in `build()`, a + // caller-reachable state, so this is a recoverable typed error, + // not a panic — but still fail-loud: never a silent stale-handle + // rekey. + let Some(inner) = Arc::get_mut(&mut self.inner) else { + return Err(FileStoreError::Busy); + }; inner.rekey(wallet_id, new_passphrase) } @@ -675,6 +680,24 @@ mod tests { ); } + #[test] + fn rekey_with_outstanding_credential_returns_busy_not_panic() { + let dir = tempfile::tempdir().unwrap(); + let mut s = store(dir.path()); + // `build()` clones the inner `Arc`; keeping the credential alive + // means the store no longer holds an exclusive reference. + let live = entry(&s, wid(1), "seed"); + live.set_secret(b"value").unwrap(); + let err = s.rekey(wid(1), SecretString::new("pw-new")).unwrap_err(); + assert!(matches!(err, FileStoreError::Busy)); + // The credential is still usable and the passphrase unchanged. + assert_eq!(live.get_secret().unwrap(), b"value"); + // Once the outstanding credential is dropped, rekey succeeds. + drop(live); + s.rekey(wid(1), SecretString::new("pw-new")).unwrap(); + assert_eq!(entry(&s, wid(1), "seed").get_secret().unwrap(), b"value"); + } + #[test] fn put_with_wrong_passphrase_to_existing_vault_is_rejected() { let dir = tempfile::tempdir().unwrap(); diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index ebdf96ad45..9deef9ba1b 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/secret.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -208,6 +208,9 @@ impl SecretBytes { /// Wrap a byte vector, zeroizing the source, best-effort `mlock`ing /// the wrapped buffer. pub fn new(mut bytes: Vec) -> Self { + // `region::lock` rejects a 0-length region (EINVAL), so an empty + // `SecretBytes` still locks one page — do not "harmonize" with + // `SecretString` and drop the `.max(1)`. let lock = region::lock(bytes.as_ptr(), bytes.capacity().max(1)) .map_err(|e| { tracing::debug!("mlock failed for SecretBytes: {e}");