From 61102e522c60e60acb000b6e26456fb3ccc51b0c Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 7 Mar 2025 12:40:06 +0100 Subject: [PATCH 1/6] Optimise performance of getting public keys --- src/SLIP10Node.test.ts | 54 ++++++++++++++++++------- src/SLIP10Node.ts | 71 +++++++++++++++++++++++++++++---- src/constants.ts | 8 ++++ src/curves/curve.ts | 5 +-- src/curves/ed25519Bip32.test.ts | 4 +- src/curves/ed25519Bip32.ts | 4 +- src/derivers/bip32.ts | 5 +-- src/derivers/bip39.ts | 9 +++-- src/derivers/cip3.ts | 2 +- src/derivers/shared.ts | 45 ++++++++++++--------- 10 files changed, 152 insertions(+), 55 deletions(-) diff --git a/src/SLIP10Node.test.ts b/src/SLIP10Node.test.ts index 404d29dd..288addc7 100644 --- a/src/SLIP10Node.test.ts +++ b/src/SLIP10Node.test.ts @@ -710,23 +710,23 @@ describe('SLIP10Node', () => { }); // getter - expect(() => (node.privateKey = 'foo')).toThrow( - /^Cannot set property privateKey of .+ which has only a getter/iu, - ); + ['privateKey', 'publicKeyBytes'].forEach((property) => { + expect(() => (node[property] = 'foo')).toThrow( + /^Cannot set property .+ of .+ which has only a getter/iu, + ); + }); // frozen / readonly - ['depth', 'privateKeyBytes', 'publicKeyBytes', 'chainCodeBytes'].forEach( - (property) => { - expect(() => (node[property] = new Uint8Array(64).fill(1))).toThrow( - expect.objectContaining({ - name: 'TypeError', - message: expect.stringMatching( - `Cannot assign to read only property '${property}' of object`, - ), - }), - ); - }, - ); + ['depth', 'privateKeyBytes', 'chainCodeBytes'].forEach((property) => { + expect(() => (node[property] = new Uint8Array(64).fill(1))).toThrow( + expect.objectContaining({ + name: 'TypeError', + message: expect.stringMatching( + `Cannot assign to read only property '${property}' of object`, + ), + }), + ); + }); }); it('throws an error if no curve is specified', async () => { @@ -1107,6 +1107,30 @@ describe('SLIP10Node', () => { }); }); + describe('publicKeyBytes', () => { + it('lazily computes the public key bytes', async () => { + const baseNode = await SLIP10Node.fromDerivationPath({ + derivationPath: [ + defaultBip39NodeToken, + BIP44PurposeNodeToken, + `bip32:0'`, + `bip32:0'`, + ], + curve: 'secp256k1', + }); + + const { publicKey, ...json } = baseNode.toJSON(); + + const spy = jest.spyOn(secp256k1, 'getPublicKey'); + + const node = await SLIP10Node.fromExtendedKey(json); + expect(spy).not.toHaveBeenCalled(); + + expect(node.publicKeyBytes).toStrictEqual(baseNode.publicKeyBytes); + expect(spy).toHaveBeenCalled(); + }); + }); + describe('compressedPublicKeyBytes', () => { it('returns the public key in compressed form', async () => { const node = await SLIP10Node.fromDerivationPath({ diff --git a/src/SLIP10Node.ts b/src/SLIP10Node.ts index 1061952b..20f4510c 100644 --- a/src/SLIP10Node.ts +++ b/src/SLIP10Node.ts @@ -2,12 +2,12 @@ import { assert, bytesToHex } from '@metamask/utils'; import type { BIP44CoinTypeNode } from './BIP44CoinTypeNode'; import type { BIP44Node } from './BIP44Node'; +import { BYTES_KEY_LENGTH, PUBLIC_KEY_GUARD } from './constants'; import type { Network, RootedSLIP10PathTuple, SLIP10PathTuple, } from './constants'; -import { BYTES_KEY_LENGTH } from './constants'; import type { CryptographicFunctions } from './cryptography'; import type { SupportedCurve } from './curves'; import { getCurveByName } from './curves'; @@ -99,18 +99,32 @@ export type SLIP10NodeInterface = JsonSLIP10Node & { toJSON(): JsonSLIP10Node; }; -export type SLIP10NodeConstructorOptions = { +type BaseSLIP10NodeConstructorOptions = { readonly depth: number; readonly masterFingerprint?: number | undefined; readonly parentFingerprint: number; readonly index: number; readonly network?: Network | undefined; readonly chainCode: Uint8Array; - readonly privateKey?: Uint8Array | undefined; - readonly publicKey: Uint8Array; readonly curve: SupportedCurve; }; +type SLIP10NodePrivateKeyConstructorOptions = + BaseSLIP10NodeConstructorOptions & { + readonly privateKey: Uint8Array; + readonly publicKey?: Uint8Array | undefined; + }; + +type SLIP10NodePublicKeyConstructorOptions = + BaseSLIP10NodeConstructorOptions & { + readonly privateKey?: Uint8Array | undefined; + readonly publicKey: Uint8Array; + }; + +export type SLIP10NodeConstructorOptions = + | SLIP10NodePrivateKeyConstructorOptions + | SLIP10NodePublicKeyConstructorOptions; + export type SLIP10ExtendedKeyOptions = { readonly depth: number; readonly masterFingerprint?: number | undefined; @@ -121,6 +135,12 @@ export type SLIP10ExtendedKeyOptions = { readonly privateKey?: string | Uint8Array | undefined; readonly publicKey?: string | Uint8Array | undefined; readonly curve: SupportedCurve; + + /** + * For internal use only. This is used to ensure the public key provided to + * the constructor is trusted. + */ + readonly guard?: typeof PUBLIC_KEY_GUARD; }; export type SLIP10DerivationPathOptions = { @@ -276,6 +296,7 @@ export class SLIP10Node implements SLIP10NodeInterface { publicKey, chainCode, curve, + guard, } = options; const chainCodeBytes = getBytes(chainCode, BYTES_KEY_LENGTH); @@ -299,11 +320,17 @@ export class SLIP10Node implements SLIP10NodeInterface { privateKey, curveObject.privateKeyLength, ); + assert( curveObject.isValidPrivateKey(privateKeyBytes), `Invalid private key: Value is not a valid ${curve} private key.`, ); + const trustedPublicKey = + guard === PUBLIC_KEY_GUARD && publicKey + ? getBytes(publicKey, curveObject.publicKeyLength) + : undefined; + return new SLIP10Node( { depth, @@ -313,7 +340,7 @@ export class SLIP10Node implements SLIP10NodeInterface { network, chainCode: chainCodeBytes, privateKey: privateKeyBytes, - publicKey: await curveObject.getPublicKey(privateKeyBytes), + publicKey: trustedPublicKey, curve, }, cryptographicFunctions, @@ -478,7 +505,7 @@ export class SLIP10Node implements SLIP10NodeInterface { public readonly privateKeyBytes?: Uint8Array | undefined; - public readonly publicKeyBytes: Uint8Array; + #publicKeyBytes: Uint8Array | undefined; readonly #cryptographicFunctions: CryptographicFunctions; @@ -503,6 +530,11 @@ export class SLIP10Node implements SLIP10NodeInterface { 'SLIP10Node can only be constructed using `SLIP10Node.fromJSON`, `SLIP10Node.fromExtendedKey`, `SLIP10Node.fromDerivationPath`, or `SLIP10Node.fromSeed`.', ); + assert( + privateKey !== undefined || publicKey !== undefined, + 'SLIP10Node requires either a private key or a public key to be set.', + ); + this.depth = depth; this.masterFingerprint = masterFingerprint; this.parentFingerprint = parentFingerprint; @@ -510,8 +542,8 @@ export class SLIP10Node implements SLIP10NodeInterface { this.network = network; this.chainCodeBytes = chainCode; this.privateKeyBytes = privateKey; - this.publicKeyBytes = publicKey; this.curve = curve; + this.#publicKeyBytes = publicKey; this.#cryptographicFunctions = cryptographicFunctions; Object.freeze(this); @@ -533,6 +565,31 @@ export class SLIP10Node implements SLIP10NodeInterface { return bytesToHex(this.publicKeyBytes); } + /** + * Get the public key bytes. This will lazily derive the public key from the + * private key if it is not already set. + * + * @returns The public key bytes. + */ + public get publicKeyBytes(): Uint8Array { + if (this.#publicKeyBytes !== undefined) { + return this.#publicKeyBytes; + } + + // This assertion is mainly for type safety, as `SLIP10Node` requires either + // a private key or a public key to always be set. + assert( + this.privateKeyBytes, + 'Either a private key or public key is required.', + ); + + this.#publicKeyBytes = getCurveByName(this.curve).getPublicKey( + this.privateKeyBytes, + ); + + return this.#publicKeyBytes; + } + public get compressedPublicKeyBytes(): Uint8Array { return getCurveByName(this.curve).compressPublicKey(this.publicKeyBytes); } diff --git a/src/constants.ts b/src/constants.ts index 2e3d2456..b17aa60b 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -6,6 +6,14 @@ export const MAX_BIP_44_DEPTH = 5; export const MAX_UNHARDENED_BIP_32_INDEX = 0x7fffffff; // 2^31 - 1 export const MAX_BIP_32_INDEX = 0xffffffff; // 2^32 - 1 +/** + * A guard symbol to prevent untrusted public keys from being passed to + * `SLIP10Node` constructors. + */ +export const PUBLIC_KEY_GUARD = Symbol( + 'Public key guard. Do not export this from the module.', +); + export type MinBIP44Depth = typeof MIN_BIP_44_DEPTH; export type MaxBIP44Depth = typeof MAX_BIP_44_DEPTH; export type BIP44Depth = MinBIP44Depth | 1 | 2 | 3 | 4 | MaxBIP44Depth; diff --git a/src/curves/curve.ts b/src/curves/curve.ts index 05dc2660..88e0a56a 100644 --- a/src/curves/curve.ts +++ b/src/curves/curve.ts @@ -27,10 +27,7 @@ export type Curve = { curve: { n: bigint; }; - getPublicKey: ( - privateKey: Uint8Array, - compressed?: boolean, - ) => Uint8Array | Promise; + getPublicKey: (privateKey: Uint8Array, compressed?: boolean) => Uint8Array; isValidPrivateKey: (privateKey: Uint8Array) => boolean; publicAdd: (publicKey: Uint8Array, tweak: Uint8Array) => Uint8Array; compressPublicKey: (publicKey: Uint8Array) => Uint8Array; diff --git a/src/curves/ed25519Bip32.test.ts b/src/curves/ed25519Bip32.test.ts index ad0d46ea..73b2ad2d 100644 --- a/src/curves/ed25519Bip32.test.ts +++ b/src/curves/ed25519Bip32.test.ts @@ -13,8 +13,8 @@ import fixtures from '../../test/fixtures'; describe('getPublicKey', () => { fixtures.cip3.forEach((fixture) => { Object.values(fixture.nodes).forEach((node) => { - it('returns correct public key from private key', async () => { - const publicKey = await ed25519Bip32.getPublicKey( + it('returns correct public key from private key', () => { + const publicKey = ed25519Bip32.getPublicKey( hexToBytes(node.privateKey), ); diff --git a/src/curves/ed25519Bip32.ts b/src/curves/ed25519Bip32.ts index d0e466ac..c4643229 100644 --- a/src/curves/ed25519Bip32.ts +++ b/src/curves/ed25519Bip32.ts @@ -99,10 +99,10 @@ export const multiplyWithBase = (key: Uint8Array): Uint8Array => { * @param _compressed - Optional parameter to indicate if the public key should be compressed. * @returns The public key. */ -export const getPublicKey = async ( +export const getPublicKey = ( privateKey: Uint8Array, _compressed?: boolean, -): Promise => { +): Uint8Array => { return multiplyWithBase(privateKey.slice(0, 32)); }; diff --git a/src/derivers/bip32.ts b/src/derivers/bip32.ts index e31ff0b0..e023cb5f 100755 --- a/src/derivers/bip32.ts +++ b/src/derivers/bip32.ts @@ -96,17 +96,16 @@ async function handleError( options: DeriveNodeArgs, cryptographicFunctions?: CryptographicFunctions, ): Promise { - const { childIndex, privateKey, publicKey, isHardened, curve, chainCode } = - options; + const { childIndex, privateKey, publicKey, isHardened, chainCode } = options; validateBIP32Index(childIndex + 1); if (privateKey) { const secretExtension = await deriveSecretExtension({ privateKey, + publicKey, childIndex: childIndex + 1, isHardened, - curve, }); const newEntropy = await generateEntropy( diff --git a/src/derivers/bip39.ts b/src/derivers/bip39.ts index bd50b036..c2f29382 100755 --- a/src/derivers/bip39.ts +++ b/src/derivers/bip39.ts @@ -10,7 +10,7 @@ import type { RootedSLIP10PathTuple, RootedSLIP10SeedPathTuple, } from '../constants'; -import { BYTES_KEY_LENGTH } from '../constants'; +import { BYTES_KEY_LENGTH, PUBLIC_KEY_GUARD } from '../constants'; import type { CryptographicFunctions } from '../cryptography'; import { hmacSha512, pbkdf2Sha512 } from '../cryptography'; import type { Curve, SupportedCurve } from '../curves'; @@ -244,14 +244,16 @@ export async function createBip39KeyFromSeed( 'Invalid private key: The private key must greater than 0 and less than the curve order.', ); + const publicKey = curve.getPublicKey(privateKey, false); const masterFingerprint = getFingerprint( - await curve.getPublicKey(privateKey, true), + curve.compressPublicKey(publicKey), curve.compressedPublicKeyLength, ); return SLIP10Node.fromExtendedKey( { privateKey, + publicKey, chainCode, masterFingerprint, network, @@ -259,6 +261,7 @@ export async function createBip39KeyFromSeed( parentFingerprint: 0, index: 0, curve: curve.name, + guard: PUBLIC_KEY_GUARD, }, cryptographicFunctions, ); @@ -311,7 +314,7 @@ export async function entropyToCip3MasterNode( assert(curve.isValidPrivateKey(privateKey), 'Invalid private key.'); const masterFingerprint = getFingerprint( - await curve.getPublicKey(privateKey), + curve.getPublicKey(privateKey), curve.compressedPublicKeyLength, ); diff --git a/src/derivers/cip3.ts b/src/derivers/cip3.ts index f13ef276..d2bb0d26 100644 --- a/src/derivers/cip3.ts +++ b/src/derivers/cip3.ts @@ -302,7 +302,7 @@ export const derivePublicKey = async ( const zl = entropy.slice(0, 32); // right = [8ZL] * B - const right = await curve.getPublicKey( + const right = curve.getPublicKey( // [8ZL] trunc28Mul8(zl), ); diff --git a/src/derivers/shared.ts b/src/derivers/shared.ts index 35acd2a0..a6703e0a 100644 --- a/src/derivers/shared.ts +++ b/src/derivers/shared.ts @@ -7,7 +7,11 @@ import { import type { DeriveChildKeyArgs, DerivedKeys } from '.'; import type { Network } from '../constants'; -import { BIP_32_HARDENED_OFFSET, UNPREFIXED_PATH_REGEX } from '../constants'; +import { + BIP_32_HARDENED_OFFSET, + PUBLIC_KEY_GUARD, + UNPREFIXED_PATH_REGEX, +} from '../constants'; import type { CryptographicFunctions } from '../cryptography'; import { hmacSha512 } from '../cryptography'; import type { Curve } from '../curves'; @@ -62,9 +66,9 @@ export async function deriveChildKey( if (node.privateKeyBytes) { const secretExtension = await deriveSecretExtension({ privateKey: node.privateKeyBytes, + publicKey: node.compressedPublicKeyBytes, childIndex, isHardened, - curve, }); const entropy = await generateEntropy( @@ -78,6 +82,7 @@ export async function deriveChildKey( return await deriveNode( { privateKey: node.privateKeyBytes, + publicKey: node.publicKeyBytes, entropy, ...args, }, @@ -124,7 +129,7 @@ type BaseDeriveNodeArgs = { type DerivePrivateKeyArgs = BaseDeriveNodeArgs & { privateKey: Uint8Array; - publicKey?: never | undefined; + publicKey: Uint8Array; }; type DerivePublicKeyArgs = BaseDeriveNodeArgs & { @@ -136,9 +141,9 @@ export type DeriveNodeArgs = DerivePrivateKeyArgs | DerivePublicKeyArgs; type DeriveSecretExtensionArgs = { privateKey: Uint8Array; + publicKey: Uint8Array; childIndex: number; isHardened: boolean; - curve: Curve; }; /** @@ -223,16 +228,16 @@ async function deriveNode( * * @param options - The options for deriving a secret extension. * @param options.privateKey - The parent private key bytes. + * @param options.publicKey - The parent public key bytes. * @param options.childIndex - The child index to derive. * @param options.isHardened - Whether the child index is hardened. - * @param options.curve - The curve to use for derivation. * @returns The secret extension bytes. */ export async function deriveSecretExtension({ privateKey, + publicKey, childIndex, isHardened, - curve, }: DeriveSecretExtensionArgs): Promise { if (isHardened) { // Hardened child @@ -244,8 +249,7 @@ export async function deriveSecretExtension({ } // Normal child - const parentPublicKey = await curve.getPublicKey(privateKey, true); - return derivePublicExtension({ parentPublicKey, childIndex }); + return derivePublicExtension({ parentPublicKey: publicKey, childIndex }); } type DerivePublicExtensionArgs = { @@ -283,23 +287,23 @@ type GenerateKeyArgs = { * @param options.curve - The curve to use for derivation. * @returns The derived key. */ -async function generateKey({ +function generateKey({ privateKey, entropy, curve, -}: GenerateKeyArgs): Promise { +}: GenerateKeyArgs): DerivedKeys & { privateKey: Uint8Array } { const keyMaterial = entropy.slice(0, 32); const childChainCode = entropy.slice(32); // If curve is ed25519: The returned child key ki is parse256(IL). // https://github.com/satoshilabs/slips/blob/133ea52a8e43d338b98be208907e144277e44c0e/slip-0010.md#private-parent-key--private-child-key if (curve.name === 'ed25519') { - const publicKey = await curve.getPublicKey(keyMaterial); + const publicKey = curve.getPublicKey(keyMaterial); return { privateKey: keyMaterial, publicKey, chainCode: childChainCode }; } const childPrivateKey = privateAdd(privateKey, keyMaterial, curve); - const publicKey = await curve.getPublicKey(childPrivateKey); + const publicKey = curve.getPublicKey(childPrivateKey); return { privateKey: childPrivateKey, publicKey, chainCode: childChainCode }; } @@ -351,16 +355,20 @@ async function derivePrivateChildKey( const actualChildIndex = childIndex + (isHardened ? BIP_32_HARDENED_OFFSET : 0); - const { privateKey: childPrivateKey, chainCode: childChainCode } = - await generateKey({ - privateKey, - entropy, - curve, - }); + const { + privateKey: childPrivateKey, + publicKey: childPublicKey, + chainCode: childChainCode, + } = generateKey({ + privateKey, + entropy, + curve, + }); return await SLIP10Node.fromExtendedKey( { privateKey: childPrivateKey, + publicKey: childPublicKey, chainCode: childChainCode, depth: depth + 1, masterFingerprint, @@ -368,6 +376,7 @@ async function derivePrivateChildKey( index: actualChildIndex, curve: curve.name, network, + guard: PUBLIC_KEY_GUARD, }, cryptographicFunctions, ); From e9c09d120600e06550a8f59d36e214548b3198f6 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 7 Mar 2025 12:57:00 +0100 Subject: [PATCH 2/6] Fix tests --- src/SLIP10Node.test.ts | 30 +++++++++++++++--------------- src/derivers/bip32.ts | 5 +++-- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/SLIP10Node.test.ts b/src/SLIP10Node.test.ts index 288addc7..c5742428 100644 --- a/src/SLIP10Node.test.ts +++ b/src/SLIP10Node.test.ts @@ -875,23 +875,23 @@ describe('SLIP10Node', () => { }); // getter - expect(() => (node.privateKey = 'foo')).toThrow( - /^Cannot set property privateKey of .+ which has only a getter/iu, - ); + ['privateKey', 'publicKeyBytes'].forEach((property) => { + expect(() => (node[property] = 'foo')).toThrow( + /^Cannot set property .+ of .+ which has only a getter/iu, + ); + }); // frozen / readonly - ['depth', 'privateKeyBytes', 'publicKeyBytes', 'chainCodeBytes'].forEach( - (property) => { - expect(() => (node[property] = new Uint8Array(64).fill(1))).toThrow( - expect.objectContaining({ - name: 'TypeError', - message: expect.stringMatching( - `Cannot assign to read only property '${property}' of object`, - ), - }), - ); - }, - ); + ['depth', 'privateKeyBytes', 'chainCodeBytes'].forEach((property) => { + expect(() => (node[property] = new Uint8Array(64).fill(1))).toThrow( + expect.objectContaining({ + name: 'TypeError', + message: expect.stringMatching( + `Cannot assign to read only property '${property}' of object`, + ), + }), + ); + }); }); it('throws an error if no curve is specified', async () => { diff --git a/src/derivers/bip32.ts b/src/derivers/bip32.ts index e023cb5f..6a61344c 100755 --- a/src/derivers/bip32.ts +++ b/src/derivers/bip32.ts @@ -96,14 +96,15 @@ async function handleError( options: DeriveNodeArgs, cryptographicFunctions?: CryptographicFunctions, ): Promise { - const { childIndex, privateKey, publicKey, isHardened, chainCode } = options; + const { childIndex, privateKey, publicKey, isHardened, chainCode, curve } = + options; validateBIP32Index(childIndex + 1); if (privateKey) { const secretExtension = await deriveSecretExtension({ privateKey, - publicKey, + publicKey: curve.compressPublicKey(publicKey), childIndex: childIndex + 1, isHardened, }); From cc54abfd15b47abaa9feafaf3bd6488da306bebe Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 7 Mar 2025 13:05:03 +0100 Subject: [PATCH 3/6] Add extra check in `deriveSecretExtension` --- src/derivers/bip32.ts | 1 + src/derivers/shared.ts | 7 ++++++- src/utils.ts | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/derivers/bip32.ts b/src/derivers/bip32.ts index 6a61344c..a6845bba 100755 --- a/src/derivers/bip32.ts +++ b/src/derivers/bip32.ts @@ -107,6 +107,7 @@ async function handleError( publicKey: curve.compressPublicKey(publicKey), childIndex: childIndex + 1, isHardened, + curve, }); const newEntropy = await generateEntropy( diff --git a/src/derivers/shared.ts b/src/derivers/shared.ts index a6703e0a..94ea0d30 100644 --- a/src/derivers/shared.ts +++ b/src/derivers/shared.ts @@ -17,7 +17,7 @@ import { hmacSha512 } from '../cryptography'; import type { Curve } from '../curves'; import { mod } from '../curves'; import { SLIP10Node } from '../SLIP10Node'; -import { isValidBytesKey, numberToUint32 } from '../utils'; +import { isValidBytesKey, numberToUint32, validateBytes } from '../utils'; type ErrorHandler = ( error: unknown, @@ -69,6 +69,7 @@ export async function deriveChildKey( publicKey: node.compressedPublicKeyBytes, childIndex, isHardened, + curve, }); const entropy = await generateEntropy( @@ -144,6 +145,7 @@ type DeriveSecretExtensionArgs = { publicKey: Uint8Array; childIndex: number; isHardened: boolean; + curve: Curve; }; /** @@ -231,6 +233,7 @@ async function deriveNode( * @param options.publicKey - The parent public key bytes. * @param options.childIndex - The child index to derive. * @param options.isHardened - Whether the child index is hardened. + * @param options.curve - The curve to use for derivation. * @returns The secret extension bytes. */ export async function deriveSecretExtension({ @@ -238,6 +241,7 @@ export async function deriveSecretExtension({ publicKey, childIndex, isHardened, + curve, }: DeriveSecretExtensionArgs): Promise { if (isHardened) { // Hardened child @@ -249,6 +253,7 @@ export async function deriveSecretExtension({ } // Normal child + validateBytes(publicKey, curve.compressedPublicKeyLength); return derivePublicExtension({ parentPublicKey: publicKey, childIndex }); } diff --git a/src/utils.ts b/src/utils.ts index be760c50..8da99dd1 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -348,7 +348,7 @@ export function getBytesUnsafe(value: unknown, length: number): Uint8Array { * @param length - The length to validate the `Uint8Array` against. * @throws An error if the `Uint8Array` is empty or has the wrong length. */ -function validateBytes( +export function validateBytes( bytes: Uint8Array, length: number, ): asserts bytes is Uint8Array { From d91c1915214f783dec3b4855b0a4ce573236cad3 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 7 Mar 2025 13:15:08 +0100 Subject: [PATCH 4/6] Move `PUBLIC_KEY_GUARD` to separate file and add comment --- src/SLIP10Node.ts | 8 ++++++-- src/constants.ts | 8 -------- src/derivers/bip39.ts | 3 ++- src/derivers/shared.ts | 7 ++----- src/guard.ts | 9 +++++++++ src/index.test.ts | 6 ++++++ 6 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 src/guard.ts diff --git a/src/SLIP10Node.ts b/src/SLIP10Node.ts index 20f4510c..bc8c95e6 100644 --- a/src/SLIP10Node.ts +++ b/src/SLIP10Node.ts @@ -2,7 +2,7 @@ import { assert, bytesToHex } from '@metamask/utils'; import type { BIP44CoinTypeNode } from './BIP44CoinTypeNode'; import type { BIP44Node } from './BIP44Node'; -import { BYTES_KEY_LENGTH, PUBLIC_KEY_GUARD } from './constants'; +import { BYTES_KEY_LENGTH } from './constants'; import type { Network, RootedSLIP10PathTuple, @@ -15,6 +15,7 @@ import { deriveKeyFromPath } from './derivation'; import { publicKeyToEthAddress } from './derivers/bip32'; import { getDerivationPathWithSeed } from './derivers/bip39'; import { decodeExtendedKey, encodeExtendedKey } from './extended-keys'; +import { PUBLIC_KEY_GUARD } from './guard'; import { getBytes, getBytesUnsafe, @@ -328,7 +329,10 @@ export class SLIP10Node implements SLIP10NodeInterface { const trustedPublicKey = guard === PUBLIC_KEY_GUARD && publicKey - ? getBytes(publicKey, curveObject.publicKeyLength) + ? // `publicKey` is typed as `string | Uint8Array`, but we know it's + // a `Uint8Array` because of the guard. We use `getBytes` to ensure + // the type is correct. + getBytes(publicKey, curveObject.publicKeyLength) : undefined; return new SLIP10Node( diff --git a/src/constants.ts b/src/constants.ts index b17aa60b..2e3d2456 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -6,14 +6,6 @@ export const MAX_BIP_44_DEPTH = 5; export const MAX_UNHARDENED_BIP_32_INDEX = 0x7fffffff; // 2^31 - 1 export const MAX_BIP_32_INDEX = 0xffffffff; // 2^32 - 1 -/** - * A guard symbol to prevent untrusted public keys from being passed to - * `SLIP10Node` constructors. - */ -export const PUBLIC_KEY_GUARD = Symbol( - 'Public key guard. Do not export this from the module.', -); - export type MinBIP44Depth = typeof MIN_BIP_44_DEPTH; export type MaxBIP44Depth = typeof MAX_BIP_44_DEPTH; export type BIP44Depth = MinBIP44Depth | 1 | 2 | 3 | 4 | MaxBIP44Depth; diff --git a/src/derivers/bip39.ts b/src/derivers/bip39.ts index c2f29382..18c77a43 100755 --- a/src/derivers/bip39.ts +++ b/src/derivers/bip39.ts @@ -10,11 +10,12 @@ import type { RootedSLIP10PathTuple, RootedSLIP10SeedPathTuple, } from '../constants'; -import { BYTES_KEY_LENGTH, PUBLIC_KEY_GUARD } from '../constants'; +import { BYTES_KEY_LENGTH } from '../constants'; import type { CryptographicFunctions } from '../cryptography'; import { hmacSha512, pbkdf2Sha512 } from '../cryptography'; import type { Curve, SupportedCurve } from '../curves'; import { getCurveByName } from '../curves'; +import { PUBLIC_KEY_GUARD } from '../guard'; import { SLIP10Node } from '../SLIP10Node'; import { getFingerprint } from '../utils'; diff --git a/src/derivers/shared.ts b/src/derivers/shared.ts index 94ea0d30..ef7b9452 100644 --- a/src/derivers/shared.ts +++ b/src/derivers/shared.ts @@ -7,15 +7,12 @@ import { import type { DeriveChildKeyArgs, DerivedKeys } from '.'; import type { Network } from '../constants'; -import { - BIP_32_HARDENED_OFFSET, - PUBLIC_KEY_GUARD, - UNPREFIXED_PATH_REGEX, -} from '../constants'; +import { BIP_32_HARDENED_OFFSET, UNPREFIXED_PATH_REGEX } from '../constants'; import type { CryptographicFunctions } from '../cryptography'; import { hmacSha512 } from '../cryptography'; import type { Curve } from '../curves'; import { mod } from '../curves'; +import { PUBLIC_KEY_GUARD } from '../guard'; import { SLIP10Node } from '../SLIP10Node'; import { isValidBytesKey, numberToUint32, validateBytes } from '../utils'; diff --git a/src/guard.ts b/src/guard.ts new file mode 100644 index 00000000..fd983280 --- /dev/null +++ b/src/guard.ts @@ -0,0 +1,9 @@ +/** + * A guard symbol to prevent untrusted public keys from being passed to + * `SLIP10Node` constructors. + * + * This is a private symbol and should not be exported from the module. + */ +export const PUBLIC_KEY_GUARD = Symbol( + 'Public key guard. Do not export this from the module.', +); diff --git a/src/index.test.ts b/src/index.test.ts index 0cc826e7..2d854cdb 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -11,6 +11,8 @@ import { getBIP44CoinTypeToAddressPathTuple, mnemonicToSeed, } from '.'; +import * as index from '.'; +import * as guard from './guard'; // This is purely for coverage shenanigans describe('index', () => { @@ -28,4 +30,8 @@ describe('index', () => { expect(getBIP44CoinTypeToAddressPathTuple).toBeDefined(); expect(mnemonicToSeed).toBeDefined(); }); + + it.each(Object.keys(guard))('does not export %s', (property) => { + expect(index).not.toHaveProperty(property); + }); }); From 6a1846c303e57a8f91572ed940606e3ec7c4255d Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 7 Mar 2025 13:19:11 +0100 Subject: [PATCH 5/6] Update documentation for `publicKey` parameter --- src/derivers/shared.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/derivers/shared.ts b/src/derivers/shared.ts index ef7b9452..f3a3c613 100644 --- a/src/derivers/shared.ts +++ b/src/derivers/shared.ts @@ -227,7 +227,7 @@ async function deriveNode( * * @param options - The options for deriving a secret extension. * @param options.privateKey - The parent private key bytes. - * @param options.publicKey - The parent public key bytes. + * @param options.publicKey - The parent compressed public key bytes. * @param options.childIndex - The child index to derive. * @param options.isHardened - Whether the child index is hardened. * @param options.curve - The curve to use for derivation. From 6920045426985b5245058d06bcc473e6542057e0 Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Fri, 7 Mar 2025 13:32:13 +0100 Subject: [PATCH 6/6] Re-use public key for deriving CIP-3 nodes --- src/derivers/bip39.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/derivers/bip39.ts b/src/derivers/bip39.ts index 18c77a43..94463680 100755 --- a/src/derivers/bip39.ts +++ b/src/derivers/bip39.ts @@ -314,14 +314,16 @@ export async function entropyToCip3MasterNode( assert(curve.isValidPrivateKey(privateKey), 'Invalid private key.'); + const publicKey = curve.getPublicKey(privateKey, false); const masterFingerprint = getFingerprint( - curve.getPublicKey(privateKey), + curve.compressPublicKey(publicKey), curve.compressedPublicKeyLength, ); return SLIP10Node.fromExtendedKey( { privateKey, + publicKey, chainCode, masterFingerprint, network, @@ -329,6 +331,7 @@ export async function entropyToCip3MasterNode( parentFingerprint: 0, index: 0, curve: curve.name, + guard: PUBLIC_KEY_GUARD, }, cryptographicFunctions, );