Respect updated service auth keys (#1765)

* bust key cache when verifying service auth

* unit tests for xrpc auth

* fix

* support option for verifying non-low-s signatures

* fix verifyJwt tests

interop-test-files/crypto/signature-fixtures.json

@@ -7,7 +7,8 @@
     "publicKeyDid": "did:key:zDnaembgSGUhZULN2Caob4HLJPaxBh92N7rtH21TErzqf8HQo",
     "publicKeyMultibase": "zxdM8dSstjrpZaRUwBmDvjGXweKuEMVN95A9oJBFjkWMh",
     "signatureBase64": "2vZNsG3UKvvO/CDlrdvyZRISOFylinBh0Jupc6KcWoJWExHptCfduPleDbG3rko3YZnn9Lw0IjpixVmexJDegg",
-    "validSignature": true
+    "validSignature": true,
+    "tags": []
   },
   {
     "comment": "valid K-256 key and signature, with low-S signature",
@@ -17,7 +18,8 @@
     "publicKeyDid": "did:key:zQ3shqwJEJyMBsBXCWyCBpUBMqxcon9oHB7mCvx4sSpMdLJwc",
     "publicKeyMultibase": "z25z9DTpsiYYJKGsWmSPJK2NFN8PcJtZig12K59UgW7q5t",
     "signatureBase64": "5WpdIuEUUfVUYaozsi8G0B3cWO09cgZbIIwg1t2YKdUn/FEznOndsz/qgiYb89zwxYCbB71f7yQK5Lr7NasfoA",
-    "validSignature": true
+    "validSignature": true,
+    "tags": []
   },
   {
     "comment": "P-256 key and signature, with non-low-S signature which is invalid in atproto",
@@ -27,7 +29,8 @@
     "publicKeyDid": "did:key:zDnaembgSGUhZULN2Caob4HLJPaxBh92N7rtH21TErzqf8HQo",
     "publicKeyMultibase": "zxdM8dSstjrpZaRUwBmDvjGXweKuEMVN95A9oJBFjkWMh",
     "signatureBase64": "2vZNsG3UKvvO/CDlrdvyZRISOFylinBh0Jupc6KcWoKp7O4VS9giSAah8k5IUbXIW00SuOrjfEqQ9HEkN9JGzw",
-    "validSignature": false
+    "validSignature": false,
+    "tags": ["high-s"]
   },
   {
     "comment": "K-256 key and signature, with non-low-S signature which is invalid in atproto",
@@ -37,6 +40,7 @@
     "publicKeyDid": "did:key:zQ3shqwJEJyMBsBXCWyCBpUBMqxcon9oHB7mCvx4sSpMdLJwc",
     "publicKeyMultibase": "z25z9DTpsiYYJKGsWmSPJK2NFN8PcJtZig12K59UgW7q5t",
     "signatureBase64": "5WpdIuEUUfVUYaozsi8G0B3cWO09cgZbIIwg1t2YKdXYA67MYxYiTMAVfdnkDCMN9S5B3vHosRe07aORmoshoQ",
-    "validSignature": false
+    "validSignature": false,
+    "tags": ["high-s"]
   }
 ]

packages/bsky/src/auth.ts

@@ -14,10 +14,17 @@ export const authVerifier =
     if (!jwtStr) {
       throw new AuthRequiredError('missing jwt', 'MissingJwt')
     }
-    const payload = await verifyJwt(jwtStr, opts.aud, async (did: string) => {
-      const atprotoData = await idResolver.did.resolveAtprotoData(did)
-      return atprotoData.signingKey
-    })
+    const payload = await verifyJwt(
+      jwtStr,
+      opts.aud,
+      async (did, forceRefresh) => {
+        const atprotoData = await idResolver.did.resolveAtprotoData(
+          did,
+          forceRefresh,
+        )
+        return atprotoData.signingKey
+      },
+    )
     return { credentials: { did: payload.iss }, artifacts: { aud: opts.aud } }
   }
 

