Skip to content

Commit a0198d8

Browse files
authored
fix account key rotation (#6105)
1 parent dfad931 commit a0198d8

File tree

1 file changed

+81
-22
lines changed

1 file changed

+81
-22
lines changed

src/api/core/accounts.rs

Lines changed: 81 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -556,14 +556,45 @@ use super::sends::{update_send_from_data, SendData};
556556
#[derive(Deserialize)]
557557
#[serde(rename_all = "camelCase")]
558558
struct KeyData {
559+
account_unlock_data: RotateAccountUnlockData,
560+
account_keys: RotateAccountKeys,
561+
account_data: RotateAccountData,
562+
old_master_key_authentication_hash: String,
563+
}
564+
565+
#[derive(Deserialize)]
566+
#[serde(rename_all = "camelCase")]
567+
struct RotateAccountUnlockData {
568+
emergency_access_unlock_data: Vec<UpdateEmergencyAccessData>,
569+
master_password_unlock_data: MasterPasswordUnlockData,
570+
organization_account_recovery_unlock_data: Vec<UpdateResetPasswordData>,
571+
}
572+
573+
#[derive(Deserialize)]
574+
#[serde(rename_all = "camelCase")]
575+
struct MasterPasswordUnlockData {
576+
kdf_type: i32,
577+
kdf_iterations: i32,
578+
kdf_parallelism: Option<i32>,
579+
kdf_memory: Option<i32>,
580+
email: String,
581+
master_key_authentication_hash: String,
582+
master_key_encrypted_user_key: String,
583+
}
584+
585+
#[derive(Deserialize)]
586+
#[serde(rename_all = "camelCase")]
587+
struct RotateAccountKeys {
588+
user_key_encrypted_account_private_key: String,
589+
account_public_key: String,
590+
}
591+
592+
#[derive(Deserialize)]
593+
#[serde(rename_all = "camelCase")]
594+
struct RotateAccountData {
559595
ciphers: Vec<CipherData>,
560596
folders: Vec<UpdateFolderData>,
561597
sends: Vec<SendData>,
562-
emergency_access_keys: Vec<UpdateEmergencyAccessData>,
563-
reset_password_keys: Vec<UpdateResetPasswordData>,
564-
key: String,
565-
master_password_hash: String,
566-
private_key: String,
567598
}
568599

569600
fn validate_keydata(
@@ -573,10 +604,24 @@ fn validate_keydata(
573604
existing_emergency_access: &[EmergencyAccess],
574605
existing_memberships: &[Membership],
575606
existing_sends: &[Send],
607+
user: &User,
576608
) -> EmptyResult {
609+
if user.client_kdf_type != data.account_unlock_data.master_password_unlock_data.kdf_type
610+
|| user.client_kdf_iter != data.account_unlock_data.master_password_unlock_data.kdf_iterations
611+
|| user.client_kdf_memory != data.account_unlock_data.master_password_unlock_data.kdf_memory
612+
|| user.client_kdf_parallelism != data.account_unlock_data.master_password_unlock_data.kdf_parallelism
613+
|| user.email != data.account_unlock_data.master_password_unlock_data.email
614+
{
615+
err!("Changing the kdf variant or email is not supported during key rotation");
616+
}
617+
if user.public_key.as_ref() != Some(&data.account_keys.account_public_key) {
618+
err!("Changing the asymmetric keypair is not possible during key rotation")
619+
}
620+
577621
// Check that we're correctly rotating all the user's ciphers
578622
let existing_cipher_ids = existing_ciphers.iter().map(|c| &c.uuid).collect::<HashSet<&CipherId>>();
579623
let provided_cipher_ids = data
624+
.account_data
580625
.ciphers
581626
.iter()
582627
.filter(|c| c.organization_id.is_none())
@@ -588,53 +633,62 @@ fn validate_keydata(
588633

589634
// Check that we're correctly rotating all the user's folders
590635
let existing_folder_ids = existing_folders.iter().map(|f| &f.uuid).collect::<HashSet<&FolderId>>();
591-
let provided_folder_ids = data.folders.iter().filter_map(|f| f.id.as_ref()).collect::<HashSet<&FolderId>>();
636+
let provided_folder_ids =
637+
data.account_data.folders.iter().filter_map(|f| f.id.as_ref()).collect::<HashSet<&FolderId>>();
592638
if !provided_folder_ids.is_superset(&existing_folder_ids) {
593639
err!("All existing folders must be included in the rotation")
594640
}
595641

596642
// Check that we're correctly rotating all the user's emergency access keys
597643
let existing_emergency_access_ids =
598644
existing_emergency_access.iter().map(|ea| &ea.uuid).collect::<HashSet<&EmergencyAccessId>>();
599-
let provided_emergency_access_ids =
600-
data.emergency_access_keys.iter().map(|ea| &ea.id).collect::<HashSet<&EmergencyAccessId>>();
645+
let provided_emergency_access_ids = data
646+
.account_unlock_data
647+
.emergency_access_unlock_data
648+
.iter()
649+
.map(|ea| &ea.id)
650+
.collect::<HashSet<&EmergencyAccessId>>();
601651
if !provided_emergency_access_ids.is_superset(&existing_emergency_access_ids) {
602652
err!("All existing emergency access keys must be included in the rotation")
603653
}
604654

605655
// Check that we're correctly rotating all the user's reset password keys
606656
let existing_reset_password_ids =
607657
existing_memberships.iter().map(|m| &m.org_uuid).collect::<HashSet<&OrganizationId>>();
608-
let provided_reset_password_ids =
609-
data.reset_password_keys.iter().map(|rp| &rp.organization_id).collect::<HashSet<&OrganizationId>>();
658+
let provided_reset_password_ids = data
659+
.account_unlock_data
660+
.organization_account_recovery_unlock_data
661+
.iter()
662+
.map(|rp| &rp.organization_id)
663+
.collect::<HashSet<&OrganizationId>>();
610664
if !provided_reset_password_ids.is_superset(&existing_reset_password_ids) {
611665
err!("All existing reset password keys must be included in the rotation")
612666
}
613667

614668
// Check that we're correctly rotating all the user's sends
615669
let existing_send_ids = existing_sends.iter().map(|s| &s.uuid).collect::<HashSet<&SendId>>();
616-
let provided_send_ids = data.sends.iter().filter_map(|s| s.id.as_ref()).collect::<HashSet<&SendId>>();
670+
let provided_send_ids = data.account_data.sends.iter().filter_map(|s| s.id.as_ref()).collect::<HashSet<&SendId>>();
617671
if !provided_send_ids.is_superset(&existing_send_ids) {
618672
err!("All existing sends must be included in the rotation")
619673
}
620674

621675
Ok(())
622676
}
623677

624-
#[post("/accounts/key", data = "<data>")]
678+
#[post("/accounts/key-management/rotate-user-account-keys", data = "<data>")]
625679
async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn, nt: Notify<'_>) -> EmptyResult {
626680
// TODO: See if we can wrap everything within a SQL Transaction. If something fails it should revert everything.
627681
let data: KeyData = data.into_inner();
628682

629-
if !headers.user.check_valid_password(&data.master_password_hash) {
683+
if !headers.user.check_valid_password(&data.old_master_key_authentication_hash) {
630684
err!("Invalid password")
631685
}
632686

633687
// Validate the import before continuing
634688
// Bitwarden does not process the import if there is one item invalid.
635689
// Since we check for the size of the encrypted note length, we need to do that here to pre-validate it.
636690
// TODO: See if we can optimize the whole cipher adding/importing and prevent duplicate code and checks.
637-
Cipher::validate_cipher_data(&data.ciphers)?;
691+
Cipher::validate_cipher_data(&data.account_data.ciphers)?;
638692

639693
let user_id = &headers.user.uuid;
640694

@@ -655,10 +709,11 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
655709
&existing_emergency_access,
656710
&existing_memberships,
657711
&existing_sends,
712+
&headers.user,
658713
)?;
659714

660715
// Update folder data
661-
for folder_data in data.folders {
716+
for folder_data in data.account_data.folders {
662717
// Skip `null` folder id entries.
663718
// See: https://github.com/bitwarden/clients/issues/8453
664719
if let Some(folder_id) = folder_data.id {
@@ -672,7 +727,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
672727
}
673728

674729
// Update emergency access data
675-
for emergency_access_data in data.emergency_access_keys {
730+
for emergency_access_data in data.account_unlock_data.emergency_access_unlock_data {
676731
let Some(saved_emergency_access) =
677732
existing_emergency_access.iter_mut().find(|ea| ea.uuid == emergency_access_data.id)
678733
else {
@@ -684,7 +739,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
684739
}
685740

686741
// Update reset password data
687-
for reset_password_data in data.reset_password_keys {
742+
for reset_password_data in data.account_unlock_data.organization_account_recovery_unlock_data {
688743
let Some(membership) =
689744
existing_memberships.iter_mut().find(|m| m.org_uuid == reset_password_data.organization_id)
690745
else {
@@ -696,7 +751,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
696751
}
697752

698753
// Update send data
699-
for send_data in data.sends {
754+
for send_data in data.account_data.sends {
700755
let Some(send) = existing_sends.iter_mut().find(|s| &s.uuid == send_data.id.as_ref().unwrap()) else {
701756
err!("Send doesn't exist")
702757
};
@@ -707,7 +762,7 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
707762
// Update cipher data
708763
use super::ciphers::update_cipher_from_data;
709764

710-
for cipher_data in data.ciphers {
765+
for cipher_data in data.account_data.ciphers {
711766
if cipher_data.organization_id.is_none() {
712767
let Some(saved_cipher) = existing_ciphers.iter_mut().find(|c| &c.uuid == cipher_data.id.as_ref().unwrap())
713768
else {
@@ -724,9 +779,13 @@ async fn post_rotatekey(data: Json<KeyData>, headers: Headers, mut conn: DbConn,
724779
// Update user data
725780
let mut user = headers.user;
726781

727-
user.akey = data.key;
728-
user.private_key = Some(data.private_key);
729-
user.reset_security_stamp();
782+
user.private_key = Some(data.account_keys.user_key_encrypted_account_private_key);
783+
user.set_password(
784+
&data.account_unlock_data.master_password_unlock_data.master_key_authentication_hash,
785+
Some(data.account_unlock_data.master_password_unlock_data.master_key_encrypted_user_key),
786+
true,
787+
None,
788+
);
730789

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

0 commit comments

Comments
 (0)