From 3fa32124c4585d6f833c0024a159cdc19f192161 Mon Sep 17 00:00:00 2001 From: UnbreakableMJ Date: Tue, 23 Jun 2026 01:50:38 +0300 Subject: [PATCH] feat(agent,core): organization-key support; tui: Space reveals from any pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes from daily-driving Vault against a Bitwarden-cloud account whose vault is almost entirely organization/Collection-owned. Organization-key support (the bulk of the change): - Vault decrypted only personal ciphers (and per-cipher keys under the user key); organization items — ~99% of a Collections-heavy vault — were skipped, their fields being encrypted under an org key Vault didn't hold. At unlock the agent now recovers the account RSA private key from `profile.privateKey`, RSA-OAEP-SHA1-unwraps each `profile.organizations[].key` into that org's symmetric key, and routes every cipher's decryption by `organization_id` (org key vs user key). Works online and from the offline cache. New `vault_core::org_key` module + `rsa` dependency (`ring` has no RSA decryption); justified RUSTSEC-2023-0071 ignore in deny.toml / CI. Items whose org key can't be unwrapped are still skipped; editing org items is refused for now (the write path would re-encrypt under the wrong key). TUI Space-to-reveal: - `Space` revealed a login's password from the item list but did nothing in the detail pane (logins have no per-field rows) or the folder pane. Target resolution moved into a testable `App::reveal_target`, so `Space` reveals from all three panes; cards/identities still reveal the cursor-selected masked field in the detail pane. Tests: RSA round-trip, org-cipher decrypt + edit-refusal, reveal_target across panes/types. Full `just ci` green; verified live (2274 items decrypt). Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 6 +- CHANGELOG.md | 22 ++++ Cargo.lock | 156 +++++++++++++++++++++++++++++ Cargo.toml | 6 ++ crates/vault-agent/src/state.rs | 112 +++++++++++++++++++-- crates/vault-agent/src/unlock.rs | 55 ++++++++++ crates/vault-api/src/sync.rs | 30 ++++++ crates/vault-core/Cargo.toml | 1 + crates/vault-core/src/error.rs | 4 + crates/vault-core/src/lib.rs | 2 + crates/vault-core/src/org_key.rs | 166 +++++++++++++++++++++++++++++++ crates/vault-tui/src/app.rs | 75 ++++++++++++++ crates/vault-tui/src/main.rs | 24 +---- deny.toml | 6 ++ justfile | 2 +- 15 files changed, 635 insertions(+), 32 deletions(-) create mode 100644 crates/vault-core/src/org_key.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d136b4..a7faaf3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,7 +111,11 @@ jobs: # ratatui); revisit when ratatui's tree updates. # RUSTSEC-2024-0436 — paste (unmaintained, build-time proc-macro) # RUSTSEC-2026-0002 — lru 0.12.5 (unsound IterMut; transitive via ratatui) - ignore: RUSTSEC-2024-0436,RUSTSEC-2026-0002 + # RUSTSEC-2023-0071 — rsa (Marvin timing sidechannel, no upstream fix). + # rsa unwraps organization keys (RSA-OAEP) once, locally, at unlock; + # the attack needs a network-observable decryption oracle, which Vault + # never exposes. Revisit if rsa ships a constant-time fix. + ignore: RUSTSEC-2024-0436,RUSTSEC-2026-0002,RUSTSEC-2023-0071 deny: name: cargo-deny diff --git a/CHANGELOG.md b/CHANGELOG.md index dd08cd9..42404ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,15 @@ range may break in any release. ### Fixed +- **TUI `Space` reveals the password from every pane.** `Space` revealed a + login's password from the item list, but did nothing while the **detail** pane + was focused (logins have no per-field detail rows, so there was nothing to + reveal) and was inert in the **folder** pane. Reveal target resolution now + lives in a testable `App::reveal_target`: the detail pane reveals the + cursor-selected masked field for cards/identities and falls back to the item's + primary secret for logins/notes, and the list and folder panes both reveal the + selected item's primary secret — so `Space` works from all three panes. + - **Login against current Bitwarden / Vaultwarden servers.** Once the master password was accepted, the server rejected the token request with `400 version_header_missing` — *"No client version header found, required to @@ -67,6 +76,19 @@ range may break in any release. ### Added +- **Organization / Collection items now decrypt (org-key support).** Vault + previously skipped every organization-owned cipher — the bulk of a vault that + uses Collections — because it held no key for them. At unlock the agent now + unwraps each organization's symmetric key from `/sync` + (`profile.organizations[].key`, an RSA-OAEP-SHA1 envelope opened with the + account key recovered from `profile.privateKey`) and routes each cipher's + decryption by `organization_id` (org key vs user key). Works online and from + the offline cache. New dependency `rsa` (RSA decryption — `ring` has none); + see the justified `RUSTSEC-2023-0071` ignore in `deny.toml` / CI. Items whose + org key can't be unwrapped are still skipped, and **editing** org items is + refused for now (the write path would re-encrypt under the wrong key) — + reads/copy work. + - **`vault sync` progress spinner.** On a TTY, `vault sync` now animates a spinner while the agent pulls and decrypts `/sync`, then prints `✓ synced N items`. Suppressed under `--json` and when stderr is not a terminal, so diff --git a/Cargo.lock b/Cargo.lock index 875e25f..aef86d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -130,6 +130,12 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" +[[package]] +name = "autocfg" +version = "1.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" + [[package]] name = "base64" version = "0.22.1" @@ -346,6 +352,12 @@ dependencies = [ "static_assertions", ] +[[package]] +name = "const-oid" +version = "0.9.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2459377285ad874054d797f3ccebf984978aa39129f6eafde5cdc8315b612f8" + [[package]] name = "cpufeatures" version = "0.2.17" @@ -448,6 +460,17 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b" +[[package]] +name = "der" +version = "0.7.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7c1832837b905bbfb5101e07cc24c8deddf52f93225eee6ead5f4d63d53ddcb" +dependencies = [ + "const-oid", + "pem-rfc7468", + "zeroize", +] + [[package]] name = "deranged" version = "0.5.8" @@ -465,6 +488,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer", + "const-oid", "crypto-common", "subtle", ] @@ -1171,6 +1195,9 @@ name = "lazy_static" version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" +dependencies = [ + "spin", +] [[package]] name = "leb128fmt" @@ -1184,6 +1211,12 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libm" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" + [[package]] name = "libredox" version = "0.1.17" @@ -1305,12 +1338,58 @@ dependencies = [ "memchr", ] +[[package]] +name = "num-bigint-dig" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e661dda6640fad38e827a6d4a310ff4763082116fe217f279885c97f511bb0b7" +dependencies = [ + "lazy_static", + "libm", + "num-integer", + "num-iter", + "num-traits", + "rand 0.8.6", + "smallvec", + "zeroize", +] + [[package]] name = "num-conv" version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" +[[package]] +name = "num-integer" +version = "0.1.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-iter" +version = "0.1.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1429034a0490724d0075ebb2bc9e875d6503c3cf69e235a8941aa757d83ef5bf" +dependencies = [ + "autocfg", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", + "libm", +] + [[package]] name = "num_cpus" version = "1.17.0" @@ -1473,6 +1552,15 @@ dependencies = [ "sha2", ] +[[package]] +name = "pem-rfc7468" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "88b39c9bfcfc231068454382784bb460aae594343fb030d46e9f50a645418412" +dependencies = [ + "base64ct", +] + [[package]] name = "percent-encoding" version = "2.3.2" @@ -1496,6 +1584,27 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "pkcs1" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8ffb9f10fa047879315e6625af03c164b16962a5368d724ed16323b68ace47f" +dependencies = [ + "der", + "pkcs8", + "spki", +] + +[[package]] +name = "pkcs8" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f950b2377845cebe5cf8b5165cb3cc1a5e0fa5cfa3e1f7f55707d8fd82e0a7b7" +dependencies = [ + "der", + "spki", +] + [[package]] name = "pkg-config" version = "0.3.33" @@ -1826,6 +1935,26 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rsa" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8573f03f5883dcaebdfcf4725caa1ecb9c15b2ef50c43a07b816e06799bb12d" +dependencies = [ + "const-oid", + "digest", + "num-bigint-dig", + "num-integer", + "num-traits", + "pkcs1", + "pkcs8", + "rand_core 0.6.4", + "signature", + "spki", + "subtle", + "zeroize", +] + [[package]] name = "rustc-hash" version = "2.1.2" @@ -2075,6 +2204,16 @@ dependencies = [ "libc", ] +[[package]] +name = "signature" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77549399552de45a898a580c1b41d445bf730df867cc44e6c0233bbc4b8329de" +dependencies = [ + "digest", + "rand_core 0.6.4", +] + [[package]] name = "slab" version = "0.4.12" @@ -2097,6 +2236,22 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "spin" +version = "0.9.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" + +[[package]] +name = "spki" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d91ed6c858b01f942cd56b37a94b3e0a1798290327d1236e4d9cf4eaca44d29d" +dependencies = [ + "base64ct", + "der", +] + [[package]] name = "stable_deref_trait" version = "1.2.1" @@ -2650,6 +2805,7 @@ dependencies = [ "hmac", "pbkdf2", "rand 0.8.6", + "rsa", "serde", "serde_json", "sha1", diff --git a/Cargo.toml b/Cargo.toml index 6f16427..c857262 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,12 @@ cipher = { version = "0.4", features = ["block-padding"] } hmac = "0.12" sha1 = "0.10" sha2 = "0.10" +# RSA-2048-OAEP-SHA1 — unwraps organization keys (Bitwarden type-4 EncStrings). +# `ring` has no RSA decryption, so this is the one non-ring asymmetric path. +# Carries RUSTSEC-2023-0071 (Marvin timing sidechannel, no upstream fix); see the +# justified `cargo audit` ignore in `justfile` / CI. Not reachable as a network +# oracle here — org keys are unwrapped once, locally, at unlock. +rsa = "0.9" pbkdf2 = { version = "0.12", default-features = false, features = ["hmac", "sha2"] } argon2 = { version = "0.5", default-features = false, features = ["alloc"] } hkdf = "0.12" diff --git a/crates/vault-agent/src/state.rs b/crates/vault-agent/src/state.rs index 4d08fde..362357a 100644 --- a/crates/vault-agent/src/state.rs +++ b/crates/vault-agent/src/state.rs @@ -37,6 +37,12 @@ pub struct Vault { pub ciphers: Vec, /// Folder id → decrypted folder name. pub folders: std::collections::HashMap, + /// Organization id → that org's symmetric `(enc, mac)` key, unwrapped from + /// `profile.organizations[]` via the account RSA key at unlock. Organization + /// ciphers (`organization_id` set) decrypt under their org key, not the user + /// key; an org absent here (key couldn't be unwrapped) leaves its ciphers + /// undecryptable and they are skipped, as before. + pub org_keys: std::collections::HashMap, /// Authenticated REST client for `Sync`/`Remove`/`Edit`/`Add`, holding the /// access token. `Some` for an online unlock; `None` for a session unlocked /// from the local cache (offline) — server ops then return `Error::Offline`. @@ -59,6 +65,20 @@ pub struct Vault { } impl Vault { + /// The `(enc, mac)` key a given cipher's fields are encrypted under: its + /// organization's key when the cipher is org-owned and we hold that key, + /// otherwise the user key. The returned pair is what's handed to + /// [`Cipher::decrypt`] / [`Cipher::decrypt_name`] — which then resolve any + /// per-cipher key on top of it. + fn base_keys(&self, cipher: &Cipher) -> (&[u8; 32], &[u8; 32]) { + if let Some(oid) = cipher.organization_id.as_deref() + && let Some((enc, mac)) = self.org_keys.get(oid) + { + return (enc, mac); + } + (&self.user_enc, &self.user_mac) + } + /// Encrypt the `/sync` response under the user key and write the account's /// cache (payload + protected user key + KDF + device id) to disk /// atomically, so a later `unlock` can reconstruct this vault offline. @@ -433,10 +453,11 @@ impl AgentState { } else { DecryptOptions::default() }; - // An item whose key Vault can't unwrap (e.g. an organization cipher, - // wrapped under an org key we don't hold) must not sink the whole - // list — skip it and tally for a one-line note afterwards. - let Ok(plain) = c.decrypt(&v.user_enc, &v.user_mac, opts) else { + // Organization ciphers decrypt under their org key; personal ones + // under the user key. An item we still can't decrypt (org key we + // don't hold) must not sink the whole list — skip it and tally. + let (base_enc, base_mac) = v.base_keys(c); + let Ok(plain) = c.decrypt(base_enc, base_mac, opts) else { skipped += 1; continue; }; @@ -483,7 +504,8 @@ impl AgentState { let mut matches: Vec<(usize, String)> = Vec::new(); for (idx, c) in v.ciphers.iter().enumerate() { // Skip items we can't decrypt rather than failing the whole lookup. - let Ok(name) = c.decrypt_name(&v.user_enc, &v.user_mac) else { + let (base_enc, base_mac) = v.base_keys(c); + let Ok(name) = c.decrypt_name(base_enc, base_mac) else { continue; }; if let Some(n) = name @@ -510,8 +532,9 @@ impl AgentState { let idx = self.resolve_cipher(selector)?; let v = self.vault.as_mut().ok_or(IpcError::Locked)?; let id = v.ciphers[idx].id.clone(); + let (base_enc, base_mac) = v.base_keys(&v.ciphers[idx]); let name = v.ciphers[idx] - .decrypt_name(&v.user_enc, &v.user_mac) + .decrypt_name(base_enc, base_mac) .map_err(|e| IpcError::Decrypt(e.to_string()))? .unwrap_or_else(|| "".to_owned()); v.ensure_online().await?; @@ -633,6 +656,15 @@ impl AgentState { let v = self.vault.as_mut().ok_or(IpcError::Locked)?; let mut cipher = v.ciphers[idx].clone(); + // Organization items are encrypted under the org key (and a per-cipher + // key wrapped under it); the edit path re-encrypts changed fields under + // the *user* key, which would corrupt them. Refuse until org-aware write + // support exists — reads/copy of org items already work. + if cipher.organization_id.is_some() { + return Err(IpcError::Internal( + "editing organization items is not supported yet".to_owned(), + )); + } if card_present && cipher.cipher_type != 3 { return Err(IpcError::Internal( "card fields can only be edited on a card item".to_owned(), @@ -721,6 +753,7 @@ impl AgentState { .map_err(|e| IpcError::Network(e.to_string()))?; let (ciphers, folders) = crate::unlock::ciphers_and_folders(&sync, &v.user_enc, &v.user_mac); + v.org_keys = crate::unlock::build_org_keys(&sync, &v.user_enc, &v.user_mac); v.ciphers = ciphers; v.folders = folders; v.last_sync = crate::unlock::now_iso(); @@ -744,10 +777,11 @@ impl AgentState { let query_lower = query.to_lowercase(); let mut matched: Option<(&Cipher, String)> = None; for c in &v.ciphers { - // Skip items we can't decrypt (e.g. organization ciphers, whose keys - // are wrapped under an org key Vault doesn't hold) — they can never - // be the target, and one must not abort the whole search. - let Ok(name) = c.decrypt_name(&v.user_enc, &v.user_mac) else { + // Decrypt under the cipher's base key (org or user). Skip items we + // still can't open (org key we don't hold) — they can't be the + // target, and one must not abort the whole search. + let (base_enc, base_mac) = v.base_keys(c); + let Ok(name) = c.decrypt_name(base_enc, base_mac) else { continue; }; let hit = id.map_or_else( @@ -819,8 +853,9 @@ impl AgentState { ..DecryptOptions::default() }, }; + let (base_enc, base_mac) = v.base_keys(cipher); let plain = cipher - .decrypt(&v.user_enc, &v.user_mac, opts) + .decrypt(base_enc, base_mac, opts) .map_err(|e| IpcError::Decrypt(e.to_string()))?; let value = match field { Field::Password => plain.password.clone(), @@ -1353,6 +1388,60 @@ mod tests { assert_eq!(list[0].name, "github"); } + #[test] + fn org_cipher_decrypts_under_org_key() { + let user_enc = [7u8; 32]; + let user_mac = [9u8; 32]; + let org_enc = [21u8; 32]; + let org_mac = [22u8; 32]; + let mut v = stub_vault(); + v.user_enc = SealedKey::new(user_enc); + v.user_mac = SealedKey::new(user_mac); + v.org_keys.insert("org-1".to_owned(), (org_enc, org_mac)); + let e = |ke: &[u8; 32], km: &[u8; 32], s: &str| { + vault_core::EncString::encrypt(ke, km, s.as_bytes()).serialize() + }; + // An org cipher: name + password under the ORG key, not the user key. + v.ciphers.push(Cipher { + id: "org-c1".into(), + cipher_type: 1, + organization_id: Some("org-1".into()), + name: Some(e(&org_enc, &org_mac, "shared-login")), + login: Some(Login { + password: Some(e(&org_enc, &org_mac, "org-secret")), + ..Login::default() + }), + ..Cipher::default() + }); + // A second org cipher whose org key we don't hold → skipped, not an error. + v.ciphers.push(Cipher { + id: "org-c2".into(), + cipher_type: 1, + organization_id: Some("org-unknown".into()), + name: Some(e(&[1u8; 32], &[2u8; 32], "invisible")), + ..Cipher::default() + }); + let mut s = AgentState::new(900); + s.vault = Some(v); + + let item = s + .get_item(Some("org-c1"), "shared-login", Field::Password) + .unwrap(); + assert_eq!(item.value, "org-secret", "decrypted under the org key"); + let list = s.list_entries().unwrap(); + assert_eq!(list.len(), 1, "the unknown-org cipher is skipped"); + assert_eq!(list[0].name, "shared-login"); + + // Editing an org item is refused (write path would corrupt it). + let rt = tokio::runtime::Builder::new_current_thread() + .build() + .expect("rt"); + let err = rt + .block_on(s.edit_cipher("org-c1", CipherWrite::default())) + .unwrap_err(); + assert!(matches!(err, IpcError::Internal(_)), "got {err:?}"); + } + #[cfg(feature = "clipboard")] #[test] fn should_clear_only_when_ours_or_unreadable() { @@ -1897,6 +1986,7 @@ mod tests { user_mac: SealedKey::new([0u8; 32]), ciphers: Vec::new(), folders: std::collections::HashMap::new(), + org_keys: std::collections::HashMap::new(), client: Some(client), protected_user_key: String::new(), kdf: vault_core::kdf::KdfParams { diff --git a/crates/vault-agent/src/unlock.rs b/crates/vault-agent/src/unlock.rs index c382ba5..24c5273 100644 --- a/crates/vault-agent/src/unlock.rs +++ b/crates/vault-agent/src/unlock.rs @@ -101,6 +101,7 @@ async fn online_unlock( let sync = client.sync().await.map_err(api_err)?; let (ciphers, folders) = ciphers_and_folders(&sync, &user_enc, &user_mac); + let org_keys = build_org_keys(&sync, &user_enc, &user_mac); // Capture the refresh token before the client is moved into the vault, so // it can be persisted (encrypted) and reused after a cache/PIN unlock. @@ -115,6 +116,7 @@ async fn online_unlock( user_mac: SealedKey::new(*user_mac), ciphers, folders, + org_keys, client: Some(client), protected_user_key: encrypted_user_key.to_owned(), kdf: params, @@ -295,6 +297,9 @@ pub fn vault_from_user_key( let sync: SyncResponse = serde_json::from_slice(&payload) .map_err(|e| IpcError::Internal(format!("parse cached vault: {e}")))?; let (ciphers, folders) = ciphers_and_folders(&sync, user_enc, user_mac); + // The cached payload is the full `/sync` (profile included), so org keys + // rebuild offline too — organization items decrypt on a cache/PIN unlock. + let org_keys = build_org_keys(&sync, user_enc, user_mac); let kdf = cache .kdf .ok_or_else(|| IpcError::Internal("cached account has no KDF params".to_owned()))?; @@ -312,6 +317,7 @@ pub fn vault_from_user_key( user_mac: SealedKey::new(*user_mac), ciphers, folders, + org_keys, client: None, protected_user_key: cache.protected_user_key.clone().unwrap_or_default(), kdf, @@ -494,6 +500,55 @@ pub fn ciphers_and_folders( (ciphers, folders) } +/// Unwrap each organization's symmetric `(enc, mac)` key from the sync `profile` +/// using the account RSA key (`profile.privateKey`). Best-effort: a missing or +/// unusable private key yields an empty map, and any single org key that won't +/// unwrap is omitted — its ciphers then stay undecryptable and are skipped, as +/// before. Org ciphers decrypt under these keys via [`Vault::base_keys`]. +pub fn build_org_keys( + sync: &vault_api::SyncResponse, + user_enc: &[u8; 32], + user_mac: &[u8; 32], +) -> HashMap { + let mut keys = HashMap::new(); + let entries = sync.organization_keys(); + if entries.is_empty() { + return keys; + } + let Some(private_key_enc) = sync.private_key() else { + eprintln!( + "vault-agent: {} organization(s) but no account private key in sync; org items will be skipped", + entries.len() + ); + return keys; + }; + let account = match vault_core::AccountKey::from_protected(private_key_enc, user_enc, user_mac) + { + Ok(account) => account, + Err(e) => { + eprintln!( + "vault-agent: could not recover account private key ({e}); org items will be skipped" + ); + return keys; + } + }; + let mut failed = 0usize; + for (id, wrapped) in entries { + match account.decrypt_org_key(&wrapped) { + Ok(key) => { + keys.insert(id, key); + } + Err(_) => failed += 1, + } + } + if failed > 0 { + eprintln!( + "vault-agent: {failed} organization key(s) could not be unwrapped; those items will be skipped" + ); + } + keys +} + fn api_err(e: vault_api::Error) -> IpcError { match e { vault_api::Error::ServerStatus { status, message } if status == 400 => { diff --git a/crates/vault-api/src/sync.rs b/crates/vault-api/src/sync.rs index 2ac942d..468cd46 100644 --- a/crates/vault-api/src/sync.rs +++ b/crates/vault-api/src/sync.rs @@ -50,4 +50,34 @@ impl SyncResponse { pub const fn folder_count(&self) -> usize { self.folders.len() } + + /// The account's RSA private-key envelope (`profile.privateKey`), if present + /// — a type-2 `EncString` wrapping the PKCS#8 DER key under the user key. Used + /// to unwrap organization keys. + #[must_use] + pub fn private_key(&self) -> Option<&str> { + self.profile + .get("privateKey") + .and_then(serde_json::Value::as_str) + } + + /// `(organization_id, wrapped_org_key)` for each organization the account + /// belongs to that ships a key (`profile.organizations[]`). Each key is a + /// type-4 (RSA-OAEP-SHA1) `EncString`; memberships without a key are skipped. + #[must_use] + pub fn organization_keys(&self) -> Vec<(String, String)> { + self.profile + .get("organizations") + .and_then(serde_json::Value::as_array) + .map(|orgs| { + orgs.iter() + .filter_map(|o| { + let id = o.get("id")?.as_str()?.to_owned(); + let key = o.get("key")?.as_str()?.to_owned(); + Some((id, key)) + }) + .collect() + }) + .unwrap_or_default() + } } diff --git a/crates/vault-core/Cargo.toml b/crates/vault-core/Cargo.toml index e1e650e..196ec85 100644 --- a/crates/vault-core/Cargo.toml +++ b/crates/vault-core/Cargo.toml @@ -27,6 +27,7 @@ cipher = { workspace = true } hmac = { workspace = true } sha1 = { workspace = true } sha2 = { workspace = true } +rsa = { workspace = true } pbkdf2 = { workspace = true } argon2 = { workspace = true } hkdf = { workspace = true } diff --git a/crates/vault-core/src/error.rs b/crates/vault-core/src/error.rs index 3c5d098..73ff74f 100644 --- a/crates/vault-core/src/error.rs +++ b/crates/vault-core/src/error.rs @@ -54,6 +54,10 @@ pub enum Error { /// A TOTP secret / `otpauth://` URI could not be parsed or used. #[error("totp: {0}")] Totp(&'static str), + + /// RSA key parsing or OAEP decryption failed (organization-key unwrapping). + #[error("rsa: {0}")] + Rsa(&'static str), } impl From for Error { diff --git a/crates/vault-core/src/lib.rs b/crates/vault-core/src/lib.rs index 69fad5a..0fd52d3 100644 --- a/crates/vault-core/src/lib.rs +++ b/crates/vault-core/src/lib.rs @@ -14,6 +14,7 @@ pub mod export; pub mod generate; pub mod kdf; pub mod login; +pub mod org_key; pub mod totp; pub use cipher::{Cipher, DecryptOptions, Login, PlainCipher, decrypt_user_key}; @@ -23,3 +24,4 @@ pub use export::EncryptedExport; pub use generate::{GenerateOptions, generate_password}; pub use kdf::{KdfParams, KdfType, derive_master_key, stretch_master_key}; pub use login::master_password_hash; +pub use org_key::AccountKey; diff --git a/crates/vault-core/src/org_key.rs b/crates/vault-core/src/org_key.rs new file mode 100644 index 0000000..6a68d23 --- /dev/null +++ b/crates/vault-core/src/org_key.rs @@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +//! Organization keys — RSA-2048-OAEP-SHA1 unwrapping. +//! +//! Personal ciphers decrypt under the user symmetric key; **organization** +//! ciphers decrypt under that organization's key. The org key is delivered in +//! `/sync` (`profile.organizations[].key`) as a Bitwarden **type-4** `EncString`: +//! the 64-byte org key (`enc(32)‖mac(32)`) RSA-OAEP-SHA1-encrypted under the +//! account's public key. The matching private key arrives as +//! `profile.privateKey` — a **type-2** (AES) `EncString` wrapping the PKCS#8 DER +//! private key under the user key. +//! +//! `ring` (Vault's symmetric crypto) has no RSA decryption, so this is the one +//! place the `rsa` crate is used — see its note in `Cargo.toml` re: the audit +//! ignore. Decryption here is local and one-shot (at unlock), never a network +//! oracle. + +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD as B64; +use rsa::Oaep; +use rsa::RsaPrivateKey; +use rsa::pkcs8::DecodePrivateKey; +use zeroize::Zeroizing; + +use crate::enc_string::EncString; +use crate::error::{Error, Result}; + +/// The account's RSA private key, recovered from `profile.privateKey`. Used to +/// unwrap each organization's symmetric key. +pub struct AccountKey(RsaPrivateKey); + +impl AccountKey { + /// Recover the account private key from its `profile.privateKey` `EncString` + /// — a type-2 (AES) `EncString` wrapping the PKCS#8 DER key under the user key. + /// + /// # Errors + /// + /// Returns [`Error::MacMismatch`] / [`Error::Unpad`] if the `EncString` fails + /// under the user key, or [`Error::Rsa`] if the plaintext is not a valid + /// PKCS#8 RSA private key. + pub fn from_protected( + private_key_enc: &str, + user_enc: &[u8; 32], + user_mac: &[u8; 32], + ) -> Result { + // `der` holds the raw private key; `Zeroizing` scrubs it after parsing. + let der = Zeroizing::new(EncString::parse(private_key_enc)?.decrypt(user_enc, user_mac)?); + let key = RsaPrivateKey::from_pkcs8_der(&der) + .map_err(|_| Error::Rsa("invalid PKCS#8 RSA private key"))?; + Ok(Self(key)) + } + + /// RSA-OAEP-SHA1-decrypt a type-4 organization-key `EncString` and split the + /// 64-byte result into its `(enc, mac)` halves. + /// + /// # Errors + /// + /// Returns [`Error::EncString`] if `org_key_enc` is not a 64-byte type-4 + /// `EncString`, [`Error::Base64`] on a malformed payload, or [`Error::Rsa`] if + /// OAEP decryption fails. + pub fn decrypt_org_key(&self, org_key_enc: &str) -> Result<([u8; 32], [u8; 32])> { + let ciphertext = parse_type4(org_key_enc)?; + // OAEP with SHA-1 for both the label hash and MGF1 (Bitwarden type 4). + let raw = Zeroizing::new( + self.0 + .decrypt(Oaep::new::(), &ciphertext) + .map_err(|_| Error::Rsa("RSA-OAEP decryption failed"))?, + ); + if raw.len() != 64 { + return Err(Error::EncString("organization key must be 64 bytes")); + } + let mut enc = [0u8; 32]; + let mut mac = [0u8; 32]; + enc.copy_from_slice(&raw[..32]); + mac.copy_from_slice(&raw[32..]); + Ok((enc, mac)) + } +} + +impl core::fmt::Debug for AccountKey { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + // Never render key material. + f.write_str("AccountKey(..)") + } +} + +/// Parse a Bitwarden **type-4** `EncString` — `"4."` — into +/// its raw ciphertext bytes. Type 4 (`Rsa2048_OaepSha1_B64`) is a single base64 +/// payload with no IV or MAC; RSA-OAEP is itself the authenticated envelope. +fn parse_type4(s: &str) -> Result> { + let rest = s + .strip_prefix("4.") + .ok_or(Error::EncString("not a type-4 (RSA) EncString"))?; + Ok(B64.decode(rest)?) +} + +#[cfg(test)] +mod tests { + use super::AccountKey; + use crate::enc_string::EncString; + use base64::Engine as _; + use rand::SeedableRng as _; + use rand::rngs::StdRng; + use rsa::RsaPrivateKey; + use rsa::pkcs8::EncodePrivateKey; + use rsa::traits::PublicKeyParts as _; + use rsa::{Oaep, RsaPublicKey}; + + const USER_ENC: [u8; 32] = [1u8; 32]; + const USER_MAC: [u8; 32] = [2u8; 32]; + + // A deterministic small-but-real RSA-2048 key would be ideal, but generating + // one needs an RNG. `rand` is a dev-dependency; use it here only. + fn test_key() -> RsaPrivateKey { + let mut rng = StdRng::seed_from_u64(42); + RsaPrivateKey::new(&mut rng, 2048).expect("generate test key") + } + + /// End-to-end: wrap the private key (type-2 under the user key) and an org + /// key (type-4 under the public key), then recover both and check the split. + #[test] + fn unwraps_org_key_through_account_key() { + let priv_key = test_key(); + let pub_key = RsaPublicKey::from(&priv_key); + assert_eq!(pub_key.size(), 256, "RSA-2048"); + + // profile.privateKey: PKCS#8 DER, AES-wrapped under the user key. + let der = priv_key.to_pkcs8_der().expect("pkcs8").as_bytes().to_vec(); + let protected = EncString::encrypt(&USER_ENC, &USER_MAC, &der).serialize(); + + // An org key (64 bytes) RSA-OAEP-SHA1-wrapped under the public key. + let mut org = [0u8; 64]; + org[..32].copy_from_slice(&[7u8; 32]); + org[32..].copy_from_slice(&[8u8; 32]); + let mut rng = StdRng::seed_from_u64(7); + let wrapped = pub_key + .encrypt(&mut rng, Oaep::new::(), &org) + .expect("wrap org key"); + let type4 = format!( + "4.{}", + base64::engine::general_purpose::STANDARD.encode(&wrapped) + ); + + let account = AccountKey::from_protected(&protected, &USER_ENC, &USER_MAC).unwrap(); + let (enc, mac) = account.decrypt_org_key(&type4).unwrap(); + assert_eq!(enc, [7u8; 32]); + assert_eq!(mac, [8u8; 32]); + } + + #[test] + fn rejects_non_type4() { + let account = AccountKey::from_protected( + &EncString::encrypt( + &USER_ENC, + &USER_MAC, + test_key().to_pkcs8_der().unwrap().as_bytes(), + ) + .serialize(), + &USER_ENC, + &USER_MAC, + ) + .unwrap(); + // A type-2 string is not a valid org-key envelope. + assert!(account.decrypt_org_key("2.aXY=|Y3Q=|bWFj").is_err()); + } +} diff --git a/crates/vault-tui/src/app.rs b/crates/vault-tui/src/app.rs index 411ff0a..fd0f92b 100644 --- a/crates/vault-tui/src/app.rs +++ b/crates/vault-tui/src/app.rs @@ -1472,6 +1472,36 @@ impl App { detail_fields(e.cipher_type).get(self.detail_field).copied() } + /// The `(id, name, field)` that `Space` should reveal given the current + /// focus and selection, or `None` when nothing is revealable. + /// + /// Cards and identities reveal the cursor-selected **masked** detail field + /// when the detail pane is focused. Logins and secure notes have no per-field + /// detail rows, so in the detail pane they fall back to the type's primary + /// secret — without this, `Space` did nothing on a login while the detail + /// pane was focused. The item list and the folder pane both reveal the + /// selected item's primary secret, so `Space` works from any of the three + /// panes. + #[must_use] + pub fn reveal_target(&self) -> Option<(String, String, Field)> { + let sel = self.selected_entry()?; + let field = if self.detail_focused() { + self.selected_detail_field() + .filter(|d| d.masked) + .map(|d| d.field) + .or_else(|| { + detail_fields(sel.cipher_type) + .is_empty() + .then(|| primary_secret_field(sel.cipher_type)) + .flatten() + })? + } else { + // Item list or folder pane: the selected item's primary secret. + primary_secret_field(sel.cipher_type)? + }; + Some((sel.id, sel.name, field)) + } + /// Whether `field` of the item with `entry_id` is currently revealed. #[must_use] pub fn is_revealed(&self, entry_id: &str, field: Field) -> bool { @@ -2312,6 +2342,51 @@ mod tests { assert!(app.revealed.is_none(), "switching panes must re-mask"); } + #[test] + fn reveal_target_reveals_login_password_in_detail_pane() { + let mut app = App::browsing(status(), vec![entry("site", None)]); // login + // Item list → the login's primary secret. + assert!(app.items_focused()); + assert_eq!( + app.reveal_target().map(|(_, _, f)| f), + Some(Field::Password) + ); + // Detail pane on a login (no per-field rows): Space must still reveal the + // password — the bug this fixes was `Space` doing nothing here. + app.focus = Focus::Detail; + assert_eq!( + app.reveal_target().map(|(_, _, f)| f), + Some(Field::Password) + ); + // Folder pane (left) also reveals the selected item's primary secret. + app.focus = Focus::Folders; + assert_eq!( + app.reveal_target().map(|(_, _, f)| f), + Some(Field::Password) + ); + } + + #[test] + fn reveal_target_uses_cursor_masked_field_for_cards() { + let card = ListEntry { + id: "id-card".into(), + name: "card".into(), + cipher_type: 3, + username: None, + folder: None, + }; + let mut app = App::browsing(status(), vec![card]); + app.focus = Focus::Detail; + // Card detail rows: [Holder, Brand, Number(masked), Exp, CVV(masked)]. + app.detail_field = 0; // Holder — not masked → nothing to reveal. + assert!(app.reveal_target().is_none()); + app.detail_field = 2; // Number — masked → the card number. + assert_eq!( + app.reveal_target().map(|(_, _, f)| f), + Some(Field::CardNumber) + ); + } + #[test] fn toast_set_and_clear() { let mut app = App::browsing(status(), vec![entry("a", None)]); diff --git a/crates/vault-tui/src/main.rs b/crates/vault-tui/src/main.rs index 8a33426..dfb914d 100644 --- a/crates/vault-tui/src/main.rs +++ b/crates/vault-tui/src/main.rs @@ -719,25 +719,11 @@ async fn copy_generated(state: &mut App, socket: &Path) { /// first press fetches the plaintext (id-targeted, so duplicate names can't /// mislead it); the second re-masks. async fn toggle_reveal(state: &mut App, socket: &Path) { - // Resolve (id, name, field): the cursor field when the detail pane is - // focused (masked fields only), else the item's primary secret. - let (id, name, field) = if state.detail_focused() { - let Some(sel) = state.selected_entry() else { - return; - }; - let Some(df) = state.selected_detail_field().filter(|d| d.masked) else { - return; - }; - (sel.id, sel.name, df.field) - } else if state.items_focused() { - let Some(sel) = state.selected_entry() else { - return; - }; - let Some(field) = app::primary_secret_field(sel.cipher_type) else { - return; - }; - (sel.id, sel.name, field) - } else { + // Resolve (id, name, field): the cursor-selected masked detail field when + // the detail pane is focused (cards/identities), else the item's primary + // secret — including logins/notes in the detail pane, which have no per-field + // detail rows. See `App::reveal_target`. + let Some((id, name, field)) = state.reveal_target() else { return; }; diff --git a/deny.toml b/deny.toml index ea16118..16fea48 100644 --- a/deny.toml +++ b/deny.toml @@ -14,6 +14,12 @@ ignore = [ # by ratatui (TUI). The author archived it with no successor we can swap to # without dropping ratatui. Revisit when ratatui's tree drops `paste`. "RUSTSEC-2024-0436", + # rsa 0.9 — RUSTSEC-2023-0071 (Marvin timing sidechannel, no upstream fix). + # `rsa` only RSA-OAEP-unwraps organization keys, once and locally, at unlock. + # The Marvin attack needs a network-observable decryption oracle timing many + # adaptively-chosen ciphertexts; Vault never exposes one. Mirrors the + # cargo-audit ignore in CI. Revisit if `rsa` ships a constant-time fix. + "RUSTSEC-2023-0071", ] [licenses] diff --git a/justfile b/justfile index 5bde885..5dad543 100644 --- a/justfile +++ b/justfile @@ -51,7 +51,7 @@ deny: # Vulnerability advisories; --ignore mirrors CI's audit job (.github/workflows/ci.yml). Needs cargo-audit. audit: - cargo audit --ignore RUSTSEC-2024-0436 --ignore RUSTSEC-2026-0002 + cargo audit --ignore RUSTSEC-2024-0436 --ignore RUSTSEC-2026-0002 --ignore RUSTSEC-2023-0071 # REUSE/SPDX licensing lint (CI: REUSE job). Needs reuse (`uvx reuse lint` or `pipx install reuse`). reuse: