diff --git a/Cargo.lock b/Cargo.lock index 402c2d9295..ca8c6f0e42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5229,6 +5229,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..3678307816 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 @@ -194,6 +199,10 @@ secrets = [ # the feature list (not a `default-features = false` rewrite) so # argon2's own default features stay intact. "argon2/zeroize", + # bincode is the producer for the Tier-2 envelope wire format and the + # three AAD encodings (`Tier2Aad`/`EntryAad`/`VerifyAad`) — see + # `secrets/wire/`. `=2.0.1` is the workspace-wide pin. + "dep:bincode", "dep:chacha20poly1305", # secrets uses serde directly (vault format + crypto envelope derive # `Serialize`/`Deserialize`); declare the dep here so @@ -213,6 +222,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/SCHEMA.md b/packages/rs-platform-wallet-storage/SCHEMA.md index c05725583f..a47dcec901 100644 --- a/packages/rs-platform-wallet-storage/SCHEMA.md +++ b/packages/rs-platform-wallet-storage/SCHEMA.md @@ -11,9 +11,9 @@ chain. ## What it stores — and the boundary The persister stores **public** wallet-state material (UTXOs, transactions, -account registrations, address pools, identities, identity public keys, -contacts, asset locks, token balances, DashPay overlays, and -platform-address sync snapshots) in a SQLite database managed by +account registrations, identities, identity public keys, contacts, asset +locks, token balances, DashPay overlays, and platform-address sync +snapshots) in a SQLite database managed by [refinery](https://crates.io/crates/refinery) migrations. **No secrets are stored here.** Mnemonics, seeds, and raw private keys never @@ -37,20 +37,18 @@ Any `meta_*` row whose parent object does not exist — because it was never cre A future garbage-collection pass is expected to reap orphan metadata — rows with no live parent object older than approximately one week — but no such GC is implemented yet. Callers should not rely on orphan metadata persisting forever, nor assume it will be cleaned up promptly. `meta_global` is intentionally parentless and always survives. -The 23 tables are split into five domain diagrams below. `WALLETS` is the root anchor and appears in each diagram. For full column listings see the [Tables](#tables) section. +The 21 tables are split into five domain diagrams below. `WALLETS` is the root anchor and appears in each diagram. For full column listings see the [Tables](#tables) section. ## Diagram 1 — Core / L1 (Bitcoin/Dash layer) -Account registrations, address-pool snapshots, transactions, UTXOs, instant locks, derived addresses, and SPV sync state. +Account registrations, transactions, UTXOs, instant locks, and SPV sync state. ```mermaid erDiagram WALLETS ||--o{ ACCOUNT_REGISTRATIONS : "registers" - WALLETS ||--o{ ACCOUNT_ADDRESS_POOLS : "snapshots" WALLETS ||--o{ CORE_TRANSACTIONS : "records" WALLETS ||--o{ CORE_UTXOS : "owns" WALLETS ||--o{ CORE_INSTANT_LOCKS : "holds" - WALLETS ||--o{ CORE_DERIVED_ADDRESSES : "derives" WALLETS ||--o| CORE_SYNC_STATE : "tracks" CORE_TRANSACTIONS ||--o{ CORE_UTXOS : "spends" @@ -62,19 +60,11 @@ erDiagram ACCOUNT_REGISTRATIONS { BLOB wallet_id PK - TEXT account_type PK "standard | coinjoin | identity_registration | ..." + TEXT account_type PK "standard_bip44 | standard_bip32 | coinjoin | identity_registration | ..." INTEGER account_index PK BLOB account_xpub_bytes "bincode-encoded AccountRegistrationEntry" } - ACCOUNT_ADDRESS_POOLS { - BLOB wallet_id PK - TEXT account_type PK - INTEGER account_index PK - TEXT pool_type PK "external | internal | absent | absent_hardened" - BLOB snapshot_blob "bincode-encoded AccountAddressPoolEntry" - } - CORE_TRANSACTIONS { BLOB wallet_id PK BLOB txid PK "32-byte Txid" @@ -102,15 +92,6 @@ erDiagram BLOB islock_blob "bincode-encoded InstantLock" } - CORE_DERIVED_ADDRESSES { - BLOB wallet_id PK - TEXT account_type PK - TEXT address PK "bech32 / Base58 address string" - INTEGER account_index - TEXT derivation_path "pool_type/derivation_index" - INTEGER used "0 | 1" - } - CORE_SYNC_STATE { BLOB wallet_id PK "one row per wallet" INTEGER last_processed_height "NULL until first block processed" @@ -364,14 +345,6 @@ lookups without blob decoding. - PK: `(wallet_id, account_type, account_index)`. - FK: `wallet_id → wallets(wallet_id) ON DELETE CASCADE`. -### `account_address_pools` - -Address-pool snapshot per `(wallet, account, pool_type)`. `pool_type` is -one of `external`, `internal`, `absent`, `absent_hardened`. - -- PK: `(wallet_id, account_type, account_index, pool_type)`. -- FK: `wallet_id → wallets(wallet_id) ON DELETE CASCADE`. - ### `core_transactions` One row per transaction the wallet has seen. `height`, `block_hash`, and @@ -401,15 +374,6 @@ finalized. Rows are removed when the transaction becomes confirmed. - PK: `(wallet_id, txid)`. - FK: `wallet_id → wallets(wallet_id) ON DELETE CASCADE`. -### `core_derived_addresses` - -Address-to-account-index map. Written before UTXOs in the same -transaction so the UTXO writer can resolve `account_index` by address. - -- PK: `(wallet_id, account_type, address)`. -- FK: `wallet_id → wallets(wallet_id) ON DELETE CASCADE`. -- Index: `idx_core_derived_addresses_addr(wallet_id, address)`. - ### `core_sync_state` One row per wallet, holding monotonically-advancing SPV sync watermarks. @@ -590,27 +554,24 @@ before the address exists. ## Enum-domain CHECK constraints -Seven TEXT columns carry a `CHECK (col IN (...))` across five enum -domains — `account_type` is reused in three tables. The IN-list is built -at migration time from `pub(crate) const *_LABELS` arrays declared next -to each writer function. Four domains mirror an upstream Rust enum; the -fifth (`contacts.state`) is a synthetic lifecycle label naming which -`ContactChangeSet` slot a row came from: +Four TEXT columns carry a `CHECK (col IN (...))` across four enum +domains. The IN-list is built at migration time from +`pub(crate) const *_LABELS` arrays declared next to each writer function. +Three domains mirror an upstream Rust enum; the fourth (`contacts.state`) +is a synthetic lifecycle label naming which `ContactChangeSet` slot a row +came from: | Table | Column | Source-of-truth const | |---|---|---| | `wallets` | `network` | `sqlite::schema::wallets::NETWORK_LABELS` | | `account_registrations` | `account_type` | `sqlite::schema::accounts::ACCOUNT_TYPE_LABELS` | -| `account_address_pools` | `account_type` | `sqlite::schema::accounts::ACCOUNT_TYPE_LABELS` | -| `account_address_pools` | `pool_type` | `sqlite::schema::accounts::POOL_TYPE_LABELS` | -| `core_derived_addresses` | `account_type` | `sqlite::schema::accounts::ACCOUNT_TYPE_LABELS` | | `asset_locks` | `status` | `sqlite::schema::asset_locks::ASSET_LOCK_STATUS_LABELS` | | `contacts` | `state` | `sqlite::schema::contacts::CONTACT_STATE_LABELS` | The const arrays are the single source of truth shared by the writer mapping functions (`network_to_str`, `account_type_db_label`, -`pool_type_db_label`, `status_str`, `contact_state_db_label`) and the -migration's CHECK clauses. +`status_str`, `contact_state_db_label`) and the migration's CHECK +clauses. Per-module `*_labels_match_enum` unit tests enforce set-equality between each const and the writer's codomain — drift (a renamed/added upstream variant) fails the test rather than landing as silent garbage @@ -619,10 +580,9 @@ in this document; the source files are canonical. ### Upstream-enum coupling -Three of the persisted enums live in the external `rust-dashcore` -crate (`key_wallet::Network`, `key_wallet::account::AccountType`, -`key_wallet::managed_account::address_pool::AddressPoolType`); the -fourth (`platform_wallet::wallet::asset_lock::tracked::AssetLockStatus`) +Two of the persisted enums live in the external `rust-dashcore` +crate (`key_wallet::Network`, `key_wallet::account::AccountType`); the +third (`platform_wallet::wallet::asset_lock::tracked::AssetLockStatus`) is in-tree and carries a `# Schema coupling` rustdoc block. Because the upstream definitions cannot be edited from this repository, @@ -639,7 +599,7 @@ mechanisms working together: TODO(rust-dashcore): once the upstream `key_wallet` crate is vendored or the project gains push access there, mirror the in-tree -`AssetLockStatus` `# Schema coupling` doc block on the three upstream +`AssetLockStatus` `# Schema coupling` doc block on the two upstream enums so a developer editing them upstream sees the constraint without having to grep this repo. @@ -683,4 +643,4 @@ having to grep this repo. | Version | File | Description | |---|---|---| -| V001 | `V001__initial.rs` | Full schema: all 23 tables (including the six `meta_*` per-object metadata tables), every index, and six triggers (`setnull_core_utxos_on_tx_delete` + the five `meta_*` soft-cascade triggers) | +| V001 | `V001__initial.rs` | Full schema: all 21 tables (including the six `meta_*` per-object metadata tables), every index, and six triggers (`setnull_core_utxos_on_tx_delete` + the five `meta_*` soft-cascade triggers) | diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index 8a1c7fcc39..9f1361702b 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,202 @@ 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. + +The canonical wire format is **bincode-encoded** under a single +`WIRE_CONFIG = standard().with_big_endian().with_no_limit()` against +two `pub(crate)` types whose shapes are the source of truth — see +[`src/secrets/wire/envelope.rs`](src/secrets/wire/envelope.rs) and +[`src/secrets/wire/mod.rs`](src/secrets/wire/mod.rs): + +```rust +struct Envelope { version: u32, payload: Payload } +enum Payload { + Unprotected(Vec), // scheme 0 + Password { // scheme 1 + kdf: KdfParamsEncoded, // id u8 ‖ m_kib u32 ‖ t u32 ‖ p u32 + salt: [u8; 32], nonce: [u8; 24], + ciphertext: Vec, // includes the 16-byte Poly1305 tag + }, +} +``` + +`ENVELOPE_VERSION = 1` is bumped only on a breaking layout change, +independent of the vault `FORMAT_VERSION`. Decoding goes through a +budget-limited `DECODE_CONFIG = WIRE_CONFIG.with_limit::()` so a +hostile blob declaring a multi-GiB length prefix is rejected before +allocation (security-positive deviation from the no-limit encoder +config). Trailing bytes after a valid decode are also refused — +`consumed == blob.len()` is a fail-closed invariant. + +- **AAD (scheme 1)** is bincode-encoded from `Tier2Aad` + ([`src/secrets/wire/aad.rs`](src/secrets/wire/aad.rs)), which binds + `domain (PWSEV-TIER2-AAD-v2) ‖ envelope_version ‖ scheme_discriminant + ‖ kdf ‖ salt ‖ wallet_id ‖ label`. The vault's own per-entry AAD goes + through `EntryAad` (`domain (PWSV-ENTRY-AAD-v2) ‖ format_version ‖ + wallet_id ‖ label`) and the vault verify-token AAD through `VerifyAad` + (`domain (PWSV-VERIFY-AAD-v2) ‖ format_version ‖ salt ‖ kdf`). All + three domain tags are pair-wise byte-disjoint by construction. 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 — both the wider + `enforce_bounds` (algorithm id + floors/ceilings) AND a tighter + per-read gate that refuses any `m_kib > default_target().m_kib` OR + `t > default_target().t`. A forged header cannot inflate memory by + more than the shipped default or CPU by more than the shipped + iteration count. +- **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`), 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 envelope version** → `UnsupportedEnvelopeVersion` — fail + closed **regardless of the password**: an envelope tagged for a + future layout can be neither safely unwrapped nor treated as + unprotected. +- **Unparseable bytes / unknown scheme tag / trailing garbage** → + `Corruption`. There is no magic-byte peek — every blob runs through + the bincode decoder, and anything that does not round-trip cleanly + with `consumed == blob.len()` fails closed. + +#### 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 +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)`** | **valid scheme-0 envelope** | **`ExpectedProtectedButUnsealed` — FAIL CLOSED** | +| `Some(pw)` | scheme-1 but truncated/corrupt | `Corruption` | +| `Some/None` | unknown envelope version | `UnsupportedEnvelopeVersion` | +| `Some/None` | unparseable / non-envelope bytes / trailing garbage | `Corruption` | +| `None` | valid scheme-1 | `NeedsPassword` (never ciphertext) | +| `None` | valid scheme-0 envelope | the secret | +| any | absent entry | `Ok(None)` (deletion = DoS, never injection) | + +The load-bearing row is **`Some(pw)` + scheme-0 envelope ⇒ +`ExpectedProtectedButUnsealed`**: with a password in hand, an +unprotected envelope can only mean a strip, so it is refused and **no +bytes are returned**. 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 +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. 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. An absent object returns +`Err(SecretStoreError::NoEntry)` — `reprotect` is operational, so absence +means the caller's protection-status record disagrees with the backend and +must not be silently dropped. 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 + +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 only — no legacy tolerance + +The envelope is the only on-disk Tier-2 format this build understands. +A decrypted entry that does not bincode-decode to a valid `Envelope` +under `WIRE_CONFIG` (including trailing-byte extension probes) surfaces +as `Corruption` on every read — there is no magic-byte peek and no +magic-less raw legacy path. The shipped wire layer is the source of +truth; older non-enveloped stored values are out of scope. + ### Internal SPI Below `SecretStore`, `EncryptedFileStore` and `default_credential_store` @@ -138,7 +348,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 +407,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 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 @@ -198,15 +430,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/migrations/V001__initial.rs b/packages/rs-platform-wallet-storage/migrations/V001__initial.rs index 7caaaf2519..30a0d8b851 100644 --- a/packages/rs-platform-wallet-storage/migrations/V001__initial.rs +++ b/packages/rs-platform-wallet-storage/migrations/V001__initial.rs @@ -29,8 +29,8 @@ //! read back) at every connection open via `open_conn` //! (`src/sqlite/conn.rs`). //! -//! Enum-shaped TEXT columns (`network`, `account_type`, `pool_type`, -//! `status`, `state`) carry a `CHECK (col IN (...))` clause whose +//! Enum-shaped TEXT columns (`network`, `account_type`, `status`, +//! `state`) carry a `CHECK (col IN (...))` clause whose //! IN-list is built from the `*_LABELS` const arrays in //! `crate::sqlite::schema::{wallets, accounts, asset_locks, //! contacts}`. The consts are the single source of truth shared with @@ -51,7 +51,6 @@ pub fn migration() -> String { let network_check = build_check_in(crate::sqlite::schema::wallets::NETWORK_LABELS); let account_type_check = build_check_in(crate::sqlite::schema::accounts::ACCOUNT_TYPE_LABELS); - let pool_type_check = build_check_in(crate::sqlite::schema::accounts::POOL_TYPE_LABELS); let asset_lock_status_check = build_check_in(crate::sqlite::schema::asset_locks::ASSET_LOCK_STATUS_LABELS); let contact_state_check = @@ -82,16 +81,6 @@ CREATE TABLE account_registrations ( FOREIGN KEY (wallet_id) REFERENCES wallets(wallet_id) ON DELETE CASCADE ); -CREATE TABLE account_address_pools ( - wallet_id BLOB NOT NULL, - account_type TEXT NOT NULL CHECK (account_type IN {account_type_check}), - account_index INTEGER NOT NULL, - pool_type TEXT NOT NULL CHECK (pool_type IN {pool_type_check}), - snapshot_blob BLOB NOT NULL, - PRIMARY KEY (wallet_id, account_type, account_index, pool_type), - FOREIGN KEY (wallet_id) REFERENCES wallets(wallet_id) ON DELETE CASCADE -); - CREATE TABLE core_transactions ( wallet_id BLOB NOT NULL, txid BLOB NOT NULL, @@ -143,19 +132,6 @@ CREATE TABLE core_instant_locks ( FOREIGN KEY (wallet_id) REFERENCES wallets(wallet_id) ON DELETE CASCADE ); -CREATE TABLE core_derived_addresses ( - wallet_id BLOB NOT NULL, - account_type TEXT NOT NULL CHECK (account_type IN {account_type_check}), - account_index INTEGER NOT NULL, - address TEXT NOT NULL, - derivation_path TEXT NOT NULL, - used INTEGER NOT NULL, - PRIMARY KEY (wallet_id, account_type, address), - FOREIGN KEY (wallet_id) REFERENCES wallets(wallet_id) ON DELETE CASCADE -); - -CREATE INDEX idx_core_derived_addresses_addr ON core_derived_addresses(wallet_id, address); - CREATE TABLE core_sync_state ( wallet_id BLOB NOT NULL PRIMARY KEY, last_processed_height INTEGER, diff --git a/packages/rs-platform-wallet-storage/src/secrets/error.rs b/packages/rs-platform-wallet-storage/src/secrets/error.rs index 94e7375e1b..f735183c7a 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/error.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/error.rs @@ -20,6 +20,41 @@ pub enum SecretStoreError { #[error("wrong passphrase")] WrongPassphrase, + /// 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), 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; for a deliberately keyless file vault use open_unprotected" + )] + 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 +74,20 @@ pub enum SecretStoreError { found: u32, }, + /// A Tier-2 secret envelope decoded with a `version` 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. + /// 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. + found: u8, + }, + /// The vault file was malformed (bad magic, truncated header, bad /// record framing) — no plaintext was produced. #[error("malformed vault file")] @@ -49,6 +98,17 @@ pub enum SecretStoreError { #[error("invalid label")] InvalidLabel, + /// No credential exists under `(service, label)` on either arm. Returned + /// by mutators that need an entry to operate on (e.g. [`reprotect`]) so + /// absence is a signal, not a silent no-op — caller's protection-status + /// record disagreeing with the backend must not be swallowed. Surfaced + /// by the file arm when `delete_bytes` reports `Ok(false)` and by the + /// OS arm when [`keyring_core::Error::NoEntry`] bubbles out. + /// + /// [`reprotect`]: crate::secrets::SecretStore::reprotect + #[error("no entry under (service, label)")] + NoEntry, + /// A pre-existing vault file had permissions looser than `0600`. /// Refuse rather than tighten-and-trust. #[error("vault file has insecure permissions")] @@ -186,8 +246,6 @@ impl From for IoError { /// [`SecretStore::Os`]: crate::secrets::SecretStore::Os #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum OsKeyringErrorKind { - /// `keyring_core::Error::NoEntry`. - NoEntry, /// `keyring_core::Error::NoStorageAccess` (store locked / inaccessible). NoStorageAccess, /// `keyring_core::Error::NoDefaultStore` (no reachable backend). @@ -203,7 +261,6 @@ pub enum OsKeyringErrorKind { impl std::fmt::Display for OsKeyringErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let s = match self { - Self::NoEntry => "no entry", Self::NoStorageAccess => "storage inaccessible", Self::NoDefaultStore => "no default store", Self::BadStoreFormat => "bad store format", @@ -231,11 +288,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. +/// - 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 +307,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 { .. } @@ -264,6 +340,7 @@ impl From for KeyringError { E::InvalidLabel => { KeyringError::Invalid("user".to_string(), "label allowlist violation".to_string()) } + E::NoEntry => KeyringError::NoEntry, E::Io(io) => KeyringError::PlatformFailure(Box::new(io.source)), } } @@ -386,6 +463,102 @@ mod tests { assert!(!format!("{k}").contains("plaintext")); } + /// 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"); + } + + /// 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; for a deliberately keyless file vault use open_unprotected" + ); + 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")); + } + } + + /// 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:?}"), + } + } + } + + /// `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 { diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs index 4bb0a350b6..bbfb6b2642 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/crypto.rs @@ -140,6 +140,33 @@ pub(crate) fn seal( Ok((nonce_bytes, ct)) } +/// Like [`seal`] but takes a caller-supplied `nonce` instead of pulling +/// from the CSPRNG. **Test-only** — golden-vector / size-budget tests +/// need byte-deterministic ciphertext output. Production code MUST use +/// [`seal`] so nonces stay unique (XChaCha20-Poly1305 nonce reuse leaks +/// the keystream). +#[cfg(test)] +pub(crate) fn seal_with_nonce( + key: &SecretBytes, + nonce_bytes: [u8; NONCE_LEN], + aad: &[u8], + plaintext: &[u8], +) -> Result<([u8; NONCE_LEN], Vec), SecretStoreError> { + let cipher = XChaCha20Poly1305::new_from_slice(key.expose_secret()) + .map_err(|_| SecretStoreError::Encrypt)?; + let nonce = XNonce::from_slice(&nonce_bytes); + let ct = cipher + .encrypt( + nonce, + chacha20poly1305::aead::Payload { + msg: plaintext, + aad, + }, + ) + .map_err(|_| SecretStoreError::Encrypt)?; + Ok((nonce_bytes, ct)) +} + /// Decrypt `ciphertext` under `key`/`nonce`/`aad`. On tag failure /// returns [`SecretStoreError::Decrypt`] and **no** plaintext — the /// combined (non-detached) API never materializes unverified bytes at diff --git a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs index bb6d90769e..c137afb243 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/file/format.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/file/format.rs @@ -37,6 +37,9 @@ use serde::{Deserialize, Serialize}; use super::crypto::{KdfParams, NONCE_LEN, SALT_LEN}; use crate::secrets::error::SecretStoreError; +use crate::secrets::wire::aad::{EntryAad, VerifyAad}; +use crate::secrets::wire::config::{ENTRY_DOMAIN_V2, VERIFY_DOMAIN_V2, WIRE_CONFIG}; +use crate::secrets::wire::kdf::KdfParamsEncoded; pub(crate) const FORMAT_VERSION: u32 = 1; pub(crate) const KDF_ID_ARGON2ID: u8 = 1; @@ -46,17 +49,6 @@ pub(crate) const KDF_ID_ARGON2ID: u8 = 1; /// value itself is not secret. pub(crate) const VERIFY_CONSTANT: &[u8] = b"PWSVAULT-VERIFY-v1"; -/// AAD slot label for the verification token. The leading NUL keeps it -/// disjoint from every allowlisted entry label, so the token can never -/// alias a real entry's AAD. -pub(crate) const VERIFY_LABEL: &str = "\0verify"; - -/// Sentinel wallet id for the verify-token AAD slot. Keeps the AAD shape -/// identical to entry AAD without aliasing a real wallet: even a real -/// `[0u8; 32]` id yields a different AAD because [`VERIFY_LABEL`] differs -/// from any allowlisted label. -const VERIFY_WALLET_ID: [u8; 32] = [0u8; 32]; - /// Minimum AEAD ciphertext length: the Poly1305 tag is always present /// even for an empty plaintext, so any `verify_ct`/`ciphertext` shorter /// than this is structurally impossible and rejected. @@ -96,43 +88,43 @@ pub(crate) struct EntryBody { pub ciphertext: Vec, } -/// Canonical length-prefixed AAD binding ciphertext to its slot: -/// `format_version ‖ wallet_id ‖ label`. A blob moved to another slot, or -/// a rolled-back `format_version`, fails the tag. +/// Canonical AAD binding a vault entry's ciphertext to its slot: +/// `domain ‖ format_version ‖ wallet_id ‖ label`, bincode-encoded +/// against [`WIRE_CONFIG`]. A blob moved to another slot, or one +/// version-rolled-back, fails the tag. /// /// Determinism invariant: AAD is built solely from this typed triple, /// never from serialized JSON bytes or key order. `format_version` is -/// always the compiled-in [`FORMAT_VERSION`]; the JSON `version` field is -/// a dispatch gate only and is never routed into AAD. +/// always the compiled-in [`FORMAT_VERSION`]; the JSON `version` field +/// is a dispatch gate only and is never routed into AAD. pub(crate) fn aad(format_version: u32, wallet_id: &[u8; 32], label: &str) -> Vec { - let lb = label.as_bytes(); - let mut v = Vec::with_capacity(4 + 4 + 32 + 4 + lb.len()); - v.extend_from_slice(&format_version.to_le_bytes()); - v.extend_from_slice(&(wallet_id.len() as u32).to_le_bytes()); - v.extend_from_slice(wallet_id); - v.extend_from_slice(&(lb.len() as u32).to_le_bytes()); - v.extend_from_slice(lb); - v + bincode::encode_to_vec( + EntryAad { + domain: ENTRY_DOMAIN_V2, + format_version, + wallet_id: *wallet_id, + label, + }, + WIRE_CONFIG, + ) + .expect("EntryAad encode is infallible") } -/// AAD for the verify-token. Reuses the entry-AAD construction (sentinel -/// wallet id + NUL-prefixed [`VERIFY_LABEL`], disjoint from any real -/// slot), then binds the KDF header: `salt` plus a length-prefixed LE -/// encoding of (`id`, `m_kib`, `t`, `p`). -/// -/// Folding the header in makes the token authenticate the salt + KDF -/// params it was derived under, so header tamper / KDF downgrade is -/// detected fail-closed (it surfaces as `WrongPassphrase` because a -/// tampered header also yields a different derived key). +/// AAD for the verify-token: bincode-encoded `VerifyAad` binding the +/// vault-wide salt + KDF header against the verify domain tag. A +/// tampered header yields a different AAD AND a different derived key, +/// so the token surfaces `WrongPassphrase`. pub(crate) fn verify_aad(format_version: u32, salt: &[u8; SALT_LEN], kdf: &KdfParams) -> Vec { - let mut v = aad(format_version, &VERIFY_WALLET_ID, VERIFY_LABEL); - v.extend_from_slice(&(salt.len() as u32).to_le_bytes()); - v.extend_from_slice(salt); - v.extend_from_slice(&[kdf.id]); - v.extend_from_slice(&kdf.m_kib.to_le_bytes()); - v.extend_from_slice(&kdf.t.to_le_bytes()); - v.extend_from_slice(&kdf.p.to_le_bytes()); - v + bincode::encode_to_vec( + VerifyAad { + domain: VERIFY_DOMAIN_V2, + format_version, + salt: *salt, + kdf: KdfParamsEncoded::from(*kdf), + }, + WIRE_CONFIG, + ) + .expect("VerifyAad encode is infallible") } /// Serde helpers encoding `Vec` as lowercase hex. Hex is already a @@ -249,70 +241,6 @@ pub(crate) fn deserialize(buf: &[u8]) -> Result { mod tests { use super::*; - #[test] - fn aad_binds_slot() { - let w = [1u8; 32]; - assert_ne!(aad(1, &w, "a"), aad(1, &w, "b")); - assert_ne!(aad(1, &w, "a"), aad(2, &w, "a")); - assert_ne!(aad(1, &w, "a"), aad(1, &[2u8; 32], "a")); - // Length-prefix defeats `"a"+"bc"` vs `"ab"+"c"` ambiguity. - assert_ne!(aad(1, &w, "ab"), { - let mut v = aad(1, &w, "a"); - v.extend_from_slice(b"b"); - v - }); - } - - #[test] - fn verify_aad_disjoint_from_every_entry_aad() { - // VERIFY_LABEL starts with NUL (allowlist-forbidden), so no real - // entry's AAD can collide — even on the all-zero wallet id. - let salt = [7u8; SALT_LEN]; - let kdf = KdfParams::default_target(); - let v = verify_aad(FORMAT_VERSION, &salt, &kdf); - assert_ne!(v, aad(FORMAT_VERSION, &VERIFY_WALLET_ID, "seed")); - assert_ne!(v, aad(FORMAT_VERSION, &[1u8; 32], "seed")); - } - - #[test] - fn verify_aad_binds_salt_and_kdf_params() { - // The verify-token AAD authenticates the salt + KDF header, so a - // flipped salt or an in-bounds KDF-param shift yields a different - // AAD (and hence a token-tag failure at verify). - let salt = [7u8; SALT_LEN]; - let kdf = KdfParams::default_target(); - let base = verify_aad(FORMAT_VERSION, &salt, &kdf); - - let mut salt2 = salt; - salt2[0] ^= 0x01; - assert_ne!(base, verify_aad(FORMAT_VERSION, &salt2, &kdf)); - - assert_ne!( - base, - verify_aad( - FORMAT_VERSION, - &salt, - &KdfParams { - m_kib: kdf.m_kib / 2, - ..kdf - } - ) - ); - assert_ne!( - base, - verify_aad( - FORMAT_VERSION, - &salt, - &KdfParams { - t: kdf.t - 1, - ..kdf - } - ) - ); - // Identical inputs are deterministic. - assert_eq!(base, verify_aad(FORMAT_VERSION, &salt, &kdf)); - } - fn test_vault(wallets: BTreeMap>) -> Vault { Vault { version: FORMAT_VERSION, 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..8131f2643b 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; @@ -52,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 @@ -134,7 +139,34 @@ 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 + // (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. + 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. 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()) + } + + /// 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. @@ -199,6 +231,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) } @@ -466,6 +503,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 +/// 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. @@ -1426,6 +1475,174 @@ 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] + 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" + ); + } + } + + /// 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"); + } + + /// `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:?}" + ); + } + + /// 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}" + ); + } + } + + /// 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/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/mod.rs index d69754429f..6989a6e69e 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/mod.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/mod.rs @@ -21,6 +21,13 @@ //! This `src/secrets/` tree is the sole secret-bearing module: //! `tests/secrets_scan.rs` exempts it, so it owns its own review //! discipline via `tests/secrets_guard.rs`. +//! +//! Cryptographic wire format lives in [`mod@wire`]: the Tier-2 +//! envelope (`wire::envelope`) and the three AAD constructions +//! (`wire::aad`) are bincode-encoded against a single `WIRE_CONFIG`, so +//! a future bincode-config drift is caught by the golden-vector tests +//! in `wire::envelope::tests` rather than silently corrupting every +//! stored blob. mod error; mod file; @@ -28,6 +35,7 @@ mod keyring; mod secret; mod store; mod validate; +mod wire; pub use error::{IoError, OsKeyringErrorKind, SecretStoreError}; pub use file::{ @@ -35,6 +43,7 @@ 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; +pub use wire::envelope::MAX_PLAINTEXT_LEN; diff --git a/packages/rs-platform-wallet-storage/src/secrets/secret.rs b/packages/rs-platform-wallet-storage/src/secrets/secret.rs index 3dd5c53746..87faf066ef 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). /// @@ -47,6 +59,10 @@ 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. + // A direct freed-buffer scan would require `unsafe`, which this crate + // forbids; the test `secret_string_new_zeroizes_string_source` instead + // pins the `String::zeroize` primitive and this call site. source.zeroize(); let lock = region::lock(buf.as_ptr(), buf.capacity()) .map_err(|e| { @@ -87,6 +103,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 +171,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) +/// 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. /// @@ -265,6 +373,21 @@ mod tests { assert_eq!(s.trimmed().expose_secret(), "abandon ability"); } + /// 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() { + let mut source = String::from("super secret seed material"); + source.zeroize(); + assert!(source.is_empty(), "String::zeroize must empty the source"); + 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. @@ -282,6 +405,112 @@ mod tests { assert_eq!(SecretString::default().len(), 0); } + /// `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" + ); + } + + /// `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"))); + } + + /// `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); + } + + /// 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 + ); + } + + /// 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); + } + + /// `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); + } + + /// `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]); diff --git a/packages/rs-platform-wallet-storage/src/secrets/store.rs b/packages/rs-platform-wallet-storage/src/secrets/store.rs index 82157f4338..7f0147f165 100644 --- a/packages/rs-platform-wallet-storage/src/secrets/store.rs +++ b/packages/rs-platform-wallet-storage/src/secrets/store.rs @@ -13,16 +13,19 @@ use keyring_core::api::CredentialStoreApi; use keyring_core::{Entry, Error as KeyringError}; 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}; +use super::wire::envelope; +use super::{default_credential_store, EncryptedFileStore, MAX_SECRET_LEN, 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. @@ -43,40 +46,118 @@ 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 { 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 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)). + /// + /// **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, + 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_secret(service, label, None) + } + + /// Read the opaque bytes stored under `(service, label)`, or + /// `Ok(None)` if absent — the raw backend value, always a Tier-2 + /// envelope (writes go through + /// [`set_secret`](SecretStore::set_secret)). 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 @@ -85,7 +166,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)), } @@ -93,18 +190,95 @@ impl SecretStore { } } - /// 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> { + /// 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)` + a protected blob → the secret (or + /// [`WrongPassword`](SecretStoreError::WrongPassword) on tag fail); + /// - `Some(pw)` + an unprotected blob → + /// [`ExpectedProtectedButUnsealed`](SecretStoreError::ExpectedProtectedButUnsealed) + /// — a strip/downgrade, refused, no bytes returned; + /// - `None` + a protected blob → + /// [`NeedsPassword`](SecretStoreError::NeedsPassword) (never ciphertext); + /// - `None` + an unprotected blob → 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) + } + + /// Add / change / remove an object password in one same-slot + /// 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 + /// `new`: + /// - **add:** `current = None`, `new = Some(pw)`; + /// - **change:** `current = Some(old)`, `new = Some(pw_new)`; + /// - **remove:** `current = Some(old)`, `new = None`. + /// + /// An absent object returns [`Err(NoEntry)`][SecretStoreError::NoEntry] — + /// `reprotect` is operational; absence means the caller's protection-status + /// record disagrees with the backend, which is a signal not to be silently + /// dropped. 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, + label: &str, + current: Option<&SecretString>, + new: Option<&SecretString>, + ) -> Result<(), SecretStoreError> { + let Some(secret) = self.get_secret(service, label, current)? else { + return Err(SecretStoreError::NoEntry); + }; + self.set_secret(service, label, &secret, new) + } + + /// Delete the secret stored under `(service, label)`. + /// + /// Returns `Ok(true)` if a credential was removed, `Ok(false)` if no + /// credential existed under `(service, label)`. Idempotent for callers + /// that don't care — `.delete(...)?;` still discards the bool; + /// race-detecting callers can `match delete()?`. + pub fn delete(&self, service: &WalletId, label: &str) -> Result { match self { - Self::File(s) => { - s.delete_bytes(service, label)?; - Ok(()) - } + Self::File(s) => s.delete_bytes(service, label), Self::Os(store) => { let entry = build_os(store, service, label)?; match entry.delete_credential() { - Ok(()) | Err(KeyringError::NoEntry) => Ok(()), + Ok(()) => Ok(true), + Err(KeyringError::NoEntry) => Ok(false), Err(e) => Err(map_spi(e)), } } @@ -152,9 +326,7 @@ impl std::fmt::Debug for SecretStore { /// The [`File`](SecretStore::File) arm never reaches this projection. fn map_spi(e: KeyringError) -> SecretStoreError { match e { - KeyringError::NoEntry => SecretStoreError::OsKeyring { - kind: OsKeyringErrorKind::NoEntry, - }, + KeyringError::NoEntry => SecretStoreError::NoEntry, KeyringError::NoStorageAccess(_) => SecretStoreError::OsKeyring { kind: OsKeyringErrorKind::NoStorageAccess, }, @@ -219,17 +391,31 @@ mod tests { } #[test] - fn delete_is_idempotent() { + fn delete_returns_false_on_absent_true_on_present() { let dir = tempfile::tempdir().unwrap(); let s = file_store(dir.path()); - // Absent → Ok, no error. - s.delete(&wid(1), "seed").unwrap(); + // Absent → Ok(false), no error. + assert!(!s.delete(&wid(1), "seed").unwrap()); s.set(&wid(1), "seed", &SecretBytes::from_slice(b"x")) .unwrap(); - s.delete(&wid(1), "seed").unwrap(); + // Present → Ok(true). + assert!(s.delete(&wid(1), "seed").unwrap()); assert!(s.get(&wid(1), "seed").unwrap().is_none()); - // Second delete on the now-absent entry is still Ok. - s.delete(&wid(1), "seed").unwrap(); + // Second delete on the now-absent entry is Ok(false). + assert!(!s.delete(&wid(1), "seed").unwrap()); + } + + #[test] + fn reprotect_absent_returns_no_entry() { + let dir = tempfile::tempdir().unwrap(); + let s = file_store(dir.path()); + let err = s + .reprotect(&wid(1), "seed", None, Some(&SecretString::new("pw"))) + .unwrap_err(); + assert!( + matches!(err, SecretStoreError::NoEntry), + "expected NoEntry on absent reprotect, got {err:?}" + ); } #[test] @@ -357,4 +543,675 @@ mod tests { ); } } + + // ===== 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 — 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 these 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() + .expose_secret() + .to_vec() + } + + fn unprotected(w: &WalletId, label: &str, secret: &[u8]) -> Vec { + envelope::wrap(w, label, None, secret) + .unwrap() + .expose_secret() + .to_vec() + } + + /// 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). 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)) => { + 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"), + } + } + } + + 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 { + // 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, + mock: Some(mock), + name: "Os", + } + } + + /// 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 + ); + + // Truncated envelope (below the bincode minimum) → Corruption, + // both with and without a password — no magic byte to peek at. + b.place_raw(&w, "broken", &[0x01]); + for arg in [None, Some(&pw)] { + assert!( + matches!( + b.store.get_secret(&w, "broken", arg).unwrap_err(), + SecretStoreError::Corruption + ), + "[{}] truncated envelope ({:?})", + b.name, + arg.map(|_| "Some") + ); + } + + // Raw, non-envelope bytes → Corruption under either password + // arg: every read goes through the bincode decoder. + b.place_raw(&w, "raw", b"raw-bytes-not-a-valid-envelope"); + for arg in [None, Some(&pw)] { + assert!( + matches!( + b.store.get_secret(&w, "raw", arg).unwrap_err(), + SecretStoreError::Corruption + ), + "[{}] raw non-envelope bytes ({:?})", + b.name, + arg.map(|_| "Some") + ); + } + + // 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()); + } + + /// 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()); + } + + /// 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"); + // (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()); + } + + /// 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()); + } + + /// Unprotected→protected 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()); + } + + /// A scheme-flip from `Password` → `Unprotected`: `Some(pw)` is + /// caught by the strict rule regardless; `None` reads the body as + /// scheme-0 opaque bytes (never the real seed) — a known residual, + /// dominated by the consumer-DB residual; pinned, not "fixed". + fn run_scheme_flip(b: &Backend) { + use crate::secrets::wire::config::WIRE_CONFIG; + use crate::secrets::wire::envelope::{Envelope, Payload}; + + let w = wid(6); + let pw = SecretString::new("pw"); + let blob = protected(&w, "x", "pw", b"real-seed"); + let (env, _): (Envelope, usize) = bincode::decode_from_slice(&blob, WIRE_CONFIG).unwrap(); + let flipped = match env.payload { + Payload::Password { ciphertext, .. } => Envelope { + version: env.version, + payload: Payload::Unprotected(ciphertext), + }, + Payload::Unprotected(_) => panic!("protected() must yield a Password payload"), + }; + let flipped_blob = bincode::encode_to_vec(&flipped, WIRE_CONFIG).unwrap(); + b.place_raw(&w, "x", &flipped_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()); + } + + // ===== 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. + + /// 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 the consumer 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()); + } + + /// 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()); + } + + /// `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()); + } + + /// 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()); + } + + /// [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 + )); + } + + /// [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 + )); + } + + /// [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 + )); + } } diff --git a/packages/rs-platform-wallet-storage/src/secrets/wire/aad.rs b/packages/rs-platform-wallet-storage/src/secrets/wire/aad.rs new file mode 100644 index 0000000000..f6b750eef7 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/wire/aad.rs @@ -0,0 +1,252 @@ +//! Bincode-encoded AAD structs for the three contexts that authenticate +//! ciphertexts under `secrets/`: Tier-2 scheme-1 envelopes, vault entry +//! bodies, and the vault passphrase-verify token. +//! +//! Each struct is `Encode`-only — AAD is producer-side; the decoder +//! re-builds it from the surrounding context and bincode-encodes again +//! against [`WIRE_CONFIG`]. Pair-wise byte disjointness is guaranteed by +//! the three domain constants declared in [`super::config`] and pinned +//! empirically by the tests `tier2_and_entry_aad_byte_disjoint`, +//! `tier2_and_verify_aad_byte_disjoint`, and +//! `entry_and_verify_aad_byte_disjoint`. + +use crate::secrets::file::crypto::SALT_LEN; +use crate::secrets::wire::kdf::KdfParamsEncoded; + +/// AAD bound into every scheme-1 (password-protected) Tier-2 envelope. +/// Binds object identity (`wallet_id` + `label`) + header +/// (`envelope_version`, `scheme_discriminant`, `kdf`, `salt`) so any +/// in-place edit of those fields fails the AEAD tag. +/// +/// `scheme_discriminant` is explicit (not inferred from a Rust enum +/// variant tag) so the AAD shape is stable under a future `Payload` +/// re-ordering. +#[derive(bincode::Encode)] +pub(crate) struct Tier2Aad<'a> { + /// Domain tag — `TIER2_DOMAIN_V2`. Length-prefixed by bincode and + /// byte-disjoint from `ENTRY_DOMAIN_V2` / `VERIFY_DOMAIN_V2` by + /// content past the common prefix; pinned by the disjointness tests + /// in [`super::aad::tests`]. + pub domain: &'static [u8], + /// Envelope wire version (`ENVELOPE_VERSION`). + pub envelope_version: u32, + /// `0 = Unprotected`, `1 = Password`. Authenticates the scheme byte + /// independently of the enum's bincode-derived tag. + pub scheme_discriminant: u8, + /// The exact bytes encoded into the envelope's `Payload::Password` + /// body — AAD == body, so a wire-edited KDF header fails the tag. + pub kdf: KdfParamsEncoded, + /// Per-wrap CSPRNG salt. + pub salt: [u8; SALT_LEN], + /// 32-byte wallet correlation id (public, not secret). + pub wallet_id: [u8; 32], + /// Caller-allowlisted slot label. + pub label: &'a str, +} + +/// AAD bound into every vault entry's AEAD seal. Replaces the +/// hand-rolled `format::aad()` byte concatenation; binds slot identity +/// (`wallet_id` + `label`) at a stable `format_version`. A relocated +/// or version-rolled-back blob fails the tag. +#[derive(bincode::Encode)] +pub(crate) struct EntryAad<'a> { + /// Domain tag — `ENTRY_DOMAIN_V2`. + pub domain: &'static [u8], + /// Vault `FORMAT_VERSION` (the compiled-in dispatch version, + /// never the parsed JSON version). + pub format_version: u32, + /// 32-byte wallet correlation id. + pub wallet_id: [u8; 32], + /// Caller-allowlisted slot label. + pub label: &'a str, +} + +/// AAD bound into the vault passphrase-verify token's AEAD seal. +/// Binds salt + KDF header so a flipped salt or KDF-param shift fails +/// the token tag (surfaces as `WrongPassphrase` — a tampered header +/// also yields a different derived key). +#[derive(bincode::Encode)] +pub(crate) struct VerifyAad { + /// Domain tag — `VERIFY_DOMAIN_V2`. + pub domain: &'static [u8], + /// Vault `FORMAT_VERSION`. + pub format_version: u32, + /// Vault-wide CSPRNG salt. + pub salt: [u8; SALT_LEN], + /// Vault-wide KDF parameters (the same wire image used by every + /// scheme-1 Tier-2 envelope). + pub kdf: KdfParamsEncoded, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::secrets::file::crypto::KdfParams; + use crate::secrets::wire::config::{ + ENTRY_DOMAIN_V2, TIER2_DOMAIN_V2, VERIFY_DOMAIN_V2, WIRE_CONFIG, + }; + + fn floor_kdf() -> KdfParamsEncoded { + KdfParamsEncoded::from(KdfParams::default_target()) + } + + fn tier2_with_domain(domain: &'static [u8]) -> Vec { + let aad = Tier2Aad { + domain, + envelope_version: 1, + scheme_discriminant: 1, + kdf: floor_kdf(), + salt: [0x77u8; SALT_LEN], + wallet_id: [0x11u8; 32], + label: "seed", + }; + bincode::encode_to_vec(aad, WIRE_CONFIG).unwrap() + } + + fn tier2_with_version(envelope_version: u32) -> Vec { + let aad = Tier2Aad { + domain: TIER2_DOMAIN_V2, + envelope_version, + scheme_discriminant: 1, + kdf: floor_kdf(), + salt: [0x77u8; SALT_LEN], + wallet_id: [0x11u8; 32], + label: "seed", + }; + bincode::encode_to_vec(aad, WIRE_CONFIG).unwrap() + } + + fn tier2_with_scheme(scheme_discriminant: u8) -> Vec { + let aad = Tier2Aad { + domain: TIER2_DOMAIN_V2, + envelope_version: 1, + scheme_discriminant, + kdf: floor_kdf(), + salt: [0x77u8; SALT_LEN], + wallet_id: [0x11u8; 32], + label: "seed", + }; + bincode::encode_to_vec(aad, WIRE_CONFIG).unwrap() + } + + fn entry(format_version: u32, wallet_id: [u8; 32], label: &str) -> Vec { + let aad = EntryAad { + domain: ENTRY_DOMAIN_V2, + format_version, + wallet_id, + label, + }; + bincode::encode_to_vec(aad, WIRE_CONFIG).unwrap() + } + + fn verify(salt: [u8; SALT_LEN], kdf: KdfParamsEncoded) -> Vec { + let aad = VerifyAad { + domain: VERIFY_DOMAIN_V2, + format_version: 1, + salt, + kdf, + }; + bincode::encode_to_vec(aad, WIRE_CONFIG).unwrap() + } + + /// Two byte strings where neither is a prefix of the other. + fn assert_prefix_disjoint(a: &[u8], b: &[u8]) { + assert!( + !a.starts_with(b) && !b.starts_with(a), + "prefix containment: a.len={} b.len={}", + a.len(), + b.len() + ); + } + + /// TC-014 — Tier2Aad.domain is bincode-encoded. + #[test] + fn tier2_aad_domain_field_binds_bytes() { + let a = tier2_with_domain(TIER2_DOMAIN_V2); + let b = tier2_with_domain(b"PWSEV-TIER2-AAD-vX"); + assert_ne!(a, b); + assert_prefix_disjoint(&a, &b); + } + + /// TC-015 — Tier2Aad.envelope_version is bincode-encoded. + #[test] + fn tier2_aad_envelope_version_field_binds_bytes() { + assert_ne!(tier2_with_version(1), tier2_with_version(2)); + } + + /// TC-016 — Tier2Aad.scheme_discriminant is bincode-encoded and + /// explicit (not inferred from a Rust enum tag). + #[test] + fn tier2_aad_scheme_discriminant_field_binds_bytes() { + assert_ne!(tier2_with_scheme(0), tier2_with_scheme(1)); + } + + /// TC-025 — Tier2Aad and EntryAad are byte-disjoint at the prefix. + #[test] + fn tier2_and_entry_aad_byte_disjoint() { + let t = tier2_with_domain(TIER2_DOMAIN_V2); + let e = entry(1, [0x11u8; 32], "seed"); + assert_prefix_disjoint(&t, &e); + } + + /// TC-026 — Tier2Aad and VerifyAad are byte-disjoint at the prefix. + #[test] + fn tier2_and_verify_aad_byte_disjoint() { + let t = tier2_with_domain(TIER2_DOMAIN_V2); + let v = verify([0x77u8; SALT_LEN], floor_kdf()); + assert_prefix_disjoint(&t, &v); + } + + /// TC-027 — EntryAad and VerifyAad are byte-disjoint at the prefix. + /// Now backed by an explicit domain constant on top of the existing + /// VERIFY_LABEL leading-NUL trick at the `format.rs` call site. + #[test] + fn entry_and_verify_aad_byte_disjoint() { + let e = entry(1, [0u8; 32], "\0verify"); + let v = verify([0x77u8; SALT_LEN], floor_kdf()); + assert_prefix_disjoint(&e, &v); + } + + /// TC-037 — EntryAad binds (format_version, wallet_id, label) and + /// the label encoding carries its length prefix (`"a"+"b"` vs + /// `"ab"` are distinct). + #[test] + fn entry_aad_binds_format_version_wallet_id_and_label() { + let base = entry(1, [1u8; 32], "a"); + assert_ne!(base, entry(2, [1u8; 32], "a")); + assert_ne!(base, entry(1, [2u8; 32], "a")); + assert_ne!(base, entry(1, [1u8; 32], "b")); + // Length-prefix sanity: "ab" must not equal the concatenation of + // the encoding of "a" with the literal byte `b`. + let ab = entry(1, [1u8; 32], "ab"); + let mut a_plus_b = base.clone(); + a_plus_b.extend_from_slice(b"b"); + assert_ne!(ab, a_plus_b); + } + + /// TC-038 — VerifyAad binds salt + KDF; identical inputs produce + /// identical bytes (determinism). + #[test] + fn verify_aad_binds_salt_and_kdf_params() { + let salt = [7u8; SALT_LEN]; + let kdf = floor_kdf(); + let base = verify(salt, kdf); + let mut salt2 = salt; + salt2[0] ^= 0x01; + assert_ne!(base, verify(salt2, kdf)); + + let kdf_mkib = KdfParamsEncoded { + m_kib: kdf.m_kib / 2, + ..kdf + }; + assert_ne!(base, verify(salt, kdf_mkib)); + let kdf_t = KdfParamsEncoded { + t: kdf.t - 1, + ..kdf + }; + assert_ne!(base, verify(salt, kdf_t)); + + // Determinism: identical inputs ⇒ identical bytes. + assert_eq!(base, verify(salt, kdf)); + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/wire/config.rs b/packages/rs-platform-wallet-storage/src/secrets/wire/config.rs new file mode 100644 index 0000000000..b6c6031a63 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/wire/config.rs @@ -0,0 +1,43 @@ +//! Single bincode configuration + domain / version constants every +//! encoder in `secrets/wire/` uses. +//! +//! `WIRE_CONFIG` matches the platform-wide +//! `bincode::config::standard().with_big_endian().with_no_limit()` +//! (`rs-platform-serialization`) — big-endian for human-readable hex +//! dumps, varint integer encoding, no decode limit. +//! +//! Changing this constant invalidates every stored Tier-2 blob; the +//! golden-vector tests in [`super::envelope::tests`] catch any drift. + +use bincode::config::{BigEndian, Configuration, NoLimit, Varint}; + +/// The one bincode config used to encode every wire byte under +/// `secrets/wire/` (envelope payload + the three AAD structs). +pub(crate) const WIRE_CONFIG: Configuration = + bincode::config::standard() + .with_big_endian() + .with_no_limit(); + +/// Tier-2 envelope wire version — bumped only on a breaking layout +/// change, independent of the vault `FORMAT_VERSION`. Bound into every +/// scheme-1 envelope's AAD so a forged version byte fails the tag. +pub(crate) const ENVELOPE_VERSION: u32 = 1; + +/// Domain-separation tag leading the Tier-2 scheme-1 AAD. `-v2` marks the +/// wire-format break from the pre-bincode hand-rolled `PWSEV-TIER2-AAD-v1`. +pub(crate) const TIER2_DOMAIN_V2: &[u8] = b"PWSEV-TIER2-AAD-v2"; + +/// Domain-separation tag leading every vault `EntryAad`. Pre-bincode +/// `aad()` had no domain tag; bound here for symmetry + cross-context +/// disjointness with [`TIER2_DOMAIN_V2`] and [`VERIFY_DOMAIN_V2`]. +pub(crate) const ENTRY_DOMAIN_V2: &[u8] = b"PWSV-ENTRY-AAD-v2"; + +/// Domain-separation tag leading every vault `VerifyAad`. Disjoint +/// from [`TIER2_DOMAIN_V2`] and [`ENTRY_DOMAIN_V2`] by **content past +/// the common prefix** (the three tags are NOT length-distinct — +/// TIER2 and VERIFY are both 18 bytes; ENTRY is 17). Pair-wise +/// byte-disjointness is pinned by the tests +/// `tier2_and_verify_aad_byte_disjoint`, +/// `tier2_and_entry_aad_byte_disjoint`, and +/// `entry_and_verify_aad_byte_disjoint`. +pub(crate) const VERIFY_DOMAIN_V2: &[u8] = b"PWSV-VERIFY-AAD-v2"; diff --git a/packages/rs-platform-wallet-storage/src/secrets/wire/envelope.rs b/packages/rs-platform-wallet-storage/src/secrets/wire/envelope.rs new file mode 100644 index 0000000000..d9ee13db5b --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/wire/envelope.rs @@ -0,0 +1,1029 @@ +//! Tier-2 envelope wire format — bincode-encoded `Envelope` / `Payload` +//! plus the [`wrap`] / [`wrap_with_params`] / [`unwrap`] API. +//! +//! Every byte that crosses the AEAD seam is produced by +//! `bincode::encode_to_vec` against [`WIRE_CONFIG`], so a future config +//! drift surfaces in the golden-vector tests, not in silently corrupted +//! blobs. Decoding goes through [`DECODE_CONFIG`] — the same +//! configuration with a byte limit, so a hostile blob declaring a +//! multi-GiB length prefix is rejected before any allocation. + +use bincode::config::{BigEndian, Configuration, Limit, Varint}; + +use crate::secrets::error::SecretStoreError; +use crate::secrets::file::crypto::{self, KdfParams, NONCE_LEN, SALT_LEN}; +use crate::secrets::secret::{SecretBytes, SecretString}; +use crate::secrets::validate::WalletId; +use crate::secrets::wire::aad::Tier2Aad; +use crate::secrets::wire::config::{ENVELOPE_VERSION, TIER2_DOMAIN_V2, WIRE_CONFIG}; +use crate::secrets::wire::kdf::KdfParamsEncoded; +use crate::secrets::MAX_SECRET_LEN; + +/// On-disk Tier-2 wire envelope. The whole struct is bincode-encoded +/// in one call; a wire-edited `version` is gated to +/// `SecretStoreError::UnsupportedEnvelopeVersion` before dispatch. +#[derive(bincode::Encode, bincode::Decode, Debug, PartialEq, Eq)] +pub(crate) struct Envelope { + /// Envelope wire version (`ENVELOPE_VERSION`). + pub version: u32, + /// Tagged payload selecting unprotected vs password-protected. + pub payload: Payload, +} + +/// Tagged payload: scheme-0 ships the plaintext as-is (the backend's +/// own at-rest crypto is the only defence); scheme-1 ships the AEAD +/// triple under an object-password-derived key. +#[derive(bincode::Encode, bincode::Decode, Debug, PartialEq, Eq)] +pub(crate) enum Payload { + /// Scheme 0 — unprotected passthrough; the bytes are the secret. + Unprotected(Vec), + /// Scheme 1 — sealed under an Argon2id-derived key with + /// XChaCha20-Poly1305. The AAD bound at seal time is + /// [`crate::secrets::wire::aad::Tier2Aad`]. + Password { + /// Argon2 parameters used to derive the key. + kdf: KdfParamsEncoded, + /// Per-wrap CSPRNG salt fed into Argon2. + salt: [u8; SALT_LEN], + /// Per-wrap CSPRNG nonce fed into XChaCha20-Poly1305. + nonce: [u8; NONCE_LEN], + /// Ciphertext + 16-byte Poly1305 tag. + ciphertext: Vec, + }, +} + +/// Upper bound on the bincode-encoded envelope overhead over its +/// plaintext (header + KDF + salt + nonce + AEAD tag + bincode framing). +/// Pinned by a runtime cross-check in `tests::max_envelope_overhead_matches_runtime` +/// so any bincode-config drift surfaces immediately. The smallest +/// scheme-1 envelope (empty plaintext sealed → 16-byte tag) measures +/// 81 bytes; rounded up to the next 16-byte boundary that satisfies a +/// 16-byte safety margin (81 + 16 = 97 → 112) for headroom against a +/// future header field. +pub(crate) const MAX_ENVELOPE_OVERHEAD: usize = 112; + +/// 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. +pub const MAX_PLAINTEXT_LEN: usize = MAX_SECRET_LEN - MAX_ENVELOPE_OVERHEAD; + +/// Decode-side budget: caps the bytes the bincode decoder will consume +/// from a single envelope. Equal to the on-disk cap. +const DECODE_BUDGET: usize = MAX_SECRET_LEN + MAX_ENVELOPE_OVERHEAD; + +/// Bincode decode config — derived from [`WIRE_CONFIG`] but with a +/// [`DECODE_BUDGET`] byte limit applied. +/// +/// **Asymmetric on purpose, security-positive deviation from +/// design-brief NF2** (which locks the wire config to +/// `with_no_limit()`). The deviation exists for hostile-decode +/// hardening: an attacker-controlled length prefix in the blob would +/// otherwise drive `Vec::with_capacity` to a multi-GiB allocation +/// before any tag check. With `Limit`, bincode refuses the +/// allocation up front and the unwrap fails closed as `Corruption`. +/// +/// The encoder retains [`WIRE_CONFIG`] (no limit) because AAD and +/// envelope encoding are producer-only — every input is library-owned +/// and bounded by `MAX_PLAINTEXT_LEN`, so a limit there has no +/// security benefit and would be a foot-gun against legitimate +/// at-cap secrets. +const DECODE_CONFIG: Configuration> = + WIRE_CONFIG.with_limit::(); + +/// Wrap `plaintext` for `(wallet_id, label)` using the shipped default +/// Argon2 target 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`). +/// +/// Returns the envelope inside a zeroizing [`SecretBytes`]. +pub(crate) fn wrap( + wallet_id: &WalletId, + label: &str, + password: Option<&SecretString>, + plaintext: &[u8], +) -> Result { + wrap_with_params( + wallet_id, + label, + password, + plaintext, + KdfParams::default_target(), + ) +} + +/// [`wrap`] with explicit Argon2 `params` (tests use floor params for +/// speed). `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 { + // Cap the PLAINTEXT (before overhead) uniformly for both schemes so + // the enveloped bytes always fit the backend cap. + if plaintext.len() > MAX_PLAINTEXT_LEN { + return Err(SecretStoreError::SecretTooLarge { + found: plaintext.len(), + max: MAX_PLAINTEXT_LEN, + }); + } + + let Some(pw) = password else { + let envelope = Envelope { + version: ENVELOPE_VERSION, + payload: Payload::Unprotected(plaintext.to_vec()), + }; + return Ok(SecretBytes::new(encode_envelope(&envelope))); + }; + + // Reject a blank object password BEFORE any salt / derive. + if pw.is_blank() { + return Err(SecretStoreError::BlankPassphrase); + } + + let mut salt = [0u8; SALT_LEN]; + crypto::random_bytes(&mut salt)?; + let key = crypto::derive_key(pw, &salt, params)?; + let kdf = KdfParamsEncoded::from(params); + let aad = encode_tier2_aad(wallet_id, label, kdf, &salt); + let (nonce, ciphertext) = crypto::seal(&key, &aad, plaintext)?; + + let envelope = Envelope { + version: ENVELOPE_VERSION, + payload: Payload::Password { + kdf, + salt, + nonce, + ciphertext, + }, + }; + Ok(SecretBytes::new(encode_envelope(&envelope))) +} + +/// Bincode-encode the scheme-1 AAD against [`WIRE_CONFIG`]. Shared by +/// [`wrap_with_params`] and [`unwrap_password_payload`] so the encode +/// and decode AADs cannot drift apart. +pub(crate) fn encode_tier2_aad( + wallet_id: &WalletId, + label: &str, + kdf: KdfParamsEncoded, + salt: &[u8; SALT_LEN], +) -> Vec { + let aad = Tier2Aad { + domain: TIER2_DOMAIN_V2, + envelope_version: ENVELOPE_VERSION, + scheme_discriminant: 1, + kdf, + salt: *salt, + wallet_id: *wallet_id.as_bytes(), + label, + }; + // AAD encode is infallible — every field is owned/borrowed bincode- + // Encode-able. A failure would be a logic bug. + bincode::encode_to_vec(aad, WIRE_CONFIG).expect("Tier2Aad encode is infallible") +} + +/// Bincode-encode the whole envelope. Wrapping in `SecretBytes::new` +/// keeps the (possibly plaintext-bearing) scheme-0 buffer zeroizing. +fn encode_envelope(envelope: &Envelope) -> Vec { + bincode::encode_to_vec(envelope, WIRE_CONFIG).expect("Envelope encode is infallible") +} + +/// Unwrap `blob` for `(wallet_id, label)`, applying the strict +/// fail-closed read. +/// +/// `password` carries the caller's protection assertion — never the +/// blob's scheme byte. Decode errors (truncated, garbage bytes, unknown +/// enum tag) collapse to `Corruption`; an envelope version this build +/// does not recognise yields `UnsupportedEnvelopeVersion` ahead of +/// dispatch. +/// +/// | `password` | `payload` | result | +/// |---|---|---| +/// | `Some(pw)` | `Password { .. }` | the secret, or `WrongPassword` on tag fail | +/// | `Some(pw)` | `Unprotected(_)` | `ExpectedProtectedButUnsealed` (strip/downgrade) | +/// | `None` | `Password { .. }` | `NeedsPassword` (never ciphertext) | +/// | `None` | `Unprotected(pt)` | the secret | +pub(crate) fn unwrap( + wallet_id: &WalletId, + label: &str, + password: Option<&SecretString>, + blob: &[u8], +) -> Result { + let (envelope, consumed) = bincode::decode_from_slice::(blob, DECODE_CONFIG) + .map_err(|_| SecretStoreError::Corruption)?; + // Trailing bytes after a valid decode are a truncation/extension + // probe — fail closed. + if consumed != blob.len() { + return Err(SecretStoreError::Corruption); + } + + if envelope.version != ENVELOPE_VERSION { + // `found` keeps the historical u8 — the error API stayed u8 for + // back-compat; an out-of-range u32 wraps but the decoder above + // already accepts every u32 so this only narrows the diagnostic. + return Err(SecretStoreError::UnsupportedEnvelopeVersion { + found: envelope.version as u8, + }); + } + + match (envelope.payload, password) { + (Payload::Unprotected(plaintext), None) => Ok(SecretBytes::new(plaintext)), + // Caller asserted protection but blob is unprotected: strip / + // downgrade — fail closed, never return the bytes. + (Payload::Unprotected(_), Some(_)) => Err(SecretStoreError::ExpectedProtectedButUnsealed), + (Payload::Password { .. }, None) => Err(SecretStoreError::NeedsPassword), + ( + Payload::Password { + kdf, + salt, + nonce, + ciphertext, + }, + Some(pw), + ) => unwrap_password_payload(wallet_id, label, pw, kdf, salt, nonce, &ciphertext), + } +} + +/// Decrypt a `Payload::Password` body. The KDF params, salt and nonce +/// come from the (attacker-controllable) envelope; `enforce_bounds` +/// AND a stricter per-read `default_target` ceiling gate the params +/// BEFORE `derive_key` allocates. +fn unwrap_password_payload( + wallet_id: &WalletId, + label: &str, + password: &SecretString, + kdf_encoded: KdfParamsEncoded, + salt: [u8; SALT_LEN], + nonce: [u8; NONCE_LEN], + ciphertext: &[u8], +) -> Result { + // (a0) Mirror wrap's invariant: a blank object password is rejected on + // read as well as enrol, so a backend-write attacker who plants a + // scheme-1 envelope sealed under the blank password cannot inject + // plaintext into a caller that accidentally forwards Some(empty). + if password.is_blank() { + return Err(SecretStoreError::BlankPassphrase); + } + // (a) Wider Argon2 floors/ceilings — refuses an inflated header + // before any allocation. + let kdf = KdfParams::try_from(kdf_encoded)?; + // (b) Per-read ceiling tighter than `enforce_bounds`: a header + // declaring more memory OR more time than this build's shipped + // target is refused before `derive_key` allocates. Closes the gaps + // between `ARGON2_MAX_M_KIB` (1 GiB) / `ARGON2_MAX_T` (16) and the + // shipped 64 MiB / t=3 default — bounds the worst-case forged read + // at the shipped target on both axes (no headroom for an attacker + // to inflate memory by 16× or CPU by 5.3×). + let target = KdfParams::default_target(); + if kdf.m_kib > target.m_kib || kdf.t > target.t { + return Err(SecretStoreError::KdfFailure); + } + // (c) AAD binds identity + header — the same bytes the encoder + // produced, by construction. + let aad = encode_tier2_aad(wallet_id, label, kdf_encoded, &salt); + 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, header tamper): + // no plaintext ever materialises (CWE-347). + Err(SecretStoreError::Decrypt) => Err(SecretStoreError::WrongPassword), + Err(e) => Err(e), + } +} + +/// Test-only deterministic encoder: takes pre-supplied `salt` and +/// `nonce` instead of pulling from the CSPRNG, so golden-vector tests +/// produce reproducible bytes. Production callers MUST use +/// [`wrap_with_params`]. +#[cfg(test)] +pub(crate) fn wrap_with_params_for_test( + wallet_id: &WalletId, + label: &str, + pw: &SecretString, + plaintext: &[u8], + params: KdfParams, + salt: [u8; SALT_LEN], + nonce: [u8; NONCE_LEN], +) -> Result { + if plaintext.len() > MAX_PLAINTEXT_LEN { + return Err(SecretStoreError::SecretTooLarge { + found: plaintext.len(), + max: MAX_PLAINTEXT_LEN, + }); + } + if pw.is_blank() { + return Err(SecretStoreError::BlankPassphrase); + } + let key = crypto::derive_key(pw, &salt, params)?; + let kdf = KdfParamsEncoded::from(params); + let aad = encode_tier2_aad(wallet_id, label, kdf, &salt); + let (nonce, ciphertext) = crypto::seal_with_nonce(&key, nonce, &aad, plaintext)?; + let envelope = Envelope { + version: ENVELOPE_VERSION, + payload: Payload::Password { + kdf, + salt, + nonce, + ciphertext, + }, + }; + Ok(SecretBytes::new(encode_envelope(&envelope))) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::secrets::file::crypto::{ARGON2_MIN_M_KIB, ARGON2_MIN_T, ARGON2_P}; + use crate::secrets::file::format::KDF_ID_ARGON2ID; + + /// Captured once from the runtime encoder; a subsequent CI failure + /// here means a wire-format drift to investigate, NOT to "fix" by + /// re-generating the constant. + /// + /// Decoding: 0x01 envelope.version=1, 0x00 Payload::Unprotected, + /// 0x05 Vec length=5, "hello". + const SCHEME0_GOLDEN_HEX: &str = "01000568656c6c6f"; + + /// scheme-1 deterministic golden: wid=[0;32], label="seed", + /// pw="pw", plaintext="hello", floor params, salt=[0x11;32], + /// nonce=[0x22;24]. Bytes: version + Payload::Password tag + + /// kdf(id,m_kib,t,p as varints) + salt[32] + nonce[24] + + /// ciphertext-with-tag length + ciphertext+tag(21B). + const SCHEME1_GOLDEN_HEX: &str = "010101fb4c000201111111111111111111111111111111111111111111111111111111111111111122222222222222222222222222222222222222222222222215e2ffdf3f0476b6bfb99b4f71b3039ff965132b92f0"; + + fn wid(b: u8) -> WalletId { + WalletId::from([b; 32]) + } + + fn pw(s: &str) -> SecretString { + SecretString::new(s) + } + + fn floor() -> KdfParams { + KdfParams { + id: KDF_ID_ARGON2ID, + m_kib: ARGON2_MIN_M_KIB, + t: ARGON2_MIN_T, + p: ARGON2_P, + } + } + + /// TC-033 — blank object password rejected at enrol (wrap-side). + #[test] + fn blank_object_password_rejected_at_wrap() { + 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:?}" + ); + } + } + + /// Symmetric guard on the read side: a `Some(blank)` password reaching + /// `unwrap_password_payload` is refused with `BlankPassphrase` BEFORE + /// any KDF or AEAD work — never `WrongPassword`, never `Decrypt`, + /// never plaintext. Pins the contract that closes the asymmetry where + /// a backend-write attacker could plant a scheme-1 envelope sealed + /// under the blank password and have a caller that accidentally + /// forwards `Some(SecretString::empty())` accept attacker-controlled + /// plaintext. + #[test] + fn unwrap_password_payload_rejects_some_blank_password() { + let blob = scheme1_blob(&pw("good")); + let err = unwrap(&wid(1), "seed", Some(&SecretString::empty()), &blob).unwrap_err(); + assert!( + matches!(err, SecretStoreError::BlankPassphrase), + "blank object password must be refused before KDF/AEAD, got {err:?}" + ); + } + + /// TC-034 — plaintext cap accept at MAX_PLAINTEXT_LEN, reject at + /// +1, for both schemes. + #[test] + fn plaintext_cap_accept_then_reject() { + let at_cap = vec![0x5Au8; MAX_PLAINTEXT_LEN]; + let over = vec![0x5Au8; MAX_PLAINTEXT_LEN + 1]; + + // Scheme 0 + 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 + )); + + // Scheme 1 — cap check fires 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 + )); + + // Scheme-0 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); + } + + /// TC-035 (size-budget half) — scheme-1 accepts plaintext at the + /// exact MAX_PLAINTEXT_LEN boundary; the enveloped bytes fit the + /// backend cap. The round-trip half is `scheme1_at_cap_round_trips_within_backend_cap`. + #[test] + fn scheme1_at_cap_envelope_fits_backend_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 ({} B) exceed backend cap ({} B)", + blob.len(), + MAX_SECRET_LEN + ); + } + + /// TC-028 — golden hex vector for the scheme-0 wire bytes. Any + /// bincode-config drift (endianness, varint mode, limit) trips this. + #[test] + fn scheme0_golden_vector_matches_const() { + let blob = wrap(&WalletId::from([0u8; 32]), "seed", None, b"hello").unwrap(); + let actual = hex::encode(blob.expose_secret()); + assert_eq!(actual, SCHEME0_GOLDEN_HEX); + } + + /// TC-029 — golden hex vector for the scheme-1 wire bytes, produced + /// via the deterministic encoder seam. + #[test] + fn scheme1_golden_vector_matches_const() { + let blob = wrap_with_params_for_test( + &WalletId::from([0u8; 32]), + "seed", + &pw("pw"), + b"hello", + floor(), + [0x11u8; SALT_LEN], + [0x22u8; NONCE_LEN], + ) + .unwrap(); + let actual = hex::encode(blob.expose_secret()); + assert_eq!(actual, SCHEME1_GOLDEN_HEX); + } + + /// Minimum overhead within budget AND the budget not absurdly above + /// the actual encoding — bound on both sides so the constant stays + /// honest as the wire shape evolves. + const SAFETY_MARGIN: usize = 16; + + /// TC-030 — `MAX_ENVELOPE_OVERHEAD` cross-checks the runtime + /// bincode encoding of the smallest possible scheme-1 envelope + /// (empty plaintext sealed → ciphertext == 16-byte AEAD tag). + #[test] + fn max_envelope_overhead_matches_runtime() { + let blob = wrap_with_params_for_test( + &WalletId::from([0u8; 32]), + "seed", + &pw("pw"), + b"", + floor(), + [0x11u8; SALT_LEN], + [0x22u8; NONCE_LEN], + ) + .unwrap(); + let actual = blob.len(); + assert!( + actual + SAFETY_MARGIN <= MAX_ENVELOPE_OVERHEAD, + "overhead {} + margin {} exceeds const {}", + actual, + SAFETY_MARGIN, + MAX_ENVELOPE_OVERHEAD + ); + assert!( + MAX_ENVELOPE_OVERHEAD - actual < 64, + "MAX_ENVELOPE_OVERHEAD {} is more than 64 B above the runtime measurement {} — tighten it", + MAX_ENVELOPE_OVERHEAD, + actual + ); + } + + // ===== Decoder: dispatch / wire-flip / fuzz / property ===== + + use crate::secrets::file::crypto::{ARGON2_MAX_M_KIB, ARGON2_MAX_T}; + use crate::secrets::wire::config::WIRE_CONFIG; + use subtle::ConstantTimeEq; + + /// Decode a real envelope so wire-flip tests can mutate one field + /// and re-encode. + fn decode(blob: &[u8]) -> Envelope { + bincode::decode_from_slice::(blob, WIRE_CONFIG) + .unwrap() + .0 + } + + fn encode(envelope: &Envelope) -> Vec { + bincode::encode_to_vec(envelope, WIRE_CONFIG).unwrap() + } + + /// Build a fresh scheme-1 envelope (under wid(1)/"seed"/pw=`p`) and + /// hand back the bytes for mutation tests. + fn scheme1_blob(p: &SecretString) -> Vec { + wrap_with_params(&wid(1), "seed", Some(p), b"seed", floor()) + .unwrap() + .expose_secret() + .to_vec() + } + + /// TC-001 — scheme-0 round-trip preserves plaintext. + #[test] + fn scheme0_round_trip_preserves_plaintext() { + let blob = wrap(&wid(1), "seed", None, b"top secret seed bytes").unwrap(); + let got = unwrap(&wid(1), "seed", None, blob.expose_secret()).unwrap(); + assert_eq!(got.expose_secret(), b"top secret seed bytes"); + } + + /// TC-002 — scheme-1 round-trip preserves plaintext. + #[test] + fn scheme1_round_trip_preserves_plaintext() { + let p = pw("hunter2"); + let blob = wrap_with_params( + &wid(7), + "seed", + Some(&p), + b"correct horse battery staple", + floor(), + ) + .unwrap(); + assert_ne!(blob.expose_secret(), b"correct horse battery staple"); + let got = unwrap(&wid(7), "seed", Some(&p), blob.expose_secret()).unwrap(); + assert_eq!(got.expose_secret(), b"correct horse battery staple"); + } + + /// TC-003 — scheme-1 produces a fresh salt + nonce per wrap. + #[test] + fn scheme1_uses_fresh_salt_and_nonce_per_wrap() { + let p = pw("pw"); + let a = scheme1_blob(&p); + let b = scheme1_blob(&p); + let (sa, na) = match decode(&a).payload { + Payload::Password { salt, nonce, .. } => (salt, nonce), + _ => panic!("scheme-1 wrap must yield Password"), + }; + let (sb, nb) = match decode(&b).payload { + Payload::Password { salt, nonce, .. } => (salt, nonce), + _ => panic!("scheme-1 wrap must yield Password"), + }; + assert_ne!(sa, sb, "salt must be fresh per wrap"); + assert_ne!(na, nb, "nonce must be fresh per wrap"); + } + + /// TC-004 — wrong object password yields WrongPassword. + #[test] + fn wrong_password_fails_closed() { + let blob = scheme1_blob(&pw("right")); + let err = unwrap(&wid(1), "seed", Some(&pw("wrong")), &blob).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// Mutate the `Payload::Password` body in-place via decode → patch + /// → encode. Returns the new blob. + fn mutate_scheme1( + blob: &[u8], + patch: impl FnOnce(&mut KdfParamsEncoded, &mut [u8; SALT_LEN], &mut [u8; NONCE_LEN]), + ) -> Vec { + let mut env = decode(blob); + match env.payload { + Payload::Password { + ref mut kdf, + ref mut salt, + ref mut nonce, + .. + } => patch(kdf, salt, nonce), + _ => panic!("mutate_scheme1 expects a Password payload"), + } + encode(&env) + } + + /// TC-005 — wire-flip of kdf.m_kib (in-bounds shift) yields WrongPassword. + #[test] + fn wire_flip_kdf_m_kib_fails_closed() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + kdf.m_kib = ARGON2_MIN_M_KIB + 1024; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TC-006 — wire-flip of kdf.t (in-bounds shift) yields WrongPassword. + #[test] + fn wire_flip_kdf_t_fails_closed() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + kdf.t = ARGON2_MIN_T + 1; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TC-007 — wire-flip of kdf.id to an unknown value is rejected by + /// `enforce_bounds` BEFORE `derive_key` allocates. + #[test] + fn wire_flip_kdf_id_unknown_rejected_pre_derive() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + kdf.id = 7; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!(matches!(err, SecretStoreError::KdfFailure), "got {err:?}"); + } + + /// TC-008 — wire-flip of salt[0] yields WrongPassword. + #[test] + fn wire_flip_salt_fails_closed() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let tampered = mutate_scheme1(&blob, |_, salt, _| { + salt[0] ^= 0x01; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TC-009 — wire-flip of nonce[0] yields WrongPassword. + #[test] + fn wire_flip_nonce_fails_closed() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let tampered = mutate_scheme1(&blob, |_, _, nonce| { + nonce[0] ^= 0x01; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TC-010 — re-binding the unwrap to a different wallet_id rejects. + #[test] + fn relocation_across_wallet_id_rejected() { + let p = pw("pw"); + let blob = wrap_with_params(&wid(0xA), "seed", Some(&p), b"seed", floor()).unwrap(); + let err = unwrap(&wid(0xB), "seed", Some(&p), blob.expose_secret()).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TC-011 — re-binding the unwrap to a different label rejects. + #[test] + fn relocation_across_label_rejected() { + let p = pw("pw"); + let blob = wrap_with_params(&wid(1), "labelA", Some(&p), b"seed", floor()).unwrap(); + let err = unwrap(&wid(1), "labelB", Some(&p), blob.expose_secret()).unwrap_err(); + assert!( + matches!(err, SecretStoreError::WrongPassword), + "got {err:?}" + ); + } + + /// TC-012 — wire-flip of envelope.version (via re-encode) is gated + /// to UnsupportedEnvelopeVersion before AAD bind. + #[test] + fn wire_flip_version_rejected_pre_aad() { + let blob = scheme1_blob(&pw("pw")); + let mut env = decode(&blob); + env.version = 2; + let tampered = encode(&env); + let err = unwrap(&wid(1), "seed", Some(&pw("pw")), &tampered).unwrap_err(); + assert!( + matches!( + err, + SecretStoreError::UnsupportedEnvelopeVersion { found: 2 } + ), + "got {err:?}" + ); + } + + /// TC-013 — forged `Payload::Unprotected` with ciphertext bytes + + /// `Some(pw)` redirects to ExpectedProtectedButUnsealed. + #[test] + fn wire_flip_scheme_dispatch_redirects_safely() { + let env = Envelope { + version: ENVELOPE_VERSION, + payload: Payload::Unprotected(vec![0xDEu8; 32]), + }; + let blob = encode(&env); + let err = unwrap(&wid(1), "seed", Some(&pw("pw")), &blob).unwrap_err(); + assert!( + matches!(err, SecretStoreError::ExpectedProtectedButUnsealed), + "got {err:?}" + ); + } + + /// TC-017 — truncated blob (< minimum envelope length) yields + /// Corruption. + #[test] + fn truncated_blob_yields_corruption() { + let blob = scheme1_blob(&pw("pw")); + let cut = blob.len() / 2; + let err = unwrap(&wid(1), "seed", Some(&pw("pw")), &blob[..cut]).unwrap_err(); + assert!(matches!(err, SecretStoreError::Corruption), "got {err:?}"); + } + + /// TC-018 — random-byte blob yields Corruption (both arms). + #[test] + fn random_garbage_yields_corruption() { + let garbage = b"NOTANEVELOPE........................."; + let err = unwrap(&wid(1), "seed", None, garbage).unwrap_err(); + assert!(matches!(err, SecretStoreError::Corruption), "got {err:?}"); + let err = unwrap(&wid(1), "seed", Some(&pw("pw")), garbage).unwrap_err(); + assert!(matches!(err, SecretStoreError::Corruption), "got {err:?}"); + } + + /// TC-019 — a manually-built envelope at version=2 fails closed + /// regardless of password. + #[test] + fn unsupported_version_rejected_for_any_password() { + let env = Envelope { + version: 2, + payload: Payload::Unprotected(b"x".to_vec()), + }; + let blob = encode(&env); + for arg in [None, Some(&pw("pw"))] { + let err = unwrap(&wid(1), "seed", arg, &blob).unwrap_err(); + assert!( + matches!( + err, + SecretStoreError::UnsupportedEnvelopeVersion { found: 2 } + ), + "got {err:?}" + ); + } + } + + /// TC-020 — a hand-crafted byte stream with an unknown payload + /// enum tag yields Corruption (bincode's natural fail-closed). + #[test] + fn unknown_scheme_discriminant_yields_corruption() { + // envelope.version = 1 (varint = 0x01) then a Payload enum tag + // of 7 (varint = 0x07) — the two-variant enum decode rejects. + let blob = [0x01u8, 0x07]; + let err = unwrap(&wid(1), "seed", None, &blob).unwrap_err(); + assert!(matches!(err, SecretStoreError::Corruption), "got {err:?}"); + } + + /// TC-021 — Some(pw) + scheme-0 yields ExpectedProtectedButUnsealed. + #[test] + fn some_pw_on_scheme0_fails_closed() { + let blob = wrap(&wid(1), "seed", None, b"attacker-seed").unwrap(); + let err = unwrap(&wid(1), "seed", Some(&pw("pw")), blob.expose_secret()).unwrap_err(); + assert!( + matches!(err, SecretStoreError::ExpectedProtectedButUnsealed), + "got {err:?}" + ); + } + + /// TC-022 — None + scheme-1 yields NeedsPassword. + #[test] + fn none_pw_on_scheme1_yields_needs_password() { + let blob = scheme1_blob(&pw("pw")); + let err = unwrap(&wid(1), "seed", None, &blob).unwrap_err(); + assert!( + matches!(err, SecretStoreError::NeedsPassword), + "got {err:?}" + ); + } + + /// TC-023 — inflated KDF param rejected by `enforce_bounds` before + /// `derive_key` allocates (a ~4 TiB allocation would OOM the test). + #[test] + fn kdf_enforce_bounds_rejects_before_derive() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + kdf.m_kib = u32::MAX; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!(matches!(err, SecretStoreError::KdfFailure), "got {err:?}"); + + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + kdf.t = ARGON2_MAX_T + 1; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!(matches!(err, SecretStoreError::KdfFailure), "got {err:?}"); + } + + /// TC-024 — per-read `default_target` ceiling rejects an envelope + /// whose `m_kib` exceeds the shipped target even when still inside + /// `enforce_bounds`. Catches inflated headers BEFORE `derive_key`. + #[test] + fn per_read_default_target_ceiling_rejects_inflated_header() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let bumped = KdfParams::default_target().m_kib * 2; + // Sanity: the bumped value stays inside the wider enforce_bounds + // ceiling, so only the per-read gate can refuse it. + assert!(bumped <= ARGON2_MAX_M_KIB); + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + kdf.m_kib = bumped; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!(matches!(err, SecretStoreError::KdfFailure), "got {err:?}"); + } + + /// Sibling to TC-024 on the `t` axis — per-read `default_target` + /// ceiling rejects an envelope whose `t` exceeds the shipped target + /// even when still inside `enforce_bounds` (`ARGON2_MAX_T = 16`). + /// Closes the CPU-axis gap that would otherwise let a forged header + /// run Argon2 at 5.3× the shipped iteration count. + #[test] + fn kdf_t_ceiling_fires_before_derive() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let target = KdfParams::default_target(); + let bumped_t = target.t + 1; + // Sanity: the bumped t stays inside the wider enforce_bounds + // ceiling, so only the per-read gate can refuse it. + assert!(bumped_t <= ARGON2_MAX_T); + let tampered = mutate_scheme1(&blob, |kdf, _, _| { + // Keep m_kib at the shipped default so the m_kib gate + // cannot fire — t must be the sole reason this rejects. + kdf.m_kib = target.m_kib; + kdf.t = bumped_t; + }); + let err = unwrap(&wid(1), "seed", Some(&p), &tampered).unwrap_err(); + assert!(matches!(err, SecretStoreError::KdfFailure), "got {err:?}"); + } + + /// Trailing bytes appended after a valid envelope are rejected as + /// `Corruption` — defends against a truncation/extension probe. + #[test] + fn decode_rejects_trailing_garbage() { + let p = pw("pw"); + let blob = scheme1_blob(&p); + let mut extended = blob.clone(); + extended.extend_from_slice(&[0xFFu8; 16]); + let err = unwrap(&wid(1), "seed", Some(&p), &extended).unwrap_err(); + assert!(matches!(err, SecretStoreError::Corruption), "got {err:?}"); + + // The same blob without the suffix still unwraps cleanly — + // proves the rejection is on the trailing bytes, not the + // envelope itself. + let ok = unwrap(&wid(1), "seed", Some(&p), &blob).unwrap(); + assert_eq!(ok.expose_secret(), b"seed"); + } + + /// TC-031 — round-tripped secret matches the original under a + /// constant-time compare. + #[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.expose_secret()).unwrap(); + assert!(bool::from(got.ct_eq(&original))); + } + + /// TC-035 (round-trip half) — scheme-1 at exact MAX_PLAINTEXT_LEN + /// round-trips and the enveloped bytes fit the backend cap. + #[test] + fn scheme1_at_cap_round_trips_within_backend_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); + let got = unwrap(&wid(1), "seed", Some(&p), blob.expose_secret()).unwrap(); + assert_eq!(got.expose_secret(), &pt[..]); + } + + /// TC-036 — value rollback is intentionally NOT defended. + #[test] + fn value_rollback_is_not_defended() { + let p = pw("pw"); + let old = wrap_with_params(&wid(1), "seed", Some(&p), b"OLD-VALUE", floor()).unwrap(); + let _new = wrap_with_params(&wid(1), "seed", Some(&p), b"NEW-VALUE", floor()).unwrap(); + let got = unwrap(&wid(1), "seed", Some(&p), old.expose_secret()).unwrap(); + assert_eq!(got.expose_secret(), b"OLD-VALUE"); + } + + /// TC-032 — random byte mutations and truncations never panic; + /// every outcome is a permitted typed variant. + #[test] + fn fuzz_byte_mutation_and_truncation_never_panics() { + let p = pw("fuzz-pw"); + let valid = scheme1_blob(&p); + // Pristine envelope unwraps cleanly. + assert_eq!( + unwrap(&wid(1), "seed", Some(&p), &valid) + .unwrap() + .expose_secret(), + b"seed" + ); + + 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(1), "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). + 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]); + } + } + + // TC-040 — proptest: no single-byte flip surfaces the plaintext. + // Minimises to the offset that breaks coverage if one exists. + proptest::proptest! { + #[test] + fn prop_single_byte_flip_never_yields_plaintext( + (offset, mask) in (0usize..200usize, 1u8..=255u8), + ) { + // Re-built per case so the proptest harness can shrink + // independently of the host RNG. + let plaintext: &[u8] = b"goldfinch"; + let p = pw("pw"); + let valid = wrap_with_params(&wid(1), "seed", Some(&p), plaintext, floor()) + .unwrap() + .expose_secret() + .to_vec(); + if offset >= valid.len() { + // Out-of-bounds offset → skip via prop_assume so proptest + // shrinks toward in-bounds offsets. + proptest::prop_assume!(offset < valid.len()); + } + let mut buf = valid.clone(); + buf[offset] ^= mask; + match unwrap(&wid(1), "seed", Some(&p), &buf) { + Ok(secret) => { + proptest::prop_assert_ne!( + secret.expose_secret(), + plaintext, + "single-byte flip at offset {} surfaced the plaintext", + offset + ); + } + Err(_) => { /* any typed error is fine */ } + } + } + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/wire/kdf.rs b/packages/rs-platform-wallet-storage/src/secrets/wire/kdf.rs new file mode 100644 index 0000000000..e869b29159 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/wire/kdf.rs @@ -0,0 +1,56 @@ +//! Bincode-encoded wire image of [`KdfParams`] — the Argon2 parameter +//! header read out of every scheme-1 envelope. +//! +//! Kept as a separate type from [`KdfParams`] (the in-memory + JSON- +//! vault type) so the wire layer owns its own bincode derives and the +//! in-memory type keeps its serde derives for the human-debuggable JSON +//! vault format. + +use crate::secrets::error::SecretStoreError; +use crate::secrets::file::crypto::KdfParams; + +/// Wire image of [`KdfParams`]: `id ‖ m_kib ‖ t ‖ p`, each a fixed- +/// width integer under the bincode varint config. Encoded once into +/// every scheme-1 envelope's `Payload::Password` body AND into the +/// scheme-1 AAD, so the two cannot disagree without failing the tag. +#[derive(bincode::Encode, bincode::Decode, Debug, PartialEq, Eq, Clone, Copy)] +pub(crate) struct KdfParamsEncoded { + /// Argon2 algorithm discriminator (only `KDF_ID_ARGON2ID = 1` + /// today; enforced by [`KdfParams::enforce_bounds`]). + pub id: u8, + /// Argon2 memory cost (KiB). Bounded. + pub m_kib: u32, + /// Argon2 time cost (iterations). Bounded. + pub t: u32, + /// Argon2 parallelism. Pinned to 1. + pub p: u32, +} + +impl From for KdfParamsEncoded { + fn from(k: KdfParams) -> Self { + Self { + id: k.id, + m_kib: k.m_kib, + t: k.t, + p: k.p, + } + } +} + +impl TryFrom for KdfParams { + type Error = SecretStoreError; + + /// Convert the wire image into the in-memory [`KdfParams`], gated on + /// [`KdfParams::enforce_bounds`] so an inflated header never + /// reaches `derive_key`. + fn try_from(k: KdfParamsEncoded) -> Result { + let out = KdfParams { + id: k.id, + m_kib: k.m_kib, + t: k.t, + p: k.p, + }; + out.enforce_bounds()?; + Ok(out) + } +} diff --git a/packages/rs-platform-wallet-storage/src/secrets/wire/mod.rs b/packages/rs-platform-wallet-storage/src/secrets/wire/mod.rs new file mode 100644 index 0000000000..d53ff9bbb9 --- /dev/null +++ b/packages/rs-platform-wallet-storage/src/secrets/wire/mod.rs @@ -0,0 +1,36 @@ +//! Bincode wire format for the Tier-2 envelope and the three AAD +//! constructions used inside `secrets/`. +//! +//! Every byte that crosses the AEAD seam — the on-disk Tier-2 blob and the +//! AAD bound into each ciphertext — is produced by a `#[derive(bincode:: +//! Encode)]` (or `Encode + Decode`) struct in this module, against the +//! single [`config::WIRE_CONFIG`] constant. A future bincode-config drift +//! is then caught by the golden vector tests in [`envelope::tests`] +//! instead of silently corrupting every stored blob. +//! +//! Module is `pub(crate)` only — the Tier-2 wire format is an +//! implementation detail of [`SecretStore`](super::store::SecretStore); +//! external callers see the unchanged `set_secret` / `get_secret` API. +//! +//! Audit-readable layout: +//! +//! - [`config`] — the single bincode config + domain-tag / version +//! constants every encoder uses. +//! - [`kdf`] — `KdfParamsEncoded`, the wire image of [`KdfParams`]. +//! - [`aad`] — the three AAD structs (`Tier2Aad` / `EntryAad` / +//! `VerifyAad`). +//! - [`envelope`] — the `Envelope` + `Payload` structs plus the +//! `wrap` / `unwrap` API. +//! +//! [`KdfParams`]: super::file::crypto::KdfParams +//! +//! Domain tags include an explicit `-v2` suffix to mark the +//! wire-format break from the pre-bincode hand-rolled layout +//! (`PWSEV-TIER2-AAD-v1` and the implicitly-untagged +//! `secrets/file/format.rs::aad` / `verify_aad` outputs). +#![deny(missing_docs)] + +pub(crate) mod aad; +pub(crate) mod config; +pub(crate) mod envelope; +pub(crate) mod kdf; diff --git a/packages/rs-platform-wallet-storage/src/sqlite/error.rs b/packages/rs-platform-wallet-storage/src/sqlite/error.rs index 17cd68daa3..4864709501 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/error.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/error.rs @@ -218,13 +218,6 @@ pub enum WalletStorageError { limit_bytes: usize, }, - /// An unspent UTXO named an address absent from - /// `core_derived_addresses`, so its account index can't be resolved; - /// persisting it would mis-file live funds, so the write is refused. - /// Spent-only placeholder rows tolerate a missing mapping. - #[error("unspent utxo address {address} is not in core_derived_addresses")] - UtxoAddressNotDerived { address: String }, - /// `PRAGMA foreign_keys = ON` was issued on open but the read-back /// reported the constraint enforcement is still off — the linked /// SQLite build silently ignores the pragma (no FK support compiled @@ -372,7 +365,6 @@ impl WalletStorageError { | Self::IdentityEntryIdMismatch | Self::AssetLockEntryMismatch { .. } | Self::BlobTooLarge { .. } - | Self::UtxoAddressNotDerived { .. } | Self::IntegerOverflow { .. } => false, } } @@ -449,7 +441,6 @@ impl WalletStorageError { Self::IdentityEntryIdMismatch => "identity_entry_id_mismatch", Self::AssetLockEntryMismatch { .. } => "asset_lock_entry_mismatch", Self::BlobTooLarge { .. } => "blob_too_large", - Self::UtxoAddressNotDerived { .. } => "utxo_address_not_derived", Self::IntegerOverflow { .. } => "integer_overflow", } } diff --git a/packages/rs-platform-wallet-storage/src/sqlite/persister.rs b/packages/rs-platform-wallet-storage/src/sqlite/persister.rs index 07b62afd97..9369fce02d 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/persister.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/persister.rs @@ -1053,9 +1053,10 @@ fn apply_changeset_to_tx( if !cs.account_registrations.is_empty() { schema::accounts::apply_registrations(tx, wallet_id, &cs.account_registrations)?; } - if !cs.account_address_pools.is_empty() { - schema::accounts::apply_pools(tx, wallet_id, &cs.account_address_pools)?; - } + // `account_address_pools` is intentionally NOT applied: UTXO attribution + // is hardcoded to the default account (index 0) in `core_state`, so the + // pool snapshot is no longer a storage input. The changeset field is kept + // for API stability and still feeds non-storage consumers. if let Some(core) = cs.core.as_ref() { schema::core_state::apply(tx, wallet_id, core)?; } diff --git a/packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs b/packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs index 0d929a6f57..7a71ad881e 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/schema/accounts.rs @@ -1,22 +1,21 @@ -//! `account_registrations` + `account_address_pools` writers + readers -//! (platform-payment registrations and the keyless account-manifest -//! reader). +//! `account_registrations` writer + keyless reader (platform-payment +//! registrations and the rehydration account-manifest oracle). use std::collections::BTreeMap; use key_wallet::bip32::ExtendedPubKey; use rusqlite::{params, Connection, Transaction}; -use platform_wallet::changeset::{AccountAddressPoolEntry, AccountRegistrationEntry}; +use platform_wallet::changeset::AccountRegistrationEntry; use platform_wallet::wallet::platform_wallet::WalletId; use crate::sqlite::error::WalletStorageError; use crate::sqlite::schema::blob; use crate::sqlite::schema::blob::impl_persistable_blob; -// PUBLIC material only: account-manifest types (account xpubs / pool -// snapshots) reaching `_blob` columns. -impl_persistable_blob!(AccountRegistrationEntry, AccountAddressPoolEntry); +// PUBLIC material only: the account-registration xpub manifest reaching +// the `account_xpub_bytes` blob column. +impl_persistable_blob!(AccountRegistrationEntry); /// Decoded `platform_payment` account registration: the DIP-17 account /// index and its extended public key, recovered from the bincode-serde @@ -125,37 +124,6 @@ pub fn apply_registrations( Ok(()) } -pub fn apply_pools( - tx: &Transaction<'_>, - wallet_id: &WalletId, - entries: &[AccountAddressPoolEntry], -) -> Result<(), WalletStorageError> { - if entries.is_empty() { - return Ok(()); - } - let mut stmt = tx.prepare_cached( - "INSERT INTO account_address_pools \ - (wallet_id, account_type, account_index, pool_type, snapshot_blob) \ - VALUES (?1, ?2, ?3, ?4, ?5) \ - ON CONFLICT(wallet_id, account_type, account_index, pool_type) DO UPDATE SET \ - snapshot_blob = excluded.snapshot_blob", - )?; - for entry in entries { - let account_type = account_type_db_label(&entry.account_type); - let account_index = account_index(&entry.account_type); - let pool_type = pool_type_db_label(&entry.pool_type); - let payload = blob::encode(entry)?; - stmt.execute(params![ - wallet_id.as_slice(), - account_type, - i64::from(account_index), - pool_type, - payload, - ])?; - } - Ok(()) -} - /// Read every `account_registrations` row for `wallet_id` into a keyless /// [`AccountRegistrationEntry`] manifest — the rehydration account-set oracle /// (which accounts to re-derive + the per-account xpubs the wrong-account gate @@ -181,14 +149,18 @@ pub fn load_state( Ok(out) } -/// Source of truth for the `account_type` TEXT domain across -/// `account_registrations`, `account_address_pools`, and -/// `core_derived_addresses`, mirroring [`key_wallet::account::AccountType`]. -/// `migrations/V001__initial.rs` interpolates it into each table's +/// Source of truth for the `account_registrations.account_type` TEXT domain, +/// mirroring [`key_wallet::account::AccountType`]. +/// `migrations/V001__initial.rs` interpolates it into the table's /// `CHECK (account_type IN (...))`; `account_type_labels_match_enum` keeps it /// in sync with [`account_type_db_label`]. +/// +/// `Standard` maps to two distinct labels by `StandardAccountType` variant +/// (`"standard_bip44"` / `"standard_bip32"`) so BIP44 and BIP32 standard +/// accounts with the same index never collide on their shared PK columns. pub(crate) const ACCOUNT_TYPE_LABELS: &[&str] = &[ - "standard", + "standard_bip44", + "standard_bip32", "coinjoin", "identity_registration", "identity_topup", @@ -205,23 +177,23 @@ pub(crate) const ACCOUNT_TYPE_LABELS: &[&str] = &[ "platform_payment", ]; -/// Single source of truth for the `account_address_pools.pool_type` -/// TEXT-column domain. -/// -/// Mirrors every variant of -/// [`key_wallet::managed_account::address_pool::AddressPoolType`] -/// (writer side: [`pool_type_db_label`]). See [`ACCOUNT_TYPE_LABELS`] -/// for the broader rationale and the parity-test contract. -pub(crate) const POOL_TYPE_LABELS: &[&str] = &["external", "internal", "absent", "absent_hardened"]; - /// Stable database label for an `AccountType` variant (the `Debug` impl is not -/// a stable format; this match is the contract). Variants sharing a label are -/// distinguished by the companion `account_index` column. An added upstream -/// variant fails this match's exhaustiveness check at compile time. +/// a stable format; this match is the contract). An added upstream variant +/// fails this match's exhaustiveness check at compile time. +/// +/// `Standard` maps to two distinct labels by `StandardAccountType` so BIP44 +/// and BIP32 accounts with the same `index` never collapse onto the same PK. pub(crate) fn account_type_db_label(at: &key_wallet::account::AccountType) -> &'static str { - use key_wallet::account::AccountType; + use key_wallet::account::{AccountType, StandardAccountType}; match at { - AccountType::Standard { .. } => "standard", + AccountType::Standard { + standard_account_type: StandardAccountType::BIP44Account, + .. + } => "standard_bip44", + AccountType::Standard { + standard_account_type: StandardAccountType::BIP32Account, + .. + } => "standard_bip32", AccountType::CoinJoin { .. } => "coinjoin", AccountType::IdentityRegistration => "identity_registration", AccountType::IdentityTopUp { .. } => "identity_topup", @@ -239,22 +211,8 @@ pub(crate) fn account_type_db_label(at: &key_wallet::account::AccountType) -> &' } } -/// Stable database label for an `AddressPoolType` variant. -pub(crate) fn pool_type_db_label( - pool: &key_wallet::managed_account::address_pool::AddressPoolType, -) -> &'static str { - use key_wallet::managed_account::address_pool::AddressPoolType; - match pool { - AddressPoolType::External => "external", - AddressPoolType::Internal => "internal", - AddressPoolType::Absent => "absent", - AddressPoolType::AbsentHardened => "absent_hardened", - } -} - /// Numeric account index embedded in an `AccountType`, persisted in the -/// `account_index` column of `account_registrations`, `account_address_pools`, -/// and `core_derived_addresses`. +/// `account_registrations.account_index` column. pub(crate) fn account_index(at: &key_wallet::account::AccountType) -> u32 { use key_wallet::account::AccountType; match at { @@ -282,7 +240,9 @@ mod tests { use std::collections::HashSet; /// Every [`key_wallet::account::AccountType`] variant; the wildcard-free - /// match below fails to compile if upstream adds one. + /// match below fails to compile if upstream adds one. `Standard` appears + /// twice — once per `StandardAccountType` — because both map to distinct + /// labels. fn all_account_type_variants() -> Vec { use key_wallet::account::{AccountType, StandardAccountType}; let variants = vec![ @@ -290,6 +250,10 @@ mod tests { index: 0, standard_account_type: StandardAccountType::BIP44Account, }, + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP32Account, + }, AccountType::CoinJoin { index: 0 }, AccountType::IdentityRegistration, AccountType::IdentityTopUp { @@ -340,25 +304,6 @@ mod tests { variants } - fn all_pool_type_variants() -> Vec { - use key_wallet::managed_account::address_pool::AddressPoolType; - let variants = vec![ - AddressPoolType::External, - AddressPoolType::Internal, - AddressPoolType::Absent, - AddressPoolType::AbsentHardened, - ]; - for v in &variants { - match v { - AddressPoolType::External - | AddressPoolType::Internal - | AddressPoolType::Absent - | AddressPoolType::AbsentHardened => {} - } - } - variants - } - #[test] fn account_type_labels_match_enum() { let from_writer: HashSet<&'static str> = all_account_type_variants() @@ -372,18 +317,4 @@ mod tests { from_const, from_writer ); } - - #[test] - fn pool_type_labels_match_enum() { - let from_writer: HashSet<&'static str> = all_pool_type_variants() - .iter() - .map(pool_type_db_label) - .collect(); - let from_const: HashSet<&'static str> = POOL_TYPE_LABELS.iter().copied().collect(); - assert_eq!( - from_writer, from_const, - "POOL_TYPE_LABELS ({:?}) drifted from pool_type_db_label codomain ({:?})", - from_const, from_writer - ); - } } diff --git a/packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs b/packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs index 1479e37530..8da392a5c7 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/schema/core_state.rs @@ -54,39 +54,15 @@ pub fn apply( ])?; } } - // Derived addresses are written before UTXOs (same tx) so the UTXO - // writer's address→account_index lookup sees the fresh rows. - if !cs.addresses_derived.is_empty() { - let mut stmt = tx.prepare_cached( - "INSERT INTO core_derived_addresses \ - (wallet_id, account_type, account_index, address, derivation_path, used) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6) \ - ON CONFLICT(wallet_id, account_type, address) DO UPDATE SET \ - account_index = excluded.account_index, \ - derivation_path = excluded.derivation_path", - )?; - for da in &cs.addresses_derived { - let account_type = - crate::sqlite::schema::accounts::account_type_db_label(&da.account_type); - let account_index = crate::sqlite::schema::accounts::account_index(&da.account_type); - let pool_type = crate::sqlite::schema::accounts::pool_type_db_label(&da.pool_type); - let address = da.address.to_string(); - let path = format!("{}/{}", pool_type, da.derivation_index); - stmt.execute(params![ - wallet_id.as_slice(), - account_type, - i64::from(account_index), - address, - path, - false - ])?; - } - } + // `addresses_derived` is intentionally NOT persisted here. The iOS + // address registry is fed by the FFI `addresses_derived` callback (fired + // before the UTXO changeset in the same round), and UTXO attribution is + // hardcoded to the default account (index 0); the storage layer keeps no + // derived-address lookup table. if !cs.new_utxos.is_empty() { let mut stmt = tx.prepare_cached(UPSERT_UTXO_SQL)?; - let mut lookup_stmt = tx.prepare_cached(ACCOUNT_INDEX_BY_ADDRESS_SQL)?; for utxo in &cs.new_utxos { - execute_upsert_utxo(&mut stmt, &mut lookup_stmt, wallet_id, utxo, false)?; + execute_upsert_utxo(&mut stmt, wallet_id, utxo, false)?; } } if !cs.spent_utxos.is_empty() { @@ -96,7 +72,6 @@ pub fn apply( "UPDATE core_utxos SET spent = 1 WHERE wallet_id = ?1 AND outpoint = ?2", )?; let mut upsert_stmt = tx.prepare_cached(UPSERT_UTXO_SQL)?; - let mut lookup_stmt = tx.prepare_cached(ACCOUNT_INDEX_BY_ADDRESS_SQL)?; for utxo in &cs.spent_utxos { let op = blob::encode_outpoint(&utxo.outpoint)?; let exists: bool = exists_stmt @@ -106,10 +81,11 @@ pub fn apply( if exists { mark_spent_stmt.execute(params![wallet_id.as_slice(), &op[..]])?; } else { - // Spent-only synthetic row: best-effort account_index. A wrong - // index is inert since spent rows are excluded from + // Spent-only synthetic row for a UTXO we never saw unspent. + // account_index is the hardcoded default like every row, and + // inert anyway since spent rows are excluded from // `list_unspent_utxos`. - execute_upsert_utxo(&mut upsert_stmt, &mut lookup_stmt, wallet_id, utxo, true)?; + execute_upsert_utxo(&mut upsert_stmt, wallet_id, utxo, true)?; } } } @@ -134,14 +110,6 @@ pub fn apply( Ok(()) } -/// Resolve a UTXO's owning account index via the `core_derived_addresses` map. -/// An address can be derived under multiple `account_type`s, so `ORDER BY` with -/// `LIMIT 1` makes the choice deterministic (SQLite would otherwise pick an -/// arbitrary matching row). -const ACCOUNT_INDEX_BY_ADDRESS_SQL: &str = "SELECT account_index FROM core_derived_addresses \ - WHERE wallet_id = ?1 AND address = ?2 \ - ORDER BY account_type, account_index LIMIT 1"; - const UPSERT_UTXO_SQL: &str = "INSERT INTO core_utxos \ (wallet_id, outpoint, value, script, height, account_index, spent, spent_in_txid) \ VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, NULL) \ @@ -152,45 +120,30 @@ const UPSERT_UTXO_SQL: &str = "INSERT INTO core_utxos \ account_index = excluded.account_index, \ spent = excluded.spent"; +/// Account index written for every `core_utxos` row. The product uses only +/// the default account (index 0); a non-default funds account causes +/// `core_bridge::warn_if_non_default_account` to emit a `warn!` log but +/// the record is still persisted under index 0 (dropping it would +/// undercount the balance and lose funds). The one reader +/// (`list_unspent_utxos` per-account grouping) groups everything under 0. +const CORE_UTXO_ACCOUNT_INDEX: i64 = 0; + +/// Upsert one `core_utxos` row. `account_index` is the hardcoded default +/// ([`CORE_UTXO_ACCOUNT_INDEX`]); `spent` marks spent-only synthetic rows. fn execute_upsert_utxo( stmt: &mut rusqlite::CachedStatement<'_>, - lookup_stmt: &mut rusqlite::CachedStatement<'_>, wallet_id: &WalletId, utxo: &Utxo, spent: bool, ) -> Result<(), WalletStorageError> { let op = blob::encode_outpoint(&utxo.outpoint)?; - let address = utxo.address.to_string(); - // `Utxo` carries no account index; recover it from the derived-address map. - let looked_up: Option = lookup_stmt - .query_row(params![wallet_id.as_slice(), &address], |row| row.get(0)) - .optional()?; - let account_index: i64 = match looked_up { - Some(idx) => idx, - // Refuse an unspent UTXO whose address we never derived — it would - // mis-bucket live money under account 0 and never re-derive. The - // spent-only path below tolerates the fallback (a wrong index is inert). - None if !spent => { - return Err(WalletStorageError::UtxoAddressNotDerived { - address: address.clone(), - }); - } - None => { - tracing::debug!( - wallet_id = %hex::encode(wallet_id), - address = %address, - "spent-only UTXO address not found in core_derived_addresses; using account_index 0 placeholder" - ); - 0 - } - }; stmt.execute(params![ wallet_id.as_slice(), &op[..], crate::sqlite::util::safe_cast::u64_to_i64("core_utxos.value", utxo.value())?, utxo.txout.script_pubkey.as_bytes(), i64::from(utxo.height), - account_index, + CORE_UTXO_ACCOUNT_INDEX, spent, ])?; Ok(()) 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::(); diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_account_zero_attribution.rs b/packages/rs-platform-wallet-storage/tests/sqlite_account_zero_attribution.rs new file mode 100644 index 0000000000..d604c48aa8 --- /dev/null +++ b/packages/rs-platform-wallet-storage/tests/sqlite_account_zero_attribution.rs @@ -0,0 +1,121 @@ +#![allow(clippy::field_reassign_with_default)] + +//! Genesis-rescan regression for the hardcoded `account_index = 0` design +//! (PR #3828). +//! +//! Before: a UTXO landing on a freshly-derived gap-limit-edge address could +//! race address-derivation persistence and be mis-attributed or dropped, so +//! the bridge smuggled a full pool snapshot in-band to resolve it. Now UTXO +//! attribution is hardcoded to the default account (index 0) at the storage +//! writer — no in-band snapshot, no address→account lookup table. This test +//! pins that a UTXO on a real gap-limit-edge address persists directly with +//! `account_index == 0`, contributes the exact balance, and never aborts +//! the flush. + +mod common; + +use common::{ensure_wallet_meta, fresh_persister, wid}; + +use key_wallet::wallet::initialization::WalletAccountCreationOptions; +use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; +use key_wallet::wallet::Wallet; +use key_wallet::AddressInfo; +use platform_wallet::changeset::{ + CoreChangeSet, PlatformWalletChangeSet, PlatformWalletPersistence, +}; +use platform_wallet::wallet::platform_wallet::WalletId; +use platform_wallet_storage::sqlite::schema::core_state; + +/// The LAST address in the wallet's Standard BIP44 external pool — the +/// gap-limit-edge address, the one most likely to be a fresh extension and +/// thus the worst case for the retired attribution race. +fn gap_limit_edge_address(seed_byte: u8) -> AddressInfo { + use key_wallet::account::AccountType; + use key_wallet::managed_account::address_pool::AddressPoolType; + + let wallet = Wallet::from_seed_bytes( + [seed_byte; 64], + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let info = ManagedWalletInfo::from_wallet(&wallet, 0); + + for managed in info.all_managed_accounts() { + let account_type = managed.managed_account_type().to_account_type(); + if !matches!(account_type, AccountType::Standard { index: 0, .. }) { + continue; + } + for pool in managed.managed_account_type().address_pools() { + if pool.pool_type != AddressPoolType::External || pool.addresses.is_empty() { + continue; + } + let infos: Vec = pool.addresses.values().cloned().collect(); + return infos.last().cloned().unwrap(); + } + } + panic!("wallet must expose a non-empty Standard BIP44 external pool"); +} + +fn utxo_at(addr: &dashcore::Address, vout: u32, value: u64) -> key_wallet::Utxo { + use dashcore::hashes::Hash; + key_wallet::Utxo { + outpoint: dashcore::OutPoint { + txid: dashcore::Txid::from_byte_array([0x7E; 32]), + vout, + }, + txout: dashcore::TxOut { + value, + script_pubkey: addr.script_pubkey(), + }, + address: addr.clone(), + height: 7, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + } +} + +/// A UTXO on a freshly-derived gap-limit-edge address persists directly with +/// the hardcoded `account_index == 0`: no snapshot, no lookup, no flush +/// abort, and the unspent balance is exact. +#[test] +fn utxo_on_fresh_gap_limit_address_persists_under_account_zero() { + let (persister, _tmp, _path) = fresh_persister(); + let w: WalletId = wid(0xD1); + ensure_wallet_meta(&persister, &w); + + let edge = gap_limit_edge_address(0x55); + let addr = edge.address.clone(); + + persister + .store( + w, + PlatformWalletChangeSet { + core: Some(CoreChangeSet { + new_utxos: vec![utxo_at(&addr, 0, 777_000)], + ..Default::default() + }), + ..Default::default() + }, + ) + .expect("a UTXO on a fresh gap-limit address must persist, not abort"); + + let conn = persister.lock_conn_for_test(); + let by_account = core_state::list_unspent_utxos(&conn, &w).unwrap(); + + let total: usize = by_account.values().map(|v| v.len()).sum(); + assert_eq!(total, 1, "the edge-address UTXO must persist"); + + let rows = by_account + .get(&0) + .expect("UTXO must be attributed to the default account (index 0)"); + assert_eq!( + rows.len(), + 1, + "exactly the one edge-address UTXO under account 0" + ); + assert_eq!(rows[0].value, 777_000, "value preserved"); +} diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_accounts_reader.rs b/packages/rs-platform-wallet-storage/tests/sqlite_accounts_reader.rs index a351cc0c7c..5839b6e097 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_accounts_reader.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_accounts_reader.rs @@ -108,7 +108,7 @@ fn a1_corrupt_blob_is_hard_error() { conn.execute( "INSERT INTO account_registrations \ (wallet_id, account_type, account_index, account_xpub_bytes) \ - VALUES (?1, 'standard', 0, X'00')", + VALUES (?1, 'standard_bip44', 0, X'00')", rusqlite::params![w.as_slice()], ) .unwrap(); diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_check_constraints.rs b/packages/rs-platform-wallet-storage/tests/sqlite_check_constraints.rs index 8321468b14..68eadff1d1 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_check_constraints.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_check_constraints.rs @@ -1,11 +1,7 @@ //! Smoke tests for the enum-domain `CHECK` constraints. The schema has -//! seven such TEXT columns across five domains (`account_type` is reused -//! by `account_registrations`, `account_address_pools`, and -//! `core_derived_addresses`). These tests exercise one column per -//! upstream-enum domain: `wallets.network`, -//! `account_registrations.account_type`, `account_address_pools.pool_type`, -//! and `asset_locks.status`. The synthetic `contacts.state` domain is not -//! exercised here. +//! four such TEXT columns across four domains: `wallets.network`, +//! `account_registrations.account_type`, `asset_locks.status`, and the +//! synthetic `contacts.state`. These tests exercise each directly. //! //! The per-module parity unit tests in `src/sqlite/schema/*` cover the //! Rust↔const-array equality. These tests cover the runtime half: a @@ -71,30 +67,6 @@ fn check_rejects_bad_account_type_on_registrations() { assert_constraint_check(res, "account_registrations.account_type"); } -#[test] -fn check_rejects_bad_pool_type() { - let (persister, _tmp, _path) = fresh_persister(); - let conn = persister.lock_conn_for_test(); - conn.execute( - "INSERT INTO wallets (wallet_id, network, birth_height) VALUES (?1, ?2, ?3)", - params![wid(3).as_slice(), "testnet", 0i64], - ) - .expect("seed wallets"); - let res = conn.execute( - "INSERT INTO account_address_pools \ - (wallet_id, account_type, account_index, pool_type, snapshot_blob) \ - VALUES (?1, ?2, ?3, ?4, ?5)", - params![ - wid(3).as_slice(), - "standard", - 0i64, - "not_a_pool", - &[0u8; 4][..] - ], - ); - assert_constraint_check(res, "account_address_pools.pool_type"); -} - #[test] fn check_rejects_bad_asset_lock_status() { let (persister, _tmp, _path) = fresh_persister(); @@ -137,3 +109,58 @@ fn check_accepts_every_known_label_network() { .unwrap_or_else(|e| panic!("network={label} should be accepted: {e}")); } } + +#[test] +fn check_rejects_bad_contact_state() { + let (persister, _tmp, _path) = fresh_persister(); + let conn = persister.lock_conn_for_test(); + // Seed a valid parent wallet so the insert trips the state CHECK, not the FK. + conn.execute( + "INSERT INTO wallets (wallet_id, network, birth_height) VALUES (?1, ?2, ?3)", + params![wid(7).as_slice(), "testnet", 0i64], + ) + .expect("seed wallets"); + let res = conn.execute( + "INSERT INTO contacts (wallet_id, owner_id, contact_id, state) \ + VALUES (?1, ?2, ?3, ?4)", + params![ + wid(7).as_slice(), + &[0xAAu8; 32][..], + &[0xBBu8; 32][..], + "not_a_contact_state" + ], + ); + assert_constraint_check(res, "contacts.state"); +} + +#[test] +fn check_accepts_every_known_contact_state_label() { + let (persister, _tmp, _path) = fresh_persister(); + let conn = persister.lock_conn_for_test(); + conn.execute( + "INSERT INTO wallets (wallet_id, network, birth_height) VALUES (?1, ?2, ?3)", + params![wid(8).as_slice(), "testnet", 0i64], + ) + .expect("seed wallets"); + // Mirrors `sqlite::schema::contacts::CONTACT_STATE_LABELS`; hardcoded + // because that const is `pub(crate)` and unreachable from this separate + // integration-test crate (same constraint as the network test above). + // The per-module `contact_state_labels_match_enum` unit test guards the + // const itself against drift, so a label added there without updating + // this list surfaces in that test, not as a silent gap here. + for (i, label) in ["sent", "received", "established"].iter().enumerate() { + // Same wallet+owner, distinct contact_id per label to keep the + // composite PK (wallet_id, owner_id, contact_id) unique. + conn.execute( + "INSERT INTO contacts (wallet_id, owner_id, contact_id, state) \ + VALUES (?1, ?2, ?3, ?4)", + params![ + wid(8).as_slice(), + &[0xC0u8; 32][..], + &[i as u8; 32][..], + *label + ], + ) + .unwrap_or_else(|e| panic!("contact state={label} should be accepted: {e}")); + } +} diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_core_state_reader.rs b/packages/rs-platform-wallet-storage/tests/sqlite_core_state_reader.rs index 8c39013416..1e619ee46b 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_core_state_reader.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_core_state_reader.rs @@ -15,11 +15,24 @@ use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::wallet::Wallet; use key_wallet::Utxo; use platform_wallet::changeset::{ - CoreChangeSet, PlatformWalletChangeSet, PlatformWalletPersistence, + AccountRegistrationEntry, CoreChangeSet, PlatformWalletChangeSet, PlatformWalletPersistence, }; use platform_wallet_storage::sqlite::schema::core_state; use platform_wallet_storage::WalletStorageError; +/// Keyless account manifest the rehydration path resolves xpubs from. +fn manifest_for(wallet: &Wallet) -> Vec { + wallet + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect() +} + fn reopen(path: &std::path::Path) -> platform_wallet_storage::SqlitePersister { platform_wallet_storage::SqlitePersister::open( platform_wallet_storage::SqlitePersisterConfig::new(path), @@ -63,31 +76,6 @@ fn wallet_and_utxo(seed: [u8; 64], value: u64, height: u32, vout: u32) -> (Walle (w, utxo) } -/// The `core_derived_addresses` row a real scan records before a UTXO on -/// `address` lands. The strict UTXO writer refuses an unspent UTXO whose -/// address was never derived, so every test paying a wallet address must -/// seed the matching derivation. The writer keys its lookup on -/// `(wallet_id, address)` only, so account_type/index/pubkey here are -/// inert placeholders — the address is the load-bearing field. -fn derived_for(address: &dashcore::Address) -> platform_wallet::DerivedAddress { - // Compressed secp256k1 generator point — a valid placeholder pubkey. - const PUBKEY_G: [u8; 33] = [ - 0x02, 0x79, 0xbe, 0x66, 0x7e, 0xf9, 0xdc, 0xbb, 0xac, 0x55, 0xa0, 0x62, 0x95, 0xce, 0x87, - 0x0b, 0x07, 0x02, 0x9b, 0xfc, 0xdb, 0x2d, 0xce, 0x28, 0xd9, 0x59, 0xf2, 0x81, 0x5b, 0x16, - 0xf8, 0x17, 0x98, - ]; - platform_wallet::DerivedAddress { - account_type: key_wallet::account::AccountType::Standard { - index: 0, - standard_account_type: key_wallet::account::StandardAccountType::BIP44Account, - }, - pool_type: key_wallet::managed_account::address_pool::AddressPoolType::External, - derivation_index: 0, - address: address.clone(), - public_key: dashcore::PublicKey::from_slice(&PUBKEY_G).expect("valid compressed pubkey"), - } -} - /// A non-zero balance survives store → drop → reopen → load, guarding /// against a silent-zero-balance reconstruction. #[test] @@ -101,7 +89,6 @@ fn rt2_nonzero_balance_survives_reopen() { let cs = PlatformWalletChangeSet { core: Some(CoreChangeSet { - addresses_derived: vec![derived_for(&utxo.address)], new_utxos: vec![utxo.clone()], last_processed_height: Some(200), synced_height: Some(200), @@ -128,8 +115,12 @@ fn rt2_nonzero_balance_survives_reopen() { // rehydration path) and assert the wallet balance is the persisted // amount — NOT a silent zero. let mut info = ManagedWalletInfo::from_wallet(&wallet, 1); - platform_wallet::manager::rehydrate::apply_persisted_core_state(&mut info, &core) - .expect("BIP44 reconstruction must not error"); + platform_wallet::manager::rehydrate::apply_persisted_core_state( + &mut info, + &manifest_for(&wallet), + &core, + ) + .expect("BIP44 reconstruction must not error"); let bal = WalletInfoInterface::balance(&info); let total = bal.confirmed() + bal.unconfirmed() + bal.immature() + bal.locked(); assert_eq!( @@ -156,7 +147,6 @@ fn b2_spent_utxo_excluded() { w, PlatformWalletChangeSet { core: Some(CoreChangeSet { - addresses_derived: vec![derived_for(&u_unspent.address)], new_utxos: vec![u_unspent.clone()], spent_utxos: vec![u_spent.clone()], ..Default::default() @@ -267,8 +257,7 @@ fn f2_no_bip44_wallet_nonzero_balance_survives_reopen() { w, PlatformWalletChangeSet { core: Some(CoreChangeSet { - addresses_derived: vec![derived_for(&utxo.address)], - new_utxos: vec![utxo], + new_utxos: vec![utxo.clone()], last_processed_height: Some(60), synced_height: Some(60), ..Default::default() @@ -286,8 +275,12 @@ fn f2_no_bip44_wallet_nonzero_balance_survives_reopen() { assert_eq!(core.new_utxos.len(), 1); let mut info = ManagedWalletInfo::from_wallet(&wallet, 1); - platform_wallet::manager::rehydrate::apply_persisted_core_state(&mut info, &core) - .expect("CoinJoin-only reconstruction must not error"); + platform_wallet::manager::rehydrate::apply_persisted_core_state( + &mut info, + &manifest_for(&wallet), + &core, + ) + .expect("CoinJoin-only reconstruction must not error"); let bal = WalletInfoInterface::balance(&info); let total = bal.confirmed() + bal.unconfirmed() + bal.immature() + bal.locked(); assert_eq!( diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs b/packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs index 4a2d11f8ec..dd235e1e4d 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_dashpay_overlay_contract.rs @@ -72,6 +72,7 @@ fn overlay_only_write_does_not_corrupt_load() { let mut core_cs = PlatformWalletChangeSet::default(); core_cs.wallet_metadata = Some(WalletMetadataEntry { network: key_wallet::Network::Testnet, + wallet_group_id: [0u8; 32], birth_height: 0, }); core_cs.core = Some(CoreChangeSet { diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_delete_partial_commit_window.rs b/packages/rs-platform-wallet-storage/tests/sqlite_delete_partial_commit_window.rs index f8ee46d83e..1d34a2b12d 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_delete_partial_commit_window.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_delete_partial_commit_window.rs @@ -23,6 +23,7 @@ fn full_changeset(synced: u32) -> PlatformWalletChangeSet { let mut cs = PlatformWalletChangeSet::default(); cs.wallet_metadata = Some(WalletMetadataEntry { network: Network::Testnet, + wallet_group_id: [0u8; 32], birth_height: 0, }); cs.core = Some(CoreChangeSet { diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs b/packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs index 7bc531f2fd..a0d0cf8a25 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_delete_real_apply_failure.rs @@ -22,6 +22,7 @@ fn full_changeset(synced: u32) -> PlatformWalletChangeSet { let mut cs = PlatformWalletChangeSet::default(); cs.wallet_metadata = Some(WalletMetadataEntry { network: Network::Testnet, + wallet_group_id: [0u8; 32], birth_height: 0, }); cs.core = Some(CoreChangeSet { diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_error_classification.rs b/packages/rs-platform-wallet-storage/tests/sqlite_error_classification.rs index 21e909fdf0..9c1e5fdb0d 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_error_classification.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_error_classification.rs @@ -157,9 +157,6 @@ fn samples() -> Vec { WalletStorageError::InvalidWalletIdLength { actual: 10 }, WalletStorageError::ConfigInvalid { reason: "bad knob" }, WalletStorageError::IdentityEntryIdMismatch, - WalletStorageError::UtxoAddressNotDerived { - address: "yMockAddress".into(), - }, // BincodeEncode / BincodeDecode / HashDecode / ConsensusCodec // need real upstream errors; omitted but covered by their arms. WalletStorageError::BlobDecode { @@ -245,7 +242,6 @@ fn tc_p2_005_is_transient_table() { (false, "asset_lock_entry_mismatch") } WalletStorageError::BlobTooLarge { .. } => (false, "blob_too_large"), - WalletStorageError::UtxoAddressNotDerived { .. } => (false, "utxo_address_not_derived"), WalletStorageError::ForeignKeysNotEnforced => (false, "foreign_keys_not_enforced"), WalletStorageError::JournalModeNotApplied { .. } => (false, "journal_mode_not_applied"), WalletStorageError::SchemaHistoryMalformed { .. } => { diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_fk_changeset_ordering.rs b/packages/rs-platform-wallet-storage/tests/sqlite_fk_changeset_ordering.rs index 546cf59d64..867a6f9ce3 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_fk_changeset_ordering.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_fk_changeset_ordering.rs @@ -249,6 +249,7 @@ fn wallets_anchor_and_children_in_same_changeset_commits() { PlatformWalletChangeSet { wallet_metadata: Some(WalletMetadataEntry { network: Network::Testnet, + wallet_group_id: [0u8; 32], birth_height: 0, }), identities: Some(identities_changeset(identity, Some(w))), diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs b/packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs index e5aa4b65ee..ff4d91744f 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_load_wiring.rs @@ -22,31 +22,6 @@ fn reopen(path: &std::path::Path) -> SqlitePersister { SqlitePersister::open(SqlitePersisterConfig::new(path)).expect("reopen") } -/// The `core_derived_addresses` row a real scan records before a UTXO on -/// `address` lands. The strict UTXO writer refuses an unspent UTXO whose -/// address was never derived, so a stored UTXO must carry its matching -/// derivation. The writer keys its lookup on `(wallet_id, address)` only -/// and the read side re-attributes by topology, so the account fields -/// here are inert placeholders — the address is the load-bearing field. -fn derived_for(address: &dashcore::Address) -> platform_wallet::DerivedAddress { - // Compressed secp256k1 generator point — a valid placeholder pubkey. - const PUBKEY_G: [u8; 33] = [ - 0x02, 0x79, 0xbe, 0x66, 0x7e, 0xf9, 0xdc, 0xbb, 0xac, 0x55, 0xa0, 0x62, 0x95, 0xce, 0x87, - 0x0b, 0x07, 0x02, 0x9b, 0xfc, 0xdb, 0x2d, 0xce, 0x28, 0xd9, 0x59, 0xf2, 0x81, 0x5b, 0x16, - 0xf8, 0x17, 0x98, - ]; - platform_wallet::DerivedAddress { - account_type: key_wallet::account::AccountType::Standard { - index: 0, - standard_account_type: key_wallet::account::StandardAccountType::BIP44Account, - }, - pool_type: key_wallet::managed_account::address_pool::AddressPoolType::External, - derivation_index: 0, - address: address.clone(), - public_key: dashcore::PublicKey::from_slice(&PUBKEY_G).expect("valid compressed pubkey"), - } -} - /// A registered wallet with UTXOs round-trips into the keyless `wallets` /// payload — manifest, network, birth height, core state. #[test] @@ -80,6 +55,7 @@ fn c1_load_populates_keyless_wallet_payload() { let reg = PlatformWalletChangeSet { wallet_metadata: Some(WalletMetadataEntry { network: key_wallet::Network::Testnet, + wallet_group_id: [0u8; 32], birth_height: 7, }), account_registrations: manifest.clone(), @@ -113,8 +89,7 @@ fn c1_load_populates_keyless_wallet_payload() { w, PlatformWalletChangeSet { core: Some(CoreChangeSet { - addresses_derived: vec![derived_for(&utxo.address)], - new_utxos: vec![utxo], + new_utxos: vec![utxo.clone()], last_processed_height: Some(50), synced_height: Some(50), ..Default::default() diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs b/packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs index 628d790c09..fb402eeb2f 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_migrations.rs @@ -86,12 +86,7 @@ fn tc027_smoke_insert_every_table() { // Labels must match the writer-side canonical strings — see the // CHECK constraint sourced from `ACCOUNT_TYPE_LABELS` in // `sqlite::schema::accounts`. - "INSERT INTO account_registrations (wallet_id, account_type, account_index, account_xpub_bytes) VALUES (?1, 'standard', 0, X'00')", - &[&wallet_id.as_slice()], - ), - ( - "account_address_pools", - "INSERT INTO account_address_pools (wallet_id, account_type, account_index, pool_type, snapshot_blob) VALUES (?1, 'standard', 0, 'external', X'00')", + "INSERT INTO account_registrations (wallet_id, account_type, account_index, account_xpub_bytes) VALUES (?1, 'standard_bip44', 0, X'00')", &[&wallet_id.as_slice()], ), ( @@ -109,11 +104,6 @@ fn tc027_smoke_insert_every_table() { "INSERT INTO core_instant_locks (wallet_id, txid, islock_blob) VALUES (?1, ?2, X'00')", &[&wallet_id.as_slice(), &txid], ), - ( - "core_derived_addresses", - "INSERT INTO core_derived_addresses (wallet_id, account_type, account_index, address, derivation_path, used) VALUES (?1, 'standard', 0, 'addr', '', 0)", - &[&wallet_id.as_slice()], - ), ( "core_sync_state", "INSERT INTO core_sync_state (wallet_id, last_processed_height, synced_height) VALUES (?1, NULL, NULL)", diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_object_metadata.rs b/packages/rs-platform-wallet-storage/tests/sqlite_object_metadata.rs index a2f748545d..1efd76e224 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_object_metadata.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_object_metadata.rs @@ -1016,12 +1016,10 @@ fn delete_wallet_leaves_no_surviving_rows() { let txid = vec![0x01u8; 32]; let outpoint = vec![0x02u8; 36]; let stmts: &[(&str, &[&dyn rusqlite::ToSql])] = &[ - ("INSERT INTO account_registrations (wallet_id, account_type, account_index, account_xpub_bytes) VALUES (?1, 'standard', 0, X'00')", &[&a.as_slice()]), - ("INSERT INTO account_address_pools (wallet_id, account_type, account_index, pool_type, snapshot_blob) VALUES (?1, 'standard', 0, 'external', X'00')", &[&a.as_slice()]), + ("INSERT INTO account_registrations (wallet_id, account_type, account_index, account_xpub_bytes) VALUES (?1, 'standard_bip44', 0, X'00')", &[&a.as_slice()]), ("INSERT INTO core_transactions (wallet_id, txid, finalized, record_blob) VALUES (?1, ?2, 0, X'00')", &[&a.as_slice(), &txid]), ("INSERT INTO core_utxos (wallet_id, outpoint, value, script, account_index, spent) VALUES (?1, ?2, 0, X'00', 0, 0)", &[&a.as_slice(), &outpoint]), ("INSERT INTO core_instant_locks (wallet_id, txid, islock_blob) VALUES (?1, ?2, X'00')", &[&a.as_slice(), &txid]), - ("INSERT INTO core_derived_addresses (wallet_id, account_type, account_index, address, derivation_path, used) VALUES (?1, 'standard', 0, 'addr', '', 0)", &[&a.as_slice()]), ("INSERT INTO core_sync_state (wallet_id, last_processed_height, synced_height) VALUES (?1, 1, 1)", &[&a.as_slice()]), ("INSERT INTO identity_keys (wallet_id, identity_id, key_id, public_key_blob, public_key_hash, derivation_blob) VALUES (?1, ?2, 0, X'00', X'00', NULL)", &[&a.as_slice(), &idy.as_slice()]), ("INSERT INTO platform_address_sync (wallet_id, sync_height, sync_timestamp, last_known_recent_block) VALUES (?1, 0, 0, 0)", &[&a.as_slice()]), @@ -1188,11 +1186,9 @@ fn delete_wallet_leaves_no_surviving_rows() { let wallet_scoped = [ "wallets", "account_registrations", - "account_address_pools", "core_transactions", "core_utxos", "core_instant_locks", - "core_derived_addresses", "core_sync_state", "identities", "contacts", diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_structural_hardening.rs b/packages/rs-platform-wallet-storage/tests/sqlite_structural_hardening.rs index 4c6d6ed4fc..9c79b36baf 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_structural_hardening.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_structural_hardening.rs @@ -116,38 +116,27 @@ fn make_utxo(addr: &Address, vout: u32, value: u64) -> Utxo { Utxo::new(outpoint, txout, addr.clone(), 10, false) } -/// UTXOs resolve their real `account_index` from the derived-address -/// map written earlier in the same transaction, instead of a hardcoded -/// 0. +/// Every `core_utxos` row is written with the hardcoded default +/// `account_index = 0` (the product uses only the default account; a +/// non-default account causes `core_bridge::warn_if_non_default_account` +/// to log a `warn!` but still persists the record under index 0 to avoid +/// fund loss), so the per-account grouping reader buckets every unspent +/// UTXO — at any address — under account 0. #[test] -fn multi_account_utxos_bucket_to_real_account() { +fn utxos_bucket_under_default_account_index_zero() { use platform_wallet_storage::sqlite::schema::core_state; let (persister, _tmp, _path) = fresh_persister(); let w: WalletId = wid(0xC7); ensure_wallet_meta(&persister, &w); - let addr_acct5 = p2pkh(0x05); - let addr_acct9 = p2pkh(0x09); + let addr_a = p2pkh(0x05); + let addr_b = p2pkh(0x09); { let mut conn = persister.lock_conn_for_test(); - // Pre-seed the derived-address map with two distinct accounts. - for (acct, addr) in [(5u32, &addr_acct5), (9u32, &addr_acct9)] { - conn.execute( - "INSERT INTO core_derived_addresses \ - (wallet_id, account_type, account_index, address, derivation_path, used) \ - VALUES (?1, 'standard', ?2, ?3, '0/0', 0)", - params![w.as_slice(), acct as i64, addr.to_string()], - ) - .unwrap(); - } - let cs = CoreChangeSet { - new_utxos: vec![ - make_utxo(&addr_acct5, 0, 1000), - make_utxo(&addr_acct9, 1, 2000), - ], + new_utxos: vec![make_utxo(&addr_a, 0, 1000), make_utxo(&addr_b, 1, 2000)], ..Default::default() }; let tx = conn.transaction().unwrap(); @@ -158,47 +147,20 @@ fn multi_account_utxos_bucket_to_real_account() { let conn = persister.lock_conn_for_test(); let by_account = core_state::list_unspent_utxos(&conn, &w).unwrap(); assert_eq!( - by_account.get(&5).map(|v| v.len()), - Some(1), - "account 5 should hold exactly one UTXO" + by_account.len(), + 1, + "all UTXOs bucket under a single (default) account" ); assert_eq!( - by_account.get(&9).map(|v| v.len()), - Some(1), - "account 9 should hold exactly one UTXO" - ); -} - -/// A NEW unspent UTXO whose address is absent from -/// `core_derived_addresses` cannot resolve an owning account, so the -/// write is refused with the typed `UtxoAddressNotDerived` instead of -/// silently mis-filing live funds under account 0. -#[test] -fn unspent_utxo_on_undeclared_address_is_rejected() { - use platform_wallet_storage::sqlite::schema::core_state; - - let (persister, _tmp, _path) = fresh_persister(); - let w: WalletId = wid(0xC8); - ensure_wallet_meta(&persister, &w); - - let addr_unknown = p2pkh(0xEE); - let mut conn = persister.lock_conn_for_test(); - let cs = CoreChangeSet { - new_utxos: vec![make_utxo(&addr_unknown, 0, 3000)], - ..Default::default() - }; - let tx = conn.transaction().unwrap(); - let err = core_state::apply(&tx, &w, &cs) - .expect_err("unspent UTXO on an undeclared address must error"); - assert!( - matches!(err, WalletStorageError::UtxoAddressNotDerived { .. }), - "expected UtxoAddressNotDerived, got {err:?}" + by_account.get(&0).map(|v| v.len()), + Some(2), + "both UTXOs are attributed to the default account (index 0)" ); } -/// A spent-only placeholder UTXO whose address was never derived still -/// persists with the account-0 fallback — spent rows are excluded from -/// the unspent set, so the placeholder index is inert. +/// A spent-only placeholder UTXO (no prior unspent row to mark) persists +/// with the hardcoded account 0 — spent rows are excluded from the unspent +/// set, so the index is inert. #[test] fn spent_only_utxo_on_undeclared_address_uses_zero_fallback() { use platform_wallet_storage::sqlite::schema::core_state; diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_wallet_db_identity.rs b/packages/rs-platform-wallet-storage/tests/sqlite_wallet_db_identity.rs index 61ea6b8fc3..ae3ef2ce92 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_wallet_db_identity.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_wallet_db_identity.rs @@ -60,6 +60,7 @@ fn fresh_wallet_db() -> (tempfile::TempDir, std::path::PathBuf) { let mut cs = PlatformWalletChangeSet::default(); cs.wallet_metadata = Some(WalletMetadataEntry { network: key_wallet::Network::Testnet, + wallet_group_id: [0u8; 32], birth_height: 0, }); cs.core = Some(CoreChangeSet { diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index b4c18917c4..cc8b705df9 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -963,8 +963,12 @@ pub struct PlatformWalletChangeSet { /// the merge policy (plain `Vec::extend`, dedup is the apply-side /// caller's job). pub account_registrations: Vec, - /// Address-pool snapshots emitted at wallet create (initial - /// gap-limit population) and on any pool extension / "used" flip. + /// Full address-pool snapshots: emitted once at wallet registration. + /// Incremental derivations are delivered via `core.addresses_derived` + /// (the `WalletEvent` bus / FFI path); no per-block in-band pool + /// snapshot is written. The storage persister intentionally ignores this + /// field (UTXO attribution is hardcoded to account 0); non-storage + /// consumers (e.g. the iOS FFI address registry) may still read it. /// See [`AccountAddressPoolEntry`] for the merge policy. pub account_address_pools: Vec, /// Shielded sub-wallet deltas: per-subwallet decrypted notes, diff --git a/packages/rs-platform-wallet/src/changeset/core_bridge.rs b/packages/rs-platform-wallet/src/changeset/core_bridge.rs index 46945667ef..a4f2cb32c3 100644 --- a/packages/rs-platform-wallet/src/changeset/core_bridge.rs +++ b/packages/rs-platform-wallet/src/changeset/core_bridge.rs @@ -41,6 +41,34 @@ use crate::changeset::changeset::{CoreChangeSet, PlatformWalletChangeSet}; use crate::changeset::traits::PlatformWalletPersistence; use crate::wallet::platform_wallet::PlatformWalletInfo; +/// Single-account observation. The storage writer hardcodes +/// `core_utxos.account_index = 0` (the product uses only the default +/// account, and that column drives only cosmetic per-account grouping). A +/// UTXO-bearing record owned by a non-default funds account is STILL +/// persisted under index 0 — never skipped, because skipping it would +/// undercount the wallet balance and lose funds. We only `warn!` so the +/// approximate grouping is visible. Identity/provider account types carry +/// no funds index (`AccountType::index() == None`) and never emit +/// `Received`/`Change` UTXOs, so they never warn. +fn warn_if_non_default_account(record: &TransactionRecord) { + if let Some(index) = non_default_account_index(record) { + tracing::warn!( + account_index = index, + txid = %record.txid, + "non-default account UTXO persisted under account_index 0; \ + per-account grouping is approximate" + ); + } +} + +/// The record's funds account index when it is a *non-default* (index != 0) +/// funds account, else `None`. Identity/provider account types carry no +/// funds index (`index() == None`) and never emit `Received`/`Change` +/// UTXOs, so they yield `None`. +fn non_default_account_index(record: &TransactionRecord) -> Option { + record.account_type.index().filter(|&index| index != 0) +} + /// Spawn the wallet-event subscriber task. /// /// Subscribes to `wallet_manager.subscribe_events()` from inside the @@ -129,18 +157,17 @@ async fn build_core_changeset( addresses_derived, .. } => { + // Persist regardless of account; warn on a non-default account. + warn_if_non_default_account(record); // Derive UTXO deltas before moving the record into `records` // so the per-record borrows are still live. CoreChangeSet { new_utxos: derive_new_utxos(record), spent_utxos: derive_spent_utxos(record), records: vec![(**record).clone()], - // Mirror the upstream-emitted derived addresses - // through to the persister so newly-extended pool - // rows are written transactionally with the tx that - // triggered the extension. See - // `CoreChangeSet.addresses_derived` for the cascade- - // link rationale. + // Forward the upstream-emitted derived addresses to the + // persister; the FFI layer feeds the iOS address registry + // from this delta. See `CoreChangeSet.addresses_derived`. addresses_derived: addresses_derived.clone(), ..CoreChangeSet::default() } @@ -171,11 +198,28 @@ async fn build_core_changeset( .. } => { let mut cs = CoreChangeSet::default(); - // Inserted records bring fresh UTXOs and may consume previous ones. + // Inserted records bring fresh UTXOs and may consume previous + // ones — always project. Non-default-account records are tallied + // and surfaced in a single aggregated warn after the loop (rather + // than one warn per record) to keep a busy block quiet. + let mut non_default_count = 0usize; + let mut non_default_sample: Option = None; for r in inserted { + if non_default_account_index(r).is_some() { + non_default_count += 1; + non_default_sample.get_or_insert(r.txid); + } cs.new_utxos.extend(derive_new_utxos(r)); cs.spent_utxos.extend(derive_spent_utxos(r)); } + if non_default_count > 0 { + tracing::warn!( + non_default_count, + sample_txid = ?non_default_sample, + "non-default account UTXO(s) persisted under account_index 0; \ + per-account grouping is approximate" + ); + } // Updated records (re-confirmation, IS-lock applied to a known // mempool tx, etc.) don't usually change UTXO topology — the // record's content does change though, so re-emit it. @@ -357,3 +401,120 @@ impl CoreChangeSet { && self.addresses_derived.is_empty() } } + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use dashcore::blockdata::transaction::Transaction; + use dashcore::hashes::Hash; + use key_wallet::account::{AccountType, StandardAccountType}; + use key_wallet::managed_account::transaction_record::{ + OutputDetail, TransactionDirection, TransactionRecord, + }; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext, TransactionType}; + use key_wallet::WalletCoreBalance; + + use super::*; + + fn standard(index: u32) -> AccountType { + AccountType::Standard { + index, + standard_account_type: StandardAccountType::BIP44Account, + } + } + + /// A throwaway testnet P2PKH address keyed off `seed`. + fn p2pkh(seed: u8) -> dashcore::Address { + use dashcore::address::Payload; + use dashcore::PubkeyHash; + dashcore::Address::new( + dashcore::Network::Testnet, + Payload::PubkeyHash(PubkeyHash::from_byte_array([seed; 20])), + ) + } + + /// A confirmed `TransactionRecord` owned by `account_type` carrying a + /// single `Received` output worth `value` at `addr`, so + /// `derive_new_utxos` yields exactly one UTXO. + fn record_with_received_output( + account_type: AccountType, + addr: &dashcore::Address, + value: u64, + ) -> TransactionRecord { + let tx = Transaction { + version: 3, + lock_time: 0, + input: vec![], + output: vec![dashcore::TxOut { + value, + script_pubkey: addr.script_pubkey(), + }], + special_transaction_payload: None, + }; + TransactionRecord::new( + tx, + account_type, + TransactionContext::InChainLockedBlock(BlockInfo::new( + 42, + dashcore::BlockHash::from_byte_array([3u8; 32]), + 1_735_689_600, + )), + TransactionType::Standard, + TransactionDirection::Incoming, + Vec::new(), + vec![OutputDetail { + index: 0, + role: OutputRole::Received, + address: Some(addr.clone()), + value, + }], + value as i64, + ) + } + + /// Project a `TransactionDetected` for `record` through the real bridge + /// path. `balance`/`account_balances` are unused by the projection. + async fn changeset_for(record: TransactionRecord) -> CoreChangeSet { + let wm = Arc::new(RwLock::new(WalletManager::::new( + key_wallet::Network::Testnet, + ))); + let event = WalletEvent::TransactionDetected { + wallet_id: [0u8; 32], + record: Box::new(record), + balance: WalletCoreBalance::default(), + account_balances: BTreeMap::new(), + addresses_derived: Vec::new(), + }; + build_core_changeset(&wm, &event).await + } + + /// A default-account (index 0) UTXO is projected into the changeset. + #[tokio::test] + async fn default_account_utxo_persists() { + let addr = p2pkh(0x11); + let cs = changeset_for(record_with_received_output(standard(0), &addr, 500_000)).await; + assert_eq!( + cs.new_utxos.len(), + 1, + "the default-account UTXO must be projected" + ); + assert_eq!(cs.new_utxos[0].value(), 500_000); + } + + /// REGRESSION (fund-loss): a non-default-account (index != 0) UTXO is + /// STILL projected — never dropped. Storage persists it under + /// `account_index 0`; the only cost is approximate per-account grouping + /// (a `warn!` is logged). Dropping it would undercount the balance. + #[tokio::test] + async fn non_default_account_utxo_persists_under_zero() { + let addr = p2pkh(0x22); + let cs = changeset_for(record_with_received_output(standard(7), &addr, 900_000)).await; + assert_eq!( + cs.new_utxos.len(), + 1, + "a non-default-account UTXO must NOT be dropped" + ); + assert_eq!(cs.new_utxos[0].value(), 900_000, "funds preserved"); + } +} diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index b6217c9839..9514f2db54 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -26,6 +26,22 @@ pub enum PlatformWalletError { utxo_count: usize, }, + /// The deep-index discovery probes did not mirror the account's real + /// address pools 1:1 during rehydration, so applying probe depths by + /// position would index the wrong pool. Fail-closed instead of risking + /// a misattributed derivation — the probes are built directly from the + /// same `address_pools()` enumeration, so a mismatch is a structural + /// invariant break, not user-reachable. + #[error( + "rehydration pool/probe mismatch: expected {expected} address pool(s) to mirror the discovery probes, found {found}" + )] + RehydrationPoolMismatch { + /// Number of discovery probes built from `address_pools()`. + expected: usize, + /// Number of real address pools from `address_pools_mut()`. + found: usize, + }, + #[error("Wallet not found: {0}")] WalletNotFound(String), diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index 1d8baf163b..6f4c118b03 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -120,9 +120,11 @@ impl PlatformWalletManager

{ // UTXOs but no funds account hard-fails here rather than // reconstructing a silent zero balance. let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, birth_height); - if let Err(e) = - super::rehydrate::apply_persisted_core_state(&mut wallet_info, &core_state) - { + if let Err(e) = super::rehydrate::apply_persisted_core_state( + &mut wallet_info, + &account_manifest, + &core_state, + ) { load_error = Some(e); break 'load; } diff --git a/packages/rs-platform-wallet/src/manager/platform_address_sync.rs b/packages/rs-platform-wallet/src/manager/platform_address_sync.rs index e1a229806c..30885258ba 100644 --- a/packages/rs-platform-wallet/src/manager/platform_address_sync.rs +++ b/packages/rs-platform-wallet/src/manager/platform_address_sync.rs @@ -97,6 +97,14 @@ pub struct PlatformAddressSyncManager { event_manager: Arc, /// Cancel token for the background loop, if running. background_cancel: StdMutex>, + /// Monotonically increasing generation counter. Bumped on every + /// `start()` so the exiting thread can tell whether its + /// generation is still the active one before clearing + /// `background_cancel`. Without this, a `stop()` → `start()` + /// overlap lets the prior thread's cleanup strip the new + /// generation's token, leaving the new loop running but + /// untrackable via `is_running()`. + background_generation: AtomicU64, interval_secs: AtomicU64, is_syncing: AtomicBool, /// Set by [`quiesce`](Self::quiesce) to gate new passes while it @@ -125,6 +133,7 @@ impl PlatformAddressSyncManager { wallets, event_manager, background_cancel: StdMutex::new(None), + background_generation: AtomicU64::new(0), interval_secs: AtomicU64::new(DEFAULT_SYNC_INTERVAL_SECS), is_syncing: AtomicBool::new(false), quiescing: AtomicBool::new(false), @@ -201,6 +210,7 @@ impl PlatformAddressSyncManager { } let cancel = CancellationToken::new(); *guard = Some(cancel.clone()); + let my_gen = self.background_generation.fetch_add(1, Ordering::AcqRel) + 1; drop(guard); let handle = tokio::runtime::Handle::current(); @@ -223,8 +233,17 @@ impl PlatformAddressSyncManager { } } + // Clear `background_cancel` only if the active + // generation is still ours. Acquire the lock + // FIRST, then check — `start()` bumps the + // generation while holding this same lock, so + // once we hold it the generation is final w.r.t. + // any concurrent token swap (no TOCTOU between + // the check and the clear). if let Ok(mut guard) = this.background_cancel.lock() { - *guard = None; + if this.background_generation.load(Ordering::Acquire) == my_gen { + *guard = None; + } } }); }) @@ -505,4 +524,56 @@ mod tests { assert_eq!(counter.completions.load(AtomicOrdering::SeqCst), 0); assert!(!mgr.is_syncing()); } + + /// Restart-in-place regression for the generation guard: a tight + /// `start()` → `stop()` → `start()` must leave the manager *running* + /// on the new generation. The cancelled gen-1 loop races to clear + /// `background_cancel` as it exits; the generation guard must stop it + /// from stripping gen-2's freshly installed token — otherwise the new + /// loop keeps running but becomes invisible to `is_running()` / + /// `stop()`. + /// + /// Determinism: the only wait is a *bounded* poll. With the guard in + /// place `is_running()` is true for the whole window, so the test + /// never fails spuriously on correct code. A regression flips it false + /// within milliseconds once the stale loop clears the slot, which the + /// poll catches. Needs the multi-thread flavor because `start()` + /// drives its loop via `Handle::current().block_on` on a dedicated OS + /// thread, which would deadlock a single-threaded test runtime. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn restart_in_place_keeps_running_after_stale_loop_exits() { + let (mgr, _counter) = make_manager(); + + // Gen 1. Wait (bounded) for the first pass to land — a real + // lifecycle signal that the loop is now parked in its interval + // sleep, so its cleanup is still pending when we stop+restart. + Arc::clone(&mgr).start(); + let mut waited = 0; + while mgr.last_sync_unix_seconds().is_none() { + assert!(waited < 200, "gen-1's first sync pass never completed"); + tokio::time::sleep(Duration::from_millis(10)).await; + waited += 1; + } + + // Tight stop→start with no await between: the just-cancelled gen-1 + // loop cannot reach its cleanup before gen 2 is installed, so the + // race window the guard protects is reliably open. + mgr.stop(); + Arc::clone(&mgr).start(); + + // Give the stale gen-1 loop ample time to run its (guarded) + // cleanup. `is_running()` must stay true throughout. + for _ in 0..100 { + assert!( + mgr.is_running(), + "stale gen-1 loop cleared gen-2's cancel token — generation guard regressed" + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } + + // The surviving loop is the tracked one: a single `stop()` fully + // reflects it, so there is no orphaned unreflectable duplicate. + mgr.stop(); + assert!(!mgr.is_running(), "stop() must reflect the live loop"); + } } diff --git a/packages/rs-platform-wallet/src/manager/rehydrate.rs b/packages/rs-platform-wallet/src/manager/rehydrate.rs index d5fca89a20..5dd7ae4b2c 100644 --- a/packages/rs-platform-wallet/src/manager/rehydrate.rs +++ b/packages/rs-platform-wallet/src/manager/rehydrate.rs @@ -89,6 +89,18 @@ pub(super) fn build_watch_only_wallet( /// Apply the keyless persisted core-state projection onto a /// freshly-minted `ManagedWalletInfo` skeleton. /// +/// # Parameters +/// +/// - `wallet_info`: the skeleton to hydrate in place. +/// - `manifest`: keyless account manifest (one entry per registered +/// account). Each entry carries an `account_type` → `account_xpub` +/// mapping used by [`extend_pools_for_restored_utxos`] to derive +/// addresses for restored UTXOs. If an account's `account_type` is +/// absent from the manifest, deep-index derivation is skipped for that +/// account (no xpub → no derivation possible); already-derived in-window +/// addresses are still marked used. +/// - `core`: the persisted core-state changeset to apply. +/// /// # Reconstructed (safety-critical-correct) /// /// - **Wallet balance** (`wallet_info.balance`, the no-silent-zero @@ -99,6 +111,10 @@ pub(super) fn build_watch_only_wallet( /// - **UTXO set**: every unspent persisted outpoint is restored into a /// funds-bearing account of the wallet (whatever topology it has — /// BIP44, BIP32, CoinJoin, DashPay). +/// - **Address-pool depth**: each pool is forward-derived to cover +/// restored UTXOs at deep derivation indices, then the gap window is +/// refilled beyond the deepest restored index so the per-address view +/// reconciles with the wallet total. /// - **Sync watermarks**: `synced_height` / `last_processed_height`. /// /// # Deferred to the first post-load `sync` (safe re-warm) @@ -109,6 +125,20 @@ pub(super) fn build_watch_only_wallet( /// first funds-bearing account and re-attributed on the next scan. /// The *wallet total* is unaffected (it is a sum across all funds /// accounts). +/// - **Deep-index address visibility**: each chain's pool scan stops +/// after [`MAX_REHYDRATION_DERIVATION_INDEX`] or after `gap_limit` +/// consecutive non-matching indices past the deepest resolved index. +/// The horizon only advances when an unspent UTXO anchors a match, so a +/// UTXO address can be left unresolved in two distinct cases: (1) it is +/// genuinely foreign (a different account's key routed here, or corrupt), +/// and (2) it is a *legitimately-owned but deep-and-sparse* address — +/// owned by this account, yet sitting past the first `gap_limit` window +/// with no nearer unspent UTXO to walk the horizon out to it. Both cases +/// are counted and logged via `tracing::warn!` and re-warm on the next +/// full sync. The wallet *total* stays exact (every UTXO is summed +/// regardless of pool visibility); only the per-address view is +/// incomplete until that sync. This is the accepted behavior of the +/// horizon-walk algorithm — see [`extend_pools_for_restored_utxos`]. /// - **`last_applied_chain_lock`**: not a persisted column (V001) and /// never written by the core-state writer; always `None` from disk. /// SPV re-applies a fresh chainlock on the first post-restart sync. @@ -127,13 +157,18 @@ pub(super) fn build_watch_only_wallet( /// than reconstructing a silent zero balance (the no-silent-zero /// mandate). An empty UTXO set is always `Ok`. /// -/// This never logs and never touches key material. +/// This never touches key material. pub fn apply_persisted_core_state( wallet_info: &mut ManagedWalletInfo, + manifest: &[AccountRegistrationEntry], core: &crate::changeset::CoreChangeSet, ) -> Result<(), PlatformWalletError> { use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + // Captured before the mutable account borrow below so it can flow into + // pool-extension diagnostics without re-borrowing `wallet_info`. + let wallet_id = wallet_info.wallet_id; + // Sync watermarks first so `update_balance`'s maturity check sees // the restored tip. if let Some(h) = core.last_processed_height { @@ -164,14 +199,17 @@ pub fn apply_persisted_core_state( .next() { Some(account) => { - for utxo in unspent { - account.utxos.insert(utxo.outpoint, utxo.clone()); + for utxo in &unspent { + account.utxos.insert(utxo.outpoint, (*utxo).clone()); } + // Eager derivation covers only `0..=gap_limit`; extend each + // chain to cover restored UTXOs at deeper indices. + extend_pools_for_restored_utxos(account, manifest, &unspent, wallet_id)?; } None => { return Err(PlatformWalletError::RehydrationTopologyUnsupported { - wallet_id: wallet_info.wallet_id, - utxo_count: core.new_utxos.len(), + wallet_id, + utxo_count: unspent.len(), }); } } @@ -184,6 +222,272 @@ pub fn apply_persisted_core_state( Ok(()) } +/// Upper bound on forward derivation while resolving a restored UTXO +/// address to its derivation index. Addresses that don't resolve within +/// this many indices (e.g. they belong to a different funds account whose +/// UTXOs were routed here, or are corrupt) are left for the next full +/// rescan to re-warm — generous enough to cover any realistic per-account +/// derivation depth. The common (single funds account) path terminates at +/// the true high-water mark well before this and never reaches the cap. +const MAX_REHYDRATION_DERIVATION_INDEX: u32 = 10_000; + +/// Soft threshold past which a single chain's discovery scan is treated as +/// abnormally deep and worth a `tracing::warn!`. Real funds chains anchor +/// well below this; reaching it means either a corrupt / foreign-heavy UTXO +/// set walking the horizon out, or an approach toward the hard +/// [`MAX_REHYDRATION_DERIVATION_INDEX`] ceiling — both worth surfacing. +const REHYDRATION_DEEP_SCAN_WARN_INDEX: u32 = 1_000; + +/// Extend `account`'s address pools so every resolved UTXO address is +/// derived at its exact `(chain, index)` slot and marked used, then refill +/// the gap window beyond — following the sync path's `mark_used` → +/// `maintain_gap_limit` sequence. Each chain is scanned independently, +/// stopping once no unresolved address matches within a `gap_limit`-sized +/// window past the deepest resolved index; [`MAX_REHYDRATION_DERIVATION_INDEX`] +/// is the hard ceiling. Addresses that don't resolve from this account's +/// xpub — foreign keys, multi-account mismatch, or legitimately-owned but +/// deep-and-sparse slots with no nearer unspent UTXO to anchor the horizon — +/// are counted and logged via `tracing::warn!`; they re-warm on the next +/// full sync. Every restored address the pools *do* hold (in-window or +/// deep-resolved) is marked used so a funded address is never handed out as +/// a fresh receive address. +/// +/// Tested with Standard BIP44 topology (External + Internal pools) and +/// CoinJoin topology (single External pool). The per-chain probe loop has no +/// topology-specific branches, so the non-hardened single-pool type +/// (`Absent`) follows the same code path with a different relative derivation +/// path. `AbsentHardened` pools cannot be derived from a public xpub at all — +/// hardened child derivation needs the private key — so under watch-only +/// rehydration their addresses never resolve and always defer to the next +/// sync (shared code path, but the outcome is "unresolved"). +/// +/// # Errors +/// +/// [`PlatformWalletError::RehydrationPoolMismatch`] if the discovery probes +/// don't mirror the real pools 1:1 (a structural invariant break, not +/// user-reachable). Fail-closed rather than apply a probe depth to the wrong +/// pool by position. +/// +/// Never touches key material — the xpub is the keyless account public key. +fn extend_pools_for_restored_utxos( + account: &mut key_wallet::managed_account::ManagedCoreFundsAccount, + manifest: &[AccountRegistrationEntry], + restored: &[&key_wallet::Utxo], + wallet_id: [u8; 32], +) -> Result<(), PlatformWalletError> { + use key_wallet::managed_account::address_pool::{AddressPool, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use std::collections::{BTreeSet, HashSet}; + + let account_type = account.managed_account_type().to_account_type(); + + // The funds account carries no key material; recover its watch-only xpub + // from the keyless manifest by account type. Without it we cannot derive + // deeper, but can still mark already-derived (in-window) addresses used. + let key_source = manifest + .iter() + .find(|e| e.account_type == account_type) + .map(|e| KeySource::Public(e.account_xpub)); + + // Probe pools mirror each real pool's chain 1:1 so the index search + // derives into throwaway state (real pools keep their own exact depth) + // and the resolved depth can be applied back by position. Re-deriving + // each probe from index 0 is an accepted, bounded one-time-load cost + // (per chain capped at MAX_REHYDRATION_DERIVATION_INDEX); rehydration + // runs once per wallet at startup, never on a hot path. + let mut probes: Vec<(AddressPool, BTreeSet)> = account + .managed_account_type() + .address_pools() + .iter() + .map(|p| { + ( + AddressPool::new_without_generation( + p.base_path.clone(), + p.pool_type, + p.gap_limit, + p.network, + ), + BTreeSet::new(), + ) + }) + .collect(); + + // Deep-index discovery (requires the xpub): resolve restored addresses the + // eager derivation didn't already cover, recording the matching index per + // chain. Each chain advances independently and stops once no unresolved + // address resolves within gap_limit indices past its deepest match + // (preventing a full scan when the UTXO set carries foreign addresses); + // MAX_REHYDRATION_DERIVATION_INDEX is the hard ceiling regardless. + if let Some(key_source) = key_source.as_ref() { + let mut unresolved: HashSet = { + let pools = account.managed_account_type().address_pools(); + restored + .iter() + .map(|u| u.address.clone()) + .filter(|addr| !pools.iter().any(|p| p.contains_address(addr))) + .collect() + }; + + for (probe, matched) in probes.iter_mut() { + if unresolved.is_empty() { + break; + } + let chain_gap = probe.gap_limit; + let mut deepest_resolved: Option = None; + let mut index: u32 = 0; + + loop { + // Horizon: gap_limit past the deepest match, or the initial + // gap_limit window when nothing has resolved yet. + let horizon = deepest_resolved + .map(|d| d.saturating_add(chain_gap)) + .unwrap_or(chain_gap); + + if index > horizon || index > MAX_REHYDRATION_DERIVATION_INDEX { + break; + } + + if let Some(addr) = ensure_derived(probe, key_source, index) { + if unresolved.remove(&addr) { + matched.insert(index); + deepest_resolved = Some(index); + } + } + + if unresolved.is_empty() { + break; + } + + index = index.saturating_add(1); + } + + // Surface an abnormally deep scan once per chain (outside the loop + // — never log inside the per-index walk). + if index > REHYDRATION_DEEP_SCAN_WARN_INDEX { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + account_type = ?account_type, + pool_type = ?probe.pool_type, + deepest_resolved = ?deepest_resolved, + scanned_to = index.saturating_sub(1), + "rehydration: chain discovery scanned abnormally deep — \ + likely a foreign-heavy or sparse UTXO set" + ); + } + } + + // Still-unresolved addresses are either foreign (a different account's + // key routed here, or corrupt) or legitimately-owned but deep-and-sparse + // (past the first gap window with no nearer unspent UTXO to anchor the + // horizon). Either way they re-warm on the next full sync; the wallet + // total is exact regardless. + if !unresolved.is_empty() { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + account_type = ?account_type, + unresolved_count = unresolved.len(), + "rehydration: UTXO address(es) unresolved for this account xpub \ + — will re-warm on next sync; balance total is exact" + ); + } + } + + // No explicit aggregate-derivation cap is needed: a funds account exposes + // a fixed, small number of chains (Standard = 2, others = 1), each already + // capped at MAX_REHYDRATION_DERIVATION_INDEX, so total derivation is bounded + // by chains × MAX with no unbounded growth — an aggregate cap would either + // equal that natural bound (no-op) or clip a legitimate deep multi-chain + // wallet. The per-chain ceiling plus the deep-scan warn above are the + // proportionate guard against a corrupt/foreign-heavy UTXO set. + + // Apply discovered depths and mark restored addresses used. `probes` is + // built directly from `address_pools()`, so it mirrors `address_pools_mut()` + // 1:1 and in chain order; verify that invariant before zipping by position. + let mut pools = account.managed_account_type_mut().address_pools_mut(); + if pools.len() != probes.len() { + return Err(PlatformWalletError::RehydrationPoolMismatch { + expected: probes.len(), + found: pools.len(), + }); + } + for (pool, (probe, matched)) in pools.iter_mut().zip(probes.iter()) { + // `iter_mut()` over `Vec<&mut AddressPool>` yields `&mut &mut _`; + // reborrow once so the pool flows into `ensure_derived` cleanly. + let pool: &mut AddressPool = pool; + debug_assert_eq!( + pool.pool_type, probe.pool_type, + "probe/pool chain order must match for by-position depth apply" + ); + + // Derive up to the deepest discovered index so its address exists in + // the real pool before we mark it used. + if let Some(&deepest) = matched.iter().next_back() { + if let Some(key_source) = key_source.as_ref() { + if ensure_derived(pool, key_source, deepest).is_none() { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + account_type = ?account_type, + pool_type = ?pool.pool_type, + index = deepest, + "rehydration: failed to derive resolved index into pool; \ + deferring its address to the next sync" + ); + } + } + } + + // Mark every restored address this pool now holds as used — covers both + // deep-resolved addresses (just derived) and in-window addresses the + // discovery scan never visits. Without this an already-derived but + // funded address keeps `used = false` and could be handed out as a fresh + // receive address. `mark_used` is a no-op for addresses not in this + // pool, so an underived (foreign / sparse) index is never marked. + let mut marked_any = false; + for u in restored { + if pool.mark_used(&u.address) { + marked_any = true; + } + } + + // Refill the gap window past the deepest used index (needs the xpub). + if marked_any { + if let Some(key_source) = key_source.as_ref() { + if let Err(e) = pool.maintain_gap_limit(key_source) { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + account_type = ?account_type, + pool_type = ?pool.pool_type, + error = %e, + "rehydration: gap-limit maintenance failed; pool window \ + may be short until the next sync" + ); + } + } + } + } + Ok(()) +} + +/// Ensure `pool` has derived through `index` (generating only the missing +/// tail), and return that index's address. `None` only on a derivation +/// error. +fn ensure_derived( + pool: &mut key_wallet::managed_account::address_pool::AddressPool, + key_source: &key_wallet::managed_account::address_pool::KeySource, + index: u32, +) -> Option { + let needs_more = match pool.highest_generated { + Some(highest) => highest < index, + None => true, + }; + if needs_more { + let start = pool.highest_generated.map(|h| h + 1).unwrap_or(0); + pool.generate_addresses(index - start + 1, key_source, true) + .ok()?; + } + pool.address_at_index(index) +} + #[cfg(test)] mod tests { use super::*; @@ -235,4 +539,755 @@ mod tests { .expect_err("empty manifest must be MissingManifest"); assert!(matches!(err, RehydrateRowError::MissingManifest)); } + + /// Regression: after restart-in-place the watch-only pools eagerly + /// cover only `0..=gap_limit`, but persisted UTXOs can sit at deeper + /// derivation indices. Rehydration must extend each chain's pool to its + /// deepest restored index so the per-address view reconciles with the + /// wallet total instead of undercounting. + /// + /// Index layout (gap_limit = 30): + /// - external idx 3: within eager window (not in `unresolved`), balance included + /// - external idx 30: first index past eager window; anchors the initial scan + /// window and extends it to idx 60 + /// - external idx 50: within extended window (50 < 60), resolved + /// - internal idx 30: within initial scan window, resolved + /// + /// Standard BIP44 topology (External + Internal pools) is exercised. + /// Asserts that maintain_gap_limit fills beyond the deepest resolved. + #[test] + fn rehydration_extends_pools_to_cover_deep_index_utxos() { + use dashcore::blockdata::transaction::txout::TxOut; + use dashcore::{OutPoint, Txid}; + use key_wallet::bip32::DerivationPath; + use key_wallet::gap_limit::DEFAULT_EXTERNAL_GAP_LIMIT; + use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::{Address, Utxo}; + use std::collections::HashSet; + + let seed = [7u8; 64]; + let wallet = Wallet::from_seed_bytes( + seed, + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let manifest = manifest_for(&wallet); + + // Mint the watch-only skeleton (pools cover only the eager gap + // window) and resolve the first funds account's keyless xpub. + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + let funds_type = wallet_info + .accounts + .all_funding_accounts() + .first() + .unwrap() + .managed_account_type() + .to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == funds_type) + .map(|e| e.account_xpub) + .expect("funds account xpub"); + + // Derive addresses on each chain from the same account xpub the + // pools use; `base_path` is record-keeping only and does not affect + // the derived address, so DerivationPath::master() is fine here. + let derive = |pool_type, index: u32| -> Address { + let mut p = AddressPool::new_without_generation( + DerivationPath::master(), + pool_type, + DEFAULT_EXTERNAL_GAP_LIMIT, + Network::Testnet, + ); + p.generate_addresses(index + 1, &KeySource::Public(xpub), true) + .unwrap(); + p.address_at_index(index).unwrap() + }; + + // idx 3: within eager window (0..=29) — covered by init, NOT in + // unresolved. Contributes to balance but needs no pool extension. + let shallow_recv = derive(AddressPoolType::External, 3); + // idx 30: first past eager window; falls in initial scan window + // (horizon = gap_limit = 30 on a chain with no prior matches). + // Anchors the external probe and extends horizon to 60. + let mid_recv = derive(AddressPoolType::External, 30); + // idx 50: within the extended window (50 < 30+30=60), resolved. + let deep_recv = derive(AddressPoolType::External, 50); + // idx 30: within the internal chain's initial scan window (<=30). + let deep_change = derive(AddressPoolType::Internal, 30); + + let utxo = |addr: Address, value: u64, n: u8| Utxo { + outpoint: OutPoint { + txid: Txid::from([n; 32]), + vout: 0, + }, + txout: TxOut { + value, + script_pubkey: addr.script_pubkey(), + }, + address: addr, + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }; + let new_utxos = vec![ + utxo(shallow_recv, 1_000, 1), + utxo(mid_recv.clone(), 10_000, 2), + utxo(deep_recv.clone(), 20_000, 3), + utxo(deep_change.clone(), 300_000, 4), + ]; + let expected_total: u64 = new_utxos.iter().map(|u| u.value()).sum(); + let core = crate::changeset::CoreChangeSet { + new_utxos, + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + + apply_persisted_core_state(&mut wallet_info, &manifest, &core).unwrap(); + + // The wallet total is exact regardless (a sum over the UTXO set). + assert_eq!(wallet_info.balance.total(), expected_total); + + // The per-address view joins pool addresses to UTXOs; every + // resolved UTXO address must now be derived into a pool. + let funds = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap(); + let pool_addresses: HashSet

= funds + .managed_account_type() + .address_pools() + .iter() + .flat_map(|p| p.addresses.values().map(|i| i.address.clone())) + .collect(); + let visible: u64 = funds + .utxos + .values() + .filter(|u| pool_addresses.contains(&u.address)) + .map(|u| u.value()) + .sum(); + assert_eq!( + visible, expected_total, + "all UTXO addresses (including deep-index) must be derived into their pools" + ); + + // Each deep address resolves to its exact derivation slot. + let pools = funds.managed_account_type().address_pools(); + let external = pools.iter().find(|p| p.is_external()).unwrap(); + let internal = pools.iter().find(|p| p.is_internal()).unwrap(); + assert_eq!(external.address_at_index(30).as_ref(), Some(&mid_recv)); + assert_eq!(external.address_at_index(50).as_ref(), Some(&deep_recv)); + assert_eq!(internal.address_at_index(30).as_ref(), Some(&deep_change)); + + // maintain_gap_limit must refill BEYOND the deepest restored + // index so the gap window is actually exercised, not just the restore. + // Deepest external resolved = idx 50; gap window must reach >= 50+30=80. + let expected_min_gen = 50 + DEFAULT_EXTERNAL_GAP_LIMIT; + assert!( + external.highest_generated >= Some(expected_min_gen), + "maintain_gap_limit must extend external pool to >= {} (got {:?})", + expected_min_gen, + external.highest_generated, + ); + } + + /// A UTXO whose address is not derivable from this account's + /// xpub (foreign key, multi-account mismatch) must not cause a panic or + /// hang. The total balance is exact (the UTXO is in the set regardless), + /// but the foreign address is absent from the pool so per-address + /// visibility is reduced. `tracing::warn!` fires for the unresolved count. + #[test] + fn rehydration_unresolvable_address_is_deferred_not_panics() { + use dashcore::blockdata::transaction::txout::TxOut; + use dashcore::{OutPoint, Txid}; + use key_wallet::bip32::DerivationPath; + use key_wallet::gap_limit::DEFAULT_EXTERNAL_GAP_LIMIT; + use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::{Address, Utxo}; + use std::collections::HashSet; + + let seed = [13u8; 64]; + let wallet = Wallet::from_seed_bytes( + seed, + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + + let funds_type = wallet_info + .accounts + .all_funding_accounts() + .first() + .unwrap() + .managed_account_type() + .to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == funds_type) + .map(|e| e.account_xpub) + .expect("funds account xpub"); + + // Normal UTXO at external index 3 (within eager window, pool-visible). + let normal_addr = { + let mut p = AddressPool::new_without_generation( + DerivationPath::master(), + AddressPoolType::External, + DEFAULT_EXTERNAL_GAP_LIMIT, + Network::Testnet, + ); + p.generate_addresses(4, &KeySource::Public(xpub), true) + .unwrap(); + p.address_at_index(3).unwrap() + }; + + // Foreign address: derive from a completely different wallet seed so + // it cannot be resolved from this wallet's xpub. + let foreign_addr = { + let fw = Wallet::from_seed_bytes( + [99u8; 64], + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let fw_info = ManagedWalletInfo::from_wallet(&fw, 1); + fw_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap() + .managed_account_type() + .address_pools() + .first() + .unwrap() + .address_at_index(0) + .unwrap() + }; + assert_ne!( + normal_addr, foreign_addr, + "test fixture: foreign address must differ from normal" + ); + + let utxo = |addr: Address, value: u64, n: u8| Utxo { + outpoint: OutPoint { + txid: Txid::from([n; 32]), + vout: 0, + }, + txout: TxOut { + value, + script_pubkey: addr.script_pubkey(), + }, + address: addr, + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }; + + let normal_val = 100_000u64; + let foreign_val = 200_000u64; + let expected_total = normal_val + foreign_val; + + let core = crate::changeset::CoreChangeSet { + new_utxos: vec![ + utxo(normal_addr, normal_val, 1), + utxo(foreign_addr, foreign_val, 2), + ], + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + + // Must not panic. tracing::warn! fires for the unresolved count. + apply_persisted_core_state(&mut wallet_info, &manifest, &core).unwrap(); + + // Total balance is exact — foreign UTXO is in the set regardless. + assert_eq!( + wallet_info.balance.total(), + expected_total, + "total must include foreign UTXO even though it is unresolved" + ); + + // Per-address visible: only the normal UTXO is in the pool. + let funds = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap(); + let pool_addresses: HashSet
= funds + .managed_account_type() + .address_pools() + .iter() + .flat_map(|p| p.addresses.values().map(|i| i.address.clone())) + .collect(); + let visible: u64 = funds + .utxos + .values() + .filter(|u| pool_addresses.contains(&u.address)) + .map(|u| u.value()) + .sum(); + assert_eq!( + visible, normal_val, + "only the non-foreign UTXO is pool-visible; foreign deferred to re-warm" + ); + assert!( + visible < expected_total, + "foreign UTXO is deferred — per-address visible < total" + ); + } + + /// CoinJoin topology (single External pool, no Internal chain). + /// Verifies that `extend_pools_for_restored_utxos` handles a single-pool + /// account at a deep derivation index (idx 30, just past the eager window). + #[test] + fn rehydration_coinjoin_single_pool_deep_index() { + use dashcore::blockdata::transaction::txout::TxOut; + use dashcore::{OutPoint, Txid}; + use key_wallet::managed_account::address_pool::{AddressPool, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::Utxo; + use std::collections::BTreeSet; + + // CoinJoin-only wallet: no BIP44, one CoinJoin account at index 0. + let mut cj_set = BTreeSet::new(); + cj_set.insert(0u32); + let opts = WalletAccountCreationOptions::SpecificAccounts( + BTreeSet::new(), + BTreeSet::new(), + cj_set, + BTreeSet::new(), + BTreeSet::new(), + None, + ); + let seed = [11u8; 64]; + let wallet = Wallet::from_seed_bytes(seed, Network::Testnet, opts).unwrap(); + assert!( + !wallet.accounts.coinjoin_accounts.is_empty(), + "fixture must have a CoinJoin account" + ); + + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + + // Extract pool metadata before the mutable borrow of wallet_info. + let (funds_type, pool_base_path, pool_type_val, pool_gap_limit) = { + let funds = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .expect("CoinJoin account must be the only funds account"); + let ft = funds.managed_account_type().to_account_type(); + let pools = funds.managed_account_type().address_pools(); + // CoinJoin has a single pool (External). + assert_eq!( + pools.len(), + 1, + "CoinJoin topology: must have exactly one pool" + ); + let p = &pools[0]; + (ft, p.base_path.clone(), p.pool_type, p.gap_limit) + }; + + let xpub = manifest + .iter() + .find(|e| e.account_type == funds_type) + .map(|e| e.account_xpub) + .expect("CoinJoin xpub must be in manifest"); + + // Derive the CoinJoin address at index 30 (first past the eager + // window 0..=29) using the real pool's base_path and pool_type. + let mut probe = AddressPool::new_without_generation( + pool_base_path, + pool_type_val, + pool_gap_limit, + Network::Testnet, + ); + probe + .generate_addresses(31, &KeySource::Public(xpub), true) + .unwrap(); + let deep_cj_addr = probe.address_at_index(30).unwrap(); + + let utxo_val = 7_777u64; + let utxo = Utxo { + outpoint: OutPoint { + txid: Txid::from([7u8; 32]), + vout: 0, + }, + txout: TxOut { + value: utxo_val, + script_pubkey: deep_cj_addr.script_pubkey(), + }, + address: deep_cj_addr.clone(), + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }; + + let core = crate::changeset::CoreChangeSet { + new_utxos: vec![utxo], + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + + apply_persisted_core_state(&mut wallet_info, &manifest, &core).unwrap(); + + // Balance is exact. + assert_eq!( + wallet_info.balance.total(), + utxo_val, + "CoinJoin deep-index balance must be exact" + ); + + // The CoinJoin pool was extended to include the deep-index address. + let funds_post = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap(); + let cj_pool = &funds_post.managed_account_type().address_pools()[0]; + assert_eq!( + cj_pool.address_at_index(30).as_ref(), + Some(&deep_cj_addr), + "CoinJoin pool must be extended to cover deep-index address at idx 30" + ); + } + + /// In-window restored UTXO: an address already covered by the eager + /// derivation (idx 3, inside `0..=gap_limit-1`) must still be marked + /// `used` during rehydration. The discovery scan never visits in-window + /// addresses, so without an explicit mark pass a funded address would keep + /// `used = false` and could later be handed out as a fresh receive address. + #[test] + fn rehydration_marks_in_window_restored_address_used() { + use dashcore::blockdata::transaction::txout::TxOut; + use dashcore::{OutPoint, Txid}; + use key_wallet::bip32::DerivationPath; + use key_wallet::gap_limit::DEFAULT_EXTERNAL_GAP_LIMIT; + use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::{Address, Utxo}; + + let wallet = Wallet::from_seed_bytes( + [5u8; 64], + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + + let funds_type = wallet_info + .accounts + .all_funding_accounts() + .first() + .unwrap() + .managed_account_type() + .to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == funds_type) + .map(|e| e.account_xpub) + .expect("funds account xpub"); + + // External idx 3 — inside the eager window, so NOT in the discovery set. + let in_window: Address = { + let mut p = AddressPool::new_without_generation( + DerivationPath::master(), + AddressPoolType::External, + DEFAULT_EXTERNAL_GAP_LIMIT, + Network::Testnet, + ); + p.generate_addresses(4, &KeySource::Public(xpub), true) + .unwrap(); + p.address_at_index(3).unwrap() + }; + + let core = crate::changeset::CoreChangeSet { + new_utxos: vec![Utxo { + outpoint: OutPoint { + txid: Txid::from([1u8; 32]), + vout: 0, + }, + txout: TxOut { + value: 12_345, + script_pubkey: in_window.script_pubkey(), + }, + address: in_window.clone(), + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }], + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + + apply_persisted_core_state(&mut wallet_info, &manifest, &core).unwrap(); + + let funds = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap(); + let pools = funds.managed_account_type().address_pools(); + let external = pools.iter().find(|p| p.is_external()).unwrap(); + let info = external + .address_info(&in_window) + .expect("in-window address must be present in the pool"); + assert!( + info.used, + "in-window restored UTXO address must be marked used" + ); + assert!( + external.used_indices.contains(&3), + "used_indices must record the in-window slot" + ); + assert_eq!( + external.highest_used, + Some(3), + "highest_used must reflect the in-window slot" + ); + } + + /// Documented limitation (solution b): a legitimately-owned but + /// deep-and-sparse UTXO — external idx 45 with nothing unspent at idx + /// <= 30 — is left unresolved because the discovery horizon (gap_limit + /// past the deepest match) never advances far enough to reach it. The + /// wallet total stays exact; only the per-address view is incomplete + /// until the next sync (a `tracing::warn!` records the deferral). + #[test] + fn rehydration_deep_sparse_utxo_left_unresolved_total_exact() { + use dashcore::blockdata::transaction::txout::TxOut; + use dashcore::{OutPoint, Txid}; + use key_wallet::bip32::DerivationPath; + use key_wallet::gap_limit::DEFAULT_EXTERNAL_GAP_LIMIT; + use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::{Address, Utxo}; + use std::collections::HashSet; + + let wallet = Wallet::from_seed_bytes( + [21u8; 64], + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + + let funds_type = wallet_info + .accounts + .all_funding_accounts() + .first() + .unwrap() + .managed_account_type() + .to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == funds_type) + .map(|e| e.account_xpub) + .expect("funds account xpub"); + + // External idx 45 — past the eager window AND past the initial scan + // window (horizon = gap_limit = 30 with no nearer match to extend it). + let sparse_deep: Address = { + let mut p = AddressPool::new_without_generation( + DerivationPath::master(), + AddressPoolType::External, + DEFAULT_EXTERNAL_GAP_LIMIT, + Network::Testnet, + ); + p.generate_addresses(46, &KeySource::Public(xpub), true) + .unwrap(); + p.address_at_index(45).unwrap() + }; + + let value = 500_000u64; + let core = crate::changeset::CoreChangeSet { + new_utxos: vec![Utxo { + outpoint: OutPoint { + txid: Txid::from([4u8; 32]), + vout: 0, + }, + txout: TxOut { + value, + script_pubkey: sparse_deep.script_pubkey(), + }, + address: sparse_deep.clone(), + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }], + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + + apply_persisted_core_state(&mut wallet_info, &manifest, &core).unwrap(); + + // The wallet total is exact regardless (a sum over the UTXO set). + assert_eq!(wallet_info.balance.total(), value); + + let funds = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap(); + let pools = funds.managed_account_type().address_pools(); + let external = pools.iter().find(|p| p.is_external()).unwrap(); + assert!( + !external.contains_address(&sparse_deep), + "deep-sparse idx 45 must be left unresolved (absent from the pool)" + ); + + // Per-address view: the deep-sparse UTXO is not pool-visible yet. + let pool_addresses: HashSet
= pools + .iter() + .flat_map(|p| p.addresses.values().map(|i| i.address.clone())) + .collect(); + let visible: u64 = funds + .utxos + .values() + .filter(|u| pool_addresses.contains(&u.address)) + .map(|u| u.value()) + .sum(); + assert_eq!( + visible, 0, + "the deep-sparse UTXO is deferred — not pool-visible until next sync" + ); + assert!(visible < value, "per-address visible < exact total"); + } + + /// Topology guard: a wallet with persisted UTXOs but NO funds-bearing + /// account cannot hold them — fail closed with + /// `RehydrationTopologyUnsupported` (reporting the persisted count) rather + /// than reconstruct a silent zero balance. + #[test] + fn rehydration_utxos_without_funds_account_errors() { + use dashcore::address::Payload; + use dashcore::blockdata::transaction::txout::TxOut; + use dashcore::hashes::Hash; + use dashcore::{OutPoint, PubkeyHash, Txid}; + use key_wallet::account::AccountType; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::{Address, Utxo}; + use std::collections::BTreeSet; + + // Keys-only wallet: a single IdentityRegistration account, no funds. + let opts = WalletAccountCreationOptions::SpecificAccounts( + BTreeSet::new(), + BTreeSet::new(), + BTreeSet::new(), + BTreeSet::new(), + BTreeSet::new(), + Some(vec![AccountType::IdentityRegistration]), + ); + let wallet = Wallet::from_seed_bytes([23u8; 64], Network::Testnet, opts).unwrap(); + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + assert!( + wallet_info.accounts.all_funding_accounts().is_empty(), + "fixture must have NO funds-bearing account" + ); + + let addr = Address::new( + Network::Testnet, + Payload::PubkeyHash(PubkeyHash::from_byte_array([9u8; 20])), + ); + let core = crate::changeset::CoreChangeSet { + new_utxos: vec![Utxo { + outpoint: OutPoint { + txid: Txid::from([2u8; 32]), + vout: 0, + }, + txout: TxOut { + value: 800_000, + script_pubkey: addr.script_pubkey(), + }, + address: addr, + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }], + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + + let err = apply_persisted_core_state(&mut wallet_info, &manifest, &core) + .expect_err("must fail closed when no funds account can hold the UTXOs"); + match err { + PlatformWalletError::RehydrationTopologyUnsupported { utxo_count, .. } => { + assert_eq!(utxo_count, 1, "utxo_count must match the persisted set"); + } + other => panic!("expected RehydrationTopologyUnsupported, got {other:?}"), + } + } + + /// Companion to the topology guard: the same keys-only wallet with an + /// EMPTY persisted UTXO set is `Ok` — there is nothing to hold, so the + /// guard does not trip. + #[test] + fn rehydration_no_funds_account_empty_utxos_ok() { + use key_wallet::account::AccountType; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use std::collections::BTreeSet; + + let opts = WalletAccountCreationOptions::SpecificAccounts( + BTreeSet::new(), + BTreeSet::new(), + BTreeSet::new(), + BTreeSet::new(), + BTreeSet::new(), + Some(vec![AccountType::IdentityRegistration]), + ); + let wallet = Wallet::from_seed_bytes([24u8; 64], Network::Testnet, opts).unwrap(); + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + assert!(wallet_info.accounts.all_funding_accounts().is_empty()); + + let core = crate::changeset::CoreChangeSet { + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + apply_persisted_core_state(&mut wallet_info, &manifest, &core) + .expect("empty UTXO set must be Ok even with no funds account"); + } } diff --git a/packages/rs-platform-wallet/src/manager/shielded_sync.rs b/packages/rs-platform-wallet/src/manager/shielded_sync.rs index 482674b432..e7892e8dff 100644 --- a/packages/rs-platform-wallet/src/manager/shielded_sync.rs +++ b/packages/rs-platform-wallet/src/manager/shielded_sync.rs @@ -261,14 +261,15 @@ impl ShieldedSyncManager { } } - // Only clear `background_cancel` if the active - // generation is still ours. Without this guard a - // tight `stop()` → `start()` reschedule has the - // exiting thread overwrite the *new* generation's - // token, leaving the new loop running but - // unreflectable via `is_running()` / `stop()`. - if this.background_generation.load(Ordering::Acquire) == my_gen { - if let Ok(mut guard) = this.background_cancel.lock() { + // Clear `background_cancel` only if the active + // generation is still ours. Acquire the lock + // FIRST, then check — `start()` bumps the + // generation while holding this same lock, so + // once we hold it the generation is final w.r.t. + // any concurrent token swap (no TOCTOU between + // the check and the clear). + if let Ok(mut guard) = this.background_cancel.lock() { + if this.background_generation.load(Ordering::Acquire) == my_gen { *guard = None; } } @@ -475,3 +476,75 @@ impl std::fmt::Debug for ShieldedSyncManager { .finish() } } + +#[cfg(test)] +mod tests { + use super::*; + + use crate::events::PlatformEventHandler; + + /// Build a manager with an empty coordinator slot and a no-handler + /// event manager. An empty slot makes `sync_now` return an empty + /// summary, but it still drives the full timestamp + completion + /// protocol and — crucially for this test — the generation-guarded + /// background loop, without needing a live `NetworkShieldedCoordinator`. + fn make_manager() -> Arc { + let event_manager = Arc::new(PlatformEventManager::new(Vec::< + Arc, + >::new())); + let coordinator_slot = Arc::new(RwLock::new(None)); + Arc::new(ShieldedSyncManager::new(event_manager, coordinator_slot)) + } + + /// Restart-in-place regression for the generation guard: a tight + /// `start()` → `stop()` → `start()` must leave the manager *running* + /// on the new generation. The cancelled gen-1 loop races to clear + /// `background_cancel` as it exits; the generation guard must stop it + /// from stripping gen-2's freshly installed token — otherwise the new + /// loop keeps running but becomes invisible to `is_running()` / + /// `stop()`. + /// + /// Determinism: the only wait is a *bounded* poll. With the guard in + /// place `is_running()` is true for the whole window, so the test + /// never fails spuriously on correct code. A regression flips it false + /// within milliseconds once the stale loop clears the slot, which the + /// poll catches. Needs the multi-thread flavor because `start()` + /// drives its loop via `Handle::current().block_on` on a dedicated OS + /// thread, which would deadlock a single-threaded test runtime. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn restart_in_place_keeps_running_after_stale_loop_exits() { + let mgr = make_manager(); + + // Gen 1. Wait (bounded) for the first pass to land — a real + // lifecycle signal that the loop is now parked in its interval + // sleep, so its cleanup is still pending when we stop+restart. + Arc::clone(&mgr).start(); + let mut waited = 0; + while mgr.last_sync_unix_seconds().is_none() { + assert!(waited < 200, "gen-1's first sync pass never completed"); + tokio::time::sleep(Duration::from_millis(10)).await; + waited += 1; + } + + // Tight stop→start with no await between: the just-cancelled gen-1 + // loop cannot reach its cleanup before gen 2 is installed, so the + // race window the guard protects is reliably open. + mgr.stop(); + Arc::clone(&mgr).start(); + + // Give the stale gen-1 loop ample time to run its (guarded) + // cleanup. `is_running()` must stay true throughout. + for _ in 0..100 { + assert!( + mgr.is_running(), + "stale gen-1 loop cleared gen-2's cancel token — generation guard regressed" + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } + + // The surviving loop is the tracked one: a single `stop()` fully + // reflects it, so there is no orphaned unreflectable duplicate. + mgr.stop(); + assert!(!mgr.is_running(), "stop() must reflect the live loop"); + } +}