Skip to content

Commit

Permalink
fix/ecdsa verification to use raw signature format per jwa spec (#179)
Browse files Browse the repository at this point in the history
  • Loading branch information
ottokruse authored Jan 2, 2025
1 parent 6072722 commit 46be74c
Show file tree
Hide file tree
Showing 8 changed files with 210 additions and 40 deletions.
1 change: 1 addition & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ jobs:
npm run test:install
npm run test:import
npm run test:browser
npm run test:speed
env:
CI: "true"
18 changes: 16 additions & 2 deletions src/node-web-compat-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@ export const nodeWebCompat: NodeWebCompat = {
// eslint-disable-next-line security/detect-object-injection
createVerify(JwtSignatureAlgorithms[alg])
.update(jwsSigningInput)
.verify(keyObject as KeyObject, signature, "base64"),
.verify(
{
key: keyObject as KeyObject,
dsaEncoding: "ieee-p1363", // Signature format r || s (not used for RSA)
},
signature,
"base64"
),
verifySignatureAsync: async ({
alg,
keyObject,
Expand All @@ -48,7 +55,14 @@ export const nodeWebCompat: NodeWebCompat = {
// eslint-disable-next-line security/detect-object-injection
createVerify(JwtSignatureAlgorithms[alg])
.update(jwsSigningInput)
.verify(keyObject as KeyObject, signature, "base64"),
.verify(
{
key: keyObject as KeyObject,
dsaEncoding: "ieee-p1363", // Signature format r || s (not used for RSA)
},
signature,
"base64"
),
defaultFetchTimeouts: {
socketIdle: 1500,
response: 3000,
Expand Down
27 changes: 10 additions & 17 deletions src/node-web-compat-web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,6 @@ import {
} from "./error.js";
import { NodeWebCompat } from "./node-web-compat.js";

/**
* Enum to map supported JWT signature algorithms with WebCrypto message digest algorithm names
*/
enum DigestFunctionsWebCrypto {
RS256 = "SHA-256",
RS384 = "SHA-384",
RS512 = "SHA-512",
}

/**
* Enum to map supported JWT signature algorithms with WebCrypto curve names
*/
Expand Down Expand Up @@ -87,10 +78,7 @@ export const nodeWebCompat: NodeWebCompat = {
alg.startsWith("RS")
? {
name: "RSASSA-PKCS1-v1_5",
// eslint-disable-next-line security/detect-object-injection
hash: DigestFunctionsWebCrypto[
alg as keyof typeof DigestFunctionsWebCrypto
],
hash: `SHA-${alg.slice(2)}`,
}
: {
name: "ECDSA",
Expand All @@ -107,11 +95,16 @@ export const nodeWebCompat: NodeWebCompat = {
"Synchronously verifying a JWT signature is not supported in the browser"
);
},
verifySignatureAsync: ({ jwsSigningInput, keyObject, signature }) =>
verifySignatureAsync: ({ jwsSigningInput, keyObject, signature, alg }) =>
crypto.subtle.verify(
{
name: "RSASSA-PKCS1-v1_5",
},
alg.startsWith("RS")
? {
name: "RSASSA-PKCS1-v1_5",
}
: {
name: "ECDSA",
hash: `SHA-${alg.slice(2)}`,
},
keyObject as CryptoKey,
bufferFromBase64url(signature),
new TextEncoder().encode(jwsSigningInput)
Expand Down
112 changes: 109 additions & 3 deletions tests/unit/jwt-verifier.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ describe("unit tests jwt verifier", () => {
});

describe("verify", () => {
test("happy flow", () => {
test("happy flow - RS256", () => {
const issuer = "https://example.com";
const audience = "1234";
const signedJwt = signJwt(
Expand All @@ -1126,6 +1126,29 @@ describe("unit tests jwt verifier", () => {
})
).resolves.toMatchObject({ hello: "world" });
});
test("happy flow - ES256", () => {
const es256keypair = generateKeyPair({
kty: "EC",
alg: "ES256",
});
const issuer = "https://example.com";
const audience = "1234";
const signedJwt = signJwt(
{ alg: "ES256", kid: keypair.jwk.kid },
{ aud: audience, iss: issuer, hello: "world" },
es256keypair.privateKey
);
mockHttpsUri("https://example.com/path/to/jwks.json", {
responsePayload: JSON.stringify(es256keypair.jwks),
});
expect.assertions(1);
return expect(
verifyJwt(signedJwt, "https://example.com/path/to/jwks.json", {
issuer,
audience,
})
).resolves.toMatchObject({ hello: "world" });
});
describe("includeJwtInErrors", () => {
test("expired jwt and includeRawJwtInErrors", async () => {
const exp = new Date();
Expand Down Expand Up @@ -1941,8 +1964,19 @@ describe("unit tests jwt verifier", () => {

describe("speed tests jwt", () => {
let keypair: ReturnType<typeof generateKeyPair>;
let keypairEs256: ReturnType<typeof generateKeyPair>;
const thresholdsInMillis = {
"verifyJwtSync()": 0.2, // max 200 microseconds per verifyJwtSync() call
"verifier.verifySync()": 0.12, // max 120 microseconds per verifier.verifySync() call
};
if (process.env.CI) {
// Increase thresholds on CI to reduce flakiness
thresholdsInMillis["verifyJwtSync()"] *= 2;
thresholdsInMillis["verifier.verifySync()"] *= 2;
}
beforeAll(() => {
keypair = generateKeyPair();
keypairEs256 = generateKeyPair({ kty: "EC", alg: "ES256" });
});
test("JWT verification is fast", () => {
const issuer = "testissuer";
Expand All @@ -1965,7 +1999,11 @@ describe("speed tests jwt", () => {
verifyJwtSync(jwt, keypair.jwk, { issuer, audience });
}
const totalTime = performance.now() - start;
expect(totalTime).toBeLessThan(testCount * 0.2); // Allowed: max 200 microseconds per verify call
const threshold = thresholdsInMillis["verifyJwtSync()"];
expect(totalTime).toBeLessThan(testCount * threshold);
console.log(
`RS256 verifyJwtSync(): time per verification: ${(totalTime / testCount).toFixed(3)} ms. (threshold: ${threshold.toFixed(3)})`
);
});

test("JWT verification with caches is even faster", () => {
Expand Down Expand Up @@ -1997,6 +2035,74 @@ describe("speed tests jwt", () => {
verifier.verifySync(jwt);
}
const totalTime = performance.now() - start;
expect(totalTime).toBeLessThan(testCount * 0.12); // Allowed: max 120 microseconds per verify call
const threshold = thresholdsInMillis["verifier.verifySync()"];
expect(totalTime).toBeLessThan(testCount * threshold);
console.log(
`RS256 verifier.verifySync(): time per verification: ${(totalTime / testCount).toFixed(3)} ms. (threshold: ${threshold.toFixed(3)})`
);
});

test("JWT verification is fast -- ES256", () => {
const issuer = "testissuer";
const audience = "testaudience";
const createSignedJwtInMap = (_: undefined, index: number) =>
signJwt(
{ kid: keypairEs256.jwk.kid, alg: "ES256" },
{
hello: `world ${index}`,
iss: issuer,
aud: audience,
now: performance.now(),
},
keypairEs256.privateKey
);
const testCount = 1000;
const aWholeLotOfJWTs = [...new Array(testCount)].map(createSignedJwtInMap);
const start = performance.now();
for (const jwt of aWholeLotOfJWTs) {
verifyJwtSync(jwt, keypairEs256.jwk, { issuer, audience });
}
const totalTime = performance.now() - start;
const threshold = thresholdsInMillis["verifyJwtSync()"];
expect(totalTime).toBeLessThan(testCount * threshold);
console.log(
`ES256 verifyJwtSync(): time per verification: ${(totalTime / testCount).toFixed(3)} ms. (threshold: ${threshold.toFixed(3)})`
);
});

test("JWT verification with caches is even faster -- ES256", () => {
const issuer = "testissuer";
const audience = "testaudience";
const createSignedJwtCallbackInMap = (_: undefined, index: number) =>
signJwt(
{ kid: keypairEs256.jwk.kid, alg: "ES256" },
{
hello: `world ${index}`,
iss: issuer,
aud: audience,
now: performance.now(),
},
keypairEs256.privateKey
);
const testCount = 1000;
const aWholeLotOfJWTs = [...new Array(testCount)].map(
createSignedJwtCallbackInMap
);
const verifier = JwtVerifier.create({
audience,
issuer,
jwksUri: "http://example.com/jwks.json",
});
verifier.cacheJwks(keypairEs256.jwks);
const start = performance.now();
for (const jwt of aWholeLotOfJWTs) {
verifier.verifySync(jwt);
}
const totalTime = performance.now() - start;
const threshold = thresholdsInMillis["verifier.verifySync()"];
expect(totalTime).toBeLessThan(testCount * threshold);
console.log(
`ES256 verifier.verifySync(): time per verification: ${(totalTime / testCount).toFixed(3)} ms. (threshold: ${threshold.toFixed(3)})`
);
});
});
12 changes: 7 additions & 5 deletions tests/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,16 @@ export function signJwt(
Buffer.from(JSON.stringify(header)).toString("base64url"),
Buffer.from(JSON.stringify(payload)).toString("base64url"),
].join(".");
const digestFunction =
JwtSignatureAlgorithms[
(header.alg as keyof typeof JwtSignatureAlgorithms) ?? "RS256"
];
const alg = (header.alg as keyof typeof JwtSignatureAlgorithms) ?? "RS256";
// eslint-disable-next-line security/detect-object-injection
const digestFunction = JwtSignatureAlgorithms[alg];
const sign = createSign(digestFunction);
sign.write(toSign);
sign.end();
const signature = sign.sign(privateKey);
const signature = sign.sign({
key: privateKey,
dsaEncoding: "ieee-p1363", // Signature format r || s (not used for RSA)
});
if (!produceValidSignature) {
signature[0] = ~signature[0]; // swap first byte
}
Expand Down
24 changes: 24 additions & 0 deletions tests/vite-app/cypress/e2e/unittests.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
JWKSURI,
VALID_TOKEN,
VALID_TOKEN_FOR_JWK_WITHOUT_ALG,
VALID_TOKEN_ES256,
VALID_TOKEN_ES512,
EXPIRED_TOKEN,
NOT_YET_VALID_TOKEN,
} from "../fixtures/example-token-data.json";
Expand Down Expand Up @@ -43,6 +45,28 @@ describe("unit tests", () => {
expect(payload).to.exist;
});

it("valid token - es256", async () => {
const verifier = JwtVerifier.create({
issuer: ISSUER,
audience: AUDIENCE,
jwksUri: JWKSURI,
});
const payload = await verifier.verify(VALID_TOKEN_ES256);

expect(payload).to.exist;
});

it("valid token - es512", async () => {
const verifier = JwtVerifier.create({
issuer: ISSUER,
audience: AUDIENCE,
jwksUri: JWKSURI,
});
const payload = await verifier.verify(VALID_TOKEN_ES512);

expect(payload).to.exist;
});

it("valid token for JWK without alg", async () => {
const verifier = JwtVerifier.create({
issuer: ISSUER,
Expand Down
29 changes: 17 additions & 12 deletions tests/vite-app/run-tests.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
#!/bin/bash -e
#!/bin/bash

set -euo pipefail

VITE_PIDS=()

cleanup() {
for pid in "${VITE_PIDS[@]}"; do
echo "Cleaning up VITE process $pid ..."
kill "$pid" 2>/dev/null || true
done
}

trap cleanup SIGINT SIGTERM EXIT

run_test() {
START_SERVER_CMD=$1
Expand All @@ -8,6 +21,7 @@ run_test() {
echo "Starting Vite server in background ..."
$START_SERVER_CMD &
VITE_PID=$!
VITE_PIDS+=($VITE_PID)
echo "Vite server launched as PID ${VITE_PID}"
WAITED=0
while ! nc -z localhost $SERVER_PORT; do
Expand All @@ -19,16 +33,13 @@ run_test() {
echo "Waiting for Vite server to come on-line ($WAITED) ..."
sleep 1
done
if [ ! -z $CI ]; then
if [ "${CI+value}" = "value" ]; then
export CYPRESS_VIDEO=false
fi
echo "Running cypress tests ..."
if ! $START_CYPRESS_CMD; then
echo "Cypress test failed :("
TEST_FAILED=true
fi
if [ ! -z $TEST_FAILED ]; then
return 1
exit 1
fi
}

Expand All @@ -48,9 +59,3 @@ main() {
}

main
if [ -z $CI ]; then
# kill the backgrounded servers if we're not running in CI (e.g. on a developer laptop)
# note: might not run if the test fails - run "ps j" to find the PGID if you need to run pkill yourself
echo "Sending kill signal to (PGID $$) ..."
pkill -2 -g $$
fi
Loading

0 comments on commit 46be74c

Please sign in to comment.