fix(be): tolerate gateway leaving line terminator on DKIM header values#3939
Merged
Conversation
The beta SMTP gateway hands the canister header values with the structural CRLF still attached (e.g. `Subject` arrives as " II-Recovery-…\r\n"). Per RFC 5322 §2.2 the CRLF that terminates a header line is framing, not field-body — the gateway should be stripping it during parsing. Today every DKIM signature lands as SignatureInvalid because our hash input ends with `…\r\n\r\n` instead of `…\r\n`. Defensively strip a trailing line terminator in `relaxed_header_value` so DKIM verification works against the current gateway. Marked FIXME(gateway) — once the gateway parser is fixed to strip the CRLF on its side this workaround can be removed and `is_wsp` left at its strict RFC 6376 §2.8 SP/HTAB definition. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a defensive workaround in the DKIM relaxed header canonicalization to tolerate an SMTP gateway bug where header values incorrectly include their trailing line terminator (\r\n / \r / \n), which currently causes DKIM verification to fail with SignatureInvalid.
Changes:
- Strip trailing
\r/\nfrom header values before applying relaxed canonicalization steps. - Add unit tests covering trailing terminator stripping (including with trailing WSP, bare
\n, and folded values).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
✅ No security or compliance issues detected. Reviewed everything up to 2d67428. Security Overview
Detected Code Changes| Change Type | Relevant files ... (code changes summary truncated to fit VCS comment limits.) |
aterga
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DKIM signature verification on beta currently fails every well-formed inbound message with
SignatureInvalid. We caught this via the newemail_recovery_logsquery (#3934) — two separate users tried to bind recovery emails over the DoH/gmail path and both hit identicaldoh verify_failed err=EmailVerificationFailed("SignatureInvalid")log lines. Inspecting a capturedSmtpRequestshowed every header value arriving with its terminating CRLF attached (e.g.Subjectas" II-Recovery-…\r\n").Per RFC 5322 §2.2 the CRLF that ends a header line is structural framing, not field-body. The gateway should be stripping it during parsing. Today's beta gateway leaves it attached, which makes our DKIM hash input end with
…\r\n\r\ninstead of…\r\n—relaxed_header_valueadds its own line terminator on top of the value's leftover one — so SHA-256(hash_input) never matches what Gmail signed.This is a defensive workaround on the canister side until the gateway is fixed:
relaxed_header_valuenow strips any trailing\r/\nfrom the input value before running the rest of the relaxed canonicalization pipeline. The trim is intentionally narrow — it pops only structural CR/LF, not whitespace inside the value — sois_wspstays at the RFC 6376 §2.8 SP/HTAB definition. MarkedFIXME(gateway)with a pointer to the right long-term fix.Changes
dkim/canonicalize.rs: strip trailing\r\n(and bare\r/\n) from header values before relaxed canonicalization. Comment marks it as a temporary tolerance for the gateway parser bug.Tests
relaxed_header_strips_trailing_line_terminator— value ending in\r\nno longer produces a double-CRLF canonical form.relaxed_header_strips_trailing_terminator_with_wsp— value ending inSP HTAB \r\nis fully cleaned.relaxed_header_strips_bare_trailing_lf— bare trailing\n(no\r) is also tolerated.relaxed_header_folded_value_with_trailing_terminator— Gmail-shaped folded DKIM-Signature value still unfolds correctly even with a trailing terminator.All 108 existing
dkim::*unit tests continue to pass.