client: Add new Mac struct for constant time eq

client/src/crypto/native.rs

@@ -123,20 +123,20 @@ pub(crate) fn mac_len() -> usize {
     MacAlg::output_size()
 }
 
-pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result<Vec<u8>, super::Error> {
+pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result<crate::Mac, super::Error> {
     let mut mac = MacAlg::new_from_slice(key.as_ref()).unwrap();
     mac.update(data.as_ref());
-    Ok(mac.finalize().into_bytes().to_vec())
+    Ok(crate::Mac::new(mac.finalize().into_bytes().to_vec()))
 }
 
 pub(crate) fn verify_mac(
     data: impl AsRef<[u8]>,
     key: &Key,
-    expected: impl AsRef<[u8]>,
+    expected_mac: impl AsRef<[u8]>,
 ) -> Result<bool, super::Error> {
     let mut mac = MacAlg::new_from_slice(key.as_ref()).unwrap();
     mac.update(data.as_ref());
-    Ok(mac.verify_slice(expected.as_ref()).is_ok())
+    Ok(mac.verify_slice(expected_mac.as_ref()).is_ok())
 }
 
 pub(crate) fn verify_checksum_md5(digest: impl AsRef<[u8]>, content: impl AsRef<[u8]>) -> bool {

client/src/crypto/openssl.rs

@@ -14,7 +14,7 @@ use openssl::{
 };
 use zeroize::Zeroizing;
 
-use crate::{Key, file};
+use crate::{Key, Mac, file};
 
 const ENC_ALG: Nid = Nid::AES_128_CBC;
 const MAC_ALG: Nid = Nid::SHA256;
@@ -142,22 +142,22 @@ pub(crate) fn mac_len() -> usize {
     md.size()
 }
 
-pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result<Vec<u8>, super::Error> {
+pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result<Mac, super::Error> {
     let md = MessageDigest::from_nid(MAC_ALG).unwrap();
     let mac_key = PKey::hmac(key.as_ref())?;
     let mut signer = Signer::new(md, &mac_key)?;
     signer.update(data.as_ref())?;
-    signer.sign_to_vec().map_err(From::from)
+    signer.sign_to_vec().map_err(From::from).map(Mac::new)
 }
 
 pub(crate) fn verify_mac(
     data: impl AsRef<[u8]>,
     key: &Key,
-    expected: impl AsRef<[u8]>,
+    expected_mac: impl AsRef<[u8]>,
 ) -> Result<bool, super::Error> {
     Ok(memcmp::eq(
         compute_mac(&data, key)?.as_slice(),
-        expected.as_ref(),
+        expected_mac.as_ref(),
     ))
 }
 

client/src/file/api/attribute_value.rs

@@ -2,14 +2,14 @@ use serde::{Deserialize, Serialize};
 use zbus::zvariant::Type;
 use zeroize::{Zeroize, ZeroizeOnDrop};
 
-use crate::{Key, crypto};
+use crate::{Key, Mac, crypto};
 
 /// An attribute value.
 #[derive(Deserialize, Serialize, Type, Clone, Debug, Eq, PartialEq, Zeroize, ZeroizeOnDrop)]
 pub struct AttributeValue(String);
 
 impl AttributeValue {
-    pub(crate) fn mac(&self, key: &Key) -> Result<Vec<u8>, crate::crypto::Error> {
+    pub(crate) fn mac(&self, key: &Key) -> Result<Mac, crate::crypto::Error> {
         crypto::compute_mac(self.0.as_bytes(), key)
     }
 }

client/src/file/api/encrypted_item.rs

@@ -4,17 +4,17 @@ use serde::{Deserialize, Serialize};
 use zbus::zvariant::Type;
 
 use super::{Error, Item};
-use crate::{Key, crypto};
+use crate::{Key, Mac, crypto};
 
 #[derive(Deserialize, Serialize, Type, Debug, Clone)]
 pub(crate) struct EncryptedItem {
-    pub(crate) hashed_attributes: HashMap<String, Vec<u8>>,
+    pub(crate) hashed_attributes: HashMap<String, Mac>,
     pub(crate) blob: Vec<u8>,
 }
 
 impl EncryptedItem {
-    pub fn has_attribute(&self, key: &str, value_mac: &[u8]) -> bool {
-        self.hashed_attributes.get(key).map(|b| b.as_slice()) == Some(value_mac)
+    pub fn has_attribute(&self, key: &str, value_mac: &Mac) -> bool {
+        self.hashed_attributes.get(key) == Some(value_mac)
     }
 
     pub fn decrypt(self, key: &Key) -> Result<Item, Error> {
@@ -43,13 +43,17 @@ impl EncryptedItem {
     }
 
     fn validate(
-        hashed_attributes: &HashMap<String, Vec<u8>>,
+        hashed_attributes: &HashMap<String, Mac>,
         item: &Item,
         key: &Key,
     ) -> Result<(), Error> {
         for (attribute_key, hashed_attribute) in hashed_attributes.iter() {
             if let Some(attribute_plaintext) = item.attributes().get(attribute_key) {
-                if !crypto::verify_mac(attribute_plaintext.as_bytes(), key, hashed_attribute)? {
+                if !crypto::verify_mac(
+                    attribute_plaintext.as_bytes(),
+                    key,
+                    hashed_attribute.as_slice(),
+                )? {
                     return Err(Error::HashedAttributeMac(attribute_key.to_owned()));
                 }
             } else {

client/src/file/item.rs

@@ -155,8 +155,8 @@ impl Item {
         let mut blob = crypto::encrypt(&*decrypted, key, iv)?;
 
         blob.extend_from_slice(iv);
-        let mut mac = crypto::compute_mac(&blob, key)?;
-        blob.append(&mut mac);
+        let mac = crypto::compute_mac(&blob, key)?;
+        blob.extend_from_slice(mac.as_slice());
 
         let hashed_attributes = self
             .attributes
@@ -228,7 +228,7 @@ mod tests {
         let encrypted_item_blob = &encrypted.blob[..n - n_mac - n_iv];
         let item_mac = crypto::compute_mac(&encrypted.blob[..n - n_mac], &key).unwrap();
 
-        assert_eq!(&blob[n - n_mac..], &item_mac);
+        assert_eq!(&blob[n - n_mac..], item_mac.as_slice());
         assert_eq!(&blob[n - n_mac - n_iv..n - n_mac], &iv);
         assert_eq!(
             encrypted_item_blob,

client/src/lib.rs

@@ -18,13 +18,15 @@ use std::collections::HashMap;
 
 mod error;
 mod key;
+mod mac;
 mod migration;
 
 #[cfg(feature = "unstable")]
 #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))]
 pub use key::Key;
 #[cfg(not(feature = "unstable"))]
 pub(crate) use key::Key;
+pub use mac::Mac;
 
 #[cfg(not(feature = "unstable"))]
 mod crypto;
@@ -63,7 +65,7 @@ pub trait AsAttributes {
     fn hash<'a>(
         &'a self,
         key: &Key,
-    ) -> Vec<(&'a str, std::result::Result<Vec<u8>, crate::crypto::Error>)> {
+    ) -> Vec<(&'a str, std::result::Result<Mac, crate::crypto::Error>)> {
         self.as_attributes()
             .into_iter()
             .map(|(k, v)| (k, crate::file::AttributeValue::from(v).mac(key)))

client/src/mac.rs

@@ -0,0 +1,39 @@
+use serde::{Deserialize, Serialize};
+#[cfg(feature = "native_crypto")]
+use subtle::ConstantTimeEq;
+use zbus::zvariant::Type;
+
+// There is no constructor to avoid performing sanity checks, e.g. length.
+/// A message authentication code. It provides constant-time comparison when
+/// compared against another mac or against a slice of bytes.
+#[derive(Deserialize, Serialize, Type, Debug, Clone)]
+pub struct Mac(Vec<u8>);
+
+impl Mac {
+    pub(crate) fn new(inner: Vec<u8>) -> Self {
+        Mac(inner)
+    }
+
+    /// Constant-time comparison against a slice of bytes.
+    pub fn verify_slice(&self, other: &[u8]) -> bool {
+        #[cfg(feature = "native_crypto")]
+        {
+            self.0.ct_eq(other).into()
+        }
+        #[cfg(feature = "openssl_crypto")]
+        {
+            openssl::memcmp::eq(&self.0, other)
+        }
+    }
+
+    // This is made private to prevent non-constant-time comparisons.
+    pub(crate) fn as_slice(&self) -> &[u8] {
+        self.0.as_slice()
+    }
+}
+
+impl PartialEq for Mac {
+    fn eq(&self, other: &Self) -> bool {
+        self.verify_slice(&other.0)
+    }
+}