From fd7e1be7884177941a330741ccd9f26774980995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Sat, 1 Aug 2026 03:39:45 +0200 Subject: [PATCH 1/4] Implement V2 registration support Signed-off-by: BlackDex --- .typos.toml | 1 + Cargo.toml | 2 +- .../2026-08-01-120000_user_crypto_v2/down.sql | 7 + .../2026-08-01-120000_user_crypto_v2/up.sql | 18 + .../2026-08-01-120000_user_crypto_v2/down.sql | 7 + .../2026-08-01-120000_user_crypto_v2/up.sql | 17 + .../2026-08-01-120000_user_crypto_v2/down.sql | 7 + .../2026-08-01-120000_user_crypto_v2/up.sql | 17 + src/api/core/accounts.rs | 345 +++++++++++++++--- src/api/core/ciphers.rs | 1 + src/api/identity.rs | 26 +- src/db/models/mod.rs | 2 + src/db/models/organization.rs | 5 + src/db/models/user.rs | 91 ++++- src/db/models/user_signature_key_pair.rs | 169 +++++++++ src/db/schema.rs | 19 + 16 files changed, 643 insertions(+), 91 deletions(-) create mode 100644 migrations/mysql/2026-08-01-120000_user_crypto_v2/down.sql create mode 100644 migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql create mode 100644 migrations/postgresql/2026-08-01-120000_user_crypto_v2/down.sql create mode 100644 migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql create mode 100644 migrations/sqlite/2026-08-01-120000_user_crypto_v2/down.sql create mode 100644 migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql create mode 100644 src/db/models/user_signature_key_pair.rs diff --git a/.typos.toml b/.typos.toml index 87c0c4a6..7034a849 100644 --- a/.typos.toml +++ b/.typos.toml @@ -15,6 +15,7 @@ extend-ignore-re = [ "(?i)helo_name", "Server name sent during.+HELO", # COSE Is short for CBOR Object Signing and Encryption, ignore these specific items + "COSE", "COSEKey", "COSEAlgorithm", # Ignore this specific string as it's valid diff --git a/Cargo.toml b/Cargo.toml index 7dcae503..a419b12b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,7 +107,7 @@ serde = { version = "1.0.229", features = ["derive"] } serde_json = "1.0.151" # A safe, extensible ORM and Query builder -diesel = { version = "2.3.13", features = ["chrono", "r2d2", "numeric"] } +diesel = { version = "2.3.13", features = ["chrono", "r2d2", "numeric", "64-column-tables"] } diesel_migrations = "2.3.2" derive_more = { version = "2.1.1", features = [ diff --git a/migrations/mysql/2026-08-01-120000_user_crypto_v2/down.sql b/migrations/mysql/2026-08-01-120000_user_crypto_v2/down.sql new file mode 100644 index 00000000..b57c715e --- /dev/null +++ b/migrations/mysql/2026-08-01-120000_user_crypto_v2/down.sql @@ -0,0 +1,7 @@ +DROP TABLE IF EXISTS user_signature_key_pairs; + +ALTER TABLE users DROP COLUMN signed_public_key; +ALTER TABLE users DROP COLUMN security_state; +ALTER TABLE users DROP COLUMN security_version; +ALTER TABLE users DROP COLUMN v2_upgrade_token; +ALTER TABLE users_organizations DROP COLUMN v2_upgrade_token; diff --git a/migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql b/migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql new file mode 100644 index 00000000..e1bb52a8 --- /dev/null +++ b/migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql @@ -0,0 +1,18 @@ +ALTER TABLE users ADD COLUMN signed_public_key TEXT; +ALTER TABLE users ADD COLUMN security_state TEXT; +ALTER TABLE users ADD COLUMN security_version INTEGER; +ALTER TABLE users ADD COLUMN v2_upgrade_token TEXT; +ALTER TABLE users_organizations ADD COLUMN v2_upgrade_token TEXT; + +DROP TABLE IF EXISTS user_signature_key_pairs; + +CREATE TABLE user_signature_key_pairs ( + uuid CHAR(36) NOT NULL PRIMARY KEY, + user_uuid CHAR(36) NOT NULL UNIQUE, + signature_algorithm INTEGER NOT NULL, -- 0 = ed25519, 1 = mldsa44 + signing_key TEXT NOT NULL, + verifying_key TEXT NOT NULL, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL, + FOREIGN KEY (user_uuid) REFERENCES users (uuid) ON DELETE CASCADE +); diff --git a/migrations/postgresql/2026-08-01-120000_user_crypto_v2/down.sql b/migrations/postgresql/2026-08-01-120000_user_crypto_v2/down.sql new file mode 100644 index 00000000..b57c715e --- /dev/null +++ b/migrations/postgresql/2026-08-01-120000_user_crypto_v2/down.sql @@ -0,0 +1,7 @@ +DROP TABLE IF EXISTS user_signature_key_pairs; + +ALTER TABLE users DROP COLUMN signed_public_key; +ALTER TABLE users DROP COLUMN security_state; +ALTER TABLE users DROP COLUMN security_version; +ALTER TABLE users DROP COLUMN v2_upgrade_token; +ALTER TABLE users_organizations DROP COLUMN v2_upgrade_token; diff --git a/migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql b/migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql new file mode 100644 index 00000000..0e2527e0 --- /dev/null +++ b/migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql @@ -0,0 +1,17 @@ +ALTER TABLE users ADD COLUMN signed_public_key TEXT; +ALTER TABLE users ADD COLUMN security_state TEXT; +ALTER TABLE users ADD COLUMN security_version INTEGER; +ALTER TABLE users ADD COLUMN v2_upgrade_token TEXT; +ALTER TABLE users_organizations ADD COLUMN v2_upgrade_token TEXT; + +DROP TABLE IF EXISTS user_signature_key_pairs; + +CREATE TABLE user_signature_key_pairs ( + uuid CHAR(36) NOT NULL PRIMARY KEY, + user_uuid CHAR(36) NOT NULL UNIQUE REFERENCES users (uuid) ON DELETE CASCADE, + signature_algorithm INTEGER NOT NULL, -- 0 = ed25519, 1 = mldsa44 + signing_key TEXT NOT NULL, + verifying_key TEXT NOT NULL, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL +); diff --git a/migrations/sqlite/2026-08-01-120000_user_crypto_v2/down.sql b/migrations/sqlite/2026-08-01-120000_user_crypto_v2/down.sql new file mode 100644 index 00000000..b57c715e --- /dev/null +++ b/migrations/sqlite/2026-08-01-120000_user_crypto_v2/down.sql @@ -0,0 +1,7 @@ +DROP TABLE IF EXISTS user_signature_key_pairs; + +ALTER TABLE users DROP COLUMN signed_public_key; +ALTER TABLE users DROP COLUMN security_state; +ALTER TABLE users DROP COLUMN security_version; +ALTER TABLE users DROP COLUMN v2_upgrade_token; +ALTER TABLE users_organizations DROP COLUMN v2_upgrade_token; diff --git a/migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql b/migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql new file mode 100644 index 00000000..0db12ac3 --- /dev/null +++ b/migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql @@ -0,0 +1,17 @@ +ALTER TABLE users ADD COLUMN signed_public_key TEXT; +ALTER TABLE users ADD COLUMN security_state TEXT; +ALTER TABLE users ADD COLUMN security_version INTEGER; +ALTER TABLE users ADD COLUMN v2_upgrade_token TEXT; +ALTER TABLE users_organizations ADD COLUMN v2_upgrade_token TEXT; + +DROP TABLE IF EXISTS user_signature_key_pairs; + +CREATE TABLE user_signature_key_pairs ( + uuid TEXT NOT NULL PRIMARY KEY, + user_uuid TEXT NOT NULL UNIQUE REFERENCES users (uuid) ON DELETE CASCADE, + signature_algorithm INTEGER NOT NULL, -- 0 = ed25519, 1 = mldsa44 + signing_key TEXT NOT NULL, + verifying_key TEXT NOT NULL, + created_at DATETIME NOT NULL, + updated_at DATETIME NOT NULL +); diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 8cc5e55b..e022807e 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -22,8 +22,8 @@ use crate::{ models::{ AuthRequest, AuthRequestId, Cipher, CipherId, Device, DeviceId, DeviceType, DeviceWithAuthRequest, EmergencyAccess, EmergencyAccessId, EventType, Folder, FolderId, Invitation, KeyId, Membership, - MembershipId, OrgPolicy, OrgPolicyType, Organization, OrganizationId, Send, SendId, User, UserId, - UserKdfType, + MembershipId, OrgPolicy, OrgPolicyType, Organization, OrganizationId, Send, SendId, SignatureAlgorithm, + User, UserId, UserKdfType, UserSignatureKeyPair, }, }, mail, @@ -42,6 +42,8 @@ pub fn routes() -> Vec { post_profile, put_avatar, get_public_keys, + get_account_public_keys, + get_keys, post_keys, post_password, post_set_password, @@ -104,6 +106,9 @@ pub struct RegisterData { #[serde(alias = "userAsymmetricKeys")] keys: Option, + // Supersedes `keys`, and the only way a v2 account can be registered. + account_keys: Option, + master_password_hint: Option, name: Option, @@ -118,36 +123,6 @@ pub struct RegisterData { org_invite_token: Option, } -impl RegisterData { - fn hash(&self) -> String { - self.compat.fold(|rdc| &rdc.master_password_hash, |rdcu| &rdcu.master_password_authentication.hash).to_owned() - } - - fn kdf(&self) -> &KDFData { - self.compat.fold(|rdc| &rdc.kdf, |rdcu| &rdcu.master_password_authentication.kdf) - } - - fn key(&self) -> String { - self.compat.fold(|rdc| &rdc.key, |rdcu| &rdcu.master_password_unlock.key).to_owned() - } - - // When comparing with salt, email need to be normalized: - // - https://github.com/bitwarden/clients/blob/web-v2026.5.0/libs/common/src/key-management/master-password/services/master-password.service.ts#L171 - fn unprocessable(&self) -> bool { - let mut unprocessable = false; - *self.compat.fold( - |_| &false, - |rdcu| { - let email = self.email.trim().to_lowercase(); - unprocessable = rdcu.master_password_authentication.kdf != rdcu.master_password_unlock.kdf - || rdcu.master_password_authentication.salt != email - || rdcu.master_password_unlock.salt != email; - &unprocessable - }, - ) - } -} - #[derive(Debug, Deserialize)] struct RegisterDataOld { #[serde(flatten)] @@ -185,6 +160,34 @@ impl RegisterDataCompat { RegisterDataCompat::RegisterDataCur(rdcu) => fcu(rdcu), } } + + fn hash(&self) -> String { + self.fold(|rdc| &rdc.master_password_hash, |rdcu| &rdcu.master_password_authentication.hash).to_owned() + } + + fn kdf(&self) -> &KDFData { + self.fold(|rdc| &rdc.kdf, |rdcu| &rdcu.master_password_authentication.kdf) + } + + fn key(&self) -> String { + self.fold(|rdc| &rdc.key, |rdcu| &rdcu.master_password_unlock.key).to_owned() + } + + // When comparing with salt, email need to be normalized: + // - https://github.com/bitwarden/clients/blob/web-v2026.5.0/libs/common/src/key-management/master-password/services/master-password.service.ts#L171 + fn unprocessable(&self, email: &str) -> bool { + let mut unprocessable = false; + *self.fold( + |_| &false, + |rdcu| { + let email = email.trim().to_lowercase(); + unprocessable = rdcu.master_password_authentication.kdf != rdcu.master_password_unlock.kdf + || rdcu.master_password_authentication.salt != email + || rdcu.master_password_unlock.salt != email; + &unprocessable + }, + ) + } } #[derive(Debug, Deserialize)] @@ -194,6 +197,167 @@ struct KeysData { public_key: String, } +/// The `accountKeys` payload, which replaces the flat `keys`/`userAsymmetricKeys` object. +/// +/// It carries either a "v1" state (just the encryption key pair) or a "v2" one, which adds a +/// signature key pair, a signed public key, and a signed security state. The two deprecated +/// top-level fields are still sent by the SDK alongside the nested ones and are only used as a +/// fallback for clients that don't send `publicKeyEncryptionKeyPair` yet. +/// +/// Ref: +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AccountKeysData { + user_key_encrypted_account_private_key: Option, + account_public_key: Option, + + public_key_encryption_key_pair: Option, + signature_key_pair: Option, + security_state: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PublicKeyEncryptionKeyPairData { + wrapped_private_key: String, + public_key: String, + signed_public_key: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct SignatureKeyPairData { + signature_algorithm: String, + wrapped_signing_key: String, + verifying_key: String, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct SecurityStateData { + security_state: String, + security_version: i32, +} + +pub struct ValidatedAccountKeys { + private_key: String, + public_key: String, + v2: Option, +} + +struct ValidatedV2AccountKeys { + signed_public_key: String, + signing_key: String, + verifying_key: String, + signature_algorithm: SignatureAlgorithm, + security_state: String, + security_version: i32, +} + +impl AccountKeysData { + /// Checks that the payload describes a complete account cryptographic state. + /// + /// The v2 fields have to be all present or all absent: a client that receives a COSE-wrapped + /// private key without the matching signature key pair and security state refuses to unlock the + /// vault, so storing half a state would produce an account nobody can log into. + pub fn validate(self) -> ApiResult { + let (private_key, public_key, signed_public_key) = if let Some(key_pair) = self.public_key_encryption_key_pair { + (key_pair.wrapped_private_key, key_pair.public_key, key_pair.signed_public_key) + // Older clients only send the deprecated top-level fields, which are always v1. + } else if let (Some(private_key), Some(public_key)) = + (self.user_key_encrypted_account_private_key, self.account_public_key) + { + (private_key, public_key, None) + } else { + err!("The account keys are missing an encryption key pair") + }; + + let v2 = match (signed_public_key, self.signature_key_pair, self.security_state) { + (Some(signed_public_key), Some(signature_key_pair), Some(security_state)) => { + let Some(signature_algorithm) = SignatureAlgorithm::from_str(&signature_key_pair.signature_algorithm) + else { + err!(format!("Unsupported signature algorithm: {}", signature_key_pair.signature_algorithm)) + }; + + Some(ValidatedV2AccountKeys { + signed_public_key, + signing_key: signature_key_pair.wrapped_signing_key, + verifying_key: signature_key_pair.verifying_key, + signature_algorithm, + security_state: security_state.security_state, + security_version: security_state.security_version, + }) + } + (None, None, None) => None, + _ => err!( + "Invalid account keys: the signed public key, signature key pair and security state must either all be present or all be absent" + ), + }; + + Ok(ValidatedAccountKeys { + private_key, + public_key, + v2, + }) + } +} + +impl From for ValidatedAccountKeys { + fn from(keys: KeysData) -> Self { + Self { + private_key: keys.encrypted_private_key, + public_key: keys.public_key, + v2: None, + } + } +} + +impl ValidatedAccountKeys { + /// Writes the parts of the state that live on the user itself. The user still needs saving, and + /// [`Self::save_signature_key_pair`] still needs calling once it has been. + /// + /// Rejects downgrading an account from v2 back to v1. + pub fn apply(&self, user: &mut User) -> EmptyResult { + if user.is_v2() && self.v2.is_none() { + err!("Cannot downgrade an account from v2 to v1 encryption") + } + + user.private_key = Some(self.private_key.clone()); + user.public_key = Some(self.public_key.clone()); + + user.signed_public_key = self.v2.as_ref().map(|v2| v2.signed_public_key.clone()); + user.security_state = self.v2.as_ref().map(|v2| v2.security_state.clone()); + user.security_version = self.v2.as_ref().map(|v2| v2.security_version); + + Ok(()) + } + + /// Persists the signature key pair. Separate from [`Self::apply`] because the row has a foreign + /// key to the user, so it can only be written once the user exists. + pub async fn save_signature_key_pair(&self, user_id: &UserId, conn: &DbConn) -> EmptyResult { + // Skip if the account is v1, since v1 accounts don't have a signature key pair. + let Some(v2) = &self.v2 else { + return Ok(()); + }; + + let mut key_pair = match UserSignatureKeyPair::find_active_by_user(user_id, conn).await { + Some(mut key_pair) => { + key_pair.signature_algorithm = v2.signature_algorithm as i32; + key_pair.signing_key.clone_from(&v2.signing_key); + key_pair.verifying_key.clone_from(&v2.verifying_key); + key_pair + } + None => UserSignatureKeyPair::new( + user_id.clone(), + v2.signature_algorithm, + v2.signing_key.clone(), + v2.verifying_key.clone(), + ), + }; + key_pair.save(conn).await + } +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct MasterPasswordAuthentication { @@ -218,11 +382,12 @@ pub struct MasterPasswordUnlock { #[serde(rename_all = "camelCase")] pub struct SetPasswordData { #[serde(flatten)] - kdf: KDFData, + compat: RegisterDataCompat, - key: String, keys: Option, - master_password_hash: String, + // Supersedes `keys`, and the only way a v2 account can be initialized here. + account_keys: Option, + master_password_hint: Option, org_identifier: Option, } @@ -265,7 +430,7 @@ pub async fn register(data: Json, email_verification: bool, conn: let mut pending_emergency_access = None; - if data.unprocessable() { + if data.compat.unprocessable(&data.email) { err_code!("Unexpected RegisterData format", Status::UnprocessableEntity.code); } @@ -388,9 +553,9 @@ pub async fn register(data: Json, email_verification: bool, conn: // Make sure we don't leave a lingering invitation. Invitation::take(&email, &conn).await; - set_kdf_data(&mut user, data.kdf())?; + set_kdf_data(&mut user, data.compat.kdf())?; - user.set_password(&data.hash(), Some(data.key()), true, None, &conn).await?; + user.set_password(&data.compat.hash(), Some(data.compat.key()), true, None, &conn).await?; user.password_hint = password_hint; // Add extra fields if present @@ -398,9 +563,13 @@ pub async fn register(data: Json, email_verification: bool, conn: user.name = name; } - if let Some(keys) = data.keys { - user.private_key = Some(keys.encrypted_private_key); - user.public_key = Some(keys.public_key); + let account_keys = match (data.account_keys, data.keys) { + (Some(account_keys), _) => Some(account_keys.validate()?), + (None, Some(keys)) => Some(keys.into()), + (None, None) => None, + }; + if let Some(ref account_keys) = account_keys { + account_keys.apply(&mut user)?; } if email_verified { @@ -424,6 +593,10 @@ pub async fn register(data: Json, email_verification: bool, conn: user.save(&conn).await?; + if let Some(account_keys) = account_keys { + account_keys.save_signature_key_pair(&user.uuid, &conn).await?; + } + // accept any open emergency access invitations if !CONFIG.mail_enabled() && CONFIG.emergency_access_allowed() { for mut emergency_invite in EmergencyAccess::find_all_invited_by_grantee_email(&user.email, &conn).await { @@ -446,16 +619,26 @@ async fn post_set_password(data: Json, headers: Headers, conn: err!("Account already initialized, cannot set password") } + if data.compat.unprocessable(&user.email) { + err_code!("Unexpected SetPasswordData format", Status::UnprocessableEntity.code); + } + // Check against the password hint setting here so if it fails, // the user can retry without losing their invitation below. let password_hint = clean_password_hint(data.master_password_hint.as_ref()); enforce_password_hint_setting(password_hint.as_ref())?; - set_kdf_data(&mut user, &data.kdf)?; + let account_keys = match (data.account_keys, data.keys) { + (Some(account_keys), _) => Some(account_keys.validate()?), + (None, Some(keys)) => Some(keys.into()), + (None, None) => None, + }; + + set_kdf_data(&mut user, data.compat.kdf())?; user.set_password( - &data.master_password_hash, - Some(data.key), + &data.compat.hash(), + Some(data.compat.key()), false, Some(vec![String::from("revision_date")]), // We need to allow revision-date to use the old security_timestamp &conn, @@ -463,9 +646,8 @@ async fn post_set_password(data: Json, headers: Headers, conn: .await?; user.password_hint = password_hint; - if let Some(keys) = data.keys { - user.private_key = Some(keys.encrypted_private_key); - user.public_key = Some(keys.public_key); + if let Some(ref account_keys) = account_keys { + account_keys.apply(&mut user)?; } if let Some(identifier) = data.org_identifier @@ -494,6 +676,10 @@ async fn post_set_password(data: Json, headers: Headers, conn: user.save(&conn).await?; + if let Some(account_keys) = account_keys { + account_keys.save_signature_key_pair(&user.uuid, &conn).await?; + } + Ok(Json(json!({ "object": "set-password", "captchaBypassToken": "", @@ -575,20 +761,77 @@ async fn get_public_keys(user_id: UserId, _headers: Headers, conn: DbConn) -> Js }))) } +#[get("/users//keys")] +async fn get_account_public_keys(user_id: UserId, _headers: Headers, conn: DbConn) -> JsonResult { + let user = match User::find_by_uuid(&user_id, &conn).await { + Some(user) if user.public_key.is_some() => user, + Some(_) => err_code!("User has no public_key", Status::NotFound.code), + None => err_code!("User doesn't exist", Status::NotFound.code), + }; + + Ok(Json(user.public_keys_json(&conn).await)) +} + +#[get("/accounts/keys")] +async fn get_keys(headers: Headers, conn: DbConn) -> JsonResult { + let user = headers.user; + + Ok(Json(json!({ + "key": user.akey, + "privateKey": user.private_key, + "publicKey": user.public_key, + "accountKeys": user.account_keys_json(&conn).await, + "object": "keys" + }))) +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PostKeysData { + #[serde(flatten)] + keys: Option, + account_keys: Option, + // The id of the user key these account keys belong to, only honored for v2 `accountKeys` + user_key_id: Option, +} + #[post("/accounts/keys", data = "")] -async fn post_keys(data: Json, headers: Headers, conn: DbConn) -> JsonResult { - let data: KeysData = data.into_inner(); +async fn post_keys(data: Json, headers: Headers, conn: DbConn) -> JsonResult { + let data: PostKeysData = data.into_inner(); let mut user = headers.user; - user.private_key = Some(data.encrypted_private_key); - user.public_key = Some(data.public_key); + // This only sets the keys of an account that has none yet. Replacing existing ones is what a key + // rotation does, with the checks that go with it. + if user.private_key.is_some() || user.public_key.is_some() { + err!("User has existing keypair") + } + // `accountKeys` supersedes the flat `keys` object when both are sent. + let account_keys = match (data.account_keys, data.keys) { + (Some(account_keys), _) => { + let account_keys = account_keys.validate()?; + if account_keys.v2.is_none() { + err!("AccountKeys are only supported for V2 encryption.") + } + if let Some(user_key_id) = data.user_key_id { + user.key_id = Some(user_key_id); + } + account_keys + } + (None, Some(keys)) => keys.into(), + (None, None) => err!("No account keys provided"), + }; + + account_keys.apply(&mut user)?; user.save(&conn).await?; + account_keys.save_signature_key_pair(&user.uuid, &conn).await?; Ok(Json(json!({ + "key": user.akey, "privateKey": user.private_key, "publicKey": user.public_key, + "accountKeys": user.account_keys_json(&conn).await, "object":"keys" }))) } diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index a5b7e58b..2831a1ad 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -207,6 +207,7 @@ async fn sync(data: SyncData, headers: Headers, client_version: Option, pub external_id: Option, + /// A copy of the user's v2 upgrade token, kept when a v1 -> v2 upgrade leaves the account + /// recovery key (`reset_password_key`) wrapped by the v1 user key. It lets the organization + /// unwrap the v2 user key from it. Opaque to us, like `User::v2_upgrade_token`. + pub v2_upgrade_token: Option, } #[derive(Identifiable, Queryable, Insertable, AsChangeset)] @@ -274,6 +278,7 @@ impl Membership { atype: MembershipType::User as i32, reset_password_key: None, external_id: None, + v2_upgrade_token: None, } } diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 3412b142..b382c8f0 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -20,6 +20,7 @@ use macros::UuidFromParam; use super::{ Cipher, Device, EmergencyAccess, Favorite, Folder, Membership, MembershipType, TwoFactor, TwoFactorIncomplete, + UserSignatureKeyPair, }; #[derive(Identifiable, Queryable, Insertable, AsChangeset, Selectable)] @@ -71,6 +72,15 @@ pub struct User { pub external_id: Option, // Todo: Needs to be removed in the future, this is not used anymore. pub key_id: Option, + + // "v2" account cryptographic state. Either all of these are set (together with a row in + // `user_signature_key_pairs`) or none of them are; see `User::is_v2`. + pub signed_public_key: Option, + pub security_state: Option, + pub security_version: Option, + /// JSON `{"wrappedUserKey1": ..., "wrappedUserKey2": ...}`, letting clients that still hold the + /// v1 user key obtain the v2 one after another client performed the upgrade. Opaque to us. + pub v2_upgrade_token: Option, } #[derive(Identifiable, Queryable, Insertable)] @@ -158,9 +168,18 @@ impl User { external_id: None, // Todo: Needs to be removed in the future, this is not used anymore. key_id: None, + + signed_public_key: None, + security_state: None, + security_version: None, + v2_upgrade_token: None, } } + pub fn is_v2(&self) -> bool { + self.signed_public_key.is_some() && self.security_state.is_some() && self.security_version.is_some() + } + pub fn check_valid_password(&self, password: &str) -> bool { crypto::verify_password_hash( password.as_bytes(), @@ -257,6 +276,61 @@ impl User { /// Database methods impl User { + async fn v2_signature_key_pair(&self, conn: &DbConn) -> Option { + if !self.is_v2() { + return None; + } + UserSignatureKeyPair::find_active_by_user(&self.uuid, conn).await + } + + pub async fn account_keys_json(&self, conn: &DbConn) -> Value { + if self.private_key.is_none() { + return Value::Null; + } + + let (signed_public_key, signature_key_pair, security_state) = match self.v2_signature_key_pair(conn).await { + Some(key_pair) => ( + json!(self.signed_public_key), + key_pair.to_json(), + json!({ + "securityState": self.security_state, + "securityVersion": self.security_version, + }), + ), + None => (Value::Null, Value::Null, Value::Null), + }; + + json!({ + "publicKeyEncryptionKeyPair": { + "wrappedPrivateKey": self.private_key, + "publicKey": self.public_key, + "signedPublicKey": signed_public_key, + "object": "publicKeyEncryptionKeyPair", + }, + "signatureKeyPair": signature_key_pair, + "securityState": security_state, + "object": "privateKeys" + }) + } + + pub async fn public_keys_json(&self, conn: &DbConn) -> Value { + let (signed_public_key, verifying_key) = match self.v2_signature_key_pair(conn).await { + Some(key_pair) => (json!(self.signed_public_key), json!(key_pair.verifying_key)), + None => (Value::Null, Value::Null), + }; + + json!({ + "publicKey": self.public_key, + "signedPublicKey": signed_public_key, + "verifyingKey": verifying_key, + "object": "publicKeys" + }) + } + + pub fn v2_upgrade_token_json(&self) -> Value { + self.v2_upgrade_token.as_ref().and_then(|token| serde_json::from_str(token).ok()).unwrap_or(Value::Null) + } + pub async fn to_json(&self, conn: &DbConn) -> Value { let mut orgs_json = Vec::new(); for c in Membership::find_confirmed_by_user(&self.uuid, conn).await { @@ -277,21 +351,7 @@ impl User { UserStatus::Enabled }; - let account_keys = if self.private_key.is_some() { - json!({ - "publicKeyEncryptionKeyPair": { - "wrappedPrivateKey": self.private_key, - "publicKey": self.public_key, - "signedPublicKey": null, - "object": "publicKeyEncryptionKeyPair", - }, - "securityState": null, - "signatureKeyPair": null, - "object": "privateKeys" - }) - } else { - Value::Null - }; + let account_keys = self.account_keys_json(conn).await; json!({ "_status": status as i32, @@ -367,6 +427,7 @@ impl User { Device::delete_all_by_user(&self.uuid, conn).await?; TwoFactor::delete_all_by_user(&self.uuid, conn).await?; TwoFactorIncomplete::delete_all_by_user(&self.uuid, conn).await?; + UserSignatureKeyPair::delete_all_by_user(&self.uuid, conn).await?; Invitation::take(&self.email, conn).await; // Delete invitation if any conn.run(move |conn| { diff --git a/src/db/models/user_signature_key_pair.rs b/src/db/models/user_signature_key_pair.rs new file mode 100644 index 00000000..b7e58f3a --- /dev/null +++ b/src/db/models/user_signature_key_pair.rs @@ -0,0 +1,169 @@ +use chrono::{NaiveDateTime, Utc}; +use derive_more::{AsRef, Deref, Display, From}; +use diesel::prelude::*; +use serde_json::Value; + +use crate::{ + api::EmptyResult, + db::{DbConn, schema::user_signature_key_pairs}, + error::MapResult, + util::get_uuid, +}; +use macros::UuidFromParam; + +use super::UserId; + +/// A user's signature key pair, part of the "v2" account cryptographic state. +/// +/// Upstream keeps this in its own table rather than as columns on the user, with a unique index on +/// the user id. The stated intent is to eventually keep superseded key pairs around (an `active` +/// flag was sketched but not shipped), which is why this is modelled as a row with its own identity +/// instead of a set of user attributes. +/// +/// Ref: +#[derive(Identifiable, Queryable, Insertable, AsChangeset, Selectable)] +#[diesel(table_name = user_signature_key_pairs)] +#[diesel(treat_none_as_null = true)] +#[diesel(primary_key(uuid))] +pub struct UserSignatureKeyPair { + pub uuid: UserSignatureKeyPairId, + pub user_uuid: UserId, + + pub signature_algorithm: i32, + /// The signing (private) key, wrapped by the user key. + pub signing_key: String, + /// The COSE-encoded public verifying key. + pub verifying_key: String, + + pub created_at: NaiveDateTime, + pub updated_at: NaiveDateTime, +} + +/// https://github.com/bitwarden/server/blob/main/src/Core/KeyManagement/Enums/SignatureAlgorithm.cs +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SignatureAlgorithm { + Ed25519 = 0, + MlDsa44 = 1, +} + +impl SignatureAlgorithm { + pub fn from_str(algorithm: &str) -> Option { + match algorithm { + "ed25519" => Some(Self::Ed25519), + "mldsa44" => Some(Self::MlDsa44), + _ => None, + } + } + + pub fn from_i32(algorithm: i32) -> Option { + match algorithm { + 0 => Some(Self::Ed25519), + 1 => Some(Self::MlDsa44), + _ => None, + } + } + + pub fn as_str(self) -> &'static str { + match self { + Self::Ed25519 => "ed25519", + Self::MlDsa44 => "mldsa44", + } + } +} + +/// Local methods +impl UserSignatureKeyPair { + pub fn new( + user_uuid: UserId, + signature_algorithm: SignatureAlgorithm, + signing_key: String, + verifying_key: String, + ) -> Self { + let now = Utc::now().naive_utc(); + + Self { + uuid: UserSignatureKeyPairId(get_uuid()), + user_uuid, + signature_algorithm: signature_algorithm as i32, + signing_key, + verifying_key, + created_at: now, + updated_at: now, + } + } + + pub fn to_json(&self) -> Value { + json!({ + "wrappedSigningKey": self.signing_key, + "verifyingKey": self.verifying_key, + "object": "signatureKeyPair", + }) + } +} + +/// Database methods +impl UserSignatureKeyPair { + pub async fn save(&mut self, conn: &DbConn) -> EmptyResult { + self.updated_at = Utc::now().naive_utc(); + + db_run! { conn: + mysql { + diesel::insert_into(user_signature_key_pairs::table) + .values(&*self) + .on_conflict(diesel::dsl::DuplicatedKeys) + .do_update() + .set(&*self) + .execute(conn) + .map_res("Error saving user signature key pair") + } + postgresql, sqlite { + diesel::insert_into(user_signature_key_pairs::table) + .values(&*self) + .on_conflict(user_signature_key_pairs::user_uuid) + .do_update() + .set(&*self) + .execute(conn) + .map_res("Error saving user signature key pair") + } + } + } + + /// The key pair currently in use by the user. There is at most one today, enforced by a unique + /// index on `user_uuid`. + pub async fn find_active_by_user(user_uuid: &UserId, conn: &DbConn) -> Option { + conn.run(move |conn| { + user_signature_key_pairs::table + .filter(user_signature_key_pairs::user_uuid.eq(user_uuid)) + .first::(conn) + .ok() + }) + .await + } + + pub async fn delete_all_by_user(user_uuid: &UserId, conn: &DbConn) -> EmptyResult { + conn.run(move |conn| { + diesel::delete(user_signature_key_pairs::table.filter(user_signature_key_pairs::user_uuid.eq(user_uuid))) + .execute(conn) + .map_res("Error deleting user signature key pairs") + }) + .await + } +} + +#[derive( + Clone, + Debug, + AsRef, + Deref, + DieselNewType, + Display, + From, + FromForm, + Hash, + PartialEq, + Eq, + Serialize, + Deserialize, + UuidFromParam, +)] +pub struct UserSignatureKeyPairId(String); diff --git a/src/db/schema.rs b/src/db/schema.rs index 98b1eda6..00a58a75 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -218,6 +218,22 @@ table! { avatar_color -> Nullable, external_id -> Nullable, key_id -> Nullable, + signed_public_key -> Nullable, + security_state -> Nullable, + security_version -> Nullable, + v2_upgrade_token -> Nullable, + } +} + +table! { + user_signature_key_pairs (uuid) { + uuid -> Text, + user_uuid -> Text, + signature_algorithm -> Integer, + signing_key -> Text, + verifying_key -> Text, + created_at -> Timestamp, + updated_at -> Timestamp, } } @@ -243,6 +259,7 @@ table! { atype -> Integer, reset_password_key -> Nullable, external_id -> Nullable, + v2_upgrade_token -> Nullable, } } @@ -383,6 +400,7 @@ joinable!(collections_groups -> groups (groups_uuid)); joinable!(event -> users_organizations (uuid)); joinable!(auth_requests -> users (user_uuid)); joinable!(sso_users -> users (user_uuid)); +joinable!(user_signature_key_pairs -> users (user_uuid)); allow_tables_to_appear_in_same_query!( archives, @@ -409,4 +427,5 @@ allow_tables_to_appear_in_same_query!( collections_groups, event, auth_requests, + user_signature_key_pairs, ); From 72a39ae90b860957f5ecbf811458ad169a1188ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Wed, 23 Sep 2026 14:36:55 +0200 Subject: [PATCH 2/4] Implement V2 key rotation support --- src/api/core/accounts.rs | 582 +++++++++++++++++++++++++++++++--- src/api/core/ciphers.rs | 79 ++++- src/api/core/organizations.rs | 2 +- src/api/mod.rs | 2 +- src/api/notifications.rs | 34 +- src/api/push.rs | 7 +- src/auth.rs | 41 +-- src/config.rs | 3 + src/db/models/cipher.rs | 265 +++++++++------- src/db/models/mod.rs | 4 +- src/db/models/user.rs | 103 +++++- 11 files changed, 891 insertions(+), 231 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index e022807e..f26ed454 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -11,7 +11,7 @@ use serde_json::Value; use crate::{ CONFIG, api::{ - AnonymousNotify, ApiResult, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, + AnonymousNotify, ApiResult, EmptyResult, JsonResult, LogOutReason, Notify, PasswordOrOtpData, UpdateType, core::{accept_org_invite, log_user_event, two_factor::email}, master_password_policy, register_push_device, unregister_push_device, }, @@ -48,8 +48,10 @@ pub fn routes() -> Vec { post_password, post_set_password, post_kdf, + get_key_rotation_data, post_rotatekey, post_user_key, + post_rotate_user_keys, post_sstamp, post_email_token, post_email, @@ -302,6 +304,35 @@ impl AccountKeysData { } } +impl WrappedAccountCryptographicState { + /// This shape is v2-only: the key pairs and security state were already required by the + /// deserializer, so only the values still need checking. A missing signed public key or an + /// unknown algorithm has to be an error rather than a fallback to v1, which would silently turn + /// a rotation into a downgrade. + fn validate(self) -> ApiResult { + let Some(signed_public_key) = self.public_key_encryption_key_pair.signed_public_key else { + err!("No signed public key provided for a v2 account") + }; + let Some(signature_algorithm) = SignatureAlgorithm::from_str(&self.signature_key_pair.signature_algorithm) + else { + err!(format!("Unsupported signature algorithm: {}", self.signature_key_pair.signature_algorithm)) + }; + + Ok(ValidatedAccountKeys { + private_key: self.public_key_encryption_key_pair.wrapped_private_key, + public_key: self.public_key_encryption_key_pair.public_key, + v2: Some(ValidatedV2AccountKeys { + signed_public_key, + signing_key: self.signature_key_pair.wrapped_signing_key, + verifying_key: self.signature_key_pair.verifying_key, + signature_algorithm, + security_state: self.security_state.security_state, + security_version: self.security_state.security_version, + }), + }) + } +} + impl From for ValidatedAccountKeys { fn from(keys: KeysData) -> Self { Self { @@ -1023,16 +1054,18 @@ struct UpdateEmergencyAccessData { #[serde(rename_all = "camelCase")] struct UpdateResetPasswordData { organization_id: OrganizationId, - reset_password_key: String, + // Absent in a v1 -> v2 upgrade, which keeps the key the organization already holds + reset_password_key: Option, } #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct KeyData { account_unlock_data: RotateAccountUnlockData, - account_keys: RotateAccountKeys, + account_keys: AccountKeysData, account_data: RotateAccountData, old_master_key_authentication_hash: String, + new_user_key_id: Option, } #[derive(Deserialize)] @@ -1041,6 +1074,8 @@ struct RotateAccountUnlockData { emergency_access_unlock_data: Vec, master_password_unlock_data: MasterPasswordUnlockData, organization_account_recovery_unlock_data: Vec, + #[serde(flatten)] + common: CommonUnlockData, } #[derive(Deserialize)] @@ -1053,13 +1088,28 @@ struct MasterPasswordUnlockData { email: String, master_key_authentication_hash: String, master_key_encrypted_user_key: String, + contained_key_id: Option, } -#[derive(Deserialize)] +/// The unlock data both rotation endpoints share. Vaultwarden has neither trusted device encryption +/// nor passkey login, so `key-rotation-data` reports none of either and these must arrive empty. +#[derive(Deserialize, Default)] #[serde(rename_all = "camelCase")] -struct RotateAccountKeys { - user_key_encrypted_account_private_key: String, - account_public_key: String, +struct CommonUnlockData { + #[serde(default)] + passkey_unlock_data: Vec, + #[serde(default)] + device_key_unlock_data: Vec, + v2_upgrade_token: Option, +} + +/// Lets clients that still hold the v1 user key derive the v2 one after another client upgraded the +/// account, so a v1 -> v2 upgrade doesn't have to log every other session out. Opaque to us. +#[derive(Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +struct V2UpgradeTokenData { + wrapped_user_key1: String, + wrapped_user_key2: String, } #[derive(Deserialize)] @@ -1070,6 +1120,131 @@ struct RotateAccountData { sends: Vec, } +/// Body of `rotate-user-keys`, which rotates the keys without touching the master password. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct RotateUserKeysData { + wrapped_account_cryptographic_state: WrappedAccountCryptographicState, + unlock_data: RotateUserKeysUnlockData, + account_data: RotateAccountData, + unlock_method_data: UnlockMethodData, + new_user_key_id: Option, +} + +/// The v2-only account cryptographic state, where all three parts are mandatory. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct WrappedAccountCryptographicState { + public_key_encryption_key_pair: PublicKeyEncryptionKeyPairData, + signature_key_pair: SignatureKeyPairData, + security_state: SecurityStateData, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct RotateUserKeysUnlockData { + emergency_access_unlock_data: Vec, + organization_account_recovery_unlock_data: Vec, + #[serde(flatten)] + common: CommonUnlockData, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct UnlockMethodData { + unlock_method: i32, + master_password_unlock_data: Option, + // `keyConnectorKeyWrappedUserKey` is deliberately not read: that unlock method is rejected. +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct RotateMasterPasswordUnlockData { + kdf: KDFData, + salt: String, + master_key_wrapped_user_key: String, + contained_key_id: Option, +} + +/// The user key wrapped in the master password unlock data has to be the new one. +/// +/// Ref: +fn validate_contained_key_id(contained_key_id: Option<&KeyId>, new_user_key_id: Option<&KeyId>) -> EmptyResult { + match (contained_key_id, new_user_key_id) { + // Neither is sent by clients that predate key ids + (None, None) => Ok(()), + (Some(contained), Some(new)) if contained == new => Ok(()), + _ => err!("Invalid user key sent in master-password unlock data."), + } +} + +/// Ref: +enum UnlockMethod { + Tde = 0, + MasterPassword = 1, + KeyConnector = 2, +} + +/// The rotation-specific checks on the new account keys. +/// +/// A rotation re-wraps the existing keys under a new user key; it must not swap the identity the +/// account presents to others. So the public key never changes, and for an account that is already +/// v2 the verifying key doesn't either — only the *wrapped* signing key does. A v1 account may gain +/// a signature key pair, which is the v1 -> v2 upgrade. +/// +/// Ref: +async fn validate_rotation_account_keys(keys: &ValidatedAccountKeys, user: &User, conn: &DbConn) -> EmptyResult { + if user.public_key.as_ref() != Some(&keys.public_key) { + err!("Changing the asymmetric keypair is not possible during key rotation") + } + + let Some(v2) = &keys.v2 else { + if user.is_v2() { + err!("Cannot downgrade an account from v2 to v1 encryption during key rotation") + } + // A v1 rotation: the private key stays wrapped by an AES-CBC-HMAC user key + if enc_string_type(&keys.private_key) != Some(ENC_TYPE_AES_CBC_256_HMAC_SHA256) { + err!("The provided account private key was not wrapped with AES-256-CBC-HMAC") + } + return Ok(()); + }; + + // Both a v2 rotation and the v1 -> v2 upgrade end up with a COSE user key wrapping the private keys + if enc_string_type(&v2.signing_key) != Some(ENC_TYPE_COSE_ENCRYPT0) { + err!("The provided signing key data is not wrapped with XChaCha20-Poly1305.") + } + if enc_string_type(&keys.private_key) != Some(ENC_TYPE_COSE_ENCRYPT0) { + err!("The provided private key encryption key is not wrapped with XChaCha20-Poly1305.") + } + if v2.verifying_key.is_empty() || v2.signed_public_key.is_empty() || v2.security_state.is_empty() { + err!("The v2 account keys are missing the verifying key, signed public key or security state") + } + + if !user.is_v2() { + // The v1 -> v2 upgrade, which is where the signature key pair comes from + return Ok(()); + } + + let Some(key_pair) = UserSignatureKeyPair::find_active_by_user(&user.uuid, conn).await else { + err!("The account is missing its signature key pair") + }; + if key_pair.verifying_key != v2.verifying_key { + err!("Changing the verifying key is not possible during key rotation") + } + + Ok(()) +} + +/// `AesCbc256_HmacSha256_B64`, the v1 user key +const ENC_TYPE_AES_CBC_256_HMAC_SHA256: &str = "2"; +/// `CoseEncrypt0B64`, the v2 user key +const ENC_TYPE_COSE_ENCRYPT0: &str = "7"; + +/// The encryption type of an EncString, the number before the first `.`. +fn enc_string_type(enc_string: &str) -> Option<&str> { + enc_string.split_once('.').map(|(enc_type, _)| enc_type) +} + fn validate_keydata( data: &KeyData, existing_ciphers: &[Cipher], @@ -1087,14 +1262,35 @@ fn validate_keydata( { err!("Changing the kdf variant or email is not supported during key rotation"); } - if user.public_key.as_ref() != Some(&data.account_keys.account_public_key) { - err!("Changing the asymmetric keypair is not possible during key rotation") - } + validate_rotation_data( + &data.account_data, + &data.account_unlock_data.emergency_access_unlock_data, + &data.account_unlock_data.organization_account_recovery_unlock_data, + existing_ciphers, + existing_folders, + existing_emergency_access, + existing_memberships, + existing_sends, + ) +} + +/// A rotation re-encrypts everything under the new user key, so anything left out would be +/// unreadable afterwards. Both rotation endpoints require the client to send the complete set. +#[expect(clippy::too_many_arguments, reason = "Every collection has to be cross-checked")] +fn validate_rotation_data( + account_data: &RotateAccountData, + emergency_access_unlock_data: &[UpdateEmergencyAccessData], + organization_account_recovery_unlock_data: &[UpdateResetPasswordData], + existing_ciphers: &[Cipher], + existing_folders: &[Folder], + existing_emergency_access: &[EmergencyAccess], + existing_memberships: &[Membership], + existing_sends: &[Send], +) -> EmptyResult { // Check that we're correctly rotating all the user's ciphers let existing_cipher_ids = existing_ciphers.iter().map(|c| &c.uuid).collect::>(); - let provided_cipher_ids = data - .account_data + let provided_cipher_ids = account_data .ciphers .iter() .filter(|c| c.organization_id.is_none()) @@ -1106,8 +1302,7 @@ fn validate_keydata( // Check that we're correctly rotating all the user's folders let existing_folder_ids = existing_folders.iter().map(|f| &f.uuid).collect::>(); - let provided_folder_ids = - data.account_data.folders.iter().filter_map(|f| f.id.as_ref()).collect::>(); + let provided_folder_ids = account_data.folders.iter().filter_map(|f| f.id.as_ref()).collect::>(); if !provided_folder_ids.is_superset(&existing_folder_ids) { err!("All existing folders must be included in the rotation") } @@ -1115,12 +1310,8 @@ fn validate_keydata( // Check that we're correctly rotating all the user's emergency access keys let existing_emergency_access_ids = existing_emergency_access.iter().map(|ea| &ea.uuid).collect::>(); - let provided_emergency_access_ids = data - .account_unlock_data - .emergency_access_unlock_data - .iter() - .map(|ea| &ea.id) - .collect::>(); + let provided_emergency_access_ids = + emergency_access_unlock_data.iter().map(|ea| &ea.id).collect::>(); if !provided_emergency_access_ids.is_superset(&existing_emergency_access_ids) { err!("All existing emergency access keys must be included in the rotation") } @@ -1128,9 +1319,7 @@ fn validate_keydata( // Check that we're correctly rotating all the user's reset password keys let existing_reset_password_ids = existing_memberships.iter().map(|m| &m.org_uuid).collect::>(); - let provided_reset_password_ids = data - .account_unlock_data - .organization_account_recovery_unlock_data + let provided_reset_password_ids = organization_account_recovery_unlock_data .iter() .map(|rp| &rp.organization_id) .collect::>(); @@ -1140,7 +1329,7 @@ fn validate_keydata( // Check that we're correctly rotating all the user's sends let existing_send_ids = existing_sends.iter().map(|s| &s.uuid).collect::>(); - let provided_send_ids = data.account_data.sends.iter().filter_map(|s| s.id.as_ref()).collect::>(); + let provided_send_ids = account_data.sends.iter().filter_map(|s| s.id.as_ref()).collect::>(); if !provided_send_ids.is_superset(&existing_send_ids) { err!("All existing sends must be included in the rotation") } @@ -1148,32 +1337,130 @@ fn validate_keydata( Ok(()) } +/// The public keys and encrypted keysets that participate in a key rotation, so the client can +/// re-share the new user key without having to piece this together from several endpoints. +/// +/// The four collections must always be present, even when empty: the SDK errors out if any of them +/// is missing. `trustedDeviceKeyData` and `passkeyKeyData` are always empty here, since vaultwarden +/// supports neither trusted device encryption nor passkey (PRF) login. +/// +/// Ref: +#[get("/accounts/key-management/key-rotation-data")] +async fn get_key_rotation_data(headers: Headers, conn: DbConn) -> JsonResult { + let user_id = &headers.user.uuid; + + let mut organization_data = Vec::new(); + for membership in Membership::find_by_user(user_id, &conn).await { + // Only memberships actually enrolled in account recovery take part in the rotation. + if membership.reset_password_key.is_none() { + continue; + } + let Some(org) = Organization::find_by_uuid(&membership.org_uuid, &conn).await else { + continue; + }; + let Some(public_key) = org.public_key else { + continue; + }; + organization_data.push(json!({ + "organizationId": org.uuid, + "organizationName": org.name, + "organizationPublicKey": public_key, + "object": "organizationPasswordResetKeyData", + })); + } + + let mut emergency_access_data = Vec::new(); + for emergency_access in EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await { + // Without a stored key there is nothing to re-share. + if emergency_access.key_encrypted.is_none() { + continue; + } + let Some(grantee_id) = emergency_access.grantee_uuid.clone() else { + continue; + }; + let Some(grantee) = User::find_by_uuid(&grantee_id, &conn).await else { + continue; + }; + let Some(public_key) = grantee.public_key.clone() else { + continue; + }; + emergency_access_data.push(json!({ + "id": emergency_access.uuid, + "granteeId": grantee_id, + "granteeName": grantee.name, + "granteeEmail": grantee.email, + "publicKey": public_key, + "object": "emergencyAccessKeyData", + })); + } + + Ok(Json(json!({ + "organizationPasswordResetKeyData": organization_data, + "emergencyAccessKeyData": emergency_access_data, + "trustedDeviceKeyData": [], + "passkeyKeyData": [], + "object": "keyRotationData", + }))) +} + +/// How the new user key gets wrapped, which is the one part the two rotation endpoints don't share. +enum UnlockRotation { + /// `rotate-user-account-keys`: the master password changes along with the keys. + PasswordChange { + authentication_hash: String, + wrapped_user_key: String, + }, + /// `rotate-user-keys`: the password is untouched, only the wrapped user key is replaced. + WrappedUserKeyOnly { + wrapped_user_key: String, + }, +} + +/// Everything a rotation replaces, once each endpoint's wrapper has been peeled off. +struct RotateData { + account_keys: ValidatedAccountKeys, + account_data: RotateAccountData, + emergency_access_unlock_data: Vec, + organization_account_recovery_unlock_data: Vec, + v2_upgrade_token: Option, + /// Only v2 (COSE) user keys have an id, so this is `None` for a v1 -> v1 rotation. + new_user_key_id: Option, + unlock: UnlockRotation, +} + +impl CommonUnlockData { + /// We advertise no trusted devices or passkeys in `key-rotation-data`, so a client sending + /// either has produced keys for something that doesn't exist here. Silently dropping them would + /// leave the client believing an unlock method was rotated when it wasn't. + fn validate(&self) -> EmptyResult { + if !self.passkey_unlock_data.is_empty() { + err!("Passkey unlock is not supported") + } + if !self.device_key_unlock_data.is_empty() { + err!("Trusted device unlock is not supported") + } + Ok(()) + } +} + #[post("/accounts/key-management/rotate-user-account-keys", data = "")] async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: Notify<'_>) -> EmptyResult { - // TODO: See if we can wrap everything within a SQL Transaction. If something fails it should revert everything. let data: KeyData = data.into_inner(); if !headers.user.check_valid_password(&data.old_master_key_authentication_hash) { err!("Invalid password") } - // Validate the import before continuing - // Bitwarden does not process the import if there is one item invalid. - // Since we check for the size of the encrypted note length, we need to do that here to pre-validate it. - // TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks. - Cipher::validate_cipher_data(&data.account_data.ciphers)?; + data.account_unlock_data.common.validate()?; let user_id = &headers.user.uuid; - - // TODO: Ideally we'd do everything after this point in a single transaction. - - let mut existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await; - let mut existing_folders = Folder::find_by_user(user_id, &conn).await; - let mut existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await; + let existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await; + let existing_folders = Folder::find_by_user(user_id, &conn).await; + let existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await; let mut existing_memberships = Membership::find_by_user(user_id, &conn).await; // We only rotate the reset password key if it is set. existing_memberships.retain(|m| m.reset_password_key.is_some()); - let mut existing_sends = Send::find_by_user(user_id, &conn).await; + let existing_sends = Send::find_by_user(user_id, &conn).await; validate_keydata( &data, @@ -1185,6 +1472,93 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: &headers.user, )?; + validate_contained_key_id( + data.account_unlock_data.master_password_unlock_data.contained_key_id.as_ref(), + data.new_user_key_id.as_ref(), + )?; + + let unlock = UnlockRotation::PasswordChange { + authentication_hash: data.account_unlock_data.master_password_unlock_data.master_key_authentication_hash, + wrapped_user_key: data.account_unlock_data.master_password_unlock_data.master_key_encrypted_user_key, + }; + + rotate_account( + RotateData { + account_keys: data.account_keys.validate()?, + account_data: data.account_data, + emergency_access_unlock_data: data.account_unlock_data.emergency_access_unlock_data, + organization_account_recovery_unlock_data: data + .account_unlock_data + .organization_account_recovery_unlock_data, + // This endpoint always logs every session out, so an upgrade token is never kept here. That + // also clears one left over from an earlier upgrade. Ref: upstream's AccountsKeyManagementController + v2_upgrade_token: None, + new_user_key_id: data.new_user_key_id, + unlock, + }, + (existing_ciphers, existing_folders, existing_emergency_access, existing_memberships, existing_sends), + headers, + conn, + nt, + ) + .await +} + +type ExistingRotationData = (Vec, Vec, Vec, Vec, Vec); + +/// The body shared by both rotation endpoints: re-save every piece of data the client re-encrypted, +/// then swap the account keys and the wrapped user key. +async fn rotate_account( + data: RotateData, + existing: ExistingRotationData, + headers: Headers, + conn: DbConn, + nt: Notify<'_>, +) -> EmptyResult { + // TODO: See if we can wrap everything within a SQL Transaction. If something fails it should revert everything. + let ( + mut existing_ciphers, + mut existing_folders, + mut existing_emergency_access, + mut existing_memberships, + mut existing_sends, + ) = existing; + + validate_rotation_account_keys(&data.account_keys, &headers.user, &conn).await?; + + // The upgrade token only means anything for a v1 account moving to v2: it lets sessions still + // holding the v1 user key pick up the v2 one instead of being forced to re-authenticate. For an + // account that is already v2 it is meaningless, so it gets discarded and the security stamp is + // reset, as a rotation otherwise would. That ends the refresh tokens, while the access tokens in + // use run out on their own, as upstream. + let is_upgrade = data.v2_upgrade_token.is_some() && !headers.user.is_v2(); + let upgrade_token = match &data.v2_upgrade_token { + Some(token) if is_upgrade => Some(serde_json::to_string(token)?), + _ => None, + }; + + // An upgrade can't re-wrap the account recovery keys: that needs the organization's public key + // to be trusted, and an upgrade shows the user no prompt. So the stored keys are kept, and each + // organization gets the upgrade token instead, to unwrap the v2 user key with. + // Ref: upstream's OrganizationUserRotationValidator + for reset_password_data in &data.organization_account_recovery_unlock_data { + let has_key = reset_password_data.reset_password_key.as_deref().is_some_and(|k| !k.is_empty()); + if is_upgrade && has_key { + err!("Account recovery keys cannot be rotated during a V1 to V2 upgrade rotation.") + } + if !is_upgrade && !has_key { + err!("Account recovery keys cannot be null or empty during rotation.") + } + } + + // Validate the import before continuing + // Bitwarden does not process the import if there is one item invalid. + // Since we check for the size of the encrypted note length, we need to do that here to pre-validate it. + // TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks. + Cipher::validate_cipher_data(&data.account_data.ciphers)?; + + // TODO: Ideally we'd do everything after this point in a single transaction. + // Update folder data for folder_data in data.account_data.folders { // Skip `null` folder id entries. @@ -1200,7 +1574,7 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: } // Update emergency access data - for emergency_access_data in data.account_unlock_data.emergency_access_unlock_data { + for emergency_access_data in data.emergency_access_unlock_data { let Some(saved_emergency_access) = existing_emergency_access.iter_mut().find(|ea| ea.uuid == emergency_access_data.id) else { @@ -1212,14 +1586,17 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: } // Update reset password data - for reset_password_data in data.account_unlock_data.organization_account_recovery_unlock_data { + for reset_password_data in data.organization_account_recovery_unlock_data { let Some(membership) = existing_memberships.iter_mut().find(|m| m.org_uuid == reset_password_data.organization_id) else { err!("Reset password doesn't exist") }; - membership.reset_password_key = Some(reset_password_data.reset_password_key); + if !is_upgrade { + membership.reset_password_key = reset_password_data.reset_password_key; + } + membership.v2_upgrade_token.clone_from(&upgrade_token); membership.save(&conn).await?; } @@ -1249,25 +1626,126 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: // Update user data let mut user = headers.user; + let keep_sessions_alive = is_upgrade; + user.v2_upgrade_token = upgrade_token; - user.private_key = Some(data.account_keys.user_key_encrypted_account_private_key); - user.set_password( - &data.account_unlock_data.master_password_unlock_data.master_key_authentication_hash, - Some(data.account_unlock_data.master_password_unlock_data.master_key_encrypted_user_key), - true, - None, - &conn, - ) - .await?; + data.account_keys.apply(&mut user)?; + // The old id names a key that no longer exists, so it is replaced even when the new key has none. + user.key_id = data.new_user_key_id; - let save_result = user.save(&conn).await; + match data.unlock { + UnlockRotation::PasswordChange { + authentication_hash, + wrapped_user_key, + } => { + user.set_password(&authentication_hash, Some(wrapped_user_key), false, None, &conn).await?; + } + UnlockRotation::WrappedUserKeyOnly { + wrapped_user_key, + } => { + user.akey = wrapped_user_key; + } + } + if !keep_sessions_alive { + user.reset_security_stamp_after_key_rotation(&conn).await?; + } + + // The key pair goes first: if saving the user then fails during an upgrade, the account is still + // v1 and the key pair unused, instead of a v2 account without one, which no client could unlock. + // A failure between the two writes of a v2 rotation still needs a transaction to undo. + data.account_keys.save_signature_key_pair(&user.uuid, &conn).await?; + user.save(&conn).await?; // Prevent logging out the client where the user requested this endpoint from. // If you do logout the user it will causes issues at the client side. // Adding the device uuid will prevent this. - nt.send_logout(&user, Some(&headers.device), &conn).await; + // When the sessions were kept alive, the reason lets the other clients sync the new keys (through + // the upgrade token) instead of logging out, if they have `pm-31050-no-logout-key-upgrade-rotation`. + let reason = keep_sessions_alive.then_some(LogOutReason::KeyRotation); + nt.send_logout_with_reason(&user, Some(&headers.device), reason, &conn).await; - save_result + Ok(()) +} + +/// Rotates the account keys without changing the master password. +/// +/// Unlike `rotate-user-account-keys` this carries no proof of the master password: the request is +/// authorized by the session alone, which is how upstream defines it, and the client has to be +/// unlocked to produce the payload in the first place. +/// +/// Ref: +#[post("/accounts/key-management/rotate-user-keys", data = "")] +async fn post_rotate_user_keys( + data: Json, + headers: Headers, + conn: DbConn, + nt: Notify<'_>, +) -> EmptyResult { + let data: RotateUserKeysData = data.into_inner(); + + data.unlock_data.common.validate()?; + + let unlock = if data.unlock_method_data.unlock_method == UnlockMethod::MasterPassword as i32 { + let Some(unlock_data) = data.unlock_method_data.master_password_unlock_data else { + err!("Missing master password unlock data") + }; + if headers.user.client_kdf_type != unlock_data.kdf.kdf + || headers.user.client_kdf_iter != unlock_data.kdf.kdf_iterations + || headers.user.client_kdf_memory != unlock_data.kdf.kdf_memory + || headers.user.client_kdf_parallelism != unlock_data.kdf.kdf_parallelism + { + err!("Changing the kdf variant is not supported during key rotation") + } + if unlock_data.salt != headers.user.email { + err!("Invalid master password salt") + } + validate_contained_key_id(unlock_data.contained_key_id.as_ref(), data.new_user_key_id.as_ref())?; + UnlockRotation::WrappedUserKeyOnly { + wrapped_user_key: unlock_data.master_key_wrapped_user_key, + } + } else if data.unlock_method_data.unlock_method == UnlockMethod::Tde as i32 { + err!("Trusted device encryption is not supported") + } else if data.unlock_method_data.unlock_method == UnlockMethod::KeyConnector as i32 { + err!("Key connector is not supported") + } else { + err!("Unrecognized unlock method") + }; + + let user_id = &headers.user.uuid; + let existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await; + let existing_folders = Folder::find_by_user(user_id, &conn).await; + let existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await; + let mut existing_memberships = Membership::find_by_user(user_id, &conn).await; + existing_memberships.retain(|m| m.reset_password_key.is_some()); + let existing_sends = Send::find_by_user(user_id, &conn).await; + + validate_rotation_data( + &data.account_data, + &data.unlock_data.emergency_access_unlock_data, + &data.unlock_data.organization_account_recovery_unlock_data, + &existing_ciphers, + &existing_folders, + &existing_emergency_access, + &existing_memberships, + &existing_sends, + )?; + + rotate_account( + RotateData { + account_keys: data.wrapped_account_cryptographic_state.validate()?, + account_data: data.account_data, + emergency_access_unlock_data: data.unlock_data.emergency_access_unlock_data, + organization_account_recovery_unlock_data: data.unlock_data.organization_account_recovery_unlock_data, + v2_upgrade_token: data.unlock_data.common.v2_upgrade_token, + new_user_key_id: data.new_user_key_id, + unlock, + }, + (existing_ciphers, existing_folders, existing_emergency_access, existing_memberships, existing_sends), + headers, + conn, + nt, + ) + .await } #[derive(Deserialize)] diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index 2831a1ad..b24d68f4 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -24,6 +24,7 @@ use crate::{ Archive, Attachment, AttachmentId, Cipher, CipherId, Collection, CollectionCipher, CollectionGroup, CollectionId, CollectionUser, EventType, Favorite, Folder, FolderCipher, FolderId, Group, KeyId, Membership, MembershipType, OrgPolicy, OrgPolicyType, OrganizationId, RepromptType, Send, UserId, + is_data_blob_encrypted, }, }, util::{NumberOrString, deser_opt_nonempty_str, save_temp_file}, @@ -195,6 +196,15 @@ async fn sync(data: SyncData, headers: Headers, client_version: Option, pub notes: Option, fields: Option, @@ -299,6 +306,9 @@ pub struct CipherData { drivers_license: Option, passport: Option, + // The sealed blob of a v2 account's cipher, which replaces all of the fields above + data: Option, + favorite: Option, reprompt: Option, @@ -320,6 +330,34 @@ pub struct CipherData { archived_date: Option, } +/// Upstream's `[StringLength(500000)]` on `CipherRequestModel.Data` +const MAX_CIPHER_DATA_LENGTH: usize = 500_000; + +impl CipherData { + /// Checks the content the way upstream's model validation does, before anything is saved. + /// On failure, returns the offending field and the message for it. + /// + /// Ref: + pub fn validate_content(&self) -> Result<(), (&'static str, String)> { + if let Some(data) = &self.data + && data.len() > MAX_CIPHER_DATA_LENGTH + { + return Err(( + "Data", + format!("The field Data must be a string with a maximum length of {MAX_CIPHER_DATA_LENGTH}."), + )); + } + + // A blob carries the name inside it, so only the other formats need one + let is_blob = self.data.as_deref().is_some_and(is_data_blob_encrypted); + if !is_blob && self.name.as_deref().is_none_or(|n| n.trim().is_empty()) { + return Err(("Name", String::from("The Name field is required."))); + } + + Ok(()) + } +} + #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct PartialCipherData { @@ -362,7 +400,7 @@ async fn post_ciphers_create( // cipher.save() below. enforce_personal_ownership_policy(Some(&data.cipher), &headers, &conn).await?; - let mut cipher = Cipher::new(data.cipher.r#type, data.cipher.name.clone()); + let mut cipher = Cipher::new(data.cipher.r#type, data.cipher.name.clone().unwrap_or_default()); cipher.user_uuid = Some(headers.user.uuid.clone()); cipher.save(&conn).await?; @@ -403,7 +441,7 @@ async fn post_ciphers(data: Json, headers: Headers, conn: DbConn, nt // needed when creating a new cipher, so just ignore it unconditionally. data.last_known_revision_date = None; - let mut cipher = Cipher::new(data.r#type, data.name.clone()); + let mut cipher = Cipher::new(data.r#type, data.name.clone().unwrap_or_default()); update_cipher_from_data(&mut cipher, data, &headers, None, &conn, &nt, UpdateType::SyncCipherCreate).await?; Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, None, CipherSyncType::User, &conn).await?)) @@ -452,6 +490,10 @@ pub async fn update_cipher_from_data( enforce_personal_ownership_policy(Some(&data), headers, conn).await?; + if let Err((_, message)) = data.validate_content() { + err!(message) + } + // Check that the client isn't updating an existing cipher with stale data. // And only perform this check when not importing ciphers, else the date/time check will fail. if ut != UpdateType::None @@ -539,6 +581,15 @@ pub async fn update_cipher_from_data( } } + // A blob replaces every content field, so the per-type data below neither applies nor exists. + // `validate_content` made sure anything else has a name. + let blob = data.data.filter(|d| is_data_blob_encrypted(d)); + let name = if blob.is_some() { + String::new() + } else { + data.name.unwrap_or_default() + }; + let type_data_opt = match data.r#type { 1 => data.login, 2 => data.secure_note, @@ -551,23 +602,25 @@ pub async fn update_cipher_from_data( _ => err!("Invalid type"), }; - let type_data = if let Some(mut data) = type_data_opt { + let stored_data = if let Some(blob) = blob { + blob + } else if let Some(mut data) = type_data_opt { // Remove the 'Response' key from the base object. data.as_object_mut().unwrap().remove("response"); // Remove the 'Response' key from every Uri. if data["uris"].is_array() { data["uris"] = clean_cipher_data(data["uris"].clone()); } - data + data.to_string() } else { err!("Data missing") }; cipher.key = data.key; - cipher.name = data.name; + cipher.name = name; cipher.notes = data.notes; cipher.fields = data.fields.map(|f| clean_cipher_data(f).to_string()); - cipher.data = type_data.to_string(); + cipher.data = stored_data; cipher.password_history = data.password_history.map(|f| f.to_string()); cipher.reprompt = data.reprompt.filter(|r| *r == RepromptType::None as i32 || *r == RepromptType::Password as i32); @@ -665,7 +718,7 @@ async fn post_ciphers_import(data: Json, headers: Headers, conn: DbC let folder_id = relations_map.get(&index).and_then(|i| folders.get(*i).cloned()); cipher_data.folder_id = folder_id; - let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone()); + let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone().unwrap_or_default()); update_cipher_from_data(&mut cipher, cipher_data, &headers, None, &conn, &nt, UpdateType::None).await?; } diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 36297d30..3ca0d326 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1885,7 +1885,7 @@ async fn post_org_import( cipher_data.folder_id = None; // Replace the client-provided, unvalidated organizationId with the real target org cipher_data.organization_id = Some(org_id.clone()); - let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone()); + let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone().unwrap_or_default()); update_cipher_from_data( &mut cipher, cipher_data, diff --git a/src/api/mod.rs b/src/api/mod.rs index 9a79ce95..57899219 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -23,7 +23,7 @@ pub use crate::api::{ icons::routes as icons_routes, identity::routes as identity_routes, notifications::routes as notifications_routes, - notifications::{AnonymousNotify, Notify, UpdateType, WS_ANONYMOUS_SUBSCRIPTIONS, WS_USERS}, + notifications::{AnonymousNotify, LogOutReason, Notify, UpdateType, WS_ANONYMOUS_SUBSCRIPTIONS, WS_USERS}, push::{ push_cipher_update, push_folder_update, push_logout, push_send_update, push_user_update, register_push_device, unregister_push_device, diff --git a/src/api/notifications.rs b/src/api/notifications.rs index 8bfcd518..55688ff9 100644 --- a/src/api/notifications.rs +++ b/src/api/notifications.rs @@ -383,23 +383,36 @@ impl WebSocketUsers { } pub async fn send_logout(&self, user: &User, acting_device: Option<&Device>, conn: &DbConn) { + self.send_logout_with_reason(user, acting_device, None, conn).await; + } + + /// A logout that clients may handle differently depending on `reason`, e.g. syncing instead of + /// logging out after a key rotation that kept the sessions alive. + pub async fn send_logout_with_reason( + &self, + user: &User, + acting_device: Option<&Device>, + reason: Option, + conn: &DbConn, + ) { // Skip any processing if both WebSockets and Push are not active if *NOTIFICATIONS_DISABLED { return; } let acting_device_id = acting_device.map(|d| d.uuid.clone()); - let data = create_update( - vec![("UserId".into(), user.uuid.to_string().into()), ("Date".into(), serialize_date(user.updated_at))], - UpdateType::LogOut, - acting_device_id, - ); + let mut payload = + vec![("UserId".into(), user.uuid.to_string().into()), ("Date".into(), serialize_date(user.updated_at))]; + if let Some(reason) = reason { + payload.push(("Reason".into(), (reason as i32).into())); + } + let data = create_update(payload, UpdateType::LogOut, acting_device_id); if CONFIG.enable_websocket() { self.send_update(&user.uuid, &data).await; } if CONFIG.push_enabled() { - push_logout(user, acting_device, conn).await; + push_logout(user, acting_device, reason, conn).await; } } @@ -667,6 +680,15 @@ fn create_ping() -> Vec { serialize(&Value::Array(vec![6.into()])) } +/// Why a logout was pushed. Absent for a plain logout. +/// +/// Ref: +/// (`KdfChange = 0` is not sent by vaultwarden yet.) +#[derive(Copy, Clone, Eq, PartialEq)] +pub enum LogOutReason { + KeyRotation = 1, +} + // https://github.com/bitwarden/server/blob/375af7c43b10d9da03525d41452f95de3f921541/src/Core/Enums/PushType.cs #[derive(Copy, Clone, Eq, PartialEq)] pub enum UpdateType { diff --git a/src/api/push.rs b/src/api/push.rs index e87a0985..323d1310 100644 --- a/src/api/push.rs +++ b/src/api/push.rs @@ -12,7 +12,7 @@ use tokio::sync::RwLock; use crate::{ CONFIG, - api::{ApiResult, EmptyResult, UpdateType}, + api::{ApiResult, EmptyResult, LogOutReason, UpdateType}, db::{ DbConn, models::{AuthRequestId, Cipher, Device, Folder, PushId, Send, User, UserId}, @@ -188,7 +188,7 @@ pub async fn push_cipher_update(ut: UpdateType, cipher: &Cipher, device: &Device } } -pub async fn push_logout(user: &User, acting_device: Option<&Device>, conn: &DbConn) { +pub async fn push_logout(user: &User, acting_device: Option<&Device>, reason: Option, conn: &DbConn) { if Device::check_user_has_push_device(&user.uuid, conn).await { tokio::task::spawn(send_to_push_relay(json!({ "userId": user.uuid, @@ -198,7 +198,8 @@ pub async fn push_logout(user: &User, acting_device: Option<&Device>, conn: &DbC "type": UpdateType::LogOut as i32, "payload": { "userId": user.uuid, - "date": format_date(&user.updated_at) + "date": format_date(&user.updated_at), + "reason": reason.map(|r| r as i32), }, "clientType": null, "installationId": null diff --git a/src/auth.rs b/src/auth.rs index 07373389..75dbc041 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -30,7 +30,7 @@ use crate::{ models::{ AttachmentId, CipherId, Collection, CollectionId, Device, DeviceId, DeviceType, EmergencyAccessId, EventType, Membership, MembershipId, MembershipStatus, MembershipType, OrgApiKeyId, OrganizationId, - SendFileId, SendId, User, UserId, UserStampException, + SendFileId, SendId, User, UserId, }, }, error::Error, @@ -662,36 +662,25 @@ impl<'r> FromRequest<'r> for Headers { err_handler!("Invalid device id") }; - let Some(user) = User::find_by_uuid(&user_id, &conn).await else { + let Some(mut user) = User::find_by_uuid(&user_id, &conn).await else { err_handler!("Device has no user associated") }; if user.security_stamp != claims.sstamp { - if let Some(stamp_exception) = - user.stamp_exception.as_deref().and_then(|s| serde_json::from_str::(s).ok()) - { - let Some(current_route) = request.route().and_then(|r| r.name.as_deref()) else { - err_handler!("Error getting current route for stamp exception") - }; + let Some(current_route) = request.route().and_then(|r| r.name.as_deref()) else { + err_handler!("Error getting current route for stamp exception") + }; - // Check if the stamp exception has expired first. - // Then, check if the current route matches any of the allowed routes. - // After that check the stamp in exception matches the one in the claims. - if Utc::now().timestamp() > stamp_exception.expire { - // If the stamp exception has been expired remove it from the database. - // This prevents checking this stamp exception for new requests. - let mut user = user; - user.reset_stamp_exception(); - if let Err(e) = user.save(&conn).await { - error!("Error updating user: {e:#?}"); - } - err_handler!("Stamp exception is expired") - } else if !stamp_exception.routes.contains(¤t_route.to_owned()) { - err_handler!("Invalid security stamp: Current route and exception route do not match") - } else if stamp_exception.security_stamp != claims.sstamp { - err_handler!("Invalid security stamp for matched stamp exception") - } - } else { + let allowed = user.stamp_exceptions().iter().any(|e| e.allows(&claims.sstamp, current_route)); + + // Drop the expired exceptions, so they aren't checked for every request from now on. + if user.retain_stamp_exceptions(|e| !e.is_expired()) + && let Err(e) = user.save_stamp_exceptions(&conn).await + { + error!("Error updating user: {e:#?}"); + } + + if !allowed { err_handler!("Invalid security stamp") } } diff --git a/src/config.rs b/src/config.rs index 9f0ae2e1..f88eb99e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1434,6 +1434,9 @@ pub const SUPPORTED_FEATURE_FLAGS: &[&str] = &[ "ssh-key-vault-item", "pm-25373-windows-biometrics-v2", "pm-26340-linux-biometrics-v2", + "pm-31050-no-logout-key-upgrade-rotation", + "pm-27278-v2-password-registration", + "enable-account-encryption-v2-jit-password-registration", // Mobile Team "anon-addy-self-host-alias", "simple-login-self-host-alias", diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index fae8aaec..a0eae75c 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -63,6 +63,17 @@ pub struct Cipher { pub reprompt: Option, } +/// Whether `data` is a sealed cipher blob rather than the legacy per-type JSON. +/// +/// Ciphers of v2 accounts are encrypted as a single blob that carries everything, name included, +/// and is opaque to us. It is recognized the same way upstream does, by a top-level +/// `format_version` key that the legacy JSON never has. +/// +/// Ref: +pub fn is_data_blob_encrypted(data: &str) -> bool { + serde_json::from_str::(data).is_ok_and(|d| d.get("format_version").is_some()) +} + pub enum RepromptType { None = 0, Password = 1, @@ -110,6 +121,10 @@ impl Cipher { .insert(format!("Ciphers[{index}].Notes"), serde_json::to_value([&max_note_size_msg]).unwrap()); } + if let Err((field, message)) = cipher.validate_content() { + validation_errors.insert(format!("Ciphers[{index}].{field}"), serde_json::to_value([message]).unwrap()); + } + // Validate the password history if it contains `null` values and if so, return a warning if let Some(Value::Array(password_history)) = &cipher.password_history { for pwh in password_history { @@ -154,6 +169,8 @@ impl Cipher { ) -> Result { use crate::util::{format_date, validate_and_format_date}; + let is_blob_encrypted = is_data_blob_encrypted(&self.data); + let mut attachments_json: Value = Value::Null; if let Some(cipher_sync_data) = cipher_sync_data { if let Some(attachments) = cipher_sync_data.cipher_attachments.get(&self.uuid) @@ -189,124 +206,62 @@ impl Cipher { (false, false, false) }; - let fields_json: Vec<_> = self - .fields - .as_ref() - .and_then(|s| { - serde_json::from_str::>>(s) - .inspect_err(|e| warn!("Error parsing fields {e:?} for {}", self.uuid)) - .ok() - }) - .map(|d| { - d.into_iter() - .map(|mut f| { - // Check if the `type` key is a number, strings break some clients - // The fallback type is the hidden type `1`. this should prevent accidental data disclosure - // If not try to convert the string value to a number and fallback to `1` - // If it is both not a number and not a string, fallback to `1` - match f.data.get("type") { - Some(t) if t.is_number() => {} - Some(t) if t.is_string() => { - let type_num = &t.as_str().unwrap_or("1").parse::().unwrap_or(1); - f.data["type"] = json!(type_num); + // Like upstream, a cipher that was stored without fields or password history reports them as + // null rather than as an empty list; clients keep the two apart. + let fields_json: Option> = self.fields.as_ref().map(|s| { + serde_json::from_str::>>(s) + .inspect_err(|e| warn!("Error parsing fields {e:?} for {}", self.uuid)) + .ok() + .map(|d| { + d.into_iter() + .map(|mut f| { + // Check if the `type` key is a number, strings break some clients + // The fallback type is the hidden type `1`. this should prevent accidental data disclosure + // If not try to convert the string value to a number and fallback to `1` + // If it is both not a number and not a string, fallback to `1` + match f.data.get("type") { + Some(t) if t.is_number() => {} + Some(t) if t.is_string() => { + let type_num = &t.as_str().unwrap_or("1").parse::().unwrap_or(1); + f.data["type"] = json!(type_num); + } + _ => { + f.data["type"] = json!(1); + } } - _ => { - f.data["type"] = json!(1); - } - } - f.data - }) - .collect() - }) - .unwrap_or_default(); + f.data + }) + .collect() + }) + .unwrap_or_default() + }); - let password_history_json: Vec<_> = self - .password_history - .as_ref() - .and_then(|s| { - serde_json::from_str::>>(s) - .inspect_err(|e| warn!("Error parsing password history {e:?} for {}", self.uuid)) - .ok() - }) - .map(|d| { - // Check every password history item if they are valid and return it. - // If a password field has the type `null` skip it, it breaks newer Bitwarden clients - // A second check is done to verify the lastUsedDate exists and is a valid DateTime string, if not the epoch start time will be used - d.into_iter() - .filter_map(|d| match d.data.get("password") { - Some(p) if p.is_string() => Some(d.data), - _ => None, - }) - .map(|mut d| { - let lud = if let Some(l) = d.get("lastUsedDate").and_then(|l| l.as_str()) { - validate_and_format_date(l) - } else { - "1970-01-01T00:00:00.000000Z".to_owned() - }; - d["lastUsedDate"] = json!(lud); - d - }) - .collect() - }) - .unwrap_or_default(); - - // Get the type_data or a default to an empty json object '{}'. - // If not passing an empty object, mobile clients will crash. - let mut type_data_json = serde_json::from_str::>(&self.data) - .inspect_err(|_| warn!("Error parsing data field for {}", self.uuid)) - .map_or_else(|_| Value::Object(serde_json::Map::new()), |d| d.data); - - // NOTE: This was marked as *Backwards Compatibility Code*, but as of January 2021 this is still being used by upstream - // Set the first element of the Uris array as Uri, this is needed several (mobile) clients. - if self.atype == 1 { - // Upstream always has an `uri` key/value - type_data_json["uri"] = Value::Null; - if let Some(uris) = type_data_json["uris"].as_array_mut() - && !uris.is_empty() - { - // Fix uri match values first, they are only allowed to be a number or null - // If it is a string, convert it to an int or null if that fails - for uri in &mut *uris { - if uri["match"].is_string() { - let match_value = match uri["match"].as_str().unwrap_or_default().parse::() { - Ok(n) => json!(n), - _ => Value::Null, - }; - uri["match"] = match_value; - } - } - type_data_json["uri"] = uris[0]["uri"].clone(); - } - - // Check if `passwordRevisionDate` is a valid date, else convert it - if let Some(pw_revision) = type_data_json["passwordRevisionDate"].as_str() { - type_data_json["passwordRevisionDate"] = json!(validate_and_format_date(pw_revision)); - } - } - - // Fix secure note issues when data is invalid - // This breaks at least the native mobile clients - if self.atype == 2 { - match type_data_json { - Value::Object(ref t) if t.get("type").is_some_and(Value::is_number) => {} - _ => { - type_data_json = json!({"type": 0}); - } - } - } - - // Fix invalid SSH Entries - // This breaks at least the native mobile client if invalid - // The only way to fix this is by setting type_data_json to `null` - // Opening this ssh-key in the mobile client will probably crash the client, but you can edit, save and afterwards delete it - if self.atype == 5 - && (type_data_json["keyFingerprint"].as_str().is_none_or(str::is_empty) - || type_data_json["privateKey"].as_str().is_none_or(str::is_empty) - || type_data_json["publicKey"].as_str().is_none_or(str::is_empty)) - { - warn!("Error parsing ssh-key, mandatory fields are invalid for {}", self.uuid); - type_data_json = Value::Null; - } + let password_history_json: Option> = self.password_history.as_ref().map(|s| { + serde_json::from_str::>>(s) + .inspect_err(|e| warn!("Error parsing password history {e:?} for {}", self.uuid)) + .ok() + .map(|d| { + // Check every password history item if they are valid and return it. + // If a password field has the type `null` skip it, it breaks newer Bitwarden clients + // A second check is done to verify the lastUsedDate exists and is a valid DateTime string, if not the epoch start time will be used + d.into_iter() + .filter_map(|d| match d.data.get("password") { + Some(p) if p.is_string() => Some(d.data), + _ => None, + }) + .map(|mut d| { + let lud = if let Some(l) = d.get("lastUsedDate").and_then(|l| l.as_str()) { + validate_and_format_date(l) + } else { + "1970-01-01T00:00:00.000000Z".to_owned() + }; + d["lastUsedDate"] = json!(lud); + d + }) + .collect() + }) + .unwrap_or_default() + }); let collection_ids = if let Some(cipher_sync_data) = cipher_sync_data { if let Some(cipher_collections) = cipher_sync_data.cipher_collections.get(&self.uuid) { @@ -403,10 +358,86 @@ impl Cipher { _ => err!(format!("Cipher {} has an invalid type {}", self.uuid, self.atype)), }; - json_object[key] = type_data_json; + if is_blob_encrypted { + // The blob holds all of the content, so it is sent back as-is and the structured fields + // stay null, as upstream does. + json_object["data"] = json!(self.data); + json_object["name"] = Value::Null; + json_object["notes"] = Value::Null; + json_object["fields"] = Value::Null; + json_object["passwordHistory"] = Value::Null; + } else { + json_object[key] = self.legacy_type_data_json(); + } Ok(json_object) } + /// The per-type data (`login`, `card`, …) of a legacy cipher, with fixups for values that are + /// known to break clients. + fn legacy_type_data_json(&self) -> Value { + use crate::util::validate_and_format_date; + + // Get the type_data or a default to an empty json object '{}'. + // If not passing an empty object, mobile clients will crash. + let mut type_data_json = serde_json::from_str::>(&self.data) + .inspect_err(|_| warn!("Error parsing data field for {}", self.uuid)) + .map_or_else(|_| Value::Object(serde_json::Map::new()), |d| d.data); + + // NOTE: This was marked as *Backwards Compatibility Code*, but as of January 2021 this is still being used by upstream + // Set the first element of the Uris array as Uri, this is needed several (mobile) clients. + if self.atype == 1 { + // Upstream always has an `uri` key/value + type_data_json["uri"] = Value::Null; + if let Some(uris) = type_data_json["uris"].as_array_mut() + && !uris.is_empty() + { + // Fix uri match values first, they are only allowed to be a number or null + // If it is a string, convert it to an int or null if that fails + for uri in &mut *uris { + if uri["match"].is_string() { + let match_value = match uri["match"].as_str().unwrap_or_default().parse::() { + Ok(n) => json!(n), + _ => Value::Null, + }; + uri["match"] = match_value; + } + } + type_data_json["uri"] = uris[0]["uri"].clone(); + } + + // Check if `passwordRevisionDate` is a valid date, else convert it + if let Some(pw_revision) = type_data_json["passwordRevisionDate"].as_str() { + type_data_json["passwordRevisionDate"] = json!(validate_and_format_date(pw_revision)); + } + } + + // Fix secure note issues when data is invalid + // This breaks at least the native mobile clients + if self.atype == 2 { + match type_data_json { + Value::Object(ref t) if t.get("type").is_some_and(Value::is_number) => {} + _ => { + type_data_json = json!({"type": 0}); + } + } + } + + // Fix invalid SSH Entries + // This breaks at least the native mobile client if invalid + // The only way to fix this is by setting type_data_json to `null` + // Opening this ssh-key in the mobile client will probably crash the client, but you can edit, save and afterwards delete it + if self.atype == 5 + && (type_data_json["keyFingerprint"].as_str().is_none_or(str::is_empty) + || type_data_json["privateKey"].as_str().is_none_or(str::is_empty) + || type_data_json["publicKey"].as_str().is_none_or(str::is_empty)) + { + warn!("Error parsing ssh-key, mandatory fields are invalid for {}", self.uuid); + type_data_json = Value::Null; + } + + type_data_json + } + pub async fn update_users_revision(&self, conn: &DbConn) -> Vec { let mut user_uuids = Vec::new(); match self.user_uuid { diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index 14eea9a2..5131a0fd 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -22,7 +22,7 @@ mod user_signature_key_pair; pub use self::archive::Archive; pub use self::attachment::{Attachment, AttachmentId}; pub use self::auth_request::{AuthRequest, AuthRequestId}; -pub use self::cipher::{Cipher, CipherId, RepromptType}; +pub use self::cipher::{Cipher, CipherId, RepromptType, is_data_blob_encrypted}; pub use self::collection::{Collection, CollectionCipher, CollectionId, CollectionUser}; pub use self::device::{Device, DeviceId, DeviceType, DeviceWithAuthRequest, PushId}; pub use self::emergency_access::{EmergencyAccess, EmergencyAccessId, EmergencyAccessStatus, EmergencyAccessType}; @@ -40,5 +40,5 @@ pub use self::sso_auth::{OIDCAuthenticatedUser, OIDCCodeResponseError, SsoAuth}; pub use self::two_factor::{TwoFactor, TwoFactorType}; pub use self::two_factor_duo_context::TwoFactorDuoContext; pub use self::two_factor_incomplete::TwoFactorIncomplete; -pub use self::user::{Invitation, KeyId, SsoUser, User, UserId, UserKdfType, UserStampException}; +pub use self::user::{Invitation, KeyId, SsoUser, User, UserId, UserKdfType}; pub use self::user_signature_key_pair::{SignatureAlgorithm, UserSignatureKeyPair}; diff --git a/src/db/models/user.rs b/src/db/models/user.rs index b382c8f0..51c56251 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -6,6 +6,7 @@ use serde_json::Value; use crate::{ CONFIG, api::EmptyResult, + auth::DEFAULT_ACCESS_VALIDITY, crypto, db::{ DbConn, @@ -109,13 +110,28 @@ enum UserStatus { _Disabled = 2, } +/// A previous security stamp that is still accepted, until `expire`. #[derive(Serialize, Deserialize)] pub struct UserStampException { - pub routes: Vec, + /// The routes the stamp is still accepted on, or `None` for any route. + pub routes: Option>, pub security_stamp: String, pub expire: i64, } +impl UserStampException { + pub fn is_expired(&self) -> bool { + Utc::now().timestamp() > self.expire + } + + /// Whether this exception lets a token carrying `security_stamp` through on `route`. + pub fn allows(&self, security_stamp: &str, route: &str) -> bool { + !self.is_expired() + && self.security_stamp == security_stamp + && self.routes.as_ref().is_none_or(|routes| routes.iter().any(|r| r == route)) + } +} + /// Local methods impl User { pub const CLIENT_KDF_TYPE_DEFAULT: i32 = UserKdfType::Pbkdf2 as i32; @@ -239,10 +255,38 @@ impl User { pub async fn reset_security_stamp(&mut self, conn: &DbConn) -> EmptyResult { self.security_stamp = get_uuid(); + // A reset is meant to end the other sessions, which a key rotation's grace would undo. The + // route exceptions stay, since they are set right before a reset, for the stamp it replaces. + self.retain_stamp_exceptions(|e| e.routes.is_some()); Device::rotate_refresh_tokens_by_user(&self.uuid, conn).await?; Ok(()) } + /// Resets the security stamp after a key rotation, while letting the access tokens already + /// issued keep working until they expire. + /// + /// This is what upstream does: there the stamp is only checked when a token is refreshed, so a + /// rotation invalidates refresh tokens but not the access tokens in use. Clients rely on it to + /// keep a session going across a rotation. Chained rotations keep every earlier stamp that is + /// still within its window. + pub async fn reset_security_stamp_after_key_rotation(&mut self, conn: &DbConn) -> EmptyResult { + let previous_stamp = self.security_stamp.clone(); + let mut grace: Vec = + self.stamp_exceptions().into_iter().filter(|e| e.routes.is_none() && !e.is_expired()).collect(); + + self.reset_security_stamp(conn).await?; + + grace.push(UserStampException { + routes: None, + security_stamp: previous_stamp, + expire: (Utc::now() + *DEFAULT_ACCESS_VALIDITY).timestamp(), + }); + let mut exceptions = self.stamp_exceptions(); + exceptions.extend(grace); + self.set_stamp_exceptions(&exceptions); + Ok(()) + } + /// Set the stamp_exception to only allow a subsequent request matching a specific route using the current security-stamp. /// /// # Arguments @@ -251,17 +295,56 @@ impl User { /// After these 2 minutes this stamp will expire. /// pub fn set_stamp_exception(&mut self, route_exception: Vec) { - let stamp_exception = UserStampException { - routes: route_exception, + self.set_stamp_exceptions(&[UserStampException { + routes: Some(route_exception), security_stamp: self.security_stamp.clone(), expire: (Utc::now() + TimeDelta::try_minutes(2).unwrap()).timestamp(), - }; - self.stamp_exception = Some(serde_json::to_string(&stamp_exception).unwrap_or_default()); + }]); } - /// Resets the stamp_exception to prevent re-use of the previous security-stamp - pub fn reset_stamp_exception(&mut self) { - self.stamp_exception = None; + /// The previous security stamps that are still accepted, expired ones included. + pub fn stamp_exceptions(&self) -> Vec { + let Some(stored) = self.stamp_exception.as_deref() else { + return Vec::new(); + }; + // Before key rotations kept sessions alive, only a single route exception was stored. + serde_json::from_str::>(stored) + .or_else(|_| serde_json::from_str::(stored).map(|e| vec![e])) + .unwrap_or_default() + } + + fn set_stamp_exceptions(&mut self, exceptions: &[UserStampException]) { + self.stamp_exception = if exceptions.is_empty() { + None + } else { + Some(serde_json::to_string(&exceptions).unwrap_or_default()) + }; + } + + /// Persists only `stamp_exception`, for callers holding a user read earlier in the request. + /// + /// A full `save` would write back every other column as it was read, undoing anything saved in + /// the meantime, such as the new keys of a key rotation made from another device. + pub async fn save_stamp_exceptions(&self, conn: &DbConn) -> EmptyResult { + let uuid = self.uuid.clone(); + let stamp_exception = self.stamp_exception.clone(); + conn.run(move |conn| { + diesel::update(users::table.filter(users::uuid.eq(uuid))) + .set(users::stamp_exception.eq(stamp_exception)) + .execute(conn) + .map_res("Error updating user stamp exceptions") + }) + .await + } + + /// Drops the stamp exceptions that don't match `keep`. Returns whether any were dropped. + pub fn retain_stamp_exceptions(&mut self, keep: impl Fn(&UserStampException) -> bool) -> bool { + let mut exceptions = self.stamp_exceptions(); + let before = exceptions.len(); + exceptions.retain(keep); + let changed = exceptions.len() != before; + self.set_stamp_exceptions(&exceptions); + changed } pub fn display_name(&self) -> &str { @@ -327,8 +410,8 @@ impl User { }) } - pub fn v2_upgrade_token_json(&self) -> Value { - self.v2_upgrade_token.as_ref().and_then(|token| serde_json::from_str(token).ok()).unwrap_or(Value::Null) + pub fn v2_upgrade_token_json(&self) -> Option { + self.v2_upgrade_token.as_ref().and_then(|token| serde_json::from_str(token).ok()) } pub async fn to_json(&self, conn: &DbConn) -> Value { From fe212a425fb2ab25f4bae80a6eb65f677ce42d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Wed, 23 Sep 2026 22:51:04 +0200 Subject: [PATCH 3/4] Add typo exception --- .typos.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/.typos.toml b/.typos.toml index 7034a849..ca870f4e 100644 --- a/.typos.toml +++ b/.typos.toml @@ -18,6 +18,7 @@ extend-ignore-re = [ "COSE", "COSEKey", "COSEAlgorithm", + "CoseEncrypt0", # Ignore this specific string as it's valid "Ensure they are valid OTPs", # This word is misspelled upstream From d01db37e75753838f0e297f955c6244e76df4c06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Wed, 23 Sep 2026 23:17:28 +0200 Subject: [PATCH 4/4] Some simplifications --- .../down.sql | 0 .../up.sql | 2 - .../down.sql | 0 .../up.sql | 2 - .../down.sql | 0 .../up.sql | 2 - src/api/core/accounts.rs | 405 ++++++++---------- src/api/core/ciphers.rs | 130 +++--- src/api/core/organizations.rs | 2 +- src/auth.rs | 15 +- src/config.rs | 2 + src/db/models/cipher.rs | 34 +- src/db/models/user.rs | 46 +- src/db/models/user_signature_key_pair.rs | 40 +- 14 files changed, 299 insertions(+), 381 deletions(-) rename migrations/mysql/{2026-08-01-120000_user_crypto_v2 => 2026-09-20-120000_user_crypto_v2}/down.sql (100%) rename migrations/mysql/{2026-08-01-120000_user_crypto_v2 => 2026-09-20-120000_user_crypto_v2}/up.sql (93%) rename migrations/postgresql/{2026-08-01-120000_user_crypto_v2 => 2026-09-20-120000_user_crypto_v2}/down.sql (100%) rename migrations/postgresql/{2026-08-01-120000_user_crypto_v2 => 2026-09-20-120000_user_crypto_v2}/up.sql (93%) rename migrations/sqlite/{2026-08-01-120000_user_crypto_v2 => 2026-09-20-120000_user_crypto_v2}/down.sql (100%) rename migrations/sqlite/{2026-08-01-120000_user_crypto_v2 => 2026-09-20-120000_user_crypto_v2}/up.sql (93%) diff --git a/migrations/mysql/2026-08-01-120000_user_crypto_v2/down.sql b/migrations/mysql/2026-09-20-120000_user_crypto_v2/down.sql similarity index 100% rename from migrations/mysql/2026-08-01-120000_user_crypto_v2/down.sql rename to migrations/mysql/2026-09-20-120000_user_crypto_v2/down.sql diff --git a/migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql b/migrations/mysql/2026-09-20-120000_user_crypto_v2/up.sql similarity index 93% rename from migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql rename to migrations/mysql/2026-09-20-120000_user_crypto_v2/up.sql index e1bb52a8..f6a9948d 100644 --- a/migrations/mysql/2026-08-01-120000_user_crypto_v2/up.sql +++ b/migrations/mysql/2026-09-20-120000_user_crypto_v2/up.sql @@ -4,8 +4,6 @@ ALTER TABLE users ADD COLUMN security_version INTEGER; ALTER TABLE users ADD COLUMN v2_upgrade_token TEXT; ALTER TABLE users_organizations ADD COLUMN v2_upgrade_token TEXT; -DROP TABLE IF EXISTS user_signature_key_pairs; - CREATE TABLE user_signature_key_pairs ( uuid CHAR(36) NOT NULL PRIMARY KEY, user_uuid CHAR(36) NOT NULL UNIQUE, diff --git a/migrations/postgresql/2026-08-01-120000_user_crypto_v2/down.sql b/migrations/postgresql/2026-09-20-120000_user_crypto_v2/down.sql similarity index 100% rename from migrations/postgresql/2026-08-01-120000_user_crypto_v2/down.sql rename to migrations/postgresql/2026-09-20-120000_user_crypto_v2/down.sql diff --git a/migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql b/migrations/postgresql/2026-09-20-120000_user_crypto_v2/up.sql similarity index 93% rename from migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql rename to migrations/postgresql/2026-09-20-120000_user_crypto_v2/up.sql index 0e2527e0..54c4c790 100644 --- a/migrations/postgresql/2026-08-01-120000_user_crypto_v2/up.sql +++ b/migrations/postgresql/2026-09-20-120000_user_crypto_v2/up.sql @@ -4,8 +4,6 @@ ALTER TABLE users ADD COLUMN security_version INTEGER; ALTER TABLE users ADD COLUMN v2_upgrade_token TEXT; ALTER TABLE users_organizations ADD COLUMN v2_upgrade_token TEXT; -DROP TABLE IF EXISTS user_signature_key_pairs; - CREATE TABLE user_signature_key_pairs ( uuid CHAR(36) NOT NULL PRIMARY KEY, user_uuid CHAR(36) NOT NULL UNIQUE REFERENCES users (uuid) ON DELETE CASCADE, diff --git a/migrations/sqlite/2026-08-01-120000_user_crypto_v2/down.sql b/migrations/sqlite/2026-09-20-120000_user_crypto_v2/down.sql similarity index 100% rename from migrations/sqlite/2026-08-01-120000_user_crypto_v2/down.sql rename to migrations/sqlite/2026-09-20-120000_user_crypto_v2/down.sql diff --git a/migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql b/migrations/sqlite/2026-09-20-120000_user_crypto_v2/up.sql similarity index 93% rename from migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql rename to migrations/sqlite/2026-09-20-120000_user_crypto_v2/up.sql index 0db12ac3..ff026d80 100644 --- a/migrations/sqlite/2026-08-01-120000_user_crypto_v2/up.sql +++ b/migrations/sqlite/2026-09-20-120000_user_crypto_v2/up.sql @@ -4,8 +4,6 @@ ALTER TABLE users ADD COLUMN security_version INTEGER; ALTER TABLE users ADD COLUMN v2_upgrade_token TEXT; ALTER TABLE users_organizations ADD COLUMN v2_upgrade_token TEXT; -DROP TABLE IF EXISTS user_signature_key_pairs; - CREATE TABLE user_signature_key_pairs ( uuid TEXT NOT NULL PRIMARY KEY, user_uuid TEXT NOT NULL UNIQUE REFERENCES users (uuid) ON DELETE CASCADE, diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index f26ed454..e4f75982 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -1,6 +1,7 @@ use std::collections::HashSet; use chrono::Utc; +use num_traits::FromPrimitive; use rocket::{ http::Status, request::{FromRequest, Outcome, Request}, @@ -175,6 +176,14 @@ impl RegisterDataCompat { self.fold(|rdc| &rdc.key, |rdcu| &rdcu.master_password_unlock.key).to_owned() } + /// The id of the user key, which only the current format carries. + fn key_id(&self) -> Option { + match self { + RegisterDataCompat::RegisterDataOld(_) => None, + RegisterDataCompat::RegisterDataCur(rdcu) => rdcu.master_password_unlock.contained_key_id.clone(), + } + } + // When comparing with salt, email need to be normalized: // - https://github.com/bitwarden/clients/blob/web-v2026.5.0/libs/common/src/key-management/master-password/services/master-password.service.ts#L171 fn unprocessable(&self, email: &str) -> bool { @@ -276,7 +285,7 @@ impl AccountKeysData { let v2 = match (signed_public_key, self.signature_key_pair, self.security_state) { (Some(signed_public_key), Some(signature_key_pair), Some(security_state)) => { - let Some(signature_algorithm) = SignatureAlgorithm::from_str(&signature_key_pair.signature_algorithm) + let Some(signature_algorithm) = SignatureAlgorithm::parse(&signature_key_pair.signature_algorithm) else { err!(format!("Unsupported signature algorithm: {}", signature_key_pair.signature_algorithm)) }; @@ -306,30 +315,18 @@ impl AccountKeysData { impl WrappedAccountCryptographicState { /// This shape is v2-only: the key pairs and security state were already required by the - /// deserializer, so only the values still need checking. A missing signed public key or an - /// unknown algorithm has to be an error rather than a fallback to v1, which would silently turn - /// a rotation into a downgrade. + /// deserializer, so a missing signed public key fails the all-or-nothing check in + /// [`AccountKeysData::validate`] rather than falling back to v1, which would silently turn a + /// rotation into a downgrade. fn validate(self) -> ApiResult { - let Some(signed_public_key) = self.public_key_encryption_key_pair.signed_public_key else { - err!("No signed public key provided for a v2 account") - }; - let Some(signature_algorithm) = SignatureAlgorithm::from_str(&self.signature_key_pair.signature_algorithm) - else { - err!(format!("Unsupported signature algorithm: {}", self.signature_key_pair.signature_algorithm)) - }; - - Ok(ValidatedAccountKeys { - private_key: self.public_key_encryption_key_pair.wrapped_private_key, - public_key: self.public_key_encryption_key_pair.public_key, - v2: Some(ValidatedV2AccountKeys { - signed_public_key, - signing_key: self.signature_key_pair.wrapped_signing_key, - verifying_key: self.signature_key_pair.verifying_key, - signature_algorithm, - security_state: self.security_state.security_state, - security_version: self.security_state.security_version, - }), - }) + AccountKeysData { + user_key_encrypted_account_private_key: None, + account_public_key: None, + public_key_encryption_key_pair: Some(self.public_key_encryption_key_pair), + signature_key_pair: Some(self.signature_key_pair), + security_state: Some(self.security_state), + } + .validate() } } @@ -344,6 +341,19 @@ impl From for ValidatedAccountKeys { } impl ValidatedAccountKeys { + /// The keys of a request that may carry either shape. `accountKeys` supersedes the flat `keys` + /// object when both are sent. + fn from_request(account_keys: Option, keys: Option) -> ApiResult> { + match (account_keys, keys) { + (Some(account_keys), _) => account_keys.validate().map(Some), + (None, keys) => Ok(keys.map(Self::from)), + } + } + + fn is_v2(&self) -> bool { + self.v2.is_some() + } + /// Writes the parts of the state that live on the user itself. The user still needs saving, and /// [`Self::save_signature_key_pair`] still needs calling once it has been. /// @@ -371,7 +381,7 @@ impl ValidatedAccountKeys { return Ok(()); }; - let mut key_pair = match UserSignatureKeyPair::find_active_by_user(user_id, conn).await { + let mut key_pair = match UserSignatureKeyPair::find_by_user(user_id, conn).await { Some(mut key_pair) => { key_pair.signature_algorithm = v2.signature_algorithm as i32; key_pair.signing_key.clone_from(&v2.signing_key); @@ -407,6 +417,7 @@ pub struct MasterPasswordUnlock { #[serde(alias = "masterKeyWrappedUserKey")] key: String, + contained_key_id: Option, } #[derive(Debug, Deserialize)] @@ -534,10 +545,11 @@ pub async fn register(data: Json, email_verification: bool, conn: err!("The field Name must be a string with a maximum length of 50."); } - // Check against the password hint setting here so if it fails, the user - // can retry without losing their invitation below. + // Check against the password hint setting and the keys here so if they fail, + // the user can retry without losing their invitation below. let password_hint = clean_password_hint(data.master_password_hint.as_ref()); enforce_password_hint_setting(password_hint.as_ref())?; + let account_keys = ValidatedAccountKeys::from_request(data.account_keys, data.keys)?; let mut user = match User::find_by_mail(&email, &conn).await { Some(user) => { @@ -594,13 +606,13 @@ pub async fn register(data: Json, email_verification: bool, conn: user.name = name; } - let account_keys = match (data.account_keys, data.keys) { - (Some(account_keys), _) => Some(account_keys.validate()?), - (None, Some(keys)) => Some(keys.into()), - (None, None) => None, - }; if let Some(ref account_keys) = account_keys { account_keys.apply(&mut user)?; + // Like upstream, only a v2 registration records the user key id. A v1 account reports it + // later, through `user-key-id`. + if account_keys.is_v2() { + user.key_id = data.compat.key_id(); + } } if email_verified { @@ -659,11 +671,7 @@ async fn post_set_password(data: Json, headers: Headers, conn: let password_hint = clean_password_hint(data.master_password_hint.as_ref()); enforce_password_hint_setting(password_hint.as_ref())?; - let account_keys = match (data.account_keys, data.keys) { - (Some(account_keys), _) => Some(account_keys.validate()?), - (None, Some(keys)) => Some(keys.into()), - (None, None) => None, - }; + let account_keys = ValidatedAccountKeys::from_request(data.account_keys, data.keys)?; set_kdf_data(&mut user, data.compat.kdf())?; @@ -679,6 +687,10 @@ async fn post_set_password(data: Json, headers: Headers, conn: if let Some(ref account_keys) = account_keys { account_keys.apply(&mut user)?; + // As in `register`, only a v2 account records the user key id here + if account_keys.is_v2() { + user.key_id = data.compat.key_id(); + } } if let Some(identifier) = data.org_identifier @@ -842,11 +854,12 @@ async fn post_keys(data: Json, headers: Headers, conn: DbConn) -> let account_keys = match (data.account_keys, data.keys) { (Some(account_keys), _) => { let account_keys = account_keys.validate()?; - if account_keys.v2.is_none() { + if !account_keys.is_v2() { err!("AccountKeys are only supported for V2 encryption.") } - if let Some(user_key_id) = data.user_key_id { - user.key_id = Some(user_key_id); + // A client that predates key ids sends none, and reports it later through `user-key-id` + if data.user_key_id.is_some() { + user.key_id = data.user_key_id; } account_keys } @@ -902,6 +915,8 @@ async fn post_password(data: Json, headers: Headers, conn: DbCon err!("Invalid master password salt") } + validate_key_id_unchanged(&user, &unlock_data)?; + (authentication_data.master_password_authentication_hash, unlock_data.master_key_wrapped_user_key) } else if let (Some(new_master_password_hash), Some(new_key)) = (data.new_master_password_hash, data.key) { (new_master_password_hash, new_key) @@ -987,6 +1002,21 @@ struct UnlockData { salt: String, kdf: KDFData, master_key_wrapped_user_key: String, + contained_key_id: Option, +} + +/// A password or KDF change re-wraps the same user key, so a key id sent with it has to be the +/// current one. Either may be missing: from a client that predates key ids, or a user whose key id +/// isn't known yet. There is nothing to compare in those cases. +/// +/// Ref: +fn validate_key_id_unchanged(user: &User, unlock_data: &UnlockData) -> EmptyResult { + if let (Some(current), Some(contained)) = (&user.key_id, &unlock_data.contained_key_id) + && current != contained + { + err!("Invalid user key sent in master-password unlock data.") + } + Ok(()) } #[derive(Deserialize)] @@ -1013,6 +1043,8 @@ async fn post_kdf(data: Json, headers: Headers, conn: DbConn, nt: err!("Invalid master password salt") } + validate_key_id_unchanged(&headers.user, &data.unlock_data)?; + let mut user = headers.user; set_kdf_data(&mut user, &data.unlock_data.kdf)?; @@ -1071,9 +1103,7 @@ struct KeyData { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct RotateAccountUnlockData { - emergency_access_unlock_data: Vec, master_password_unlock_data: MasterPasswordUnlockData, - organization_account_recovery_unlock_data: Vec, #[serde(flatten)] common: CommonUnlockData, } @@ -1081,10 +1111,8 @@ struct RotateAccountUnlockData { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct MasterPasswordUnlockData { - kdf_type: i32, - kdf_iterations: i32, - kdf_parallelism: Option, - kdf_memory: Option, + #[serde(flatten)] + kdf: KDFData, email: String, master_key_authentication_hash: String, master_key_encrypted_user_key: String, @@ -1093,9 +1121,13 @@ struct MasterPasswordUnlockData { /// The unlock data both rotation endpoints share. Vaultwarden has neither trusted device encryption /// nor passkey login, so `key-rotation-data` reports none of either and these must arrive empty. -#[derive(Deserialize, Default)] +/// +/// Ref: +#[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct CommonUnlockData { + emergency_access_unlock_data: Vec, + organization_account_recovery_unlock_data: Vec, #[serde(default)] passkey_unlock_data: Vec, #[serde(default)] @@ -1125,7 +1157,7 @@ struct RotateAccountData { #[serde(rename_all = "camelCase")] struct RotateUserKeysData { wrapped_account_cryptographic_state: WrappedAccountCryptographicState, - unlock_data: RotateUserKeysUnlockData, + unlock_data: CommonUnlockData, account_data: RotateAccountData, unlock_method_data: UnlockMethodData, new_user_key_id: Option, @@ -1140,15 +1172,6 @@ struct WrappedAccountCryptographicState { security_state: SecurityStateData, } -#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] -struct RotateUserKeysUnlockData { - emergency_access_unlock_data: Vec, - organization_account_recovery_unlock_data: Vec, - #[serde(flatten)] - common: CommonUnlockData, -} - #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct UnlockMethodData { @@ -1179,6 +1202,7 @@ fn validate_contained_key_id(contained_key_id: Option<&KeyId>, new_user_key_id: } /// Ref: +#[derive(num_derive::FromPrimitive)] enum UnlockMethod { Tde = 0, MasterPassword = 1, @@ -1225,7 +1249,7 @@ async fn validate_rotation_account_keys(keys: &ValidatedAccountKeys, user: &User return Ok(()); } - let Some(key_pair) = UserSignatureKeyPair::find_active_by_user(&user.uuid, conn).await else { + let Some(key_pair) = UserSignatureKeyPair::find_by_user(&user.uuid, conn).await else { err!("The account is missing its signature key pair") }; if key_pair.verifying_key != v2.verifying_key { @@ -1245,49 +1269,28 @@ fn enc_string_type(enc_string: &str) -> Option<&str> { enc_string.split_once('.').map(|(enc_type, _)| enc_type) } -fn validate_keydata( - data: &KeyData, - existing_ciphers: &[Cipher], - existing_folders: &[Folder], - existing_emergency_access: &[EmergencyAccess], - existing_memberships: &[Membership], - existing_sends: &[Send], - user: &User, -) -> EmptyResult { - if user.client_kdf_type != data.account_unlock_data.master_password_unlock_data.kdf_type - || user.client_kdf_iter != data.account_unlock_data.master_password_unlock_data.kdf_iterations - || user.client_kdf_memory != data.account_unlock_data.master_password_unlock_data.kdf_memory - || user.client_kdf_parallelism != data.account_unlock_data.master_password_unlock_data.kdf_parallelism - || user.email != data.account_unlock_data.master_password_unlock_data.email - { - err!("Changing the kdf variant or email is not supported during key rotation"); +impl KDFData { + /// Whether these are the settings the user already has, which a key rotation can't change. + fn is_unchanged_for(&self, user: &User) -> bool { + user.client_kdf_type == self.kdf + && user.client_kdf_iter == self.kdf_iterations + && user.client_kdf_memory == self.kdf_memory + && user.client_kdf_parallelism == self.kdf_parallelism } - - validate_rotation_data( - &data.account_data, - &data.account_unlock_data.emergency_access_unlock_data, - &data.account_unlock_data.organization_account_recovery_unlock_data, - existing_ciphers, - existing_folders, - existing_emergency_access, - existing_memberships, - existing_sends, - ) } /// A rotation re-encrypts everything under the new user key, so anything left out would be /// unreadable afterwards. Both rotation endpoints require the client to send the complete set. -#[expect(clippy::too_many_arguments, reason = "Every collection has to be cross-checked")] fn validate_rotation_data( - account_data: &RotateAccountData, - emergency_access_unlock_data: &[UpdateEmergencyAccessData], - organization_account_recovery_unlock_data: &[UpdateResetPasswordData], + data: &RotateData, existing_ciphers: &[Cipher], existing_folders: &[Folder], existing_emergency_access: &[EmergencyAccess], existing_memberships: &[Membership], existing_sends: &[Send], ) -> EmptyResult { + let account_data = &data.account_data; + // Check that we're correctly rotating all the user's ciphers let existing_cipher_ids = existing_ciphers.iter().map(|c| &c.uuid).collect::>(); let provided_cipher_ids = account_data @@ -1311,7 +1314,7 @@ fn validate_rotation_data( let existing_emergency_access_ids = existing_emergency_access.iter().map(|ea| &ea.uuid).collect::>(); let provided_emergency_access_ids = - emergency_access_unlock_data.iter().map(|ea| &ea.id).collect::>(); + data.unlock_data.emergency_access_unlock_data.iter().map(|ea| &ea.id).collect::>(); if !provided_emergency_access_ids.is_superset(&existing_emergency_access_ids) { err!("All existing emergency access keys must be included in the rotation") } @@ -1319,7 +1322,9 @@ fn validate_rotation_data( // Check that we're correctly rotating all the user's reset password keys let existing_reset_password_ids = existing_memberships.iter().map(|m| &m.org_uuid).collect::>(); - let provided_reset_password_ids = organization_account_recovery_unlock_data + let provided_reset_password_ids = data + .unlock_data + .organization_account_recovery_unlock_data .iter() .map(|rp| &rp.organization_id) .collect::>(); @@ -1403,29 +1408,17 @@ async fn get_key_rotation_data(headers: Headers, conn: DbConn) -> JsonResult { }))) } -/// How the new user key gets wrapped, which is the one part the two rotation endpoints don't share. -enum UnlockRotation { - /// `rotate-user-account-keys`: the master password changes along with the keys. - PasswordChange { - authentication_hash: String, - wrapped_user_key: String, - }, - /// `rotate-user-keys`: the password is untouched, only the wrapped user key is replaced. - WrappedUserKeyOnly { - wrapped_user_key: String, - }, -} - /// Everything a rotation replaces, once each endpoint's wrapper has been peeled off. struct RotateData { account_keys: ValidatedAccountKeys, account_data: RotateAccountData, - emergency_access_unlock_data: Vec, - organization_account_recovery_unlock_data: Vec, - v2_upgrade_token: Option, + unlock_data: CommonUnlockData, /// Only v2 (COSE) user keys have an id, so this is `None` for a v1 -> v1 rotation. new_user_key_id: Option, - unlock: UnlockRotation, + /// The new user key, wrapped by the master key. + wrapped_user_key: String, + /// Only for `rotate-user-account-keys`, where the master password changes along with the keys. + new_password_hash: Option, } impl CommonUnlockData { @@ -1451,52 +1444,26 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: err!("Invalid password") } - data.account_unlock_data.common.validate()?; + let unlock_data = data.account_unlock_data.master_password_unlock_data; + if !unlock_data.kdf.is_unchanged_for(&headers.user) || unlock_data.email != headers.user.email { + err!("Changing the kdf variant or email is not supported during key rotation"); + } + validate_contained_key_id(unlock_data.contained_key_id.as_ref(), data.new_user_key_id.as_ref())?; - let user_id = &headers.user.uuid; - let existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await; - let existing_folders = Folder::find_by_user(user_id, &conn).await; - let existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await; - let mut existing_memberships = Membership::find_by_user(user_id, &conn).await; - // We only rotate the reset password key if it is set. - existing_memberships.retain(|m| m.reset_password_key.is_some()); - let existing_sends = Send::find_by_user(user_id, &conn).await; - - validate_keydata( - &data, - &existing_ciphers, - &existing_folders, - &existing_emergency_access, - &existing_memberships, - &existing_sends, - &headers.user, - )?; - - validate_contained_key_id( - data.account_unlock_data.master_password_unlock_data.contained_key_id.as_ref(), - data.new_user_key_id.as_ref(), - )?; - - let unlock = UnlockRotation::PasswordChange { - authentication_hash: data.account_unlock_data.master_password_unlock_data.master_key_authentication_hash, - wrapped_user_key: data.account_unlock_data.master_password_unlock_data.master_key_encrypted_user_key, - }; + let mut common = data.account_unlock_data.common; + // This endpoint always logs every session out, so an upgrade token is never kept here. That + // also clears one left over from an earlier upgrade. Ref: upstream's AccountsKeyManagementController + common.v2_upgrade_token = None; rotate_account( RotateData { account_keys: data.account_keys.validate()?, account_data: data.account_data, - emergency_access_unlock_data: data.account_unlock_data.emergency_access_unlock_data, - organization_account_recovery_unlock_data: data - .account_unlock_data - .organization_account_recovery_unlock_data, - // This endpoint always logs every session out, so an upgrade token is never kept here. That - // also clears one left over from an earlier upgrade. Ref: upstream's AccountsKeyManagementController - v2_upgrade_token: None, + unlock_data: common, new_user_key_id: data.new_user_key_id, - unlock, + wrapped_user_key: unlock_data.master_key_encrypted_user_key, + new_password_hash: Some(unlock_data.master_key_authentication_hash), }, - (existing_ciphers, existing_folders, existing_emergency_access, existing_memberships, existing_sends), headers, conn, nt, @@ -1504,25 +1471,29 @@ async fn post_rotatekey(data: Json, headers: Headers, conn: DbConn, nt: .await } -type ExistingRotationData = (Vec, Vec, Vec, Vec, Vec); - -/// The body shared by both rotation endpoints: re-save every piece of data the client re-encrypted, -/// then swap the account keys and the wrapped user key. -async fn rotate_account( - data: RotateData, - existing: ExistingRotationData, - headers: Headers, - conn: DbConn, - nt: Notify<'_>, -) -> EmptyResult { +/// The body shared by both rotation endpoints: check that the client re-encrypted everything, re-save +/// it, then swap the account keys and the wrapped user key. +async fn rotate_account(data: RotateData, headers: Headers, conn: DbConn, nt: Notify<'_>) -> EmptyResult { // TODO: See if we can wrap everything within a SQL Transaction. If something fails it should revert everything. - let ( - mut existing_ciphers, - mut existing_folders, - mut existing_emergency_access, - mut existing_memberships, - mut existing_sends, - ) = existing; + data.unlock_data.validate()?; + + let user_id = &headers.user.uuid; + let mut existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await; + let mut existing_folders = Folder::find_by_user(user_id, &conn).await; + let mut existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await; + let mut existing_memberships = Membership::find_by_user(user_id, &conn).await; + // We only rotate the reset password key if it is set. + existing_memberships.retain(|m| m.reset_password_key.is_some()); + let mut existing_sends = Send::find_by_user(user_id, &conn).await; + + validate_rotation_data( + &data, + &existing_ciphers, + &existing_folders, + &existing_emergency_access, + &existing_memberships, + &existing_sends, + )?; validate_rotation_account_keys(&data.account_keys, &headers.user, &conn).await?; @@ -1531,8 +1502,8 @@ async fn rotate_account( // account that is already v2 it is meaningless, so it gets discarded and the security stamp is // reset, as a rotation otherwise would. That ends the refresh tokens, while the access tokens in // use run out on their own, as upstream. - let is_upgrade = data.v2_upgrade_token.is_some() && !headers.user.is_v2(); - let upgrade_token = match &data.v2_upgrade_token { + let is_upgrade = data.unlock_data.v2_upgrade_token.is_some() && !headers.user.is_v2(); + let upgrade_token = match &data.unlock_data.v2_upgrade_token { Some(token) if is_upgrade => Some(serde_json::to_string(token)?), _ => None, }; @@ -1541,7 +1512,7 @@ async fn rotate_account( // to be trusted, and an upgrade shows the user no prompt. So the stored keys are kept, and each // organization gets the upgrade token instead, to unwrap the v2 user key with. // Ref: upstream's OrganizationUserRotationValidator - for reset_password_data in &data.organization_account_recovery_unlock_data { + for reset_password_data in &data.unlock_data.organization_account_recovery_unlock_data { let has_key = reset_password_data.reset_password_key.as_deref().is_some_and(|k| !k.is_empty()); if is_upgrade && has_key { err!("Account recovery keys cannot be rotated during a V1 to V2 upgrade rotation.") @@ -1574,7 +1545,7 @@ async fn rotate_account( } // Update emergency access data - for emergency_access_data in data.emergency_access_unlock_data { + for emergency_access_data in data.unlock_data.emergency_access_unlock_data { let Some(saved_emergency_access) = existing_emergency_access.iter_mut().find(|ea| ea.uuid == emergency_access_data.id) else { @@ -1586,7 +1557,7 @@ async fn rotate_account( } // Update reset password data - for reset_password_data in data.organization_account_recovery_unlock_data { + for reset_password_data in data.unlock_data.organization_account_recovery_unlock_data { let Some(membership) = existing_memberships.iter_mut().find(|m| m.org_uuid == reset_password_data.organization_id) else { @@ -1618,35 +1589,26 @@ async fn rotate_account( }; // Prevent triggering cipher updates via WebSockets by settings UpdateType::None - // The user sessions are invalidated because all the ciphers were re-encrypted and thus triggering an update could cause issues. - // We force the users to logout after the user has been saved to try and prevent these issues. + // Other sessions still hold the old user key, so an update to a re-encrypted cipher could cause issues. + // After the user is saved they are logged out, or on an upgrade, told to sync the new key. update_cipher_from_data(saved_cipher, cipher_data, &headers, None, &conn, &nt, UpdateType::None).await?; } } // Update user data let mut user = headers.user; - let keep_sessions_alive = is_upgrade; user.v2_upgrade_token = upgrade_token; data.account_keys.apply(&mut user)?; // The old id names a key that no longer exists, so it is replaced even when the new key has none. user.key_id = data.new_user_key_id; - match data.unlock { - UnlockRotation::PasswordChange { - authentication_hash, - wrapped_user_key, - } => { - user.set_password(&authentication_hash, Some(wrapped_user_key), false, None, &conn).await?; - } - UnlockRotation::WrappedUserKeyOnly { - wrapped_user_key, - } => { - user.akey = wrapped_user_key; - } + user.akey = data.wrapped_user_key; + if let Some(new_password_hash) = data.new_password_hash { + user.set_password(&new_password_hash, None, false, None, &conn).await?; } - if !keep_sessions_alive { + // An upgrade keeps the sessions alive, see `is_upgrade` above + if !is_upgrade { user.reset_security_stamp_after_key_rotation(&conn).await?; } @@ -1661,7 +1623,7 @@ async fn rotate_account( // Adding the device uuid will prevent this. // When the sessions were kept alive, the reason lets the other clients sync the new keys (through // the upgrade token) instead of logging out, if they have `pm-31050-no-logout-key-upgrade-rotation`. - let reason = keep_sessions_alive.then_some(LogOutReason::KeyRotation); + let reason = is_upgrade.then_some(LogOutReason::KeyRotation); nt.send_logout_with_reason(&user, Some(&headers.device), reason, &conn).await; Ok(()) @@ -1683,64 +1645,34 @@ async fn post_rotate_user_keys( ) -> EmptyResult { let data: RotateUserKeysData = data.into_inner(); - data.unlock_data.common.validate()?; - - let unlock = if data.unlock_method_data.unlock_method == UnlockMethod::MasterPassword as i32 { - let Some(unlock_data) = data.unlock_method_data.master_password_unlock_data else { - err!("Missing master password unlock data") - }; - if headers.user.client_kdf_type != unlock_data.kdf.kdf - || headers.user.client_kdf_iter != unlock_data.kdf.kdf_iterations - || headers.user.client_kdf_memory != unlock_data.kdf.kdf_memory - || headers.user.client_kdf_parallelism != unlock_data.kdf.kdf_parallelism - { - err!("Changing the kdf variant is not supported during key rotation") + let wrapped_user_key = match UnlockMethod::from_i32(data.unlock_method_data.unlock_method) { + Some(UnlockMethod::MasterPassword) => { + let Some(unlock_data) = data.unlock_method_data.master_password_unlock_data else { + err!("Missing master password unlock data") + }; + if !unlock_data.kdf.is_unchanged_for(&headers.user) { + err!("Changing the kdf variant is not supported during key rotation") + } + if unlock_data.salt != headers.user.email { + err!("Invalid master password salt") + } + validate_contained_key_id(unlock_data.contained_key_id.as_ref(), data.new_user_key_id.as_ref())?; + unlock_data.master_key_wrapped_user_key } - if unlock_data.salt != headers.user.email { - err!("Invalid master password salt") - } - validate_contained_key_id(unlock_data.contained_key_id.as_ref(), data.new_user_key_id.as_ref())?; - UnlockRotation::WrappedUserKeyOnly { - wrapped_user_key: unlock_data.master_key_wrapped_user_key, - } - } else if data.unlock_method_data.unlock_method == UnlockMethod::Tde as i32 { - err!("Trusted device encryption is not supported") - } else if data.unlock_method_data.unlock_method == UnlockMethod::KeyConnector as i32 { - err!("Key connector is not supported") - } else { - err!("Unrecognized unlock method") + Some(UnlockMethod::Tde) => err!("Trusted device encryption is not supported"), + Some(UnlockMethod::KeyConnector) => err!("Key connector is not supported"), + None => err!("Unrecognized unlock method"), }; - let user_id = &headers.user.uuid; - let existing_ciphers = Cipher::find_owned_by_user(user_id, &conn).await; - let existing_folders = Folder::find_by_user(user_id, &conn).await; - let existing_emergency_access = EmergencyAccess::find_all_confirmed_by_grantor_uuid(user_id, &conn).await; - let mut existing_memberships = Membership::find_by_user(user_id, &conn).await; - existing_memberships.retain(|m| m.reset_password_key.is_some()); - let existing_sends = Send::find_by_user(user_id, &conn).await; - - validate_rotation_data( - &data.account_data, - &data.unlock_data.emergency_access_unlock_data, - &data.unlock_data.organization_account_recovery_unlock_data, - &existing_ciphers, - &existing_folders, - &existing_emergency_access, - &existing_memberships, - &existing_sends, - )?; - rotate_account( RotateData { account_keys: data.wrapped_account_cryptographic_state.validate()?, account_data: data.account_data, - emergency_access_unlock_data: data.unlock_data.emergency_access_unlock_data, - organization_account_recovery_unlock_data: data.unlock_data.organization_account_recovery_unlock_data, - v2_upgrade_token: data.unlock_data.common.v2_upgrade_token, + unlock_data: data.unlock_data, new_user_key_id: data.new_user_key_id, - unlock, + wrapped_user_key, + new_password_hash: None, }, - (existing_ciphers, existing_folders, existing_emergency_access, existing_memberships, existing_sends), headers, conn, nt, @@ -1757,8 +1689,9 @@ struct KeyIdData { #[post("/accounts/key-management/user-key-id", data = "")] async fn post_user_key(data: Json, headers: Headers, conn: DbConn) -> EmptyResult { let mut user = headers.user; + // Only a backfill for accounts that have none. Afterwards the id changes with the key, in a rotation. if user.key_id.is_some() { - err_code!("Unexpected data", Status::UnprocessableEntity.code); + err!("User key id is already set.") } user.key_id = Some(data.into_inner().user_key_id); diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index b24d68f4..3ffda3b3 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -23,7 +23,7 @@ use crate::{ models::{ Archive, Attachment, AttachmentId, Cipher, CipherId, Collection, CollectionCipher, CollectionGroup, CollectionId, CollectionUser, EventType, Favorite, Folder, FolderCipher, FolderId, Group, KeyId, - Membership, MembershipType, OrgPolicy, OrgPolicyType, OrganizationId, RepromptType, Send, UserId, + Membership, MembershipType, OrgPolicy, OrgPolicyType, OrganizationId, RepromptType, Send, User, UserId, is_data_blob_encrypted, }, }, @@ -190,7 +190,8 @@ async fn sync(data: SyncData, headers: Headers, client_version: Option, } +/// A field of a [`CipherData`] that fails upstream's model validation. +#[derive(Debug)] +pub struct CipherValidationError { + pub field: &'static str, + pub message: String, +} + +impl From for crate::Error { + fn from(e: CipherValidationError) -> Self { + Self::new_msg(e.message) + } +} + +/// A user-owned cipher must be encrypted with the user's current key. Organization ciphers use the +/// organization key, which has no id yet, and either id may be missing: from a client that predates +/// the field, or a user whose key id isn't known yet. There is nothing to compare in those cases. +/// +/// Ref: upstream's `CiphersController.ValidateCipherEncryptedByUser` +fn validate_encrypted_by_user_key(data: &CipherData, user: &User, is_org_cipher: bool) -> EmptyResult { + if !is_org_cipher + && let (Some(cipher_key_id), Some(user_key_id)) = (&data.encrypted_by_key_id, &user.key_id) + && cipher_key_id != user_key_id + { + err!("Cipher was not encrypted with the current user key. Please try again.") + } + Ok(()) +} + /// Upstream's `[StringLength(500000)]` on `CipherRequestModel.Data` const MAX_CIPHER_DATA_LENGTH: usize = 500_000; impl CipherData { + /// Whether the content is a single blob rather than the per-type fields. This parses `data`, so + /// callers that need the answer more than once should keep it. + pub fn is_blob(&self) -> bool { + self.data.as_deref().is_some_and(is_data_blob_encrypted) + } + /// Checks the content the way upstream's model validation does, before anything is saved. - /// On failure, returns the offending field and the message for it. + /// `is_blob` is [`Self::is_blob`]. /// /// Ref: - pub fn validate_content(&self) -> Result<(), (&'static str, String)> { + pub fn validate_content(&self, is_blob: bool) -> Result<(), CipherValidationError> { if let Some(data) = &self.data && data.len() > MAX_CIPHER_DATA_LENGTH { - return Err(( - "Data", - format!("The field Data must be a string with a maximum length of {MAX_CIPHER_DATA_LENGTH}."), - )); + return Err(CipherValidationError { + field: "Data", + message: format!("The field Data must be a string with a maximum length of {MAX_CIPHER_DATA_LENGTH}."), + }); } // A blob carries the name inside it, so only the other formats need one - let is_blob = self.data.as_deref().is_some_and(is_data_blob_encrypted); if !is_blob && self.name.as_deref().is_none_or(|n| n.trim().is_empty()) { - return Err(("Name", String::from("The Name field is required."))); + return Err(CipherValidationError { + field: "Name", + message: String::from("The Name field is required."), + }); } Ok(()) @@ -394,13 +431,14 @@ async fn post_ciphers_create( if data.cipher.encrypted_for != headers.user.uuid { err_code!("Invalid user cipher", Status::UnprocessableEntity.code); } + validate_encrypted_by_user_key(&data.cipher, &headers.user, data.cipher.organization_id.is_some())?; // This check is usually only needed in update_cipher_from_data(), but we // need it here as well to avoid creating an empty cipher in the call to // cipher.save() below. enforce_personal_ownership_policy(Some(&data.cipher), &headers, &conn).await?; - let mut cipher = Cipher::new(data.cipher.r#type, data.cipher.name.clone().unwrap_or_default()); + let mut cipher = Cipher::new(data.cipher.r#type, String::new()); cipher.user_uuid = Some(headers.user.uuid.clone()); cipher.save(&conn).await?; @@ -428,12 +466,7 @@ async fn post_ciphers(data: Json, headers: Headers, conn: DbConn, nt err_code!("Invalid user cipher", Status::UnprocessableEntity.code); } - if let Some(cipher_key_id) = &data.encrypted_by_key_id - && let Some(user_key_id) = &headers.user.key_id - && cipher_key_id != user_key_id - { - err_code!("Invalid key cipher", Status::UnprocessableEntity.code); - } + validate_encrypted_by_user_key(&data, &headers.user, data.organization_id.is_some())?; // The web/browser clients set this field to null as expected, but the // mobile clients seem to set the invalid value `0001-01-01T00:00:00`, @@ -441,7 +474,7 @@ async fn post_ciphers(data: Json, headers: Headers, conn: DbConn, nt // needed when creating a new cipher, so just ignore it unconditionally. data.last_known_revision_date = None; - let mut cipher = Cipher::new(data.r#type, data.name.clone().unwrap_or_default()); + let mut cipher = Cipher::new(data.r#type, String::new()); update_cipher_from_data(&mut cipher, data, &headers, None, &conn, &nt, UpdateType::SyncCipherCreate).await?; Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, None, CipherSyncType::User, &conn).await?)) @@ -490,9 +523,8 @@ pub async fn update_cipher_from_data( enforce_personal_ownership_policy(Some(&data), headers, conn).await?; - if let Err((_, message)) = data.validate_content() { - err!(message) - } + let is_blob = data.is_blob(); + data.validate_content(is_blob)?; // Check that the client isn't updating an existing cipher with stale data. // And only perform this check when not importing ciphers, else the date/time check will fail. @@ -581,15 +613,6 @@ pub async fn update_cipher_from_data( } } - // A blob replaces every content field, so the per-type data below neither applies nor exists. - // `validate_content` made sure anything else has a name. - let blob = data.data.filter(|d| is_data_blob_encrypted(d)); - let name = if blob.is_some() { - String::new() - } else { - data.name.unwrap_or_default() - }; - let type_data_opt = match data.r#type { 1 => data.login, 2 => data.secure_note, @@ -602,26 +625,35 @@ pub async fn update_cipher_from_data( _ => err!("Invalid type"), }; - let stored_data = if let Some(blob) = blob { - blob - } else if let Some(mut data) = type_data_opt { - // Remove the 'Response' key from the base object. - data.as_object_mut().unwrap().remove("response"); - // Remove the 'Response' key from every Uri. - if data["uris"].is_array() { - data["uris"] = clean_cipher_data(data["uris"].clone()); - } - data.to_string() + if let Some(blob) = data.data.filter(|_| is_blob) { + // A blob holds all of the content, the name included, so nothing is kept outside it. The name + // column can't be null, so it's left empty; `to_json` reports it as null, as upstream does. + // TODO: Make `ciphers.name` nullable and store `None` here instead. + cipher.name = String::new(); + cipher.notes = None; + cipher.fields = None; + cipher.password_history = None; + cipher.data = blob; } else { - err!("Data missing") - }; + let Some(mut type_data) = type_data_opt else { + err!("Data missing") + }; + // Remove the 'Response' key from the base object. + type_data.as_object_mut().unwrap().remove("response"); + // Remove the 'Response' key from every Uri. + if type_data["uris"].is_array() { + type_data["uris"] = clean_cipher_data(type_data["uris"].clone()); + } + + // `validate_content` made sure there is a name + cipher.name = data.name.unwrap_or_default(); + cipher.notes = data.notes; + cipher.fields = data.fields.map(|f| clean_cipher_data(f).to_string()); + cipher.password_history = data.password_history.map(|f| f.to_string()); + cipher.data = type_data.to_string(); + } cipher.key = data.key; - cipher.name = name; - cipher.notes = data.notes; - cipher.fields = data.fields.map(|f| clean_cipher_data(f).to_string()); - cipher.data = stored_data; - cipher.password_history = data.password_history.map(|f| f.to_string()); cipher.reprompt = data.reprompt.filter(|r| *r == RepromptType::None as i32 || *r == RepromptType::Password as i32); cipher.save(conn).await?; @@ -718,7 +750,7 @@ async fn post_ciphers_import(data: Json, headers: Headers, conn: DbC let folder_id = relations_map.get(&index).and_then(|i| folders.get(*i).cloned()); cipher_data.folder_id = folder_id; - let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone().unwrap_or_default()); + let mut cipher = Cipher::new(cipher_data.r#type, String::new()); update_cipher_from_data(&mut cipher, cipher_data, &headers, None, &conn, &nt, UpdateType::None).await?; } @@ -786,6 +818,8 @@ async fn put_cipher( err!("Cipher is not write accessible") } + validate_encrypted_by_user_key(&data, &headers.user, cipher.organization_uuid.is_some())?; + update_cipher_from_data(&mut cipher, data, &headers, None, &conn, &nt, UpdateType::SyncCipherUpdate).await?; Ok(Json(cipher.to_json(&headers.host, &headers.user.uuid, None, CipherSyncType::User, &conn).await?)) diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 3ca0d326..d51eac1a 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -1885,7 +1885,7 @@ async fn post_org_import( cipher_data.folder_id = None; // Replace the client-provided, unvalidated organizationId with the real target org cipher_data.organization_id = Some(org_id.clone()); - let mut cipher = Cipher::new(cipher_data.r#type, cipher_data.name.clone().unwrap_or_default()); + let mut cipher = Cipher::new(cipher_data.r#type, String::new()); update_cipher_from_data( &mut cipher, cipher_data, diff --git a/src/auth.rs b/src/auth.rs index 75dbc041..8f4d0ada 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -662,7 +662,7 @@ impl<'r> FromRequest<'r> for Headers { err_handler!("Invalid device id") }; - let Some(mut user) = User::find_by_uuid(&user_id, &conn).await else { + let Some(user) = User::find_by_uuid(&user_id, &conn).await else { err_handler!("Device has no user associated") }; @@ -671,16 +671,9 @@ impl<'r> FromRequest<'r> for Headers { err_handler!("Error getting current route for stamp exception") }; - let allowed = user.stamp_exceptions().iter().any(|e| e.allows(&claims.sstamp, current_route)); - - // Drop the expired exceptions, so they aren't checked for every request from now on. - if user.retain_stamp_exceptions(|e| !e.is_expired()) - && let Err(e) = user.save_stamp_exceptions(&conn).await - { - error!("Error updating user: {e:#?}"); - } - - if !allowed { + // Expired exceptions are dropped the next time the stamp is reset, not here: writing them + // back from this request could undo a reset made since the user was read. + if !user.stamp_exceptions().iter().any(|e| e.allows(&claims.sstamp, current_route)) { err_handler!("Invalid security stamp") } } diff --git a/src/config.rs b/src/config.rs index f88eb99e..fd03342d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1437,6 +1437,8 @@ pub const SUPPORTED_FEATURE_FLAGS: &[&str] = &[ "pm-31050-no-logout-key-upgrade-rotation", "pm-27278-v2-password-registration", "enable-account-encryption-v2-jit-password-registration", + "pm-30144-sdk-key-rotation", + "force-upgrade-v2-encryption", // Mobile Team "anon-addy-self-host-alias", "simple-login-self-host-alias", diff --git a/src/db/models/cipher.rs b/src/db/models/cipher.rs index a0eae75c..164f5f23 100644 --- a/src/db/models/cipher.rs +++ b/src/db/models/cipher.rs @@ -71,7 +71,12 @@ pub struct Cipher { /// /// Ref: pub fn is_data_blob_encrypted(data: &str) -> bool { - serde_json::from_str::(data).is_ok_and(|d| d.get("format_version").is_some()) + serde_json::from_str::(data).is_ok_and(|d| is_blob_value(&d)) +} + +/// [`is_data_blob_encrypted`] for `data` that was already parsed. +fn is_blob_value(data: &Value) -> bool { + data.get("format_version").is_some() } pub enum RepromptType { @@ -121,8 +126,9 @@ impl Cipher { .insert(format!("Ciphers[{index}].Notes"), serde_json::to_value([&max_note_size_msg]).unwrap()); } - if let Err((field, message)) = cipher.validate_content() { - validation_errors.insert(format!("Ciphers[{index}].{field}"), serde_json::to_value([message]).unwrap()); + if let Err(e) = cipher.validate_content(cipher.is_blob()) { + validation_errors + .insert(format!("Ciphers[{index}].{}", e.field), serde_json::to_value([e.message]).unwrap()); } // Validate the password history if it contains `null` values and if so, return a warning @@ -169,7 +175,10 @@ impl Cipher { ) -> Result { use crate::util::{format_date, validate_and_format_date}; - let is_blob_encrypted = is_data_blob_encrypted(&self.data); + // Parsed once here, since `data` can be large and this runs for every cipher in a sync. + // `LowerCase` only lowercases the first letter of each key, so it keeps `format_version`. + let type_data = serde_json::from_str::>(&self.data).map(|d| d.data); + let is_blob_encrypted = type_data.as_ref().is_ok_and(is_blob_value); let mut attachments_json: Value = Value::Null; if let Some(cipher_sync_data) = cipher_sync_data { @@ -360,28 +369,25 @@ impl Cipher { if is_blob_encrypted { // The blob holds all of the content, so it is sent back as-is and the structured fields - // stay null, as upstream does. + // stay null, as upstream does. Only the name needs clearing, the others are never stored. json_object["data"] = json!(self.data); json_object["name"] = Value::Null; - json_object["notes"] = Value::Null; - json_object["fields"] = Value::Null; - json_object["passwordHistory"] = Value::Null; } else { - json_object[key] = self.legacy_type_data_json(); + json_object[key] = self.legacy_type_data_json(type_data); } Ok(json_object) } - /// The per-type data (`login`, `card`, …) of a legacy cipher, with fixups for values that are - /// known to break clients. - fn legacy_type_data_json(&self) -> Value { + /// The per-type data (`login`, `card`, …) of a legacy cipher, from `type_data` as parsed from + /// `self.data`, with fixups for values that are known to break clients. + fn legacy_type_data_json(&self, type_data: Result) -> Value { use crate::util::validate_and_format_date; // Get the type_data or a default to an empty json object '{}'. // If not passing an empty object, mobile clients will crash. - let mut type_data_json = serde_json::from_str::>(&self.data) + let mut type_data_json = type_data .inspect_err(|_| warn!("Error parsing data field for {}", self.uuid)) - .map_or_else(|_| Value::Object(serde_json::Map::new()), |d| d.data); + .unwrap_or_else(|_| Value::Object(serde_json::Map::new())); // NOTE: This was marked as *Backwards Compatibility Code*, but as of January 2021 this is still being used by upstream // Set the first element of the Uris array as Uri, this is needed several (mobile) clients. diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 51c56251..6b4658ed 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -257,7 +257,10 @@ impl User { self.security_stamp = get_uuid(); // A reset is meant to end the other sessions, which a key rotation's grace would undo. The // route exceptions stay, since they are set right before a reset, for the stamp it replaces. - self.retain_stamp_exceptions(|e| e.routes.is_some()); + // This is also where expired exceptions get dropped. + let mut exceptions = self.stamp_exceptions(); + exceptions.retain(|e| e.routes.is_some() && !e.is_expired()); + self.set_stamp_exceptions(&exceptions); Device::rotate_refresh_tokens_by_user(&self.uuid, conn).await?; Ok(()) } @@ -321,32 +324,6 @@ impl User { }; } - /// Persists only `stamp_exception`, for callers holding a user read earlier in the request. - /// - /// A full `save` would write back every other column as it was read, undoing anything saved in - /// the meantime, such as the new keys of a key rotation made from another device. - pub async fn save_stamp_exceptions(&self, conn: &DbConn) -> EmptyResult { - let uuid = self.uuid.clone(); - let stamp_exception = self.stamp_exception.clone(); - conn.run(move |conn| { - diesel::update(users::table.filter(users::uuid.eq(uuid))) - .set(users::stamp_exception.eq(stamp_exception)) - .execute(conn) - .map_res("Error updating user stamp exceptions") - }) - .await - } - - /// Drops the stamp exceptions that don't match `keep`. Returns whether any were dropped. - pub fn retain_stamp_exceptions(&mut self, keep: impl Fn(&UserStampException) -> bool) -> bool { - let mut exceptions = self.stamp_exceptions(); - let before = exceptions.len(); - exceptions.retain(keep); - let changed = exceptions.len() != before; - self.set_stamp_exceptions(&exceptions); - changed - } - pub fn display_name(&self) -> &str { // default to email if name is empty if self.name.is_empty() { @@ -363,7 +340,7 @@ impl User { if !self.is_v2() { return None; } - UserSignatureKeyPair::find_active_by_user(&self.uuid, conn).await + UserSignatureKeyPair::find_by_user(&self.uuid, conn).await } pub async fn account_keys_json(&self, conn: &DbConn) -> Value { @@ -699,7 +676,18 @@ pub struct UserId(String); )] #[deref(forward)] #[from(forward)] -pub struct KeyId(String); +pub struct KeyId(#[serde(deserialize_with = "deserialize_key_id")] String); + +/// Rejects a key id from a request that isn't 16 bytes as lowercase hex, as upstream's `[KeyId]` does. +/// +/// Ref: +fn deserialize_key_id<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result { + let key_id = ::deserialize(deserializer)?; + if key_id.len() != 32 || !key_id.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'f')) { + return Err(serde::de::Error::custom("Key id must be a 32 character lowercase hex-encoded string.")); + } + Ok(key_id) +} impl SsoUser { pub async fn save(&self, conn: &DbConn) -> EmptyResult { diff --git a/src/db/models/user_signature_key_pair.rs b/src/db/models/user_signature_key_pair.rs index b7e58f3a..5af984bd 100644 --- a/src/db/models/user_signature_key_pair.rs +++ b/src/db/models/user_signature_key_pair.rs @@ -9,7 +9,6 @@ use crate::{ error::MapResult, util::get_uuid, }; -use macros::UuidFromParam; use super::UserId; @@ -47,28 +46,13 @@ pub enum SignatureAlgorithm { } impl SignatureAlgorithm { - pub fn from_str(algorithm: &str) -> Option { + pub fn parse(algorithm: &str) -> Option { match algorithm { "ed25519" => Some(Self::Ed25519), "mldsa44" => Some(Self::MlDsa44), _ => None, } } - - pub fn from_i32(algorithm: i32) -> Option { - match algorithm { - 0 => Some(Self::Ed25519), - 1 => Some(Self::MlDsa44), - _ => None, - } - } - - pub fn as_str(self) -> &'static str { - match self { - Self::Ed25519 => "ed25519", - Self::MlDsa44 => "mldsa44", - } - } } /// Local methods @@ -128,9 +112,8 @@ impl UserSignatureKeyPair { } } - /// The key pair currently in use by the user. There is at most one today, enforced by a unique - /// index on `user_uuid`. - pub async fn find_active_by_user(user_uuid: &UserId, conn: &DbConn) -> Option { + /// The user's key pair. There is at most one, enforced by a unique index on `user_uuid`. + pub async fn find_by_user(user_uuid: &UserId, conn: &DbConn) -> Option { conn.run(move |conn| { user_signature_key_pairs::table .filter(user_signature_key_pairs::user_uuid.eq(user_uuid)) @@ -150,20 +133,5 @@ impl UserSignatureKeyPair { } } -#[derive( - Clone, - Debug, - AsRef, - Deref, - DieselNewType, - Display, - From, - FromForm, - Hash, - PartialEq, - Eq, - Serialize, - Deserialize, - UuidFromParam, -)] +#[derive(Clone, Debug, AsRef, Deref, DieselNewType, Display, From, Hash, PartialEq, Eq, Serialize, Deserialize)] pub struct UserSignatureKeyPairId(String);