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

[experimental] Validate that the public key generated from createKeyPairFromBytes() belongs to the private key #2329

Merged
merged 10 commits into from
Mar 26, 2024
Merged
28 changes: 28 additions & 0 deletions packages/assertions/src/__tests__/crypto-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED, SolanaError } from '@solana/errors';

import { assertPRNGIsAvailable } from '../crypto';

describe('assertPRNGIsAvailable()', () => {
it('resolves to `undefined` without throwing', async () => {
expect.assertions(1);
await expect(assertPRNGIsAvailable()).resolves.toBeUndefined();
});
describe('when getRandomValues is not available', () => {
let oldCrypto: InstanceType<typeof Crypto>['getRandomValues'];
beforeEach(() => {
oldCrypto = globalThis.crypto.getRandomValues;
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
globalThis.crypto.getRandomValues = undefined;
});
afterEach(() => {
globalThis.crypto.getRandomValues = oldCrypto;
});
it('rejects', async () => {
expect.assertions(1);
await expect(() => assertPRNGIsAvailable()).rejects.toThrow(
new SolanaError(SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED),
);
});
});
});
7 changes: 7 additions & 0 deletions packages/assertions/src/crypto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED, SolanaError } from '@solana/errors';

export async function assertPRNGIsAvailable() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm… I wonder why some of these were made async. Seems like a mistake we shouldn't replicate here.

if (typeof globalThis.crypto === 'undefined' || typeof globalThis.crypto.getRandomValues !== 'function') {
throw new SolanaError(SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED);
}
}
7 changes: 7 additions & 0 deletions packages/errors/src/codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,17 @@ export const SOLANA_ERROR__SUBTLE_CRYPTO__GENERATE_FUNCTION_UNIMPLEMENTED = 3610
export const SOLANA_ERROR__SUBTLE_CRYPTO__SIGN_FUNCTION_UNIMPLEMENTED = 3610005 as const;
export const SOLANA_ERROR__SUBTLE_CRYPTO__VERIFY_FUNCTION_UNIMPLEMENTED = 3610006 as const;

// Crypto-related errors.
luu-alex marked this conversation as resolved.
Show resolved Hide resolved
// Reserve error codes in the range [3611000-3611050].
export const SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED = 3610007 as const;

// Key-related errors.
// Reserve error codes in the range [3704000-3704999].
export const SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH = 3704000 as const;
export const SOLANA_ERROR__KEYS__INVALID_PRIVATE_KEY_BYTE_LENGTH = 3704001 as const;
export const SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH = 3704002 as const;
export const SOLANA_ERROR__KEYS__SIGNATURE_STRING_LENGTH_OUT_OF_RANGE = 3704003 as const;
export const SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY = 3704004 as const;

