Skip to content

Commit

Permalink
Update Key Rotation web-vault v2024.3.x (#4446)
Browse files Browse the repository at this point in the history
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: dani-garcia/bw_web_builds#157
  • Loading branch information
BlackDex authored Apr 6, 2024
1 parent e42a37c commit e1a8df9
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 41 deletions.
110 changes: 89 additions & 21 deletions src/api/core/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,24 +438,46 @@ async fn post_kdf(data: JsonUpcase<ChangeKdfData>, 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<String>,
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<CipherData>,
Folders: Vec<UpdateFolderData>,
Sends: Vec<SendData>,
EmergencyAccessKeys: Vec<UpdateEmergencyAccessData>,
ResetPasswordKeys: Vec<UpdateResetPasswordData>,
Key: String,
PrivateKey: String,
MasterPasswordHash: String,
PrivateKey: String,
}

#[post("/accounts/key", data = "<data>")]
async fn post_rotatekey(data: JsonUpcase<KeyData>, 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) {
Expand All @@ -472,37 +494,83 @@ async fn post_rotatekey(data: JsonUpcase<KeyData>, 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
Expand Down
2 changes: 1 addition & 1 deletion src/api/core/ciphers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub struct CipherData {
// Folder id is not included in import
FolderId: Option<String>,
// TODO: Some of these might appear all the time, no need for Option
OrganizationId: Option<String>,
pub OrganizationId: Option<String>,

Key: Option<String>,

Expand Down
7 changes: 5 additions & 2 deletions src/api/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,17 @@ fn version() -> Json<&'static str> {
#[get("/config")]
fn config() -> Json<Value> {
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",
Expand Down
45 changes: 28 additions & 17 deletions src/api/core/sends.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
Expand All @@ -65,6 +65,9 @@ struct SendData {
Text: Option<Value>,
File: Option<Value>,
FileLength: Option<NumberOrString>,

// Used for key rotations
pub Id: Option<String>,
}

/// Enforces the `Disable Send` policy. A non-owner/admin user belonging to
Expand Down Expand Up @@ -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")
}
Expand All @@ -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 {
Expand All @@ -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();
Expand All @@ -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/<id>")]
Expand Down

0 comments on commit e1a8df9

Please sign in to comment.