diff --git a/app/email/status.py b/app/email/status.py index 05982d99..b0403749 100644 --- a/app/email/status.py +++ b/app/email/status.py @@ -21,6 +21,7 @@ E212 = "250 SL E212 Bounce Reply phase handled" E213 = "250 SL E213 Unknown email ignored" E214 = "250 SL E214 Unauthorized for using reverse alias" E215 = "250 SL E215 Handled dmarc policy" +E216 = "250 SL E216 Handled spf policy" # endregion diff --git a/app/email_utils.py b/app/email_utils.py index 12850b05..f84b755f 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -71,6 +71,8 @@ from app.models import ( IgnoreBounceSender, InvalidMailboxDomain, DmarcCheckResult, + SpamdResult, + SPFCheckResult, ) from app.utils import ( random_string, @@ -1441,19 +1443,26 @@ def save_email_for_debugging(msg: Message, file_name_prefix=None) -> str: return "" -def get_dmarc_status(msg: Message) -> Optional[DmarcCheckResult]: - spam_result = msg.get_all(headers.SPAMD_RESULT) - if not spam_result: +def get_spamd_result(msg: Message) -> Optional[SpamdResult]: + spam_result_header = msg.get_all(headers.SPAMD_RESULT) + if not spam_result_header: + newrelic.agent.record_custom_event("SpamdCheck", {"header": "missing"}) return None - spam_entries = [entry.strip() for entry in str(spam_result[-1]).split("\n")] + spam_entries = [entry.strip() for entry in str(spam_result_header[-1]).split("\n")] for entry_pos in range(len(spam_entries)): sep = spam_entries[entry_pos].find("(") if sep > -1: spam_entries[entry_pos] = spam_entries[entry_pos][:sep] + spamd_result = SpamdResult() + for header_value, dmarc_result in DmarcCheckResult.get_string_dict().items(): if header_value in spam_entries: - return dmarc_result + spamd_result.set_dmarc_result(dmarc_result) + for header_value, spf_result in SPFCheckResult.get_string_dict().items(): + if header_value in spam_entries: + spamd_result.set_spf_result(spf_result) - return None + newrelic.agent.record_custom_event("SpamdCheck", spamd_result.event_data()) + return spamd_result diff --git a/app/models.py b/app/models.py index c0eca242..a756e1de 100644 --- a/app/models.py +++ b/app/models.py @@ -3,7 +3,7 @@ import os import random import uuid from email.utils import formataddr -from typing import List, Tuple, Optional +from typing import List, Tuple, Optional, Dict import arrow import sqlalchemy as sa @@ -263,7 +263,7 @@ class SPFCheckResult(EnumE): soft_fail = 1 neutral = 2 temp_error = 3 - none = 4 + not_available = 4 perm_error = 5 @staticmethod @@ -274,11 +274,27 @@ class SPFCheckResult(EnumE): "R_SPF_SOFTFAIL": SPFCheckResult.soft_fail, "R_SPF_NEUTRAL": SPFCheckResult.neutral, "R_SPF_DNSFAIL": SPFCheckResult.temp_error, - "R_SPF_NA": SPFCheckResult.none, + "R_SPF_NA": SPFCheckResult.not_available, "R_SPF_PERMFAIL": SPFCheckResult.perm_error, } +class SpamdResult: + def __init__(self): + self.dmarc: DmarcCheckResult = DmarcCheckResult.not_available + self.spf: SPFCheckResult = SPFCheckResult.not_available + self.domain = "unknown" + + def set_dmarc_result(self, dmarc_result: DmarcCheckResult): + self.dmarc = dmarc_result + + def set_spf_result(self, spf_result: SPFCheckResult): + self.spf = spf_result + + def event_data(self) -> Dict: + return {"header": "present", "dmarc": self.dmarc, "spf": self.spf} + + class Hibp(Base, ModelMixin): __tablename__ = "hibp" name = sa.Column(sa.String(), nullable=False, unique=True, index=True) diff --git a/email_handler.py b/email_handler.py index 4f03854a..ffdcca4d 100644 --- a/email_handler.py +++ b/email_handler.py @@ -130,7 +130,7 @@ from app.email_utils import ( get_orig_message_from_yahoo_complaint, get_mailbox_bounce_info, save_email_for_debugging, - get_dmarc_status, + get_spamd_result, ) from app.errors import ( NonReverseAliasInReplyPhase, @@ -156,6 +156,7 @@ from app.models import ( DomainDeletedAlias, Notification, DmarcCheckResult, + SPFCheckResult, ) from app.pgp_utils import PGPException, sign_data_with_pgpy, sign_data from app.utils import sanitize_email @@ -541,25 +542,28 @@ def handle_email_sent_to_ourself(alias, from_addr: str, msg: Message, user): def apply_dmarc_policy( - alias: Alias, contact: Contact, envelope: Envelope, msg: Message, from_header + alias: Alias, contact: Contact, envelope: Envelope, msg: Message ) -> Optional[str]: - dmarc_result = get_dmarc_status(msg) - if dmarc_result: - newrelic.agent.record_custom_event("DmarcCheck", {"result": dmarc_result.name}) - else: - newrelic.agent.record_custom_event("DmarcCheck", {"result": "unknown"}) - - if not DMARC_CHECK_ENABLED or not dmarc_result: + spam_result = get_spamd_result(msg) + if not DMARC_CHECK_ENABLED or not spam_result: return None - if dmarc_result in ( + from_header = get_header_unicode(msg[headers.FROM]) + # todo: remove when soft_fail email is put into quarantine + if spam_result.dmarc == DmarcCheckResult.soft_fail: + LOG.w( + f"dmarc soft_fail from contact {contact.email} to alias {alias.email}." + f"mail_from:{envelope.mail_from}, from_header: {from_header}" + ) + return None + if spam_result.dmarc in ( DmarcCheckResult.quarantine, DmarcCheckResult.reject, # todo: disable soft_fail for now # DmarcCheckResult.soft_fail, ): LOG.w( - f"put email from {contact} to {alias} to quarantine. {dmarc_result}, " + f"put email from {contact} to {alias} to quarantine. {spam_result}, " f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" ) email_log = quarantine_dmarc_failed_email(alias, contact, envelope, msg) @@ -593,13 +597,6 @@ def apply_dmarc_policy( ignore_smtp_error=True, ) return status.E215 - # todo: remove when soft_fail email is put into quarantine - elif dmarc_result == DmarcCheckResult.soft_fail: - LOG.w( - f"dmarc soft_fail from {contact} to {alias}." - f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" - ) - return None @@ -718,9 +715,7 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str return [(True, res_status)] # Check if we need to reject or quarantine based on dmarc - dmarc_delivery_status = apply_dmarc_policy( - alias, contact, envelope, msg, from_header - ) + dmarc_delivery_status = apply_dmarc_policy(alias, contact, envelope, msg) if dmarc_delivery_status is not None: return [(False, dmarc_delivery_status)] @@ -2597,19 +2592,28 @@ class MailHandler: ) with create_light_app().app_context(): - ret = handle(envelope, msg) + return_status = handle(envelope, msg) elapsed = time.time() - start + if return_status[0] == "5": + if get_spamd_result(msg).spf in ( + SPFCheckResult.fail, + SPFCheckResult.soft_fail, + ): + LOG.i( + "Replacing 5XX to 216 status because the return-path failed the spf check" + ) + return_status = status.E216 LOG.i( "Finish mail_from %s, rcpt_tos %s, takes %s seconds with return code '%s'<<===", envelope.mail_from, envelope.rcpt_tos, elapsed, - ret, + return_status, ) newrelic.agent.record_custom_metric("Custom/email_handler_time", elapsed) newrelic.agent.record_custom_metric("Custom/number_incoming_email", 1) - return ret + return return_status def main(port: int): diff --git a/tests/example_emls/5xx_overwrite_spf.eml b/tests/example_emls/5xx_overwrite_spf.eml new file mode 100644 index 00000000..d4a2be7e --- /dev/null +++ b/tests/example_emls/5xx_overwrite_spf.eml @@ -0,0 +1,28 @@ +X-SimpleLogin-Client-IP: 54.39.200.130 +Received-SPF: Softfail (mailfrom) identity=mailfrom; client-ip=34.59.200.130; + helo=relay.somewhere.net; envelope-from=everwaste@gmail.com; + receiver= +Received: from relay.somewhere.net (relay.somewhere.net [34.59.200.130]) + (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) + (No client certificate requested) + by mx1.sldev.ovh (Postfix) with ESMTPS id 6D8C13F069 + for ; Thu, 17 Mar 2022 16:50:20 +0000 (UTC) +Date: Thu, 17 Mar 2022 16:50:18 +0000 +To: {{ alias_email }} +From: somewhere@rainbow.com +Subject: test Thu, 17 Mar 2022 16:50:18 +0000 +Message-Id: <20220317165018.000191@somewhere-5488dd4b6b-7crp6> +X-Mailer: swaks v20201014.0 jetmore.org/john/code/swaks/ +X-Rspamd-Queue-Id: 6D8C13F069 +X-Rspamd-Server: staging1 +X-Spamd-Result: default: False [0.50 / 13.00]; + MID_RHS_NOT_FQDN(0.50)[]; + DMARC_NA(0.10); + MIME_GOOD(-0.10)[text/plain]; + MIME_TRACE(0.00)[0:+]; + TO_DN_NONE(0.00)[]; + {{ spf_result }}(0.00[]; + TO_MATCH_ENVRCPT_ALL(0.00)[]; + ARC_NA(0.00)[] + +This is a test mailing diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 0403c96f..8c2b56ed 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -3,6 +3,7 @@ from email.message import EmailMessage from aiosmtpd.smtp import Envelope import email_handler +from app.config import BOUNCE_EMAIL from app.email import headers, status from app.models import ( User, @@ -116,3 +117,31 @@ def test_dmarc_quarantine(flask_client): # email_log = email_logs[0] # assert email_log.blocked # assert email_log.refused_email_id + + +def test_prevent_5xx_from_spf(flask_client): + user = create_random_user() + alias = Alias.create_new_random(user) + msg = load_eml_file( + "5xx_overwrite_spf.eml", + {"alias_email": alias.email, "spf_result": "R_SPF_FAIL"}, + ) + envelope = Envelope() + envelope.mail_from = BOUNCE_EMAIL.format(999999999999999999) + envelope.rcpt_tos = [msg["to"]] + result = email_handler.MailHandler()._handle(envelope, msg) + assert result == status.E216 + + +def test_preserve_5xx_with_valid_spf(flask_client): + user = create_random_user() + alias = Alias.create_new_random(user) + msg = load_eml_file( + "5xx_overwrite_spf.eml", + {"alias_email": alias.email, "spf_result": "R_SPF_ALLOW"}, + ) + envelope = Envelope() + envelope.mail_from = BOUNCE_EMAIL.format(999999999999999999) + envelope.rcpt_tos = [msg["to"]] + result = email_handler.MailHandler()._handle(envelope, msg) + assert result == status.E512 diff --git a/tests/test_email_utils.py b/tests/test_email_utils.py index 466f860c..e5d16744 100644 --- a/tests/test_email_utils.py +++ b/tests/test_email_utils.py @@ -36,7 +36,7 @@ from app.email_utils import ( get_orig_message_from_bounce, get_mailbox_bounce_info, is_invalid_mailbox_domain, - get_dmarc_status, + get_spamd_result, ) from app.models import ( User, @@ -797,29 +797,29 @@ def test_is_invalid_mailbox_domain(flask_client): def test_dmarc_result_softfail(): msg = load_eml_file("dmarc_gmail_softfail.eml") - assert DmarcCheckResult.soft_fail == get_dmarc_status(msg) + assert DmarcCheckResult.soft_fail == get_spamd_result(msg).dmarc def test_dmarc_result_quarantine(): msg = load_eml_file("dmarc_quarantine.eml") - assert DmarcCheckResult.quarantine == get_dmarc_status(msg) + assert DmarcCheckResult.quarantine == get_spamd_result(msg).dmarc def test_dmarc_result_reject(): msg = load_eml_file("dmarc_reject.eml") - assert DmarcCheckResult.reject == get_dmarc_status(msg) + assert DmarcCheckResult.reject == get_spamd_result(msg).dmarc def test_dmarc_result_allow(): msg = load_eml_file("dmarc_allow.eml") - assert DmarcCheckResult.allow == get_dmarc_status(msg) + assert DmarcCheckResult.allow == get_spamd_result(msg).dmarc def test_dmarc_result_na(): msg = load_eml_file("dmarc_na.eml") - assert DmarcCheckResult.not_available == get_dmarc_status(msg) + assert DmarcCheckResult.not_available == get_spamd_result(msg).dmarc def test_dmarc_result_bad_policy(): msg = load_eml_file("dmarc_bad_policy.eml") - assert DmarcCheckResult.bad_policy == get_dmarc_status(msg) + assert DmarcCheckResult.bad_policy == get_spamd_result(msg).dmarc