Switch to a static constructor with automatic verification

docs/generated/changelog.html

@@ -13,9 +13,11 @@ <h1>Agent-JS Changelog</h1>
       <h2>Version 0.12.0</h2>
       <ul>
         <li>
-          Changes the certificate verification interface and fixed its logic. The constructor now
-          takes a root key, and verification takes a canister ID. Additionally, verification now
-          checks that the delegation is authoritative for the given canister ID.
+          Changed the certificate verification interface and fixed its logic. The public constructor
+          is now static and asynchronous. There is no separate verification method, the check is
+          done automatically in the constructor and newly also checks that the delegation is
+          authoritative for the given canister ID, as required by the Internet Computer interface
+          specification.
         </li>
       </ul>
       <h2>Version 0.11.2</h2>

e2e/node/basic/basic.test.ts

@@ -14,14 +14,8 @@ test('read_state', async () => {
   const response = await resolvedAgent.readState(canisterId, {
     paths: [path],
   });
-  const rootKey =
-    resolvedAgent.rootKey == null
-      ? resolvedAgent.fetchRootKey()
-      : Promise.resolve(resolvedAgent.rootKey);
-  const cert = new Certificate(response.certificate, resolvedAgent.fetchRootKey());
-
-  expect(() => cert.lookup(path)).toThrow(/Cannot lookup unverified certificate/);
-  expect(await cert.verify(canisterId)).toBe(true);
+  if (resolvedAgent.rootKey == null) throw new Error(`The agent doesn't have a root key yet`);
+  const cert = await Certificate.create(response.certificate, resolvedAgent.rootKey, canisterId);
   expect(cert.lookup([new TextEncoder().encode('Time')])).toBe(undefined);
   // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
   const rawTime = cert.lookup(path)!;

packages/agent/src/canisterStatus/index.ts

@@ -87,15 +87,7 @@ export const request = async (options: {
         const response = await agent.readState(canisterId, {
           paths: [encodedPaths[index]],
         });
-        const rootKey =
-          agent.rootKey == null ? agent.fetchRootKey() : Promise.resolve(agent.rootKey);
-        const cert = new Certificate(response.certificate, rootKey);
-        const verified = await cert.verify(canisterId);
-        if (!verified) {
-          throw new Error(
-            'There was a problem certifying the response data. Please verify your connection to the mainnet, or be sure to call fetchRootKey on your agent if you are developing locally',
-          );
-        }
+        const cert = await Certificate.create(response.certificate, agent.rootKey, canisterId);
 
         const data = cert.lookup(encodePath(uniquePaths[index], canisterId));
         if (!data) {

packages/agent/src/certificate.test.ts

@@ -3,12 +3,11 @@
  * an instance of ArrayBuffer).
  * @jest-environment node
  */
-import { fromHexString } from '@dfinity/candid';
 import * as cbor from './cbor';
 import * as Cert from './certificate';
 import { fromHex, toHex } from './utils/buffer';
 import { Principal } from '@dfinity/principal';
-import { verify } from 'crypto';
+import { AgentError } from './errors';
 
 function label(str: string): ArrayBuffer {
   return new TextEncoder().encode(str);
@@ -142,30 +141,44 @@ const SAMPLE_CERT: string =
 
 test('delegation works for canisters within the subnet range', async () => {
   const canisterId = Principal.fromText('ivg37-qiaaa-aaaab-aaaga-cai');
-  const cert = new Cert.Certificate(fromHex(SAMPLE_CERT), Promise.resolve(fromHex(IC_ROOT_KEY)));
-  const result = await cert.verify(canisterId);
-  expect(result).toEqual(true);
+  // Test that create doesn't throw
+  const cert = await Cert.Certificate.create(
+    fromHex(SAMPLE_CERT),
+    fromHex(IC_ROOT_KEY),
+    canisterId,
+  );
 });
 
+function fail(reason) {
+  throw new Error(reason);
+}
+
 test('delegation check fails for canisters outside of the subnet range', async () => {
   // Use a different principal than the happy path, which isn't in the delegation ranges.
   const canisterId = Principal.fromText('ryjl3-tyaaa-aaaaa-aaaba-cai');
-  const cert = new Cert.Certificate(fromHex(SAMPLE_CERT), Promise.resolve(fromHex(IC_ROOT_KEY)));
-  // This is a bit crufty; verify returns a boolean, but can also indicate verification errors
-  // by throwing exceptions
   try {
-    const result = await cert.verify(canisterId);
-    expect(result).toEqual(false);
-  } catch (e) {}
+    const cert = await Cert.Certificate.create(
+      fromHex(SAMPLE_CERT),
+      fromHex(IC_ROOT_KEY),
+      canisterId,
+    );
+    fail('The create method should throw on an invalid certificate');
+  } catch (err) {
+    if (!(err instanceof AgentError) || !err.message.includes('Invalid certificate')) {
+      fail(
+        `The create method should throw an ${AgentError.name} mentioning an invalid certificate`,
+      );
+    }
+  }
 });
 
 // The only situation in which one can read state of the IC management canister
 // is when the user calls provisional_create_canister_with_cycles. In this case,
 // we shouldn't check the delegations.
 test('delegation check succeeds for the management canister', async () => {
-  const cert = new Cert.Certificate(fromHex(SAMPLE_CERT), Promise.resolve(fromHex(IC_ROOT_KEY)));
-  // This is a bit crufty; verify returns a boolean, but can also indicate verification errors
-  // by throwing exceptions
-  const result = await cert.verify(Principal.managementCanister());
-  expect(result).toEqual(true);
+  const cert = await Cert.Certificate.create(
+    fromHex(SAMPLE_CERT),
+    fromHex(IC_ROOT_KEY),
+    Principal.managementCanister(),
+  );
 });

packages/agent/src/certificate.ts

@@ -6,12 +6,11 @@ import { concat, fromHex, toHex } from './utils/buffer';
 import { Principal } from '@dfinity/principal';
 
 /**
- * A certificate needs to be verified (using {@link Certificate.prototype.verify})
- * before it can be used.
+ * A certificate may fail verification with respect to the provided public key
  */
-export class UnverifiedCertificateError extends AgentError {
-  constructor() {
-    super(`Cannot lookup unverified certificate. Call 'verify()' first.`);
+export class CertificateVerificationError extends AgentError {
+  constructor(reason: string) {
+    super(`Invalid certificate: ${reason}`);
   }
 }
 
@@ -103,18 +102,35 @@ export class Certificate {
   private readonly cert: Cert;
   private verified = false;
 
-  constructor(certificate: ArrayBuffer, private _rootKey: Promise<ArrayBuffer>) {
+  /**
+   * Create a new instance of a certificate, automatically verifying it.
+   * @throws {CertificateVerificationError}
+   */
+  public static async create(
+    certificate: ArrayBuffer,
+    rootKey: ArrayBuffer,
+    canisterId: Principal,
+  ): Promise<Certificate> {
+    const cert = new Certificate(certificate, rootKey, canisterId);
+    await cert.verify();
+    return cert;
+  }
+
+  private constructor(
+    certificate: ArrayBuffer,
+    private _rootKey: ArrayBuffer,
+    private _canisterId: Principal,
+  ) {
     this.cert = cbor.decode(new Uint8Array(certificate));
   }
 
   public lookup(path: Array<ArrayBuffer | string>): ArrayBuffer | undefined {
-    this.checkState();
     return lookup_path(path, this.cert.tree);
   }
 
-  public async verify(canisterId: Principal): Promise<boolean> {
+  private async verify(): Promise<boolean> {
     const rootHash = await reconstruct(this.cert.tree);
-    const derKey = await this._checkDelegationAndGetKey(canisterId, this.cert.delegation);
+    const derKey = await this._checkDelegationAndGetKey(this.cert.delegation);
     const sig = this.cert.signature;
     const key = extractDER(derKey);
     const msg = concat(domain_sep('ic-state-root'), rootHash);
@@ -123,39 +139,37 @@ export class Certificate {
     return res;
   }
 
-  protected checkState(): void {
-    if (!this.verified) {
-      throw new UnverifiedCertificateError();
-    }
-  }
-
-  private async _checkDelegationAndGetKey(
-    canisterId: Principal,
-    d?: Delegation,
-  ): Promise<ArrayBuffer> {
+  private async _checkDelegationAndGetKey(d?: Delegation): Promise<ArrayBuffer> {
     if (!d) {
       return this._rootKey;
     }
-    const cert: Certificate = new Certificate(d.certificate, this._rootKey);
-    if (!(await cert.verify(canisterId))) {
-      throw new Error('fail to verify delegation certificate');
-    }
+    const cert: Certificate = await Certificate.create(
+      d.certificate,
+      this._rootKey,
+      this._canisterId,
+    );
 
-    if (canisterId.compareTo(Principal.managementCanister()) != 'eq') {
+    if (this._canisterId.compareTo(Principal.managementCanister()) != 'eq') {
       const rangeLookup = cert.lookup(['subnet', d.subnet_id, 'canister_ranges']);
       if (!rangeLookup) {
-        throw new Error(`Could not find canister ranges for subnet 0x${toHex(d.subnet_id)}`);
+        throw new CertificateVerificationError(
+          `Could not find canister ranges for subnet 0x${toHex(d.subnet_id)}`,
+        );
       }
       const ranges_arr: Array<[Uint8Array, Uint8Array]> = cbor.decode(rangeLookup);
       const ranges: Array<[Principal, Principal]> = ranges_arr.map(v => [
         Principal.fromUint8Array(v[0]),
         Principal.fromUint8Array(v[1]),
       ]);
 
-      const canisterInRange = ranges.some(r => r[0].ltEq(canisterId) && r[1].gtEq(canisterId));
+      const canisterInRange = ranges.some(
+        r => r[0].ltEq(this._canisterId) && r[1].gtEq(this._canisterId),
+      );
       if (!canisterInRange) {
-        throw new Error(
-          `Canister ${canisterId} not in range of delegations for subnet 0x${toHex(d.subnet_id)}`,
+        throw new CertificateVerificationError(
+          `Canister ${this._canisterId} not in range of delegations for subnet 0x${toHex(
+            d.subnet_id,
+          )}`,
         );
       }
     }

packages/agent/src/polling/index.ts

@@ -29,12 +29,8 @@ export async function pollForResponse(
 ): Promise<ArrayBuffer> {
   const path = [new TextEncoder().encode('request_status'), requestId];
   const state = await agent.readState(canisterId, { paths: [path] });
-  const rootKey = agent.rootKey == null ? agent.fetchRootKey() : Promise.resolve(agent.rootKey);
-  const cert = new Certificate(state.certificate, rootKey);
-  const verified = await cert.verify(canisterId);
-  if (!verified) {
-    throw new Error('Fail to verify certificate');
-  }
+  if (agent.rootKey == null) throw new Error('Agent root key not initialized before polling');
+  const cert = await Certificate.create(state.certificate, agent.rootKey, canisterId);
   const maybeBuf = cert.lookup([...path, new TextEncoder().encode('status')]);
   let status;
   if (typeof maybeBuf === 'undefined') {