-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-10929] Support Key Connector #959
Changes from all commits
8b0cc6d
3f5cb9e
52154b8
64a0fbb
be2650a
b0161a0
a874395
9fe447a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
use bitwarden_crypto::{CryptoError, MasterKey, RsaKeyPair}; | ||
|
||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
pub struct KeyConnectorResponse { | ||
pub master_key: String, | ||
pub encrypted_user_key: String, | ||
pub keys: RsaKeyPair, | ||
} | ||
|
||
pub(super) fn make_key_connector_keys( | ||
mut rng: impl rand::RngCore, | ||
) -> Result<KeyConnectorResponse, CryptoError> { | ||
let master_key = MasterKey::generate(&mut rng); | ||
let (user_key, encrypted_user_key) = master_key.make_user_key()?; | ||
let keys = user_key.make_key_pair()?; | ||
|
||
Ok(KeyConnectorResponse { | ||
master_key: master_key.to_base64(), | ||
encrypted_user_key: encrypted_user_key.to_string(), | ||
keys, | ||
}) | ||
Comment on lines
+13
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is quite different compared to our current approach, but helps avoiding the weird steps in our ts project @jlf0dev. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps I don't understand the key connector flow, but I had thought the encrypting keys were generated by key connect -- are they just stored there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct - just stored there. |
||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use rand::SeedableRng; | ||
use rand_chacha::ChaCha8Rng; | ||
|
||
use super::*; | ||
|
||
#[test] | ||
fn test_make_key_connector_keys() { | ||
let mut rng = ChaCha8Rng::from_seed([0u8; 32]); | ||
|
||
let result = make_key_connector_keys(&mut rng).unwrap(); | ||
|
||
assert_eq!( | ||
result.master_key, | ||
"PgDvL4lfQNZ/W7joHwmloSyEDsPOmn87GBvhiO9xGh4=" | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,18 @@ | ||
use std::collections::HashMap; | ||
|
||
use bitwarden_crypto::{AsymmetricEncString, EncString}; | ||
#[cfg(feature = "internal")] | ||
use bitwarden_crypto::{Kdf, KeyDecryptable, KeyEncryptable, MasterKey, SymmetricCryptoKey}; | ||
use bitwarden_crypto::{ | ||
AsymmetricEncString, EncString, Kdf, KeyDecryptable, KeyEncryptable, MasterKey, | ||
SymmetricCryptoKey, | ||
}; | ||
use schemars::JsonSchema; | ||
use serde::{Deserialize, Serialize}; | ||
|
||
#[cfg(feature = "internal")] | ||
use crate::client::{LoginMethod, UserLoginMethod}; | ||
use crate::{ | ||
client::{LoginMethod, UserLoginMethod}, | ||
error::{Error, Result}, | ||
Client, | ||
}; | ||
|
||
#[cfg(feature = "internal")] | ||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
|
@@ -28,7 +27,6 @@ | |
pub method: InitUserCryptoMethod, | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] | ||
|
@@ -64,9 +62,14 @@ | |
/// The user's symmetric crypto key, encrypted with the Device Key. | ||
device_protected_user_key: AsymmetricEncString, | ||
}, | ||
KeyConnector { | ||
/// Base64 encoded master key, retrieved from the key connector. | ||
master_key: String, | ||
/// The user's encrypted symmetric crypto key | ||
user_key: String, | ||
}, | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] | ||
|
@@ -83,7 +86,6 @@ | |
}, | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
pub async fn initialize_user_crypto(client: &Client, req: InitUserCryptoRequest) -> Result<()> { | ||
use bitwarden_crypto::{DeviceKey, PinKey}; | ||
|
||
|
@@ -151,6 +153,17 @@ | |
.internal | ||
.initialize_user_crypto_decrypted_key(user_key, private_key)?; | ||
} | ||
InitUserCryptoMethod::KeyConnector { | ||
master_key, | ||
user_key, | ||
} => { | ||
let master_key = MasterKey::new(SymmetricCryptoKey::try_from(master_key)?); | ||
let user_key: EncString = user_key.parse()?; | ||
|
||
client | ||
.internal | ||
.initialize_user_crypto_master_key(master_key, user_key, private_key)?; | ||
} | ||
} | ||
|
||
client | ||
|
@@ -166,7 +179,6 @@ | |
Ok(()) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
|
@@ -175,22 +187,19 @@ | |
pub organization_keys: HashMap<uuid::Uuid, AsymmetricEncString>, | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
pub async fn initialize_org_crypto(client: &Client, req: InitOrgCryptoRequest) -> Result<()> { | ||
let organization_keys = req.organization_keys.into_iter().collect(); | ||
client.internal.initialize_org_crypto(organization_keys)?; | ||
Ok(()) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
pub async fn get_user_encryption_key(client: &Client) -> Result<String> { | ||
let enc = client.internal.get_encryption_settings()?; | ||
let user_key = enc.get_key(&None)?; | ||
|
||
Ok(user_key.to_base64()) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
|
@@ -233,7 +242,6 @@ | |
}) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
|
@@ -244,7 +252,6 @@ | |
encrypted_pin: EncString, | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
pub fn derive_pin_key(client: &Client, pin: String) -> Result<DerivePinKeyResponse> { | ||
let enc = client.internal.get_encryption_settings()?; | ||
let user_key = enc.get_key(&None)?; | ||
|
@@ -262,7 +269,6 @@ | |
}) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
pub fn derive_pin_user_key(client: &Client, encrypted_pin: EncString) -> Result<EncString> { | ||
let enc = client.internal.get_encryption_settings()?; | ||
let user_key = enc.get_key(&None)?; | ||
|
@@ -276,7 +282,6 @@ | |
derive_pin_protected_user_key(&pin, &login_method, user_key) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
fn derive_pin_protected_user_key( | ||
pin: &str, | ||
login_method: &LoginMethod, | ||
|
@@ -296,7 +301,6 @@ | |
Ok(derived_key.encrypt_user_key(user_key)?) | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
pub(super) fn enroll_admin_password_reset( | ||
client: &Client, | ||
public_key: String, | ||
|
@@ -314,8 +318,30 @@ | |
)?) | ||
} | ||
|
||
#[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
pub struct DeriveKeyConnectorRequest { | ||
/// Encrypted user key, used to validate the master key | ||
pub user_key_encrypted: EncString, | ||
|
||
pub password: String, | ||
pub kdf: Kdf, | ||
pub email: String, | ||
} | ||
|
||
/// Derive the master key for migrating to the key connector | ||
pub(super) fn derive_key_connector(request: DeriveKeyConnectorRequest) -> Result<String> { | ||
let master_key = MasterKey::derive(&request.password, &request.email, &request.kdf)?; | ||
master_key | ||
.decrypt_user_key(request.user_key_encrypted) | ||
.map_err(|_| "wrong password")?; | ||
|
||
Ok(master_key.to_base64()) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use std::num::NonZeroU32; | ||
|
||
use super::*; | ||
use crate::Client; | ||
|
||
|
@@ -495,11 +521,8 @@ | |
); | ||
} | ||
|
||
#[cfg(feature = "internal")] | ||
#[test] | ||
fn test_enroll_admin_password_reset() { | ||
use std::num::NonZeroU32; | ||
|
||
use base64::{engine::general_purpose::STANDARD, Engine}; | ||
use bitwarden_crypto::AsymmetricCryptoKey; | ||
|
||
|
@@ -534,4 +557,20 @@ | |
let expected = enc.get_key(&None).unwrap(); | ||
assert_eq!(&decrypted, &expected.to_vec()); | ||
} | ||
|
||
#[test] | ||
fn test_derive_key_connector() { | ||
let request = DeriveKeyConnectorRequest { | ||
password: "asdfasdfasdf".to_string(), | ||
email: "[email protected]".to_string(), | ||
kdf: Kdf::PBKDF2 { | ||
iterations: NonZeroU32::new(600_000).unwrap(), | ||
}, | ||
user_key_encrypted: "2.Q/2PhzcC7GdeiMHhWguYAQ==|GpqzVdr0go0ug5cZh1n+uixeBC3oC90CIe0hd/HWA/pTRDZ8ane4fmsEIcuc8eMKUt55Y2q/fbNzsYu41YTZzzsJUSeqVjT8/iTQtgnNdpo=|dwI+uyvZ1h/iZ03VQ+/wrGEFYVewBUUl/syYgjsNMbE=".parse().unwrap(), | ||
}; | ||
|
||
let result = derive_key_connector(request).unwrap(); | ||
|
||
assert_eq!(result, "ySXq1RVLKEaV1eoQE/ui9aFKIvXTl9PAXwp1MljfF50="); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not return a
Result
? Equivalently, why does this method return a specific error type rather than the base error enum?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're transitioning away from a single massive enum as that meshes badly with the new crate structure. And it's generally a better experience knowing what errors you get vs needing to validate all errors.