From 68839a6bd5f4e844c227654208996a9f6114d194 Mon Sep 17 00:00:00 2001 From: NathanosDev Date: Mon, 13 Jan 2025 13:26:03 +0100 Subject: [PATCH] feat: await fetchRootKey calls while making API calls move fetchRootKey calls from constructor to API calls BREAKING CHANGE: --- packages/agent/src/actor.ts | 5 +- packages/agent/src/agent/api.ts | 7 ++- packages/agent/src/agent/http/http.test.ts | 3 +- packages/agent/src/agent/http/index.ts | 56 +++++++++++++++------- packages/agent/src/agent/proxy.ts | 19 ++++++-- packages/agent/src/canisterStatus/index.ts | 8 ++-- packages/agent/src/polling/index.ts | 3 +- 7 files changed, 66 insertions(+), 35 deletions(-) diff --git a/packages/agent/src/actor.ts b/packages/agent/src/actor.ts index 05f7f2de4..3962b5df5 100644 --- a/packages/agent/src/actor.ts +++ b/packages/agent/src/actor.ts @@ -531,9 +531,6 @@ function _createActorMethod( const ecid = effectiveCanisterId !== undefined ? Principal.from(effectiveCanisterId) : cid; const arg = IDL.encode(func.argTypes, args); - if (agent.rootKey == null) - throw new AgentError('Agent root key not initialized before making call'); - const { requestId, response, requestDetails } = await agent.call(cid, { methodName, arg, @@ -545,7 +542,7 @@ function _createActorMethod( const cert = (response.body as v3ResponseBody).certificate; certificate = await Certificate.create({ certificate: bufFromBufLike(cert), - rootKey: agent.rootKey, + rootKey: await agent.getRootKey(), canisterId: Principal.from(canisterId), blsVerify, }); diff --git a/packages/agent/src/agent/api.ts b/packages/agent/src/agent/api.ts index 6d6cd9061..4dbdf8df9 100644 --- a/packages/agent/src/agent/api.ts +++ b/packages/agent/src/agent/api.ts @@ -147,7 +147,12 @@ export interface SubmitResponse { * An Agent able to make calls and queries to a Replica. */ export interface Agent { - readonly rootKey: ArrayBuffer | null; + /** + * Returns the root key of the network. This is used to verify the authenticity of + * responses from the network. + */ + getRootKey(): Promise; + /** * Returns the principal ID associated with this agent (by default). It only shows * the principal of the default identity in the agent, which is the principal used diff --git a/packages/agent/src/agent/http/http.test.ts b/packages/agent/src/agent/http/http.test.ts index 07b253a74..b4d9e265e 100644 --- a/packages/agent/src/agent/http/http.test.ts +++ b/packages/agent/src/agent/http/http.test.ts @@ -800,10 +800,9 @@ test('retry requests that fail due to a network failure', async () => { const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch, + rootKey: new Uint8Array(32), }); - agent.rootKey = new Uint8Array(32); - try { await agent.call(Principal.managementCanister(), { methodName: 'test', diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 5deee1b90..a5a8fc561 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -150,6 +150,14 @@ export interface HttpAgentOptions { * Alternate root key to use for verifying certificates. If not provided, the default IC root key will be used. */ rootKey?: ArrayBuffer; + + /** + * Whether the agent should fetch the root key from the network. Defaults to false. + * + * WARNING!!! Do not enable this in production environments, + * as it can be used as an attack vector by malicious nodes. + */ + shouldFetchRootKey?: boolean; } function getDefaultFetch(): typeof fetch { @@ -245,7 +253,6 @@ other computations so that this class can stay as simple as possible while allowing extensions. */ export class HttpAgent implements Agent { - public rootKey: ArrayBuffer; #identity: Promise | null; readonly #fetch: typeof fetch; readonly #fetchOptions?: Record; @@ -253,11 +260,14 @@ export class HttpAgent implements Agent { #timeDiffMsecs = 0; readonly host: URL; readonly #credentials: string | undefined; - #rootKeyFetched = false; readonly #retryTimes; // Retry requests N times before erroring by default #backoffStrategy: BackoffStrategyFactory; readonly #maxIngressExpiryInMinutes: number; + #rootKey: ArrayBuffer; + readonly #shouldFetchRootKey: boolean; + #fetchRootKeyPromise: Promise | null = null; + // Public signature to help with type checking. public readonly _isAgent = true; public config: HttpAgentOptions = {}; @@ -288,7 +298,11 @@ export class HttpAgent implements Agent { this.#fetch = options.fetch || getDefaultFetch() || fetch.bind(global); this.#fetchOptions = options.fetchOptions; this.#callOptions = options.callOptions; - this.rootKey = options.rootKey ? options.rootKey : fromHex(IC_ROOT_KEY); + + this.#rootKey = options.rootKey ?? fromHex(IC_ROOT_KEY); + this.#shouldFetchRootKey = options.shouldFetchRootKey ?? false; + // kick off the fetchRootKey process asynchronously, if needed + (async () => await this.fetchRootKey())(); const host = determineHost(options.host); this.host = new URL(host); @@ -354,16 +368,9 @@ export class HttpAgent implements Agent { return new this({ ...options }); } - public static async create( - options: HttpAgentOptions & { shouldFetchRootKey?: boolean } = { - shouldFetchRootKey: false, - }, - ): Promise { + public static async create(options: HttpAgentOptions = {}): Promise { const agent = HttpAgent.createSync(options); const initPromises: Promise[] = [agent.syncTime()]; - if (agent.host.toString() !== 'https://icp-api.io' && options.shouldFetchRootKey) { - initPromises.push(agent.fetchRootKey()); - } await Promise.all(initPromises); return agent; } @@ -437,7 +444,7 @@ export class HttpAgent implements Agent { ): Promise { // TODO - restore this value const callSync = options.callSync ?? true; - const id = await (identity !== undefined ? await identity : await this.#identity); + const id = identity !== undefined ? await identity : await this.#identity; if (!id) { throw new IdentityInvalidError( "This identity has expired due this application's security policy. Please refresh your authentication.", @@ -1125,14 +1132,27 @@ export class HttpAgent implements Agent { return cbor.decode(await response.arrayBuffer()); } + async getRootKey(): Promise { + return this.fetchRootKey(); + } + public async fetchRootKey(): Promise { - if (!this.#rootKeyFetched) { - const status = await this.status(); - // Hex-encoded version of the replica root key - this.rootKey = (status as JsonObject & { root_key: ArrayBuffer }).root_key; - this.#rootKeyFetched = true; + if (!this.#shouldFetchRootKey) { + return this.#rootKey; + } + + if (this.#fetchRootKeyPromise === null) { + this.#fetchRootKeyPromise = this.#fetchRootKey(); } - return this.rootKey; + + return await this.#fetchRootKeyPromise; + } + + async #fetchRootKey(): Promise { + const status = await this.status(); + // Hex-encoded version of the replica root key + this.#rootKey = (status as JsonObject & { root_key: ArrayBuffer }).root_key; + return this.#rootKey; } public invalidateIdentity(): void { diff --git a/packages/agent/src/agent/proxy.ts b/packages/agent/src/agent/proxy.ts index d08ef4c36..a4580944b 100644 --- a/packages/agent/src/agent/proxy.ts +++ b/packages/agent/src/agent/proxy.ts @@ -158,7 +158,8 @@ export class ProxyStubAgent { export class ProxyAgent implements Agent { private _nextId = 0; private _pendingCalls = new Map void, (reject: any) => void]>(); - public rootKey = null; + + #fetchRootKeyPromise: Promise | null = null; constructor(private _backend: (msg: ProxyMessage) => void) {} @@ -241,10 +242,20 @@ export class ProxyAgent implements Agent { }); } + async getRootKey(): Promise { + return this.fetchRootKey(); + } + public async fetchRootKey(): Promise { + if (this.#fetchRootKeyPromise === null) { + this.#fetchRootKeyPromise = this.#fetchRootKey(); + } + + return await this.#fetchRootKeyPromise; + } + + async #fetchRootKey(): Promise { // Hex-encoded version of the replica root key - const rootKey = ((await this.status()) as any).root_key; - this.rootKey = rootKey; - return rootKey; + return ((await this.status()) as any).root_key; } } diff --git a/packages/agent/src/canisterStatus/index.ts b/packages/agent/src/canisterStatus/index.ts index c46ba2ca6..4fe858195 100644 --- a/packages/agent/src/canisterStatus/index.ts +++ b/packages/agent/src/canisterStatus/index.ts @@ -147,13 +147,13 @@ export const request = async (options: { }); const cert = await Certificate.create({ certificate: response.certificate, - rootKey: agent.rootKey, + rootKey: await agent.getRootKey(), canisterId: canisterId, }); - const lookup = (cert: Certificate, path: Path) => { + const lookup = async (cert: Certificate, path: Path) => { if (path === 'subnet') { - const data = fetchNodeKeys(response.certificate, canisterId, agent.rootKey); + const data = fetchNodeKeys(response.certificate, canisterId, await agent.getRootKey()); return { path: path, data, @@ -167,7 +167,7 @@ export const request = async (options: { }; // must pass in the rootKey if we have no delegation - const { path, data } = lookup(cert, uniquePaths[index]); + const { path, data } = await lookup(cert, uniquePaths[index]); if (!data) { // Typically, the cert lookup will throw console.warn(`Expected to find result for path ${path}, but instead found nothing.`); diff --git a/packages/agent/src/polling/index.ts b/packages/agent/src/polling/index.ts index 3e8bf344a..321d8e001 100644 --- a/packages/agent/src/polling/index.ts +++ b/packages/agent/src/polling/index.ts @@ -92,10 +92,9 @@ export async function pollForResponse( } const state = await agent.readState(canisterId, { paths: [path] }, undefined, currentRequest); - if (agent.rootKey == null) throw new Error('Agent root key not initialized before polling'); const cert = await Certificate.create({ certificate: state.certificate, - rootKey: agent.rootKey, + rootKey: await agent.getRootKey(), canisterId: canisterId, blsVerify, });