Replace 5XX with 2XX for return path that fail SPF check

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
 

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]:
+def get_spamd_result(msg: Message) -> Optional[SpamdResult]:
-    spam_result = msg.get_all(headers.SPAMD_RESULT)
+    spam_result_header = msg.get_all(headers.SPAMD_RESULT)
-    if not spam_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

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)

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)
+    spam_result = get_spamd_result(msg)
-    if dmarc_result:
+    if not DMARC_CHECK_ENABLED or not spam_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:
         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(
+    dmarc_delivery_status = apply_dmarc_policy(alias, contact, envelope, msg)
-        alias, contact, envelope, msg, from_header
-    )
     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):

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=<UNKNOWN>
+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 <wehrman_mannequin@sldev.ovh>; 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

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

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