feat(be): verify a browser's key and its announced successor - #4264
Merged
sea-snake merged 63 commits intoSep 9, 2026
Conversation
This was referenced Aug 22, 2026
sea-snake
marked this pull request as ready for review
August 22, 2026 18:16
|
✅ No security or compliance issues detected. Reviewed everything up to 6ef8323. Security Overview
Detected Code Changes
|
sea-snake
force-pushed
the
feat/session-device-key-proof
branch
from
August 22, 2026 18:44
b3521fc to
52cdad9
Compare
A sign-in that names a browser has to prove the browser holds the key it names, and the successor it announces for its next sign-in. Two signatures, under two domains: the current key over the session key and the successor, and the successor over the session key and the current key. Separate domains are what stop either signature being replayed in the other's role. Ingress messages are public, so a key read off the wire could otherwise be announced by someone who does not hold it, and claimed when its browser next presents one. P-256 only, and no fallback: a key that does not parse, a signature of the wrong length and an empty signature are each refused rather than skipped. The module is annotated `allow(dead_code)` because its caller is the sign-in ceremony, which lands two PRs up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
force-pushed
the
feat/session-device-key-proof
branch
from
August 22, 2026 18:58
52cdad9 to
960f5be
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The critical domain-separator mismatch must be resolved before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds backend P-256 verification for current and successor browser keys used during session creation.
Changes:
- Verifies domain-separated signatures binding both browser keys to the session key.
- Adds acceptance and rejection tests.
- Enables PKCS#8 support and updates dependencies.
File summaries
| File | Description |
|---|---|
src/internet_identity/src/sessions/browser_key.rs |
Implements key verification and tests; domain separators conflict with the documented wire format. |
src/internet_identity/src/sessions.rs |
Exposes the sessions module. |
src/internet_identity/src/main.rs |
Registers the sessions module. |
src/canister_tests/Cargo.toml |
Adds an unused direct P-256 dependency. |
Cargo.toml |
Enables P-256 PKCS#8 support. |
Cargo.lock |
Records dependency changes. |
Review details
Suppressed comments (1)
src/canister_tests/Cargo.toml:12
canister_testsdoes not usep256in this PR; the only match in that crate is this manifest entry, and the referencedprepare_account_sessioncaller is deferred to #4268. This adds an unused direct dependency and lockfile entry to a verifier-only change. Please add it with the integration-test helper that actually uses it and regenerate the lockfile then.
# Signs the browser-key proof `prepare_account_session` requires.
p256.workspace = true
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
MRmarioruci
reviewed
Sep 9, 2026
MRmarioruci
reviewed
Sep 9, 2026
MRmarioruci
approved these changes
Sep 9, 2026
Verification returned a `bool`, so `create_session` registering a browser from the keys it was handed depended on its caller having checked them first. `verify_browser_keys` now returns a `VerifiedBrowserKeys` with private fields and no other constructor, so the type is the proof. That makes storage depend on the verifier, which is why the module moves to the crate root: a P-256 verifier that knows nothing of sessions or storage, which either layer may use. `from_public_key_der` accepts a compressed SEC1 point as readily as an uncompressed one, and both are valid DER — so one private key had two spellings, and `resolve_browser` compares the bytes it is given. Only the uncompressed form is accepted now, which is the only one WebCrypto can emit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
MRmarioruci
approved these changes
Sep 9, 2026
sea-snake
added a commit
that referenced
this pull request
Sep 9, 2026
Design: #4224. Overview: #4230. The registry only — #4264 proves and rotates a browser's key, #4249 renders the list. Sessions live per account, so a user who wants to sign one browser out has nothing to name it by. A browser registry groups a browser's sessions across apps so they can be revoked together. - `StorableAnchor` gains `browsers` (field 7) and `next_browser_id` (field 8), both `Option` so existing anchors decode cleanly. Each entry is `{id, description, created_at, last_used, current_browser_key, next_browser_key}`, capped at 20 because the anchor blob is read on nearly every authenticated path. - `current_browser_key` is the key the browser last proved with and `next_browser_key` the successor it announced then. Either resolves the entry, and presenting the successor promotes it, retiring the key it replaced. That is what makes a browser profile copied off disk stop working: the copy dies as soon as the real browser signs in again. - Accepting both is what makes a lost response harmless. The browser advances only once a sign-in succeeded, so an unanswered call leaves it proving with the key the entry still holds rather than looking like a new machine. - A successor another browser of the same anchor already holds is refused (`SuccessorAlreadyInUse`), so one public key belongs to at most one entry and resolving a presented key never depends on list order. #4264 adds the stronger rule, proof of possession of the successor. - The id never changes across rotations, so rotating costs a session nothing: sessions record the id, not the key. Ids come from a monotonic per-anchor counter and are never reused, so a flood of registrations leaves a permanent gap in the sequence rather than a list that looks untouched. - At the cap the least recently used entry is dropped rather than the registration failing, and its id is returned so the caller can end that browser's sessions too. - Eviction orders on `last_used`, not `created_at`. Clearing browser storage loses the key, so every wipe enrols a fresh entry and the wiping browser always holds the newest `created_at` — under enrolment order it would never be its own victim, and twenty wipes would evict twenty browsers the user actually signs in from. - **`description` is what the browser reported, in tokens rather than a name to show.** `BrowserBrand`, `OperatingSystem` and `FormFactor` are variants, plus the hardware model where a client can name one; each variant carries an `Other : text` for something this list does not name, because an unrecognised browser is worth seeing rather than worth hiding behind a generic label. Tokens because products get renamed — "Chrome OS" became "ChromeOS", "Mac OS X" became "macOS" — so the wording a user reads lives in the frontend (#4249), where changing it reaches every stored record at once. The canister stores these and never interprets them. - **A description is fixed at registration.** An entry that is advanced keeps what it was created with, so what a browser reports is a fact about a registration rather than about the last sign-in. A browser reporting something else is one this anchor has not seen, and the client is the party that decides so, by presenting a key pair no entry holds (#4271) — which is the path that already registers. - Entries ride on `identity_info` alongside `mcp_config`, via a new `BrowserInfo` candid type in an `opt` field, backwards compatible in both directions. Tests: `browser_tests` (19) covering registration and reuse, rotation including a lost response and ten rotations keeping one id, the one-key-one-entry invariant, and the cap dropping the least recently used — including twenty storage wipes interleaved with use of one browser leaving that browser listed. `browser_description` round-trips every token through storage, including the unrecognised ones, so adding a variant on one side without the other fails there rather than mapping to something else. PocketIC asserts an anchor from the previous release decodes with the new field absent. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. This is the caller #4266 and #4264 were waiting for, so it removes their `#[allow(dead_code)]` annotations. **`prepare_account_session` / `get_account_session`.** The first creates a session and signs its delegation to the II frontend's key; the second witnesses it. A separate pair from `prepare_account_delegation`, not an option on it: both mint, but one proves a live session and identifies the account by its principal while the other proves an access method and names the anchor outright. Merging them would mean one method with two authorizers and two argument shapes, and would drag the frontend's internal surface into the public API. **Everything that can refuse does so before anything is written**, cheapest first. The account check leads — an account the identity does not hold is the one failure a caller can provoke, and returning it after the writes would leave a browser registered for a sign-in that never happened — so a request that was never going to succeed does not pay for two P-256 verifications on the way to being told so. Nothing is revealed by that order: `check_authz_and_record_activity` above is the auth guard. The browser proof follows, using the verifier from #4264, and `create_session` takes the `VerifiedBrowserKeys` it produces rather than two byte strings. **The session credential is scoped to Internet Identity.** It exists to mint app delegations, which is an update call on this canister and nothing else, so it is signed with `targets = [id()]` and can be presented nowhere else. Leaving that to the II frontend would have left it to the party holding the session key, who can decline to add it. `permissions` stays absent for the same reason it is set on app delegations: minting is an update call, so a read-only session that could not make one could not sign in to an app at all — read-only travels on the app delegation instead. **`get_account_session` tells three failures apart.** No stored session for that id is `NoSuchSession`; a session that is present with no signature for the asked-for key and expiration is `NoSuchDelegation`, because the remedy is to ask with the parameters that were signed rather than to sign in again; a seed that will not derive is the salt being unset, which is `InternalCanisterError`. An over-long origin is that too, rather than a rejected message the caller cannot read as a response. **The request carries what the browser is, not a label for it.** `browser_description` holds the tokens the registry stores (#4242), and each token a client writes for itself — an unrecognised brand, an unrecognised system, and the hardware model — is bounded at 64 bytes. The named variants carry no text, so a description of nothing but those is within the limit whatever it says. Refused rather than truncated: a cut-off token would put a value in the record that no parser ever produced. **Every later failure traps rather than returning.** On the IC, returning an error commits state and only a trap rolls the message back, so once the browser registration is written a failure has to trap or a caller could be told "no" and still have a browser enrolled. **`valid_for`** is the lifetime the user chose at consent, clamped by the canister to between 10 minutes and 30 days. Every ceremony creates, so it always applies: the replacement's expiry is measured from the ceremony that made it, and no session is renewed in place. Tests: `integration/sessions.rs` (18) drives the real ceremony — creating and verifying a session, a request for another identity refused, the registry cap dropping the least recently used and ending its sessions, the key proof's rejections at the endpoint, rotation keeping the entry, a retired key returning as a new browser, and two browsers each keeping the description it registered with. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. Nothing imports this until the sign-in in #4273. The canister identifies a browser by a key it proves possession of (#4264), so the frontend has to hold one, and rotate it, so a key captured from one sign-in cannot claim that browser at the next. A non-extractable P-256 keypair in IndexedDB, per identity, plus a successor generated alongside it. A sign-in signs with the current key and announces the successor; the successor is promoted only once the canister has confirmed the sign-in. **Promoting after confirmation rather than before is what survives a lost response.** If the reply never arrives, the browser still holds the key the canister has, so its next attempt presents the same one rather than a successor the canister never saw — which the canister would treat as an unknown browser and enrol as a second entry for the same machine. **Sign-ins are serialised with a web lock**, because two at once would leave whichever wrote last holding a key the canister never accepted. Where the Web Locks API is missing the calls run unserialised: not blocking sign-in on it is the deliberate trade, and the window is one concurrent sign-in on the same device. **A browser that no longer matches what it registered as signs in as a new one.** The description it reported (#4270) is stored beside the key pair, and compared before anything is sent. A registered entry keeps the description it was created with — the canister ignores what a sign-in reports once an entry is being advanced — so a browser whose brand, system, form factor or model has changed would otherwise keep rotating an entry describing something it no longer is. Where it differs, this presents a fresh key pair, which no entry holds and which therefore registers under its own. Deciding here rather than being told keeps that a purely local operation: nothing has been sent when the comparison happens, so the entry left behind is untouched and a retry hits the same state and takes the same branch. The description is written with the key pair and never on its own, so the comparison is always against what the canister was actually sent. The private key never leaves IndexedDB and never appears in a delegation chain. Tests: `browser-key.store.test.ts` (21), including the successor promoted only after the call succeeds, a lost response leaving the current key in place, concurrent sign-ins serialised, the store working with the lock API absent, and a changed description signing in on a fresh key pair while an unchanged one keeps rotating. --------- Co-authored-by: Claude Opus 5 (1M context) <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.
Design: #4224. Overview: #4230. The verifier only — its caller is the sign-in ceremony in #4268, which also removes the
#[allow(dead_code)]this carries.A session records which browser created it, so the user can see where they are signed in and end one browser's access everywhere at once. That is only worth anything if the browser named in a sign-in is the browser that made it; otherwise an attacker holding an access method could attribute their session to a browser the user recognises, and the settings list would lie. Ingress messages are public, so a key presented in one sign-in can be read off the wire by anyone.
verify_browser_keyschecks two signatures before a sign-in may name a browser: the current key over the session key and the announced successor, and the successor over the session key and the current key. Each is made under its own domain separator,ii-session-browser-keyandii-session-browser-successor, so neither can be replayed in the other's role.Requiring possession of the successor is what closes the wire-reading attack: announcing a key you do not hold is impossible without its private half, so a key read off the wire cannot be claimed when its owner next presents one.
Verifying yields evidence, not a boolean. It returns a
VerifiedBrowserKeyswhose fields are private and whose only constructor is this function, andCreateSessionParamsrequires one — socreate_sessionregistering a browser from these keys cannot depend on a caller having remembered to check. That is also why the module sits at the crate root rather than undersessions: storage must not depend upward on sessions, and a P-256 verifier that knows nothing of either is something both layers may use.One encoding per key. P-256 only, no fallback. A key that does not parse, a signature of the wrong length, and an empty signature are each refused rather than skipped — and so is a compressed SEC1 point, although it is valid DER for the same key.
from_public_key_deraccepts both, so one private key had two spellings andresolve_browser, which compares the bytes it is handed, would have seen two browsers. WebCrypto emits only the uncompressed form, so nothing legitimate is turned away and nothing stored needs migrating.Tests:
browser_key::tests(11), one accepting case and ten refusals — including a successor signature replayed as the current one, a signature over another session key, one paired with a substituted successor, and a compressed key thatfrom_public_key_deraccepts.🤖 Generated with Claude Code
https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