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

Handle errors when resulting public or private key is invalid #120

Merged
merged 8 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: 2 additions & 0 deletions src/BIP44CoinTypeNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ export async function deriveBIP44AddressKey(
const childNode = await deriveChildNode({
path,
node,
specification: 'bip32',
});

return new BIP44Node(childNode);
Expand Down Expand Up @@ -413,6 +414,7 @@ export async function getBIP44AddressKeyDeriver(
: getUnhardenedBIP32NodeToken(address_index),
],
node: actualNode,
specification: 'bip32',
});

return new BIP44Node(slip10Node);
Expand Down
2 changes: 2 additions & 0 deletions src/BIP44Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export class BIP44Node implements BIP44NodeInterface {
parentFingerprint,
index,
curve: 'secp256k1',
specification: 'bip32',
});

return new BIP44Node(node);
Expand Down Expand Up @@ -213,6 +214,7 @@ export class BIP44Node implements BIP44NodeInterface {
const node = await SLIP10Node.fromDerivationPath({
derivationPath,
curve: 'secp256k1',
specification: 'bip32',
});

return new BIP44Node(node);
Expand Down
32 changes: 26 additions & 6 deletions src/SLIP10Node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ describe('SLIP10Node', () => {
privateKey: node.privateKey,
publicKey: node.publicKey,
chainCode: node.chainCode,
specification: node.specification,
});
});

Expand Down Expand Up @@ -519,20 +520,37 @@ describe('SLIP10Node', () => {
});

it('throws an error if no curve is specified', async () => {
// @ts-expect-error No curve specified, but required in type
await expect(SLIP10Node.fromDerivationPath({})).rejects.toThrow(
'Invalid curve: Must specify a curve.',
);
await expect(
// @ts-expect-error No curve specified, but required in type
SLIP10Node.fromDerivationPath({
specification: 'bip32',
}),
).rejects.toThrow('Invalid curve: Must specify a curve.');
});

it('throws an error for unsupported curves', async () => {
await expect(
// @ts-expect-error Invalid curve name for type
SLIP10Node.fromDerivationPath({ curve: 'foo bar' }),
SLIP10Node.fromDerivationPath({
// @ts-expect-error Invalid curve name for type
curve: 'foo bar',
specification: 'bip32',
}),
).rejects.toThrow(
'Invalid curve: Only the following curves are supported: secp256k1, ed25519.',
);
});

it('throws an error for unsupported specifications', async () => {
await expect(
SLIP10Node.fromDerivationPath({
curve: 'secp256k1',
// @ts-expect-error Invalid specification name for type
specification: 'foo bar',
}),
).rejects.toThrow(
'Invalid specification: Must be one of bip32, slip10. Received "foo bar".',
);
});
});

describe('derive', () => {
Expand Down Expand Up @@ -845,6 +863,7 @@ describe('SLIP10Node', () => {
privateKey: node.privateKey,
publicKey: node.publicKey,
chainCode: node.chainCode,
specification: node.specification,
});

expect(JSON.parse(JSON.stringify(nodeJson))).toStrictEqual({
Expand All @@ -856,6 +875,7 @@ describe('SLIP10Node', () => {
privateKey: node.privateKey,
publicKey: node.publicKey,
chainCode: node.chainCode,
specification: node.specification,
});
});
});
Expand Down
66 changes: 44 additions & 22 deletions src/SLIP10Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ import {
RootedSLIP10PathTuple,
SLIP10PathTuple,
} from './constants';
import { curves, getCurveByName, SupportedCurve } from './curves';
import { getCurveByName, SupportedCurve } from './curves';
import { deriveKeyFromPath } from './derivation';
import { Specification } from './derivers';
import { publicKeyToEthAddress } from './derivers/bip32';
import {
getBytes,
getBytesUnsafe,
getFingerprint,
getSpecification,
isValidInteger,
validateBIP32Index,
validateCurve,
validateSpecification,
} from './utils';

/**
Expand Down Expand Up @@ -64,6 +68,21 @@ export type JsonSLIP10Node = {
* The name of the curve used by the node.
*/
readonly curve: SupportedCurve;

/**
* The specification used to derive this node. Defaults to `bip32` when the
* `secp256k1` curve is used, and `slip10` when the `ed25519` curve is used.
*
* While SLIP-10 and BIP-32 are largely compatible when the `secp256k1` curve
* is used, they differ in the way they handle key derivation errors. The
* probability of this happening is extremely low, but it is possible. When
* full compatibility with one of these specifications is required, this field
* should be set to the appropriate value.
*
* Note that `ed25519` is not supported by BIP-32, so the `bip32`
* specification is not available for this curve.
*/
readonly specification: Specification;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As alternative to the specification field, we could change the derivation paths to accept slip10:${index}, but that requires some refactoring.

};

