Improved control over JWT's typ claim (#2743)

* Add "jwtAlg" option to verifySignature() function * Verify service JWT header values. Add iat claim to service JWT * Allow missing 'typ' claim in service auth jwt * Add, and verify, a "typ" header to access and refresh tokens * tidy * Properly identify JWT typ missmatch * tidy * exclude known invalid "typ" from service auth headers * tidy * tidy changeset --------- Co-authored-by: devin ivy <[email protected]>
bluesky-social · Aug 27, 2024 · ebb3183 · ebb3183
1 parent dee817b
commit ebb3183
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 18 deletions.
diff --git a/.changeset/curvy-lemons-cross.md b/.changeset/curvy-lemons-cross.md
@@ -0,0 +1,5 @@
+---
+"@atproto/xrpc-server": patch
+---
+
+Add `iat` claim to service JWTs
diff --git a/.changeset/gentle-horses-listen.md b/.changeset/gentle-horses-listen.md
@@ -0,0 +1,5 @@
+---
+"@atproto/xrpc-server": patch
+---
+
+Ensure that service auth JWT headers contain an `alg` claim, and ensure that `typ`, if present, is not an unexpected type (e.g. not an access or DPoP token).
diff --git a/.changeset/many-bikes-poke.md b/.changeset/many-bikes-poke.md
@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Add, and verify, a "typ" header to access and refresh tokens
diff --git a/.changeset/polite-peaches-repair.md b/.changeset/polite-peaches-repair.md
@@ -0,0 +1,5 @@
+---
+"@atproto/crypto": patch
+---
+
+Add "`jwtAlg`" option to `verifySignature()` function
diff --git a/packages/crypto/src/verify.ts b/packages/crypto/src/verify.ts
@@ -7,9 +7,14 @@ export const verifySignature = (
   didKey: string,
   data: Uint8Array,
   sig: Uint8Array,
-  opts?: VerifyOptions,
+  opts?: VerifyOptions & {
+    jwtAlg?: string
+  },
 ): Promise<boolean> => {
   const parsed = parseDidKey(didKey)
+  if (opts?.jwtAlg && opts.jwtAlg !== parsed.jwtAlg) {
+    throw new Error(`Expected key alg ${opts.jwtAlg}, got ${parsed.jwtAlg}`)
+  }
   const plugin = plugins.find((p) => p.jwtAlg === parsed.jwtAlg)
   if (!plugin) {
     throw new Error(`Unsupported signature alg: ${parsed.jwtAlg}`)

diff --git a/packages/pds/src/account-manager/helpers/auth.ts b/packages/pds/src/account-manager/helpers/auth.ts
@@ -46,7 +46,10 @@ export const createAccessToken = (opts: {
     expiresIn = '120mins',
   } = opts
   const signer = new jose.SignJWT({ scope })
-    .setProtectedHeader({ alg: 'HS256' }) // only symmetric keys supported
+    .setProtectedHeader({
+      typ: 'at+jwt', // https://www.rfc-editor.org/rfc/rfc9068.html
+      alg: 'HS256', // only symmetric keys supported
+    })
     .setAudience(serviceDid)
     .setSubject(did)
     .setIssuedAt()
@@ -69,7 +72,10 @@ export const createRefreshToken = (opts: {
     expiresIn = '90days',
   } = opts
   const signer = new jose.SignJWT({ scope: AuthScope.Refresh })
-    .setProtectedHeader({ alg: 'HS256' }) // only symmetric keys supported
+    .setProtectedHeader({
+      typ: 'refresh+jwt',
+      alg: 'HS256', // only symmetric keys supported
+    })
     .setAudience(serviceDid)
     .setSubject(did)
     .setJti(jti)

diff --git a/packages/pds/src/auth-verifier.ts b/packages/pds/src/auth-verifier.ts
@@ -320,10 +320,11 @@ export class AuthVerifier {
 
   protected async validateRefreshToken(
     ctx: ReqCtx,
-    verifyOptions?: Omit<jose.JWTVerifyOptions, 'audience'>,
+    verifyOptions?: Omit<jose.JWTVerifyOptions, 'audience' | 'typ'>,
   ): Promise<ValidatedRefreshBearer> {
     const result = await this.validateBearerToken(ctx, [AuthScope.Refresh], {
       ...verifyOptions,
+      typ: 'refresh+jwt',
       // when using entryway, proxying refresh credentials
       audience: this.dids.entryway ? this.dids.entryway : this.dids.pds,
     })
@@ -340,7 +341,8 @@ export class AuthVerifier {
   protected async validateBearerToken(
     ctx: ReqCtx,
     scopes: AuthScope[],
-    verifyOptions?: jose.JWTVerifyOptions,
+    verifyOptions: jose.JWTVerifyOptions &
+      Required<Pick<jose.JWTVerifyOptions, 'audience' | 'typ'>>,
   ): Promise<ValidatedBearer> {
     this.setAuthHeaders(ctx)
 
@@ -351,15 +353,26 @@ export class AuthVerifier {
 
     const { payload, protectedHeader } = await this.jwtVerify(
       token,
-      verifyOptions,
+      // @TODO: Once all access & refresh tokens have a "typ" claim (i.e. 90
+      // days after this code was deployed), replace the following line with
+      // "verifyOptions," (to re-enable the verification of the "typ" property
+      // from verifyJwt()). Once the change is made, the "if" block below that
+      // checks for "typ" can be removed.
+      {
+        ...verifyOptions,
+        typ: undefined,
+      },
     )
 
-    if (protectedHeader.typ === 'dpop+jwt') {
-      // @TODO we should make sure that bearer access tokens do have their "typ"
-      // claim, and allow list the possible value(s) here (typically "at+jwt"),
-      // instead of using a deny list. This would be more secure & future proof
-      // against new token types that would be introduced in the future
-      throw new InvalidRequestError('Malformed token', 'InvalidToken')
+    // @TODO: remove the next check once all access & refresh tokens have "typ"
+    // Note: when removing the check, make sure that the "verifyOptions"
+    // contains the "typ" property, so that the token is verified correctly by
+    // this.verifyJwt()
+    if (protectedHeader.typ && verifyOptions.typ !== protectedHeader.typ) {
+      // Temporarily allow historical tokens without "typ" to pass through. See:
+      // createAccessToken() and createRefreshToken() in
+      // src/account-manager/helpers/auth.ts
+      throw new InvalidRequestError('Invalid token type', 'InvalidToken')
     }
 
     const { sub, aud, scope } = payload
@@ -372,8 +385,9 @@ export class AuthVerifier {
     ) {
       throw new InvalidRequestError('Malformed token', 'InvalidToken')
     }
-    if ((payload.cnf as any)?.jkt) {
-      // DPoP bound tokens must not be usable as regular Bearer tokens
+    if (payload['cnf'] !== undefined) {
+      // Proof-of-Possession (PoP) tokens are not allowed here
+      // https://www.rfc-editor.org/rfc/rfc7800.html
       throw new InvalidRequestError('Malformed token', 'InvalidToken')
     }
     if (!isAuthScope(scope) || (scopes.length > 0 && !scopes.includes(scope))) {
@@ -550,7 +564,7 @@ export class AuthVerifier {
     const { did, scope, token, audience } = await this.validateBearerToken(
       ctx,
       scopes,
-      { audience: this.dids.pds },
+      { audience: this.dids.pds, typ: 'at+jwt' },
     )
     const isPrivileged = [
       AuthScope.Access,

diff --git a/packages/pds/tests/auth.test.ts b/packages/pds/tests/auth.test.ts
@@ -243,7 +243,7 @@ describe('auth', () => {
       password: 'password',
     })
     const refreshWithAccess = refreshSession(account.accessJwt)
-    await expect(refreshWithAccess).rejects.toThrow('Bad token scope')
+    await expect(refreshWithAccess).rejects.toThrow('Invalid token type')
   })
 
   it('expired refresh token cannot be used to refresh a session.', async () => {

diff --git a/packages/xrpc-server/src/auth.ts b/packages/xrpc-server/src/auth.ts
@@ -7,11 +7,16 @@ import { AuthRequiredError } from './types'
 type ServiceJwtParams = {
   iss: string
   aud: string
+  iat?: number
   exp?: number
   lxm: string | null
   keypair: crypto.Keypair
 }
 
+type ServiceJwtHeaders = {
+  alg: string
+} & Record<string, unknown>
+
 type ServiceJwtPayload = {
   iss: string
   aud: string
@@ -24,14 +29,16 @@ export const createServiceJwt = async (
   params: ServiceJwtParams,
 ): Promise<string> => {
   const { iss, aud, keypair } = params
-  const exp = params.exp ?? Math.floor((Date.now() + MINUTE) / 1000)
+  const iat = params.iat ?? Math.floor(Date.now() / 1e3)
+  const exp = params.exp ?? iat + MINUTE / 1e3
   const lxm = params.lxm ?? undefined
   const jti = await crypto.randomStr(16, 'hex')
   const header = {
     typ: 'JWT',
     alg: keypair.jwtAlg,
   }
   const payload = common.noUndefinedVals({
+    iat,
     iss,
     aud,
     exp,
@@ -65,6 +72,27 @@ export const verifyJwt = async (
   if (parts.length !== 3) {
     throw new AuthRequiredError('poorly formatted jwt', 'BadJwt')
   }
+
+  const header = parseHeader(parts[0])
+
+  // The spec does not describe what to do with the "typ" claim. We can,
+  // however, forbid some values that are not compatible with our use case.
+  if (
+    // service tokens are not OAuth 2.0 access tokens
+    // https://datatracker.ietf.org/doc/html/rfc9068
+    header['typ'] === 'at+jwt' ||
+    // "refresh+jwt" is a non-standard type used by the @atproto packages
+    header['typ'] === 'refresh+jwt' ||
+    // "DPoP" proofs are not meant to be used as service tokens
+    // https://datatracker.ietf.org/doc/html/rfc9449
+    header['typ'] === 'dpop+jwt'
+  ) {
+    throw new AuthRequiredError(
+      `Invalid jwt type "${header['typ']}"`,
+      'BadJwtType',
+    )
+  }
+
   const payload = parsePayload(parts[1])
   const sig = parts[2]
 
@@ -88,8 +116,9 @@ export const verifyJwt = async (
 
   const msgBytes = ui8.fromString(parts.slice(0, 2).join('.'), 'utf8')
   const sigBytes = ui8.fromString(sig, 'base64url')
-  const verifySignatureWithKey = (key: string) => {
+  const verifySignatureWithKey = async (key: string) => {
     return crypto.verifySignature(key, msgBytes, sigBytes, {
+      jwtAlg: header.alg,
       allowMalleableSig: true,
     })
   }
@@ -136,6 +165,14 @@ const parseB64UrlToJson = (b64: string) => {
   return JSON.parse(common.b64UrlToUtf8(b64))
 }
 
+const parseHeader = (b64: string): ServiceJwtHeaders => {
+  const header = parseB64UrlToJson(b64)
+  if (!header || typeof header !== 'object' || typeof header.alg !== 'string') {
+    throw new AuthRequiredError('poorly formatted jwt', 'BadJwt')
+  }
+  return header
+}
+
 const parsePayload = (b64: string): ServiceJwtPayload => {
   const payload = parseB64UrlToJson(b64)
   if (