packages/bsky/tests/auth.test.ts

@@ -0,0 +1,64 @@
+import AtpAgent from '@atproto/api'
+import { SeedClient, TestNetwork } from '@atproto/dev-env'
+import usersSeed from './seeds/users'
+import { createServiceJwt } from '@atproto/xrpc-server'
+import { Keypair, Secp256k1Keypair } from '@atproto/crypto'
+
+describe('auth', () => {
+  let network: TestNetwork
+  let agent: AtpAgent
+  let sc: SeedClient
+
+  beforeAll(async () => {
+    network = await TestNetwork.create({
+      dbPostgresSchema: 'bsky_auth',
+    })
+    agent = network.bsky.getClient()
+    sc = network.getSeedClient()
+    await usersSeed(sc)
+    await network.processAll()
+  })
+
+  afterAll(async () => {
+    await network.close()
+  })
+
+  it('handles signing key change for service auth.', async () => {
+    const issuer = sc.dids.alice
+    const attemptWithKey = async (keypair: Keypair) => {
+      const jwt = await createServiceJwt({
+        iss: issuer,
+        aud: network.bsky.ctx.cfg.serverDid,
+        keypair,
+      })
+      return agent.api.app.bsky.actor.getProfile(
+        { actor: sc.dids.carol },
+        { headers: { authorization: `Bearer ${jwt}` } },
+      )
+    }
+    const origSigningKey = network.pds.ctx.repoSigningKey
+    const newSigningKey = await Secp256k1Keypair.create({ exportable: true })
+    // confirm original signing key works
+    await expect(attemptWithKey(origSigningKey)).resolves.toBeDefined()
+    // confirm next signing key doesn't work yet
+    await expect(attemptWithKey(newSigningKey)).rejects.toThrow(
+      'jwt signature does not match jwt issuer',
+    )
+    // update to new signing key
+    await network.plc
+      .getClient()
+      .updateAtprotoKey(
+        issuer,
+        network.pds.ctx.plcRotationKey,
+        newSigningKey.did(),
+      )
+    // old signing key still works due to did doc cache
+    await expect(attemptWithKey(origSigningKey)).resolves.toBeDefined()
+    // new signing key works
+    await expect(attemptWithKey(newSigningKey)).resolves.toBeDefined()
+    // old signing key no longer works after cache is updated
+    await expect(attemptWithKey(origSigningKey)).rejects.toThrow(
+      'jwt signature does not match jwt issuer',
+    )
+  })
+})

packages/crypto/src/p256/operations.ts

@@ -2,24 +2,29 @@ import { p256 } from '@noble/curves/p256'
 import { sha256 } from '@noble/hashes/sha256'
 import { P256_JWT_ALG } from '../const'
 import { parseDidKey } from '../did'
+import { VerifyOptions } from '../types'
 
 export const verifyDidSig = async (
   did: string,
   data: Uint8Array,
   sig: Uint8Array,
+  opts?: VerifyOptions,
 ): Promise<boolean> => {
   const { jwtAlg, keyBytes } = parseDidKey(did)
   if (jwtAlg !== P256_JWT_ALG) {
     throw new Error(`Not a P-256 did:key: ${did}`)
   }
-  return verifySig(keyBytes, data, sig)
+  return verifySig(keyBytes, data, sig, opts)
 }
 
 export const verifySig = async (
   publicKey: Uint8Array,
   data: Uint8Array,
   sig: Uint8Array,
+  opts?: VerifyOptions,
 ): Promise<boolean> => {
   const msgHash = await sha256(data)
-  return p256.verify(sig, msgHash, publicKey, { lowS: true })
+  return p256.verify(sig, msgHash, publicKey, {
+    lowS: opts?.lowS ?? true,
+  })
 }

packages/crypto/src/secp256k1/operations.ts

@@ -2,24 +2,29 @@ import { secp256k1 as k256 } from '@noble/curves/secp256k1'
 import { sha256 } from '@noble/hashes/sha256'
 import { SECP256K1_JWT_ALG } from '../const'
 import { parseDidKey } from '../did'
