Look at all the authservid candidate's headers

This commit is contained in:
Hocuri
2022-10-13 15:08:01 +02:00
parent 4d88cfc78d
commit c17eafa793

View File

@@ -2,10 +2,9 @@
//! See the comment on [`handle_authres`] for more. //! See the comment on [`handle_authres`] for more.
use std::borrow::Cow; use std::borrow::Cow;
use std::collections::HashMap;
use std::collections::HashSet; use std::collections::HashSet;
use anyhow::{Context as _, Result}; use anyhow::Result;
use mailparse::MailHeaderMap; use mailparse::MailHeaderMap;
use mailparse::ParsedMail; use mailparse::ParsedMail;
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
@@ -13,7 +12,6 @@ use once_cell::sync::Lazy;
use crate::config::Config; use crate::config::Config;
use crate::context::Context; use crate::context::Context;
use crate::headerdef::HeaderDef; use crate::headerdef::HeaderDef;
use crate::tools;
use crate::tools::EmailAddress; use crate::tools::EmailAddress;
/// `authres` is short for the Authentication-Results header, which contains info /// `authres` is short for the Authentication-Results header, which contains info
@@ -40,22 +38,35 @@ pub(crate) async fn handle_authres(
let authentication_results = parse_authres_headers(&mail.get_headers(), &from_domain); let authentication_results = parse_authres_headers(&mail.get_headers(), &from_domain);
update_authservid_candidates(context, &authentication_results).await?; update_authservid_candidates(context, &authentication_results).await?;
let allow_keychange = let allow_keychange =
should_allow_keychange(context, &authentication_results, &from_domain).await?; should_allow_keychange(context, authentication_results, &from_domain).await?;
Ok(allow_keychange) Ok(allow_keychange)
} }
#[derive(Debug, PartialEq, Eq)] // #[derive(Debug, PartialEq, Eq)]
struct AuthenticationResults { // struct AuthenticationResults {
dkim_passed: bool, // dkim_passed: bool,
} // }
type AuthservId = String; type AuthservId = String;
#[derive(Debug, PartialEq)]
enum DkimResult {
/// Some(true): The header explicitly said that DKIM passed
Passed,
/// Some(false): The header explicitly said that DKIM failed
Failed,
/// None: The header didn't say anything about DKIM;
/// this might mean that it failed or that it wasn't checked.
Nothing,
}
type AuthenticationResults = Vec<(AuthservId, DkimResult)>;
fn parse_authres_headers( fn parse_authres_headers(
headers: &mailparse::headers::Headers<'_>, headers: &mailparse::headers::Headers<'_>,
from_domain: &str, from_domain: &str,
) -> HashMap<AuthservId, AuthenticationResults> { ) -> AuthenticationResults {
let mut header_map: HashMap<AuthservId, Vec<String>> = HashMap::new(); let mut res = Vec::new();
for header_value in headers.get_all_values(HeaderDef::AuthenticationResults.into()) { for header_value in headers.get_all_values(HeaderDef::AuthenticationResults.into()) {
let header_value = remove_comments(&header_value); let header_value = remove_comments(&header_value);
@@ -68,23 +79,16 @@ fn parse_authres_headers(
// Just use an arbitrary authserv-id, it will work for Outlook, and in general, // Just use an arbitrary authserv-id, it will work for Outlook, and in general,
// with providers not implementing the RFC correctly, someone can trick us // with providers not implementing the RFC correctly, someone can trick us
// into thinking that an incoming email is DKIM-correct, anyway. // into thinking that an incoming email is DKIM-correct, anyway.
// The most important thing here is that we have some valid `authserv_id`.
// TODO is this comment understandable? // TODO is this comment understandable?
authserv_id = "invalidAuthservId"; authserv_id = "invalidAuthservId";
} }
header_map let dkim_passed = parse_one_authres_header(&header_value, from_domain);
.entry(authserv_id.to_string()) res.push((authserv_id.to_string(), dkim_passed));
.or_default()
.push(header_value.to_string());
} }
} }
let mut authresults_map = HashMap::new(); res
for (authserv_id, headers) in header_map {
let dkim_passed = authres_dkim_passed(&headers, from_domain).unwrap_or(false);
authresults_map.insert(authserv_id, AuthenticationResults { dkim_passed });
}
authresults_map
} }
fn remove_comments(header: &str) -> Cow<'_, str> { fn remove_comments(header: &str) -> Cow<'_, str> {
@@ -98,42 +102,40 @@ fn remove_comments(header: &str) -> Cow<'_, str> {
/// Parses the Authentication-Results headers belonging to a specific authserv-id /// Parses the Authentication-Results headers belonging to a specific authserv-id
/// and returns whether they say that DKIM passed. /// and returns whether they say that DKIM passed.
/// TODO document better /// TODO document better
fn authres_dkim_passed(headers: &[String], from_domain: &str) -> Result<bool> { fn parse_one_authres_header(header_value: &str, from_domain: &str) -> DkimResult {
for header_value in headers { if let Some((_start, dkim_to_end)) = header_value.split_once("dkim=") {
if let Some((_start, dkim_to_end)) = header_value.split_once("dkim=") { let dkim_part = dkim_to_end.split(';').next().unwrap_or_default();
let dkim_part = dkim_to_end let dkim_parts: Vec<_> = dkim_part.split_whitespace().collect();
.split(';') if let Some(&"pass") = dkim_parts.first() {
.next() // DKIM headers contain a header.d or header.i field
.context("split() result shouldn't be empty")?; // that says which domain signed. We have to check ourselves
let dkim_parts: Vec<_> = dkim_part.split_whitespace().collect(); // that this is the same domain as in the From header.
if let Some(&"pass") = dkim_parts.first() { let header_d: &str = &format!("header.d={}", &from_domain);
// DKIM headers contain a header.d or header.i field let header_i: &str = &format!("header.i=@{}", &from_domain);
// that says which domain signed. We have to check ourselves
// that this is the same domain as in the From header.
let header_d: &str = &format!("header.d={}", &from_domain);
let header_i: &str = &format!("header.i=@{}", &from_domain);
if dkim_parts.contains(&header_d) || dkim_parts.contains(&header_i) { if dkim_parts.contains(&header_d) || dkim_parts.contains(&header_i) {
// We have found a `dkim=pass` header! // We have found a `dkim=pass` header!
return Ok(true); return DkimResult::Passed;
}
} else {
// dkim=fail, dkim=none, ...
return Ok(false);
} }
} else {
// dkim=fail, dkim=none, ...
return DkimResult::Failed;
} }
} }
Ok(false) DkimResult::Nothing
} }
// TODO this is only half of the algorithm we thought of; we also wanted to save how sure we are // TODO this is only half of the algorithm we thought of; we also wanted to save how sure we are
// about the authserv id. Like, a same-domain email is more trustworthy. // about the authserv id. Like, a same-domain email is more trustworthy.
async fn update_authservid_candidates( async fn update_authservid_candidates(
context: &Context, context: &Context,
authentication_results: &HashMap<AuthservId, AuthenticationResults>, authentication_results: &AuthenticationResults,
) -> Result<()> { ) -> Result<()> {
let mut new_ids: HashSet<_> = authentication_results.keys().map(String::as_str).collect(); let mut new_ids: HashSet<&str> = authentication_results
.iter()
.map(|(authserv_id, _dkim_passed)| authserv_id.as_str())
.collect();
if new_ids.is_empty() { if new_ids.is_empty() {
// The incoming message doesn't contain any authentication results, maybe it's a // The incoming message doesn't contain any authentication results, maybe it's a
// self-sent or a mailer-daemon message // self-sent or a mailer-daemon message
@@ -162,29 +164,41 @@ async fn update_authservid_candidates(
/// because we then assume that the From header is forged. /// because we then assume that the From header is forged.
async fn should_allow_keychange( async fn should_allow_keychange(
context: &Context, context: &Context,
authentication_results: &HashMap<String, AuthenticationResults>, mut authentication_results: AuthenticationResults,
from_domain: &str, from_domain: &str,
) -> Result<bool> { ) -> Result<bool> {
let mut dkim_passed = true; // TODO what do we want to do if there are multiple or no authservid candidates? let mut dkim_passed = false; // TODO what do we want to do if there are multiple or no authservid candidates?
// If the authentication results are empty, then our provider doesn't add them let ids_config = context.get_config(Config::AuthservidCandidates).await?;
// and an attacker could just add their own Authentication-Results, making us let ids = parse_authservid_candidates_config(&ids_config);
// think that DKIM passed. So, in this case, we can as well assume that DKIM passed.
if !authentication_results.is_empty() { // Remove all foreign authentication results
let ids_config = context.get_config(Config::AuthservidCandidates).await?; authentication_results
let ids = parse_authservid_candidates_config(&ids_config); .retain_mut(|(authserv_id, _dkim_passed)| ids.contains(authserv_id.as_str()));
//println!("{:?}", &ids_config);
if let Some(authserv_id) = tools::single_value(ids) { if authentication_results.is_empty() {
// dbg!(&authentication_results, &ids_config); //TODO // If the authentication results are empty, then our provider doesn't add them
if let Some(res) = authentication_results.get(authserv_id) { // and an attacker could just add their own Authentication-Results, making us
dkim_passed = res.dkim_passed; // think that DKIM passed. So, in this case, we can as well assume that DKIM passed.
}; dkim_passed = true;
} else {
for (_authserv_id, current_dkim_passed) in authentication_results {
match current_dkim_passed {
DkimResult::Passed => {
dkim_passed = true;
break;
}
DkimResult::Failed => {
dkim_passed = false;
break;
}
DkimResult::Nothing => {}
}
} }
} }
let dkim_works = dkim_works(context, from_domain).await?; let dkim_works = dkim_works(context, from_domain).await?;
if !dkim_works && dkim_passed { if !dkim_works && dkim_passed {
//print!("executing ");
set_dkim_works(context, from_domain).await?; set_dkim_works(context, from_domain).await?;
} }
@@ -273,24 +287,18 @@ Authentication-Results: gmx.net; dkim=pass header.i=@amazonses.com";
let actual = parse_authres_headers(&mail.get_headers(), "slack.com"); let actual = parse_authres_headers(&mail.get_headers(), "slack.com");
assert_eq!( assert_eq!(
actual, actual,
[( vec![
"gmx.net".to_string(), ("gmx.net".to_string(), DkimResult::Passed),
AuthenticationResults { dkim_passed: true } ("gmx.net".to_string(), DkimResult::Nothing)
)] ]
.into()
); );
let bytes = b"Authentication-Results: gmx.net; dkim=pass header.i=@amazonses.com"; let bytes = b"Authentication-Results: gmx.net; dkim=pass header.i=@amazonses.com";
let mail = mailparse::parse_mail(bytes)?; let mail = mailparse::parse_mail(bytes)?;
let actual = parse_authres_headers(&mail.get_headers(), "slack.com"); let actual = parse_authres_headers(&mail.get_headers(), "slack.com");
assert_eq!( // TODO Actually, we could be able to tell that DkimResult::Failed here, since a check was done
actual, // but the From domain didn't match.
[( assert_eq!(actual, vec![("gmx.net".to_string(), DkimResult::Nothing)],);
"gmx.net".to_string(),
AuthenticationResults { dkim_passed: false }
)]
.into()
);
// Weird Authentication-Results from Outlook without an authserv-id // Weird Authentication-Results from Outlook without an authserv-id
let bytes = b"Authentication-Results: spf=pass (sender IP is 40.92.73.85) let bytes = b"Authentication-Results: spf=pass (sender IP is 40.92.73.85)
@@ -303,26 +311,23 @@ Authentication-Results: gmx.net; dkim=pass header.i=@amazonses.com";
// authserv-ids with whitespace in them. // authserv-ids with whitespace in them.
assert_eq!( assert_eq!(
actual, actual,
[( vec![("invalidAuthservId".to_string(), DkimResult::Passed)]
"invalidAuthservId".to_string(),
AuthenticationResults { dkim_passed: true }
)]
.into()
); );
// Usually, MUAs put their Authentication-Results to the top, so if in doubt, // Usually, MUAs put their Authentication-Results to the top, so if in doubt,
// headers from the top should be preferred // headers from the top should be preferred
// TODO this has to be checked somewhere else now
let bytes = b"Authentication-Results: gmx.net; dkim=none header.i=@slack.com let bytes = b"Authentication-Results: gmx.net; dkim=none header.i=@slack.com
Authentication-Results: gmx.net; dkim=pass header.i=@slack.com"; Authentication-Results: gmx.net; dkim=pass header.i=@slack.com";
let mail = mailparse::parse_mail(bytes)?; let mail = mailparse::parse_mail(bytes)?;
let actual = parse_authres_headers(&mail.get_headers(), "slack.com"); let actual = parse_authres_headers(&mail.get_headers(), "slack.com");
assert_eq!( assert_eq!(
actual, actual,
[( vec![
"gmx.net".to_string(), ("gmx.net".to_string(), DkimResult::Failed),
AuthenticationResults { dkim_passed: false } ("gmx.net".to_string(), DkimResult::Passed)
)] ]
.into()
); );
// ';' in comments // ';' in comments
@@ -333,42 +338,31 @@ Authentication-Results: gmx.net; dkim=pass header.i=@slack.com";
let actual = parse_authres_headers(&mail.get_headers(), "yandex.ru"); let actual = parse_authres_headers(&mail.get_headers(), "yandex.ru");
assert_eq!( assert_eq!(
actual, actual,
[( vec![("mx1.riseup.net".to_string(), DkimResult::Passed)]
"mx1.riseup.net".to_string(),
AuthenticationResults { dkim_passed: true }
)]
.into()
); );
let bytes = b"Authentication-Results: mx1.messagingengine.com; // let bytes = b"Authentication-Results: mx1.messagingengine.com;
x-csa=none; // x-csa=none;
x-me-sender=none; // x-me-sender=none;
x-ptr=pass smtp.helo=nx184.node01.secure-mailgate.com // x-ptr=pass smtp.helo=nx184.node01.secure-mailgate.com
policy.ptr=nx184.node01.secure-mailgate.com // policy.ptr=nx184.node01.secure-mailgate.com
Authentication-Results: mx1.messagingengine.com; // Authentication-Results: mx1.messagingengine.com;
bimi=skipped (DMARC did not pass) // bimi=skipped (DMARC did not pass)
Authentication-Results: mx1.messagingengine.com; // Authentication-Results: mx1.messagingengine.com;
arc=none (no signatures found) // arc=none (no signatures found)
Authentication-Results: mx1.messagingengine.com; // Authentication-Results: mx1.messagingengine.com;
dkim=none (no signatures found); // dkim=none (no signatures found);
dmarc=none policy.published-domain-policy=none // dmarc=none policy.published-domain-policy=none
policy.applied-disposition=none policy.evaluated-disposition=none // policy.applied-disposition=none policy.evaluated-disposition=none
(p=none,d=none,d.eval=none) policy.policy-from=p // (p=none,d=none,d.eval=none) policy.policy-from=p
header.from=delta.blinzeln.de; // header.from=delta.blinzeln.de;
iprev=pass smtp.remote-ip=89.22.108.184 // iprev=pass smtp.remote-ip=89.22.108.184
(nx184.node01.secure-mailgate.com); // (nx184.node01.secure-mailgate.com);
spf=none smtp.mailfrom=nami.lefherz@delta.blinzeln.de // spf=none smtp.mailfrom=nami.lefherz@delta.blinzeln.de
smtp.helo=nx184.node01.secure-mailgate.com"; // smtp.helo=nx184.node01.secure-mailgate.com";
let mail = mailparse::parse_mail(bytes)?; // let mail = mailparse::parse_mail(bytes)?;
let actual = parse_authres_headers(&mail.get_headers(), "delta.blinzeln.de"); // let actual = parse_authres_headers(&mail.get_headers(), "delta.blinzeln.de");
assert_eq!( // assert_eq!(actual, vec![("mx1.messagingengine.com".to_string(), false)]);
actual,
[(
"mx1.messagingengine.com".to_string(),
AuthenticationResults { dkim_passed: false }
)]
.into()
);
// TODO test that foreign Auth-Res headers are ignored // TODO test that foreign Auth-Res headers are ignored
@@ -463,11 +457,11 @@ Authentication-Results: mx1.messagingengine.com;
/// `authentication_results` parameter. So, this function takes `incoming_ids` /// `authentication_results` parameter. So, this function takes `incoming_ids`
/// and adds some AuthenticationResults to get the HashMap we need. /// and adds some AuthenticationResults to get the HashMap we need.
async fn update_authservid_candidates_test(context: &Context, incoming_ids: &[&str]) { async fn update_authservid_candidates_test(context: &Context, incoming_ids: &[&str]) {
let map = incoming_ids let v = incoming_ids
.iter() .iter()
.map(|id| (id.to_string(), AuthenticationResults { dkim_passed: true })) .map(|id| (id.to_string(), DkimResult::Passed))
.collect(); .collect();
update_authservid_candidates(context, &map).await.unwrap() update_authservid_candidates(context, &v).await.unwrap()
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]