adam/procurement-ingest
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/procurement-ingest.git synced 2026-09-30 06:03:14 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Harden AR parsing and alarm on sender-auth rejects

Browse source 
The SES-stamped Authentication-Results value echoes attacker-controlled
SMTP-session tokens (envelope-from, helo, header.from) as their own
semicolon-delimited property clauses. A naive split(";") tore an RFC 5321
quoted-local-part MAIL FROM apart and manufactured a forged dkim=pass
clause, so a fully spoofed email was accepted on the genuinely
SES-stamped topmost header. Tokenise comment- and quoted-string-aware
(RFC 8601 / RFC 5322): strip CFWS comments, split clauses only on
semicolons outside a quoted-string, and fail closed on unbalanced
quotes/comments so a ';' inside a quoted pvalue can never start a clause.

Rejected mail returns normally (no error, no retry, no DLQ message), so a
signing-domain drift or a wrong allowlist would silently discard 100% of
legitimate mail while every alarm stayed green. Add a CloudWatch Logs
metric filter + alarm on the sender_auth_rejected warning to both stacks
so a false-reject storm pages instead of vanishing. This is also the
safety net for the WO seahaven.com allowlist assumption, which must be
validated against a live SES-stamped header (a plain Gmail auto-forward
re-signs under the sending Workspace domain, not seahaven.com).

Refs: INFRA-107
This commit is contained in:
Adam Moussa 2026-07-15 19:13:55 -04:00
parent fa5f60208f
commit 7c74ac1761
No known key found for this signature in database
6 changed files with 509 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
README.md
View file

