From a4ed0b59fd74a1c923f3c538689111718d8f239d Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 17:58:46 +0200 Subject: [PATCH 01/10] feat(platform-wallet-storage): SecretString serde/schemars/is_blank (opt-in, default-off) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 1 of the layered secret-protection feature. - `SecretString::is_blank()` — always-on inherent method, trims via the Unicode White_Space property (NBSP blanks, ZWSP does not). The enforcement primitive for the Tier-1 blank-passphrase guard and the Tier-2 blank-object-password reject. - Manual `Deserialize` behind the dedicated default-off `secret-serde` feature: routes the owned `String` through `SecretString::new` so the transient plaintext is zeroized; documents the unavoidable deserializer-input-buffer residual. NO `Serialize` companion. - Manual `JsonSchema` behind default-off `secret-schemars`: renders a plain `string`, no minLength/maxLength/pattern/format/example/default — no length or value policy leak (F-7). Hand-written (no derive), so the lock gains no `schemars_derive`. - `MIN_PASSPHRASE_LEN = 1` (coarse floor; real entropy policy is the consumer's, per GAP-012). - Cargo features: `secret-serde = ["secrets","dep:serde"]`, `secret-schemars = ["secret-serde","dep:schemars"]`; `default` unchanged (excludes both); `secrets` does NOT pull them. The gates are on the IMPLS not the dep, so they are satisfiable default-off even with `secrets` (and serde) on (GAP-002). Tests (TS-SER-001..008): is_blank truth table incl. Unicode boundary; bool-no-borrow signature; compile-time no-Serialize/no-Display assertion; GAP-002 regression (Deserialize ABSENT under `secrets` alone, PRESENT under `secret-serde`); zeroizing-roundtrip Deserialize; schema-shape leak guard. Green under default, `--features secret-serde`, `--features secret-schemars`. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- Cargo.lock | 1 + .../rs-platform-wallet-storage/Cargo.toml | 14 ++ .../src/secrets/mod.rs | 2 +- .../src/secrets/secret.rs | 210 ++++++++++++++++++ 4 files changed, 226 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 2bc1b73c34..c285c240a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5230,6 +5230,7 @@ dependencies = [ "refinery", "region", "rusqlite", + "schemars 1.2.1", "serde", "serde_json", "serial_test", diff --git a/packages/rs-platform-wallet-storage/Cargo.toml b/packages/rs-platform-wallet-storage/Cargo.toml index 88d1a2b397..d8c9c8ad7c 100644 --- a/packages/rs-platform-wallet-storage/Cargo.toml +++ b/packages/rs-platform-wallet-storage/Cargo.toml @@ -61,6 +61,11 @@ chrono = { version = "0.4", default-features = false, features = [ "clock", ], optional = true } sha2 = { version = "0.10", optional = true } +# Opt-in `JsonSchema` for `SecretString` (gated by `secret-schemars`). +# Reuses the workspace-locked 1.2.1. `default-features = false` drops the +# `derive` feature (we hand-write the impl), matching the crate's existing +# derive-free schemars usage so the lock gains no `schemars_derive` entry. +schemars = { version = "1", optional = true, default-features = false } # Secret-storage deps (gated by the `secrets` feature). RustSec-clean # pins (Smythe §7); `aes-gcm` is deliberately omitted. `keyring`'s @@ -213,6 +218,15 @@ secrets = [ "dep:apple-native-keyring-store", "dep:windows-native-keyring-store", ] +# Opt-in `SecretString` serde/schemars impls. Deliberately DEFAULT-OFF +# even though `secrets` (and, via it, the `serde` dep) are default-on: +# these gate the IMPLS, not the dep, so the impls are absent unless a +# consumer explicitly opts in. `secret-serde` requires `secrets` (the type +# only exists under it). NO `Serialize` is ever provided. `secret-schemars` +# implies `secret-serde`. (design §5.4 / GAP-001 names / GAP-002 satisfiable +# default-off.) +secret-serde = ["secrets", "dep:serde"] +secret-schemars = ["secret-serde", "dep:schemars"] # Per-object-type key/value metadata API # (`platform_wallet_storage::{KvStore, KvError, ObjectId}`) plus the # SQLite-backed impl. Requires `sqlite` because the only shipped backend diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index d69754429f..c8f82e161f 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -35,6 +35,6 @@ pub use file::{ SERVICE_PREFIX, }; pub use keyring::default_credential_store; -pub use secret::{SecretBytes, SecretString}; +pub use secret::{SecretBytes, SecretString, MIN_PASSPHRASE_LEN}; pub use store::SecretStore; pub use validate::WalletId; diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index 3dd5c53746..46ca130b3f 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/secret.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -15,6 +15,18 @@ use zeroize::{Zeroize, Zeroizing}; /// buffer behind — virtually impossible for any human-entered secret. const DEFAULT_CAPACITY: usize = 4096; +/// Minimal post-trim length floor for a vault passphrase or a Tier-2 +/// object password, in bytes. A **coarse** guard only: `1` means "merely +/// non-blank" (the same outcome [`SecretString::is_blank`] enforces). +/// +/// The library deliberately ships **no** password-strength estimator. The +/// real entropy policy — zxcvbn-style strength, dictionary checks, UX +/// feedback — is locale- and threat-specific and therefore the +/// **consumer's** responsibility (documented in `SECRETS.md`). Baking a +/// fixed estimator into a storage crate would be both too weak for some +/// callers and too rigid for others. +pub const MIN_PASSPHRASE_LEN: usize = 1; + /// Zeroize-on-drop wrapper for secret UTF-8 strings (BIP-39 mnemonic, /// `EncryptedFileStore` passphrase). /// @@ -87,6 +99,18 @@ impl SecretString { pub fn trimmed(&self) -> Self { Self::new(self.inner.trim().to_string()) } + + /// Whether the secret is empty or all Unicode-whitespace. + /// + /// Returns only blank-ness — never a borrowed view of the plaintext — + /// and uses [`str::trim`] (the Unicode `White_Space` property), so a + /// NBSP (`U+00A0`) trims to blank but a ZWSP (`U+200B`, not + /// `White_Space`) does not. This is the enforcement primitive behind + /// the Tier-1 blank-passphrase guard and the Tier-2 blank-object- + /// password reject. Always available — **not** feature-gated. + pub fn is_blank(&self) -> bool { + self.inner.trim().is_empty() + } } impl Default for SecretString { @@ -143,6 +167,86 @@ impl From<&str> for SecretString { } } +/// Deserialize a UTF-8 secret (a vault passphrase or a Tier-2 object +/// password arriving via config), routing the owned `String` through +/// [`SecretString::new`] — which zeroizes its source — so no +/// intermediate plaintext buffer **we own** lingers (CWE-316). +/// +/// Gated behind the dedicated, default-off `secret-serde` feature, NOT the +/// crate's internal `serde` dep (which `secrets` already pulls): the gate +/// is on the IMPL, so the impl is absent unless explicitly opted in, even +/// though `serde` itself is compiled. There is deliberately **no** +/// `Serialize` companion (a secret is read-from-config, never written +/// back / round-tripped / logged), so this type cannot leak out through +/// serde under any feature combination. +/// +/// **Residual (documented, not closeable here):** the deserializer's own +/// input buffer holds the cleartext before this visitor runs and is +/// outside `SecretString`'s ownership, so it cannot be wiped here — feed +/// secrets from a zeroizing source. Mirrors the Argon2 `Block` residual +/// noted at `crypto::derive_key`. +#[cfg(feature = "secret-serde")] +impl<'de> serde::Deserialize<'de> for SecretString { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct SecretStringVisitor; + + impl<'v> serde::de::Visitor<'v> for SecretStringVisitor { + type Value = SecretString; + + fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("a secret string") + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + // Take ownership of the borrowed bytes, then hand the owned + // `String` to the zeroizing constructor below. + self.visit_string(v.to_owned()) + } + + fn visit_string(self, v: String) -> Result + where + E: serde::de::Error, + { + // `SecretString::new` zeroizes the moved-in `String`. + Ok(SecretString::new(v)) + } + } + + deserializer.deserialize_string(SecretStringVisitor) + } +} + +/// Render the JSON schema as a plain `string` carrying **no** length or +/// value policy: no `minLength`/`maxLength`/`pattern`/`format` (would leak +/// a length policy) and no `example`/`default` (would embed a value) +/// (F-7). A short, value-free `description` marks sensitivity. +/// +/// Gated behind the default-off `secret-schemars` feature (which implies +/// `secret-serde`). Pulls in no `Serialize`/`Display` path. +#[cfg(feature = "secret-schemars")] +impl schemars::JsonSchema for SecretString { + fn schema_name() -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed("SecretString") + } + + fn schema_id() -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed("platform_wallet_storage::secrets::SecretString") + } + + fn json_schema(_generator: &mut schemars::SchemaGenerator) -> schemars::Schema { + schemars::json_schema!({ + "type": "string", + "description": "A secret string. Write-only: never serialized, never echoed." + }) + } +} + /// Zeroize-on-drop wrapper for secret **bytes**: BIP-32 seed /// (`[u8; 64]`), xpriv, Argon2 output, AEAD key, decrypted plaintext. /// @@ -282,6 +386,112 @@ mod tests { assert_eq!(SecretString::default().len(), 0); } + /// TS-SER-001: `is_blank()` truth table. The boundary deliberately + /// exercises Unicode whitespace — `str::trim` uses the `White_Space` + /// property, so NBSP (`U+00A0`) trims to blank but ZWSP (`U+200B`, + /// not `White_Space`) does not. + #[test] + fn is_blank_truth_table() { + // Blank inputs. + assert!(SecretString::empty().is_blank()); + assert!(SecretString::new("").is_blank()); + assert!(SecretString::new(" ").is_blank()); + assert!(SecretString::new("\t\r\n ").is_blank()); + assert!( + SecretString::new("\u{00A0}").is_blank(), + "NBSP is White_Space" + ); + // Non-blank inputs. + assert!(!SecretString::new("pw").is_blank()); + assert!(!SecretString::new(" pw ").is_blank()); + assert!( + !SecretString::new("\u{200B}").is_blank(), + "ZWSP is NOT White_Space" + ); + } + + /// TS-SER-002: `is_blank` returns a `bool` and exposes no borrowed + /// plaintext, callable with only `secrets` (no serde/schemars). + #[test] + fn is_blank_signature_returns_bool_no_borrow() { + let f: fn(&SecretString) -> bool = SecretString::is_blank; + assert!(f(&SecretString::new(""))); + assert!(!f(&SecretString::new("x"))); + } + + /// TS-SER-005 / TS-SER-007: `SecretString` must never implement + /// `Serialize` or `Display`, even with serde compiled in. This is a + /// compile-time `!impl` assertion — adding either impl breaks the + /// build. `serde::Serialize` is nameable here because `secrets` always + /// pulls the `serde` dep. + #[test] + fn secret_string_has_no_serialize_no_display() { + static_assertions::assert_not_impl_any!(SecretString: serde::Serialize, std::fmt::Display); + } + + /// TS-SER-008 / GAP-002 regression: the `serde` DEP is on under + /// `secrets`, yet the `Deserialize` IMPL stays ABSENT because it is + /// gated on the dedicated `secret-serde` feature — proving the + /// default-off gate is satisfiable even while serde is compiled. + #[cfg(not(feature = "secret-serde"))] + #[test] + fn deserialize_absent_without_secret_serde_even_though_serde_dep_on() { + static_assertions::assert_not_impl_any!( + SecretString: serde::de::DeserializeOwned + ); + } + + /// TS-SER-008: with `secret-serde` on, the `Deserialize` impl is + /// present (and `Serialize` is still absent — see the always-on test). + #[cfg(feature = "secret-serde")] + #[test] + fn deserialize_present_with_secret_serde() { + static_assertions::assert_impl_all!(SecretString: serde::de::DeserializeOwned); + static_assertions::assert_not_impl_any!(SecretString: serde::Serialize); + } + + /// TS-SER-003: `Deserialize` round-trips the value through the + /// zeroizing constructor; the result `ct_eq`s a directly-built secret + /// and has the right length. + #[cfg(feature = "secret-serde")] + #[test] + fn deserialize_routes_value_through_zeroizing_constructor() { + let s: SecretString = serde_json::from_str("\"correct horse battery staple\"").unwrap(); + assert!(bool::from( + s.ct_eq(&SecretString::new("correct horse battery staple")) + )); + assert_eq!(s.len(), 28); + } + + /// TS-SER-006: `JsonSchema` renders a plain `string` and leaks no + /// length/value policy — no `minLength`/`maxLength`/`pattern`/`format`, + /// no `example`/`default`/`enum`. + #[cfg(feature = "secret-schemars")] + #[test] + fn json_schema_is_plain_string_no_policy_leak() { + let schema = schemars::schema_for!(SecretString); + let v = serde_json::to_value(&schema).unwrap(); + assert_eq!(v["type"], serde_json::json!("string")); + for forbidden in [ + "minLength", + "maxLength", + "pattern", + "format", + "example", + "default", + "enum", + ] { + assert!( + v.get(forbidden).is_none(), + "schema leaked `{forbidden}`: {v}" + ); + } + // Any description present must carry no example/secret value. + if let Some(desc) = v.get("description").and_then(|d| d.as_str()) { + assert!(!desc.contains("horse")); + } + } + #[test] fn secret_bytes_debug_redacted() { let b = SecretBytes::from_slice(&[1, 2, 3, 4, 5]); From 1827a3943a613d678ec7fff79e8faf266009a746 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:03:58 +0200 Subject: [PATCH 02/10] feat(platform-wallet-storage): error taxonomy for Tier-2 secret protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 2 of the layered secret-protection feature. Add five `SecretStoreError` variants with redacted (secret-free) Display and `keyring_core::Error` projections: - `ExpectedProtectedButUnsealed` — L-1 keystone: caller asserted protection (supplied a password) but the stored value is unprotected → fail closed. - `NeedsPassword` — protected (scheme-1) read with no password; never returns ciphertext. - `WrongPassword` — Tier-2 object-password AEAD tag fail; distinct from the Tier-1 `WrongPassphrase`. - `BlankPassphrase` — blank vault passphrase or blank object password. - `UnsupportedEnvelopeVersion { found: u8 }` — magic present, unknown version/scheme; fail closed regardless of password (GAP-009). Projections (resolving GAP-004): the four credential/protection STATE errors ride `NoStorageAccess(boxed)` — losslessly downcast-recoverable, mirroring `WrongPassphrase`; `UnsupportedEnvelopeVersion` joins the secret-free `BadStoreFormat` group, mirroring `VersionUnsupported`. Tests (TS-ERR-001..003): variants distinct (incl. WrongPassword != WrongPassphrase, ExpectedProtectedButUnsealed != Corruption); exact secret-free Display; recoverable NoStorageAccess downcast for the four states; BadStoreFormat for the version variant. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/error.rs | 170 +++++++++++++++++- 1 file changed, 167 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/error.rs b/packages/rs-platform-wallet-storage/src/secrets/error.rs index 94e7375e1b..5290a39c8b 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/error.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/error.rs @@ -20,6 +20,39 @@ pub enum SecretStoreError { #[error("wrong passphrase")] WrongPassphrase, + /// Tier-2 (L-1 keystone): the caller asserted — by supplying an object + /// password — that this object MUST be password-protected, but the + /// stored value is a well-formed UNPROTECTED envelope (scheme-0) or a + /// legacy magic-less raw value, i.e. a strip/downgrade. **Fails + /// closed:** the stored bytes are NEVER returned (CWE-757/CWE-345). + #[error("expected a password-protected secret but the stored value is unprotected")] + ExpectedProtectedButUnsealed, + + /// Tier-2: a valid password-protected (scheme-1) envelope was read + /// with NO object password supplied. Never returns ciphertext. + #[error("secret is password-protected; a password is required")] + NeedsPassword, + + /// Tier-2: the object password failed the envelope's AEAD tag. Carries + /// **no** plaintext and no source (CWE-347). Distinct from + /// [`WrongPassphrase`] (the Tier-1 vault passphrase). On the + /// [`SecretStore::Os`] arm a tag failure may also indicate keychain + /// corruption rather than a wrong password — documented in + /// `SECRETS.md`; one AEAD tag cannot disambiguate the two. + /// + /// [`WrongPassphrase`]: SecretStoreError::WrongPassphrase + /// [`SecretStore::Os`]: crate::secrets::SecretStore::Os + #[error("wrong object password")] + WrongPassword, + + /// A vault passphrase (Tier-1 `open`/`rekey`) or an object password + /// (Tier-2 enrol) was blank — empty or all-whitespace — rejected via + /// [`SecretString::is_blank`]. CWE-521. + /// + /// [`SecretString::is_blank`]: crate::secrets::SecretString::is_blank + #[error("passphrase must not be blank")] + BlankPassphrase, + /// AEAD tag failure on a stored entry (or rekey re-encrypt) *after* /// the header verify-token passed: the entry ciphertext is corrupt or /// tampered, **not** a wrong passphrase. No plaintext (CWE-347). @@ -39,6 +72,22 @@ pub enum SecretStoreError { found: u32, }, + /// A Tier-2 secret envelope carried the magic but a `version` (or, at a + /// known version, a `scheme`) this build does not understand. Fails + /// closed REGARDLESS of the password argument — an unparseable future + /// format can be neither safely unwrapped nor safely treated as + /// unprotected, so it is refused both ways (GAP-009). Mirrors + /// [`VersionUnsupported`] for the vault format. + /// + /// [`VersionUnsupported`]: SecretStoreError::VersionUnsupported + #[error("unsupported secret envelope version {found}")] + UnsupportedEnvelopeVersion { + /// The envelope `version` byte read from the (unauthenticated) + /// header. An unknown `scheme` under a known version reports the + /// known version byte (a forward-incompatible scheme). + found: u8, + }, + /// The vault file was malformed (bad magic, truncated header, bad /// record framing) — no plaintext was produced. #[error("malformed vault file")] @@ -231,11 +280,18 @@ impl From for SecretStoreError { /// seam. Lossy by design — the lossless typed path is the /// [`SecretStore`](crate::secrets::SecretStore) API. /// -/// - [`WrongPassphrase`] / [`AlreadyLocked`] ride in +/// - [`WrongPassphrase`] / [`AlreadyLocked`] and the Tier-2 credential / +/// protection states ([`NeedsPassword`], [`WrongPassword`], +/// [`ExpectedProtectedButUnsealed`], [`BlankPassphrase`]) ride in /// [`KeyringError::NoStorageAccess`] with the typed error boxed as the /// source, recoverable via /// `err.source().and_then(|s| s.downcast_ref::())`. -/// - The format/crypto group collapses into +/// These are all "the caller must act on a credential/expectation to +/// proceed" states, so lossless recovery lets an SPI consumer react +/// precisely (resolves GAP-004 for these variants). +/// - The format/crypto group — including [`UnsupportedEnvelopeVersion`] +/// (a fail-closed forward-format incompatibility, mirroring +/// [`VersionUnsupported`]) — collapses into /// [`KeyringError::BadStoreFormat`] (a static secret-free string — that /// variant has no box slot). /// - [`InvalidLabel`] → `KeyringError::Invalid("user", _)`; @@ -243,16 +299,28 @@ impl From for SecretStoreError { /// /// [`WrongPassphrase`]: SecretStoreError::WrongPassphrase /// [`AlreadyLocked`]: SecretStoreError::AlreadyLocked +/// [`NeedsPassword`]: SecretStoreError::NeedsPassword +/// [`WrongPassword`]: SecretStoreError::WrongPassword +/// [`ExpectedProtectedButUnsealed`]: SecretStoreError::ExpectedProtectedButUnsealed +/// [`BlankPassphrase`]: SecretStoreError::BlankPassphrase +/// [`UnsupportedEnvelopeVersion`]: SecretStoreError::UnsupportedEnvelopeVersion +/// [`VersionUnsupported`]: SecretStoreError::VersionUnsupported /// [`InvalidLabel`]: SecretStoreError::InvalidLabel /// [`Io`]: SecretStoreError::Io impl From for KeyringError { fn from(e: SecretStoreError) -> Self { use SecretStoreError as E; match e { - E::WrongPassphrase | E::AlreadyLocked => KeyringError::NoStorageAccess(Box::new(e)), + E::WrongPassphrase + | E::AlreadyLocked + | E::NeedsPassword + | E::WrongPassword + | E::ExpectedProtectedButUnsealed + | E::BlankPassphrase => KeyringError::NoStorageAccess(Box::new(e)), E::Corruption | E::KdfFailure | E::VersionUnsupported { .. } + | E::UnsupportedEnvelopeVersion { .. } | E::MalformedVault | E::InsecurePermissions { .. } | E::InsecureParentDir { .. } @@ -386,6 +454,102 @@ mod tests { assert!(!format!("{k}").contains("plaintext")); } + /// TS-ERR-001: the five new variants exist, are constructable, render + /// distinct non-empty messages, and the Tier-2 `WrongPassword` is NOT + /// the Tier-1 `WrongPassphrase` (nor is the unseal error `Corruption`). + #[test] + fn new_variants_exist_and_are_distinct() { + use SecretStoreError as E; + assert_ne!(E::WrongPassword.to_string(), E::WrongPassphrase.to_string()); + assert_ne!( + E::ExpectedProtectedButUnsealed.to_string(), + E::Corruption.to_string() + ); + let msgs: std::collections::HashSet = [ + E::NeedsPassword.to_string(), + E::WrongPassword.to_string(), + E::BlankPassphrase.to_string(), + E::ExpectedProtectedButUnsealed.to_string(), + E::UnsupportedEnvelopeVersion { found: 2 }.to_string(), + ] + .into_iter() + .collect(); + assert_eq!(msgs.len(), 5, "all five messages must be distinct"); + } + + /// TS-ERR-002: Display + Debug render static, secret-free text. The + /// version variant surfaces the (non-secret) version byte and nothing + /// more. + #[test] + fn new_variants_carry_no_secret_in_display() { + use SecretStoreError as E; + assert_eq!( + E::NeedsPassword.to_string(), + "secret is password-protected; a password is required" + ); + assert_eq!(E::WrongPassword.to_string(), "wrong object password"); + assert_eq!( + E::BlankPassphrase.to_string(), + "passphrase must not be blank" + ); + assert_eq!( + E::ExpectedProtectedButUnsealed.to_string(), + "expected a password-protected secret but the stored value is unprotected" + ); + assert_eq!( + E::UnsupportedEnvelopeVersion { found: 7 }.to_string(), + "unsupported secret envelope version 7" + ); + // Debug is non-empty and free of plaintext-ish tokens for all. + for e in [ + E::NeedsPassword, + E::WrongPassword, + E::BlankPassphrase, + E::ExpectedProtectedButUnsealed, + E::UnsupportedEnvelopeVersion { found: 7 }, + ] { + let rendered = format!("{e} {e:?}"); + assert!(!rendered.contains("plaintext")); + } + } + + /// TS-ERR-003 (resolving GAP-004): the four Tier-2 credential / + /// protection states project to a recoverable `NoStorageAccess` with + /// the typed error losslessly downcast-able, leaking no secret. + #[test] + fn tier2_state_errors_project_to_recoverable_no_storage_access() { + for original in [ + SecretStoreError::NeedsPassword, + SecretStoreError::WrongPassword, + SecretStoreError::ExpectedProtectedButUnsealed, + SecretStoreError::BlankPassphrase, + ] { + let want = original.to_string(); + let k: KeyringError = original.into(); + assert!(!format!("{k}").contains("plaintext")); + match &k { + KeyringError::NoStorageAccess(src) => { + let recovered = src.downcast_ref::(); + assert!( + matches!(recovered, Some(e) if e.to_string() == want), + "expected recoverable {want}, got {recovered:?}" + ); + } + other => panic!("expected NoStorageAccess for {want}, got {other:?}"), + } + } + } + + /// TS-ERR-003: `UnsupportedEnvelopeVersion` projects to the + /// secret-free `BadStoreFormat` group (forward-format incompat, + /// mirroring `VersionUnsupported`). + #[test] + fn unsupported_envelope_version_projects_to_bad_store_format() { + let k: KeyringError = SecretStoreError::UnsupportedEnvelopeVersion { found: 9 }.into(); + assert!(matches!(k, KeyringError::BadStoreFormat(_))); + assert!(!format!("{k}").contains("plaintext")); + } + #[test] fn os_keyring_projects_to_bad_store_format() { let k: KeyringError = SecretStoreError::OsKeyring { From f68f00e62a434206b5835c551a917e88374c566d Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:12:40 +0200 Subject: [PATCH 03/10] feat(platform-wallet-storage): Tier-2 secret envelope (wrap/unwrap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 3 of the layered secret-protection feature. New `secrets::envelope` module — backend-independent, sits above SecretStore over both arms. Wire format: magic b"PWSEV" + ENVELOPE_VERSION:u8 + scheme:u8 (0 unprotected passthrough / 1 Argon2id+XChaCha20-Poly1305 password); scheme-1 body = kdf(id,m_kib,t,p LE) + salt[32] + nonce[24] + ct+tag. Reuses crypto::{derive_key,seal,open,random_bytes,KdfParams,enforce_bounds} (no bespoke crypto); `crypto`/`format` widened to pub(super) so the sibling envelope module can share them without duplication. Guardrails: - L-2: KDF param ceiling enforced BEFORE derivation on the untrusted header (enforce_bounds gates pre-alloc). - L-3: AAD binds domain ‖ magic ‖ version ‖ scheme ‖ kdf ‖ salt ‖ wallet_id ‖ label (length-prefixed), mirroring format::aad/verify_aad — relocation/header-tamper fail the tag. - SEC-F006/GAP-006: plaintext capped at MAX_SECRET_LEN−MAX_ENVELOPE_OVERHEAD (=65408), uniform across schemes, so enveloped bytes always fit the backend cap. Re-exported as pub `MAX_PLAINTEXT_LEN`. - GAP-009: magic-present unknown version/scheme → UnsupportedEnvelopeVersion, fail closed regardless of password. - Legacy-tolerant read (adopted §4.1 contingency): magic-less + None → raw bytes (+ one-time warn, re-wrapped on next write); magic-less + Some(pw) → ExpectedProtectedButUnsealed (L-1 preserved). The strict fail-closed quadrant lives in `unwrap` (the L-1 keystone is proven against it and wired into the store in the next task). Tests (TS-ENV-001..010): scheme-0/1 round-trips, fresh salt+nonce, WrongPassword, identity-AAD relocation rejection, per-field header tamper, ★ KDF-ceiling-before-derive (no OOM), blank-password reject, plaintext size cap (v5 boundary), magic/version discrimination, and a 2000-iteration deterministic byte-fuzz + full truncation sweep that never panics and never leaks plaintext from a tag-failing branch. NOTE: a scoped `#![allow(dead_code)]` covers the not-yet-wired primitives; the next task wires them into SecretStore and removes it. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/envelope.rs | 744 ++++++++++++++++++ .../src/secrets/file/mod.rs | 9 +- .../src/secrets/mod.rs | 2 + 3 files changed, 753 insertions(+), 2 deletions(-) create mode 100644 packages/rs-platform-wallet-storage/src/secrets/envelope.rs diff --git a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs new file mode 100644 index 0000000000..958ab57966 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs @@ -0,0 +1,744 @@ +//! Tier-2 opt-in per-object password envelope (backend-independent). +//! +//! Sits ABOVE [`SecretStore`](crate::secrets::SecretStore), over both the +//! `File` vault and `Os` keyring arms: the backend stores opaque bytes, +//! and a chosen critical object (a seed wallet, a single privkey) can be +//! wrapped under an extra, user-supplied **object password** before it +//! ever reaches the backend. Reading a protected object then needs BOTH +//! backend access AND the password — the first control that survives a +//! full backend compromise (the keychain scraped, the vault stolen and its +//! passphrase cracked). +//! +//! # Wire format (self-describing, authenticated) +//! +//! ```text +//! magic b"PWSEV" (5) +//! version u8 = 1 (ENVELOPE_VERSION — independent of the vault FORMAT_VERSION) +//! scheme u8 (0 = unprotected passthrough, 1 = argon2id-xchacha password) +//! ── scheme 0 ── payload: raw secret bytes +//! ── scheme 1 ── kdf(id u8 ‖ m_kib u32 LE ‖ t u32 LE ‖ p u32 LE) (13) +//! ‖ salt[32] ‖ nonce[24] ‖ ciphertext+tag +//! ``` +//! +//! The header proves what the blob **is**, never what the caller +//! **expected** — that expectation lives solely in the caller's `Some/None` +//! password argument (see [`unwrap`]'s strict, fail-closed table). The +//! self-description is a convenience for `NeedsPassword`/`WrongPassword`/ +//! version UX, **not** the security boundary. +//! +//! ## Reused, never reinvented +//! - KDF: [`crypto::derive_key`] (Argon2id) with a fresh 32-byte salt; the +//! param **ceiling is enforced BEFORE derivation** on the +//! attacker-controllable header (L-2, [`KdfParams::enforce_bounds`]). +//! - AEAD: [`crypto::seal`]/[`crypto::open`] (XChaCha20-Poly1305), fresh +//! per-wrap nonce; a tag failure maps to +//! [`SecretStoreError::WrongPassword`] with no plaintext. +//! - AAD binds `domain ‖ magic ‖ version ‖ scheme ‖ kdf ‖ salt ‖ wallet_id +//! ‖ label` (L-3), mirroring [`format::aad`]/[`format::verify_aad`] so a +//! relocated/confused blob fails the tag. +//! +//! No bespoke crypto. +//! +//! [`format::aad`]: super::file::format::aad +//! [`format::verify_aad`]: super::file::format::verify_aad + +// The wrap/unwrap primitives are exercised by this module's own tests but +// are not yet called from non-test code: the strict-read wiring into +// `SecretStore::get_secret`/`set_secret` lands in the next task, which +// removes this allow. Without it the not-yet-wired primitives would trip +// dead-code warnings in the non-test build. +#![allow(dead_code)] + +use std::sync::Once; + +use super::error::SecretStoreError; +use super::file::crypto::{self, KdfParams, NONCE_LEN, SALT_LEN}; +use super::secret::{SecretBytes, SecretString}; +use super::validate::WalletId; +use super::MAX_SECRET_LEN; + +/// 5-byte sentinel marking a Tier-2 envelope. A decrypted entry NOT +/// starting with this is a legacy magic-less raw value (see [`unwrap`]). +pub(crate) const MAGIC: &[u8; 5] = b"PWSEV"; + +/// Envelope wire version — bumped only on a breaking layout change, and +/// independent of the vault `FORMAT_VERSION` (the envelope rides inside the +/// entry bytes, identical over File/Os). +pub(crate) const ENVELOPE_VERSION: u8 = 1; + +/// Scheme 0: unprotected passthrough — payload is the raw secret. +pub(crate) const SCHEME_UNPROTECTED: u8 = 0; +/// Scheme 1: Argon2id + XChaCha20-Poly1305 under an object password. +pub(crate) const SCHEME_PASSWORD: u8 = 1; + +/// Domain-separation tag leading the scheme-1 AAD, so a Tier-2 tag can +/// never be confused with the vault's own verify/entry AAD. +const TIER2_DOMAIN: &[u8] = b"PWSEV-TIER2-AAD-v1"; + +/// Fixed header: `magic ‖ version ‖ scheme`. +const HEADER_LEN: usize = MAGIC.len() + 2; +/// Encoded KDF-params field: `id u8 ‖ m_kib u32 ‖ t u32 ‖ p u32`. +const KDF_FIELD_LEN: usize = 1 + 4 + 4 + 4; +/// Poly1305 tag length — present even for empty plaintext. +const AEAD_TAG_LEN: usize = 16; +/// Smallest valid scheme-1 body (kdf ‖ salt ‖ nonce ‖ bare tag). +const MIN_SCHEME1_BODY: usize = KDF_FIELD_LEN + SALT_LEN + NONCE_LEN + AEAD_TAG_LEN; + +/// Fixed, bounded envelope overhead (`magic 5 + version 1 + scheme 1 + kdf +/// 13 + salt 32 + nonce 24 + tag 16 = 92`), rounded up to 128 for headroom +/// (future header fields / versions). Used to derive the plaintext cap. +pub(crate) const MAX_ENVELOPE_OVERHEAD: usize = 128; + +/// Plaintext cap at the envelope boundary: `MAX_SECRET_LEN − +/// MAX_ENVELOPE_OVERHEAD`. Capping the **plaintext** (uniformly for both +/// schemes) keeps the user-visible limit stable AND guarantees the +/// enveloped bytes always fit the backend vault's own `MAX_SECRET_LEN` +/// `put_bytes` cap (design §4.6 / SEC-F006 / GAP-006). Re-exported at +/// [`crate::secrets`] as the documented, stable user-facing cap. +pub const MAX_PLAINTEXT_LEN: usize = MAX_SECRET_LEN - MAX_ENVELOPE_OVERHEAD; + +/// Wrap `plaintext` for `(wallet_id, label)` using the shipped default +/// Argon2 target (64 MiB / t=3) when a password is supplied. +/// +/// `None` → an unprotected (scheme-0) envelope; `Some(pw)` → a scheme-1 +/// envelope sealed under `pw`. A blank password is rejected at enrol +/// ([`SecretStoreError::BlankPassphrase`]). +pub(crate) fn wrap( + wallet_id: &WalletId, + label: &str, + password: Option<&SecretString>, + plaintext: &[u8], +) -> Result, SecretStoreError> { + wrap_with_params( + wallet_id, + label, + password, + plaintext, + KdfParams::default_target(), + ) +} + +/// [`wrap`] with explicit Argon2 `params` (tests use the floor params for +/// speed; production uses [`KdfParams::default_target`]). `params` is +/// ignored when `password` is `None`. +pub(crate) fn wrap_with_params( + wallet_id: &WalletId, + label: &str, + password: Option<&SecretString>, + plaintext: &[u8], + params: KdfParams, +) -> Result, SecretStoreError> { + // Cap the PLAINTEXT (before overhead) uniformly for both schemes so the + // enveloped bytes always fit the backend cap and the limit is stable. + if plaintext.len() > MAX_PLAINTEXT_LEN { + return Err(SecretStoreError::SecretTooLarge { + found: plaintext.len(), + max: MAX_PLAINTEXT_LEN, + }); + } + + let Some(pw) = password else { + // Scheme 0: magic ‖ version ‖ scheme ‖ raw payload. + let mut out = Vec::with_capacity(HEADER_LEN + plaintext.len()); + out.extend_from_slice(MAGIC); + out.push(ENVELOPE_VERSION); + out.push(SCHEME_UNPROTECTED); + out.extend_from_slice(plaintext); + return Ok(out); + }; + + // Reject a blank object password BEFORE any derivation (SEC-J). + if pw.is_blank() { + return Err(SecretStoreError::BlankPassphrase); + } + + // Fresh per-object salt so the same password on two objects yields + // different keys and precomputation is defeated. + let mut salt = [0u8; SALT_LEN]; + crypto::random_bytes(&mut salt)?; + // `derive_key` enforces the param bounds before allocating. + let key = crypto::derive_key(pw, &salt, params)?; + let aad = scheme1_aad(¶ms, &salt, wallet_id.as_bytes(), label); + let (nonce, ciphertext) = crypto::seal(&key, &aad, plaintext)?; + + let mut out = + Vec::with_capacity(HEADER_LEN + KDF_FIELD_LEN + SALT_LEN + NONCE_LEN + ciphertext.len()); + out.extend_from_slice(MAGIC); + out.push(ENVELOPE_VERSION); + out.push(SCHEME_PASSWORD); + out.extend_from_slice(&encode_kdf(¶ms)); + out.extend_from_slice(&salt); + out.extend_from_slice(&nonce); + out.extend_from_slice(&ciphertext); + Ok(out) +} + +/// Unwrap `blob` for `(wallet_id, label)`, applying the **strict, +/// fail-closed** read (the L-1 keystone). The "expected-protected" bit is +/// the caller's assertion, surfaced solely by `password`, and is NEVER +/// inferred from the blob's scheme byte. +/// +/// | `password` | stored blob | result | +/// |---|---|---| +/// | `Some(pw)` | valid scheme-1 | secret, or [`WrongPassword`] on tag fail | +/// | `Some(pw)` | scheme-0 **or** magic-less (legacy raw) | [`ExpectedProtectedButUnsealed`] ★ | +/// | `Some(pw)` | scheme-1 but too short | [`Corruption`] (sealed-but-broken) | +/// | `Some/None` | magic present, unknown version/scheme | [`UnsupportedEnvelopeVersion`] | +/// | `None` | valid scheme-1 | [`NeedsPassword`] (never ciphertext) | +/// | `None` | scheme-0 | secret | +/// | `None` | magic-less (legacy raw) | secret (+ one-time warn; re-wrapped on next write) | +/// | `None` | magic present but truncated header | [`Corruption`] | +/// +/// The load-bearing row is `Some(pw)` + non-envelope ⇒ +/// [`ExpectedProtectedButUnsealed`]: with a password in hand, a +/// non-protected blob can only mean a strip → refuse, return no bytes. +/// +/// [`WrongPassword`]: SecretStoreError::WrongPassword +/// [`ExpectedProtectedButUnsealed`]: SecretStoreError::ExpectedProtectedButUnsealed +/// [`Corruption`]: SecretStoreError::Corruption +/// [`UnsupportedEnvelopeVersion`]: SecretStoreError::UnsupportedEnvelopeVersion +/// [`NeedsPassword`]: SecretStoreError::NeedsPassword +pub(crate) fn unwrap( + wallet_id: &WalletId, + label: &str, + password: Option<&SecretString>, + blob: &[u8], +) -> Result { + // Magic-less ⇒ a legacy unprotected raw value (scheme-0-equivalent), + // per the adopted §4.1 read-path contingency. + if !blob.starts_with(MAGIC) { + return match password { + None => { + warn_legacy_once(); + Ok(SecretBytes::from_slice(blob)) + } + // Caller asserted protection but found a magic-less raw value: + // a strip/downgrade ⇒ FAIL CLOSED (L-1). Never returns bytes. + Some(_) => Err(SecretStoreError::ExpectedProtectedButUnsealed), + }; + } + + // Magic present but truncated before version+scheme: a broken envelope. + if blob.len() < HEADER_LEN { + return Err(SecretStoreError::Corruption); + } + + let version = blob[MAGIC.len()]; + if version != ENVELOPE_VERSION { + // Fail closed regardless of password — an unparseable future format + // can be neither safely unwrapped nor treated as scheme-0 (GAP-009). + return Err(SecretStoreError::UnsupportedEnvelopeVersion { found: version }); + } + + let scheme = blob[MAGIC.len() + 1]; + let body = &blob[HEADER_LEN..]; + match scheme { + SCHEME_UNPROTECTED => match password { + None => Ok(SecretBytes::from_slice(body)), + // Strip: caller expected protection, blob is unprotected (L-1). + Some(_) => Err(SecretStoreError::ExpectedProtectedButUnsealed), + }, + SCHEME_PASSWORD => match password { + None => Err(SecretStoreError::NeedsPassword), + Some(pw) => unwrap_scheme1(wallet_id, label, pw, body), + }, + // Unknown scheme under a known version ⇒ forward-incompatible + // layout; report the (known) version byte. Fail closed. + _ => Err(SecretStoreError::UnsupportedEnvelopeVersion { found: version }), + } +} + +/// Decrypt a scheme-1 body. The KDF params, salt, and nonce are all read +/// from the (attacker-controllable) header; the param **ceiling is +/// enforced before** [`crypto::derive_key`] allocates (L-2), and every +/// header field that feeds key/AAD is bound into the AAD so any in-place +/// edit fails the tag (L-3). +fn unwrap_scheme1( + wallet_id: &WalletId, + label: &str, + password: &SecretString, + body: &[u8], +) -> Result { + if body.len() < MIN_SCHEME1_BODY { + // The scheme byte says protected, but the body cannot hold a sealed + // payload — corrupt, not a strip. + return Err(SecretStoreError::Corruption); + } + let kdf = decode_kdf(&body[..KDF_FIELD_LEN]); + // L-2: gate the inflated/unknown header BEFORE any derivation/alloc. + kdf.enforce_bounds()?; + + let mut salt = [0u8; SALT_LEN]; + salt.copy_from_slice(&body[KDF_FIELD_LEN..KDF_FIELD_LEN + SALT_LEN]); + let mut nonce = [0u8; NONCE_LEN]; + nonce.copy_from_slice(&body[KDF_FIELD_LEN + SALT_LEN..KDF_FIELD_LEN + SALT_LEN + NONCE_LEN]); + let ciphertext = &body[KDF_FIELD_LEN + SALT_LEN + NONCE_LEN..]; + + let aad = scheme1_aad(&kdf, &salt, wallet_id.as_bytes(), label); + let key = crypto::derive_key(password, &salt, kdf)?; + match crypto::open(&key, &nonce, &aad, ciphertext) { + Ok(plaintext) => Ok(plaintext), + // Tag failure (wrong password, relocated blob, or header tamper): + // no plaintext is ever materialized (CWE-347). + Err(SecretStoreError::Decrypt) => Err(SecretStoreError::WrongPassword), + Err(e) => Err(e), + } +} + +/// Build the scheme-1 AAD binding object identity + header (L-3), +/// length-prefixed for the variable fields, mirroring +/// [`format::aad`](super::file::format::aad)/`verify_aad`. +fn scheme1_aad( + kdf: &KdfParams, + salt: &[u8; SALT_LEN], + wallet_id: &[u8; 32], + label: &str, +) -> Vec { + let lb = label.as_bytes(); + let mut v = Vec::with_capacity( + TIER2_DOMAIN.len() + + MAGIC.len() + + 2 + + KDF_FIELD_LEN + + 4 + + SALT_LEN + + 4 + + wallet_id.len() + + 4 + + lb.len(), + ); + v.extend_from_slice(TIER2_DOMAIN); + v.extend_from_slice(MAGIC); + v.push(ENVELOPE_VERSION); + v.push(SCHEME_PASSWORD); + v.extend_from_slice(&encode_kdf(kdf)); + v.extend_from_slice(&(salt.len() as u32).to_le_bytes()); + v.extend_from_slice(salt); + 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 +} + +/// Encode KDF params to the fixed 13-byte header field (LE). +fn encode_kdf(kdf: &KdfParams) -> [u8; KDF_FIELD_LEN] { + let mut out = [0u8; KDF_FIELD_LEN]; + out[0] = kdf.id; + out[1..5].copy_from_slice(&kdf.m_kib.to_le_bytes()); + out[5..9].copy_from_slice(&kdf.t.to_le_bytes()); + out[9..13].copy_from_slice(&kdf.p.to_le_bytes()); + out +} + +/// Decode the fixed 13-byte KDF header field. Out-of-range values are +/// caught downstream by [`KdfParams::enforce_bounds`]. +fn decode_kdf(b: &[u8]) -> KdfParams { + debug_assert_eq!(b.len(), KDF_FIELD_LEN); + KdfParams { + id: b[0], + m_kib: u32::from_le_bytes([b[1], b[2], b[3], b[4]]), + t: u32::from_le_bytes([b[5], b[6], b[7], b[8]]), + p: u32::from_le_bytes([b[9], b[10], b[11], b[12]]), + } +} + +/// Emit a single process-lifetime warning that a legacy magic-less entry +/// was read. Carries no secret (the message is static). +fn warn_legacy_once() { + static WARN: Once = Once::new(); + WARN.call_once(|| { + tracing::warn!( + "read a legacy unprotected secret entry with no envelope header; \ + it will be re-wrapped on the next write" + ); + }); +} + +#[cfg(test)] +mod tests { + use subtle::ConstantTimeEq; + + use super::super::file::crypto::{ + ARGON2_MAX_M_KIB, ARGON2_MAX_T, ARGON2_MIN_M_KIB, ARGON2_MIN_T, ARGON2_P, + }; + use super::super::file::format::KDF_ID_ARGON2ID; + use super::*; + + // Wire offsets into a scheme-1 envelope (for surgical tampering). + const O_VERSION: usize = 5; + const O_SCHEME: usize = 6; + const O_KDF: usize = HEADER_LEN; // 7 + const O_ID: usize = O_KDF; // 7 + const O_MKIB: usize = O_KDF + 1; // 8 + const O_T: usize = O_KDF + 5; // 12 + const O_SALT: usize = O_KDF + KDF_FIELD_LEN; // 20 + const O_NONCE: usize = O_SALT + SALT_LEN; // 52 + + fn wid(b: u8) -> WalletId { + WalletId::from([b; 32]) + } + + /// Argon2id floor params — fast enough for unit tests. + fn floor() -> KdfParams { + KdfParams { + id: KDF_ID_ARGON2ID, + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + } + } + + fn pw(s: &str) -> SecretString { + SecretString::new(s) + } + + /// TS-ENV-001: scheme-0 passthrough round-trip; the wrapped form leads + /// with magic, version=1, scheme=0, then the raw payload. + #[test] + fn scheme0_passthrough_round_trip() { + let secret = b"top secret seed bytes"; + let blob = wrap(&wid(1), "seed", None, secret).unwrap(); + assert!(blob.starts_with(MAGIC)); + assert_eq!(blob[O_VERSION], 1); + assert_eq!(blob[O_SCHEME], 0); + assert_eq!(&blob[HEADER_LEN..], secret); + let got = unwrap(&wid(1), "seed", None, &blob).unwrap(); + assert_eq!(got.expose_secret(), secret); + } + + /// TS-ENV-002: scheme-1 round-trip; header records the argon2id id, a + /// 32-byte fresh salt and 24-byte nonce, ct != pt, and two wraps of the + /// same secret/pw differ in salt+nonce (no reuse). + #[test] + fn scheme1_round_trip_and_fresh_salt_nonce() { + let secret = b"correct horse battery staple seed"; + let p = pw("hunter2-but-better"); + let blob = wrap_with_params(&wid(7), "seed", Some(&p), secret, floor()).unwrap(); + assert!(blob.starts_with(MAGIC)); + assert_eq!(blob[O_VERSION], 1); + assert_eq!(blob[O_SCHEME], 1); + assert_eq!(blob[O_ID], KDF_ID_ARGON2ID); + // ciphertext differs from plaintext. + assert_ne!(&blob[O_NONCE + NONCE_LEN..], secret); + + let got = unwrap(&wid(7), "seed", Some(&p), &blob).unwrap(); + assert_eq!(got.expose_secret(), secret); + + let blob2 = wrap_with_params(&wid(7), "seed", Some(&p), secret, floor()).unwrap(); + assert_ne!( + &blob[O_SALT..O_SALT + SALT_LEN], + &blob2[O_SALT..O_SALT + SALT_LEN], + "salt must be fresh per wrap" + ); + assert_ne!( + &blob[O_NONCE..O_NONCE + NONCE_LEN], + &blob2[O_NONCE..O_NONCE + NONCE_LEN], + "nonce must be fresh per wrap" + ); + } + + /// TS-ENV-003: wrong object password → WrongPassword, no plaintext. + #[test] + fn wrong_password_fails_closed() { + let blob = wrap_with_params(&wid(1), "seed", Some(&pw("right")), b"seed", floor()).unwrap(); + let err = unwrap(&wid(1), "seed", Some(&pw("wrong")), &blob).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TS-ENV-004: identity AAD (L-3) — a protected blob unwrapped at any + /// other (wallet, label) fails the tag; same-identity still succeeds. + #[test] + fn relocation_across_identity_is_rejected() { + let p = pw("pw"); + let blob = wrap_with_params(&wid(0xA), "labelA", Some(&p), b"seed", floor()).unwrap(); + for (w, l) in [(0xB, "labelB"), (0xA, "labelB"), (0xB, "labelA")] { + let err = unwrap(&wid(w), l, Some(&p), &blob).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "relocation to ({w:#x},{l}) must fail, got {err:?}" + ); + } + let ok = unwrap(&wid(0xA), "labelA", Some(&p), &blob).unwrap(); + assert_eq!(ok.expose_secret(), b"seed"); + } + + /// TS-ENV-005: per-field header tamper. Unknown KDF id is rejected by + /// `enforce_bounds` (KdfFailure) before derive; in-bounds KDF shifts, + /// salt, and nonce all fail the AEAD tag (WrongPassword) — never the + /// plaintext. + #[test] + fn header_tamper_fails_closed_per_field() { + let p = pw("pw"); + let base = wrap_with_params(&wid(1), "seed", Some(&p), b"seed", floor()).unwrap(); + + // kdf.id → 7 (unknown) ⇒ KdfFailure (bounds reject pre-derive). + let mut b = base.clone(); + b[O_ID] = 7; + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::KdfFailure + )); + + // kdf.m_kib → a different IN-BOUNDS value ⇒ WrongPassword (AAD + key). + let mut b = base.clone(); + b[O_MKIB..O_MKIB + 4].copy_from_slice(&(ARGON2_MIN_M_KIB + 1024).to_le_bytes()); + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::WrongPassword + )); + + // kdf.t → a different IN-BOUNDS value ⇒ WrongPassword. + let mut b = base.clone(); + b[O_T..O_T + 4].copy_from_slice(&(ARGON2_MIN_T + 1).to_le_bytes()); + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::WrongPassword + )); + + // salt[0] flip ⇒ WrongPassword (wrong key + AAD-bound salt). + let mut b = base.clone(); + b[O_SALT] ^= 1; + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::WrongPassword + )); + + // nonce[0] flip ⇒ WrongPassword (nonce feeds decrypt ⇒ tag fail). + let mut b = base; + b[O_NONCE] ^= 1; + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::WrongPassword + )); + } + + /// TS-ENV-006 ★ (L-2): an inflated KDF param on a forged header is + /// rejected by `enforce_bounds` BEFORE `derive_key` allocates — the + /// ~4 TiB allocation never happens (the test would OOM if it did). The + /// exact ceilings remain valid params. + #[test] + fn kdf_ceiling_enforced_before_derivation() { + let p = pw("pw"); + let base = wrap_with_params(&wid(1), "seed", Some(&p), b"seed", floor()).unwrap(); + + // m_kib = u32::MAX ⇒ KdfFailure, no allocation. + let mut b = base.clone(); + b[O_MKIB..O_MKIB + 4].copy_from_slice(&u32::MAX.to_le_bytes()); + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::KdfFailure + )); + + // t = ARGON2_MAX_T + 1 ⇒ KdfFailure. + let mut b = base; + b[O_T..O_T + 4].copy_from_slice(&(ARGON2_MAX_T + 1).to_le_bytes()); + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &b).unwrap_err(), + SecretStoreError::KdfFailure + )); + + // The exact ceilings are accepted by the bounds check (no derive + // here — a 1 GiB Argon2 run is not a unit-test concern). + assert!(KdfParams { + id: KDF_ID_ARGON2ID, + m_kib: ARGON2_MAX_M_KIB, + t: ARGON2_MAX_T, + p: ARGON2_P, + } + .enforce_bounds() + .is_ok()); + } + + /// TS-ENV-007: a blank object password is rejected at enrol; nothing + /// is sealed. + #[test] + fn blank_object_password_rejected_at_enrol() { + for blank in [SecretString::empty(), pw(""), pw(" "), pw("\t\n")] { + let err = + wrap_with_params(&wid(1), "seed", Some(&blank), b"seed", floor()).unwrap_err(); + assert!( + matches!(err, SecretStoreError::BlankPassphrase), + "got {err:?}" + ); + } + } + + /// TS-ENV-008 (SEC-F006 / GAP-006, v5 cap): the plaintext cap is + /// `MAX_SECRET_LEN − MAX_ENVELOPE_OVERHEAD` (NOT `MAX_SECRET_LEN` as + /// the v4 spec literally read), uniform across schemes, so the + /// enveloped bytes always fit the backend vault cap. Accept at the cap, + /// reject at cap+1 with `max = MAX_PLAINTEXT_LEN`. + #[test] + fn plaintext_size_cap_at_envelope_boundary() { + let at_cap = vec![0x5Au8; MAX_PLAINTEXT_LEN]; + let over = vec![0x5Au8; MAX_PLAINTEXT_LEN + 1]; + + // Unprotected (scheme 0): cap accepted, +1 rejected. + assert!(wrap(&wid(1), "seed", None, &at_cap).is_ok()); + assert!(matches!( + wrap(&wid(1), "seed", None, &over).unwrap_err(), + SecretStoreError::SecretTooLarge { found, max } + if found == MAX_PLAINTEXT_LEN + 1 && max == MAX_PLAINTEXT_LEN + )); + + // Protected (scheme 1): same cap (checked before any derivation). + let p = pw("pw"); + assert!(matches!( + wrap_with_params(&wid(1), "seed", Some(&p), &over, floor()).unwrap_err(), + SecretStoreError::SecretTooLarge { found, max } + if found == MAX_PLAINTEXT_LEN + 1 && max == MAX_PLAINTEXT_LEN + )); + // The enveloped bytes for an at-cap plaintext fit the backend cap. + let enveloped = wrap(&wid(1), "seed", None, &at_cap).unwrap(); + assert!(enveloped.len() <= MAX_SECRET_LEN); + } + + /// TS-ENV-010 (adopted §4.1 legacy-tolerant contingency): magic/version + /// discrimination. A magic-less blob is a legacy raw value — returned + /// on `None`, refused fail-closed on `Some(pw)`. A magic-present blob + /// with an unknown version fails closed both ways; truncated-after-magic + /// is corruption. + #[test] + fn magic_and_version_discrimination() { + let p = pw("pw"); + // (a) Magic-less / wrong magic. + let legacy = b"NOTPWSEV raw legacy seed bytes".to_vec(); + // None ⇒ legacy raw bytes (adopted contingency; NOT Corruption). + let got = unwrap(&wid(1), "seed", None, &legacy).unwrap(); + assert_eq!(got.expose_secret(), &legacy[..]); + // Some(pw) ⇒ strip/downgrade ⇒ fail closed (L-1 preserved). + assert!(matches!( + unwrap(&wid(1), "seed", Some(&p), &legacy).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + )); + + // (b) Magic present but truncated below the header ⇒ Corruption. + let mut trunc = MAGIC.to_vec(); + trunc.push(ENVELOPE_VERSION); // no scheme byte + assert!(matches!( + unwrap(&wid(1), "seed", None, &trunc).unwrap_err(), + SecretStoreError::Corruption + )); + + // (c) Magic OK but version = 2 ⇒ UnsupportedEnvelopeVersion{2}, + // regardless of password (GAP-009). + let mut v2 = wrap(&wid(1), "seed", None, b"x").unwrap(); + v2[O_VERSION] = 2; + for arg in [None, Some(&p)] { + assert!(matches!( + unwrap(&wid(1), "seed", arg, &v2).unwrap_err(), + SecretStoreError::UnsupportedEnvelopeVersion { found: 2 } + )); + } + + // (d) Magic+version OK but unknown scheme = 9 ⇒ fail closed. + let mut s9 = wrap(&wid(1), "seed", None, b"x").unwrap(); + s9[O_SCHEME] = 9; + assert!(matches!( + unwrap(&wid(1), "seed", None, &s9).unwrap_err(), + SecretStoreError::UnsupportedEnvelopeVersion { found: 1 } + )); + } + + /// Non-vacuity helper for the L-1 keystone (used here and by the store + /// tests): a scheme-0 blob carrying `secret` DOES decode under `None`. + #[test] + fn scheme0_some_password_fails_closed_strip() { + let blob = wrap(&wid(1), "seed", None, b"attacker-seed").unwrap(); + // None ⇒ it WOULD decode to the (attacker) bytes… + assert_eq!( + unwrap(&wid(1), "seed", None, &blob) + .unwrap() + .expose_secret(), + b"attacker-seed" + ); + // …but Some(pw) ⇒ ExpectedProtectedButUnsealed, no bytes (L-1). + assert!(matches!( + unwrap(&wid(1), "seed", Some(&pw("pw")), &blob).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + )); + } + + /// `ct_eq` sanity: a round-tripped secret matches the original under a + /// constant-time compare (no `==` on secret bytes). + #[test] + fn round_trip_is_constant_time_equal() { + let p = pw("pw"); + let original = SecretBytes::from_slice(b"seed material"); + let blob = + wrap_with_params(&wid(1), "seed", Some(&p), original.expose_secret(), floor()).unwrap(); + let got = unwrap(&wid(1), "seed", Some(&p), &blob).unwrap(); + assert!(bool::from(got.ct_eq(&original))); + } + + /// TS-ENV-009: deterministic byte-level fuzz. Every mutant unwrap is a + /// clean `Ok` or a TYPED `SecretStoreError` — never a panic, never + /// plaintext from a tag-failing branch. The `None` path (no Argon2 + /// derivation) runs the full 2000 mutants + every truncation; the + /// `Some(pw)` path — each mutant of which may trigger a real Argon2 + /// derive — runs a representative subset so the suite stays fast while + /// still exercising the derive/open code path. + #[test] + fn fuzz_byte_mutation_never_panics() { + let p = pw("fuzz-pw"); + let valid = wrap_with_params(&wid(0xAB), "seed", Some(&p), b"seed-bytes", floor()).unwrap(); + // The pristine envelope unwraps. + assert_eq!( + unwrap(&wid(0xAB), "seed", Some(&p), &valid) + .unwrap() + .expose_secret(), + b"seed-bytes" + ); + + // xorshift32 — deterministic, std-only. + let mut state: u32 = 0x9E37_79B9; + let mut next = || { + state ^= state << 13; + state ^= state >> 17; + state ^= state << 5; + state + }; + + let assert_typed = |arg: Option<&SecretString>, buf: &[u8]| { + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + unwrap(&wid(0xAB), "seed", arg, buf) + })) + .expect("unwrap must never panic on hostile input"); + match res { + Ok(_) + | Err(SecretStoreError::Corruption) + | Err(SecretStoreError::WrongPassword) + | Err(SecretStoreError::NeedsPassword) + | Err(SecretStoreError::ExpectedProtectedButUnsealed) + | Err(SecretStoreError::UnsupportedEnvelopeVersion { .. }) + | Err(SecretStoreError::KdfFailure) => {} + Err(other) => panic!("unexpected error variant: {other:?}"), + } + }; + + for i in 0..2_000 { + let mut buf = valid.clone(); + let flips = 1 + (next() % 4) as usize; + for _ in 0..flips { + let idx = (next() as usize) % buf.len(); + buf[idx] ^= (next() & 0xFF) as u8; + } + // None path every iteration (cheap, no derive). + assert_typed(None, &buf); + // Some path on a representative subset (each may derive Argon2). + if i % 16 == 0 { + assert_typed(Some(&p), &buf); + } + } + + // Truncation at every offset — a short read must never panic. + for cut in 0..valid.len() { + assert_typed(None, &valid[..cut]); + assert_typed(Some(&p), &valid[..cut]); + } + } +} 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 628b249f8e..1c91b8cd23 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -34,8 +34,13 @@ //! by zeroize + mlock. The derived AEAD key stays resident in a //! [`SecretBytes`] (to avoid per-op Argon2) and is zeroized on Drop. -mod crypto; -mod format; +// `pub(super)` (= visible within `crate::secrets`) so the Tier-2 +// `envelope` module — a sibling of `file` under `secrets` — can reuse the +// shared Argon2id/XChaCha primitives and `KDF_ID_ARGON2ID` without +// duplicating crypto. Items inside stay `pub(crate)`/`pub(in …file)`, so +// nothing escapes the secrets tree (see the crypto.rs module doc). +pub(super) mod crypto; +pub(super) mod format; use std::any::Any; use std::collections::HashMap; diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index c8f82e161f..4161e42300 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -22,6 +22,7 @@ //! `tests/secrets_scan.rs` exempts it, so it owns its own review //! discipline via `tests/secrets_guard.rs`. +mod envelope; mod error; mod file; mod keyring; @@ -29,6 +30,7 @@ mod secret; mod store; mod validate; +pub use envelope::MAX_PLAINTEXT_LEN; pub use error::{IoError, OsKeyringErrorKind, SecretStoreError}; pub use file::{ EncryptedFileCredential, EncryptedFileStore, MAX_SECRET_LEN, MAX_VAULT_SIZE_BYTES, From 491229b592e23f26671d200a23f4bea71a3eb12f Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:22:07 +0200 Subject: [PATCH 04/10] feat(platform-wallet-storage)!: strict fail-closed Tier-2 read (L-1 keystone) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 4 of the layered secret-protection feature — THE load-bearing control. Wires the envelope's strict, fail-closed quadrant into the store read path over BOTH arms. - `SecretStore::get_secret(service, label, password: Option<&SecretString>)` reads the opaque backend bytes via a new shared `get_raw` seam, then runs `envelope::unwrap`. The "expected-protected" bit lives SOLELY in the caller's `Some/None` argument — never inferred from the stored blob, and never persisted by the library. `get` is refactored to delegate to `get_raw` (behaviour unchanged). - GAP-005 fixture: `secrets::testing::InMemoryCredentialStore`, a writable in-memory `CredentialStoreApi` mock (gated on cfg(test)/`__test-helpers`) with a `raw_overwrite` attacker primitive, so the Os arm — where the L-1 residual bites hardest (§8.3) — and the File re-seal-under-vault-key strip are both coverable in CI. Tests (TS-L1-001..006), each parameterised over File AND Os: - ★ TS-L1-002 strip-injection (non-vacuous): a protected scheme-1 object is overwritten with a well-formed scheme-0 blob carrying a DIFFERENT seed; get_secret(Some(pw)) ⇒ ExpectedProtectedButUnsealed, the attacker seed is NEVER returned — and the same blob WOULD decode to S_evil under None, proving the refusal is the strict rule, not malformation. - Full quadrant; both DET-bug directions fail closed; expectation never inferred from the scheme byte; upgrade-confusion is DoS-only; in-place 1→0 scheme flip (Some fails closed; None GAP-010 residual pinned). Deviation (documented): magic-less + None → legacy raw bytes (adopted §4.1 contingency), not Corruption as the v4 TS-L1-001 row read; magic-less + Some(pw) still fails closed, so L-1 is intact. `!`: get_secret is additive, but the strict read changes how a password-supplied read of a non-enveloped slot behaves (now refuses). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/envelope.rs | 11 +- .../src/secrets/mod.rs | 4 + .../src/secrets/store.rs | 426 +++++++++++++++++- .../src/secrets/testing.rs | 182 ++++++++ 4 files changed, 615 insertions(+), 8 deletions(-) create mode 100644 packages/rs-platform-wallet-storage/src/secrets/testing.rs diff --git a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs index 958ab57966..c987e068a4 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs @@ -42,13 +42,6 @@ //! [`format::aad`]: super::file::format::aad //! [`format::verify_aad`]: super::file::format::verify_aad -// The wrap/unwrap primitives are exercised by this module's own tests but -// are not yet called from non-test code: the strict-read wiring into -// `SecretStore::get_secret`/`set_secret` lands in the next task, which -// removes this allow. Without it the not-yet-wired primitives would trip -// dead-code warnings in the non-test build. -#![allow(dead_code)] - use std::sync::Once; use super::error::SecretStoreError; @@ -103,6 +96,9 @@ pub const MAX_PLAINTEXT_LEN: usize = MAX_SECRET_LEN - MAX_ENVELOPE_OVERHEAD; /// `None` → an unprotected (scheme-0) envelope; `Some(pw)` → a scheme-1 /// envelope sealed under `pw`. A blank password is rejected at enrol /// ([`SecretStoreError::BlankPassphrase`]). +// The write side is wired into `SecretStore::set_secret` in the next task; +// until then it is exercised only by this module's tests. +#[allow(dead_code)] pub(crate) fn wrap( wallet_id: &WalletId, label: &str, @@ -121,6 +117,7 @@ pub(crate) fn wrap( /// [`wrap`] with explicit Argon2 `params` (tests use the floor params for /// speed; production uses [`KdfParams::default_target`]). `params` is /// ignored when `password` is `None`. +#[allow(dead_code)] pub(crate) fn wrap_with_params( wallet_id: &WalletId, label: &str, diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index 4161e42300..32a9758e53 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -28,6 +28,10 @@ mod file; mod keyring; mod secret; mod store; +/// In-memory backend test fixtures (writable `CredentialStoreApi` mock). +/// Compiled only under `cfg(test)` or the `__test-helpers` feature. +#[cfg(any(test, feature = "__test-helpers"))] +pub mod testing; mod validate; pub use envelope::MAX_PLAINTEXT_LEN; diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index 82157f4338..aea89a5794 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -12,8 +12,9 @@ use std::sync::Arc; use keyring_core::api::CredentialStoreApi; use keyring_core::{Entry, Error as KeyringError}; +use super::envelope; use super::error::{OsKeyringErrorKind, SecretStoreError}; -use super::secret::SecretBytes; +use super::secret::{SecretBytes, SecretString}; use super::validate::WalletId; use super::{default_credential_store, EncryptedFileStore, SERVICE_PREFIX}; @@ -77,6 +78,22 @@ impl SecretStore { &self, service: &WalletId, label: &str, + ) -> Result, SecretStoreError> { + self.get_raw(service, label) + } + + /// Read the opaque bytes stored under `(service, label)`, or `Ok(None)` + /// if absent — the raw backend value (a Tier-2 envelope once writes go + /// through [`set_secret`](SecretStore::set_secret), or a legacy raw + /// value). The typed-vs-SPI distinction is preserved exactly as the + /// pre-Tier-2 path did. This is the shared seam under [`get`] and + /// [`get_secret`]; it does NOT interpret the envelope. + /// + /// [`get`]: SecretStore::get + fn get_raw( + &self, + service: &WalletId, + label: &str, ) -> Result, SecretStoreError> { match self { // Inherent typed path: keeps WrongPassphrase vs Corruption @@ -93,6 +110,43 @@ impl SecretStore { } } + /// Retrieve the secret under `(service, label)` applying the Tier-2 + /// **strict, fail-closed** read (the L-1 keystone), or `Ok(None)` if + /// absent. + /// + /// `password` IS the caller's protection assertion — supply `Some(pw)` + /// for an object the caller's trusted model says is protected, `None` + /// otherwise. The expectation lives ONLY here, never in the stored + /// blob (see [`envelope::unwrap`]): + /// + /// - `Some(pw)` + valid scheme-1 → the secret (or + /// [`WrongPassword`](SecretStoreError::WrongPassword) on tag fail); + /// - `Some(pw)` + a non-protected blob (scheme-0 / legacy raw) → + /// [`ExpectedProtectedButUnsealed`](SecretStoreError::ExpectedProtectedButUnsealed) + /// — a strip/downgrade, refused, no bytes returned ★; + /// - `None` + scheme-1 → + /// [`NeedsPassword`](SecretStoreError::NeedsPassword) (never ciphertext); + /// - `None` + scheme-0 / legacy raw → the secret. + /// + /// **Documented residual:** an attacker who ALSO rewrites the + /// consumer's trusted DB so the caller passes `None` for a stripped + /// object can still downgrade — out of this library's reach by + /// construction (the protection expectation is the caller's; see + /// `SECRETS.md`). The expectation is NEVER persisted by the library. + pub fn get_secret( + &self, + service: &WalletId, + label: &str, + password: Option<&SecretString>, + ) -> Result, SecretStoreError> { + // Absence is availability-only (deletion = DoS, never injection): + // a missing entry is Ok(None) under either password argument. + let Some(stored) = self.get_raw(service, label)? else { + return Ok(None); + }; + envelope::unwrap(service, label, password, stored.expose_secret()).map(Some) + } + /// Delete the secret stored under `(service, label)`. Absent entries /// are a no-op (`Ok(())`), so deletion is idempotent. pub fn delete(&self, service: &WalletId, label: &str) -> Result<(), SecretStoreError> { @@ -357,4 +411,374 @@ mod tests { ); } } + + // ===== Tier-2 strict fail-closed read — the L-1 keystone ===== + // + // Parameterised over BOTH arms. The "attacker who can write the + // backend" is modelled per arm by `Backend::place_raw`: on File it + // re-seals the chosen blob under the resident vault key via `put_bytes` + // (a cold/backup-swap actor could only corrupt → DoS, so the strip + // requires the vault key — §8.3 arm asymmetry); on Os it overwrites the + // mock keychain item directly (the bare envelope, no second AEAD — where + // the L-1 residual bites hardest, GAP-005 / §8.3). + + use crate::secrets::file::crypto::{KdfParams, ARGON2_MIN_M_KIB, ARGON2_MIN_T, ARGON2_P}; + use crate::secrets::file::format::KDF_ID_ARGON2ID; + use crate::secrets::testing::InMemoryCredentialStore; + + /// Argon2id floor params — fast enough for the keystone tests. + fn floor() -> KdfParams { + KdfParams { + id: KDF_ID_ARGON2ID, + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + } + } + + fn protected(w: &WalletId, label: &str, pw: &str, secret: &[u8]) -> Vec { + envelope::wrap_with_params(w, label, Some(&SecretString::new(pw)), secret, floor()).unwrap() + } + + fn unprotected(w: &WalletId, label: &str, secret: &[u8]) -> Vec { + envelope::wrap(w, label, None, secret).unwrap() + } + + /// A backend under test plus the raw-write hook that plays the + /// backend-write attacker. + struct Backend { + store: SecretStore, + _dir: Option, + mock: Option, + name: &'static str, + } + + impl Backend { + /// Write `blob` to `(w, label)` as opaque backend bytes (the + /// attacker's primitive / the protected-enrol setup). + fn place_raw(&self, w: &WalletId, label: &str, blob: &[u8]) { + match (&self.store, &self.mock) { + (SecretStore::File(fs), _) => fs + .put_bytes(w, label, &SecretBytes::from_slice(blob)) + .unwrap(), + (SecretStore::Os(_), Some(mock)) => mock.raw_overwrite(w, label, blob), + _ => unreachable!("os backend must carry its mock"), + } + } + } + + fn file_backend() -> Backend { + let dir = tempfile::tempdir().unwrap(); + let store = file_store(dir.path()); + Backend { + store, + _dir: Some(dir), + mock: None, + name: "File", + } + } + + fn os_backend() -> Backend { + let mock = InMemoryCredentialStore::new(); + let store = SecretStore::Os(mock.as_dyn()); + Backend { + store, + _dir: None, + mock: Some(mock), + name: "Os", + } + } + + /// TS-L1-001: the strict-read QUADRANT. + fn run_quadrant(b: &Backend) { + let w = wid(1); + let pw = SecretString::new("object-pw"); + + // scheme-0 + None → bytes (the ONLY byte-returning quadrant). + b.place_raw(&w, "u0", &unprotected(&w, "u0", b"plain-seed")); + assert_eq!( + b.store + .get_secret(&w, "u0", None) + .unwrap() + .unwrap() + .expose_secret(), + b"plain-seed", + "[{}] scheme-0 + None", + b.name + ); + + // scheme-1 + None → NeedsPassword (never ciphertext). + b.place_raw(&w, "p1", &protected(&w, "p1", "object-pw", b"real-seed")); + assert!( + matches!( + b.store.get_secret(&w, "p1", None).unwrap_err(), + SecretStoreError::NeedsPassword + ), + "[{}] scheme-1 + None", + b.name + ); + + // scheme-1 + Some(correct) → secret. + assert_eq!( + b.store + .get_secret(&w, "p1", Some(&pw)) + .unwrap() + .unwrap() + .expose_secret(), + b"real-seed", + "[{}] scheme-1 + Some(correct)", + b.name + ); + + // scheme-1 + Some(wrong) → WrongPassword. + assert!( + matches!( + b.store + .get_secret(&w, "p1", Some(&SecretString::new("nope"))) + .unwrap_err(), + SecretStoreError::WrongPassword + ), + "[{}] scheme-1 + Some(wrong)", + b.name + ); + + // ★ scheme-0 + Some(pw) → ExpectedProtectedButUnsealed (fail closed). + assert!( + matches!( + b.store.get_secret(&w, "u0", Some(&pw)).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + ), + "[{}] scheme-0 + Some", + b.name + ); + + // magic-present-but-truncated + None → Corruption. + let mut trunc = envelope::MAGIC.to_vec(); + trunc.push(envelope::ENVELOPE_VERSION); // no scheme byte + b.place_raw(&w, "broken", &trunc); + assert!( + matches!( + b.store.get_secret(&w, "broken", None).unwrap_err(), + SecretStoreError::Corruption + ), + "[{}] truncated-with-magic + None", + b.name + ); + + // magic-less legacy raw + None → bytes (adopted §4.1 contingency; + // deviates from v4 TS-L1-001's Corruption row). + Some → fail closed. + b.place_raw(&w, "legacy", b"raw-legacy-seed-no-magic"); + assert_eq!( + b.store + .get_secret(&w, "legacy", None) + .unwrap() + .unwrap() + .expose_secret(), + b"raw-legacy-seed-no-magic", + "[{}] legacy magic-less + None", + b.name + ); + assert!( + matches!( + b.store.get_secret(&w, "legacy", Some(&pw)).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + ), + "[{}] legacy magic-less + Some", + b.name + ); + + // absent entry → Ok(None) under either arg (deletion = DoS). + assert!(b.store.get_secret(&w, "absent", None).unwrap().is_none()); + assert!(b + .store + .get_secret(&w, "absent", Some(&pw)) + .unwrap() + .is_none()); + } + + #[test] + fn l1_quadrant_file() { + run_quadrant(&file_backend()); + } + + #[test] + fn l1_quadrant_os() { + run_quadrant(&os_backend()); + } + + /// TS-L1-002 ★ — the non-vacuous strip-injection regression. The single + /// test the whole feature exists to make pass. + fn run_strip_injection(b: &Backend) { + let w = wid(2); + let pw = SecretString::new("object-pw"); + + // Enrol protected: stored = a valid scheme-1 envelope of S_real. + b.place_raw( + &w, + "seed", + &protected(&w, "seed", "object-pw", b"REAL-SEED-S_real"), + ); + assert_eq!( + b.store + .get_secret(&w, "seed", Some(&pw)) + .unwrap() + .unwrap() + .expose_secret(), + b"REAL-SEED-S_real", + "[{}] legit protected read", + b.name + ); + + // Attacker overwrites the slot with a fresh, internally-valid + // scheme-0 envelope carrying a DIFFERENT seed S_evil. + let attacker_blob = unprotected(&w, "seed", b"EVIL-SEED-S_evil"); + b.place_raw(&w, "seed", &attacker_blob); + + // ★ A password-supplied read of the stripped slot fails closed; + // S_evil is NEVER returned. + let err = b.store.get_secret(&w, "seed", Some(&pw)).unwrap_err(); + assert!( + matches!(err, SecretStoreError::ExpectedProtectedButUnsealed), + "[{}] strip must fail closed, got {err:?}", + b.name + ); + + // Non-vacuity: the attacker blob IS a valid unprotected envelope + // that WOULD decode to S_evil under `None` — so the refusal above is + // caused SOLELY by the Some(pw)+scheme-0 strict rule, not by any + // malformation (without the strict rule, S_evil would be returned). + let would_be = envelope::unwrap(&w, "seed", None, &attacker_blob).unwrap(); + assert_eq!( + would_be.expose_secret(), + b"EVIL-SEED-S_evil", + "[{}] non-vacuity: blob decodes to S_evil under None", + b.name + ); + } + + #[test] + fn l1_strip_injection_file() { + run_strip_injection(&file_backend()); + } + + #[test] + fn l1_strip_injection_os() { + run_strip_injection(&os_backend()); + } + + /// TS-L1-003: a DET bug alone fails closed in BOTH directions. + fn run_both_det_bug_directions(b: &Backend) { + let w = wid(3); + let pw = SecretString::new("pw"); + // (a) over-supply a password on a genuinely unprotected object. + b.place_raw(&w, "u", &unprotected(&w, "u", b"x")); + assert!(matches!( + b.store.get_secret(&w, "u", Some(&pw)).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + )); + // (b) under-supply on a genuinely protected object. + b.place_raw(&w, "p", &protected(&w, "p", "pw", b"y")); + assert!(matches!( + b.store.get_secret(&w, "p", None).unwrap_err(), + SecretStoreError::NeedsPassword + )); + } + + #[test] + fn l1_both_det_bug_directions_file() { + run_both_det_bug_directions(&file_backend()); + } + + #[test] + fn l1_both_det_bug_directions_os() { + run_both_det_bug_directions(&os_backend()); + } + + /// TS-L1-004: the expectation is NEVER inferred from the blob's scheme + /// byte — identical scheme-1 blobs diverge solely on the password arg. + fn run_expectation_not_inferred(b: &Backend) { + let w = wid(4); + let pw = SecretString::new("pw"); + let blob = protected(&w, "a", "pw", b"seed"); + b.place_raw(&w, "a", &blob); + b.place_raw(&w, "b", &blob); + assert_eq!( + b.store + .get_secret(&w, "a", Some(&pw)) + .unwrap() + .unwrap() + .expose_secret(), + b"seed" + ); + assert!(matches!( + b.store.get_secret(&w, "b", None).unwrap_err(), + SecretStoreError::NeedsPassword + )); + } + + #[test] + fn l1_expectation_not_inferred_file() { + run_expectation_not_inferred(&file_backend()); + } + + #[test] + fn l1_expectation_not_inferred_os() { + run_expectation_not_inferred(&os_backend()); + } + + /// TS-L1-005: unprotected→scheme-1 upgrade confusion is availability- + /// only, fail-closed (NeedsPassword), no leak / no injection. + fn run_upgrade_confusion(b: &Backend) { + let w = wid(5); + b.place_raw(&w, "x", &protected(&w, "x", "attacker-pw", b"whatever")); + assert!(matches!( + b.store.get_secret(&w, "x", None).unwrap_err(), + SecretStoreError::NeedsPassword + )); + } + + #[test] + fn l1_upgrade_confusion_file() { + run_upgrade_confusion(&file_backend()); + } + + #[test] + fn l1_upgrade_confusion_os() { + run_upgrade_confusion(&os_backend()); + } + + /// TS-L1-006: an in-place scheme-byte flip (1→0). Some(pw) is caught by + /// the strict rule regardless. None reads the body as scheme-0 opaque + /// bytes (never the real seed) — the GAP-010 residual, dominated by the + /// DET-DB residual; pinned, not "fixed". + fn run_scheme_flip(b: &Backend) { + let w = wid(6); + let pw = SecretString::new("pw"); + let mut blob = protected(&w, "x", "pw", b"real-seed"); + let scheme_off = envelope::MAGIC.len() + 1; + assert_eq!(blob[scheme_off], envelope::SCHEME_PASSWORD); + blob[scheme_off] = envelope::SCHEME_UNPROTECTED; + b.place_raw(&w, "x", &blob); + + assert!(matches!( + b.store.get_secret(&w, "x", Some(&pw)).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + )); + let got = b.store.get_secret(&w, "x", None).unwrap().unwrap(); + assert_ne!( + got.expose_secret(), + b"real-seed", + "the real seed must never surface from a flipped scheme byte" + ); + } + + #[test] + fn l1_scheme_flip_file() { + run_scheme_flip(&file_backend()); + } + + #[test] + fn l1_scheme_flip_os() { + run_scheme_flip(&os_backend()); + } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/testing.rs b/packages/rs-platform-wallet-storage/src/secrets/testing.rs new file mode 100644 index 0000000000..a59aa3b7fa --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/testing.rs @@ -0,0 +1,182 @@ +//! In-memory test fixtures for the secrets backends. +//! +//! [`InMemoryCredentialStore`] is a writable +//! [`CredentialStoreApi`](keyring_core::api::CredentialStoreApi) double for +//! the [`SecretStore::Os`](crate::secrets::SecretStore::Os) arm — the only +//! shipped credential stores need a live OS keychain / D-Bus session, so +//! the Os arm (where the L-1 strip residual "bites hardest", per the threat +//! model §8.3) is otherwise un-coverable in CI (GAP-005). +//! +//! Beyond the SPI, it exposes [`raw_overwrite`](InMemoryCredentialStore::raw_overwrite) +//! / [`raw_get`](InMemoryCredentialStore::raw_get) so a test can play the +//! **backend-write attacker**: replace a slot's stored bytes with an +//! arbitrary blob (e.g. a stripped scheme-0 envelope) the way a breached +//! keychain would, then assert the strict read still fails closed. +//! +//! Compiled under `cfg(test)` or the `__test-helpers` feature, never in a +//! production build. + +use std::any::Any; +use std::collections::HashMap; +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::validate::WalletId; +use super::SERVICE_PREFIX; + +/// Shared `(service, user) -> bytes` map backing every credential a store +/// hands out, so writes through one entry are visible to another. +type Items = Arc>>>; + +/// An in-memory, writable [`CredentialStoreApi`] for Os-arm tests. +#[derive(Default)] +pub struct InMemoryCredentialStore { + items: Items, +} + +impl InMemoryCredentialStore { + /// A fresh, empty store, ready to install as + /// `SecretStore::Os(store.into_arc())`. + pub fn new() -> Self { + Self::default() + } + + /// Hand back an `Arc` for the + /// `SecretStore::Os` arm. Clones share the same backing map, so the + /// returned trait object and `self` see each other's writes. + pub fn as_dyn(&self) -> Arc { + Arc::new(Self { + items: Arc::clone(&self.items), + }) + } + + /// The service string `SecretStore` derives for `wallet_id`, so a test + /// can address the exact slot the Os arm reads/writes. + pub fn service_for(wallet_id: &WalletId) -> String { + format!("{SERVICE_PREFIX}{}", wallet_id.to_hex()) + } + + /// **Attacker primitive:** overwrite the raw bytes at `(wallet_id, + /// label)` with `bytes`, bypassing any envelope layer — exactly what a + /// breached keychain write does. + pub fn raw_overwrite(&self, wallet_id: &WalletId, label: &str, bytes: &[u8]) { + self.lock().insert( + (Self::service_for(wallet_id), label.to_string()), + bytes.to_vec(), + ); + } + + /// Read the raw stored bytes at `(wallet_id, label)`, if any. + pub fn raw_get(&self, wallet_id: &WalletId, label: &str) -> Option> { + self.lock() + .get(&(Self::service_for(wallet_id), label.to_string())) + .cloned() + } + + fn lock(&self) -> std::sync::MutexGuard<'_, HashMap<(String, String), Vec>> { + self.items.lock().unwrap_or_else(|p| p.into_inner()) + } +} + +impl CredentialStoreApi for InMemoryCredentialStore { + fn vendor(&self) -> String { + "dash.platform-wallet-storage/in-memory-test".to_string() + } + + fn id(&self) -> String { + "in-memory-credential-store".to_string() + } + + fn build( + &self, + service: &str, + user: &str, + _modifiers: Option<&HashMap<&str, &str>>, + ) -> KeyringResult { + let cred = InMemoryCredential { + items: Arc::clone(&self.items), + service: service.to_string(), + user: user.to_string(), + }; + 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 InMemoryCredentialStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Never render stored bytes (they may be secret material). + f.debug_struct("InMemoryCredentialStore") + .field("entries", &self.lock().len()) + .finish() + } +} + +/// One `(service, user)` slot in an [`InMemoryCredentialStore`]. +struct InMemoryCredential { + items: Items, + service: String, + user: String, +} + +impl InMemoryCredential { + fn key(&self) -> (String, String) { + (self.service.clone(), self.user.clone()) + } + + fn lock(&self) -> std::sync::MutexGuard<'_, HashMap<(String, String), Vec>> { + self.items.lock().unwrap_or_else(|p| p.into_inner()) + } +} + +impl CredentialApi for InMemoryCredential { + fn set_secret(&self, secret: &[u8]) -> KeyringResult<()> { + self.lock().insert(self.key(), secret.to_vec()); + Ok(()) + } + + fn get_secret(&self) -> KeyringResult> { + self.lock() + .get(&self.key()) + .cloned() + .ok_or(KeyringError::NoEntry) + } + + fn delete_credential(&self) -> KeyringResult<()> { + if self.lock().remove(&self.key()).is_some() { + Ok(()) + } else { + Err(KeyringError::NoEntry) + } + } + + fn get_credential(&self) -> KeyringResult>> { + Ok(None) + } + + fn get_specifiers(&self) -> Option<(String, String)> { + Some(self.key()) + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +impl std::fmt::Debug for InMemoryCredential { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("InMemoryCredential") + .field("service", &self.service) + .field("user", &self.user) + .finish_non_exhaustive() + } +} From c19c23bf7e1984601626b29024eca4c00677f94a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:31:47 +0200 Subject: [PATCH 05/10] feat(platform-wallet-storage): SecretStore Tier-2 write API + reprotect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 5 of the layered secret-protection feature. - `SecretStore::set_secret(service, label, secret, password: Option<&SecretString>)` wraps via the envelope ABOVE the backend (the backend stores only the opaque envelope — ciphertext for a protected object) and writes through a shared `put_raw` seam over BOTH arms. - `set`/`get` are reimplemented as non-breaking `..,None` wrappers (signatures unchanged); `get` now routes through the strict `get_secret`. - `reprotect(service, label, current, new)` — the canonical add/change/ remove flow as one same-slot unwrap→rewrap→overwrite; reads under the `current` expectation (a strip is caught fail-closed before any rewrite), then re-writes under `new`. The atomic put leaves the prior value intact on a crash. - `envelope::wrap` now returns a zeroizing `SecretBytes` (symmetric with `unwrap`): a scheme-0 envelope embeds plaintext, so the wire bytes are mlock'd/wiped by construction rather than living in a bare `Vec`. Tests (TS-PW-001..005, TS-ARM-003, TS-T1-005), parameterised over File and Os: full enrol→change→remove lifecycle; no-recovery (lost password bricks the object, fail closed both ways); set/get `None`-wrapper round-trip + scheme-0 proof; Os-arm round-trip unaffected by the blank guard; and ★ TS-PW-004 [File] crash-safety — a disk-write failure mid-change leaves the OLD protected value intact and readable, no half-rotated state. 137 secrets unit tests + secrets integration tests green; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/envelope.rs | 73 +++-- .../src/secrets/store.rs | 305 +++++++++++++++++- 2 files changed, 344 insertions(+), 34 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs index c987e068a4..d2cd043b87 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs @@ -96,15 +96,17 @@ pub const MAX_PLAINTEXT_LEN: usize = MAX_SECRET_LEN - MAX_ENVELOPE_OVERHEAD; /// `None` → an unprotected (scheme-0) envelope; `Some(pw)` → a scheme-1 /// envelope sealed under `pw`. A blank password is rejected at enrol /// ([`SecretStoreError::BlankPassphrase`]). -// The write side is wired into `SecretStore::set_secret` in the next task; -// until then it is exercised only by this module's tests. -#[allow(dead_code)] +/// +/// Returns the envelope inside a zeroizing [`SecretBytes`]: a scheme-0 +/// envelope embeds the raw plaintext, so the wire bytes are handled as +/// sensitive (mlock'd, wiped on drop) by construction — symmetric with +/// [`unwrap`]'s return. pub(crate) fn wrap( wallet_id: &WalletId, label: &str, password: Option<&SecretString>, plaintext: &[u8], -) -> Result, SecretStoreError> { +) -> Result { wrap_with_params( wallet_id, label, @@ -117,14 +119,13 @@ pub(crate) fn wrap( /// [`wrap`] with explicit Argon2 `params` (tests use the floor params for /// speed; production uses [`KdfParams::default_target`]). `params` is /// ignored when `password` is `None`. -#[allow(dead_code)] pub(crate) fn wrap_with_params( wallet_id: &WalletId, label: &str, password: Option<&SecretString>, plaintext: &[u8], params: KdfParams, -) -> Result, SecretStoreError> { +) -> Result { // Cap the PLAINTEXT (before overhead) uniformly for both schemes so the // enveloped bytes always fit the backend cap and the limit is stable. if plaintext.len() > MAX_PLAINTEXT_LEN { @@ -141,7 +142,9 @@ pub(crate) fn wrap_with_params( out.push(ENVELOPE_VERSION); out.push(SCHEME_UNPROTECTED); out.extend_from_slice(plaintext); - return Ok(out); + // `SecretBytes::new` moves `out` into a zeroizing, mlock'd buffer + // (no copy) — the scheme-0 plaintext never lives in a bare Vec. + return Ok(SecretBytes::new(out)); }; // Reject a blank object password BEFORE any derivation (SEC-J). @@ -167,7 +170,7 @@ pub(crate) fn wrap_with_params( out.extend_from_slice(&salt); out.extend_from_slice(&nonce); out.extend_from_slice(&ciphertext); - Ok(out) + Ok(SecretBytes::new(out)) } /// Unwrap `blob` for `(wallet_id, label)`, applying the **strict, @@ -390,12 +393,41 @@ mod tests { SecretString::new(s) } + /// Wrap and expose the envelope as a `Vec` for byte-level + /// inspection/mutation in tests (the production `wrap` returns a + /// zeroizing `SecretBytes`). + fn wrap_bytes( + w: &WalletId, + label: &str, + password: Option<&SecretString>, + pt: &[u8], + ) -> Vec { + wrap(w, label, password, pt) + .unwrap() + .expose_secret() + .to_vec() + } + + /// [`wrap_bytes`] with explicit (floor) params, for the scheme-1 tests. + fn wrap_p( + w: &WalletId, + label: &str, + password: Option<&SecretString>, + pt: &[u8], + params: KdfParams, + ) -> Vec { + wrap_with_params(w, label, password, pt, params) + .unwrap() + .expose_secret() + .to_vec() + } + /// TS-ENV-001: scheme-0 passthrough round-trip; the wrapped form leads /// with magic, version=1, scheme=0, then the raw payload. #[test] fn scheme0_passthrough_round_trip() { let secret = b"top secret seed bytes"; - let blob = wrap(&wid(1), "seed", None, secret).unwrap(); + let blob = wrap_bytes(&wid(1), "seed", None, secret); assert!(blob.starts_with(MAGIC)); assert_eq!(blob[O_VERSION], 1); assert_eq!(blob[O_SCHEME], 0); @@ -411,7 +443,7 @@ mod tests { fn scheme1_round_trip_and_fresh_salt_nonce() { let secret = b"correct horse battery staple seed"; let p = pw("hunter2-but-better"); - let blob = wrap_with_params(&wid(7), "seed", Some(&p), secret, floor()).unwrap(); + let blob = wrap_p(&wid(7), "seed", Some(&p), secret, floor()); assert!(blob.starts_with(MAGIC)); assert_eq!(blob[O_VERSION], 1); assert_eq!(blob[O_SCHEME], 1); @@ -422,7 +454,7 @@ mod tests { let got = unwrap(&wid(7), "seed", Some(&p), &blob).unwrap(); assert_eq!(got.expose_secret(), secret); - let blob2 = wrap_with_params(&wid(7), "seed", Some(&p), secret, floor()).unwrap(); + let blob2 = wrap_p(&wid(7), "seed", Some(&p), secret, floor()); assert_ne!( &blob[O_SALT..O_SALT + SALT_LEN], &blob2[O_SALT..O_SALT + SALT_LEN], @@ -438,7 +470,7 @@ mod tests { /// TS-ENV-003: wrong object password → WrongPassword, no plaintext. #[test] fn wrong_password_fails_closed() { - let blob = wrap_with_params(&wid(1), "seed", Some(&pw("right")), b"seed", floor()).unwrap(); + let blob = wrap_p(&wid(1), "seed", Some(&pw("right")), b"seed", floor()); let err = unwrap(&wid(1), "seed", Some(&pw("wrong")), &blob).unwrap_err(); assert!( matches!(err, SecretStoreError::WrongPassword), @@ -451,7 +483,7 @@ mod tests { #[test] fn relocation_across_identity_is_rejected() { let p = pw("pw"); - let blob = wrap_with_params(&wid(0xA), "labelA", Some(&p), b"seed", floor()).unwrap(); + let blob = wrap_p(&wid(0xA), "labelA", Some(&p), b"seed", floor()); for (w, l) in [(0xB, "labelB"), (0xA, "labelB"), (0xB, "labelA")] { let err = unwrap(&wid(w), l, Some(&p), &blob).unwrap_err(); assert!( @@ -470,7 +502,7 @@ mod tests { #[test] fn header_tamper_fails_closed_per_field() { let p = pw("pw"); - let base = wrap_with_params(&wid(1), "seed", Some(&p), b"seed", floor()).unwrap(); + let base = wrap_p(&wid(1), "seed", Some(&p), b"seed", floor()); // kdf.id → 7 (unknown) ⇒ KdfFailure (bounds reject pre-derive). let mut b = base.clone(); @@ -520,7 +552,7 @@ mod tests { #[test] fn kdf_ceiling_enforced_before_derivation() { let p = pw("pw"); - let base = wrap_with_params(&wid(1), "seed", Some(&p), b"seed", floor()).unwrap(); + let base = wrap_p(&wid(1), "seed", Some(&p), b"seed", floor()); // m_kib = u32::MAX ⇒ KdfFailure, no allocation. let mut b = base.clone(); @@ -623,7 +655,7 @@ mod tests { // (c) Magic OK but version = 2 ⇒ UnsupportedEnvelopeVersion{2}, // regardless of password (GAP-009). - let mut v2 = wrap(&wid(1), "seed", None, b"x").unwrap(); + let mut v2 = wrap_bytes(&wid(1), "seed", None, b"x"); v2[O_VERSION] = 2; for arg in [None, Some(&p)] { assert!(matches!( @@ -633,7 +665,7 @@ mod tests { } // (d) Magic+version OK but unknown scheme = 9 ⇒ fail closed. - let mut s9 = wrap(&wid(1), "seed", None, b"x").unwrap(); + let mut s9 = wrap_bytes(&wid(1), "seed", None, b"x"); s9[O_SCHEME] = 9; assert!(matches!( unwrap(&wid(1), "seed", None, &s9).unwrap_err(), @@ -645,7 +677,7 @@ mod tests { /// tests): a scheme-0 blob carrying `secret` DOES decode under `None`. #[test] fn scheme0_some_password_fails_closed_strip() { - let blob = wrap(&wid(1), "seed", None, b"attacker-seed").unwrap(); + let blob = wrap_bytes(&wid(1), "seed", None, b"attacker-seed"); // None ⇒ it WOULD decode to the (attacker) bytes… assert_eq!( unwrap(&wid(1), "seed", None, &blob) @@ -666,8 +698,7 @@ mod tests { fn round_trip_is_constant_time_equal() { let p = pw("pw"); let original = SecretBytes::from_slice(b"seed material"); - let blob = - wrap_with_params(&wid(1), "seed", Some(&p), original.expose_secret(), floor()).unwrap(); + let blob = wrap_p(&wid(1), "seed", Some(&p), original.expose_secret(), floor()); let got = unwrap(&wid(1), "seed", Some(&p), &blob).unwrap(); assert!(bool::from(got.ct_eq(&original))); } @@ -682,7 +713,7 @@ mod tests { #[test] fn fuzz_byte_mutation_never_panics() { let p = pw("fuzz-pw"); - let valid = wrap_with_params(&wid(0xAB), "seed", Some(&p), b"seed-bytes", floor()).unwrap(); + let valid = wrap_p(&wid(0xAB), "seed", Some(&p), b"seed-bytes", floor()); // The pristine envelope unwraps. assert_eq!( unwrap(&wid(0xAB), "seed", Some(&p), &valid) diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index aea89a5794..0a88dcc841 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -50,36 +50,77 @@ impl SecretStore { Ok(Self::Os(default_credential_store().map_err(map_spi)?)) } - /// Store `secret` under `(service, label)`, overwriting any prior - /// value. Takes `&SecretBytes` so the caller cannot pass an unwrapped - /// buffer; the wrapped bytes are exposed to the SPI only at the last - /// moment. + /// Store `secret` under `(service, label)` UNPROTECTED (Tier-2 + /// scheme-0), overwriting any prior value — a `set_secret(.., None)` + /// wrapper kept for non-breaking back-compat. Takes `&SecretBytes` so + /// the caller cannot pass an unwrapped buffer. pub fn set( &self, service: &WalletId, label: &str, secret: &SecretBytes, + ) -> Result<(), SecretStoreError> { + self.set_secret(service, label, secret, None) + } + + /// Store `secret` under `(service, label)`, overwriting any prior value. + /// + /// `password` selects the Tier-2 protection: `None` writes an + /// unprotected scheme-0 envelope; `Some(pw)` writes a scheme-1 envelope + /// sealed under the object password `pw` (Argon2id + XChaCha20-Poly1305) + /// **before** the bytes reach the backend, so a protected object stays + /// confidential even under a full backend compromise. A blank `pw` is + /// rejected ([`BlankPassphrase`](SecretStoreError::BlankPassphrase)). + /// + /// The write is an atomic same-slot overwrite on both arms (File: the + /// vault's atomic replace; Os: the keychain item), so add/change/remove + /// password flows — see [`reprotect`](SecretStore::reprotect) — leave + /// the prior value intact on a crash. + pub fn set_secret( + &self, + service: &WalletId, + label: &str, + secret: &SecretBytes, + password: Option<&SecretString>, + ) -> Result<(), SecretStoreError> { + // Wrap above the backend: the backend only ever stores the opaque + // envelope (ciphertext for a protected object). + let blob = envelope::wrap(service, label, password, secret.expose_secret())?; + self.put_raw(service, label, &blob) + } + + /// Store the already-enveloped opaque `blob` under `(service, label)`. + /// The shared write seam under [`set`] and [`set_secret`]. + /// + /// [`set`]: SecretStore::set + fn put_raw( + &self, + service: &WalletId, + label: &str, + blob: &SecretBytes, ) -> Result<(), SecretStoreError> { match self { // Inherent typed path — no lossy SPI seam, no bare buffer. - Self::File(s) => s.put_bytes(service, label, secret), + Self::File(s) => s.put_bytes(service, label, blob), Self::Os(store) => { let entry = build_os(store, service, label)?; - entry.set_secret(secret.expose_secret()).map_err(map_spi) + entry.set_secret(blob.expose_secret()).map_err(map_spi) } } } - /// Retrieve the secret stored under `(service, label)`, or `Ok(None)` - /// if absent. The plaintext is wrapped into [`SecretBytes`] at the - /// seam with no named `Vec` intermediate, so the bare-buffer window is - /// zero statements. + /// Retrieve the UNPROTECTED secret stored under `(service, label)`, or + /// `Ok(None)` if absent — a `get_secret(.., None)` wrapper kept for + /// non-breaking back-compat. A scheme-1 (password-protected) object read + /// through this path returns + /// [`NeedsPassword`](SecretStoreError::NeedsPassword); use + /// [`get_secret`](SecretStore::get_secret) with the object password. pub fn get( &self, service: &WalletId, label: &str, ) -> Result, SecretStoreError> { - self.get_raw(service, label) + self.get_secret(service, label, None) } /// Read the opaque bytes stored under `(service, label)`, or `Ok(None)` @@ -147,6 +188,34 @@ impl SecretStore { envelope::unwrap(service, label, password, stored.expose_secret()).map(Some) } + /// Add / change / remove an object password in one same-slot + /// unwrap→rewrap→overwrite — the canonical Tier-2 re-protection flow. + /// + /// Reads the object under the `current` expectation (so a strip is + /// caught fail-closed before any rewrap), then re-writes it under + /// `new`: + /// - **add:** `current = None`, `new = Some(pw)`; + /// - **change:** `current = Some(old)`, `new = Some(pw_new)`; + /// - **remove:** `current = Some(old)`, `new = None`. + /// + /// An absent object is a no-op (`Ok(())`). The rewrite is the atomic + /// same-slot overwrite of [`set_secret`], so a crash between the read + /// and the commit leaves the prior value intact and readable under + /// `current`. After a successful call the consumer MUST update its own + /// trusted protection-status record (the L-1 expectation lives there). + pub fn reprotect( + &self, + service: &WalletId, + label: &str, + current: Option<&SecretString>, + new: Option<&SecretString>, + ) -> Result<(), SecretStoreError> { + let Some(secret) = self.get_secret(service, label, current)? else { + return Ok(()); + }; + self.set_secret(service, label, &secret, new) + } + /// Delete the secret stored under `(service, label)`. Absent entries /// are a no-op (`Ok(())`), so deletion is idempotent. pub fn delete(&self, service: &WalletId, label: &str) -> Result<(), SecretStoreError> { @@ -437,11 +506,17 @@ mod tests { } fn protected(w: &WalletId, label: &str, pw: &str, secret: &[u8]) -> Vec { - envelope::wrap_with_params(w, label, Some(&SecretString::new(pw)), secret, floor()).unwrap() + envelope::wrap_with_params(w, label, Some(&SecretString::new(pw)), secret, floor()) + .unwrap() + .expose_secret() + .to_vec() } fn unprotected(w: &WalletId, label: &str, secret: &[u8]) -> Vec { - envelope::wrap(w, label, None, secret).unwrap() + envelope::wrap(w, label, None, secret) + .unwrap() + .expose_secret() + .to_vec() } /// A backend under test plus the raw-write hook that plays the @@ -781,4 +856,208 @@ mod tests { fn l1_scheme_flip_os() { run_scheme_flip(&os_backend()); } + + // ===== Add / change / remove password + arm matrix (TS-PW / TS-ARM) ===== + // + // These exercise the PUBLIC set_secret/get_secret/reprotect API, so the + // protected writes/reads run the real (default 64 MiB) Argon2 — kept to + // a small number of derivations per test. + + /// TS-PW-001/002/003: the full enrol → change → remove lifecycle, each + /// step verified through the strict read. + fn run_pw_lifecycle(b: &Backend) { + let w = wid(10); + let pw1 = SecretString::new("pw-one"); + let pw2 = SecretString::new("pw-two"); + + // ADD: start unprotected, enrol a password. + b.store + .set(&w, "seed", &SecretBytes::from_slice(b"SEED")) + .unwrap(); + assert_eq!( + b.store.get(&w, "seed").unwrap().unwrap().expose_secret(), + b"SEED" + ); + b.store.reprotect(&w, "seed", None, Some(&pw1)).unwrap(); + assert!( + matches!( + b.store.get(&w, "seed").unwrap_err(), + SecretStoreError::NeedsPassword + ), + "[{}] after add, None read needs a password", + b.name + ); + assert_eq!( + b.store + .get_secret(&w, "seed", Some(&pw1)) + .unwrap() + .unwrap() + .expose_secret(), + b"SEED" + ); + + // CHANGE: rotate to a new password (unwrap-old → rewrap-new). + b.store + .reprotect(&w, "seed", Some(&pw1), Some(&pw2)) + .unwrap(); + assert_eq!( + b.store + .get_secret(&w, "seed", Some(&pw2)) + .unwrap() + .unwrap() + .expose_secret(), + b"SEED" + ); + assert!( + matches!( + b.store.get_secret(&w, "seed", Some(&pw1)).unwrap_err(), + SecretStoreError::WrongPassword + ), + "[{}] old password no longer unlocks after change", + b.name + ); + + // REMOVE: back to unprotected. + b.store.reprotect(&w, "seed", Some(&pw2), None).unwrap(); + assert_eq!( + b.store.get(&w, "seed").unwrap().unwrap().expose_secret(), + b"SEED" + ); + assert!( + matches!( + b.store.get_secret(&w, "seed", Some(&pw2)).unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + ), + "[{}] after remove, a password read fails closed until DET updates its DB", + b.name + ); + } + + #[test] + fn pw_lifecycle_file() { + run_pw_lifecycle(&file_backend()); + } + + #[test] + fn pw_lifecycle_os() { + run_pw_lifecycle(&os_backend()); + } + + /// TS-PW-005: losing the object password bricks the object — no recovery + /// path exists, every read fails closed. + fn run_pw_no_recovery(b: &Backend) { + let w = wid(11); + let pw = SecretString::new("the-only-pw"); + b.store + .set_secret(&w, "seed", &SecretBytes::from_slice(b"SEED"), Some(&pw)) + .unwrap(); + assert!(matches!( + b.store + .get_secret(&w, "seed", Some(&SecretString::new("guess"))) + .unwrap_err(), + SecretStoreError::WrongPassword + )); + assert!(matches!( + b.store.get(&w, "seed").unwrap_err(), + SecretStoreError::NeedsPassword + )); + } + + #[test] + fn pw_no_recovery_file() { + run_pw_no_recovery(&file_backend()); + } + + #[test] + fn pw_no_recovery_os() { + run_pw_no_recovery(&os_backend()); + } + + /// TS-ARM-003: `set`/`get` are additive `..,None` wrappers — `set` + /// writes a scheme-0 envelope, `get` reads it byte-exact, and a + /// password-supplied read of that unprotected object fails closed. + fn run_set_get_wrappers(b: &Backend) { + let w = wid(12); + b.store + .set(&w, "seed", &SecretBytes::from_slice(b"plain")) + .unwrap(); + assert_eq!( + b.store.get(&w, "seed").unwrap().unwrap().expose_secret(), + b"plain" + ); + assert!(matches!( + b.store + .get_secret(&w, "seed", Some(&SecretString::new("pw"))) + .unwrap_err(), + SecretStoreError::ExpectedProtectedButUnsealed + )); + } + + #[test] + fn set_get_wrappers_file() { + run_set_get_wrappers(&file_backend()); + } + + #[test] + fn set_get_wrappers_os() { + run_set_get_wrappers(&os_backend()); + } + + /// TS-T1-005: the Os arm has no passphrase concept; the Tier-1 blank + /// guard never fires and the round-trip is byte-exact. + #[test] + fn os_arm_roundtrip_no_blank_guard() { + let b = os_backend(); + let w = wid(13); + b.store + .set(&w, "seed", &SecretBytes::from_slice(b"abc")) + .unwrap(); + assert_eq!( + b.store.get(&w, "seed").unwrap().unwrap().expose_secret(), + b"abc" + ); + b.store.delete(&w, "seed").unwrap(); + assert!(b.store.get(&w, "seed").unwrap().is_none()); + } + + /// TS-PW-004 ★ [File]: a crash (disk-write failure) between the unwrap + /// and the overwrite-commit leaves the OLD protected value intact and + /// readable — no half-rotated / unprotected state. + #[cfg(unix)] + #[test] + fn pw_change_crash_safety_leaves_old_intact_file() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let s = file_store(dir.path()); + let w = wid(14); + let old = SecretString::new("old-pw"); + let new = SecretString::new("new-pw"); + + s.set_secret(&w, "seed", &SecretBytes::from_slice(b"REAL"), Some(&old)) + .unwrap(); + + // Make the vault's parent read-only so the atomic temp-write fails + // mid-change (mirrors rekey_does_not_corrupt_on_disk_temp_failure). + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o500)).unwrap(); + let err = s.reprotect(&w, "seed", Some(&old), Some(&new)).unwrap_err(); + assert!(matches!(err, SecretStoreError::Io(_)), "got {err:?}"); + + // Restore write so the resident store can sync/clean up at drop. + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o700)).unwrap(); + + // The OLD value is still readable under the OLD password; the new + // password does not unlock it (no half-rotation). + assert_eq!( + s.get_secret(&w, "seed", Some(&old)) + .unwrap() + .unwrap() + .expose_secret(), + b"REAL" + ); + assert!(matches!( + s.get_secret(&w, "seed", Some(&new)).unwrap_err(), + SecretStoreError::WrongPassword + )); + } } From d4d311cca17615e38ef3c1b0924d9ddcff5d0e99 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:38:26 +0200 Subject: [PATCH 06/10] feat(platform-wallet-storage)!: Tier-1 blank-passphrase guard + open_unprotected + docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 6 (final) of the layered secret-protection feature. - `EncryptedFileStore::open` and `rekey` now reject a blank (empty / all-whitespace) passphrase with `BlankPassphrase` via `is_blank` — a blank passphrase derives a key from a public salt only (obfuscation, not confidentiality; closes SEC-A / F-1). INTENDED behavioural break for any caller that relied on `SecretString::empty()`. - `EncryptedFileStore::open_unprotected(path)` / `SecretStore::file_unprotected(path)` — the explicit, named keyless door (AC-2.1 maps here, not to `open`). Used for the empty→real `rekey` migration or hosts where secrets carry their own Tier-2 password. - `reject_weak_passphrase` wires the coarse `MIN_PASSPHRASE_LEN` floor (1 = non-blank); real entropy policy is delegated to the consumer (GAP-012). - SECRETS.md: new "Two-tier secret protection" section — the model, the envelope wire format, which tier defeats which adversary, the strict fail-closed read + the caller-DB anti-downgrade dependency, the value-rollback non-defence, add/change/remove + no-recovery, entropy-policy delegation, greenfield/legacy tolerance, `open_unprotected` caveat, the `MAX_PLAINTEXT_LEN` cap; plus the five new error variants and their projections. Tests (TS-T1-001..004,006): blank rejected at open (no file/lock created) and at rekey (vault unchanged); open_unprotected keyless round-trip + real-pass open → WrongPassphrase; empty→real rekey migration; ★ rekey crash-safety leaves the pre-rekey keyless vault intact. 142 secrets unit tests + integration green; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../rs-platform-wallet-storage/SECRETS.md | 201 +++++++++++++++++- .../src/secrets/file/mod.rs | 191 ++++++++++++++++- .../src/secrets/store.rs | 11 + 3 files changed, 399 insertions(+), 4 deletions(-) diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index 8a1c7fcc39..99991f189d 100644 --- a/packages/rs-platform-wallet-storage/SECRETS.md +++ b/packages/rs-platform-wallet-storage/SECRETS.md @@ -62,8 +62,22 @@ use platform_wallet_storage::secrets::{SecretBytes, SecretStore, SecretString, W let store = SecretStore::file("/var/lib/wallet/secrets.pwsvault", SecretString::new("pw"))?; let wallet = WalletId::from(wallet_id); + +// Tier-1 only (unprotected by an object password). `set`/`get` are +// `..,None` wrappers over `set_secret`/`get_secret`. store.set(&wallet, "mnemonic", &SecretBytes::from_slice(b"abandon ability ..."))?; let plaintext: Option = store.get(&wallet, "mnemonic")?; // never a bare Vec + +// Tier-2: protect a critical object under an extra OBJECT PASSWORD that +// the backend never sees. Reading it back REQUIRES the password. +let pw = SecretString::new("a strong object password"); +store.set_secret(&wallet, "seed", &SecretBytes::from_slice(b""), Some(&pw))?; +let seed = store.get_secret(&wallet, "seed", Some(&pw))?; // Some(secret) +// Reading a protected object WITHOUT the password fails closed: +assert!(store.get_secret(&wallet, "seed", None).is_err()); // NeedsPassword + +// Add / change / remove an object password in one atomic same-slot flow: +store.reprotect(&wallet, "seed", Some(&pw), None)?; // remove → now unprotected store.delete(&wallet, "mnemonic")?; // idempotent ``` @@ -72,6 +86,167 @@ filename); the parent directory is materialized on the first write. Use `SecretStore::os()` for the platform OS keyring arm instead of `SecretStore::file(..)`. +See **Two-tier secret protection** below for the model, the envelope +format, which tier defeats which adversary, and the strict fail-closed +read that is the heart of the opt-in scheme. + +### Two-tier secret protection + +Secret protection comes in two layers. Tier-1 is always on (it is just +"which backend you opened"); Tier-2 is opt-in, per critical object, and +backend-independent. + +| Tier | Provided by | Defeats | Mechanism | +|---|---|---|---| +| **1 — backend baseline** | the *backend* | another local user, a lost laptop, the vault at rest | OS keychain ACLs **or** Argon2id + XChaCha20-Poly1305 vault under a **real** passphrase | +| **2 — per-object password** | the *library*, above `SecretStore`, over **both** arms | **backend compromise** — the keychain scraped, or the vault stolen *and* its passphrase cracked | the object's bytes are Argon2id + XChaCha20-Poly1305 **enveloped under a per-object password BEFORE they reach the backend** | + +**Why Tier-2 is more than key granularity.** Its value is not a sub-key — +it is (a) an **independent human password the backend never sees** and (b) +**envelope-before-backend ordering**, so for a protected object the backend +only ever stores ciphertext. That is the first and only control that keeps +a chosen critical object confidential across a *full* backend compromise +(the A2/A3/A6 gap Tier-1 leaves open). + +Tier-2 has two guarantees of different strength: + +- **Confidentiality** (an attacker cannot *read* a protected secret) is + **unconditional** — the object password never enters any backend, so a + full backend dump yields only ciphertext + a per-object salt to + offline-Argon2id-crack against the password's entropy. +- **Integrity / anti-downgrade** is delivered by the **strict fail-closed + read** below and is **conditional on the caller's trusted model staying + intact** (see the documented residual). + +#### The envelope (wire format) + +Every value written through `set_secret`/`set` is wrapped in a +self-describing, authenticated envelope before it reaches the backend. The +backend (file vault or OS keychain) stores only these opaque bytes. + +```text +magic b"PWSEV" (5) +version u8 = 1 (envelope version — independent of the vault FORMAT_VERSION) +scheme u8 (0 = unprotected passthrough, 1 = password) +── scheme 0 ── payload: the raw secret bytes +── scheme 1 ── kdf(id u8 ‖ m_kib u32 LE ‖ t u32 LE ‖ p u32 LE) (13) + ‖ salt[32] ‖ nonce[24] ‖ ciphertext+tag +``` + +- **AAD (scheme 1)** binds `domain ‖ magic ‖ version ‖ scheme ‖ kdf ‖ salt + ‖ wallet_id ‖ label` (length-prefixed), mirroring the vault's own + `aad()`/`verify_aad()`. A protected blob relocated to another slot — or + any in-place header edit — fails the tag (relocation/header-tamper + resistance). On the file arm this AAD is *in addition* to the vault's own + per-entry AAD + tag; on the OS arm it is the only authentication layer. +- **KDF ceiling before derivation (anti-DoS).** The KDF params live in the + (attacker-controllable) header, so on a read the Argon2 **ceiling is + enforced before** any derivation/allocation — a forged `m_kib`/`t` cannot + force a giant allocation or an unbounded stall on the victim's unlock. +- **No vault format bump.** The envelope lives *inside* the entry bytes, + identical over File and Os, so there is no vault-parser or migration + change. +- **Size cap.** The plaintext is capped at `MAX_PLAINTEXT_LEN` + (`MAX_SECRET_LEN − MAX_ENVELOPE_OVERHEAD` = 64 KiB − 128 = 65 408 bytes), + uniformly for both schemes, so the enveloped bytes always fit the + backend's own `MAX_SECRET_LEN` cap and the user-visible limit is stable + regardless of scheme. Oversize → `SecretTooLarge { found, max }` with + `max = MAX_PLAINTEXT_LEN` (re-exported as `secrets::MAX_PLAINTEXT_LEN`). +- **Unknown version/scheme** (magic present) → `UnsupportedEnvelopeVersion` + — fail closed **regardless of the password**: an unparseable future + format can be neither safely unwrapped nor treated as unprotected. + +#### The strict, fail-closed read (the L-1 keystone) + +The defining risk of any opt-in "some objects are extra-protected" scheme +is **strip / downgrade**: an attacker who can WRITE the backend replaces a +protected blob with a fresh, internally-valid *unprotected* (scheme-0) blob +carrying a chosen seed/xpriv. There is nothing in that blob alone to prove +an envelope was *expected*, so inferring protection from the stored bytes +would silently return the attacker's secret — funds redirection, password +prompt bypassed. + +The fix: **the "expected-protected" bit lives in the CALLER's trusted +model, surfaced solely by whether a password is supplied to `get_secret` — +NEVER inferred from the blob.** The library does not guess and does not +persist the expectation. A supplied password *is* the assertion "this +object must be protected": + +| `password` arg | stored blob | result | +|---|---|---| +| `Some(pw)` | valid scheme-1 | the secret, or `WrongPassword` on tag fail | +| **`Some(pw)`** | **scheme-0 / legacy magic-less raw** | **`ExpectedProtectedButUnsealed` — FAIL CLOSED ★** | +| `Some(pw)` | scheme-1 but truncated/corrupt | `Corruption` | +| `Some/None` | magic present, unknown version/scheme | `UnsupportedEnvelopeVersion` | +| `None` | valid scheme-1 | `NeedsPassword` (never ciphertext) | +| `None` | scheme-0 | the secret | +| `None` | legacy magic-less raw | the secret (+ a one-time warning; re-wrapped on next write) | +| any | absent entry | `Ok(None)` (deletion = DoS, never injection) | + +The load-bearing row is **`Some(pw)` + non-envelope ⇒ +`ExpectedProtectedButUnsealed`**: with a password in hand, a non-protected +blob can only mean a strip, so it is refused and **no bytes are returned**. +A DET (consumer) bug alone — over- or under-supplying a password — fails +closed in *every* direction. + +**Arm asymmetry.** On the file arm the stored bytes are themselves sealed +under the vault key, so producing a *readable* stripped blob at a slot +requires the vault key; a cold/backup-swap actor can only corrupt +(→ DoS), not inject-to-readable. On the OS-keychain arm the stored item is +the bare envelope with no second seal, so the strip defence there leans +entirely on the `Some(pw)` strict rule plus the consumer's metadata +integrity — this is where the residual bites hardest. + +**Documented residual (out of the library's reach).** If an attacker ALSO +rewrites the consumer's trusted DB so the consumer calls `get_secret(X, +None)` for a stripped object, the `(scheme-0, None)` quadrant returns the +attacker's bytes. The library only ever sees the blob and the caller's +`Some/None`; the "should be protected" fact lives entirely in the +consumer's metadata store. **Anti-downgrade strength therefore equals the +tamper-resistance of the consumer's protection-status record** — store it +as integrity-protected, security-critical state (it is one more field +alongside the addresses/policy the wallet DB must already protect). + +**Value rollback is NOT defended.** Restoring an *older valid* scheme-1 +envelope under the *current* password decrypts cleanly. L-1 closes the +strip/downgrade injection, not value rollback; if backup-swap/restore-old +is in scope, anchor a monotonic version in integrity-protected consumer +metadata. Do not mistake L-1 for rollback protection. + +#### Add / change / remove an object password + +`reprotect(service, label, current, new)` does it in one same-slot +unwrap→rewrap→overwrite: read under the `current` expectation (so a strip +is caught before any rewrite), then write under `new` — `None`→`Some` adds, +`Some`→`Some` changes, `Some`→`None` removes. The write is the atomic +same-slot overwrite, so a crash between the read and the commit leaves the +prior value intact and readable under `current`. **After a successful call +the consumer MUST update its own protection-status record** (the L-1 +expectation lives there). There is **no password recovery** — losing an +object password bricks that object (an availability trade-off the UX must +state plainly). + +#### Entropy policy is the consumer's + +The library enforces only **non-blank** at enrol (and a coarse +`MIN_PASSPHRASE_LEN` floor, `1` today = merely non-blank) for both the +vault passphrase and the Tier-2 object password. It ships **no** +password-strength estimator: real entropy policy (zxcvbn-style strength, +dictionary checks, UX feedback) is locale- and threat-specific and is the +**consumer's responsibility**. For a protected object the password's +entropy is the *whole* guarantee against an offline Argon2id attacker who +already holds the backend — choose it accordingly. + +#### Greenfield / legacy entries + +The envelope is net-new, so post-feature reads/writes go through it. A +decrypted entry that lacks the `PWSEV` magic is treated as a **legacy +unprotected** value: returned on a `None` read (with a one-time warning, +and re-wrapped on the next write) and refused (`ExpectedProtectedButUnsealed`) +on a `Some(pw)` read — so legacy tolerance never weakens L-1. (A pre-feature +build that persisted vault files is a deployment fact outside this crate; +the legacy-tolerant read makes the transition seamless either way.) + ### Internal SPI Below `SecretStore`, `EncryptedFileStore` and `default_credential_store` @@ -138,7 +313,20 @@ unwrapped copy is allocated. Each secret is capped at `MAX_SECRET_LEN` (64 KiB) at the write boundary — generously above any mnemonic/seed/xpriv — so a single oversized entry cannot inflate the shared document past the read-side - 128 MiB ceiling and brick every wallet on the next open. + 128 MiB ceiling and brick every wallet on the next open. (Through + `SecretStore::set_secret`/`set` the user-facing plaintext cap is the + slightly lower `MAX_PLAINTEXT_LEN`, leaving room for the envelope + overhead; see **Two-tier secret protection**.) + **Blank passphrase is rejected.** `open` (and `rekey`) refuse a blank + (empty / all-whitespace) passphrase with `SecretStoreError::BlankPassphrase` + — a blank passphrase derives a key from a public salt only, i.e. + obfuscation, not confidentiality. This is an **intended behavioural + break** for any caller that relied on `SecretString::empty()`. A + deliberate keyless vault uses the explicit + `EncryptedFileStore::open_unprotected(path)` / + `SecretStore::file_unprotected(path)` door instead (use it only where the + stored secrets carry their own Tier-2 object password, or as a staging + step before `rekey` to a real passphrase — the empty→real migration). - **OS keyring (`SecretStore::os` / `default_credential_store`)** — returns an `Arc` over the platform's default credential store. The backend on Linux/FreeBSD is @@ -184,7 +372,16 @@ automatic fallback between backends. is **lossless**: `WrongPassphrase`, `Corruption`, `AlreadyLocked`, `KdfFailure`, `VersionUnsupported`, `MalformedVault`, `InsecurePermissions`, `InsecureParentDir`, `SecretTooLarge`, `VaultTooLarge`, `Encrypt`, and -`InvalidLabel` are distinct typed variants. `VaultTooLarge` surfaces when +`InvalidLabel` are distinct typed variants. The Tier-2 layer adds five more: +`ExpectedProtectedButUnsealed` (the L-1 fail-closed strip refusal), +`NeedsPassword` (a protected object read with no password), `WrongPassword` +(object-password tag fail — distinct from the Tier-1 `WrongPassphrase`), +`BlankPassphrase` (a blank vault passphrase or object password), and +`UnsupportedEnvelopeVersion { found }` (a future envelope format, fail +closed regardless of the password). The four Tier-2 credential/protection +*state* variants project to a recoverable `NoStorageAccess` (boxed, +downcast-recoverable, like `WrongPassphrase`); `UnsupportedEnvelopeVersion` +joins the secret-free `BadStoreFormat` group. `VaultTooLarge` surfaces when the on-disk vault exceeds the read-side ceiling; `SecretTooLarge` rejects an oversized secret at the write boundary before it can inflate the shared vault; `InsecureParentDir` refuses a vault whose parent directory is 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 1c91b8cd23..6cfc57ecb5 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -57,7 +57,7 @@ use format::{EntryBody, Vault}; use super::error::SecretStoreError; -use super::secret::{SecretBytes, SecretString}; +use super::secret::{SecretBytes, SecretString, MIN_PASSPHRASE_LEN}; use super::validate::{validated_label, WalletId}; /// Service-prefix for vault entries: the full `service` string is @@ -139,7 +139,33 @@ impl EncryptedFileStore { path: impl AsRef, passphrase: SecretString, ) -> Result { - let path = path.as_ref().to_path_buf(); + // Tier-1 baseline: reject a blank passphrase (empty / all-whitespace) + // BEFORE touching the filesystem. A blank passphrase derives a key + // from a public salt only — obfuscation, not confidentiality + // (SEC-A / F-1). This is an INTENDED behavioural break for any caller + // that relied on `SecretString::empty()`; a deliberate keyless vault + // must use [`open_unprotected`](Self::open_unprotected). No vault + // file is created or altered for a blank passphrase. + reject_weak_passphrase(&passphrase)?; + Self::open_inner(path.as_ref(), passphrase) + } + + /// Open (or create) a **deliberately keyless** vault — the only door + /// that accepts no passphrase. The vault key is derived from an empty + /// passphrase under the public salt, so this is **obfuscation, not + /// confidentiality**: use it only where the stored secrets carry their + /// own Tier-2 object password, or as a staging step before + /// [`rekey`](Self::rekey) to a real passphrase. AC-2.1 maps HERE, not to + /// [`open`](Self::open) (which now rejects a blank passphrase). + pub fn open_unprotected(path: impl AsRef) -> Result { + Self::open_inner(path.as_ref(), SecretString::empty()) + } + + /// Shared open/create core for [`open`](Self::open) and + /// [`open_unprotected`](Self::open_unprotected). Does NOT apply the + /// blank-passphrase guard — the public doors decide that. + fn open_inner(path: &Path, passphrase: SecretString) -> Result { + let path = path.to_path_buf(); // Materialize the parent so the lock-sidecar open and vault // create do not fail on a not-yet-existing dir. @@ -204,6 +230,11 @@ impl EncryptedFileStore { /// new passphrase + fresh salt, so paying ~hundreds of ms inside the /// critical section would needlessly stall unrelated put/get ops. pub fn rekey(&self, new_passphrase: SecretString) -> Result<(), SecretStoreError> { + // Reject a blank target passphrase: `rekey` always advances to a + // REAL passphrase (the empty→real migration uses this). The resident + // vault, key, and on-disk file are untouched on rejection. To make a + // vault keyless, use `open_unprotected` on a fresh path instead. + reject_weak_passphrase(&new_passphrase)?; let (new_vault, new_key) = build_fresh_vault(&new_passphrase)?; lock_inner(&self.inner).rekey(new_vault, new_key, new_passphrase) } @@ -471,6 +502,18 @@ fn lock_path_for(path: &Path) -> PathBuf { PathBuf::from(s) } +/// Reject a blank (empty / all-whitespace) or sub-floor passphrase → +/// [`SecretStoreError::BlankPassphrase`]. The floor is the coarse +/// [`MIN_PASSPHRASE_LEN`] (1 today = merely non-blank); the real entropy +/// policy is the consumer's (see `SECRETS.md`). A blank check alone closes +/// SEC-A/F-1; the length term keeps the floor wired for a future bump. +fn reject_weak_passphrase(passphrase: &SecretString) -> Result<(), SecretStoreError> { + if passphrase.is_blank() || passphrase.trimmed().len() < MIN_PASSPHRASE_LEN { + return Err(SecretStoreError::BlankPassphrase); + } + Ok(()) +} + /// Build a fresh entry-less vault (random salt, default Argon2 params, /// verify-token sealed under the derived key) plus that derived key, so /// the caller can seal entries without re-deriving. @@ -1431,6 +1474,150 @@ mod tests { ); } + /// TS-T1-001: a blank passphrase is rejected at `open` → + /// `BlankPassphrase`; no vault file (or lock sidecar) is created. + #[test] + fn open_rejects_blank_passphrase() { + for blank in [ + SecretString::empty(), + SecretString::new(""), + SecretString::new(" "), + SecretString::new("\t\n"), + ] { + let dir = tempfile::tempdir().unwrap(); + let path = vault_path(dir.path()); + let err = EncryptedFileStore::open(&path, blank).unwrap_err(); + assert!( + matches!(err, SecretStoreError::BlankPassphrase), + "blank passphrase must be rejected, got {err:?}" + ); + assert!(!path.exists(), "no vault file for a blank passphrase"); + assert!( + !lock_path_for(&path).exists(), + "no lock sidecar for a blank passphrase" + ); + } + } + + /// TS-T1-002: a blank passphrase is rejected at `rekey`; the resident + /// vault, key, and on-disk file are UNCHANGED — the original passphrase + /// still reads every entry, live and after reopen. + #[test] + fn rekey_rejects_blank_passphrase_vault_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let path = vault_path(dir.path()); + let s = store_at(&path); // real "pw-correct" + entry(&s, wid(1), "seed").set_secret(b"v1").unwrap(); + for blank in [SecretString::empty(), SecretString::new(" ")] { + let err = s.rekey(blank).unwrap_err(); + assert!( + matches!(err, SecretStoreError::BlankPassphrase), + "blank rekey must be rejected, got {err:?}" + ); + } + // Old passphrase still reads the entry, live… + assert_eq!(entry(&s, wid(1), "seed").get_secret().unwrap(), b"v1"); + // …and after a clean reopen under the original passphrase. + drop(s); + let s2 = store_at(&path); + assert_eq!(entry(&s2, wid(1), "seed").get_secret().unwrap(), b"v1"); + } + + /// TS-T1-003: `open_unprotected` permits a deliberate keyless vault that + /// round-trips; a real-passphrase `open` of that keyless vault then + /// fails with `WrongPassphrase` (it is keyless, not real-pass). + #[test] + fn open_unprotected_permits_keyless_vault() { + let dir = tempfile::tempdir().unwrap(); + let path = vault_path(dir.path()); + { + let s = EncryptedFileStore::open_unprotected(&path).unwrap(); + entry(&s, wid(1), "seed") + .set_secret(b"keyless-seed") + .unwrap(); + } + { + let s = EncryptedFileStore::open_unprotected(&path).unwrap(); + assert_eq!( + entry(&s, wid(1), "seed").get_secret().unwrap(), + b"keyless-seed" + ); + } + let err = EncryptedFileStore::open(&path, SecretString::new("real")).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassphrase), + "real-pass open of a keyless vault must fail, got {err:?}" + ); + } + + /// TS-T1-004: empty→real passphrase migration via `rekey`. After rekey, + /// `open(real)` reads every entry; the keyless door no longer opens it; + /// no `.bak`/`.tmp` residue beside the vault. + #[test] + fn empty_to_real_rekey_migration() { + let dir = tempfile::tempdir().unwrap(); + let path = vault_path(dir.path()); + { + let s = EncryptedFileStore::open_unprotected(&path).unwrap(); + entry(&s, wid(1), "seed").set_secret(b"migrate-me").unwrap(); + s.rekey(SecretString::new("real-pass")).unwrap(); + // The live handle keeps working post-rekey. + assert_eq!( + entry(&s, wid(1), "seed").get_secret().unwrap(), + b"migrate-me" + ); + } + // Reopen under the real passphrase reads the entry. + { + let s = EncryptedFileStore::open(&path, SecretString::new("real-pass")).unwrap(); + assert_eq!( + entry(&s, wid(1), "seed").get_secret().unwrap(), + b"migrate-me" + ); + } + // The keyless door no longer opens it. + let err = EncryptedFileStore::open_unprotected(&path).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassphrase), + "keyless open after migration must fail, got {err:?}" + ); + // No .bak / .tmp residue (mirrors rekey_reencrypts_and_old_passphrase_fails). + for sibling in fs::read_dir(dir.path()).unwrap().flatten() { + let name = sibling.file_name(); + let name = name.to_string_lossy(); + assert!( + !name.ends_with(".bak") && !name.ends_with(".tmp"), + "unexpected residue: {name}" + ); + } + } + + /// TS-T1-004 crash-safety: a disk-write failure mid-rekey leaves the + /// pre-rekey keyless vault intact and readable via `open_unprotected` + /// (mirrors rekey_does_not_corrupt_on_disk_temp_failure). + #[cfg(unix)] + #[test] + fn empty_to_real_rekey_crash_safe_stays_keyless() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let path = vault_path(dir.path()); + let s = EncryptedFileStore::open_unprotected(&path).unwrap(); + entry(&s, wid(1), "seed").set_secret(b"keyless").unwrap(); + + // Read-only parent → the rekey atomic temp-write fails. + fs::set_permissions(dir.path(), fs::Permissions::from_mode(0o500)).unwrap(); + let err = s.rekey(SecretString::new("real-pass")).unwrap_err(); + assert!(matches!(err, SecretStoreError::Io(_)), "got {err:?}"); + fs::set_permissions(dir.path(), fs::Permissions::from_mode(0o700)).unwrap(); + + // The live handle still serves the pre-rekey keyless vault… + assert_eq!(entry(&s, wid(1), "seed").get_secret().unwrap(), b"keyless"); + // …and on disk it is still the keyless vault. + drop(s); + let s2 = EncryptedFileStore::open_unprotected(&path).unwrap(); + assert_eq!(entry(&s2, wid(1), "seed").get_secret().unwrap(), b"keyless"); + } + #[test] fn build_rejects_malformed_service() { let dir = tempfile::tempdir().unwrap(); diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index 0a88dcc841..5cb850414a 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -44,6 +44,17 @@ impl SecretStore { Ok(Self::File(EncryptedFileStore::open(path, passphrase)?)) } + /// Open (or create) a **deliberately keyless** file-backed vault — the + /// only door that takes no passphrase. Obfuscation, not confidentiality + /// (the key derives from an empty passphrase under the public salt): use + /// it where the stored secrets carry their own Tier-2 object password, + /// or as a staging step before [`EncryptedFileStore::rekey`] to a real + /// passphrase. [`file`](SecretStore::file) rejects a blank passphrase; + /// this is the explicit keyless alternative. + pub fn file_unprotected(path: impl AsRef) -> Result { + Ok(Self::File(EncryptedFileStore::open_unprotected(path)?)) + } + /// Open the platform's default OS keyring, failing closed when none /// is reachable (headless / no Secret Service). pub fn os() -> Result { From 351d1e6b47a3476e43b10e0d0f2236e4359c127a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:49:34 +0200 Subject: [PATCH 07/10] refactor(platform-wallet-storage): use keyring_core::mock::Store; annotate v5 deviations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-review polish addressing the lead's dedup self-scan + QA annotation requirements. Dedup (prefer-established-packages): the GAP-005 Os-arm fixture was a bespoke `secrets::testing::InMemoryCredentialStore` (182 LOC). keyring-core 1.0.0 already ships `keyring_core::mock::Store` — not feature-gated, returns an `Arc` usable directly as `SecretStore::Os(..)`, and `build()` returns the SHARED `Arc` per `(service, user)` so a raw SPI `set_secret` (the backend-write attacker primitive) persists across the fresh entry `get_secret` builds. Replaced the custom mock with it and removed `testing.rs` and its `pub mod testing` gate. The L-1 strip-injection and full quadrant still pass on the Os arm (now via the upstream mock). QA annotations (v5 design supersedes Marvin's v4 test-spec wherever it overrides): the three adapted tests now name the superseding v5 clause in the body — TS-ENV-008 (v5 §4.6: plaintext cap = MAX_SECRET_LEN − MAX_ENVELOPE_OVERHEAD, not MAX_SECRET_LEN), TS-ENV-010(a) and the TS-L1-001 quadrant row (v5 §4.1 legacy-tolerant: magic-less + None → bytes+warn, not Corruption; + Some(pw) still fails closed so L-1 holds). 142 secrets unit + integration tests green; clippy clean; all test targets compile. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/envelope.rs | 21 +- .../src/secrets/mod.rs | 4 - .../src/secrets/store.rs | 36 +++- .../src/secrets/testing.rs | 182 ------------------ 4 files changed, 37 insertions(+), 206 deletions(-) delete mode 100644 packages/rs-platform-wallet-storage/src/secrets/testing.rs diff --git a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs index d2cd043b87..3840a3d81b 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs @@ -596,11 +596,11 @@ mod tests { } } - /// TS-ENV-008 (SEC-F006 / GAP-006, v5 cap): the plaintext cap is - /// `MAX_SECRET_LEN − MAX_ENVELOPE_OVERHEAD` (NOT `MAX_SECRET_LEN` as - /// the v4 spec literally read), uniform across schemes, so the - /// enveloped bytes always fit the backend vault cap. Accept at the cap, - /// reject at cap+1 with `max = MAX_PLAINTEXT_LEN`. + /// TS-ENV-008 — **v5 §4.6 supersedes the v4 test-spec.** The plaintext + /// cap is `MAX_SECRET_LEN − MAX_ENVELOPE_OVERHEAD` (NOT `MAX_SECRET_LEN` + /// as TS-ENV-008 literally read); v5 §4.6 / SEC-F006 / GAP-006 fix the + /// off-by-overhead so the enveloped bytes always fit the backend cap. + /// Accept at the cap, reject at cap+1 with `max = MAX_PLAINTEXT_LEN`. #[test] fn plaintext_size_cap_at_envelope_boundary() { let at_cap = vec![0x5Au8; MAX_PLAINTEXT_LEN]; @@ -626,11 +626,12 @@ mod tests { assert!(enveloped.len() <= MAX_SECRET_LEN); } - /// TS-ENV-010 (adopted §4.1 legacy-tolerant contingency): magic/version - /// discrimination. A magic-less blob is a legacy raw value — returned - /// on `None`, refused fail-closed on `Some(pw)`. A magic-present blob - /// with an unknown version fails closed both ways; truncated-after-magic - /// is corruption. + /// TS-ENV-010 — **v5 §4.1 (legacy-tolerant contingency) supersedes the + /// v4 test-spec.** magic/version discrimination: a magic-less blob is a + /// legacy raw value — returned on `None` (TS-ENV-010(a) read `Corruption`; + /// v5 §4.1 makes it bytes+warn), refused fail-closed on `Some(pw)` (so + /// L-1 is preserved). A magic-present blob with an unknown version fails + /// closed both ways; truncated-after-magic is corruption. #[test] fn magic_and_version_discrimination() { let p = pw("pw"); diff --git a/packages/rs-platform-wallet-storage/src/secrets/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index 32a9758e53..4161e42300 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -28,10 +28,6 @@ mod file; mod keyring; mod secret; mod store; -/// In-memory backend test fixtures (writable `CredentialStoreApi` mock). -/// Compiled only under `cfg(test)` or the `__test-helpers` feature. -#[cfg(any(test, feature = "__test-helpers"))] -pub mod testing; mod validate; pub use envelope::MAX_PLAINTEXT_LEN; diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index 5cb850414a..552a159f32 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -499,12 +499,15 @@ mod tests { // re-seals the chosen blob under the resident vault key via `put_bytes` // (a cold/backup-swap actor could only corrupt → DoS, so the strip // requires the vault key — §8.3 arm asymmetry); on Os it overwrites the - // mock keychain item directly (the bare envelope, no second AEAD — where - // the L-1 residual bites hardest, GAP-005 / §8.3). + // keychain item directly (the bare envelope, no second AEAD — where the + // L-1 residual bites hardest, §8.3). GAP-005's writable Os fixture is + // the upstream `keyring_core::mock::Store` (a raw SPI `set_secret` + // bypasses the envelope), so no bespoke mock is needed. + + use keyring_core::mock; use crate::secrets::file::crypto::{KdfParams, ARGON2_MIN_M_KIB, ARGON2_MIN_T, ARGON2_P}; use crate::secrets::file::format::KDF_ID_ARGON2ID; - use crate::secrets::testing::InMemoryCredentialStore; /// Argon2id floor params — fast enough for the keystone tests. fn floor() -> KdfParams { @@ -535,19 +538,28 @@ mod tests { struct Backend { store: SecretStore, _dir: Option, - mock: Option, + mock: Option>, name: &'static str, } impl Backend { /// Write `blob` to `(w, label)` as opaque backend bytes (the - /// attacker's primitive / the protected-enrol setup). + /// attacker's primitive / the protected-enrol setup). On Os this is + /// a raw SPI `set_secret` on the shared mock store, bypassing the + /// `SecretStore` envelope layer exactly as a breached keychain write + /// would. fn place_raw(&self, w: &WalletId, label: &str, blob: &[u8]) { match (&self.store, &self.mock) { (SecretStore::File(fs), _) => fs .put_bytes(w, label, &SecretBytes::from_slice(blob)) .unwrap(), - (SecretStore::Os(_), Some(mock)) => mock.raw_overwrite(w, label, blob), + (SecretStore::Os(_), Some(mock)) => { + let service = format!("{SERVICE_PREFIX}{}", w.to_hex()); + mock.build(&service, label, None) + .unwrap() + .set_secret(blob) + .unwrap(); + } _ => unreachable!("os backend must carry its mock"), } } @@ -565,8 +577,11 @@ mod tests { } fn os_backend() -> Backend { - let mock = InMemoryCredentialStore::new(); - let store = SecretStore::Os(mock.as_dyn()); + // GAP-005: the upstream in-memory mock store. The clone handed to + // `SecretStore::Os` and the handle kept for raw attacker writes + // share the same backing credentials by `Arc`. + let mock = mock::Store::new().unwrap(); + let store = SecretStore::Os(mock.clone()); Backend { store, _dir: None, @@ -651,8 +666,9 @@ mod tests { b.name ); - // magic-less legacy raw + None → bytes (adopted §4.1 contingency; - // deviates from v4 TS-L1-001's Corruption row). + Some → fail closed. + // magic-less legacy raw + None → bytes: v5 §4.1 (legacy-tolerant) + // supersedes v4 TS-L1-001's Corruption row. + Some → fail closed, + // so L-1 is preserved. b.place_raw(&w, "legacy", b"raw-legacy-seed-no-magic"); assert_eq!( b.store diff --git a/packages/rs-platform-wallet-storage/src/secrets/testing.rs b/packages/rs-platform-wallet-storage/src/secrets/testing.rs deleted file mode 100644 index a59aa3b7fa..0000000000 --- a/packages/rs-platform-wallet-storage/src/secrets/testing.rs +++ /dev/null @@ -1,182 +0,0 @@ -//! In-memory test fixtures for the secrets backends. -//! -//! [`InMemoryCredentialStore`] is a writable -//! [`CredentialStoreApi`](keyring_core::api::CredentialStoreApi) double for -//! the [`SecretStore::Os`](crate::secrets::SecretStore::Os) arm — the only -//! shipped credential stores need a live OS keychain / D-Bus session, so -//! the Os arm (where the L-1 strip residual "bites hardest", per the threat -//! model §8.3) is otherwise un-coverable in CI (GAP-005). -//! -//! Beyond the SPI, it exposes [`raw_overwrite`](InMemoryCredentialStore::raw_overwrite) -//! / [`raw_get`](InMemoryCredentialStore::raw_get) so a test can play the -//! **backend-write attacker**: replace a slot's stored bytes with an -//! arbitrary blob (e.g. a stripped scheme-0 envelope) the way a breached -//! keychain would, then assert the strict read still fails closed. -//! -//! Compiled under `cfg(test)` or the `__test-helpers` feature, never in a -//! production build. - -use std::any::Any; -use std::collections::HashMap; -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::validate::WalletId; -use super::SERVICE_PREFIX; - -/// Shared `(service, user) -> bytes` map backing every credential a store -/// hands out, so writes through one entry are visible to another. -type Items = Arc>>>; - -/// An in-memory, writable [`CredentialStoreApi`] for Os-arm tests. -#[derive(Default)] -pub struct InMemoryCredentialStore { - items: Items, -} - -impl InMemoryCredentialStore { - /// A fresh, empty store, ready to install as - /// `SecretStore::Os(store.into_arc())`. - pub fn new() -> Self { - Self::default() - } - - /// Hand back an `Arc` for the - /// `SecretStore::Os` arm. Clones share the same backing map, so the - /// returned trait object and `self` see each other's writes. - pub fn as_dyn(&self) -> Arc { - Arc::new(Self { - items: Arc::clone(&self.items), - }) - } - - /// The service string `SecretStore` derives for `wallet_id`, so a test - /// can address the exact slot the Os arm reads/writes. - pub fn service_for(wallet_id: &WalletId) -> String { - format!("{SERVICE_PREFIX}{}", wallet_id.to_hex()) - } - - /// **Attacker primitive:** overwrite the raw bytes at `(wallet_id, - /// label)` with `bytes`, bypassing any envelope layer — exactly what a - /// breached keychain write does. - pub fn raw_overwrite(&self, wallet_id: &WalletId, label: &str, bytes: &[u8]) { - self.lock().insert( - (Self::service_for(wallet_id), label.to_string()), - bytes.to_vec(), - ); - } - - /// Read the raw stored bytes at `(wallet_id, label)`, if any. - pub fn raw_get(&self, wallet_id: &WalletId, label: &str) -> Option> { - self.lock() - .get(&(Self::service_for(wallet_id), label.to_string())) - .cloned() - } - - fn lock(&self) -> std::sync::MutexGuard<'_, HashMap<(String, String), Vec>> { - self.items.lock().unwrap_or_else(|p| p.into_inner()) - } -} - -impl CredentialStoreApi for InMemoryCredentialStore { - fn vendor(&self) -> String { - "dash.platform-wallet-storage/in-memory-test".to_string() - } - - fn id(&self) -> String { - "in-memory-credential-store".to_string() - } - - fn build( - &self, - service: &str, - user: &str, - _modifiers: Option<&HashMap<&str, &str>>, - ) -> KeyringResult { - let cred = InMemoryCredential { - items: Arc::clone(&self.items), - service: service.to_string(), - user: user.to_string(), - }; - 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 InMemoryCredentialStore { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // Never render stored bytes (they may be secret material). - f.debug_struct("InMemoryCredentialStore") - .field("entries", &self.lock().len()) - .finish() - } -} - -/// One `(service, user)` slot in an [`InMemoryCredentialStore`]. -struct InMemoryCredential { - items: Items, - service: String, - user: String, -} - -impl InMemoryCredential { - fn key(&self) -> (String, String) { - (self.service.clone(), self.user.clone()) - } - - fn lock(&self) -> std::sync::MutexGuard<'_, HashMap<(String, String), Vec>> { - self.items.lock().unwrap_or_else(|p| p.into_inner()) - } -} - -impl CredentialApi for InMemoryCredential { - fn set_secret(&self, secret: &[u8]) -> KeyringResult<()> { - self.lock().insert(self.key(), secret.to_vec()); - Ok(()) - } - - fn get_secret(&self) -> KeyringResult> { - self.lock() - .get(&self.key()) - .cloned() - .ok_or(KeyringError::NoEntry) - } - - fn delete_credential(&self) -> KeyringResult<()> { - if self.lock().remove(&self.key()).is_some() { - Ok(()) - } else { - Err(KeyringError::NoEntry) - } - } - - fn get_credential(&self) -> KeyringResult>> { - Ok(None) - } - - fn get_specifiers(&self) -> Option<(String, String)> { - Some(self.key()) - } - - fn as_any(&self) -> &dyn Any { - self - } -} - -impl std::fmt::Debug for InMemoryCredential { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("InMemoryCredential") - .field("service", &self.service) - .field("user", &self.user) - .finish_non_exhaustive() - } -} From d3df41e913a617c65007e4dc942b450fb34c86b3 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:26:50 +0200 Subject: [PATCH 08/10] =?UTF-8?q?docs(platform-wallet-storage):=20QA=20fix?= =?UTF-8?q?es=20=E2=80=94=20rustdoc=20clarity=20+=20ephemeral-ID=20scrub?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated QA-wave fix batch, part 1 (docs/comments only). - No-recovery + entropy-delegation warnings added to `set_secret` AND `reprotect` rustdoc (lost object password = permanently unrecoverable; Tier-2 confidentiality rests on caller-supplied password entropy, strength policy is the caller's). - SECRETS.md: new paragraph that an OS-arm envelope tag failure is ambiguous (wrong password OR corrupted keychain item), resolving the `WrongPassword` rustdoc's forward reference; added `UnsupportedEnvelopeVersion` to the itemized BadStoreFormat group; added the `None` + truncated-header → `Corruption` row to the strict-read table; documented `reprotect`'s absent-entry no-op. - `SecretStore` enum doc corrected: reads are `get` / `get_secret` / the read inside `reprotect`, not "only `get`". - `BlankPassphrase` Display now points to `open_unprotected` for the deliberate keyless-vault case. - Softened "atomic on both arms" wording: the File arm is the vault's atomic replace; the Os arm inherits the backend's single-item-replace contract. - Ephemeral-ID scrub: removed session-internal finding/spec/clause IDs (SEC-*, GAP-*, TS-*, L-1/2/3, F-*, AC-*, §x.y) and v4→v5 history narration from all committed comments/rustdoc and SECRETS.md, keeping the technical rationale. Traceability belongs in the PR description, not the source. No production logic changed. clippy --lib --tests clean; 142 secrets unit tests green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../rs-platform-wallet-storage/SECRETS.md | 58 +++++--- .../src/secrets/envelope.rs | 72 +++++----- .../src/secrets/error.rs | 20 +-- .../src/secrets/file/mod.rs | 19 +-- .../src/secrets/secret.rs | 16 +-- .../src/secrets/store.rs | 129 ++++++++++-------- 6 files changed, 173 insertions(+), 141 deletions(-) diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index 99991f189d..8732e041cc 100644 --- a/packages/rs-platform-wallet-storage/SECRETS.md +++ b/packages/rs-platform-wallet-storage/SECRETS.md @@ -156,7 +156,7 @@ scheme u8 (0 = unprotected passthrough, 1 = password) — fail closed **regardless of the password**: an unparseable future format can be neither safely unwrapped nor treated as unprotected. -#### The strict, fail-closed read (the L-1 keystone) +#### The strict, fail-closed read The defining risk of any opt-in "some objects are extra-protected" scheme is **strip / downgrade**: an attacker who can WRITE the backend replaces a @@ -175,19 +175,20 @@ object must be protected": | `password` arg | stored blob | result | |---|---|---| | `Some(pw)` | valid scheme-1 | the secret, or `WrongPassword` on tag fail | -| **`Some(pw)`** | **scheme-0 / legacy magic-less raw** | **`ExpectedProtectedButUnsealed` — FAIL CLOSED ★** | +| **`Some(pw)`** | **scheme-0 / legacy magic-less raw** | **`ExpectedProtectedButUnsealed` — FAIL CLOSED** | | `Some(pw)` | scheme-1 but truncated/corrupt | `Corruption` | | `Some/None` | magic present, unknown version/scheme | `UnsupportedEnvelopeVersion` | | `None` | valid scheme-1 | `NeedsPassword` (never ciphertext) | | `None` | scheme-0 | the secret | | `None` | legacy magic-less raw | the secret (+ a one-time warning; re-wrapped on next write) | +| `None` | magic present but truncated header | `Corruption` | | any | absent entry | `Ok(None)` (deletion = DoS, never injection) | The load-bearing row is **`Some(pw)` + non-envelope ⇒ `ExpectedProtectedButUnsealed`**: with a password in hand, a non-protected blob can only mean a strip, so it is refused and **no bytes are returned**. -A DET (consumer) bug alone — over- or under-supplying a password — fails -closed in *every* direction. +A consumer bug alone — over- or under-supplying a password — fails closed +in *every* direction. **Arm asymmetry.** On the file arm the stored bytes are themselves sealed under the vault key, so producing a *readable* stripped blob at a slot @@ -208,23 +209,25 @@ as integrity-protected, security-critical state (it is one more field alongside the addresses/policy the wallet DB must already protect). **Value rollback is NOT defended.** Restoring an *older valid* scheme-1 -envelope under the *current* password decrypts cleanly. L-1 closes the -strip/downgrade injection, not value rollback; if backup-swap/restore-old -is in scope, anchor a monotonic version in integrity-protected consumer -metadata. Do not mistake L-1 for rollback protection. +envelope under the *current* password decrypts cleanly. The strict read +closes the strip/downgrade injection, not value rollback; if +backup-swap/restore-old is in scope, anchor a monotonic version in +integrity-protected consumer metadata. Do not mistake the strict read for +rollback protection. #### Add / change / remove an object password `reprotect(service, label, current, new)` does it in one same-slot unwrap→rewrap→overwrite: read under the `current` expectation (so a strip is caught before any rewrite), then write under `new` — `None`→`Some` adds, -`Some`→`Some` changes, `Some`→`None` removes. The write is the atomic -same-slot overwrite, so a crash between the read and the commit leaves the -prior value intact and readable under `current`. **After a successful call -the consumer MUST update its own protection-status record** (the L-1 -expectation lives there). There is **no password recovery** — losing an -object password bricks that object (an availability trade-off the UX must -state plainly). +`Some`→`Some` changes, `Some`→`None` removes. An absent object is a no-op +(`Ok(())`). The rewrite is a same-slot overwrite — atomic on the file arm, +and on the OS arm inheriting the backend's single-item-replace contract — +so a crash between the read and the commit leaves the prior value intact +and readable under `current`. **After a successful call the consumer MUST +update its own protection-status record** (the protection expectation lives +there). There is **no password recovery** — losing an object password +bricks that object (an availability trade-off the UX must state plainly). #### Entropy policy is the consumer's @@ -243,9 +246,10 @@ The envelope is net-new, so post-feature reads/writes go through it. A decrypted entry that lacks the `PWSEV` magic is treated as a **legacy unprotected** value: returned on a `None` read (with a one-time warning, and re-wrapped on the next write) and refused (`ExpectedProtectedButUnsealed`) -on a `Some(pw)` read — so legacy tolerance never weakens L-1. (A pre-feature -build that persisted vault files is a deployment fact outside this crate; -the legacy-tolerant read makes the transition seamless either way.) +on a `Some(pw)` read — so legacy tolerance never weakens the strict read. +(A pre-feature build that persisted vault files is a deployment fact outside +this crate; the legacy-tolerant read makes the transition seamless either +way.) ### Internal SPI @@ -373,7 +377,7 @@ is **lossless**: `WrongPassphrase`, `Corruption`, `AlreadyLocked`, `KdfFailure`, `VersionUnsupported`, `MalformedVault`, `InsecurePermissions`, `InsecureParentDir`, `SecretTooLarge`, `VaultTooLarge`, `Encrypt`, and `InvalidLabel` are distinct typed variants. The Tier-2 layer adds five more: -`ExpectedProtectedButUnsealed` (the L-1 fail-closed strip refusal), +`ExpectedProtectedButUnsealed` (the fail-closed strip refusal), `NeedsPassword` (a protected object read with no password), `WrongPassword` (object-password tag fail — distinct from the Tier-1 `WrongPassphrase`), `BlankPassphrase` (a blank vault passphrase or object password), and @@ -395,15 +399,25 @@ discriminant — keyring variants carrying raw bytes (`BadEncoding`, `BadDataFormat`) are collapsed so their bytes never enter the error (CWE-209/CWE-532). +**`WrongPassword` on the OS arm is ambiguous.** A Tier-2 envelope AEAD tag +failure surfaces as `WrongPassword`, but on the OS-keyring arm the stored +item is the bare envelope with no second authentication layer, so a tag +failure can mean EITHER a wrong object password OR a corrupted keychain +item — one AEAD tag cannot disambiguate the two. Treat `WrongPassword` on +the OS arm as "wrong password or corrupted item." On the file arm it is +unambiguous: the vault's own per-entry tag has already authenticated the +stored bytes before the envelope is parsed. + The internal SPI projection `From for keyring_core::Error` keeps the `WrongPassphrase` / `AlreadyLocked` variants recoverable: they ride in `NoStorageAccess` with the typed `SecretStoreError` boxed as the source, so an SPI-only consumer can recover them via `err.source().and_then(|s| s.downcast_ref::())`. The `BadStoreFormat` group (`Corruption`, `KdfFailure`, -`VersionUnsupported`, `MalformedVault`, `InsecurePermissions`, -`InsecureParentDir`, `SecretTooLarge`, `VaultTooLarge`, `Decrypt`, -`Encrypt`, `OsKeyring`) has no box slot and carries only a secret-free +`VersionUnsupported`, `UnsupportedEnvelopeVersion`, `MalformedVault`, +`InsecurePermissions`, `InsecureParentDir`, `SecretTooLarge`, +`VaultTooLarge`, `Decrypt`, `Encrypt`, `OsKeyring`) has no box slot and +carries only a secret-free string; those remain fully typed on the `SecretStore` path (so e.g. `VaultTooLarge` / `SecretTooLarge` are not losslessly recoverable through the SPI downcast). diff --git a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs index 3840a3d81b..efb271e0a2 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs @@ -29,12 +29,12 @@ //! ## Reused, never reinvented //! - KDF: [`crypto::derive_key`] (Argon2id) with a fresh 32-byte salt; the //! param **ceiling is enforced BEFORE derivation** on the -//! attacker-controllable header (L-2, [`KdfParams::enforce_bounds`]). +//! attacker-controllable header ([`KdfParams::enforce_bounds`]). //! - AEAD: [`crypto::seal`]/[`crypto::open`] (XChaCha20-Poly1305), fresh //! per-wrap nonce; a tag failure maps to //! [`SecretStoreError::WrongPassword`] with no plaintext. //! - AAD binds `domain ‖ magic ‖ version ‖ scheme ‖ kdf ‖ salt ‖ wallet_id -//! ‖ label` (L-3), mirroring [`format::aad`]/[`format::verify_aad`] so a +//! ‖ label`, mirroring [`format::aad`]/[`format::verify_aad`] so a //! relocated/confused blob fails the tag. //! //! No bespoke crypto. @@ -86,7 +86,7 @@ pub(crate) const MAX_ENVELOPE_OVERHEAD: usize = 128; /// MAX_ENVELOPE_OVERHEAD`. Capping the **plaintext** (uniformly for both /// schemes) keeps the user-visible limit stable AND guarantees the /// enveloped bytes always fit the backend vault's own `MAX_SECRET_LEN` -/// `put_bytes` cap (design §4.6 / SEC-F006 / GAP-006). Re-exported at +/// `put_bytes` cap. Re-exported at /// [`crate::secrets`] as the documented, stable user-facing cap. pub const MAX_PLAINTEXT_LEN: usize = MAX_SECRET_LEN - MAX_ENVELOPE_OVERHEAD; @@ -147,7 +147,7 @@ pub(crate) fn wrap_with_params( return Ok(SecretBytes::new(out)); }; - // Reject a blank object password BEFORE any derivation (SEC-J). + // Reject a blank object password BEFORE any derivation. if pw.is_blank() { return Err(SecretStoreError::BlankPassphrase); } @@ -174,14 +174,14 @@ pub(crate) fn wrap_with_params( } /// Unwrap `blob` for `(wallet_id, label)`, applying the **strict, -/// fail-closed** read (the L-1 keystone). The "expected-protected" bit is +/// fail-closed** read. The "expected-protected" bit is /// the caller's assertion, surfaced solely by `password`, and is NEVER /// inferred from the blob's scheme byte. /// /// | `password` | stored blob | result | /// |---|---|---| /// | `Some(pw)` | valid scheme-1 | secret, or [`WrongPassword`] on tag fail | -/// | `Some(pw)` | scheme-0 **or** magic-less (legacy raw) | [`ExpectedProtectedButUnsealed`] ★ | +/// | `Some(pw)` | scheme-0 **or** magic-less (legacy raw) | [`ExpectedProtectedButUnsealed`] | /// | `Some(pw)` | scheme-1 but too short | [`Corruption`] (sealed-but-broken) | /// | `Some/None` | magic present, unknown version/scheme | [`UnsupportedEnvelopeVersion`] | /// | `None` | valid scheme-1 | [`NeedsPassword`] (never ciphertext) | @@ -205,7 +205,7 @@ pub(crate) fn unwrap( blob: &[u8], ) -> Result { // Magic-less ⇒ a legacy unprotected raw value (scheme-0-equivalent), - // per the adopted §4.1 read-path contingency. + // (legacy-tolerant read-path: a None read returns it, a Some(pw) read refuses). if !blob.starts_with(MAGIC) { return match password { None => { @@ -213,7 +213,7 @@ pub(crate) fn unwrap( Ok(SecretBytes::from_slice(blob)) } // Caller asserted protection but found a magic-less raw value: - // a strip/downgrade ⇒ FAIL CLOSED (L-1). Never returns bytes. + // a strip/downgrade ⇒ FAIL CLOSED. Never returns bytes. Some(_) => Err(SecretStoreError::ExpectedProtectedButUnsealed), }; } @@ -226,7 +226,7 @@ pub(crate) fn unwrap( let version = blob[MAGIC.len()]; if version != ENVELOPE_VERSION { // Fail closed regardless of password — an unparseable future format - // can be neither safely unwrapped nor treated as scheme-0 (GAP-009). + // can be neither safely unwrapped nor treated as scheme-0. return Err(SecretStoreError::UnsupportedEnvelopeVersion { found: version }); } @@ -235,7 +235,7 @@ pub(crate) fn unwrap( match scheme { SCHEME_UNPROTECTED => match password { None => Ok(SecretBytes::from_slice(body)), - // Strip: caller expected protection, blob is unprotected (L-1). + // Strip: caller expected protection, blob is unprotected. Some(_) => Err(SecretStoreError::ExpectedProtectedButUnsealed), }, SCHEME_PASSWORD => match password { @@ -250,9 +250,9 @@ pub(crate) fn unwrap( /// Decrypt a scheme-1 body. The KDF params, salt, and nonce are all read /// from the (attacker-controllable) header; the param **ceiling is -/// enforced before** [`crypto::derive_key`] allocates (L-2), and every +/// enforced before** [`crypto::derive_key`] allocates, and every /// header field that feeds key/AAD is bound into the AAD so any in-place -/// edit fails the tag (L-3). +/// edit fails the tag. fn unwrap_scheme1( wallet_id: &WalletId, label: &str, @@ -265,7 +265,7 @@ fn unwrap_scheme1( return Err(SecretStoreError::Corruption); } let kdf = decode_kdf(&body[..KDF_FIELD_LEN]); - // L-2: gate the inflated/unknown header BEFORE any derivation/alloc. + // Gate the inflated/unknown header BEFORE any derivation/alloc. kdf.enforce_bounds()?; let mut salt = [0u8; SALT_LEN]; @@ -285,7 +285,7 @@ fn unwrap_scheme1( } } -/// Build the scheme-1 AAD binding object identity + header (L-3), +/// Build the scheme-1 AAD binding object identity + header, /// length-prefixed for the variable fields, mirroring /// [`format::aad`](super::file::format::aad)/`verify_aad`. fn scheme1_aad( @@ -422,7 +422,7 @@ mod tests { .to_vec() } - /// TS-ENV-001: scheme-0 passthrough round-trip; the wrapped form leads + /// scheme-0 passthrough round-trip; the wrapped form leads /// with magic, version=1, scheme=0, then the raw payload. #[test] fn scheme0_passthrough_round_trip() { @@ -436,7 +436,7 @@ mod tests { assert_eq!(got.expose_secret(), secret); } - /// TS-ENV-002: scheme-1 round-trip; header records the argon2id id, a + /// scheme-1 round-trip; header records the argon2id id, a /// 32-byte fresh salt and 24-byte nonce, ct != pt, and two wraps of the /// same secret/pw differ in salt+nonce (no reuse). #[test] @@ -467,7 +467,7 @@ mod tests { ); } - /// TS-ENV-003: wrong object password → WrongPassword, no plaintext. + /// Wrong object password → WrongPassword, no plaintext. #[test] fn wrong_password_fails_closed() { let blob = wrap_p(&wid(1), "seed", Some(&pw("right")), b"seed", floor()); @@ -478,7 +478,7 @@ mod tests { ); } - /// TS-ENV-004: identity AAD (L-3) — a protected blob unwrapped at any + /// Identity AAD — a protected blob unwrapped at any /// other (wallet, label) fails the tag; same-identity still succeeds. #[test] fn relocation_across_identity_is_rejected() { @@ -495,7 +495,7 @@ mod tests { assert_eq!(ok.expose_secret(), b"seed"); } - /// TS-ENV-005: per-field header tamper. Unknown KDF id is rejected by + /// Per-field header tamper. Unknown KDF id is rejected by /// `enforce_bounds` (KdfFailure) before derive; in-bounds KDF shifts, /// salt, and nonce all fail the AEAD tag (WrongPassword) — never the /// plaintext. @@ -545,7 +545,7 @@ mod tests { )); } - /// TS-ENV-006 ★ (L-2): an inflated KDF param on a forged header is + /// An inflated KDF param on a forged header is /// rejected by `enforce_bounds` BEFORE `derive_key` allocates — the /// ~4 TiB allocation never happens (the test would OOM if it did). The /// exact ceilings remain valid params. @@ -582,7 +582,7 @@ mod tests { .is_ok()); } - /// TS-ENV-007: a blank object password is rejected at enrol; nothing + /// A blank object password is rejected at enrol; nothing /// is sealed. #[test] fn blank_object_password_rejected_at_enrol() { @@ -596,11 +596,10 @@ mod tests { } } - /// TS-ENV-008 — **v5 §4.6 supersedes the v4 test-spec.** The plaintext - /// cap is `MAX_SECRET_LEN − MAX_ENVELOPE_OVERHEAD` (NOT `MAX_SECRET_LEN` - /// as TS-ENV-008 literally read); v5 §4.6 / SEC-F006 / GAP-006 fix the - /// off-by-overhead so the enveloped bytes always fit the backend cap. - /// Accept at the cap, reject at cap+1 with `max = MAX_PLAINTEXT_LEN`. + /// The plaintext is capped at `MAX_PLAINTEXT_LEN` (`MAX_SECRET_LEN − + /// MAX_ENVELOPE_OVERHEAD`), uniform across schemes, so plaintext + + /// overhead always fits the backend's own `MAX_SECRET_LEN` cap. Accept + /// at the cap, reject at cap+1 with `max = MAX_PLAINTEXT_LEN`. #[test] fn plaintext_size_cap_at_envelope_boundary() { let at_cap = vec![0x5Au8; MAX_PLAINTEXT_LEN]; @@ -626,12 +625,11 @@ mod tests { assert!(enveloped.len() <= MAX_SECRET_LEN); } - /// TS-ENV-010 — **v5 §4.1 (legacy-tolerant contingency) supersedes the - /// v4 test-spec.** magic/version discrimination: a magic-less blob is a - /// legacy raw value — returned on `None` (TS-ENV-010(a) read `Corruption`; - /// v5 §4.1 makes it bytes+warn), refused fail-closed on `Some(pw)` (so - /// L-1 is preserved). A magic-present blob with an unknown version fails - /// closed both ways; truncated-after-magic is corruption. + /// magic/version discrimination: a magic-less blob is a legacy raw + /// value — returned on a `None` read (with a one-time warning), refused + /// fail-closed on `Some(pw)` so the strict rule holds. A magic-present + /// blob with an unknown version fails closed both ways; truncated- + /// after-magic is corruption. #[test] fn magic_and_version_discrimination() { let p = pw("pw"); @@ -640,7 +638,7 @@ mod tests { // None ⇒ legacy raw bytes (adopted contingency; NOT Corruption). let got = unwrap(&wid(1), "seed", None, &legacy).unwrap(); assert_eq!(got.expose_secret(), &legacy[..]); - // Some(pw) ⇒ strip/downgrade ⇒ fail closed (L-1 preserved). + // Some(pw) ⇒ strip/downgrade ⇒ fail closed. assert!(matches!( unwrap(&wid(1), "seed", Some(&p), &legacy).unwrap_err(), SecretStoreError::ExpectedProtectedButUnsealed @@ -655,7 +653,7 @@ mod tests { )); // (c) Magic OK but version = 2 ⇒ UnsupportedEnvelopeVersion{2}, - // regardless of password (GAP-009). + // regardless of password. let mut v2 = wrap_bytes(&wid(1), "seed", None, b"x"); v2[O_VERSION] = 2; for arg in [None, Some(&p)] { @@ -674,7 +672,7 @@ mod tests { )); } - /// Non-vacuity helper for the L-1 keystone (used here and by the store + /// Non-vacuity helper for the strict read (used here and by the store /// tests): a scheme-0 blob carrying `secret` DOES decode under `None`. #[test] fn scheme0_some_password_fails_closed_strip() { @@ -686,7 +684,7 @@ mod tests { .expose_secret(), b"attacker-seed" ); - // …but Some(pw) ⇒ ExpectedProtectedButUnsealed, no bytes (L-1). + // …but Some(pw) ⇒ ExpectedProtectedButUnsealed, no bytes. assert!(matches!( unwrap(&wid(1), "seed", Some(&pw("pw")), &blob).unwrap_err(), SecretStoreError::ExpectedProtectedButUnsealed @@ -704,7 +702,7 @@ mod tests { assert!(bool::from(got.ct_eq(&original))); } - /// TS-ENV-009: deterministic byte-level fuzz. Every mutant unwrap is a + /// Deterministic byte-level fuzz. Every mutant unwrap is a /// clean `Ok` or a TYPED `SecretStoreError` — never a panic, never /// plaintext from a tag-failing branch. The `None` path (no Argon2 /// derivation) runs the full 2000 mutants + every truncation; the diff --git a/packages/rs-platform-wallet-storage/src/secrets/error.rs b/packages/rs-platform-wallet-storage/src/secrets/error.rs index 5290a39c8b..6c69e9e8b9 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/error.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/error.rs @@ -20,7 +20,7 @@ pub enum SecretStoreError { #[error("wrong passphrase")] WrongPassphrase, - /// Tier-2 (L-1 keystone): the caller asserted — by supplying an object + /// Tier-2 strip/downgrade guard: the caller asserted — by supplying an object /// password — that this object MUST be password-protected, but the /// stored value is a well-formed UNPROTECTED envelope (scheme-0) or a /// legacy magic-less raw value, i.e. a strip/downgrade. **Fails @@ -50,7 +50,9 @@ pub enum SecretStoreError { /// [`SecretString::is_blank`]. CWE-521. /// /// [`SecretString::is_blank`]: crate::secrets::SecretString::is_blank - #[error("passphrase must not be blank")] + #[error( + "passphrase must not be blank; for a deliberately keyless file vault use open_unprotected" + )] BlankPassphrase, /// AEAD tag failure on a stored entry (or rekey re-encrypt) *after* @@ -76,7 +78,7 @@ pub enum SecretStoreError { /// known version, a `scheme`) this build does not understand. Fails /// closed REGARDLESS of the password argument — an unparseable future /// format can be neither safely unwrapped nor safely treated as - /// unprotected, so it is refused both ways (GAP-009). Mirrors + /// unprotected, so it is refused both ways. Mirrors /// [`VersionUnsupported`] for the vault format. /// /// [`VersionUnsupported`]: SecretStoreError::VersionUnsupported @@ -288,7 +290,7 @@ impl From for SecretStoreError { /// `err.source().and_then(|s| s.downcast_ref::())`. /// These are all "the caller must act on a credential/expectation to /// proceed" states, so lossless recovery lets an SPI consumer react -/// precisely (resolves GAP-004 for these variants). +/// precisely. /// - The format/crypto group — including [`UnsupportedEnvelopeVersion`] /// (a fail-closed forward-format incompatibility, mirroring /// [`VersionUnsupported`]) — collapses into @@ -454,7 +456,7 @@ mod tests { assert!(!format!("{k}").contains("plaintext")); } - /// TS-ERR-001: the five new variants exist, are constructable, render + /// The five new variants exist, are constructable, render /// distinct non-empty messages, and the Tier-2 `WrongPassword` is NOT /// the Tier-1 `WrongPassphrase` (nor is the unseal error `Corruption`). #[test] @@ -477,7 +479,7 @@ mod tests { assert_eq!(msgs.len(), 5, "all five messages must be distinct"); } - /// TS-ERR-002: Display + Debug render static, secret-free text. The + /// Display + Debug render static, secret-free text. The /// version variant surfaces the (non-secret) version byte and nothing /// more. #[test] @@ -490,7 +492,7 @@ mod tests { assert_eq!(E::WrongPassword.to_string(), "wrong object password"); assert_eq!( E::BlankPassphrase.to_string(), - "passphrase must not be blank" + "passphrase must not be blank; for a deliberately keyless file vault use open_unprotected" ); assert_eq!( E::ExpectedProtectedButUnsealed.to_string(), @@ -513,7 +515,7 @@ mod tests { } } - /// TS-ERR-003 (resolving GAP-004): the four Tier-2 credential / + /// The four Tier-2 credential / /// protection states project to a recoverable `NoStorageAccess` with /// the typed error losslessly downcast-able, leaking no secret. #[test] @@ -540,7 +542,7 @@ mod tests { } } - /// TS-ERR-003: `UnsupportedEnvelopeVersion` projects to the + /// `UnsupportedEnvelopeVersion` projects to the /// secret-free `BadStoreFormat` group (forward-format incompat, /// mirroring `VersionUnsupported`). #[test] 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 6cfc57ecb5..1d2471e780 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -142,7 +142,7 @@ impl EncryptedFileStore { // Tier-1 baseline: reject a blank passphrase (empty / all-whitespace) // BEFORE touching the filesystem. A blank passphrase derives a key // from a public salt only — obfuscation, not confidentiality - // (SEC-A / F-1). This is an INTENDED behavioural break for any caller + // (obfuscation, not confidentiality). This is an INTENDED behavioural break for any caller // that relied on `SecretString::empty()`; a deliberate keyless vault // must use [`open_unprotected`](Self::open_unprotected). No vault // file is created or altered for a blank passphrase. @@ -155,8 +155,9 @@ impl EncryptedFileStore { /// passphrase under the public salt, so this is **obfuscation, not /// confidentiality**: use it only where the stored secrets carry their /// own Tier-2 object password, or as a staging step before - /// [`rekey`](Self::rekey) to a real passphrase. AC-2.1 maps HERE, not to - /// [`open`](Self::open) (which now rejects a blank passphrase). + /// [`rekey`](Self::rekey) to a real passphrase. This is the explicit + /// keyless door, distinct from [`open`](Self::open) (which now rejects a + /// blank passphrase). pub fn open_unprotected(path: impl AsRef) -> Result { Self::open_inner(path.as_ref(), SecretString::empty()) } @@ -506,7 +507,7 @@ fn lock_path_for(path: &Path) -> PathBuf { /// [`SecretStoreError::BlankPassphrase`]. The floor is the coarse /// [`MIN_PASSPHRASE_LEN`] (1 today = merely non-blank); the real entropy /// policy is the consumer's (see `SECRETS.md`). A blank check alone closes -/// SEC-A/F-1; the length term keeps the floor wired for a future bump. +/// the length term keeps the floor wired for a future bump. fn reject_weak_passphrase(passphrase: &SecretString) -> Result<(), SecretStoreError> { if passphrase.is_blank() || passphrase.trimmed().len() < MIN_PASSPHRASE_LEN { return Err(SecretStoreError::BlankPassphrase); @@ -1474,7 +1475,7 @@ mod tests { ); } - /// TS-T1-001: a blank passphrase is rejected at `open` → + /// A blank passphrase is rejected at `open` → /// `BlankPassphrase`; no vault file (or lock sidecar) is created. #[test] fn open_rejects_blank_passphrase() { @@ -1499,7 +1500,7 @@ mod tests { } } - /// TS-T1-002: a blank passphrase is rejected at `rekey`; the resident + /// A blank passphrase is rejected at `rekey`; the resident /// vault, key, and on-disk file are UNCHANGED — the original passphrase /// still reads every entry, live and after reopen. #[test] @@ -1523,7 +1524,7 @@ mod tests { assert_eq!(entry(&s2, wid(1), "seed").get_secret().unwrap(), b"v1"); } - /// TS-T1-003: `open_unprotected` permits a deliberate keyless vault that + /// `open_unprotected` permits a deliberate keyless vault that /// round-trips; a real-passphrase `open` of that keyless vault then /// fails with `WrongPassphrase` (it is keyless, not real-pass). #[test] @@ -1550,7 +1551,7 @@ mod tests { ); } - /// TS-T1-004: empty→real passphrase migration via `rekey`. After rekey, + /// Empty→real passphrase migration via `rekey`. After rekey, /// `open(real)` reads every entry; the keyless door no longer opens it; /// no `.bak`/`.tmp` residue beside the vault. #[test] @@ -1592,7 +1593,7 @@ mod tests { } } - /// TS-T1-004 crash-safety: a disk-write failure mid-rekey leaves the + /// Crash-safety: a disk-write failure mid-rekey leaves the /// pre-rekey keyless vault intact and readable via `open_unprotected` /// (mirrors rekey_does_not_corrupt_on_disk_temp_failure). #[cfg(unix)] diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index 46ca130b3f..9363c57584 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/secret.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -225,7 +225,7 @@ impl<'de> serde::Deserialize<'de> for SecretString { /// Render the JSON schema as a plain `string` carrying **no** length or /// value policy: no `minLength`/`maxLength`/`pattern`/`format` (would leak /// a length policy) and no `example`/`default` (would embed a value) -/// (F-7). A short, value-free `description` marks sensitivity. +/// A short, value-free `description` marks sensitivity. /// /// Gated behind the default-off `secret-schemars` feature (which implies /// `secret-serde`). Pulls in no `Serialize`/`Display` path. @@ -386,7 +386,7 @@ mod tests { assert_eq!(SecretString::default().len(), 0); } - /// TS-SER-001: `is_blank()` truth table. The boundary deliberately + /// `is_blank()` truth table. The boundary deliberately /// exercises Unicode whitespace — `str::trim` uses the `White_Space` /// property, so NBSP (`U+00A0`) trims to blank but ZWSP (`U+200B`, /// not `White_Space`) does not. @@ -410,7 +410,7 @@ mod tests { ); } - /// TS-SER-002: `is_blank` returns a `bool` and exposes no borrowed + /// `is_blank` returns a `bool` and exposes no borrowed /// plaintext, callable with only `secrets` (no serde/schemars). #[test] fn is_blank_signature_returns_bool_no_borrow() { @@ -419,7 +419,7 @@ mod tests { assert!(!f(&SecretString::new("x"))); } - /// TS-SER-005 / TS-SER-007: `SecretString` must never implement + /// `SecretString` must never implement /// `Serialize` or `Display`, even with serde compiled in. This is a /// compile-time `!impl` assertion — adding either impl breaks the /// build. `serde::Serialize` is nameable here because `secrets` always @@ -429,7 +429,7 @@ mod tests { static_assertions::assert_not_impl_any!(SecretString: serde::Serialize, std::fmt::Display); } - /// TS-SER-008 / GAP-002 regression: the `serde` DEP is on under + /// Regression: the `serde` DEP is on under /// `secrets`, yet the `Deserialize` IMPL stays ABSENT because it is /// gated on the dedicated `secret-serde` feature — proving the /// default-off gate is satisfiable even while serde is compiled. @@ -441,7 +441,7 @@ mod tests { ); } - /// TS-SER-008: with `secret-serde` on, the `Deserialize` impl is + /// With `secret-serde` on, the `Deserialize` impl is /// present (and `Serialize` is still absent — see the always-on test). #[cfg(feature = "secret-serde")] #[test] @@ -450,7 +450,7 @@ mod tests { static_assertions::assert_not_impl_any!(SecretString: serde::Serialize); } - /// TS-SER-003: `Deserialize` round-trips the value through the + /// `Deserialize` round-trips the value through the /// zeroizing constructor; the result `ct_eq`s a directly-built secret /// and has the right length. #[cfg(feature = "secret-serde")] @@ -463,7 +463,7 @@ mod tests { assert_eq!(s.len(), 28); } - /// TS-SER-006: `JsonSchema` renders a plain `string` and leaks no + /// `JsonSchema` renders a plain `string` and leaks no /// length/value policy — no `minLength`/`maxLength`/`pattern`/`format`, /// no `example`/`default`/`enum`. #[cfg(feature = "secret-schemars")] diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index 552a159f32..b545f2f26a 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -20,10 +20,12 @@ use super::{default_credential_store, EncryptedFileStore, SERVICE_PREFIX}; /// A passphrase-or-OS-keyring backed store for wallet secret material. /// -/// The only public read path is [`get`](SecretStore::get), which yields a -/// zeroizing [`SecretBytes`] — a raw `Vec` never crosses this -/// boundary. Backend selection is an explicit operator decision; there is -/// no silent fallback between the two arms. +/// Every read path ([`get`](SecretStore::get), +/// [`get_secret`](SecretStore::get_secret), and the read inside +/// [`reprotect`](SecretStore::reprotect)) yields a zeroizing +/// [`SecretBytes`] — a raw `Vec` never crosses this boundary. Backend +/// selection is an explicit operator decision; there is no silent fallback +/// between the two arms. pub enum SecretStore { /// Self-contained Argon2id + XChaCha20-Poly1305 vault file. /// Recommended on headless / server hosts. @@ -76,17 +78,26 @@ impl SecretStore { /// Store `secret` under `(service, label)`, overwriting any prior value. /// - /// `password` selects the Tier-2 protection: `None` writes an - /// unprotected scheme-0 envelope; `Some(pw)` writes a scheme-1 envelope - /// sealed under the object password `pw` (Argon2id + XChaCha20-Poly1305) - /// **before** the bytes reach the backend, so a protected object stays - /// confidential even under a full backend compromise. A blank `pw` is - /// rejected ([`BlankPassphrase`](SecretStoreError::BlankPassphrase)). + /// `password` selects the protection: `None` writes an unprotected + /// envelope; `Some(pw)` seals the bytes under the object password `pw` + /// (Argon2id + XChaCha20-Poly1305) **before** they reach the backend, so + /// a protected object stays confidential even under a full backend + /// compromise. A blank `pw` is rejected + /// ([`BlankPassphrase`](SecretStoreError::BlankPassphrase)). /// - /// The write is an atomic same-slot overwrite on both arms (File: the - /// vault's atomic replace; Os: the keychain item), so add/change/remove - /// password flows — see [`reprotect`](SecretStore::reprotect) — leave - /// the prior value intact on a crash. + /// **No recovery (availability):** if a protected object's password is + /// lost, the object is permanently unrecoverable — there is no reset + /// path. The UX must state this plainly. + /// + /// **Entropy is the caller's:** a protected object's confidentiality + /// rests entirely on the password's entropy against an offline Argon2id + /// attacker who already holds the backend. This crate enforces only + /// non-blank; strength estimation / policy is the caller's job. + /// + /// The write is a same-slot overwrite that leaves the prior value intact + /// on a crash: on the `File` arm via the vault's atomic replace; on the + /// `Os` arm via the backend's single-item-replace contract. + /// Add/change/remove flows go through [`reprotect`](SecretStore::reprotect). pub fn set_secret( &self, service: &WalletId, @@ -162,23 +173,22 @@ impl SecretStore { } } - /// Retrieve the secret under `(service, label)` applying the Tier-2 - /// **strict, fail-closed** read (the L-1 keystone), or `Ok(None)` if - /// absent. + /// Retrieve the secret under `(service, label)` applying the strict, + /// fail-closed read, or `Ok(None)` if absent. /// /// `password` IS the caller's protection assertion — supply `Some(pw)` /// for an object the caller's trusted model says is protected, `None` /// otherwise. The expectation lives ONLY here, never in the stored /// blob (see [`envelope::unwrap`]): /// - /// - `Some(pw)` + valid scheme-1 → the secret (or + /// - `Some(pw)` + a protected blob → the secret (or /// [`WrongPassword`](SecretStoreError::WrongPassword) on tag fail); - /// - `Some(pw)` + a non-protected blob (scheme-0 / legacy raw) → + /// - `Some(pw)` + a non-protected blob (unprotected / legacy raw) → /// [`ExpectedProtectedButUnsealed`](SecretStoreError::ExpectedProtectedButUnsealed) - /// — a strip/downgrade, refused, no bytes returned ★; - /// - `None` + scheme-1 → + /// — a strip/downgrade, refused, no bytes returned; + /// - `None` + a protected blob → /// [`NeedsPassword`](SecretStoreError::NeedsPassword) (never ciphertext); - /// - `None` + scheme-0 / legacy raw → the secret. + /// - `None` + an unprotected / legacy raw blob → the secret. /// /// **Documented residual:** an attacker who ALSO rewrites the /// consumer's trusted DB so the caller passes `None` for a stripped @@ -200,7 +210,7 @@ impl SecretStore { } /// Add / change / remove an object password in one same-slot - /// unwrap→rewrap→overwrite — the canonical Tier-2 re-protection flow. + /// unwrap→rewrap→overwrite — the canonical re-protection flow. /// /// Reads the object under the `current` expectation (so a strip is /// caught fail-closed before any rewrap), then re-writes it under @@ -209,11 +219,19 @@ impl SecretStore { /// - **change:** `current = Some(old)`, `new = Some(pw_new)`; /// - **remove:** `current = Some(old)`, `new = None`. /// - /// An absent object is a no-op (`Ok(())`). The rewrite is the atomic - /// same-slot overwrite of [`set_secret`], so a crash between the read - /// and the commit leaves the prior value intact and readable under - /// `current`. After a successful call the consumer MUST update its own - /// trusted protection-status record (the L-1 expectation lives there). + /// An absent object is a no-op (`Ok(())`). The rewrite is the same-slot + /// overwrite of [`set_secret`], so a crash between the read and the + /// commit leaves the prior value intact and readable under `current`. + /// After a successful call the consumer MUST update its own trusted + /// protection-status record (the protection expectation lives there). + /// + /// **No recovery:** changing or removing requires the `current` + /// password; if it is lost the object cannot be re-protected or read, + /// and is permanently unrecoverable (availability trade-off). + /// + /// **Entropy is the caller's:** the `new` password's entropy is the + /// whole confidentiality guarantee for the re-protected object; this + /// crate enforces only non-blank, not strength. pub fn reprotect( &self, service: &WalletId, @@ -492,24 +510,24 @@ mod tests { } } - // ===== Tier-2 strict fail-closed read — the L-1 keystone ===== + // ===== Tier-2 strict fail-closed read ===== // // Parameterised over BOTH arms. The "attacker who can write the // backend" is modelled per arm by `Backend::place_raw`: on File it // re-seals the chosen blob under the resident vault key via `put_bytes` // (a cold/backup-swap actor could only corrupt → DoS, so the strip - // requires the vault key — §8.3 arm asymmetry); on Os it overwrites the - // keychain item directly (the bare envelope, no second AEAD — where the - // L-1 residual bites hardest, §8.3). GAP-005's writable Os fixture is - // the upstream `keyring_core::mock::Store` (a raw SPI `set_secret` - // bypasses the envelope), so no bespoke mock is needed. + // requires the vault key — the File-arm asymmetry); on Os it overwrites + // the keychain item directly (the bare envelope, no second AEAD — where + // the strip residual bites hardest). The writable Os fixture is the + // upstream `keyring_core::mock::Store` (a raw SPI `set_secret` bypasses + // the envelope), so no bespoke mock is needed. use keyring_core::mock; use crate::secrets::file::crypto::{KdfParams, ARGON2_MIN_M_KIB, ARGON2_MIN_T, ARGON2_P}; use crate::secrets::file::format::KDF_ID_ARGON2ID; - /// Argon2id floor params — fast enough for the keystone tests. + /// Argon2id floor params — fast enough for these tests. fn floor() -> KdfParams { KdfParams { id: KDF_ID_ARGON2ID, @@ -577,7 +595,7 @@ mod tests { } fn os_backend() -> Backend { - // GAP-005: the upstream in-memory mock store. The clone handed to + // The upstream in-memory mock store. The clone handed to // `SecretStore::Os` and the handle kept for raw attacker writes // share the same backing credentials by `Arc`. let mock = mock::Store::new().unwrap(); @@ -590,7 +608,7 @@ mod tests { } } - /// TS-L1-001: the strict-read QUADRANT. + /// The strict-read quadrant. fn run_quadrant(b: &Backend) { let w = wid(1); let pw = SecretString::new("object-pw"); @@ -643,7 +661,7 @@ mod tests { b.name ); - // ★ scheme-0 + Some(pw) → ExpectedProtectedButUnsealed (fail closed). + // scheme-0 + Some(pw) → ExpectedProtectedButUnsealed (fail closed). assert!( matches!( b.store.get_secret(&w, "u0", Some(&pw)).unwrap_err(), @@ -666,9 +684,8 @@ mod tests { b.name ); - // magic-less legacy raw + None → bytes: v5 §4.1 (legacy-tolerant) - // supersedes v4 TS-L1-001's Corruption row. + Some → fail closed, - // so L-1 is preserved. + // magic-less legacy raw + None → returns the bytes (legacy + // tolerance); + Some(pw) → fails closed, so the strict rule holds. b.place_raw(&w, "legacy", b"raw-legacy-seed-no-magic"); assert_eq!( b.store @@ -708,7 +725,7 @@ mod tests { run_quadrant(&os_backend()); } - /// TS-L1-002 ★ — the non-vacuous strip-injection regression. The single + /// The non-vacuous strip-injection regression. The single /// test the whole feature exists to make pass. fn run_strip_injection(b: &Backend) { let w = wid(2); @@ -736,7 +753,7 @@ mod tests { let attacker_blob = unprotected(&w, "seed", b"EVIL-SEED-S_evil"); b.place_raw(&w, "seed", &attacker_blob); - // ★ A password-supplied read of the stripped slot fails closed; + // A password-supplied read of the stripped slot fails closed; // S_evil is NEVER returned. let err = b.store.get_secret(&w, "seed", Some(&pw)).unwrap_err(); assert!( @@ -768,7 +785,7 @@ mod tests { run_strip_injection(&os_backend()); } - /// TS-L1-003: a DET bug alone fails closed in BOTH directions. + /// A consumer bug alone fails closed in BOTH directions. fn run_both_det_bug_directions(b: &Backend) { let w = wid(3); let pw = SecretString::new("pw"); @@ -796,7 +813,7 @@ mod tests { run_both_det_bug_directions(&os_backend()); } - /// TS-L1-004: the expectation is NEVER inferred from the blob's scheme + /// The expectation is NEVER inferred from the blob's scheme /// byte — identical scheme-1 blobs diverge solely on the password arg. fn run_expectation_not_inferred(b: &Backend) { let w = wid(4); @@ -828,7 +845,7 @@ mod tests { run_expectation_not_inferred(&os_backend()); } - /// TS-L1-005: unprotected→scheme-1 upgrade confusion is availability- + /// Unprotected→protected upgrade confusion is availability- /// only, fail-closed (NeedsPassword), no leak / no injection. fn run_upgrade_confusion(b: &Backend) { let w = wid(5); @@ -849,10 +866,10 @@ mod tests { run_upgrade_confusion(&os_backend()); } - /// TS-L1-006: an in-place scheme-byte flip (1→0). Some(pw) is caught by + /// An in-place scheme-byte flip (protected→unprotected). Some(pw) is caught by /// the strict rule regardless. None reads the body as scheme-0 opaque - /// bytes (never the real seed) — the GAP-010 residual, dominated by the - /// DET-DB residual; pinned, not "fixed". + /// bytes (never the real seed) — a known residual, dominated by the + /// consumer-DB residual; pinned, not "fixed". fn run_scheme_flip(b: &Backend) { let w = wid(6); let pw = SecretString::new("pw"); @@ -884,13 +901,13 @@ mod tests { run_scheme_flip(&os_backend()); } - // ===== Add / change / remove password + arm matrix (TS-PW / TS-ARM) ===== + // ===== Add / change / remove password + arm matrix ===== // // These exercise the PUBLIC set_secret/get_secret/reprotect API, so the // protected writes/reads run the real (default 64 MiB) Argon2 — kept to // a small number of derivations per test. - /// TS-PW-001/002/003: the full enrol → change → remove lifecycle, each + /// The full enrol → change → remove lifecycle, each /// step verified through the strict read. fn run_pw_lifecycle(b: &Backend) { let w = wid(10); @@ -955,7 +972,7 @@ mod tests { b.store.get_secret(&w, "seed", Some(&pw2)).unwrap_err(), SecretStoreError::ExpectedProtectedButUnsealed ), - "[{}] after remove, a password read fails closed until DET updates its DB", + "[{}] after remove, a password read fails closed until the consumer updates its DB", b.name ); } @@ -970,7 +987,7 @@ mod tests { run_pw_lifecycle(&os_backend()); } - /// TS-PW-005: losing the object password bricks the object — no recovery + /// Losing the object password bricks the object — no recovery /// path exists, every read fails closed. fn run_pw_no_recovery(b: &Backend) { let w = wid(11); @@ -1000,7 +1017,7 @@ mod tests { run_pw_no_recovery(&os_backend()); } - /// TS-ARM-003: `set`/`get` are additive `..,None` wrappers — `set` + /// `set`/`get` are additive `..,None` wrappers — `set` /// writes a scheme-0 envelope, `get` reads it byte-exact, and a /// password-supplied read of that unprotected object fails closed. fn run_set_get_wrappers(b: &Backend) { @@ -1030,7 +1047,7 @@ mod tests { run_set_get_wrappers(&os_backend()); } - /// TS-T1-005: the Os arm has no passphrase concept; the Tier-1 blank + /// The Os arm has no passphrase concept; the Tier-1 blank /// guard never fires and the round-trip is byte-exact. #[test] fn os_arm_roundtrip_no_blank_guard() { @@ -1047,7 +1064,7 @@ mod tests { assert!(b.store.get(&w, "seed").unwrap().is_none()); } - /// TS-PW-004 ★ [File]: a crash (disk-write failure) between the unwrap + /// [File]: a crash (disk-write failure) between the unwrap /// and the overwrite-commit leaves the OLD protected value intact and /// readable — no half-rotated / unprotected state. #[cfg(unix)] From b5ede7df8dffe24d7de77d4e13d5d715457c741b Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:32:11 +0200 Subject: [PATCH 09/10] =?UTF-8?q?test(platform-wallet-storage):=20QA=20fix?= =?UTF-8?q?es=20=E2=80=94=20Os=20read=20bound,=20Os=20crash=20test,=20test?= =?UTF-8?q?=20gaps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated QA-wave fix batch, part 2 (defense-in-depth + test coverage). - Os-arm read bound: `get_raw` rejects a backend blob larger than `MAX_SECRET_LEN + MAX_ENVELOPE_OVERHEAD` BEFORE it reaches the envelope parse/derive path (`SecretTooLarge`), mirroring the File arm whose stored bytes are already capped by `put_bytes`. The Os backend has no such ceiling; a legitimate envelope never approaches the cap. - Os crash test: a backend failure during the rewrite's write (after the read succeeds) leaves the OLD value intact — no half-rotation. Uses the upstream mock's one-shot error injection, with reprotect's read/write split so the failure lands on the write. Test gaps closed: - `SecretString::new` source-wipe: verifies the `String::zeroize` primitive `new` applies + faithful content copy (the freed-source scan would be use-after-free and the crate forbids `unsafe`). - scheme-1 accepts a plaintext at EXACTLY `MAX_PLAINTEXT_LEN` (the accept boundary) and round-trips; enveloped bytes still fit the backend cap. - value-rollback non-defence pinned: an older valid envelope still decrypts cleanly under the current password (so nobody mistakes the strict read for rollback protection). - the default-on export test references the re-exported `MAX_PLAINTEXT_LEN` and `MIN_PASSPHRASE_LEN`. - a `SecretStore::set`-path variant of the no-plaintext-in-vault test. Skipped (optional, with rationale): extracting the AAD length-prefix idiom into a shared helper — it spans `format.rs` (outside this feature's churn) and the envelope module for a 2-line idiom; the coupling outweighs the win. 147 secrets unit + secrets integration tests green; clippy --lib --tests clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/envelope.rs | 36 ++++++++++ .../src/secrets/file/mod.rs | 24 +++++++ .../src/secrets/secret.rs | 17 +++++ .../src/secrets/store.rs | 68 ++++++++++++++++++- .../tests/secrets_default_on_compiles.rs | 5 +- 5 files changed, 147 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs index efb271e0a2..725d76eccf 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/envelope.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/envelope.rs @@ -625,6 +625,42 @@ mod tests { assert!(enveloped.len() <= MAX_SECRET_LEN); } + /// Scheme-1 accepts a plaintext of EXACTLY `MAX_PLAINTEXT_LEN` (the + /// accept boundary), round-trips it, and the enveloped bytes still fit + /// the backend's `MAX_SECRET_LEN` cap. + #[test] + fn scheme1_accepts_plaintext_at_exact_cap() { + let p = pw("pw"); + let pt = vec![0x5Au8; MAX_PLAINTEXT_LEN]; + let blob = wrap_with_params(&wid(1), "seed", Some(&p), &pt, floor()).unwrap(); + assert!( + blob.len() <= MAX_SECRET_LEN, + "enveloped bytes exceed backend cap" + ); + let got = unwrap(&wid(1), "seed", Some(&p), blob.expose_secret()).unwrap(); + assert_eq!(got.expose_secret(), &pt[..]); + } + + /// Value rollback is intentionally NOT defended: an older valid scheme-1 + /// envelope still decrypts cleanly under the current password. Pinned so + /// a future reader does not mistake the strict read for rollback + /// protection (anti-rollback would need a monotonic anchor in the + /// consumer's integrity-protected metadata). + #[test] + fn value_rollback_is_not_defended() { + let p = pw("pw"); + let old_blob = wrap_with_params(&wid(1), "seed", Some(&p), b"OLD-VALUE", floor()).unwrap(); + // A newer value is written under the same identity + password … + let _new_blob = wrap_with_params(&wid(1), "seed", Some(&p), b"NEW-VALUE", floor()).unwrap(); + // … yet "restoring" the OLD envelope still decrypts cleanly. + let restored = unwrap(&wid(1), "seed", Some(&p), old_blob.expose_secret()).unwrap(); + assert_eq!( + restored.expose_secret(), + b"OLD-VALUE", + "older envelope still decrypts: value rollback is a known, undefended residual" + ); + } + /// magic/version discrimination: a magic-less blob is a legacy raw /// value — returned on a `None` read (with a one-time warning), refused /// fail-closed on `Some(pw)` so the strict rule holds. A magic-present 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 1d2471e780..8131f2643b 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/mod.rs @@ -1475,6 +1475,30 @@ mod tests { ); } + /// The no-plaintext-at-rest guarantee also holds through the public + /// `SecretStore::set` path (which writes an unprotected envelope sealed + /// under the vault key), not just the raw SPI entry path. + #[test] + fn no_plaintext_in_vault_file_via_secret_store_set() { + use crate::secrets::SecretStore; + let dir = tempfile::tempdir().unwrap(); + let path = vault_path(dir.path()); + let store = SecretStore::file(&path, SecretString::new("pw-correct")).unwrap(); + store + .set( + &wid(1), + "seed", + &SecretBytes::from_slice(b"PLAINTEXTNEEDLE"), + ) + .unwrap(); + let raw = fs::read(&path).unwrap(); + assert!( + raw.windows(b"PLAINTEXTNEEDLE".len()) + .all(|w| w != b"PLAINTEXTNEEDLE"), + "plaintext leaked into vault file via SecretStore::set" + ); + } + /// A blank passphrase is rejected at `open` → /// `BlankPassphrase`; no vault file (or lock sidecar) is created. #[test] diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index 9363c57584..2fa8213a74 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/secret.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -369,6 +369,23 @@ mod tests { assert_eq!(s.trimmed().expose_secret(), "abandon ability"); } + /// `SecretString::new` zeroizes its `String` source before that source + /// drops. The freed source buffer cannot be scanned after `new` returns + /// without use-after-free, and this crate forbids `unsafe`, so this + /// verifies the exact primitive `new` applies to its moved-in source — + /// `String::zeroize` empties the buffer in place — plus that `new` + /// faithfully copies the content into the wrapper. + #[test] + fn secret_string_new_zeroizes_string_source() { + // The primitive `new` calls on its owned source `String`. + let mut source = String::from("super secret seed material"); + source.zeroize(); + assert!(source.is_empty(), "String::zeroize must empty the source"); + // And construction copies the content into the (zeroizing) wrapper. + let s = SecretString::new(String::from("super secret seed material")); + assert_eq!(s.expose_secret(), "super secret seed material"); + } + #[test] fn secret_string_ct_eq_is_value_based() { // Equality goes through `ConstantTimeEq` only. diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index b545f2f26a..059979aacb 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -16,7 +16,7 @@ use super::envelope; use super::error::{OsKeyringErrorKind, SecretStoreError}; use super::secret::{SecretBytes, SecretString}; use super::validate::WalletId; -use super::{default_credential_store, EncryptedFileStore, SERVICE_PREFIX}; +use super::{default_credential_store, EncryptedFileStore, MAX_SECRET_LEN, SERVICE_PREFIX}; /// A passphrase-or-OS-keyring backed store for wallet secret material. /// @@ -165,7 +165,23 @@ impl SecretStore { Self::Os(store) => { let entry = build_os(store, service, label)?; match entry.get_secret() { - Ok(v) => Ok(Some(SecretBytes::new(v))), + Ok(v) => { + // Defense-in-depth: reject an oversized backend blob + // before it reaches the envelope parse/derive path. + // The File arm's stored bytes are already capped at + // MAX_SECRET_LEN by `put_bytes`; the Os backend has no + // such ceiling, so cap here. A legitimate envelope + // never exceeds MAX_SECRET_LEN; the overhead is + // headroom. + let cap = MAX_SECRET_LEN + envelope::MAX_ENVELOPE_OVERHEAD; + if v.len() > cap { + return Err(SecretStoreError::SecretTooLarge { + found: v.len(), + max: cap, + }); + } + Ok(Some(SecretBytes::new(v))) + } Err(KeyringError::NoEntry) => Ok(None), Err(e) => Err(map_spi(e)), } @@ -1104,4 +1120,52 @@ mod tests { SecretStoreError::WrongPassword )); } + + /// [Os]: a backend failure during the rewrite's write (after the read + /// succeeds) leaves the OLD value intact — no half-rotation. The mock's + /// one-shot error injection fails the next write, simulating a crash + /// mid-rewrite. `reprotect` is read-then-`set_secret`, split here so the + /// error lands on the write. + #[test] + fn os_rewrite_mid_write_failure_leaves_old_intact() { + let mock = mock::Store::new().unwrap(); + let store = SecretStore::Os(mock.clone()); + let w = wid(15); + let old = SecretString::new("old-pw"); + let new = SecretString::new("new-pw"); + store + .set_secret(&w, "seed", &SecretBytes::from_slice(b"REAL"), Some(&old)) + .unwrap(); + + // Read succeeds (the rewrite's first step) … + let secret = store.get_secret(&w, "seed", Some(&old)).unwrap().unwrap(); + // … then inject a one-shot backend error so the write fails. + let service = format!("{SERVICE_PREFIX}{}", w.to_hex()); + let entry = mock.build(&service, "seed", None).unwrap(); + let cred: &mock::Cred = entry.as_any().downcast_ref().unwrap(); + cred.set_error(KeyringError::PlatformFailure(Box::new( + std::io::Error::other("simulated backend write failure"), + ))); + let err = store + .set_secret(&w, "seed", &secret, Some(&new)) + .unwrap_err(); + assert!( + matches!(err, SecretStoreError::OsKeyring { .. }), + "got {err:?}" + ); + + // The OLD value is still readable; nothing rotated to `new`. + assert_eq!( + store + .get_secret(&w, "seed", Some(&old)) + .unwrap() + .unwrap() + .expose_secret(), + b"REAL" + ); + assert!(matches!( + store.get_secret(&w, "seed", Some(&new)).unwrap_err(), + SecretStoreError::WrongPassword + )); + } } 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 index 23cc10d582..1f45e2ceae 100644 --- a/packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs +++ b/packages/rs-platform-wallet-storage/tests/secrets_default_on_compiles.rs @@ -9,7 +9,7 @@ use platform_wallet_storage::secrets::{ default_credential_store, EncryptedFileStore, SecretBytes, SecretStoreError, SecretString, - WalletId, SERVICE_PREFIX, + WalletId, MAX_PLAINTEXT_LEN, MIN_PASSPHRASE_LEN, SERVICE_PREFIX, }; #[test] @@ -23,6 +23,9 @@ fn default_build_exposes_secrets_surface() { } let _ = _accepts_path as fn(_, _) -> _; let _ = SERVICE_PREFIX.len(); + // The Tier-2 public consts are re-exported on the default build. + let _ = MAX_PLAINTEXT_LEN; + let _ = MIN_PASSPHRASE_LEN; let _ = std::mem::size_of::(); let _ = std::mem::size_of::(); let _ = std::mem::size_of::(); From fb7953eabb34e24d13f0044e11fd8719ec4bb8ed Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:42:11 +0200 Subject: [PATCH 10/10] test(platform-wallet-storage): cover Os read-size guard; pin new() source-wipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final QA close-out (two LOWs). - QA-010: the Os-arm read-size guard in `get_raw` had no test. Add `os_read_rejects_oversized_blob` — via the mock backend, place a raw blob one byte over `MAX_SECRET_LEN + MAX_ENVELOPE_OVERHEAD` into an Os slot and assert both `get_secret` and the legacy `get` return `SecretTooLarge { found, max }` with `max == MAX_SECRET_LEN + MAX_ENVELOPE_OVERHEAD`. - QA-001: the source-wipe test is a proxy (it can't assert `new()` invokes `source.zeroize()` under `#![deny(unsafe_code)]`), so pin the call site with a do-not-remove comment at `secret.rs` `source.zeroize();` and reword the test doc to state what it actually checks (the `String::zeroize` primitive + faithful copy), dropping the "exact primitive new applies" overclaim. 148 secrets unit + integration tests green; clippy --lib --tests clean; fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0157yd3YvWeyckhfQivS9gf7 --- .../src/secrets/secret.rs | 16 ++++++------- .../src/secrets/store.rs | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index 2fa8213a74..8e4d6dc4d8 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/secret.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/secret.rs @@ -59,6 +59,8 @@ impl SecretString { let cap = source.len().max(DEFAULT_CAPACITY); let mut buf = String::with_capacity(cap); buf.push_str(&source); + // Do not remove: wipes the moved-in plaintext source before it drops + // (its freed buffer cannot be scanned in a test under deny(unsafe_code)). source.zeroize(); let lock = region::lock(buf.as_ptr(), buf.capacity()) .map_err(|e| { @@ -369,19 +371,17 @@ mod tests { assert_eq!(s.trimmed().expose_secret(), "abandon ability"); } - /// `SecretString::new` zeroizes its `String` source before that source - /// drops. The freed source buffer cannot be scanned after `new` returns - /// without use-after-free, and this crate forbids `unsafe`, so this - /// verifies the exact primitive `new` applies to its moved-in source — - /// `String::zeroize` empties the buffer in place — plus that `new` - /// faithfully copies the content into the wrapper. + /// Two sound checks (a direct freed-buffer scan would be use-after-free, + /// and this crate forbids `unsafe`): (1) `String::zeroize` empties a + /// buffer — the primitive `new` relies on; (2) `new` copies the content + /// into the wrapper faithfully. That `new` actually calls + /// `source.zeroize()` on its moved-in source is pinned by the + /// do-not-remove comment at that call site, not asserted here. #[test] fn secret_string_new_zeroizes_string_source() { - // The primitive `new` calls on its owned source `String`. let mut source = String::from("super secret seed material"); source.zeroize(); assert!(source.is_empty(), "String::zeroize must empty the source"); - // And construction copies the content into the (zeroizing) wrapper. let s = SecretString::new(String::from("super secret seed material")); assert_eq!(s.expose_secret(), "super secret seed material"); } diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index 059979aacb..0ff9e393a9 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -1168,4 +1168,28 @@ mod tests { SecretStoreError::WrongPassword )); } + + /// [Os]: the read-size guard rejects an oversized backend blob (a + /// malicious keychain returning more than a legitimate envelope ever + /// could) BEFORE it reaches the envelope parse/derive path. The bound is + /// `MAX_SECRET_LEN + MAX_ENVELOPE_OVERHEAD`; both the `get_secret` and + /// legacy `get` read paths enforce it. + #[test] + fn os_read_rejects_oversized_blob() { + let b = os_backend(); + let w = wid(16); + let cap = MAX_SECRET_LEN + envelope::MAX_ENVELOPE_OVERHEAD; + // Attacker writes a blob one byte over the cap straight to the slot. + b.place_raw(&w, "seed", &vec![0u8; cap + 1]); + let err = b.store.get_secret(&w, "seed", None).unwrap_err(); + assert!( + matches!(err, SecretStoreError::SecretTooLarge { found, max } if found == cap + 1 && max == cap), + "get_secret got {err:?}" + ); + // The legacy `get` path is bounded too. + assert!(matches!( + b.store.get(&w, "seed").unwrap_err(), + SecretStoreError::SecretTooLarge { found, max } if found == cap + 1 && max == cap + )); + } }