[PM-22621] Fix missing key ID and add testvectors for cose decryption (#305)

## 🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-22621

## 📔 Objective

This PR adds the key identifier value on encryption to the encrypt0
message.

To quote the cose spec:
> This parameter identifies one piece of data that can be used as
      input to find the needed cryptographic key.  The value of this
      parameter can be matched against the 'kid' member in a COSE_Key
      structure.  Other methods of key distribution can define an
      equivalent field to be matched.  Applications MUST NOT assume that
      'kid' values are unique.  There may be more than one key with the
      same 'kid' value, so all of the keys associated with this 'kid'
      may need to be checked.  The internal structure of 'kid' values is
      not defined and cannot be relied on by applications.  Key
      identifier values are hints about which key to use.  This is not a
      security-critical field.  For this reason, it can be placed in the
      unprotected headers bucket.

While our code does not make use yet of the KID value for hinting at the
correct key, we do want to include it, and it had not been included so
far. Since we do not use cose yet, no migration is needed.

We also fail decryption on a mismatched key-id, but this is not a
security feature, instead this is a traceability feature, and makes the
failure reason clearer.
      
## ⏰ 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

Author: quexten

crates/bitwarden-crypto/src/cose.rs

@@ -21,7 +21,9 @@ pub(crate) fn encrypt_xchacha20_poly1305(
     plaintext: &[u8],
     key: &crate::XChaCha20Poly1305Key,
 ) -> Result<Vec<u8>, CryptoError> {
-    let mut protected_header = coset::HeaderBuilder::new().build();
+    let mut protected_header = coset::HeaderBuilder::new()
+        .key_id(key.key_id.to_vec())
+        .build();
     // This should be adjusted to use the builder pattern once implemented in coset.
     // The related coset upstream issue is:
     // https://github.com/google/coset/issues/105
@@ -59,6 +61,9 @@ pub(crate) fn decrypt_xchacha20_poly1305(
     if *alg != coset::Algorithm::PrivateUse(XCHACHA20_POLY1305) {
         return Err(CryptoError::WrongKeyType);
     }
+    if key.key_id != *msg.protected.header.key_id {
+        return Err(CryptoError::WrongCoseKeyId);
+    }
 
     let decrypted_message = msg.decrypt(&[], |data, aad| {
         let nonce = msg.unprotected.iv.as_slice();
@@ -116,6 +121,21 @@ impl TryFrom<&coset::CoseKey> for SymmetricCryptoKey {
 mod test {
     use super::*;
 
+    const KEY_ID: [u8; 16] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
+    const KEY_DATA: [u8; 32] = [
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e,
+        0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d,
+        0x1e, 0x1f,
+    ];
+    const TEST_VECTOR_PLAINTEXT: &[u8] = b"Message test vector";
+    const TEST_VECTOR_COSE_ENCRYPT0: &[u8] = &[
+        131, 88, 25, 162, 1, 58, 0, 1, 17, 111, 4, 80, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
+        13, 14, 15, 161, 5, 88, 24, 39, 48, 159, 48, 215, 77, 21, 100, 241, 209, 216, 65, 99, 221,
+        83, 63, 118, 204, 200, 175, 126, 202, 53, 33, 88, 35, 218, 136, 132, 223, 131, 246, 169,
+        120, 134, 49, 56, 173, 169, 133, 232, 109, 248, 101, 59, 226, 90, 97, 210, 181, 76, 68,
+        158, 159, 94, 65, 67, 23, 112, 253, 83,
+    ];
+
     #[test]
     fn test_encrypt_decrypt_roundtrip() {
         let SymmetricCryptoKey::XChaCha20Poly1305Key(ref key) =
@@ -129,4 +149,50 @@ mod test {
         let decrypted = decrypt_xchacha20_poly1305(&encrypted, key).unwrap();
         assert_eq!(decrypted, plaintext);
     }
+
+    #[test]
+    fn test_decrypt_test_vector() {
+        let key = XChaCha20Poly1305Key {
+            key_id: KEY_ID,
+            enc_key: Box::pin(*GenericArray::from_slice(&KEY_DATA)),
+        };
+        let decrypted = decrypt_xchacha20_poly1305(TEST_VECTOR_COSE_ENCRYPT0, &key).unwrap();
+        assert_eq!(decrypted, TEST_VECTOR_PLAINTEXT);
+    }
+
+    #[test]
+    fn test_fail_wrong_key_id() {
+        let key = XChaCha20Poly1305Key {
+            key_id: [1; 16], // Different key ID
+            enc_key: Box::pin(*GenericArray::from_slice(&KEY_DATA)),
+        };
+        assert!(matches!(
+            decrypt_xchacha20_poly1305(TEST_VECTOR_COSE_ENCRYPT0, &key),
+            Err(CryptoError::WrongCoseKeyId)
+        ));
+    }
+
+    #[test]
+    fn test_fail_wrong_algorithm() {
+        let protected_header = coset::HeaderBuilder::new()
+            .algorithm(iana::Algorithm::A256GCM)
+            .key_id(KEY_ID.to_vec())
+            .build();
+        let nonce = [0u8; 16];
+        let cose_encrypt0 = coset::CoseEncrypt0Builder::new()
+            .protected(protected_header)
+            .create_ciphertext(&[], &[], |_, _| Vec::new())
+            .unprotected(coset::HeaderBuilder::new().iv(nonce.to_vec()).build())
+            .build();
+        let serialized_message = cose_encrypt0.to_vec().unwrap();
+
+        let key = XChaCha20Poly1305Key {
+            key_id: KEY_ID,
+            enc_key: Box::pin(*GenericArray::from_slice(&KEY_DATA)),
+        };
+        assert!(matches!(
+            decrypt_xchacha20_poly1305(&serialized_message, &key),
+            Err(CryptoError::WrongKeyType)
+        ));
+    }
 }

crates/bitwarden-crypto/src/error.rs

@@ -55,6 +55,9 @@ pub enum CryptoError {
     #[error("Key algorithm does not match encrypted data type")]
     WrongKeyType,
 
+    #[error("Key ID in the COSE Encrypt0 message does not match the key ID in the key")]
+    WrongCoseKeyId,
+
     #[error("Invalid nonce length")]
     InvalidNonceLength,
 }