+import { VerifyOptions } from '../types'
 
 export const verifyDidSig = async (
   did: string,
   data: Uint8Array,
   sig: Uint8Array,
+  opts?: VerifyOptions,
 ): Promise<boolean> => {
   const { jwtAlg, keyBytes } = parseDidKey(did)
   if (jwtAlg !== SECP256K1_JWT_ALG) {
     throw new Error(`Not a secp256k1 did:key: ${did}`)
   }
-  return verifySig(keyBytes, data, sig)
+  return verifySig(keyBytes, data, sig, opts)
 }
 
 export const verifySig = async (
   publicKey: Uint8Array,
   data: Uint8Array,
   sig: Uint8Array,
+  opts?: VerifyOptions,
 ): Promise<boolean> => {
   const msgHash = await sha256(data)
-  return k256.verify(sig, msgHash, publicKey, { lowS: true })
+  return k256.verify(sig, msgHash, publicKey, {
+    lowS: opts?.lowS ?? true,
+  })
 }

packages/crypto/src/types.ts

@@ -16,5 +16,10 @@ export type DidKeyPlugin = {
     did: string,
     msg: Uint8Array,
     data: Uint8Array,
+    opts?: VerifyOptions,
   ) => Promise<boolean>
 }
+
+export type VerifyOptions = {
+  lowS?: boolean
+}

packages/crypto/src/verify.ts

@@ -1,26 +1,29 @@
 import * as uint8arrays from 'uint8arrays'
 import { parseDidKey } from './did'
 import plugins from './plugins'
+import { VerifyOptions } from './types'
 
 export const verifySignature = (
   didKey: string,
   data: Uint8Array,
   sig: Uint8Array,
+  opts?: VerifyOptions,
 ): Promise<boolean> => {
   const parsed = parseDidKey(didKey)
   const plugin = plugins.find((p) => p.jwtAlg === parsed.jwtAlg)
   if (!plugin) {
-    throw new Error(`Unsupported signature alg: :${parsed.jwtAlg}`)
+    throw new Error(`Unsupported signature alg: ${parsed.jwtAlg}`)
   }
-  return plugin.verifySignature(didKey, data, sig)
+  return plugin.verifySignature(didKey, data, sig, opts)
 }
 
 export const verifySignatureUtf8 = async (
   didKey: string,
   data: string,
   sig: string,
+  opts?: VerifyOptions,
 ): Promise<boolean> => {
   const dataBytes = uint8arrays.fromString(data, 'utf8')
   const sigBytes = uint8arrays.fromString(sig, 'base64url')
-  return verifySignature(didKey, dataBytes, sigBytes)
+  return verifySignature(didKey, dataBytes, sigBytes, opts)
 }

packages/crypto/tests/signatures.test.ts

@@ -57,6 +57,45 @@ describe('signatures', () => {
       }
     }
   })
+
+  it('verifies high-s signatures with explicit option', async () => {
+    const highSVectors = vectors.filter((vec) => vec.tags.includes('high-s'))
+    expect(highSVectors.length).toBeGreaterThanOrEqual(2)
+    for (const vector of highSVectors) {
+      const messageBytes = uint8arrays.fromString(
+        vector.messageBase64,
+        'base64',
+      )
+      const signatureBytes = uint8arrays.fromString(
+        vector.signatureBase64,
+        'base64',
+      )
+      const keyBytes = multibaseToBytes(vector.publicKeyMultibase)
+      const didKey = parseDidKey(vector.publicKeyDid)
+      expect(uint8arrays.equals(keyBytes, didKey.keyBytes))
+      if (vector.algorithm === P256_JWT_ALG) {
+        const verified = await p256.verifySig(
+          keyBytes,
+          messageBytes,
+          signatureBytes,
+          { lowS: false },
+        )
+        expect(verified).toEqual(true)
+        expect(vector.validSignature).toEqual(false) // otherwise would fail per low-s requirement
+      } else if (vector.algorithm === SECP256K1_JWT_ALG) {
+        const verified = await secp.verifySig(
+          keyBytes,
+          messageBytes,
+          signatureBytes,
+          { lowS: false },
+        )
+        expect(verified).toEqual(true)
+        expect(vector.validSignature).toEqual(false) // otherwise would fail per low-s requirement
+      } else {
+        throw new Error('Unsupported test vector')
+      }
+    }
+  })
 })
 
 // eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -79,6 +118,7 @@ async function generateTestVectors(): Promise<TestVector[]> {
         'base64',
       ),
       validSignature: true,
