From 6dd3055fdefd036bd085b783e667ae16f9ba404d Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Tue, 8 Jul 2025 10:27:21 -0700 Subject: [PATCH] Fix Ledger block hashing and caching. Ledger app requires open blocks to have zeroed frontier bytes instead of account public key, diverging from spec, so zero out "parent" block like Ledger expects if public key is the frontier. Ensure block data is correct length before hashing. Merge shared functionality of signing blocks and nonces, and document bug with Ledger nano app that is preventing nonces from being signed. Skip online tests for now. --- src/lib/block.ts | 4 +- src/lib/constants.ts | 1 - src/lib/wallets/ledger-wallet.ts | 100 +++++++++++++++++-------------- test/test.create-wallet.mjs | 5 +- test/test.derive-accounts.mjs | 6 +- test/test.refresh-accounts.mjs | 6 +- test/test.sign-blocks.mjs | 94 ++++++++++++++++++++++++++++- 7 files changed, 157 insertions(+), 59 deletions(-) diff --git a/src/lib/block.ts b/src/lib/block.ts index 63e4c4b..13a3e6d 100644 --- a/src/lib/block.ts +++ b/src/lib/block.ts @@ -50,10 +50,10 @@ abstract class Block { const data = [ PREAMBLE, this.account.publicKey, - this.previous, + this.previous.padStart(64, '0'), this.representative.publicKey, dec.toHex(this.balance, 32), - this.link + this.link.padStart(64, '0') ] const hash = new Blake2b(32) data.forEach(str => hash.update(hex.toBytes(str))) diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 937b84f..e49b25b 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -44,7 +44,6 @@ export const LEDGER_ADPU_CODES: { [key: string]: number } = Object.freeze({ signBlock: 0x04, signNonce: 0x05, paramUnused: 0x00 - }) export const UNITS: { [key: string]: number } = Object.freeze({ diff --git a/src/lib/wallets/ledger-wallet.ts b/src/lib/wallets/ledger-wallet.ts index 7fe56c5..720e009 100644 --- a/src/lib/wallets/ledger-wallet.ts +++ b/src/lib/wallets/ledger-wallet.ts @@ -74,6 +74,15 @@ export class LedgerWallet extends Wallet { return wallet } + /** + * Removes encrypted secrets in storage and releases variable references to + * allow garbage collection. + */ + async destroy (): Promise { + await super.destroy() + await this.close() + } + /** * Retrieves an existing Ledger wallet from session storage using its ID. * @@ -299,11 +308,11 @@ export class LedgerWallet extends Wallet { * Sign a nonce with the Ledger device. * * @param {number} index - Account number - * @param {string} nonce - 128-bit string to sign + * @param {Uint8Array} nonce - 128-bit value to sign * @returns {Promise} Status and signature */ - async sign (index: number, nonce: string): Promise - async sign (index: number = 0, input: string | SendBlock | ReceiveBlock | ChangeBlock): Promise { + async sign (index: number, nonce: Uint8Array): Promise + async sign (index: number = 0, input: Uint8Array | SendBlock | ReceiveBlock | ChangeBlock): Promise { if (typeof index !== 'number' || index < 0 || index >= HARDENED_OFFSET) { throw new TypeError('Invalid account index') } @@ -311,51 +320,46 @@ export class LedgerWallet extends Wallet { const coin = dec.toBytes(BIP44_COIN_NANO + HARDENED_OFFSET, 4) const account = dec.toBytes(index + HARDENED_OFFSET, 4) - const transport = await this.DynamicTransport.create(this.openTimeout, this.listenTimeout) - if (typeof input === 'string') { + let instruction: number + let data: Uint8Array + + if (input instanceof Uint8Array) { + // nonce signing is currently broken: https://github.com/LedgerHQ/app-nano/pull/14 // input is a nonce - const nonce = utf8.toBytes(input) - if (nonce.length !== 16) { + instruction = LEDGER_ADPU_CODES.signNonce + if (input.byteLength !== 16) { throw new RangeError('Nonce must be 16-byte string') } - const data = new Uint8Array([LEDGER_ADPU_CODES.bip32DerivationLevel, ...purpose, ...coin, ...account, ...nonce]) - const response = await transport.send(LEDGER_ADPU_CODES.class, LEDGER_ADPU_CODES.signNonce, LEDGER_ADPU_CODES.paramUnused, LEDGER_ADPU_CODES.paramUnused, data as Buffer) - .catch(err => dec.toBytes(err.statusCode)) as Uint8Array - await transport.close() - - if (response.length === 2) { - const statusCode = bytes.toDec(response) as number - const status = LEDGER_STATUS_CODES[statusCode] ?? 'UNKNOWN_ERROR' - return { status, signature: null } - } - - const signature = bytes.toHex(response.slice(0, 64)) - const statusCode = bytes.toDec(response.slice(-2)) as number - const status = LEDGER_STATUS_CODES[statusCode] - return { status, signature } + data = new Uint8Array([LEDGER_ADPU_CODES.bip32DerivationLevel, ...purpose, ...coin, ...account, ...input]) } else { // input is a block - const previous = hex.toBytes(input.previous) - const link = hex.toBytes(input.link) - const representative = hex.toBytes(input.representative.publicKey) + instruction = LEDGER_ADPU_CODES.signBlock + const previous = hex.toBytes(input.previous, 32) + const link = hex.toBytes(input.link, 32) + const representative = hex.toBytes(input.representative.publicKey, 32) const balance = hex.toBytes(BigInt(input.balance).toString(16), 16) - const data = new Uint8Array([LEDGER_ADPU_CODES.bip32DerivationLevel, ...purpose, ...coin, ...account, ...previous, ...link, ...representative, ...balance]) - const response = await transport.send(LEDGER_ADPU_CODES.class, LEDGER_ADPU_CODES.signBlock, LEDGER_ADPU_CODES.paramUnused, LEDGER_ADPU_CODES.paramUnused, data as Buffer) - .catch(err => dec.toBytes(err.statusCode)) as Uint8Array - await transport.close() - - if (response.length === 2) { - const statusCode = bytes.toDec(response) as number - const status = LEDGER_STATUS_CODES[statusCode] ?? 'UNKNOWN_ERROR' - return { status, signature: null } - } + data = new Uint8Array([LEDGER_ADPU_CODES.bip32DerivationLevel, ...purpose, ...coin, ...account, ...previous, ...link, ...representative, ...balance]) + } + const transport = await this.DynamicTransport.create(this.openTimeout, this.listenTimeout) + const response = await transport.send(LEDGER_ADPU_CODES.class, instruction, LEDGER_ADPU_CODES.paramUnused, LEDGER_ADPU_CODES.paramUnused, data as Buffer) + .catch(err => dec.toBytes(err.statusCode)) as Uint8Array + await transport.close() - const hash = bytes.toHex(response.slice(0, 32)) - const signature = bytes.toHex(response.slice(32, 96)) - const statusCode = bytes.toDec(response.slice(-2)) as number - const status = LEDGER_STATUS_CODES[statusCode] - return { status, signature, hash } + const statusCode = bytes.toDec(response.slice(-2)) as number + const status = LEDGER_STATUS_CODES[statusCode] ?? 'UNKNOWN_ERROR' + + if (status !== 'OK') { + return { status, signature: null } } + if (response.byteLength > 66) { + const signature = bytes.toHex(response.slice(0, 64)) + return { status, signature } + } + + const hash = bytes.toHex(response.slice(0, 32)) + const signature = bytes.toHex(response.slice(32, 96)) + return { status, signature, hash } + } /** @@ -386,7 +390,11 @@ export class LedgerWallet extends Wallet { } input = res.contents } - return this.#cacheBlock(index, input) + const { status } = await this.#cacheBlock(index, input) + if (status !== 'OK') { + throw new Error(status) + } + return { status } } /** @@ -447,11 +455,11 @@ export class LedgerWallet extends Wallet { const purpose = dec.toBytes(BIP44_PURPOSE + HARDENED_OFFSET, 4) const coin = dec.toBytes(BIP44_COIN_NANO + HARDENED_OFFSET, 4) const account = dec.toBytes(index + HARDENED_OFFSET, 4) - const previous = hex.toBytes(block.previous) - const link = hex.toBytes(block.link) - const representative = hex.toBytes(block.representative.publicKey) - const balance = hex.toBytes(BigInt(block.balance).toString(16), 16) - const signature = hex.toBytes(block.signature) + const previous = hex.toBytes(block.previous === block.account.publicKey ? '0' : block.previous, 32) + const link = hex.toBytes(block.link, 32) + const representative = hex.toBytes(block.representative.publicKey, 32) + const balance = hex.toBytes(block.balance.toString(16), 16) + const signature = hex.toBytes(block.signature, 64) const data = new Uint8Array([LEDGER_ADPU_CODES.bip32DerivationLevel, ...purpose, ...coin, ...account, ...previous, ...link, ...representative, ...balance, ...signature]) const transport = await this.DynamicTransport.create(this.openTimeout, this.listenTimeout) diff --git a/test/test.create-wallet.mjs b/test/test.create-wallet.mjs index 385d78a..4899787 100644 --- a/test/test.create-wallet.mjs +++ b/test/test.create-wallet.mjs @@ -71,9 +71,6 @@ await suite('Create wallets', async () => { }) assert.equals(status, 'CONNECTED') - - status = (await wallet.close()).status - - assert.equals(status, 'OK') + assert.equals((await wallet.close()).status, 'OK') }) }) diff --git a/test/test.derive-accounts.mjs b/test/test.derive-accounts.mjs index 7626976..7237176 100644 --- a/test/test.derive-accounts.mjs +++ b/test/test.derive-accounts.mjs @@ -7,7 +7,7 @@ import { assert, isNode, suite, test } from './GLOBALS.mjs' import { NANO_TEST_VECTORS } from './VECTORS.js' import { Bip44Wallet, Blake2bWallet, LedgerWallet } from '../dist/main.min.js' -await suite('Account derivation', async () => { +await suite('BIP-44 account derivation', async () => { await test('should derive the first account from the given BIP-44 seed', async () => { const wallet = await Bip44Wallet.fromSeed(NANO_TEST_VECTORS.PASSWORD, NANO_TEST_VECTORS.BIP39_SEED) await wallet.unlock(NANO_TEST_VECTORS.PASSWORD) @@ -58,7 +58,9 @@ await suite('Account derivation', async () => { await wallet.destroy() }) +}) +await suite('BLAKE2b account derivation', async () => { await test('should derive accounts for a BLAKE2b wallet', async () => { const wallet = await Blake2bWallet.create(NANO_TEST_VECTORS.PASSWORD) await wallet.unlock(NANO_TEST_VECTORS.PASSWORD) @@ -91,7 +93,7 @@ await suite('Account derivation', async () => { /** * This suite requires a connected unlocked Ledger device to execute tests. */ -await suite('should derive accounts for a Ledger device wallet', { skip: false || isNode }, async () => { +await suite('Ledger device account derivation', { skip: false || isNode }, async () => { const wallet = await LedgerWallet.create() await wallet.connect() diff --git a/test/test.refresh-accounts.mjs b/test/test.refresh-accounts.mjs index 54063f4..b54b6d5 100644 --- a/test/test.refresh-accounts.mjs +++ b/test/test.refresh-accounts.mjs @@ -9,7 +9,7 @@ import { Account, Bip44Wallet, Rpc } from '../dist/main.min.js' const rpc = new Rpc(process.env.NODE_URL ?? '', process.env.API_KEY_NAME) -await suite('refreshing account info', { skip: false }, async () => { +await suite('refreshing account info', { skip: true }, async () => { await test('fetch balance, frontier, and representative', async () => { const wallet = await Bip44Wallet.fromSeed(NANO_TEST_VECTORS.PASSWORD, NANO_TEST_VECTORS.BIP39_SEED) @@ -70,7 +70,7 @@ await suite('refreshing account info', { skip: false }, async () => { }) }) -await suite('Fetch next unopened account', { skip: false }, async () => { +await suite('Fetch next unopened account', { skip: true }, async () => { await test('return correct account from test vector', async () => { const wallet = await Bip44Wallet.fromSeed(NANO_TEST_VECTORS.PASSWORD, NANO_TEST_VECTORS.BIP39_SEED) @@ -135,7 +135,7 @@ await suite('Fetch next unopened account', { skip: false }, async () => { }) }) -await suite('Refreshing wallet accounts', { skip: false }, async () => { +await suite('Refreshing wallet accounts', { skip: true }, async () => { await test('should get balance, frontier, and representative for one account', async () => { const wallet = await Bip44Wallet.fromSeed(NANO_TEST_VECTORS.PASSWORD, NANO_TEST_VECTORS.BIP39_SEED) diff --git a/test/test.sign-blocks.mjs b/test/test.sign-blocks.mjs index 6997be3..90e62e9 100644 --- a/test/test.sign-blocks.mjs +++ b/test/test.sign-blocks.mjs @@ -5,7 +5,7 @@ import { assert, suite, test } from './GLOBALS.mjs' import { NANO_TEST_VECTORS } from './VECTORS.js' -import { SendBlock, ReceiveBlock, ChangeBlock } from '../dist/main.min.js' +import { SendBlock, ReceiveBlock, ChangeBlock, LedgerWallet } from '../dist/main.min.js' await suite('Valid blocks', async () => { @@ -163,3 +163,95 @@ await suite('Block signing tests using official test vectors', async () => { assert.equals(block.work, '') }) }) + +await suite('Ledger device signing tests', { skip: false }, async () => { + const wallet = await LedgerWallet.create() + const account = await new Promise(resolve => { + const button = document.createElement('button') + button.innerText = 'Unlock Ledger, then click to continue' + button.addEventListener('click', async (event) => { + await wallet.connect() + resolve(await wallet.account()) + document.body.removeChild(button) + }) + document.body.appendChild(button) + }) + + // nonce signing is currently broken: https://github.com/LedgerHQ/app-nano/pull/14 + await test('should sign a nonce with a Ledger device', { skip: true }, async () => { + let status = await new Promise(resolve => { + const button = document.createElement('button') + button.innerText = 'Unlock Ledger, then click to continue' + button.addEventListener('click', async (event) => { + await wallet.connect() + const result = await wallet.sign(0, new TextEncoder().encode('0123456789abcdef')) + + assert.equals(result.status, 'OK') + assert.equals(result.signature.toUpperCase(), '2BD2F905E74B5BEE3E2277CED1D1E3F7535E5286B6E22F7B08A814AA9E5C4E1FEA69B61D60B435ADC2CE756E6EE5F5BE7EC691FE87E024A0B22A3D980CA5B305') + + document.body.removeChild(button) + resolve(result) + }) + document.body.appendChild(button) + }) + }) + + await test('should sign an open block and send block with a Ledger device', async () => { + const openBlock = new ReceiveBlock( + account, + '0', + NANO_TEST_VECTORS.RECEIVE_BLOCK.link, + NANO_TEST_VECTORS.RECEIVE_BLOCK.balance, + NANO_TEST_VECTORS.RECEIVE_BLOCK.representative, + '0' + ) + await new Promise(resolve => { + const button = document.createElement('button') + button.innerText = 'Unlock Ledger, then click to test signing open ReceiveBlock' + button.addEventListener('click', async (event) => { + assert.nullish(openBlock.signature) + await openBlock.sign(0) + assert.ok(/[A-Fa-f0-9]{128}/.test(openBlock.signature)) + resolve(document.body.removeChild(button)) + }) + document.body.appendChild(button) + }) + assert.ok(/[A-Fa-f0-9]{128}/.test(openBlock.signature)) + assert.ok(/[A-Fa-f0-9]{64}/.test(openBlock.hash)) + + const sendBlock = new SendBlock( + account, + NANO_TEST_VECTORS.RECEIVE_BLOCK.balance, + NANO_TEST_VECTORS.RECEIVE_BLOCK.account, + '0', + NANO_TEST_VECTORS.RECEIVE_BLOCK.representative, + openBlock.hash + ) + await new Promise(resolve => { + const button = document.createElement('button') + button.innerText = 'Unlock Ledger, then click to test signing SendBlock' + button.addEventListener('click', async (event) => { + assert.nullish(sendBlock.signature) + await sendBlock.sign(0, openBlock) + assert.ok(/[A-Fa-f0-9]{128}/.test(sendBlock.signature)) + resolve(document.body.removeChild(button)) + }) + document.body.appendChild(button) + }) + }) + + await test('should fail sign a block with a Ledger device without caching frontier', async () => { + const block = new SendBlock( + NANO_TEST_VECTORS.SEND_BLOCK.account, + NANO_TEST_VECTORS.SEND_BLOCK.balance, + NANO_TEST_VECTORS.SEND_BLOCK.link, + '0', + NANO_TEST_VECTORS.SEND_BLOCK.representative, + NANO_TEST_VECTORS.SEND_BLOCK.previous, + NANO_TEST_VECTORS.SEND_BLOCK.work + ) + assert.rejects(block.sign(0)) + }) + + await wallet.destroy() +}) -- 2.47.3