Use return values instead of out arguments for PGP signatures

This commit is contained in:
link2xt
2021-12-05 04:15:34 +00:00
parent 572260ec29
commit bb3353397d
2 changed files with 41 additions and 82 deletions

View File

@@ -2,7 +2,7 @@
use std::collections::HashSet; use std::collections::HashSet;
use anyhow::{bail, ensure, format_err, Result}; use anyhow::{bail, format_err, Result};
use mailparse::ParsedMail; use mailparse::ParsedMail;
use num_traits::FromPrimitive; use num_traits::FromPrimitive;
@@ -179,7 +179,6 @@ pub async fn try_decrypt(
// Possibly perform decryption // Possibly perform decryption
let private_keyring: Keyring<SignedSecretKey> = Keyring::new_self(context).await?; let private_keyring: Keyring<SignedSecretKey> = Keyring::new_self(context).await?;
let mut public_keyring_for_validate: Keyring<SignedPublicKey> = Keyring::new(); let mut public_keyring_for_validate: Keyring<SignedPublicKey> = Keyring::new();
let mut signatures = HashSet::default();
if let Some(ref mut peerstate) = peerstate { if let Some(ref mut peerstate) = peerstate {
peerstate peerstate
@@ -192,14 +191,17 @@ pub async fn try_decrypt(
} }
} }
let out_mail = decrypt_if_autocrypt_message( let (out_mail, signatures) = match decrypt_if_autocrypt_message(
context, context,
mail, mail,
private_keyring, private_keyring,
public_keyring_for_validate, public_keyring_for_validate,
&mut signatures,
) )
.await?; .await?
{
Some((out_mail, signatures)) => (Some(out_mail), signatures),
None => (None, Default::default()),
};
if let Some(mut peerstate) = peerstate { if let Some(mut peerstate) = peerstate {
// If message is not encrypted and it is not a read receipt, degrade encryption. // If message is not encrypted and it is not a read receipt, degrade encryption.
@@ -275,8 +277,7 @@ async fn decrypt_if_autocrypt_message(
mail: &ParsedMail<'_>, mail: &ParsedMail<'_>,
private_keyring: Keyring<SignedSecretKey>, private_keyring: Keyring<SignedSecretKey>,
public_keyring_for_validate: Keyring<SignedPublicKey>, public_keyring_for_validate: Keyring<SignedPublicKey>,
ret_valid_signatures: &mut HashSet<Fingerprint>, ) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> {
) -> Result<Option<Vec<u8>>> {
let encrypted_data_part = match get_autocrypt_mime(mail).or_else(|| get_mixed_up_mime(mail)) { let encrypted_data_part = match get_autocrypt_mime(mail).or_else(|| get_mixed_up_mime(mail)) {
None => { None => {
// not an autocrypt mime message, abort and ignore // not an autocrypt mime message, abort and ignore
@@ -290,7 +291,6 @@ async fn decrypt_if_autocrypt_message(
encrypted_data_part, encrypted_data_part,
private_keyring, private_keyring,
public_keyring_for_validate, public_keyring_for_validate,
ret_valid_signatures,
) )
.await .await
} }
@@ -300,26 +300,17 @@ async fn decrypt_part(
mail: &ParsedMail<'_>, mail: &ParsedMail<'_>,
private_keyring: Keyring<SignedSecretKey>, private_keyring: Keyring<SignedSecretKey>,
public_keyring_for_validate: Keyring<SignedPublicKey>, public_keyring_for_validate: Keyring<SignedPublicKey>,
ret_valid_signatures: &mut HashSet<Fingerprint>, ) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> {
) -> Result<Option<Vec<u8>>> {
let data = mail.get_body_raw()?; let data = mail.get_body_raw()?;
if has_decrypted_pgp_armor(&data) { if has_decrypted_pgp_armor(&data) {
// we should only have one decryption happening let (plain, ret_valid_signatures) =
ensure!(ret_valid_signatures.is_empty(), "corrupt signatures"); pgp::pk_decrypt(data, private_keyring, public_keyring_for_validate).await?;
let plain = pgp::pk_decrypt(
data,
private_keyring,
public_keyring_for_validate,
Some(ret_valid_signatures),
)
.await?;
// If the message was wrongly or not signed, still return the plain text. // If the message was wrongly or not signed, still return the plain text.
// The caller has to check the signatures then. // The caller has to check the signatures then.
return Ok(Some(plain)); return Ok(Some((plain, ret_valid_signatures)));
} }
Ok(None) Ok(None)

View File

@@ -277,7 +277,7 @@ pub async fn pk_encrypt(
/// Receiver private keys are provided in /// Receiver private keys are provided in
/// `private_keys_for_decryption`. /// `private_keys_for_decryption`.
/// ///
/// If `ret_signature_fingerprints` is not `None`, stores fingerprints /// Returns decrypted message and fingerprints
/// of all keys from the `public_keys_for_validation` keyring that /// of all keys from the `public_keys_for_validation` keyring that
/// have valid signatures there. /// have valid signatures there.
#[allow(clippy::implicit_hasher)] #[allow(clippy::implicit_hasher)]
@@ -285,8 +285,9 @@ pub async fn pk_decrypt(
ctext: Vec<u8>, ctext: Vec<u8>,
private_keys_for_decryption: Keyring<SignedSecretKey>, private_keys_for_decryption: Keyring<SignedSecretKey>,
public_keys_for_validation: Keyring<SignedPublicKey>, public_keys_for_validation: Keyring<SignedPublicKey>,
ret_signature_fingerprints: Option<&mut HashSet<Fingerprint>>, ) -> Result<(Vec<u8>, HashSet<Fingerprint>)> {
) -> Result<Vec<u8>> { let mut ret_signature_fingerprints: HashSet<Fingerprint> = Default::default();
let msgs = async_std::task::spawn_blocking(move || { let msgs = async_std::task::spawn_blocking(move || {
let cursor = Cursor::new(ctext); let cursor = Cursor::new(ctext);
let (msg, _) = Message::from_armor_single(cursor)?; let (msg, _) = Message::from_armor_single(cursor)?;
@@ -308,28 +309,26 @@ pub async fn pk_decrypt(
None => bail!("The decrypted message is empty"), None => bail!("The decrypted message is empty"),
}; };
if let Some(ret_signature_fingerprints) = ret_signature_fingerprints { if !public_keys_for_validation.is_empty() {
if !public_keys_for_validation.is_empty() { let fingerprints = async_std::task::spawn_blocking(move || {
let fingerprints = async_std::task::spawn_blocking(move || { let pkeys = public_keys_for_validation.keys();
let pkeys = public_keys_for_validation.keys();
let mut fingerprints: Vec<Fingerprint> = Vec::new(); let mut fingerprints: Vec<Fingerprint> = Vec::new();
if let signed_msg @ pgp::composed::Message::Signed { .. } = msg { if let signed_msg @ pgp::composed::Message::Signed { .. } = msg {
for pkey in pkeys { for pkey in pkeys {
if signed_msg.verify(&pkey.primary_key).is_ok() { if signed_msg.verify(&pkey.primary_key).is_ok() {
let fp = DcKey::fingerprint(pkey); let fp = DcKey::fingerprint(pkey);
fingerprints.push(fp); fingerprints.push(fp);
}
} }
} }
fingerprints }
}) fingerprints
.await; })
.await;
ret_signature_fingerprints.extend(fingerprints); ret_signature_fingerprints.extend(fingerprints);
}
} }
Ok(content) Ok((content, ret_signature_fingerprints))
} else { } else {
bail!("No valid messages found"); bail!("No valid messages found");
} }
@@ -492,12 +491,10 @@ mod tests {
decrypt_keyring.add(KEYS.alice_secret.clone()); decrypt_keyring.add(KEYS.alice_secret.clone());
let mut sig_check_keyring: Keyring<SignedPublicKey> = Keyring::new(); let mut sig_check_keyring: Keyring<SignedPublicKey> = Keyring::new();
sig_check_keyring.add(KEYS.alice_public.clone()); sig_check_keyring.add(KEYS.alice_public.clone());
let mut valid_signatures: HashSet<Fingerprint> = Default::default(); let (plain, valid_signatures) = pk_decrypt(
let plain = pk_decrypt(
CTEXT_SIGNED.as_bytes().to_vec(), CTEXT_SIGNED.as_bytes().to_vec(),
decrypt_keyring, decrypt_keyring,
sig_check_keyring, sig_check_keyring
Some(&mut valid_signatures),
) )
.await .await
.map_err(|err| println!("{:?}", err)) .map_err(|err| println!("{:?}", err))
@@ -510,12 +507,10 @@ mod tests {
decrypt_keyring.add(KEYS.bob_secret.clone()); decrypt_keyring.add(KEYS.bob_secret.clone());
let mut sig_check_keyring = Keyring::new(); let mut sig_check_keyring = Keyring::new();
sig_check_keyring.add(KEYS.alice_public.clone()); sig_check_keyring.add(KEYS.alice_public.clone());
let mut valid_signatures: HashSet<Fingerprint> = Default::default(); let (plain, valid_signatures) = pk_decrypt(
let plain = pk_decrypt(
CTEXT_SIGNED.as_bytes().to_vec(), CTEXT_SIGNED.as_bytes().to_vec(),
decrypt_keyring, decrypt_keyring,
sig_check_keyring, sig_check_keyring
Some(&mut valid_signatures),
) )
.await .await
.map_err(|err| println!("{:?}", err)) .map_err(|err| println!("{:?}", err))
@@ -529,15 +524,10 @@ mod tests {
let mut keyring = Keyring::new(); let mut keyring = Keyring::new();
keyring.add(KEYS.alice_secret.clone()); keyring.add(KEYS.alice_secret.clone());
let empty_keyring = Keyring::new(); let empty_keyring = Keyring::new();
let mut valid_signatures: HashSet<Fingerprint> = Default::default(); let (plain, valid_signatures) =
let plain = pk_decrypt( pk_decrypt(CTEXT_SIGNED.as_bytes().to_vec(), keyring, empty_keyring)
CTEXT_SIGNED.as_bytes().to_vec(), .await
keyring, .unwrap();
empty_keyring,
Some(&mut valid_signatures),
)
.await
.unwrap();
assert_eq!(plain, CLEARTEXT); assert_eq!(plain, CLEARTEXT);
assert_eq!(valid_signatures.len(), 0); assert_eq!(valid_signatures.len(), 0);
} }
@@ -549,12 +539,10 @@ mod tests {
decrypt_keyring.add(KEYS.bob_secret.clone()); decrypt_keyring.add(KEYS.bob_secret.clone());
let mut sig_check_keyring = Keyring::new(); let mut sig_check_keyring = Keyring::new();
sig_check_keyring.add(KEYS.bob_public.clone()); sig_check_keyring.add(KEYS.bob_public.clone());
let mut valid_signatures: HashSet<Fingerprint> = Default::default(); let (plain, valid_signatures) = pk_decrypt(
let plain = pk_decrypt(
CTEXT_SIGNED.as_bytes().to_vec(), CTEXT_SIGNED.as_bytes().to_vec(),
decrypt_keyring, decrypt_keyring,
sig_check_keyring, sig_check_keyring,
Some(&mut valid_signatures),
) )
.await .await
.unwrap(); .unwrap();
@@ -567,34 +555,14 @@ mod tests {
let mut decrypt_keyring = Keyring::new(); let mut decrypt_keyring = Keyring::new();
decrypt_keyring.add(KEYS.bob_secret.clone()); decrypt_keyring.add(KEYS.bob_secret.clone());
let sig_check_keyring = Keyring::new(); let sig_check_keyring = Keyring::new();
let mut valid_signatures: HashSet<Fingerprint> = Default::default(); let (plain, valid_signatures) = pk_decrypt(
let plain = pk_decrypt(
CTEXT_UNSIGNED.as_bytes().to_vec(), CTEXT_UNSIGNED.as_bytes().to_vec(),
decrypt_keyring, decrypt_keyring,
sig_check_keyring, sig_check_keyring,
Some(&mut valid_signatures),
) )
.await .await
.unwrap(); .unwrap();
assert_eq!(plain, CLEARTEXT); assert_eq!(plain, CLEARTEXT);
assert_eq!(valid_signatures.len(), 0); assert_eq!(valid_signatures.len(), 0);
} }
#[async_std::test]
async fn test_decrypt_signed_no_sigret() {
// Check decrypting signed cyphertext without providing the HashSet for signatures.
let mut decrypt_keyring = Keyring::new();
decrypt_keyring.add(KEYS.bob_secret.clone());
let mut sig_check_keyring = Keyring::new();
sig_check_keyring.add(KEYS.alice_public.clone());
let plain = pk_decrypt(
CTEXT_SIGNED.as_bytes().to_vec(),
decrypt_keyring,
sig_check_keyring,
None,
)
.await
.unwrap();
assert_eq!(plain, CLEARTEXT);
}
} }