-
Notifications
You must be signed in to change notification settings - Fork 181
Enforce that any SharedArrayBuffers that get passed to crypto operations get cloned as non-shared
#1116
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
Enforce that any SharedArrayBuffers that get passed to crypto operations get cloned as non-shared
#1116
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| '@solana/codecs-numbers': patch | ||
| '@solana/codecs-strings': patch | ||
| '@solana/codecs-core': patch | ||
| '@solana/compat': patch | ||
| '@solana/keys': patch | ||
| --- | ||
|
|
||
| Any `SharedArrayBuffer` that gets passed to a crypto operation like `signBytes` or `verifySignature` will now be copied as non-shared. Crypto operations like `sign` and `verify` reject `SharedArrayBuffers` otherwise |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import { SOLANA_ERROR__KEYS__INVALID_PRIVATE_KEY_BYTE_LENGTH, SolanaError } from | |
|
|
||
| import { ED25519_ALGORITHM_IDENTIFIER } from './algorithm'; | ||
|
|
||
| function addPkcs8Header(bytes: ReadonlyUint8Array): ReadonlyUint8Array { | ||
| function addPkcs8Header(bytes: ReadonlyUint8Array): ReadonlyUint8Array<ArrayBuffer> { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, we're able to express that it's readonly, and non-shared (because we just created it inline). |
||
| // prettier-ignore | ||
| return new Uint8Array([ | ||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { assertSigningCapabilityIsAvailable, assertVerificationCapabilityIsAvailable } from '@solana/assertions'; | ||
| import { Encoder, ReadonlyUint8Array } from '@solana/codecs-core'; | ||
| import { Encoder, ReadonlyUint8Array, toArrayBuffer } from '@solana/codecs-core'; | ||
| import { getBase58Encoder } from '@solana/codecs-strings'; | ||
| import { | ||
| SOLANA_ERROR__KEYS__INVALID_SIGNATURE_BYTE_LENGTH, | ||
|
|
@@ -185,7 +185,7 @@ export function isSignatureBytes(putativeSignatureBytes: ReadonlyUint8Array): pu | |
| */ | ||
| export async function signBytes(key: CryptoKey, data: ReadonlyUint8Array): Promise<SignatureBytes> { | ||
| assertSigningCapabilityIsAvailable(); | ||
| const signedData = await crypto.subtle.sign(ED25519_ALGORITHM_IDENTIFIER, key, data); | ||
| const signedData = await crypto.subtle.sign(ED25519_ALGORITHM_IDENTIFIER, key, toArrayBuffer(data)); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Allows people to pass |
||
| return new Uint8Array(signedData) as SignatureBytes; | ||
| } | ||
|
|
||
|
|
@@ -249,5 +249,5 @@ export async function verifySignature( | |
| data: ReadonlyUint8Array, | ||
| ): Promise<boolean> { | ||
| assertVerificationCapabilityIsAvailable(); | ||
| return await crypto.subtle.verify(ED25519_ALGORITHM_IDENTIFIER, key, signature, data); | ||
| return await crypto.subtle.verify(ED25519_ALGORITHM_IDENTIFIER, key, toArrayBuffer(signature), toArrayBuffer(data)); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about this narrowing, @lorisleiva. I presume that encoders should never return a
SharedArrayBuffer.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I can't think of a scenario where you'd want the returned bytes to be shared — and if that was the case, you'd probably want to be super explicit about it and wrap the encoder into something else.