From 659a3aa317a87065573b98c643500d74238b0229 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Sun, 31 Dec 2023 11:48:28 +0100 Subject: [PATCH] SMTP sender validation --- CHANGELOG.md | 1 + crates/smtp/src/config/mod.rs | 1 + crates/smtp/src/config/session.rs | 3 +++ crates/smtp/src/core/mod.rs | 5 +++++ crates/smtp/src/core/params.rs | 1 + crates/smtp/src/inbound/auth.rs | 14 +++++++++----- crates/smtp/src/inbound/mail.rs | 11 +++++++++++ crates/smtp/src/scripts/exec.rs | 5 +---- tests/src/smtp/inbound/auth.rs | 10 +++++++--- tests/src/smtp/mod.rs | 1 + tests/src/smtp/session.rs | 8 ++++---- 11 files changed, 44 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8fe4a65..e6da7e62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. This projec ## Added - SMTP smuggling protection: Sanitization of outgoing messages that do not use `CRLF` as line endings. +- SMTP sender validation for authenticated users: Added the `session.auth.must-match-sender` configuration option to enforce that the sender address used in the `MAIL FROM` command matches the authenticated user or any of their associated e-mail addresses. ### Changed diff --git a/crates/smtp/src/config/mod.rs b/crates/smtp/src/config/mod.rs index 92da2458..9fa7e8ad 100644 --- a/crates/smtp/src/config/mod.rs +++ b/crates/smtp/src/config/mod.rs @@ -227,6 +227,7 @@ pub struct Auth { pub mechanisms: IfBlock, pub require: IfBlock, pub allow_plain_text: IfBlock, + pub must_match_sender: IfBlock, pub errors_max: IfBlock, pub errors_wait: IfBlock, } diff --git a/crates/smtp/src/config/session.rs b/crates/smtp/src/config/session.rs index fad79705..c86a348f 100644 --- a/crates/smtp/src/config/session.rs +++ b/crates/smtp/src/config/session.rs @@ -287,6 +287,9 @@ impl ConfigSession for Config { allow_plain_text: self .parse_if_block("session.auth.allow-plain-text", ctx, &available_keys)? .unwrap_or_else(|| IfBlock::new(false)), + must_match_sender: self + .parse_if_block("session.auth.must-match-sender", ctx, &available_keys)? + .unwrap_or_else(|| IfBlock::new(true)), }) } diff --git a/crates/smtp/src/core/mod.rs b/crates/smtp/src/core/mod.rs index 50a00d8c..4a3eecbe 100644 --- a/crates/smtp/src/core/mod.rs +++ b/crates/smtp/src/core/mod.rs @@ -198,6 +198,7 @@ pub struct SessionData { pub message: Vec, pub authenticated_as: String, + pub authenticated_emails: Vec, pub auth_errors: usize, pub priority: i16, @@ -238,6 +239,7 @@ pub struct SessionParameters { pub auth_errors_max: usize, pub auth_errors_wait: Duration, pub auth_plain_text: bool, + pub auth_match_sender: bool, // Rcpt parameters pub rcpt_errors_max: usize, @@ -264,6 +266,7 @@ impl SessionData { mail_from: None, rcpt_to: Vec::new(), authenticated_as: String::new(), + authenticated_emails: Vec::new(), priority: 0, valid_until: Instant::now(), rcpt_errors: 0, @@ -527,6 +530,7 @@ impl Session { rcpt_max: Default::default(), rcpt_dsn: Default::default(), max_message_size: Default::default(), + auth_match_sender: false, iprev: crate::config::VerifyStrategy::Disable, spf_ehlo: crate::config::VerifyStrategy::Disable, spf_mail_from: crate::config::VerifyStrategy::Disable, @@ -582,6 +586,7 @@ impl SessionData { rcpt_errors: 0, message, authenticated_as: "local".into(), + authenticated_emails: vec![], auth_errors: 0, priority: 0, delivery_by: 0, diff --git a/crates/smtp/src/core/params.rs b/crates/smtp/src/core/params.rs index 570b24d6..583fa52c 100644 --- a/crates/smtp/src/core/params.rs +++ b/crates/smtp/src/core/params.rs @@ -48,6 +48,7 @@ impl Session { self.params.auth_errors_max = *ac.errors_max.eval(self).await; self.params.auth_errors_wait = *ac.errors_wait.eval(self).await; self.params.auth_plain_text = *ac.allow_plain_text.eval(self).await; + self.params.auth_match_sender = *ac.must_match_sender.eval(self).await; // VRFY/EXPN parameters let ec = &self.core.session.config.extensions; diff --git a/crates/smtp/src/inbound/auth.rs b/crates/smtp/src/inbound/auth.rs index fbd9eb02..66e05092 100644 --- a/crates/smtp/src/inbound/auth.rs +++ b/crates/smtp/src/inbound/auth.rs @@ -181,19 +181,23 @@ impl Session { | Credentials::XOauth2 { username, .. } | Credentials::OAuthBearer { token: username } => username.to_string(), }; - if let Ok(is_authenticated) = lookup + if let Ok(principal) = lookup .query(QueryBy::Credentials(&credentials), false) .await - .map(|r| r.is_some()) { tracing::debug!( parent: &self.span, context = "auth", event = "authenticate", - result = if is_authenticated {"success"} else {"failed"} + result = if principal.is_some() {"success"} else {"failed"} ); - return if is_authenticated { - self.data.authenticated_as = authenticated_as; + return if let Some(principal) = principal { + self.data.authenticated_as = authenticated_as.to_lowercase(); + self.data.authenticated_emails = principal + .emails + .into_iter() + .map(|e| e.trim().to_lowercase()) + .collect(); self.eval_post_auth_params().await; self.write(b"235 2.7.0 Authentication succeeded.\r\n") .await?; diff --git a/crates/smtp/src/inbound/mail.rs b/crates/smtp/src/inbound/mail.rs index dbd352d7..b678ddab 100644 --- a/crates/smtp/src/inbound/mail.rs +++ b/crates/smtp/src/inbound/mail.rs @@ -104,6 +104,17 @@ impl Session { (String::new(), String::new(), String::new()) }; + // Make sure that the authenticated user is allowed to send from this address + if !self.data.authenticated_as.is_empty() + && self.params.auth_match_sender + && (self.data.authenticated_as != address_lcase + && !self.data.authenticated_emails.contains(&address_lcase)) + { + return self + .write(b"501 5.5.4 You are not allowed to send from this address.\r\n") + .await; + } + let has_dsn = from.env_id.is_some(); self.data.mail_from = SessionAddress { address, diff --git a/crates/smtp/src/scripts/exec.rs b/crates/smtp/src/scripts/exec.rs index b51f6899..f0965f52 100644 --- a/crates/smtp/src/scripts/exec.rs +++ b/crates/smtp/src/scripts/exec.rs @@ -45,10 +45,7 @@ impl Session { .set_variable("remote_ip", self.data.remote_ip.to_string()) .set_variable("remote_ip.reverse", self.data.remote_ip.to_reverse_name()) .set_variable("helo_domain", self.data.helo_domain.to_lowercase()) - .set_variable( - "authenticated_as", - self.data.authenticated_as.to_lowercase(), - ) + .set_variable("authenticated_as", self.data.authenticated_as.clone()) .set_variable( "now", SystemTime::now() diff --git a/tests/src/smtp/inbound/auth.rs b/tests/src/smtp/inbound/auth.rs index f38b7844..13d0eb1f 100644 --- a/tests/src/smtp/inbound/auth.rs +++ b/tests/src/smtp/inbound/auth.rs @@ -31,7 +31,7 @@ use crate::smtp::{ ParseTestConfig, TestConfig, }; use smtp::{ - config::{ConfigContext, EnvelopeKey}, + config::{ConfigContext, EnvelopeKey, IfBlock}, core::{Session, State, SMTP}, }; @@ -87,12 +87,13 @@ async fn auth() { ) .as_str() .parse_if(&ctx); + config.must_match_sender = IfBlock::new(true); core.session.config.extensions.future_release = r"[{if = 'authenticated-as', ne = '', then = '1d'}, {else = false}]" .parse_if(&ConfigContext::new(&[])); - // EHLO should not avertise plain text auth without TLS + // EHLO should not advertise plain text auth without TLS let mut session = Session::test(core); session.data.remote_ip = "10.0.0.1".parse().unwrap(); session.eval_session_params().await; @@ -134,7 +135,10 @@ async fn auth() { session .cmd("AUTH PLAIN AGpvaG4Ac2VjcmV0", "235 2.7.0") .await; - session.mail_from("bill@foobar.org", "250").await; + + // Users should be able to send emails only from their own email addresses + session.mail_from("bill@foobar.org", "501 5.5.4").await; + session.mail_from("john@example.org", "250").await; session.data.mail_from.take(); // Should not be able to authenticate twice diff --git a/tests/src/smtp/mod.rs b/tests/src/smtp/mod.rs index e170053d..c15bf085 100644 --- a/tests/src/smtp/mod.rs +++ b/tests/src/smtp/mod.rs @@ -238,6 +238,7 @@ impl TestConfig for SessionConfig { errors_max: IfBlock::new(10), errors_wait: IfBlock::new(Duration::from_secs(1)), allow_plain_text: IfBlock::new(false), + must_match_sender: IfBlock::new(false), }, mail: Mail { script: IfBlock::new(None), diff --git a/tests/src/smtp/session.rs b/tests/src/smtp/session.rs index c32cf55f..0570809a 100644 --- a/tests/src/smtp/session.rs +++ b/tests/src/smtp/session.rs @@ -311,7 +311,7 @@ pub trait VerifyResponse { fn assert_code(self, expected_code: &str) -> Self; fn assert_contains(self, expected_text: &str) -> Self; fn assert_not_contains(self, expected_text: &str) -> Self; - fn assert_count(self, text: &str, occurences: usize) -> Self; + fn assert_count(self, text: &str, occurrences: usize) -> Self; } impl VerifyResponse for Vec { @@ -343,12 +343,12 @@ impl VerifyResponse for Vec { } } - fn assert_count(self, text: &str, occurences: usize) -> Self { + fn assert_count(self, text: &str, occurrences: usize) -> Self { assert_eq!( self.iter().filter(|l| l.contains(text)).count(), - occurences, + occurrences, "Expected {} occurrences of {:?}, found {}.", - occurences, + occurrences, text, self.iter().filter(|l| l.contains(text)).count() );