export type SLIP10NodeInterface = JsonSLIP10Node & {
Expand Down Expand Up @@ -95,6 +114,7 @@ export type SLIP10NodeConstructorOptions = {
readonly privateKey?: Uint8Array;
readonly publicKey: Uint8Array;
readonly curve: SupportedCurve;
readonly specification?: Specification;
};

export type SLIP10ExtendedKeyOptions = {
Expand All @@ -106,11 +126,13 @@ export type SLIP10ExtendedKeyOptions = {
readonly privateKey?: string | Uint8Array;
readonly publicKey?: string | Uint8Array;
readonly curve: SupportedCurve;
readonly specification?: Specification;
};

export type SLIP10DerivationPathOptions = {
readonly derivationPath: RootedSLIP10PathTuple;
readonly curve: SupportedCurve;
readonly specification?: Specification;
};

export class SLIP10Node implements SLIP10NodeInterface {
Expand Down Expand Up @@ -145,6 +167,7 @@ export class SLIP10Node implements SLIP10NodeInterface {
* specified, this parameter is ignored.
* @param options.chainCode - The chain code for the node.
* @param options.curve - The curve used by the node.
* @param options.specification - The specification used to derive this node.
*/
static async fromExtendedKey({
depth,
Expand All @@ -155,10 +178,12 @@ export class SLIP10Node implements SLIP10NodeInterface {
publicKey,
chainCode,
curve,
specification = getSpecification(curve),
}: SLIP10ExtendedKeyOptions) {
const chainCodeBytes = getBytes(chainCode, BYTES_KEY_LENGTH);

validateCurve(curve);
validateSpecification(specification);
validateBIP32Depth(depth);
validateBIP32Index(index);
validateRootIndex(index, depth);
Expand Down Expand Up @@ -188,6 +213,7 @@ export class SLIP10Node implements SLIP10NodeInterface {
privateKey: privateKeyBytes,
publicKey: await curveObject.getPublicKey(privateKeyBytes),
curve,
specification,
},
this.#constructorGuard,
);
Expand All @@ -205,6 +231,7 @@ export class SLIP10Node implements SLIP10NodeInterface {
chainCode: chainCodeBytes,
publicKey: publicKeyBytes,
curve,
specification,
},
this.#constructorGuard,
);
Expand Down Expand Up @@ -236,13 +263,17 @@ export class SLIP10Node implements SLIP10NodeInterface {
* @param options.derivationPath - The rooted HD tree path that will be used
* to derive the key of this node.
* @param options.curve - The curve used by the node.
* @param options.specification - The specification used to derive the key.
* Defaults to `slip10`.
* @returns A new SLIP-10 node.
*/
static async fromDerivationPath({
derivationPath,
curve,
specification = getSpecification(curve),
}: SLIP10DerivationPathOptions) {
validateCurve(curve);
validateSpecification(specification);

if (!derivationPath) {
throw new Error('Invalid options: Must provide a derivation path.');
Expand All @@ -258,13 +289,16 @@ export class SLIP10Node implements SLIP10NodeInterface {
path: derivationPath,
depth: derivationPath.length - 1,
curve,
specification,
});
}

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

public readonly curve: SupportedCurve;

public readonly specification: Specification;

public readonly depth: number;

public readonly masterFingerprint?: number;
Expand All @@ -290,6 +324,7 @@ export class SLIP10Node implements SLIP10NodeInterface {
privateKey,
publicKey,
curve,
specification = getSpecification(curve),
}: SLIP10NodeConstructorOptions,
constructorGuard?: symbol,
) {
Expand All @@ -306,6 +341,7 @@ export class SLIP10Node implements SLIP10NodeInterface {
this.privateKeyBytes = privateKey;
this.publicKeyBytes = publicKey;
this.curve = curve;
this.specification = specification;

Object.freeze(this);
}
Expand Down Expand Up @@ -382,6 +418,7 @@ export class SLIP10Node implements SLIP10NodeInterface {
return await deriveChildNode({
path,
node: this,
specification: this.specification,
});
}

Expand All @@ -396,31 +433,11 @@ export class SLIP10Node implements SLIP10NodeInterface {
privateKey: this.privateKey,
publicKey: this.publicKey,
chainCode: this.chainCode,
specification: this.specification,
};
}
}

/**
* Validates the curve name.
*
* @param curveName - The name of the curve to validate.
*/
function validateCurve(
curveName: unknown,
): asserts curveName is SupportedCurve {
if (!curveName || typeof curveName !== 'string') {
throw new Error('Invalid curve: Must specify a curve.');
}

if (!Object.keys(curves).includes(curveName)) {
throw new Error(
`Invalid curve: Only the following curves are supported: ${Object.keys(
curves,
).join(', ')}.`,
);
}
}

/**
* Validates a BIP-32 path depth. Effectively, asserts that the depth is an
* integer `number`. Throws an error if validation fails.
Expand Down Expand Up @@ -524,6 +541,7 @@ export function validateRootIndex(index: number, depth: number) {
type DeriveChildNodeArgs = {
path: SLIP10PathTuple;
node: SLIP10Node | BIP44Node | BIP44CoinTypeNode;
specification: Specification;
};

/**
Expand All @@ -532,11 +550,14 @@ type DeriveChildNodeArgs = {
* @param options - The options to use when deriving the child key.
* @param options.node - The node to derive from.
* @param options.path - The path to the child node / key.
* @param options.specification - The specification to use when deriving the
* child key.
* @returns The derived key and depth.
*/
export async function deriveChildNode({
path,
node,
specification,
}: DeriveChildNodeArgs): Promise<SLIP10Node> {
if (path.length === 0) {
throw new Error(
Expand All @@ -553,5 +574,6 @@ export async function deriveChildNode({
path,
node,
depth: newDepth,
specification,
});
}
2 changes: 2 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ export const BYTES_KEY_LENGTH = 32;
export const MIN_BIP_44_DEPTH = 0;
export const MAX_BIP_44_DEPTH = 5;

export const MAX_BIP_32_INDEX = 0xffffffff;

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;
Expand Down
Loading