diff --git a/CHANGELOG.md b/CHANGELOG.md index d2955f2..dd08cd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,8 +31,47 @@ range may break in any release. `cargo audit` scans the lockfile literally, so the patched release is pulled in to keep the supply-chain gate green. +### Fixed + +- **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 + prevent encryption errors"* — because `vault-api` sent no client-identification + headers. The client now sends the Bitwarden trio (`Bitwarden-Client-Name`, + `Bitwarden-Client-Version`, `Device-Type`) as `reqwest` default headers on + every request. The advertised version (`vault_api::CLIENT_VERSION`) is a recent + *Bitwarden* client version — not Vault's own — chosen to clear the server's + min-version gate while keeping it on the EncString type-2 / PBKDF2+Argon2 crypto + path Vault implements. Because the check sits on the **post-authentication** + path, a wrong password still fails first with `invalid_grant` ("bad password"), + which is why the missing header only surfaced once the password was correct. + +- **2FA challenge misreported as "bad password".** Bitwarden's hosted server + returns the `TwoFactorProviders` list with provider ids as JSON **strings** + (`["0","7"]`); `vault-api`'s parser expected integers, so the whole 2FA error + body failed to deserialize and fell through to a generic `invalid_grant` — + which the agent rendered as "bad password", even with a correct password and a + valid authenticator. The provider-id parse now accepts strings *or* integers + (and never fails the surrounding 2FA detection). The agent also logs the raw + server body on a login `400`, so an opaque "bad password" can't mask an + actionable response again. + +- **`vault sync` reported 0 items from a populated vault.** The `/sync` response + (and every nested cipher) is **camelCase** on current Bitwarden cloud and + Vaultwarden, but `SyncResponse` and the `vault-core` cipher types were + `PascalCase`; every field fell back to its `#[serde(default)]` empty value, so + a full vault parsed as zero ciphers. The API-service wire structs are now + camelCase (identity-service structs — `TokenResponse`, `TwoFactorErrorBody` — + stay PascalCase, matching that endpoint), folder-name extraction tolerates + both casings, and the test fixtures were corrected to the real shape. + ### Added +- **`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 + scripts and machine consumers are unaffected. + - **EncString fuzz soak passed (PRD §11.4 / RELEASING.md gate #1).** A ≥ 24 h libFuzzer soak of the `EncString` type-2 parser completed with **0 findings**: 8,874,210,317 executions over 86,401 s (~102.7 k exec/s), coverage flat at 312 diff --git a/crates/vault-agent/src/state.rs b/crates/vault-agent/src/state.rs index b4e4da6..4d08fde 100644 --- a/crates/vault-agent/src/state.rs +++ b/crates/vault-agent/src/state.rs @@ -423,6 +423,7 @@ impl AgentState { pub fn list_entries(&self) -> Result, IpcError> { let v = self.vault.as_ref().ok_or(IpcError::Locked)?; let mut out = Vec::with_capacity(v.ciphers.len()); + let mut skipped = 0usize; for c in &v.ciphers { // For ListEntry we want name + username (login items). Username // is cheap; decrypt it eagerly even though the wire shape allows @@ -432,9 +433,13 @@ impl AgentState { } else { DecryptOptions::default() }; - let plain = c - .decrypt(&v.user_enc, &v.user_mac, opts) - .map_err(|e| IpcError::Decrypt(e.to_string()))?; + // 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 { + skipped += 1; + continue; + }; let Some(name) = plain.name.clone() else { continue; // skip unnamed items in the list view }; @@ -450,6 +455,11 @@ impl AgentState { folder, }); } + if skipped > 0 { + eprintln!( + "vault-agent: list: skipped {skipped} undecryptable cipher(s) (likely organization items)" + ); + } out.sort_by_key(|e| e.name.to_lowercase()); Ok(out) } @@ -472,9 +482,10 @@ impl AgentState { let sel_lower = selector.to_lowercase(); let mut matches: Vec<(usize, String)> = Vec::new(); for (idx, c) in v.ciphers.iter().enumerate() { - let name = c - .decrypt_name(&v.user_enc, &v.user_mac) - .map_err(|e| IpcError::Decrypt(e.to_string()))?; + // 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 { + continue; + }; if let Some(n) = name && n.to_lowercase() == sel_lower { @@ -733,9 +744,12 @@ impl AgentState { let query_lower = query.to_lowercase(); let mut matched: Option<(&Cipher, String)> = None; for c in &v.ciphers { - let name = c - .decrypt_name(&v.user_enc, &v.user_mac) - .map_err(|e| IpcError::Decrypt(e.to_string()))?; + // 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 { + continue; + }; let hit = id.map_or_else( || { name.as_deref() @@ -1301,6 +1315,44 @@ mod tests { assert_eq!(item.value, "first-secret"); } + #[test] + fn search_skips_undecryptable_ciphers() { + let enc = [7u8; 32]; + let mac = [9u8; 32]; + let mut v = stub_vault(); + v.user_enc = SealedKey::new(enc); + v.user_mac = SealedKey::new(mac); + // An organization-like cipher whose per-item key is wrapped under a key + // Vault doesn't hold — decrypt_name fails. It precedes the real target, + // so neither get_item nor list_entries may abort on it. + let wrong = [0xAAu8; 32]; + let mut material = [0u8; 64]; + material[..32].copy_from_slice(&[0xBBu8; 32]); + material[32..].copy_from_slice(&[0xCCu8; 32]); + v.ciphers.push(Cipher { + id: "org-1".into(), + cipher_type: 1, + key: Some(vault_core::EncString::encrypt(&wrong, &wrong, &material).serialize()), + name: Some( + vault_core::EncString::encrypt(&[0xBBu8; 32], &[0xCCu8; 32], b"org").serialize(), + ), + ..Cipher::default() + }); + v.ciphers.push(login_with_password( + "real-1", "github", "hunter2", &enc, &mac, + )); + let mut s = AgentState::new(900); + s.vault = Some(v); + + // The undecryptable cipher must not abort the reveal of a good item. + let item = s.get_item(None, "github", Field::Password).unwrap(); + assert_eq!(item.value, "hunter2"); + // …nor the list, which simply omits it. + let list = s.list_entries().unwrap(); + assert_eq!(list.len(), 1); + assert_eq!(list[0].name, "github"); + } + #[cfg(feature = "clipboard")] #[test] fn should_clear_only_when_ours_or_unreadable() { diff --git a/crates/vault-agent/src/unlock.rs b/crates/vault-agent/src/unlock.rs index 72f345e..c382ba5 100644 --- a/crates/vault-agent/src/unlock.rs +++ b/crates/vault-agent/src/unlock.rs @@ -468,10 +468,20 @@ pub fn ciphers_and_folders( let mut folders = HashMap::new(); for f in &sync.folders { let Some(obj) = f.as_object() else { continue }; - let Some(id) = obj.get("Id").and_then(|v| v.as_str()) else { + // Accept both casings: real servers send camelCase (`id`/`name`); older + // fixtures and any PascalCase deployment send `Id`/`Name`. + let Some(id) = obj + .get("id") + .or_else(|| obj.get("Id")) + .and_then(|v| v.as_str()) + else { continue; }; - let Some(name_enc) = obj.get("Name").and_then(|v| v.as_str()) else { + let Some(name_enc) = obj + .get("name") + .or_else(|| obj.get("Name")) + .and_then(|v| v.as_str()) + else { continue; }; if let Ok(enc) = vault_core::EncString::parse(name_enc) @@ -487,6 +497,11 @@ pub fn ciphers_and_folders( fn api_err(e: vault_api::Error) -> IpcError { match e { vault_api::Error::ServerStatus { status, message } if status == 400 => { + // Surface the raw server body to agent.log: a flat "bad password" + // otherwise hides actionable 400s (new-device verification, captcha, + // region mismatch). The body is the auth *response* — it carries no + // secret (the password hash lives in the request). + eprintln!("vault-agent: login 400 body: {message}"); if message.contains("invalid_grant") || message.to_lowercase().contains("username") { IpcError::BadPassword } else { @@ -563,20 +578,20 @@ mod tests { let sync = SyncResponse { profile: serde_json::Value::Null, folders: vec![serde_json::json!({ - "Id": "fid-1", - "Name": enc_str(&enc, &mac, "Work"), + "id": "fid-1", + "name": enc_str(&enc, &mac, "Work"), })], collections: vec![], ciphers: vec![ serde_json::json!({ - "Id": "c1", - "Type": 1, - "Name": enc_str(&enc, &mac, "github.com"), + "id": "c1", + "type": 1, + "name": enc_str(&enc, &mac, "github.com"), }), serde_json::json!({ - "Id": "c2", - "Type": 2, - "Name": enc_str(&enc, &mac, "note"), + "id": "c2", + "type": 2, + "name": enc_str(&enc, &mac, "note"), }), ], domains: serde_json::Value::Null, @@ -623,9 +638,9 @@ mod tests { folders: vec![], collections: vec![], ciphers: vec![serde_json::json!({ - "Id": "c1", - "Type": 1, - "Name": enc_str(&user_enc, &user_mac, "github.com"), + "id": "c1", + "type": 1, + "name": enc_str(&user_enc, &user_mac, "github.com"), })], domains: serde_json::Value::Null, sends: vec![], @@ -682,9 +697,9 @@ mod tests { folders: vec![], collections: vec![], ciphers: vec![serde_json::json!({ - "Id": "c1", - "Type": 1, - "Name": enc_str(user_enc, user_mac, "github.com"), + "id": "c1", + "type": 1, + "name": enc_str(user_enc, user_mac, "github.com"), })], domains: serde_json::Value::Null, sends: vec![], @@ -770,10 +785,10 @@ mod tests { let mac = [4u8; 32]; let sync = SyncResponse { profile: serde_json::Value::Null, - // Missing Name, and an undecryptable Name — both dropped silently. + // Missing name, and an undecryptable name — both dropped silently. folders: vec![ - serde_json::json!({ "Id": "fid-missing-name" }), - serde_json::json!({ "Id": "fid-bad", "Name": "not-an-encstring" }), + serde_json::json!({ "id": "fid-missing-name" }), + serde_json::json!({ "id": "fid-bad", "name": "not-an-encstring" }), ], collections: vec![], ciphers: vec![], diff --git a/crates/vault-api/src/client.rs b/crates/vault-api/src/client.rs index 33d2f1e..26b7aed 100644 --- a/crates/vault-api/src/client.rs +++ b/crates/vault-api/src/client.rs @@ -9,6 +9,7 @@ //! the master-password hash. use reqwest::Client; +use reqwest::header::{HeaderMap, HeaderValue}; use uuid::Uuid; use zeroize::Zeroizing; @@ -25,6 +26,49 @@ pub const CLIENT_ID: &str = "cli"; /// Bitwarden device-type for desktop / CLI clients. pub const DEVICE_TYPE_CLI: u32 = 14; +/// `Bitwarden-Client-Name` header value (matches the official CLI). +pub const CLIENT_NAME: &str = "cli"; + +/// Bitwarden client/protocol version advertised in the `Bitwarden-Client-Version` +/// header. +/// +/// Bitwarden's hosted server (and recent Vaultwarden) reject any request that +/// omits this header with `400 version_header_missing` — *"No client version +/// header found, required to prevent encryption errors"*. That check lives on +/// the **post-authentication** path that formats the encrypted key material, so +/// it only fires once credentials are accepted (a wrong password still returns +/// `invalid_grant` first). +/// +/// The value is a recent *Bitwarden* client version, deliberately **not** Vault's +/// own `CARGO_PKG_VERSION`: the server parses this string as a Bitwarden client +/// version, so `0.0.1` would read as an ancient client and trip min-version +/// gates. It is kept a step behind bleeding-edge on purpose, so the server stays +/// on the `EncString` type-2 / PBKDF2+Argon2 crypto path that `vault-core` +/// implements — the 2025 account-crypto-v2 work is server-feature-gated and is +/// intentionally not advertised here. Bump only alongside verified support for +/// any newer crypto format it would opt us into. +pub const CLIENT_VERSION: &str = "2024.12.1"; + +/// Default headers sent on **every** request: the Bitwarden client-identification +/// trio that the server requires (see [`CLIENT_VERSION`]). Installed once as the +/// `reqwest` client's default header set in [`BitwardenClient::new`]; per-request +/// headers (`Authorization`, `Auth-Email`) are layered on top and never collide +/// with these names. +fn client_headers() -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert( + "Bitwarden-Client-Name", + HeaderValue::from_static(CLIENT_NAME), + ); + headers.insert( + "Bitwarden-Client-Version", + HeaderValue::from_static(CLIENT_VERSION), + ); + // Reuse the one device-type source of truth; `u32 -> HeaderValue` is infallible. + headers.insert("Device-Type", HeaderValue::from(DEVICE_TYPE_CLI)); + headers +} + /// A two-factor token to satisfy a 2FA challenge on the password grant. /// /// `token` is the code (e.g. a 6-digit authenticator code); `provider` is the @@ -63,6 +107,7 @@ impl BitwardenClient { let user_agent = format!("vault/{} (Spacecraft-Software)", env!("CARGO_PKG_VERSION")); let builder = Client::builder() .user_agent(&user_agent) + .default_headers(client_headers()) .https_only(urls.api.scheme() == "https"); // With the `pqc` feature, prefer the X25519MLKEM768 hybrid key exchange // on the TLS layer (falls back to classical when the server lacks it). @@ -84,6 +129,11 @@ impl BitwardenClient { /// Construct from an existing `reqwest::Client` — primarily for tests /// that need to point a fresh client at a wiremock origin. + /// + /// Unlike [`new`](Self::new), this does **not** install the Bitwarden + /// client-identification headers ([`client_headers`]): default headers can + /// only be set when the `reqwest::Client` is built, so a caller that needs a + /// real server to accept the requests must bake them into `http` itself. pub fn new_with_http( http: Client, urls: BaseUrls, @@ -567,3 +617,26 @@ fn base64_url_no_pad(b: &[u8]) -> String { use base64::engine::general_purpose::URL_SAFE_NO_PAD as B64URL; B64URL.encode(b) } + +#[cfg(test)] +mod tests { + use super::{CLIENT_NAME, CLIENT_VERSION, DEVICE_TYPE_CLI, client_headers}; + + /// The Bitwarden client-identification trio must be present and correct, or + /// the server rejects authenticated requests with `version_header_missing`. + #[test] + fn client_headers_carry_bitwarden_identity() { + let headers = client_headers(); + + assert_eq!( + headers.get("Bitwarden-Client-Version").unwrap(), + CLIENT_VERSION, + "version header is the one the server actually requires", + ); + assert_eq!(headers.get("Bitwarden-Client-Name").unwrap(), CLIENT_NAME); + assert_eq!( + headers.get("Device-Type").unwrap(), + DEVICE_TYPE_CLI.to_string().as_str(), + ); + } +} diff --git a/crates/vault-api/src/identity.rs b/crates/vault-api/src/identity.rs index dd98cc1..418915d 100644 --- a/crates/vault-api/src/identity.rs +++ b/crates/vault-api/src/identity.rs @@ -72,5 +72,74 @@ pub struct TwoFactorErrorBody { /// Map of provider id → provider parameters (Vault doesn't read the params yet). pub two_factor_providers2: Option, /// Legacy form — list of provider ids only. + #[serde(default, deserialize_with = "deserialize_provider_ids")] pub two_factor_providers: Option>, } + +/// Deserialize the legacy `TwoFactorProviders` list tolerantly. +/// +/// Bitwarden's hosted server serializes the provider ids as **strings** +/// (`["0","7"]`), while other deployments have sent them as integers (`[0,7]`). +/// A strict `Vec` rejected the string form — and because that failed the +/// *entire* [`TwoFactorErrorBody`] parse, a real 2FA challenge was silently +/// downgraded into a generic `invalid_grant` ("bad password"). Accept either +/// representation, and never fail on an unexpected entry: 2FA detection must be +/// robust, and the ids are only used as a non-empty signal. +fn deserialize_provider_ids<'de, D>(deserializer: D) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, +{ + let raw = Option::>::deserialize(deserializer)?; + Ok(raw.map(|values| { + values + .iter() + .filter_map(|v| { + v.as_u64() + .and_then(|n| u32::try_from(n).ok()) + .or_else(|| v.as_str().and_then(|s| s.parse().ok())) + }) + .collect() + })) +} + +#[cfg(test)] +mod tests { + use super::TwoFactorErrorBody; + + /// The exact shape Bitwarden's hosted server returns: provider ids as + /// **strings** in the legacy array, plus the modern providers map. Regression + /// guard for the bug where the string ids failed the whole parse and a 2FA + /// challenge surfaced as "bad password". + #[test] + fn parses_bitwarden_cloud_two_factor_challenge() { + let body = r#"{ + "error":"invalid_grant", + "error_description":"Two factor required.", + "TwoFactorProviders":["0","7"], + "TwoFactorProviders2":{"0":null,"7":{"challenge":"abc"}}, + "MasterPasswordPolicy":{"Object":"masterPasswordPolicy"} + }"#; + let parsed: TwoFactorErrorBody = serde_json::from_str(body).expect("2FA body must parse"); + assert_eq!(parsed.two_factor_providers, Some(vec![0, 7])); + assert!(parsed.two_factor_providers2.is_some()); + } + + /// Integer-id deployments must still parse. + #[test] + fn parses_integer_provider_ids() { + let parsed: TwoFactorErrorBody = + serde_json::from_str(r#"{"TwoFactorProviders":[0,7]}"#).unwrap(); + assert_eq!(parsed.two_factor_providers, Some(vec![0, 7])); + } + + /// A genuine bad-password 400 (no 2FA fields) parses to all-None, so the + /// caller correctly treats it as a credential failure rather than 2FA. + #[test] + fn genuine_bad_password_has_no_providers() { + let body = + r#"{"error":"invalid_grant","error_description":"username or password is incorrect"}"#; + let parsed: TwoFactorErrorBody = serde_json::from_str(body).unwrap(); + assert!(parsed.two_factor_providers.is_none()); + assert!(parsed.two_factor_providers2.is_none()); + } +} diff --git a/crates/vault-api/src/lib.rs b/crates/vault-api/src/lib.rs index 30f900a..47c9f01 100644 --- a/crates/vault-api/src/lib.rs +++ b/crates/vault-api/src/lib.rs @@ -14,7 +14,9 @@ pub mod pqc; pub mod sync; pub mod urls; -pub use client::{BitwardenClient, CLIENT_ID, DEVICE_TYPE_CLI, TwoFactor}; +pub use client::{ + BitwardenClient, CLIENT_ID, CLIENT_NAME, CLIENT_VERSION, DEVICE_TYPE_CLI, TwoFactor, +}; pub use error::{Error, Result}; pub use identity::{PreloginResponse, TokenResponse}; pub use sync::SyncResponse; diff --git a/crates/vault-api/src/sync.rs b/crates/vault-api/src/sync.rs index decd629..2ac942d 100644 --- a/crates/vault-api/src/sync.rs +++ b/crates/vault-api/src/sync.rs @@ -11,8 +11,13 @@ use serde::{Deserialize, Serialize}; /// `GET /sync` response. +/// +/// Bitwarden's hosted API and Vaultwarden both serialize this (and the nested +/// cipher objects) in **camelCase** (`ciphers`, `folderId`, `revisionDate`, …). +/// An earlier `PascalCase` assumption parsed every field to its `#[serde(default)]` +/// empty value, so a fully-populated vault silently synced as zero items. #[derive(Clone, Debug, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct SyncResponse { /// User profile metadata (id, email, KDF, etc.). #[serde(default)] diff --git a/crates/vault-api/tests/login_sync.rs b/crates/vault-api/tests/login_sync.rs index a0a88df..3157d50 100644 --- a/crates/vault-api/tests/login_sync.rs +++ b/crates/vault-api/tests/login_sync.rs @@ -91,6 +91,9 @@ async fn login_sync_cache_round_trip() { .and(path("/identity/connect/token")) .and(body_string_contains("grant_type=password")) .and(body_string_contains(urlencoded(&expected_hash))) + // Bitwarden's server requires this header on the post-auth path; matchers + // are AND'd, so login only succeeds if the client actually sends it. + .and(header("Bitwarden-Client-Version", vault_api::CLIENT_VERSION)) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "access_token": "test-access-token", "expires_in": 3600, @@ -103,18 +106,19 @@ async fn login_sync_cache_round_trip() { .await; // --- /api/sync ------------------------------------------------------ + // camelCase, matching what Bitwarden's hosted API and Vaultwarden return. let sync_body = json!({ - "Profile": { "Id": "user-id-1", "Email": EMAIL }, - "Folders": [ - { "Id": "folder-1", "Name": "2.iv==|ct==|mac==" } + "profile": { "id": "user-id-1", "email": EMAIL }, + "folders": [ + { "id": "folder-1", "name": "2.iv==|ct==|mac==" } ], - "Collections": [], - "Ciphers": [ - { "Id": "cipher-1", "Type": 1, "Name": "2.iv==|ct==|mac==" }, - { "Id": "cipher-2", "Type": 1, "Name": "2.iv==|ct==|mac==" } + "collections": [], + "ciphers": [ + { "id": "cipher-1", "type": 1, "name": "2.iv==|ct==|mac==" }, + { "id": "cipher-2", "type": 1, "name": "2.iv==|ct==|mac==" } ], - "Domains": {}, - "Sends": [] + "domains": {}, + "sends": [] }); Mock::given(method("GET")) .and(path("/api/sync")) @@ -159,8 +163,8 @@ async fn login_sync_cache_round_trip() { assert!(reloaded.last_sync.is_some()); let pt = reloaded.load_payload(&enc_key, &mac_key).unwrap(); let pt_json: serde_json::Value = serde_json::from_slice(&pt).unwrap(); - assert_eq!(pt_json["Ciphers"].as_array().unwrap().len(), 2); - assert_eq!(pt_json["Folders"].as_array().unwrap().len(), 1); + assert_eq!(pt_json["ciphers"].as_array().unwrap().len(), 2); + assert_eq!(pt_json["folders"].as_array().unwrap().len(), 1); } #[tokio::test] @@ -271,7 +275,7 @@ async fn create_and_update_cipher_send_authorized_requests() { .and(path("/api/ciphers")) .and(header("Authorization", "Bearer write-test-token")) .and(body_string_contains("\"name\":\"2.")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "Id": new_id }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "id":new_id }))) .mount(&server) .await; @@ -280,7 +284,7 @@ async fn create_and_update_cipher_send_authorized_requests() { .and(path(format!("/api/ciphers/{cipher_id}"))) .and(header("Authorization", "Bearer write-test-token")) .and(body_string_contains("\"name\":\"2.")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "Id": cipher_id }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "id":cipher_id }))) .mount(&server) .await; @@ -337,7 +341,7 @@ async fn create_secure_note_carries_securenote_marker() { Mock::given(method("POST")) .and(path("/api/ciphers")) .and(body_string_contains("\"secureNote\"")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "Id": "note-id-1" }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "id":"note-id-1" }))) .mount(&server) .await; diff --git a/crates/vault-api/tests/parsing.rs b/crates/vault-api/tests/parsing.rs index edce716..75b335b 100644 --- a/crates/vault-api/tests/parsing.rs +++ b/crates/vault-api/tests/parsing.rs @@ -45,13 +45,16 @@ fn token_response_minimal_decodes() { #[test] fn sync_response_counts() { + // camelCase — the shape Bitwarden's hosted API and Vaultwarden actually + // return. A PascalCase body would parse to empty vecs (every field is + // `#[serde(default)]`), which silently synced a full vault as zero items. let json = r#"{ - "Profile": {}, - "Folders": [{"Id":"f1"}, {"Id":"f2"}], - "Collections": [], - "Ciphers": [{"Id":"c1"}, {"Id":"c2"}, {"Id":"c3"}], - "Domains": {}, - "Sends": [] + "profile": {}, + "folders": [{"id":"f1"}, {"id":"f2"}], + "collections": [], + "ciphers": [{"id":"c1"}, {"id":"c2"}, {"id":"c3"}], + "domains": {}, + "sends": [] }"#; let s: SyncResponse = serde_json::from_str(json).unwrap(); assert_eq!(s.cipher_count(), 3); diff --git a/crates/vault-cli/src/main.rs b/crates/vault-cli/src/main.rs index 6cb1a1a..db8d206 100644 --- a/crates/vault-cli/src/main.rs +++ b/crates/vault-cli/src/main.rs @@ -1350,9 +1350,48 @@ async fn cmd_ack(ep: Endpoint<'_>, req: Request, action: &str, json: bool) -> Re } } +/// Run `fut` while animating a spinner on stderr, clearing the line when it +/// resolves. No-ops to a plain await when stderr isn't a TTY (piped/redirected), +/// so machine consumers never see escape codes. The work itself happens in the +/// agent; this is purely a "something is happening" affordance. +async fn with_spinner(label: &str, fut: F) -> F::Output { + // Braille spinner — matches the dot aesthetic used elsewhere in the UI. + const FRAMES: [&str; 10] = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"]; + if !io::stderr().is_terminal() { + return fut.await; + } + tokio::pin!(fut); + let mut ticker = tokio::time::interval(std::time::Duration::from_millis(80)); + let mut frame = 0usize; + loop { + tokio::select! { + output = &mut fut => { + // Carriage return + clear-to-end-of-line wipes the spinner. + let mut err = io::stderr(); + let _ = write!(err, "\r\x1b[2K"); + let _ = err.flush(); + return output; + } + _ = ticker.tick() => { + let mut err = io::stderr(); + let _ = write!(err, "\r{} {label}", FRAMES[frame]); + let _ = err.flush(); + frame = (frame + 1) % FRAMES.len(); + } + } + } +} + async fn cmd_sync(ep: Endpoint<'_>, json: bool) -> Result<(), u8> { let mut stream = connect(ep).await?; - let resp = exchange(&mut stream, &Request::Sync).await?; + let req = exchange(&mut stream, &Request::Sync); + // Animate while the agent pulls and decrypts `/sync`; `--json` stays quiet + // so structured output is never polluted by the spinner or summary line. + let resp = if json { + req.await? + } else { + with_spinner("Syncing…", req).await? + }; match resp { // The agent answers a successful re-sync with a fresh Status snapshot. Response::Status(s) => { @@ -1363,6 +1402,8 @@ async fn cmd_sync(ep: Endpoint<'_>, json: bool) -> Result<(), u8> { "last_sync": s.last_sync, }); println!("{v}"); + } else if io::stderr().is_terminal() { + eprintln!("✓ synced {} items", s.items.unwrap_or(0)); } Ok(()) } diff --git a/crates/vault-core/src/cipher.rs b/crates/vault-core/src/cipher.rs index 862f642..8d99b45 100644 --- a/crates/vault-core/src/cipher.rs +++ b/crates/vault-core/src/cipher.rs @@ -20,13 +20,13 @@ use crate::error::{Error, Result}; /// `/sync` cipher object, kept generous with `serde(default)` so future /// server-side additions don't break the cache round-trip. #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct Cipher { /// Server-assigned UUID. #[serde(default)] pub id: String, /// Cipher type (1 = login, 2 = secure note, 3 = card, 4 = identity). - #[serde(rename = "Type", default)] + #[serde(rename = "type", default)] pub cipher_type: u8, /// Folder this cipher belongs to, or `None` for unfiled. #[serde(default)] @@ -34,6 +34,13 @@ pub struct Cipher { /// Organization this cipher belongs to, if any. #[serde(default)] pub organization_id: Option, + /// Per-cipher symmetric key ("cipher key encryption"): an `EncString` + /// wrapping 64 bytes (enc(32)‖mac(32)) under the user key. When present — + /// the default for current Bitwarden vaults — every other field of this item + /// is encrypted under *this* key, not the user key directly. See + /// [`Cipher::decrypt`]. + #[serde(default)] + pub key: Option, /// Encrypted display name. #[serde(default)] pub name: Option, @@ -56,7 +63,7 @@ pub struct Cipher { /// Login-specific encrypted fields. #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct Login { /// Encrypted username. #[serde(default)] @@ -74,7 +81,7 @@ pub struct Login { /// One URI on a login cipher. #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct LoginUri { /// Encrypted URI. #[serde(default)] @@ -83,7 +90,7 @@ pub struct LoginUri { /// Card-specific encrypted fields (cipher type 3). #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct Card { /// Encrypted cardholder name. #[serde(default)] @@ -107,7 +114,7 @@ pub struct Card { /// Identity-specific encrypted fields (cipher type 4). #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct Identity { /// Encrypted title (`Mr`, `Ms`, …). #[serde(default)] @@ -167,7 +174,7 @@ pub struct Identity { /// User-defined `Fields[]` entry. #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "PascalCase")] +#[serde(rename_all = "camelCase")] pub struct CustomField { /// Encrypted field name. #[serde(default)] @@ -176,7 +183,7 @@ pub struct CustomField { #[serde(default)] pub value: Option, /// Field type (0 = text, 1 = hidden, 2 = boolean, 3 = linked). - #[serde(rename = "Type", default)] + #[serde(rename = "type", default)] pub field_type: u8, } @@ -354,16 +361,53 @@ impl DecryptOptions { } } +/// A per-cipher field key — the (enc, mac) pair recovered from a cipher's own +/// `key` ("cipher key encryption"), each half zeroized on drop. +type ItemKey = (Zeroizing<[u8; 32]>, Zeroizing<[u8; 32]>); + impl Cipher { - /// Decrypt this cipher's name under `(enc_key, mac_key)`. Returns - /// `Ok(None)` for ciphers with no name field (rare; mostly secure notes - /// that never had one set). + /// Resolve the per-cipher field key for "cipher key encryption". + /// + /// When [`key`](Self::key) is present it is an `EncString` wrapping 64 bytes + /// of key material (enc(32)‖mac(32)) under the user key; every other field of + /// the cipher is then encrypted under *that* key. Returns `Ok(None)` for + /// older items with no per-cipher key (their fields use the user key + /// directly), so callers can fall back to `(enc_key, mac_key)`. /// /// # Errors /// - /// Returns [`Error::MacMismatch`] or [`Error::Unpad`] if the name field is - /// present but fails authentication or decryption under the given keys. + /// Returns [`Error::MacMismatch`] when the wrapped key fails authentication + /// under the user key — e.g. an organization item, whose key is wrapped + /// under an org key Vault does not hold — or [`Error::EncString`] if the + /// unwrapped key is not 64 bytes. + fn item_field_key(&self, enc_key: &[u8; 32], mac_key: &[u8; 32]) -> Result> { + let Some(raw_key) = self.key.as_deref() else { + return Ok(None); + }; + let material = Zeroizing::new(EncString::parse(raw_key)?.decrypt(enc_key, mac_key)?); + if material.len() != 64 { + return Err(Error::EncString("cipher key must unwrap to 64 bytes")); + } + let mut item_enc = Zeroizing::new([0u8; 32]); + let mut item_mac = Zeroizing::new([0u8; 32]); + item_enc.copy_from_slice(&material[..32]); + item_mac.copy_from_slice(&material[32..]); + Ok(Some((item_enc, item_mac))) + } + + /// Decrypt this cipher's name. Returns `Ok(None)` for ciphers with no name + /// field (rare; mostly secure notes that never had one set). + /// + /// # Errors + /// + /// Returns [`Error::MacMismatch`] or [`Error::Unpad`] if the name field (or + /// the per-cipher key) is present but fails authentication or decryption. pub fn decrypt_name(&self, enc_key: &[u8; 32], mac_key: &[u8; 32]) -> Result> { + let item_key = self.item_field_key(enc_key, mac_key)?; + let (enc_key, mac_key): (&[u8; 32], &[u8; 32]) = match &item_key { + Some((e, m)) => (&**e, &**m), + None => (enc_key, mac_key), + }; decrypt_optional(self.name.as_deref(), enc_key, mac_key) } @@ -379,6 +423,15 @@ impl Cipher { mac_key: &[u8; 32], opts: DecryptOptions, ) -> Result { + // Cipher-key encryption: when this item carries its own `key`, every + // field below is encrypted under that per-item key. Shadow the user key + // with it so the rest of this method needs no special-casing. + let item_key = self.item_field_key(enc_key, mac_key)?; + let (enc_key, mac_key): (&[u8; 32], &[u8; 32]) = match &item_key { + Some((e, m)) => (&**e, &**m), + None => (enc_key, mac_key), + }; + let name = decrypt_optional(self.name.as_deref(), enc_key, mac_key)?; let notes = if opts.notes { decrypt_optional(self.notes.as_deref(), enc_key, mac_key)? @@ -521,6 +574,10 @@ impl Cipher { cipher_type: plain.cipher_type, folder_id: plain.folder_id.clone(), organization_id: None, + // Vault writes fields under the user key directly (no per-cipher + // key); the server accepts this. Round-tripping a server item's own + // `key` would require re-wrapping it, which write paths don't do. + key: None, name: plain.name.as_deref().map(&enc), notes: plain.notes.as_deref().map(&enc), login, @@ -573,3 +630,92 @@ fn decrypt_optional( let txt = String::from_utf8(pt).map_err(|_| Error::EncString("field is not valid UTF-8"))?; Ok(Some(txt)) } + +#[cfg(test)] +mod tests { + use super::{Cipher, DecryptOptions, Login}; + use crate::enc_string::EncString; + + const USER_ENC: [u8; 32] = [1u8; 32]; + const USER_MAC: [u8; 32] = [2u8; 32]; + const ITEM_ENC: [u8; 32] = [3u8; 32]; + const ITEM_MAC: [u8; 32] = [4u8; 32]; + + fn under(enc: &[u8; 32], mac: &[u8; 32], plain: &str) -> String { + EncString::encrypt(enc, mac, plain.as_bytes()).serialize() + } + + /// "Cipher key encryption": the item's fields are encrypted under a per-item + /// key, and that key is an `EncString` (64 bytes) wrapped under the user key. + /// Regression guard — decrypting fields with the user key directly fails MAC. + #[test] + fn cipher_key_encryption_round_trips() { + // The 64-byte item key (enc‖mac) wrapped under the user key. + let mut material = [0u8; 64]; + material[..32].copy_from_slice(&ITEM_ENC); + material[32..].copy_from_slice(&ITEM_MAC); + let wrapped = EncString::encrypt(&USER_ENC, &USER_MAC, &material).serialize(); + + let cipher = Cipher { + cipher_type: 1, + key: Some(wrapped), + // name + password encrypted under the ITEM key, not the user key. + name: Some(under(&ITEM_ENC, &ITEM_MAC, "github.com")), + login: Some(Login { + password: Some(under(&ITEM_ENC, &ITEM_MAC, "hunter2")), + ..Login::default() + }), + ..Cipher::default() + }; + + assert_eq!( + cipher + .decrypt_name(&USER_ENC, &USER_MAC) + .unwrap() + .as_deref(), + Some("github.com"), + ); + let plain = cipher + .decrypt(&USER_ENC, &USER_MAC, DecryptOptions::all()) + .unwrap(); + assert_eq!(plain.password.as_deref(), Some("hunter2")); + } + + /// Items without a per-cipher `key` (older vaults) still decrypt directly + /// under the user key. + #[test] + fn no_cipher_key_uses_user_key_directly() { + let cipher = Cipher { + cipher_type: 2, + key: None, + name: Some(under(&USER_ENC, &USER_MAC, "legacy note")), + ..Cipher::default() + }; + assert_eq!( + cipher + .decrypt_name(&USER_ENC, &USER_MAC) + .unwrap() + .as_deref(), + Some("legacy note"), + ); + } + + /// A `key` that can't be unwrapped under the user key (e.g. an organization + /// item) surfaces as an error rather than silently mis-decrypting. + #[test] + fn unwrappable_cipher_key_errors() { + let wrong = [9u8; 32]; + let mut material = [0u8; 64]; + material[..32].copy_from_slice(&ITEM_ENC); + material[32..].copy_from_slice(&ITEM_MAC); + // Key wrapped under a DIFFERENT user key than we'll decrypt with. + let wrapped = EncString::encrypt(&wrong, &wrong, &material).serialize(); + let cipher = Cipher { + cipher_type: 1, + key: Some(wrapped), + name: Some(under(&ITEM_ENC, &ITEM_MAC, "github.com")), + ..Cipher::default() + }; + assert!(cipher.decrypt_name(&USER_ENC, &USER_MAC).is_err()); + } +}