Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-05-20 07:16:31 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge "Validate signatures in try_decrypt() even if the message isn't encrypted" (#3859)

Browse Source
This commit is contained in:
link2xt
2022-12-23 01:19:09 +00:00
parent bad04f9a0b 736950ab3f
commit 7c343411b8
3 changed files with 112 additions and 159 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

43
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(
@@ -219,7 +221,7 @@ async fn decrypt_part(
// If decrypted part is a multipart/signed, then there is a detached signature. // If decrypted part is a multipart/signed, then there is a detached signature.
let decrypted_part = mailparse::parse_mail(&plain)?; let decrypted_part = mailparse::parse_mail(&plain)?;
if let Some((content, valid_detached_signatures)) = if let Some((content, valid_detached_signatures)) =
validate_detached_signature(&decrypted_part, &public_keyring_for_validate)? validate_detached_signature(&decrypted_part, &public_keyring_for_validate)
{ {
return Ok(Some((content, valid_detached_signatures))); return Ok(Some((content, valid_detached_signatures)));
} else { } else {
@@ -251,24 +253,25 @@ 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>)>> { ) -> Option<(Vec<u8>, HashSet<Fingerprint>)> {
if mail.ctype.mimetype != "multipart/signed" { if mail.ctype.mimetype != "multipart/signed" {
return Ok(None); return None;
} }
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))) };
Some((content.to_vec(), ret_valid_signatures))
} else { } else {
Ok(None) None
} }
} }

227
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,96 +230,99 @@ 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))) => { // autocrypt message.
// Encrypted, but maybe unsigned message. Only if
// `signatures` set is non-empty, it is a valid
// autocrypt message.
mail_raw = raw; mail_raw = raw;
let decrypted_mail = mailparse::parse_mail(&mail_raw)?; let decrypted_mail = mailparse::parse_mail(&mail_raw)?;
if std::env::var(crate::DCC_MIME_DEBUG).is_ok() { if std::env::var(crate::DCC_MIME_DEBUG).is_ok() {
info!(context, "decrypted message mime-body:"); info!(context, "decrypted message mime-body:");
println!("{}", String::from_utf8_lossy(&mail_raw)); println!("{}", String::from_utf8_lossy(&mail_raw));
} }
// Handle any gossip headers if the mail was encrypted. See section if encrypted {
// "3.6 Key Gossip" of <https://autocrypt.org/autocrypt-spec-1.1.0.pdf> // Handle any gossip headers if the mail was encrypted. See section
// but only if the mail was correctly signed: // "3.6 Key Gossip" of <https://autocrypt.org/autocrypt-spec-1.1.0.pdf>
if !signatures.is_empty() { // but only if the mail was correctly signed:
let gossip_headers = decrypted_mail.headers.get_all_values("Autocrypt-Gossip"); if !signatures.is_empty() {
gossiped_addr = update_gossip_peerstates( let gossip_headers =
context, decrypted_mail.headers.get_all_values("Autocrypt-Gossip");
message_time, gossiped_addr = update_gossip_peerstates(
&from.addr, context,
&mail, message_time,
gossip_headers, &from.addr,
) &mail,
.await?; gossip_headers,
} )
.await?;
}
// let known protected headers from the decrypted // let known protected headers from the decrypted
// part override the unencrypted top-level // part override the unencrypted top-level
// Signature was checked for original From, so we // Signature was checked for original From, so we
// 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
// See <https://github.com/deltachat/deltachat-core-rust/issues/1790>. // user might falsely think that the subject is safe.
headers.remove("subject"); // See <https://github.com/deltachat/deltachat-core-rust/issues/1790>.
headers.remove("subject");
MimeMessage::merge_headers( MimeMessage::merge_headers(
context,
&mut headers,
&mut recipients,
&mut signed_from,
&mut list_post,
&mut chat_disposition_notification_to,
&decrypted_mail.headers,
);
if let Some(signed_from) = signed_from {
if addr_cmp(&signed_from.addr, &from.addr) {
from_is_signed = true;
} else {
// There is a From: header in the encrypted &
// signed part, but it doesn't match the outer one.
// This _might_ be because the sender's mail server
// replaced the sending address, e.g. in a mailing list.
// Or it's because someone is doing some replay attack
// - OTOH, I can't come up with an attack scenario
// where this would be useful.
warn!(
context, context,
"From header in signed part does't match the outer one", &mut headers,
&mut recipients,
&mut signed_from,
&mut list_post,
&mut chat_disposition_notification_to,
&decrypted_mail.headers,
); );
if let Some(signed_from) = signed_from {
if addr_cmp(&signed_from.addr, &from.addr) {
from_is_signed = true;
} else {
// There is a From: header in the encrypted &
// signed part, but it doesn't match the outer one.
// This _might_ be because the sender's mail server
// replaced the sending address, e.g. in a mailing list.
// Or it's because someone is doing some replay attack
// - OTOH, I can't come up with an attack scenario
// where this would be useful.
warn!(
context,
"From header in signed part does't match the outer one",
);
}
}
} }
(Ok(decrypted_mail), signatures, encrypted)
} }
Ok(None) => (Ok(mail), HashSet::new(), false),
Err(err) => {
warn!(context, "decryption failed: {}", err);
(Err(err), HashSet::new(), true)
}
};
(Ok(decrypted_mail), signatures, true) if signatures.is_empty() {
} // If it is not a read receipt, degrade encryption.
Ok(None) => { if let (Some(peerstate), Ok(mail)) = (&mut decryption_info.peerstate, &mail) {
// Message was not encrypted. if message_time > peerstate.last_seen_autocrypt
// If it is not a read receipt, degrade encryption. && mail.ctype.mimetype != "multipart/report"
if let Some(peerstate) = &mut decryption_info.peerstate { // Disallowing keychanges is disabled for now:
if message_time > peerstate.last_seen_autocrypt // && decryption_info.dkim_results.allow_keychange
&& mail.ctype.mimetype != "multipart/report" {
// Disallowing keychanges is disabled for now: peerstate.degrade_encryption(message_time);
// && decryption_info.dkim_results.allow_keychange
{
peerstate.degrade_encryption(message_time);
}
} }
(Ok(mail), HashSet::new(), false)
} }
Err(err) => { }
warn!(context, "decryption failed: {}", err); if !encrypted {
(Err(err), HashSet::new(), true) signatures.clear();
} }
};
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", peerstate.addr,
"using attached PGP key for peer '{}' with prefer-encrypt=mutual as the mail is \ );
encrypted", peerstate.prefer_encrypt = EncryptPreference::Mutual;
peerstate.addr, peerstate.save_to_db(&context.sql).await?;
);
peerstate.prefer_encrypt = EncryptPreference::Mutual;
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)
} }