diff --git a/client/src/crypto/error.rs b/client/src/crypto/error.rs new file mode 100644 index 00000000..d4307f6d --- /dev/null +++ b/client/src/crypto/error.rs @@ -0,0 +1,55 @@ +/// Cryptography specific errors. +#[derive(Debug)] +pub enum Error { + #[cfg(feature = "openssl_crypto")] + Openssl(openssl::error::ErrorStack), + #[cfg(feature = "native_crypto")] + PadError(cipher::inout::PadError), + #[cfg(feature = "native_crypto")] + UnpadError(cipher::block_padding::UnpadError), +} + +#[cfg(feature = "openssl_crypto")] +impl From for Error { + fn from(value: openssl::error::ErrorStack) -> Self { + Self::Openssl(value) + } +} + +#[cfg(feature = "native_crypto")] +impl From for Error { + fn from(value: cipher::block_padding::UnpadError) -> Self { + Self::UnpadError(value) + } +} + +#[cfg(feature = "native_crypto")] +impl From for Error { + fn from(value: cipher::inout::PadError) -> Self { + Self::PadError(value) + } +} + +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + #[cfg(feature = "openssl_crypto")] + Self::Openssl(e) => Some(e), + #[cfg(feature = "native_crypto")] + Self::UnpadError(_) | Self::PadError(_) => None, + } + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + #[cfg(feature = "openssl_crypto")] + Self::Openssl(e) => f.write_fmt(format_args!("Openssl error: {e}")), + #[cfg(feature = "native_crypto")] + Self::UnpadError(e) => f.write_fmt(format_args!("Wrong padding error: {e}")), + #[cfg(feature = "native_crypto")] + Self::PadError(e) => f.write_fmt(format_args!("Wrong padding error: {e}")), + } + } +} diff --git a/client/src/crypto/mod.rs b/client/src/crypto/mod.rs index 52bdd11e..c0410357 100644 --- a/client/src/crypto/mod.rs +++ b/client/src/crypto/mod.rs @@ -7,6 +7,9 @@ pub(crate) use native::*; #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))] pub use native::*; +mod error; +pub use error::Error; + #[cfg(feature = "openssl_crypto")] mod openssl; #[cfg(all(feature = "openssl_crypto", not(feature = "unstable")))] @@ -33,10 +36,10 @@ mod test { 78, 82, 67, 158, 214, 102, 48, 109, 84, 107, 94, 54, 225, 29, 186, 246, ]; - let encrypted = encrypt(data, &aes_key, aes_iv); + let encrypted = encrypt(data, &aes_key, aes_iv).unwrap(); assert_eq!(encrypted, expected_encrypted); - let decrypted = decrypt(&encrypted, &aes_key, aes_iv); + let decrypted = decrypt(&encrypted, &aes_key, aes_iv).unwrap(); assert_eq!(decrypted.to_vec(), data); } @@ -53,7 +56,7 @@ mod test { let salt = &[0x92, 0xf4, 0xc0, 0x34, 0x0f, 0x5f, 0x36, 0xf9]; let iteration_count = 1782; let password = b"test"; - let (key, iv) = legacy_derive_key_and_iv(password, Ok(()), salt, iteration_count); + let (key, iv) = legacy_derive_key_and_iv(password, Ok(()), salt, iteration_count).unwrap(); assert_eq!(key.as_ref(), &expected_key[..]); assert_eq!(iv, &expected_iv[..]); } diff --git a/client/src/crypto/native.rs b/client/src/crypto/native.rs index f802688c..16642f31 100644 --- a/client/src/crypto/native.rs +++ b/client/src/crypto/native.rs @@ -22,69 +22,74 @@ type EncAlg = cbc::Encryptor; type DecAlg = cbc::Decryptor; type MacAlg = hmac::Hmac; -pub fn encrypt(data: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>) -> Vec { +pub fn encrypt( + data: impl AsRef<[u8]>, + key: &Key, + iv: impl AsRef<[u8]>, +) -> Result, super::Error> { let mut blob = vec![0; data.as_ref().len() + EncAlg::block_size()]; // Unwrapping since adding `CIPHER_BLOCK_SIZE` to array is enough space for // PKCS7 let encrypted_len = EncAlg::new_from_slices(key.as_ref(), iv.as_ref()) .expect("Invalid key length") - .encrypt_padded_b2b_mut::(data.as_ref(), &mut blob) - .unwrap() + .encrypt_padded_b2b_mut::(data.as_ref(), &mut blob)? .len(); blob.truncate(encrypted_len); - blob + Ok(blob) } -pub fn decrypt(blob: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>) -> Zeroizing> { +pub fn decrypt( + blob: impl AsRef<[u8]>, + key: &Key, + iv: impl AsRef<[u8]>, +) -> Result>, super::Error> { let mut data = blob.as_ref().to_vec(); - DecAlg::new_from_slices(key.as_ref(), iv.as_ref()) + Ok(DecAlg::new_from_slices(key.as_ref(), iv.as_ref()) .expect("Invalid key length") - .decrypt_padded_mut::(&mut data) - .unwrap() + .decrypt_padded_mut::(&mut data)? .to_vec() - .into() + .into()) } pub(crate) fn decrypt_no_padding( blob: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>, -) -> Zeroizing> { +) -> Result>, super::Error> { let mut data = blob.as_ref().to_vec(); - DecAlg::new_from_slices(key.as_ref(), iv.as_ref()) + Ok(DecAlg::new_from_slices(key.as_ref(), iv.as_ref()) .expect("Invalid key length") - .decrypt_padded_mut::(&mut data) - .unwrap() + .decrypt_padded_mut::(&mut data)? .to_vec() - .into() + .into()) } pub(crate) fn iv_len() -> usize { DecAlg::iv_size() } -pub(crate) fn generate_private_key() -> Zeroizing> { +pub(crate) fn generate_private_key() -> Result>, super::Error> { let generic_array = EncAlg::generate_key(cipher::rand_core::OsRng); - Zeroizing::new(generic_array.to_vec()) + Ok(Zeroizing::new(generic_array.to_vec())) } -pub(crate) fn generate_public_key(private_key: impl AsRef<[u8]>) -> Vec { +pub(crate) fn generate_public_key(private_key: impl AsRef<[u8]>) -> Result, super::Error> { let private_key_uint = BigUint::from_bytes_be(private_key.as_ref()); static DH_GENERATOR: LazyLock = LazyLock::new(|| BigUint::from_u64(0x2).unwrap()); let public_key_uint = powm(&DH_GENERATOR, private_key_uint); - public_key_uint.to_bytes_be() + Ok(public_key_uint.to_bytes_be()) } pub(crate) fn generate_aes_key( private_key: impl AsRef<[u8]>, server_public_key: impl AsRef<[u8]>, -) -> Zeroizing> { +) -> Result>, super::Error> { let server_public_key_uint = BigUint::from_bytes_be(server_public_key.as_ref()); let private_key_uint = BigUint::from_bytes_be(private_key.as_ref()); let common_secret = powm(&server_public_key_uint, private_key_uint); @@ -107,27 +112,31 @@ pub(crate) fn generate_aes_key( hk.expand(&info, okm.as_mut()) .expect("hkdf expand should never fail"); - okm + Ok(okm) } -pub fn generate_iv() -> Vec { - EncAlg::generate_iv(cipher::rand_core::OsRng).to_vec() +pub fn generate_iv() -> Result, super::Error> { + Ok(EncAlg::generate_iv(cipher::rand_core::OsRng).to_vec()) } pub(crate) fn mac_len() -> usize { MacAlg::output_size() } -pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Vec { +pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result, super::Error> { let mut mac = MacAlg::new_from_slice(key.as_ref()).unwrap(); mac.update(data.as_ref()); - mac.finalize().into_bytes().to_vec() + Ok(mac.finalize().into_bytes().to_vec()) } -pub(crate) fn verify_mac(data: impl AsRef<[u8]>, key: &Key, expected: impl AsRef<[u8]>) -> bool { +pub(crate) fn verify_mac( + data: impl AsRef<[u8]>, + key: &Key, + expected: impl AsRef<[u8]>, +) -> Result { let mut mac = MacAlg::new_from_slice(key.as_ref()).unwrap(); mac.update(data.as_ref()); - mac.verify_slice(expected.as_ref()).is_ok() + Ok(mac.verify_slice(expected.as_ref()).is_ok()) } pub(crate) fn verify_checksum_md5(digest: impl AsRef<[u8]>, content: impl AsRef<[u8]>) -> bool { @@ -141,7 +150,7 @@ pub(crate) fn derive_key( key_strength: Result<(), file::WeakKeyError>, salt: impl AsRef<[u8]>, iteration_count: usize, -) -> Key { +) -> Result { let mut key = Key::new_with_strength(vec![0; EncAlg::block_size()], key_strength); pbkdf2::pbkdf2::>( @@ -152,7 +161,7 @@ pub(crate) fn derive_key( ) .expect("HMAC can be initialized with any key length"); - key + Ok(key) } pub(crate) fn legacy_derive_key_and_iv( @@ -160,7 +169,7 @@ pub(crate) fn legacy_derive_key_and_iv( key_strength: Result<(), file::WeakKeyError>, salt: impl AsRef<[u8]>, iteration_count: usize, -) -> (Key, Vec) { +) -> Result<(Key, Vec), super::Error> { let mut buffer = vec![0; EncAlg::key_size() + EncAlg::iv_size()]; let mut hasher = Sha256::new(); let mut digest_buffer = vec![0; ::output_size()]; @@ -198,7 +207,7 @@ pub(crate) fn legacy_derive_key_and_iv( } let iv = buffer.split_off(EncAlg::key_size()); - (Key::new_with_strength(buffer, key_strength), iv) + Ok((Key::new_with_strength(buffer, key_strength), iv)) } /// from https://github.com/plietar/librespot/blob/master/core/src/util/mod.rs#L53 diff --git a/client/src/crypto/openssl.rs b/client/src/crypto/openssl.rs index 92808b59..d59d188a 100644 --- a/client/src/crypto/openssl.rs +++ b/client/src/crypto/openssl.rs @@ -19,7 +19,11 @@ use crate::{file, Key}; const ENC_ALG: Nid = Nid::AES_128_CBC; const MAC_ALG: Nid = Nid::SHA256; -pub fn encrypt(data: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>) -> Vec { +pub fn encrypt( + data: impl AsRef<[u8]>, + key: &Key, + iv: impl AsRef<[u8]>, +) -> Result, super::Error> { let cipher = Cipher::from_nid(ENC_ALG).unwrap(); let mut encryptor = Crypter::new(cipher, Mode::Encrypt, key.as_ref(), Some(iv.as_ref())) .expect("Invalid key or IV length"); @@ -28,12 +32,12 @@ pub fn encrypt(data: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>) -> Vec, pad: bool, -) -> Zeroizing> { +) -> Result>, super::Error> { let cipher = Cipher::from_nid(ENC_ALG).unwrap(); let mut decrypter = Crypter::new(cipher, Mode::Decrypt, key.as_ref(), Some(iv.as_ref())) .expect("Invalid key or IV length"); decrypter.pad(pad); let mut data = Zeroizing::new(vec![0; blob.as_ref().len() + cipher.block_size()]); - let mut decrypted_len = decrypter.update(blob.as_ref(), &mut data).unwrap(); - decrypted_len += decrypter.finalize(&mut data[decrypted_len..]).unwrap(); + let mut decrypted_len = decrypter.update(blob.as_ref(), &mut data)?; + decrypted_len += decrypter.finalize(&mut data[decrypted_len..])?; data.truncate(decrypted_len); - data + Ok(data) } -pub fn decrypt(blob: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>) -> Zeroizing> { +pub fn decrypt( + blob: impl AsRef<[u8]>, + key: &Key, + iv: impl AsRef<[u8]>, +) -> Result>, super::Error> { decrypt_with_padding(blob, key, iv, true) } @@ -64,7 +72,7 @@ pub(crate) fn decrypt_no_padding( blob: impl AsRef<[u8]>, key: &Key, iv: impl AsRef<[u8]>, -) -> Zeroizing> { +) -> Result>, super::Error> { decrypt_with_padding(blob, key, iv, false) } @@ -73,41 +81,37 @@ pub(crate) fn iv_len() -> usize { cipher.iv_len().unwrap() } -pub(crate) fn generate_private_key() -> Zeroizing> { +pub(crate) fn generate_private_key() -> Result>, super::Error> { let cipher = Cipher::from_nid(ENC_ALG).unwrap(); let mut buf = Zeroizing::new(vec![0; cipher.key_len()]); - // FIXME: should return an error? - rand_bytes(&mut buf).unwrap(); - buf + rand_bytes(&mut buf)?; + Ok(buf) } -pub(crate) fn generate_public_key(private_key: impl AsRef<[u8]>) -> Vec { +pub(crate) fn generate_public_key(private_key: impl AsRef<[u8]>) -> Result, super::Error> { let private_key_bn = BigNum::from_slice(private_key.as_ref()).unwrap(); - Dh::from_pqg( + let dh = Dh::from_pqg( BigNum::get_rfc2409_prime_1024().unwrap(), None, BigNum::from_u32(2).unwrap(), - ) - .and_then(|key| key.set_private_key(private_key_bn)) - .unwrap() - .public_key() - .to_vec() + )?; + Ok(dh.set_private_key(private_key_bn)?.public_key().to_vec()) } pub(crate) fn generate_aes_key( private_key: impl AsRef<[u8]>, server_public_key: impl AsRef<[u8]>, -) -> Zeroizing> { +) -> Result>, super::Error> { let private_key_bn = BigNum::from_slice(private_key.as_ref()).unwrap(); let server_public_key_bn = BigNum::from_slice(server_public_key.as_ref()).unwrap(); - let mut common_secret_bytes = Dh::from_pqg( + let dh = Dh::from_pqg( BigNum::get_rfc2409_prime_1024().unwrap(), None, BigNum::from_u32(2).unwrap(), - ) - .and_then(|key| key.set_private_key(private_key_bn)) - .and_then(|key| key.compute_key(&server_public_key_bn)) - .unwrap(); + )?; + let mut common_secret_bytes = dh + .set_private_key(private_key_bn)? + .compute_key(&server_public_key_bn)?; let mut common_secret_padded = vec![0; 128 - common_secret_bytes.len()]; // inefficient, but ok for now @@ -118,20 +122,19 @@ pub(crate) fn generate_aes_key( let ikm = common_secret_padded; let mut okm = Zeroizing::new(vec![0; 16]); - let mut ctx = PkeyCtx::new_id(Id::HKDF).unwrap(); - ctx.derive_init().unwrap(); - ctx.set_hkdf_md(Md::sha256()).unwrap(); - ctx.set_hkdf_key(&ikm).unwrap(); + let mut ctx = PkeyCtx::new_id(Id::HKDF)?; + ctx.derive_init()?; + ctx.set_hkdf_md(Md::sha256())?; + ctx.set_hkdf_key(&ikm)?; ctx.derive(Some(okm.as_mut())) .expect("hkdf expand should never fail"); - okm + Ok(okm) } -pub fn generate_iv() -> Vec { +pub fn generate_iv() -> Result, super::Error> { let mut buf = vec![0; iv_len()]; - // FIXME: should return an error? - rand_bytes(&mut buf).unwrap(); - buf + rand_bytes(&mut buf)?; + Ok(buf) } pub(crate) fn mac_len() -> usize { @@ -139,16 +142,23 @@ pub(crate) fn mac_len() -> usize { md.size() } -pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Vec { +pub(crate) fn compute_mac(data: impl AsRef<[u8]>, key: &Key) -> Result, super::Error> { let md = MessageDigest::from_nid(MAC_ALG).unwrap(); - let mac_key = PKey::hmac(key.as_ref()).unwrap(); - let mut signer = Signer::new(md, &mac_key).unwrap(); - signer.update(data.as_ref()).unwrap(); - signer.sign_to_vec().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) } -pub(crate) fn verify_mac(data: impl AsRef<[u8]>, key: &Key, expected: impl AsRef<[u8]>) -> bool { - memcmp::eq(compute_mac(&data, key).as_slice(), expected.as_ref()) +pub(crate) fn verify_mac( + data: impl AsRef<[u8]>, + key: &Key, + expected: impl AsRef<[u8]>, +) -> Result { + Ok(memcmp::eq( + compute_mac(&data, key)?.as_slice(), + expected.as_ref(), + )) } pub(crate) fn verify_checksum_md5(digest: impl AsRef<[u8]>, content: impl AsRef<[u8]>) -> bool { @@ -163,7 +173,7 @@ pub(crate) fn derive_key( key_strength: Result<(), file::WeakKeyError>, salt: impl AsRef<[u8]>, iteration_count: usize, -) -> Key { +) -> Result { let cipher = Cipher::from_nid(ENC_ALG).unwrap(); let mut key = Key::new_with_strength(vec![0; cipher.block_size()], key_strength); @@ -174,10 +184,9 @@ pub(crate) fn derive_key( iteration_count, md, key.as_mut(), - ) - .unwrap(); + )?; - key + Ok(key) } pub(crate) fn legacy_derive_key_and_iv( @@ -185,24 +194,24 @@ pub(crate) fn legacy_derive_key_and_iv( key_strength: Result<(), file::WeakKeyError>, salt: impl AsRef<[u8]>, iteration_count: usize, -) -> (Key, Vec) { +) -> Result<(Key, Vec), super::Error> { let cipher = Cipher::from_nid(ENC_ALG).unwrap(); let mut buffer = vec![0; cipher.key_len() + cipher.iv_len().unwrap()]; - let mut hasher = Hasher::new(MessageDigest::sha256()).unwrap(); + let mut hasher = Hasher::new(MessageDigest::sha256())?; let mut pos = 0usize; loop { - hasher.update(secret.as_ref()).unwrap(); - hasher.update(salt.as_ref()).unwrap(); - let mut digest = hasher.finish().unwrap(); + hasher.update(secret.as_ref())?; + hasher.update(salt.as_ref())?; + let mut digest = hasher.finish()?; for _ in 1..iteration_count { // We can't pass an instance, the borrow checker // would complain about digest being dropped at the end of // for block #[allow(clippy::needless_borrows_for_generic_args)] - hasher.update(&digest).unwrap(); - digest = hasher.finish().unwrap(); + hasher.update(&digest)?; + digest = hasher.finish()?; } let to_read = usize::min(digest.len(), buffer.len() - pos); @@ -217,9 +226,9 @@ pub(crate) fn legacy_derive_key_and_iv( // would complain about digest being dropped at the end of // for block #[allow(clippy::needless_borrows_for_generic_args)] - hasher.update(&digest).unwrap(); + hasher.update(&digest)?; } let iv = buffer.split_off(cipher.key_len()); - (Key::new_with_strength(buffer, key_strength), iv) + Ok((Key::new_with_strength(buffer, key_strength), iv)) } diff --git a/client/src/dbus/api/secret.rs b/client/src/dbus/api/secret.rs index 5df7e64e..5fb55386 100644 --- a/client/src/dbus/api/secret.rs +++ b/client/src/dbus/api/secret.rs @@ -43,15 +43,15 @@ impl<'a> DBusSecret<'a> { session: Arc>, secret: impl Into, aes_key: &Key, - ) -> Self { - let iv = crypto::generate_iv(); + ) -> Result { + let iv = crypto::generate_iv()?; let secret = secret.into(); - Self { + Ok(Self { session, - value: crypto::encrypt(secret.as_bytes(), aes_key, &iv), + value: crypto::encrypt(secret.as_bytes(), aes_key, &iv)?, parameters: iv, content_type: secret.content_type().to_owned(), - } + }) } pub(crate) async fn from_inner( @@ -68,7 +68,7 @@ impl<'a> DBusSecret<'a> { pub(crate) fn decrypt(&self, key: Option<&Arc>) -> Result { let value = match key { - Some(key) => &crypto::decrypt(&self.value, key, &self.parameters), + Some(key) => &crypto::decrypt(&self.value, key, &self.parameters)?, None => &self.value, }; diff --git a/client/src/dbus/collection.rs b/client/src/dbus/collection.rs index f1338a0e..4971c70d 100644 --- a/client/src/dbus/collection.rs +++ b/client/src/dbus/collection.rs @@ -168,7 +168,7 @@ impl<'a> Collection<'a> { Arc::clone(&self.session), secret, self.aes_key.as_ref().unwrap(), - ), + )?, }; let item = self .inner diff --git a/client/src/dbus/error.rs b/client/src/dbus/error.rs index 23890de3..97ab31bd 100644 --- a/client/src/dbus/error.rs +++ b/client/src/dbus/error.rs @@ -33,6 +33,8 @@ pub enum Error { IO(std::io::Error), /// Secret to string conversion failure. Utf8(FromUtf8Error), + /// Crypto related error. + Crypto(crate::crypto::Error), } impl From for Error { @@ -71,6 +73,12 @@ impl From for Error { } } +impl From for Error { + fn from(value: crate::crypto::Error) -> Self { + Self::Crypto(value) + } +} + impl std::error::Error for Error {} impl fmt::Display for Error { @@ -83,6 +91,7 @@ impl fmt::Display for Error { Self::NotFound(name) => write!(f, "The collection '{name}' doesn't exists"), Self::Dismissed => write!(f, "Prompt was dismissed"), Self::Utf8(e) => write!(f, "Failed to convert a text/plain secret to string, {e}"), + Self::Crypto(e) => write!(f, "Failed to do a cryptography operation, {e}"), } } } diff --git a/client/src/dbus/item.rs b/client/src/dbus/item.rs index 8bf6a1b0..19e746ff 100644 --- a/client/src/dbus/item.rs +++ b/client/src/dbus/item.rs @@ -155,7 +155,7 @@ impl<'a> Item<'a> { Algorithm::Plain => api::DBusSecret::new(Arc::clone(&self.session), secret), Algorithm::Encrypted => { let aes_key = self.aes_key.as_ref().unwrap(); - api::DBusSecret::new_encrypted(Arc::clone(&self.session), secret, aes_key) + api::DBusSecret::new_encrypted(Arc::clone(&self.session), secret, aes_key)? } }; self.inner.set_secret(&secret).await?; diff --git a/client/src/dbus/service.rs b/client/src/dbus/service.rs index 16bc1e4b..d18c6769 100644 --- a/client/src/dbus/service.rs +++ b/client/src/dbus/service.rs @@ -82,11 +82,13 @@ impl<'a> Service<'a> { Algorithm::Encrypted => { #[cfg(feature = "tracing")] tracing::debug!("Starting an encrypted Secret Service session"); - let private_key = Key::generate_private_key(); - let public_key = Key::generate_public_key(&private_key); + let private_key = Key::generate_private_key()?; + let public_key = Key::generate_public_key(&private_key)?; let (service_key, session) = service.open_session(Some(&public_key)).await?; let aes_key = service_key - .map(|service_key| Arc::new(Key::generate_aes_key(&private_key, &service_key))); + .map(|service_key| Key::generate_aes_key(&private_key, &service_key)) + .transpose()? + .map(Arc::new); (aes_key, session) } diff --git a/client/src/file/api/attribute_value.rs b/client/src/file/api/attribute_value.rs index db51d719..5a1142ab 100644 --- a/client/src/file/api/attribute_value.rs +++ b/client/src/file/api/attribute_value.rs @@ -9,8 +9,8 @@ use crate::{crypto, Key}; pub struct AttributeValue(String); impl AttributeValue { - pub(crate) fn mac(&self, key: &Key) -> Zeroizing> { - Zeroizing::new(crypto::compute_mac(self.0.as_bytes(), key)) + pub(crate) fn mac(&self, key: &Key) -> Result>, crate::crypto::Error> { + Ok(Zeroizing::new(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 b2fdd6cc..d67d4581 100644 --- a/client/src/file/api/encrypted_item.rs +++ b/client/src/file/api/encrypted_item.rs @@ -21,14 +21,14 @@ impl EncryptedItem { let mac_tag = self.blob.split_off(self.blob.len() - crypto::mac_len()); // verify item - if !crypto::verify_mac(&self.blob, key, mac_tag) { + if !crypto::verify_mac(&self.blob, key, mac_tag)? { return Err(Error::MacError); } let iv = self.blob.split_off(self.blob.len() - crypto::iv_len()); // decrypt item - let decrypted = crypto::decrypt(self.blob, key, iv); + let decrypted = crypto::decrypt(self.blob, key, iv)?; let item = Item::try_from(decrypted.as_slice())?; @@ -44,7 +44,7 @@ impl EncryptedItem { ) -> 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)? { return Err(Error::HashedAttributeMac(attribute_key.to_owned())); } } else { diff --git a/client/src/file/api/legacy_keyring.rs b/client/src/file/api/legacy_keyring.rs index 54a1298f..72332cbc 100644 --- a/client/src/file/api/legacy_keyring.rs +++ b/client/src/file/api/legacy_keyring.rs @@ -35,8 +35,8 @@ impl Keyring { self.key_strength(secret), &self.salt, self.iteration_count.try_into().unwrap(), - ); - let decrypted = crypto::decrypt_no_padding(&self.encrypted_content, &key, iv); + )?; + let decrypted = crypto::decrypt_no_padding(&self.encrypted_content, &key, iv)?; let (digest, content) = decrypted.split_at(16); if !crypto::verify_checksum_md5(digest, content) { return Err(Error::ChecksumMismatch); diff --git a/client/src/file/api/mod.rs b/client/src/file/api/mod.rs index 01f84f76..4af7156f 100644 --- a/client/src/file/api/mod.rs +++ b/client/src/file/api/mod.rs @@ -190,7 +190,11 @@ impl Keyring { self.items .iter() - .filter(|e| hashed_search.iter().all(|(k, v)| e.has_attribute(k, v))) + .filter(|e| { + hashed_search + .iter() + .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k, v))) + }) .map(|e| (*e).clone().decrypt(key)) .collect() } @@ -204,7 +208,11 @@ impl Keyring { self.items .iter() - .find(|e| hashed_search.iter().all(|(k, v)| e.has_attribute(k, v))) + .find(|e| { + hashed_search + .iter() + .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k, v))) + }) .map(|e| (*e).clone().decrypt(key)) .transpose() } @@ -212,19 +220,22 @@ impl Keyring { pub fn lookup_item_index(&self, attributes: &impl AsAttributes, key: &Key) -> Option { let hashed_search = attributes.hash(key); - self.items - .iter() - .position(|e| hashed_search.iter().all(|(k, v)| e.has_attribute(k, v))) + self.items.iter().position(|e| { + hashed_search + .iter() + .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k, v))) + }) } pub fn remove_items(&mut self, attributes: &impl AsAttributes, key: &Key) -> Result<(), Error> { let hashed_search = attributes.hash(key); - let (remove, keep): (Vec, _) = self - .items - .clone() - .into_iter() - .partition(|e| hashed_search.iter().all(|(k, v)| e.has_attribute(k, v))); + let (remove, keep): (Vec, _) = + self.items.clone().into_iter().partition(|e| { + hashed_search + .iter() + .all(|(k, v)| v.as_ref().is_ok_and(|v| e.has_attribute(k, v))) + }); // check hashes for the ones to be removed for item in remove { @@ -263,7 +274,7 @@ impl Keyring { Self::path("default", LEGACY_MAJOR_VERSION) } - pub fn derive_key(&self, secret: &Secret) -> Key { + pub fn derive_key(&self, secret: &Secret) -> Result { crypto::derive_key( &**secret, self.key_strength(secret), @@ -337,7 +348,7 @@ mod tests { let needle = HashMap::from([("key", "value")]); let mut keyring = Keyring::new(); - let key = keyring.derive_key(&SECRET.to_vec().into()); + let key = keyring.derive_key(&SECRET.to_vec().into())?; keyring .items @@ -357,7 +368,7 @@ mod tests { let _silent = std::fs::remove_file("/tmp/test.keyring"); let mut new_keyring = Keyring::new(); - let key = new_keyring.derive_key(&SECRET.to_vec().into()); + let key = new_keyring.derive_key(&SECRET.to_vec().into())?; new_keyring.items.push( Item::new( diff --git a/client/src/file/error.rs b/client/src/file/error.rs index 5c04f1c3..a6895c57 100644 --- a/client/src/file/error.rs +++ b/client/src/file/error.rs @@ -37,6 +37,8 @@ pub enum Error { AlgorithmMismatch(u8), /// Incorrect secret IncorrectSecret, + /// Crypto related error. + Crypto(crate::crypto::Error), } impl From for Error { @@ -69,39 +71,46 @@ impl From for Error { } } +impl From for Error { + fn from(value: crate::crypto::Error) -> Self { + Self::Crypto(value) + } +} + impl std::error::Error for Error {} impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Error::FileHeaderMismatch(e) => { + Self::FileHeaderMismatch(e) => { write!(f, "File header doesn't match FILE_HEADER {e:#?}") } - Error::VersionMismatch(e) => write!( + Self::VersionMismatch(e) => write!( f, "Version doesn't match MAJOR_VERSION OR MICRO_VERSION {e:#?}", ), - Error::NoData => write!(f, "No data behind header and version bytes"), - Error::NoParentDir(e) => write!(f, "No Parent Directory {e}"), - Error::GVariantDeserialization(e) => write!(f, "Failed to deserialize {e}"), - Error::SaltSizeMismatch(arr, explicit) => write!( + Self::NoData => write!(f, "No data behind header and version bytes"), + Self::NoParentDir(e) => write!(f, "No Parent Directory {e}"), + Self::GVariantDeserialization(e) => write!(f, "Failed to deserialize {e}"), + Self::SaltSizeMismatch(arr, explicit) => write!( f, "Salt size is not as expected. Array: {arr}, Explicit: {explicit}" ), - Error::WeakKey(err) => write!(f, "{err}"), - Error::Io(e) => write!(f, "IO error {e}"), - Error::MacError => write!(f, "Mac digest is not equal to the expected value"), - Error::ChecksumMismatch => write!(f, "Checksum is not equal to the expected value"), - Error::HashedAttributeMac(e) => write!(f, "Failed to validate hashed attribute {e}"), - Error::NoDataDir => write!(f, "Couldn't retrieve XDG_DATA_DIR"), - Error::TargetFileChanged(e) => write!(f, "The target file has changed {e}"), - Error::Portal(e) => write!(f, "Portal communication failed {e}"), - Error::InvalidItemIndex(index) => { + Self::WeakKey(err) => write!(f, "{err}"), + Self::Io(e) => write!(f, "IO error {e}"), + Self::MacError => write!(f, "Mac digest is not equal to the expected value"), + Self::ChecksumMismatch => write!(f, "Checksum is not equal to the expected value"), + Self::HashedAttributeMac(e) => write!(f, "Failed to validate hashed attribute {e}"), + Self::NoDataDir => write!(f, "Couldn't retrieve XDG_DATA_DIR"), + Self::TargetFileChanged(e) => write!(f, "The target file has changed {e}"), + Self::Portal(e) => write!(f, "Portal communication failed {e}"), + Self::InvalidItemIndex(index) => { write!(f, "The addressed item index {index} does not exist") } - Error::Utf8(e) => write!(f, "UTF-8 encoding error {e}"), - Error::AlgorithmMismatch(e) => write!(f, "Unknown algorithm {e}"), - Error::IncorrectSecret => write!(f, "Incorrect secret"), + Self::Utf8(e) => write!(f, "UTF-8 encoding error {e}"), + Self::AlgorithmMismatch(e) => write!(f, "Unknown algorithm {e}"), + Self::IncorrectSecret => write!(f, "Incorrect secret"), + Self::Crypto(e) => write!(f, "Failed to do a cryptography operation, {e}"), } } } diff --git a/client/src/file/item.rs b/client/src/file/item.rs index 5d2e8d6f..65ae1beb 100644 --- a/client/src/file/item.rs +++ b/client/src/file/item.rs @@ -106,17 +106,17 @@ impl Item { let decrypted = Zeroizing::new(zvariant::to_bytes(*GVARIANT_ENCODING, &self)?.to_vec()); - let iv = crypto::generate_iv(); + let iv = crypto::generate_iv()?; - let mut blob = crypto::encrypt(&*decrypted, key, &iv); + let mut blob = crypto::encrypt(&*decrypted, key, &iv)?; blob.append(&mut iv.as_slice().into()); - blob.append(&mut crypto::compute_mac(&blob, key).as_slice().into()); + blob.append(&mut crypto::compute_mac(&blob, key)?.as_slice().into()); let hashed_attributes = self .attributes .iter() - .map(|(k, v)| (k.to_owned(), v.mac(key).as_slice().into())) + .filter_map(|(k, v)| Some((k.to_owned(), v.mac(key).ok()?.as_slice().into()))) .collect(); Ok(EncryptedItem { diff --git a/client/src/file/mod.rs b/client/src/file/mod.rs index 03b4a7cb..0716ffc5 100644 --- a/client/src/file/mod.rs +++ b/client/src/file/mod.rs @@ -118,7 +118,7 @@ impl Keyring { } }; - let key = keyring.derive_key(&secret); + let key = keyring.derive_key(&secret)?; for encrypted_item in &mut keyring.items { if encrypted_item.clone().decrypt(&key).is_err() { return Err(Error::IncorrectSecret); @@ -171,7 +171,7 @@ impl Keyring { let legacy_keyring = api::LegacyKeyring::try_from(content.as_slice())?; let mut keyring = api::Keyring::new(); - let key = keyring.derive_key(&secret); + let key = keyring.derive_key(&secret)?; for item in legacy_keyring.decrypt_items(&secret)? { let encrypted_item = item.encrypt(&key)?; @@ -236,10 +236,10 @@ impl Keyring { /// /// If items cannot be decrypted, [`InvalidItemError`]s are returned for /// them instead of [`Item`]s. - pub async fn items(&self) -> Vec> { - let key = self.derive_key().await; + pub async fn items(&self) -> Result>, Error> { + let key = self.derive_key().await?; let keyring = self.keyring.read().await; - keyring + Ok(keyring .items .iter() .map(|e| { @@ -250,35 +250,38 @@ impl Keyring { ) }) }) - .collect() + .collect()) } /// Search items matching the attributes. pub async fn search_items(&self, attributes: &impl AsAttributes) -> Result, Error> { - let key = self.derive_key().await; + let key = self.derive_key().await?; let keyring = self.keyring.read().await; keyring.search_items(attributes, &key) } /// Find the first item matching the attributes. pub async fn lookup_item(&self, attributes: &impl AsAttributes) -> Result, Error> { - let key = self.derive_key().await; + let key = self.derive_key().await?; let keyring = self.keyring.read().await; keyring.lookup_item(attributes, &key) } /// Find the index in the list of items of the first item matching the /// attributes. - pub async fn lookup_item_index(&self, attributes: &impl AsAttributes) -> Option { - let key = self.derive_key().await; + pub async fn lookup_item_index( + &self, + attributes: &impl AsAttributes, + ) -> Result, Error> { + let key = self.derive_key().await?; let keyring = self.keyring.read().await; - keyring.lookup_item_index(attributes, &key) + Ok(keyring.lookup_item_index(attributes, &key)) } /// Delete an item. pub async fn delete(&self, attributes: &impl AsAttributes) -> Result<(), Error> { { - let key = self.derive_key().await; + let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; keyring.remove_items(attributes, &key)?; }; @@ -303,7 +306,7 @@ impl Keyring { replace: bool, ) -> Result { let item = { - let key = self.derive_key().await; + let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; if replace { keyring.remove_items(attributes, &key)?; @@ -326,7 +329,7 @@ impl Keyring { /// returns an error. pub async fn replace_item_index(&self, index: usize, item: &Item) -> Result<(), Error> { { - let key = self.derive_key().await; + let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; if let Some(item_store) = keyring.items.get_mut(index) { @@ -358,7 +361,7 @@ impl Keyring { /// Helper used for migration to avoid re-writing the file multiple times pub(crate) async fn create_items(&self, items: Vec) -> Result<(), Error> { - let key = self.derive_key().await; + let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; for (label, attributes, secret, replace) in items { if replace { @@ -402,7 +405,7 @@ impl Keyring { } /// Return key, derive and store it first if not initialized - async fn derive_key(&self) -> Arc { + async fn derive_key(&self) -> Result, crate::crypto::Error> { let keyring = Arc::clone(&self.keyring); let secret_lock = self.secret.lock().await; let secret = Arc::clone(&secret_lock); @@ -414,17 +417,17 @@ impl Keyring { let key = blocking::unblock(move || { async_io::block_on(async { keyring.read().await.derive_key(&secret) }) }) - .await; + .await?; #[cfg(feature = "tokio")] let key = tokio::task::spawn_blocking(move || keyring.blocking_read().derive_key(&secret)) .await - .unwrap(); + .unwrap()?; *key_lock = Some(Arc::new(key)); } - Arc::clone(key_lock.as_ref().unwrap()) + Ok(Arc::clone(key_lock.as_ref().unwrap())) } /// Change keyring secret @@ -437,7 +440,7 @@ impl Keyring { tracing::debug!("Changing keyring secret and key"); let keyring = self.keyring.read().await; - let key = self.derive_key().await; + let key = self.derive_key().await?; let mut items = Vec::with_capacity(keyring.items.len()); for item in &keyring.items { items.push(item.clone().decrypt(&key)?); @@ -459,7 +462,7 @@ impl Keyring { drop(keyring); // Set new key - let key = self.derive_key().await; + let key = self.derive_key().await?; let mut keyring = self.keyring.write().await; for item in items { @@ -570,7 +573,7 @@ mod tests { async fn check_items(keyring: &Keyring) -> Result<(), Error> { assert_eq!(keyring.n_items().await, 1); - let items: Result, _> = keyring.items().await.into_iter().collect(); + let items: Result, _> = keyring.items().await?.into_iter().collect(); let items = items.expect("unable to retrieve items"); assert_eq!(items.len(), 1); assert_eq!(items[0].label(), "foo"); diff --git a/client/src/key.rs b/client/src/key.rs index db87cff1..6baa92b0 100644 --- a/client/src/key.rs +++ b/client/src/key.rs @@ -39,16 +39,21 @@ impl Key { Self { key, strength } } - pub fn generate_private_key() -> Self { - Self::new(crypto::generate_private_key().to_vec()) + pub fn generate_private_key() -> Result { + Ok(Self::new(crypto::generate_private_key()?.to_vec())) } - pub fn generate_public_key(private_key: &Self) -> Self { - Self::new(crypto::generate_public_key(private_key)) + pub fn generate_public_key(private_key: &Self) -> Result { + Ok(Self::new(crypto::generate_public_key(private_key)?)) } - pub fn generate_aes_key(private_key: &Self, server_public_key: &Self) -> Self { - Self::new(crypto::generate_aes_key(private_key, server_public_key).to_vec()) + pub fn generate_aes_key( + private_key: &Self, + server_public_key: &Self, + ) -> Result { + Ok(Self::new( + crypto::generate_aes_key(private_key, server_public_key)?.to_vec(), + )) } } @@ -127,7 +132,7 @@ mod tests { let public_key = Key::generate_public_key(&private_key); let aes_key = Key::generate_aes_key(&private_key, &server_public_key); - assert_eq!(public_key.as_ref(), expected_public_key); - assert_eq!(aes_key.as_ref(), expected_aes_key); + assert_eq!(public_key.unwrap().as_ref(), expected_public_key); + assert_eq!(aes_key.unwrap().as_ref(), expected_aes_key); } } diff --git a/client/src/keyring.rs b/client/src/keyring.rs index 7c282714..19c0c0de 100644 --- a/client/src/keyring.rs +++ b/client/src/keyring.rs @@ -100,7 +100,7 @@ impl Keyring { items.into_iter().map(Item::for_dbus).collect::>() } Self::File(backend) => { - let items = backend.items().await; + let items = backend.items().await?; items .into_iter() // Ignore invalid items @@ -224,7 +224,7 @@ impl Item { Self::File(item, backend) => { let index = backend .lookup_item_index(item.read().await.attributes()) - .await; + .await?; item.write().await.set_attributes(attributes); let item_guard = item.read().await; diff --git a/client/src/lib.rs b/client/src/lib.rs index a0e29e83..00244c16 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -54,7 +54,14 @@ pub const XDG_SCHEMA_ATTRIBUTE: &str = "xdg:schema"; pub trait AsAttributes { fn as_attributes(&self) -> HashMap<&str, &str>; - fn hash<'a>(&'a self, key: &Key) -> Vec<(&'a str, zeroize::Zeroizing>)> { + #[allow(clippy::type_complexity)] + fn hash<'a>( + &'a self, + key: &Key, + ) -> Vec<( + &'a str, + std::result::Result>, crate::crypto::Error>, + )> { self.as_attributes() .into_iter() .map(|(k, v)| (k, crate::file::AttributeValue::from(v).mac(key))) diff --git a/server/src/collection.rs b/server/src/collection.rs index 49e40952..fca6c68f 100644 --- a/server/src/collection.rs +++ b/server/src/collection.rs @@ -95,7 +95,11 @@ impl Collection { }; let secret = match session.aes_key() { - Some(key) => oo7::crypto::decrypt(secret, &key, &iv), + Some(key) => oo7::crypto::decrypt(secret, &key, &iv).map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new(zbus::fdo::Error::Failed( + format!("Failed to decrypt secret {err}."), + )))) + })?, None => zeroize::Zeroizing::new(secret), }; @@ -275,7 +279,7 @@ impl Collection { } pub async fn dispatch_items(&self) -> Result<(), Error> { - let keyring_items = self.keyring.items().await; + let keyring_items = self.keyring.items().await?; let mut items = self.items.lock().await; let object_server = self.service.object_server(); let mut n_items = 1; diff --git a/server/src/item.rs b/server/src/item.rs index accadcd0..16acb7db 100644 --- a/server/src/item.rs +++ b/server/src/item.rs @@ -83,8 +83,16 @@ impl Item { match session.aes_key() { Some(key) => { - let iv = oo7::crypto::generate_iv(); - let encrypted = oo7::crypto::encrypt(secret, &key, &iv); + let iv = oo7::crypto::generate_iv().map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new(zbus::fdo::Error::Failed( + format!("Failed to generate iv {err}."), + )))) + })?; + let encrypted = oo7::crypto::encrypt(secret, &key, &iv).map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new(zbus::fdo::Error::Failed( + format!("Failed to encrypt secret {err}."), + )))) + })?; Ok((DBusSecretInner( session.path().clone(), @@ -117,7 +125,11 @@ impl Item { match session.aes_key() { Some(key) => { - let decrypted = oo7::crypto::decrypt(secret, &key, &iv); + let decrypted = oo7::crypto::decrypt(secret, &key, &iv).map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new(zbus::fdo::Error::Failed( + format!("Failed to decrypt secret {err}."), + )))) + })?; inner.set_secret(decrypted); } None => { diff --git a/server/src/service.rs b/server/src/service.rs index f5c1bb9a..d34938de 100644 --- a/server/src/service.rs +++ b/server/src/service.rs @@ -44,10 +44,26 @@ impl Service { Algorithm::Plain => (None, None), Algorithm::Encrypted => { let client_public_key = Key::from(input); - let private_key = Key::generate_private_key(); + let private_key = Key::generate_private_key().map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new(zbus::fdo::Error::Failed( + format!("Failed to generate private key {err}."), + )))) + })?; ( - Some(Key::generate_public_key(&private_key)), - Some(Key::generate_aes_key(&private_key, &client_public_key)), + Some(Key::generate_public_key(&private_key).map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new(zbus::fdo::Error::Failed( + format!("Failed to generate public key {err}."), + )))) + })?), + Some( + Key::generate_aes_key(&private_key, &client_public_key).map_err(|err| { + ServiceError::ZBus(zbus::Error::FDO(Box::new( + zbus::fdo::Error::Failed(format!( + "Failed to generate aes key {err}." + )), + ))) + })?, + ), ) } };