Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 61 additions & 9 deletions crates/vault-agent/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ impl AgentState {
pub fn list_entries(&self) -> Result<Vec<ListEntry>, 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
Expand All @@ -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
};
Expand All @@ -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)
}
Expand All @@ -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
{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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() {
Expand Down
53 changes: 34 additions & 19 deletions crates/vault-agent/src/unlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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![],
Expand Down Expand Up @@ -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![],
Expand Down Expand Up @@ -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![],
Expand Down
73 changes: 73 additions & 0 deletions crates/vault-api/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
//! the master-password hash.

use reqwest::Client;
use reqwest::header::{HeaderMap, HeaderValue};
use uuid::Uuid;
use zeroize::Zeroizing;

Expand All @@ -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
Expand Down Expand Up @@ -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).
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
);
}
}
Loading
Loading