// Instruction-related errors.
// Reserve error codes in the range [4128000-4128999].
Expand Down Expand Up @@ -328,6 +333,7 @@ export type SolanaErrorCode =
| typeof SOLANA_ERROR__CODECS__INVALID_STRING_FOR_BASE
| typeof SOLANA_ERROR__CODECS__NUMBER_OUT_OF_RANGE
| typeof SOLANA_ERROR__CODECS__OFFSET_OUT_OF_RANGE
| typeof SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED
| typeof SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_ACCOUNTS
| typeof SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_DATA
| typeof SOLANA_ERROR__INSTRUCTION__PROGRAM_ID_MISMATCH
Expand Down Expand Up @@ -416,6 +422,7 @@ export type SolanaErrorCode =
| typeof SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH
| typeof SOLANA_ERROR__KEYS__INVALID_PRIVATE_KEY_BYTE_LENGTH
| typeof SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH
| typeof SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY
| typeof SOLANA_ERROR__KEYS__SIGNATURE_STRING_LENGTH_OUT_OF_RANGE
| typeof SOLANA_ERROR__LAMPORTS_OUT_OF_RANGE
| typeof SOLANA_ERROR__MALFORMED_BIGINT_STRING
Expand Down
5 changes: 5 additions & 0 deletions packages/errors/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
SOLANA_ERROR__CODECS__INVALID_STRING_FOR_BASE,
SOLANA_ERROR__CODECS__NUMBER_OUT_OF_RANGE,
SOLANA_ERROR__CODECS__OFFSET_OUT_OF_RANGE,
SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED,
SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_ACCOUNTS,
SOLANA_ERROR__INSTRUCTION__EXPECTED_TO_HAVE_DATA,
SOLANA_ERROR__INSTRUCTION__PROGRAM_ID_MISMATCH,
Expand Down Expand Up @@ -120,6 +121,7 @@ import {
SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH,
SOLANA_ERROR__KEYS__INVALID_PRIVATE_KEY_BYTE_LENGTH,
SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH,
SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY,
SOLANA_ERROR__KEYS__SIGNATURE_STRING_LENGTH_OUT_OF_RANGE,
SOLANA_ERROR__LAMPORTS_OUT_OF_RANGE,
SOLANA_ERROR__MALFORMED_BIGINT_STRING,
Expand Down Expand Up @@ -274,6 +276,7 @@ export const SolanaErrorMessages: Readonly<{
'Codec [$codecDescription] expected number to be in the range [$min, $max], got $value.',
[SOLANA_ERROR__CODECS__OFFSET_OUT_OF_RANGE]:
'Codec [$codecDescription] expected offset to be in the range [0, $bytesLength], got $offset.',
[SOLANA_ERROR__CRYPTO__RANDOM_VALUES_FUNCTION_UNIMPLEMENTED]: 'No random values implementation could be found.',
[SOLANA_ERROR__INSTRUCTION_ERROR__ACCOUNT_ALREADY_INITIALIZED]: 'instruction requires an uninitialized account',
[SOLANA_ERROR__INSTRUCTION_ERROR__ACCOUNT_BORROW_FAILED]:
'instruction tries to borrow reference for an account which is already borrowed',
Expand Down Expand Up @@ -400,6 +403,8 @@ export const SolanaErrorMessages: Readonly<{
'Expected private key bytes with length 32. Actual length: $actualLength.',
[SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH]:
'Expected base58-encoded signature to decode to a byte array of length 64. Actual length: $actualLength.',
[SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY]:
'The provided private key does not match the provided public key.',
[SOLANA_ERROR__KEYS__SIGNATURE_STRING_LENGTH_OUT_OF_RANGE]:
'Expected base58-encoded signature string of length in the range [64, 88]. Actual length: $actualLength.',
[SOLANA_ERROR__LAMPORTS_OUT_OF_RANGE]: 'Lamports value must be in the range [0, 2e64-1]',
Expand Down
113 changes: 83 additions & 30 deletions packages/keys/src/__tests__/key-pair-test.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,88 @@
import { generateKeyPair } from '../key-pair';
import {
SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH,
SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY,
SolanaError,
} from '@solana/errors';

describe('generateKeyPair', () => {
it.each(['private', 'public'])('generates an ed25519 %s `CryptoKey`', async type => {
expect.assertions(1);
const keyPair = await generateKeyPair();
expect(keyPair).toMatchObject({
[`${type}Key`]: expect.objectContaining({
[Symbol.toStringTag]: 'CryptoKey',
algorithm: { name: 'Ed25519' },
type,
}),
import { createKeyPairFromBytes, generateKeyPair } from '../key-pair';

const MOCK_KEY_BYTES = new Uint8Array([
0xeb, 0xfa, 0x65, 0xeb, 0x93, 0xdc, 0x79, 0x15, 0x7a, 0xba, 0xde, 0xa2, 0xf7, 0x94, 0x37, 0x9d, 0xfc, 0x07, 0x1d,
0x68, 0x86, 0x87, 0x37, 0x6d, 0xc5, 0xd5, 0xa0, 0x54, 0x12, 0x1d, 0x34, 0x4a, 0x1d, 0x0e, 0x93, 0x86, 0x4d, 0xcc,
0x81, 0x5f, 0xc3, 0xf2, 0x86, 0x18, 0x09, 0x11, 0xd0, 0x0a, 0x3f, 0xd2, 0x06, 0xde, 0x31, 0xa1, 0xc9, 0x42, 0x87,
0xcb, 0x43, 0xf0, 0x5f, 0xc9, 0xf2, 0xb5,
]);

const MOCK_INVALID_KEY_BYTES = new Uint8Array([
0xeb, 0xfa, 0x65, 0xeb, 0x93, 0xdc, 0x79, 0x15, 0x7a, 0xba, 0xde, 0xa2, 0xf7, 0x94, 0x37, 0x9d, 0xfc, 0x07, 0x1d,
0x68, 0x86, 0x87, 0x37, 0x6d, 0xc5, 0xd5, 0xa0, 0x54, 0x12, 0x1d, 0x34, 0x4a, 0x1d, 0x0e, 0x93, 0x86, 0x4d, 0xcc,
0x81, 0x5f, 0xc3, 0xf2, 0x86, 0x18, 0x09, 0x11, 0xd0, 0x0a, 0x3f, 0xd2, 0x06, 0xde, 0x31, 0xa1, 0xc9, 0x42, 0x87,
0xcb, 0x43, 0xf0, 0x5f, 0xc9, 0xf2, 0xb1,
]);

describe('key-pair', () => {
describe('generateKeyPair', () => {
it.each(['private', 'public'])('generates an ed25519 %s `CryptoKey`', async type => {
expect.assertions(1);
const keyPair = await generateKeyPair();
expect(keyPair).toMatchObject({
[`${type}Key`]: expect.objectContaining({
[Symbol.toStringTag]: 'CryptoKey',
algorithm: { name: 'Ed25519' },
type,
}),
});
});
it('generates a non-extractable private key', async () => {
expect.assertions(1);
const { privateKey } = await generateKeyPair();
expect(privateKey).toHaveProperty('extractable', false);
});
it('generates a private key usable for signing operations', async () => {
expect.assertions(1);
const { privateKey } = await generateKeyPair();
expect(privateKey).toHaveProperty('usages', ['sign']);
});
it('generates an extractable public key', async () => {
expect.assertions(1);
const { publicKey } = await generateKeyPair();
expect(publicKey).toHaveProperty('extractable', true);
});
it('generates a public key usable for verifying signatures', async () => {
expect.assertions(1);
const { publicKey } = await generateKeyPair();
expect(publicKey).toHaveProperty('usages', ['verify']);
});
});
it('generates a non-extractable private key', async () => {
expect.assertions(1);
const { privateKey } = await generateKeyPair();
expect(privateKey).toHaveProperty('extractable', false);
});
it('generates a private key usable for signing operations', async () => {
expect.assertions(1);
const { privateKey } = await generateKeyPair();
expect(privateKey).toHaveProperty('usages', ['sign']);
});
it('generates an extractable public key', async () => {
expect.assertions(1);
const { publicKey } = await generateKeyPair();
expect(publicKey).toHaveProperty('extractable', true);
});
it('generates a public key usable for verifying signatures', async () => {
expect.assertions(1);
const { publicKey } = await generateKeyPair();
expect(publicKey).toHaveProperty('usages', ['verify']);

describe('createKeyPairFromBytes', () => {
it('creates a key pair from a 64-byte array', async () => {
expect.assertions(1);
const keyPair = await createKeyPairFromBytes(MOCK_KEY_BYTES);
expect(keyPair).toMatchObject({
privateKey: expect.objectContaining({
[Symbol.toStringTag]: 'CryptoKey',
algorithm: { name: 'Ed25519' },
type: 'private',
}),
publicKey: expect.objectContaining({
[Symbol.toStringTag]: 'CryptoKey',
algorithm: { name: 'Ed25519' },
type: 'public',
}),
});
});
it('errors when the byte array is not 64 bytes', async () => {
expect.assertions(1);
await expect(createKeyPairFromBytes(MOCK_KEY_BYTES.slice(0, 31))).rejects.toThrow(
new SolanaError(SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH, { byteLength: 31 }),
);
});
it('errors when public key fails signature verification', async () => {
expect.assertions(1);
await expect(createKeyPairFromBytes(MOCK_INVALID_KEY_BYTES)).rejects.toThrow(
new SolanaError(SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY),
);
});
});
});
17 changes: 16 additions & 1 deletion packages/keys/src/key-pair.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { assertKeyGenerationIsAvailable } from '@solana/assertions';
import { SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH, SolanaError } from '@solana/errors';
import {
SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH,
SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY,
SolanaError,
} from '@solana/errors';

import { createPrivateKeyFromBytes } from './private-key';
import { signBytes, verifySignature } from './signatures';

export async function generateKeyPair(): Promise<CryptoKeyPair> {
await assertKeyGenerationIsAvailable();
Copy link
Collaborator

Choose a reason for hiding this comment

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

To generate a test message randomly each time, we'll need to assert that the PRNG is available (that crypto.getRandomValues is a function. Can you add that, and a related error code, to @solana/assertions`.

Suggested change
await assertKeyGenerationIsAvailable();
await Promise.all([
assertKeyGenerationIsAvailable(),
assertPRNGIsAvailable(), // <-- add this to `@solana/assertions` & `@solana/errors`
]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would make more sense to put assertPRNGIsAvailable() in createKeyPairFromBytes than generateKeyPair right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, of course. Oops; I left my comment in the wrong spot!

Expand All @@ -21,5 +26,15 @@ export async function createKeyPairFromBytes(bytes: Uint8Array, extractable?: bo
crypto.subtle.importKey('raw', bytes.slice(32), 'Ed25519', /* extractable */ true, ['verify']),
createPrivateKeyFromBytes(bytes.slice(0, 32), extractable),
]);

// Verify the key pair
const randomBytes = new Uint8Array(32);
crypto.getRandomValues(randomBytes);
const signedData = await signBytes(privateKey, randomBytes);
const isValid = await verifySignature(publicKey, signedData, randomBytes);
if (!isValid) {
throw new SolanaError(SOLANA_ERROR__KEYS__PUBLIC_KEY_MUST_MATCH_PRIVATE_KEY);
}

return { privateKey, publicKey } as CryptoKeyPair;
}