@ -47,7 +47,7 @@ Amazon APM work order emails (from Hexagon EAM / HxGN SmartCloud) are received a
2. Gmail filter forwards APM emails to `apm@int.seahaven.com` (SES).
3. SES drops the raw MIME into `s3://workorder-ingest-emails-{AccountId}/inbound/`.
4. S3 triggers the `workorder-email-processor` Lambda.
5. Fail-closed sender authentication (INFRA-107): the SES-stamped `Authentication-Results` header must show `dkim=pass` for `seahaven.com` (the Google Groups forward re-signs the mail; see [Sender authentication](#sender-authentication-infra-107)); otherwise the email is logged and dropped.
5. Fail-closed sender authentication (INFRA-107): the SES-stamped `Authentication-Results` header must show `dkim=pass` for the domain that re-signs the forward (currently allowlisted as `seahaven.com` — see the validation caveat under [Sender authentication](#sender-authentication-infra-107)); otherwise the email is logged and dropped.
6. Claude extracts structured JSON (work order ID, site code, severity, priority, dates, assigned technician).
7. Work order upserted to `WorkOrders`, event/comment appended to `WorkOrderComments`.
@ -85,10 +85,12 @@ The allowlist is the `ALLOWED_DKIM_DOMAINS` Lambda environment variable (comma-s
| Pipeline | `ALLOWED_DKIM_DOMAINS` | Why |
|---|---|---|
| Work orders | `seahaven.com` | APM mail arrives via the `apm@` Google Groups forward, which re-signs as `seahaven.com` (the original `hxgnsmartcloud.com` signature does not survive the forward) |
| Work orders | `seahaven.com` | APM mail reaches `apm@int.seahaven.com` via a forward off `amazon@seahavenind.com`; the allowlist trusts the domain that **re-signs** DKIM on that forward (the original `hxgnsmartcloud.com` signature does not survive it). **Must be validated against a live SES-stamped header** — a plain Gmail auto-forward re-signs under the *sending Workspace* domain (`seahavenind.com` / a `*.gappssmtp.com` key), **not** `seahaven.com`; only a Google Group (or Workspace routing) with "sign as `seahaven.com`" produces `dkim=pass header.i=@seahaven.com`. If the observed re-signing domain differs, update this value (do **not** widen it to a shared key like `*.gappssmtp.com`, which any Google customer's mail would pass). The `workorder-email-processor-sender-auth-rejected` alarm pages if this assumption is wrong instead of silently dropping every work order. |
| Purchase orders | `amazon.coupahost.com` | Coupa signs as `amazon.coupahost.com`. `amazonses.com` also passes but is deliberately not allowlisted — every SES customer's mail passes for it |
On any failure (env var unset, header missing/unparseable, verdict fail, unaligned domain) the processor logs a structured `sender_auth_rejected` warning with the reason and S3 key, skips the email, and returns normally — rejected mail never triggers Lambda retries or DLQ messages. Unit tests live in `tests/test_ses_auth.py`.
> **Clause-injection hardening:** SES echoes attacker-controlled SMTP-session tokens (`envelope-from`, `helo`, `header.from`) into its own `Authentication-Results` value, and an RFC 5321 quoted-local-part MAIL FROM may legally contain `;` and spaces. The parser therefore tokenises comment- and quoted-string-aware (RFC 8601 / RFC 5322): CFWS comments `(...)` are stripped and clauses are split only on semicolons **outside** a quoted string, so a `;` inside a quoted `envelope-from=` value can never be torn into a forged `dkim=pass` clause.
On any failure (env var unset, header missing/unparseable, verdict fail, unaligned domain) the processor logs a structured `sender_auth_rejected` warning with the reason and S3 key, skips the email, and returns normally — rejected mail never triggers Lambda retries or DLQ messages, but the `<fn>-sender-auth-rejected` CloudWatch alarm (see [CloudWatch alarms](#cloudwatch-alarms)) pages on a rejection spike so a drift-induced outage is not silent. Unit tests live in `tests/test_ses_auth.py`.
**Failure handling (INFRA-41):** Each email-processor is async-invoked (S3 → Lambda). Both have a CDK-managed SQS dead-letter queue (`dead_letter_queue=`, 14-day retention, SSL-enforced) so a failed parse is captured rather than silently dropped after Lambda's retries.
@ -103,6 +105,9 @@ Every alarm is **ALARM-only** (no OK action), sends to the shared `site-alerts`
| `<fn>-errors` | `po-email-processor`, `po-ingest-site-extractor`, `workorder-email-processor` | `Errors` Sum, 5 min, `> 0`, eval 1 |
| `<fn>-throttles` | `po-email-processor`, `po-ingest-site-extractor`, `po-web-ui`, `workorder-email-processor` | `Throttles` Sum, 5 min, `> 0`, eval 1 |
| `<fn>-duration` | `po-email-processor`, `po-ingest-site-extractor`, `po-web-ui` (p99); `workorder-email-processor` (p95) | `Duration` percentile, 5 min, `>= 45000` ms (75% of the 60s timeout), eval 3 / datapoints 2 |
| `<fn>-sender-auth-rejected` | `po-email-processor`, `workorder-email-processor` | Log-metric-filter count (namespace `Seahaven/ProcurementIngest`, `default_value=0`) on `sender_auth_rejected` warnings, `Sum` 15 min, `>= 3`, eval 1 |
The `<fn>-sender-auth-rejected` alarm closes the silent-drop gap in INFRA-107: a rejected email returns normally (no error, no retry, no DLQ message), so without a log-metric filter a signing-domain drift or a wrong allowlist would discard 100% of legitimate mail while every other alarm stayed green. `>= 3` rejections in 15 minutes tolerates the occasional spoofed probe to the internal ingest address but pages quickly on a genuine false-reject storm; the threshold is easy to tune in the CDK helper.
The `<fn>-duration` and `<fn>-throttles` alarms for `po-email-processor` and `workorder-email-processor` supersede the orphaned, CLI-created `Lambda-Duration-*` / `Lambda-Throttles-*` alarms (deleted post-deploy).

75
cdk/po_stack.py
View file

@ -77,6 +77,67 @@ def _add_ddb_alarms(scope, id_prefix, table, alarm_name_prefix, alarm_topic):
).add_alarm_action(cw_actions.SnsAction(alarm_topic))
# CloudWatch namespace for the log-derived sender-authentication metrics.
_SENDER_AUTH_METRIC_NAMESPACE = "Seahaven/ProcurementIngest"
def _add_sender_auth_rejected_alarm(scope, id_prefix, function_name, alarm_topic):
"""Metric-filter + alarm on ``sender_auth_rejected`` warnings (INFRA-107).
A rejected inbound email is skipped without erroring the invocation, so it
is invisible to the Errors/Throttles/DLQ alarms. This turns the structured
warning log into a CloudWatch metric and pages when rejections spike --
catching a silent false-reject storm (allowlist wrong, signing-domain
drift, SES header-format change) that would otherwise discard legitimate
mail while the pipeline reports healthy.
ALARM-only SnsAction to site-alerts; no OK action. The metric filter reads
the function's own log group (imported by the deterministic
``/aws/lambda/<fn>`` name, created by the function's log_retention). A plain
substring pattern is used because Lambda prefixes each line with its own
level/timestamp/request-id, so the JSON payload is not a standalone JSON
log event a `{$.event=...}` pattern could match.
"""
metric_name = f"{function_name}-sender-auth-rejected"
logs.MetricFilter(
scope,
f"{id_prefix}SenderAuthRejectedFilter",
log_group=logs.LogGroup.from_log_group_name(
scope,
f"{id_prefix}LogGroup",
f"/aws/lambda/{function_name}",
),
filter_pattern=logs.FilterPattern.literal('"sender_auth_rejected"'),
metric_namespace=_SENDER_AUTH_METRIC_NAMESPACE,
metric_name=metric_name,
metric_value="1",
default_value=0,
)
# >=3 rejections in a 15-min window pages: a lone spoofed probe to the
# (obscure, internal) ingest address is tolerated, but a genuine
# false-reject storm -- where legitimate senders are being dropped -- trips
# quickly. Threshold is intentionally conservative and easy to tune.
cloudwatch.Metric(
namespace=_SENDER_AUTH_METRIC_NAMESPACE,
metric_name=metric_name,
period=Duration.minutes(15),
statistic="Sum",
).create_alarm(
scope,
f"{id_prefix}SenderAuthRejectedAlarm",
alarm_name=f"{function_name}-sender-auth-rejected",
alarm_description=(
f"{function_name} rejected inbound mail on sender authentication "
"(possible allowlist/DKIM-domain drift silently dropping real mail)"
),
threshold=3,
evaluation_periods=1,
comparison_operator=cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
).add_alarm_action(cw_actions.SnsAction(alarm_topic))
class PoIngestStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs):
super().__init__(scope, construct_id, **kwargs)
@ -218,6 +279,20 @@ class PoIngestStack(Stack):
treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
).add_alarm_action(cw_actions.SnsAction(alarm_topic))
# --- Sender-auth rejection alarm (INFRA-107) ---
# A rejected email (bad/unaligned DKIM verdict) returns normally, so it
# produces NO Lambda error, NO DLQ message and NO retry -- only a
# `sender_auth_rejected` warning log. Without this metric filter + alarm a
# domain drift (Coupa rotates its signing subdomain, SES changes its
# Authentication-Results format, the allowlist is wrong) would silently
# discard 100% of legitimate PO mail while every other alarm stays green.
# A CloudWatch Logs metric filter turns those warnings into a metric so a
# false-reject storm pages instead of vanishing. default_value=0 keeps the
# series populated (alarm stays OK, never INSUFFICIENT_DATA) between events.
_add_sender_auth_rejected_alarm(
self, "EmailProcessor", "po-email-processor", alarm_topic
)
# --- Throttles alarm: po-email-processor ---
# Any throttled invocation (concurrency cap hit) in a 5-min window pages.
# ALARM-only to site-alerts; no OK action; NOT_BREACHING when no data.

77
cdk/wo_stack.py
View file

@ -76,6 +76,70 @@ def _add_ddb_alarms(scope, id_prefix, table, alarm_name_prefix, alarm_topic):
).add_alarm_action(cw_actions.SnsAction(alarm_topic))
# CloudWatch namespace for the log-derived sender-authentication metrics.
_SENDER_AUTH_METRIC_NAMESPACE = "Seahaven/ProcurementIngest"
def _add_sender_auth_rejected_alarm(scope, id_prefix, function_name, alarm_topic):
"""Metric-filter + alarm on ``sender_auth_rejected`` warnings (INFRA-107).
A rejected inbound email is skipped without erroring the invocation, so it
is invisible to the Errors/Throttles/DLQ alarms. This turns the structured
warning log into a CloudWatch metric and pages when rejections spike --
catching a silent false-reject storm (allowlist wrong, signing-domain
drift, SES header-format change) that would otherwise discard legitimate
mail while the pipeline reports healthy. This is the safety net for the WO
allowlist domain assumption (seahaven.com) -- if the real Gmail-forward
re-signing domain differs, this alarm surfaces it instead of a silent
work-order outage.
ALARM-only SnsAction to site-alerts; no OK action. The metric filter reads
the function's own log group (imported by the deterministic
``/aws/lambda/<fn>`` name, created by the function's log_retention). A plain
substring pattern is used because Lambda prefixes each line with its own
level/timestamp/request-id, so the JSON payload is not a standalone JSON
log event a `{$.event=...}` pattern could match.
"""
metric_name = f"{function_name}-sender-auth-rejected"
logs.MetricFilter(
scope,
f"{id_prefix}SenderAuthRejectedFilter",
log_group=logs.LogGroup.from_log_group_name(
scope,
f"{id_prefix}LogGroup",
f"/aws/lambda/{function_name}",
),
filter_pattern=logs.FilterPattern.literal('"sender_auth_rejected"'),
metric_namespace=_SENDER_AUTH_METRIC_NAMESPACE,
metric_name=metric_name,
metric_value="1",
default_value=0,
)
# >=3 rejections in a 15-min window pages: a lone spoofed probe to the
# (obscure, internal) ingest address is tolerated, but a genuine
# false-reject storm -- where legitimate senders are being dropped -- trips
# quickly. Threshold is intentionally conservative and easy to tune.
cloudwatch.Metric(
namespace=_SENDER_AUTH_METRIC_NAMESPACE,
metric_name=metric_name,
period=Duration.minutes(15),
statistic="Sum",
).create_alarm(
scope,
f"{id_prefix}SenderAuthRejectedAlarm",
alarm_name=f"{function_name}-sender-auth-rejected",
alarm_description=(
f"{function_name} rejected inbound mail on sender authentication "
"(possible allowlist/DKIM-domain drift silently dropping real mail)"
),
threshold=3,
evaluation_periods=1,
comparison_operator=cloudwatch.ComparisonOperator.GREATER_THAN_OR_EQUAL_TO_THRESHOLD,
treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
).add_alarm_action(cw_actions.SnsAction(alarm_topic))
class WorkorderIngestStack(Stack):
def __init__(self, scope: Construct, construct_id: str, **kwargs):
super().__init__(scope, construct_id, **kwargs)
@ -236,6 +300,19 @@ class WorkorderIngestStack(Stack):
treat_missing_data=cloudwatch.TreatMissingData.NOT_BREACHING,
).add_alarm_action(cw_actions.SnsAction(alarm_topic))
# --- Sender-auth rejection alarm (INFRA-107) ---
# A rejected email (bad/unaligned DKIM verdict) returns normally, so it
# produces NO Lambda error, NO DLQ message and NO retry -- only a
# `sender_auth_rejected` warning log. The WO allowlist trusts dkim=pass
# for seahaven.com on the assumption the apm@ forward re-signs there; if
# that assumption is wrong (e.g. a Gmail auto-forward re-signs under a
# different domain), 100% of legitimate work-order mail is silently
# dropped. This metric filter + alarm turns those warnings into a paging
# signal so a false-reject storm surfaces instead of a silent outage.
_add_sender_auth_rejected_alarm(
self, "EmailProcessor", "workorder-email-processor", alarm_topic
)
# --- Throttles alarm: workorder-email-processor ---
# Any throttled invocation (concurrency cap hit) in a 5-min window pages.
# ALARM-only to site-alerts; no OK action; NOT_BREACHING when no data.

136
lambdas/po/email_processor/ses_auth.py
View file

@ -27,6 +27,25 @@ Observed SES format (2026-07-15, both ingest buckets)::
Note SES reports the passing DKIM identity as ``header.i=@<domain>``
(RFC 6376 AUID), not ``header.d=``; the parser accepts both.
**Clause injection defence (INFRA-107 hardening).** SES echoes several
attacker-controlled SMTP-session tokens into its own Authentication-Results
value as their own semicolon-delimited property clauses -- notably
``envelope-from=<MAIL FROM>``, ``helo=<HELO>`` and ``header.from=<From
domain>``. An RFC 5321 quoted-local-part MAIL FROM may legally contain
spaces and semicolons, e.g.::
MAIL FROM:<"x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com>
which SES renders verbatim as ``envelope-from="x; dkim=pass
header.i=@amazon.coupahost.com"@attacker.com;``. A naive ``split(";")``
would tear the quoted string apart and manufacture a synthetic
``dkim=pass header.i=@amazon.coupahost.com`` clause out of attacker input.
The parser therefore tokenises per RFC 8601 / RFC 5322 structure: CFWS
comments ``(...)`` are stripped first, and the value is split into clauses
only on semicolons that sit *outside* a quoted-string. A ``;`` inside a
quoted ``pvalue`` stays part of that one property clause and can never be
read as the start of a ``dkim=`` methodspec.
"""
import email.parser
@ -44,9 +63,11 @@ 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.
# 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)
# The result token must be terminated by end-of-clause or whitespace so
# "dkim=pass-anything" can never be read as "pass". (Comments are stripped
# before matching, so the pre-strip "dkim=pass(comment)" form arrives here
# as "dkim=pass ..." and still terminates on whitespace.)
_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)
@ -64,15 +85,120 @@ def _unfold(value: str) -> str:
return re.sub(r"[\r\n\t ]+", " ", value).strip()
def _strip_comments(value: str) -> tuple:
"""Remove RFC 5322 CFWS comments ``(...)`` from an unfolded header value.
Comments may nest and may contain quoted pairs (``\\)``). A ``(`` that
appears *inside* a quoted-string is literal text, not a comment start,
so quoted-strings (which carry attacker-controlled ``pvalue`` content
such as a quoted MAIL FROM local part) are passed through untouched.
Dropping comments first means comment-embedded ``header.i=`` / ``;``
fakes can never influence clause splitting or domain extraction.
Returns ``(stripped_text, well_formed)``. ``well_formed`` is False when
the value ends inside an unterminated comment or quoted-string, i.e. the
parens/quotes are unbalanced. Callers reject on ``not well_formed`` so a
malformed header (which could otherwise be mis-tokenised) fails closed
rather than being partially parsed.
"""
out = []
depth = 0 # comment nesting depth
in_quote = False # inside a quoted-string (only tracked at depth 0)
i = 0
n = len(value)
while i < n:
c = value[i]
if depth > 0:
# Inside a comment: only quoted-pairs and nested parens matter.
if c == "\\":
i += 2
continue
if c == "(":
depth += 1
elif c == ")":
depth -= 1
i += 1
continue
if in_quote:
out.append(c)
if c == "\\" and i + 1 < n:
out.append(value[i + 1])
i += 2
continue
if c == '"':
in_quote = False
i += 1
continue
# Normal context (outside any comment or quoted-string).
if c == "(":
depth += 1
i += 1
continue
if c == '"':
in_quote = True
out.append(c)
i += 1
well_formed = depth == 0 and not in_quote
return "".join(out), well_formed
def _split_clauses(value: str) -> list:
"""Split a comment-free header value into clauses on top-level ``;``.
A semicolon inside a quoted-string is preserved as part of the clause,
so an attacker-controlled quoted ``pvalue`` (e.g. a quoted MAIL FROM
echoed into ``envelope-from=``) cannot smuggle in a fake ``dkim=pass``
clause. Callers must run :func:`_strip_comments` first.
"""
clauses = []
buf = []
in_quote = False
i = 0
n = len(value)
while i < n:
c = value[i]
if in_quote:
buf.append(c)
if c == "\\" and i + 1 < n:
buf.append(value[i + 1])
i += 2
continue
if c == '"':
in_quote = False
i += 1
continue
if c == '"':
in_quote = True
buf.append(c)
i += 1
continue
if c == ";":
clauses.append("".join(buf))
buf = []
i += 1
continue
buf.append(c)
i += 1
clauses.append("".join(buf))
return clauses
def parse_authentication_results(value: str) -> tuple:
"""Parse one Authentication-Results header value.
Returns ``(authserv_id, passing_dkim_domains)`` where the domains are
the lowercased d=/i= domains of every ``dkim=pass`` clause. Malformed
input yields ``("", frozenset())``, which callers treat as a rejection.
Tokenisation is comment- and quoted-string-aware (RFC 8601 / RFC 5322)
so attacker-controlled tokens SES echoes into its header (envelope-from,
helo, header.from) cannot be split into a forged ``dkim=pass`` clause.
"""
text = _unfold(value)
clauses = [c.strip() for c in text.split(";")]
text, well_formed = _strip_comments(_unfold(value))
if not well_formed:
# Unbalanced quotes/comments: refuse to guess how to tokenise it.
return "", frozenset()
clauses = [c.strip() for c in _split_clauses(text)]
if not clauses or not clauses[0]:
return "", frozenset()

136
lambdas/wo/email_processor/ses_auth.py
View file

@ -27,6 +27,25 @@ Observed SES format (2026-07-15, both ingest buckets)::
Note SES reports the passing DKIM identity as ``header.i=@<domain>``
(RFC 6376 AUID), not ``header.d=``; the parser accepts both.
**Clause injection defence (INFRA-107 hardening).** SES echoes several
attacker-controlled SMTP-session tokens into its own Authentication-Results
value as their own semicolon-delimited property clauses -- notably
``envelope-from=<MAIL FROM>``, ``helo=<HELO>`` and ``header.from=<From
domain>``. An RFC 5321 quoted-local-part MAIL FROM may legally contain
spaces and semicolons, e.g.::
MAIL FROM:<"x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com>
which SES renders verbatim as ``envelope-from="x; dkim=pass
header.i=@amazon.coupahost.com"@attacker.com;``. A naive ``split(";")``
would tear the quoted string apart and manufacture a synthetic
``dkim=pass header.i=@amazon.coupahost.com`` clause out of attacker input.
The parser therefore tokenises per RFC 8601 / RFC 5322 structure: CFWS
comments ``(...)`` are stripped first, and the value is split into clauses
only on semicolons that sit *outside* a quoted-string. A ``;`` inside a
quoted ``pvalue`` stays part of that one property clause and can never be
read as the start of a ``dkim=`` methodspec.
"""
import email.parser
@ -44,9 +63,11 @@ 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.
# 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)
# The result token must be terminated by end-of-clause or whitespace so
# "dkim=pass-anything" can never be read as "pass". (Comments are stripped
# before matching, so the pre-strip "dkim=pass(comment)" form arrives here
# as "dkim=pass ..." and still terminates on whitespace.)
_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)
@ -64,15 +85,120 @@ def _unfold(value: str) -> str:
return re.sub(r"[\r\n\t ]+", " ", value).strip()
def _strip_comments(value: str) -> tuple:
"""Remove RFC 5322 CFWS comments ``(...)`` from an unfolded header value.
Comments may nest and may contain quoted pairs (``\\)``). A ``(`` that
appears *inside* a quoted-string is literal text, not a comment start,
so quoted-strings (which carry attacker-controlled ``pvalue`` content
such as a quoted MAIL FROM local part) are passed through untouched.
Dropping comments first means comment-embedded ``header.i=`` / ``;``
fakes can never influence clause splitting or domain extraction.
Returns ``(stripped_text, well_formed)``. ``well_formed`` is False when
the value ends inside an unterminated comment or quoted-string, i.e. the
parens/quotes are unbalanced. Callers reject on ``not well_formed`` so a
malformed header (which could otherwise be mis-tokenised) fails closed
rather than being partially parsed.
"""
out = []
depth = 0 # comment nesting depth
in_quote = False # inside a quoted-string (only tracked at depth 0)
i = 0
n = len(value)
while i < n:
c = value[i]
if depth > 0:
# Inside a comment: only quoted-pairs and nested parens matter.
if c == "\\":
i += 2
continue
if c == "(":
depth += 1
elif c == ")":
depth -= 1
i += 1
continue
if in_quote:
out.append(c)
if c == "\\" and i + 1 < n:
out.append(value[i + 1])
i += 2
continue
if c == '"':
in_quote = False
i += 1
continue
# Normal context (outside any comment or quoted-string).
if c == "(":
depth += 1
i += 1
continue
if c == '"':
in_quote = True
out.append(c)
i += 1
well_formed = depth == 0 and not in_quote
return "".join(out), well_formed
def _split_clauses(value: str) -> list:
"""Split a comment-free header value into clauses on top-level ``;``.
A semicolon inside a quoted-string is preserved as part of the clause,
so an attacker-controlled quoted ``pvalue`` (e.g. a quoted MAIL FROM
echoed into ``envelope-from=``) cannot smuggle in a fake ``dkim=pass``
clause. Callers must run :func:`_strip_comments` first.
"""
clauses = []
buf = []
in_quote = False
i = 0
n = len(value)
while i < n:
c = value[i]
if in_quote:
buf.append(c)
if c == "\\" and i + 1 < n:
buf.append(value[i + 1])
i += 2
continue
if c == '"':
in_quote = False
i += 1
continue
if c == '"':
in_quote = True
buf.append(c)
i += 1
continue
if c == ";":
clauses.append("".join(buf))
buf = []
i += 1
continue
buf.append(c)
i += 1
clauses.append("".join(buf))
return clauses
def parse_authentication_results(value: str) -> tuple:
"""Parse one Authentication-Results header value.
Returns ``(authserv_id, passing_dkim_domains)`` where the domains are
the lowercased d=/i= domains of every ``dkim=pass`` clause. Malformed
input yields ``("", frozenset())``, which callers treat as a rejection.
Tokenisation is comment- and quoted-string-aware (RFC 8601 / RFC 5322)
so attacker-controlled tokens SES echoes into its header (envelope-from,
helo, header.from) cannot be split into a forged ``dkim=pass`` clause.
"""
text = _unfold(value)
clauses = [c.strip() for c in text.split(";")]
text, well_formed = _strip_comments(_unfold(value))
if not well_formed:
# Unbalanced quotes/comments: refuse to guess how to tokenise it.
return "", frozenset()
clauses = [c.strip() for c in _split_clauses(text)]
if not clauses or not clauses[0]:
return "", frozenset()

87
tests/test_ses_auth.py
View file

@ -108,6 +108,73 @@ class TestParseAuthenticationResults:
)
assert passing == frozenset({"seahaven.com"})
def test_quoted_semicolon_in_envelope_from_is_not_split(self, ses_auth):
# RFC 5321 quoted-local-part MAIL FROM can carry ';' and spaces; SES
# echoes it into envelope-from=. The ';' inside the quotes must stay
# part of the one envelope-from clause, never a synthetic dkim clause.
value = (
"amazonses.com; spf=pass client-ip=1.2.3.4;"
' envelope-from="x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com;'
" helo=mail.attacker.com; dkim=fail header.i=@attacker.com;"
)
_, passing = ses_auth.parse_authentication_results(value)
assert passing == frozenset()
def test_quoted_pair_in_envelope_from_is_not_split(self, ses_auth):
# A quoted-pair (\") inside the quoted local part must not prematurely
# end the quoted-string and re-expose the smuggled tokens.
value = (
'amazonses.com; envelope-from="a\\"; dkim=pass header.d=seahaven.com"@evil.com;'
" dkim=fail header.i=@evil.com;"
)
_, passing = ses_auth.parse_authentication_results(value)
assert passing == frozenset()
def test_helo_injection_is_not_split(self, ses_auth):
# helo= is attacker-influenced too; a crafted value quoting a fake
# methodspec must not manufacture a passing clause.
value = (
'amazonses.com; helo="h; dkim=pass header.i=@seahaven.com";'
" dkim=fail header.i=@attacker.com;"
)
_, passing = ses_auth.parse_authentication_results(value)
assert passing == frozenset()
def test_comment_embedded_header_i_is_ignored(self, ses_auth):
# A CFWS comment carrying a fake header.i must be stripped before
# domain extraction, so only the real (failing) result is considered.
_, passing = ses_auth.parse_authentication_results(
"amazonses.com; dkim=fail (header.i=@seahaven.com) header.i=@attacker.com"
)
assert passing == frozenset()
def test_comment_hiding_semicolon_does_not_split(self, ses_auth):
# A ';' inside a comment is not a clause separator either.
_, passing = ses_auth.parse_authentication_results(
"amazonses.com; spf=pass (note: a; b) client-ip=1.2.3.4;"
" dkim=pass header.i=@seahaven.com"
)
assert passing == frozenset({"seahaven.com"})
def test_unbalanced_quote_fails_closed(self, ses_auth):
# An unterminated quoted-string is malformed; refuse to parse it so a
# dkim=pass clause "swallowed" by the runaway quote can't be salvaged
# (and, conversely, a runaway quote can't be used to mis-tokenise).
authserv_id, passing = ses_auth.parse_authentication_results(
'amazonses.com; envelope-from="oops@attacker.com;'
" dkim=pass header.i=@seahaven.com"
)
assert authserv_id == ""
assert passing == frozenset()
def test_unbalanced_comment_fails_closed(self, ses_auth):
# An unterminated comment is malformed and must fail closed.
authserv_id, passing = ses_auth.parse_authentication_results(
"amazonses.com; dkim=pass header.i=@seahaven.com (runaway comment"
)
assert authserv_id == ""
assert passing == frozenset()
class TestEvaluateSenderAuthentication:
def test_ses_stamped_pass_accepted(self, ses_auth):
@ -206,6 +273,26 @@ class TestEvaluateSenderAuthentication:
assert not accepted
assert reason == "allowlist_not_configured"
def test_envelope_from_clause_injection_rejected(self, ses_auth):
# Full forged email: attacker's quoted MAIL FROM is echoed by SES into
# its own (topmost, genuinely SES-stamped) Authentication-Results as
# envelope-from=, trying to smuggle a dkim=pass for an allowlisted
# domain. The real DKIM verdict is fail. Must fail closed.
injected = (
"Authentication-Results: amazonses.com;\r\n"
" spf=pass (spfCheck: domain of attacker.com designates 1.2.3.4 as"
" permitted sender) client-ip=1.2.3.4;\r\n"
' envelope-from="x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com;'
" helo=mail.attacker.com;\r\n"
" dkim=fail header.i=@attacker.com;\r\n"
" dmarc=fail header.from=attacker.com;\r\n"
)
accepted, reason, _ = ses_auth.evaluate_sender_authentication(
raw(injected), {"amazon.coupahost.com", "seahaven.com"}
)
assert not accepted
assert reason == "no_passing_dkim_signature"
class TestAuthenticateInboundEmail:
S3_KEY = "s3://bucket/inbound/abc123"