From ac0550de0ca589f04edffa864b6bdb9cf7a102ba Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Thu, 3 Oct 2024 09:27:45 +0200 Subject: [PATCH] refactor!: future proof Ed25519 BREAKING CHANGE: specifying Ed448 curve for EdDSA is no longer supported, EdDSA is now just an alias for the fully-specified Ed25519 JWS algorithm BREAKING CHANGE: assertions signed with an Ed25519 CryptoKey will now use the Ed25519 JWS alg value instead of EdDSA. This can be reverted using the modifyAssertion symbol export --- src/index.ts | 102 ++++++++++++++++---------------- tap/generate.ts | 16 ----- tap/keys.ts | 2 +- tap/request_object.ts | 73 +++++++++++++---------- test/_setup.ts | 36 ++++++----- test/authorization_code.test.ts | 57 +++++++++--------- test/client_auth.test.ts | 26 -------- 7 files changed, 140 insertions(+), 172 deletions(-) diff --git a/src/index.ts b/src/index.ts index bfcc7056..1a2b7cee 100644 --- a/src/index.ts +++ b/src/index.ts @@ -104,18 +104,18 @@ function CodedTypeError(message: string, code: codes, cause?: unknown) { * for which Digital Signature validation is implemented. */ export type JWSAlgorithm = - // widely used | 'PS256' | 'ES256' | 'RS256' - | 'EdDSA' - // less used + | 'Ed25519' | 'ES384' | 'PS384' | 'RS384' | 'ES512' | 'PS512' | 'RS512' + // Deprecated + | 'EdDSA' export interface JWK { readonly kty?: string @@ -312,6 +312,35 @@ export const customFetch: unique symbol = Symbol() * }, * }) * ``` + * + * @example + * + * Changing the `alg: "Ed25519"` back to `alg: "EdDSA"` + * + * ```ts + * let as!: oauth.AuthorizationServer + * let client!: oauth.Client + * let parameters!: URLSearchParams + * let key!: oauth.CryptoKey | oauth.PrivateKey + * let keyPair!: oauth.CryptoKeyPair + * + * let remapEd25519: oauth.ModifyAssertionOptions = { + * [oauth.modifyAssertion]: (header) => { + * if (header.alg === 'Ed25519') { + * header.alg = 'EdDSA' + * } + * }, + * } + * + * // For JAR + * oauth.issueRequestObject(as, client, parameters, key, remapEd25519) + * + * // For Private Key JWT + * oauth.PrivateKeyJwt(key, remapEd25519) + * + * // For DPoP + * oauth.DPoP(client, keyPair, remapEd25519) + * ``` */ export const modifyAssertion: unique symbol = Symbol() @@ -1490,8 +1519,8 @@ function keyToJws(key: CryptoKey) { case 'ECDSA': return esAlg(key) case 'Ed25519': // Fall through - case 'Ed448': - return 'EdDSA' + case 'EdDSA': + return 'Ed25519' default: throw new UnsupportedOperationError('unsupported CryptoKey algorithm name', { cause: key }) } @@ -2873,7 +2902,8 @@ async function getPublicSigKeyFromIssuerJwksUri( case alg === 'ES256' && jwk.crv !== 'P-256': // Fall through case alg === 'ES384' && jwk.crv !== 'P-384': // Fall through case alg === 'ES512' && jwk.crv !== 'P-521': // Fall through - case alg === 'EdDSA' && !(jwk.crv === 'Ed25519' || jwk.crv === 'Ed448'): + case alg === 'Ed25519' && jwk.crv !== 'Ed25519': // Fall through + case alg === 'EdDSA' && jwk.crv !== 'Ed25519': // Fall through return false } @@ -4516,6 +4546,7 @@ function supported(alg: string) { case 'PS512': case 'ES512': case 'RS512': + case 'Ed25519': case 'EdDSA': return true default: @@ -4579,8 +4610,8 @@ function keyToSubtle(key: CryptoKey): AlgorithmIdentifier | RsaPssParams | Ecdsa case 'RSASSA-PKCS1-v1_5': checkRsaKeyAlgorithm(key) return key.algorithm.name - case 'Ed448': // Fall through - case 'Ed25519': + case 'Ed25519': // Fall through + case 'EdDSA': return key.algorithm.name } throw new UnsupportedOperationError('unsupported CryptoKey algorithm name', { cause: key }) @@ -4790,12 +4821,7 @@ export async function validateJwtAuthResponse( return validateAuthResponse(as, client, result, expectedState) } -async function idTokenHash( - data: string, - key: CryptoKey, - header: CompactJWSHeaderParameters, - claimName: string, -) { +async function idTokenHash(data: string, header: CompactJWSHeaderParameters, claimName: string) { let algorithm: string switch (header.alg) { case 'RS256': // Fall through @@ -4810,17 +4836,11 @@ async function idTokenHash( break case 'RS512': // Fall through case 'PS512': // Fall through - case 'ES512': + case 'ES512': // Fall through + case 'Ed25519': // Fall through + case 'EdDSA': algorithm = 'SHA-512' break - case 'EdDSA': - if (key.algorithm.name === 'Ed25519') { - algorithm = 'SHA-512' - break - } - throw new UnsupportedOperationError(`unsupported EdDSA curve for ${claimName} calculation`, { - cause: key, - }) default: throw new UnsupportedOperationError( `unsupported JWS algorithm for ${claimName} calculation`, @@ -4835,11 +4855,10 @@ async function idTokenHash( async function idTokenHashMatches( data: string, actual: string, - key: CryptoKey, header: CompactJWSHeaderParameters, claimName: string, ) { - const expected = await idTokenHash(data, key, header, claimName) + const expected = await idTokenHash(data, header, claimName) return actual === expected } @@ -5103,7 +5122,7 @@ async function validateHybridResponse( const key = await getPublicSigKeyFromIssuerJwksUri(as, options, header) await validateJwsSignature(protectedHeader, payload, key, signature) - if ((await idTokenHashMatches(code, claims.c_hash, key!, header, 'c_hash')) !== true) { + if ((await idTokenHashMatches(code, claims.c_hash, header, 'c_hash')) !== true) { throw OPE('invalid ID Token "c_hash" (code hash) claim value', JWT_CLAIM_COMPARISON, { code, alg: header.alg, @@ -5118,7 +5137,7 @@ async function validateHybridResponse( }) assertString(state, '"state" response parameter', INVALID_RESPONSE, { parameters }) - if ((await idTokenHashMatches(state, claims.s_hash, key!, header, 's_hash')) !== true) { + if ((await idTokenHashMatches(state, claims.s_hash, header, 's_hash')) !== true) { throw OPE('invalid ID Token "s_hash" (state hash) claim value', JWT_CLAIM_COMPARISON, { state, alg: header.alg, @@ -5325,10 +5344,7 @@ export function validateAuthResponse( return brand(new URLSearchParams(parameters)) } -function algToSubtle( - alg: string, - crv?: string, -): RsaHashedImportParams | EcKeyImportParams | AlgorithmIdentifier { +function algToSubtle(alg: string): RsaHashedImportParams | EcKeyImportParams | AlgorithmIdentifier { switch (alg) { case 'PS256': // Fall through case 'PS384': // Fall through @@ -5343,15 +5359,9 @@ function algToSubtle( return { name: 'ECDSA', namedCurve: `P-${alg.slice(-3)}` } case 'ES512': return { name: 'ECDSA', namedCurve: 'P-521' } - case 'EdDSA': { - switch (crv) { - case 'Ed25519': // Fall through - case 'Ed448': - return crv - default: - throw new UnsupportedOperationError('unsupported EdDSA curve', { cause: { alg, crv } }) - } - } + case 'Ed25519': + case 'EdDSA': + return 'Ed25519' default: throw new UnsupportedOperationError('unsupported JWS algorithm', { cause: { alg } }) } @@ -5359,7 +5369,7 @@ function algToSubtle( async function importJwk(alg: string, jwk: JWK) { const { ext, key_ops, use, ...key } = jwk - return crypto.subtle.importKey('jwk', key, algToSubtle(alg, jwk.crv), true, ['verify']) + return crypto.subtle.importKey('jwk', key, algToSubtle(alg), true, ['verify']) } export interface DeviceAuthorizationRequestOptions @@ -5608,11 +5618,6 @@ export interface GenerateKeyPairOptions { * (RSA algorithms only) The length, in bits, of the RSA modulus. Default is `2048`. */ modulusLength?: number - - /** - * (EdDSA algorithm only) The EdDSA sub-type. Default is `Ed25519`. - */ - crv?: 'Ed25519' | 'Ed448' } /** @@ -5629,10 +5634,7 @@ export async function generateKeyPair( ): Promise { assertString(alg, '"alg"') - const algorithm: RsaHashedKeyGenParams | EcKeyGenParams | AlgorithmIdentifier = algToSubtle( - alg, - alg === 'EdDSA' ? (options?.crv ?? 'Ed25519') : undefined, - ) + const algorithm: RsaHashedKeyGenParams | EcKeyGenParams | AlgorithmIdentifier = algToSubtle(alg) if (alg.startsWith('PS') || alg.startsWith('RS')) { Object.assign(algorithm, { diff --git a/tap/generate.ts b/tap/generate.ts index 60a168ba..7fbe570c 100644 --- a/tap/generate.ts +++ b/tap/generate.ts @@ -1,7 +1,6 @@ import type QUnit from 'qunit' import * as lib from '../src/index.js' import { fails, keys } from './keys.js' -import * as env from './env.js' function isRSA(alg: string) { return alg.startsWith('RS') || alg.startsWith('PS') @@ -33,21 +32,6 @@ export default (QUnit: QUnit) => { }) } - if (env.isNode || env.isEdgeRuntime) { - test('EdDSA w/ Ed448', async (t) => { - const { publicKey, privateKey } = await lib.generateKeyPair('EdDSA', { crv: 'Ed448' }) - testGeneratedCryptoKeyPair(t, privateKey, publicKey) - - for (const key of [privateKey, publicKey]) { - t.equal(key.algorithm.name, 'Ed448') - } - }) - } else { - test(`[not supported] EdDSA w/ Ed448 fails to generate`, async (t) => { - await t.rejects(lib.generateKeyPair('EdDSA', { crv: 'Ed448' })) - }) - } - for (const [alg, kp] of Object.entries(keys)) { test(`${alg} defaults`, async (t) => { const { publicKey, privateKey } = await kp diff --git a/tap/keys.ts b/tap/keys.ts index 19d3660f..77c82711 100644 --- a/tap/keys.ts +++ b/tap/keys.ts @@ -7,7 +7,7 @@ export const fails: string[] = [] if (!env.isDeno) { algs.push('ES512') } -;(env.isBlink ? fails : algs).push('EdDSA') +;(env.isBlink ? fails : algs).push('EdDSA', 'Ed25519') export const keys = algs.reduce( (acc, alg) => { diff --git a/tap/request_object.ts b/tap/request_object.ts index 49d47f30..ed6141a6 100644 --- a/tap/request_object.ts +++ b/tap/request_object.ts @@ -10,36 +10,49 @@ export default (QUnit: QUnit) => { const { module, test } = QUnit module('request_object.ts') - for (const [alg, kp] of Object.entries(keys)) { - test(`issueRequestObject() w/ ${alg}`, async (t) => { - const { privateKey, publicKey } = await kp - for (const parameters of [ - new URLSearchParams({ response_type: 'code', resource: 'urn:example:resource' }), - { response_type: 'code', resource: 'urn:example:resource' }, - [ - ['response_type', 'code'], - ['resource', 'urn:example:resource'], - ], - ]) { - const jwt = await lib.issueRequestObject(issuer, client, parameters, { key: privateKey }) - - const { payload, protectedHeader } = await jose.jwtVerify(jwt, publicKey) - t.propEqual(protectedHeader, { alg, typ: 'oauth-authz-req+jwt' }) - const { exp, iat, nbf, jti, ...claims } = payload - t.equal(typeof exp, 'number') - t.equal(typeof nbf, 'number') - t.equal(typeof iat, 'number') - t.equal(typeof jti, 'string') - t.propEqual(claims, { - iss: client.client_id, - aud: issuer.issuer, - response_type: 'code', - resource: 'urn:example:resource', - client_id: client.client_id, - }) - } - }) - } + test(`issueRequestObject()`, async (t) => { + const { privateKey, publicKey } = await lib.generateKeyPair('ES256') + for (const parameters of [ + new URLSearchParams({ response_type: 'code', resource: 'urn:example:resource' }), + { response_type: 'code', resource: 'urn:example:resource' }, + [ + ['response_type', 'code'], + ['resource', 'urn:example:resource'], + ], + ]) { + const jwt = await lib.issueRequestObject( + issuer, + client, + parameters, + { key: privateKey }, + { + [lib.modifyAssertion]: (header) => { + if (header.alg === 'Ed25519') { + header.alg = 'EdDSA' + } + }, + }, + ) + + const { payload, protectedHeader } = await jose.jwtVerify(jwt, publicKey) + t.propEqual(protectedHeader, { + alg: 'ES256', + typ: 'oauth-authz-req+jwt', + }) + const { exp, iat, nbf, jti, ...claims } = payload + t.equal(typeof exp, 'number') + t.equal(typeof nbf, 'number') + t.equal(typeof iat, 'number') + t.equal(typeof jti, 'string') + t.propEqual(claims, { + iss: client.client_id, + aud: issuer.issuer, + response_type: 'code', + resource: 'urn:example:resource', + client_id: client.client_id, + }) + } + }) test('issueRequestObject() multiple resource parameters', async (t) => { const kp = await keys.ES256 diff --git a/test/_setup.ts b/test/_setup.ts index 26182810..972fee82 100644 --- a/test/_setup.ts +++ b/test/_setup.ts @@ -4,18 +4,6 @@ import * as jose from 'jose' import type { AuthorizationServer, Client } from '../src/index.js' -export const ALGS = [ - 'ES256', - 'RS256', - 'PS256', - 'ES384', - 'RS384', - 'PS384', - 'ES512', - 'RS512', - 'PS512', - 'EdDSA', -] interface ContextAlgs { [key: string]: CryptoKeyPair } @@ -37,8 +25,19 @@ export default (t: ExecutionContext) => { } export async function setupContextKeys(t: ExecutionContext) { - await Promise.all( - ALGS.map(async (alg) => (t.context[alg] = await jose.generateKeyPair(alg))), + t.context.ES256 = await crypto.subtle.generateKey({ name: 'ECDSA', namedCurve: 'P-256' }, true, [ + 'sign', + 'verify', + ]) + t.context.RS256 = await crypto.subtle.generateKey( + { + name: 'RSASSA-PKCS1-v1_5', + hash: 'SHA-256', + modulusLength: 2048, + publicExponent: new Uint8Array([0x01, 0x00, 0x01]), + }, + true, + ['sign', 'verify'], ) } @@ -55,11 +54,10 @@ export async function setupJwks(t: ExecutionContext) { .reply( 200, { - keys: await Promise.all( - ALGS.map((alg) => - jose.exportJWK(t.context[alg].publicKey).then((jwk) => ({ ...jwk, alg })), - ), - ), + keys: [ + await jose.exportJWK(t.context.ES256.publicKey).then((jwk) => ({ ...jwk, alg: 'ES256' })), + await jose.exportJWK(t.context.RS256.publicKey).then((jwk) => ({ ...jwk, alg: 'RS256' })), + ], }, { headers: { 'content-type': coinflip() ? 'application/json' : 'application/jwk-set+json' }, diff --git a/test/authorization_code.test.ts b/test/authorization_code.test.ts index fb729674..dca9ed60 100644 --- a/test/authorization_code.test.ts +++ b/test/authorization_code.test.ts @@ -1,7 +1,6 @@ import anyTest, { type TestFn } from 'ava' import * as querystring from 'node:querystring' import setup, { - ALGS, client, endpoint, getResponse, @@ -643,37 +642,35 @@ test('processAuthorizationCodeResponse() with an ID Token typ: "application/jwt" ) }) -for (const alg of ALGS) { - test(`processAuthorizationCodeResponse() with an ${alg} ID Token`, async (t) => { - const tIssuer: lib.AuthorizationServer = { - ...issuer, - id_token_signing_alg_values_supported: [alg], - jwks_uri: endpoint('jwks'), - } +test(`processAuthorizationCodeResponse() with an ID Token`, async (t) => { + const tIssuer: lib.AuthorizationServer = { + ...issuer, + id_token_signing_alg_values_supported: ['ES256'], + jwks_uri: endpoint('jwks'), + } - await t.notThrowsAsync( - lib.processAuthorizationCodeResponse( - tIssuer, - client, - getResponse( - JSON.stringify({ - access_token: - 'YmJiZTAwYmYtMzgyOC00NzhkLTkyOTItNjJjNDM3MGYzOWIy9sFhvH8K_x8UIHj1osisS57f5DduL', - token_type: 'Bearer', - id_token: await new jose.SignJWT({}) - .setProtectedHeader({ alg }) - .setIssuer(issuer.issuer) - .setSubject('urn:example:subject') - .setAudience(client.client_id) - .setExpirationTime('5m') - .setIssuedAt() - .sign(t.context[alg].privateKey), - }), - ), + await t.notThrowsAsync( + lib.processAuthorizationCodeResponse( + tIssuer, + client, + getResponse( + JSON.stringify({ + access_token: + 'YmJiZTAwYmYtMzgyOC00NzhkLTkyOTItNjJjNDM3MGYzOWIy9sFhvH8K_x8UIHj1osisS57f5DduL', + token_type: 'Bearer', + id_token: await new jose.SignJWT({}) + .setProtectedHeader({ alg: 'ES256' }) + .setIssuer(issuer.issuer) + .setSubject('urn:example:subject') + .setAudience(client.client_id) + .setExpirationTime('5m') + .setIssuedAt() + .sign(t.context.ES256.privateKey), + }), ), - ) - }) -} + ), + ) +}) test('processAuthorizationCodeResponse() nonce checks', async (t) => { const tIssuer: lib.AuthorizationServer = { ...issuer, jwks_uri: endpoint('jwks') } diff --git a/test/client_auth.test.ts b/test/client_auth.test.ts index 9f98f5f0..e6ec778a 100644 --- a/test/client_auth.test.ts +++ b/test/client_auth.test.ts @@ -1,6 +1,5 @@ import anyTest, { type TestFn } from 'ava' import setup, { - ALGS, client, endpoint, issuer, @@ -216,31 +215,6 @@ test('private_key_jwt ({ key: CryptoKey, kid: string })', async (t) => { t.pass() }) -for (const alg of ALGS) { - test(`private_key_jwt using ${alg}`, async (t) => { - let assertion!: string - t.context - .intercept({ - path: `/test-${alg}`, - method: 'POST', - body(body) { - assertion = new URLSearchParams(body).get('client_assertion')! - return jose.decodeProtectedHeader(assertion).alg === alg - }, - }) - .reply(200, '') - - await lib.revocationRequest( - { ...issuer, revocation_endpoint: endpoint(`test-${alg}`) }, - client, - lib.PrivateKeyJwt(t.context[alg].privateKey), - 'token', - ) - await jose.compactVerify(assertion, t.context[alg].publicKey) - t.pass() - }) -} - test('none', async (t) => { t.context .intercept({