Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve extended key validation #121

Merged
merged 3 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/BIP44CoinTypeNode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('BIP44CoinTypeNode', () => {
const options = {
depth: 2,
index: 0,
parentFingerprint: 0,
parentFingerprint: 1,
};

const inputs = [
Expand Down
13 changes: 11 additions & 2 deletions src/BIP44Node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('BIP44Node', () => {
privateKey,
chainCode,
depth: 2,
parentFingerprint: 0,
parentFingerprint: 1,
index: 0,
});

Expand All @@ -54,7 +54,7 @@ describe('BIP44Node', () => {
privateKey,
chainCode,
depth: 2,
parentFingerprint: 0,
parentFingerprint: 1,
index: 0,
});

Expand Down Expand Up @@ -101,6 +101,15 @@ describe('BIP44Node', () => {
`Invalid HD tree path depth: The depth must be a positive integer N such that 0 <= N <= 5. Received: "6"`,
);
});

it.each(fixtures.bip32InvalidExtendedKeys)(
'throws if the extended key is invalid',
async (extendedKey) => {
await expect(BIP44Node.fromExtendedKey(extendedKey)).rejects.toThrow(
/Invalid extended key: .*\./u,
);
},
);
});

describe('fromDerivationPath', () => {
Expand Down
86 changes: 86 additions & 0 deletions src/SLIP10Node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,31 @@ const defaultBip39NodeToken = `bip39:${fixtures.local.mnemonic}` as const;
const defaultBip39BytesToken = mnemonicPhraseToBytes(fixtures.local.mnemonic);

describe('SLIP10Node', () => {
describe('constructor', () => {
it('throws an error when the constructor guard is not provided', async () => {
const { privateKey, chainCode } = await deriveChildKey({
path: fixtures.local.mnemonic,
curve: secp256k1,
});

expect(
() =>
// @ts-expect-error - Constructor is private.
new SLIP10Node({
privateKey,
chainCode,
depth: 0,
masterFingerprint: 0,
parentFingerprint: 0,
index: 0,
curve: 'secp256k1',
}),
).toThrow(
'SLIP10Node can only be constructed using `SLIP10Node.fromJSON`, `SLIP10Node.fromExtendedKey`, or `SLIP10Node.fromDerivationPath`.',
);
});
});

describe('fromExtendedKey', () => {
it('initializes a new node from a private key', async () => {
const { privateKey, chainCode } = await deriveChildKey({
Expand Down Expand Up @@ -300,6 +325,67 @@ describe('SLIP10Node', () => {
'Invalid private key: Value is not a valid secp256k1 private key.',
);
});

it('throws if the depth is zero and the parent fingerprint is not zero', async () => {
await expect(
SLIP10Node.fromExtendedKey({
privateKey: new Uint8Array(32).fill(1),
chainCode: new Uint8Array(32).fill(1),
depth: 0,
parentFingerprint: 1,
index: 0,
curve: 'secp256k1',
}),
).rejects.toThrow(
'Invalid parent fingerprint: The fingerprint of the root node must be 0. Received: "1".',
);
});

it('throws if the depth is not zero and the parent fingerprint is zero', async () => {
await expect(
SLIP10Node.fromExtendedKey({
privateKey: new Uint8Array(32).fill(1),
chainCode: new Uint8Array(32).fill(1),
depth: 1,
parentFingerprint: 0,
index: 0,
curve: 'secp256k1',
}),
).rejects.toThrow(
'Invalid parent fingerprint: The fingerprint of a child node must not be 0. Received: "0".',
);
});

it('throws if the depth is >= 2 and the parent fingerprint is equal to the master fingerprint', async () => {
await expect(
SLIP10Node.fromExtendedKey({
privateKey: new Uint8Array(32).fill(1),
chainCode: new Uint8Array(32).fill(1),
depth: 2,
parentFingerprint: 1,
masterFingerprint: 1,
index: 0,
curve: 'secp256k1',
}),
).rejects.toThrow(
'Invalid parent fingerprint: The fingerprint of a child node cannot be equal to the master fingerprint. Received: "1".',
);
});

it('throws if the depth is zero and the index is not zero', async () => {
await expect(
SLIP10Node.fromExtendedKey({
privateKey: new Uint8Array(32).fill(1),
chainCode: new Uint8Array(32).fill(1),
depth: 0,
parentFingerprint: 0,
index: 1,
curve: 'secp256k1',
}),
).rejects.toThrow(
'Invalid index: The index of the root node must be 0. Received: "1".',
);
});
});

describe('fromDerivationPath', () => {
Expand Down
170 changes: 131 additions & 39 deletions src/SLIP10Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,13 @@ export class SLIP10Node implements SLIP10NodeInterface {
validateCurve(curve);
validateBIP32Depth(depth);
validateBIP32Index(index);
validateParentFingerprint(parentFingerprint);
validateRootIndex(index, depth);
validateParentFingerprint(parentFingerprint, depth);
validateMasterParentFingerprint(
masterFingerprint,
parentFingerprint,
depth,
);

const curveObject = getCurveByName(curve);

Expand All @@ -172,30 +178,36 @@ export class SLIP10Node implements SLIP10NodeInterface {
`Invalid private key: Value is not a valid ${curve} private key.`,
);

return new SLIP10Node({
depth,
masterFingerprint,
parentFingerprint,
index,
chainCode: chainCodeBytes,
privateKey: privateKeyBytes,
publicKey: await curveObject.getPublicKey(privateKeyBytes),
curve,
});
return new SLIP10Node(
{
depth,
masterFingerprint,
parentFingerprint,
index,
chainCode: chainCodeBytes,
privateKey: privateKeyBytes,
publicKey: await curveObject.getPublicKey(privateKeyBytes),
curve,
},
this.#constructorGuard,
);
}

if (publicKey) {
const publicKeyBytes = getBytes(publicKey, curveObject.publicKeyLength);

return new SLIP10Node({
depth,
masterFingerprint,
parentFingerprint,
index,
chainCode: chainCodeBytes,
publicKey: publicKeyBytes,
curve,
});
return new SLIP10Node(
{
depth,
masterFingerprint,
parentFingerprint,
index,
chainCode: chainCodeBytes,
publicKey: publicKeyBytes,
curve,
},
this.#constructorGuard,
);
}

throw new Error(
Expand Down Expand Up @@ -249,6 +261,8 @@ export class SLIP10Node implements SLIP10NodeInterface {
});
}

static #constructorGuard = Symbol('SLIP10Node.constructor');

public readonly curve: SupportedCurve;

public readonly depth: number;
Expand All @@ -265,16 +279,25 @@ export class SLIP10Node implements SLIP10NodeInterface {

public readonly publicKeyBytes: Uint8Array;

constructor({
depth,
masterFingerprint,
parentFingerprint,
index,
chainCode,
privateKey,
publicKey,
curve,
}: SLIP10NodeConstructorOptions) {
// eslint-disable-next-line no-restricted-syntax
private constructor(
{
depth,
masterFingerprint,
parentFingerprint,
index,
chainCode,
privateKey,
publicKey,
curve,
}: SLIP10NodeConstructorOptions,
constructorGuard?: symbol,
) {
assert(
constructorGuard === SLIP10Node.#constructorGuard,
'SLIP10Node can only be constructed using `SLIP10Node.fromJSON`, `SLIP10Node.fromExtendedKey`, or `SLIP10Node.fromDerivationPath`.',
);

this.depth = depth;
this.masterFingerprint = masterFingerprint;
this.parentFingerprint = parentFingerprint;
Expand Down Expand Up @@ -331,15 +354,18 @@ export class SLIP10Node implements SLIP10NodeInterface {
* @returns A neutered version of this node.
*/
public neuter(): SLIP10Node {
return new SLIP10Node({
depth: this.depth,
masterFingerprint: this.masterFingerprint,
parentFingerprint: this.parentFingerprint,
index: this.index,
chainCode: this.chainCodeBytes,
publicKey: this.publicKeyBytes,
curve: this.curve,
});
return new SLIP10Node(
{
depth: this.depth,
masterFingerprint: this.masterFingerprint,
parentFingerprint: this.parentFingerprint,
index: this.index,
chainCode: this.chainCodeBytes,
publicKey: this.publicKeyBytes,
curve: this.curve,
},
SLIP10Node.#constructorGuard,
);
}

/**
Expand Down Expand Up @@ -416,9 +442,13 @@ export function validateBIP32Depth(depth: unknown): asserts depth is number {
* integer `number`. Throws an error if validation fails.
*
* @param parentFingerprint - The parent fingerprint to validate.
* @param depth - The depth of the node to validate.
* @throws If the parent fingerprint is not a positive integer, or invalid for
* the current depth.
*/
export function validateParentFingerprint(
parentFingerprint: unknown,
depth: number,
): asserts parentFingerprint is number {
if (!isValidInteger(parentFingerprint)) {
throw new Error(
Expand All @@ -427,6 +457,68 @@ export function validateParentFingerprint(
)}".`,
);
}

if (depth === 0 && parentFingerprint !== 0) {
throw new Error(
`Invalid parent fingerprint: The fingerprint of the root node must be 0. Received: "${String(
parentFingerprint,
)}".`,
);
}

if (depth > 0 && parentFingerprint === 0) {
throw new Error(
`Invalid parent fingerprint: The fingerprint of a child node must not be 0. Received: "${String(
parentFingerprint,
)}".`,
);
}
}

/**
* Validate that a given combination of master fingerprint and parent
* fingerprint is valid for the given depth.
*
* @param masterFingerprint - The master fingerprint to validate.
* @param parentFingerprint - The parent fingerprint to validate.
* @param depth - The depth of the node to validate.
* @throws If the combination of master fingerprint and parent fingerprint is
* invalid for the given depth.
*/
export function validateMasterParentFingerprint(
masterFingerprint: number | undefined,
parentFingerprint: number,
depth: number,
) {
// The master fingerprint is optional.
if (!masterFingerprint) {
return;
}

if (depth >= 2 && masterFingerprint === parentFingerprint) {
throw new Error(
`Invalid parent fingerprint: The fingerprint of a child node cannot be equal to the master fingerprint. Received: "${String(
parentFingerprint,
)}".`,
);
}
}

/**
* Validate that the index is zero for the root node.
*
* @param index - The index to validate.
* @param depth - The depth of the node to validate.
* @throws If the index is not zero for the root node.
*/
export function validateRootIndex(index: number, depth: number) {
if (depth === 0 && index !== 0) {
throw new Error(
`Invalid index: The index of the root node must be 0. Received: "${String(
index,
)}".`,
);
}
}

type DeriveChildNodeArgs = {
Expand Down
2 changes: 1 addition & 1 deletion src/extended-keys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('decodeExtendedKey', () => {
'xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMP';

expect(() => decodeExtendedKey(extendedKey)).toThrow(
'Invalid value: Value is not base58-encoded, or the checksum is invalid.',
'Invalid extended key: Value is not base58-encoded, or the checksum is invalid.',
);
});

Expand Down
2 changes: 1 addition & 1 deletion src/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ describe('decodeBase58Check', () => {

it('throws if the checksum is invalid', () => {
expect(() => decodeBase58check('SQHFQMRT97ajZff')).toThrow(
'Invalid value: Value is not base58-encoded, or the checksum is invalid.',
'Invalid extended key: Value is not base58-encoded, or the checksum is invalid.',
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ export const decodeBase58check = (value: string): Uint8Array => {
return base58Check.decode(value);
} catch {
throw new Error(
`Invalid value: Value is not base58-encoded, or the checksum is invalid.`,
`Invalid extended key: Value is not base58-encoded, or the checksum is invalid.`,
);
}
};
Expand Down
Loading