Skip to content

Commit

Permalink
Validate note sizes on key-rotation.
Browse files Browse the repository at this point in the history
We also need to validate the note sizes on key-rotation.
If we do not validate them before we store them, that could lead to a
partial or total loss of the password vault. Validating these
restrictions before actually processing them to store/replace the
existing ciphers should prevent this.

There was also a small bug when using web-sockets. The client which is
triggering the password/key-rotation change should not be forced to
logout via a web-socket request. That is something the client will
handle it self. Refactored the logout notification to either send the
device uuid or not on specific actions.

Fixes #3152
  • Loading branch information
BlackDex committed Jan 20, 2023
1 parent 9b7e86e commit a112d9d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 8 deletions.
6 changes: 3 additions & 3 deletions src/api/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rocket::{
};

use crate::{
api::{core::log_event, ApiResult, EmptyResult, JsonResult, Notify, NumberOrString, UpdateType},
api::{core::log_event, ApiResult, EmptyResult, JsonResult, Notify, NumberOrString},
auth::{decode_admin, encode_jwt, generate_admin_claims, ClientIp},
config::ConfigBuilder,
db::{backup_database, get_sql_server_version, models::*, DbConn, DbConnType},
Expand Down Expand Up @@ -372,7 +372,7 @@ async fn deauth_user(uuid: String, _token: AdminToken, mut conn: DbConn, nt: Not

let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
nt.send_logout(&user, None).await;

save_result
}
Expand All @@ -386,7 +386,7 @@ async fn disable_user(uuid: String, _token: AdminToken, mut conn: DbConn, nt: No

let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
nt.send_logout(&user, None).await;

save_result
}
Expand Down
22 changes: 17 additions & 5 deletions src/api/core/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,10 @@ async fn post_password(
user.akey = data.Key;
let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
// Prevent loging 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.uuid)).await;

save_result
}
Expand Down Expand Up @@ -354,7 +357,7 @@ async fn post_kdf(data: JsonUpcase<ChangeKdfData>, headers: Headers, mut conn: D
user.akey = data.Key;
let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
nt.send_logout(&user, None).await;

save_result
}
Expand Down Expand Up @@ -392,6 +395,12 @@ async fn post_rotatekey(
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_notes(&data.Ciphers)?;

let user_uuid = &headers.user.uuid;

// Update folder data
Expand Down Expand Up @@ -438,7 +447,10 @@ async fn post_rotatekey(

let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
// Prevent loging 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.uuid)).await;

save_result
}
Expand All @@ -461,7 +473,7 @@ async fn post_sstamp(
user.reset_security_stamp();
let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
nt.send_logout(&user, None).await;

save_result
}
Expand Down Expand Up @@ -564,7 +576,7 @@ async fn post_email(
user.akey = data.Key;
let save_result = user.save(&mut conn).await;

nt.send_user_update(UpdateType::LogOut, &user).await;
nt.send_logout(&user, None).await;

save_result
}
Expand Down
10 changes: 10 additions & 0 deletions src/api/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,16 @@ impl WebSocketUsers {
self.send_update(&user.uuid, &data).await;
}

pub async fn send_logout(&self, user: &User, acting_device_uuid: Option<String>) {
let data = create_update(
vec![("UserId".into(), user.uuid.clone().into()), ("Date".into(), serialize_date(user.updated_at))],
UpdateType::LogOut,
acting_device_uuid,
);

self.send_update(&user.uuid, &data).await;
}

pub async fn send_folder_update(&self, ut: UpdateType, folder: &Folder, acting_device_uuid: &String) {
let data = create_update(
vec![
Expand Down

0 comments on commit a112d9d

Please sign in to comment.