From 67c2c6afad547691b462f7f1863dff8b7e84fd47 Mon Sep 17 00:00:00 2001 From: Son Date: Wed, 30 Mar 2022 11:59:32 +0700 Subject: [PATCH 01/34] add warning to email content when dmarc softfail --- app/errors.py | 5 +++++ email_handler.py | 24 +++++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/app/errors.py b/app/errors.py index 26be6091..e041c763 100644 --- a/app/errors.py +++ b/app/errors.py @@ -56,3 +56,8 @@ class MailSentFromReverseAlias(SLException): """raised when receiving an email sent from a reverse alias""" pass + + +class DmarcSoftFail(SLException): + + pass diff --git a/email_handler.py b/email_handler.py index 1eb37147..a7d033f4 100644 --- a/email_handler.py +++ b/email_handler.py @@ -138,6 +138,7 @@ from app.errors import ( VERPForward, VERPReply, CannotCreateContactForReverseAlias, + DmarcSoftFail, ) from app.log import LOG, set_message_id from app.models import ( @@ -549,18 +550,17 @@ def apply_dmarc_policy( return None 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 + raise DmarcSoftFail + 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. {spam_result.event_data()}, " @@ -597,6 +597,7 @@ def apply_dmarc_policy( ignore_smtp_error=True, ) return status.E215 + return None @@ -715,9 +716,18 @@ 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) - if dmarc_delivery_status is not None: - return [(False, dmarc_delivery_status)] + try: + dmarc_delivery_status = apply_dmarc_policy( + alias, contact, envelope, msg, from_header + ) + 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’s received by SimpleLogin, be careful with its content.""", + f"""This email failed anti-phishing checks when it’s received by SimpleLogin, be careful with its content.""", + ) ret = [] mailboxes = alias.mailboxes From 1b5521efcfb889f414f1785691f74469dc070a16 Mon Sep 17 00:00:00 2001 From: Son Date: Wed, 30 Mar 2022 12:17:50 +0700 Subject: [PATCH 02/34] use red color for warning --- email_handler.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/email_handler.py b/email_handler.py index a7d033f4..e2a94903 100644 --- a/email_handler.py +++ b/email_handler.py @@ -726,7 +726,11 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str msg = add_header( msg, f"""This email failed anti-phishing checks when it’s received by SimpleLogin, be careful with its content.""", - f"""This email failed anti-phishing checks when it’s received by SimpleLogin, be careful with its content.""", + f""" +

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

+""", ) ret = [] From 215561dec1e84a149db6ea37b3482cb2db563179 Mon Sep 17 00:00:00 2001 From: Son Date: Wed, 30 Mar 2022 20:54:42 +0700 Subject: [PATCH 03/34] fix test --- email_handler.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/email_handler.py b/email_handler.py index e2a94903..ddba3e4b 100644 --- a/email_handler.py +++ b/email_handler.py @@ -717,9 +717,7 @@ 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, from_header - ) + dmarc_delivery_status = apply_dmarc_policy(alias, contact, envelope, msg) if dmarc_delivery_status is not None: return [(False, dmarc_delivery_status)] except DmarcSoftFail: From c83bea665060a0cdce58f91232e3ec9f32f8529b Mon Sep 17 00:00:00 2001 From: Son Date: Sat, 2 Apr 2022 17:11:42 +0700 Subject: [PATCH 04/34] improve wording --- email_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/email_handler.py b/email_handler.py index ddba3e4b..6745ddb2 100644 --- a/email_handler.py +++ b/email_handler.py @@ -723,10 +723,10 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str except DmarcSoftFail: msg = add_header( msg, - f"""This email failed anti-phishing checks when it’s 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.""", f"""

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

""", ) From 43a6c87fd6d267145597ea437452b7320882fe27 Mon Sep 17 00:00:00 2001 From: Son Date: Sat, 2 Apr 2022 17:36:33 +0700 Subject: [PATCH 05/34] format some html files using pycharm --- templates/dashboard/index.html | 14 +++++++------- templates/dashboard/setting.html | 24 ++++++++++++++---------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/templates/dashboard/index.html b/templates/dashboard/index.html index 225c3cf0..cb1933f3 100644 --- a/templates/dashboard/index.html +++ b/templates/dashboard/index.html @@ -579,13 +579,13 @@ {% endblock %} diff --git a/templates/dashboard/setting.html b/templates/dashboard/setting.html index da0b1097..16ac1050 100644 --- a/templates/dashboard/setting.html +++ b/templates/dashboard/setting.html @@ -494,7 +494,8 @@
Disabled alias/Blocked contact
- When an email is sent to a disabled alias or sent from a blocked contact, you can decide what response the sender should see.
+ When an email is sent to a disabled alias or sent from a blocked contact, you can decide what + response the sender should see.
Ignore means they will see the message as delivered, but SimpleLogin won't actually forward it to you. This is the default option as you can start receiving the emails again by re-enabling the alias or unblocking a contact.
@@ -504,14 +505,16 @@ @@ -525,7 +528,8 @@
Include original sender in email headers
- SimpleLogin forwards emails to your mailbox from the reverse-alias and not from the original sender address.
+ SimpleLogin forwards emails to your mailbox from the reverse-alias and not from the original + sender address.
If this option is enabled, the original sender addresses is stored in the email header X-SimpleLogin-Envelope-From. You can choose to display this header in your email client.
As email headers aren't encrypted, your mailbox service can know the sender address via this header. From 9aeceb9119e8a39c21a7264b9c7f1eec5d5fc39e Mon Sep 17 00:00:00 2001 From: Son Date: Tue, 5 Apr 2022 11:52:43 +0200 Subject: [PATCH 06/34] change logging for icloud bounce case --- email_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/email_handler.py b/email_handler.py index 6745ddb2..e772406f 100644 --- a/email_handler.py +++ b/email_handler.py @@ -2367,10 +2367,10 @@ def handle(envelope: Envelope, msg: Message) -> str: email_log = EmailLog.get(email_log_id) alias = Alias.get_by(email=rcpt_tos[0]) LOG.w( - "iCloud bounces %s %s msg=%s", + "iCloud bounces %s %s, saved to%s", email_log, alias, - msg.as_string(), + save_email_for_debugging(msg, file_name_prefix="icloud_bounce_"), ) return handle_bounce(envelope, email_log, msg) From 754bd4964c0ff6db09e6f423f23d1d4e4debe117 Mon Sep 17 00:00:00 2001 From: Son Date: Tue, 5 Apr 2022 11:56:45 +0200 Subject: [PATCH 07/34] Set CONTENT_TRANSFER_ENCODING if absent --- email_handler.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/email_handler.py b/email_handler.py index e772406f..3286b9d8 100644 --- a/email_handler.py +++ b/email_handler.py @@ -2223,6 +2223,11 @@ def handle(envelope: Envelope, msg: Message) -> str: envelope.mail_from = mail_from envelope.rcpt_tos = rcpt_tos + # some emails don't have this header, set the default value (7bit) in this case + if headers.CONTENT_TRANSFER_ENCODING not in msg: + LOG.i("Set CONTENT_TRANSFER_ENCODING") + msg[headers.CONTENT_TRANSFER_ENCODING] = "7bit" + postfix_queue_id = get_queue_id(msg) if postfix_queue_id: set_message_id(postfix_queue_id) 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 08/34] 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 09/34] 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 0e3c46d944d2ff8194b7c9897b52ceb76fcd397e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Wed, 6 Apr 2022 17:31:46 +0200 Subject: [PATCH 10/34] Save original envelope for debugging --- app/email_utils.py | 23 ++++++++++++++++++++++- email_handler.py | 5 +++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/app/email_utils.py b/app/email_utils.py index f84b755f..a0ce7a0a 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -6,6 +6,9 @@ import random import time import uuid from copy import deepcopy + +from aiosmtpd.smtp import Envelope + from email import policy, message_from_bytes, message_from_string from email.header import decode_header, Header from email.message import Message, EmailMessage @@ -1432,7 +1435,7 @@ def save_email_for_debugging(msg: Message, file_name_prefix=None) -> str: if TEMP_DIR: file_name = str(uuid.uuid4()) + ".eml" if file_name_prefix: - file_name = file_name_prefix + file_name + file_name = "{}-{}".format(file_name_prefix, file_name) with open(os.path.join(TEMP_DIR, file_name), "wb") as f: f.write(msg.as_bytes()) @@ -1443,6 +1446,24 @@ def save_email_for_debugging(msg: Message, file_name_prefix=None) -> str: return "" +def save_envelope_for_debugging(envelope: Envelope, file_name_prefix=None) -> str: + """Save envelope for debugging to temporary location + Return the file path + """ + if TEMP_DIR: + file_name = str(uuid.uuid4()) + ".eml" + if file_name_prefix: + file_name = "{}-{}".format(file_name_prefix, file_name) + + with open(os.path.join(TEMP_DIR, file_name), "wb") as f: + f.write(envelope.original_content) + + LOG.d("envelope saved to %s", file_name) + return file_name + + return "" + + def get_spamd_result(msg: Message) -> Optional[SpamdResult]: spam_result_header = msg.get_all(headers.SPAMD_RESULT) if not spam_result_header: diff --git a/email_handler.py b/email_handler.py index 3286b9d8..a2d56620 100644 --- a/email_handler.py +++ b/email_handler.py @@ -131,6 +131,7 @@ from app.email_utils import ( get_mailbox_bounce_info, save_email_for_debugging, get_spamd_result, + save_envelope_for_debugging, ) from app.errors import ( NonReverseAliasInReplyPhase, @@ -2584,8 +2585,8 @@ class MailHandler: envelope.rcpt_tos, msg[headers.FROM], msg[headers.TO], - save_email_for_debugging( - msg, file_name_prefix=e.__class__.__name__ + save_envelope_for_debugging( + envelope, file_name_prefix=e.__class__.__name__ ), # todo: remove ) return status.E404 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 11/34] 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 12/34] 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 13/34] 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 d26fc6ecf0e217102eaf7a68ef8a8b39bfe4333e Mon Sep 17 00:00:00 2001 From: Son Date: Fri, 8 Apr 2022 11:10:43 +0200 Subject: [PATCH 14/34] update email wording --- .../transactional/hotmail-complaint-reply-phase.txt.jinja2 | 2 +- templates/emails/transactional/hotmail-complaint.html | 2 +- templates/emails/transactional/hotmail-complaint.txt.jinja2 | 2 +- .../emails/transactional/hotmail-transactional-complaint.html | 2 +- .../transactional/hotmail-transactional-complaint.txt.jinja2 | 2 +- templates/emails/transactional/yahoo-complaint.html | 2 +- templates/emails/transactional/yahoo-complaint.txt.jinja2 | 2 +- .../emails/transactional/yahoo-transactional-complaint.html | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/templates/emails/transactional/hotmail-complaint-reply-phase.txt.jinja2 b/templates/emails/transactional/hotmail-complaint-reply-phase.txt.jinja2 index 94aa6a63..934a96d1 100644 --- a/templates/emails/transactional/hotmail-complaint-reply-phase.txt.jinja2 +++ b/templates/emails/transactional/hotmail-complaint-reply-phase.txt.jinja2 @@ -11,5 +11,5 @@ Please note that sending non-solicited from a SimpleLogin alias infringes our te If somehow the recipient's Hotmail considers a forwarded email as Spam, it helps us a lot if you can ask them to move the email out of their Spam folder. -Looking to hear back from you. +Don't hesitate to get in touch with us if you need more information. {% endblock %} diff --git a/templates/emails/transactional/hotmail-complaint.html b/templates/emails/transactional/hotmail-complaint.html index 904ee183..fbc59520 100644 --- a/templates/emails/transactional/hotmail-complaint.html +++ b/templates/emails/transactional/hotmail-complaint.html @@ -28,7 +28,7 @@ {% endcall %} {% call text() %} - Looking to hear back from you. + Don't hesitate to get in touch with us if you need more information. {% endcall %} {% call text() %} diff --git a/templates/emails/transactional/hotmail-complaint.txt.jinja2 b/templates/emails/transactional/hotmail-complaint.txt.jinja2 index 7862561c..e27a96e0 100644 --- a/templates/emails/transactional/hotmail-complaint.txt.jinja2 +++ b/templates/emails/transactional/hotmail-complaint.txt.jinja2 @@ -16,5 +16,5 @@ If that’s the case, please disable the alias instead if you don't want to rece If somehow Hotmail considers a forwarded email as Spam, it will help us if you can move the email out of the Spam folder. You can also set up a filter to avoid this from happening in the future using this guide at https://simplelogin.io/help/ -Looking to hear back from you. +Don't hesitate to get in touch with us if you need more information. {% endblock %} diff --git a/templates/emails/transactional/hotmail-transactional-complaint.html b/templates/emails/transactional/hotmail-transactional-complaint.html index c71d4dcb..0d794bab 100644 --- a/templates/emails/transactional/hotmail-transactional-complaint.html +++ b/templates/emails/transactional/hotmail-transactional-complaint.html @@ -29,7 +29,7 @@ {% endcall %} {% call text() %} - Looking to hear back from you. + Don't hesitate to get in touch with us if you need more information. {% endcall %} {% call text() %} diff --git a/templates/emails/transactional/hotmail-transactional-complaint.txt.jinja2 b/templates/emails/transactional/hotmail-transactional-complaint.txt.jinja2 index 7d936849..7969130c 100644 --- a/templates/emails/transactional/hotmail-transactional-complaint.txt.jinja2 +++ b/templates/emails/transactional/hotmail-transactional-complaint.txt.jinja2 @@ -19,5 +19,5 @@ If somehow Hotmail considers a forwarded email as Spam, it helps us if you can m Please don't put our emails into the Spam folder. This can end up in your account being disabled on SimpleLogin. -Looking to hear back from you. +Don't hesitate to get in touch with us if you need more information. {% endblock %} diff --git a/templates/emails/transactional/yahoo-complaint.html b/templates/emails/transactional/yahoo-complaint.html index d7be34f7..c9814ce1 100644 --- a/templates/emails/transactional/yahoo-complaint.html +++ b/templates/emails/transactional/yahoo-complaint.html @@ -24,7 +24,7 @@ {% endcall %} {% call text() %} - Looking to hear back from you. + Don't hesitate to get in touch with us if you need more information. {% endcall %} {% call text() %} diff --git a/templates/emails/transactional/yahoo-complaint.txt.jinja2 b/templates/emails/transactional/yahoo-complaint.txt.jinja2 index 74fab1b5..8007e985 100644 --- a/templates/emails/transactional/yahoo-complaint.txt.jinja2 +++ b/templates/emails/transactional/yahoo-complaint.txt.jinja2 @@ -14,5 +14,5 @@ If that’s the case, please disable the alias instead if you don't want to rece If SimpleLogin isn’t useful for you, please know that you can simply delete your account on the Settings page. -Looking to hear back from you. +Don't hesitate to get in touch with us if you need more information. {% endblock %} diff --git a/templates/emails/transactional/yahoo-transactional-complaint.html b/templates/emails/transactional/yahoo-transactional-complaint.html index 250d6b3a..9d8db38f 100644 --- a/templates/emails/transactional/yahoo-transactional-complaint.html +++ b/templates/emails/transactional/yahoo-transactional-complaint.html @@ -29,7 +29,7 @@ {% endcall %} {% call text() %} - Looking to hear back from you. + Don't hesitate to get in touch with us if you need more information. {% endcall %} {% call text() %} From 42f89b71d7220e99705a94d897f70986126d08fd Mon Sep 17 00:00:00 2001 From: Son Date: Fri, 8 Apr 2022 11:06:01 +0200 Subject: [PATCH 15/34] ignore VERPTransactional --- email_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/email_handler.py b/email_handler.py index a2d56620..edb60c68 100644 --- a/email_handler.py +++ b/email_handler.py @@ -2565,7 +2565,7 @@ class MailHandler: msg[headers.TO], ) return status.E524 - except (VERPReply, VERPForward) as e: + except (VERPReply, VERPForward, VERPTransactional) as e: LOG.w( "email handling fail with error:%s " "mail_from:%s, rcpt_tos:%s, header_from:%s, header_to:%s", 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 16/34] 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 17/34] 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 18/34] 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 60a070731ec4ed63da88eb7175f55bef4963aa61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 10:18:22 +0200 Subject: [PATCH 19/34] Send newrelic events on login and register --- app/auth/views/login.py | 5 +++++ app/auth/views/register.py | 7 ++++++- app/events/auth_event.py | 31 +++++++++++++++++++++++++++++++ app/pgp_utils.py | 4 ++-- 4 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 app/events/auth_event.py diff --git a/app/auth/views/login.py b/app/auth/views/login.py index c68ff887..a08ca774 100644 --- a/app/auth/views/login.py +++ b/app/auth/views/login.py @@ -5,6 +5,7 @@ from wtforms import StringField, validators from app.auth.base import auth_bp from app.auth.views.login_utils import after_login +from app.events.auth_event import LoginEvent from app.extensions import limiter from app.log import LOG from app.models import User @@ -43,18 +44,22 @@ def login(): g.deduct_limit = True form.password.data = None flash("Email or password incorrect", "error") + LoginEvent(LoginEvent.ActionType.failed).send() elif user.disabled: flash( "Your account is disabled. Please contact SimpleLogin team to re-enable your account.", "error", ) + LoginEvent(LoginEvent.ActionType.disabled_login).send() elif not user.activated: show_resend_activation = True flash( "Please check your inbox for the activation email. You can also have this email re-sent", "error", ) + LoginEvent(LoginEvent.ActionType.not_activated).send() else: + LoginEvent(LoginEvent.ActionType.success).send() return after_login(user, next_url) return render_template( diff --git a/app/auth/views/register.py b/app/auth/views/register.py index abbc56f5..5ed8b47d 100644 --- a/app/auth/views/register.py +++ b/app/auth/views/register.py @@ -13,6 +13,7 @@ from app.email_utils import ( email_can_be_used_as_mailbox, personal_email_already_used, ) +from app.events.auth_event import RegisterEvent from app.log import LOG from app.models import User, ActivationCode from app.utils import random_string, encode_url, sanitize_email @@ -60,6 +61,7 @@ def register(): hcaptcha_res, ) flash("Wrong Captcha", "error") + RegisterEvent(RegisterEvent.ActionType.regsiter_catpcha_failed).send() return render_template( "auth/register.html", form=form, @@ -70,10 +72,11 @@ def register(): email = sanitize_email(form.email.data) if not email_can_be_used_as_mailbox(email): flash("You cannot use this email address as your personal inbox.", "error") - + RegisterEvent(RegisterEvent.ActionType.email_in_use).send() else: if personal_email_already_used(email): flash(f"Email {email} already used", "error") + RegisterEvent(RegisterEvent.ActionType.email_in_use).send() else: LOG.d("create user %s", email) user = User.create( @@ -86,8 +89,10 @@ def register(): try: send_activation_email(user, next_url) + RegisterEvent(RegisterEvent.ActionType.success).send() except Exception: flash("Invalid email, are you sure the email is correct?", "error") + RegisterEvent(RegisterEvent.ActionType.invalid_email).send() return redirect(url_for("auth.register")) return render_template("auth/register_waiting_activation.html") diff --git a/app/events/auth_event.py b/app/events/auth_event.py new file mode 100644 index 00000000..8e2a830c --- /dev/null +++ b/app/events/auth_event.py @@ -0,0 +1,31 @@ +import newrelic + +from app.models import EnumE + + +class LoginEvent: + class ActionType(EnumE): + success = 0 + failed = 1 + disabled_login = 2 + not_activated = 3 + + def __init__(self, action: ActionType): + self.action = action + + def send(self): + newrelic.agent.record_custom_event("LoginEvent", {"action": self.action}) + + +class RegisterEvent: + class ActionType(EnumE): + success = 0 + catpcha_failed = 1 + email_in_use = 2 + invalid_email = 3 + + def __init__(self, action: ActionType): + self.action = action + + def send(self): + newrelic.agent.record_custom_event("RegisterEvent", {"action": self.action}) diff --git a/app/pgp_utils.py b/app/pgp_utils.py index 211ec00d..8934ddec 100644 --- a/app/pgp_utils.py +++ b/app/pgp_utils.py @@ -65,7 +65,7 @@ def encrypt_file(data: BytesIO, fingerprint: str) -> str: LOG.d("mem_usage %s", mem_usage) r = gpg.encrypt_file(data, fingerprint, always_trust=True) - if not r.ok: + if not r.success: # maybe the fingerprint is not loaded on this host, try to load it found = False # searching for the key in mailbox @@ -87,7 +87,7 @@ def encrypt_file(data: BytesIO, fingerprint: str) -> str: data.seek(0) r = gpg.encrypt_file(data, fingerprint, always_trust=True) - if not r.ok: + if not r.success: raise PGPException(f"Cannot encrypt, status: {r.status}") return str(r) 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 20/34] 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 dc59b61fba6f936298a4074337ffb2e3da9bd587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 10:20:02 +0200 Subject: [PATCH 21/34] Revert changes to pgp_utils --- app/pgp_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/pgp_utils.py b/app/pgp_utils.py index 8934ddec..211ec00d 100644 --- a/app/pgp_utils.py +++ b/app/pgp_utils.py @@ -65,7 +65,7 @@ def encrypt_file(data: BytesIO, fingerprint: str) -> str: LOG.d("mem_usage %s", mem_usage) r = gpg.encrypt_file(data, fingerprint, always_trust=True) - if not r.success: + if not r.ok: # maybe the fingerprint is not loaded on this host, try to load it found = False # searching for the key in mailbox @@ -87,7 +87,7 @@ def encrypt_file(data: BytesIO, fingerprint: str) -> str: data.seek(0) r = gpg.encrypt_file(data, fingerprint, always_trust=True) - if not r.success: + if not r.ok: raise PGPException(f"Cannot encrypt, status: {r.status}") return str(r) From 7649f6b82274d524670a617982e0ce13ab3dd65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 14:19:32 +0200 Subject: [PATCH 22/34] Do not show an error if we receive an unsubscribe from a different address --- app/email_utils.py | 1 + email_handler.py | 2 +- pytest.ini | 2 +- tests/test_email_handler.py | 9 +++++++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/email_utils.py b/app/email_utils.py index a0ce7a0a..b6d19505 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -1450,6 +1450,7 @@ def save_envelope_for_debugging(envelope: Envelope, file_name_prefix=None) -> st """Save envelope for debugging to temporary location Return the file path """ + LOG.d("TE {}".format( TEMP_DIR)) if TEMP_DIR: file_name = str(uuid.uuid4()) + ".eml" if file_name_prefix: diff --git a/email_handler.py b/email_handler.py index edb60c68..d3c50783 100644 --- a/email_handler.py +++ b/email_handler.py @@ -2083,7 +2083,7 @@ def handle_unsubscribe_user(user_id: int, mail_from: str) -> str: return status.E510 if mail_from != user.email: - LOG.e("Unauthorized mail_from %s %s", user, mail_from) + LOG.w("Unauthorized mail_from %s %s", user, mail_from) return status.E511 user.notification = False diff --git a/pytest.ini b/pytest.ini index 3d362baf..c0f5472c 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -addopts = +xaddopts = --cov --cov-config coverage.ini --cov-report=html:htmlcov diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 1f7cdd59..c89589eb 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -159,3 +159,12 @@ 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 test_lol(): + msg = load_eml_file("KeyErrorbf6219c1-3bed-419f-ae40-bec264204e2d.eml") + msg.as_string() + envelope = Envelope() + envelope.mail_from = msg[headers.FROM] + envelope.rcpt_tos = [msg["to"]] + envelope.original_content = msg.as_string() + result = email_handler.MailHandler()._handle(envelope, msg) From ae8824a356fdf664266f740e807782938673c489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 14:20:56 +0200 Subject: [PATCH 23/34] Revert unwanted changes --- app/email_utils.py | 1 - pytest.ini | 2 +- tests/test_email_handler.py | 9 --------- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/app/email_utils.py b/app/email_utils.py index b6d19505..a0ce7a0a 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -1450,7 +1450,6 @@ def save_envelope_for_debugging(envelope: Envelope, file_name_prefix=None) -> st """Save envelope for debugging to temporary location Return the file path """ - LOG.d("TE {}".format( TEMP_DIR)) if TEMP_DIR: file_name = str(uuid.uuid4()) + ".eml" if file_name_prefix: diff --git a/pytest.ini b/pytest.ini index c0f5472c..3d362baf 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -xaddopts = +addopts = --cov --cov-config coverage.ini --cov-report=html:htmlcov diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index c89589eb..1f7cdd59 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -159,12 +159,3 @@ 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 test_lol(): - msg = load_eml_file("KeyErrorbf6219c1-3bed-419f-ae40-bec264204e2d.eml") - msg.as_string() - envelope = Envelope() - envelope.mail_from = msg[headers.FROM] - envelope.rcpt_tos = [msg["to"]] - envelope.original_content = msg.as_string() - result = email_handler.MailHandler()._handle(envelope, msg) From dbc55c50a261c80a2ee2bdf841052249283b7fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 14:51:33 +0200 Subject: [PATCH 24/34] Add missing formatting place --- email_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/email_handler.py b/email_handler.py index d3c50783..a66b6f95 100644 --- a/email_handler.py +++ b/email_handler.py @@ -283,7 +283,7 @@ def get_or_create_reply_to_contact( return contact else: LOG.d( - "create contact %s for alias %s via reply-to header", + "create contact %s for alias %s via reply-to header %s", contact_address, alias, reply_to_header, From c16fd25b2ec8948d9e269143ad99923b6d87038e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 15:52:31 +0200 Subject: [PATCH 25/34] Added fix for parts that are not messages --- app/email_utils.py | 5 ++- pytest.ini | 2 +- tests/example_emls/multipart_alternative.eml | 25 +++++++++++++ tests/test_email_handler.py | 37 ++++++++++---------- tests/test_email_utils.py | 12 +++++++ 5 files changed, 60 insertions(+), 21 deletions(-) create mode 100644 tests/example_emls/multipart_alternative.eml diff --git a/app/email_utils.py b/app/email_utils.py index a0ce7a0a..26194158 100644 --- a/app/email_utils.py +++ b/app/email_utils.py @@ -970,7 +970,10 @@ def add_header(msg: Message, text_header, html_header) -> Message: elif content_type in ("multipart/alternative", "multipart/related"): new_parts = [] for part in msg.get_payload(): - new_parts.append(add_header(part, text_header, html_header)) + if isinstance(part, Message): + new_parts.append(add_header(part, text_header, html_header)) + else: + new_parts.append(part) clone_msg = copy(msg) clone_msg.set_payload(new_parts) return clone_msg diff --git a/pytest.ini b/pytest.ini index 3d362baf..c0f5472c 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -addopts = +xaddopts = --cov --cov-config coverage.ini --cov-report=html:htmlcov diff --git a/tests/example_emls/multipart_alternative.eml b/tests/example_emls/multipart_alternative.eml new file mode 100644 index 00000000..27fa7d67 --- /dev/null +++ b/tests/example_emls/multipart_alternative.eml @@ -0,0 +1,25 @@ +Content-Type: multipart/alternative; boundary="===============5006593052976639648==" +MIME-Version: 1.0 +Subject: My subject +From: foo@example.org +To: bar@example.net + +--===============5006593052976639648== +Content-Type: text/plain; charset="us-ascii" +MIME-Version: 1.0 +Content-Transfer-Encoding: 7bit + +This is HTML +--===============5006593052976639648== +Content-Type: text/html; charset="us-ascii" +MIME-Version: 1.0 +Content-Transfer-Encoding: 7bit + + + + This is HTML + + + +--===============5006593052976639648==-- + diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 1f7cdd59..77517c5a 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -98,25 +98,24 @@ def test_dmarc_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.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_prevent_5xx_from_spf(flask_client): diff --git a/tests/test_email_utils.py b/tests/test_email_utils.py index e5d16744..30c95ae6 100644 --- a/tests/test_email_utils.py +++ b/tests/test_email_utils.py @@ -823,3 +823,15 @@ def test_dmarc_result_na(): 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"] + msg.set_payload(parts) + msg = add_header(msg, "INJECT", "INJECT") + for i, part in enumerate(msg.get_payload()): + if i < 2: + assert part.get_payload().index("INJECT") > -1 + else: + assert part == "invalid" From edf34656b6399a3aece327d39e4e73bcd6676599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 15:53:37 +0200 Subject: [PATCH 26/34] revert changes --- pytest.ini | 2 +- tests/test_email_handler.py | 37 +++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pytest.ini b/pytest.ini index c0f5472c..3d362baf 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,5 +1,5 @@ [pytest] -xaddopts = +addopts = --cov --cov-config coverage.ini --cov-report=html:htmlcov diff --git a/tests/test_email_handler.py b/tests/test_email_handler.py index 77517c5a..1f7cdd59 100644 --- a/tests/test_email_handler.py +++ b/tests/test_email_handler.py @@ -98,24 +98,25 @@ def test_dmarc_quarantine(flask_client): assert f"{alias.email} has a new mail in quarantine" == notifications[0].title -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 +# 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_prevent_5xx_from_spf(flask_client): From 8da4293305ee6bbfdc0a4c964a8ee3ebe990b178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 16:04:28 +0200 Subject: [PATCH 27/34] typo --- app/auth/views/register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/auth/views/register.py b/app/auth/views/register.py index 5ed8b47d..60e6b01d 100644 --- a/app/auth/views/register.py +++ b/app/auth/views/register.py @@ -61,7 +61,7 @@ def register(): hcaptcha_res, ) flash("Wrong Captcha", "error") - RegisterEvent(RegisterEvent.ActionType.regsiter_catpcha_failed).send() + RegisterEvent(RegisterEvent.ActionType.catpcha_failed).send() return render_template( "auth/register.html", form=form, From 2b149747f5f864eecd40803254caef0b7305b2d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Mon, 11 Apr 2022 16:11:01 +0200 Subject: [PATCH 28/34] Also track login and register events from the api routes --- app/api/views/auth.py | 12 ++++++++++++ app/events/auth_event.py | 29 ++++++++++++++++++++++------- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/app/api/views/auth.py b/app/api/views/auth.py index 105e0bb6..a149f9b4 100644 --- a/app/api/views/auth.py +++ b/app/api/views/auth.py @@ -19,6 +19,7 @@ from app.email_utils import ( send_email, render, ) +from app.events.auth_event import LoginEvent, RegisterEvent from app.extensions import limiter from app.log import LOG from app.models import User, ApiKey, SocialAuth, AccountActivation @@ -55,16 +56,20 @@ def auth_login(): user = User.filter_by(email=email).first() if not user or not user.check_password(password): + LoginEvent(LoginEvent.ActionType.failed, LoginEvent.Source.api).send() return jsonify(error="Email or password incorrect"), 400 elif user.disabled: + LoginEvent(LoginEvent.ActionType.disabled_login, LoginEvent.Source.api).send() return jsonify(error="Account disabled"), 400 elif not user.activated: + LoginEvent(LoginEvent.ActionType.not_activated, LoginEvent.Source.api).send() return jsonify(error="Account not activated"), 422 elif user.fido_enabled(): # allow user who has TOTP enabled to continue using the mobile app if not user.enable_otp: return jsonify(error="Currently we don't support FIDO on mobile yet"), 403 + LoginEvent(LoginEvent.ActionType.success, LoginEvent.Source.api).send() return jsonify(**auth_payload(user, device)), 200 @@ -88,14 +93,20 @@ def auth_register(): password = data.get("password") if DISABLE_REGISTRATION: + RegisterEvent(RegisterEvent.ActionType.failed, RegisterEvent.Source.api).send() return jsonify(error="registration is closed"), 400 if not email_can_be_used_as_mailbox(email) or personal_email_already_used(email): + RegisterEvent( + RegisterEvent.ActionType.invalid_email, RegisterEvent.Source.api + ).send() return jsonify(error=f"cannot use {email} as personal inbox"), 400 if not password or len(password) < 8: + RegisterEvent(RegisterEvent.ActionType.failed, RegisterEvent.Source.api).send() return jsonify(error="password too short"), 400 if len(password) > 100: + RegisterEvent(RegisterEvent.ActionType.failed, RegisterEvent.Source.api).send() return jsonify(error="password too long"), 400 LOG.d("create user %s", email) @@ -114,6 +125,7 @@ def auth_register(): render("transactional/code-activation.html", code=code), ) + RegisterEvent(RegisterEvent.ActionType.success, RegisterEvent.Source.api).send() return jsonify(msg="User needs to confirm their account"), 200 diff --git a/app/events/auth_event.py b/app/events/auth_event.py index 8e2a830c..2ed9296e 100644 --- a/app/events/auth_event.py +++ b/app/events/auth_event.py @@ -10,22 +10,37 @@ class LoginEvent: disabled_login = 2 not_activated = 3 - def __init__(self, action: ActionType): + class Source(EnumE): + web = 0 + api = 1 + + def __init__(self, action: ActionType, source: Source = Source.web): self.action = action + self.source = source def send(self): - newrelic.agent.record_custom_event("LoginEvent", {"action": self.action}) + newrelic.agent.record_custom_event( + "LoginEvent", {"action": self.action, "source": self.source} + ) class RegisterEvent: class ActionType(EnumE): success = 0 - catpcha_failed = 1 - email_in_use = 2 - invalid_email = 3 + failed = 1 + catpcha_failed = 2 + email_in_use = 3 + invalid_email = 4 - def __init__(self, action: ActionType): + class Source(EnumE): + web = 0 + api = 1 + + def __init__(self, action: ActionType, source: Source = Source.web): self.action = action + self.source = source def send(self): - newrelic.agent.record_custom_event("RegisterEvent", {"action": self.action}) + newrelic.agent.record_custom_event( + "RegisterEvent", {"action": self.action, "source": self.source} + ) From 9928525cf95f98e2587000c7cc1b97133615d6fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Casaj=C3=BAs?= Date: Tue, 12 Apr 2022 09:04:57 +0200 Subject: [PATCH 29/34] Only send enum name for events intead of the full class.enum --- app/events/auth_event.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/events/auth_event.py b/app/events/auth_event.py index 2ed9296e..492c4e89 100644 --- a/app/events/auth_event.py +++ b/app/events/auth_event.py @@ -20,7 +20,7 @@ class LoginEvent: def send(self): newrelic.agent.record_custom_event( - "LoginEvent", {"action": self.action, "source": self.source} + "LoginEvent", {"action": self.action.name, "source": self.source.name} ) @@ -42,5 +42,5 @@ class RegisterEvent: def send(self): newrelic.agent.record_custom_event( - "RegisterEvent", {"action": self.action, "source": self.source} + "RegisterEvent", {"action": self.action.name, "source": self.source.name} ) 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 30/34] 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 31/34] 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"] From 95770de4d581b6ce1f83653b7923455adcfcb9b1 Mon Sep 17 00:00:00 2001 From: Son Date: Thu, 14 Apr 2022 09:23:49 +0200 Subject: [PATCH 32/34] improve email wording --- app/handler/dmarc.py | 2 +- .../emails/transactional/spoof-reply.html | 26 ++++++++++--------- .../emails/transactional/spoof-reply.txt | 8 ------ .../transactional/spoof-reply.txt.jinja2 | 10 +++++++ 4 files changed, 25 insertions(+), 21 deletions(-) delete mode 100644 templates/emails/transactional/spoof-reply.txt create mode 100644 templates/emails/transactional/spoof-reply.txt.jinja2 diff --git a/app/handler/dmarc.py b/app/handler/dmarc.py index 8ba6d0a9..75633000 100644 --- a/app/handler/dmarc.py +++ b/app/handler/dmarc.py @@ -140,7 +140,7 @@ def apply_dmarc_policy_for_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", + "transactional/spoof-reply.txt.jinja2", contact=contact_recipient, alias=alias_from, sender=envelope.mail_from, diff --git a/templates/emails/transactional/spoof-reply.html b/templates/emails/transactional/spoof-reply.html index 83766dba..576c1a2b 100644 --- a/templates/emails/transactional/spoof-reply.html +++ b/templates/emails/transactional/spoof-reply.html @@ -2,19 +2,21 @@ {% 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() %} +

+ Unauthorized attempt to send an 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() %} + To protect against email spoofing, only your mailbox can send emails on behalf of your alias. + SimpleLogin also refuses emails that claim to come from your mailbox but fail DMARC. + {% endcall %} - {% call text() %} - Best,
- SimpleLogin Team. - {% endcall %} + {% call text() %} + Best,
+ SimpleLogin Team. + {% endcall %} {% endblock %} diff --git a/templates/emails/transactional/spoof-reply.txt b/templates/emails/transactional/spoof-reply.txt deleted file mode 100644 index 22d27018..00000000 --- a/templates/emails/transactional/spoof-reply.txt +++ /dev/null @@ -1,8 +0,0 @@ -{% 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/templates/emails/transactional/spoof-reply.txt.jinja2 b/templates/emails/transactional/spoof-reply.txt.jinja2 new file mode 100644 index 00000000..7a1e0d9f --- /dev/null +++ b/templates/emails/transactional/spoof-reply.txt.jinja2 @@ -0,0 +1,10 @@ +{% extends "base.txt.jinja2" %} + +{% block content %} + Unauthorized attempt to send an email to {{ contact.email }} from your alias {{ alias.email }} using + {{ sender }} has been blocked. + + To protect against email spoofing, only your mailbox can send emails on behalf of your alias. + SimpleLogin also refuses emails that claim to come from your mailbox but fail DMARC. +{% endblock %} + From 1709de93ef7c81759802c6b7b796175483740479 Mon Sep 17 00:00:00 2001 From: Son Date: Thu, 14 Apr 2022 09:28:26 +0200 Subject: [PATCH 33/34] add link to the anti phishing page --- app/handler/dmarc.py | 7 +++++-- .../emails/transactional/message-quarantine-dmarc.html | 4 ++-- .../transactional/message-quarantine-dmarc.txt.jinja2 | 5 ++++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/handler/dmarc.py b/app/handler/dmarc.py index 75633000..2126065c 100644 --- a/app/handler/dmarc.py +++ b/app/handler/dmarc.py @@ -41,10 +41,13 @@ def apply_dmarc_policy_for_forward_phase( ) 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. +More info on https://simplelogin.io/docs/getting-started/anti-phishing/ + """, f"""

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

""", ) diff --git a/templates/emails/transactional/message-quarantine-dmarc.html b/templates/emails/transactional/message-quarantine-dmarc.html index 2953db12..ad96152d 100644 --- a/templates/emails/transactional/message-quarantine-dmarc.html +++ b/templates/emails/transactional/message-quarantine-dmarc.html @@ -8,8 +8,8 @@ {% endcall %} {% call text() %} - An email from {{ from_header }} to {{ alias.email }} is put into Quarantine as it fails DMARC check. - DMARC is an email authentication protocol designed for detecting phishing. + An email from {{ from_header }} to {{ alias.email }} is put into Quarantine as it fails + anti-phishing measure check. {% endcall %} {{ render_button("View the original email", refused_email_url) }} diff --git a/templates/emails/transactional/message-quarantine-dmarc.txt.jinja2 b/templates/emails/transactional/message-quarantine-dmarc.txt.jinja2 index 96b540d9..5b917c1f 100644 --- a/templates/emails/transactional/message-quarantine-dmarc.txt.jinja2 +++ b/templates/emails/transactional/message-quarantine-dmarc.txt.jinja2 @@ -1,8 +1,11 @@ {% extends "base.txt.jinja2" %} {% block content %} - An email from {{ from_header }} to {{ alias.email }} is put into Quarantine as it fails DMARC check. + An email from {{ from_header }} to {{ alias.email }} is put into Quarantine as it fails anti-phishing check. + You can view the email at {{ refused_email_url }}. This email is automatically deleted in 7 days. + + More info about the anti-phishing measure on https://simplelogin.io/docs/getting-started/anti-phishing/ {% endblock %} From a957cbb3c0518ddfabc6a60b08e401d2e1dcb85b Mon Sep 17 00:00:00 2001 From: Son Date: Thu, 14 Apr 2022 09:47:58 +0200 Subject: [PATCH 34/34] fix flake8 --- 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 2126065c..62eed243 100644 --- a/app/handler/dmarc.py +++ b/app/handler/dmarc.py @@ -46,7 +46,7 @@ More info on https://simplelogin.io/docs/getting-started/anti-phishing/ """, f"""

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

""",