Skip to content

Commit ae9b9f1

Browse files
quextenMGibson1HintonThomas-Avery
authored
[PM-24263] Pin protected key envelope unlock (#372)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-24263 ## 📔 Objective Adds enrollment functionality for Password(PIN)Protected user key envelope. Both crypto initialization via the init request, *and* a function exposing the raw key material are provided. The latter is required since unlock is not yet done via the init methods on WASM/`clients`. ## ⏰ 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: Matt Gibson <[email protected]> Co-authored-by: Oscar Hinton <[email protected]> Co-authored-by: Thomas Avery <[email protected]>
1 parent bc66e3d commit ae9b9f1

File tree

6 files changed

+257
-16
lines changed

6 files changed

+257
-16
lines changed

crates/bitwarden-core/src/client/encryption_settings.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ pub enum EncryptionSettingsError {
4545

4646
#[error(transparent)]
4747
UserIdAlreadySetError(#[from] UserIdAlreadySetError),
48+
49+
#[error("Wrong Pin")]
50+
WrongPin,
4851
}
4952

5053
#[allow(clippy::large_enum_variant)]

crates/bitwarden-core/src/client/internal.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ use crate::{
2525
login_method::UserLoginMethod,
2626
},
2727
error::NotAuthenticatedError,
28-
key_management::{crypto::InitUserCryptoRequest, SecurityState, SignedSecurityState},
28+
key_management::{
29+
crypto::InitUserCryptoRequest, PasswordProtectedKeyEnvelope, SecurityState,
30+
SignedSecurityState,
31+
},
2932
};
3033

3134
/// Represents the user's keys, that are encrypted by the user key, and the signed security state.
@@ -309,6 +312,31 @@ impl InternalClient {
309312
self.initialize_user_crypto_decrypted_key(decrypted_user_key, key_state)
310313
}
311314

315+
#[cfg(feature = "internal")]
316+
pub(crate) fn initialize_user_crypto_pin_envelope(
317+
&self,
318+
pin: String,
319+
pin_protected_user_key_envelope: PasswordProtectedKeyEnvelope,
320+
key_state: UserKeyState,
321+
) -> Result<(), EncryptionSettingsError> {
322+
let decrypted_user_key = {
323+
// Note: This block ensures ctx is dropped. Otherwise it would cause a deadlock when
324+
// initializing the user crypto
325+
use crate::key_management::SymmetricKeyId;
326+
let ctx = &mut self.key_store.context_mut();
327+
let decrypted_user_key_id = pin_protected_user_key_envelope
328+
.unseal(SymmetricKeyId::Local("tmp_unlock_pin"), &pin, ctx)
329+
.map_err(|_| EncryptionSettingsError::WrongPin)?;
330+
331+
// Allowing deprecated here, until a refactor to pass the Local key ids to
332+
// `initialized_user_crypto_decrypted_key`
333+
#[allow(deprecated)]
334+
ctx.dangerous_get_symmetric_key(decrypted_user_key_id)?
335+
.clone()
336+
};
337+
self.initialize_user_crypto_decrypted_key(decrypted_user_key, key_state)
338+
}
339+
312340
#[cfg(feature = "secrets")]
313341
pub(crate) fn initialize_crypto_single_org_key(
314342
&self,

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

Lines changed: 115 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@
77
use std::collections::HashMap;
88

99
use bitwarden_crypto::{
10-
dangerous_get_v2_rotated_account_keys, AsymmetricCryptoKey, CoseSerializable, CryptoError,
11-
EncString, Kdf, KeyDecryptable, KeyEncryptable, MasterKey, Pkcs8PrivateKeyBytes,
12-
SignatureAlgorithm, SignedPublicKey, SigningKey, SpkiPublicKeyBytes, SymmetricCryptoKey,
13-
UnsignedSharedKey, UserKey,
10+
dangerous_get_v2_rotated_account_keys, safe::PasswordProtectedKeyEnvelopeError,
11+
AsymmetricCryptoKey, CoseSerializable, CryptoError, EncString, Kdf, KeyDecryptable,
12+
KeyEncryptable, MasterKey, Pkcs8PrivateKeyBytes, PrimitiveEncryptable, SignatureAlgorithm,
13+
SignedPublicKey, SigningKey, SpkiPublicKeyBytes, SymmetricCryptoKey, UnsignedSharedKey,
14+
UserKey,
1415
};
1516
use bitwarden_encoding::B64;
1617
use bitwarden_error::bitwarden_error;
@@ -23,7 +24,8 @@ use crate::{
2324
client::{encryption_settings::EncryptionSettingsError, LoginMethod, UserLoginMethod},
2425
error::StatefulCryptoError,
2526
key_management::{
26-
AsymmetricKeyId, SecurityState, SignedSecurityState, SigningKeyId, SymmetricKeyId,
27+
non_generic_wrappers::PasswordProtectedKeyEnvelope, AsymmetricKeyId, SecurityState,
28+
SignedSecurityState, SigningKeyId, SymmetricKeyId,
2729
},
2830
Client, NotAuthenticatedError, OrganizationId, UserId, VaultLockedError, WrongPasswordError,
2931
};
@@ -39,6 +41,8 @@ pub enum CryptoClientError {
3941
VaultLocked(#[from] VaultLockedError),
4042
#[error(transparent)]
4143
Crypto(#[from] bitwarden_crypto::CryptoError),
44+
#[error(transparent)]
45+
PasswordProtectedKeyEnvelope(#[from] PasswordProtectedKeyEnvelopeError),
4246
}
4347

4448
/// State used for initializing the user cryptographic state.
@@ -68,6 +72,7 @@ pub struct InitUserCryptoRequest {
6872
#[serde(rename_all = "camelCase", deny_unknown_fields)]
6973
#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))]
7074
#[cfg_attr(feature = "wasm", derive(Tsify), tsify(into_wasm_abi, from_wasm_abi))]
75+
#[allow(clippy::large_enum_variant)]
7176
pub enum InitUserCryptoMethod {
7277
/// Password
7378
Password {
@@ -89,6 +94,13 @@ pub enum InitUserCryptoMethod {
8994
/// this.
9095
pin_protected_user_key: EncString,
9196
},
97+
/// PIN Envelope
98+
PinEnvelope {
99+
/// The user's PIN
100+
pin: String,
101+
/// The user's symmetric crypto key, encrypted with the PIN-protected key envelope.
102+
pin_protected_user_key_envelope: PasswordProtectedKeyEnvelope,
103+
},
92104
/// Auth request
93105
AuthRequest {
94106
/// Private Key generated by the `crate::auth::new_auth_request`.
@@ -173,6 +185,16 @@ pub(super) async fn initialize_user_crypto(
173185
key_state,
174186
)?;
175187
}
188+
InitUserCryptoMethod::PinEnvelope {
189+
pin,
190+
pin_protected_user_key_envelope,
191+
} => {
192+
client.internal.initialize_user_crypto_pin_envelope(
193+
pin,
194+
pin_protected_user_key_envelope,
195+
key_state,
196+
)?;
197+
}
176198
InitUserCryptoMethod::AuthRequest {
177199
request_private_key,
178200
method,
@@ -313,6 +335,38 @@ pub(super) fn update_password(
313335
})
314336
}
315337

338+
/// Request for deriving a pin protected user key
339+
#[derive(Serialize, Deserialize, Debug)]
340+
#[serde(rename_all = "camelCase", deny_unknown_fields)]
341+
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))]
342+
#[cfg_attr(feature = "wasm", derive(Tsify), tsify(into_wasm_abi, from_wasm_abi))]
343+
pub struct EnrollPinResponse {
344+
/// [UserKey] protected by PIN
345+
pub pin_protected_user_key_envelope: PasswordProtectedKeyEnvelope,
346+
/// PIN protected by [UserKey]
347+
pub user_key_encrypted_pin: EncString,
348+
}
349+
350+
pub(super) fn enroll_pin(
351+
client: &Client,
352+
pin: String,
353+
) -> Result<EnrollPinResponse, CryptoClientError> {
354+
let key_store = client.internal.get_key_store();
355+
let mut ctx = key_store.context_mut();
356+
357+
let key_envelope =
358+
PasswordProtectedKeyEnvelope(bitwarden_crypto::safe::PasswordProtectedKeyEnvelope::seal(
359+
SymmetricKeyId::User,
360+
&pin,
361+
&ctx,
362+
)?);
363+
let encrypted_pin = pin.encrypt(&mut ctx, SymmetricKeyId::User)?;
364+
Ok(EnrollPinResponse {
365+
pin_protected_user_key_envelope: key_envelope,
366+
user_key_encrypted_pin: encrypted_pin,
367+
})
368+
}
369+
316370
/// Request for deriving a pin protected user key
317371
#[derive(Serialize, Deserialize, Debug)]
318372
#[serde(rename_all = "camelCase", deny_unknown_fields)]
@@ -930,6 +984,62 @@ mod tests {
930984
assert_eq!(client_key, client3_key);
931985
}
932986

987+
#[tokio::test]
988+
async fn test_initialize_user_crypto_pin_envelope() {
989+
let user_key = "5yKAZ4TSSEGje54MV5lc5ty6crkqUz4xvl+8Dm/piNLKf6OgRi2H0uzttNTXl9z6ILhkmuIXzGpAVc2YdorHgQ==";
990+
let test_pin = "1234";
991+
992+
let client1 = Client::new(None);
993+
initialize_user_crypto(
994+
&client1,
995+
InitUserCryptoRequest {
996+
user_id: Some(UserId::new_v4()),
997+
kdf_params: Kdf::PBKDF2 {
998+
iterations: 100_000.try_into().unwrap(),
999+
},
1000+
email: "[email protected]".into(),
1001+
private_key: make_key_pair(user_key.try_into().unwrap())
1002+
.unwrap()
1003+
.user_key_encrypted_private_key,
1004+
signing_key: None,
1005+
security_state: None,
1006+
method: InitUserCryptoMethod::DecryptedKey {
1007+
decrypted_user_key: user_key.to_string(),
1008+
},
1009+
},
1010+
)
1011+
.await
1012+
.unwrap();
1013+
1014+
let enroll_response = client1.crypto().enroll_pin(test_pin.to_string()).unwrap();
1015+
1016+
let client2 = Client::new(None);
1017+
initialize_user_crypto(
1018+
&client2,
1019+
InitUserCryptoRequest {
1020+
user_id: Some(UserId::new_v4()),
1021+
// NOTE: THIS CHANGES KDF SETTINGS. We ensure in this test that even with different
1022+
// KDF settings the pin can unlock the user key.
1023+
kdf_params: Kdf::PBKDF2 {
1024+
iterations: 600_000.try_into().unwrap(),
1025+
},
1026+
email: "[email protected]".into(),
1027+
private_key: make_key_pair(user_key.try_into().unwrap())
1028+
.unwrap()
1029+
.user_key_encrypted_private_key,
1030+
signing_key: None,
1031+
security_state: None,
1032+
method: InitUserCryptoMethod::PinEnvelope {
1033+
pin: test_pin.to_string(),
1034+
pin_protected_user_key_envelope: enroll_response
1035+
.pin_protected_user_key_envelope,
1036+
},
1037+
},
1038+
)
1039+
.await
1040+
.unwrap();
1041+
}
1042+
9331043
#[test]
9341044
fn test_enroll_admin_password_reset() {
9351045
let client = Client::new(None);

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

Lines changed: 88 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use bitwarden_crypto::CryptoError;
1+
use bitwarden_crypto::{CryptoError, Decryptable};
22
#[cfg(feature = "internal")]
33
use bitwarden_crypto::{EncString, UnsignedSharedKey};
44
use bitwarden_encoding::B64;
@@ -10,18 +10,23 @@ use super::crypto::{
1010
DeriveKeyConnectorRequest, EnrollAdminPasswordResetError, MakeKeyPairResponse,
1111
VerifyAsymmetricKeysRequest, VerifyAsymmetricKeysResponse,
1212
};
13+
#[cfg(any(feature = "wasm", test))]
14+
use crate::key_management::PasswordProtectedKeyEnvelope;
1315
#[cfg(feature = "internal")]
14-
use crate::key_management::crypto::{
15-
derive_pin_key, derive_pin_user_key, enroll_admin_password_reset, get_user_encryption_key,
16-
initialize_org_crypto, initialize_user_crypto, update_password, DerivePinKeyResponse,
17-
InitOrgCryptoRequest, InitUserCryptoRequest, UpdatePasswordResponse,
16+
use crate::key_management::{
17+
crypto::{
18+
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,
21+
},
22+
SymmetricKeyId,
1823
};
1924
use crate::{
2025
client::encryption_settings::EncryptionSettingsError,
2126
error::StatefulCryptoError,
2227
key_management::crypto::{
23-
get_v2_rotated_account_keys, make_v2_keys_for_v1_user, CryptoClientError,
24-
UserCryptoV2KeysResponse,
28+
enroll_pin, get_v2_rotated_account_keys, make_v2_keys_for_v1_user, CryptoClientError,
29+
EnrollPinResponse, UserCryptoV2KeysResponse,
2530
},
2631
Client,
2732
};
@@ -81,6 +86,45 @@ impl CryptoClient {
8186
) -> Result<UserCryptoV2KeysResponse, StatefulCryptoError> {
8287
get_v2_rotated_account_keys(&self.client)
8388
}
89+
90+
/// Protects the current user key with the provided PIN. The result can be stored and later
91+
/// used to initialize another client instance by using the PIN and the PIN key with
92+
/// `initialize_user_crypto`.
93+
pub fn enroll_pin(&self, pin: String) -> Result<EnrollPinResponse, CryptoClientError> {
94+
enroll_pin(&self.client, pin)
95+
}
96+
97+
/// Protects the current user key with the provided PIN. The result can be stored and later
98+
/// used to initialize another client instance by using the PIN and the PIN key with
99+
/// `initialize_user_crypto`. The provided pin is encrypted with the user key.
100+
pub fn enroll_pin_with_encrypted_pin(
101+
&self,
102+
// Note: This will be replaced by `EncString` with https://bitwarden.atlassian.net/browse/PM-24775
103+
encrypted_pin: String,
104+
) -> Result<EnrollPinResponse, CryptoClientError> {
105+
let encrypted_pin: EncString = encrypted_pin.parse()?;
106+
let pin = encrypted_pin.decrypt(
107+
&mut self.client.internal.get_key_store().context_mut(),
108+
SymmetricKeyId::User,
109+
)?;
110+
enroll_pin(&self.client, pin)
111+
}
112+
113+
/// Decrypts a `PasswordProtectedKeyEnvelope`, returning the user key, if successful.
114+
/// This is a stop-gap solution, until initialization of the SDK is used.
115+
#[cfg(any(feature = "wasm", test))]
116+
pub fn unseal_password_protected_key_envelope(
117+
&self,
118+
pin: String,
119+
envelope: PasswordProtectedKeyEnvelope,
120+
) -> Result<Vec<u8>, CryptoClientError> {
121+
let mut ctx = self.client.internal.get_key_store().context_mut();
122+
let key_slot = SymmetricKeyId::Local("unseal_password_protected_key_envelope");
123+
envelope.unseal(key_slot, pin.as_str(), &mut ctx)?;
124+
#[allow(deprecated)]
125+
let key = ctx.dangerous_get_symmetric_key(key_slot)?;
126+
Ok(key.to_encoded().to_vec())
127+
}
84128
}
85129

86130
impl CryptoClient {
@@ -141,3 +185,40 @@ impl Client {
141185
}
142186
}
143187
}
188+
189+
#[cfg(test)]
190+
mod tests {
191+
use bitwarden_crypto::{BitwardenLegacyKeyBytes, SymmetricCryptoKey};
192+
193+
use super::*;
194+
use crate::client::test_accounts::test_bitwarden_com_account;
195+
196+
#[tokio::test]
197+
async fn test_enroll_pin_envelope() {
198+
// Initialize a test client with user crypto
199+
let client = Client::init_test_account(test_bitwarden_com_account()).await;
200+
let user_key_initial =
201+
SymmetricCryptoKey::try_from(client.crypto().get_user_encryption_key().await.unwrap())
202+
.unwrap();
203+
204+
// Enroll with a PIN, then re-enroll
205+
let pin = "1234";
206+
let enroll_response = client.crypto().enroll_pin(pin.to_string()).unwrap();
207+
let re_enroll_response = client
208+
.crypto()
209+
.enroll_pin_with_encrypted_pin(enroll_response.user_key_encrypted_pin.to_string())
210+
.unwrap();
211+
212+
let secret = BitwardenLegacyKeyBytes::from(
213+
client
214+
.crypto()
215+
.unseal_password_protected_key_envelope(
216+
pin.to_string(),
217+
re_enroll_response.pin_protected_user_key_envelope,
218+
)
219+
.unwrap(),
220+
);
221+
let user_key_final = SymmetricCryptoKey::try_from(&secret).unwrap();
222+
assert_eq!(user_key_initial, user_key_final);
223+
}
224+
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ pub use crypto_client::CryptoClient;
2121

2222
#[cfg(feature = "internal")]
2323
mod non_generic_wrappers;
24-
#[allow(unused_imports)]
2524
#[cfg(feature = "internal")]
2625
pub(crate) use non_generic_wrappers::*;
2726
#[cfg(feature = "internal")]

crates/bitwarden-uniffi/src/crypto.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use bitwarden_core::key_management::crypto::{
2-
DeriveKeyConnectorRequest, DerivePinKeyResponse, InitOrgCryptoRequest, InitUserCryptoRequest,
3-
UpdatePasswordResponse,
2+
DeriveKeyConnectorRequest, DerivePinKeyResponse, EnrollPinResponse, InitOrgCryptoRequest,
3+
InitUserCryptoRequest, UpdatePasswordResponse,
44
};
55
use bitwarden_crypto::{EncString, UnsignedSharedKey};
66
use bitwarden_encoding::B64;
@@ -68,6 +68,26 @@ impl CryptoClient {
6868
.map_err(Error::MobileCrypto)?)
6969
}
7070

71+
/// Protects the current user key with the provided PIN. The result can be stored and later
72+
/// used to initialize another client instance by using the PIN and the PIN key with
73+
/// `initialize_user_crypto`.
74+
pub fn enroll_pin(&self, pin: String) -> Result<EnrollPinResponse> {
75+
Ok(self.0.enroll_pin(pin).map_err(Error::MobileCrypto)?)
76+
}
77+
78+
/// Protects the current user key with the provided PIN. The result can be stored and later
79+
/// used to initialize another client instance by using the PIN and the PIN key with
80+
/// `initialize_user_crypto`. The provided pin is encrypted with the user key.
81+
pub fn enroll_pin_with_encrypted_pin(
82+
&self,
83+
encrypted_pin: EncString,
84+
) -> Result<EnrollPinResponse> {
85+
Ok(self
86+
.0
87+
.enroll_pin_with_encrypted_pin(encrypted_pin.to_string())
88+
.map_err(Error::MobileCrypto)?)
89+
}
90+
7191
pub fn enroll_admin_password_reset(&self, public_key: B64) -> Result<UnsignedSharedKey> {
7292
Ok(self
7393
.0

0 commit comments

Comments
 (0)