-
Notifications
You must be signed in to change notification settings - Fork 14
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
Pad SDK browser Import Keys #323
Changes from 17 commits
20f973a
852822a
e04b54e
c427f0d
e3187d7
782f50b
4fa3a99
23d137b
98cb871
4088579
be750fa
af19868
5a81f26
be40867
e75b2f5
d486dc9
b30fa8d
e491d2c
30c94b8
f4b607f
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,80 @@ | ||
import { test, expect } from "@jest/globals"; | ||
import { pointDecode } from "../tink/elliptic_curves"; | ||
import { | ||
uint8ArrayFromHexString, | ||
uint8ArrayToHexString, | ||
} from "@turnkey/encoding"; | ||
|
||
test("pointDecode -> uncompressed invalid", async function () { | ||
// Invalid uncompressed key (the last byte has been changed) | ||
const uncompPubKey = | ||
"0400d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b366c4872345594c12a7f7c47c62dd8074542934820fce5ee0ddc55d6d1d8dd313"; | ||
expect(() => pointDecode(uint8ArrayFromHexString(uncompPubKey))).toThrow( | ||
"invalid uncompressed x and y coordinates" | ||
); | ||
}); | ||
|
||
test("pointDecode -> uncompressed valid", async function () { | ||
// Valid uncompressed public key with 00 as the first bit (test against 'x' field of JWK getting truncated) | ||
const uncompPubKey = | ||
"0400d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b366c4872345594c12a7f7c47c62dd8074542934820fce5ee0ddc55d6d1d8dd312"; | ||
const jwk = pointDecode(uint8ArrayFromHexString(uncompPubKey)); | ||
expect(jwk.x).toBeDefined(); | ||
expect(jwk.y).toBeDefined(); | ||
|
||
// Convert x value to make sure it matches first half of uncompressed key WITHOUT truncating the first 0 bit | ||
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. This is great! If you'd like, you could also try importing the resulting JWK as a sanity check as well (e.g. the Note: if you do go down this path, you might need to import
Can use 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. Totally fine if you think this is overkill given some of the manual validation you're doing here 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. Thanks for the suggestion here!! I'll leave the manual validation as is for right now so we can get this over the line in a timely manner |
||
let xString: string = jwk.x !== undefined ? jwk.x : "_"; | ||
let xBytes = new Uint8Array(base64urlToBuffer(xString)); | ||
let xHex = uint8ArrayToHexString(xBytes); | ||
expect(xHex).toBe(uncompPubKey.substring(2, 66)); | ||
|
||
// Convert y value to make sure it's the same as second half of uncompressed key | ||
let yString: string = jwk.y !== undefined ? jwk.y : "_"; | ||
let yBytes = new Uint8Array(base64urlToBuffer(yString)); | ||
let yHex = uint8ArrayToHexString(yBytes); | ||
expect(yHex).toBe(uncompPubKey.substring(66, 130)); | ||
}); | ||
|
||
test("pointDecode -> compressed", async function () { | ||
// Valid compressed public key with 00 as the first bit (test against 'x' field of JWK getting truncated) | ||
const uncompPubKey = | ||
"0400d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b366c4872345594c12a7f7c47c62dd8074542934820fce5ee0ddc55d6d1d8dd312"; | ||
// corresponding compressed public key | ||
const compPubKey = | ||
"0200d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b3"; | ||
|
||
const jwk = pointDecode(uint8ArrayFromHexString(compPubKey)); | ||
expect(jwk.x).toBeDefined(); | ||
expect(jwk.y).toBeDefined(); | ||
|
||
// Convert x value to make sure it matches first half of uncompressed key WITHOUT truncating the first 0 bit | ||
let xString: string = jwk.x !== undefined ? jwk.x : "_"; | ||
let xBytes = new Uint8Array(base64urlToBuffer(xString)); | ||
let xHex = uint8ArrayToHexString(xBytes); | ||
expect(xHex).toBe(uncompPubKey.substring(2, 66)); | ||
|
||
// Convert y value to make sure it's the same as second half of uncompressed key | ||
let yString: string = jwk.y !== undefined ? jwk.y : "_"; | ||
let yBytes = new Uint8Array(base64urlToBuffer(yString)); | ||
let yHex = uint8ArrayToHexString(yBytes); | ||
expect(yHex).toBe(uncompPubKey.substring(66, 130)); | ||
}); | ||
|
||
// Convert base64 url encoded string to an array -- used here to test that output pads correctly and doesn't get truncated | ||
function base64urlToBuffer(baseurl64String: string): ArrayBuffer { | ||
// Base64url to Base64 | ||
const padding = "==".slice(0, (4 - (baseurl64String.length % 4)) % 4); | ||
const base64String = | ||
baseurl64String.replace(/-/g, "+").replace(/_/g, "/") + padding; | ||
|
||
// Base64 to binary string | ||
const str = atob(base64String); | ||
|
||
// Binary string to buffer | ||
const buffer = new ArrayBuffer(str.length); | ||
const byteView = new Uint8Array(buffer); | ||
for (let i = 0; i < str.length; i++) { | ||
byteView[i] = str.charCodeAt(i); | ||
} | ||
return buffer; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
/** | ||
* Code modified from https://github.com/google/tink/blob/6f74b99a2bfe6677e3670799116a57268fd067fa/javascript/subtle/elliptic_curves.ts | ||
* - The implementation of integerToByteArray has been modified to augment the resulting byte array to a certain length. | ||
* - The implementation of PointDecode has been modified to decode both compressed and uncompressed points by checking for correct format | ||
* - Methoed isP256CurvePoint added to check whether an uncompressed point is valid | ||
omkarshanbhag marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* @license | ||
* Copyright 2020 Google LLC | ||
|
@@ -136,35 +138,86 @@ function getY(x: bigint, lsb: boolean): bigint { | |
} | ||
|
||
/** | ||
* Decodes a public key in _compressed_ format. | ||
* | ||
* Given x and y coordinates of a JWK, checks whether these are valid points on | ||
* the P-256 elliptic curve. | ||
* | ||
* P-256 only | ||
* | ||
* @param x x-coordinate | ||
* @param y y-coordinate | ||
* @return boolean validity | ||
*/ | ||
function isP256CurvePoint(x: bigint, y: bigint): boolean { | ||
const p = getModulus(); | ||
const a = p - BigInt(3); | ||
const b = getB(); | ||
const rhs = ((x * x + a) * x + b) % p; | ||
const lhs = y ** BigInt(2) % p; | ||
return lhs === rhs; | ||
} | ||
|
||
/** | ||
* Decodes a public key in _compressed_ OR _uncompressed_ format. | ||
* Augmented to ensure that the x and y components are padded to fit 32 bytes. | ||
* | ||
* P-256 only | ||
*/ | ||
export function pointDecode(point: Uint8Array): JsonWebKey { | ||
const fieldSize = fieldSizeInBytes(); | ||
|
||
if (point.length !== 1 + fieldSize) { | ||
throw new Error("compressed point has wrong length"); | ||
} | ||
if (point[0] !== 2 && point[0] !== 3) { | ||
throw new Error("invalid format"); | ||
const compressedLength = fieldSize + 1; | ||
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. This looks like a lot of new code, and we're in a vendored file! At this point it'd be really hard for someone to understand how to upgrade tink if we ever had to. Still, can you add a summary of what was done in this PR at the top of the file? 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. Ah yeah wanted to follow the earlier suggestion of moving the uncompressed Pub key point into pointDecode -- didn't realize this is a vendored file until later will definitely add some context on top. Also -- it seems like we've changed the file significantly from what the original implementation is for our own purposes. The original implementation also does have the capacity to check uncompressed points! 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. (all this is to say that our implementation is already quite different from what's in the original, and this actually kinda brings it closer to the original) |
||
const uncompressedLength = 2 * fieldSize + 1; | ||
if ( | ||
point.length !== compressedLength && | ||
point.length !== uncompressedLength | ||
) { | ||
throw new Error( | ||
"Invalid length: point is not in compressed or uncompressed format" | ||
); | ||
} | ||
const lsb = point[0] === 3; // point[0] must be 2 (false) or 3 (true). | ||
const x = byteArrayToInteger(point.subarray(1, point.length)); | ||
const p = getModulus(); | ||
if (x < BigInt(0) || x >= p) { | ||
throw new Error("x is out of range"); | ||
// Decodes point if its length and first bit match the compressed format | ||
if ((point[0] === 2 || point[0] === 3) && point.length == compressedLength) { | ||
omkarshanbhag marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const lsb = point[0] === 3; // point[0] must be 2 (false) or 3 (true). | ||
const x = byteArrayToInteger(point.subarray(1, point.length)); | ||
const p = getModulus(); | ||
if (x < BigInt(0) || x >= p) { | ||
throw new Error("x is out of range"); | ||
} | ||
const y = getY(x, lsb); | ||
const result: JsonWebKey = { | ||
kty: "EC", | ||
crv: "P-256", | ||
x: Bytes.toBase64(integerToByteArray(x, 32), /* websafe */ true), | ||
y: Bytes.toBase64(integerToByteArray(y, 32), /* websafe */ true), | ||
ext: true, | ||
}; | ||
return result; | ||
// Decodes point if its length and first bit match the uncompressed format | ||
} else if (point[0] === 4 && point.length == uncompressedLength) { | ||
const x = byteArrayToInteger(point.subarray(1, fieldSize + 1)); | ||
const y = byteArrayToInteger( | ||
point.subarray(fieldSize + 1, 2 * fieldSize + 1) | ||
); | ||
const p = getModulus(); | ||
if ( | ||
x < BigInt(0) || | ||
x >= p || | ||
y < BigInt(0) || | ||
y >= p || | ||
!isP256CurvePoint(x, y) | ||
) { | ||
throw new Error("invalid uncompressed x and y coordinates"); | ||
} | ||
const result: JsonWebKey = { | ||
kty: "EC", | ||
crv: "P-256", | ||
x: Bytes.toBase64(integerToByteArray(x, 32), /* websafe */ true), | ||
y: Bytes.toBase64(integerToByteArray(y, 32), /* websafe */ true), | ||
ext: true, | ||
}; | ||
return result; | ||
} | ||
const y = getY(x, lsb); | ||
const result: JsonWebKey = { | ||
kty: "EC", | ||
crv: "P-256", | ||
x: Bytes.toBase64(integerToByteArray(x, 32), /* websafe */ true), | ||
y: Bytes.toBase64(integerToByteArray(y, 32), /* websafe */ true), | ||
ext: true, | ||
}; | ||
return result; | ||
throw new Error("invalid format"); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { test, expect } from "@jest/globals"; | ||
import { createEmbeddedAPIKey } from "../utils"; | ||
|
||
test("createEmbeddedAPIKey", async function () { | ||
// Test valid uncompressed public key | ||
const uncompPubKey = | ||
"0400d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b366c4872345594c12a7f7c47c62dd8074542934820fce5ee0ddc55d6d1d8dd312"; | ||
await expect(createEmbeddedAPIKey(uncompPubKey)).resolves.not.toThrow(); | ||
omkarshanbhag marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// test invalid uncompressed public key (last byte has been changed) | ||
const uncompPubKeyInvalid = | ||
"0400d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b366c4872345594c12a7f7c47c62dd8074542934820fce5ee0ddc55d6d1d8dd311"; | ||
await expect(createEmbeddedAPIKey(uncompPubKeyInvalid)).rejects.toThrow(); | ||
}); |
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.
I don't see a test for compressed keys and invalid lengths cases. Let's cover these?
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.
I'll add that in!