[PM-12023] Enforce hmac when mac key is present (#1041)

## 🎟️ Tracking

https://bitwarden.atlassian.net/issues/PM-12023
(https://bitwarden.atlassian.net/issues/PM-4185)

## 📔 Objective

Prevent downgrading of encstring types to hmac-less by enforcing hmac
when the symmetric key used has a mac key.

## ⏰ 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: Oscar Hinton <[email protected]>

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

@@ -237,6 +237,10 @@ impl KeyDecryptable<SymmetricCryptoKey, Vec<u8>> for EncString {
     fn decrypt_with_key(&self, key: &SymmetricCryptoKey) -> Result<Vec<u8>> {
         match self {
             EncString::AesCbc256_B64 { iv, data } => {
+                if key.mac_key.is_some() {
+                    return Err(CryptoError::MacNotProvided);
+                }
+
                 let dec = crate::aes::decrypt_aes256(iv, data.clone(), &key.key)?;
                 Ok(dec)
             }
@@ -296,7 +300,9 @@ mod tests {
     use schemars::schema_for;
 
     use super::EncString;
-    use crate::{derive_symmetric_key, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey};
+    use crate::{
+        derive_symmetric_key, CryptoError, KeyDecryptable, KeyEncryptable, SymmetricCryptoKey,
+    };
 
     #[test]
     fn test_enc_string_roundtrip() {
@@ -418,6 +424,24 @@ mod tests {
         assert_eq!(dec_str, "EncryptMe!");
     }
 
+    #[test]
+    fn test_decrypt_downgrade_encstring_prevention() {
+        // Simulate a potential downgrade attack by removing the mac portion of the `EncString` and
+        // attempt to decrypt it using a `SymmetricCryptoKey` with a mac key.
+        let key = "hvBMMb1t79YssFZkpetYsM3deyVuQv4r88Uj9gvYe0+G8EwxvW3v1iywVmSl61iwzd17JW5C/ivzxSP2C9h7Tw==".to_string();
+        let key = SymmetricCryptoKey::try_from(key).unwrap();
+
+        // A "downgraded" `EncString` from `EncString::AesCbc256_HmacSha256_B64` (2) to
+        // `EncString::AesCbc256_B64` (0), with the mac portion removed.
+        // <enc_string>
+        let enc_str = "0.NQfjHLr6za7VQVAbrpL81w==|wfrjmyJ0bfwkQlySrhw8dA==";
+        let enc_string: EncString = enc_str.parse().unwrap();
+        assert_eq!(enc_string.enc_type(), 0);
+
+        let result: Result<String, CryptoError> = enc_string.decrypt_with_key(&key);
+        assert!(matches!(result, Err(CryptoError::MacNotProvided)));
+    }
+
     #[test]
     fn test_decrypt_cbc128_hmac() {
         let key = "Gt1aZ8kTTgkF80bLtb7LiMZBcxEA2FA5mbvV4x7K208=".to_string();

crates/bitwarden-crypto/src/error.rs

@@ -11,6 +11,8 @@ pub enum CryptoError {
     InvalidKey,
     #[error("The cipher's MAC doesn't match the expected value")]
     InvalidMac,
+    #[error("The key provided expects mac protected encstrings, but the mac is missing")]
+    MacNotProvided,
     #[error("Error while decrypting EncString")]
     KeyDecrypt,
     #[error("The cipher key has an invalid length")]