Skip to content

Commit 745287f

Browse files
quextenmzieniukbwdani-garciaHintonjlf0dev
authored
[PM-24683] Add updateKdf function (#383)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-24683 ## 📔 Objective Exposes functionality to update the KDF, with the new masterpassword unlock data, and masterpassword authentication data models. These can be directly passed to the server models. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+1:`) or similar for great changes - 📝 (`:memo:`) or ℹ️ (`:information_source:`) for notes or general info - ❓ (`:question:`) for questions - 🤔 (`:thinking:`) or 💭 (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - 🎨 (`:art:`) for suggestions / improvements - ❌ (`:x:`) or ⚠️ (`:warning:`) for more significant problems or concerns needing attention - 🌱 (`:seedling:`) or ♻️ (`:recycle:`) for future improvements or indications of technical debt - ⛏ (`:pick:`) for minor or nitpick changes --------- Co-authored-by: Maciej Zieniuk <mzieniuk@bitwarden.com> Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com> Co-authored-by: Oscar Hinton <Hinton@users.noreply.github.com> Co-authored-by: Jake Fink <jfink@bitwarden.com>
1 parent 72f4866 commit 745287f

File tree

4 files changed

+324
-25
lines changed

4 files changed

+324
-25
lines changed

crates/bitwarden-core/src/key_management/crypto.rs

Lines changed: 159 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ use crate::{
2424
client::{encryption_settings::EncryptionSettingsError, LoginMethod, UserLoginMethod},
2525
error::StatefulCryptoError,
2626
key_management::{
27-
non_generic_wrappers::PasswordProtectedKeyEnvelope, AsymmetricKeyId, SecurityState,
28-
SignedSecurityState, SigningKeyId, SymmetricKeyId,
27+
master_password::{MasterPasswordAuthenticationData, MasterPasswordUnlockData},
28+
non_generic_wrappers::PasswordProtectedKeyEnvelope,
29+
AsymmetricKeyId, SecurityState, SignedSecurityState, SigningKeyId, SymmetricKeyId,
2930
},
3031
Client, NotAuthenticatedError, OrganizationId, UserId, WrongPasswordError,
3132
};
@@ -39,6 +40,8 @@ pub enum CryptoClientError {
3940
NotAuthenticated(#[from] NotAuthenticatedError),
4041
#[error(transparent)]
4142
Crypto(#[from] bitwarden_crypto::CryptoError),
43+
#[error("Invalid KDF settings")]
44+
InvalidKdfSettings,
4245
#[error(transparent)]
4346
PasswordProtectedKeyEnvelope(#[from] PasswordProtectedKeyEnvelopeError),
4447
}
@@ -283,6 +286,64 @@ pub(super) async fn get_user_encryption_key(client: &Client) -> Result<B64, Cryp
283286
Ok(user_key.to_base64())
284287
}
285288

289+
/// Response from the `update_kdf` function
290+
#[derive(Serialize, Deserialize, Debug)]
291+
#[serde(rename_all = "camelCase", deny_unknown_fields)]
292+
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
293+
#[cfg_attr(feature = "wasm", derive(Tsify), tsify(into_wasm_abi, from_wasm_abi))]
294+
pub struct UpdateKdfResponse {
295+
/// The authentication data for the new KDF setting
296+
master_password_authentication_data: MasterPasswordAuthenticationData,
297+
/// The unlock data for the new KDF setting
298+
master_password_unlock_data: MasterPasswordUnlockData,
299+
/// The authentication data for the KDF setting prior to the change
300+
old_master_password_authentication_data: MasterPasswordAuthenticationData,
301+
}
302+
303+
pub(super) fn make_update_kdf(
304+
client: &Client,
305+
password: &str,
306+
new_kdf: &Kdf,
307+
) -> Result<UpdateKdfResponse, CryptoClientError> {
308+
let key_store = client.internal.get_key_store();
309+
let ctx = key_store.context();
310+
// FIXME: [PM-18099] Once MasterKey deals with KeyIds, this should be updated
311+
#[allow(deprecated)]
312+
let user_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
313+
314+
let login_method = client
315+
.internal
316+
.get_login_method()
317+
.ok_or(NotAuthenticatedError)?;
318+
let email = match login_method.as_ref() {
319+
LoginMethod::User(
320+
UserLoginMethod::Username { email, .. } | UserLoginMethod::ApiKey { email, .. },
321+
) => email,
322+
#[cfg(feature = "secrets")]
323+
LoginMethod::ServiceAccount(_) => return Err(NotAuthenticatedError)?,
324+
};
325+
326+
let authentication_data = MasterPasswordAuthenticationData::derive(password, new_kdf, email)
327+
.map_err(|_| CryptoClientError::InvalidKdfSettings)?;
328+
let unlock_data = MasterPasswordUnlockData::derive(password, new_kdf, email, user_key)
329+
.map_err(|_| CryptoClientError::InvalidKdfSettings)?;
330+
let old_authentication_data = MasterPasswordAuthenticationData::derive(
331+
password,
332+
&client
333+
.internal
334+
.get_kdf()
335+
.map_err(|_| NotAuthenticatedError)?,
336+
email,
337+
)
338+
.map_err(|_| CryptoClientError::InvalidKdfSettings)?;
339+
340+
Ok(UpdateKdfResponse {
341+
master_password_authentication_data: authentication_data,
342+
master_password_unlock_data: unlock_data,
343+
old_master_password_authentication_data: old_authentication_data,
344+
})
345+
}
346+
286347
/// Response from the `update_password` function
287348
#[derive(Serialize, Deserialize, Debug)]
288349
#[serde(rename_all = "camelCase", deny_unknown_fields)]
@@ -295,7 +356,7 @@ pub struct UpdatePasswordResponse {
295356
new_key: EncString,
296357
}
297358

298-
pub(super) fn update_password(
359+
pub(super) fn make_update_password(
299360
client: &Client,
300361
new_password: String,
301362
) -> Result<UpdatePasswordResponse, CryptoClientError> {
@@ -780,6 +841,100 @@ mod tests {
780841
"pgEBAlAmkP0QgfdMVbIujX55W/yNAycEgQIgBiFYIEM6JxBmjWQTruAm3s6BTaJy1q6BzQetMBacNeRJ0kxR";
781842
const TEST_VECTOR_SECURITY_STATE_V2: &str = "hFgepAEnAxg8BFAmkP0QgfdMVbIujX55W/yNOgABOH8CoFgkomhlbnRpdHlJZFBHOOw2BI9OQoNq+Vl1xZZKZ3ZlcnNpb24CWEAlchbJR0vmRfShG8On7Q2gknjkw4Dd6MYBLiH4u+/CmfQdmjNZdf6kozgW/6NXyKVNu8dAsKsin+xxXkDyVZoG";
782843

844+
#[tokio::test]
845+
async fn test_update_kdf() {
846+
let client = Client::new(None);
847+
848+
let priv_key: EncString = "2.kmLY8NJVuiKBFJtNd/ZFpA==|qOodlRXER+9ogCe3yOibRHmUcSNvjSKhdDuztLlucs10jLiNoVVVAc+9KfNErLSpx5wmUF1hBOJM8zwVPjgQTrmnNf/wuDpwiaCxNYb/0v4FygPy7ccAHK94xP1lfqq7U9+tv+/yiZSwgcT+xF0wFpoxQeNdNRFzPTuD9o4134n8bzacD9DV/WjcrXfRjbBCzzuUGj1e78+A7BWN7/5IWLz87KWk8G7O/W4+8PtEzlwkru6Wd1xO19GYU18oArCWCNoegSmcGn7w7NDEXlwD403oY8Oa7ylnbqGE28PVJx+HLPNIdSC6YKXeIOMnVs7Mctd/wXC93zGxAWD6ooTCzHSPVV50zKJmWIG2cVVUS7j35H3rGDtUHLI+ASXMEux9REZB8CdVOZMzp2wYeiOpggebJy6MKOZqPT1R3X0fqF2dHtRFPXrNsVr1Qt6bS9qTyO4ag1/BCvXF3P1uJEsI812BFAne3cYHy5bIOxuozPfipJrTb5WH35bxhElqwT3y/o/6JWOGg3HLDun31YmiZ2HScAsUAcEkA4hhoTNnqy4O2s3yVbCcR7jF7NLsbQc0MDTbnjxTdI4VnqUIn8s2c9hIJy/j80pmO9Bjxp+LQ9a2hUkfHgFhgHxZUVaeGVth8zG2kkgGdrp5VHhxMVFfvB26Ka6q6qE/UcS2lONSv+4T8niVRJz57qwctj8MNOkA3PTEfe/DP/LKMefke31YfT0xogHsLhDkx+mS8FCc01HReTjKLktk/Jh9mXwC5oKwueWWwlxI935ecn+3I2kAuOfMsgPLkoEBlwgiREC1pM7VVX1x8WmzIQVQTHd4iwnX96QewYckGRfNYWz/zwvWnjWlfcg8kRSe+68EHOGeRtC5r27fWLqRc0HNcjwpgHkI/b6czerCe8+07TWql4keJxJxhBYj3iOH7r9ZS8ck51XnOb8tGL1isimAJXodYGzakwktqHAD7MZhS+P02O+6jrg7d+yPC2ZCuS/3TOplYOCHQIhnZtR87PXTUwr83zfOwAwCyv6KP84JUQ45+DItrXLap7nOVZKQ5QxYIlbThAO6eima6Zu5XHfqGPMNWv0bLf5+vAjIa5np5DJrSwz9no/hj6CUh0iyI+SJq4RGI60lKtypMvF6MR3nHLEHOycRUQbZIyTHWl4QQLdHzuwN9lv10ouTEvNr6sFflAX2yb6w3hlCo7oBytH3rJekjb3IIOzBpeTPIejxzVlh0N9OT5MZdh4sNKYHUoWJ8mnfjdM+L4j5Q2Kgk/XiGDgEebkUxiEOQUdVpePF5uSCE+TPav/9FIRGXGiFn6NJMaU7aBsDTFBLloffFLYDpd8/bTwoSvifkj7buwLYM+h/qcnfdy5FWau1cKav+Blq/ZC0qBpo658RTC8ZtseAFDgXoQZuksM10hpP9bzD04Bx30xTGX81QbaSTNwSEEVrOtIhbDrj9OI43KH4O6zLzK+t30QxAv5zjk10RZ4+5SAdYndIlld9Y62opCfPDzRy3ubdve4ZEchpIKWTQvIxq3T5ogOhGaWBVYnkMtM2GVqvWV//46gET5SH/MdcwhACUcZ9kCpMnWH9CyyUwYvTT3UlNyV+DlS27LMPvaw7tx7qa+GfNCoCBd8S4esZpQYK/WReiS8=|pc7qpD42wxyXemdNPuwxbh8iIaryrBPu8f/DGwYdHTw=".parse().unwrap();
849+
850+
let kdf = Kdf::PBKDF2 {
851+
iterations: 100_000.try_into().unwrap(),
852+
};
853+
854+
initialize_user_crypto(
855+
& client,
856+
InitUserCryptoRequest {
857+
user_id: Some(UserId::new_v4()),
858+
kdf_params: kdf.clone(),
859+
email: "test@bitwarden.com".into(),
860+
private_key: priv_key.to_owned(),
861+
signing_key: None,
862+
security_state: None,
863+
method: InitUserCryptoMethod::Password {
864+
password: "asdfasdfasdf".into(),
865+
user_key: "2.u2HDQ/nH2J7f5tYHctZx6Q==|NnUKODz8TPycWJA5svexe1wJIz2VexvLbZh2RDfhj5VI3wP8ZkR0Vicvdv7oJRyLI1GyaZDBCf9CTBunRTYUk39DbZl42Rb+Xmzds02EQhc=|rwuo5wgqvTJf3rgwOUfabUyzqhguMYb3sGBjOYqjevc=".parse().unwrap(),
866+
},
867+
},
868+
)
869+
.await
870+
.unwrap();
871+
872+
let new_kdf = Kdf::PBKDF2 {
873+
iterations: 600_000.try_into().unwrap(),
874+
};
875+
let new_kdf_response = make_update_kdf(&client, "123412341234", &new_kdf).unwrap();
876+
877+
let client2 = Client::new(None);
878+
879+
initialize_user_crypto(
880+
&client2,
881+
InitUserCryptoRequest {
882+
user_id: Some(UserId::new_v4()),
883+
kdf_params: new_kdf.clone(),
884+
email: "test@bitwarden.com".into(),
885+
private_key: priv_key.to_owned(),
886+
signing_key: None,
887+
security_state: None,
888+
method: InitUserCryptoMethod::Password {
889+
password: "123412341234".into(),
890+
user_key: new_kdf_response
891+
.master_password_unlock_data
892+
.master_key_wrapped_user_key,
893+
},
894+
},
895+
)
896+
.await
897+
.unwrap();
898+
899+
let new_hash = client2
900+
.kdf()
901+
.hash_password(
902+
"test@bitwarden.com".into(),
903+
"123412341234".into(),
904+
new_kdf.clone(),
905+
bitwarden_crypto::HashPurpose::ServerAuthorization,
906+
)
907+
.await
908+
.unwrap();
909+
910+
assert_eq!(
911+
new_hash,
912+
new_kdf_response
913+
.master_password_authentication_data
914+
.master_password_authentication_hash
915+
);
916+
917+
let client_key = {
918+
let key_store = client.internal.get_key_store();
919+
let ctx = key_store.context();
920+
#[allow(deprecated)]
921+
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
922+
.unwrap()
923+
.to_base64()
924+
};
925+
926+
let client2_key = {
927+
let key_store = client2.internal.get_key_store();
928+
let ctx = key_store.context();
929+
#[allow(deprecated)]
930+
ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)
931+
.unwrap()
932+
.to_base64()
933+
};
934+
935+
assert_eq!(client_key, client2_key);
936+
}
937+
783938
#[tokio::test]
784939
async fn test_update_password() {
785940
let client = Client::new(None);
@@ -808,7 +963,7 @@ mod tests {
808963
.await
809964
.unwrap();
810965

811-
let new_password_response = update_password(&client, "123412341234".into()).unwrap();
966+
let new_password_response = make_update_password(&client, "123412341234".into()).unwrap();
812967

813968
let client2 = Client::new(None);
814969

crates/bitwarden-core/src/key_management/crypto_client.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use bitwarden_crypto::{CryptoError, Decryptable};
1+
use bitwarden_crypto::{CryptoError, Decryptable, Kdf};
22
#[cfg(feature = "internal")]
33
use bitwarden_crypto::{EncString, UnsignedSharedKey};
44
use bitwarden_encoding::B64;
@@ -16,17 +16,18 @@ use crate::key_management::PasswordProtectedKeyEnvelope;
1616
use crate::key_management::{
1717
crypto::{
1818
derive_pin_key, derive_pin_user_key, enroll_admin_password_reset, get_user_encryption_key,
19-
initialize_org_crypto, initialize_user_crypto, update_password, DerivePinKeyResponse,
20-
InitOrgCryptoRequest, InitUserCryptoRequest, UpdatePasswordResponse,
19+
initialize_org_crypto, initialize_user_crypto, DerivePinKeyResponse, InitOrgCryptoRequest,
20+
InitUserCryptoRequest, UpdatePasswordResponse,
2121
},
2222
SymmetricKeyId,
2323
};
2424
use crate::{
2525
client::encryption_settings::EncryptionSettingsError,
2626
error::StatefulCryptoError,
2727
key_management::crypto::{
28-
enroll_pin, get_v2_rotated_account_keys, make_v2_keys_for_v1_user, CryptoClientError,
29-
EnrollPinResponse, UserCryptoV2KeysResponse,
28+
enroll_pin, get_v2_rotated_account_keys, make_update_kdf, make_update_password,
29+
make_v2_keys_for_v1_user, CryptoClientError, EnrollPinResponse, UpdateKdfResponse,
30+
UserCryptoV2KeysResponse,
3031
},
3132
Client,
3233
};
@@ -87,6 +88,17 @@ impl CryptoClient {
8788
get_v2_rotated_account_keys(&self.client)
8889
}
8990

91+
/// Create the data necessary to update the user's kdf settings. The user's encryption key is
92+
/// re-encrypted for the password under the new kdf settings. This returns the re-encrypted
93+
/// user key and the new password hash but does not update sdk state.
94+
pub fn make_update_kdf(
95+
&self,
96+
password: String,
97+
kdf: Kdf,
98+
) -> Result<UpdateKdfResponse, CryptoClientError> {
99+
make_update_kdf(&self.client, &password, &kdf)
100+
}
101+
90102
/// Protects the current user key with the provided PIN. The result can be stored and later
91103
/// used to initialize another client instance by using the PIN and the PIN key with
92104
/// `initialize_user_crypto`.
@@ -134,13 +146,14 @@ impl CryptoClient {
134146
get_user_encryption_key(&self.client).await
135147
}
136148

137-
/// Update the user's password, which will re-encrypt the user's encryption key with the new
138-
/// password. This returns the new encrypted user key and the new password hash.
139-
pub fn update_password(
149+
/// Create the data necessary to update the user's password. The user's encryption key is
150+
/// re-encrypted with the new password. This returns the new encrypted user key and the new
151+
/// password hash but does not update sdk state.
152+
pub fn make_update_password(
140153
&self,
141154
new_password: String,
142155
) -> Result<UpdatePasswordResponse, CryptoClientError> {
143-
update_password(&self.client, new_password)
156+
make_update_password(&self.client, new_password)
144157
}
145158

146159
/// Generates a PIN protected user key from the provided PIN. The result can be stored and later

0 commit comments

Comments
 (0)