feat(terminal): add deriveEntropy(session, productId, key) (RFC-0007) - #260
feat(terminal): add deriveEntropy(session, productId, key) (RFC-0007)#260ReinhardHatko wants to merge 1 commit into
Conversation
📦 Bundle size impactComparing
Thresholds — warn: ≥10% or ≥5.0 KB · fail: ≥20% or ≥15.0 KB (bundled). Percentage only applies once the baseline is ≥ 10 KB. |
|
Closing — the implementation here is incorrect and should not merge. It derives entropy as Constructions ruled out (both mismatch the host):
Root problem: The underlying need still stands — see #254, which I'm reframing accordingly. |
|
Reopened as a draft — parking, not abandoning. The public surface ( |
|
Updated with the correct derivation (still draft). The original Changes:
Open question for reviewers: this re-derives the RFC-0007 scheme client-side. To avoid two implementations drifting, |
ReinhardHatko
changed the title
1ed6ade to
3cc06b8
Compare
ReinhardHatko
changed the title
Client-side RFC-0007 product-entropy derivation for terminal (QR/SSO, out-of-container) sessions, at parity with @parity/product-sdk-host's deriveEntropy. The paired UserSession carries rootEntropySource (layer 1), so layers 2 and 3 are computed locally with no host round-trip: perProduct = blake2b256(rootEntropySource, key = blake2b256(utf8(productId))) entropy = blake2b256(perProduct, key = key) Byte-for-byte identical to the host's host_derive_entropy handler (@novasamatech/host-container's deriveProductEntropyFromSource), so keys derived here interoperate with in-container apps for the same wallet + product + key. Deterministic from the wallet, so derived keys survive device loss. In-source golden-vector test pins the construction to host-container@0.8.9. Closes #254.
3cc06b8 to
334b44b
Compare
Imod7
left a comment
Imod7
left a comment
There was a problem hiding this comment.
Three more notes on top of the inline comments — none of them blocking.
1. dist ships the test suites. Pre-existing, but this PR adds to it: terminal/dist/index.js ships all 14 in-source describe blocks, including entropy.ts's golden-vector hex. The define: {"import.meta.vitest": "undefined"} rewrites the condition, but esbuild only drops the dead block with treeshake: true - packages/signer has the flag and ships 0 of them. fix/tsup-treeshake-toplevelawait adds the flag to 14 packages but not terminal.
→ Action: add treeshake: true to packages/terminal/tsup.config.ts, or fold terminal into that branch.
2. "Closes #254" overstates what ships. This adds deriveEntropy but not #254's getEncrSecret / getEncrPublicKey / createSharedSecret. Fine as a cut - entropy is the drift-prone part, and @parity/product-sdk-crypto already has boxEncrypt/sealedBoxEncrypt for the rest. But as written, the issue closes with its encryption helpers undelivered.
→ Action: either narrow the description to "Refs #254" and open a follow-up for the three helpers, or add them here.
3. The RFC-0007 product-entropy derivation is specified, not just published. host-spec §C.8 defines it - the three keyed BLAKE2b-256 layers over session secret, productId and caller key - at spec/C-account-derivation.md, which truapi's Rust implementation cites at a pinned commit. Worth pointing at the spec rather than at host-container: mirrors drift, and these five already disagree on whether an empty caller key is legal.
→ Recommendation: cite host-spec §C.8 instead of the implementation in the two places that name it - the changeset (pending-changesets/terminal-derive-entropy.md: "byte-for-byte identical to the host's host_derive_entropy", "pins the construction against host-container"), which ships to the published changelog, and the module header comment.
| * @param productId - The calling product's dotNS identifier, e.g. `"my-app.dot"`. | ||
| * The host scopes entropy per product; pass the same identifier the host would. |
There was a problem hiding this comment.
Please document the productId contract - it's now the only thing between a caller and a silent mismatch. The host doesn't take it from the caller - it computes ${label}.dot from the iframe's deployment label (dotli's packages/ui/src/container.ts passes exactly that into deriveProductEntropyFromSource). So the correct value is that deployment's label, and it differs per deployment: production, each PR preview, and local dev (localhost:5173.dot) all yield a different id. A CLI hardcoding the production id derives valid-but-different bytes against a preview, with no error anywhere.
Please spell out that per-deployment rule, add "must match the productId you pass to requestResourceAllocation (#262)", and say why it's required rather than defaulting to adapter.appId.
| test("matches host-container deriveProductEntropyFromSource (golden vector)", () => { | ||
| // Computed with @novasamatech/host-container@0.8.9's | ||
| // deriveProductEntropyFromSource(fill(1), "my-app.dot", [1,2,3,4]). | ||
| expect(toHex(deriveEntropy(session, "my-app.dot", new Uint8Array([1, 2, 3, 4])))).toBe( | ||
| "993750d5f3f4b941cef5a8084fdd0bcd6a6946fdc0e1fe87c0c575fe65e7dc03", | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The golden vector pins this file against itself. Nothing here can reproduce the hex, and it can't catch host-container drift - which is what the NOTE above says we're guarding against.
Cheapest fix, and stronger than minting a new hex: reuse the vectors the other implementations already share with each other. truapi's Rust tests carry vectors copied byte-for-byte from polkadot-app-ios-v2's ProductRootEntropyDeriverTests. I ran your math against both sets and got 6/6 matches - e.g. secret = bytes 0..31, productId "myapp.dot", key [1] gives 4bafd6a34182959bad8914dcff88c6b6842d551d6f0067afbd407e9584223404. They start from the raw secret rather than rootEntropySource, so the test needs the layer-1 step first (blake2b256 keyed with "product-entropy-derivation") - one line. No new dependency, and it pins us against Rust and Swift rather than against a single TS package.
@novasamatech/host-container is also public on npm, so the devDependency plus an entropy.interop.test.ts asserting equality against the real deriveProductEntropyFromSource is worth having too - same pattern as testing.interop.test.ts. The shared vectors prove we match today; the interop test catches future drift on its own. Keep the golden vector as well; it documents the expected bytes for a human reader.
| */ | ||
| export function deriveEntropy( |
There was a problem hiding this comment.
deriveEntropy reads nothing account-identifying, and terminal supports multiple paired sessions (waitForSessions). Pass the wrong one and you get valid entropy for a different wallet: the intended user can't decrypt, the other wallet's holder can. Explicit session is the right design - could you add a @remarks telling callers to pin identity via the exported sessionRootPublicKey(session) before deriving long-lived keys? Same block could note that the return value is raw key material - don't log it or persist it unwrapped.
| ); | ||
| }); | ||
|
|
||
| test("rejects a key outside 1..32 bytes", () => { |
There was a problem hiding this comment.
[nice-to-have]: The 1..32 tests only probe outside the boundary (0 and 33). Could you also assert that 1 and 32 are accepted? An off-by-one to >= 32 would pass the suite while rejecting the max-length key the host allows. Same for rootEntropySource.length !== 32 on line 60: only the missing-field case is covered. Note @noble accepts keys up to 64 bytes, so line 65 is the only thing enforcing the host's bound — a compatibility contract, not defensive boilerplate.
Worth knowing that the implementations already disagree at exactly this boundary: iOS and Android only check <= 32 and accept an empty key, where you, truapi's Rust and host-container all reject it. So these two bounds are the one place a cross-platform mismatch is already live, which is more reason to assert both ends rather than just outside them.
| * @returns 32 bytes of derived entropy. | ||
| * @throws if the session lacks `rootEntropySource`, or `key` is not 1..32 bytes. | ||
| */ | ||
| export function deriveEntropy( |
There was a problem hiding this comment.
#262 made productId an optional option defaulting to adapter.appId; here it's a required positional. Required is correct for entropy — a wrong default produces undecryptable data rather than a loud error, and #262 notes d3pot's appId differs. Could you add one JSDoc line saying it's deliberate, so nobody "fixes" it into a default?
| * NOTE: this re-derives the RFC-0007 scheme locally and MUST stay byte-identical | ||
| * to `host-container`'s `deriveProductEntropyFromSource`. The golden-vector test | ||
| * below guards against drift, but the derivation ideally belongs in a shared | ||
| * crypto package that both host-container and terminal import (see PR #260). |
There was a problem hiding this comment.
On your open question: keep the derivation here, but split it in three. Doesn't block the PR.
The shared package the NOTE wants can't exist — this algorithm has five implementations in three languages (host-container, truapi's Rust host_logic/entropy.rs, Android's RealDeriveEntropyUseCase.kt, iOS's ProductRootEntropyDeriver.swift, and this one). Swift and Kotlin can't import a TS package, so the shared artifact is the spec (host-spec §C.8) and the safety net is shared test vectors — which Rust and iOS already swap byte-for-byte. Importing host-container here isn't a shortcut either — nothing terminal already depends on exports the derivation (host-papp carries rootEntropySource, host-api only the error type), so it trades the novasama import we're shedding for a new one.
keys/src/product-account.ts already answers the in-repo version of the question. Nothing here needs doing in this PR — an issue is enough for now, so the NOTE has something to link to, and the move lands as its own PR later (~40 lines, no behaviour change):
blake2b256Keyed→crypto/src/hashing.ts: the missing sibling of theblake2b256already there, and we're the only one of the five platforms without a keyed BLAKE2b in its crypto layer. (utils/src/hashing.tshas a byte-identical copy of those three, so worth settling which is canonical - I'd saycrypto, wherekeysalready importsblake2b256from.)- the pure derivation →
keys/src/product-entropy.ts, besideproduct-account.ts(same species: client-side mirror of a wallet-side derivation, header listing its mirrors, frozen-vector test).terminalalready depends onkeys, so this adds no dependency. deriveEntropy(session, productId, key)stays exactly as you wrote it — the session wrapper. Public API and changeset unchanged.
And please turn the NOTE into the decision plus an issue link - an open question in merged code stays open. While rewriting it, cite host-spec C.8 rather than host-container's implementation (mirrors drift; these five already disagree on whether an empty key is legal), and steal iOS's clarification that layer 1's input is raw BIP-39 entropy, not the 64-byte PBKDF2 seed. Happy to open the issue and write up the full reasoning there.
2 participants
Closes #254.
What
Adds
deriveEntropy(session, productId, key): Uint8Arrayto@parity/product-sdk-terminal— client-side RFC-0007 product-entropy derivation for QR/SSO (out-of-container) sessions, at parity with@parity/product-sdk-host'sderiveEntropy.Why
The host package derives entropy in-container via
truApi.entropy.derive. Terminal apps run out-of-container and had no equivalent, even though the pairedUserSessionalready carriesrootEntropySource(RFC-0007 layer 1). Consumers were forced to import@novasamatech/host-pappinternals and re-implement the derivation.How
The session's
rootEntropySource(layer 1) lets layers 2 and 3 be computed locally, with no host round-trip:This is byte-for-byte identical to the host's
host_derive_entropyhandler — i.e.@novasamatech/host-container'sderiveProductEntropyFromSource— so entropy derived here matches what an in-container app gets from@parity/product-sdk-host'sderiveEntropyfor the same wallet + product + key. Keys therefore interoperate across web (in-container) and terminal (QR/SSO) clients. Deterministic from the wallet (not the device), so derived keys survive device loss as long as the wallet is recoverable.productIdis the calling product's dotNS identifier (e.g.my-app.dot) — the host scopes entropy per product.keyis the caller key, 1..32 bytes (the layer-3 BLAKE2b key).Notes
rootEntropySourceis read as the real typedUserSessionfield (an upstream rename is a compile error here), with a runtime guard for older V1 persisted sessions that predate RFC-0007.import.meta.vitest) cover determinism, 32-byte length, product/key scoping, the missing-field guard, key-length bounds, and a golden-vector assertion pinning the output to@novasamatech/host-container@0.8.9'sderiveProductEntropyFromSource.🤖 Generated with Claude Code