From e1a8df96dbadfbf5ad36ce9aa2f31f34396166c2 Mon Sep 17 00:00:00 2001 From: Mathijs van Veluw Date: Sat, 6 Apr 2024 14:42:53 +0200 Subject: [PATCH] Update Key Rotation web-vault v2024.3.x (#4446) Key rotation was changed since 2024.1.x. Multiple other items were added to be rotated like password-reset and emergency-access data to be part of just one POST instead of having multiple. See: https://github.com/dani-garcia/bw_web_builds/pull/157 --- src/api/core/accounts.rs | 110 +++++++++++++++++++++++++++++++-------- src/api/core/ciphers.rs | 2 +- src/api/core/mod.rs | 7 ++- src/api/core/sends.rs | 45 ++++++++++------ 4 files changed, 123 insertions(+), 41 deletions(-) diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index da25d488..ff74c84f 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -438,24 +438,46 @@ async fn post_kdf(data: JsonUpcase, headers: Headers, mut conn: D #[derive(Deserialize)] #[allow(non_snake_case)] struct UpdateFolderData { - Id: String, + // There is a bug in 2024.3.x which adds a `null` item. + // To bypass this we allow a Option here, but skip it during the updates + // See: https://github.com/bitwarden/clients/issues/8453 + Id: Option, Name: String, } +#[derive(Deserialize)] +#[allow(non_snake_case)] +struct UpdateEmergencyAccessData { + Id: String, + KeyEncrypted: String, +} + +#[derive(Deserialize)] +#[allow(non_snake_case)] +struct UpdateResetPasswordData { + OrganizationId: String, + ResetPasswordKey: String, +} + use super::ciphers::CipherData; +use super::sends::{update_send_from_data, SendData}; #[derive(Deserialize)] #[allow(non_snake_case)] struct KeyData { Ciphers: Vec, Folders: Vec, + Sends: Vec, + EmergencyAccessKeys: Vec, + ResetPasswordKeys: Vec, Key: String, - PrivateKey: String, MasterPasswordHash: String, + PrivateKey: String, } #[post("/accounts/key", data = "")] async fn post_rotatekey(data: JsonUpcase, headers: Headers, mut 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().data; if !headers.user.check_valid_password(&data.MasterPasswordHash) { @@ -472,37 +494,83 @@ async fn post_rotatekey(data: JsonUpcase, headers: Headers, mut conn: D // Update folder data for folder_data in data.Folders { - let mut saved_folder = match Folder::find_by_uuid(&folder_data.Id, &mut conn).await { - Some(folder) => folder, - None => err!("Folder doesn't exist"), + // Skip `null` folder id entries. + // See: https://github.com/bitwarden/clients/issues/8453 + if let Some(folder_id) = folder_data.Id { + let mut saved_folder = match Folder::find_by_uuid(&folder_id, &mut conn).await { + Some(folder) => folder, + None => err!("Folder doesn't exist"), + }; + + if &saved_folder.user_uuid != user_uuid { + err!("The folder is not owned by the user") + } + + saved_folder.name = folder_data.Name; + saved_folder.save(&mut conn).await? + } + } + + // Update emergency access data + for emergency_access_data in data.EmergencyAccessKeys { + let mut saved_emergency_access = match EmergencyAccess::find_by_uuid(&emergency_access_data.Id, &mut conn).await + { + Some(emergency_access) => emergency_access, + None => err!("Emergency access doesn't exist"), }; - if &saved_folder.user_uuid != user_uuid { - err!("The folder is not owned by the user") + if &saved_emergency_access.grantor_uuid != user_uuid { + err!("The emergency access is not owned by the user") } - saved_folder.name = folder_data.Name; - saved_folder.save(&mut conn).await? + saved_emergency_access.key_encrypted = Some(emergency_access_data.KeyEncrypted); + saved_emergency_access.save(&mut conn).await? + } + + // Update reset password data + for reset_password_data in data.ResetPasswordKeys { + let mut user_org = + match UserOrganization::find_by_user_and_org(user_uuid, &reset_password_data.OrganizationId, &mut conn) + .await + { + Some(reset_password) => reset_password, + None => err!("Reset password doesn't exist"), + }; + + user_org.reset_password_key = Some(reset_password_data.ResetPasswordKey); + user_org.save(&mut conn).await? + } + + // Update send data + for send_data in data.Sends { + let mut send = match Send::find_by_uuid(send_data.Id.as_ref().unwrap(), &mut conn).await { + Some(send) => send, + None => err!("Send doesn't exist"), + }; + + update_send_from_data(&mut send, send_data, &headers, &mut conn, &nt, UpdateType::None).await?; } // Update cipher data use super::ciphers::update_cipher_from_data; for cipher_data in data.Ciphers { - let mut saved_cipher = match Cipher::find_by_uuid(cipher_data.Id.as_ref().unwrap(), &mut conn).await { - Some(cipher) => cipher, - None => err!("Cipher doesn't exist"), - }; + if cipher_data.OrganizationId.is_none() { + let mut saved_cipher = match Cipher::find_by_uuid(cipher_data.Id.as_ref().unwrap(), &mut conn).await { + Some(cipher) => cipher, + None => err!("Cipher doesn't exist"), + }; - if saved_cipher.user_uuid.as_ref().unwrap() != user_uuid { - err!("The cipher is not owned by the user") + if saved_cipher.user_uuid.as_ref().unwrap() != user_uuid { + err!("The cipher is not owned by the user") + } + + // 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. + update_cipher_from_data(&mut saved_cipher, cipher_data, &headers, false, &mut conn, &nt, UpdateType::None) + .await? } - - // 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. - update_cipher_from_data(&mut saved_cipher, cipher_data, &headers, false, &mut conn, &nt, UpdateType::None) - .await? } // Update user data diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index b3dca3b6..51a9589d 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -205,7 +205,7 @@ pub struct CipherData { // Folder id is not included in import FolderId: Option, // TODO: Some of these might appear all the time, no need for Option - OrganizationId: Option, + pub OrganizationId: Option, Key: Option, diff --git a/src/api/core/mod.rs b/src/api/core/mod.rs index 7712ea82..1d31b27c 100644 --- a/src/api/core/mod.rs +++ b/src/api/core/mod.rs @@ -191,14 +191,17 @@ fn version() -> Json<&'static str> { #[get("/config")] fn config() -> Json { let domain = crate::CONFIG.domain(); - let feature_states = parse_experimental_client_feature_flags(&crate::CONFIG.experimental_client_feature_flags()); + let mut feature_states = + parse_experimental_client_feature_flags(&crate::CONFIG.experimental_client_feature_flags()); + // Force the new key rotation feature + feature_states.insert("key-rotation-improvements".to_string(), true); Json(json!({ // Note: The clients use this version to handle backwards compatibility concerns // This means they expect a version that closely matches the Bitwarden server version // We should make sure that we keep this updated when we support the new server features // Version history: // - Individual cipher key encryption: 2023.9.1 - "version": "2023.9.1", + "version": "2024.2.0", "gitHash": option_env!("GIT_REV"), "server": { "name": "Vaultwarden", diff --git a/src/api/core/sends.rs b/src/api/core/sends.rs index 6842d398..338510c6 100644 --- a/src/api/core/sends.rs +++ b/src/api/core/sends.rs @@ -49,7 +49,7 @@ pub async fn purge_sends(pool: DbPool) { #[derive(Deserialize)] #[allow(non_snake_case)] -struct SendData { +pub struct SendData { Type: i32, Key: String, Password: Option, @@ -65,6 +65,9 @@ struct SendData { Text: Option, File: Option, FileLength: Option, + + // Used for key rotations + pub Id: Option, } /// Enforces the `Disable Send` policy. A non-owner/admin user belonging to @@ -549,6 +552,19 @@ async fn put_send( None => err!("Send not found"), }; + update_send_from_data(&mut send, data, &headers, &mut conn, &nt, UpdateType::SyncSendUpdate).await?; + + Ok(Json(send.to_json())) +} + +pub async fn update_send_from_data( + send: &mut Send, + data: SendData, + headers: &Headers, + conn: &mut DbConn, + nt: &Notify<'_>, + ut: UpdateType, +) -> EmptyResult { if send.user_uuid.as_ref() != Some(&headers.user.uuid) { err!("Send is not owned by user") } @@ -557,6 +573,12 @@ async fn put_send( err!("Sends can't change type") } + if data.DeletionDate > Utc::now() + TimeDelta::try_days(31).unwrap() { + err!( + "You cannot have a Send with a deletion date that far into the future. Adjust the Deletion Date to a value less than 31 days from now and try again." + ); + } + // When updating a file Send, we receive nulls in the File field, as it's immutable, // so we only need to update the data field in the Text case if data.Type == SendType::Text as i32 { @@ -569,11 +591,6 @@ async fn put_send( send.data = data_str; } - if data.DeletionDate > Utc::now() + TimeDelta::try_days(31).unwrap() { - err!( - "You cannot have a Send with a deletion date that far into the future. Adjust the Deletion Date to a value less than 31 days from now and try again." - ); - } send.name = data.Name; send.akey = data.Key; send.deletion_date = data.DeletionDate.naive_utc(); @@ -591,17 +608,11 @@ async fn put_send( send.set_password(Some(&password)); } - send.save(&mut conn).await?; - nt.send_send_update( - UpdateType::SyncSendUpdate, - &send, - &send.update_users_revision(&mut conn).await, - &headers.device.uuid, - &mut conn, - ) - .await; - - Ok(Json(send.to_json())) + send.save(conn).await?; + if ut != UpdateType::None { + nt.send_send_update(ut, send, &send.update_users_revision(conn).await, &headers.device.uuid, conn).await; + } + Ok(()) } #[delete("/sends/")]