From f8e93468b8c41840909b0db0cebf4abf3a097b17 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Tue, 6 Feb 2024 13:27:12 -0800 Subject: [PATCH 01/15] chore: cleaning up linting errors and logs --- packages/agent/src/agent/http/http.test.ts | 3 ++ packages/agent/src/agent/http/index.ts | 4 +-- .../agent/src/canisterStatus/index.test.ts | 9 +++--- packages/agent/src/certificate.test.ts | 29 +++++++++++-------- packages/agent/test-setup.ts | 11 +++++++ packages/auth-client/test-setup.ts | 10 ++----- 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index 49d76db26..ca662f586 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -34,6 +34,8 @@ function createIdentity(seed: number): Ed25519KeyIdentity { const originalDateNowFn = global.Date.now; const originalWindow = global.window; const originalFetch = global.fetch; + + beforeEach(() => { global.Date.now = jest.fn(() => new Date(NANOSECONDS_PER_MILLISECONDS).getTime()); Object.assign(global, 'window', { @@ -45,6 +47,7 @@ beforeEach(() => { global.fetch = originalFetch; }); + afterEach(() => { global.Date.now = originalDateNowFn; global.window = originalWindow; diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 36b6df53f..66a4600f1 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -704,7 +704,7 @@ export class HttpAgent implements Agent { const replicaTime = status.get('time'); if (replicaTime) { - this._timeDiffMsecs = Number(replicaTime as any) - Number(callTime); + this._timeDiffMsecs = Number(replicaTime as bigint) - Number(callTime); } } catch (error) { console.error('Caught exception while attempting to sync time:', error); @@ -728,7 +728,7 @@ export class HttpAgent implements Agent { public async fetchRootKey(): Promise { if (!this._rootKeyFetched) { // Hex-encoded version of the replica root key - this.rootKey = ((await this.status()) as any).root_key; + this.rootKey = ((await this.status()) as JsonObject & { root_key: ArrayBuffer }).root_key; this._rootKeyFetched = true; } return this.rootKey; diff --git a/packages/agent/src/canisterStatus/index.test.ts b/packages/agent/src/canisterStatus/index.test.ts index 27eb00a45..340966767 100644 --- a/packages/agent/src/canisterStatus/index.test.ts +++ b/packages/agent/src/canisterStatus/index.test.ts @@ -52,6 +52,7 @@ const testCases = [ ]; // Used for repopulating the certificate +// eslint-disable-next-line @typescript-eslint/no-unused-vars const getRealStatus = async () => { const identity = (await Ed25519KeyIdentity.generate( new Uint8Array( @@ -206,7 +207,7 @@ describe('node keys', () => { const { mainnetApplication } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T19:38:58.129Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(mainnetApplication), canisterId: Principal.fromText('erxue-5aaaa-aaaab-qaagq-cai'), rootKey: fromHex(IC_ROOT_KEY), @@ -224,7 +225,7 @@ describe('node keys', () => { const { mainnetSystem } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T19:58:19.412Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(mainnetSystem), canisterId: Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'), rootKey: fromHex(IC_ROOT_KEY), @@ -242,7 +243,7 @@ describe('node keys', () => { const { localApplication } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T20:14:59.406Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(localApplication), canisterId: Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'), rootKey: fromHex(IC_ROOT_KEY), @@ -260,7 +261,7 @@ describe('node keys', () => { const { localSystem } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T20:15:03.406Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(localSystem), canisterId: Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'), rootKey: fromHex(IC_ROOT_KEY), diff --git a/packages/agent/src/certificate.test.ts b/packages/agent/src/certificate.test.ts index 2ff710e62..d0970709b 100644 --- a/packages/agent/src/certificate.test.ts +++ b/packages/agent/src/certificate.test.ts @@ -10,6 +10,7 @@ import { Principal } from '@dfinity/principal'; import { decodeTime } from './utils/leb'; import { lookupResultToBuffer, lookup_path } from './certificate'; import { readFileSync } from 'fs'; +import path from 'path'; function label(str: string): ArrayBuffer { return new TextEncoder().encode(str); @@ -120,7 +121,7 @@ test('lookup', () => { function toText(buff: ArrayBuffer): string { const decoder = new TextDecoder(); - let t = decoder.decode(buff); + const t = decoder.decode(buff); return t; } function fromText(str: string): ArrayBuffer { @@ -144,7 +145,7 @@ const SAMPLE_CERT: string = 'd9d9f7a364747265658301830182045820250f5e26868d9c1ea7ab29cbe9c15bf1c47c0d7605e803e39e375a7fe09c6ebb830183024e726571756573745f7374617475738301820458204b268227774ec77ff2b37ecb12157329d54cf376694bdd59ded7803efd82386f83025820edad510eaaa08ed2acd4781324e6446269da6753ec17760f206bbe81c465ff528301830183024b72656a6563745f636f64658203410383024e72656a6563745f6d6573736167658203584443616e69737465722069766733372d71696161612d61616161622d61616167612d63616920686173206e6f20757064617465206d6574686f64202772656769737465722783024673746174757382034872656a65637465648204582097232f31f6ab7ca4fe53eb6568fc3e02bc22fe94ab31d010e5fb3c642301f1608301820458203a48d1fc213d49307103104f7d72c2b5930edba8787b90631f343b3aa68a5f0a83024474696d65820349e2dc939091c696eb16697369676e6174757265583089a2be21b5fa8ac9fab1527e041327ce899d7da971436a1f2165393947b4d942365bfe5488710e61a619ba48388a21b16a64656c65676174696f6ea2697375626e65745f6964581dd77b2a2f7199b9a8aec93fe6fb588661358cf12223e9a3af7b4ebac4026b6365727469666963617465590231d9d9f7a26474726565830182045820ae023f28c3b9d966c8fb09f9ed755c828aadb5152e00aaf700b18c9c067294b483018302467375626e6574830182045820e83bb025f6574c8f31233dc0fe289ff546dfa1e49bd6116dd6e8896d90a4946e830182045820e782619092d69d5bebf0924138bd4116b0156b5a95e25c358ea8cf7e7161a661830183018204582062513fa926c9a9ef803ac284d620f303189588e1d3904349ab63b6470856fc4883018204582060e9a344ced2c9c4a96a0197fd585f2d259dbd193e4eada56239cac26087f9c58302581dd77b2a2f7199b9a8aec93fe6fb588661358cf12223e9a3af7b4ebac402830183024f63616e69737465725f72616e6765738203581bd9d9f781824a000000000020000001014a00000000002fffff010183024a7075626c69635f6b657982035885308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c050302010361009933e1f89e8a3c4d7fdcccdbd518089e2bd4d8180a261f18d9c247a52768ebce98dc7328a39814a8f911086a1dd50cbe015e2a53b7bf78b55288893daa15c346640e8831d72a12bdedd979d28470c34823b8d1c3f4795d9c3984a247132e94fe82045820996f17bb926be3315745dea7282005a793b58e76afeb5d43d1a28ce29d2d158583024474696d6582034995b8aac0e4eda2ea16697369676e61747572655830ace9fcdd9bc977e05d6328f889dc4e7c99114c737a494653cb27a1f55c06f4555e0f160980af5ead098acc195010b2f7'; const parseTimeFromCert = (cert: ArrayBuffer): Date => { - const certObj = cbor.decode(new Uint8Array(cert)) as any; + const certObj = cbor.decode(new Uint8Array(cert)) as { tree: Cert.HashTree }; if (!certObj.tree) throw new Error('Invalid certificate'); const lookup = lookupResultToBuffer(lookup_path(['time'], certObj.tree)); if (!lookup) throw new Error('Invalid certificate'); @@ -264,11 +265,13 @@ test('certificate verification fails on nested delegations', async () => { // This is a recorded certificate from a read_state request to the II // subnet, with the /subnet tree included. Thus, it could be used as its // own delegation, according to the old interface spec definition. - const withSubnetSubtree = readFileSync('packages/agent/src/bin/with_subnet_key.bin'); - const canisterId = Principal.fromText("rdmx6-jaaaa-aaaaa-aaadq-cai"); - const subnetId = Principal.fromText("uzr34-akd3s-xrdag-3ql62-ocgoh-ld2ao-tamcv-54e7j-krwgb-2gm4z-oqe"); + const withSubnetSubtree = readFileSync(path.join(__dirname, 'bin/with_subnet_key.bin')); + const canisterId = Principal.fromText('rdmx6-jaaaa-aaaaa-aaadq-cai'); + const subnetId = Principal.fromText( + 'uzr34-akd3s-xrdag-3ql62-ocgoh-ld2ao-tamcv-54e7j-krwgb-2gm4z-oqe', + ); jest.setSystemTime(new Date(Date.parse('2023-12-12T10:40:00.652Z'))); - let cert: Cert.Cert = cbor.decode(withSubnetSubtree); + const cert: Cert.Cert = cbor.decode(withSubnetSubtree); const overlyNested = cbor.encode({ tree: cert.tree, signature: cert.signature, @@ -276,10 +279,12 @@ test('certificate verification fails on nested delegations', async () => { subnet_id: subnetId.toUint8Array(), certificate: withSubnetSubtree, }, - }) - await expect(Cert.Certificate.create({ - certificate: overlyNested, - rootKey: fromHex(IC_ROOT_KEY), - canisterId: canisterId, - })).rejects.toThrow('Invalid certificate: Delegation certificates cannot be nested'); + }); + await expect( + Cert.Certificate.create({ + certificate: overlyNested, + rootKey: fromHex(IC_ROOT_KEY), + canisterId: canisterId, + }), + ).rejects.toThrow('Invalid certificate: Delegation certificates cannot be nested'); }); diff --git a/packages/agent/test-setup.ts b/packages/agent/test-setup.ts index 52260b396..fccf11271 100644 --- a/packages/agent/test-setup.ts +++ b/packages/agent/test-setup.ts @@ -18,3 +18,14 @@ Object.defineProperty(global, 'performance', { writable: true, value: { ...global.performance }, }); + +Object.defineProperty(global, 'console', { + writable: true, + value: { + ...global.console, + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + testingLog: global.console.log, + }, +}); diff --git a/packages/auth-client/test-setup.ts b/packages/auth-client/test-setup.ts index 31b39824a..753deee7b 100644 --- a/packages/auth-client/test-setup.ts +++ b/packages/auth-client/test-setup.ts @@ -18,11 +18,7 @@ Object.defineProperty(globalThis, 'crypto', { value: crypto, }); -Object.defineProperty(console, 'warn', { - value: jest.fn(), +Object.defineProperty(global, 'console', { + writable: true, + value: { ...global.console, log: jest.fn(), warn: jest.fn(), error: jest.fn() }, }); - -Object.defineProperty(console, 'log', { - value: jest.fn(), -}); - \ No newline at end of file From e1616c23659b2127069679552e7deb63357e52b3 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Tue, 6 Feb 2024 15:38:44 -0800 Subject: [PATCH 02/15] pulling watermark from readstate response --- packages/agent/src/agent/http/http.test.ts | 18 ++++++++++++ packages/agent/src/agent/http/index.ts | 33 +++++++++++++++++++--- packages/agent/src/utils/buffer.ts | 16 +++++++++-- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index ca662f586..3379e78cd 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -799,3 +799,21 @@ test('retry requests that fail due to a network failure', async () => { }); test.todo('retry query signature validation after refreshing the subnet node keys'); + + +test.only('asdf', async () => { + const foo = new HttpAgent({ host: 'https://icp-api.io' }); + jest.useFakeTimers(); + // await foo.syncTime(); + const bar = await foo + .query(Principal.fromText('qg33c-4aaaa-aaaab-qaica-cai'), { + methodName: 'test', + arg: new Uint8Array().buffer, + }) + .catch(e => { + console.log(e); + }) + .then(e => { + console.log(e); + }); +}); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 66a4600f1..291660cd9 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -4,7 +4,7 @@ import { AgentError } from '../../errors'; import { AnonymousIdentity, Identity } from '../../auth'; import * as cbor from '../../cbor'; import { hashOfMap, requestIdOf } from '../../request_id'; -import { concat, fromHex } from '../../utils/buffer'; +import { bufFromBufLike, concat, fromHex } from '../../utils/buffer'; import { Agent, ApiQueryResponse, @@ -22,16 +22,19 @@ import { HttpAgentRequestTransformFn, HttpAgentSubmitRequest, makeNonce, + Nonce, QueryRequest, ReadRequestType, SubmitRequestType, } from './types'; import { AgentHTTPResponseError } from './errors'; import { SubnetStatus, request } from '../../canisterStatus'; -import { CertificateVerificationError } from '../../certificate'; +import { CertificateVerificationError, HashTree, lookup_path } from '../../certificate'; import { ed25519 } from '@noble/curves/ed25519'; import { ExpirableMap } from '../../utils/expirableMap'; import { Ed25519PublicKey } from '../../public_key'; +import { decodeTime } from '../../utils/leb'; +import { isArrayBuffer } from 'util/types'; export * from './transforms'; export { Nonce, makeNonce } from './types'; @@ -108,7 +111,6 @@ export interface HttpAgentOptions { /** * Adds a unique {@link Nonce} with each query. * Enabling will prevent queries from being answered with a cached response. - * * @example * const agent = new HttpAgent({ useQueryNonces: true }); * agent.addTransform(makeNonceTransform(makeNonce); @@ -184,6 +186,9 @@ export class HttpAgent implements Agent { private readonly _retryTimes; // Retry requests N times before erroring by default public readonly _isAgent = true; + // The UTC time in milliseconds when the latest request was made + public waterMark = 0; + #queryPipeline: HttpAgentRequestTransformFn[] = []; #updatePipeline: HttpAgentRequestTransformFn[] = []; @@ -679,7 +684,27 @@ export class HttpAgent implements Agent { ` Body: ${await response.text()}\n`, ); } - return cbor.decode(await response.arrayBuffer()); + const decodedResponse: ReadStateResponse = cbor.decode(await response.arrayBuffer()); + + this.waterMark = await this.parseTimeFromResponse(decodedResponse); + + return decodedResponse; + } + + public async parseTimeFromResponse(response: ReadStateResponse): Promise { + let tree: HashTree; + const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate); + if (decoded && 'tree' in decoded) { + tree = decoded.tree; + } else { + throw new Error('Could not decode time from response'); + } + const timeLookup = lookup_path(['time'], tree); + if (!timeLookup || !isArrayBuffer(timeLookup)) { + throw new Error('Time was not found in the response or was not in its expected format.'); + } + const date = decodeTime(bufFromBufLike(timeLookup)); + return date.getUTCMilliseconds(); } /** diff --git a/packages/agent/src/utils/buffer.ts b/packages/agent/src/utils/buffer.ts index 0792d0cea..e858b1c31 100644 --- a/packages/agent/src/utils/buffer.ts +++ b/packages/agent/src/utils/buffer.ts @@ -86,7 +86,14 @@ export function uint8ToBuf(arr: Uint8Array): ArrayBuffer { * @returns ArrayBuffer */ export function bufFromBufLike( - bufLike: ArrayBuffer | Uint8Array | DataView | ArrayBufferView | ArrayBufferLike, + bufLike: + | ArrayBuffer + | Uint8Array + | DataView + | ArrayBufferView + | ArrayBufferLike + | [number] + | { buffer: ArrayBuffer }, ): ArrayBuffer { if (bufLike instanceof Uint8Array) { return uint8ToBuf(bufLike); @@ -94,8 +101,11 @@ export function bufFromBufLike( if (bufLike instanceof ArrayBuffer) { return bufLike; } + if (Array.isArray(bufLike)) { + return uint8ToBuf(new Uint8Array(bufLike)); + } if ('buffer' in bufLike) { - return bufLike.buffer; + return bufFromBufLike(bufLike.buffer); } - return new Uint8Array(bufLike); + return uint8ToBuf(new Uint8Array(bufLike)); } From c55dfec620079905079351fba1469b9694145e32 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Mon, 12 Feb 2024 12:25:35 -0800 Subject: [PATCH 03/15] test failing (good) --- e2e/node/basic/watermark.test.ts | 102 +++++++++++++++++++++ packages/agent/src/agent/http/http.test.ts | 16 ---- packages/agent/src/agent/http/index.ts | 60 +++++++++--- packages/agent/src/observable.ts | 30 +++--- 4 files changed, 162 insertions(+), 46 deletions(-) create mode 100644 e2e/node/basic/watermark.test.ts diff --git a/e2e/node/basic/watermark.test.ts b/e2e/node/basic/watermark.test.ts new file mode 100644 index 000000000..95bee10f9 --- /dev/null +++ b/e2e/node/basic/watermark.test.ts @@ -0,0 +1,102 @@ +import { test, expect, vi } from 'vitest'; +import { createActor } from '../canisters/counter'; +import { Actor, HttpAgent } from '@dfinity/agent'; +import { Agent } from 'http'; + +class FetchProxy { + #history: Response[] = []; + #replyIndex = 0; + + async fetch(...args): Promise { + if (this.#replyIndex) { + return this.#history[this.#replyIndex].clone(); + } + + const response = await global.fetch(...args); + this.#history.push(response); + return response.clone(); + } + + get history() { + return this.#history; + } + + clearHistory() { + this.#history = []; + } + + replayFromHistory(index: number) { + this.#replyIndex = index; + } +} + +test('basic', async () => { + const fetchProxy = new FetchProxy(); + global.fetch; + + const actor = await createActor({ + fetch: fetchProxy.fetch.bind(fetchProxy), + }); + + fetchProxy.clearHistory(); + const startValue = await actor.read(); + expect(startValue).toBe(0n); + expect(fetchProxy.history).toHaveLength(1); +}); + +test('replay queries only', async () => { + const fetchProxy = new FetchProxy(); + global.fetch; + + const actor = await createActor({ + fetch: fetchProxy.fetch.bind(fetchProxy), + }); + + fetchProxy.clearHistory(); + const startValue = await actor.read(); + expect(startValue).toBe(0n); + expect(fetchProxy.history).toHaveLength(1); + + fetchProxy.replayFromHistory(0); + const startValue2 = await actor.read(); + expect(startValue2).toBe(0n); + expect(fetchProxy.history).toHaveLength(2); +}); + +test('replay attack', async () => { + const fetchProxy = new FetchProxy(); + global.fetch; + + const actor = await createActor({ + fetch: fetchProxy.fetch.bind(fetchProxy), + }); + + const agent = Actor.agentOf(actor) as HttpAgent; + const logFn = vi.fn(); + agent.log.subscribe(logFn); + + fetchProxy.clearHistory(); + const startValue = await actor.read(); + expect(startValue).toBe(0n); + expect(fetchProxy.history).toHaveLength(1); + + const startValue2 = await actor.read(); + expect(startValue2).toBe(0n); + expect(fetchProxy.history).toHaveLength(2); + + await actor.inc(); + + // wait 1 second + await new Promise(resolve => setTimeout(resolve, 1000)); + const startValue3 = await actor.read(); + expect(startValue3).toBe(1n); + + fetchProxy.replayFromHistory(1); + // the replayed request should throw an error + expect(fetchProxy.history).toHaveLength(6); + + await expect(actor.read()).rejects.toThrow(); + + // The agent should have made 3 additional requests + expect(fetchProxy.history).toHaveLength(9); +}, 10_000); diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index c8378d944..84dbed426 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -798,22 +798,6 @@ test('retry requests that fail due to a network failure', async () => { test.todo('retry query signature validation after refreshing the subnet node keys'); -test.only('asdf', async () => { - const foo = new HttpAgent({ host: 'https://icp-api.io' }); - jest.useFakeTimers(); - // await foo.syncTime(); - const bar = await foo - .query(Principal.fromText('qg33c-4aaaa-aaaab-qaica-cai'), { - methodName: 'test', - arg: new Uint8Array().buffer, - }) - .catch(e => { - console.log(e); - }) - .then(e => { - console.log(e); - }); -}); test('it should log errors to console if the option is set', async () => { const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: jest.fn(), logToConsole: true }); await agent.syncTime(); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index dd3bcf1d1..1021289cc 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -8,6 +8,7 @@ import { bufFromBufLike, concat, fromHex } from '../../utils/buffer'; import { Agent, ApiQueryResponse, + NodeSignature, QueryFields, QueryResponse, ReadStateOptions, @@ -254,7 +255,7 @@ export class HttpAgent implements Agent { ); } else { this._host = new URL('https://icp-api.io'); - console.warn( + this.log.warn( 'Could not infer host from window.location, defaulting to mainnet gateway of https://icp-api.io. Please provide a host to the HttpAgent constructor to avoid this warning.', ); } @@ -428,7 +429,7 @@ export class HttpAgent implements Agent { response = await request(); } catch (error) { if (this._retryTimes > tries) { - console.warn( + this.log.warn( `Caught exception while attempting to make request:\n` + ` ${error}\n` + ` Retrying request.`, @@ -448,7 +449,7 @@ export class HttpAgent implements Agent { ` Body: ${responseText}\n`; if (this._retryTimes > tries) { - console.warn(errorMessage + ` Retrying request.`); + this.log.warn(errorMessage + ` Retrying request.`); return await this._requestAndRetry(request, tries + 1); } @@ -541,6 +542,24 @@ export class HttpAgent implements Agent { }; // Make query and fetch subnet keys in parallel const [query, subnetStatus] = await Promise.all([makeQuery(), getSubnetStatus()]); + const timestamp = query.signatures?.[0]?.timestamp; + if (!timestamp) { + throw new Error('aaaaaaaa'); + } + const timeStampInMs = Number(BigInt(timestamp) / BigInt(1_000_000)); + this.log('watermark and timestamp', { + waterMark: this.waterMark, + timestamp: timeStampInMs, + }); + if (timestamp && Number(this.waterMark) > timeStampInMs) { + const error = new Error('Timestamp is less than the watermark'); + this.log.error('Timestamp is less than the watermark', error, { + timestamp, + waterMark: this.waterMark, + }); + throw error; + } + this.log('Query response:', query); // Skip verification if the user has disabled it if (!this.#verifyQuerySignatures) { return query; @@ -549,7 +568,7 @@ export class HttpAgent implements Agent { return this.#verifyQueryResponse(query, subnetStatus); } catch (_) { // In case the node signatures have changed, refresh the subnet keys and try again - console.warn('Query response verification failed. Retrying with fresh subnet keys.'); + this.log.warn('Query response verification failed. Retrying with fresh subnet keys.'); this.#subnetKeys.delete(canisterId.toString()); await this.fetchSubnetKeys(canisterId.toString()); @@ -703,25 +722,36 @@ export class HttpAgent implements Agent { } const decodedResponse: ReadStateResponse = cbor.decode(await response.arrayBuffer()); - this.waterMark = await this.parseTimeFromResponse(decodedResponse); + this.log('Read state response:', decodedResponse); + const parsedTime = await this.parseTimeFromResponse(decodedResponse); + if (parsedTime > 0) { + this.log('Read state response time:', parsedTime); + this.waterMark = parsedTime; + } return decodedResponse; } public async parseTimeFromResponse(response: ReadStateResponse): Promise { let tree: HashTree; - const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate); - if (decoded && 'tree' in decoded) { - tree = decoded.tree; + if (response.certificate) { + const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate); + if (decoded && 'tree' in decoded) { + tree = decoded.tree; + } else { + throw new Error('Could not decode time from response'); + } + const timeLookup = lookup_path(['time'], tree); + if (!timeLookup || !isArrayBuffer(timeLookup)) { + throw new Error('Time was not found in the response or was not in its expected format.'); + } + const date = decodeTime(bufFromBufLike(timeLookup)); + this.log('Time from response:', date); + return Number(date); } else { - throw new Error('Could not decode time from response'); - } - const timeLookup = lookup_path(['time'], tree); - if (!timeLookup || !isArrayBuffer(timeLookup)) { - throw new Error('Time was not found in the response or was not in its expected format.'); + this.log.warn('No certificate found in response'); } - const date = decodeTime(bufFromBufLike(timeLookup)); - return date.getUTCMilliseconds(); + return 0; } /** diff --git a/packages/agent/src/observable.ts b/packages/agent/src/observable.ts index 89ff89526..e82789eb2 100644 --- a/packages/agent/src/observable.ts +++ b/packages/agent/src/observable.ts @@ -1,6 +1,6 @@ import { AgentError } from './errors'; -export type ObserveFunction = (data: T) => void; +export type ObserveFunction = (data: T, ...rest: unknown[]) => void; export class Observable extends Function { observers: ObserveFunction[]; @@ -9,12 +9,12 @@ export class Observable extends Function { super(); this.observers = []; return new Proxy(this, { - apply: (target, _, args) => target.#call(args[0]), + apply: (target, _, args) => target.#call(args[0], ...args.slice(1)), }); } - #call(message: T) { - this.notify(message); + #call(message: T, ...rest: unknown[]) { + this.notify(message, ...rest); } subscribe(func: ObserveFunction) { @@ -25,8 +25,8 @@ export class Observable extends Function { this.observers = this.observers.filter(observer => observer !== func); } - notify(data: T) { - this.observers.forEach(observer => observer(data)); + notify(data: T, ...rest: unknown[]) { + this.observers.forEach(observer => observer(data, ...rest)); } } @@ -45,19 +45,19 @@ export class ObservableLog extends Observable { constructor() { super(); return new Proxy(this, { - apply: (target, _, args) => target.#call(args[0]), + apply: (target, _, args) => target.#call(args[0], ...args.slice(1)), }); } - log(message: string) { - this.notify({ message, level: 'info' }); + log(message: string, ...rest: unknown[]) { + this.notify({ message, level: 'info' }, ...rest); } - warn(message: string) { - this.notify({ message, level: 'warn' }); + warn(message: string, ...rest: unknown[]) { + this.notify({ message, level: 'warn' }, ...rest); } - error(message: string, error: AgentError) { - this.notify({ message, level: 'error', error }); + error(message: string, error: AgentError, ...rest: unknown[]) { + this.notify({ message, level: 'error', error }, ...rest); } - #call(message: string) { - this.log(message); + #call(message: string, ...rest: unknown[]) { + this.log(message, ...rest); } } From 281016bf0e26a53164fbcc2c35f2ed7475e5d874 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Wed, 21 Feb 2024 15:35:17 -0800 Subject: [PATCH 04/15] retry query refactor and tests passing --- e2e/node/basic/watermark.test.ts | 16 +-- packages/agent/src/agent/http/index.ts | 137 +++++++++++++++++-------- 2 files changed, 105 insertions(+), 48 deletions(-) diff --git a/e2e/node/basic/watermark.test.ts b/e2e/node/basic/watermark.test.ts index 95bee10f9..2d519c757 100644 --- a/e2e/node/basic/watermark.test.ts +++ b/e2e/node/basic/watermark.test.ts @@ -1,7 +1,6 @@ import { test, expect, vi } from 'vitest'; import { createActor } from '../canisters/counter'; import { Actor, HttpAgent } from '@dfinity/agent'; -import { Agent } from 'http'; class FetchProxy { #history: Response[] = []; @@ -9,10 +8,13 @@ class FetchProxy { async fetch(...args): Promise { if (this.#replyIndex) { - return this.#history[this.#replyIndex].clone(); + const response = this.#history[this.#replyIndex].clone(); + this.#history.push(response); + return response; } - const response = await global.fetch(...args); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const response = await(global.fetch as any)(...args); this.#history.push(response); return response.clone(); } @@ -95,8 +97,10 @@ test('replay attack', async () => { // the replayed request should throw an error expect(fetchProxy.history).toHaveLength(6); - await expect(actor.read()).rejects.toThrow(); + await expect(actor.read()).rejects.toThrowError( + 'Timestamp failed to pass the watermark after retrying the configured 3 times. We cannot guarantee the integrity of the response since it could be a replay attack.', + ); - // The agent should have made 3 additional requests - expect(fetchProxy.history).toHaveLength(9); + // The agent should should have made 4 additional requests (3 retries + 1 original request) + expect(fetchProxy.history).toHaveLength(10); }, 10_000); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 1021289cc..f9aae3974 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -3,12 +3,11 @@ import { Principal } from '@dfinity/principal'; import { AgentError } from '../../errors'; import { AnonymousIdentity, Identity } from '../../auth'; import * as cbor from '../../cbor'; -import { hashOfMap, requestIdOf } from '../../request_id'; +import { RequestId, hashOfMap, requestIdOf } from '../../request_id'; import { bufFromBufLike, concat, fromHex } from '../../utils/buffer'; import { Agent, ApiQueryResponse, - NodeSignature, QueryFields, QueryResponse, ReadStateOptions, @@ -263,9 +262,8 @@ export class HttpAgent implements Agent { if (options.verifyQuerySignatures !== undefined) { this.#verifyQuerySignatures = options.verifyQuerySignatures; } - // Default is 3, only set from option if greater or equal to 0 - this._retryTimes = - options.retryTimes !== undefined && options.retryTimes >= 0 ? options.retryTimes : 3; + // Default is 3 + this._retryTimes = options.retryTimes ?? 3; // Rewrite to avoid redirects if (this._host.hostname.endsWith(IC0_SUB_DOMAIN)) { this._host.hostname = IC0_DOMAIN; @@ -423,6 +421,86 @@ export class HttpAgent implements Agent { }; } + async #requestAndRetryQuery( + args: { + canister: string; + transformedRequest: HttpAgentRequest; + body: ArrayBuffer; + requestId: RequestId; + }, + tries = 0, + ): Promise { + const { canister, transformedRequest, body, requestId } = args; + let response: ApiQueryResponse; + // Make the request and retry if it throws an error + try { + const fetchResponse = await this._fetch( + '' + new URL(`/api/v2/canister/${canister}/query`, this._host), + { + ...this._fetchOptions, + ...transformedRequest.request, + body, + }, + ); + const queryResponse: QueryResponse = cbor.decode(await fetchResponse.arrayBuffer()); + response = { + ...queryResponse, + httpDetails: { + ok: fetchResponse.ok, + status: fetchResponse.status, + statusText: fetchResponse.statusText, + headers: httpHeadersTransform(fetchResponse.headers), + }, + requestId, + }; + } catch (error) { + if (tries < this._retryTimes) { + this.log.warn( + `Caught exception while attempting to make query:\n` + + ` ${error}\n` + + ` Retrying query.`, + ); + return await this.#requestAndRetryQuery(args, tries + 1); + } + throw error; + } + + const timestamp = response.signatures?.[0]?.timestamp; + + if (!timestamp) { + throw new Error( + 'Timestamp not found in query response. This suggests a malformed or malicious response.', + ); + } + + // Convert the timestamp to milliseconds + const timeStampInMs = Number(BigInt(timestamp) / BigInt(1_000_000)); + + this.log('watermark and timestamp', { + waterMark: this.waterMark, + timestamp: timeStampInMs, + }); + + // If the timestamp is less than the watermark, retry the request up to the retry limit + if (timestamp && Number(this.waterMark) > timeStampInMs) { + const error = new AgentError('Timestamp is below the watermark. Retrying query.'); + this.log.error('Timestamp is below', error, { + timestamp, + waterMark: this.waterMark, + }); + if (tries < this._retryTimes) { + return await this.#requestAndRetryQuery(args, tries + 1); + } + { + throw new AgentError( + `Timestamp failed to pass the watermark after retrying the configured ${this._retryTimes} times. We cannot guarantee the integrity of the response since it could be a replay attack.`, + ); + } + } + + return response; + } + private async _requestAndRetry(request: () => Promise, tries = 0): Promise { let response: Response; try { @@ -490,7 +568,7 @@ export class HttpAgent implements Agent { // TODO: remove this any. This can be a Signed or UnSigned request. // eslint-disable-next-line @typescript-eslint/no-explicit-any - let transformedRequest: any = await this._transform({ + let transformedRequest: HttpAgentRequest = await this._transform({ request: { method: 'POST', headers: { @@ -503,30 +581,18 @@ export class HttpAgent implements Agent { }); // Apply transform for identity. - transformedRequest = await id?.transformRequest(transformedRequest); + transformedRequest = (await id?.transformRequest(transformedRequest)) as HttpAgentRequest; const body = cbor.encode(transformedRequest.body); - const response = await this._requestAndRetry(() => - this._fetch('' + new URL(`/api/v2/canister/${canister.toText()}/query`, this._host), { - ...this._fetchOptions, - ...transformedRequest.request, - body, - }), - ); - - const queryResponse: QueryResponse = cbor.decode(await response.arrayBuffer()); - - return { - ...queryResponse, - httpDetails: { - ok: response.ok, - status: response.status, - statusText: response.statusText, - headers: httpHeadersTransform(response.headers), - }, + const args = { + canister: canister.toText(), + transformedRequest, + body, requestId, }; + + return await this.#requestAndRetryQuery(args); }; const getSubnetStatus = async (): Promise => { @@ -540,30 +606,17 @@ export class HttpAgent implements Agent { await this.fetchSubnetKeys(canisterId.toString()); return this.#subnetKeys.get(canisterId.toString()); }; + // Attempt to make the query i=retryTimes times + // eslint-disable-next-line @typescript-eslint/no-unused-vars // Make query and fetch subnet keys in parallel const [query, subnetStatus] = await Promise.all([makeQuery(), getSubnetStatus()]); - const timestamp = query.signatures?.[0]?.timestamp; - if (!timestamp) { - throw new Error('aaaaaaaa'); - } - const timeStampInMs = Number(BigInt(timestamp) / BigInt(1_000_000)); - this.log('watermark and timestamp', { - waterMark: this.waterMark, - timestamp: timeStampInMs, - }); - if (timestamp && Number(this.waterMark) > timeStampInMs) { - const error = new Error('Timestamp is less than the watermark'); - this.log.error('Timestamp is less than the watermark', error, { - timestamp, - waterMark: this.waterMark, - }); - throw error; - } + this.log('Query response:', query); // Skip verification if the user has disabled it if (!this.#verifyQuerySignatures) { return query; } + try { return this.#verifyQueryResponse(query, subnetStatus); } catch (_) { From a47fd01a22a91c7093843449e6f761bc4f7dc86a Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Wed, 21 Feb 2024 15:57:59 -0800 Subject: [PATCH 05/15] changelog --- docs/generated/changelog.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/generated/changelog.html b/docs/generated/changelog.html index 2351eb5fe..9efab479a 100644 --- a/docs/generated/changelog.html +++ b/docs/generated/changelog.html @@ -11,7 +11,9 @@

Agent-JS Changelog

Version x.x.x

-
    +
      +
    • feat: HttpAgent tracks a watermark from the latest readState call. Queries with signatures made before the watermark will be automatically retried, and rejected if they are still behind.
      • +

    Version 1.0.1

    • fix: ed25519KeyIdentity was not generating unique identities when no seed was provided. This issue was introduced in v0.20.0-beta.0. If your code was affected please upgrade to >=1.0.1
      • From c008d66e632f0e4901810375cbdf83b9bf8b1abd Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Tue, 6 Feb 2024 13:27:12 -0800 Subject: [PATCH 06/15] chore: cleaning up linting errors and logs --- packages/agent/src/agent/http/http.test.ts | 3 +++ packages/agent/src/canisterStatus/index.test.ts | 9 +++++---- packages/agent/src/certificate.test.ts | 6 +++--- packages/agent/test-setup.ts | 11 +++++++++++ packages/auth-client/test-setup.ts | 10 +++------- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index 5947c8c1b..3f33391bb 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -34,6 +34,8 @@ function createIdentity(seed: number): Ed25519KeyIdentity { const originalDateNowFn = global.Date.now; const originalWindow = global.window; const originalFetch = global.fetch; + + beforeEach(() => { global.Date.now = jest.fn(() => new Date(NANOSECONDS_PER_MILLISECONDS).getTime()); Object.assign(global, 'window', { @@ -45,6 +47,7 @@ beforeEach(() => { global.fetch = originalFetch; }); + afterEach(() => { global.Date.now = originalDateNowFn; global.window = originalWindow; diff --git a/packages/agent/src/canisterStatus/index.test.ts b/packages/agent/src/canisterStatus/index.test.ts index 27eb00a45..340966767 100644 --- a/packages/agent/src/canisterStatus/index.test.ts +++ b/packages/agent/src/canisterStatus/index.test.ts @@ -52,6 +52,7 @@ const testCases = [ ]; // Used for repopulating the certificate +// eslint-disable-next-line @typescript-eslint/no-unused-vars const getRealStatus = async () => { const identity = (await Ed25519KeyIdentity.generate( new Uint8Array( @@ -206,7 +207,7 @@ describe('node keys', () => { const { mainnetApplication } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T19:38:58.129Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(mainnetApplication), canisterId: Principal.fromText('erxue-5aaaa-aaaab-qaagq-cai'), rootKey: fromHex(IC_ROOT_KEY), @@ -224,7 +225,7 @@ describe('node keys', () => { const { mainnetSystem } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T19:58:19.412Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(mainnetSystem), canisterId: Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'), rootKey: fromHex(IC_ROOT_KEY), @@ -242,7 +243,7 @@ describe('node keys', () => { const { localApplication } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T20:14:59.406Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(localApplication), canisterId: Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'), rootKey: fromHex(IC_ROOT_KEY), @@ -260,7 +261,7 @@ describe('node keys', () => { const { localSystem } = goldenCertificates; jest.useFakeTimers(); jest.setSystemTime(new Date(Date.parse('2023-09-27T20:15:03.406Z'))); - const cert = await Cert.Certificate.create({ + await Cert.Certificate.create({ certificate: fromHex(localSystem), canisterId: Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai'), rootKey: fromHex(IC_ROOT_KEY), diff --git a/packages/agent/src/certificate.test.ts b/packages/agent/src/certificate.test.ts index 853d8ba7d..d0970709b 100644 --- a/packages/agent/src/certificate.test.ts +++ b/packages/agent/src/certificate.test.ts @@ -121,7 +121,7 @@ test('lookup', () => { function toText(buff: ArrayBuffer): string { const decoder = new TextDecoder(); - let t = decoder.decode(buff); + const t = decoder.decode(buff); return t; } function fromText(str: string): ArrayBuffer { @@ -145,7 +145,7 @@ const SAMPLE_CERT: string = 'd9d9f7a364747265658301830182045820250f5e26868d9c1ea7ab29cbe9c15bf1c47c0d7605e803e39e375a7fe09c6ebb830183024e726571756573745f7374617475738301820458204b268227774ec77ff2b37ecb12157329d54cf376694bdd59ded7803efd82386f83025820edad510eaaa08ed2acd4781324e6446269da6753ec17760f206bbe81c465ff528301830183024b72656a6563745f636f64658203410383024e72656a6563745f6d6573736167658203584443616e69737465722069766733372d71696161612d61616161622d61616167612d63616920686173206e6f20757064617465206d6574686f64202772656769737465722783024673746174757382034872656a65637465648204582097232f31f6ab7ca4fe53eb6568fc3e02bc22fe94ab31d010e5fb3c642301f1608301820458203a48d1fc213d49307103104f7d72c2b5930edba8787b90631f343b3aa68a5f0a83024474696d65820349e2dc939091c696eb16697369676e6174757265583089a2be21b5fa8ac9fab1527e041327ce899d7da971436a1f2165393947b4d942365bfe5488710e61a619ba48388a21b16a64656c65676174696f6ea2697375626e65745f6964581dd77b2a2f7199b9a8aec93fe6fb588661358cf12223e9a3af7b4ebac4026b6365727469666963617465590231d9d9f7a26474726565830182045820ae023f28c3b9d966c8fb09f9ed755c828aadb5152e00aaf700b18c9c067294b483018302467375626e6574830182045820e83bb025f6574c8f31233dc0fe289ff546dfa1e49bd6116dd6e8896d90a4946e830182045820e782619092d69d5bebf0924138bd4116b0156b5a95e25c358ea8cf7e7161a661830183018204582062513fa926c9a9ef803ac284d620f303189588e1d3904349ab63b6470856fc4883018204582060e9a344ced2c9c4a96a0197fd585f2d259dbd193e4eada56239cac26087f9c58302581dd77b2a2f7199b9a8aec93fe6fb588661358cf12223e9a3af7b4ebac402830183024f63616e69737465725f72616e6765738203581bd9d9f781824a000000000020000001014a00000000002fffff010183024a7075626c69635f6b657982035885308182301d060d2b0601040182dc7c0503010201060c2b0601040182dc7c050302010361009933e1f89e8a3c4d7fdcccdbd518089e2bd4d8180a261f18d9c247a52768ebce98dc7328a39814a8f911086a1dd50cbe015e2a53b7bf78b55288893daa15c346640e8831d72a12bdedd979d28470c34823b8d1c3f4795d9c3984a247132e94fe82045820996f17bb926be3315745dea7282005a793b58e76afeb5d43d1a28ce29d2d158583024474696d6582034995b8aac0e4eda2ea16697369676e61747572655830ace9fcdd9bc977e05d6328f889dc4e7c99114c737a494653cb27a1f55c06f4555e0f160980af5ead098acc195010b2f7'; const parseTimeFromCert = (cert: ArrayBuffer): Date => { - const certObj = cbor.decode(new Uint8Array(cert)) as any; + const certObj = cbor.decode(new Uint8Array(cert)) as { tree: Cert.HashTree }; if (!certObj.tree) throw new Error('Invalid certificate'); const lookup = lookupResultToBuffer(lookup_path(['time'], certObj.tree)); if (!lookup) throw new Error('Invalid certificate'); @@ -271,7 +271,7 @@ test('certificate verification fails on nested delegations', async () => { 'uzr34-akd3s-xrdag-3ql62-ocgoh-ld2ao-tamcv-54e7j-krwgb-2gm4z-oqe', ); jest.setSystemTime(new Date(Date.parse('2023-12-12T10:40:00.652Z'))); - let cert: Cert.Cert = cbor.decode(withSubnetSubtree); + const cert: Cert.Cert = cbor.decode(withSubnetSubtree); const overlyNested = cbor.encode({ tree: cert.tree, signature: cert.signature, diff --git a/packages/agent/test-setup.ts b/packages/agent/test-setup.ts index 52260b396..fccf11271 100644 --- a/packages/agent/test-setup.ts +++ b/packages/agent/test-setup.ts @@ -18,3 +18,14 @@ Object.defineProperty(global, 'performance', { writable: true, value: { ...global.performance }, }); + +Object.defineProperty(global, 'console', { + writable: true, + value: { + ...global.console, + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + testingLog: global.console.log, + }, +}); diff --git a/packages/auth-client/test-setup.ts b/packages/auth-client/test-setup.ts index 31b39824a..753deee7b 100644 --- a/packages/auth-client/test-setup.ts +++ b/packages/auth-client/test-setup.ts @@ -18,11 +18,7 @@ Object.defineProperty(globalThis, 'crypto', { value: crypto, }); -Object.defineProperty(console, 'warn', { - value: jest.fn(), +Object.defineProperty(global, 'console', { + writable: true, + value: { ...global.console, log: jest.fn(), warn: jest.fn(), error: jest.fn() }, }); - -Object.defineProperty(console, 'log', { - value: jest.fn(), -}); - \ No newline at end of file From 7b402c471e1f284c984d27444cd0fc1f3a88dd2f Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Tue, 6 Feb 2024 15:38:44 -0800 Subject: [PATCH 07/15] pulling watermark from readstate response --- packages/agent/src/agent/http/index.ts | 30 +++++++++++++++++++++++--- packages/agent/src/utils/buffer.ts | 16 +++++++++++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 62932b3bf..4b08c48ca 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -4,7 +4,7 @@ import { AgentError } from '../../errors'; import { AnonymousIdentity, Identity } from '../../auth'; import * as cbor from '../../cbor'; import { hashOfMap, requestIdOf } from '../../request_id'; -import { concat, fromHex } from '../../utils/buffer'; +import { bufFromBufLike, concat, fromHex } from '../../utils/buffer'; import { Agent, ApiQueryResponse, @@ -29,11 +29,13 @@ import { } from './types'; import { AgentHTTPResponseError } from './errors'; import { SubnetStatus, request } from '../../canisterStatus'; -import { CertificateVerificationError } from '../../certificate'; +import { CertificateVerificationError, HashTree, lookup_path } from '../../certificate'; import { ed25519 } from '@noble/curves/ed25519'; import { ExpirableMap } from '../../utils/expirableMap'; import { Ed25519PublicKey } from '../../public_key'; import { ObservableLog } from '../../observable'; +import { decodeTime } from '../../utils/leb'; +import { isArrayBuffer } from 'util/types'; export * from './transforms'; export { Nonce, makeNonce } from './types'; @@ -190,6 +192,8 @@ export class HttpAgent implements Agent { public readonly _isAgent = true; public log: ObservableLog = new ObservableLog(); + // The UTC time in milliseconds when the latest request was made + public waterMark = 0; #queryPipeline: HttpAgentRequestTransformFn[] = []; #updatePipeline: HttpAgentRequestTransformFn[] = []; @@ -697,7 +701,27 @@ export class HttpAgent implements Agent { ` Body: ${await response.text()}\n`, ); } - return cbor.decode(await response.arrayBuffer()); + const decodedResponse: ReadStateResponse = cbor.decode(await response.arrayBuffer()); + + this.waterMark = await this.parseTimeFromResponse(decodedResponse); + + return decodedResponse; + } + + public async parseTimeFromResponse(response: ReadStateResponse): Promise { + let tree: HashTree; + const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate); + if (decoded && 'tree' in decoded) { + tree = decoded.tree; + } else { + throw new Error('Could not decode time from response'); + } + const timeLookup = lookup_path(['time'], tree); + if (!timeLookup || !isArrayBuffer(timeLookup)) { + throw new Error('Time was not found in the response or was not in its expected format.'); + } + const date = decodeTime(bufFromBufLike(timeLookup)); + return date.getUTCMilliseconds(); } /** diff --git a/packages/agent/src/utils/buffer.ts b/packages/agent/src/utils/buffer.ts index 0792d0cea..e858b1c31 100644 --- a/packages/agent/src/utils/buffer.ts +++ b/packages/agent/src/utils/buffer.ts @@ -86,7 +86,14 @@ export function uint8ToBuf(arr: Uint8Array): ArrayBuffer { * @returns ArrayBuffer */ export function bufFromBufLike( - bufLike: ArrayBuffer | Uint8Array | DataView | ArrayBufferView | ArrayBufferLike, + bufLike: + | ArrayBuffer + | Uint8Array + | DataView + | ArrayBufferView + | ArrayBufferLike + | [number] + | { buffer: ArrayBuffer }, ): ArrayBuffer { if (bufLike instanceof Uint8Array) { return uint8ToBuf(bufLike); @@ -94,8 +101,11 @@ export function bufFromBufLike( if (bufLike instanceof ArrayBuffer) { return bufLike; } + if (Array.isArray(bufLike)) { + return uint8ToBuf(new Uint8Array(bufLike)); + } if ('buffer' in bufLike) { - return bufLike.buffer; + return bufFromBufLike(bufLike.buffer); } - return new Uint8Array(bufLike); + return uint8ToBuf(new Uint8Array(bufLike)); } From 3eca72bed0e3ca880bb57700ab7ec4e9be56dcfb Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Mon, 12 Feb 2024 12:25:35 -0800 Subject: [PATCH 08/15] test failing (good) --- e2e/node/basic/watermark.test.ts | 102 +++++++++++++++++++++++++ packages/agent/src/agent/http/index.ts | 60 +++++++++++---- packages/agent/src/observable.ts | 30 ++++---- 3 files changed, 162 insertions(+), 30 deletions(-) create mode 100644 e2e/node/basic/watermark.test.ts diff --git a/e2e/node/basic/watermark.test.ts b/e2e/node/basic/watermark.test.ts new file mode 100644 index 000000000..95bee10f9 --- /dev/null +++ b/e2e/node/basic/watermark.test.ts @@ -0,0 +1,102 @@ +import { test, expect, vi } from 'vitest'; +import { createActor } from '../canisters/counter'; +import { Actor, HttpAgent } from '@dfinity/agent'; +import { Agent } from 'http'; + +class FetchProxy { + #history: Response[] = []; + #replyIndex = 0; + + async fetch(...args): Promise { + if (this.#replyIndex) { + return this.#history[this.#replyIndex].clone(); + } + + const response = await global.fetch(...args); + this.#history.push(response); + return response.clone(); + } + + get history() { + return this.#history; + } + + clearHistory() { + this.#history = []; + } + + replayFromHistory(index: number) { + this.#replyIndex = index; + } +} + +test('basic', async () => { + const fetchProxy = new FetchProxy(); + global.fetch; + + const actor = await createActor({ + fetch: fetchProxy.fetch.bind(fetchProxy), + }); + + fetchProxy.clearHistory(); + const startValue = await actor.read(); + expect(startValue).toBe(0n); + expect(fetchProxy.history).toHaveLength(1); +}); + +test('replay queries only', async () => { + const fetchProxy = new FetchProxy(); + global.fetch; + + const actor = await createActor({ + fetch: fetchProxy.fetch.bind(fetchProxy), + }); + + fetchProxy.clearHistory(); + const startValue = await actor.read(); + expect(startValue).toBe(0n); + expect(fetchProxy.history).toHaveLength(1); + + fetchProxy.replayFromHistory(0); + const startValue2 = await actor.read(); + expect(startValue2).toBe(0n); + expect(fetchProxy.history).toHaveLength(2); +}); + +test('replay attack', async () => { + const fetchProxy = new FetchProxy(); + global.fetch; + + const actor = await createActor({ + fetch: fetchProxy.fetch.bind(fetchProxy), + }); + + const agent = Actor.agentOf(actor) as HttpAgent; + const logFn = vi.fn(); + agent.log.subscribe(logFn); + + fetchProxy.clearHistory(); + const startValue = await actor.read(); + expect(startValue).toBe(0n); + expect(fetchProxy.history).toHaveLength(1); + + const startValue2 = await actor.read(); + expect(startValue2).toBe(0n); + expect(fetchProxy.history).toHaveLength(2); + + await actor.inc(); + + // wait 1 second + await new Promise(resolve => setTimeout(resolve, 1000)); + const startValue3 = await actor.read(); + expect(startValue3).toBe(1n); + + fetchProxy.replayFromHistory(1); + // the replayed request should throw an error + expect(fetchProxy.history).toHaveLength(6); + + await expect(actor.read()).rejects.toThrow(); + + // The agent should have made 3 additional requests + expect(fetchProxy.history).toHaveLength(9); +}, 10_000); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 4b08c48ca..c3236f2bf 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -8,6 +8,7 @@ import { bufFromBufLike, concat, fromHex } from '../../utils/buffer'; import { Agent, ApiQueryResponse, + NodeSignature, QueryFields, QueryResponse, ReadStateOptions, @@ -254,7 +255,7 @@ export class HttpAgent implements Agent { ); } else { this._host = new URL('https://icp-api.io'); - console.warn( + this.log.warn( 'Could not infer host from window.location, defaulting to mainnet gateway of https://icp-api.io. Please provide a host to the HttpAgent constructor to avoid this warning.', ); } @@ -428,7 +429,7 @@ export class HttpAgent implements Agent { response = await request(); } catch (error) { if (this._retryTimes > tries) { - console.warn( + this.log.warn( `Caught exception while attempting to make request:\n` + ` ${error}\n` + ` Retrying request.`, @@ -448,7 +449,7 @@ export class HttpAgent implements Agent { ` Body: ${responseText}\n`; if (this._retryTimes > tries) { - console.warn(errorMessage + ` Retrying request.`); + this.log.warn(errorMessage + ` Retrying request.`); return await this._requestAndRetry(request, tries + 1); } @@ -541,6 +542,24 @@ export class HttpAgent implements Agent { }; // Make query and fetch subnet keys in parallel const [query, subnetStatus] = await Promise.all([makeQuery(), getSubnetStatus()]); + const timestamp = query.signatures?.[0]?.timestamp; + if (!timestamp) { + throw new Error('aaaaaaaa'); + } + const timeStampInMs = Number(BigInt(timestamp) / BigInt(1_000_000)); + this.log('watermark and timestamp', { + waterMark: this.waterMark, + timestamp: timeStampInMs, + }); + if (timestamp && Number(this.waterMark) > timeStampInMs) { + const error = new Error('Timestamp is less than the watermark'); + this.log.error('Timestamp is less than the watermark', error, { + timestamp, + waterMark: this.waterMark, + }); + throw error; + } + this.log('Query response:', query); // Skip verification if the user has disabled it if (!this.#verifyQuerySignatures) { return query; @@ -549,7 +568,7 @@ export class HttpAgent implements Agent { return this.#verifyQueryResponse(query, subnetStatus); } catch (_) { // In case the node signatures have changed, refresh the subnet keys and try again - console.warn('Query response verification failed. Retrying with fresh subnet keys.'); + this.log.warn('Query response verification failed. Retrying with fresh subnet keys.'); this.#subnetKeys.delete(canisterId.toString()); await this.fetchSubnetKeys(canisterId.toString()); @@ -703,25 +722,36 @@ export class HttpAgent implements Agent { } const decodedResponse: ReadStateResponse = cbor.decode(await response.arrayBuffer()); - this.waterMark = await this.parseTimeFromResponse(decodedResponse); + this.log('Read state response:', decodedResponse); + const parsedTime = await this.parseTimeFromResponse(decodedResponse); + if (parsedTime > 0) { + this.log('Read state response time:', parsedTime); + this.waterMark = parsedTime; + } return decodedResponse; } public async parseTimeFromResponse(response: ReadStateResponse): Promise { let tree: HashTree; - const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate); - if (decoded && 'tree' in decoded) { - tree = decoded.tree; + if (response.certificate) { + const decoded: { tree: HashTree } | undefined = cbor.decode(response.certificate); + if (decoded && 'tree' in decoded) { + tree = decoded.tree; + } else { + throw new Error('Could not decode time from response'); + } + const timeLookup = lookup_path(['time'], tree); + if (!timeLookup || !isArrayBuffer(timeLookup)) { + throw new Error('Time was not found in the response or was not in its expected format.'); + } + const date = decodeTime(bufFromBufLike(timeLookup)); + this.log('Time from response:', date); + return Number(date); } else { - throw new Error('Could not decode time from response'); - } - const timeLookup = lookup_path(['time'], tree); - if (!timeLookup || !isArrayBuffer(timeLookup)) { - throw new Error('Time was not found in the response or was not in its expected format.'); + this.log.warn('No certificate found in response'); } - const date = decodeTime(bufFromBufLike(timeLookup)); - return date.getUTCMilliseconds(); + return 0; } /** diff --git a/packages/agent/src/observable.ts b/packages/agent/src/observable.ts index 89ff89526..e82789eb2 100644 --- a/packages/agent/src/observable.ts +++ b/packages/agent/src/observable.ts @@ -1,6 +1,6 @@ import { AgentError } from './errors'; -export type ObserveFunction = (data: T) => void; +export type ObserveFunction = (data: T, ...rest: unknown[]) => void; export class Observable extends Function { observers: ObserveFunction[]; @@ -9,12 +9,12 @@ export class Observable extends Function { super(); this.observers = []; return new Proxy(this, { - apply: (target, _, args) => target.#call(args[0]), + apply: (target, _, args) => target.#call(args[0], ...args.slice(1)), }); } - #call(message: T) { - this.notify(message); + #call(message: T, ...rest: unknown[]) { + this.notify(message, ...rest); } subscribe(func: ObserveFunction) { @@ -25,8 +25,8 @@ export class Observable extends Function { this.observers = this.observers.filter(observer => observer !== func); } - notify(data: T) { - this.observers.forEach(observer => observer(data)); + notify(data: T, ...rest: unknown[]) { + this.observers.forEach(observer => observer(data, ...rest)); } } @@ -45,19 +45,19 @@ export class ObservableLog extends Observable { constructor() { super(); return new Proxy(this, { - apply: (target, _, args) => target.#call(args[0]), + apply: (target, _, args) => target.#call(args[0], ...args.slice(1)), }); } - log(message: string) { - this.notify({ message, level: 'info' }); + log(message: string, ...rest: unknown[]) { + this.notify({ message, level: 'info' }, ...rest); } - warn(message: string) { - this.notify({ message, level: 'warn' }); + warn(message: string, ...rest: unknown[]) { + this.notify({ message, level: 'warn' }, ...rest); } - error(message: string, error: AgentError) { - this.notify({ message, level: 'error', error }); + error(message: string, error: AgentError, ...rest: unknown[]) { + this.notify({ message, level: 'error', error }, ...rest); } - #call(message: string) { - this.log(message); + #call(message: string, ...rest: unknown[]) { + this.log(message, ...rest); } } From 659464e26c6d2d401f27f3f96386e38cdfc42a6d Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Wed, 21 Feb 2024 15:35:17 -0800 Subject: [PATCH 09/15] retry query refactor and tests passing --- e2e/node/basic/watermark.test.ts | 16 +-- packages/agent/src/agent/http/index.ts | 137 +++++++++++++++++-------- 2 files changed, 105 insertions(+), 48 deletions(-) diff --git a/e2e/node/basic/watermark.test.ts b/e2e/node/basic/watermark.test.ts index 95bee10f9..2d519c757 100644 --- a/e2e/node/basic/watermark.test.ts +++ b/e2e/node/basic/watermark.test.ts @@ -1,7 +1,6 @@ import { test, expect, vi } from 'vitest'; import { createActor } from '../canisters/counter'; import { Actor, HttpAgent } from '@dfinity/agent'; -import { Agent } from 'http'; class FetchProxy { #history: Response[] = []; @@ -9,10 +8,13 @@ class FetchProxy { async fetch(...args): Promise { if (this.#replyIndex) { - return this.#history[this.#replyIndex].clone(); + const response = this.#history[this.#replyIndex].clone(); + this.#history.push(response); + return response; } - const response = await global.fetch(...args); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const response = await(global.fetch as any)(...args); this.#history.push(response); return response.clone(); } @@ -95,8 +97,10 @@ test('replay attack', async () => { // the replayed request should throw an error expect(fetchProxy.history).toHaveLength(6); - await expect(actor.read()).rejects.toThrow(); + await expect(actor.read()).rejects.toThrowError( + 'Timestamp failed to pass the watermark after retrying the configured 3 times. We cannot guarantee the integrity of the response since it could be a replay attack.', + ); - // The agent should have made 3 additional requests - expect(fetchProxy.history).toHaveLength(9); + // The agent should should have made 4 additional requests (3 retries + 1 original request) + expect(fetchProxy.history).toHaveLength(10); }, 10_000); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index c3236f2bf..7a8101a24 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -3,12 +3,11 @@ import { Principal } from '@dfinity/principal'; import { AgentError } from '../../errors'; import { AnonymousIdentity, Identity } from '../../auth'; import * as cbor from '../../cbor'; -import { hashOfMap, requestIdOf } from '../../request_id'; +import { RequestId, hashOfMap, requestIdOf } from '../../request_id'; import { bufFromBufLike, concat, fromHex } from '../../utils/buffer'; import { Agent, ApiQueryResponse, - NodeSignature, QueryFields, QueryResponse, ReadStateOptions, @@ -263,9 +262,8 @@ export class HttpAgent implements Agent { if (options.verifyQuerySignatures !== undefined) { this.#verifyQuerySignatures = options.verifyQuerySignatures; } - // Default is 3, only set from option if greater or equal to 0 - this._retryTimes = - options.retryTimes !== undefined && options.retryTimes >= 0 ? options.retryTimes : 3; + // Default is 3 + this._retryTimes = options.retryTimes ?? 3; // Rewrite to avoid redirects if (this._host.hostname.endsWith(IC0_SUB_DOMAIN)) { this._host.hostname = IC0_DOMAIN; @@ -423,6 +421,86 @@ export class HttpAgent implements Agent { }; } + async #requestAndRetryQuery( + args: { + canister: string; + transformedRequest: HttpAgentRequest; + body: ArrayBuffer; + requestId: RequestId; + }, + tries = 0, + ): Promise { + const { canister, transformedRequest, body, requestId } = args; + let response: ApiQueryResponse; + // Make the request and retry if it throws an error + try { + const fetchResponse = await this._fetch( + '' + new URL(`/api/v2/canister/${canister}/query`, this._host), + { + ...this._fetchOptions, + ...transformedRequest.request, + body, + }, + ); + const queryResponse: QueryResponse = cbor.decode(await fetchResponse.arrayBuffer()); + response = { + ...queryResponse, + httpDetails: { + ok: fetchResponse.ok, + status: fetchResponse.status, + statusText: fetchResponse.statusText, + headers: httpHeadersTransform(fetchResponse.headers), + }, + requestId, + }; + } catch (error) { + if (tries < this._retryTimes) { + this.log.warn( + `Caught exception while attempting to make query:\n` + + ` ${error}\n` + + ` Retrying query.`, + ); + return await this.#requestAndRetryQuery(args, tries + 1); + } + throw error; + } + + const timestamp = response.signatures?.[0]?.timestamp; + + if (!timestamp) { + throw new Error( + 'Timestamp not found in query response. This suggests a malformed or malicious response.', + ); + } + + // Convert the timestamp to milliseconds + const timeStampInMs = Number(BigInt(timestamp) / BigInt(1_000_000)); + + this.log('watermark and timestamp', { + waterMark: this.waterMark, + timestamp: timeStampInMs, + }); + + // If the timestamp is less than the watermark, retry the request up to the retry limit + if (timestamp && Number(this.waterMark) > timeStampInMs) { + const error = new AgentError('Timestamp is below the watermark. Retrying query.'); + this.log.error('Timestamp is below', error, { + timestamp, + waterMark: this.waterMark, + }); + if (tries < this._retryTimes) { + return await this.#requestAndRetryQuery(args, tries + 1); + } + { + throw new AgentError( + `Timestamp failed to pass the watermark after retrying the configured ${this._retryTimes} times. We cannot guarantee the integrity of the response since it could be a replay attack.`, + ); + } + } + + return response; + } + private async _requestAndRetry(request: () => Promise, tries = 0): Promise { let response: Response; try { @@ -490,7 +568,7 @@ export class HttpAgent implements Agent { // TODO: remove this any. This can be a Signed or UnSigned request. // eslint-disable-next-line @typescript-eslint/no-explicit-any - let transformedRequest: any = await this._transform({ + let transformedRequest: HttpAgentRequest = await this._transform({ request: { method: 'POST', headers: { @@ -503,30 +581,18 @@ export class HttpAgent implements Agent { }); // Apply transform for identity. - transformedRequest = await id?.transformRequest(transformedRequest); + transformedRequest = (await id?.transformRequest(transformedRequest)) as HttpAgentRequest; const body = cbor.encode(transformedRequest.body); - const response = await this._requestAndRetry(() => - this._fetch('' + new URL(`/api/v2/canister/${canister.toText()}/query`, this._host), { - ...this._fetchOptions, - ...transformedRequest.request, - body, - }), - ); - - const queryResponse: QueryResponse = cbor.decode(await response.arrayBuffer()); - - return { - ...queryResponse, - httpDetails: { - ok: response.ok, - status: response.status, - statusText: response.statusText, - headers: httpHeadersTransform(response.headers), - }, + const args = { + canister: canister.toText(), + transformedRequest, + body, requestId, }; + + return await this.#requestAndRetryQuery(args); }; const getSubnetStatus = async (): Promise => { @@ -540,30 +606,17 @@ export class HttpAgent implements Agent { await this.fetchSubnetKeys(canisterId.toString()); return this.#subnetKeys.get(canisterId.toString()); }; + // Attempt to make the query i=retryTimes times + // eslint-disable-next-line @typescript-eslint/no-unused-vars // Make query and fetch subnet keys in parallel const [query, subnetStatus] = await Promise.all([makeQuery(), getSubnetStatus()]); - const timestamp = query.signatures?.[0]?.timestamp; - if (!timestamp) { - throw new Error('aaaaaaaa'); - } - const timeStampInMs = Number(BigInt(timestamp) / BigInt(1_000_000)); - this.log('watermark and timestamp', { - waterMark: this.waterMark, - timestamp: timeStampInMs, - }); - if (timestamp && Number(this.waterMark) > timeStampInMs) { - const error = new Error('Timestamp is less than the watermark'); - this.log.error('Timestamp is less than the watermark', error, { - timestamp, - waterMark: this.waterMark, - }); - throw error; - } + this.log('Query response:', query); // Skip verification if the user has disabled it if (!this.#verifyQuerySignatures) { return query; } + try { return this.#verifyQueryResponse(query, subnetStatus); } catch (_) { From ae45497bb502fb7fa6117716e2e610dc80d68a7a Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Wed, 21 Feb 2024 15:57:59 -0800 Subject: [PATCH 10/15] changelog --- docs/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 029347f17..ac9e990a1 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,7 +2,10 @@ ## [Unreleased] -* feat: adds `fromPem` method for `identity-secp256k1` +### Added + +* feat: adds `fromPem` method for `identity-secp256k1` +* feat: HttpAgent tracks a watermark from the latest readState call. Queries with signatures made before the watermark will be automatically retried, and rejected if they are still behind. ## [1.0.1] - 2024-02-20 From faf98d6efba5beb500dff0f75bc6f96963d467a4 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Thu, 22 Feb 2024 09:00:36 -0800 Subject: [PATCH 11/15] fixing merge inconsistencies --- packages/agent/src/agent/http/index.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 47cc9633e..f9aae3974 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -36,8 +36,6 @@ import { Ed25519PublicKey } from '../../public_key'; import { decodeTime } from '../../utils/leb'; import { isArrayBuffer } from 'util/types'; import { ObservableLog } from '../../observable'; -import { decodeTime } from '../../utils/leb'; -import { isArrayBuffer } from 'util/types'; export * from './transforms'; export { Nonce, makeNonce } from './types'; @@ -196,8 +194,6 @@ export class HttpAgent implements Agent { // The UTC time in milliseconds when the latest request was made public waterMark = 0; public log: ObservableLog = new ObservableLog(); - // The UTC time in milliseconds when the latest request was made - public waterMark = 0; #queryPipeline: HttpAgentRequestTransformFn[] = []; #updatePipeline: HttpAgentRequestTransformFn[] = []; From dbfe6a230e5efdceb7d79b0d9bf5b72cca3f4475 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Fri, 23 Feb 2024 10:04:53 -0800 Subject: [PATCH 12/15] different strategy for isArrayBuffer --- packages/agent/src/agent/http/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index f9aae3974..f6d105ee8 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -34,7 +34,6 @@ import { ed25519 } from '@noble/curves/ed25519'; import { ExpirableMap } from '../../utils/expirableMap'; import { Ed25519PublicKey } from '../../public_key'; import { decodeTime } from '../../utils/leb'; -import { isArrayBuffer } from 'util/types'; import { ObservableLog } from '../../observable'; export * from './transforms'; @@ -795,7 +794,11 @@ export class HttpAgent implements Agent { throw new Error('Could not decode time from response'); } const timeLookup = lookup_path(['time'], tree); - if (!timeLookup || !isArrayBuffer(timeLookup)) { + if (!timeLookup) { + throw new Error('Time was not found in the response or was not in its expected format.'); + } + + if (!(timeLookup instanceof ArrayBuffer) && !ArrayBuffer.isView(timeLookup)) { throw new Error('Time was not found in the response or was not in its expected format.'); } const date = decodeTime(bufFromBufLike(timeLookup)); From 44dc346e63e7d8b45c7b1a3e98c2c242dc7d5163 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Fri, 23 Feb 2024 13:09:19 -0800 Subject: [PATCH 13/15] skip watermark verification if validation is disabled --- packages/agent/src/agent/http/index.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index f6d105ee8..ff3ce54bb 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -466,6 +466,11 @@ export class HttpAgent implements Agent { const timestamp = response.signatures?.[0]?.timestamp; + // Skip watermark verification if the user has set verifyQuerySignatures to false + if (!this.#verifyQuerySignatures) { + return response; + } + if (!timestamp) { throw new Error( 'Timestamp not found in query response. This suggests a malformed or malicious response.', From c954fa5fbc2cd6fae87c13bf4e4940c0c5f2d9cf Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Tue, 12 Mar 2024 11:44:50 -0700 Subject: [PATCH 14/15] fix: watermark tests now run query verification --- .gitignore | 1 + e2e/node/basic/watermark.test.ts | 51 +++++++++++++++++++------- packages/agent/src/agent/http/index.ts | 11 +++++- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index a2d07d6d9..07e24c768 100644 --- a/.gitignore +++ b/.gitignore @@ -78,6 +78,7 @@ build_info.json # parcel-bundler cache (https://parceljs.org/) .cache +.parcel-cache # Next.js build output .next diff --git a/e2e/node/basic/watermark.test.ts b/e2e/node/basic/watermark.test.ts index 2d519c757..d8133c002 100644 --- a/e2e/node/basic/watermark.test.ts +++ b/e2e/node/basic/watermark.test.ts @@ -4,12 +4,13 @@ import { Actor, HttpAgent } from '@dfinity/agent'; class FetchProxy { #history: Response[] = []; - #replyIndex = 0; + #calls = 0; + #replyIndex: number | null = null; async fetch(...args): Promise { - if (this.#replyIndex) { + this.#calls++; + if (this.#replyIndex !== null) { const response = this.#history[this.#replyIndex].clone(); - this.#history.push(response); return response; } @@ -23,8 +24,14 @@ class FetchProxy { return this.#history; } + get calls() { + return this.#calls; + } + clearHistory() { this.#history = []; + this.#calls = 0; + this.#replyIndex = null; } replayFromHistory(index: number) { @@ -32,19 +39,24 @@ class FetchProxy { } } +function indexOfQueryResponse(history: Response[]) { + return history.findIndex(response => response.url.endsWith('query')); +} + test('basic', async () => { const fetchProxy = new FetchProxy(); global.fetch; const actor = await createActor({ fetch: fetchProxy.fetch.bind(fetchProxy), + verifyQuerySignatures: true, }); fetchProxy.clearHistory(); const startValue = await actor.read(); expect(startValue).toBe(0n); - expect(fetchProxy.history).toHaveLength(1); -}); + expect(fetchProxy.calls).toBe(2); +}, 10_000); test('replay queries only', async () => { const fetchProxy = new FetchProxy(); @@ -52,24 +64,29 @@ test('replay queries only', async () => { const actor = await createActor({ fetch: fetchProxy.fetch.bind(fetchProxy), + verifyQuerySignatures: true, }); fetchProxy.clearHistory(); const startValue = await actor.read(); expect(startValue).toBe(0n); - expect(fetchProxy.history).toHaveLength(1); + expect(fetchProxy.calls).toBe(2); + + const queryResponseIndex = indexOfQueryResponse(fetchProxy.history); + + fetchProxy.replayFromHistory(queryResponseIndex); - fetchProxy.replayFromHistory(0); const startValue2 = await actor.read(); expect(startValue2).toBe(0n); - expect(fetchProxy.history).toHaveLength(2); -}); + expect(fetchProxy.calls).toBe(3); +}, 10_000); test('replay attack', async () => { const fetchProxy = new FetchProxy(); global.fetch; const actor = await createActor({ + verifyQuerySignatures: true, fetch: fetchProxy.fetch.bind(fetchProxy), }); @@ -80,11 +97,14 @@ test('replay attack', async () => { fetchProxy.clearHistory(); const startValue = await actor.read(); expect(startValue).toBe(0n); - expect(fetchProxy.history).toHaveLength(1); + + // 1: make query + // 2: fetch subnet keys + expect(fetchProxy.calls).toBe(2); const startValue2 = await actor.read(); expect(startValue2).toBe(0n); - expect(fetchProxy.history).toHaveLength(2); + expect(fetchProxy.calls).toBe(3); await actor.inc(); @@ -93,14 +113,17 @@ test('replay attack', async () => { const startValue3 = await actor.read(); expect(startValue3).toBe(1n); - fetchProxy.replayFromHistory(1); + const queryResponseIndex = indexOfQueryResponse(fetchProxy.history); + + fetchProxy.replayFromHistory(queryResponseIndex); + // the replayed request should throw an error - expect(fetchProxy.history).toHaveLength(6); + expect(fetchProxy.calls).toBe(7); await expect(actor.read()).rejects.toThrowError( 'Timestamp failed to pass the watermark after retrying the configured 3 times. We cannot guarantee the integrity of the response since it could be a replay attack.', ); // The agent should should have made 4 additional requests (3 retries + 1 original request) - expect(fetchProxy.history).toHaveLength(10); + expect(fetchProxy.calls).toBe(11); }, 10_000); diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index ff3ce54bb..052bb3804 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -191,7 +191,12 @@ export class HttpAgent implements Agent { public readonly _isAgent = true; // The UTC time in milliseconds when the latest request was made - public waterMark = 0; + #waterMark = 0; + + get waterMark(): number { + return this.#waterMark; + } + public log: ObservableLog = new ObservableLog(); #queryPipeline: HttpAgentRequestTransformFn[] = []; @@ -548,6 +553,7 @@ export class HttpAgent implements Agent { fields: QueryFields, identity?: Identity | Promise, ): Promise { + this.log(`making query to canister ${canisterId} with fields:`, fields); const makeQuery = async () => { const id = await (identity !== undefined ? await identity : await this._identity); if (!id) { @@ -783,7 +789,7 @@ export class HttpAgent implements Agent { const parsedTime = await this.parseTimeFromResponse(decodedResponse); if (parsedTime > 0) { this.log('Read state response time:', parsedTime); - this.waterMark = parsedTime; + this.#waterMark = parsedTime; } return decodedResponse; @@ -808,6 +814,7 @@ export class HttpAgent implements Agent { } const date = decodeTime(bufFromBufLike(timeLookup)); this.log('Time from response:', date); + this.log('Time from response in milliseconds:', Number(date)); return Number(date); } else { this.log.warn('No certificate found in response'); From c3bf0af57c66472718cf54c6055752c37f377af7 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Wed, 13 Mar 2024 09:03:40 -0700 Subject: [PATCH 15/15] remove redundant check for timestamp --- packages/agent/src/agent/http/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 052bb3804..e8ca5a1d0 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -491,7 +491,7 @@ export class HttpAgent implements Agent { }); // If the timestamp is less than the watermark, retry the request up to the retry limit - if (timestamp && Number(this.waterMark) > timeStampInMs) { + if (Number(this.waterMark) > timeStampInMs) { const error = new AgentError('Timestamp is below the watermark. Retrying query.'); this.log.error('Timestamp is below', error, { timestamp,