diff --git a/.changeset/five-eagles-tap.md b/.changeset/five-eagles-tap.md new file mode 100644 index 00000000000..415e5bf8691 --- /dev/null +++ b/.changeset/five-eagles-tap.md @@ -0,0 +1,5 @@ +--- +'@clerk/backend': patch +--- + +The JWT claims are verified after the signature to avoid leaking information through error messages on forged tokens. diff --git a/packages/backend/src/jwt/verifyJwt.ts b/packages/backend/src/jwt/verifyJwt.ts index 3070ddd5d6c..5b828bd8a18 100644 --- a/packages/backend/src/jwt/verifyJwt.ts +++ b/packages/backend/src/jwt/verifyJwt.ts @@ -145,20 +145,12 @@ export async function verifyJwt( assertHeaderType(typ, headerType); assertHeaderAlgorithm(alg); - - // Payload verifications - const { azp, sub, aud, iat, exp, nbf } = payload; - - assertSubClaim(sub); - assertAudienceClaim([aud], [audience]); - assertAuthorizedPartiesClaim(azp, authorizedParties); - assertExpirationClaim(exp, clockSkew); - assertActivationClaim(nbf, clockSkew); - assertIssuedAtClaim(iat, clockSkew); } catch (err) { return { errors: [err as TokenVerificationError] }; } + // Verify signature before validating claims to prevent oracle attacks + // that could leak configuration details through differential error responses const { data: signatureValid, errors: signatureErrors } = await hasValidSignature(decoded, key); if (signatureErrors) { return { @@ -183,5 +175,19 @@ export async function verifyJwt( }; } + // Payload verifications (only after signature is confirmed valid) + try { + const { azp, sub, aud, iat, exp, nbf } = payload; + + assertSubClaim(sub); + assertAudienceClaim([aud], [audience]); + assertAuthorizedPartiesClaim(azp, authorizedParties); + assertExpirationClaim(exp, clockSkew); + assertActivationClaim(nbf, clockSkew); + assertIssuedAtClaim(iat, clockSkew); + } catch (err) { + return { errors: [err as TokenVerificationError] }; + } + return { data: payload }; } diff --git a/packages/backend/src/tokens/__tests__/request.test.ts b/packages/backend/src/tokens/__tests__/request.test.ts index b9cc1d67f68..e9b5fa6bfda 100644 --- a/packages/backend/src/tokens/__tests__/request.test.ts +++ b/packages/backend/src/tokens/__tests__/request.test.ts @@ -8,7 +8,7 @@ import { mockJwks, mockJwt, mockJwtPayload, - mockMalformedJwt, + signingJwks, } from '../../fixtures'; import { mockMachineAuthResponses, @@ -16,6 +16,7 @@ import { mockTokens, mockVerificationResults, } from '../../fixtures/machine'; +import { signJwt } from '../../jwt/signJwt'; import { server } from '../../mock-server'; import type { AuthReason } from '../authStatus'; import { AuthErrorReason, AuthStatus } from '../authStatus'; @@ -1193,13 +1194,20 @@ describe('tokens.authenticateRequest(options)', () => { }), ); + // Create a properly signed JWT that is missing the 'sub' claim + const { sub: _, ...payloadWithoutSub } = mockJwtPayload; + const { data: malformedJwt } = await signJwt(payloadWithoutSub, signingJwks, { + algorithm: 'RS256', + header: { typ: 'JWT', kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' }, + }); + const requestState = await authenticateRequest( mockRequestWithCookies( {}, { __clerk_db_jwt: 'deadbeef', __client_uat: `${mockJwtPayload.iat - 10}`, - __session: mockMalformedJwt, + __session: malformedJwt!, }, ), mockOptions(), diff --git a/packages/backend/src/tokens/__tests__/verify.test.ts b/packages/backend/src/tokens/__tests__/verify.test.ts index b682db6ef37..a396d796504 100644 --- a/packages/backend/src/tokens/__tests__/verify.test.ts +++ b/packages/backend/src/tokens/__tests__/verify.test.ts @@ -20,14 +20,15 @@ import { signJwt } from '../../jwt/signJwt'; import { server, validateHeaders } from '../../mock-server'; import { verifyMachineAuthToken, verifyToken } from '../verify'; -function createOAuthJwt( +async function createSignedOAuthJwt( payload = mockOAuthAccessTokenJwtPayload, typ: 'at+jwt' | 'application/at+jwt' | 'JWT' = 'at+jwt', ) { - return createJwt({ + const { data } = await signJwt(payload, signingJwks, { + algorithm: 'RS256', header: { typ, kid: 'ins_2GIoQhbUpy0hX7B2cVkuTMinXoD' }, - payload, }); + return data!; } async function createSignedM2MJwt(payload = mockM2MJwtPayload) { @@ -85,6 +86,32 @@ describe('tokens.verify(token, options)', () => { expect(data).toEqual(mockJwtPayload); }); + + it('returns signature error before claims error when both are invalid', async () => { + server.use( + http.get( + 'https://api.clerk.test/v1/jwks', + validateHeaders(() => { + return HttpResponse.json(mockJwks); + }), + ), + ); + + // Create a JWT with expired claims AND an invalid signature + const expiredJwt = createJwt({ + payload: { ...mockJwtPayload, exp: mockJwtPayload.iat - 100 }, + }); + + const { errors } = await verifyToken(expiredJwt, { + apiUrl: 'https://api.clerk.test', + secretKey: 'a-valid-key', + authorizedParties: ['https://accounts.inspired.puma-74.lcl.dev'], + skipJwksCache: true, + }); + + expect(errors).toBeDefined(); + expect(errors?.[0].message).toContain('signature'); + }); }); describe('tokens.verifyMachineAuthToken(token, options)', () => { @@ -392,7 +419,7 @@ describe('tokens.verifyMachineAuthToken(token, options)', () => { ), ); - const oauthJwt = createOAuthJwt(mockOAuthAccessTokenJwtPayload, 'JWT'); + const oauthJwt = await createSignedOAuthJwt(mockOAuthAccessTokenJwtPayload, 'JWT'); const result = await verifyMachineAuthToken(oauthJwt, { apiUrl: 'https://api.clerk.test', @@ -472,7 +499,7 @@ describe('tokens.verifyMachineAuthToken(token, options)', () => { exp: mockOAuthAccessTokenJwtPayload.iat - 100, }; - const oauthJwt = createOAuthJwt(expiredPayload, 'at+jwt'); + const oauthJwt = await createSignedOAuthJwt(expiredPayload); const result = await verifyMachineAuthToken(oauthJwt, { apiUrl: 'https://api.clerk.test',