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 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_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 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") - } - 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, + 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 } - } 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 { + 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 { + let Some(mut type_data) = type_data_opt else { + err!("Data missing") + }; // Remove the 'Response' key from the base object. - data.as_object_mut().unwrap().remove("response"); + type_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()); + if type_data["uris"].is_array() { + type_data["uris"] = clean_cipher_data(type_data["uris"].clone()); } - data.to_string() - } else { - err!("Data missing") - }; + + // `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);