Skip to content
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

Convert symmetric crypto keys to use marker traits #256

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/bitwarden/src/auth/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};

use crate::{
client::auth_settings::Kdf,
crypto::{HashPurpose, MasterKey, RsaKeyPair},
crypto::{keys::{HashPurpose, FromMasterPassword}, RsaKeyPair, SymmetricCryptoKey},
error::Result,
util::default_pbkdf2_iterations,
Client,
Expand Down Expand Up @@ -64,7 +64,7 @@ pub(super) fn make_register_keys(
password: String,
kdf: Kdf,
) -> Result<RegisterKeyResponse> {
let master_key = MasterKey::derive(password.as_bytes(), email.as_bytes(), &kdf)?;
let master_key = SymmetricCryptoKey::<FromMasterPassword>::derive(password.as_bytes(), email.as_bytes(), &kdf)?;
let master_password_hash =
master_key.derive_master_key_hash(password.as_bytes(), HashPurpose::ServerAuthorization)?;
let (user_key, encrypted_user_key) = master_key.make_user_key()?;
Expand Down
8 changes: 7 additions & 1 deletion crates/bitwarden/src/client/access_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ use crate::{
util::BASE64_ENGINE,
};

/// Marker struct to indicate a key associated with an access token
pub struct AccessTokenEncryption {}
impl crate::crypto::keys::KeyPurpose for AccessTokenEncryption {}
// TODO: It seems odd that this needs to be a sharable key -- perhaps that concept is misnamed, or this key should be build differently?
impl crate::crypto::keys::ShareableKey for AccessTokenEncryption {}

pub struct AccessToken {
pub service_account_id: Uuid,
pub client_secret: String,
pub encryption_key: SymmetricCryptoKey,
pub(crate) encryption_key: SymmetricCryptoKey<AccessTokenEncryption>,
}

impl FromStr for AccessToken {
Expand Down
6 changes: 4 additions & 2 deletions crates/bitwarden/src/client/auth_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};

#[cfg(feature = "internal")]
use crate::{
crypto::{HashPurpose, MasterKey},
crypto::keys::HashPurpose,
error::Result,
};

Expand Down Expand Up @@ -69,7 +69,9 @@ impl AuthSettings {

#[cfg(feature = "internal")]
pub fn derive_user_password_hash(&self, password: &str) -> Result<String> {
let master_key = MasterKey::derive(password.as_bytes(), self.email.as_bytes(), &self.kdf)?;
use crate::crypto::{SymmetricCryptoKey, keys::FromMasterPassword};

let master_key = SymmetricCryptoKey::<FromMasterPassword>::derive(password.as_bytes(), self.email.as_bytes(), &self.kdf)?;
master_key.derive_master_key_hash(password.as_bytes(), HashPurpose::ServerAuthorization)
}
}
13 changes: 9 additions & 4 deletions crates/bitwarden/src/client/client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::time::{Duration, Instant};
use std::{time::{Duration, Instant}, str::FromStr};

use reqwest::header::{self};
use uuid::Uuid;
Expand Down Expand Up @@ -28,10 +28,12 @@ use crate::{
client_settings::{ClientSettings, DeviceType},
encryption_settings::EncryptionSettings,
},
crypto::SymmetricCryptoKey,
crypto::{SymmetricCryptoKey, keys::UserEncryption},
error::{Error, Result},
};

use super::access_token::AccessTokenEncryption;

#[derive(Debug)]
pub(crate) struct ApiConfigurations {
pub identity: bitwarden_api_identity::apis::configuration::Configuration,
Expand Down Expand Up @@ -235,9 +237,12 @@ impl Client {

pub(crate) fn initialize_crypto_single_key(
&mut self,
key: SymmetricCryptoKey,
key: SymmetricCryptoKey<AccessTokenEncryption>,
) -> &EncryptionSettings {
self.encryption_settings = Some(EncryptionSettings::new_single_key(key));
// TODO: this is a hack to convert access token keys to user keys. We need to rework how encryption settings work to fix.
let user_key = SymmetricCryptoKey::<UserEncryption>::from_str(&key.to_base64()).unwrap();

self.encryption_settings = Some(EncryptionSettings::new_single_key(user_key));
self.encryption_settings.as_ref().unwrap()
}

Expand Down
19 changes: 13 additions & 6 deletions crates/bitwarden/src/client/encryption_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ use {
};

use crate::{
crypto::{encrypt_aes256_hmac, EncString, SymmetricCryptoKey},
crypto::{encrypt_aes256_hmac, EncString, SymmetricCryptoKey, keys::{UserEncryption, OrganizationEncryption, KeyPurpose}},
error::{CryptoError, Result},
};

pub struct EncryptionSettings {
user_key: SymmetricCryptoKey,
user_key: SymmetricCryptoKey<UserEncryption>,
private_key: Option<RsaPrivateKey>,
org_keys: HashMap<Uuid, SymmetricCryptoKey>,
org_keys: HashMap<Uuid, SymmetricCryptoKey<OrganizationEncryption>>,
}

impl std::fmt::Debug for EncryptionSettings {
Expand All @@ -25,6 +25,12 @@ impl std::fmt::Debug for EncryptionSettings {
}
}

// TODO: This is a hack to allow Encryptable and Decryptable to automatically determine which key to use.
// Removing this would require a refactor of the traits
// to require keys to use. However, it would allow for greater assurances of key usage from the compiler. struct UniversalKeyPurpose {}
struct UniversalKeyPurpose {}
impl KeyPurpose for UniversalKeyPurpose {}

impl EncryptionSettings {
#[cfg(feature = "internal")]
pub(crate) fn new(
Expand All @@ -33,7 +39,7 @@ impl EncryptionSettings {
user_key: EncString,
private_key: EncString,
) -> Result<Self> {
use crate::crypto::MasterKey;
use crate::crypto::keys::MasterKey;

// Derive master key from password
let master_key = MasterKey::derive(password.as_bytes(), auth.email.as_bytes(), &auth.kdf)?;
Expand All @@ -54,7 +60,7 @@ impl EncryptionSettings {
})
}

pub(crate) fn new_single_key(key: SymmetricCryptoKey) -> Self {
pub(crate) fn new_single_key(key: SymmetricCryptoKey<UserEncryption>) -> Self {
EncryptionSettings {
user_key: key,
private_key: None,
Expand Down Expand Up @@ -90,7 +96,8 @@ impl EncryptionSettings {
Ok(self)
}

fn get_key(&self, org_id: &Option<Uuid>) -> Option<&SymmetricCryptoKey> {
// TODO: Remove UniversalKeyPurpose
fn get_key(&self, org_id: &Option<Uuid>) -> Option<&SymmetricCryptoKey<UniversalKeyPurpose>> {
// If we don't have a private key set (to decode multiple org keys), we just use the main user key
if self.private_key.is_none() {
return Some(&self.user_key);
Expand Down
4 changes: 3 additions & 1 deletion crates/bitwarden/src/crypto/enc_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use crate::{
util::BASE64_ENGINE,
};

use super::keys::KeyPurpose;

#[derive(Clone)]
#[allow(unused, non_camel_case_types)]
pub enum EncString {
Expand Down Expand Up @@ -305,7 +307,7 @@ impl EncString {
}
}

pub fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
pub(crate) fn decrypt_with_key<TKeyPurpose: KeyPurpose>(&self, key: &SymmetricCryptoKey<TKeyPurpose>) -> Result<Vec<u8>> {
match self {
EncString::AesCbc256_HmacSha256_B64 { iv, mac, data } => {
let mac_key = key.mac_key.ok_or(CryptoError::InvalidMac)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use rand::Rng;

use crate::util::BASE64_ENGINE;

use super::{
encrypt_aes256, hkdf_expand, EncString, PbkdfSha256Hmac, SymmetricCryptoKey, UserKey,
PBKDF_SHA256_HMAC_OUT_SIZE,
use crate::crypto::{
encrypt_aes256, hkdf_expand, EncString, PbkdfSha256Hmac, SymmetricCryptoKey,
PBKDF_SHA256_HMAC_OUT_SIZE, keys::UserEncryption,
};
use {
crate::{client::auth_settings::Kdf, error::Result},
Expand All @@ -20,11 +20,10 @@ pub(crate) enum HashPurpose {
// LocalAuthorization = 2,
}

/// A Master Key.
pub(crate) struct MasterKey(SymmetricCryptoKey);
pub struct FromMasterPassword {}
impl super::KeyPurpose for FromMasterPassword {}

impl MasterKey {
/// Derives a users master key from their password, email and KDF.
impl SymmetricCryptoKey<FromMasterPassword> {
pub fn derive(password: &[u8], email: &[u8], kdf: &Kdf) -> Result<Self> {
derive_key(password, email, kdf).map(Self)
}
Expand All @@ -45,17 +44,17 @@ impl MasterKey {
Ok(BASE64_ENGINE.encode(hash))
}

pub(crate) fn make_user_key(&self) -> Result<(UserKey, EncString)> {
pub(crate) fn make_user_key(&self) -> Result<(SymmetricCryptoKey<UserEncryption>, EncString)> {
let mut user_key = [0u8; 64];
rand::thread_rng().fill(&mut user_key);

let protected = encrypt_aes256(&user_key, self.0.key)?;

let u: &[u8] = &user_key;
Ok((UserKey::new(SymmetricCryptoKey::try_from(u)?), protected))
Ok((SymmetricCryptoKey::try_from(u)?, protected))
}

pub(crate) fn decrypt_user_key(&self, user_key: EncString) -> Result<SymmetricCryptoKey> {
pub(crate) fn decrypt_user_key(&self, user_key: EncString) -> Result<SymmetricCryptoKey<UserEncryption>> {
let stretched_key = stretch_master_key(self)?;

let dec = user_key.decrypt_with_key(&stretched_key)?;
Expand All @@ -64,7 +63,7 @@ impl MasterKey {
}

/// Derive a generic key from a secret and salt using the provided KDF.
fn derive_key(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<SymmetricCryptoKey> {
fn derive_key<TKeyPurpose>(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<TKeyPurpose> {
let hash = match kdf {
Kdf::PBKDF2 { iterations } => pbkdf2::pbkdf2_array::<
PbkdfSha256Hmac,
Expand Down Expand Up @@ -103,19 +102,16 @@ fn derive_key(secret: &[u8], salt: &[u8], kdf: &Kdf) -> Result<SymmetricCryptoKe
SymmetricCryptoKey::try_from(hash.as_slice())
}

fn stretch_master_key(master_key: &MasterKey) -> Result<SymmetricCryptoKey> {
fn stretch_master_key(master_key: &MasterKey) -> Result<SymmetricCryptoKey<UserEncryption>> {
let key: GenericArray<u8, U32> = hkdf_expand(&master_key.0.key, Some("enc"))?;
let mac_key: GenericArray<u8, U32> = hkdf_expand(&master_key.0.key, Some("mac"))?;

Ok(SymmetricCryptoKey {
key,
mac_key: Some(mac_key),
})
Ok(SymmetricCryptoKey::new(key, Some(mac_key)))
}

#[cfg(test)]
mod tests {
use crate::crypto::SymmetricCryptoKey;
use crate::crypto::{SymmetricCryptoKey, keys::UserEncryption};

use super::{stretch_master_key, HashPurpose, MasterKey};
use {crate::client::auth_settings::Kdf, std::num::NonZeroU32};
Expand Down Expand Up @@ -166,14 +162,13 @@ mod tests {

#[test]
fn test_stretch_master_key() {
let master_key = MasterKey(SymmetricCryptoKey {
key: [
let master_key = MasterKey(SymmetricCryptoKey::<UserEncryption>::new([
31, 79, 104, 226, 150, 71, 177, 90, 194, 80, 172, 209, 17, 129, 132, 81, 138, 167,
69, 167, 254, 149, 2, 27, 39, 197, 64, 42, 22, 195, 86, 75,
]
.into(),
mac_key: None,
});
None,
));

let stretched = stretch_master_key(&master_key).unwrap();

Expand Down
15 changes: 15 additions & 0 deletions crates/bitwarden/src/crypto/keys/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
mod master_key;
pub use master_key::*;
mod shareable_key;
pub use shareable_key::*;
mod symmetric_crypto_key;
pub use symmetric_crypto_key::*;

#[cfg(feature = "internal")]
mod user_key;
#[cfg(feature = "internal")]
pub use user_key::*;
#[cfg(feature = "internal")]
mod organization_key;
#[cfg(feature = "internal")]
pub use organization_key::*;
9 changes: 9 additions & 0 deletions crates/bitwarden/src/crypto/keys/organization_key.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use crate::crypto::SymmetricCryptoKey;

use super::{AsymmetricKeyPairGeneration, KeyPurpose};

pub struct OrganizationEncryption {}
impl KeyPurpose for OrganizationEncryption {}
impl AsymmetricKeyPairGeneration for OrganizationEncryption {}

pub type OrganizationKey = SymmetricCryptoKey<OrganizationEncryption>;
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,28 @@ use hmac::{Hmac, Mac};

use crate::crypto::{hkdf_expand, SymmetricCryptoKey};

use super::KeyPurpose;

/// Marker trait to annotate that the key is intended shareable beyond the current account
pub trait ShareableKey : KeyPurpose {}

impl<TKeyPurpose : ShareableKey> SymmetricCryptoKey<TKeyPurpose> {
pub fn generate(name: &str) -> Self {
use rand::Rng;
let secret: [u8; 16] = rand::thread_rng().gen();
derive_shareable_key::<TKeyPurpose>(secret, name, None)
}
}

/// Derive a shareable key using hkdf from secret and name.
///
/// A specialized variant of this function was called `CryptoService.makeSendKey` in the Bitwarden
/// `clients` repository.
pub(crate) fn derive_shareable_key(
pub(crate) fn derive_shareable_key<TKeyPurpose: ShareableKey>(
secret: [u8; 16],
name: &str,
info: Option<&str>,
) -> SymmetricCryptoKey {
) -> SymmetricCryptoKey<TKeyPurpose> {
// Because all inputs are fixed size, we can unwrap all errors here without issue

// TODO: Are these the final `key` and `info` parameters or should we change them? I followed the pattern used for sends
Expand Down
Loading
Loading