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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3e8eb1d
add createKeyPairFromBytes_DANGEROUSLY_SKIP_VALIDATION and validation
Mar 16, 2024
1c3c7c0
add testcases and error codes for createKeyPairFromBytes
Mar 16, 2024
4e9304b
update error message
Mar 16, 2024
0e7548f
formatting
Mar 16, 2024
62c7121
address feedback
Mar 18, 2024
70611e0
address feedback add assertPRNGIsAvailable
Mar 20, 2024
99ff817
update
Mar 20, 2024
9292acf
address feedback
Mar 23, 2024
0f1a0ea
Create thick-radios-search.md
steveluscher Mar 23, 2024
6e50c49
run prettier
Mar 23, 2024
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
4 changes: 3 additions & 1 deletion packages/errors/src/codes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ 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__VERIFY_SIGNATURE_FAILURE = 3704004 as const;
luu-alex marked this conversation as resolved.
Show resolved Hide resolved

// Instruction-related errors.
// Reserve error codes in the range [4128000-4128999].
Expand Down Expand Up @@ -417,6 +418,7 @@ export type SolanaErrorCode =
| typeof SOLANA_ERROR__KEYS__INVALID_PRIVATE_KEY_BYTE_LENGTH
| typeof SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH
| typeof SOLANA_ERROR__KEYS__SIGNATURE_STRING_LENGTH_OUT_OF_RANGE
| typeof SOLANA_ERROR__KEYS__VERIFY_SIGNATURE_FAILURE
| typeof SOLANA_ERROR__LAMPORTS_OUT_OF_RANGE
| typeof SOLANA_ERROR__MALFORMED_BIGINT_STRING
| typeof SOLANA_ERROR__MALFORMED_NUMBER_STRING
Expand Down Expand Up @@ -501,4 +503,4 @@ export type SolanaErrorCode =
| typeof SOLANA_ERROR__TRANSACTION_ERROR__WOULD_EXCEED_ACCOUNT_DATA_TOTAL_LIMIT
| typeof SOLANA_ERROR__TRANSACTION_ERROR__WOULD_EXCEED_MAX_ACCOUNT_COST_LIMIT
| typeof SOLANA_ERROR__TRANSACTION_ERROR__WOULD_EXCEED_MAX_BLOCK_COST_LIMIT
| typeof SOLANA_ERROR__TRANSACTION_ERROR__WOULD_EXCEED_MAX_VOTE_COST_LIMIT;
| typeof SOLANA_ERROR__TRANSACTION_ERROR__WOULD_EXCEED_MAX_VOTE_COST_LIMIT;
2 changes: 2 additions & 0 deletions packages/errors/src/messages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ import {
SOLANA_ERROR__KEYS__INVALID_PRIVATE_KEY_BYTE_LENGTH,
SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH,
SOLANA_ERROR__KEYS__SIGNATURE_STRING_LENGTH_OUT_OF_RANGE,
SOLANA_ERROR__KEYS__VERIFY_SIGNATURE_FAILURE,
SOLANA_ERROR__LAMPORTS_OUT_OF_RANGE,
SOLANA_ERROR__MALFORMED_BIGINT_STRING,
SOLANA_ERROR__MALFORMED_NUMBER_STRING,
Expand Down Expand Up @@ -402,6 +403,7 @@ export const SolanaErrorMessages: Readonly<{
'Expected base58-encoded signature to decode to a byte array of length 64. Actual length: $actualLength.',
[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__KEYS__VERIFY_SIGNATURE_FAILURE]: 'The provided private key and public key are not a valid key pair',
luu-alex marked this conversation as resolved.
Show resolved Hide resolved
[SOLANA_ERROR__LAMPORTS_OUT_OF_RANGE]: 'Lamports value must be in the range [0, 2e64-1]',
[SOLANA_ERROR__MALFORMED_BIGINT_STRING]: '`$value` cannot be parsed as a `BigInt`',
[SOLANA_ERROR__MALFORMED_NUMBER_STRING]: '`$value` cannot be parsed as a `Number`',
Expand Down
128 changes: 97 additions & 31 deletions packages/keys/src/__tests__/key-pair-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,101 @@
import { generateKeyPair } from '../key-pair';
import { SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH, SolanaError, SOLANA_ERROR__KEYS__VERIFY_SIGNATURE_FAILURE } from '@solana/errors';

Check failure on line 1 in packages/keys/src/__tests__/key-pair-test.ts

View workflow job for this annotation

GitHub Actions / Build & Test on Node lts/* 

Run autofix to sort these imports!
import { generateKeyPair, createKeyPairFromBytes, createKeyPairFromBytes_DANGEROUSLY_SKIP_VALIDATION } from '../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,
}),
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__VERIFY_SIGNATURE_FAILURE));
})
})

describe('createKeyPairFromBytes_DANGEROUSLY_SKIP_VALIDATION', () => {
it('creates a key pair from a 64-byte array', async () => {
expect.assertions(1);
const keyPair = await createKeyPairFromBytes_DANGEROUSLY_SKIP_VALIDATION(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 }));
});
});
});
});
26 changes: 25 additions & 1 deletion packages/keys/src/key-pair.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { assertKeyGenerationIsAvailable } from '@solana/assertions';

Check failure on line 1 in packages/keys/src/key-pair.ts

View workflow job for this annotation

GitHub Actions / Build & Test on Node lts/* 

Run autofix to sort these imports!
import { SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH, SolanaError } from '@solana/errors';
import { SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH, SolanaError, SOLANA_ERROR__KEYS__VERIFY_SIGNATURE_FAILURE } from '@solana/errors';

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

const TEST_DATA = new Uint8Array([1, 2, 3, 4, 5]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like this to be randomly generated on each pass through the function.


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 +24,26 @@
crypto.subtle.importKey('raw', bytes.slice(32), 'Ed25519', /* extractable */ true, ['verify']),
createPrivateKeyFromBytes(bytes.slice(0, 32), extractable),
]);

// Verify the key pair
const signedData = await signBytes(privateKey, TEST_DATA);
const isValid = await verifySignature(publicKey, signedData, TEST_DATA);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const signedData = await signBytes(privateKey, TEST_DATA);
const isValid = await verifySignature(publicKey, signedData, TEST_DATA);
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__VERIFY_SIGNATURE_FAILURE);
}

return { privateKey, publicKey } as CryptoKeyPair;
}

// Similar to createKeyPairFromBytes, but does not verify the key pair
export async function createKeyPairFromBytes_DANGEROUSLY_SKIP_VALIDATION(bytes: Uint8Array, extractable?: boolean): Promise<CryptoKeyPair> {
if (bytes.byteLength !== 64) {
throw new SolanaError(SOLANA_ERROR__KEYS__INVALID_KEY_PAIR_BYTE_LENGTH, { byteLength: bytes.byteLength });
}
const [publicKey, privateKey] = await Promise.all([
crypto.subtle.importKey('raw', bytes.slice(32), 'Ed25519', /* extractable */ true, ['verify']),
createPrivateKeyFromBytes(bytes.slice(0, 32), extractable),
]);

return { privateKey, publicKey } as CryptoKeyPair;
}
luu-alex marked this conversation as resolved.
Show resolved Hide resolved
Loading