Skip to content

Commit b30fd7c

Browse files
sea-snakeatergaclaude
authored
feat(dmarc): RFC 7489 alignment + combined DKIM+DMARC verifier (PR 3 of email-recovery stack) (dfinity#3878)
## Summary PR 3 of the email-recovery stack (`docs/ongoing/email-recovery.md` §6). Stacks on top of dfinity#3877 (DKIM verifier). Lands a hand-rolled DMARC alignment check and reshapes the verifier API: `dkim::verify_dkim` becomes a DKIM-only primitive, and the new `dmarc::verify_email` is the public top-level entry point that produces the combined `EmailVerificationStatus`. **Note:** This PR targets `main` but includes PRs 1+2's commits as its base. Review the DMARC-specific changes by looking at commits on top of `ec371aae3` (PR 2's tip). Once PRs 1+2 merge, this PR's diff shrinks to just the DMARC additions. ## What's in this PR ### `src/internet_identity/src/dmarc/` - **`types.rs`** — `DmarcOutcome` (Aligned / Misaligned / NoRecord / Malformed), `DmarcPolicy` (None / Quarantine / Reject), `AlignmentMode` (Strict / Relaxed), `DmarcRecord`, plus the combined `EmailVerificationStatus` that carries both DKIM diagnostics and the DMARC outcome on success. - **`parse.rs`** (RFC 7489 §6.3) — DMARC TXT record parser. Enforces `v=DMARC1` must be first, `p=` must be one of {none, quarantine, reject}, `pct=` 0..=100, rejects duplicate tags, ignores unknown / reporting tags. 12 unit tests. - **`from_header.rs`** (RFC 5322 / RFC 7489 §3.1.1) — single-mailbox From-header parser. Accepts bare addr-spec, name-addr, and quoted-display-name forms; rejects zero/multiple From: headers, address-lists, group syntax. Tolerates comma/colon inside quoted display names. 16 unit tests. - **`alignment.rs`** — strict (exact match) + relaxed (exact match OR label-aligned subdomain in either direction). Stricter than RFC-compliant relaxed alignment because we deliberately don't consult the PSL — design doc §6.4 documents the trust + asymmetric-failure-mode reasoning. The dot anchor on the subdomain check prevents `evilexample.com` from aliasing `example.com`. 8 unit tests. - **`verify.rs`** — orchestration. DKIM first; on failure, surface the DKIM reason verbatim. On DKIM pass, parse From and check DMARC alignment. Accepted iff Aligned, OR NoRecord with `dkim_domain == from_domain`. 8 unit tests. - **`test_vectors.rs`** — 5 end-to-end tests reusing PR 2's synthetic .eml fixtures. ### `src/internet_identity/src/dkim/types.rs` (rename + new variants) - Renamed `EmailVerificationStatus` → `DkimVerifyResult` (DKIM-only). The combined verdict moved to `dmarc::EmailVerificationStatus` so it can carry the `DmarcOutcome`. - Added `MalformedFromHeader(String)`, `DmarcMalformed(String)`, `DmarcMisaligned` to `VerificationFailReason`. ### `src/internet_identity/src/dkim/mod.rs` - Re-exports `verify` as `verify_dkim` so downstream callers (the dmarc layer) don't have to deal with both a `dkim::verify` and `dmarc::verify` in scope at the same time. ## Test plan - [x] `cargo check -p internet_identity --target wasm32-unknown-unknown` — clean. - [x] `cargo test -p internet_identity --bin internet_identity dmarc` — 49 tests pass (12 parse + 16 from_header + 8 alignment + 8 verify + 5 e2e). - [x] `cargo test -p internet_identity --bin internet_identity` — 365 tests pass total (was 313 with PR 2; +49 dmarc + 3 small reshape adjustments). - [x] `cargo clippy -p internet_identity --bin internet_identity --tests -- -D warnings` — clean. - [x] `cargo fmt --check` — clean (modulo pre-existing unrelated diffs). ## PR Stack | # | PR | Description | Status | |---|---|---|---| | 0 | [dfinity#3836](dfinity#3836) | Design doc | Open | | 1 | [dfinity#3838](dfinity#3838) | DNSSEC verifier scaffold | Open | | 2 | [dfinity#3877](dfinity#3877) | DKIM verifier (RFC 6376) | Open | | 3 | [dfinity#3878](dfinity#3878) | DMARC alignment (RFC 7489) | Open | | 4 | [dfinity#3879](dfinity#3879) | DoH fallback | Open | | 5+6 | [dfinity#3880](dfinity#3880) | Setup flow (storage + smtp_request) | Open | | 7 | [dfinity#3881](dfinity#3881) | Recovery flow (delegation) | Open | | 8 | [dfinity#3882](dfinity#3882) | Frontend + feature flag | Open | | 9 | [dfinity#3883](dfinity#3883) | Deploy/upgrade scripts: dnssec_config + doh_config | Open | | 10 | [dfinity#3884](dfinity#3884) | Email-recovery UX overhaul | Open | --------- Co-authored-by: Arshavir Ter-Gabrielyan <arshavir.ter.gabrielyan@dfinity.org> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 74ad617 commit b30fd7c

12 files changed

Lines changed: 1446 additions & 36 deletions

File tree

src/internet_identity/src/dkim/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,11 @@ mod verify;
3737

3838
#[allow(unused_imports)]
3939
pub use types::{
40-
Algorithm, BodyCanon, DkimCheck, DkimCheckName, DkimCheckStatus, EmailVerificationStatus,
41-
HeaderCanon, VerificationFailReason,
40+
Algorithm, BodyCanon, DkimCheck, DkimCheckName, DkimCheckStatus, DkimVerifyResult, HeaderCanon,
41+
VerificationFailReason,
4242
};
43+
// Re-exported as `verify_dkim` so downstream callers (the dmarc layer)
44+
// don't have to deal with both a `dkim::verify` and `dmarc::verify`
45+
// in scope at the same time.
4346
#[allow(unused_imports)]
44-
pub use verify::verify;
47+
pub use verify::verify as verify_dkim;

src/internet_identity/src/dkim/test_vectors.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
//! public key) live in `test_vectors/dkim/`. See that directory's
1515
//! README for the regeneration procedure.
1616
17-
use super::types::{EmailVerificationStatus, VerificationFailReason};
17+
use super::types::{DkimVerifyResult, VerificationFailReason};
1818
use super::verify::verify;
1919
use internet_identity_interface::internet_identity::types::smtp::{
2020
SmtpAddress, SmtpEnvelope, SmtpHeader, SmtpMessage, SmtpRequest,
@@ -177,7 +177,7 @@ fn verifies_synthetic_rsa_relaxed_relaxed() {
177177
let req = parse_eml(SYNTH_RSA_RELAXED_RELAXED);
178178
let result = verify(&req, SYNTH_RSA_TXT, frozen_now());
179179
match result {
180-
EmailVerificationStatus::Verified { dkim_domain, .. } => {
180+
DkimVerifyResult::Verified { dkim_domain, .. } => {
181181
assert_eq!(dkim_domain, "test.example.com");
182182
}
183183
other => panic!("expected Verified, got {:?}", other),
@@ -189,7 +189,7 @@ fn verifies_synthetic_rsa_relaxed_simple_body() {
189189
let req = parse_eml(SYNTH_RSA_RELAXED_SIMPLE);
190190
let result = verify(&req, SYNTH_RSA_TXT, frozen_now());
191191
match result {
192-
EmailVerificationStatus::Verified { dkim_domain, .. } => {
192+
DkimVerifyResult::Verified { dkim_domain, .. } => {
193193
assert_eq!(dkim_domain, "test.example.com");
194194
}
195195
other => panic!("expected Verified, got {:?}", other),
@@ -201,7 +201,7 @@ fn rejects_simple_simple_canonicalization() {
201201
let req = parse_eml(SYNTH_RSA_SIMPLE_SIMPLE);
202202
let result = verify(&req, SYNTH_RSA_TXT, frozen_now());
203203
match result {
204-
EmailVerificationStatus::Unverified { reason, .. } => {
204+
DkimVerifyResult::Unverified { reason, .. } => {
205205
assert_eq!(reason, VerificationFailReason::UnsupportedCanonicalization);
206206
}
207207
other => panic!(
@@ -223,7 +223,7 @@ fn rejects_flipped_body_byte() {
223223
message.body = ByteBuf::from(body);
224224
let result = verify(&req, SYNTH_RSA_TXT, frozen_now());
225225
match result {
226-
EmailVerificationStatus::Unverified { reason, .. } => {
226+
DkimVerifyResult::Unverified { reason, .. } => {
227227
assert_eq!(reason, VerificationFailReason::BodyHashMismatch);
228228
}
229229
other => panic!("expected BodyHashMismatch, got {:?}", other),
@@ -257,7 +257,7 @@ fn rejects_flipped_signature_byte() {
257257
assert!(
258258
matches!(
259259
result,
260-
EmailVerificationStatus::Unverified {
260+
DkimVerifyResult::Unverified {
261261
reason: VerificationFailReason::SignatureInvalid
262262
| VerificationFailReason::SignatureMalformed(_)
263263
| VerificationFailReason::BodyHashMismatch,
@@ -279,7 +279,7 @@ fn rejects_wrong_public_key() {
279279
assert!(
280280
matches!(
281281
result,
282-
EmailVerificationStatus::Unverified {
282+
DkimVerifyResult::Unverified {
283283
reason: VerificationFailReason::SignatureInvalid
284284
| VerificationFailReason::DnsRecordMalformed(_),
285285
..
@@ -302,7 +302,7 @@ fn no_dkim_signature_header() {
302302
let result = verify(&req, SYNTH_RSA_TXT, frozen_now());
303303
assert!(matches!(
304304
result,
305-
EmailVerificationStatus::Unverified {
305+
DkimVerifyResult::Unverified {
306306
reason: VerificationFailReason::NoSignature,
307307
..
308308
}

src/internet_identity/src/dkim/types.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Type definitions for the DKIM verifier.
22
//!
33
//! Mirrors the result-shape design from `docs/ongoing/email-recovery.md`
4-
//! §6.6: a [`EmailVerificationStatus`] is the verifier's externally
4+
//! §6.6: a [`DkimVerifyResult`] is the verifier's externally
55
//! visible verdict, carrying a per-step breakdown so the UI can show
66
//! *why* a signature failed when one did.
77
@@ -110,19 +110,30 @@ pub enum VerificationFailReason {
110110
/// DNS record's `t=y` testing flag is set; the verifier treats every
111111
/// signature from such a record as inconclusive.
112112
TestingMode,
113+
/// `From:` header missing, malformed, or carrying more than one
114+
/// mailbox (RFC 7489 §3.1.1 requires exactly one). The string
115+
/// carries a parser diagnostic.
116+
MalformedFromHeader(String),
117+
/// DMARC TXT record was supplied but couldn't be parsed.
118+
DmarcMalformed(String),
119+
/// DKIM signed `d=` doesn't align with the From-header domain
120+
/// under the published `adkim=` mode (or, when no DMARC record
121+
/// exists, doesn't equal it exactly).
122+
DmarcMisaligned,
113123
}
114124

115-
/// Overall verifier verdict.
125+
/// DKIM-only verdict, the result of [`super::verify::verify`].
116126
///
117127
/// Per RFC 6376 §5.5 / design §5.5, an email may carry multiple
118128
/// `DKIM-Signature` headers (e.g. original sender + mailing list
119-
/// forwarder). The verifier accepts the email as soon as *any one*
120-
/// signature passes; it returns `Verified` together with the `d=` of the
121-
/// signature that won. If all signatures fail, it returns `Unverified`
122-
/// with a single best-fit reason and the per-check breakdown for every
123-
/// signature it tried.
129+
/// forwarder). The DKIM verifier accepts the email as soon as *any
130+
/// one* signature passes the cryptographic check.
131+
///
132+
/// The combined DKIM + DMARC verdict (what callers actually want) is
133+
/// [`crate::dmarc::EmailVerificationStatus`], returned by
134+
/// `dmarc::verify_email`.
124135
#[derive(Clone, Debug, Eq, PartialEq)]
125-
pub enum EmailVerificationStatus {
136+
pub enum DkimVerifyResult {
126137
/// At least one signature passed.
127138
Verified {
128139
/// The `d=` of the signature that verified.
@@ -141,8 +152,8 @@ pub enum EmailVerificationStatus {
141152
},
142153
}
143154

144-
impl EmailVerificationStatus {
155+
impl DkimVerifyResult {
145156
pub fn is_verified(&self) -> bool {
146-
matches!(self, EmailVerificationStatus::Verified { .. })
157+
matches!(self, DkimVerifyResult::Verified { .. })
147158
}
148159
}

src/internet_identity/src/dkim/verify.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
//! DKIM verification entry point — orchestrates parsing, canonicalisation,
22
//! body hash, and signature verification per RFC 6376 §6.1.
33
//!
4-
//! Caller-facing API:
4+
//! Caller-facing API (re-exported as `crate::dkim::verify_dkim`):
55
//!
66
//! ```ignore
7-
//! pub fn verify(
7+
//! pub fn verify_dkim(
88
//! email: &SmtpRequest,
99
//! dkim_txt: &str,
1010
//! now_secs: u64,
11-
//! ) -> EmailVerificationStatus
11+
//! ) -> DkimVerifyResult
1212
//! ```
1313
//!
1414
//! `dkim_txt` is the (already-trusted) content of the DKIM TXT record
@@ -37,7 +37,7 @@ use super::dns_record::parse_dkim_txt;
3737
use super::parse::{parse_dkim_signature, DkimSignature};
3838
use super::signature::{body_hash_sha256, verify_signature, VerifyOutcome};
3939
use super::types::{
40-
DkimCheck, DkimCheckName, DkimCheckStatus, EmailVerificationStatus, HeaderCanon,
40+
DkimCheck, DkimCheckName, DkimCheckStatus, DkimVerifyResult, HeaderCanon,
4141
VerificationFailReason,
4242
};
4343
use internet_identity_interface::internet_identity::types::smtp::{SmtpHeader, SmtpRequest};
@@ -47,7 +47,7 @@ const DKIM_SIGNATURE_HEADER: &str = "DKIM-Signature";
4747
/// Verify an `SmtpRequest` against an already-trusted DKIM TXT record.
4848
///
4949
/// `now_secs` is Unix seconds — passed in so unit tests can pin time.
50-
pub fn verify(email: &SmtpRequest, dkim_txt: &str, now_secs: u64) -> EmailVerificationStatus {
50+
pub fn verify(email: &SmtpRequest, dkim_txt: &str, now_secs: u64) -> DkimVerifyResult {
5151
let message = match email.message.as_ref() {
5252
Some(m) => m,
5353
None => {
@@ -79,11 +79,15 @@ pub fn verify(email: &SmtpRequest, dkim_txt: &str, now_secs: u64) -> EmailVerifi
7979

8080
for dkim_header in &dkim_headers {
8181
match try_verify_signature(email, message, dkim_header, dkim_txt, now_secs) {
82-
Ok((dkim_domain, mut checks)) => {
83-
all_checks.append(&mut checks);
84-
return EmailVerificationStatus::Verified {
82+
Ok((dkim_domain, checks)) => {
83+
// Per `DkimVerifyResult::Verified.checks` ("checks for
84+
// the winning signature"), surface only the successful
85+
// attempt's per-step trail — not the accumulated trail
86+
// of every failed signature before it. Callers (and the
87+
// DMARC layer) reason about the winning signature only.
88+
return DkimVerifyResult::Verified {
8589
dkim_domain,
86-
checks: all_checks,
90+
checks,
8791
};
8892
}
8993
Err((reason, mut checks)) => {
@@ -97,8 +101,8 @@ pub fn verify(email: &SmtpRequest, dkim_txt: &str, now_secs: u64) -> EmailVerifi
97101
}
98102

99103
#[allow(non_snake_case)]
100-
fn Unverified(reason: VerificationFailReason, checks: Vec<DkimCheck>) -> EmailVerificationStatus {
101-
EmailVerificationStatus::Unverified { reason, checks }
104+
fn Unverified(reason: VerificationFailReason, checks: Vec<DkimCheck>) -> DkimVerifyResult {
105+
DkimVerifyResult::Unverified { reason, checks }
102106
}
103107

104108
/// Try to verify one specific DKIM-Signature header. Returns the `d=`
@@ -622,7 +626,7 @@ mod tests {
622626
};
623627
let result = verify(&req, "v=DKIM1; p=YWJj", 1_700_000_000);
624628
match result {
625-
EmailVerificationStatus::Unverified { reason, .. } => {
629+
DkimVerifyResult::Unverified { reason, .. } => {
626630
assert_eq!(reason, VerificationFailReason::NoSignature);
627631
}
628632
other => panic!("expected Unverified(NoSignature), got {:?}", other),
@@ -669,7 +673,7 @@ mod tests {
669673
};
670674
let result = verify(&req, "v=DKIM1; p=YWJj", 1_700_000_000);
671675
match result {
672-
EmailVerificationStatus::Unverified { reason, .. } => {
676+
DkimVerifyResult::Unverified { reason, .. } => {
673677
assert_eq!(reason, VerificationFailReason::UnsupportedCanonicalization);
674678
}
675679
other => panic!(
@@ -717,7 +721,7 @@ mod tests {
717721
};
718722
let result = verify(&req, "v=DKIM1; p=YWJj", 1_700_000_000);
719723
match result {
720-
EmailVerificationStatus::Unverified { reason, .. } => {
724+
DkimVerifyResult::Unverified { reason, .. } => {
721725
assert_eq!(reason, VerificationFailReason::SignatureExpired);
722726
}
723727
other => panic!("expected Unverified(SignatureExpired), got {:?}", other),
@@ -763,7 +767,7 @@ mod tests {
763767
};
764768
let result = verify(&req, "v=DKIM1; p=YWJj", 1_700_000_000);
765769
match result {
766-
EmailVerificationStatus::Unverified { reason, .. } => {
770+
DkimVerifyResult::Unverified { reason, .. } => {
767771
assert_eq!(reason, VerificationFailReason::AuidMisaligned);
768772
}
769773
other => panic!("expected Unverified(AuidMisaligned), got {:?}", other),
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
//! Domain alignment check for DMARC.
2+
//!
3+
//! Per design doc §6.3 we accept two cases:
4+
//!
5+
//! - **Strict** (`adkim=s`): the DKIM `d=` must equal the From-header
6+
//! domain byte-for-byte after ASCII-lowercasing.
7+
//! - **Relaxed** (`adkim=r`): the two domains must equal *or* one must
8+
//! be a strict subdomain of the other (label-aligned suffix).
9+
//!
10+
//! This is *stricter* than RFC-7489-compliant relaxed alignment, which
11+
//! uses the Public Suffix List to compute the "organizational domain"
12+
//! and accepts e.g. `gmail.com` ↔ `googlemail.com` if both are listed
13+
//! as the same org. We deliberately don't consult the PSL — design
14+
//! doc §6.4 documents the trust + asymmetric-failure-mode reasoning.
15+
//! In exchange we fail closed on multi-domain orgs (we reject
16+
//! `googlemail.com` + `From: gmail.com` mail), which is the safe
17+
//! direction for an account-recovery surface.
18+
//!
19+
//! Both inputs are expected to already be ASCII-lowercased.
20+
21+
use super::types::AlignmentMode;
22+
23+
/// Returns true iff the DKIM `d=` aligns with the From-header domain
24+
/// under `mode`.
25+
pub fn aligns(dkim_domain: &str, from_domain: &str, mode: AlignmentMode) -> bool {
26+
if dkim_domain == from_domain {
27+
return true;
28+
}
29+
if matches!(mode, AlignmentMode::Strict) {
30+
return false;
31+
}
32+
is_subdomain_of(dkim_domain, from_domain) || is_subdomain_of(from_domain, dkim_domain)
33+
}
34+
35+
/// Returns true iff `child` is a strict subdomain of `parent` — i.e.
36+
/// `child == "<labels>." + parent`. We anchor on the dot so a suffix
37+
/// match like `evilexample.com` ↔ `example.com` does **not** align.
38+
fn is_subdomain_of(child: &str, parent: &str) -> bool {
39+
if child.len() <= parent.len() {
40+
return false;
41+
}
42+
if !child.ends_with(parent) {
43+
return false;
44+
}
45+
let dot_pos = child.len() - parent.len() - 1;
46+
child.as_bytes()[dot_pos] == b'.'
47+
}
48+
49+
#[cfg(test)]
50+
mod tests {
51+
use super::*;
52+
53+
#[test]
54+
fn strict_exact_match() {
55+
assert!(aligns("example.com", "example.com", AlignmentMode::Strict));
56+
}
57+
58+
#[test]
59+
fn strict_subdomain_does_not_align() {
60+
assert!(!aligns(
61+
"mail.example.com",
62+
"example.com",
63+
AlignmentMode::Strict
64+
));
65+
assert!(!aligns(
66+
"example.com",
67+
"mail.example.com",
68+
AlignmentMode::Strict
69+
));
70+
}
71+
72+
#[test]
73+
fn relaxed_exact_match() {
74+
assert!(aligns("example.com", "example.com", AlignmentMode::Relaxed));
75+
}
76+
77+
#[test]
78+
fn relaxed_dkim_subdomain_of_from() {
79+
assert!(aligns(
80+
"mail.example.com",
81+
"example.com",
82+
AlignmentMode::Relaxed
83+
));
84+
}
85+
86+
#[test]
87+
fn relaxed_from_subdomain_of_dkim() {
88+
// The opposite direction (sending service signs as the
89+
// registered domain on behalf of a sub-domain of itself) is
90+
// valid per spec. We accept both directions in relaxed mode.
91+
assert!(aligns(
92+
"example.com",
93+
"mail.example.com",
94+
AlignmentMode::Relaxed
95+
));
96+
}
97+
98+
#[test]
99+
fn relaxed_evil_suffix_does_not_align() {
100+
// The whole point of the dot anchor: a suffix match without a
101+
// label boundary must NOT align.
102+
assert!(!aligns(
103+
"evilexample.com",
104+
"example.com",
105+
AlignmentMode::Relaxed
106+
));
107+
assert!(!aligns(
108+
"example.com",
109+
"evilexample.com",
110+
AlignmentMode::Relaxed
111+
));
112+
}
113+
114+
#[test]
115+
fn relaxed_unrelated_does_not_align() {
116+
assert!(!aligns(
117+
"gmail.com",
118+
"googlemail.com",
119+
AlignmentMode::Relaxed
120+
));
121+
assert!(!aligns("example.com", "evil.com", AlignmentMode::Relaxed));
122+
}
123+
124+
#[test]
125+
fn relaxed_deep_subdomain_aligns() {
126+
assert!(aligns(
127+
"deep.nested.example.com",
128+
"example.com",
129+
AlignmentMode::Relaxed
130+
));
131+
}
132+
}

0 commit comments

Comments
 (0)