+      tags: [],
     },
     {
       messageBase64,
@@ -93,6 +133,7 @@ async function generateTestVectors(): Promise<TestVector[]> {
         'base64',
       ),
       validSignature: true,
+      tags: [],
     },
     // these vectors test to ensure we don't allow high-s signatures
     {
@@ -109,6 +150,7 @@ async function generateTestVectors(): Promise<TestVector[]> {
         P256_JWT_ALG,
       ),
       validSignature: false,
+      tags: ['high-s'],
     },
     {
       messageBase64,
@@ -124,6 +166,7 @@ async function generateTestVectors(): Promise<TestVector[]> {
         SECP256K1_JWT_ALG,
       ),
       validSignature: false,
+      tags: ['high-s'],
     },
   ]
 }
@@ -159,4 +202,5 @@ type TestVector = {
   messageBase64: string
   signatureBase64: string
   validSignature: boolean
+  tags: string[]
 }

packages/xrpc-server/package.json

@@ -45,6 +45,8 @@
     "@types/http-errors": "^2.0.1",
     "@types/ws": "^8.5.4",
     "get-port": "^6.1.2",
+    "jose": "^4.15.4",
+    "key-encoder": "^2.0.3",
     "multiformats": "^9.9.0"
   }
 }

packages/xrpc-server/src/auth.ts

@@ -48,7 +48,7 @@ const jsonToB64Url = (json: Record<string, unknown>): string => {
 export const verifyJwt = async (
   jwtStr: string,
   ownDid: string | null, // null indicates to skip the audience check
-  getSigningKey: (did: string) => Promise<string>,
+  getSigningKey: (did: string, forceRefresh: boolean) => Promise<string>,
 ): Promise<ServiceJwtPayload> => {
   const parts = jwtStr.split('.')
   if (parts.length !== 3) {
@@ -69,18 +69,40 @@ export const verifyJwt = async (
 
   const msgBytes = ui8.fromString(parts.slice(0, 2).join('.'), 'utf8')
   const sigBytes = ui8.fromString(sig, 'base64url')
+  const verifySignatureWithKey = (key: string) => {
+    return crypto.verifySignature(key, msgBytes, sigBytes, {
+      lowS: false,
+    })
+  }
 
-  const signingKey = await getSigningKey(payload.iss)
+  const signingKey = await getSigningKey(payload.iss, false)
 
   let validSig: boolean
   try {
-    validSig = await crypto.verifySignature(signingKey, msgBytes, sigBytes)
+    validSig = await verifySignatureWithKey(signingKey)
   } catch (err) {
     throw new AuthRequiredError(
       'could not verify jwt signature',
       'BadJwtSignature',
     )
   }
+
+  if (!validSig) {
+    // get fresh signing key in case it failed due to a recent rotation
+    const freshSigningKey = await getSigningKey(payload.iss, true)
+    try {
+      validSig =
+        freshSigningKey !== signingKey
+          ? await verifySignatureWithKey(freshSigningKey)
+          : false
+    } catch (err) {
+      throw new AuthRequiredError(
+        'could not verify jwt signature',
+        'BadJwtSignature',
+      )
+    }
+  }
+
   if (!validSig) {
     throw new AuthRequiredError(
       'jwt signature does not match jwt issuer',