Skip to content

Commit bc2f203

Browse files
quextenMGibson1dani-garciaHinton
authored
[PM-15097] Implement COSE/XChaCha20 key format and encstring (#181)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-15097 ## 📔 Objective Adds the initial version of the new key format - using COSE, and adds XChaCha20-Poly1305 encryption using it. Specifically, to separate COSE keys from other types of keys, they are padded using a PKCS7-like padding to be larger than the existing AES256_CBC_HMAC keys. When wrapped using asymmetric encryption, or using old symmetric encryption key types, COSE messages are serialized to the byte array and padded. When encrypted by other COSE keys, there is no padding needed, instead the content_format will indicate what format the content message (key) is using, one of which is `cosekey`. Content formats will be added in a follow-up PR. The new encstring type shall not be used until then, since this is another format change. COSE keys are assigned a `key-id`, 24 random bytes, which is considered enough for random collision resistance. Citing the XChaCha doc on why a 24-byte nonce is enough: > Assuming a secure random number generator, random 192-bit nonces should experience a single collision (with probability 50%) after roughly 2^96 messages (approximately 7.2998163e+28). A more conservative threshold (2^-32 chance of collision) still allows for 2^80 messages to be sent under a single key. (https://datatracker.ietf.org/doc/html/draft-arciszewski-xchacha). ## ⏰ 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: Daniel García <[email protected]> Co-authored-by: Oscar Hinton <[email protected]>
1 parent b185bc6 commit bc2f203

File tree

37 files changed

+1058
-203
lines changed

37 files changed

+1058
-203
lines changed

.vscode/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
"Bitwarden",
55
"Cdecl",
66
"chrono",
7+
"ciphertext",
78
"cloc",
9+
"COSE",
810
"dealloc",
911
"decryptable",
1012
"dylib",
@@ -22,6 +24,7 @@
2224
"totp",
2325
"uniffi",
2426
"wordlist",
27+
"XCHACHA",
2528
"Zeroize",
2629
"Zeroizing",
2730
"zxcvbn"

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ fn test_auth_request() {
130130

131131
let decrypted = auth_request_decrypt_user_key(request.private_key, encrypted).unwrap();
132132

133-
assert_eq!(decrypted.to_vec(), secret);
133+
assert_eq!(decrypted.to_encoded(), secret);
134134
}
135135

136136
#[cfg(test)]
@@ -183,7 +183,7 @@ mod tests {
183183
let dec = auth_request_decrypt_user_key(private_key.to_owned(), enc_user_key).unwrap();
184184

185185
assert_eq!(
186-
&dec.to_vec(),
186+
&dec.to_encoded(),
187187
&[
188188
201, 37, 234, 213, 21, 75, 40, 70, 149, 213, 234, 16, 19, 251, 162, 245, 161, 74,
189189
34, 245, 211, 151, 211, 192, 95, 10, 117, 50, 88, 223, 23, 157
@@ -202,7 +202,7 @@ mod tests {
202202
.unwrap();
203203

204204
assert_eq!(
205-
&dec.to_vec(),
205+
&dec.to_encoded(),
206206
&[
207207
109, 128, 172, 147, 206, 123, 134, 95, 16, 36, 155, 113, 201, 18, 186, 230, 216,
208208
212, 173, 188, 74, 11, 134, 131, 137, 242, 105, 178, 105, 126, 52, 139, 248, 91,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub(crate) fn validate_password_user_key(
6565
#[allow(deprecated)]
6666
let existing_key = ctx.dangerous_get_symmetric_key(SymmetricKeyId::User)?;
6767

68-
if user_key.to_vec() != existing_key.to_vec() {
68+
if user_key != *existing_key {
6969
return Err(AuthValidateError::WrongUserKey);
7070
}
7171

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ pub(crate) fn validate_pin(
3737
return Ok(false);
3838
};
3939

40-
Ok(user_key.to_vec() == decrypted_key.to_vec())
40+
Ok(*user_key == decrypted_key)
4141
}
4242
}
4343
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@ pub(super) fn make_register_tde_keys(
1717
) -> Result<RegisterTdeKeyResponse, EncryptionSettingsError> {
1818
let public_key = AsymmetricPublicCryptoKey::from_der(&STANDARD.decode(org_public_key)?)?;
1919

20-
let mut rng = rand::thread_rng();
21-
22-
let user_key = UserKey::new(SymmetricCryptoKey::generate(&mut rng));
20+
let user_key = UserKey::new(SymmetricCryptoKey::make_aes256_cbc_hmac_key());
2321
let key_pair = user_key.make_key_pair()?;
2422

2523
let admin_reset = UnsignedSharedKey::encapsulate_key_unsigned(&user_key.0, &public_key)?;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ mod tests {
776776
.dangerous_get_symmetric_key(SymmetricKeyId::User)
777777
.unwrap();
778778

779-
assert_eq!(&decrypted.to_vec(), &expected.to_vec());
779+
assert_eq!(decrypted, *expected);
780780
}
781781

782782
#[test]

crates/bitwarden-crypto/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ base64 = ">=0.22.1, <0.23"
3131
bitwarden-error = { workspace = true }
3232
cbc = { version = ">=0.1.2, <0.2", features = ["alloc", "zeroize"] }
3333
chacha20poly1305 = { version = "0.10.1" }
34+
ciborium = { version = ">=0.2.2, <0.3" }
35+
coset = { version = ">=0.3.8, <0.4" }
3436
generic-array = { version = ">=0.14.7, <1.0", features = ["zeroize"] }
3537
hkdf = ">=0.12.3, <0.13"
3638
hmac = ">=0.12.1, <0.13"
@@ -48,6 +50,7 @@ sha2 = ">=0.10.6, <0.11"
4850
subtle = ">=2.5.0, <3.0"
4951
thiserror = { workspace = true }
5052
tsify-next = { workspace = true, optional = true }
53+
typenum = ">=1.18.0, <1.19.0"
5154
uniffi = { workspace = true, optional = true }
5255
uuid = { workspace = true }
5356
wasm-bindgen = { workspace = true, optional = true }

crates/bitwarden-crypto/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ secure.
1616
use bitwarden_crypto::{SymmetricCryptoKey, KeyEncryptable, KeyDecryptable, CryptoError};
1717

1818
async fn example() -> Result<(), CryptoError> {
19-
let key = SymmetricCryptoKey::generate(rand::thread_rng());
19+
let key = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
2020

2121
let data = "Hello, World!".to_owned();
2222
let encrypted = data.clone().encrypt_with_key(&key)?;

crates/bitwarden-crypto/src/aes.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@
55
//! In most cases you should use the [EncString][crate::EncString] with
66
//! [KeyEncryptable][crate::KeyEncryptable] & [KeyDecryptable][crate::KeyDecryptable] instead.
77
8-
use aes::cipher::{
9-
block_padding::Pkcs7, typenum::U32, BlockDecryptMut, BlockEncryptMut, KeyIvInit,
10-
};
8+
use aes::cipher::{block_padding::Pkcs7, BlockDecryptMut, BlockEncryptMut, KeyIvInit};
119
use generic_array::GenericArray;
1210
use hmac::Mac;
1311
use subtle::ConstantTimeEq;
12+
use typenum::U32;
1413

1514
use crate::{
1615
error::{CryptoError, Result},

crates/bitwarden-crypto/src/cose.rs

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
//! This file contains private-use constants for COSE encoded key types and algorithms.
2+
//! Standardized values from <https://www.iana.org/assignments/cose/cose.xhtml> should always be preferred
3+
//! unless there is a a clear benefit, such as a clear cryptographic benefit, which MUST
4+
//! be documented publicly.
5+
6+
use coset::{iana, CborSerializable, Label};
7+
use generic_array::GenericArray;
8+
use typenum::U32;
9+
10+
use crate::{
11+
error::EncStringParseError, xchacha20, CryptoError, SymmetricCryptoKey, XChaCha20Poly1305Key,
12+
};
13+
14+
/// XChaCha20 <https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-xchacha-03> is used over ChaCha20
15+
/// to be able to randomly generate nonces, and to not have to worry about key wearout. Since
16+
/// the draft was never published as an RFC, we use a private-use value for the algorithm.
17+
pub(crate) const XCHACHA20_POLY1305: i64 = -70000;
18+
19+
/// Encrypts a plaintext message using XChaCha20Poly1305 and returns a COSE Encrypt0 message
20+
pub(crate) fn encrypt_xchacha20_poly1305(
21+
plaintext: &[u8],
22+
key: &crate::XChaCha20Poly1305Key,
23+
) -> Result<Vec<u8>, CryptoError> {
24+
let mut protected_header = coset::HeaderBuilder::new().build();
25+
// This should be adjusted to use the builder pattern once implemented in coset.
26+
// The related coset upstream issue is:
27+
// https://github.com/google/coset/issues/105
28+
protected_header.alg = Some(coset::Algorithm::PrivateUse(XCHACHA20_POLY1305));
29+
30+
let mut nonce = [0u8; xchacha20::NONCE_SIZE];
31+
let cose_encrypt0 = coset::CoseEncrypt0Builder::new()
32+
.protected(protected_header)
33+
.create_ciphertext(plaintext, &[], |data, aad| {
34+
let ciphertext =
35+
crate::xchacha20::encrypt_xchacha20_poly1305(&(*key.enc_key).into(), data, aad);
36+
nonce = ciphertext.nonce();
37+
ciphertext.encrypted_bytes().to_vec()
38+
})
39+
.unprotected(coset::HeaderBuilder::new().iv(nonce.to_vec()).build())
40+
.build();
41+
42+
cose_encrypt0
43+
.to_vec()
44+
.map_err(|err| CryptoError::EncString(EncStringParseError::InvalidCoseEncoding(err)))
45+
}
46+
47+
/// Decrypts a COSE Encrypt0 message, using a XChaCha20Poly1305 key
48+
pub(crate) fn decrypt_xchacha20_poly1305(
49+
cose_encrypt0_message: &[u8],
50+
key: &crate::XChaCha20Poly1305Key,
51+
) -> Result<Vec<u8>, CryptoError> {
52+
let msg = coset::CoseEncrypt0::from_slice(cose_encrypt0_message)
53+
.map_err(|err| CryptoError::EncString(EncStringParseError::InvalidCoseEncoding(err)))?;
54+
let Some(ref alg) = msg.protected.header.alg else {
55+
return Err(CryptoError::EncString(
56+
EncStringParseError::CoseMissingAlgorithm,
57+
));
58+
};
59+
if *alg != coset::Algorithm::PrivateUse(XCHACHA20_POLY1305) {
60+
return Err(CryptoError::WrongKeyType);
61+
}
62+
63+
let decrypted_message = msg.decrypt(&[], |data, aad| {
64+
let nonce = msg.unprotected.iv.as_slice();
65+
crate::xchacha20::decrypt_xchacha20_poly1305(
66+
nonce
67+
.try_into()
68+
.map_err(|_| CryptoError::InvalidNonceLength)?,
69+
&(*key.enc_key).into(),
70+
data,
71+
aad,
72+
)
73+
})?;
74+
Ok(decrypted_message)
75+
}
76+
77+
const SYMMETRIC_KEY: Label = Label::Int(iana::SymmetricKeyParameter::K as i64);
78+
79+
impl TryFrom<&coset::CoseKey> for SymmetricCryptoKey {
80+
type Error = CryptoError;
81+
82+
fn try_from(cose_key: &coset::CoseKey) -> Result<Self, Self::Error> {
83+
let key_bytes = cose_key
84+
.params
85+
.iter()
86+
.find_map(|(label, value)| match (label, value) {
87+
(&SYMMETRIC_KEY, ciborium::Value::Bytes(bytes)) => Some(bytes),
88+
_ => None,
89+
})
90+
.ok_or(CryptoError::InvalidKey)?;
91+
let alg = cose_key.alg.as_ref().ok_or(CryptoError::InvalidKey)?;
92+
93+
match alg {
94+
coset::Algorithm::PrivateUse(XCHACHA20_POLY1305) => {
95+
// Ensure the length is correct since `GenericArray::clone_from_slice` panics if it
96+
// receives the wrong length.
97+
if key_bytes.len() != xchacha20::KEY_SIZE {
98+
return Err(CryptoError::InvalidKey);
99+
}
100+
let enc_key = Box::pin(GenericArray::<u8, U32>::clone_from_slice(key_bytes));
101+
let key_id = cose_key
102+
.key_id
103+
.as_slice()
104+
.try_into()
105+
.map_err(|_| CryptoError::InvalidKey)?;
106+
Ok(SymmetricCryptoKey::XChaCha20Poly1305Key(
107+
XChaCha20Poly1305Key { enc_key, key_id },
108+
))
109+
}
110+
_ => Err(CryptoError::InvalidKey),
111+
}
112+
}
113+
}
114+
115+
#[cfg(test)]
116+
mod test {
117+
use super::*;
118+
119+
#[test]
120+
fn test_encrypt_decrypt_roundtrip() {
121+
let SymmetricCryptoKey::XChaCha20Poly1305Key(ref key) =
122+
SymmetricCryptoKey::make_xchacha20_poly1305_key()
123+
else {
124+
panic!("Failed to create XChaCha20Poly1305Key");
125+
};
126+
127+
let plaintext = b"Hello, world!";
128+
let encrypted = encrypt_xchacha20_poly1305(plaintext, key).unwrap();
129+
let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
130+
assert_eq!(decrypted, plaintext);
131+
}
132+
}

crates/bitwarden-crypto/src/enc_string/asymmetric.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ impl UnsignedSharedKey {
163163
) -> Result<UnsignedSharedKey> {
164164
let enc = encrypt_rsa2048_oaep_sha1(
165165
encapsulation_key.to_public_key(),
166-
&encapsulated_key.to_vec(),
166+
&encapsulated_key.to_encoded(),
167167
)?;
168168
Ok(UnsignedSharedKey::Rsa2048_OaepSha1_B64 { data: enc })
169169
}

0 commit comments

Comments
 (0)