diff --git a/packages/hdwallet-core/src/wallet.ts b/packages/hdwallet-core/src/wallet.ts index abd9830d..ef92ed83 100644 --- a/packages/hdwallet-core/src/wallet.ts +++ b/packages/hdwallet-core/src/wallet.ts @@ -52,6 +52,15 @@ export interface ResetDevice { pin?: boolean; autoLockDelayMs?: number; u2fCounter?: number; + /** + * Collect dice rolls ON THE DEVICE and fold them into the internal entropy + * before it is committed. The rolls never reach the host, which is the + * whole point: desktop-entered dice only help while the device's own + * randomness stays secret, so they do not defend against a compromised + * host combined with a weak device RNG. Requires firmware >= 7.15.0; + * older firmware ignores the field. + */ + diceEntropy?: boolean; } export interface RecoverDevice { diff --git a/packages/hdwallet-keepkey/package.json b/packages/hdwallet-keepkey/package.json index 93e3f263..1d36e2e3 100644 --- a/packages/hdwallet-keepkey/package.json +++ b/packages/hdwallet-keepkey/package.json @@ -17,7 +17,7 @@ "dependencies": { "@ethereumjs/common": "^2.4.0", "@ethereumjs/tx": "^3.3.0", - "@keepkey/device-protocol": "https://github.com/keepkey/device-protocol.git#674777f6d4dd16e2b8c4c2df10608976375ee879", + "@keepkey/device-protocol": "https://github.com/keepkey/device-protocol.git#fbb483f1cadc6394d51566ca4be0cfe6daa40000", "@keepkey/hdwallet-core": "1.53.16", "@keepkey/proto-tx-builder": "^0.9.1", "@shapeshiftoss/bitcoinjs-lib": "5.2.0-shapeshift.2", diff --git a/packages/hdwallet-keepkey/src/keepkey.ts b/packages/hdwallet-keepkey/src/keepkey.ts index ddfb3446..6e8b3bb4 100644 --- a/packages/hdwallet-keepkey/src/keepkey.ts +++ b/packages/hdwallet-keepkey/src/keepkey.ts @@ -893,7 +893,15 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW GPK.setAddressNList(addressNList); GPK.setShowDisplay(showDisplay || false); GPK.setEcdsaCurveName(curve || "secp256k1"); - GPK.setScriptType(translateInputScriptType(scriptType || core.BTCInputScriptType.SpendAddress)); + // A BIP-86 account xpub uses the same serialization as a legacy account + // xpub. Current KeepKey firmware derives it from the BIP-86 path, but its + // GetPublicKey handler rejects SPENDTAPROOT. Keep P2TR on address/signing + // requests and use SPENDADDRESS only for this xpub-derivation message. + const publicKeyScriptType = + scriptType === core.BTCInputScriptType.SpendTaproot + ? core.BTCInputScriptType.SpendAddress + : scriptType || core.BTCInputScriptType.SpendAddress; + GPK.setScriptType(translateInputScriptType(publicKeyScriptType)); const event = await this.transport.call(Messages.MessageType.MESSAGETYPE_GETPUBLICKEY, GPK, { msgTimeout: showDisplay ? core.LONG_TIMEOUT : core.DEFAULT_TIMEOUT, @@ -929,6 +937,22 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW resetDevice.setAutoLockDelayMs(msg.autoLockDelayMs); } resetDevice.setU2fCounter(msg.u2fCounter || Math.floor(+new Date() / 1000)); + if (msg.diceEntropy) { + // Refuse rather than send. Firmware before v7.15.0 has no dice_entropy + // field, and nanopb SKIPS unknown fields instead of rejecting them + // (lib/transport/pb_decode.c:904, "No match found, skip data" — same on + // v7.14.1). So sending the flag to old firmware SUCCEEDS and quietly + // produces an ordinary RNG-only seed while the caller believes dice + // entropy was folded in. A silent downgrade of a security property the + // caller explicitly asked for is worse than a failed reset. + if (!(await this.supportsDiceEntropy())) { + throw new Error( + `Dice entropy requires KeepKey firmware v7.15.0 or later; device reports ${await this.getFirmwareVersion()}. ` + + `Refusing to reset, because this firmware would ignore the request and create an ordinary RNG-only wallet.` + ); + } + resetDevice.setDiceEntropy(true); + } // resetDevice.setWordsPerGape(wordsPerScreen) // Re-enable when patch gets in // Send await this.transport.call(Messages.MessageType.MESSAGETYPE_RESETDEVICE, resetDevice, { @@ -1359,6 +1383,20 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW return semver.gte(await this.getFirmwareVersion(), "v7.2.1"); } + /** + * Whether the device honours ResetDevice.dice_entropy (on-device dice rolls + * mixed into the seed entropy). Support starts in v7.15.0. + * + * Callers must check this before offering dice entropy: older firmware does + * not reject the unknown field, it silently ignores it, so an ungated + * request produces an ordinary RNG-only wallet with no error. Fails closed — + * an unreadable firmware version reports false rather than assuming support. + */ + public async supportsDiceEntropy(): Promise { + const version = await this.getFirmwareVersion(); + return !!semver.valid(version) && semver.gte(version, "v7.15.0"); + } + public async btcSignMessage(msg: core.BTCSignMessage): Promise { return Btc.btcSignMessage(this, this.transport, msg); } diff --git a/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts new file mode 100644 index 00000000..5d7be23c --- /dev/null +++ b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts @@ -0,0 +1,178 @@ +/** + * Unit tests for the dice-entropy flag on KeepKeyHDWallet.reset(). + * + * `diceEntropy` asks the device to collect dice rolls with its own button and + * fold them into the internal entropy before the seed is committed. The rolls + * never reach the host — that is the point, since desktop-entered dice only + * help while the device's own randomness stays secret. + * + * Two properties are load-bearing, and both are security properties: + * + * 1. The field is left ABSENT unless requested, so the message stays + * byte-identical to today's for every caller that does not ask for dice. + * + * 2. Requesting it against firmware that cannot honour it FAILS, rather than + * silently producing an ordinary RNG-only wallet. Firmware before v7.15.0 + * has no dice_entropy field, and nanopb SKIPS unknown fields rather than + * rejecting them (lib/transport/pb_decode.c:904, "No match found, skip + * data" — verified identical on v7.14.1). So the naive "just set it, old + * firmware will reject it" assumption is wrong: the reset would succeed + * and the caller would believe dice entropy was used when it was not. + */ +import * as Messages from "@keepkey/device-protocol/lib/messages_pb"; + +import { KeepKeyHDWallet } from "./keepkey"; + +/** + * Mock transport that answers GetFeatures with a chosen firmware version and + * accepts anything else. `features: null` simulates a device whose Features + * carry no version fields. + */ +function makeMockTransport(opts: { major?: number; minor?: number; patch?: number; features?: null } = {}) { + const { major = 7, minor = 15, patch = 0 } = opts; + const featuresMessage = + opts.features === null + ? { deviceId: "mock-device-id" } + : { deviceId: "mock-device-id", majorVersion: major, minorVersion: minor, patchVersion: patch }; + + return { + debugLink: false, + call: jest.fn().mockImplementation((messageType: number) => { + if (messageType === Messages.MessageType.MESSAGETYPE_GETFEATURES) { + return Promise.resolve({ message_type: "Features", message: featuresMessage }); + } + return Promise.resolve({ message_type: "Success", message: {} }); + }), + getDeviceID: jest.fn().mockResolvedValue("mock-device-id"), + keyring: { addAlias: jest.fn() }, + } as any; +} + +/** The ResetDevice protobuf the wallet handed to the transport, if any. */ +function sentResetDevice(transport: any): Messages.ResetDevice { + const call = transport.call.mock.calls.find((c: any[]) => c[0] === Messages.MessageType.MESSAGETYPE_RESETDEVICE); + expect(call).toBeDefined(); + return call[1] as Messages.ResetDevice; +} + +function resetWasSent(transport: any): boolean { + return transport.call.mock.calls.some((c: any[]) => c[0] === Messages.MessageType.MESSAGETYPE_RESETDEVICE); +} + +const BASE_RESET = { entropy: 128, label: "test", pin: true, passphrase: false } as const; + +describe("KeepKeyHDWallet.reset() dice entropy", () => { + it("sets dice_entropy when requested on supporting firmware", async () => { + const transport = makeMockTransport({ minor: 15 }); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true }); + + const sent = sentResetDevice(transport); + expect(sent.hasDiceEntropy()).toBe(true); + expect(sent.getDiceEntropy()).toBe(true); + }); + + it("leaves dice_entropy ABSENT when diceEntropy is omitted", async () => { + const transport = makeMockTransport(); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET }); + + expect(sentResetDevice(transport).hasDiceEntropy()).toBe(false); + }); + + it("leaves dice_entropy ABSENT when diceEntropy is explicitly false", async () => { + const transport = makeMockTransport(); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: false }); + + expect(sentResetDevice(transport).hasDiceEntropy()).toBe(false); + }); + + it("serializes without the field when absent, and with it when set", async () => { + // Round-trip through the wire format: "present but false" is not the same + // as absent, and only absent leaves old firmware's parse unchanged. + const off = makeMockTransport(); + await new KeepKeyHDWallet(off).reset({ ...BASE_RESET }); + expect(Messages.ResetDevice.deserializeBinary(sentResetDevice(off).serializeBinary()).hasDiceEntropy()).toBe(false); + + const on = makeMockTransport({ minor: 15 }); + await new KeepKeyHDWallet(on).reset({ ...BASE_RESET, diceEntropy: true }); + const withDice = Messages.ResetDevice.deserializeBinary(sentResetDevice(on).serializeBinary()); + expect(withDice.hasDiceEntropy()).toBe(true); + expect(withDice.getDiceEntropy()).toBe(true); + }); + + it("still populates the other reset fields when dice entropy is on", async () => { + const transport = makeMockTransport({ minor: 15 }); + await new KeepKeyHDWallet(transport).reset({ + entropy: 256, + label: "dice wallet", + pin: true, + passphrase: true, + diceEntropy: true, + }); + + const sent = sentResetDevice(transport); + expect(sent.getStrength()).toBe(256); + expect(sent.getLabel()).toBe("dice wallet"); + expect(sent.getPinProtection()).toBe(true); + expect(sent.getPassphraseProtection()).toBe(true); + expect(sent.getDisplayRandom()).toBe(false); + }); +}); + +describe("KeepKeyHDWallet.reset() refuses a silent dice downgrade", () => { + // The regression this whole gate exists for. Old firmware does not reject + // dice_entropy — it skips it — so without the gate the wallet is created + // with RNG-only entropy and nobody is told. + it("throws instead of resetting when firmware predates v7.15.0", async () => { + const transport = makeMockTransport({ minor: 14, patch: 1 }); + + await expect(new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true })).rejects.toThrow( + /Dice entropy requires KeepKey firmware v7\.15\.0 or later/ + ); + // The critical assertion: nothing was sent. A thrown error after a + // successful reset would still have created an RNG-only wallet. + expect(resetWasSent(transport)).toBe(false); + }); + + it("names the offending firmware version in the error", async () => { + const transport = makeMockTransport({ major: 7, minor: 14, patch: 1 }); + const err = await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true }).catch((e: any) => e); + + expect(String(err.message)).toContain("v7.14.1"); + }); + + it("fails closed when the firmware version is unreadable", async () => { + const transport = makeMockTransport({ features: null }); + + await expect(new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true })).rejects.toThrow( + /Dice entropy requires KeepKey firmware/ + ); + expect(resetWasSent(transport)).toBe(false); + }); + + it("still resets normally on old firmware when dice entropy is not requested", async () => { + // The gate must not become a general old-firmware block. + const transport = makeMockTransport({ minor: 14, patch: 1 }); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET }); + + expect(resetWasSent(transport)).toBe(true); + expect(sentResetDevice(transport).hasDiceEntropy()).toBe(false); + }); +}); + +describe("KeepKeyHDWallet.supportsDiceEntropy()", () => { + it.each([ + [{ minor: 15, patch: 0 }, true], + [{ minor: 15, patch: 1 }, true], + [{ major: 8, minor: 0, patch: 0 }, true], + [{ minor: 14, patch: 1 }, false], + [{ minor: 10, patch: 0 }, false], + ])("reports %o as %s", async (version, expected) => { + const wallet = new KeepKeyHDWallet(makeMockTransport(version as any)); + expect(await wallet.supportsDiceEntropy()).toBe(expected); + }); + + it("reports false when Features carry no version fields", async () => { + const wallet = new KeepKeyHDWallet(makeMockTransport({ features: null })); + expect(await wallet.supportsDiceEntropy()).toBe(false); + }); +}); diff --git a/packages/hdwallet-keepkey/src/taproot.test.ts b/packages/hdwallet-keepkey/src/taproot.test.ts index c5942687..2b95f8d5 100644 --- a/packages/hdwallet-keepkey/src/taproot.test.ts +++ b/packages/hdwallet-keepkey/src/taproot.test.ts @@ -63,6 +63,31 @@ describe("KeepKey Taproot host support", () => { ).resolves.toBe("bc1ptest"); }); + it("derives a BIP-86 account xpub with the firmware-compatible SPENDADDRESS wire type", async () => { + const call = jest.fn().mockImplementation((messageType: number, msg: Messages.GetPublicKey) => { + expect(messageType).toBe(Messages.MessageType.MESSAGETYPE_GETPUBLICKEY); + expect(msg.getAddressNList()).toEqual(BIP86_ACCOUNT); + expect(msg.getCoinName()).toBe("Bitcoin"); + expect(msg.getScriptType()).toBe(Types.InputScriptType.SPENDADDRESS); + + const response = new Messages.PublicKey(); + response.setXpub("xpub-bip86"); + return Promise.resolve({ proto: response }); + }); + const wallet = new KeepKeyHDWallet(makeMockTransport(call)); + + await expect( + wallet.getPublicKeys([ + { + coin: "Bitcoin", + addressNList: BIP86_ACCOUNT, + curve: "secp256k1", + scriptType: core.BTCInputScriptType.SpendTaproot, + }, + ]) + ).resolves.toEqual([{ xpub: "xpub-bip86" }]); + }); + it("requires the firmware-reported supports_taproot capability", async () => { const supported = new KeepKeyHDWallet( makeMockTransport(jest.fn().mockResolvedValue({ message: { supportsTaproot: true } })) diff --git a/yarn.lock b/yarn.lock index 418ce772..4c19699a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1151,9 +1151,9 @@ "@jridgewell/resolve-uri" "^3.1.0" "@jridgewell/sourcemap-codec" "^1.4.14" -"@keepkey/device-protocol@https://github.com/keepkey/device-protocol.git#674777f6d4dd16e2b8c4c2df10608976375ee879": +"@keepkey/device-protocol@https://github.com/keepkey/device-protocol.git#fbb483f1cadc6394d51566ca4be0cfe6daa40000": version "7.14.1" - resolved "https://github.com/keepkey/device-protocol.git#674777f6d4dd16e2b8c4c2df10608976375ee879" + resolved "https://github.com/keepkey/device-protocol.git#fbb483f1cadc6394d51566ca4be0cfe6daa40000" dependencies: google-protobuf "3.21.4" pbjs "^0.0.5"