Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-05-09 01:46:30 +03:00
Code Issues Packages Projects Releases Wiki Activity

Validate signatures in try_decrypt() even if the message isn't encrypted (#3844)

Browse Source 
This way we don't need a separate code path for signatures validation for unencrypted
messages. Also, now we degrade encryption only if there are no valid signatures, so the code for
upgrading encryption back isn't needed.
This commit is contained in:
iequidoo
2022-12-20 16:00:33 -03:00
parent 1f7ad78f40
commit ba82ce2798
3 changed files with 107 additions and 154 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -3,6 +3,7 @@
## Unreleased ## Unreleased
### Changes ### Changes
- Validate signatures in try_decrypt() even if the message isn't encrypted #3859
### API-Changes ### API-Changes

33
src/decrypt.rs
View File

@@ -18,18 +18,17 @@ use crate::peerstate::Peerstate;
use crate::pgp; use crate::pgp;
/// Tries to decrypt a message, but only if it is structured as an /// Tries to decrypt a message, but only if it is structured as an
/// Autocrypt message. /// Autocrypt message, otherwise just validates signatures.
/// ///
/// Returns decrypted body and a set of valid signature fingerprints /// If successful and the message is encrypted or signed, returns \[decrypted\] body, a set of valid
/// if successful. /// signature fingerprints and whether the message is encrypted.
/// ///
/// If the message is wrongly signed, this will still return the decrypted /// If the message is wrongly signed, HashSet will be empty.
/// message but the HashSet will be empty.
pub async fn try_decrypt( pub async fn try_decrypt(
context: &Context, context: &Context,
mail: &ParsedMail<'_>, mail: &ParsedMail<'_>,
decryption_info: &DecryptionInfo, decryption_info: &DecryptionInfo,
) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> { ) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>, bool)>> {
// Possibly perform decryption // Possibly perform decryption
let public_keyring_for_validate = keyring_from_peerstate(decryption_info.peerstate.as_ref()); let public_keyring_for_validate = keyring_from_peerstate(decryption_info.peerstate.as_ref());
@@ -38,8 +37,10 @@ pub async fn try_decrypt(
.or_else(|| get_attachment_mime(mail)) .or_else(|| get_attachment_mime(mail))
{ {
None => { None => {
// not an autocrypt mime message, abort and ignore return Ok(
return Ok(None); validate_detached_signature(mail, &public_keyring_for_validate)?
.map(|(raw, fprints)| (raw, fprints, false)),
)
} }
Some(res) => res, Some(res) => res,
}; };
@@ -48,12 +49,13 @@ pub async fn try_decrypt(
.await .await
.context("failed to get own keyring")?; .context("failed to get own keyring")?;
decrypt_part( Ok(decrypt_part(
encrypted_data_part, encrypted_data_part,
private_keyring, private_keyring,
public_keyring_for_validate, public_keyring_for_validate,
) )
.await .await?
.map(|(raw, fprints)| (raw, fprints, true)))
} }
pub(crate) async fn prepare_decryption( pub(crate) async fn prepare_decryption(
@@ -251,7 +253,7 @@ fn has_decrypted_pgp_armor(input: &[u8]) -> bool {
/// ///
/// Returns `None` if the part is not a Multipart/Signed part, otherwise retruns the set of key /// Returns `None` if the part is not a Multipart/Signed part, otherwise retruns the set of key
/// fingerprints for which there is a valid signature. /// fingerprints for which there is a valid signature.
pub(crate) fn validate_detached_signature( fn validate_detached_signature(
mail: &ParsedMail<'_>, mail: &ParsedMail<'_>,
public_keyring_for_validate: &Keyring<SignedPublicKey>, public_keyring_for_validate: &Keyring<SignedPublicKey>,
) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> { ) -> Result<Option<(Vec<u8>, HashSet<Fingerprint>)>> {
@@ -262,10 +264,11 @@ pub(crate) fn validate_detached_signature(
if let [first_part, second_part] = &mail.subparts[..] { if let [first_part, second_part] = &mail.subparts[..] {
// First part is the content, second part is the signature. // First part is the content, second part is the signature.
let content = first_part.raw_bytes; let content = first_part.raw_bytes;
let signature = second_part.get_body_raw()?; let ret_valid_signatures = match second_part.get_body_raw() {
let ret_valid_signatures = Ok(signature) => pgp::pk_validate(content, &signature, public_keyring_for_validate)
pgp::pk_validate(content, &signature, public_keyring_for_validate)?; .unwrap_or_default(),
Err(_) => Default::default(),
};
Ok(Some((content.to_vec(), ret_valid_signatures))) Ok(Some((content.to_vec(), ret_valid_signatures)))
} else { } else {
Ok(None) Ok(None)

101
src/mimeparser.rs
View File

@@ -18,10 +18,7 @@ use crate::blob::BlobObject;
use crate::constants::{DC_DESIRED_TEXT_LINES, DC_DESIRED_TEXT_LINE_LEN}; use crate::constants::{DC_DESIRED_TEXT_LINES, DC_DESIRED_TEXT_LINE_LEN};
use crate::contact::{addr_cmp, addr_normalize, ContactId}; use crate::contact::{addr_cmp, addr_normalize, ContactId};
use crate::context::Context; use crate::context::Context;
use crate::decrypt::{ use crate::decrypt::{prepare_decryption, try_decrypt, DecryptionInfo};
keyring_from_peerstate, prepare_decryption, try_decrypt, validate_detached_signature,
DecryptionInfo,
};
use crate::dehtml::dehtml; use crate::dehtml::dehtml;
use crate::events::EventType; use crate::events::EventType;
use crate::format_flowed::unformat_flowed; use crate::format_flowed::unformat_flowed;
@@ -67,8 +64,6 @@ pub struct MimeMessage {
/// If a message is not encrypted or the signature is not valid, /// If a message is not encrypted or the signature is not valid,
/// this set is empty. /// this set is empty.
pub signatures: HashSet<Fingerprint>, pub signatures: HashSet<Fingerprint>,
/// Whether the message is encrypted in a domestic (not Autocrypt) sense
pub encrypted: bool,
/// The set of mail recipient addresses for which gossip headers were applied, regardless of /// The set of mail recipient addresses for which gossip headers were applied, regardless of
/// whether they modified any peerstates. /// whether they modified any peerstates.
pub gossiped_addr: HashSet<String>, pub gossiped_addr: HashSet<String>,
@@ -235,13 +230,10 @@ impl MimeMessage {
hop_info += "\n\n"; hop_info += "\n\n";
hop_info += &decryption_info.dkim_results.to_string(); hop_info += &decryption_info.dkim_results.to_string();
// `signatures` is non-empty exactly if the message was encrypted and correctly signed. let (mail, mut signatures, encrypted) =
let (mail, signatures, encrypted) = match try_decrypt(context, &mail, &decryption_info) match try_decrypt(context, &mail, &decryption_info).await {
.await Ok(Some((raw, signatures, encrypted))) => {
{ // Only if `encrypted` and `signatures` set is non-empty, it is a valid
Ok(Some((raw, signatures))) => {
// Encrypted, but maybe unsigned message. Only if
// `signatures` set is non-empty, it is a valid
// autocrypt message. // autocrypt message.
mail_raw = raw; mail_raw = raw;
@@ -251,11 +243,13 @@ impl MimeMessage {
println!("{}", String::from_utf8_lossy(&mail_raw)); println!("{}", String::from_utf8_lossy(&mail_raw));
} }
if encrypted {
// Handle any gossip headers if the mail was encrypted. See section // Handle any gossip headers if the mail was encrypted. See section
// "3.6 Key Gossip" of <https://autocrypt.org/autocrypt-spec-1.1.0.pdf> // "3.6 Key Gossip" of <https://autocrypt.org/autocrypt-spec-1.1.0.pdf>
// but only if the mail was correctly signed: // but only if the mail was correctly signed:
if !signatures.is_empty() { if !signatures.is_empty() {
let gossip_headers = decrypted_mail.headers.get_all_values("Autocrypt-Gossip"); let gossip_headers =
decrypted_mail.headers.get_all_values("Autocrypt-Gossip");
gossiped_addr = update_gossip_peerstates( gossiped_addr = update_gossip_peerstates(
context, context,
message_time, message_time,
@@ -273,7 +267,8 @@ impl MimeMessage {
// do not allow overriding it. // do not allow overriding it.
let mut signed_from = None; let mut signed_from = None;
// We do not want to allow unencrypted subject in encrypted emails because the user might falsely think that the subject is safe. // We do not want to allow unencrypted subject in encrypted emails because the
// user might falsely think that the subject is safe.
// See <https://github.com/deltachat/deltachat-core-rust/issues/1790>. // See <https://github.com/deltachat/deltachat-core-rust/issues/1790>.
headers.remove("subject"); headers.remove("subject");
@@ -303,13 +298,19 @@ impl MimeMessage {
); );
} }
} }
(Ok(decrypted_mail), signatures, true)
} }
Ok(None) => { (Ok(decrypted_mail), signatures, encrypted)
// Message was not encrypted. }
Ok(None) => (Ok(mail), HashSet::new(), false),
Err(err) => {
warn!(context, "decryption failed: {}", err);
(Err(err), HashSet::new(), true)
}
};
if signatures.is_empty() {
// If it is not a read receipt, degrade encryption. // If it is not a read receipt, degrade encryption.
if let Some(peerstate) = &mut decryption_info.peerstate { if let (Some(peerstate), Ok(mail)) = (&mut decryption_info.peerstate, &mail) {
if message_time > peerstate.last_seen_autocrypt if message_time > peerstate.last_seen_autocrypt
&& mail.ctype.mimetype != "multipart/report" && mail.ctype.mimetype != "multipart/report"
// Disallowing keychanges is disabled for now: // Disallowing keychanges is disabled for now:
@@ -318,13 +319,10 @@ impl MimeMessage {
peerstate.degrade_encryption(message_time); peerstate.degrade_encryption(message_time);
} }
} }
(Ok(mail), HashSet::new(), false)
} }
Err(err) => { if !encrypted {
warn!(context, "decryption failed: {}", err); signatures.clear();
(Err(err), HashSet::new(), true)
} }
};
let mut parser = MimeMessage { let mut parser = MimeMessage {
parts: Vec::new(), parts: Vec::new(),
@@ -339,7 +337,6 @@ impl MimeMessage {
// only non-empty if it was a valid autocrypt message // only non-empty if it was a valid autocrypt message
signatures, signatures,
encrypted,
gossiped_addr, gossiped_addr,
is_forwarded: false, is_forwarded: false,
mdn_reports: Vec::new(), mdn_reports: Vec::new(),
@@ -868,26 +865,6 @@ impl MimeMessage {
.parse_mime_recursive(context, first, is_related) .parse_mime_recursive(context, first, is_related)
.await?; .await?;
} }
if let Some(peerstate) = &mut self.decryption_info.peerstate {
let keyring = keyring_from_peerstate(Some(peerstate));
match validate_detached_signature(mail, &keyring) {
Ok(Some((_, fprints))) => {
if fprints.is_empty() {
warn!(context, "signed message is not signed with a known key");
} else if peerstate.prefer_encrypt != EncryptPreference::Mutual {
info!(
context,
"message is signed with the known key, setting \
prefer-encrypt=mutual for '{}'",
peerstate.addr,
);
Self::upgrade_to_mutual_encryption(context, peerstate).await?;
}
}
Ok(None) => warn!(context, "not a 'multipart/signed' part??"),
Err(err) => warn!(context, "signed message validation failed: {}", err),
}
}
} }
(mime::MULTIPART, "report") => { (mime::MULTIPART, "report") => {
/* RFC 6522: the first part is for humans, the second for machines */ /* RFC 6522: the first part is for humans, the second for machines */
@@ -965,17 +942,6 @@ impl MimeMessage {
Ok(any_part_added) Ok(any_part_added)
} }
async fn upgrade_to_mutual_encryption(
context: &Context,
peerstate: &mut Peerstate,
) -> Result<()> {
if peerstate.public_key.is_none() {
peerstate.public_key = peerstate.gossip_key.take();
}
peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.save_to_db(&context.sql).await
}
/// Returns true if any part was added, false otherwise. /// Returns true if any part was added, false otherwise.
async fn add_single_part_if_known( async fn add_single_part_if_known(
&mut self, &mut self,
@@ -1153,13 +1119,7 @@ impl MimeMessage {
if peerstate.prefer_encrypt != EncryptPreference::Mutual if peerstate.prefer_encrypt != EncryptPreference::Mutual
&& mime_type.type_() == mime::APPLICATION && mime_type.type_() == mime::APPLICATION
&& mime_type.subtype().as_str() == "pgp-keys" && mime_type.subtype().as_str() == "pgp-keys"
&& Self::try_set_peer_key_from_file_part( && Self::try_set_peer_key_from_file_part(context, peerstate, decoded_data).await?
context,
peerstate,
decoded_data,
self.encrypted,
)
.await?
{ {
return Ok(()); return Ok(());
} }
@@ -1249,7 +1209,6 @@ impl MimeMessage {
context: &Context, context: &Context,
peerstate: &mut Peerstate, peerstate: &mut Peerstate,
decoded_data: &[u8], decoded_data: &[u8],
mail_is_encrypted: bool,
) -> Result<bool> { ) -> Result<bool> {
let key = match str::from_utf8(decoded_data) { let key = match str::from_utf8(decoded_data) {
Err(err) => { Err(err) => {
@@ -1295,22 +1254,12 @@ impl MimeMessage {
} }
} }
peerstate.public_key = Some(key); peerstate.public_key = Some(key);
if mail_is_encrypted {
info!( info!(
context, context,
"using attached PGP key for peer '{}' with prefer-encrypt=mutual as the mail is \ "using attached PGP key for peer '{}' with prefer-encrypt=mutual", peerstate.addr,
encrypted",
peerstate.addr,
); );
peerstate.prefer_encrypt = EncryptPreference::Mutual; peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.save_to_db(&context.sql).await?; peerstate.save_to_db(&context.sql).await?;
} else {
info!(
context,
"using attached PGP key for peer '{}'", peerstate.addr,
);
peerstate.prefer_encrypt = EncryptPreference::NoPreference;
}
Ok(true) Ok(true)
} }