Service auth method binding (lxm) (#2663)

* add scopes to service auth impl * add error to getServiceAuth * send scoped tokens from pds * clean up privileged access scopes & allow simple service auth tokens for app passwords * integration into ozone * fix up bsky tests * cleanup xrpc-server tests * fix up tests & types * one more test * fix read after write tests * fix mod auth test * convert scopes to be a single method name * add scope check callback for auth verifier * pds changes only * fix feed generation tests * use scope for ozone service profile * dont verify scopes on pds yet * tidy * tidy imports * changeset * add tests * tidy * another changeset * scope -> lxm * tidy * clean up scope references * update nonce size * pr feedback * trim trailing slash * nonce -> jti * fix xrpc-server test * allow service auth on uploadBlob * fix build error * changeset * build, tidy * xrpc-server: update lxm claim check error * appview: temporarily permit labeler service calls to omit lxm claim * xrpc-server: fix test * changeset * fix merged tests --------- Co-authored-by: Devin Ivy <[email protected]>
bluesky-social · Aug 18, 2024 · 50c0ec1 · 50c0ec1
1 parent 2c65cb9
commit 50c0ec1
Show file tree

Hide file tree

Showing 77 changed files with 2,388 additions and 575 deletions.
diff --git a/.changeset/quick-rules-film.md b/.changeset/quick-rules-film.md
@@ -0,0 +1,5 @@
+---
+"@atproto/xrpc-server": patch
+---
+
+Update lxm check error name to BadJwtLexiconMethod
diff --git a/.changeset/red-dolls-marry.md b/.changeset/red-dolls-marry.md
@@ -0,0 +1,5 @@
+---
+"@atproto/xrpc-server": patch
+---
+
+Support http.IncomingMessage input to parseReqNsid()
diff --git a/.changeset/silver-beers-buy.md b/.changeset/silver-beers-buy.md
@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Validate lxm claims in service auth
diff --git a/.changeset/slow-dolls-rhyme.md b/.changeset/slow-dolls-rhyme.md
@@ -0,0 +1,5 @@
+---
+"@atproto/ozone": patch
+---
+
+Set lxm claim on service auth calls from ozone
diff --git a/.github/workflows/build-and-push-bsky-ghcr.yaml b/.github/workflows/build-and-push-bsky-ghcr.yaml
@@ -3,7 +3,7 @@ on:
   push:
     branches:
       - main
-      - divy/mod-full-thread
+      - service-auth-scopes
 env:
   REGISTRY: ghcr.io
   USERNAME: ${{ github.actor }}

diff --git a/.github/workflows/build-and-push-ozone-aws.yaml b/.github/workflows/build-and-push-ozone-aws.yaml
@@ -3,6 +3,7 @@ on:
   push:
     branches:
       - main
+      - service-auth-scopes
 env:
   REGISTRY: ${{ secrets.AWS_ECR_REGISTRY_USEAST2_PACKAGES_REGISTRY }}
   USERNAME: ${{ secrets.AWS_ECR_REGISTRY_USEAST2_PACKAGES_USERNAME }}

diff --git a/.github/workflows/build-and-push-pds-ghcr.yaml b/.github/workflows/build-and-push-pds-ghcr.yaml
@@ -3,7 +3,7 @@ on:
   push:
     branches:
       - main
-      - divy/starter-packs
+      - service-auth-scopes
 env:
   REGISTRY: ghcr.io
   USERNAME: ${{ github.actor }}

diff --git a/packages/bsky/src/api/app/bsky/actor/getProfiles.ts b/packages/bsky/src/api/app/bsky/actor/getProfiles.ts
@@ -10,11 +10,20 @@ import {
   Hydrator,
 } from '../../../../hydration/hydrator'
 import { Views } from '../../../../views'
+import { ids } from '../../../../lexicon/lexicons'
 
 export default function (server: Server, ctx: AppContext) {
   const getProfile = createPipeline(skeleton, hydration, noRules, presentation)
   server.app.bsky.actor.getProfiles({
-    auth: ctx.authVerifier.standardOptional,
+    auth: ctx.authVerifier.standardOptionalParameterized({
+      lxmCheck: (method) => {
+        if (!method) return false
+        return (
+          method === ids.AppBskyActorGetProfiles ||
+          method.startsWith('chat.bsky.')
+        )
+      },
+    }),
     handler: async ({ auth, params, req }) => {
       const viewer = auth.credentials.iss
       const labelers = ctx.reqLabelers(req)

diff --git a/packages/bsky/src/api/app/bsky/feed/getFeed.ts b/packages/bsky/src/api/app/bsky/feed/getFeed.ts
@@ -29,6 +29,7 @@ import {
   unpackIdentityServices,
 } from '../../../../data-plane'
 import { resHeaders } from '../../../util'
+import { ids } from '../../../../lexicon/lexicons'
 
 export default function (server: Server, ctx: AppContext) {
   const getFeed = createPipeline(
@@ -38,7 +39,17 @@ export default function (server: Server, ctx: AppContext) {
     presentation,
   )
   server.app.bsky.feed.getFeed({
-    auth: ctx.authVerifier.standardOptionalAnyAud,
+    auth: ctx.authVerifier.standardOptionalParameterized({
+      lxmCheck: (method) => {
+        return (
+          method !== undefined &&
+          [ids.AppBskyFeedGetFeedSkeleton, ids.AppBskyFeedGetFeed].includes(
+            method,
+          )
+        )
+      },
+      skipAudCheck: true,
+    }),
     handler: async ({ params, auth, req }) => {
       const viewer = auth.credentials.iss
       const labelers = ctx.reqLabelers(req)

diff --git a/packages/bsky/src/api/app/bsky/feed/getPosts.ts b/packages/bsky/src/api/app/bsky/feed/getPosts.ts
@@ -11,11 +11,19 @@ import {
 import { Views } from '../../../../views'
 import { creatorFromUri } from '../../../../views/util'
 import { resHeaders } from '../../../util'
+import { ids } from '../../../../lexicon/lexicons'
 
 export default function (server: Server, ctx: AppContext) {
   const getPosts = createPipeline(skeleton, hydration, noBlocks, presentation)
   server.app.bsky.feed.getPosts({
-    auth: ctx.authVerifier.standardOptional,
+    auth: ctx.authVerifier.standardOptionalParameterized({
+      lxmCheck: (method) => {
+        if (!method) return false
+        return (
+          method === ids.AppBskyFeedGetPosts || method.startsWith('chat.bsky.')
+        )
+      },
+    }),
     handler: async ({ params, auth, req }) => {
       const viewer = auth.credentials.iss
       const labelers = ctx.reqLabelers(req)

diff --git a/packages/bsky/src/auth-verifier.ts b/packages/bsky/src/auth-verifier.ts
@@ -1,5 +1,6 @@
 import {
   AuthRequiredError,
+  parseReqNsid,
   verifyJwt as verifyServiceJwt,
 } from '@atproto/xrpc-server'
 import * as ui8 from 'uint8arrays'
@@ -17,6 +18,11 @@ type ReqCtx = {
   req: express.Request
 }
 
+type StandardAuthOpts = {
+  skipAudCheck?: boolean
+  lxmCheck?: (method?: string) => boolean
+}
+
 export enum RoleStatus {
   Valid,
   Invalid,
@@ -80,61 +86,55 @@ export class AuthVerifier {
   }
 
   // verifiers (arrow fns to preserve scope)
-
-  standard = async (ctx: ReqCtx): Promise<StandardOutput> => {
-    // @TODO remove! basic auth + did supported just for testing.
-    if (isBasicToken(ctx.req)) {
-      const aud = this.ownDid
-      const iss = ctx.req.headers['appview-as-did']
-      if (typeof iss !== 'string' || !iss.startsWith('did:')) {
-        throw new AuthRequiredError('bad issuer')
-      }
-      if (!this.parseRoleCreds(ctx.req).admin) {
-        throw new AuthRequiredError('bad credentials')
-      }
-      return {
-        credentials: { type: 'standard', iss, aud },
+  standardOptionalParameterized =
+    (opts: StandardAuthOpts) =>
+    async (ctx: ReqCtx): Promise<StandardOutput | NullOutput> => {
+      // @TODO remove! basic auth + did supported just for testing.
+      if (isBasicToken(ctx.req)) {
+        const aud = this.ownDid
+        const iss = ctx.req.headers['appview-as-did']
+        if (typeof iss !== 'string' || !iss.startsWith('did:')) {
+          throw new AuthRequiredError('bad issuer')
+        }
+        if (!this.parseRoleCreds(ctx.req).admin) {
+          throw new AuthRequiredError('bad credentials')
+        }
+        return {
+          credentials: { type: 'standard', iss, aud },
+        }
+      } else if (isBearerToken(ctx.req)) {
+        const { iss, aud } = await this.verifyServiceJwt(ctx, {
+          lxmCheck: opts.lxmCheck,
+          iss: null,
+          aud: null,
+        })
+        if (!opts.skipAudCheck && !this.standardAudienceDids.has(aud)) {
+          throw new AuthRequiredError(
+            'jwt audience does not match service did',
+            'BadJwtAudience',
+          )
+        }
+        return {
+          credentials: {
+            type: 'standard',
+            iss,
+            aud,
+          },
+        }
+      } else {
+        return this.nullCreds()
       }
     }
-    const { iss, aud } = await this.verifyServiceJwt(ctx, {
-      aud: null,
-      iss: null,
-    })
-    if (!this.standardAudienceDids.has(aud)) {
-      throw new AuthRequiredError(
-        'jwt audience does not match service did',
-        'BadJwtAudience',
-      )
-    }
-    return {
-      credentials: {
-        type: 'standard',
-        iss,
-        aud,
-      },
-    }
-  }
 
-  standardOptional = async (
-    ctx: ReqCtx,
-  ): Promise<StandardOutput | NullOutput> => {
-    if (isBearerToken(ctx.req) || isBasicToken(ctx.req)) {
-      return this.standard(ctx)
-    }
-    return this.nullCreds()
-  }
+  standardOptional: (ctx: ReqCtx) => Promise<StandardOutput | NullOutput> =
+    this.standardOptionalParameterized({})
 
-  standardOptionalAnyAud = async (
-    ctx: ReqCtx,
-  ): Promise<StandardOutput | NullOutput> => {
-    if (!isBearerToken(ctx.req)) {
-      return this.nullCreds()
+  standard = async (ctx: ReqCtx): Promise<StandardOutput> => {
+    const output = await this.standardOptional(ctx)
+    if (output.credentials.type === 'none') {
+      throw new AuthRequiredError(undefined, 'AuthMissing')
     }
-    const { iss, aud } = await this.verifyServiceJwt(ctx, {
-      aud: null,
-      iss: null,
-    })
-    return { credentials: { type: 'standard', iss, aud } }
+    return output as StandardOutput
   }
 
   role = (ctx: ReqCtx): RoleOutput => {
@@ -215,7 +215,11 @@ export class AuthVerifier {
 
   async verifyServiceJwt(
     reqCtx: ReqCtx,
-    opts: { aud: string | null; iss: string[] | null },
+    opts: {
+      iss: string[] | null
+      aud: string | null
+      lxmCheck?: (method?: string) => boolean
+    },
   ) {
     const getSigningKey = async (
       iss: string,
@@ -243,17 +247,40 @@ export class AuthVerifier {
       }
       return didKey
     }
+    const assertLxmCheck = () => {
+      const lxm = parseReqNsid(reqCtx.req)
+      if (
+        (opts.lxmCheck && !opts.lxmCheck(payload.lxm)) ||
+        (!opts.lxmCheck && payload.lxm !== lxm)
+      ) {
+        throw new AuthRequiredError(
+          payload.lxm !== undefined
+            ? `bad jwt lexicon method ("lxm"). must match: ${lxm}`
+            : `missing jwt lexicon method ("lxm"). must match: ${lxm}`,
+          'BadJwtLexiconMethod',
+        )
+      }
+    }
 
     const jwtStr = bearerTokenFromReq(reqCtx.req)
     if (!jwtStr) {
       throw new AuthRequiredError('missing jwt', 'MissingJwt')
     }
+    // if validating additional scopes, skip scope check in initial validation & follow up afterwards
     const payload = await verifyServiceJwt(
       jwtStr,
       opts.aud,
       null,
       getSigningKey,
     )
+    if (
+      !payload.iss.endsWith('#atproto_labeler') ||
+      payload.lxm !== undefined
+    ) {
+      // @TODO currently permissive of labelers who dont set lxm yet.
+      // we'll allow ozone self-hosters to upgrade before removing this condition.
+      assertLxmCheck()
+    }
     return { iss: payload.iss, aud: payload.aud }
   }
 

diff --git a/packages/bsky/tests/admin/admin-auth.test.ts b/packages/bsky/tests/admin/admin-auth.test.ts
@@ -3,6 +3,7 @@ import { AtpAgent } from '@atproto/api'
 import { Secp256k1Keypair } from '@atproto/crypto'
 import { createServiceAuthHeaders } from '@atproto/xrpc-server'
 import { RepoRef } from '../../src/lexicon/types/com/atproto/admin/defs'
+import { ids } from '../../src/lexicon/lexicons'
 
 describe('admin auth', () => {
   let network: TestNetwork
@@ -68,10 +69,10 @@ describe('admin auth', () => {
   })
 
   it('allows service auth requests from the configured appview did', async () => {
-    const headers = await createServiceAuthHeaders({
+    const updateHeaders = await createServiceAuthHeaders({
       iss: modServiceDid,
       aud: bskyDid,
-      lxm: null,
+      lxm: ids.ComAtprotoAdminUpdateSubjectStatus,
       keypair: modServiceKey,
     })
     await agent.api.com.atproto.admin.updateSubjectStatus(
@@ -80,14 +81,20 @@ describe('admin auth', () => {
         takedown: { applied: true, ref: 'test-repo' },
       },
       {
-        ...headers,
+        ...updateHeaders,
         encoding: 'application/json',
       },
     )
 
+    const getHeaders = await createServiceAuthHeaders({
+      iss: modServiceDid,
+      aud: bskyDid,
+      lxm: ids.ComAtprotoAdminGetSubjectStatus,
+      keypair: modServiceKey,
+    })
     const res = await agent.api.com.atproto.admin.getSubjectStatus(
       { did: repoSubject.did },
-      headers,
+      getHeaders,
     )
     expect(res.data.subject.did).toBe(repoSubject.did)
     expect(res.data.takedown?.applied).toBe(true)
@@ -97,7 +104,7 @@ describe('admin auth', () => {
     const headers = await createServiceAuthHeaders({
       iss: altModDid,
       aud: bskyDid,
-      lxm: null,
+      lxm: ids.ComAtprotoAdminUpdateSubjectStatus,
       keypair: modServiceKey,
     })
     const attempt = agent.api.com.atproto.admin.updateSubjectStatus(
@@ -118,7 +125,7 @@ describe('admin auth', () => {
     const headers = await createServiceAuthHeaders({
       iss: sc.dids.alice,
       aud: bskyDid,
-      lxm: null,
+      lxm: ids.ComAtprotoAdminUpdateSubjectStatus,
       keypair: aliceKey,
     })
     const attempt = agent.api.com.atproto.admin.updateSubjectStatus(
@@ -139,7 +146,7 @@ describe('admin auth', () => {
     const headers = await createServiceAuthHeaders({
       iss: modServiceDid,
       aud: bskyDid,
-      lxm: null,
+      lxm: ids.ComAtprotoAdminUpdateSubjectStatus,
       keypair: badKey,
     })
     const attempt = agent.api.com.atproto.admin.updateSubjectStatus(
@@ -162,7 +169,7 @@ describe('admin auth', () => {
     const headers = await createServiceAuthHeaders({
       iss: modServiceDid,
       aud: sc.dids.alice,
-      lxm: null,
+      lxm: ids.ComAtprotoAdminUpdateSubjectStatus,
       keypair: modServiceKey,
     })
     const attempt = agent.api.com.atproto.admin.updateSubjectStatus(

diff --git a/packages/bsky/tests/auth.test.ts b/packages/bsky/tests/auth.test.ts
@@ -2,6 +2,7 @@ import { AtpAgent } from '@atproto/api'
 import { SeedClient, TestNetwork, usersSeed } from '@atproto/dev-env'
 import { createServiceJwt } from '@atproto/xrpc-server'
 import { Keypair, Secp256k1Keypair } from '@atproto/crypto'
+import { ids } from '../src/lexicon/lexicons'
 
 describe('auth', () => {
   let network: TestNetwork
@@ -29,7 +30,7 @@ describe('auth', () => {
       const jwt = await createServiceJwt({
         iss: issuer,
         aud: network.bsky.ctx.cfg.serverDid,
-        lxm: null,
+        lxm: ids.AppBskyActorGetProfile,
         keypair,
       })
       return agent.api.app.bsky.actor.getProfile(