feat(session): per-profile methods over derive_identity_sk_at (#1024) - #5
Merged
Conversation
Add profile_public_key / profile_sign / profile_derive_symmetric_key to
UnlockedMasterSeed, deriving each profile's identity key from the master seed at
m/12381'/8444'/9'/{profile_ix}' via dig-identity 0.5.0's derive_identity_sk_at.
profile_derive_symmetric_key reuses the shared frozen HKDF
(derive_symmetric_key_from_scalar); the HKDF is not duplicated.
ADDITIVE (§5.1): all 0.2.0/0.3.0 methods unchanged. profile_ix == 0 is
byte-identical to the default methods (derive_identity_sk_at(m,0) ==
derive_identity_sk(m)). dig-identity dep 0.4 -> 0.5; no dig-wallet-backend dep
(reference-DOWN @10 preserved). Per-profile scalars/DEKs zeroized.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
MichaelTaylor3d
commented
Jul 21, 2026
MichaelTaylor3d
left a comment
MichaelTaylor3d
left a comment
Contributor
Author
There was a problem hiding this comment.
Correctness gate: PASS
Fresh-context correctness review of v0.4.0 additive per-profile methods.
Verified
- 3 methods (
src/master_seed.rs:194-246):profile_public_key,profile_sign,profile_derive_symmetric_keyeach derive the profile scalar viaderive_identity_sk_at(&master_secret_key_from_seed(seed), profile_ix)(path m/12381'/8444'/9'/{profile_ix}'). The DEK reuses the shared frozenderive_symmetric_key_from_scalar(src/unlocked.rs:153) — HKDF is NOT duplicated. Reads cleanly, doc-comments on every public method (§2.5/§6.2). - profile_ix==0 byte-identity is runtime-ENFORCED, not merely asserted in docs: PROF-1/2/3 assert
profile_*(0,…) ==the default methods byte-for-byte; the propertyderive_identity_sk_at(master,0)==derive_identity_sk(master)is thus test-gated (CI green ⇒ it holds in dig-identity 0.5.0). PROF-4 pins the canonicalderive_identity_sk_at; PROF-5 distinct+deterministic; PROF-6 cross-profile verify isolation (sig under wrong profile key rejected); PROF-7 profile-1 golden. - Goldens frozen / regression-proof: PROF-7 (and MS-3) compare against
dig_app_reference_dek, an independent oracle re-implementing dig-app's HKDF from FROZEN literal constants (version prefix2, saltdig-app:dek-salt:v1, HKDF-SHA256, label verbatim). Any drift in either production path fails the test — consistent with the established 0.3.0 golden convention. - Additive (§5.1): 0.2.0/0.3.0 methods + C-1..10 + MS-1..8 unchanged; test diff is add-only; version 0.3.0→0.4.0 (minor = new capability, correct); version-increment gate green.
- Deps: dig-identity "0.5" (crates.io), Cargo.lock relocked; no dig-wallet-backend (@10, reference-DOWN respected); no git deps.
- SPEC.md §3.3.1 + PROF-* conformance list + profile_ix==0 invariant + derivation path documented, matches code.
- Gates: 8/8 checks green — coverage 97.65% (≥80%), CodeQL/Analyze pass, commitlint, version-increment. Zero unresolved review threads.
Non-gating follow-up (resolved, not a merge blocker): pre-existing transitive chia-bls skew — 0.22.0 and 0.26.0 both in Cargo.lock (dig-keystore vs dig-identity). Both compile, tests green; not introduced by this PR. Recommend an alignment follow-up ticket.
Verdict: PASS. Merge is unblocked (orchestrator owns the squash-merge).
MichaelTaylor3d
marked this pull request as ready for review
July 21, 2026 01:21
MichaelTaylor3d
added a commit
that referenced
this pull request
Jul 29, 2026
…nvelope, recovery_phrase() (#6) Closes the #1759 decision by implementing option **(b) conform to the standard Chia/BIP-39 derivation**, plus option (c)'s versioned envelope so the legacy shape fails closed rather than being reinterpreted. ## The bug A DIG 24-word recovery phrase resolved to a **different address** than Sage and every other conforming Chia wallet. The 32 stored bytes were handed straight to `SecretKey::from_seed`, skipping BIP-39's PBKDF2 expansion. That does not fail — it derives a different, entirely plausible wallet, so a user who lost their machine and typed their DIG phrase into Sage would see an empty account, a valid-looking phrase, and no error at all. ## The fix The 32 stored bytes are now BIP-39 **entropy**, re-expanded `entropy → 24 words → to_seed("") → SecretKey::from_seed` at `master_seed()`. Identity, per-profile identity, per-profile DEK and the wallet seed all read that one expanded seed, so no two can disagree about the root. **`SEED_LEN` did not need to grow to 64.** BIP-39 entropy for 24 words IS 32 bytes and re-expands losslessly, so there is no at-rest length change and no §5.1 stored-secret length break — the ticket's premise that this requires 32→64 is refuted. ### The versioned envelope, and why legacy must fail closed A legacy blob's 32 bytes (raw seed) and a current blob's 32 bytes (entropy) are **byte-for-byte indistinguishable**. Silently reinterpreting one as the other hands back a *working* handle for the *wrong* account — strictly worse than an error. So a stored blob now DECLARES what its bytes mean: - Sealed via `dig_keystore::opaque` (magic `DIGOP1`) — the same audited container (Argon2id + AES-256-GCM + CRC-32) every `Keystore<K>` file uses — with plaintext `version:u8 || kind:u8 || entropy`. Kinds are append-only (§5.1). - A pre-envelope `DIGVK1` blob is detected **before** decryption and rejected with `SessionError::LegacySeedFormat`, which tells the caller to re-enrol from the phrase. - An unrecognised version or kind is refused, never guessed. This needed **no dig-keystore change**: `format::is_known_magic` is a closed allow-list there, so a new `KeyScheme` defined outside dig-keystore could not be decoded, and a `KeyScheme`'s fixed `SECRET_LEN` cannot carry a version tag anyway. `opaque` is the sanctioned arbitrary-length door into the same container. That avoided a third-repo write and an extra publish hop while a consumer lane was blocked. ### New API - `UnlockedMasterSeed::recovery_phrase() -> Zeroizing<String>` over **`&self`** — showing a user their backup must not cost them their session. - `Session::enroll_from_recovery_phrase(...)` — the restore-on-a-new-machine path. Accepts any capitalisation and any whitespace run; rejecting a correct phrase over a capital letter is a trap, not a safety measure. ## BREAKING (0.4.0 → 0.5.1) - `SEED_LEN` is **removed**, replaced by `ENTROPY_LEN` (32, stored) and `MASTER_SEED_LEN` (64, expanded). Removing the ambiguous name is deliberate: a stale call site now fails to **compile** rather than deriving a silently wrong key. - `master_seed()` returns the 64-byte expanded seed. - For a given 32 stored bytes this **MOVES the identity public key and the profile DEK**. ### Legacy accounts EXIST — an earlier draft of this PR said the exposed population was zero. That was FALSE. The security gate proved it, and the correction now lives in `SPEC.md` §3.3.0, the `LegacySeedFormat` rustdoc, the README, and dig-account's `keys/dek.rs` + `SPEC.md` §10: - dig-app `origin/main` (3.5.0) depends on the **published** `dig-session = "0.4"` + `dig-account = "0.1.1"`, and `dig-app-core/src/account/boot.rs:122`/`:149` **auto-enrol `account.default`** into `FileBackend::new(brand_dir.join("account"))` at **first boot on every Windows/macOS host, with no user action**. - The artifact was found on the reference machine: `%LOCALAPPDATA%\DigNetwork\account\account.default.dks`, **105 bytes**, magic `DIGVK1` — 53-byte header + (32 secret + 16 GCM tag) + 4 CRC = exactly one legacy 32-byte raw seed. So the population is *every installed dig-app on Windows/macOS that has booted once*, at least one verified. **The true, narrower reason the re-pin is sound:** no sealed **artifact** keyed by the old derivation exists — no `profiles/` dir, no sealed profile blobs, no wallet store, no funded account (money path parked unmerged on #1702), pre-release per §3.7. Nothing *encrypted* under the old DEK became unreadable, which is the only thing re-pinning a DEK can break. A **further** change to a stored-secret derivation needs a migration, not a second re-pin. **That legacy file must NOT be treated as safe to delete.** It is password-sealed and nobody has read the OS credential store, so a balance cannot be ruled out. ### A legacy account is WEDGED, not merely fail-closed — and the crate now says so `unlock_master_seed` returns `LegacySeedFormat` and never a handle (`session.rs:180` -> `envelope.rs:127`); `enroll_master_seed` at the same key returns `AlreadyExists` (`envelope.rs:90`) because enrolment refuses to overwrite a custody root; and no pre-envelope release exposed `recovery_phrase()`, so the user was never shown 24 words. There is no in-crate route back. The code fix belongs on the **dig-app adoption path** (detect -> preserve/back-up -> re-enrol, surfaced in the UI rather than a `tracing::warn!` that silently yields "signing channel not started"). What this PR adds is the **contract**, so the wedge cannot be adopted blindly: the `LegacySeedFormat` rustdoc spells out the four required remediation steps, and `SPEC.md` §3.3.0 makes a legacy-detection-and-re-enrolment path a **normative requirement of adopting 0.5**. dig-account #5 carries the same requirement in its §10 + README. ## Blast radius checked `master_seed()` — every in-crate caller (`public_key`, `sign`, `signing_fn`, `derive_symmetric_key`, `profile_public_key`, `profile_sign`, `profile_derive_symmetric_key`) plus `Session::{enroll_master_seed, unlock_master_seed}`, all updated in this diff. Cross-repo consumers of the changed symbols: **`dig-account` only** (`store.rs`, `session.rs`, `unlocked.rs`, `auth/policy.rs`, `keys/dek.rs`, `profile_mint.rs`, `signer.rs`, `wallet/authorizer.rs`) — migrated in the same unit of work on DIG-Network/dig-account#5. `dig-wallet-backend` needs **no production change** (already standard); confirmed by a new test on DIG-Network/dig-wallet-backend#23. Grepped the whole superproject for `SEED_LEN` / `enroll_master_seed` / `unlock_master_seed` / `UnlockedMasterSeed` — no other consumer. `UnlockedIdentity` and the whole `enroll_identity` / `L1WalletBls` path are untouched, so the 0.2.0 identity blobs are unaffected. **⚠️ Risk: HIGH** — this is custody code and it moves a stored-secret derivation. It is safe ONLY under the zero-population finding above. ## Memory hygiene: `bip39`'s `zeroize` feature is now ENABLED An earlier revision of `master_seed.rs` claimed "bip39 2.x exposes no `zeroize` feature". **Wrong** — bip39 2.2 derives `Zeroize + ZeroizeOnDrop` on `Mnemonic` behind an *optional-dependency* feature, which is why it does not appear in the `[features]` table (I had read only that table). `expand_entropy` builds a `Mnemonic` on **every** derivation and its word indices are a complete copy of the account root, so without the feature un-wiped copies of the root accumulated in memory. Feature enabled, comment corrected, and **CONF-11 asserts the trait bound at compile time** — dropping the feature now fails to build. Residual, documented, upstream-owned: bip39 does not wipe its intermediate `to_string()` heap buffer or its transient `[u8; 33]` entropy+checksum stack array. ## Tests (43 total, coverage 97.52% lines) The load-bearing one is **cross-implementation**: a fixed public mnemonic → a **hardcoded literal** bech32m address produced independently via `chia-wallet-sdk`. Computing both sides live would let a dependency bump move them together and mask a regression (the `dig-keystore` `l1_wallet_bls.rs` precedent). | | property | |---|---| | CONF-1 | `abandon…art` → `xch16grurcglcwcv6arjarr720yd9wqhp9gkx3k8h25lhwg8pl7vl6ysuax0gy` (the Sage address) | | CONF-2 | non-vacuity: the fixture still reproduces the OLD `xch1jcvy96pjk…` and the crate no longer does | | CONF-3 | a legacy blob fails closed — no handle, no address | | CONF-4 / 4b | phrase round-trip, and with **two accounts** each phrase restores ITS OWN account | | CONF-5 | `recovery_phrase()` does not consume the handle | | CONF-6 | identity + DEK derive from the EXPANDED seed (placement, not just outcome) | | CONF-7..10 | invalid phrases rejected without echoing them; user-typed case/whitespace; no overwrite; unknown envelope version/kind refused | | CONF-11 | `Mnemonic: ZeroizeOnDrop` — a COMPILE-TIME pin on the `zeroize` feature | | CONF-12 | a **valid** 12/15/18/21-word phrase is REJECTED, never a panic | CONF-1/4/5/6/8 now drive from the **non-degenerate** fixture rather than the all-zero one, so the account-identity-blind class has three nets instead of one. The two `tests/master_seed.rs` DEK goldens are now **hex literals** (`7bf5db71...`, `eddb8924...`) as their comments always claimed — both sides had been computed live from `bip39` + `dig-identity`, so a bump in either could have moved them together; the independently-reconstructed dig-app reference is still asserted alongside, since the literal alone cannot distinguish "frozen bytes drifted" from "the two HKDFs diverged". ### Mutations executed (each reverted; suite green after) | mutation | result | |---|---| | `expand_entropy` returns raw entropy padded to 64 | **9 tests fail** | | `MASTER_SEED_LEN = 32` + expansion removed (literally the pre-#1759 code) | CONF-1, CONF-2, CONF-6 fail | | `to_seed("dig")` — non-empty passphrase | CONF-1 fails | | legacy-magic guard deleted from the envelope reader | CONF-3 fails | | `recovery_phrase()` ignores `self.entropy` | **SURVIVED at first** — see below | | `enroll_from_recovery_phrase` ignores the phrase | CONF-4b fails | | **delete the `entropy.len() != ENTROPY_LEN` guard** (`master_seed.rs:178`) | CONF-12 fails — panics at `copy_from_slice` | | **drop the `bip39` `zeroize` feature** | CONF-11 fails to COMPILE | | `recovery_phrase()` ignores the root, re-run after widening the fixtures | now **3** tests fail, not 1 | The guard-deletion mutation is the second false green this lane found: **deleting the entropy-length guard left all 41 tests green**, while making a perfectly valid 12-word BIP-39 phrase a reachable `copy_from_slice` panic on the restore path — CONF-7's malformed cases all fail inside bip39 first and never reach the guard. CONF-12 uses valid shorter phrases, with a fixture check asserting bip39 itself accepts them, so the rejection provably comes from DIG's 24-word requirement. **The first false green, found and fixed earlier:** `recovery_phrase()` returning a phrase derived from a *fixed* entropy passed every test, because the fixture was the **all-zero-entropy** mnemonic — the strongest-looking canonical vector is exactly the one blind to *which* account is in play. A bug of that shape would hand a user a phrase that restores someone else's account. Fixed by adding CONF-4b: a second account with unrelated entropy (`fog spot notable…equal` → `xch1vpxzuu6aqfu790qcrcppcr2gmju4f5tpuuznuv2lx3g79v2jxc7qxttpzt`) as a truthful control, so each phrase must resolve to its own frozen address. The mutation now fails. ## Docs `SPEC.md` gains §3.3.0 (the root is entropy; every derivation reads the expanded seed; empty passphrase) and §3.4 (the envelope + the fail-closed rule), the CONF-1..10 conformance list, and two custody invariants (no secret in an error message; never log a phrase). README gains the account/phrase section. `dig-logging`'s BIP-39 redactor (`redact.rs` `MIN_MNEMONIC_RUN = 12`, case-insensitive) covers the 24-word form, but `recovery_phrase`'s docs now warn that it is **not protection**: `Zeroizing<String>` derefs to `String`, so `info!("{phrase}")` compiles, and the redactor runs at **bundle time**, so a logged phrase sits in plaintext on disk until a bundle is built. dig-session itself has no logging dependency, so it adds no log surface of its own. INFO, no action: every derivation now costs one PBKDF2-HMAC-SHA512/2048 — **0.84 ms measured**. Not an amplification lever, but `sign()` is no longer free. Refs DIG-Network/dig_ecosystem#1759 --------- Co-authored-by: Claude <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
1 participant
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.
PR0.75b of the #1024 PR0.75 cascade. Adds per-profile identity methods to
UnlockedMasterSeedover dig-identity 0.5.0'sderive_identity_sk_at.What changed
dig-identitydep 0.4 -> 0.5 (crates.io, live); version 0.3.0 -> 0.4.0; Cargo.lock relocked.UnlockedMasterSeed:profile_public_key(profile_ix: u32) -> [u8; 48]profile_sign(profile_ix: u32, msg: &[u8]) -> [u8; 96]profile_derive_symmetric_key(profile_ix: u32, label: &[u8]) -> Zeroizing<[u8; 32]>m/12381'/8444'/9'/{profile_ix}'viaderive_identity_sk_at. The DEK path REUSES the shared frozen HKDF (derive_symmetric_key_from_scalar) — HKDF not duplicated.Byte-identity (§5.1, HARD)
derive_identity_sk_at(m, 0) == derive_identity_sk(m), so for every seed/msg/label:profile_public_key(0) == public_key(),profile_sign(0,m) == sign(m),profile_derive_symmetric_key(0,label) == derive_symmetric_key(label)— byte-for-byte. Proven PROF-1..PROF-7 + golden vectors.Golden (fixed seed 0x00..0x1f, label
dig-app:profile-dek:v2):824d13413aa564f0747468dc54f94e33956d8bd85578cfac2bd17ee3b15fdfcb87f14863a1ae916df9d08ca7465bcbac== default pk (byte-identical)a9cf779643cee48e2713d1a16154b8e7bc4ed14ad765fd5305b2583b6737d868aa99dc87a9f6f2dc3a9462fe2be67294e50056ace5a899659740c8fb6d3b2a438543ec42b46aa312d85dadbbbf826f6e(distinct)a4244a5e93dd0f60856173c66319cd2949df0a1da7ff7543477a9555d1541dd4(distinct, deterministic)Blast radius
Additive-only; no existing symbol edited (0.2.0/0.3.0 methods + C-1..C-10 + MS-* unchanged). No consumer edited (dig-app PR2 adopts downstream). No
dig-wallet-backenddep (reference-DOWN @10 preserved). Per-profile scalars + DEKs wrapped inZeroizing; #908 identity boundary held.Evidence
Custody + new key-model API -> full triple gate by orchestrator before merge. Do NOT merge without it.