fix: Reject message with forged From even if no valid signatures are found

There are many reasons why we may fail to find valid signatures in a message, e.g. we don't yet know
a public key attached in the same message, anyway, if From is forged, the message must be rejected.

Also always take the displayname from encrypted From, even if no valid signatures are found.
This commit is contained in:
iequidoo
2024-07-22 13:33:05 -03:00
committed by iequidoo
parent a710c034e4
commit 04fd2cdcab
2 changed files with 40 additions and 8 deletions

View File

@@ -396,13 +396,10 @@ impl MimeMessage {
&mail.headers,
);
if let (Some(inner_from), true) = (inner_from, !signatures.is_empty()) {
if addr_cmp(&inner_from.addr, &from.addr) {
from_is_signed = true;
from = inner_from;
} else {
// There is a From: header in the encrypted &
// signed part, but it doesn't match the outer one.
if let Some(inner_from) = inner_from {
if !addr_cmp(&inner_from.addr, &from.addr) {
// There is a From: header in the encrypted
// 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.
@@ -411,7 +408,7 @@ impl MimeMessage {
// so we return an error below.
warn!(
context,
"From header in signed part doesn't match the outer one",
"From header in encrypted part doesn't match the outer one",
);
// Return an error from the parser.
@@ -420,6 +417,8 @@ impl MimeMessage {
// as if the MIME structure is broken.
bail!("From header is forged");
}
from = inner_from;
from_is_signed = !signatures.is_empty();
}
}
if signatures.is_empty() {

View File

@@ -3559,6 +3559,39 @@ async fn test_prefer_encrypt_mutual_if_encrypted() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_forged_from_and_no_valid_signatures() -> Result<()> {
let t = &TestContext::new_bob().await;
let raw = include_bytes!("../../test-data/message/thunderbird_encrypted_signed.eml");
let received_msg = receive_imf(t, raw, false).await?.unwrap();
assert!(!received_msg.from_is_signed);
let msg = t.get_last_msg().await;
assert!(!msg.chat_id.is_trash());
assert!(!msg.get_showpadlock());
let t = &TestContext::new_bob().await;
let raw = String::from_utf8(raw.to_vec())?.replace("alice@example.org", "clarice@example.org");
let received_msg = receive_imf(t, raw.as_bytes(), false).await?.unwrap();
assert!(received_msg.chat_id.is_trash());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_wrong_from_name_and_no_valid_signatures() -> Result<()> {
let t = &TestContext::new_bob().await;
let raw = include_bytes!("../../test-data/message/thunderbird_encrypted_signed.eml");
let raw = String::from_utf8(raw.to_vec())?.replace("From: Alice", "From: A");
let received_msg = receive_imf(t, raw.as_bytes(), false).await?.unwrap();
assert!(!received_msg.from_is_signed);
let msg = t.get_last_msg().await;
assert!(!msg.chat_id.is_trash());
assert!(!msg.get_showpadlock());
let contact = Contact::get_by_id(t, msg.from_id).await?;
assert_eq!(contact.get_authname(), "Alice");
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_thunderbird_autocrypt_unencrypted() -> Result<()> {
let t = TestContext::new_bob().await;