feat(doh): DoH fallback for DKIM/DMARC TXT records on unsigned domains#3841
Closed
sea-snake wants to merge 3 commits into
Closed
feat(doh): DoH fallback for DKIM/DMARC TXT records on unsigned domains#3841sea-snake wants to merge 3 commits into
sea-snake wants to merge 3 commits into
Conversation
Closed
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds DNSSEC verification and a DoH (DNS-over-HTTPS) fallback subsystem intended to fetch DKIM/DMARC TXT records for domains that are not DNSSEC-signed, along with extensive test vectors and configuration plumbing (init/upgrade args + persistent state).
Changes:
- Introduces a DNSSEC proof-bundle verifier (trust-anchor based) with JSON test-vector loader and captured chains.
- Introduces a DoH TXT-fetch path with 3-of-5 quorum across multiple providers, heap caching, and in-flight deduplication.
- Wires new
dnssec_config/doh_configthrough Candid (.did), interface types, persistent state serialization, and init/upgrade application.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test_vectors/dnssec/iana-root-anchors-2026-05.json | Adds root-anchor test vector file used by DNSSEC verifier tests. |
| test_vectors/dnssec/cloudflare-com-2026-05.json | Adds captured DNSSEC chain test vector for cloudflare.com. |
| test_vectors/dkim/synth-rsa-test1._domainkey.test.example.com.txt | Adds synthetic DKIM TXT record fixture. |
| test_vectors/dkim/synth-rsa-simple-simple.eml | Adds synthetic DKIM email fixture (simple/simple). |
| test_vectors/dkim/synth-rsa-relaxed-simple.eml | Adds synthetic DKIM email fixture (relaxed/simple). |
| test_vectors/dkim/synth-rsa-relaxed-relaxed.eml | Adds synthetic DKIM email fixture (relaxed/relaxed). |
| test_vectors/dkim/README.md | Documents DKIM test-vector provenance/regeneration. |
| src/internet_identity/src/storage/storable/storable_persistent_state.rs | Persists dnssec_config / doh_config across upgrades via stable storage. |
| src/internet_identity/src/state.rs | Adds dnssec_config / doh_config to PersistentState defaults and docs. |
| src/internet_identity/src/main.rs | Registers new modules and propagates/apply init+upgrade args for DNSSEC/DoH config. |
| src/internet_identity/src/doh/types.rs | Defines DoH provider set, quorum constants, and DoH error types. |
| src/internet_identity/src/doh/quorum.rs | Implements provider fan-out outcome aggregation + quorum decision logic (and tests). |
| src/internet_identity/src/doh/parser.rs | Implements minimal DNS wire-format query builder and TXT extractor for DoH. |
| src/internet_identity/src/doh/cache.rs | Implements heap cache + in-flight deduplication using a waker-based primitive. |
| src/internet_identity/src/doh/mod.rs | Public DoH fetch entry point + production outcall path with transform + tests. |
| src/internet_identity/src/dnssec/mod.rs | Adds DNSSEC verifier module entry point and re-exports. |
| src/internet_identity/src/dnssec/types.rs | Defines DNSSEC proof bundle types and verification error taxonomy. |
| src/internet_identity/src/dnssec/canonical.rs | Implements canonical encodings needed for DNSSEC signed-data and DS digest inputs. |
| src/internet_identity/src/dnssec/signature.rs | Implements signature verification for alg {8,13,15} and DS digest matching. |
| src/internet_identity/src/dnssec/verify.rs | Implements DNSSEC chain verification algorithm + freshness checks + tests. |
| src/internet_identity/src/dnssec/test_vectors.rs | Loads DNSSEC JSON vectors into in-memory proof-bundle structures for tests. |
| src/internet_identity/src/dmarc/mod.rs | Adds DMARC module entry point and re-exports. |
| src/internet_identity/src/dmarc/types.rs | Defines DMARC record/outcome types and combined email verification status type. |
| src/internet_identity/src/dmarc/parse.rs | Implements DMARC TXT parser. |
| src/internet_identity/src/dmarc/from_header.rs | Extracts From-domain per DMARC single-mailbox constraint. |
| src/internet_identity/src/dmarc/alignment.rs | Implements strict/relaxed alignment logic. |
| src/internet_identity/src/dmarc/verify.rs | Orchestrates DKIM verification + DMARC alignment. |
| src/internet_identity/src/dmarc/test_vectors.rs | Adds end-to-end DKIM+DMARC tests using synthetic fixtures. |
| src/internet_identity/src/dkim/mod.rs | Adds DKIM module and re-exports verify as verify_dkim. |
| src/internet_identity/src/dkim/types.rs | Defines DKIM verification result/check types and failure reasons. |
| src/internet_identity/src/dkim/parse.rs | Parses DKIM-Signature header into structured tags. |
| src/internet_identity/src/dkim/dns_record.rs | Parses DKIM DNS TXT record (public key, flags). |
| src/internet_identity/src/dkim/canonicalize.rs | Implements DKIM relaxed canonicalization (headers/body). |
| src/internet_identity/src/dkim/signature.rs | Verifies DKIM signatures (RSA-SHA256 / Ed25519-SHA256) and body hash helper. |
| src/internet_identity/src/dkim/test_vectors.rs | Adds DKIM end-to-end tests consuming fixtures. |
| src/internet_identity/internet_identity.did | Extends init args with dnssec_config and doh_config types. |
| src/internet_identity/Cargo.toml | Adds dependencies for DNSSEC (p256, ed25519-dalek) and DoH fan-out (futures). |
| src/internet_identity_interface/src/internet_identity/types/smtp.rs | Adds SMTP gateway Candid types and validation helpers. |
| src/internet_identity_interface/src/internet_identity/types/dnssec.rs | Adds interface types for DNSSEC trust anchors/config. |
| src/internet_identity_interface/src/internet_identity/types/doh.rs | Adds interface types for DoH allowlist/cache tuning. |
| src/internet_identity_interface/src/internet_identity/types.rs | Re-exports new dnssec/doh/smtp interface types; extends InternetIdentityInit. |
| scripts/capture-dnssec-chain.py | Adds script to capture DNSSEC chains into test-vector JSON format. |
| Cargo.toml | Adds workspace deps for DNSSEC verification crates. |
| Cargo.lock | Locks new dependencies and transitive updates (e.g., futures, p256, ed25519-dalek). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 5, 2026
0c04244 to
4d0daec
Compare
Lays the foundation for PR 4 (DoH outcall fallback for unsigned
domains, design doc §7.6). Two pieces in this commit:
1. Wiring layer:
- new DohConfig type in internet_identity_interface (allowed_domains,
max_cache_age_secs)
- new InternetIdentityInit.doh_config: opt opt DohConfig (set/clear
pattern, like dnssec_config)
- new PersistentState.doh_config + StorablePersistentState mirror
- apply_install_arg + config() round-trip wired
- .did file updated; existing 365-test bin suite still passes
2. doh/ module scaffold:
- parser.rs: minimal wire-format DNS message helpers — build_txt_query
constructs a query for <name> IN TXT, parse_txt_response walks the
answer section and extracts the TXT RDATA character-strings
concatenated. Compression-pointer skipping has a hop cap so a
malicious response can't loop us. 9 unit tests.
- types.rs: DohProvider + the three providers we'll quorum across
(Quad9 CH, CIRA Canadian Shield CA, Cloudflare US — three
jurisdictions, three independent operators, all running as long-
term public services), QUORUM_THRESHOLD=2, default + cap on cache
age. DohError enum. 4 unit tests.
Cache, quorum, and the public fetch_txt API land in subsequent commits.
Wires up the body of the DoH fallback for DKIM/DMARC TXT records on unsigned domains: - Wire-format DNS query/parse (parser.rs) - 3-of-5 quorum across Cloudflare/Google/Quad9/CIRA/IIJ — strict majority across 4 jurisdictions; no single jurisdiction can reach quorum on its own (quorum.rs) - Heap cache with concurrent-fetch dedup via a hand-rolled Waker primitive; PENDING_STALE_AFTER_SECS guards against committed-state hangs from a trapped post-await continuation; lazy eviction on lookup + opportunistic sweep on publish bound cache size (cache.rs) - Public `fetch_txt(name, registered_domain)` async API: allowlist gate, cache lookup, parallel outcalls via futures::future::join_all, quorum decision, publish (mod.rs) - Production outcall path uses ic_cdk's http_request_with_closure with a transform query function that reduces wire responses to TXT bytes for replica consensus (TTLs in raw responses drift across replicas) Test coverage: 43 unit tests covering parser, bucketing, dedup, stale-pending eviction, sweep-on-publish, and end-to-end fetch_txt flow including allowlist, cache TTL, MAX_CACHE_AGE_SECS cap, and quorum-failure-not-cached. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… + dfinity#3841 DKIM (PR 2's code): - canonicalize.rs: relaxed_body empty body → CRLF per RFC 6376 §3.4.3 - verify.rs: simple_body empty body → CRLF (same rule) - parse.rs: enforce that h= includes the From header per RFC 6376 §5.4 (without this, DMARC alignment can be defeated by a post-sign rewrite) - types.rs: doc comment now points at dmarc::EmailVerificationStatus rather than the misleading DkimVerifyResult DNSSEC (PR 1's code): - verify.rs: try ALL matching trust-anchor candidates before giving up. During KSK rollovers operators configure both the rolling-out and rolling-in keys; first-match early-exit could fail under the inactive one and never try the active one. - mod.rs: doc comment no longer claims the email-recovery stack has no HTTP outcalls (the DoH fallback module does). - iana-root-anchors-2026-05.json: corrected stale "_comment" that claimed the historical 19036 KSK was included. DMARC (PR 3's code): - parse.rs: enforce p= immediately after v=DMARC1 per RFC 7489 §6.3. Real-world records all do this; OpenDMARC and other reference parsers reject the alternative. - from_header.rs: honour backslash escapes inside quoted-string display names so `"Alice \"Ops, Inc\"" <a@e.com>` parses correctly. - verify.rs: removed empty placeholder test that asserted nothing; the e2e path is exercised by test_vectors::*. DoH (PR 4's code): - parser.rs: encode_name now rejects invalid names (label > 63, total > 255, empty labels) rather than silently truncating — truncation would change which name we ask for, which is a real correctness issue. - mod.rs: fetch_txt now enforces a label-anchored suffix match between `name` and `registered_domain` (defence-in-depth: a caller bug otherwise lets an allowlisted registered_domain authorise an outcall for an unrelated FQDN). - mod.rs: transform_doh now signals parse failure via HTTP status 422 + empty body instead of a sentinel string in the body. The prior sentinel could collide with a (legal-but-unusual) TXT payload and silently turn valid records into "malformed". - types.rs / quorum.rs / interface doc: fixed stale comments that said "three providers" or implied parallelism in the sync test helper (the production async path uses futures::future::join_all; run_quorum is sequential). - New error variants: DohError::InvalidName, NameOutsideRegisteredDomain. - New unit tests cover the new validation paths. Test counts: 408 → 414 (added 6). Note on the alignment.rs / no-PSL flag: the Copilot comment that relaxed alignment is "fail-open without a PSL" is correct in isolation but reflects a documented design decision. We deliberately don't ship a PSL in the canister (size, freshness, and trust-root concerns — see design doc §6.4). The mitigation is the dot-anchored suffix check that prevents `evilexample.com` from matching `example.com`. Left unchanged; will reply with the design rationale. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4d0daec to
c14b0c9
Compare
Contributor
Author
|
Replaced with a new PR from an upstream branch (enables direct collaboration). Same content, new PR number. |
This was referenced May 12, 2026
Merged
2 tasks
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
DKIM (PR 2's code): - canonicalize.rs: relaxed_body empty body → CRLF per RFC 6376 §3.4.3 - verify.rs: simple_body empty body → CRLF (same rule) - parse.rs: enforce that h= includes the From header per RFC 6376 §5.4 (without this, DMARC alignment can be defeated by a post-sign rewrite) - types.rs: doc comment now points at dmarc::EmailVerificationStatus rather than the misleading DkimVerifyResult DNSSEC (PR 1's code): - verify.rs: try ALL matching trust-anchor candidates before giving up. During KSK rollovers operators configure both the rolling-out and rolling-in keys; first-match early-exit could fail under the inactive one and never try the active one. - mod.rs: doc comment no longer claims the email-recovery stack has no HTTP outcalls (the DoH fallback module does). - iana-root-anchors-2026-05.json: corrected stale "_comment" that claimed the historical 19036 KSK was included. DMARC (PR 3's code): - parse.rs: enforce p= immediately after v=DMARC1 per RFC 7489 §6.3. Real-world records all do this; OpenDMARC and other reference parsers reject the alternative. - from_header.rs: honour backslash escapes inside quoted-string display names so `"Alice \"Ops, Inc\"" <a@e.com>` parses correctly. - verify.rs: removed empty placeholder test that asserted nothing; the e2e path is exercised by test_vectors::*. DoH (PR 4's code): - parser.rs: encode_name now rejects invalid names (label > 63, total > 255, empty labels) rather than silently truncating — truncation would change which name we ask for, which is a real correctness issue. - mod.rs: fetch_txt now enforces a label-anchored suffix match between `name` and `registered_domain` (defence-in-depth: a caller bug otherwise lets an allowlisted registered_domain authorise an outcall for an unrelated FQDN). - mod.rs: transform_doh now signals parse failure via HTTP status 422 + empty body instead of a sentinel string in the body. The prior sentinel could collide with a (legal-but-unusual) TXT payload and silently turn valid records into "malformed". - types.rs / quorum.rs / interface doc: fixed stale comments that said "three providers" or implied parallelism in the sync test helper (the production async path uses futures::future::join_all; run_quorum is sequential). - New error variants: DohError::InvalidName, NameOutsideRegisteredDomain. - New unit tests cover the new validation paths. Test counts: 408 → 414 (added 6). Note on the alignment.rs / no-PSL flag: the Copilot comment that relaxed alignment is "fail-open without a PSL" is correct in isolation but reflects a documented design decision. We deliberately don't ship a PSL in the canister (size, freshness, and trust-root concerns — see design doc §6.4). The mitigation is the dot-anchored suffix check that prevents `evilexample.com` from matching `example.com`. Left unchanged; will reply with the design rationale. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
pull Bot
pushed a commit
to mikeyhodl/internet-identity
that referenced
this pull request
May 12, 2026
…iring (dfinity#3838) ## Summary First PR in the email-recovery stack (`docs/ongoing/email-recovery.md` §10 Phase 0). Lands a working RFC-4035-compliant DNSSEC verifier for caller-supplied DNS proof bundles, plus the trust-anchor wiring that drives it. PR 2 (DKIM verifier) and PRs 4–9 (storage + recovery methods) build on this. ## What's in this PR ### Verifier core - New `dnssec/` module under `src/internet_identity/src/`: - `types.rs` — `DnsProofBundle`, `SignedRRset`, `DelegationLink`, `Rrsig`, `DnsName`, `DnssecError`, `VerifiedRecord`. - `canonical.rs` — owner-name canonicalization, RR canonical form, RRSIG signed-data construction (RFC 4034 §3.1.8.1, §6.2, §6.3), DS digest input. - `signature.rs` — algorithm dispatch + DS digest matching (SHA-256). - `verify.rs` — four-step algorithm (root anchor match → chain walk → leaf RRSIG → freshness). - Algorithm coverage (RFC 8624 MUST set): - **alg 8** — RSA-SHA256 (RFC 5702): root, com., most legacy zones. - **alg 13** — ECDSA-P256-SHA256 (RFC 6605): most TLDs, Cloudflare, modern zones. - **alg 15** — Ed25519 (RFC 8080): rare in production but mandatory. - Anything else returns `UnsupportedAlgorithm`. ### Wiring - New `DnssecConfig` and `DnssecRootAnchor` types in `internet_identity_interface`, exposed at the top of `InternetIdentityInit` as `dnssec_config: opt opt DnssecConfig` (set/clear semantics matching `analytics_config` and `dummy_auth`). - Trust-anchor list plumbed through `init`/`post_upgrade` into `PersistentState.dnssec_config` (and `StorablePersistentState` for cross-upgrade persistence). - `internet_identity.did` updated. ### Tests 13 unit tests in `dnssec/` covering: - Real cloudflare.com chain verifies end-to-end (exercises alg 8 at root and alg 13 at com → cloudflare.com → leaf). - Empty trust-anchor list rejected with `NoTrustAnchors`. - Wrong trust anchor rejected with `RootAnchorMismatch`. - Flipped byte in root DNSKEY → `RootAnchorMismatch` or `BadSignature`. - Flipped byte in leaf signature → `BadSignature`. - Stale signature (clock advanced past expiration) → `StaleOrFutureSignature`. - Unsupported algorithm (alg 5 / RSA-SHA1) → `UnsupportedAlgorithm(5)`. - Plus canonical-encoding + RFC 3110 RSA key parsing unit tests. ### Test infrastructure - `test_vectors/dnssec/cloudflare-com-2026-05.json` — real DoH-captured chain (root DNSKEY + 2 delegation links + leaf TXT). - `test_vectors/dnssec/iana-root-anchors-2026-05.json` — IANA root KSK trust anchors (Klajeyz/2017 + Kmyv6jo/2024). - `scripts/capture-dnssec-chain.py` — reproducible capture script using dnspython + DoH wire format. Tests use a frozen now from the capture's metadata so freshness checks stay stable indefinitely. ### New deps - `domain` (NLnet Labs, pure Rust) — referenced in docstrings for canonicalisation primitives; signature verification is hand-rolled on top of RustCrypto. - `p256` — ECDSA P-256 verification. - `ed25519-dalek` — Ed25519 verification. All three build cleanly for wasm32-unknown-unknown. ## What's deferred to later PRs in the stack - Captures for additional providers (proton.me, protonmail.com, tutanota.com — gmail.com / icloud.com / outlook.com / fastmail.com don't sign with DNSSEC; this is acknowledged in design doc §7.6). - Synthetic Ed25519 (alg 15) test vector — most production zones are alg 8 or 13; alg 15 is structurally exercised by the dispatch logic but doesn't have a real captured chain in this PR. ## Test plan - [x] `cargo check -p internet_identity --target wasm32-unknown-unknown` — clean (no warnings). - [x] `cargo test -p internet_identity --bin internet_identity` — 238 tests pass (was 227 pre-PR). - [x] `cargo test -p internet_identity_interface --lib` — 42 tests pass. - [x] `cargo clippy -p internet_identity --bin internet_identity --tests -- -D warnings` — clean. - [x] `cargo fmt --check` — clean (modulo a pre-existing diff in attributes.rs unrelated to this PR). - [x] CI on dfinity#3838 — fully green. ## Design doc https://github.com/sea-snake/internet-identity/blob/design/email-recovery/docs/ongoing/email-recovery.md (PR pending review on dfinity/internet-identity) ## Stack This is PR 1 of a 12-PR series. Subsequent PRs: - **PR 2** — mail-auth-backed DKIM verifier, consuming DnsProofBundle from this PR. - **PR 3** — DMARC alignment. - **PRs 4–9** — storage + Candid + behavior for email recovery. - **PRs 10–12** — frontend (DoH walker, Manage UI, recovery wizard). ## PR Stack | # | PR | Description | Status | |---|---|---|---| | 0 | [dfinity#3836](dfinity#3836) | Design doc | Open | | 1 | [dfinity#3838](dfinity#3838) | DNSSEC verifier scaffold | Open | | 2 | [dfinity#3839](dfinity#3839) | DKIM verifier (RFC 6376) | Open | | 3 | [dfinity#3840](dfinity#3840) | DMARC alignment (RFC 7489) | Open | | 4 | [dfinity#3841](dfinity#3841) | DoH fallback | Open | | 5+6 | [dfinity#3842](dfinity#3842) | Setup flow (storage + smtp_request) | Open | | 7 | [dfinity#3843](dfinity#3843) | Recovery flow (delegation) | Open | | 8 | [dfinity#3844](dfinity#3844) | Frontend + feature flag | Open | | 9 | [dfinity#3855](dfinity#3855) | Deploy/upgrade scripts: dnssec_config + doh_config | Open | | 10 | [dfinity#3857](dfinity#3857) | Email-recovery UX overhaul | Open |
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.
Summary
PR 4 of the email-recovery stack. Adds the DoH (DNS-over-HTTPS) fallback path for fetching DKIM/DMARC TXT records when a domain isn't DNSSEC-signed (Gmail, Outlook, iCloud, etc.). Stacks on #3840 (DMARC alignment).
DohConfig.allowed_domains(deploy/upgrade arg, persisted inPersistentState). Domains not on the list returnDomainNotAllowedwithout touching the network.oneshot-styleWakerprimitive collapses concurrent fetches for the same FQDN to one outcall fan-out.http_request_with_closurewith atransformquery function that reduces the wire DNS response to its TXT RDATA (drops TTL/transaction-ID drift across replicas, so consensus succeeds).max_response_bytes = 4 KiB, cycle budget mirrors the OIDC discovery outcall.PENDING_STALE_AFTER_SECS = 120) recovers from the IC-specific hazard where a trapped post-yield continuation leaves an InFlight entry stuck forever (state across.awaitis committed, not transactional). Lazy eviction on lookup + opportunistic sweep on publish bound the value-cache size to {unique FQDNs queried inmax_cache_age_secs}.Public API:
pub async fn fetch_txt(name, registered_domain) -> Result<Vec<u8>, DohError>. Allowlist match is case-insensitive on the registered domain. Steady-state outcall load for a continuous Gmail flow at default 1h TTL: ~10 outcalls/hr per domain (2 fan-outs × 5 providers), independent of email volume.PR 8 onwards will wire this into the DKIM/DMARC verifiers from PRs 2/3 alongside the DNSSEC chain from PR 1.
Test plan
cargo check --target wasm32-unknown-unknownclean.cargo clippy --tests -- -D warningsclean.cargo fmt --checkclean.public.dns.iij.jp) with a real outcall before deploy.PR Stack