Skip to content

Commit ab7ae36

Browse files
quextendani-garciaHinton
authored
[PM-27230] Introduce Account Cryptographic State (#563)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-27230 https://bitwarden.atlassian.net/browse/PM-27313 <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> Client PR, fixing breaking changes: bitwarden/clients#17488 ## 📔 Objective Account cryptographic state describes the core cryptographic objects making up a user. For a V1 encryption user these are: The RSA Keypair. For a V2 user these are: - The Public-key Encryption Keypair - The Signature Keypair - The Signed public key (used to bind a public-key encryption key pair to signing keypair) - The Signed security state (used for a safe upgrade path that prevents insecure features) Note: The wrapped version omits the public key and verifying key for the public-key encryption key-pair and signature key-pair, and only includes the wrapped private / signing keys. Not included are: - Keys shared to the user via an organization memebership or emergency access - Cipher keys which are not part of the user's cryptography, but do interact with it. Provided is a function to generate such a cryptographic state for v2 users, and conversion to API request models. Further, this changes SDK initialization to be based on the account cryptographic state. ## 🚨 Breaking Changes Please note that the public API for initializing the user's account cryptography is updated to instead take an enum. The variants for the enum contain the private key for V1 users, and the private key, signing key, signed public key, signed security state for V2 users. Aside from re-packaging into an enum variant, no other changes should be needed on the consuming side. This both helps prevent inconsistent states from being passed in by enforcing consistent state via type safetey, but also cleans up the primitive obsession anti-pattern (https://contributing.bitwarden.com/architecture/server/#avoid-primitive-obsession) that the crypto initialization was facing and will make future changes much easier. Note that since so far clients did not store the signed public key, it is optional for now, but will be made mandatory later after clients save it for a sufficient time. To migrate, simply repack the existing values into the corresponding enum. Note that previously the signed public key was not passed in, now it is passed in (optional for now, but required later on). Please ensure it is saved to state on sync / login. <!-- Does this PR introduce any breaking changes? If so, please describe the impact and migration path for clients. If you're unsure, the automated TypeScript compatibility check will run when you open/update this PR and provide feedback. For breaking changes: 1. Describe what changed in the client interface 2. Explain why the change was necessary 3. Provide migration steps for client developers 4. Link to any paired client PRs if needed Otherwise, you can remove this section. --> ## ⏰ 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: Daniel García <[email protected]> Co-authored-by: Oscar Hinton <[email protected]>
1 parent 7e4b2cc commit ab7ae36

File tree

33 files changed

+1087
-532
lines changed

33 files changed

+1087
-532
lines changed

crates/bitwarden-collections/src/collection.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ impl From<bitwarden_api_api::models::CollectionType> for CollectionType {
152152
#[cfg(test)]
153153
mod tests {
154154
use bitwarden_core::key_management::{KeyIds, SymmetricKeyId};
155-
use bitwarden_crypto::{KeyStore, PrimitiveEncryptable, SymmetricCryptoKey};
155+
use bitwarden_crypto::{KeyStore, PrimitiveEncryptable, SymmetricKeyAlgorithm};
156156

157157
use super::*;
158158

@@ -162,14 +162,14 @@ mod tests {
162162
// Helper function to create a test key store with a symmetric key
163163
fn create_test_key_store() -> KeyStore<KeyIds> {
164164
let store = KeyStore::<KeyIds>::default();
165-
let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
166165
let org_id = ORGANIZATION_ID.parse().unwrap();
167166

168-
#[allow(deprecated)]
169-
store
170-
.context_mut()
171-
.set_symmetric_key(SymmetricKeyId::Organization(org_id), key)
167+
let mut ctx = store.context_mut();
168+
169+
let local_key_id = ctx.make_symmetric_key(SymmetricKeyAlgorithm::Aes256CbcHmac);
170+
ctx.persist_symmetric_key(local_key_id, SymmetricKeyId::Organization(org_id))
172171
.unwrap();
172+
drop(ctx);
173173

174174
store
175175
}

crates/bitwarden-core/src/auth/auth_request.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ mod tests {
112112
use super::*;
113113
use crate::{
114114
UserId,
115-
client::internal::UserKeyState,
116115
key_management::{
117116
SymmetricKeyId,
117+
account_cryptographic_state::WrappedAccountCryptographicState,
118118
crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest},
119119
},
120120
};
@@ -165,11 +165,7 @@ mod tests {
165165
.initialize_user_crypto_master_key(
166166
master_key,
167167
user_key,
168-
UserKeyState {
169-
private_key,
170-
signing_key: None,
171-
security_state: None,
172-
},
168+
WrappedAccountCryptographicState::V1 { private_key },
173169
)
174170
.unwrap();
175171

@@ -240,10 +236,8 @@ mod tests {
240236
.initialize_user_crypto_master_key(
241237
master_key,
242238
user_key,
243-
UserKeyState {
239+
WrappedAccountCryptographicState::V1 {
244240
private_key: private_key.clone(),
245-
signing_key: None,
246-
security_state: None,
247241
},
248242
)
249243
.unwrap();
@@ -262,9 +256,7 @@ mod tests {
262256
user_id: Some(UserId::new_v4()),
263257
kdf_params: kdf,
264258
email: email.to_owned(),
265-
private_key,
266-
signing_key: None,
267-
security_state: None,
259+
account_cryptographic_state: WrappedAccountCryptographicState::V1 { private_key },
268260
method: InitUserCryptoMethod::AuthRequest {
269261
request_private_key: auth_req.private_key,
270262
method: AuthRequestMethod::UserKey {

crates/bitwarden-core/src/auth/login/api_key.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ use crate::{
88
api::{request::ApiTokenRequest, response::IdentityTokenResponse},
99
login::{LoginError, PasswordLoginResponse, response::two_factor::TwoFactorProviders},
1010
},
11-
client::{LoginMethod, UserLoginMethod, internal::UserKeyState},
12-
key_management::UserDecryptionData,
11+
client::{LoginMethod, UserLoginMethod},
12+
key_management::{
13+
UserDecryptionData, account_cryptographic_state::WrappedAccountCryptographicState,
14+
},
1315
require,
1416
};
1517

@@ -31,11 +33,7 @@ pub(crate) async fn login_api_key(
3133

3234
let private_key: EncString = require!(&r.private_key).parse()?;
3335

34-
let user_key_state = UserKeyState {
35-
private_key,
36-
signing_key: None,
37-
security_state: None,
38-
};
36+
let user_key_state = WrappedAccountCryptographicState::V1 { private_key };
3937

4038
let master_password_unlock = r
4139
.user_decryption_options

crates/bitwarden-core/src/auth/login/auth_request.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::{
1212
},
1313
key_management::{
1414
UserDecryptionData,
15+
account_cryptographic_state::WrappedAccountCryptographicState,
1516
crypto::{AuthRequestMethod, InitUserCryptoMethod, InitUserCryptoRequest},
1617
},
1718
require,
@@ -127,9 +128,9 @@ pub(crate) async fn complete_auth_request(
127128
user_id: None,
128129
kdf_params: kdf,
129130
email: salt,
130-
private_key: require!(r.private_key).parse()?,
131-
signing_key: None,
132-
security_state: None,
131+
account_cryptographic_state: WrappedAccountCryptographicState::V1 {
132+
private_key: require!(r.private_key).parse()?,
133+
},
133134
method: InitUserCryptoMethod::AuthRequest {
134135
request_private_key: auth_req.private_key,
135136
method,

crates/bitwarden-core/src/auth/login/password.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@ pub(crate) async fn login_password(
2121
) -> Result<PasswordLoginResponse, LoginError> {
2222
use bitwarden_crypto::EncString;
2323

24-
use crate::{
25-
client::{UserLoginMethod, internal::UserKeyState},
26-
require,
27-
};
24+
use crate::{client::UserLoginMethod, require};
2825

2926
info!("password logging in");
3027

@@ -40,6 +37,8 @@ pub(crate) async fn login_password(
4037
let response = request_identity_tokens(client, input, &password_hash).await?;
4138

4239
if let IdentityTokenResponse::Authenticated(r) = &response {
40+
use crate::key_management::account_cryptographic_state::WrappedAccountCryptographicState;
41+
4342
client.internal.set_tokens(
4443
r.access_token.clone(),
4544
r.refresh_token.clone(),
@@ -48,11 +47,7 @@ pub(crate) async fn login_password(
4847

4948
let private_key: EncString = require!(&r.private_key).parse()?;
5049

51-
let user_key_state = UserKeyState {
52-
private_key,
53-
signing_key: None,
54-
security_state: None,
55-
};
50+
let user_key_state = WrappedAccountCryptographicState::V1 { private_key };
5651

5752
let master_password_unlock = r
5853
.user_decryption_options

crates/bitwarden-core/src/auth/password/validate.rs

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ mod tests {
8585

8686
use crate::{
8787
auth::password::{validate::validate_password_user_key, validate_password},
88-
client::internal::UserKeyState,
88+
key_management::account_cryptographic_state::WrappedAccountCryptographicState,
8989
};
9090

9191
#[test]
@@ -149,11 +149,7 @@ mod tests {
149149
.initialize_user_crypto_master_key(
150150
master_key,
151151
user_key.clone(),
152-
UserKeyState {
153-
private_key,
154-
signing_key: None,
155-
security_state: None,
156-
},
152+
WrappedAccountCryptographicState::V1 { private_key },
157153
)
158154
.unwrap();
159155

@@ -203,11 +199,7 @@ mod tests {
203199
.initialize_user_crypto_master_key(
204200
master_key,
205201
user_key.parse().unwrap(),
206-
UserKeyState {
207-
private_key,
208-
signing_key: None,
209-
security_state: None,
210-
},
202+
WrappedAccountCryptographicState::V1 { private_key },
211203
)
212204
.unwrap();
213205

crates/bitwarden-core/src/auth/pin.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ mod tests {
4949
use bitwarden_crypto::{Kdf, MasterKey};
5050

5151
use super::*;
52-
use crate::client::{Client, LoginMethod, UserLoginMethod, internal::UserKeyState};
52+
use crate::{
53+
client::{Client, LoginMethod, UserLoginMethod},
54+
key_management::account_cryptographic_state::WrappedAccountCryptographicState,
55+
};
5356

5457
fn init_client() -> Client {
5558
let client = Client::new(None);
@@ -78,11 +81,7 @@ mod tests {
7881
.initialize_user_crypto_master_key(
7982
master_key,
8083
user_key.parse().unwrap(),
81-
UserKeyState {
82-
private_key,
83-
signing_key: None,
84-
security_state: None,
85-
},
84+
WrappedAccountCryptographicState::V1 { private_key },
8685
)
8786
.unwrap();
8887

crates/bitwarden-core/src/auth/tde.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use bitwarden_crypto::{
55
use bitwarden_encoding::B64;
66

77
use crate::{
8-
Client,
9-
client::{encryption_settings::EncryptionSettingsError, internal::UserKeyState},
8+
Client, client::encryption_settings::EncryptionSettingsError,
9+
key_management::account_cryptographic_state::WrappedAccountCryptographicState,
1010
};
1111

1212
/// This function generates a new user key and key pair, initializes the client's crypto with the
@@ -34,12 +34,10 @@ pub(super) fn make_register_tde_keys(
3434

3535
client.internal.initialize_user_crypto_decrypted_key(
3636
user_key.0,
37-
UserKeyState {
37+
// TODO (https://bitwarden.atlassian.net/browse/PM-21771) Signing keys are not supported on registration yet. This needs to be changed as
38+
// soon as registration is supported.
39+
WrappedAccountCryptographicState::V1 {
3840
private_key: key_pair.private.clone(),
39-
// TODO (https://bitwarden.atlassian.net/browse/PM-21771) Signing keys are not supported on registration yet. This needs to be changed as
40-
// soon as registration is supported.
41-
signing_key: None,
42-
security_state: None,
4341
},
4442
)?;
4543

0 commit comments

Comments
 (0)