For setting dkim_works, use the message's time

This commit is contained in:
Hocuri
2022-10-22 22:16:13 +02:00
parent c5e93bf057
commit 8aaea4817d
2 changed files with 19 additions and 9 deletions

View File

@@ -30,6 +30,7 @@ pub(crate) async fn handle_authres(
context: &Context, context: &Context,
mail: &ParsedMail<'_>, mail: &ParsedMail<'_>,
from: &str, from: &str,
message_time: i64,
) -> Result<DkimResults> { ) -> Result<DkimResults> {
let from_domain = match EmailAddress::new(from) { let from_domain = match EmailAddress::new(from) {
Ok(email) => email.domain, Ok(email) => email.domain,
@@ -43,7 +44,7 @@ pub(crate) async fn handle_authres(
let authres = parse_authres_headers(&mail.get_headers(), &from_domain); let authres = parse_authres_headers(&mail.get_headers(), &from_domain);
update_authservid_candidates(context, &authres).await?; update_authservid_candidates(context, &authres).await?;
compute_dkim_results(context, authres, &from_domain).await compute_dkim_results(context, authres, &from_domain, message_time).await
} }
#[derive(Default, Debug)] #[derive(Default, Debug)]
@@ -233,6 +234,7 @@ async fn compute_dkim_results(
context: &Context, context: &Context,
mut authres: ParsedAuthresHeaders, mut authres: ParsedAuthresHeaders,
from_domain: &str, from_domain: &str,
message_time: i64,
) -> Result<DkimResults> { ) -> Result<DkimResults> {
let mut dkim_passed = false; let mut dkim_passed = false;
@@ -267,7 +269,7 @@ async fn compute_dkim_results(
let mut dkim_works = dkim_works(context, from_domain).await?; let mut dkim_works = dkim_works(context, from_domain).await?;
if !dkim_works && dkim_passed { if !dkim_works && dkim_passed {
set_dkim_works(context, from_domain).await?; set_dkim_works(context, from_domain, message_time).await?;
dkim_works = true; dkim_works = true;
} }
@@ -294,17 +296,23 @@ async fn dkim_works(context: &Context, from_domain: &str) -> Result<bool> {
let should_work_until = last_working_timestamp + 3600 * 24 * 30; let should_work_until = last_working_timestamp + 3600 * 24 * 30;
let dkim_ever_worked = last_working_timestamp > 0; let dkim_ever_worked = last_working_timestamp > 0;
// We're using time() here and not the time when the message
// claims to have been sent (passed around as `message_time`)
// because otherwise an attacker could just put a time way
// in the future into the `Date` header and then we would
// assume that DKIM doesn't have to be valid anymore.
let dkim_should_work_now = should_work_until > time(); let dkim_should_work_now = should_work_until > time();
Ok(dkim_ever_worked && dkim_should_work_now) Ok(dkim_ever_worked && dkim_should_work_now)
} }
async fn set_dkim_works(context: &Context, from_domain: &str) -> Result<()> { async fn set_dkim_works(context: &Context, from_domain: &str, timestamp: i64) -> Result<()> {
context context
.sql .sql
.execute( .execute(
"INSERT INTO sending_domains (domain, dkim_works) VALUES (?,?) "INSERT INTO sending_domains (domain, dkim_works) VALUES (?,?)
ON CONFLICT(domain) DO UPDATE SET dkim_works=excluded.dkim_works", ON CONFLICT(domain) DO UPDATE SET dkim_works=excluded.dkim_works",
paramsv![from_domain, time()], paramsv![from_domain, timestamp],
) )
.await?; .await?;
Ok(()) Ok(())
@@ -541,7 +549,7 @@ Authentication-Results: box.hispanilandia.net; spf=pass smtp.mailfrom=adbenitez@
let mail = mailparse::parse_mail(&bytes)?; let mail = mailparse::parse_mail(&bytes)?;
let from = &mimeparser::get_from(&mail.headers)[0].addr; let from = &mimeparser::get_from(&mail.headers)[0].addr;
let res = handle_authres(&t, &mail, from).await?; let res = handle_authres(&t, &mail, from, time()).await?;
assert!(res.allow_keychange); assert!(res.allow_keychange);
} }
@@ -553,7 +561,7 @@ Authentication-Results: box.hispanilandia.net; spf=pass smtp.mailfrom=adbenitez@
let mail = mailparse::parse_mail(&bytes)?; let mail = mailparse::parse_mail(&bytes)?;
let from = &mimeparser::get_from(&mail.headers)[0].addr; let from = &mimeparser::get_from(&mail.headers)[0].addr;
let res = handle_authres(&t, &mail, from).await?; let res = handle_authres(&t, &mail, from, time()).await?;
if !res.allow_keychange { if !res.allow_keychange {
println!( println!(
"!!!!!! FAILURE Receiving {:?}, keychange is not allowed !!!!!!", "!!!!!! FAILURE Receiving {:?}, keychange is not allowed !!!!!!",
@@ -601,7 +609,9 @@ Authentication-Results: box.hispanilandia.net; spf=pass smtp.mailfrom=adbenitez@
// to the database and downloaded again and again // to the database and downloaded again and again
let bytes = b"Authentication-Results: dkim="; let bytes = b"Authentication-Results: dkim=";
let mail = mailparse::parse_mail(bytes).unwrap(); let mail = mailparse::parse_mail(bytes).unwrap();
handle_authres(&t, &mail, "invalidfrom.com").await.unwrap(); handle_authres(&t, &mail, "invalidfrom.com", time())
.await
.unwrap();
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -618,7 +628,7 @@ Authentication-Results: box.hispanilandia.net; spf=pass smtp.mailfrom=adbenitez@
// Assume Alice receives an email from bob@example.net with // Assume Alice receives an email from bob@example.net with
// correct DKIM -> `set_dkim_works()` was called // correct DKIM -> `set_dkim_works()` was called
set_dkim_works(&alice, "example.net").await?; set_dkim_works(&alice, "example.net", time()).await?;
// And Alice knows her server's authserv-id // And Alice knows her server's authserv-id
alice alice
.set_config(Config::AuthservIdCandidates, Some("example.org")) .set_config(Config::AuthservIdCandidates, Some("example.org"))

View File

@@ -72,7 +72,7 @@ pub async fn prepare_decryption(
.ok_or_log_msg(context, "Failed to parse Autocrypt header") .ok_or_log_msg(context, "Failed to parse Autocrypt header")
.flatten(); .flatten();
let dkim_results = handle_authres(context, mail, from).await?; let dkim_results = handle_authres(context, mail, from, message_time).await?;
let peerstate = get_autocrypt_peerstate( let peerstate = get_autocrypt_peerstate(
context, context,