feat(email-recovery): storage + smtp_request setup flow#3842
Closed
sea-snake wants to merge 23 commits into
Closed
feat(email-recovery): storage + smtp_request setup flow#3842sea-snake wants to merge 23 commits into
sea-snake wants to merge 23 commits into
Conversation
7 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the storage and canister API surface for the email recovery setup (binding) flow, including heap-resident pending challenges, PRNG seeding, SMTP gateway entry point, and persistence of DNS verification configuration needed by the DKIM/DMARC pipeline.
Changes:
- Persist email-recovery credential on anchors (stable) and add heap-only pending-challenge tracking + nonce generation.
- Expose new Candid endpoints for email-recovery setup (
prepare_add,status,remove) plussmtp_request. - Introduce / wire supporting DKIM+DMARC(+DNSSEC/DoH config) modules and test vectors used by the verification pipeline.
Reviewed changes
Copilot reviewed 58 out of 59 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test_vectors/dnssec/iana-root-anchors-2026-05.json | Adds root-anchor fixture for DNSSEC test vectors. |
| test_vectors/dkim/synth-rsa-test1._domainkey.test.example.com.txt | Adds synthetic DKIM TXT fixture for verifier tests. |
| test_vectors/dkim/synth-rsa-simple-simple.eml | Adds synthetic DKIM email fixture (unsupported canon case). |
| 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/tests.rs | Updates storage migration tests for new anchor field. |
| src/internet_identity/src/storage/storable/storable_persistent_state.rs | Persists dnssec_config/doh_config in stable state. |
| src/internet_identity/src/storage/storable/email_recovery_credential.rs | Adds stable representation for email recovery credential. |
| src/internet_identity/src/storage/storable/anchor.rs | Adds email_recovery field to stable anchor encoding. |
| src/internet_identity/src/storage/storable.rs | Exports new storable module. |
| src/internet_identity/src/storage/anchor/tests.rs | Updates anchor invariants tests for new field. |
| src/internet_identity/src/storage/anchor.rs | Adds in-memory anchor field + conversions to/from storable form. |
| src/internet_identity/src/storage.rs | Ensures anchor write path handles new field (no index sync needed). |
| src/internet_identity/src/state.rs | Adds dnssec_config/doh_config to persistent state defaults. |
| src/internet_identity/src/main.rs | Wires configs into config()/install arg, adds email-recovery canister methods. |
| src/internet_identity/src/email_recovery/mod.rs | New module for setup flow orchestration + constants. |
| src/internet_identity/src/email_recovery/pending.rs | Heap-only pending challenge map with TTL + eviction policy. |
| src/internet_identity/src/email_recovery/rng.rs | Heap PRNG seeded once via raw_rand for nonce generation. |
| src/internet_identity/src/email_recovery/prepare.rs | Implements prepare_add: validation, allowlist gate, nonce issuance. |
| src/internet_identity/src/email_recovery/remove.rs | Implements credential removal logic + archive operation placeholder. |
| src/internet_identity/src/email_recovery/smtp.rs | Implements smtp_request dispatch + end-to-end verification/binding. |
| src/internet_identity/src/doh/types.rs | Internal DoH provider/quorum configuration types. |
| src/internet_identity/src/dnssec/mod.rs | DNSSEC verifier module scaffold + exports. |
| src/internet_identity/src/dnssec/types.rs | DNSSEC proof-bundle internal types. |
| src/internet_identity/src/dnssec/canonical.rs | DNSSEC canonicalization helpers for signature verification. |
| src/internet_identity/src/dnssec/signature.rs | DNSSEC signature verification + DS digest matching. |
| src/internet_identity/src/dnssec/test_vectors.rs | Test-vector loader for DNSSEC chain/anchor fixtures. |
| src/internet_identity/src/dmarc/mod.rs | DMARC module scaffold + exports. |
| src/internet_identity/src/dmarc/types.rs | DMARC types + combined DKIM/DMARC verification status type. |
| src/internet_identity/src/dmarc/parse.rs | DMARC TXT parser. |
| src/internet_identity/src/dmarc/from_header.rs | Minimal From-header mailbox/domain extractor for DMARC. |
| src/internet_identity/src/dmarc/alignment.rs | Strict/relaxed domain alignment check. |
| src/internet_identity/src/dmarc/verify.rs | Orchestrates DKIM verification + DMARC alignment decision. |
| src/internet_identity/src/dmarc/test_vectors.rs | End-to-end DKIM+DMARC tests using committed fixtures. |
| src/internet_identity/src/dkim/mod.rs | DKIM verifier module scaffold + exports. |
| src/internet_identity/src/dkim/types.rs | DKIM verifier result/check types and failure reasons. |
| src/internet_identity/src/dkim/parse.rs | (Implicit in module) DKIM-Signature parsing support. |
| src/internet_identity/src/dkim/canonicalize.rs | DKIM relaxed canonicalization implementation. |
| src/internet_identity/src/dkim/dns_record.rs | DKIM TXT record parser. |
| src/internet_identity/src/dkim/signature.rs | DKIM cryptographic signature verification (RSA/Ed25519). |
| src/internet_identity/src/dkim/test_vectors.rs | DKIM fixture loader + end-to-end DKIM verification tests. |
| src/internet_identity/internet_identity.did | Extends Candid with DNSSEC/DoH config and email-recovery/smtp APIs. |
| src/internet_identity/Cargo.toml | Adds deps needed by DNSSEC/DoH/email pipeline. |
| src/internet_identity_interface/src/internet_identity/types/smtp.rs | Adds SMTP gateway wire types + validation helpers. |
| src/internet_identity_interface/src/internet_identity/types/email_recovery.rs | Adds email recovery wire types/errors/statuses. |
| src/internet_identity_interface/src/internet_identity/types/doh.rs | Adds DoH config type to the interface crate. |
| src/internet_identity_interface/src/internet_identity/types/dnssec.rs | Adds DNSSEC config/anchor types to the interface crate. |
| src/internet_identity_interface/src/internet_identity/types.rs | Re-exports new interface types and extends init arg struct. |
| scripts/capture-dnssec-chain.py | Adds utility to capture DNSSEC chains into test-vector JSON. |
| Cargo.toml | Adds workspace deps (p256, ed25519-dalek). |
| Cargo.lock | Lockfile updates for new dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 6, 2026
Closed
sea-snake
force-pushed
the
feat/email-recovery-storage-and-smtp
branch
from
8083ac7 to
561ecb7
Compare
Lays the storage foundation for the email-recovery flow (PRs 5+6 of the email-recovery stack — see docs/ongoing/email-recovery.md): - New module `internet_identity_interface::email_recovery` with the Candid types: EmailRecoveryCredential, EmailRecoveryChallenge, EmailRecoveryDnsInput (DohAllowlist + Dnssec stub), EmailRecoveryError, EmailRecoveryStatus, EmailRecoveryGetDelegationArgs. - New `StorableEmailRecoveryCredential` mirrors the on-disk shape via `minicbor-derive`. Lives inline on `StorableAnchor` as `#[n(5)] email_recovery: Option<...>`. minicbor's `#[cbor(map)]` is forward-compatible across optional-field additions, so old anchors decoded under the previous schema deserialize cleanly with `None`. - `Anchor` (in-memory) gains `email_recovery: Option<EmailRecoveryCredential>`, threaded through the three Storable<->Anchor conversion sites and the storage write-merge in `Storage::write_anchor` (the new field needs no merge — it's an overwrite-on-write single-slot credential). This commit only adds the data structure. Behaviour (canister methods, nonce PRNG, reverse address index, pending-challenge map, smtp_request) lands in subsequent commits on this branch. 414 unit tests pass; wasm32 build clean.
Continues the email-recovery setup flow (PRs 5+6 of the email-recovery
stack). Adds three building blocks under a new `email_recovery` module:
`email_recovery/rng.rs`
- A heap-resident ChaCha20Rng PRNG, seeded once per canister lifetime
via `random_salt()` (which calls `raw_rand` under the hood). The
first prepare call after canister start triggers the async fetch;
every subsequent draw is a single PRNG step.
- `format_nonce` produces `II-Recovery-{16-hex-chars}` — the verbose
prefix is deliberate (every legitimate UI surface starts with it,
so a phisher's alternative format is an obvious tell — see design
§3.2).
`email_recovery/pending.rs`
- Heap-only `HashMap<String, PendingChallenge>` with 30-min TTL and
oldest-first eviction at the bounded capacity (10 000).
- Heap rather than stable: pending entries expire in 30 minutes and
losing them on canister upgrade just costs an in-flight wizard one
retry, not worth the stable-memory churn.
- Random-nonce-only key gives the §8.8 untargetability property —
attackers can fill the map but can't pick whose entry gets evicted.
- Lazy TTL eviction on `status_of` reads + sweep on `insert`, so
polling sees `Expired` at the right moment without paying O(n) on
every read.
`email_recovery/prepare.rs`
- `prepare_add(anchor, dns_input, now_secs)` — async, validates
input, draws a fresh nonce, parks a `Pending` challenge.
- Variant dispatch: only `DohAllowlist` is wired up;
`EmailRecoveryDnsInput::Dnssec` returns
`DnssecPathNotYetSupported`. The DNSSEC path lands in a follow-up.
- DoH allowlist gate happens up front so a non-supported domain
fails at prepare with a clear error, not after the user has
composed an email.
- Address normalisation is intentionally minimal: lowercase + reject
obvious-garbage shapes. Strict RFC 5322 validation lives in the
verifier (PRs 2-3).
- Nonce-collision retry loop guards against the vanishingly small
birthday-collision case (8 attempts, then bail).
17 new unit tests (rng + pending + prepare); full unit suite green.
The `prepare_add` canister method is wired up in a subsequent commit
on this branch.
…methods Adds the canister-method surface for the setup-half of email recovery: - `email_recovery_credential_prepare_add(identity_number, dns_input)` — authenticated update. Validates that the caller owns the anchor, delegates to `email_recovery::prepare_add` for input validation + nonce issuance, returns the user-visible `EmailRecoveryChallenge`. - `email_recovery_status(nonce)` — anonymous query the FE polls while the user composes the email. Returns `Pending` → `RegistrationSucceeded` / `Failed` / `Expired`. - `email_recovery_credential_remove(identity_number, address)` — authenticated update. Detaches the recovery email from the anchor via the new `email_recovery::remove_credential` helper. Inlined authz (rather than using `anchor_operation_with_authz_check`) to avoid an awkward orphan-rule From<IdentityUpdateError> impl on an interface-crate error type. `.did` file: adds `EmailRecoveryCredential`, `EmailRecoveryChallenge`, `EmailRecoveryDnsInput`, `EmailRecoveryError`, `EmailRecoveryStatus`, `DohAllowlistDnsInput` types and the three new methods. `email_recovery::remove_credential` lives in a new `remove.rs` submodule with five unit tests covering the happy path, case- insensitive match, unbound anchor, wrong address (no partial mutation), and double-remove idempotency. `pending_status` is exposed as a thin `mod.rs` wrapper so `main.rs` doesn't need to know which submodule the heap state lives in. The `smtp_request` path (where the gateway forwards an inbound email and the canister verifies + binds) is the next commit on this branch. 436 unit tests pass (up from 414); wasm32 build clean; .did matches.
Two related changes:
1. **smtp_request handler.** New `email_recovery::smtp` submodule
that accepts an inbound email from the gateway and runs the full
verification pipeline:
- Bound-checks the SmtpRequest payload.
- Dispatches by recipient: `register@id.ai` for setup; other
recipients are silently dropped (no per-recipient signal back
to the gateway, so it can't probe the canister for which
mailboxes exist).
- Extracts the canister-issued nonce from the `Subject:` header
(case-insensitive prefix match, hex-suffix length-checked).
- Looks up the pending challenge, snapshotting it under a brief
borrow so the async DoH fetches don't hold a `RefCell` across
awaits.
- Resolves the DKIM key via `crate::doh::fetch_txt(<selector>.
_domainkey.<domain>, domain)` — heap-cached after the first
lookup per provider per TTL window.
- Optionally fetches `_dmarc.<domain>`; absence forces strict
alignment in the verifier.
- Runs `crate::dmarc::verify_email` end-to-end (full DKIM +
DMARC check + the `Subject in h=` enforcement added during
the PR 5 review).
- On success, writes the credential to the anchor and flips
the pending challenge to `Succeeded`. Failures flip it to
`Failed(reason)` for the FE poll. The gateway always sees
`SmtpResponse::Ok`.
Wired up as `smtp_request` in main.rs's email_recovery_api
submodule and added to the .did.
2. **Path-agnostic prepare API.** Per review feedback ("the FE
shouldn't need to know about the DoH allowlist"), drop the
`EmailRecoveryDnsInput::DohAllowlist | Dnssec` variant and
replace it with a flat `record { address; selector }`. The FE
just submits these; the canister picks the path:
- Future DNSSEC follow-up adds an optional `dns_proof` field
(Candid-forward-compatible). When supplied, the canister
validates the chain. When absent, falls through to the DoH
allowlist check.
- Currently only the DoH branch is wired up; non-allowlisted
domains return `DomainNotSupported` (renamed from the now-
removed `DomainNotAllowlisted`).
Drops the `DnssecPathNotYetSupported` error variant — the FE
never sees a DNSSEC-specific rejection, so there's no need to
carry the variant.
`.did` updates: `EmailRecoveryDnsInput` is now a record, the SMTP
gateway types (SmtpRequest/SmtpResponse and friends) are declared,
and the `smtp_request` method is exposed.
441 unit tests pass (up from 436, +5 smtp parser/extract tests);
wasm32 build clean; clippy `-D warnings` clean.
Adds an integration-test module exercising the canister-method surface end-to-end via PocketIC. Covers: - Nonce shape from `email_recovery_credential_prepare_add` (prefix + length + mailbox + expires_at). - Allowlist gate (non-allowlisted domain → `DomainNotSupported`). - Authorization (a non-owner principal cannot prepare for an anchor). - Status query semantics: `Expired` for unknown nonces, `Pending` after prepare_add, `Expired` after TTL via `env.advance_time`. - `smtp_request` silent-drop paths: missing nonce in Subject, unknown nonce, non-`register@` recipient. - `email_recovery_credential_remove` rejection when nothing bound. - **Full end-to-end binding flow**: prepare_add → in-test DKIM signer → `smtp_request` → mocked DoH outcalls → status flips to `RegistrationSucceeded` → `credential_remove` succeeds (proves the binding actually persisted to the anchor). The DKIM signer is a small in-test module (~200 LOC) that mirrors the verifier's relaxed canonicalization rules, generates a fresh 2048-bit RSA keypair per test, and signs against `<selector>._domainkey.<domain>`. The matching `v=DKIM1; k=rsa; p=…` TXT record is wrapped in a wire-format DNS response and served back via `env.mock_canister_http_response` for each of the 5 DoH provider outcalls. DMARC outcalls are answered with 404 (no record), which forces strict alignment — and our test setup satisfies that (DKIM `d=` equals From: domain). Adds matching helpers to `canister_tests::api::internet_identity`: - `email_recovery_credential_prepare_add` - `email_recovery_status` - `email_recovery_credential_remove` - `smtp_request` The integration test crate's existing build setup (II_WASM env var, PocketIC binary on PATH) is reused unchanged.
…lure
CI failure (`expected RegistrationSucceeded, got Failed(...DnsRecordMalformed("RSA SPKI decode...")...)`):
- The in-test DKIM signer was encoding the public key as PKCS#1
RsaPublicKey, but the canister verifier expects an X.509
SubjectPublicKeyInfo (SPKI) per RFC 5280 §4.1 — which is also what
real DKIM records publish in the `p=` tag. Switch to
`to_public_key_der()`.
Copilot review comments addressed:
- `EmailRecoveryChallenge.expires_at` now in **nanoseconds** since
epoch (matches `Timestamp` semantics — was off by a factor of 1e9).
- Non-allowlisted domain at prepare time now returns
`DomainNotAllowlisted(domain)` instead of `DomainNotSupported`.
`DomainNotSupported` is reserved for genuinely unsupported domain
shapes (and for the future DNSSEC-attempted-but-failed case).
- `smtp_request` recipient dispatch now matches on the **full**
recipient address (user + domain), case-insensitive on both
halves. New `SETUP_RECIPIENT_DOMAIN = "id.ai"` constant. A direct
caller can no longer set `to.user="register"` with an arbitrary
domain to bypass dispatch.
- `smtp_request` now maps `DohError` variants to typed
`EmailRecoveryError`s: `NotConfigured` / `DomainNotAllowed` →
`DomainNotAllowlisted` (so the FE shows "operator hasn't enabled
this domain"); `AllProvidersFailed` / `QuorumFailed` /
`ResponseMalformed` → `DohFetchFailed` (transient-error UX);
`InvalidName` / `NameOutsideRegisteredDomain` →
`InternalCanisterError` (caller-bug variants `prepare_add` should
have caught).
- Doc fixes:
- Interface module-level: `5-of-3 quorum` → `3-of-5 quorum`.
- `EmailRecoveryCredential` and `StorableEmailRecoveryCredential`:
`created_at` / `last_used` units corrected from "Unix-seconds"
to "nanoseconds since epoch".
- `EmailRecoveryChallenge.nonce` doc: "~16 base32 characters" →
"16 lowercase hex characters (8 random bytes)".
- `EmailRecoveryChallenge.expires_at` doc: "Unix seconds" →
"nanoseconds since epoch".
- `email_recovery::remove`: doc no longer claims to be wrapped in
`anchor_operation_with_authz_check` — that helper isn't used at
the canister-method layer (orphan-rule constraint on the
interface-crate error type, see `main.rs::email_recovery_api`).
Tests updated for the new units; 441 unit tests still pass.
Adds the DNSSEC verification path to the setup flow. The FE can now
optionally pass a `DnsProofBundle` alongside the address+selector;
when supplied, the canister:
1. Validates the chain synchronously against its configured
`DnssecConfig.root_anchors` at prepare time.
2. Confirms the bundle's leaf is a TXT record at the expected
`<selector>._domainkey.<registered_domain>` name (otherwise an
attacker who got a chain validated for *some* TXT record could
bind that record's content as a DKIM key).
3. Caches the verified TXT bytes on the pending challenge.
`smtp_request` then uses the cached TXT directly — no DoH outcall
fan-out at email-arrival time. When `dns_proof` is absent, falls
through to the existing DoH allowlist path unchanged.
Wire-level changes:
- `internet_identity_interface::types::dnssec`: adds Candid-friendly
mirror types for `DnsProofBundle`, `SignedRRset`, `Rrsig`,
`DelegationLink`. Wire-format owner names are `blob`s; the
canister-side verifier types stay as-is and are bridged via
`From<>` impls in `dnssec::types::interface_conversions`.
- `EmailRecoveryDnsInput` gains an optional `dns_proof:
Option<DnsProofBundle>` field. Forward-compatible Candid extension.
- `.did` declares the new types.
- `dnssec::types`: adds `TYPE_TXT` / `TYPE_DS` / `TYPE_DNSKEY`
constants and re-exports them from `dnssec::mod`.
- `dnssec::mod`: makes the `types` submodule `pub(crate)` so the
email-recovery module can reach the internal `DnsProofBundle`
via `From` conversions.
- `PendingChallenge` gains `cached_dkim_txt: Option<Vec<u8>>` so
`smtp_request` knows whether to skip the DoH path.
Two new PocketIC integration tests cover the plumbing end-to-end:
- `dnssec_path_rejects_when_no_trust_anchors_configured` — supplies
a structurally-valid bundle without anchor config; expects
`DomainNotSupported("…trust anchors not configured…")`.
- `dnssec_path_takes_precedence_over_doh_allowlist` — supplies a
malformed bundle for a domain that *is* on the DoH allowlist;
expects DNSSEC failure rather than fall-through to DoH.
A happy-path DNSSEC test (with a real `<selector>._domainkey.<domain>`
chain) is a follow-up — generating a DNSSEC chain at test time would
be a multi-hundred-line in-test signer; the existing
`crate::dnssec::test_vectors` cloudflare.com fixture's leaf is a
plain `cloudflare.com` TXT, not a `_domainkey` TXT.
Also addresses the earlier Copilot review feedback on PR dfinity#3842 (in a
separate commit on this branch): expires_at units, error variant
mapping, recipient-dispatch full-address match, DoH error mapping.
…ted split The Copilot-feedback fix introduced a dedicated `DomainNotAllowlisted` variant for the DoH allowlist-miss case (previously collapsed into `DomainNotSupported`), but missed updating this integration test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…path test The DNSSEC path used to short-circuit only the DKIM DoH outcall — DMARC still fanned out at email-arrival time, defeating the goal of "DNSSEC on, DoH off." Generalise `DnsProofBundle` to carry one *or more* leaves under the same chain walk; `prepare_add` extracts DKIM (required) and DMARC (optional) by name, caches both on the pending challenge, and `smtp_request` skips the DoH fan-out entirely on this path. Also fix `status_flips_to_expired_after_ttl`: PocketIC's query path doesn't see `advance_time` without a `tick()` to roll the round over. Adds `full_setup_flow_via_dnssec_path` — the missing happy-path integration test, with an in-test Ed25519 DNSSEC signer that builds a fresh root → leaf-zone chain and asserts zero DoH outcalls observed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ng dot The root terminator (`\x00`) was treated as an extra empty label and got its own '.' appended, on top of the trailing dot already added after the final real label. So `\x05test1\x0a_domainkey…\x00` decoded to `test1._domainkey.…com..` — never equal to the FQDN we compare it to. Existing tests masked this: the only callers reached this branch on the trust-anchors-empty short-circuit, never the name compare. Surfaced now by full_setup_flow_via_dnssec_path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…are equal e2e flows
The module-level docstring presented DoH-with-mocked-outcalls as the
canonical e2e setup, and the DoH happy-path test was titled "Full
end-to-end test" with a comment that read "this one does the real
work" — implicitly demoting the DNSSEC happy-path test to second-tier.
Both are full e2e flows; reframe the docs to make that explicit and
add parallel section headers ("DoH path: full end-to-end setup flow"
vs "DNSSEC path: full end-to-end setup flow + protocol-shape checks").
Comment-only change.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`normalize_address` accepted any non-empty, no-whitespace, has-`@` string. With the 2 MiB ingress limit a caller could submit a multi-KB address and inflate the heap-resident pending-challenge map. Apply the standard caps at every address-handling boundary: - `MAX_ADDRESS = 254` (path limit minus the `<>` framing — RFC 5321 §4.5.3.1.3 with the corrected interpretation, errata to RFC 3696). - `MAX_LOCAL_PART = 64` (§4.5.3.1.1). - `MAX_DOMAIN = 255` (§4.5.3.1.2). Enforced at prepare time (so an invalid address never gets a nonce) and at `smtp_request` From-extraction time (defense in depth — the gateway shouldn't pass through oversized addresses but we don't rely on it). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Several module-level and inline comments still claimed DoH was the only path, that DNSSEC was deferred, that callers hadn't landed, or referenced specific PR numbers in the now-collapsed stack. Now that PR dfinity#3842 landed both paths, the on-anchor credential, and the DKIM / DMARC / DoH / DNSSEC consumers, the comments needed to catch up. Concrete updates: - main.rs: prepare_add doc no longer says "DoH path only in this PR"; describes the actual path picker. - email_recovery.rs interface module + EmailRecoveryDnsInput doc: drops "DNSSEC deferred to a follow-up PR" / "DoH only in initial cut" framing; now describes both paths as live with the canister picking per call. - dkim/mod.rs, dmarc/mod.rs, dnssec/mod.rs: dead-code-allow rationale rewritten to point at the actual in-canister consumer (`crate::email_recovery::smtp::verify_setup_email`) instead of promising future PRs. - dkim/verify.rs, dmarc/verify.rs: data-flow doc no longer lists inputs as coming from "PR 1 / PR 4"; describes them as the cached DNSSEC-verified bytes or a DoH fetch. - dmarc/test_vectors.rs: drops "PR 2" references in favor of the module path the fixtures live at. - email_recovery/smtp.rs: pipeline summary mentions both DKIM-source paths (cached DNSSEC vs DoH); drops a stale "added during the PR 5 review" historical note. - email_recovery/remove.rs: TODO rephrased to be self-contained. Comment-only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The DNSSEC path stores the DKIM TXT (and optionally DMARC TXT) bytes on each pending entry so `smtp_request` can skip the DoH outcall. Those bytes come straight from the verified RDATA — `verify_dnssec_chain` authenticates them but doesn't bound their size. An attacker who controls a signed zone could publish a multi-KB TXT record, push it through prepare_add, and inflate every pending entry; at the 10 000-slot map cap that compounds. Add pragmatic caps on the cached bytes: - `MAX_DKIM_TXT_BYTES = 4096` — ~10× a real RSA-4096 DKIM record. - `MAX_DMARC_TXT_BYTES = 1024` — ~2× the largest realistic DMARC. Enforced in `verify_dnssec_chain` after the chain validates and before the bytes land on the pending challenge. Returns `EmailVerificationFailed` with a clear message naming the leaf and the cap. The DoH path is already bounded — `MAX_DOH_RESPONSE_BYTES = 4096` caps the per-outcall response, and the DoH-fetched bytes aren't stored on a pending entry anyway (they're consumed during `smtp_request` only). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The current canister API still caps a recovery email at one entry per anchor; this only changes the data model shape so multi-credential support can land later without another schema bump. - `Anchor.email_recovery: Option<EmailRecoveryCredential>` → `Vec<EmailRecoveryCredential>`. Empty vec replaces \`None\`. - `StorableAnchor.email_recovery: Option<StorableEmailRecoveryCredential>` → `Option<Vec<StorableEmailRecoveryCredential>>` — same wrapping as `passkey_credentials` / `recovery_keys` so anchors written under the prior schema decode cleanly (`Vec<T>` alone would `MissingValue` on decode; verified empirically against this minicbor-derive version). - Bind / remove logic and storage round-trip tests updated for the new shape.
Two-phase email-recovery only ever needs one leaf per call (the
optional DMARC TXT at prepare time, the DKIM TXT at submit_dkim_leaf
time), so a `Vec<SignedRRset>` was always-overkill — and a smaller
candid argument is a meaningful ingress saving for FE callers. Narrow
`DnsProofBundle.leaves: Vec<SignedRRset>` to
`DnsProofBundle.leaf: Option<SignedRRset>` across:
- `internet_identity_interface::types::DnsProofBundle` (candid + Rust)
- `crate::dnssec::types::DnsProofBundle` (verifier-internal)
- `interface_conversions` From<> impl
- `verify_with_clock` now returns `Result<VerifiedRecord, _>`
(single record); `bundle.leaf = None` triggers Malformed
- The `verify` wall-clock wrapper follows the same return shape
- The `WireBundle` JSON loader and the `cloudflare-com-2026-05.json`
test vector switch from `"leaves": [{...}]` to `"leaf": {...}`
- The `capture-dnssec-chain.py` capture script emits the new shape
- The DNSSEC-path happy-path integration test composes a
single-leaf bundle inline (the helper now exposes `skeleton`,
`dkim_leaf`, and optional `dmarc_leaf` separately so two-phase
callers can construct the bundle they need)
- `internet_identity.did`: `leaves : vec SignedRRset` →
`leaf : opt SignedRRset`
`prepare.rs::verify_dnssec_chain` is the only consumer that
previously expected DKIM + DMARC in one bundle. It now handles the
single-leaf shape — picking the leaf by owner-name match — but
otherwise keeps the legacy "DKIM at prepare time" pipeline since
the two-phase rewrite ships in the next commit. The integration
test still exercises that legacy pipeline.
All 13 dnssec unit tests still pass. The legacy DNSSEC happy-path
integration test was updated to construct its bundle from the new
`ChainOut` components.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Drops the FE selector-probing problem entirely: at prepare time the
FE submits a DNSSEC *skeleton* (chain only, optionally with the
DMARC leaf), and the canister caches the validated zone DNSKEY. When
the email arrives the canister parses `s=` from the DKIM-Signature
header — that's the authoritative selector — flips polled status to
`NeedDkimLeaf { selector }`, and the FE walks DNSSEC for that one
leaf and submits it via the new `email_recovery_submit_dkim_leaf`
update. See `docs/ongoing/email-recovery.md` §8.4 / §8.5.
What landed:
- **Interface types** (`internet_identity_interface`):
- `EmailRecoveryDnsInput.selector` removed.
- `EmailRecoveryStatus::NeedDkimLeaf { selector }` added (mid-flow).
- `EmailRecoveryError::SelectorMismatch` removed; replaced by
`DkimLeafMismatch` and `NoDkimLeafExpected`.
- `EmailRecoverySubmitDkimLeafArg { nonce, dkim_leaf }` added.
- **Pending state** (`email_recovery::pending`):
- `PendingChallenge.selector` and `cached_dkim_txt` removed.
- `registered_domain`, `cached_zone_dnskey: Option<SignedRRset>`,
and `partial_verification: Option<PartialVerification>` added.
- `PartialVerification { headers_digest 32 B + signature blob +
selector + signing_domain + algorithm + from_address_lc +
subject_signed }` (~500 B per pending entry).
- `PendingStatus::NeedDkimLeaf { selector }` added.
- **prepare.rs**:
- Drops selector input.
- Validates skeleton chain via `verify_chain_with_clock`, caching
the deepest-zone DNSKEY.
- Optionally validates a DMARC leaf at `_dmarc.<domain>`; rejects
any other leaf (including DKIM, which now belongs in the
submit-leaf call).
- **smtp.rs** (largely rewritten):
- On the DNSSEC path: parses `DKIM-Signature`, verifies `bh=`,
builds the canonical signed-headers input, computes its
SHA-256, and stashes `PartialVerification`. Body is dropped
once `bh=` validates. Pins From↔claimed_address and
`Subject` in `h=` early. Flips status to
`NeedDkimLeaf { selector }`.
- On the DoH path: behaves as before — fetch DKIM TXT, run
`dmarc::verify_email`, bind, finish synchronously.
- Idempotent: redelivered emails are silent no-ops once the
challenge has advanced past `Pending`.
- `bind_credential_to_anchor` → `bind_credential` taking
primitives so submit-leaf can reuse it.
- **submit_leaf.rs** (new module):
- Validates the FE-supplied `SignedRRset` against the cached
zone DNSKEY (no chain re-walk).
- Confirms the leaf's owner name is exactly
`<expected_selector>._domainkey.<registered_domain>.`.
- Parses the DKIM TXT, runs the cryptographic signature check
using a new prehash-aware helper:
- RSA-SHA256: `RsaVerifyingKey::verify_prehash` (PKCS#1 v1.5).
- Ed25519-SHA256: RFC 8463 — Ed25519 verify directly over
the cached SHA-256 digest.
- Verifies DMARC alignment against the cached DMARC bytes (or
falls back to strict `d=` ↔ From: equality when no DMARC
was cached).
- Binds the credential and flips status to `Succeeded`.
- **dnssec verifier**: chain/leaf split landed in a prior commit; this
commit consumes both halves.
- **DKIM/DMARC**: a few helpers raised to `pub(crate)` so smtp.rs and
submit_leaf.rs can compose the parts they need without re-running
the full single-pass verifier on a body we've already dropped.
- **main.rs**: `email_recovery_submit_dkim_leaf(arg)` update
endpoint added — anonymous (the nonce is the only authentication;
it's a 64-bit canister-issued secret bound to one pending entry).
- **.did** + regenerated FE bindings.
All 441 canister unit tests pass. The integration-test happy-path
fixture compiles but its runtime expectation (single-shot DKIM-leaf
bundle to prepare) is no longer correct under the two-phase flow —
the test needs a rewrite to exercise prepare → smtp_request →
submit_dkim_leaf in sequence. That rewrite ships with the FE work
that exercises the same endpoints.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Multiple lint failures across the stack were masked because earlier CI jobs (clippy) short-circuited before the later jobs (fmt, frontend-checks) got to run. Fix them all in one pass: - **clippy `doc_lazy_continuation`**: the `EmailRecoverySubmitDkimLeafArg` doc comment had a line starting with `+ signature blob)` — markdown saw the `+ ` as a list bullet and complained that the next lines weren't indented as continuation. Reflow the prose to drop the line break. - **clippy `unnecessary closure on Option::None`**: collapse a `ok_or_else(|| ...)` to `ok_or(...)` in submit_leaf.rs. - **fmt drift on existing files**: `dnssec/mod.rs`, `prepare.rs`, `submit_leaf.rs`, `email_recovery.rs`, plus a few outside the email-recovery surface (`openid/generic.rs`, `attributes.rs`, `oidc_configs.rs`) that previously slipped past CI. `cargo fmt` noop, just whitespace reflow. - **frontend-checks tsc error**: the mock `InternetIdentityInit` literal in `iiConnection.test.ts` and the actor-construction call in `vc-flow/index/+page.svelte` were missing the `doh_config` and `dnssec_config` fields that landed in earlier PRs in this stack. Add them as `[]` (the `opt opt T` shape's empty form). All 441 unit tests still pass; FE `npm run lint` is clean (0 errors); `svelte-check` reports 0 errors; `cargo fmt --check` clean; `cargo clippy --workspace --all-targets -- -D warnings` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The integration test \`full_setup_flow_via_dnssec_path\` was still
sending a DKIM-leaf bundle to \`prepare_add\`, which the canister
now rejects (skeleton bundles only carry DMARC at most; the DKIM
leaf belongs in \`submit_dkim_leaf\` post-email). Six shards of
canister-tests-run failed on it.
Rewrite the test to drive the full two-phase flow:
1. prepare_add with skeleton + DMARC leaf
2. smtp_request — body validates, status flips to NeedDkimLeaf
3. assert NeedDkimLeaf{selector} matches the test selector
4. submit_dkim_leaf with the DKIM leaf bundle → returns
RegistrationSucceeded
5. polled status confirms; remove path works
Also adds the matching \`email_recovery_submit_dkim_leaf\` helper
to canister_tests/src/api/internet_identity.rs.
The mod.rs preamble said the recovery half lives in a follow-up PR because it needs a reverse address index. That PR landed as the flow branch (dfinity#3843) and is part of the same stack now, so the note read as out-of-date in the cumulative diff. Trim it to point at where the recovery half actually lives. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The off-chain SMTP gateway calls a query method at RCPT TO time, before it pulls the message body from the sending MTA, to decide whether to accept the connection at all. The PoC defined this as \`smtp_request_validate\` and gated acceptance on the user-part parsing as a numeric anchor number — so on the email-recovery deploy, the gateway rejects \`register@id.ai\` and \`recover@id.ai\` outright, and \`smtp_request\` is never called. Bring the query back, with the email-recovery-specific shape: - \`register@id.ai\` (case-insensitive) → Ok. - \`recover@id.ai\` (case-insensitive) → Ok. Recognised at the validate query even on this PR (where the actual handler lives in the recovery follow-up dfinity#3843), since a "yes accept" here with no handler is harmless and avoids a deploy-step ordering constraint between this PR and the recovery one. - Everything else → 550 (mailbox unavailable). Numeric postbox- style addresses are no longer handled; the gateway should bounce them. The query is open (anyone can call it) but has no side effects and leaks nothing beyond the two recipient labels themselves, both already documented in the design doc. Six new unit tests cover register / recover / case-insensitivity, unknown user, known user with wrong domain, missing envelope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The off-chain SMTP gateway routes mail at a domain that varies per deploy: id.ai on prod, beta.id.ai on beta. The canister was hardcoding `id.ai` everywhere — recipient dispatch, the validate query, and the user-facing label returned from prepare — so on the beta canister mail to `register@beta.id.ai` reached the canister but failed the recipient match and was silently dropped. Drop the hardcoded constant. Derive the accepted mailbox domains from `related_origins`, which is already a per-deploy arg the deploy scripts wire through (and the same one used for security headers + the FE's `getPrimaryOrigin`). All entries are treated as equal aliases — recipient dispatch and the `smtp_request_validate` query accept `register@<host>` / `recover@<host>` for any host listed in `related_origins`. So a prod deploy with `id.ai` + the `*.icp0.io` aliases accepts mail at all of them; a beta deploy with `beta.id.ai` accepts that one. Drop the `mailbox` field from `EmailRecoveryChallenge` too. The FE already knows which origin the user is on (`window.location.hostname`), so it pairs that with `register` / `recover` to render the label — each tab automatically shows the alias matching the origin the user is on, and the canister never has to single one out as canonical. Empty / unset `related_origins` → no domains accepted; the canister drops every inbound recipient. Real deploys always configure this. Tests: extended `email_recovery::smtp::tests` with `set_related_origins` helper + 15 cases (single host, multi-alias prod, beta-only, unknown user, wrong domain, no-origins-configured); all pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The bundle's `chain: Vec<DelegationLink>` / `leaf: Option<SignedRRset>`
got replaced by `chains: Vec<DelegationChain>` / `hops: Vec<SignedRRset>`
when the multi-zone + CNAME-aware verifier landed in PR 1. Bring the
storage-and-smtp consumers in line:
- internet_identity_interface::types::dnssec: new bundle shape +
the new `DelegationChain { links: Vec<DelegationLink> }` wrapper,
CandidType-derived for the FE.
- internet_identity.did: matching candid type definitions.
- dnssec::types::interface_conversions: From<> bridge for the new
shape (CandidType-derived i_types → verifier-internal types).
- email_recovery::prepare::verify_dnssec_skeleton: was using the
old `verify_chain_with_clock(bundle, anchors, now)` (returned the
zone DNSKEY directly) plus `verify_leaf_against_dnskey(...)` for
the optional DMARC. Now uses the new entry points:
1) verify_root_dnskey_with_clock for the anchors check,
2) verify_extra_chains_with_clock to walk the chains into a
ZoneKeysMap,
3) verify_hops_with_clock for the optional DMARC hop.
Caches the single skeleton-zone DNSKEY for the submit phase via
ZoneKeysMap::iter() (added in PR 1).
- email_recovery::submit_leaf::run_submit: was calling
verify_leaf_against_dnskey directly against the cached zone
DNSKEY. Now wraps the cached DNSKEY in a 1-entry ZoneKeysMap and
the DKIM leaf in a 1-hop slice, calls verify_hops_with_clock.
- integration tests: update the three places that construct a
`DnsProofBundle` literal in tests (allowlist-precedence test,
happy-path-via-dnssec helper, in-test signer's `skeleton` field).
460 tests pass; cargo clippy clean with -D warnings.
sea-snake
force-pushed
the
feat/email-recovery-storage-and-smtp
branch
from
561ecb7 to
f04ea77
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
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
CI failure (`expected RegistrationSucceeded, got Failed(...DnsRecordMalformed("RSA SPKI decode...")...)`):
- The in-test DKIM signer was encoding the public key as PKCS#1
RsaPublicKey, but the canister verifier expects an X.509
SubjectPublicKeyInfo (SPKI) per RFC 5280 §4.1 — which is also what
real DKIM records publish in the `p=` tag. Switch to
`to_public_key_der()`.
Copilot review comments addressed:
- `EmailRecoveryChallenge.expires_at` now in **nanoseconds** since
epoch (matches `Timestamp` semantics — was off by a factor of 1e9).
- Non-allowlisted domain at prepare time now returns
`DomainNotAllowlisted(domain)` instead of `DomainNotSupported`.
`DomainNotSupported` is reserved for genuinely unsupported domain
shapes (and for the future DNSSEC-attempted-but-failed case).
- `smtp_request` recipient dispatch now matches on the **full**
recipient address (user + domain), case-insensitive on both
halves. New `SETUP_RECIPIENT_DOMAIN = "id.ai"` constant. A direct
caller can no longer set `to.user="register"` with an arbitrary
domain to bypass dispatch.
- `smtp_request` now maps `DohError` variants to typed
`EmailRecoveryError`s: `NotConfigured` / `DomainNotAllowed` →
`DomainNotAllowlisted` (so the FE shows "operator hasn't enabled
this domain"); `AllProvidersFailed` / `QuorumFailed` /
`ResponseMalformed` → `DohFetchFailed` (transient-error UX);
`InvalidName` / `NameOutsideRegisteredDomain` →
`InternalCanisterError` (caller-bug variants `prepare_add` should
have caught).
- Doc fixes:
- Interface module-level: `5-of-3 quorum` → `3-of-5 quorum`.
- `EmailRecoveryCredential` and `StorableEmailRecoveryCredential`:
`created_at` / `last_used` units corrected from "Unix-seconds"
to "nanoseconds since epoch".
- `EmailRecoveryChallenge.nonce` doc: "~16 base32 characters" →
"16 lowercase hex characters (8 random bytes)".
- `EmailRecoveryChallenge.expires_at` doc: "Unix seconds" →
"nanoseconds since epoch".
- `email_recovery::remove`: doc no longer claims to be wrapped in
`anchor_operation_with_authz_check` — that helper isn't used at
the canister-method layer (orphan-rule constraint on the
interface-crate error type, see `main.rs::email_recovery_api`).
Tests updated for the new units; 441 unit tests still pass.
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
Adds the DNSSEC verification path to the setup flow. The FE can now
optionally pass a `DnsProofBundle` alongside the address+selector;
when supplied, the canister:
1. Validates the chain synchronously against its configured
`DnssecConfig.root_anchors` at prepare time.
2. Confirms the bundle's leaf is a TXT record at the expected
`<selector>._domainkey.<registered_domain>` name (otherwise an
attacker who got a chain validated for *some* TXT record could
bind that record's content as a DKIM key).
3. Caches the verified TXT bytes on the pending challenge.
`smtp_request` then uses the cached TXT directly — no DoH outcall
fan-out at email-arrival time. When `dns_proof` is absent, falls
through to the existing DoH allowlist path unchanged.
Wire-level changes:
- `internet_identity_interface::types::dnssec`: adds Candid-friendly
mirror types for `DnsProofBundle`, `SignedRRset`, `Rrsig`,
`DelegationLink`. Wire-format owner names are `blob`s; the
canister-side verifier types stay as-is and are bridged via
`From<>` impls in `dnssec::types::interface_conversions`.
- `EmailRecoveryDnsInput` gains an optional `dns_proof:
Option<DnsProofBundle>` field. Forward-compatible Candid extension.
- `.did` declares the new types.
- `dnssec::types`: adds `TYPE_TXT` / `TYPE_DS` / `TYPE_DNSKEY`
constants and re-exports them from `dnssec::mod`.
- `dnssec::mod`: makes the `types` submodule `pub(crate)` so the
email-recovery module can reach the internal `DnsProofBundle`
via `From` conversions.
- `PendingChallenge` gains `cached_dkim_txt: Option<Vec<u8>>` so
`smtp_request` knows whether to skip the DoH path.
Two new PocketIC integration tests cover the plumbing end-to-end:
- `dnssec_path_rejects_when_no_trust_anchors_configured` — supplies
a structurally-valid bundle without anchor config; expects
`DomainNotSupported("…trust anchors not configured…")`.
- `dnssec_path_takes_precedence_over_doh_allowlist` — supplies a
malformed bundle for a domain that *is* on the DoH allowlist;
expects DNSSEC failure rather than fall-through to DoH.
A happy-path DNSSEC test (with a real `<selector>._domainkey.<domain>`
chain) is a follow-up — generating a DNSSEC chain at test time would
be a multi-hundred-line in-test signer; the existing
`crate::dnssec::test_vectors` cloudflare.com fixture's leaf is a
plain `cloudflare.com` TXT, not a `_domainkey` TXT.
Also addresses the earlier Copilot review feedback on PR #3842 (in a
separate commit on this branch): expires_at units, error variant
mapping, recipient-dispatch full-address match, DoH error mapping.
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
Several module-level and inline comments still claimed DoH was the only path, that DNSSEC was deferred, that callers hadn't landed, or referenced specific PR numbers in the now-collapsed stack. Now that PR #3842 landed both paths, the on-anchor credential, and the DKIM / DMARC / DoH / DNSSEC consumers, the comments needed to catch up. Concrete updates: - main.rs: prepare_add doc no longer says "DoH path only in this PR"; describes the actual path picker. - email_recovery.rs interface module + EmailRecoveryDnsInput doc: drops "DNSSEC deferred to a follow-up PR" / "DoH only in initial cut" framing; now describes both paths as live with the canister picking per call. - dkim/mod.rs, dmarc/mod.rs, dnssec/mod.rs: dead-code-allow rationale rewritten to point at the actual in-canister consumer (`crate::email_recovery::smtp::verify_setup_email`) instead of promising future PRs. - dkim/verify.rs, dmarc/verify.rs: data-flow doc no longer lists inputs as coming from "PR 1 / PR 4"; describes them as the cached DNSSEC-verified bytes or a DoH fetch. - dmarc/test_vectors.rs: drops "PR 2" references in favor of the module path the fixtures live at. - email_recovery/smtp.rs: pipeline summary mentions both DKIM-source paths (cached DNSSEC vs DoH); drops a stale "added during the PR 5 review" historical note. - email_recovery/remove.rs: TODO rephrased to be self-contained. Comment-only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
CI failure (`expected RegistrationSucceeded, got Failed(...DnsRecordMalformed("RSA SPKI decode...")...)`):
- The in-test DKIM signer was encoding the public key as PKCS#1
RsaPublicKey, but the canister verifier expects an X.509
SubjectPublicKeyInfo (SPKI) per RFC 5280 §4.1 — which is also what
real DKIM records publish in the `p=` tag. Switch to
`to_public_key_der()`.
Copilot review comments addressed:
- `EmailRecoveryChallenge.expires_at` now in **nanoseconds** since
epoch (matches `Timestamp` semantics — was off by a factor of 1e9).
- Non-allowlisted domain at prepare time now returns
`DomainNotAllowlisted(domain)` instead of `DomainNotSupported`.
`DomainNotSupported` is reserved for genuinely unsupported domain
shapes (and for the future DNSSEC-attempted-but-failed case).
- `smtp_request` recipient dispatch now matches on the **full**
recipient address (user + domain), case-insensitive on both
halves. New `SETUP_RECIPIENT_DOMAIN = "id.ai"` constant. A direct
caller can no longer set `to.user="register"` with an arbitrary
domain to bypass dispatch.
- `smtp_request` now maps `DohError` variants to typed
`EmailRecoveryError`s: `NotConfigured` / `DomainNotAllowed` →
`DomainNotAllowlisted` (so the FE shows "operator hasn't enabled
this domain"); `AllProvidersFailed` / `QuorumFailed` /
`ResponseMalformed` → `DohFetchFailed` (transient-error UX);
`InvalidName` / `NameOutsideRegisteredDomain` →
`InternalCanisterError` (caller-bug variants `prepare_add` should
have caught).
- Doc fixes:
- Interface module-level: `5-of-3 quorum` → `3-of-5 quorum`.
- `EmailRecoveryCredential` and `StorableEmailRecoveryCredential`:
`created_at` / `last_used` units corrected from "Unix-seconds"
to "nanoseconds since epoch".
- `EmailRecoveryChallenge.nonce` doc: "~16 base32 characters" →
"16 lowercase hex characters (8 random bytes)".
- `EmailRecoveryChallenge.expires_at` doc: "Unix seconds" →
"nanoseconds since epoch".
- `email_recovery::remove`: doc no longer claims to be wrapped in
`anchor_operation_with_authz_check` — that helper isn't used at
the canister-method layer (orphan-rule constraint on the
interface-crate error type, see `main.rs::email_recovery_api`).
Tests updated for the new units; 441 unit tests still pass.
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
Adds the DNSSEC verification path to the setup flow. The FE can now
optionally pass a `DnsProofBundle` alongside the address+selector;
when supplied, the canister:
1. Validates the chain synchronously against its configured
`DnssecConfig.root_anchors` at prepare time.
2. Confirms the bundle's leaf is a TXT record at the expected
`<selector>._domainkey.<registered_domain>` name (otherwise an
attacker who got a chain validated for *some* TXT record could
bind that record's content as a DKIM key).
3. Caches the verified TXT bytes on the pending challenge.
`smtp_request` then uses the cached TXT directly — no DoH outcall
fan-out at email-arrival time. When `dns_proof` is absent, falls
through to the existing DoH allowlist path unchanged.
Wire-level changes:
- `internet_identity_interface::types::dnssec`: adds Candid-friendly
mirror types for `DnsProofBundle`, `SignedRRset`, `Rrsig`,
`DelegationLink`. Wire-format owner names are `blob`s; the
canister-side verifier types stay as-is and are bridged via
`From<>` impls in `dnssec::types::interface_conversions`.
- `EmailRecoveryDnsInput` gains an optional `dns_proof:
Option<DnsProofBundle>` field. Forward-compatible Candid extension.
- `.did` declares the new types.
- `dnssec::types`: adds `TYPE_TXT` / `TYPE_DS` / `TYPE_DNSKEY`
constants and re-exports them from `dnssec::mod`.
- `dnssec::mod`: makes the `types` submodule `pub(crate)` so the
email-recovery module can reach the internal `DnsProofBundle`
via `From` conversions.
- `PendingChallenge` gains `cached_dkim_txt: Option<Vec<u8>>` so
`smtp_request` knows whether to skip the DoH path.
Two new PocketIC integration tests cover the plumbing end-to-end:
- `dnssec_path_rejects_when_no_trust_anchors_configured` — supplies
a structurally-valid bundle without anchor config; expects
`DomainNotSupported("…trust anchors not configured…")`.
- `dnssec_path_takes_precedence_over_doh_allowlist` — supplies a
malformed bundle for a domain that *is* on the DoH allowlist;
expects DNSSEC failure rather than fall-through to DoH.
A happy-path DNSSEC test (with a real `<selector>._domainkey.<domain>`
chain) is a follow-up — generating a DNSSEC chain at test time would
be a multi-hundred-line in-test signer; the existing
`crate::dnssec::test_vectors` cloudflare.com fixture's leaf is a
plain `cloudflare.com` TXT, not a `_domainkey` TXT.
Also addresses the earlier Copilot review feedback on PR #3842 (in a
separate commit on this branch): expires_at units, error variant
mapping, recipient-dispatch full-address match, DoH error mapping.
sea-snake
added a commit
that referenced
this pull request
May 12, 2026
Several module-level and inline comments still claimed DoH was the only path, that DNSSEC was deferred, that callers hadn't landed, or referenced specific PR numbers in the now-collapsed stack. Now that PR #3842 landed both paths, the on-anchor credential, and the DKIM / DMARC / DoH / DNSSEC consumers, the comments needed to catch up. Concrete updates: - main.rs: prepare_add doc no longer says "DoH path only in this PR"; describes the actual path picker. - email_recovery.rs interface module + EmailRecoveryDnsInput doc: drops "DNSSEC deferred to a follow-up PR" / "DoH only in initial cut" framing; now describes both paths as live with the canister picking per call. - dkim/mod.rs, dmarc/mod.rs, dnssec/mod.rs: dead-code-allow rationale rewritten to point at the actual in-canister consumer (`crate::email_recovery::smtp::verify_setup_email`) instead of promising future PRs. - dkim/verify.rs, dmarc/verify.rs: data-flow doc no longer lists inputs as coming from "PR 1 / PR 4"; describes them as the cached DNSSEC-verified bytes or a DoH fetch. - dmarc/test_vectors.rs: drops "PR 2" references in favor of the module path the fixtures live at. - email_recovery/smtp.rs: pipeline summary mentions both DKIM-source paths (cached DNSSEC vs DoH); drops a stale "added during the PR 5 review" historical note. - email_recovery/remove.rs: TODO rephrased to be self-contained. Comment-only. 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 |
sea-snake
added a commit
that referenced
this pull request
May 13, 2026
CI failure (`expected RegistrationSucceeded, got Failed(...DnsRecordMalformed("RSA SPKI decode...")...)`):
- The in-test DKIM signer was encoding the public key as PKCS#1
RsaPublicKey, but the canister verifier expects an X.509
SubjectPublicKeyInfo (SPKI) per RFC 5280 §4.1 — which is also what
real DKIM records publish in the `p=` tag. Switch to
`to_public_key_der()`.
Copilot review comments addressed:
- `EmailRecoveryChallenge.expires_at` now in **nanoseconds** since
epoch (matches `Timestamp` semantics — was off by a factor of 1e9).
- Non-allowlisted domain at prepare time now returns
`DomainNotAllowlisted(domain)` instead of `DomainNotSupported`.
`DomainNotSupported` is reserved for genuinely unsupported domain
shapes (and for the future DNSSEC-attempted-but-failed case).
- `smtp_request` recipient dispatch now matches on the **full**
recipient address (user + domain), case-insensitive on both
halves. New `SETUP_RECIPIENT_DOMAIN = "id.ai"` constant. A direct
caller can no longer set `to.user="register"` with an arbitrary
domain to bypass dispatch.
- `smtp_request` now maps `DohError` variants to typed
`EmailRecoveryError`s: `NotConfigured` / `DomainNotAllowed` →
`DomainNotAllowlisted` (so the FE shows "operator hasn't enabled
this domain"); `AllProvidersFailed` / `QuorumFailed` /
`ResponseMalformed` → `DohFetchFailed` (transient-error UX);
`InvalidName` / `NameOutsideRegisteredDomain` →
`InternalCanisterError` (caller-bug variants `prepare_add` should
have caught).
- Doc fixes:
- Interface module-level: `5-of-3 quorum` → `3-of-5 quorum`.
- `EmailRecoveryCredential` and `StorableEmailRecoveryCredential`:
`created_at` / `last_used` units corrected from "Unix-seconds"
to "nanoseconds since epoch".
- `EmailRecoveryChallenge.nonce` doc: "~16 base32 characters" →
"16 lowercase hex characters (8 random bytes)".
- `EmailRecoveryChallenge.expires_at` doc: "Unix seconds" →
"nanoseconds since epoch".
- `email_recovery::remove`: doc no longer claims to be wrapped in
`anchor_operation_with_authz_check` — that helper isn't used at
the canister-method layer (orphan-rule constraint on the
interface-crate error type, see `main.rs::email_recovery_api`).
Tests updated for the new units; 441 unit tests still pass.
sea-snake
added a commit
that referenced
this pull request
May 13, 2026
Adds the DNSSEC verification path to the setup flow. The FE can now
optionally pass a `DnsProofBundle` alongside the address+selector;
when supplied, the canister:
1. Validates the chain synchronously against its configured
`DnssecConfig.root_anchors` at prepare time.
2. Confirms the bundle's leaf is a TXT record at the expected
`<selector>._domainkey.<registered_domain>` name (otherwise an
attacker who got a chain validated for *some* TXT record could
bind that record's content as a DKIM key).
3. Caches the verified TXT bytes on the pending challenge.
`smtp_request` then uses the cached TXT directly — no DoH outcall
fan-out at email-arrival time. When `dns_proof` is absent, falls
through to the existing DoH allowlist path unchanged.
Wire-level changes:
- `internet_identity_interface::types::dnssec`: adds Candid-friendly
mirror types for `DnsProofBundle`, `SignedRRset`, `Rrsig`,
`DelegationLink`. Wire-format owner names are `blob`s; the
canister-side verifier types stay as-is and are bridged via
`From<>` impls in `dnssec::types::interface_conversions`.
- `EmailRecoveryDnsInput` gains an optional `dns_proof:
Option<DnsProofBundle>` field. Forward-compatible Candid extension.
- `.did` declares the new types.
- `dnssec::types`: adds `TYPE_TXT` / `TYPE_DS` / `TYPE_DNSKEY`
constants and re-exports them from `dnssec::mod`.
- `dnssec::mod`: makes the `types` submodule `pub(crate)` so the
email-recovery module can reach the internal `DnsProofBundle`
via `From` conversions.
- `PendingChallenge` gains `cached_dkim_txt: Option<Vec<u8>>` so
`smtp_request` knows whether to skip the DoH path.
Two new PocketIC integration tests cover the plumbing end-to-end:
- `dnssec_path_rejects_when_no_trust_anchors_configured` — supplies
a structurally-valid bundle without anchor config; expects
`DomainNotSupported("…trust anchors not configured…")`.
- `dnssec_path_takes_precedence_over_doh_allowlist` — supplies a
malformed bundle for a domain that *is* on the DoH allowlist;
expects DNSSEC failure rather than fall-through to DoH.
A happy-path DNSSEC test (with a real `<selector>._domainkey.<domain>`
chain) is a follow-up — generating a DNSSEC chain at test time would
be a multi-hundred-line in-test signer; the existing
`crate::dnssec::test_vectors` cloudflare.com fixture's leaf is a
plain `cloudflare.com` TXT, not a `_domainkey` TXT.
Also addresses the earlier Copilot review feedback on PR #3842 (in a
separate commit on this branch): expires_at units, error variant
mapping, recipient-dispatch full-address match, DoH error mapping.
sea-snake
added a commit
that referenced
this pull request
May 13, 2026
Several module-level and inline comments still claimed DoH was the only path, that DNSSEC was deferred, that callers hadn't landed, or referenced specific PR numbers in the now-collapsed stack. Now that PR #3842 landed both paths, the on-anchor credential, and the DKIM / DMARC / DoH / DNSSEC consumers, the comments needed to catch up. Concrete updates: - main.rs: prepare_add doc no longer says "DoH path only in this PR"; describes the actual path picker. - email_recovery.rs interface module + EmailRecoveryDnsInput doc: drops "DNSSEC deferred to a follow-up PR" / "DoH only in initial cut" framing; now describes both paths as live with the canister picking per call. - dkim/mod.rs, dmarc/mod.rs, dnssec/mod.rs: dead-code-allow rationale rewritten to point at the actual in-canister consumer (`crate::email_recovery::smtp::verify_setup_email`) instead of promising future PRs. - dkim/verify.rs, dmarc/verify.rs: data-flow doc no longer lists inputs as coming from "PR 1 / PR 4"; describes them as the cached DNSSEC-verified bytes or a DoH fetch. - dmarc/test_vectors.rs: drops "PR 2" references in favor of the module path the fixtures live at. - email_recovery/smtp.rs: pipeline summary mentions both DKIM-source paths (cached DNSSEC vs DoH); drops a stale "added during the PR 5 review" historical note. - email_recovery/remove.rs: TODO rephrased to be self-contained. Comment-only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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 5+6 of the email-recovery stack — the storage layer plus the canister-side surface for the setup (binding) flow. Stacks on #3841 (DoH fallback).
The recovery half (
prepare_delegation,recover@id.aidispatch, delegation issuance) is deferred to a follow-up PR — it needs a stable reverseaddress → AnchorNumberindex for the verified-From:lookup, which is best landed as its own focused change.What this PR adds
Storage:
EmailRecoveryCredentiallives directly on the anchor struct asOption<EmailRecoveryCredential>via a new#[n(5)]field onStorableAnchor. minicbor is forward-compatible across optional-field additions, so old anchors deserialize cleanly withNone— no schema migration.raw_rand. Per-call cost is one PRNG draw, not a management-canister round trip.Canister surface (Candid):
email_recovery_credential_prepare_add(IdentityNumber, EmailRecoveryDnsInput) -> Result<EmailRecoveryChallenge, EmailRecoveryError>— authenticated update.email_recovery_status(text) -> EmailRecoveryStatus— anonymous query (FE polls).email_recovery_credential_remove(IdentityNumber, text) -> Result<(), EmailRecoveryError>— authenticated update.smtp_request(SmtpRequest) -> SmtpResponse— open update; the gateway entry point.Two verification paths (FE is path-agnostic):
The FE just submits
{ address, selector, dns_proof: Option<DnsProofBundle> }. The canister picks the path:dns_proofisSomedns_proofisNonecrate::doh::fetch_txt(3-of-5 quorum, heap-cached)DomainNotSupported/DomainNotAllowlistedThe DNSSEC path validates the chain synchronously against
DnssecConfig.root_anchorsand confirms the bundle's leaf is at<selector>._domainkey.<domain>. Candid-friendly mirror types (DnsProofBundle,SignedRRset,Rrsig,DelegationLink) are defined in the interface crate withFrom<>conversions to the canister-internal verifier types.Verification pipeline (smtp_request):
register@id.ai) — defence-in-depth against direct callers spoofing just the user-part.Subject:header (case-insensitive prefix match, hex-suffix length-checked)._dmarc.<domain>; absence forces strict alignment in the verifier.crate::dmarc::verify_emailend-to-end (full DKIM + DMARC +Subjectinh=).Succeeded. Failures flip toFailed(reason). The gateway always seesOk.What this PR does NOT add (intentional, deferred to follow-ups)
prepare_delegation,recover@id.aidispatch, delegation issuance. Needs a reverse address index in stable storage; cleaner as its own PR.Test plan
cargo check --target wasm32-unknown-unknownclean.cargo clippy --tests -- -D warningsclean.cargo fmt --checkclean..didinterface stays in sync (the existingcheck_candid_interface_compatibilitytest enforces this)._domainkeychain).🤖 Generated with Claude Code
PR Stack
Changes during review
email_recoveryfield is now aVec(Option<Vec<...>>on the storable form, matching the existingpasskey_credentials/recovery_keyspattern). The canister API still enforces ≤1 entry — this is a data-model widening so future multi-credential support doesn't need another schema bump.Option<Vec<T>>means anchors written under the previous schema decode cleanly withemail_recovery: None(verified empirically against the in-tree minicbor-derive version).