From 8ca1be01668371e56abb5cbc4a420cff26525103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 6 Apr 2022 12:51:04 +0200 Subject: [PATCH 01/11] Apply dmarc policy to the reply phase --- app/config.py | 3 + app/email_utils.py | 10 ++- app/models.py | 16 ++++- email_handler.py | 68 ++++++++++++++++--- .../emails/transactional/spoof-reply.html | 20 ++++++ .../emails/transactional/spoof-reply.txt | 8 +++ tests/{email => email_tests}/__init__.py | 0 .../{email => email_tests}/test_rate_limit.py | 0 tests/example_emls/dmarc_reply_check.eml | 25 +++++++ tests/test_email_handler.py | 48 ++++++++++++- 10 files changed, 182 insertions(+), 16 deletions(-) create mode 100644 templates/emails/transactional/spoof-reply.html create mode 100644 templates/emails/transactional/spoof-reply.txt rename tests/{email => email_tests}/__init__.py (100%) rename tests/{email => email_tests}/test_rate_limit.py (100%) create mode 100644 tests/example_emls/dmarc_reply_check.eml diff --git a/app/config.py b/app/config.py index 4a83f985..82626d41 100644 --- a/app/config.py +++ b/app/config.py @@ -304,6 +304,9 @@ MAX_ALERT_24H = 4 # When a reverse-alias receives emails from un unknown mailbox ALERT_REVERSE_ALIAS_UNKNOWN_MAILBOX = "reverse_alias_unknown_mailbox" +# When somebody is trying to spoof a reply +ALERT_DMARC_FAILED_REPLY_PHASE = "dmarc_failed_reply_phase" + # When a forwarding email is bounced ALERT_BOUNCE_EMAIL = "bounce" diff --git a/app/email_utils.py b/app/email_utils.py index f84b755f..207e113d 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -73,6 +73,7 @@ from app.models import ( DmarcCheckResult, SpamdResult, SPFCheckResult, + Phase, ) from app.utils import ( random_string, @@ -1443,7 +1444,9 @@ def save_email_for_debugging(msg: Message, file_name_prefix=None) -> str: return "" -def get_spamd_result(msg: Message) -> Optional[SpamdResult]: +def get_spamd_result( + msg: Message, send_event: bool = True, phase: Phase = Phase.unknown +) -> Optional[SpamdResult]: spam_result_header = msg.get_all(headers.SPAMD_RESULT) if not spam_result_header: newrelic.agent.record_custom_event("SpamdCheck", {"header": "missing"}) @@ -1455,7 +1458,7 @@ def get_spamd_result(msg: Message) -> Optional[SpamdResult]: if sep > -1: spam_entries[entry_pos] = spam_entries[entry_pos][:sep] - spamd_result = SpamdResult() + spamd_result = SpamdResult(phase) for header_value, dmarc_result in DmarcCheckResult.get_string_dict().items(): if header_value in spam_entries: @@ -1464,5 +1467,6 @@ def get_spamd_result(msg: Message) -> Optional[SpamdResult]: if header_value in spam_entries: spamd_result.set_spf_result(spf_result) - newrelic.agent.record_custom_event("SpamdCheck", spamd_result.event_data()) + if send_event: + newrelic.agent.record_custom_event("SpamdCheck", spamd_result.event_data()) return spamd_result diff --git a/app/models.py b/app/models.py index b40ca741..3e13d422 100644 --- a/app/models.py +++ b/app/models.py @@ -237,6 +237,12 @@ class AuditLogActionEnum(EnumE): extend_subscription = 7 +class Phase(EnumE): + unknown = 0 + forward = 1 + reply = 2 + + class DmarcCheckResult(EnumE): allow = 0 soft_fail = 1 @@ -280,7 +286,8 @@ class SPFCheckResult(EnumE): class SpamdResult: - def __init__(self): + def __init__(self, phase: Phase = Phase.unknown): + self.phase: Phase = phase self.dmarc: DmarcCheckResult = DmarcCheckResult.not_available self.spf: SPFCheckResult = SPFCheckResult.not_available @@ -291,7 +298,12 @@ class SpamdResult: self.spf = spf_result def event_data(self) -> Dict: - return {"header": "present", "dmarc": self.dmarc, "spf": self.spf} + return { + "header": "present", + "dmarc": self.dmarc, + "spf": self.spf, + "phase": self.phase, + } class Hibp(Base, ModelMixin): diff --git a/email_handler.py b/email_handler.py index e772406f..edeefda5 100644 --- a/email_handler.py +++ b/email_handler.py @@ -89,6 +89,7 @@ from app.config import ( ALERT_TO_NOREPLY, DMARC_CHECK_ENABLED, ALERT_QUARANTINE_DMARC, + ALERT_DMARC_FAILED_REPLY_PHASE, ) from app.db import Session from app.email import status, headers @@ -158,6 +159,7 @@ from app.models import ( Notification, DmarcCheckResult, SPFCheckResult, + Phase, ) from app.pgp_utils import PGPException, sign_data_with_pgpy, sign_data from app.utils import sanitize_email @@ -542,10 +544,10 @@ def handle_email_sent_to_ourself(alias, from_addr: str, msg: Message, user): ) -def apply_dmarc_policy( +def apply_dmarc_policy_for_forward_phase( alias: Alias, contact: Contact, envelope: Envelope, msg: Message ) -> Optional[str]: - spam_result = get_spamd_result(msg) + spam_result = get_spamd_result(msg, Phase.forward) if not DMARC_CHECK_ENABLED or not spam_result: return None @@ -553,7 +555,7 @@ def apply_dmarc_policy( if spam_result.dmarc == DmarcCheckResult.soft_fail: LOG.w( - f"dmarc soft_fail from contact {contact.email} to alias {alias.email}." + f"dmarc forward: dmarc soft_fail from contact {contact.email} to alias {alias.email}." f"mail_from:{envelope.mail_from}, from_header: {from_header}" ) raise DmarcSoftFail @@ -563,10 +565,10 @@ def apply_dmarc_policy( DmarcCheckResult.reject, ): LOG.w( - f"put email from {contact} to {alias} to quarantine. {spam_result.event_data()}, " + f"dmarc forward: put email from {contact} to {alias} to quarantine. {spam_result.event_data()}, " f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" ) - email_log = quarantine_dmarc_failed_email(alias, contact, envelope, msg) + email_log = quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) Notification.create( user_id=alias.user_id, title=f"{alias.email} has a new mail in quarantine", @@ -601,7 +603,7 @@ def apply_dmarc_policy( return None -def quarantine_dmarc_failed_email(alias, contact, envelope, msg) -> EmailLog: +def quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) -> EmailLog: add_or_replace_header(msg, headers.SL_DIRECTION, "Forward") msg[headers.SL_ENVELOPE_TO] = alias.email msg[headers.SL_ENVELOPE_FROM] = envelope.mail_from @@ -635,6 +637,44 @@ def quarantine_dmarc_failed_email(alias, contact, envelope, msg) -> EmailLog: ) +def apply_dmarc_policy_for_reply_phase( + alias_from: Alias, contact_recipient: Contact, envelope: Envelope, msg: Message +) -> Optional[str]: + spam_result = get_spamd_result(msg, Phase.reply) + if not DMARC_CHECK_ENABLED or not spam_result: + return None + + if spam_result.dmarc not in ( + DmarcCheckResult.quarantine, + DmarcCheckResult.reject, + DmarcCheckResult.soft_fail, + ): + return None + LOG.w( + f"dmarc reply: Put email from {alias_from.email} to {contact_recipient} into quarantine. {spam_result.event_data()}, " + f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" + ) + send_email_with_rate_control( + alias_from.user, + ALERT_DMARC_FAILED_REPLY_PHASE, + alias_from.user.email, + f"Attempt to send an email to your contact {contact_recipient.email} from {envelope.mail_from}", + render( + "transactional/spoof-reply.txt", + contact=contact_recipient, + alias=alias_from, + sender=envelope.mail_from, + ), + render( + "transactional/spoof-reply.html", + contact=contact_recipient, + alias=alias_from, + sender=envelope.mail_from, + ), + ) + return status.E215 + + def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str]]: """return an array of SMTP status (is_success, smtp_status) is_success indicates whether an email has been delivered and @@ -717,7 +757,9 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str # Check if we need to reject or quarantine based on dmarc try: - dmarc_delivery_status = apply_dmarc_policy(alias, contact, envelope, msg) + dmarc_delivery_status = apply_dmarc_policy_for_forward_phase( + alias, contact, envelope, msg + ) if dmarc_delivery_status is not None: return [(False, dmarc_delivery_status)] except DmarcSoftFail: @@ -1041,6 +1083,7 @@ def handle_reply(envelope, msg: Message, rcpt_to: str) -> (bool, str): Return whether an email has been delivered and the smtp status ("250 Message accepted", "550 Non-existent email address", etc) """ + reply_email = rcpt_to # reply_email must end with EMAIL_DOMAIN @@ -1076,7 +1119,14 @@ def handle_reply(envelope, msg: Message, rcpt_to: str) -> (bool, str): alias, contact, ) - return [(False, status.E504)] + return (False, status.E504) + + # Check if we need to reject or quarantine based on dmarc + dmarc_delivery_status = apply_dmarc_policy_for_reply_phase( + alias, contact, envelope, msg + ) + if dmarc_delivery_status is not None: + return (False, dmarc_delivery_status) # Anti-spoofing mailbox = get_mailbox_from_mail_from(mail_from, alias) @@ -2608,7 +2658,7 @@ class MailHandler: elapsed = time.time() - start # Only bounce messages if the return-path passes the spf check. Otherwise black-hole it. if return_status[0] == "5": - spamd_result = get_spamd_result(msg) + spamd_result = get_spamd_result(msg, send_event=False) if spamd_result and get_spamd_result(msg).spf in ( SPFCheckResult.fail, SPFCheckResult.soft_fail, diff --git a/templates/emails/transactional/spoof-reply.html b/templates/emails/transactional/spoof-reply.html new file mode 100644 index 00000000..83766dba --- /dev/null +++ b/templates/emails/transactional/spoof-reply.html @@ -0,0 +1,20 @@ +{% extends "base.html" %} + +{% block content %} + + {% call text() %} +

