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

Pad SDK browser Import Keys #323

Merged
merged 20 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 19 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
80 changes: 80 additions & 0 deletions packages/api-key-stamper/src/__tests__/elliptic-curves-test.ts
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 () {
Copy link
Contributor

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?

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'll add that in!

// 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 crypto.subtle.importKey... codepath that happens in packages/sdk-browser/src/utils.ts).

Note: if you do go down this path, you might need to import crypto to have it defined in the test setting:

Object.defineProperty(globalThis, "crypto", {
  value: {
    getRandomValues: (arr) => crypto.randomBytes(arr.length),
    subtle: crypto.subtle,
  },
});

Can use packages/api-key-stamper/src/__tests__/utils-test.ts in this diff as a reference

Copy link
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
}
2 changes: 2 additions & 0 deletions packages/api-key-stamper/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,5 @@ export class ApiKeyStamper {
};
}
}

export { pointDecode } from "./tink/elliptic_curves";
95 changes: 74 additions & 21 deletions packages/api-key-stamper/src/tink/elliptic_curves.ts
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
* - Method isP256CurvePoint added to check whether an uncompressed point is valid
*
* @license
* Copyright 2020 Google LLC
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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");
}

/**
Expand Down
16 changes: 16 additions & 0 deletions packages/encoding/src/__tests__/index-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@ test("uint8ArrayFromHexString", async function () {
expect(() => {
uint8ArrayFromHexString("0100", 1).toString(); // the number 256 cannot fit into 1 byte
}).toThrow("hex value cannot fit in a buffer of 1 byte(s)");

// TOO SHORT - test a hex string with less bytes than the "length" parameter provided
const hexString2 =
"5234d08dfa2c815f3097b8ba848a28172e85bec78886e8e201afccb166fc"; // length is 30 bytes, so must be padded with 2 0's at the beginning
const expectedUint8Array2 = new Uint8Array([
0, 0, 82, 52, 208, 141, 250, 44, 129, 95, 48, 151, 184, 186, 132, 138, 40,
23, 46, 133, 190, 199, 136, 134, 232, 226, 1, 175, 204, 177, 102, 252,
]);
expect(uint8ArrayFromHexString(hexString2, 32)).toEqual(expectedUint8Array2); // Hex string => Uint8Array

// TOO LONG - test a hex string with less bytes than the "length" parameter provided -- Should error
const hexString3 =
"5234d08dfa2c815f3097b8ba848a28172e85bec78886e8e201afccb166fcfafbfcfd"; // length is 34 bytes, so no additional padding will be added
expect(() => uint8ArrayFromHexString(hexString3, 32)).toThrow(
"hex value cannot fit in a buffer of 32 byte(s)"
);
});

// Test for hexStringToBase64url
Expand Down
23 changes: 23 additions & 0 deletions packages/sdk-browser/src/__tests__/utils-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
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

const embAPIKey = await createEmbeddedAPIKey(uncompPubKey);
expect(embAPIKey.authBundle).toBeDefined();
expect(embAPIKey.authBundle).not.toBeNull();
expect(embAPIKey.authBundle).not.toEqual("");

expect(embAPIKey.publicKey).toBeDefined();
expect(embAPIKey.publicKey).not.toBeNull();
expect(embAPIKey.authBundle).not.toEqual("");

// test invalid uncompressed public key (last byte has been changed)
const uncompPubKeyInvalid =
"0400d2eb47be2006c29db5fe9941dd686d19ddeea85a0328894f08091f6b5be9b366c4872345594c12a7f7c47c62dd8074542934820fce5ee0ddc55d6d1d8dd311";
await expect(createEmbeddedAPIKey(uncompPubKeyInvalid)).rejects.toThrow();
});
7 changes: 5 additions & 2 deletions packages/sdk-browser/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import bs58check from "bs58check";
import { AeadId, CipherSuite, KdfId, KemId } from "hpke-js";

import type { EmbeddedAPIKey } from "./models";
import { pointDecode } from "@turnkey/api-key-stamper";

// createEmbeddedAPIKey creates an embedded API key encrypted to a target key (typically embedded within an iframe).
// This returns a bundle that can be decrypted by that target key, as well as the public key of the newly created API key.
Expand All @@ -30,9 +31,11 @@ export const createEmbeddedAPIKey = async (

// 3: import the targetPublicKey (i.e. passed in from the iframe)
const targetKeyBytes = uint8ArrayFromHexString(targetPublicKey);
const jwk = pointDecode(targetKeyBytes);

const targetKey = await crypto.subtle.importKey(
"raw",
targetKeyBytes,
"jwk",
jwk,
{
name: "ECDH",
namedCurve: "P-256",
Expand Down
Loading