diff --git a/lambdas/po/email_processor/ses_auth.py b/lambdas/po/email_processor/ses_auth.py index 6ad786b..cf2b43e 100644 --- a/lambdas/po/email_processor/ses_auth.py +++ b/lambdas/po/email_processor/ses_auth.py @@ -30,6 +30,7 @@ Note SES reports the passing DKIM identity as ``header.i=@`` """ import email.parser +import email.policy import json import logging import os @@ -43,7 +44,9 @@ SES_AUTHSERV_ID = "amazonses.com" # One resinfo clause of an Authentication-Results value, e.g. # "dkim=pass header.i=@seahaven.com". The clause must START with the # method=result pair; header.d= / header.i= may appear anywhere after it. -_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)", re.IGNORECASE) +# The result token must be terminated by end-of-clause, whitespace, or a +# comment so "dkim=pass-anything" can never be read as "pass". +_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|[\s(])", re.IGNORECASE) _HEADER_D_RE = re.compile(r"header\.d\s*=\s*\"?([^\s\";]+)", re.IGNORECASE) _HEADER_I_RE = re.compile(r"header\.i\s*=\s*\"?([^\s\";]+)", re.IGNORECASE) @@ -51,7 +54,9 @@ _HEADER_I_RE = re.compile(r"header\.i\s*=\s*\"?([^\s\";]+)", re.IGNORECASE) def get_allowed_dkim_domains() -> frozenset: """Read the DKIM-domain allowlist from the environment (may be empty).""" raw = os.environ.get(ALLOWED_DKIM_DOMAINS_ENV, "") - return frozenset(d.strip().lower().lstrip("@") for d in raw.split(",") if d.strip()) + return frozenset( + d.strip().lower().lstrip("@").rstrip(".") for d in raw.split(",") if d.strip() + ) def _unfold(value: str) -> str: @@ -108,7 +113,9 @@ def evaluate_sender_authentication(raw_email: bytes, allowed_domains) -> tuple: # compat32 keeps header values as raw strings (we unfold ourselves) # and never raises on structurally odd headers; headersonly avoids # parsing the body at all. - msg = email.parser.BytesParser().parsebytes(raw_email, headersonly=True) + msg = email.parser.BytesParser(policy=email.policy.compat32).parsebytes( + raw_email, headersonly=True + ) except Exception: return False, "unparseable_message", {} diff --git a/lambdas/wo/email_processor/ses_auth.py b/lambdas/wo/email_processor/ses_auth.py index 6ad786b..cf2b43e 100644 --- a/lambdas/wo/email_processor/ses_auth.py +++ b/lambdas/wo/email_processor/ses_auth.py @@ -30,6 +30,7 @@ Note SES reports the passing DKIM identity as ``header.i=@`` """ import email.parser +import email.policy import json import logging import os @@ -43,7 +44,9 @@ SES_AUTHSERV_ID = "amazonses.com" # One resinfo clause of an Authentication-Results value, e.g. # "dkim=pass header.i=@seahaven.com". The clause must START with the # method=result pair; header.d= / header.i= may appear anywhere after it. -_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)", re.IGNORECASE) +# The result token must be terminated by end-of-clause, whitespace, or a +# comment so "dkim=pass-anything" can never be read as "pass". +_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|[\s(])", re.IGNORECASE) _HEADER_D_RE = re.compile(r"header\.d\s*=\s*\"?([^\s\";]+)", re.IGNORECASE) _HEADER_I_RE = re.compile(r"header\.i\s*=\s*\"?([^\s\";]+)", re.IGNORECASE) @@ -51,7 +54,9 @@ _HEADER_I_RE = re.compile(r"header\.i\s*=\s*\"?([^\s\";]+)", re.IGNORECASE) def get_allowed_dkim_domains() -> frozenset: """Read the DKIM-domain allowlist from the environment (may be empty).""" raw = os.environ.get(ALLOWED_DKIM_DOMAINS_ENV, "") - return frozenset(d.strip().lower().lstrip("@") for d in raw.split(",") if d.strip()) + return frozenset( + d.strip().lower().lstrip("@").rstrip(".") for d in raw.split(",") if d.strip() + ) def _unfold(value: str) -> str: @@ -108,7 +113,9 @@ def evaluate_sender_authentication(raw_email: bytes, allowed_domains) -> tuple: # compat32 keeps header values as raw strings (we unfold ourselves) # and never raises on structurally odd headers; headersonly avoids # parsing the body at all. - msg = email.parser.BytesParser().parsebytes(raw_email, headersonly=True) + msg = email.parser.BytesParser(policy=email.policy.compat32).parsebytes( + raw_email, headersonly=True + ) except Exception: return False, "unparseable_message", {} diff --git a/tests/test_ses_auth.py b/tests/test_ses_auth.py index 8c3337b..86ad49d 100644 --- a/tests/test_ses_auth.py +++ b/tests/test_ses_auth.py @@ -82,6 +82,32 @@ class TestParseAuthenticationResults: assert authserv_id == "" assert passing == frozenset() + def test_result_token_boundary(self, ses_auth): + # "pass-anything" / "passfail" must never be read as "pass". + for result in ("pass-fake", "passfail"): + _, passing = ses_auth.parse_authentication_results( + f"amazonses.com; dkim={result} header.i=@seahaven.com" + ) + assert passing == frozenset() + + def test_result_followed_by_comment(self, ses_auth): + _, passing = ses_auth.parse_authentication_results( + "amazonses.com; dkim=pass(good signature) header.i=@seahaven.com" + ) + assert passing == frozenset({"seahaven.com"}) + + def test_quoted_domain_value(self, ses_auth): + _, passing = ses_auth.parse_authentication_results( + 'amazonses.com; dkim=pass header.i="@seahaven.com"' + ) + assert passing == frozenset({"seahaven.com"}) + + def test_folding_inside_dkim_clause(self, ses_auth): + _, passing = ses_auth.parse_authentication_results( + "amazonses.com;\r\n dkim=pass\r\n header.i=@seahaven.com" + ) + assert passing == frozenset({"seahaven.com"}) + class TestEvaluateSenderAuthentication: def test_ses_stamped_pass_accepted(self, ses_auth): @@ -189,7 +215,8 @@ class TestAuthenticateInboundEmail: assert ses_auth.authenticate_inbound_email(raw(WO_SES_HEADER), self.S3_KEY) def test_allowlist_is_comma_separated_and_normalized(self, ses_auth, monkeypatch): - monkeypatch.setenv("ALLOWED_DKIM_DOMAINS", " Other.Example , @SEAHAVEN.com ,") + # Stray spaces, case, a leading @, and a trailing dot all normalize. + monkeypatch.setenv("ALLOWED_DKIM_DOMAINS", " Other.Example , @SEAHAVEN.com. ,") assert ses_auth.authenticate_inbound_email(raw(WO_SES_HEADER), self.S3_KEY) def test_env_var_unset_fails_closed(self, ses_auth, monkeypatch, caplog):