diff --git a/client/src/crypto/native.rs b/client/src/crypto/native.rs index 1078fbe75..ef1b746e2 100644 --- a/client/src/crypto/native.rs +++ b/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, super::Error> { +pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result { 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 { 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 { diff --git a/client/src/crypto/openssl.rs b/client/src/crypto/openssl.rs index 1b6ba2a96..fcd18e46f 100644 --- a/client/src/crypto/openssl.rs +++ b/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, super::Error> { +pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result { 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 { Ok(memcmp::eq( compute_mac(&data, key)?.as_slice(), - expected.as_ref(), + expected_mac.as_ref(), )) } diff --git a/client/src/file/api/attribute_value.rs b/client/src/file/api/attribute_value.rs index eb6790545..74e8e29ec 100644 --- a/client/src/file/api/attribute_value.rs +++ b/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, crate::crypto::Error> { + pub(crate) fn mac(&self, key: &Key) -> Result { crypto::compute_mac(self.0.as_bytes(), key) } } diff --git a/client/src/file/api/encrypted_item.rs b/client/src/file/api/encrypted_item.rs index f3e4a1197..6d801b686 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/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>, + pub(crate) hashed_attributes: HashMap, pub(crate) blob: Vec, } 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 { @@ -43,13 +43,17 @@ impl EncryptedItem { } fn validate( - hashed_attributes: &HashMap>, + hashed_attributes: &HashMap, 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 { diff --git a/client/src/file/item.rs b/client/src/file/item.rs index ecf00a0a7..2a8c7af92 100644 --- a/client/src/file/item.rs +++ b/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, diff --git a/client/src/lib.rs b/client/src/lib.rs index ddef7c098..bb566f078 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -18,6 +18,7 @@ use std::collections::HashMap; mod error; mod key; +mod mac; mod migration; #[cfg(feature = "unstable")] @@ -25,6 +26,7 @@ mod migration; 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, crate::crypto::Error>)> { + ) -> Vec<(&'a str, std::result::Result)> { self.as_attributes() .into_iter() .map(|(k, v)| (k, crate::file::AttributeValue::from(v).mac(key))) diff --git a/client/src/mac.rs b/client/src/mac.rs new file mode 100644 index 000000000..088cd1c39 --- /dev/null +++ b/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); + +impl Mac { + pub(crate) fn new(inner: Vec) -> 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) + } +}