From b220a91c700ddd378e941229f931a20ce8a7fda0 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 10 Aug 2026 12:18:52 -0400 Subject: [PATCH 1/2] fix(sdk): emit RFC 7518 raw ECDSA signatures in JWS (DSPX-3397) cryptoService.sign() returns DER-encoded ECDSA signatures, but JWS requires raw IEEE P1363 (R || S) per RFC 7518 section 3.4. Both JWS emitters -- the KAS rewrap request token (tdf3/src/crypto/jwt.ts) and the DPoP proof signer (src/auth/dpop.ts) -- were shipping DER, so any EC-keyed token was rejected by conformant verifiers (Keycloak, panva-jose). verifyJwt had the mirror bug: it fed a raw JWS signature to a verifier expecting DER. Also fixes reqSignature defaulting to RS256 for the rewrap request token. WebCrypto rejects signing an EC private key with RSA params ("Unable to use this key to sign"), so an EC dpop key could not produce a rewrap token at all; the alg is now derived from the key's algorithm. Supporting changes: - export ieeeP1363ToDer / derToIeeeP1363 for these callers - add isAsymmetricSigningAlgorithm() so the JWS `alg` header is validated rather than blind-cast to the narrower set CryptoService can actually sign with (the JWS alg space includes PS256/EdDSA, which we do not support) The round-trip this creates (sign encodes to DER, caller decodes back to raw) is tracked for removal in DSPX-3634. Signed-off-by: Dave Mihalcik --- lib/src/auth/dpop.ts | 39 +++-- lib/tdf3/src/crypto/core/signing.ts | 14 +- lib/tdf3/src/crypto/declarations.ts | 21 +++ lib/tdf3/src/crypto/jwt.ts | 36 +++-- lib/tdf3/src/tdf.ts | 30 +++- lib/tests/mocha/dpop-proof.spec.ts | 184 +++++++++++++++++++++++ lib/tests/mocha/encrypt-decrypt.spec.ts | 47 ++++++ lib/tests/mocha/helpers/jws-keys.ts | 88 +++++++++++ lib/tests/mocha/reqsignature-jws.spec.ts | 84 +++++++++++ 9 files changed, 518 insertions(+), 25 deletions(-) create mode 100644 lib/tests/mocha/dpop-proof.spec.ts create mode 100644 lib/tests/mocha/helpers/jws-keys.ts create mode 100644 lib/tests/mocha/reqsignature-jws.spec.ts diff --git a/lib/src/auth/dpop.ts b/lib/src/auth/dpop.ts index 4801fee83..b72ea68f9 100644 --- a/lib/src/auth/dpop.ts +++ b/lib/src/auth/dpop.ts @@ -8,7 +8,11 @@ import type { AsymmetricSigningAlgorithm, KeyAlgorithm, } from '../../tdf3/src/crypto/declarations.js'; -import { isRsaKeyAlgorithm } from '../../tdf3/src/crypto/declarations.js'; +import { + isAsymmetricSigningAlgorithm, + isRsaKeyAlgorithm, +} from '../../tdf3/src/crypto/declarations.js'; +import { derToIeeeP1363 } from '../../tdf3/src/crypto/core/signing.js'; export type JsonObject = { [Key in string]?: JsonValue }; export type JsonArray = JsonValue[]; @@ -21,11 +25,11 @@ function buf(input: string): Uint8Array { return encoder.encode(input); } -interface DPoPJwtHeaderParameters { +type DPoPJwtHeaderParameters = { alg: JWSAlgorithm; - typ: string; + typ: 'dpop+jwt'; jwk: JsonWebKey; -} +}; /** * Minimal JWT sign() implementation using CryptoService. @@ -37,11 +41,24 @@ async function jwt( cryptoService: CryptoService ) { const input = `${b64u(buf(JSON.stringify(header)))}.${b64u(buf(JSON.stringify(claimsSet)))}`; - const signature = await cryptoService.sign( - buf(input), - privateKey, - header.alg as AsymmetricSigningAlgorithm - ); + const { alg } = header; + // The header alg is a JWSAlgorithm, which is wider than what CryptoService can + // actually sign with (it documents forward-looking values like PS256/EdDSA). + // Validate rather than blind-cast so an unsupported alg fails here with a clear + // error instead of surfacing deep inside getSigningAlgorithmParams. + if (!isAsymmetricSigningAlgorithm(alg)) { + throw new UnsupportedOperationError(`unsupported DPoP alg: ${alg}`); + } + let signature = await cryptoService.sign(buf(input), privateKey, alg); + // JWS requires raw IEEE P1363 (R || S) for ECDSA per RFC 7518 §3.4, but + // cryptoService.sign currently returns DER. Convert here so DPoP proofs are + // accepted by RFC-conformant verifiers (Keycloak, panva-jose). RSA signatures + // are already raw bytes — no conversion (EdDSA is rejected by the guard above + // and never reaches here). See DSPX-3634 for the broader cleanup that would + // make this transform unnecessary. + if (alg.startsWith('ES')) { + signature = derToIeeeP1363(signature, alg); + } return `${input}.${b64u(signature)}`; } @@ -120,8 +137,10 @@ class UnsupportedOperationError extends Error { /** * Determines a supported JWS `alg` identifier from PublicKeyInfo algorithm string. + * Returns an AsymmetricSigningAlgorithm (the subset CryptoService can sign with); + * it never produces the forward-looking PS256/EdDSA members of JWSAlgorithm. */ -function determineJWSAlgorithmFromKeyInfo(algorithm: KeyAlgorithm): JWSAlgorithm { +function determineJWSAlgorithmFromKeyInfo(algorithm: KeyAlgorithm): AsymmetricSigningAlgorithm { if (isRsaKeyAlgorithm(algorithm)) { return 'RS256'; } diff --git a/lib/tdf3/src/crypto/core/signing.ts b/lib/tdf3/src/crypto/core/signing.ts index c3b824f9d..a678e93b7 100644 --- a/lib/tdf3/src/crypto/core/signing.ts +++ b/lib/tdf3/src/crypto/core/signing.ts @@ -43,7 +43,10 @@ function getSigningAlgorithmParams(algorithm: AsymmetricSigningAlgorithm): { * Convert IEEE P1363 signature format (used by WebCrypto ECDSA) to DER format (used by JWT). * RS256 signatures don't need conversion. */ -function ieeeP1363ToDer(signature: Uint8Array, algorithm: AsymmetricSigningAlgorithm): Uint8Array { +export function ieeeP1363ToDer( + signature: Uint8Array, + algorithm: AsymmetricSigningAlgorithm +): Uint8Array { if (algorithm === 'RS256') { return signature; } @@ -96,8 +99,15 @@ function ieeeP1363ToDer(signature: Uint8Array, algorithm: AsymmetricSigningAlgor /** * Convert DER signature format (used by JWT) to IEEE P1363 format (used by WebCrypto ECDSA). * RS256 signatures don't need conversion. + * + * Exported because callers that emit JWS (e.g. DPoP proofs in lib/src/auth/dpop.ts) + * must produce raw R||S per RFC 7518 section 3.4, while cryptoService.sign() + * currently returns DER. See DSPX-3634 for the broader cleanup. */ -function derToIeeeP1363(signature: Uint8Array, algorithm: AsymmetricSigningAlgorithm): Uint8Array { +export function derToIeeeP1363( + signature: Uint8Array, + algorithm: AsymmetricSigningAlgorithm +): Uint8Array { if (algorithm === 'RS256') { return signature; } diff --git a/lib/tdf3/src/crypto/declarations.ts b/lib/tdf3/src/crypto/declarations.ts index 91ec76b5b..8503e23ca 100644 --- a/lib/tdf3/src/crypto/declarations.ts +++ b/lib/tdf3/src/crypto/declarations.ts @@ -190,6 +190,27 @@ export type ECCurve = 'P-256' | 'P-384' | 'P-521'; */ export type AsymmetricSigningAlgorithm = 'RS256' | 'ES256' | 'ES384' | 'ES512'; +/** + * Runtime list of {@link AsymmetricSigningAlgorithm} values, kept in sync with + * the type above. Used to validate untyped/JWS-header algorithm strings. + */ +export const ASYMMETRIC_SIGNING_ALGORITHMS: readonly AsymmetricSigningAlgorithm[] = [ + 'RS256', + 'ES256', + 'ES384', + 'ES512', +]; + +/** + * Type guard narrowing an arbitrary string to an algorithm CryptoService can + * actually sign/verify with. The JWS `alg` space is wider (e.g. forward-looking + * PS256/EdDSA identifiers) than this runtime-supported subset; those must be + * rejected, not cast. + */ +export function isAsymmetricSigningAlgorithm(alg: string): alg is AsymmetricSigningAlgorithm { + return (ASYMMETRIC_SIGNING_ALGORITHMS as readonly string[]).includes(alg); +} + /** * Symmetric signing algorithm (requires raw key bytes). */ diff --git a/lib/tdf3/src/crypto/jwt.ts b/lib/tdf3/src/crypto/jwt.ts index 5ae08fd20..1c6c04e9f 100644 --- a/lib/tdf3/src/crypto/jwt.ts +++ b/lib/tdf3/src/crypto/jwt.ts @@ -1,5 +1,5 @@ import { - type AsymmetricSigningAlgorithm, + isAsymmetricSigningAlgorithm, type CryptoService, type PrivateKey, type PublicKey, @@ -17,6 +17,7 @@ import { } from 'jose'; import jwtClaimsSet from './jose/jwt-claims-set.js'; import validateCrit from './jose/validate-crit.js'; +import { derToIeeeP1363, ieeeP1363ToDer } from './core/signing.js'; export type JwtHeader = JWTHeaderParameters & { alg: SigningAlgorithm }; export type JwtPayload = JWTPayload; @@ -134,11 +135,19 @@ export async function signJwt( if (key._brand !== 'PrivateKey') { throw new Error(`${header.alg} requires a PrivateKey`); } - signature = await cryptoService.sign( - signingInputBytes, - key, - header.alg as AsymmetricSigningAlgorithm - ); + if (!isAsymmetricSigningAlgorithm(header.alg)) { + throw new Error(`Unsupported JWS signing algorithm: ${header.alg}`); + } + const alg = header.alg; + signature = await cryptoService.sign(signingInputBytes, key, alg); + // JWS requires raw IEEE P1363 (R || S) for ECDSA per RFC 7518 §3.4, but + // cryptoService.sign returns DER. Convert here so the JWT (e.g. the KAS + // rewrap request token) is accepted by RFC-conformant verifiers. RSA + // signatures are already raw bytes — no conversion (only RS256 and ES* + // reach here). Mirrors the DPoP proof signer in src/auth/dpop.ts. + if (alg.startsWith('ES')) { + signature = derToIeeeP1363(signature, alg); + } } // Return compact JWT @@ -232,12 +241,15 @@ export async function verifyJwt( typeof key === 'string' ? await cryptoService.importPublicKey(key, { usage: 'sign' }) : (key as PublicKey); - valid = await cryptoService.verify( - signingInputBytes, - signature, - publicKey, - header.alg as AsymmetricSigningAlgorithm - ); + if (!isAsymmetricSigningAlgorithm(header.alg)) { + throw new joseErrors.JWTInvalid(`Invalid JWT: unsupported algorithm "${header.alg}"`); + } + const alg = header.alg; + // JWS carries ECDSA signatures as raw IEEE P1363 (RFC 7518 §3.4), but + // cryptoService.verify expects DER. Convert here so we accept RFC-conformant + // ES* JWTs (matches the signJwt signer above). RSA is unchanged. + const verifySignature = alg.startsWith('ES') ? ieeeP1363ToDer(signature, alg) : signature; + valid = await cryptoService.verify(signingInputBytes, verifySignature, publicKey, alg); } if (!valid) { diff --git a/lib/tdf3/src/tdf.ts b/lib/tdf3/src/tdf.ts index 750ad0344..bcfa06b23 100644 --- a/lib/tdf3/src/tdf.ts +++ b/lib/tdf3/src/tdf.ts @@ -38,9 +38,11 @@ import { SymmetricCipher } from './ciphers/symmetric-cipher-base.js'; import { DecryptParams } from './client/builders.js'; import { DecoratedReadableStream } from './client/DecoratedReadableStream.js'; import { + type AsymmetricSigningAlgorithm, type CryptoService, type DecryptResult, isMlKemKeyAlgorithm, + type KeyAlgorithm, type KeyPair, mlKemAlgorithmToLevel, type SymmetricKey, @@ -757,6 +759,27 @@ type RewrapResponseData = { requiredObligations: string[]; }; +/** + * Map an opaque key's algorithm to the JWS signing algorithm used to sign the + * rewrap request token. RSA keys sign with RS256; EC keys sign with the ECDSA + * algorithm matching their curve. + */ +function signingAlgForKeyAlgorithm(algorithm: KeyAlgorithm): AsymmetricSigningAlgorithm { + switch (algorithm) { + case 'rsa:2048': + case 'rsa:4096': + return 'RS256'; + case 'ec:secp256r1': + return 'ES256'; + case 'ec:secp384r1': + return 'ES384'; + case 'ec:secp521r1': + return 'ES512'; + default: + throw new ConfigurationError(`Unsupported signing key algorithm [${algorithm}]`); + } +} + async function unwrapKey({ manifest, allowedKases, @@ -855,7 +878,12 @@ async function unwrapKey({ const requestBodyStr = toJsonString(UnsignedRewrapRequestSchema, unsignedRequest); const jwtPayload = { requestBody: requestBodyStr }; - const signedRequestToken = await reqSignature(jwtPayload, dpopKeys.privateKey, cryptoService); + // The request token must be signed with the algorithm matching the dpop key + // type. Defaulting to RS256 breaks EC keys (e.g. DPoP ES256), since WebCrypto + // rejects signing an EC key with RSA params ("Unable to use this key to sign"). + const signedRequestToken = await reqSignature(jwtPayload, dpopKeys.privateKey, cryptoService, { + alg: signingAlgForKeyAlgorithm(dpopKeys.privateKey.algorithm), + }); const rewrapResp = await fetchWrappedKey( url, diff --git a/lib/tests/mocha/dpop-proof.spec.ts b/lib/tests/mocha/dpop-proof.spec.ts new file mode 100644 index 000000000..63736d78d --- /dev/null +++ b/lib/tests/mocha/dpop-proof.spec.ts @@ -0,0 +1,184 @@ +import { expect } from 'chai'; +import * as jose from 'jose'; + +import dpopFn from '../../src/auth/dpop.js'; +import { DefaultCryptoService } from '../../tdf3/src/crypto/index.js'; +import type { KeyPair } from '../../tdf3/src/crypto/declarations.js'; +import { CURVES, ecdsaKeyPair, rsaKeyPair } from './helpers/jws-keys.js'; + +/** + * End-to-end DPoP proof signing tests. + * + * These tests verify the proofs minted by `dpopFn` (the function called from + * `AccessToken.doPost` and `withCreds`) against an independent, RFC 9449 / + * RFC 7518 §3.4 conformant verifier (`jose.jwtVerify`). + * + * Why these tests exist: the SDK's internal sign/verify pair is symmetric + * (both encode/decode ECDSA signatures as DER), so it round-trips inside this + * SDK even when the wire format is non-conformant. `jose.jwtVerify` is the + * same library used by real Keycloak under the hood — feeding our proofs + * through it catches DER-vs-raw and similar bugs that the in-SDK round-trip + * cannot. The earlier DSPX-3397 "Invalid token signature" failure from + * Keycloak would have been caught locally by these tests. + */ + +const HTU = 'https://example.test/protocol/openid-connect/token'; +const HTM = 'POST'; + +describe('DPoP proof — JWS conformance vs jose.jwtVerify (RFC 9449 + RFC 7518 §3.4)', function (this: Mocha.Suite) { + this.timeout(10_000); + + for (const { namedCurve, alg } of CURVES) { + it(`${alg} proof verifies against jose.jwtVerify`, async () => { + const { sdk: kp } = await ecdsaKeyPair(namedCurve); + const proof = await dpopFn(kp, DefaultCryptoService, HTU, HTM); + + // Verify with the public key extracted from the proof's own header, the + // way a real DPoP-aware server (Keycloak) would. + const header = jose.decodeProtectedHeader(proof); + expect(header.typ).to.equal('dpop+jwt'); + expect(header.alg).to.equal(alg); + expect(header.jwk).to.exist; + + const key = await jose.importJWK(header.jwk as jose.JWK, alg); + const { payload } = await jose.jwtVerify(proof, key); + expect(payload.htu).to.equal(HTU); + expect(payload.htm).to.equal(HTM); + expect(payload.jti).to.be.a('string').and.have.length.greaterThan(0); + expect(payload.iat).to.be.a('number'); + }); + + it(`${alg} proof verification rejects a flipped signature byte`, async () => { + const { sdk: kp } = await ecdsaKeyPair(namedCurve); + const proof = await dpopFn(kp, DefaultCryptoService, HTU, HTM); + const tampered = flipOneBitInSignatureSegment(proof); + + const header = jose.decodeProtectedHeader(proof); + const key = await jose.importJWK(header.jwk as jose.JWK, alg); + let threw = false; + try { + await jose.jwtVerify(tampered, key); + } catch { + threw = true; + } + expect(threw, 'jose.jwtVerify must reject a tampered signature').to.equal(true); + }); + + it(`${alg} proof verification rejects a swapped jwk header (binding intact, key wrong)`, async () => { + const { sdk: kp1 } = await ecdsaKeyPair(namedCurve); + const { sdk: kp2 } = await ecdsaKeyPair(namedCurve); + + const proof = await dpopFn(kp1, DefaultCryptoService, HTU, HTM); + + // Build a forged proof: same payload + signature but kp2's public JWK in + // the header. A correct verifier must reject because the signature was + // made by kp1.privateKey. + const [hdrB64, payloadB64, sigB64] = proof.split('.'); + const realHeader = JSON.parse( + new TextDecoder().decode(jose.base64url.decode(hdrB64)) + ) as jose.ProtectedHeaderParameters; + const fakeJwk = await crypto.subtle.exportKey( + 'jwk', + (await jose.importJWK((await proofHeaderJwkFor(kp2, alg)) as jose.JWK, alg)) as CryptoKey + ); + delete (fakeJwk as Record).d; + delete (fakeJwk as Record).key_ops; + realHeader.jwk = fakeJwk as jose.JWK; + const forgedHdrB64 = jose.base64url.encode( + new TextEncoder().encode(JSON.stringify(realHeader)) + ); + const forged = `${forgedHdrB64}.${payloadB64}.${sigB64}`; + + const key = await jose.importJWK(realHeader.jwk as jose.JWK, alg); + let threw = false; + try { + await jose.jwtVerify(forged, key); + } catch { + threw = true; + } + expect(threw, 'jose.jwtVerify must reject a forged proof with mismatched jwk').to.equal(true); + }); + } +}); + +describe('DPoP proof — RS256 JWS conformance vs jose.jwtVerify (RFC 9449)', function (this: Mocha.Suite) { + this.timeout(10_000); + + it('RS256 proof verifies against jose.jwtVerify', async () => { + const { sdk: kp } = await rsaKeyPair(); + const proof = await dpopFn(kp, DefaultCryptoService, HTU, HTM); + + const header = jose.decodeProtectedHeader(proof); + expect(header.typ).to.equal('dpop+jwt'); + expect(header.alg).to.equal('RS256'); + expect(header.jwk).to.exist; + + const key = await jose.importJWK(header.jwk as jose.JWK, 'RS256'); + const { payload } = await jose.jwtVerify(proof, key); + expect(payload.htu).to.equal(HTU); + expect(payload.htm).to.equal(HTM); + expect(payload.jti).to.be.a('string').and.have.length.greaterThan(0); + expect(payload.iat).to.be.a('number'); + }); + + it('RS256 proof verification rejects a flipped signature byte', async () => { + const { sdk: kp } = await rsaKeyPair(); + const proof = await dpopFn(kp, DefaultCryptoService, HTU, HTM); + const tampered = flipOneBitInSignatureSegment(proof); + + const header = jose.decodeProtectedHeader(proof); + const key = await jose.importJWK(header.jwk as jose.JWK, 'RS256'); + let threw = false; + try { + await jose.jwtVerify(tampered, key); + } catch { + threw = true; + } + expect(threw, 'jose.jwtVerify must reject a tampered RS256 signature').to.equal(true); + }); +}); + +describe('DPoP proof — unsupported key algorithm', function () { + it('throws before signing when the key algorithm is not a supported JWS alg', async () => { + // determineJWSAlgorithmFromKeyInfo (now typed to return only the four + // AsymmetricSigningAlgorithm values) must still reject an unknown key + // algorithm string up front, rather than the type change silently widening + // what flows into the signer. + const bogusKeyPair = { + publicKey: { algorithm: 'ec:brainpoolP256r1' }, + privateKey: {}, + } as unknown as KeyPair; + + let err: Error | undefined; + try { + await dpopFn(bogusKeyPair, DefaultCryptoService, HTU, HTM); + } catch (e) { + err = e as Error; + } + expect(err, 'expected an unsupported-algorithm error').to.be.instanceOf(Error); + expect(err?.message).to.match(/unsupported key algorithm/); + }); +}); + +/** + * Mint a real proof solely to extract a clean JWK for the public key. + * Round-tripping through `dpopFn` ensures the JWK shape matches what the + * SDK emits in real proofs. + */ +async function proofHeaderJwkFor(kp: KeyPair, alg: 'ES256' | 'ES384' | 'ES512'): Promise { + const proof = await dpopFn(kp, DefaultCryptoService, HTU, HTM); + const header = jose.decodeProtectedHeader(proof); + void alg; // alg unused; kept in signature for caller clarity + return header.jwk; +} + +/** + * Flip exactly one bit of the base64url-decoded signature segment. + * Re-encodes back into the JWT compact form. + */ +function flipOneBitInSignatureSegment(jwt: string): string { + const [h, p, s] = jwt.split('.'); + const sig = jose.base64url.decode(s); + sig[0] ^= 0x01; + return `${h}.${p}.${jose.base64url.encode(sig)}`; +} diff --git a/lib/tests/mocha/encrypt-decrypt.spec.ts b/lib/tests/mocha/encrypt-decrypt.spec.ts index 9677746cc..eac97d5b6 100644 --- a/lib/tests/mocha/encrypt-decrypt.spec.ts +++ b/lib/tests/mocha/encrypt-decrypt.spec.ts @@ -420,6 +420,53 @@ describe('encrypt decrypt test', async function () { assert.equal(new TextDecoder().decode(decryptedText), expectedVal); }); + it('decrypt signs the rewrap request token with EC dpop keys (ES256)', async function () { + // Regression for DSPX-3397: the rewrap request token was always signed with + // RS256, which made WebCrypto reject EC dpop keys ("Unable to use this key to + // sign"). The token alg must follow the dpop key algorithm. + const cipher = new AesGcmCipher(WebCryptoService); + const encryptionInformation = new SplitKey(cipher); + const key1 = await encryptionInformation.generateKey(); + const keyMiddleware = async () => ({ keyForEncryption: key1, keyForManifest: key1 }); + + const client = new Client.Client({ + kasEndpoint: kasUrl, + platformUrl: kasUrl, + dpopKeys: Mocks.entityECKeyPair(), + clientId: 'id', + authProvider, + }); + + const scope: Scope = { + dissem: ['user@domain.com'], + attributes: [], + }; + + const encryptedStream = await client.encrypt({ + metadata: Mocks.getMetadataObject(), + wrappingKeyAlgorithm: 'rsa:2048', + offline: true, + scope, + keyMiddleware, + source: new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(expectedVal)); + controller.close(); + }, + }), + }); + + const decryptStream = await client.decrypt({ + source: { + type: 'stream', + location: encryptedStream.stream, + }, + }); + + const { value: decryptedText } = await decryptStream.stream.getReader().read(); + assert.equal(new TextDecoder().decode(decryptedText), expectedVal); + }); + it('encrypt-decrypt with system metadata assertion', async function () { const cipher = new AesGcmCipher(WebCryptoService); const encryptionInformation = new SplitKey(cipher); diff --git a/lib/tests/mocha/helpers/jws-keys.ts b/lib/tests/mocha/helpers/jws-keys.ts new file mode 100644 index 000000000..69d008b10 --- /dev/null +++ b/lib/tests/mocha/helpers/jws-keys.ts @@ -0,0 +1,88 @@ +import { importPrivateKey, importPublicKey } from '../../../tdf3/src/crypto/core/key-format.js'; +import type { KeyPair } from '../../../tdf3/src/crypto/declarations.js'; + +/** + * Shared key fixtures for the JWS conformance suites (dpop-proof, reqsignature-jws). + * + * Both suites need SDK-opaque KeyPairs generated the way real callers get them: + * raw WebCrypto keygen, exported to DER, wrapped as PEM, then imported through + * the SDK's key-format layer (the same dance `cli/src/dpop-helpers.ts` does). + * The PEM is returned alongside so tests can hand it to `jose.importSPKI` for + * independent verification. + */ + +export type NamedCurve = 'P-256' | 'P-384' | 'P-521'; +export type EcdsaAlg = 'ES256' | 'ES384' | 'ES512'; + +export const CURVES: Array<{ namedCurve: NamedCurve; alg: EcdsaAlg }> = [ + { namedCurve: 'P-256', alg: 'ES256' }, + { namedCurve: 'P-384', alg: 'ES384' }, + { namedCurve: 'P-521', alg: 'ES512' }, +]; + +export type PemKeyPair = { sdk: KeyPair; pubPem: string }; + +export function derToPem(der: Uint8Array, label: string): string { + let b = ''; + for (let i = 0; i < der.length; i++) b += String.fromCharCode(der[i]); + const b64 = + btoa(b) + .match(/.{1,64}/g) + ?.join('\n') ?? btoa(b); + return `-----BEGIN ${label}-----\n${b64}\n-----END ${label}-----`; +} + +export function decodeBase64url(value: string): Uint8Array { + const base64 = value + .replace(/-/g, '+') + .replace(/_/g, '/') + .padEnd(Math.ceil(value.length / 4) * 4, '='); + return Uint8Array.from(atob(base64), (character) => character.charCodeAt(0)); +} + +export function encodeBase64url(value: Uint8Array): string { + let binary = ''; + for (const byte of value) binary += String.fromCharCode(byte); + return btoa(binary).replace(/=/g, '').replace(/\+/g, '-').replace(/\//g, '_'); +} + +async function toSdkKeyPair(raw: CryptoKeyPair): Promise { + const [privDer, pubDer] = await Promise.all([ + crypto.subtle.exportKey('pkcs8', raw.privateKey), + crypto.subtle.exportKey('spki', raw.publicKey), + ]); + const privPem = derToPem(new Uint8Array(privDer), 'PRIVATE KEY'); + const pubPem = derToPem(new Uint8Array(pubDer), 'PUBLIC KEY'); + const [privateKey, publicKey] = await Promise.all([ + importPrivateKey(privPem, { usage: 'sign', extractable: true }), + importPublicKey(pubPem, { usage: 'sign', extractable: true }), + ]); + return { sdk: { publicKey, privateKey }, pubPem }; +} + +export async function ecdsaKeyPair(namedCurve: NamedCurve): Promise { + const raw = await crypto.subtle.generateKey({ name: 'ECDSA', namedCurve }, true, [ + 'sign', + 'verify', + ]); + return toSdkKeyPair(raw); +} + +/** + * RS256 is the default DPoP alg for any RSA key and, unlike ES*, its signature + * is passed through unconverted (no DER<->P1363 transform). Suites use this to + * exercise that pass-through branch against the same conformant verifier. + */ +export async function rsaKeyPair(): Promise { + const raw = await crypto.subtle.generateKey( + { + name: 'RSASSA-PKCS1-v1_5', + modulusLength: 2048, + publicExponent: new Uint8Array([1, 0, 1]), + hash: 'SHA-256', + }, + true, + ['sign', 'verify'] + ); + return toSdkKeyPair(raw); +} diff --git a/lib/tests/mocha/reqsignature-jws.spec.ts b/lib/tests/mocha/reqsignature-jws.spec.ts new file mode 100644 index 000000000..f82268d15 --- /dev/null +++ b/lib/tests/mocha/reqsignature-jws.spec.ts @@ -0,0 +1,84 @@ +import { expect } from 'chai'; +import * as jose from 'jose'; + +import { reqSignature } from '../../src/auth/auth.js'; +import { signJwt, verifyJwt } from '../../tdf3/src/crypto/jwt.js'; +import { DefaultCryptoService } from '../../tdf3/src/crypto/index.js'; +import { CURVES, ecdsaKeyPair, rsaKeyPair } from './helpers/jws-keys.js'; + +/** + * RFC 7518 §3.4 conformance for `signJwt`/`reqSignature` (the KAS rewrap request + * token signer). + * + * Regression for DSPX-3397: the rewrap request token was signed with ECDSA + * signatures in DER form, which a real (RFC-conformant) KAS rejects with + * "unable to verify request token". The mock test server only `decodeJwt`s the + * token (no signature check), so the in-SDK round-trip and the mock both passed + * while the real platform failed. Verifying against `jose.jwtVerify` — which + * requires raw IEEE P1363 (R||S) signatures — catches the DER-vs-raw bug. + */ + +describe('reqSignature / signJwt — JWS conformance vs jose.jwtVerify (RFC 7518 §3.4)', function (this: Mocha.Suite) { + this.timeout(10_000); + + for (const { namedCurve, alg } of CURVES) { + it(`reqSignature ${alg} token verifies against jose.jwtVerify`, async () => { + const { sdk, pubPem } = await ecdsaKeyPair(namedCurve); + + const token = await reqSignature( + { requestBody: 'hello' }, + sdk.privateKey, + DefaultCryptoService, + { + alg, + } + ); + + // jose requires raw IEEE P1363 signatures — this rejects DER. + const key = await jose.importSPKI(pubPem, alg); + const { payload } = await jose.jwtVerify(token, key); + expect(payload.requestBody).to.equal('hello'); + expect(payload.iat).to.be.a('number'); + expect(payload.exp).to.be.a('number'); + }); + + it(`signJwt ${alg} round-trips through verifyJwt`, async () => { + const { sdk } = await ecdsaKeyPair(namedCurve); + const token = await signJwt(DefaultCryptoService, { sub: 'test' }, sdk.privateKey, { alg }); + const { payload } = await verifyJwt(DefaultCryptoService, token, sdk.publicKey, { + algorithms: [alg], + }); + expect(payload.sub).to.equal('test'); + }); + } + + it('reqSignature RS256 token verifies against jose.jwtVerify', async () => { + const { sdk, pubPem } = await rsaKeyPair(); + + const token = await reqSignature( + { requestBody: 'hello' }, + sdk.privateKey, + DefaultCryptoService, + { + alg: 'RS256', + } + ); + + const key = await jose.importSPKI(pubPem, 'RS256'); + const { payload } = await jose.jwtVerify(token, key); + expect(payload.requestBody).to.equal('hello'); + expect(payload.iat).to.be.a('number'); + expect(payload.exp).to.be.a('number'); + }); + + it('signJwt RS256 round-trips through verifyJwt', async () => { + const { sdk } = await rsaKeyPair(); + const token = await signJwt(DefaultCryptoService, { sub: 'test' }, sdk.privateKey, { + alg: 'RS256', + }); + const { payload } = await verifyJwt(DefaultCryptoService, token, sdk.publicKey, { + algorithms: ['RS256'], + }); + expect(payload.sub).to.equal('test'); + }); +}); From 5722f0b339926264269805f5fe0c29bc9f5da98f Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 10 Aug 2026 12:22:15 -0400 Subject: [PATCH 2/2] fix(sdk): bounds-check the DER ECDSA signature parser (DSPX-3397) derToIeeeP1363 indexed into the signature buffer without checking bounds: a truncated or malformed DER blob read `undefined` length bytes, sliced empty components, and then computed a negative offset in `result.set()`. It also only stripped a single leading zero, and only when the component was already over-long, so a validly-padded value could survive at the wrong width. ieeeP1363ToDer had the mirror gap: it derived the component length by halving the input, so an odd-length or wrong-curve input silently produced a fractional split rather than an error. Both now derive the fixed component width from the algorithm (getEcdsaComponentLength) and validate every read. Malformed input raises ConfigurationError with a specific message instead of producing a garbage signature. Signed-off-by: Dave Mihalcik --- lib/tdf3/src/crypto/core/signing.ts | 130 ++++++++++-------- lib/tests/mocha/reqsignature-jws.spec.ts | 31 ++++- .../mocha/unit/crypto/der-signature.spec.ts | 125 +++++++++++++++++ 3 files changed, 231 insertions(+), 55 deletions(-) create mode 100644 lib/tests/mocha/unit/crypto/der-signature.spec.ts diff --git a/lib/tdf3/src/crypto/core/signing.ts b/lib/tdf3/src/crypto/core/signing.ts index a678e93b7..8a7ee7fc8 100644 --- a/lib/tdf3/src/crypto/core/signing.ts +++ b/lib/tdf3/src/crypto/core/signing.ts @@ -39,8 +39,29 @@ function getSigningAlgorithmParams(algorithm: AsymmetricSigningAlgorithm): { } } +/** Fixed-width byte length of each ECDSA signature component (R or S). */ +function getEcdsaComponentLength(algorithm: AsymmetricSigningAlgorithm): number { + switch (algorithm) { + case 'ES256': + return 32; + case 'ES384': + return 48; + case 'ES512': + return 66; + default: + throw new ConfigurationError(`Unsupported algorithm for ECDSA conversion: ${algorithm}`); + } +} + +/** Drop leading zero bytes, keeping at least one so the value stays non-empty. */ +function trimLeadingZeros(arr: Uint8Array): Uint8Array { + let i = 0; + while (i < arr.length - 1 && arr[i] === 0) i++; + return arr.slice(i); +} + /** - * Convert IEEE P1363 signature format (used by WebCrypto ECDSA) to DER format (used by JWT). + * Convert IEEE P1363 signature format (used by WebCrypto ECDSA) to DER format. * RS256 signatures don't need conversion. */ export function ieeeP1363ToDer( @@ -51,18 +72,19 @@ export function ieeeP1363ToDer( return signature; } + const componentLen = getEcdsaComponentLength(algorithm); + const expectedLength = componentLen * 2; + if (signature.length !== expectedLength) { + throw new ConfigurationError( + `Invalid IEEE P1363 signature: expected ${expectedLength} bytes for ${algorithm}, got ${signature.length}` + ); + } + // IEEE P1363: r || s where each is padded to key size - const halfLen = signature.length / 2; - const r = signature.slice(0, halfLen); - const s = signature.slice(halfLen); + const r = signature.slice(0, componentLen); + const s = signature.slice(componentLen); // Remove leading zeros but keep one if the high bit is set - const trimLeadingZeros = (arr: Uint8Array): Uint8Array => { - let i = 0; - while (i < arr.length - 1 && arr[i] === 0) i++; - return arr.slice(i); - }; - let rTrimmed = trimLeadingZeros(r); let sTrimmed = trimLeadingZeros(s); @@ -97,12 +119,12 @@ export function ieeeP1363ToDer( } /** - * Convert DER signature format (used by JWT) to IEEE P1363 format (used by WebCrypto ECDSA). - * RS256 signatures don't need conversion. + * Convert DER-encoded ECDSA signature to raw IEEE P1363 (R||S) format. + * RS256 signatures pass through unchanged. * * Exported because callers that emit JWS (e.g. DPoP proofs in lib/src/auth/dpop.ts) - * must produce raw R||S per RFC 7518 section 3.4, while cryptoService.sign() - * currently returns DER. See DSPX-3634 for the broader cleanup. + * must produce raw R||S per RFC 7518 §3.4, while cryptoService.sign() currently + * returns DER. See DSPX-3634 for the broader cleanup. */ export function derToIeeeP1363( signature: Uint8Array, @@ -112,20 +134,14 @@ export function derToIeeeP1363( return signature; } - // Determine the expected component length based on algorithm - let componentLen: number; - switch (algorithm) { - case 'ES256': - componentLen = 32; - break; - case 'ES384': - componentLen = 48; - break; - case 'ES512': - componentLen = 66; - break; - default: - throw new ConfigurationError(`Unsupported algorithm for DER conversion: ${algorithm}`); + const componentLen = getEcdsaComponentLength(algorithm); + + // Smallest well-formed ECDSA DER SEQUENCE is 8 bytes: + // 0x30 seqLen 0x02 rLen r(>=1) 0x02 sLen s(>=1) + // Anything shorter cannot be parsed; reject before indexing so a malformed + // input throws a clean ConfigurationError rather than coercing undefined. + if (signature.length < 8) { + throw new ConfigurationError('Invalid DER signature: too short'); } if (signature[0] !== 0x30) { @@ -141,40 +157,46 @@ export function derToIeeeP1363( throw new ConfigurationError('Invalid DER signature: invalid long-form length'); } offset += 1 + lenBytesCount; - if (offset > signature.length) { - throw new ConfigurationError('Invalid DER signature: length bytes exceed signature length'); - } } else { // Short-form: single length byte. offset += 1; } - // Parse r INTEGER - if (signature[offset] !== 0x02) { - throw new ConfigurationError('Invalid DER signature: expected INTEGER for r'); - } - const rLen = signature[offset + 1]; - offset += 2; - let r = signature.slice(offset, offset + rLen); - offset += rLen; - - // Parse s INTEGER - if (signature[offset] !== 0x02) { - throw new ConfigurationError('Invalid DER signature: expected INTEGER for s'); - } - const sLen = signature[offset + 1]; - offset += 2; - let s = signature.slice(offset, offset + sLen); + // Parse a DER INTEGER at `offset`, advancing past it. Every read is + // bounds-checked so a truncated or over-long length field throws a clean + // ConfigurationError instead of silently slicing a short/empty component. + const readInteger = (label: 'r' | 's'): Uint8Array => { + if (offset + 1 >= signature.length) { + throw new ConfigurationError(`Invalid DER signature: truncated before ${label} INTEGER`); + } + if (signature[offset] !== 0x02) { + throw new ConfigurationError(`Invalid DER signature: expected INTEGER for ${label}`); + } + const len = signature[offset + 1]; + const start = offset + 2; + const end = start + len; + if (len === 0 || end > signature.length) { + throw new ConfigurationError(`Invalid DER signature: ${label} INTEGER length out of range`); + } + offset = end; + return signature.slice(start, end); + }; - // Remove leading zero padding if present - if (r[0] === 0 && r.length > componentLen) { - r = r.slice(1); - } - if (s[0] === 0 && s.length > componentLen) { - s = s.slice(1); + let r = readInteger('r'); + let s = readInteger('s'); + + // Strip DER's leading zero padding (INTEGERs are zero-prefixed to stay positive). + r = trimLeadingZeros(r); + s = trimLeadingZeros(s); + + // After stripping, each component must fit its fixed-width slot; a larger value + // means the signature does not belong to this curve (and would otherwise produce + // a negative offset in result.set below). + if (r.length > componentLen || s.length > componentLen) { + throw new ConfigurationError('Invalid DER signature: component larger than expected for curve'); } - // Pad to component length + // Pad to component length (right-aligned): result = r_padded || s_padded. const result = new Uint8Array(componentLen * 2); result.set(r, componentLen - r.length); result.set(s, componentLen * 2 - s.length); diff --git a/lib/tests/mocha/reqsignature-jws.spec.ts b/lib/tests/mocha/reqsignature-jws.spec.ts index f82268d15..896d221fb 100644 --- a/lib/tests/mocha/reqsignature-jws.spec.ts +++ b/lib/tests/mocha/reqsignature-jws.spec.ts @@ -4,7 +4,13 @@ import * as jose from 'jose'; import { reqSignature } from '../../src/auth/auth.js'; import { signJwt, verifyJwt } from '../../tdf3/src/crypto/jwt.js'; import { DefaultCryptoService } from '../../tdf3/src/crypto/index.js'; -import { CURVES, ecdsaKeyPair, rsaKeyPair } from './helpers/jws-keys.js'; +import { + CURVES, + decodeBase64url, + ecdsaKeyPair, + encodeBase64url, + rsaKeyPair, +} from './helpers/jws-keys.js'; /** * RFC 7518 §3.4 conformance for `signJwt`/`reqSignature` (the KAS rewrap request @@ -81,4 +87,27 @@ describe('reqSignature / signJwt — JWS conformance vs jose.jwtVerify (RFC 7518 }); expect(payload.sub).to.equal('test'); }); + + it('verifyJwt rejects a truncated ES256 signature', async () => { + const { sdk } = await ecdsaKeyPair('P-256'); + const token = await signJwt(DefaultCryptoService, { sub: 'test' }, sdk.privateKey, { + alg: 'ES256', + }); + const [header, payload, signature] = token.split('.'); + const truncated = encodeBase64url(decodeBase64url(signature).subarray(1)); + + let caught: unknown; + try { + await verifyJwt(DefaultCryptoService, `${header}.${payload}.${truncated}`, sdk.publicKey, { + algorithms: ['ES256'], + }); + } catch (error) { + caught = error; + } + + expect(caught).to.be.instanceOf(Error); + expect((caught as Error).message).to.include( + 'Invalid IEEE P1363 signature: expected 64 bytes for ES256, got 63' + ); + }); }); diff --git a/lib/tests/mocha/unit/crypto/der-signature.spec.ts b/lib/tests/mocha/unit/crypto/der-signature.spec.ts new file mode 100644 index 000000000..d655a1f63 --- /dev/null +++ b/lib/tests/mocha/unit/crypto/der-signature.spec.ts @@ -0,0 +1,125 @@ +import { expect } from 'chai'; + +import { derToIeeeP1363, ieeeP1363ToDer } from '../../../../tdf3/src/crypto/core/signing.js'; +import { ConfigurationError } from '../../../../src/errors.js'; + +/** + * Direct unit tests for derToIeeeP1363's DER parsing. The happy path (real + * signatures round-tripping through sign→verify) is covered in + * crypto-service.spec.ts; these focus on malformed input, which must always + * throw a controlled ConfigurationError rather than an out-of-bounds + * RangeError/TypeError or a silently-truncated component. + */ +describe('derToIeeeP1363 DER validation', () => { + it('RS256 passes through unchanged (no DER parsing)', () => { + const sig = new Uint8Array([1, 2, 3]); + expect(derToIeeeP1363(sig, 'RS256')).to.equal(sig); + }); + + describe('well-formed DER (positive controls)', () => { + it('parses a minimal r=0x01, s=0x02 into a right-aligned 64-byte ES256 output', () => { + // 0x30 seqLen 0x02 rLen r 0x02 sLen s + const der = new Uint8Array([0x30, 0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x02]); + const out = derToIeeeP1363(der, 'ES256'); + expect(out).to.have.length(64); + expect(out[31]).to.equal(0x01); // r right-aligned in first 32 bytes + expect(out[63]).to.equal(0x02); // s right-aligned in second 32 bytes + // everything else zero-padded + expect(out.slice(0, 31).every((b) => b === 0)).to.be.true; + expect(out.slice(32, 63).every((b) => b === 0)).to.be.true; + }); + + it('strips a DER leading-zero pad byte (high-bit component)', () => { + // r = 0x00 0x80 (zero-prefixed to stay positive) → 0x80 after stripping + const der = new Uint8Array([0x30, 0x07, 0x02, 0x02, 0x00, 0x80, 0x02, 0x01, 0x01]); + const out = derToIeeeP1363(der, 'ES256'); + expect(out).to.have.length(64); + expect(out[31]).to.equal(0x80); + expect(out[63]).to.equal(0x01); + }); + }); + + describe('malformed DER throws ConfigurationError', () => { + const cases: Array<{ name: string; bytes: number[]; match: RegExp }> = [ + { name: 'empty', bytes: [], match: /too short/ }, + { name: 'single 0x30 byte', bytes: [0x30], match: /too short/ }, + { + name: 'wrong SEQUENCE tag', + bytes: [0x31, 0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x01], + match: /expected SEQUENCE/, + }, + { + name: 'missing INTEGER tag for r', + bytes: [0x30, 0x06, 0x03, 0x01, 0x01, 0x02, 0x01, 0x01], + match: /expected INTEGER for r/, + }, + { + name: 'r INTEGER length overruns the buffer', + bytes: [0x30, 0x06, 0x02, 0x40, 0x01, 0x02, 0x01, 0x01], + match: /r INTEGER length out of range/, + }, + { + name: 's INTEGER length overruns the buffer', + bytes: [0x30, 0x08, 0x02, 0x01, 0x01, 0x02, 0x40, 0x01], + match: /s INTEGER length out of range/, + }, + { + name: 'truncated before s INTEGER', + bytes: [0x30, 0x82, 0x00, 0x08, 0x02, 0x01, 0x01, 0x02], + match: /truncated before s INTEGER/, + }, + { + name: 'invalid long-form length (too many length bytes)', + bytes: [0x30, 0x85, 0, 0, 0, 0, 0, 0], + match: /invalid long-form length/, + }, + ]; + + for (const { name, bytes, match } of cases) { + it(name, () => { + expect(() => derToIeeeP1363(new Uint8Array(bytes), 'ES256')).to.throw( + ConfigurationError, + match + ); + }); + } + + it('rejects an r component larger than the curve size (ES256)', () => { + // r = 33 bytes, no leading zero → cannot fit a 32-byte P-256 slot. + const rBytes = new Array(33).fill(0x7f); + const der = new Uint8Array([ + 0x30, + 2 + 33 + 3, // seqLen (short form): r INTEGER (2+33) + s INTEGER (2+1) + 0x02, + 33, + ...rBytes, + 0x02, + 0x01, + 0x01, + ]); + expect(() => derToIeeeP1363(der, 'ES256')).to.throw( + ConfigurationError, + /component larger than expected/ + ); + }); + }); +}); + +describe('ieeeP1363ToDer fixed-width validation', () => { + for (const [algorithm, expectedLength] of [ + ['ES256', 64], + ['ES384', 96], + ['ES512', 132], + ] as const) { + it(`${algorithm} accepts exactly ${expectedLength} bytes`, () => { + expect(ieeeP1363ToDer(new Uint8Array(expectedLength), algorithm)[0]).to.equal(0x30); + }); + + it(`${algorithm} rejects a shortened signature`, () => { + expect(() => ieeeP1363ToDer(new Uint8Array(expectedLength - 1), algorithm)).to.throw( + ConfigurationError, + `expected ${expectedLength} bytes` + ); + }); + } +});