From 85784368c3f136e42966f140b5d686fe99126cc2 Mon Sep 17 00:00:00 2001 From: William Calderipe Date: Tue, 26 Mar 2024 18:26:59 +0100 Subject: [PATCH] Verify data stores signatures on fetch (#189) --- .../integration/data-store.service.spec.ts | 242 +++++++++++++----- .../engine/core/service/data-store.service.ts | 94 ++++++- .../core/open-policy-agent.engine.ts | 14 +- .../src/shared/testing/data-store.testing.ts | 26 ++ .../src/lib/schema/entity.schema.ts | 5 +- .../src/lib/util/entity.util.ts | 13 + .../src/lib/__test__/unit/sign.spec.ts | 28 +- 7 files changed, 318 insertions(+), 104 deletions(-) create mode 100644 apps/policy-engine/src/shared/testing/data-store.testing.ts diff --git a/apps/policy-engine/src/engine/core/service/__test__/integration/data-store.service.spec.ts b/apps/policy-engine/src/engine/core/service/__test__/integration/data-store.service.spec.ts index 8ee2229cb..a9ac99316 100644 --- a/apps/policy-engine/src/engine/core/service/__test__/integration/data-store.service.spec.ts +++ b/apps/policy-engine/src/engine/core/service/__test__/integration/data-store.service.spec.ts @@ -1,17 +1,19 @@ import { - Action, - Criterion, EntityData, EntitySignature, + EntityStore, + EntityUtil, FIXTURE, PolicyData, PolicySignature, - Then + PolicyStore } from '@narval/policy-engine-shared' +import { Jwk, secp256k1PrivateKeyToJwk } from '@narval/signature' import { HttpModule } from '@nestjs/axios' import { HttpStatus } from '@nestjs/common' import { Test } from '@nestjs/testing' import nock from 'nock' +import { getEntityStore, getPolicyStore, signData } from '../../../../../shared/testing/data-store.testing' import { withTempJsonFile } from '../../../../../shared/testing/with-temp-json-file.testing' import { FileSystemDataStoreRepository } from '../../../../persistence/repository/file-system-data-store.repository' import { HttpDataStoreRepository } from '../../../../persistence/repository/http-data-store.repository' @@ -19,43 +21,19 @@ import { DataStoreException } from '../../../exception/data-store.exception' import { DataStoreRepositoryFactory } from '../../../factory/data-store-repository.factory' import { DataStoreService } from '../../data-store.service' +const UNSAFE_PRIVATE_KEY = '7cfef3303797cbc7515d9ce22ffe849c701b0f2812f999b0847229c47951fca5' + describe(DataStoreService.name, () => { let service: DataStoreService + let jwk: Jwk + let entityStore: EntityStore + let policyStore: PolicyStore + let entityData: EntityData + let policyData: PolicyData + let signatureStore: EntitySignature & PolicySignature const remoteDataStoreUrl = 'http://9.9.9.9:9000' - const entityData: EntityData = { - entity: { - data: FIXTURE.ENTITIES - } - } - - const policyData: PolicyData = { - policy: { - data: [ - { - then: Then.PERMIT, - name: 'test-policy', - when: [ - { - criterion: Criterion.CHECK_ACTION, - args: [Action.SIGN_TRANSACTION] - } - ] - } - ] - } - } - - const signatureStore: EntitySignature & PolicySignature = { - entity: { - signature: 'test-entity-signature' - }, - policy: { - signature: 'test-policy-signature' - } - } - beforeEach(async () => { const module = await Test.createTestingModule({ imports: [HttpModule], @@ -63,6 +41,32 @@ describe(DataStoreService.name, () => { }).compile() service = module.get(DataStoreService) + + jwk = secp256k1PrivateKeyToJwk(`0x${UNSAFE_PRIVATE_KEY}`) + + entityStore = await getEntityStore(FIXTURE.ENTITIES, jwk) + policyStore = await getPolicyStore(FIXTURE.POLICIES, jwk) + + entityData = { + entity: { + data: entityStore.data + } + } + + policyData = { + policy: { + data: policyStore.data + } + } + + signatureStore = { + entity: { + signature: entityStore.signature + }, + policy: { + signature: policyStore.signature + } + } }) describe('fetch', () => { @@ -76,12 +80,12 @@ describe(DataStoreService.name, () => { entity: { dataUrl: `${remoteDataStoreUrl}/entity`, signatureUrl: url, - keys: [] + keys: [jwk] }, policy: { dataUrl: `${remoteDataStoreUrl}/policy`, signatureUrl: url, - keys: [] + keys: [jwk] } } @@ -99,10 +103,10 @@ describe(DataStoreService.name, () => { }) const testThrowDataStoreException = async (params: { - store: unknown + stores: unknown expect: { message: string; status: HttpStatus } }): Promise => { - await withTempJsonFile(JSON.stringify(params.store), async (path) => { + await withTempJsonFile(JSON.stringify(params.stores), async (path) => { const url = `file://${path}` expect.assertions(3) @@ -112,12 +116,12 @@ describe(DataStoreService.name, () => { entity: { dataUrl: url, signatureUrl: url, - keys: [] + keys: [jwk] }, policy: { dataUrl: url, signatureUrl: url, - keys: [] + keys: [jwk] } }) } catch (error) { @@ -130,15 +134,12 @@ describe(DataStoreService.name, () => { it('throws DataStoreException when entity schema is invalid', async () => { await testThrowDataStoreException({ - store: { + stores: { entity: { data: ['invalid', 'schema'], - signature: 'test-signature' + signature: entityStore.signature }, - policy: { - data: policyData.policy.data, - signature: 'test-signature' - } + policy: policyStore }, expect: { message: 'Invalid store schema', @@ -156,28 +157,26 @@ describe(DataStoreService.name, () => { id: '1' } ] + const entities = { + userGroups: duplicateUserGroups, + addressBook: [], + credentials: [], + tokens: [], + userGroupMembers: [], + userWallets: [], + users: [], + walletGroupMembers: [], + walletGroups: [], + wallets: [] + } await testThrowDataStoreException({ - store: { + stores: { entity: { - data: { - userGroups: duplicateUserGroups, - addressBook: [], - credentials: [], - tokens: [], - userGroupMembers: [], - userWallets: [], - users: [], - walletGroupMembers: [], - walletGroups: [], - wallets: [] - }, - signature: 'test-signature' + data: entities, + signature: await signData(entities, jwk) }, - policy: { - data: policyData.policy.data, - signature: 'test-signature' - } + policy: policyStore }, expect: { message: 'Invalid entity domain invariant', @@ -188,15 +187,12 @@ describe(DataStoreService.name, () => { it('throws DataStoreException when policy schema is invalid', async () => { await testThrowDataStoreException({ - store: { + stores: { policy: { data: { invalid: 'schema' }, - signature: 'test-signature' + signature: policyStore.signature }, - entity: { - data: FIXTURE.ENTITIES, - signature: 'test-signature' - } + entity: entityStore }, expect: { message: 'Invalid store schema', @@ -204,5 +200,109 @@ describe(DataStoreService.name, () => { } }) }) + + it('throws DataStoreException when entity signature is invalid', async () => { + const entityStoreOne = await getEntityStore(FIXTURE.ENTITIES, jwk) + const entityStoreTwo = await getEntityStore(EntityUtil.empty(), jwk) + + await testThrowDataStoreException({ + stores: { + entity: { + data: entityStoreOne.data, + signature: entityStoreTwo.signature + }, + policy: policyStore + }, + expect: { + message: 'Data signature mismatch', + status: HttpStatus.UNAUTHORIZED + } + }) + }) + + it('throws DataStoreException when policy signature is invalid', async () => { + const policyStoreOne = await getPolicyStore(FIXTURE.POLICIES, jwk) + const policyStoreTwo = await getPolicyStore([], jwk) + + await testThrowDataStoreException({ + stores: { + policy: { + data: policyStoreOne.data, + signature: policyStoreTwo.signature + }, + entity: entityStore + }, + expect: { + message: 'Data signature mismatch', + status: HttpStatus.UNAUTHORIZED + } + }) + }) + }) + + describe('verifySignature', () => { + // WARN: The Jest error equality doesn't check custom properties of errors + // like `suggestedHttpStatusCode` from ApplicationException. + + it('returns error when jwk is not found', async () => { + const verification = await service.verifySignature({ + data: entityStore.data, + signature: entityStore.signature, + keys: [] + }) + + expect(verification).toEqual({ + success: false, + error: new DataStoreException({ + message: 'JWK not found', + suggestedHttpStatusCode: HttpStatus.NOT_FOUND + }) + }) + }) + + it('returns error when signature mismatch', async () => { + const entityStoreOne = await getEntityStore(FIXTURE.ENTITIES, jwk) + const entityStoreTwo = await getEntityStore(EntityUtil.empty(), jwk) + + const verification = await service.verifySignature({ + data: entityStoreOne.data, + signature: entityStoreTwo.signature, + keys: [jwk] + }) + + expect(verification).toEqual({ + success: false, + error: new DataStoreException({ + message: 'Data signature mismatch', + suggestedHttpStatusCode: HttpStatus.UNAUTHORIZED + }) + }) + }) + + it('returns error when jwt verification fails', async () => { + const verification = await service.verifySignature({ + data: entityStore.data, + signature: 'invalid-signature', + keys: [jwk] + }) + + expect(verification).toEqual({ + success: false, + error: new DataStoreException({ + message: 'Invalid signature', + suggestedHttpStatusCode: HttpStatus.UNAUTHORIZED + }) + }) + }) + + it('returns success when signature is valid', async () => { + const verification = await service.verifySignature({ + data: entityStore.data, + signature: entityStore.signature, + keys: [jwk] + }) + + expect(verification).toEqual({ success: true }) + }) }) }) diff --git a/apps/policy-engine/src/engine/core/service/data-store.service.ts b/apps/policy-engine/src/engine/core/service/data-store.service.ts index ba9cb0c4c..63d2e8cce 100644 --- a/apps/policy-engine/src/engine/core/service/data-store.service.ts +++ b/apps/policy-engine/src/engine/core/service/data-store.service.ts @@ -1,14 +1,18 @@ import { DataStoreConfiguration, + Entities, EntityStore, EntityUtil, + Policy, PolicyStore, entityDataSchema, entitySignatureSchema, policyDataSchema, policySignatureSchema } from '@narval/policy-engine-shared' +import { Jwk, decode, hash, verifyJwt } from '@narval/signature' import { HttpStatus, Injectable } from '@nestjs/common' +import { JwtError } from 'packages/signature/src/lib/error' import { ZodObject, z } from 'zod' import { DataStoreException } from '../exception/data-store.exception' import { DataStoreRepositoryFactory } from '../factory/data-store-repository.factory' @@ -41,10 +45,20 @@ export class DataStoreService { const validation = EntityUtil.validate(entityData.entity.data) if (validation.success) { - return { + const signatureVerification = await this.verifySignature({ data: entityData.entity.data, - signature: entitySignature.entity.signature + signature: entitySignature.entity.signature, + keys: store.keys + }) + + if (signatureVerification.success) { + return { + data: entityData.entity.data, + signature: entitySignature.entity.signature + } } + + throw signatureVerification.error } throw new DataStoreException({ @@ -52,7 +66,7 @@ export class DataStoreService { suggestedHttpStatusCode: HttpStatus.UNPROCESSABLE_ENTITY, context: { url: store.dataUrl, - errors: validation.issues + errors: validation.success ? {} : validation.issues } }) } @@ -63,10 +77,20 @@ export class DataStoreService { this.fetchByUrl(store.signatureUrl, policySignatureSchema) ]) - return { + const signatureVerification = await this.verifySignature({ data: policyData.policy.data, - signature: policySignature.policy.signature + signature: policySignature.policy.signature, + keys: store.keys + }) + + if (signatureVerification.success) { + return { + data: policyData.policy.data, + signature: policySignature.policy.signature + } } + + throw signatureVerification.error } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -95,4 +119,64 @@ export class DataStoreService { } }) } + + async verifySignature(params: { + data: Entities | Policy[] + signature: string + keys: Jwk[] + }): Promise<{ success: true } | { success: false; error: DataStoreException }> { + try { + const jwt = decode(params.signature) + const jwk = params.keys.find(({ kid }) => kid === jwt.header.kid) + + if (!jwk) { + return { + success: false, + error: new DataStoreException({ + message: 'JWK not found', + suggestedHttpStatusCode: HttpStatus.NOT_FOUND, + context: { + kid: jwt.header.kid + } + }) + } + } + + const verification = await verifyJwt(params.signature, jwk) + + if (verification.payload.requestHash !== hash(params.data)) { + return { + success: false, + error: new DataStoreException({ + message: 'Data signature mismatch', + suggestedHttpStatusCode: HttpStatus.UNAUTHORIZED + }) + } + } + } catch (error) { + if (error instanceof JwtError) { + return { + success: false, + error: new DataStoreException({ + message: 'Invalid signature', + suggestedHttpStatusCode: HttpStatus.UNAUTHORIZED, + origin: error + }) + } + } + + return { + success: false, + error: new DataStoreException({ + message: 'Unknown error', + suggestedHttpStatusCode: HttpStatus.INTERNAL_SERVER_ERROR, + origin: error + }) + } + } + + return { + success: true + } + } } diff --git a/apps/policy-engine/src/open-policy-agent/core/open-policy-agent.engine.ts b/apps/policy-engine/src/open-policy-agent/core/open-policy-agent.engine.ts index 97e0f1863..595da639c 100644 --- a/apps/policy-engine/src/open-policy-agent/core/open-policy-agent.engine.ts +++ b/apps/policy-engine/src/open-policy-agent/core/open-policy-agent.engine.ts @@ -5,6 +5,7 @@ import { Decision, Engine, Entities, + EntityUtil, EvaluationRequest, EvaluationResponse, JwtString, @@ -61,18 +62,7 @@ export class OpenPolicyAgentEngine implements Engine { static empty(params: { resourcePath: string; privateKey: Hex }): OpenPolicyAgentEngine { return new OpenPolicyAgentEngine({ - entities: { - addressBook: [], - credentials: [], - tokens: [], - userGroupMembers: [], - userGroups: [], - userWallets: [], - users: [], - walletGroupMembers: [], - walletGroups: [], - wallets: [] - }, + entities: EntityUtil.empty(), policies: [], resourcePath: params.resourcePath, privateKey: params.privateKey diff --git a/apps/policy-engine/src/shared/testing/data-store.testing.ts b/apps/policy-engine/src/shared/testing/data-store.testing.ts new file mode 100644 index 000000000..ff7dbdd26 --- /dev/null +++ b/apps/policy-engine/src/shared/testing/data-store.testing.ts @@ -0,0 +1,26 @@ +import { Entities, EntityStore, Policy, PolicyStore } from '@narval/policy-engine-shared' +import { Jwk, hash, signJwt } from '@narval/signature' +import { getUnixTime } from 'date-fns' + +export const signData = (data: Entities | Policy[], jwk: Jwk) => + signJwt( + { + requestHash: hash(data), + iat: getUnixTime(new Date()) + }, + jwk + ) + +export const getEntityStore = async (entities: Entities, jwk: Jwk): Promise => { + return { + data: entities, + signature: await signData(entities, jwk) + } +} + +export const getPolicyStore = async (policies: Policy[], jwk: Jwk): Promise => { + return { + data: policies, + signature: await signData(policies, jwk) + } +} diff --git a/packages/policy-engine-shared/src/lib/schema/entity.schema.ts b/packages/policy-engine-shared/src/lib/schema/entity.schema.ts index ce1a20124..64512546e 100644 --- a/packages/policy-engine-shared/src/lib/schema/entity.schema.ts +++ b/packages/policy-engine-shared/src/lib/schema/entity.schema.ts @@ -1,5 +1,6 @@ import { publicKeySchema } from '@narval/signature' import { z } from 'zod' +import { accountIdSchema, assetIdSchema } from '../util/caip.util' import { addressSchema } from './address.schema' export const userRoleSchema = z.nativeEnum({ @@ -68,14 +69,14 @@ export const walletGroupMemberEntitySchema = z.object({ }) export const addressBookAccountEntitySchema = z.object({ - id: z.string(), + id: accountIdSchema, address: addressSchema, chainId: z.number(), classification: accountClassificationSchema }) export const tokenEntitySchema = z.object({ - id: z.string(), + id: assetIdSchema, address: addressSchema, symbol: z.string().nullable(), chainId: z.number(), diff --git a/packages/policy-engine-shared/src/lib/util/entity.util.ts b/packages/policy-engine-shared/src/lib/util/entity.util.ts index 3919e7fa2..3fd96f933 100644 --- a/packages/policy-engine-shared/src/lib/util/entity.util.ts +++ b/packages/policy-engine-shared/src/lib/util/entity.util.ts @@ -127,3 +127,16 @@ export const validate = (entities: Entities, options?: ValidationOption): Valida success: true } } + +export const empty = (): Entities => ({ + addressBook: [], + credentials: [], + tokens: [], + userGroupMembers: [], + userGroups: [], + userWallets: [], + users: [], + walletGroupMembers: [], + walletGroups: [], + wallets: [] +}) diff --git a/packages/signature/src/lib/__test__/unit/sign.spec.ts b/packages/signature/src/lib/__test__/unit/sign.spec.ts index 9c9dd8037..8c9c762a3 100644 --- a/packages/signature/src/lib/__test__/unit/sign.spec.ts +++ b/packages/signature/src/lib/__test__/unit/sign.spec.ts @@ -18,7 +18,7 @@ import { verifyJwt } from '../../verify' import { HEADER_PART, PAYLOAD_PART, PRIVATE_KEY_PEM } from './mock' describe('sign', () => { - const ENGINE_PRIVATE_KEY = '7cfef3303797cbc7515d9ce22ffe849c701b0f2812f999b0847229c47951fca5' + const UNSAFE_PRIVATE_KEY = '7cfef3303797cbc7515d9ce22ffe849c701b0f2812f999b0847229c47951fca5' const payload: Payload = { requestHash: '608abe908cffeab1fc33edde6b44586f9dacbc9c6fe6f0a13fa307237290ce5a', @@ -52,8 +52,8 @@ describe('sign', () => { }) it('should build & sign a EIP191 JWT', async () => { - const jwk = secp256k1PrivateKeyToJwk(`0x${ENGINE_PRIVATE_KEY}`) - const signer = buildSignerEip191(ENGINE_PRIVATE_KEY) + const jwk = secp256k1PrivateKeyToJwk(`0x${UNSAFE_PRIVATE_KEY}`) + const signer = buildSignerEip191(UNSAFE_PRIVATE_KEY) const jwt = await signJwt(payload, jwk, { alg: SigningAlg.EIP191 }, signer) @@ -78,9 +78,9 @@ describe('sign', () => { }) it('should sign ES256k correctly', async () => { - const pubkey = secp256k1.getPublicKey(Buffer.from(ENGINE_PRIVATE_KEY, 'hex'), false) + const pubkey = secp256k1.getPublicKey(Buffer.from(UNSAFE_PRIVATE_KEY, 'hex'), false) const message = [HEADER_PART, PAYLOAD_PART].join('.') - const signer = buildSignerEs256k(ENGINE_PRIVATE_KEY) + const signer = buildSignerEs256k(UNSAFE_PRIVATE_KEY) const signature = await signer(message) const msgHash = sha256Hash(message) @@ -92,7 +92,7 @@ describe('sign', () => { it('should sign EIP191 correctly', async () => { const message = [HEADER_PART, PAYLOAD_PART].join('.') - const signer = buildSignerEip191(ENGINE_PRIVATE_KEY) + const signer = buildSignerEip191(UNSAFE_PRIVATE_KEY) const signature = await signer(message) expect(signature).toBe('afu_-8eYXRpHAt_nTVRksRmiwVZpq7iC2rBVhGQT5YcJlViKV9wD3OIlRYAxa7JkNd1Yqzf_x2ohLzqGjmlb2hs') @@ -101,12 +101,12 @@ describe('sign', () => { it('should sign EIP191 the same as viem', async () => { // Just double-check that we're building a signature & base64url encoding the same thing we'd get from Viem. const message = [HEADER_PART, PAYLOAD_PART].join('.') - const signer = buildSignerEip191(ENGINE_PRIVATE_KEY) + const signer = buildSignerEip191(UNSAFE_PRIVATE_KEY) const signature = await signer(message) const viemSig = await signMessage({ message, - privateKey: `0x${ENGINE_PRIVATE_KEY}` + privateKey: `0x${UNSAFE_PRIVATE_KEY}` }) const sigHex = base64UrlToHex(signature) @@ -149,11 +149,11 @@ describe('sign', () => { // This is testing that we can turn a private key into a JWK, and the way we know it's a "correct" JWK is by using the `createPublicKey` function from node's crypto module. If it throws an error, that means it's not a valid JWK it('should make keyobject', async () => { - const publicKey = secp256k1.getPublicKey(ENGINE_PRIVATE_KEY, false) - const viemPubKey = privateKeyToAccount(`0x${ENGINE_PRIVATE_KEY}`).publicKey + const publicKey = secp256k1.getPublicKey(UNSAFE_PRIVATE_KEY, false) + const viemPubKey = privateKeyToAccount(`0x${UNSAFE_PRIVATE_KEY}`).publicKey expect(toHex(publicKey)).toBe(viemPubKey) // Confirm that our key is in fact the same as what viem would give. - const jwk = secp256k1PrivateKeyToJwk(`0x${ENGINE_PRIVATE_KEY}`) + const jwk = secp256k1PrivateKeyToJwk(`0x${UNSAFE_PRIVATE_KEY}`) const k = await createPublicKey({ format: 'jwk', @@ -164,13 +164,13 @@ describe('sign', () => { }) it('should convert to and from jwk', async () => { - const jwk = secp256k1PrivateKeyToJwk(`0x${ENGINE_PRIVATE_KEY}`) + const jwk = secp256k1PrivateKeyToJwk(`0x${UNSAFE_PRIVATE_KEY}`) const pk = secp256k1PrivateKeyToHex(jwk) - expect(pk).toBe(`0x${ENGINE_PRIVATE_KEY}`) + expect(pk).toBe(`0x${UNSAFE_PRIVATE_KEY}`) }) it('should convert to and from public jwk', async () => { - const publicKey = secp256k1.getPublicKey(ENGINE_PRIVATE_KEY, false) + const publicKey = secp256k1.getPublicKey(UNSAFE_PRIVATE_KEY, false) const jwk = secp256k1PublicKeyToJwk(toHex(publicKey)) const pk = secp256k1PublicKeyToHex(jwk) expect(pk).toBe(toHex(publicKey))