Skip to content

Commit 66c345f

Browse files
quextenquexten
committed
Fix keywrap
1 parent 5447cbb commit 66c345f

File tree

5 files changed

+151
-19
lines changed

5 files changed

+151
-19
lines changed

‎crates/bitwarden-crypto/src/cose.rs

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ pub(crate) fn encrypt_xchacha20_poly1305(
9292
pub(crate) fn decrypt_xchacha20_poly1305(
9393
cose_encrypt0_message: &[u8],
9494
key: &crate::XChaCha20Poly1305Key,
95-
) -> Result<Vec<u8>, CryptoError> {
95+
) -> Result<(Vec<u8>, ContentFormat), CryptoError> {
9696
let msg = coset::CoseEncrypt0::from_slice(cose_encrypt0_message)
9797
.map_err(|err| CryptoError::EncString(EncStringParseError::InvalidCoseEncoding(err)))?;
9898
let Some(ref alg) = msg.protected.header.alg else {
@@ -117,12 +117,30 @@ pub(crate) fn decrypt_xchacha20_poly1305(
117117
})?;
118118

119119
if let Some(ref content_type) = msg.protected.header.content_type {
120-
if *content_type == ContentType::Text(CONTENT_TYPE_PADDED_UTF8.to_string()) {
121-
// Unpad the data to get the original plaintext
122-
return crate::keys::utils::unpad_bytes(&decrypted_message).map(|bytes| bytes.to_vec());
120+
match content_type {
121+
ContentType::Text(format) if format == CONTENT_TYPE_PADDED_UTF8 => {
122+
if *content_type == ContentType::Text(CONTENT_TYPE_PADDED_UTF8.to_string()) {
123+
// Unpad the data to get the original plaintext
124+
let data = crate::keys::utils::unpad_bytes(&decrypted_message)
125+
.map_err(|_| CryptoError::InvalidPadding)?;
126+
return Ok((data.to_vec(), ContentFormat::Utf8));
127+
}
128+
},
129+
ContentType::Assigned(content_format) if *content_format == CoapContentFormat::Pkcs8 => {
130+
return Ok((decrypted_message.to_vec(), ContentFormat::Pkcs8));
131+
}
132+
ContentType::Assigned(content_format) if *content_format == CoapContentFormat::CoseKey => {
133+
return Ok((decrypted_message.to_vec(), ContentFormat::CoseKey));
134+
}
135+
ContentType::Assigned(content_format) if *content_format == CoapContentFormat::OctetStream => {
136+
return Ok((decrypted_message.to_vec(), ContentFormat::OctetStream));
137+
}
138+
_ => {}
123139
}
124140
}
125-
Ok(decrypted_message)
141+
Err(CryptoError::EncString(
142+
EncStringParseError::CoseMissingContentType,
143+
))
126144
}
127145

128146
const SYMMETRIC_KEY: Label = Label::Int(iana::SymmetricKeyParameter::K as i64);
@@ -168,7 +186,7 @@ mod test {
168186
use super::*;
169187

170188
#[test]
171-
fn test_encrypt_decrypt_roundtrip() {
189+
fn test_encrypt_decrypt_roundtrip_octetstream() {
172190
let SymmetricCryptoKey::XChaCha20Poly1305Key(ref key) =
173191
SymmetricCryptoKey::make_xchacha20_poly1305_key()
174192
else {
@@ -179,6 +197,49 @@ mod test {
179197
let encrypted =
180198
encrypt_xchacha20_poly1305(plaintext, key, ContentFormat::OctetStream).unwrap();
181199
let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
182-
assert_eq!(decrypted, plaintext);
200+
assert_eq!(decrypted, (plaintext.to_vec(), ContentFormat::OctetStream));
201+
}
202+
203+
#[test]
204+
fn test_encrypt_decrypt_roundtrip_utf8() {
205+
let SymmetricCryptoKey::XChaCha20Poly1305Key(ref key) =
206+
SymmetricCryptoKey::make_xchacha20_poly1305_key()
207+
else {
208+
panic!("Failed to create XChaCha20Poly1305Key");
209+
};
210+
211+
let plaintext = b"Hello, world!";
212+
let encrypted = encrypt_xchacha20_poly1305(plaintext, key, ContentFormat::Utf8).unwrap();
213+
let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
214+
assert_eq!(decrypted, (plaintext.to_vec(), ContentFormat::Utf8));
183215
}
216+
217+
#[test]
218+
fn test_encrypt_decrypt_roundtrip_pkcs8() {
219+
let SymmetricCryptoKey::XChaCha20Poly1305Key(ref key) =
220+
SymmetricCryptoKey::make_xchacha20_poly1305_key()
221+
else {
222+
panic!("Failed to create XChaCha20Poly1305Key");
223+
};
224+
225+
let plaintext = b"Hello, world!";
226+
let encrypted = encrypt_xchacha20_poly1305(plaintext, key, ContentFormat::Pkcs8).unwrap();
227+
let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
228+
assert_eq!(decrypted, (plaintext.to_vec(), ContentFormat::Pkcs8));
229+
}
230+
231+
#[test]
232+
fn test_encrypt_decrypt_roundtrip_cosekey() {
233+
let SymmetricCryptoKey::XChaCha20Poly1305Key(ref key) =
234+
SymmetricCryptoKey::make_xchacha20_poly1305_key()
235+
else {
236+
panic!("Failed to create XChaCha20Poly1305Key");
237+
};
238+
239+
let plaintext = b"Hello, world!";
240+
let encrypted = encrypt_xchacha20_poly1305(plaintext, key, ContentFormat::CoseKey).unwrap();
241+
let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
242+
assert_eq!(decrypted, (plaintext.to_vec(), ContentFormat::CoseKey));
243+
}
244+
184245
}

‎crates/bitwarden-crypto/src/enc_string/symmetric.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
307307
EncString::Cose_Encrypt0_B64 { data },
308308
SymmetricCryptoKey::XChaCha20Poly1305Key(key),
309309
) => {
310-
let decrypted_message =
310+
let (decrypted_message, _) =
311311
crate::cose::decrypt_xchacha20_poly1305(data.as_slice(), key)?;
312312
Ok(decrypted_message)
313313
}

‎crates/bitwarden-crypto/src/error.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ pub enum EncStringParseError {
8383
InvalidCoseEncoding(coset::CoseError),
8484
#[error("Algorithm missing in COSE header")]
8585
CoseMissingAlgorithm,
86+
#[error("Content type missing in COSE header")]
87+
CoseMissingContentType,
8688
}
8789

8890
#[derive(Debug, Error)]

‎crates/bitwarden-crypto/src/keys/symmetric_crypto_key.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,13 @@ impl SymmetricCryptoKey {
208208
}
209209
}
210210

211+
pub(crate) fn try_from_cose(serialized_key: &[u8]) -> Result<Self, CryptoError> {
212+
let cose_key = coset::CoseKey::from_slice(serialized_key)
213+
.map_err(|_| CryptoError::InvalidKey)?;
214+
let key = SymmetricCryptoKey::try_from(&cose_key)?;
215+
Ok(key)
216+
}
217+
211218
pub fn to_base64(&self) -> String {
212219
STANDARD.encode(self.to_encoded())
213220
}
@@ -286,9 +293,7 @@ impl TryFrom<&mut [u8]> for SymmetricCryptoKey {
286293
Ok(Self::Aes256CbcKey(Aes256CbcKey { enc_key }))
287294
} else if value.len() > Self::AES256_CBC_HMAC_KEY_LEN {
288295
let unpadded_value = unpad_key(value)?;
289-
let cose_key =
290-
coset::CoseKey::from_slice(unpadded_value).map_err(|_| CryptoError::InvalidKey)?;
291-
SymmetricCryptoKey::try_from(&cose_key)
296+
Ok(Self::try_from_cose(unpadded_value)?)
292297
} else {
293298
Err(CryptoError::InvalidKeyLen)
294299
};

‎crates/bitwarden-crypto/src/store/context.rs

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,24 +132,44 @@ impl<Ids: KeyIds> KeyStoreContext<'_, Ids> {
132132
///
133133
/// # Arguments
134134
///
135-
/// * `encryption_key` - The key id used to decrypt the `encrypted_key`. It must already exist
135+
/// * `wrapping_key` - The key id used to decrypt the `wrapped_key`. It must already exist
136136
/// in the context
137137
/// * `new_key_id` - The key id where the decrypted key will be stored. If it already exists, it
138138
/// will be overwritten
139-
/// * `encrypted_key` - The key to decrypt
139+
/// * `wrapped_key` - The key to decrypt
140140
pub fn unwrap_symmetric_key(
141141
&mut self,
142-
encryption_key: Ids::Symmetric,
142+
wrapping_key: Ids::Symmetric,
143143
new_key_id: Ids::Symmetric,
144-
encrypted_key: &EncString,
144+
wrapped_key: &EncString,
145145
) -> Result<Ids::Symmetric> {
146-
let mut new_key_material =
147-
self.decrypt_data_with_symmetric_key(encryption_key, encrypted_key)?;
146+
let wrapping_key = self.get_symmetric_key(wrapping_key)?;
147+
148+
let key = match (wrapped_key, wrapping_key) {
149+
(EncString::Aes256Cbc_B64 { iv, data }, SymmetricCryptoKey::Aes256CbcKey(key)) => {
150+
SymmetricCryptoKey::try_from(crate::aes::decrypt_aes256(iv, data.clone(), &key.enc_key)?)?
151+
}
152+
(
153+
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
154+
SymmetricCryptoKey::Aes256CbcHmacKey(key),
155+
) => {
156+
SymmetricCryptoKey::try_from(crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), &key.mac_key, &key.enc_key)?)?
157+
}
158+
(EncString::Cose_Encrypt0_B64 { data }, SymmetricCryptoKey::XChaCha20Poly1305Key(key)) => {
159+
let (content_bytes, content_format) = crate::cose::decrypt_xchacha20_poly1305(data, key)?;
160+
match content_format {
161+
ContentFormat::OctetStream => SymmetricCryptoKey::try_from(content_bytes)?,
162+
ContentFormat::CoseKey => SymmetricCryptoKey::try_from_cose(&content_bytes)?,
163+
_ => return Err(CryptoError::InvalidKey)
164+
}
165+
}
166+
_ => return Err(CryptoError::InvalidKey),
167+
};
148168

149169
#[allow(deprecated)]
150170
self.set_symmetric_key(
151171
new_key_id,
152-
SymmetricCryptoKey::try_from(new_key_material.as_mut_slice())?,
172+
key,
153173
)?;
154174

155175
// Returning the new key identifier for convenience
@@ -179,7 +199,7 @@ impl<Ids: KeyIds> KeyStoreContext<'_, Ids> {
179199
// or `Aes256CbcKey`, or by specifying the content format to be CoseKey, in case the
180200
// wrapped key is a `XChaCha20Poly1305Key`.
181201
match (wrapping_key_instance, key_to_wrap_instance) {
182-
(Aes256CbcHmacKey(_), Aes256CbcHmacKey(_) | Aes256CbcKey(_)) => self
202+
(Aes256CbcHmacKey(_), Aes256CbcHmacKey(_) | Aes256CbcKey(_) | XChaCha20Poly1305Key(_)) => self
183203
.encrypt_data_with_symmetric_key(
184204
wrapping_key,
185205
key_to_wrap_instance.to_encoded().as_slice(),
@@ -368,6 +388,10 @@ impl<Ids: KeyIds> KeyStoreContext<'_, Ids> {
368388
EncString::Aes256Cbc_HmacSha256_B64 { iv, mac, data },
369389
SymmetricCryptoKey::Aes256CbcHmacKey(key),
370390
) => crate::aes::decrypt_aes256_hmac(iv, mac, data.clone(), &key.mac_key, &key.enc_key),
391+
(EncString::Cose_Encrypt0_B64 { data }, SymmetricCryptoKey::XChaCha20Poly1305Key(key)) => {
392+
let (data, _) = crate::cose::decrypt_xchacha20_poly1305(data, key)?;
393+
Ok(data)
394+
}
371395
_ => Err(CryptoError::InvalidKey),
372396
}
373397
}
@@ -468,4 +492,44 @@ mod tests {
468492
// Assert that the decrypted data is the same
469493
assert_eq!(decrypted1.0, decrypted2.0);
470494
}
495+
496+
#[test]
497+
fn test_wrap_unwrap_aes256_cbc_hmac() {
498+
let store: KeyStore<TestIds> = KeyStore::default();
499+
let mut ctx = store.context_mut();
500+
501+
// Aes256 CBC HMAC keys
502+
let key_aes_1_id = TestSymmKey::A(1);
503+
let key_aes_1 = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
504+
ctx.set_symmetric_key(key_aes_1_id, key_aes_1.clone()).unwrap();
505+
let key_aes_2_id = TestSymmKey::A(2);
506+
let key_aes_2 = SymmetricCryptoKey::make_aes256_cbc_hmac_key();
507+
ctx.set_symmetric_key(key_aes_2_id, key_aes_2.clone()).unwrap();
508+
509+
// XChaCha20 Poly1305 keys
510+
let key_xchacha_3_id = TestSymmKey::A(3);
511+
let key_xchacha_3 = SymmetricCryptoKey::make_xchacha20_poly1305_key();
512+
ctx.set_symmetric_key(key_xchacha_3_id, key_xchacha_3.clone()).unwrap();
513+
let key_xchacha_4_id = TestSymmKey::A(4);
514+
let key_xchacha_4 = SymmetricCryptoKey::make_xchacha20_poly1305_key();
515+
ctx.set_symmetric_key(key_xchacha_4_id, key_xchacha_4.clone()).unwrap();
516+
517+
// Wrap and unwrap the keys
518+
let wrapped_key_1_2 = ctx.wrap_symmetric_key(key_aes_1_id, key_aes_2_id).unwrap();
519+
let wrapped_key_1_3 = ctx.wrap_symmetric_key(key_aes_1_id, key_xchacha_3_id).unwrap();
520+
let wrapped_key_3_1 = ctx.wrap_symmetric_key(key_xchacha_3_id, key_aes_1_id).unwrap();
521+
let wrapped_key_3_4 = ctx.wrap_symmetric_key(key_xchacha_3_id, key_xchacha_4_id).unwrap();
522+
523+
// Unwrap the keys
524+
let unwrapped_key_2 = ctx.unwrap_symmetric_key(key_aes_1_id, key_aes_2_id, &wrapped_key_1_2).unwrap();
525+
let unwrapped_key_3 = ctx.unwrap_symmetric_key(key_aes_1_id, key_xchacha_3_id, &wrapped_key_1_3).unwrap();
526+
let unwrapped_key_1 = ctx.unwrap_symmetric_key(key_xchacha_3_id, key_aes_1_id, &wrapped_key_3_1).unwrap();
527+
let unwrapped_key_4 = ctx.unwrap_symmetric_key(key_xchacha_3_id, key_xchacha_4_id, &wrapped_key_3_4).unwrap();
528+
529+
// Assert that the unwrapped keys are the same as the original keys
530+
assert_eq!(unwrapped_key_2, key_aes_2_id);
531+
assert_eq!(unwrapped_key_3, key_xchacha_3_id);
532+
assert_eq!(unwrapped_key_1, key_aes_1_id);
533+
assert_eq!(unwrapped_key_4, key_xchacha_4_id);
534+
}
471535
}

0 commit comments

Comments
 (0)