From 6a9e96c164510db7fdabc7a9cbac0f4cea647e56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Garci=CC=81a?=
 <dani-garcia@users.noreply.github.com>
Date: Fri, 17 Nov 2023 13:33:22 +0100
Subject: [PATCH] Improve MAC validation

---
 Cargo.lock                             |  1 +
 crates/bitwarden/Cargo.toml            |  1 +
 crates/bitwarden/src/crypto/aes_ops.rs | 11 ++++++-----
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 6a02ad594..5107d5e68 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -351,6 +351,7 @@ dependencies = [
  "serde_repr",
  "sha1",
  "sha2",
+ "subtle",
  "thiserror",
  "tokio",
  "uniffi",
diff --git a/crates/bitwarden/Cargo.toml b/crates/bitwarden/Cargo.toml
index 65db9db7d..c8b096ead 100644
--- a/crates/bitwarden/Cargo.toml
+++ b/crates/bitwarden/Cargo.toml
@@ -42,6 +42,7 @@ pbkdf2 = { version = ">=0.12.1, <0.13", default-features = false }
 argon2 = { version = ">=0.5.0, <0.6", features = [
     "alloc",
 ], default-features = false }
+subtle = ">=2.5.0, <3.0"
 rand = ">=0.8.5, <0.9"
 num-bigint = ">=0.4, <0.5"
 num-traits = ">=0.2.15, <0.3"
diff --git a/crates/bitwarden/src/crypto/aes_ops.rs b/crates/bitwarden/src/crypto/aes_ops.rs
index 132718349..f5d1c99da 100644
--- a/crates/bitwarden/src/crypto/aes_ops.rs
+++ b/crates/bitwarden/src/crypto/aes_ops.rs
@@ -14,6 +14,7 @@ use aes::cipher::{
 };
 use hmac::Mac;
 use rand::RngCore;
+use subtle::ConstantTimeEq;
 
 use crate::{
     crypto::{EncString, PbkdfSha256Hmac, PBKDF_SHA256_HMAC_OUT_SIZE},
@@ -48,8 +49,8 @@ pub fn decrypt_aes256_hmac(
     mac_key: GenericArray<u8, U32>,
     key: GenericArray<u8, U32>,
 ) -> Result<Vec<u8>> {
-    let res = validate_mac(&mac_key, iv, &data)?;
-    if res != *mac {
+    let res = generate_mac(&mac_key, iv, &data)?;
+    if res.ct_ne(mac).into() {
         return Err(CryptoError::InvalidMac.into());
     }
     decrypt_aes256(iv, data, key)
@@ -82,7 +83,7 @@ pub fn encrypt_aes256_hmac(
     key: GenericArray<u8, U32>,
 ) -> Result<EncString> {
     let (iv, data) = encrypt_aes256_internal(data_dec, key);
-    let mac = validate_mac(&mac_key, &iv, &data)?;
+    let mac = generate_mac(&mac_key, &iv, &data)?;
 
     Ok(EncString::AesCbc256_HmacSha256_B64 { iv, mac, data })
 }
@@ -101,8 +102,8 @@ fn encrypt_aes256_internal(data_dec: &[u8], key: GenericArray<u8, U32>) -> ([u8;
     (iv, data)
 }
 
-/// Validate a MAC using HMAC-SHA256.
-fn validate_mac(mac_key: &[u8], iv: &[u8], data: &[u8]) -> Result<[u8; 32]> {
+/// Generate a MAC using HMAC-SHA256.
+fn generate_mac(mac_key: &[u8], iv: &[u8], data: &[u8]) -> Result<[u8; 32]> {
     let mut hmac = PbkdfSha256Hmac::new_from_slice(mac_key).expect("HMAC can take key of any size");
     hmac.update(iv);
     hmac.update(data);