+ An attempt to send a fake email to {{ contact.email }} from your alias {{ alias.email }} using {{ sender }} has been blocked. +

+ {% endcall %} + + {% call text() %} + As a measure to protect against email spoofing, we have blocked an attempt to send an email from your alias {{ alias.email }} using {{ sender }}. + {% endcall %} + + {% call text() %} + Best,
+ SimpleLogin Team. + {% endcall %} +{% endblock %} + diff --git a/templates/emails/transactional/spoof-reply.txt b/templates/emails/transactional/spoof-reply.txt new file mode 100644 index 00000000..22d27018 --- /dev/null +++ b/templates/emails/transactional/spoof-reply.txt @@ -0,0 +1,8 @@ +{% extends "base.txt.jinja2" %} + +{% block content %} +An attempt to send a fake email to {{ contact.email }} from your alias {{ alias.email }} using {{ sender }} has been blocked. + +As a measure to protect against email spoofing, we have blocked an attempt to send an email from your alias {{ alias.email }} using {{ sender }}. +{% endblock %} + diff --git a/tests/email/__init__.py b/tests/email_tests/__init__.py similarity index 100% rename from tests/email/__init__.py rename to tests/email_tests/__init__.py diff --git a/tests/email/test_rate_limit.py b/tests/email_tests/test_rate_limit.py similarity index 100% rename from tests/email/test_rate_limit.py rename to tests/email_tests/test_rate_limit.py diff --git a/tests/example_emls/dmarc_reply_check.eml b/tests/example_emls/dmarc_reply_check.eml new file mode 100644 index 00000000..adedcb27 --- /dev/null +++ b/tests/example_emls/dmarc_reply_check.eml @@ -0,0 +1,25 @@ +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: {{ contact_email }} +From: {{ alias_email }} +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]; + {{ dmarc_result }}(0.00)[]; +X-Rspamd-Pre-Result: action=add header; + module=force_actions; + unknown reason +X-Spam: Yes + +This is a test mailing diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 1f7cdd59..d0030a33 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -1,9 +1,13 @@ +import random from email.message import EmailMessage +from typing import List +import pytest from aiosmtpd.smtp import Envelope import email_handler -from app.config import BOUNCE_EMAIL +from app.config import BOUNCE_EMAIL, EMAIL_DOMAIN, ALERT_DMARC_FAILED_REPLY_PHASE +from app.db import Session from app.email import headers, status from app.models import ( User, @@ -12,6 +16,8 @@ from app.models import ( IgnoredEmail, EmailLog, Notification, + Contact, + SentAlert, ) from email_handler import ( get_mailbox_from_mail_from, @@ -75,7 +81,7 @@ def test_is_automatic_out_of_office(): assert is_automatic_out_of_office(msg) -def test_dmarc_quarantine(flask_client): +def test_dmarc_forward_quarantine(flask_client): user = create_random_user() alias = Alias.create_new_random(user) msg = load_eml_file("dmarc_quarantine.eml", {"alias_email": alias.email}) @@ -159,3 +165,41 @@ def test_preserve_5xx_with_no_header(flask_client): envelope.rcpt_tos = [msg["to"]] result = email_handler.MailHandler()._handle(envelope, msg) assert result == status.E512 + + +def generate_dmarc_result() -> List: + return ["DMARC_POLICY_QUARANTINE", "DMARC_POLICY_REJECT", "DMARC_POLICY_SOFTFAIL"] + + +@pytest.mark.parametrize("dmarc_result", generate_dmarc_result()) +def test_dmarc_reply_quarantine(dmarc_result: str): + user = create_random_user() + alias = Alias.create_new_random(user) + Session.commit() + contact = Contact.create( + user_id=alias.user_id, + alias_id=alias.id, + website_email="random-{}@nowhere.net".format(int(random.random())), + name="Name {}".format(int(random.random())), + reply_email="random-{}@{}".format(random.random(), EMAIL_DOMAIN), + automatic_created=True, + flush=True, + commit=True, + ) + msg = load_eml_file( + "dmarc_reply_check.eml", + { + "alias_email": alias.email, + "contact_email": contact.reply_email, + "dmarc_result": dmarc_result, + }, + ) + envelope = Envelope() + envelope.mail_from = msg["from"] + envelope.rcpt_tos = [msg["to"]] + result = email_handler.handle(envelope, msg) + assert result == status.E215 + alerts = SentAlert.filter_by( + user_id=user.id, alert_type=ALERT_DMARC_FAILED_REPLY_PHASE + ).all() + assert len(alerts) == 1 From 61b8bbdfcc86ff5c9834d8b6973a922408a03460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 6 Apr 2022 17:07:36 +0200 Subject: [PATCH 02/11] Fix tests --- tests/test_email_handler.py | 5 ++--- tests/utils.py | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index d0030a33..f16faa8c 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -172,7 +172,7 @@ def generate_dmarc_result() -> List: @pytest.mark.parametrize("dmarc_result", generate_dmarc_result()) -def test_dmarc_reply_quarantine(dmarc_result: str): +def test_dmarc_reply_quarantine(flask_client, dmarc_result): user = create_random_user() alias = Alias.create_new_random(user) Session.commit() @@ -183,9 +183,8 @@ def test_dmarc_reply_quarantine(dmarc_result: str): name="Name {}".format(int(random.random())), reply_email="random-{}@{}".format(random.random(), EMAIL_DOMAIN), automatic_created=True, - flush=True, - commit=True, ) + Session.commit() msg = load_eml_file( "dmarc_reply_check.eml", { diff --git a/tests/utils.py b/tests/utils.py index 3621d9f5..3b4c7e56 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -39,11 +39,11 @@ def random_token(length: int = 10) -> str: def create_random_user() -> User: - email = "{}@{}.com".format(random_token(), random_token()) + random_email = "{}@{}.com".format(random_token(), random_token()) return User.create( - email=email, + email=random_email, password="password", - name="Test User", + name="Test {}".format(random_token()), activated=True, commit=True, ) From 33e83fc15367910c2092b38a73df3063920e0612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 6 Apr 2022 17:37:55 +0200 Subject: [PATCH 03/11] fix message --- email_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/email_handler.py b/email_handler.py index edeefda5..12e71137 100644 --- a/email_handler.py +++ b/email_handler.py @@ -555,7 +555,7 @@ def apply_dmarc_policy_for_forward_phase( if spam_result.dmarc == DmarcCheckResult.soft_fail: LOG.w( - f"dmarc forward: dmarc soft_fail from contact {contact.email} to alias {alias.email}." + f"dmarc forward: soft_fail from contact {contact.email} to alias {alias.email}." f"mail_from:{envelope.mail_from}, from_header: {from_header}" ) raise DmarcSoftFail From 44c77439c157bf8d1296f56210ebf84914d95829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 6 Apr 2022 17:44:05 +0200 Subject: [PATCH 04/11] PR comments --- email_handler.py | 4 ++-- tests/test_email_handler.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/email_handler.py b/email_handler.py index 12e71137..3734ffd6 100644 --- a/email_handler.py +++ b/email_handler.py @@ -1119,14 +1119,14 @@ def handle_reply(envelope, msg: Message, rcpt_to: str) -> (bool, str): alias, contact, ) - return (False, status.E504) + return False, status.E504 # Check if we need to reject or quarantine based on dmarc dmarc_delivery_status = apply_dmarc_policy_for_reply_phase( alias, contact, envelope, msg ) if dmarc_delivery_status is not None: - return (False, dmarc_delivery_status) + return False, dmarc_delivery_status # Anti-spoofing mailbox = get_mailbox_from_mail_from(mail_from, alias) diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index f16faa8c..805c7524 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -182,7 +182,6 @@ def test_dmarc_reply_quarantine(flask_client, dmarc_result): website_email="random-{}@nowhere.net".format(int(random.random())), name="Name {}".format(int(random.random())), reply_email="random-{}@{}".format(random.random(), EMAIL_DOMAIN), - automatic_created=True, ) Session.commit() msg = load_eml_file( From b128d64563ed69890799eba7915abac88cdf8e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Thu, 7 Apr 2022 19:17:37 +0200 Subject: [PATCH 05/11] Moved spamd check to a custom file and cached the result --- app/email_utils.py | 32 -------- app/handler/__init__.py | 0 app/handler/spamd_result.py | 125 +++++++++++++++++++++++++++++ app/models.py | 71 +--------------- email_handler.py | 20 +++-- tests/handler/__init__.py | 0 tests/handler/test_spamd_result.py | 34 ++++++++ tests/test_email_utils.py | 32 -------- 8 files changed, 172 insertions(+), 142 deletions(-) create mode 100644 app/handler/__init__.py create mode 100644 app/handler/spamd_result.py create mode 100644 tests/handler/__init__.py create mode 100644 tests/handler/test_spamd_result.py diff --git a/app/email_utils.py b/app/email_utils.py index 207e113d..6ab133c2 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -70,10 +70,6 @@ from app.models import ( TransactionalEmail, IgnoreBounceSender, InvalidMailboxDomain, - DmarcCheckResult, - SpamdResult, - SPFCheckResult, - Phase, ) from app.utils import ( random_string, @@ -1442,31 +1438,3 @@ def save_email_for_debugging(msg: Message, file_name_prefix=None) -> str: return file_name return "" - - -def get_spamd_result( - msg: Message, send_event: bool = True, phase: Phase = Phase.unknown -) -> 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_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(phase) - - for header_value, dmarc_result in DmarcCheckResult.get_string_dict().items(): - if header_value in spam_entries: - 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) - - if send_event: - newrelic.agent.record_custom_event("SpamdCheck", spamd_result.event_data()) - return spamd_result diff --git a/app/handler/__init__.py b/app/handler/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/app/handler/spamd_result.py b/app/handler/spamd_result.py new file mode 100644 index 00000000..cd5c8ce6 --- /dev/null +++ b/app/handler/spamd_result.py @@ -0,0 +1,125 @@ +from __future__ import annotations +from typing import Dict, Optional + +import newrelic + +from app.email import headers +from app.models import EnumE +from email.message import Message + + +class Phase(EnumE): + unknown = 0 + forward = 1 + reply = 2 + + +class DmarcCheckResult(EnumE): + allow = 0 + soft_fail = 1 + quarantine = 2 + reject = 3 + not_available = 4 + bad_policy = 5 + + @staticmethod + def get_string_dict(): + return { + "DMARC_POLICY_ALLOW": DmarcCheckResult.allow, + "DMARC_POLICY_SOFTFAIL": DmarcCheckResult.soft_fail, + "DMARC_POLICY_QUARANTINE": DmarcCheckResult.quarantine, + "DMARC_POLICY_REJECT": DmarcCheckResult.reject, + "DMARC_NA": DmarcCheckResult.not_available, + "DMARC_BAD_POLICY": DmarcCheckResult.bad_policy, + } + + +class SPFCheckResult(EnumE): + allow = 0 + fail = 1 + soft_fail = 1 + neutral = 2 + temp_error = 3 + not_available = 4 + perm_error = 5 + + @staticmethod + def get_string_dict(): + return { + "R_SPF_ALLOW": SPFCheckResult.allow, + "R_SPF_FAIL": SPFCheckResult.fail, + "R_SPF_SOFTFAIL": SPFCheckResult.soft_fail, + "R_SPF_NEUTRAL": SPFCheckResult.neutral, + "R_SPF_DNSFAIL": SPFCheckResult.temp_error, + "R_SPF_NA": SPFCheckResult.not_available, + "R_SPF_PERMFAIL": SPFCheckResult.perm_error, + } + + +class SpamdResult: + def __init__(self, phase: Phase = Phase.unknown): + self.phase: Phase = phase + self.dmarc: DmarcCheckResult = DmarcCheckResult.not_available + self.spf: SPFCheckResult = SPFCheckResult.not_available + + 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, + "phase": self.phase, + } + + @classmethod + def extract_from_headers( + cls, msg: Message, phase: Phase = Phase.unknown + ) -> Optional[SpamdResult]: + cached = cls._get_from_message(msg) + if cached: + return cached + + spam_result_header = msg.get_all(headers.SPAMD_RESULT) + if not spam_result_header: + return None + + 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(phase) + + for header_value, dmarc_result in DmarcCheckResult.get_string_dict().items(): + if header_value in spam_entries: + 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) + + cls._store_in_message(spamd_result, msg) + return spamd_result + + @classmethod + def _store_in_message(cls, check: SpamdResult, msg: Message): + msg.spamd_check = check + + @classmethod + def _get_from_message(cls, msg: Message) -> Optional[SpamdResult]: + return getattr(msg, "spamd_check", None) + + @classmethod + def send_to_new_relic(cls, msg: Message): + check = cls._get_from_message(msg) + if check: + newrelic.agent.record_custom_event("SpamdCheck", check.event_data()) + else: + newrelic.agent.record_custom_event("SpamdCheck", {"header": "missing"}) diff --git a/app/models.py b/app/models.py index 3e13d422..3f77b506 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, Dict +from typing import List, Tuple, Optional import arrow import sqlalchemy as sa @@ -237,75 +237,6 @@ class AuditLogActionEnum(EnumE): extend_subscription = 7 -class Phase(EnumE): - unknown = 0 - forward = 1 - reply = 2 - - -class DmarcCheckResult(EnumE): - allow = 0 - soft_fail = 1 - quarantine = 2 - reject = 3 - not_available = 4 - bad_policy = 5 - - @staticmethod - def get_string_dict(): - return { - "DMARC_POLICY_ALLOW": DmarcCheckResult.allow, - "DMARC_POLICY_SOFTFAIL": DmarcCheckResult.soft_fail, - "DMARC_POLICY_QUARANTINE": DmarcCheckResult.quarantine, - "DMARC_POLICY_REJECT": DmarcCheckResult.reject, - "DMARC_NA": DmarcCheckResult.not_available, - "DMARC_BAD_POLICY": DmarcCheckResult.bad_policy, - } - - -class SPFCheckResult(EnumE): - allow = 0 - fail = 1 - soft_fail = 1 - neutral = 2 - temp_error = 3 - not_available = 4 - perm_error = 5 - - @staticmethod - def get_string_dict(): - return { - "R_SPF_ALLOW": SPFCheckResult.allow, - "R_SPF_FAIL": SPFCheckResult.fail, - "R_SPF_SOFTFAIL": SPFCheckResult.soft_fail, - "R_SPF_NEUTRAL": SPFCheckResult.neutral, - "R_SPF_DNSFAIL": SPFCheckResult.temp_error, - "R_SPF_NA": SPFCheckResult.not_available, - "R_SPF_PERMFAIL": SPFCheckResult.perm_error, - } - - -class SpamdResult: - def __init__(self, phase: Phase = Phase.unknown): - self.phase: Phase = phase - self.dmarc: DmarcCheckResult = DmarcCheckResult.not_available - self.spf: SPFCheckResult = SPFCheckResult.not_available - - 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, - "phase": self.phase, - } - - 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 3734ffd6..a8fe37d6 100644 --- a/email_handler.py +++ b/email_handler.py @@ -92,6 +92,12 @@ from app.config import ( ALERT_DMARC_FAILED_REPLY_PHASE, ) from app.db import Session +from app.handler.spamd_result import ( + SpamdResult, + Phase, + DmarcCheckResult, + SPFCheckResult, +) from app.email import status, headers from app.email.rate_limit import rate_limited from app.email.spam import get_spam_score @@ -131,7 +137,6 @@ from app.email_utils import ( get_orig_message_from_yahoo_complaint, get_mailbox_bounce_info, save_email_for_debugging, - get_spamd_result, ) from app.errors import ( NonReverseAliasInReplyPhase, @@ -157,9 +162,6 @@ from app.models import ( DeletedAlias, DomainDeletedAlias, Notification, - DmarcCheckResult, - SPFCheckResult, - Phase, ) from app.pgp_utils import PGPException, sign_data_with_pgpy, sign_data from app.utils import sanitize_email @@ -547,7 +549,7 @@ def handle_email_sent_to_ourself(alias, from_addr: str, msg: Message, user): def apply_dmarc_policy_for_forward_phase( alias: Alias, contact: Contact, envelope: Envelope, msg: Message ) -> Optional[str]: - spam_result = get_spamd_result(msg, Phase.forward) + spam_result = SpamdResult.extract_from_headers(msg, Phase.forward) if not DMARC_CHECK_ENABLED or not spam_result: return None @@ -640,7 +642,7 @@ def quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) -> Emai def apply_dmarc_policy_for_reply_phase( alias_from: Alias, contact_recipient: Contact, envelope: Envelope, msg: Message ) -> Optional[str]: - spam_result = get_spamd_result(msg, Phase.reply) + spam_result = SpamdResult.extract_from_headers(msg, Phase.reply) if not DMARC_CHECK_ENABLED or not spam_result: return None @@ -2657,9 +2659,9 @@ class MailHandler: return_status = handle(envelope, msg) elapsed = time.time() - start # Only bounce messages if the return-path passes the spf check. Otherwise black-hole it. + spamd_result = SpamdResult.extract_from_headers(msg) if return_status[0] == "5": - spamd_result = get_spamd_result(msg, send_event=False) - if spamd_result and get_spamd_result(msg).spf in ( + if spamd_result and spamd_result.spf in ( SPFCheckResult.fail, SPFCheckResult.soft_fail, ): @@ -2675,6 +2677,8 @@ class MailHandler: elapsed, return_status, ) + + SpamdResult.send_to_new_relic(msg) newrelic.agent.record_custom_metric("Custom/email_handler_time", elapsed) newrelic.agent.record_custom_metric("Custom/number_incoming_email", 1) return return_status diff --git a/tests/handler/__init__.py b/tests/handler/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/handler/test_spamd_result.py b/tests/handler/test_spamd_result.py new file mode 100644 index 00000000..641a8fab --- /dev/null +++ b/tests/handler/test_spamd_result.py @@ -0,0 +1,34 @@ +from app.handler.spamd_result import DmarcCheckResult, SpamdResult +from tests.utils import load_eml_file + + +def test_dmarc_result_softfail(): + msg = load_eml_file("dmarc_gmail_softfail.eml") + assert DmarcCheckResult.soft_fail == SpamdResult.extract_from_headers(msg).dmarc + + +def test_dmarc_result_quarantine(): + msg = load_eml_file("dmarc_quarantine.eml") + assert DmarcCheckResult.quarantine == SpamdResult.extract_from_headers(msg).dmarc + + +def test_dmarc_result_reject(): + msg = load_eml_file("dmarc_reject.eml") + assert DmarcCheckResult.reject == SpamdResult.extract_from_headers(msg).dmarc + + +def test_dmarc_result_allow(): + msg = load_eml_file("dmarc_allow.eml") + assert DmarcCheckResult.allow == SpamdResult.extract_from_headers(msg).dmarc + + +def test_dmarc_result_na(): + msg = load_eml_file("dmarc_na.eml") + assert DmarcCheckResult.not_available == SpamdResult.extract_from_headers(msg).dmarc + + +def test_dmarc_result_bad_policy(): + msg = load_eml_file("dmarc_bad_policy.eml") + assert SpamdResult._get_from_message(msg) is None + assert DmarcCheckResult.bad_policy == SpamdResult.extract_from_headers(msg).dmarc + assert SpamdResult._get_from_message(msg) is not None diff --git a/tests/test_email_utils.py b/tests/test_email_utils.py index e5d16744..91f439f1 100644 --- a/tests/test_email_utils.py +++ b/tests/test_email_utils.py @@ -36,7 +36,6 @@ from app.email_utils import ( get_orig_message_from_bounce, get_mailbox_bounce_info, is_invalid_mailbox_domain, - get_spamd_result, ) from app.models import ( User, @@ -46,7 +45,6 @@ from app.models import ( EmailLog, IgnoreBounceSender, InvalidMailboxDomain, - DmarcCheckResult, ) # flake8: noqa: E101, W191 @@ -793,33 +791,3 @@ def test_is_invalid_mailbox_domain(flask_client): assert is_invalid_mailbox_domain("sub1.sub2.ab.cd") assert not is_invalid_mailbox_domain("xy.zt") - - -def test_dmarc_result_softfail(): - msg = load_eml_file("dmarc_gmail_softfail.eml") - 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_spamd_result(msg).dmarc - - -def test_dmarc_result_reject(): - msg = load_eml_file("dmarc_reject.eml") - assert DmarcCheckResult.reject == get_spamd_result(msg).dmarc - - -def test_dmarc_result_allow(): - msg = load_eml_file("dmarc_allow.eml") - 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_spamd_result(msg).dmarc - - -def test_dmarc_result_bad_policy(): - msg = load_eml_file("dmarc_bad_policy.eml") - assert DmarcCheckResult.bad_policy == get_spamd_result(msg).dmarc From 68e58c08765b82ac20ab60a6d26c479815ac0515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Fri, 8 Apr 2022 11:28:14 +0200 Subject: [PATCH 06/11] Move dmarc management to its own file --- app/errors.py | 5 -- app/handler/dmarc.py | 157 ++++++++++++++++++++++++++++++++++ email_handler.py | 162 ++---------------------------------- tests/test_email_handler.py | 30 +++---- 4 files changed, 177 insertions(+), 177 deletions(-) create mode 100644 app/handler/dmarc.py diff --git a/app/errors.py b/app/errors.py index e041c763..26be6091 100644 --- a/app/errors.py +++ b/app/errors.py @@ -56,8 +56,3 @@ class MailSentFromReverseAlias(SLException): """raised when receiving an email sent from a reverse alias""" pass - - -class DmarcSoftFail(SLException): - - pass diff --git a/app/handler/dmarc.py b/app/handler/dmarc.py new file mode 100644 index 00000000..03671578 --- /dev/null +++ b/app/handler/dmarc.py @@ -0,0 +1,157 @@ +import uuid +from io import BytesIO +from typing import Optional + +from aiosmtpd.handlers import Message +from aiosmtpd.smtp import Envelope + +from app import s3 +from app.config import ( + DMARC_CHECK_ENABLED, + ALERT_QUARANTINE_DMARC, + ALERT_DMARC_FAILED_REPLY_PHASE, +) +from app.email import headers, status +from app.email_utils import ( + get_header_unicode, + send_email_with_rate_control, + render, + add_or_replace_header, + to_bytes, + add_header, +) +from app.handler.spamd_result import SpamdResult, Phase, DmarcCheckResult +from app.log import LOG +from app.models import Alias, Contact, Notification, EmailLog, RefusedEmail + + +def apply_dmarc_policy_for_forward_phase( + alias: Alias, contact: Contact, envelope: Envelope, msg: Message +) -> Optional[str]: + spam_result = SpamdResult.extract_from_headers(msg, Phase.forward) + if not DMARC_CHECK_ENABLED or not spam_result: + return None + + from_header = get_header_unicode(msg[headers.FROM]) + + if spam_result.dmarc == DmarcCheckResult.soft_fail: + LOG.w( + f"dmarc forward: soft_fail from contact {contact.email} to alias {alias.email}." + f"mail_from:{envelope.mail_from}, from_header: {from_header}" + ) + changed_msg = add_header( + msg, + f"""This email failed anti-phishing checks when it was received by SimpleLogin, be careful with its content.""", + f""" +

+ This email failed anti-phishing checks when it was received by SimpleLogin, be careful with its content. +

+ """, + ) + # Change the payload inline + msg.set_payload(changed_msg.get_payload()) + return None + + if spam_result.dmarc in ( + DmarcCheckResult.quarantine, + DmarcCheckResult.reject, + ): + LOG.w( + f"dmarc forward: put email from {contact} to {alias} to quarantine. {spam_result.event_data()}, " + f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" + ) + email_log = quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) + Notification.create( + user_id=alias.user_id, + title=f"{alias.email} has a new mail in quarantine", + message=Notification.render( + "notification/message-quarantine.html", alias=alias + ), + commit=True, + ) + user = alias.user + send_email_with_rate_control( + user, + ALERT_QUARANTINE_DMARC, + user.email, + f"An email sent to {alias.email} has been quarantined", + render( + "transactional/message-quarantine-dmarc.txt.jinja2", + from_header=from_header, + alias=alias, + refused_email_url=email_log.get_dashboard_url(), + ), + render( + "transactional/message-quarantine-dmarc.html", + from_header=from_header, + alias=alias, + refused_email_url=email_log.get_dashboard_url(), + ), + max_nb_alert=10, + ignore_smtp_error=True, + ) + return status.E215 + + return None + + +def quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) -> EmailLog: + add_or_replace_header(msg, headers.SL_DIRECTION, "Forward") + msg[headers.SL_ENVELOPE_FROM] = envelope.mail_from + random_name = str(uuid.uuid4()) + s3_report_path = f"refused-emails/full-{random_name}.eml" + s3.upload_email_from_bytesio( + s3_report_path, BytesIO(to_bytes(msg)), f"full-{random_name}" + ) + refused_email = RefusedEmail.create( + full_report_path=s3_report_path, user_id=alias.user_id, flush=True + ) + return EmailLog.create( + user_id=alias.user_id, + mailbox_id=alias.mailbox_id, + contact_id=contact.id, + alias_id=alias.id, + message_id=str(msg[headers.MESSAGE_ID]), + refused_email_id=refused_email.id, + is_spam=True, + blocked=True, + commit=True, + ) + + +def apply_dmarc_policy_for_reply_phase( + alias_from: Alias, contact_recipient: Contact, envelope: Envelope, msg: Message +) -> Optional[str]: + spam_result = SpamdResult.extract_from_headers(msg, Phase.reply) + if not DMARC_CHECK_ENABLED or not spam_result: + return None + + if spam_result.dmarc not in ( + DmarcCheckResult.quarantine, + DmarcCheckResult.reject, + DmarcCheckResult.soft_fail, + ): + return None + LOG.w( + f"dmarc reply: Put email from {alias_from.email} to {contact_recipient} into quarantine. {spam_result.event_data()}, " + f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" + ) + send_email_with_rate_control( + alias_from.user, + ALERT_DMARC_FAILED_REPLY_PHASE, + alias_from.user.email, + f"Attempt to send an email to your contact {contact_recipient.email} from {envelope.mail_from}", + render( + "transactional/spoof-reply.txt", + contact=contact_recipient, + alias=alias_from, + sender=envelope.mail_from, + ), + render( + "transactional/spoof-reply.html", + contact=contact_recipient, + alias=alias_from, + sender=envelope.mail_from, + ), + ) + return status.E215 diff --git a/email_handler.py b/email_handler.py index a8fe37d6..faa196c0 100644 --- a/email_handler.py +++ b/email_handler.py @@ -87,15 +87,14 @@ from app.config import ( OLD_UNSUBSCRIBER, ALERT_FROM_ADDRESS_IS_REVERSE_ALIAS, ALERT_TO_NOREPLY, - DMARC_CHECK_ENABLED, - ALERT_QUARANTINE_DMARC, - ALERT_DMARC_FAILED_REPLY_PHASE, ) from app.db import Session +from app.handler.dmarc import ( + apply_dmarc_policy_for_reply_phase, + apply_dmarc_policy_for_forward_phase, +) from app.handler.spamd_result import ( SpamdResult, - Phase, - DmarcCheckResult, SPFCheckResult, ) from app.email import status, headers @@ -144,7 +143,6 @@ from app.errors import ( VERPForward, VERPReply, CannotCreateContactForReverseAlias, - DmarcSoftFail, ) from app.log import LOG, set_message_id from app.models import ( @@ -546,137 +544,6 @@ def handle_email_sent_to_ourself(alias, from_addr: str, msg: Message, user): ) -def apply_dmarc_policy_for_forward_phase( - alias: Alias, contact: Contact, envelope: Envelope, msg: Message -) -> Optional[str]: - spam_result = SpamdResult.extract_from_headers(msg, Phase.forward) - if not DMARC_CHECK_ENABLED or not spam_result: - return None - - from_header = get_header_unicode(msg[headers.FROM]) - - if spam_result.dmarc == DmarcCheckResult.soft_fail: - LOG.w( - f"dmarc forward: soft_fail from contact {contact.email} to alias {alias.email}." - f"mail_from:{envelope.mail_from}, from_header: {from_header}" - ) - raise DmarcSoftFail - - if spam_result.dmarc in ( - DmarcCheckResult.quarantine, - DmarcCheckResult.reject, - ): - LOG.w( - f"dmarc forward: put email from {contact} to {alias} to quarantine. {spam_result.event_data()}, " - f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" - ) - email_log = quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) - Notification.create( - user_id=alias.user_id, - title=f"{alias.email} has a new mail in quarantine", - message=Notification.render( - "notification/message-quarantine.html", alias=alias - ), - commit=True, - ) - user = alias.user - send_email_with_rate_control( - user, - ALERT_QUARANTINE_DMARC, - user.email, - f"An email sent to {alias.email} has been quarantined", - render( - "transactional/message-quarantine-dmarc.txt.jinja2", - from_header=from_header, - alias=alias, - refused_email_url=email_log.get_dashboard_url(), - ), - render( - "transactional/message-quarantine-dmarc.html", - from_header=from_header, - alias=alias, - refused_email_url=email_log.get_dashboard_url(), - ), - max_nb_alert=10, - ignore_smtp_error=True, - ) - return status.E215 - - return None - - -def quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) -> EmailLog: - add_or_replace_header(msg, headers.SL_DIRECTION, "Forward") - msg[headers.SL_ENVELOPE_TO] = alias.email - msg[headers.SL_ENVELOPE_FROM] = envelope.mail_from - add_or_replace_header(msg, "From", contact.new_addr()) - # replace CC & To emails by reverse-alias for all emails that are not alias - try: - replace_header_when_forward(msg, alias, "Cc") - replace_header_when_forward(msg, alias, "To") - except CannotCreateContactForReverseAlias: - Session.commit() - raise - - random_name = str(uuid.uuid4()) - s3_report_path = f"refused-emails/full-{random_name}.eml" - s3.upload_email_from_bytesio( - s3_report_path, BytesIO(to_bytes(msg)), f"full-{random_name}" - ) - refused_email = RefusedEmail.create( - full_report_path=s3_report_path, user_id=alias.user_id, flush=True - ) - return EmailLog.create( - user_id=alias.user_id, - mailbox_id=alias.mailbox_id, - contact_id=contact.id, - alias_id=alias.id, - message_id=str(msg[headers.MESSAGE_ID]), - refused_email_id=refused_email.id, - is_spam=True, - blocked=True, - commit=True, - ) - - -def apply_dmarc_policy_for_reply_phase( - alias_from: Alias, contact_recipient: Contact, envelope: Envelope, msg: Message -) -> Optional[str]: - spam_result = SpamdResult.extract_from_headers(msg, Phase.reply) - if not DMARC_CHECK_ENABLED or not spam_result: - return None - - if spam_result.dmarc not in ( - DmarcCheckResult.quarantine, - DmarcCheckResult.reject, - DmarcCheckResult.soft_fail, - ): - return None - LOG.w( - f"dmarc reply: Put email from {alias_from.email} to {contact_recipient} into quarantine. {spam_result.event_data()}, " - f"mail_from:{envelope.mail_from}, from_header: {msg[headers.FROM]}" - ) - send_email_with_rate_control( - alias_from.user, - ALERT_DMARC_FAILED_REPLY_PHASE, - alias_from.user.email, - f"Attempt to send an email to your contact {contact_recipient.email} from {envelope.mail_from}", - render( - "transactional/spoof-reply.txt", - contact=contact_recipient, - alias=alias_from, - sender=envelope.mail_from, - ), - render( - "transactional/spoof-reply.html", - contact=contact_recipient, - alias=alias_from, - sender=envelope.mail_from, - ), - ) - return status.E215 - - def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str]]: """return an array of SMTP status (is_success, smtp_status) is_success indicates whether an email has been delivered and @@ -758,22 +625,11 @@ 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 - try: - dmarc_delivery_status = apply_dmarc_policy_for_forward_phase( - alias, contact, envelope, msg - ) - if dmarc_delivery_status is not None: - return [(False, dmarc_delivery_status)] - except DmarcSoftFail: - msg = add_header( - msg, - f"""This email failed anti-phishing checks when it was received by SimpleLogin, be careful with its content.""", - f""" -

- This email failed anti-phishing checks when it was received by SimpleLogin, be careful with its content. -

-""", - ) + dmarc_delivery_status = apply_dmarc_policy_for_forward_phase( + alias, contact, envelope, msg + ) + if dmarc_delivery_status is not None: + return [(False, dmarc_delivery_status)] ret = [] mailboxes = alias.mailboxes diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 805c7524..30cbbd2a 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -104,25 +104,17 @@ def test_dmarc_forward_quarantine(flask_client): assert f"{alias.email} has a new mail in quarantine" == notifications[0].title -# todo: re-enable test when softfail is quarantined -# def test_gmail_dmarc_softfail(flask_client): -# user = create_random_user() -# alias = Alias.create_new_random(user) -# msg = load_eml_file("dmarc_gmail_softfail.eml", {"alias_email": alias.email}) -# envelope = Envelope() -# envelope.mail_from = msg["from"] -# envelope.rcpt_tos = [msg["to"]] -# result = email_handler.handle(envelope, msg) -# assert result == status.E215 -# email_logs = ( -# EmailLog.filter_by(user_id=user.id, alias_id=alias.id) -# .order_by(EmailLog.id.desc()) -# .all() -# ) -# assert len(email_logs) == 1 -# email_log = email_logs[0] -# assert email_log.blocked -# assert email_log.refused_email_id +def test_gmail_dmarc_softfail(flask_client): + user = create_random_user() + alias = Alias.create_new_random(user) + msg = load_eml_file("dmarc_gmail_softfail.eml", {"alias_email": alias.email}) + envelope = Envelope() + envelope.mail_from = msg["from"] + envelope.rcpt_tos = [msg["to"]] + result = email_handler.handle(envelope, msg) + assert result == status.E200 + payload = msg.get_payload() + assert payload.find("failed anti-phishing checks") > -1 def test_prevent_5xx_from_spf(flask_client): From 0dbe504329c9864f4990d916c3dd14fd3bc4db42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Fri, 8 Apr 2022 14:23:59 +0200 Subject: [PATCH 07/11] format --- app/email_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/email_utils.py b/app/email_utils.py index e1e07e2d..ccbe398a 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -1459,4 +1459,3 @@ def save_envelope_for_debugging(envelope: Envelope, file_name_prefix=None) -> st return file_name return "" - From 7fdd7d7f6a10e8d863d06056e58a4f9cf8efe07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 09:28:57 +0200 Subject: [PATCH 08/11] PR changes --- app/handler/dmarc.py | 14 ++++++-------- app/handler/spamd_result.py | 2 ++ email_handler.py | 2 +- tests/test_email_handler.py | 5 +++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/handler/dmarc.py b/app/handler/dmarc.py index 03671578..24e39550 100644 --- a/app/handler/dmarc.py +++ b/app/handler/dmarc.py @@ -1,6 +1,6 @@ import uuid from io import BytesIO -from typing import Optional +from typing import Optional, Tuple, Union from aiosmtpd.handlers import Message from aiosmtpd.smtp import Envelope @@ -27,10 +27,10 @@ from app.models import Alias, Contact, Notification, EmailLog, RefusedEmail def apply_dmarc_policy_for_forward_phase( alias: Alias, contact: Contact, envelope: Envelope, msg: Message -) -> Optional[str]: +) -> Tuple[Message, Optional[str]]: spam_result = SpamdResult.extract_from_headers(msg, Phase.forward) if not DMARC_CHECK_ENABLED or not spam_result: - return None + return msg, None from_header = get_header_unicode(msg[headers.FROM]) @@ -48,9 +48,7 @@ def apply_dmarc_policy_for_forward_phase(

""", ) - # Change the payload inline - msg.set_payload(changed_msg.get_payload()) - return None + return changed_msg, None if spam_result.dmarc in ( DmarcCheckResult.quarantine, @@ -90,9 +88,9 @@ def apply_dmarc_policy_for_forward_phase( max_nb_alert=10, ignore_smtp_error=True, ) - return status.E215 + return msg, status.E215 - return None + return msg, None def quarantine_dmarc_failed_forward_email(alias, contact, envelope, msg) -> EmailLog: diff --git a/app/handler/spamd_result.py b/app/handler/spamd_result.py index cd5c8ce6..834a4b32 100644 --- a/app/handler/spamd_result.py +++ b/app/handler/spamd_result.py @@ -101,9 +101,11 @@ class SpamdResult: for header_value, dmarc_result in DmarcCheckResult.get_string_dict().items(): if header_value in spam_entries: spamd_result.set_dmarc_result(dmarc_result) + break for header_value, spf_result in SPFCheckResult.get_string_dict().items(): if header_value in spam_entries: spamd_result.set_spf_result(spf_result) + break cls._store_in_message(spamd_result, msg) return spamd_result diff --git a/email_handler.py b/email_handler.py index ccc7bb28..7d5cf021 100644 --- a/email_handler.py +++ b/email_handler.py @@ -626,7 +626,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_for_forward_phase( + msg, dmarc_delivery_status = apply_dmarc_policy_for_forward_phase( alias, contact, envelope, msg ) if dmarc_delivery_status is not None: diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 30cbbd2a..f1aff34c 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -113,8 +113,9 @@ def test_gmail_dmarc_softfail(flask_client): envelope.rcpt_tos = [msg["to"]] result = email_handler.handle(envelope, msg) assert result == status.E200 - payload = msg.get_payload() - assert payload.find("failed anti-phishing checks") > -1 + # Enable when we can verify that the actual message sent has this content + # payload = msg.get_payload() + # assert payload.find("failed anti-phishing checks") > -1 def test_prevent_5xx_from_spf(flask_client): From f333bb00c53f1a6284234886c015e2775259322d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 10:19:25 +0200 Subject: [PATCH 09/11] fix import --- app/handler/dmarc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/handler/dmarc.py b/app/handler/dmarc.py index 24e39550..8ba6d0a9 100644 --- a/app/handler/dmarc.py +++ b/app/handler/dmarc.py @@ -1,6 +1,6 @@ import uuid from io import BytesIO -from typing import Optional, Tuple, Union +from typing import Optional, Tuple from aiosmtpd.handlers import Message from aiosmtpd.smtp import Envelope From 0f91effce972bc2f29a67e89f37264c2d1f03843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 12 Apr 2022 09:34:05 +0200 Subject: [PATCH 10/11] Only send enum names --- app/handler/spamd_result.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/handler/spamd_result.py b/app/handler/spamd_result.py index 834a4b32..57cc250b 100644 --- a/app/handler/spamd_result.py +++ b/app/handler/spamd_result.py @@ -71,9 +71,9 @@ class SpamdResult: def event_data(self) -> Dict: return { "header": "present", - "dmarc": self.dmarc, - "spf": self.spf, - "phase": self.phase, + "dmarc": self.dmarc.name, + "spf": self.spf.name, + "phase": self.phase.name, } @classmethod From fc13171f3d63b7a77b0826da7fc715b9f32cad9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 12 Apr 2022 12:51:11 +0200 Subject: [PATCH 11/11] Move tests --- tests/test_email_utils.py | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/tests/test_email_utils.py b/tests/test_email_utils.py index b1c0a178..27e99b37 100644 --- a/tests/test_email_utils.py +++ b/tests/test_email_utils.py @@ -793,36 +793,6 @@ def test_is_invalid_mailbox_domain(flask_client): assert not is_invalid_mailbox_domain("xy.zt") -def test_dmarc_result_softfail(): - msg = load_eml_file("dmarc_gmail_softfail.eml") - 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_spamd_result(msg).dmarc - - -def test_dmarc_result_reject(): - msg = load_eml_file("dmarc_reject.eml") - assert DmarcCheckResult.reject == get_spamd_result(msg).dmarc - - -def test_dmarc_result_allow(): - msg = load_eml_file("dmarc_allow.eml") - 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_spamd_result(msg).dmarc - - -def test_dmarc_result_bad_policy(): - msg = load_eml_file("dmarc_bad_policy.eml") - assert DmarcCheckResult.bad_policy == get_spamd_result(msg).dmarc - - def test_add_header_multipart_with_invalid_part(): msg = load_eml_file("multipart_alternative.eml") parts = msg.get_payload() + ["invalid"]