Allow "use" claims only in public jwk (#4103)

Disallow use of `use` claim in private JWK (replaced with `key_ops`)

Author: matthieusieben

.changeset/heavy-kangaroos-care.md

@@ -0,0 +1,5 @@
+---
+"@atproto/jwk": patch
+---
+
+Silently ignore invalid JWKs from JSON Web Key Set (as per spec)

.changeset/selfish-wasps-return.md

@@ -0,0 +1,5 @@
+---
+"@atproto/jwk": minor
+---
+
+Update key matching algorithm to support `key_ops`

.changeset/shiny-seals-wave.md

@@ -0,0 +1,5 @@
+---
+"@atproto/jwk": patch
+---
+
+Avoid using `revoked` and inactive keys from JWK sets

.changeset/tough-emus-explain.md

@@ -0,0 +1,6 @@
+---
+"@atproto/jwk-webcrypto": minor
+"@atproto/jwk": minor
+---
+
+Only allow `"use"` claims in public jwk

.github/workflows/repo.yaml

@@ -47,6 +47,7 @@ jobs:
     name: Test
     needs: build
     strategy:
+      fail-fast: false
       matrix:
         shard: [1/8, 2/8, 3/8, 4/8, 5/8, 6/8, 7/8, 8/8]
     runs-on: ubuntu-22.04

packages/oauth/jwk-jose/src/jose-key.ts

@@ -23,6 +23,7 @@ import {
   SignedJwt,
   VerifyOptions,
   VerifyResult,
+  isPrivateJwk,
   jwkSchema,
   jwtHeaderSchema,
   jwtPayloadSchema,
@@ -163,12 +164,12 @@ export class JoseKey<J extends Jwk = Jwk> extends Key<J> {
     allowedAlgos: string[] = ['ES256'],
     kid?: string,
     options?: Omit<GenerateKeyPairOptions, 'extractable'>,
-  ) {
+  ): Promise<JoseKey> {
     const kp = await this.generateKeyPair(allowedAlgos, {
       ...options,
       extractable: true,
     })
-    return this.fromImportable(kp.privateKey, kid)
+    return this.fromKeyLike(kp.privateKey, kid)
   }
 
   static async fromImportable(
@@ -239,8 +240,16 @@ export class JoseKey<J extends Jwk = Jwk> extends Key<J> {
     if (!jwk || typeof jwk !== 'object') throw new JwkError('Invalid JWK')
 
     const kid = either(jwk.kid, inputKid)
-    const use = jwk.use || 'sig'
 
-    return new JoseKey(jwkSchema.parse({ ...jwk, kid, use }))
+    // Backwards compatibility with old behavior
+    if (jwk.use != null && isPrivateJwk(jwk)) {
+      console.warn(
+        'Deprecation warning: Private JWK with a "use" property will be rejected in the future. Please remove replace "use" with (valid) "key_ops".',
+      )
+      jwk.key_ops ??= jwk.use === 'sig' ? ['sign'] : ['encrypt']
+      delete jwk.use
+    }
+
+    return new JoseKey<Jwk>(jwkSchema.parse({ ...jwk, kid }))
   }
 }

packages/oauth/jwk-webcrypto/src/webcrypto-key.ts

@@ -1,26 +1,15 @@
-import { z } from 'zod'
-import { JwkError, jwkSchema } from '@atproto/jwk'
+import { Jwk, JwkError, jwkSchema } from '@atproto/jwk'
 import { GenerateKeyPairOptions, JoseKey } from '@atproto/jwk-jose'
 import { fromSubtleAlgorithm, isCryptoKeyPair } from './util.js'
 
-// Webcrypto keys are bound to a single algorithm
-export const jwkWithAlgSchema = z.intersection(
-  jwkSchema,
-  z.object({ alg: z.string() }),
-)
-
-export type JwkWithAlg = z.infer<typeof jwkWithAlgSchema>
-
-export class WebcryptoKey<
-  J extends JwkWithAlg = JwkWithAlg,
-> extends JoseKey<J> {
+export class WebcryptoKey<J extends Jwk = Jwk> extends JoseKey<J> {
   // We need to override the static method generate from JoseKey because
   // the browser needs both the private and public keys
   static override async generate(
     allowedAlgos: string[] = ['ES256'],
     kid: string = crypto.randomUUID(),
     options?: GenerateKeyPairOptions,
-  ) {
+  ): Promise<WebcryptoKey> {
     const keyPair = await this.generateKeyPair(allowedAlgos, options)
 
     // Type safety only: in the browser, 'jose' always generates a CryptoKeyPair
@@ -31,14 +20,11 @@ export class WebcryptoKey<
     return this.fromKeypair(keyPair, kid)
   }
 
-  static async fromKeypair(cryptoKeyPair: CryptoKeyPair, kid?: string) {
-    // https://datatracker.ietf.org/doc/html/rfc7517
-    // > The "use" and "key_ops" JWK members SHOULD NOT be used together; [...]
-    // > Applications should specify which of these members they use.
-
+  static async fromKeypair(
+    cryptoKeyPair: CryptoKeyPair,
+    kid?: string,
+  ): Promise<WebcryptoKey> {
     const {
-      key_ops,
-      use,
       alg = fromSubtleAlgorithm(cryptoKeyPair.privateKey.algorithm),
       ...jwk
     } = await crypto.subtle.exportKey(
@@ -48,17 +34,8 @@ export class WebcryptoKey<
         : cryptoKeyPair.publicKey,
     )
 
-    if (use && use !== 'sig') {
-      throw new TypeError(`Unsupported JWK use "${use}"`)
-    }
-
-    if (key_ops && !key_ops.some((o) => o === 'sign' || o === 'verify')) {
-      // Make sure that "key_ops", if present, is compatible with "use"
-      throw new TypeError(`Invalid key_ops "${key_ops}" for "sig" use`)
-    }
-
-    return new WebcryptoKey(
-      jwkWithAlgSchema.parse({ ...jwk, kid, alg, use: 'sig' }),
+    return new WebcryptoKey<Jwk>(
+      jwkSchema.parse({ ...jwk, kid, alg }),
       cryptoKeyPair,
     )
   }
@@ -67,18 +44,16 @@ export class WebcryptoKey<
     jwk: Readonly<J>,
     readonly cryptoKeyPair: CryptoKeyPair,
   ) {
+    // Webcrypto keys are bound to a single algorithm
+    if (!jwk.alg) throw new JwkError('JWK "alg" is required for Webcrypto keys')
+
     super(jwk)
   }
 
   get isPrivate() {
     return true
   }
 
-  get privateJwk(): Readonly<J> | undefined {
-    if (super.isPrivate) return this.jwk
-    throw new Error('Private Webcrypto Key not exportable')
-  }
-
   protected override async getKeyObj(alg: string) {
     if (this.jwk.alg !== alg) {
       throw new JwkError(`Key cannot be used with algorithm "${alg}"`)

packages/oauth/jwk/src/alg.ts

@@ -1,29 +1,29 @@
 import { JwkError } from './errors.js'
-import { Jwk } from './jwk.js'
+import { JwkBase, isEncKeyUsage, isSigKeyUsage } from './jwk.js'
 
 // Copy variable to prevent bundlers from automatically polyfilling "process" (e.g. parcel)
 const { process } = globalThis
 const IS_NODE_RUNTIME =
   typeof process !== 'undefined' && typeof process?.versions?.node === 'string'
 
-export function* jwkAlgorithms(jwk: Jwk): Generator<string> {
+export function* jwkAlgorithms(jwk: JwkBase): Generator<string, void, unknown> {
   // Ed25519, Ed448, and secp256k1 always have "alg"
-  // OKP always has "use"
-  if (jwk.alg) {
+
+  if (typeof jwk.alg === 'string') {
     yield jwk.alg
     return
   }
 
   switch (jwk.kty) {
     case 'EC': {
-      if (jwk.use === 'enc' || jwk.use === undefined) {
+      if (jwkSupportsEnc(jwk)) {
         yield 'ECDH-ES'
         yield 'ECDH-ES+A128KW'
         yield 'ECDH-ES+A192KW'
         yield 'ECDH-ES+A256KW'
       }
 
-      if (jwk.use === 'sig' || jwk.use === undefined) {
+      if (jwkSupportsSig(jwk)) {
         const crv = 'crv' in jwk ? jwk.crv : undefined
         switch (crv) {
           case 'P-256':
@@ -54,15 +54,15 @@ export function* jwkAlgorithms(jwk: Jwk): Generator<string> {
     }
 
     case 'RSA': {
-      if (jwk.use === 'enc' || jwk.use === undefined) {
+      if (jwkSupportsEnc(jwk)) {
         yield 'RSA-OAEP'
         yield 'RSA-OAEP-256'
         yield 'RSA-OAEP-384'
         yield 'RSA-OAEP-512'
         if (IS_NODE_RUNTIME) yield 'RSA1_5'
       }
 
-      if (jwk.use === 'sig' || jwk.use === undefined) {
+      if (jwkSupportsSig(jwk)) {
         yield 'PS256'
         yield 'PS384'
         yield 'PS512'
@@ -75,7 +75,7 @@ export function* jwkAlgorithms(jwk: Jwk): Generator<string> {
     }
 
     case 'oct': {
-      if (jwk.use === 'enc' || jwk.use === undefined) {
+      if (jwkSupportsEnc(jwk)) {
         yield 'A128GCMKW'
         yield 'A192GCMKW'
         yield 'A256GCMKW'
@@ -84,7 +84,7 @@ export function* jwkAlgorithms(jwk: Jwk): Generator<string> {
         yield 'A256KW'
       }
 
-      if (jwk.use === 'sig' || jwk.use === undefined) {
+      if (jwkSupportsSig(jwk)) {
         yield 'HS256'
         yield 'HS384'
         yield 'HS512'
@@ -97,3 +97,15 @@ export function* jwkAlgorithms(jwk: Jwk): Generator<string> {
       throw new JwkError(`Unsupported kty "${jwk.kty}"`)
   }
 }
+
+function jwkSupportsEnc(jwk: JwkBase): boolean {
+  return (
+    jwk.key_ops?.some(isEncKeyUsage) ?? (jwk.use == null || jwk.use === 'enc')
+  )
+}
+
+function jwkSupportsSig(jwk: JwkBase): boolean {
+  return (
+    jwk.key_ops?.some(isSigKeyUsage) ?? (jwk.use == null || jwk.use === 'sig')
+  )
+}