From 34ac16e9d77272a74c17eaaa90e79ca9d20f3af2 Mon Sep 17 00:00:00 2001
From: BlackDex <black.dex@gmail.com>
Date: Fri, 20 Jan 2023 15:43:45 +0100
Subject: [PATCH 1/2] Validate note sizes on key-rotation.
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
---
src/api/admin.rs | 6 +++---
src/api/core/accounts.rs | 22 +++++++++++++++++-----
src/api/notifications.rs | 10 ++++++++++
3 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/src/api/admin.rs b/src/api/admin.rs
index 6e0c2acf..f22d3bc2 100644
--- a/src/api/admin.rs
+++ b/src/api/admin.rs
@@ -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},
@@ -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
}
@@ -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
}
diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs
index 758d9028..6220c1b5 100644
--- a/src/api/core/accounts.rs
+++ b/src/api/core/accounts.rs
@@ -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
}
@@ -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, Some(headers.device.uuid)).await;
save_result
}
@@ -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
@@ -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
}
@@ -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
}
@@ -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
}
diff --git a/src/api/notifications.rs b/src/api/notifications.rs
index b51e1380..b4dc55e9 100644
--- a/src/api/notifications.rs
+++ b/src/api/notifications.rs
@@ -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![
From d121cce0d2dd1287eac6bb479736bb144bf59885 Mon Sep 17 00:00:00 2001
From: sirux88 <sirux88@gmail.com>
Date: Sat, 14 Jan 2023 10:16:03 +0100
Subject: [PATCH 2/2] include key into user.set_password
---
src/api/core/accounts.rs | 13 ++++++-------
src/api/core/emergency_access.rs | 5 ++---
src/db/models/user.rs | 5 +++--
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs
index 6220c1b5..0bf8b416 100644
--- a/src/api/core/accounts.rs
+++ b/src/api/core/accounts.rs
@@ -161,8 +161,7 @@ pub async fn _register(data: JsonUpcase<RegisterData>, mut conn: DbConn) -> Json
user.client_kdf_type = client_kdf_type;
}
- user.set_password(&data.MasterPasswordHash, None);
- user.akey = data.Key;
+ user.set_password(&data.MasterPasswordHash, &data.Key, None);
user.password_hint = password_hint;
// Add extra fields if present
@@ -318,9 +317,10 @@ async fn post_password(
user.set_password(
&data.NewMasterPasswordHash,
+ &data.Key,
Some(vec![String::from("post_rotatekey"), String::from("get_contacts"), String::from("get_public_keys")]),
);
- user.akey = data.Key;
+
let save_result = user.save(&mut conn).await;
// Prevent loging out the client where the user requested this endpoint from.
@@ -353,8 +353,7 @@ async fn post_kdf(data: JsonUpcase<ChangeKdfData>, headers: Headers, mut conn: D
user.client_kdf_iter = data.KdfIterations;
user.client_kdf_type = data.Kdf;
- user.set_password(&data.NewMasterPasswordHash, None);
- user.akey = data.Key;
+ user.set_password(&data.NewMasterPasswordHash, &data.Key, None);
let save_result = user.save(&mut conn).await;
nt.send_logout(&user, Some(headers.device.uuid)).await;
@@ -572,8 +571,8 @@ async fn post_email(
user.email_new = None;
user.email_new_token = None;
- user.set_password(&data.NewMasterPasswordHash, None);
- user.akey = data.Key;
+ user.set_password(&data.NewMasterPasswordHash, &data.Key, None);
+
let save_result = user.save(&mut conn).await;
nt.send_logout(&user, None).await;
diff --git a/src/api/core/emergency_access.rs b/src/api/core/emergency_access.rs
index f15c1b8e..d6c45d15 100644
--- a/src/api/core/emergency_access.rs
+++ b/src/api/core/emergency_access.rs
@@ -644,7 +644,7 @@ async fn password_emergency_access(
let data: EmergencyAccessPasswordData = data.into_inner().data;
let new_master_password_hash = &data.NewMasterPasswordHash;
- let key = data.Key;
+ let key = &data.Key;
let requesting_user = headers.user;
let emergency_access = match EmergencyAccess::find_by_uuid(&emer_id, &mut conn).await {
@@ -662,8 +662,7 @@ async fn password_emergency_access(
};
// change grantor_user password
- grantor_user.set_password(new_master_password_hash, None);
- grantor_user.akey = key;
+ grantor_user.set_password(new_master_password_hash, key, None);
grantor_user.save(&mut conn).await?;
// Disable TwoFactor providers since they will otherwise block logins
diff --git a/src/db/models/user.rs b/src/db/models/user.rs
index 611c4ebb..5a1bb4b5 100644
--- a/src/db/models/user.rs
+++ b/src/db/models/user.rs
@@ -147,17 +147,18 @@ impl User {
/// # Arguments
///
/// * `password` - A str which contains a hashed version of the users master password.
+ /// * `new_key` - A String which contains the new aKey value of the users master password.
/// * `allow_next_route` - A Option<Vec<String>> with the function names of the next allowed (rocket) routes.
/// These routes are able to use the previous stamp id for the next 2 minutes.
/// After these 2 minutes this stamp will expire.
///
- pub fn set_password(&mut self, password: &str, allow_next_route: Option<Vec<String>>) {
+ pub fn set_password(&mut self, password: &str, new_key: &str, allow_next_route: Option<Vec<String>>) {
self.password_hash = crypto::hash_password(password.as_bytes(), &self.salt, self.password_iterations as u32);
if let Some(route) = allow_next_route {
self.set_stamp_exception(route);
}
-
+ self.akey = String::from(new_key);
self.reset_security_stamp()
}