Skip to content

Commit 4b80d30

Browse files
authored
[PM-22271] Expose KDF calculation in PureCrypto (#339)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-22271 bitwarden/clients#15401 ## 📔 Objective We want to drop our three different argon2 implementations on `clients` (desktop-native rustcrypto, patched argon2-browser WASM, native C++ module on cli). Each of these have impactful bugs (https://bitwarden.atlassian.net/browse/PM-17450, https://bitwarden.atlassian.net/browse/PM-22217), and tech debt (dependency updates, and the patching scripts for node modules) associated. In this case, there is a small amount of tech debt introduced in the SDK because we introduce a deprecated function on PureCrypto. This small amount of tech debt introduced is far outweight by the dependencies that can be dropped on ts. (The clients PR: <img width="128" alt="image" src="https://github.com/user-attachments/assets/642c6ee8-2df3-406c-aba1-167a1647b65d" />) Specifically, since we want to fix immediate bugs, we do not yet do the work of porting all higher-level usages (masterpassword-unlock, masterkeyhash auth, pin unlock/enrollment, send password "key"(hash) deriving) to the SDK. This is follow-up feature work, and larger in scope, and involves other teams (auth,tools). Once this follow-up feature-work is complete, the tech-debt function introduced in this ticket will be removed, and a tech-debt ticket for this is linked. The tech debt function is clearly marked as `dangerous_` and `deprecated` with a note. ## ⏰ 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
1 parent 8423a4c commit 4b80d30

File tree

3 files changed

+64
-0
lines changed

3 files changed

+64
-0
lines changed

crates/bitwarden-crypto/src/keys/kdf.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,22 @@ impl KdfDerivedKeyMaterial {
9898
}
9999
}
100100

101+
#[deprecated(
102+
note = "This function is only meant as a temporary stop-gap to expose KDF derivation in PureCrypto until the higher-level consumers are moved to the SDK directly. DO NOT USE THIS OUTSIDE OF PureCrypto!"
103+
)]
104+
/// Derives KDF material given a password, salt and kdf configuration. This function is
105+
/// a stop-gap solution and should not be used outside of PureCrypto.
106+
///
107+
/// The clean-up ticket is tracked here:
108+
/// `https://bitwarden.atlassian.net/browse/PM-23168`
109+
pub fn dangerous_derive_kdf_material(
110+
password: &[u8],
111+
salt: &[u8],
112+
kdf: &Kdf,
113+
) -> Result<Vec<u8>, CryptoError> {
114+
KdfDerivedKeyMaterial::derive_kdf_key(password, salt, kdf).map(|kdf_key| kdf_key.0.to_vec())
115+
}
116+
101117
/// Key Derivation Function for Bitwarden Account
102118
///
103119
/// In Bitwarden accounts can use multiple KDFs to derive their master key from their password. This

crates/bitwarden-crypto/src/keys/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ pub use device_key::{DeviceKey, TrustDeviceResponse};
2525
mod pin_key;
2626
pub use pin_key::PinKey;
2727
mod kdf;
28+
#[allow(deprecated)]
29+
pub use kdf::dangerous_derive_kdf_material;
2830
mod key_id;
2931
pub use kdf::{
3032
default_argon2_iterations, default_argon2_memory, default_argon2_parallelism,

crates/bitwarden-wasm-internal/src/pure_crypto.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::str::FromStr;
22

33
use bitwarden_core::key_management::{KeyIds, SymmetricKeyId};
4+
#[allow(deprecated)]
5+
use bitwarden_crypto::dangerous_derive_kdf_material;
46
use bitwarden_crypto::{
57
AsymmetricCryptoKey, AsymmetricPublicCryptoKey, BitwardenLegacyKeyBytes, CoseKeyBytes,
68
CoseSerializable, CoseSign1Bytes, CryptoError, Decryptable, EncString, Kdf, KeyDecryptable,
@@ -312,6 +314,16 @@ impl PureCrypto {
312314
.map(|public_key| public_key.to_der())?
313315
.map(|pk| pk.to_vec())
314316
}
317+
318+
/// Derive output of the KDF for a [bitwarden_crypto::Kdf] configuration.
319+
pub fn derive_kdf_material(
320+
password: &[u8],
321+
salt: &[u8],
322+
kdf: Kdf,
323+
) -> Result<Vec<u8>, CryptoError> {
324+
#[allow(deprecated)]
325+
dangerous_derive_kdf_material(password, salt, &kdf)
326+
}
315327
}
316328

317329
#[cfg(test)]
@@ -427,6 +439,16 @@ DnqOsltgPomWZ7xVfMkm9niL2OA=
427439
73, 4, 134, 242, 24, 56, 54, 38, 178, 59, 11, 118, 230, 159, 87, 91, 20, 237, 188, 186,
428440
216, 86, 189, 50, 46, 173, 117, 36, 54, 105, 216, 9,
429441
];
442+
443+
const DERIVED_KDF_MATERIAL_PBKDF2: &[u8] = &[
444+
129, 57, 137, 140, 156, 220, 110, 212, 201, 255, 52, 182, 22, 206, 221, 66, 136, 199, 181,
445+
89, 252, 175, 82, 168, 79, 204, 88, 174, 166, 60, 52, 79,
446+
];
447+
const DERIVED_KDF_MATERIAL_ARGON2ID: &[u8] = &[
448+
221, 57, 158, 206, 27, 154, 188, 170, 33, 198, 250, 144, 191, 231, 29, 74, 201, 102, 253,
449+
77, 8, 128, 173, 111, 217, 41, 125, 9, 156, 52, 112, 140,
450+
];
451+
430452
#[test]
431453
fn test_symmetric_decrypt() {
432454
let enc_string = EncString::from_str(ENCRYPTED).unwrap();
@@ -620,4 +642,28 @@ DnqOsltgPomWZ7xVfMkm9niL2OA=
620642
.unwrap();
621643
assert_eq!(public_key, PUBLIC_KEY);
622644
}
645+
646+
#[test]
647+
fn test_derive_pbkdf2_output() {
648+
let password = "test_password".as_bytes();
649+
let email = "[email protected]".as_bytes();
650+
let kdf = Kdf::PBKDF2 {
651+
iterations: NonZero::try_from(600000).unwrap(),
652+
};
653+
let derived_key = PureCrypto::derive_kdf_material(password, email, kdf).unwrap();
654+
assert_eq!(derived_key, DERIVED_KDF_MATERIAL_PBKDF2);
655+
}
656+
657+
#[test]
658+
fn test_derived_argon2_output() {
659+
let password = "test_password".as_bytes();
660+
let email = "[email protected]".as_bytes();
661+
let kdf = Kdf::Argon2id {
662+
iterations: NonZero::try_from(3).unwrap(),
663+
memory: NonZero::try_from(64).unwrap(),
664+
parallelism: NonZero::try_from(4).unwrap(),
665+
};
666+
let derived_key = PureCrypto::derive_kdf_material(password, email, kdf).unwrap();
667+
assert_eq!(derived_key, DERIVED_KDF_MATERIAL_ARGON2ID);
668+
}
623669
}

0 commit comments

Comments
 (0)