Try to not allow a message_time in the future

This commit is contained in:
Hocuri
2022-10-26 20:44:10 +02:00
parent 3db9336181
commit 525e5b0b58
2 changed files with 25 additions and 14 deletions

View File

@@ -13,7 +13,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::time;
use crate::tools::EmailAddress; use crate::tools::EmailAddress;
/// `authres` is short for the Authentication-Results header, defined in /// `authres` is short for the Authentication-Results header, defined in
@@ -273,7 +272,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, message_time).await?;
if !dkim_works && dkim_passed { if !dkim_works && dkim_passed {
set_dkim_works(context, from_domain, message_time).await?; set_dkim_works(context, from_domain, message_time).await?;
dkim_works = true; dkim_works = true;
@@ -287,7 +286,7 @@ async fn compute_dkim_results(
} }
/// Whether DKIM in emails from this domain is known to work. /// Whether DKIM in emails from this domain is known to work.
async fn dkim_works(context: &Context, from_domain: &str) -> Result<bool> { async fn dkim_works(context: &Context, from_domain: &str, message_time: i64) -> Result<bool> {
let last_working_timestamp: i64 = context let last_working_timestamp: i64 = context
.sql .sql
.query_get_value( .query_get_value(
@@ -303,12 +302,7 @@ async fn dkim_works(context: &Context, from_domain: &str) -> Result<bool> {
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 let dkim_should_work_now = should_work_until > message_time;
// 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();
Ok(dkim_ever_worked && dkim_should_work_now) Ok(dkim_ever_worked && dkim_should_work_now)
} }
@@ -358,6 +352,7 @@ mod tests {
use crate::test_utils::TestContext; use crate::test_utils::TestContext;
use crate::test_utils::TestContextManager; use crate::test_utils::TestContextManager;
use crate::tools; use crate::tools;
use crate::tools::time;
#[test] #[test]
fn test_remove_comments() { fn test_remove_comments() {
@@ -589,7 +584,10 @@ Authentication-Results: box.hispanilandia.net; spf=pass smtp.mailfrom=adbenitez@
} }
let from_domain = EmailAddress::new(from).unwrap().domain; let from_domain = EmailAddress::new(from).unwrap().domain;
assert_eq!(res.dkim_works, dkim_works(&t, &from_domain).await.unwrap()); assert_eq!(
res.dkim_works,
dkim_works(&t, &from_domain, time()).await.unwrap()
);
assert_eq!(res.dkim_passed, res.dkim_works); assert_eq!(res.dkim_passed, res.dkim_works);
// delta.blinzeln.de and gmx.de have invalid DKIM, so the DKIM check should fail // delta.blinzeln.de and gmx.de have invalid DKIM, so the DKIM check should fail

View File

@@ -1,5 +1,6 @@
//! # MIME message parsing module. //! # MIME message parsing module.
use std::cmp::min;
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use std::future::Future; use std::future::Future;
use std::pin::Pin; use std::pin::Pin;
@@ -28,7 +29,7 @@ use crate::peerstate::Peerstate;
use crate::simplify::{simplify, SimplifiedText}; use crate::simplify::{simplify, SimplifiedText};
use crate::stock_str; use crate::stock_str;
use crate::sync::SyncItems; use crate::sync::SyncItems;
use crate::tools::{get_filemeta, parse_receive_headers, truncate_by_lines}; use crate::tools::{get_filemeta, parse_receive_headers, smeared_time, time, truncate_by_lines};
/// A parsed MIME message. /// A parsed MIME message.
/// ///
@@ -173,11 +174,23 @@ impl MimeMessage {
) -> Result<Self> { ) -> Result<Self> {
let mail = mailparse::parse_mail(body)?; let mail = mailparse::parse_mail(body)?;
let message_time = mail let rcvd_timestamp = time();
.headers let date_header = mail.headers.get_header_value(HeaderDef::Date);
.get_header_value(HeaderDef::Date) let alleged_message_time = date_header
.as_ref()
.and_then(|v| mailparse::dateparse(&v).ok()) .and_then(|v| mailparse::dateparse(&v).ok())
.unwrap_or_default(); .unwrap_or_default();
let message_time = min(alleged_message_time, rcvd_timestamp);
if message_time != alleged_message_time {
info!(
context,
"Message claims to come from the future ({:?}), using current time ({:?}) instead",
date_header,
message_time
)
}
let mut hop_info = parse_receive_headers(&mail.get_headers()); let mut hop_info = parse_receive_headers(&mail.get_headers());
let mut headers = Default::default(); let mut headers = Default::default();