always check subnet delegations in a certificate (#647)

authored-by: Martin Raszyk <martin.raszyk@dfinity.org>
dfinity · Oct 18, 2022 · 7687265 · 7687265
1 parent 0ff62dc
commit 7687265
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 48 deletions.
diff --git a/packages/agent/src/certificate.test.ts b/packages/agent/src/certificate.test.ts
@@ -186,19 +186,6 @@ test('delegation check fails for canisters outside of the subnet range', async (
   await certificateFails(afterRange);
 });
 
-// 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 () => {
-  await expect(
-    Cert.Certificate.create({
-      certificate: fromHex(SAMPLE_CERT),
-      rootKey: fromHex(IC_ROOT_KEY),
-      canisterId: Principal.managementCanister(),
-    }),
-  ).resolves.not.toThrow();
-});
-
 type FakeCert = {
   tree: Cert.HashTree;
   signature: ArrayBuffer;

diff --git a/packages/agent/src/certificate.ts b/packages/agent/src/certificate.ts
@@ -190,29 +190,27 @@ export class Certificate {
       canisterId: this._canisterId,
     });
 
-    if (this._canisterId.compareTo(Principal.managementCanister()) !== 'eq') {
-      const rangeLookup = cert.lookup(['subnet', d.subnet_id, 'canister_ranges']);
-      if (!rangeLookup) {
-        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 rangeLookup = cert.lookup(['subnet', d.subnet_id, 'canister_ranges']);
+    if (!rangeLookup) {
+      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(this._canisterId) && r[1].gtEq(this._canisterId),
+    const canisterInRange = ranges.some(
+      r => r[0].ltEq(this._canisterId) && r[1].gtEq(this._canisterId),
+    );
+    if (!canisterInRange) {
+      throw new CertificateVerificationError(
+        `Canister ${this._canisterId} not in range of delegations for subnet 0x${toHex(
+          d.subnet_id,
+        )}`,
       );
-      if (!canisterInRange) {
-        throw new CertificateVerificationError(
-          `Canister ${this._canisterId} not in range of delegations for subnet 0x${toHex(
-            d.subnet_id,
-          )}`,
-        );
-      }
     }
     const publicKeyLookup = cert.lookup(['subnet', d.subnet_id, 'public_key']);
     if (!publicKeyLookup) {

diff --git a/packages/bls-verify/src/index.test.ts b/packages/bls-verify/src/index.test.ts
@@ -64,20 +64,6 @@ test('delegation check fails for canisters outside of the subnet range', async (
   await certificateFails(afterRange);
 });
 
-// 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 () => {
-  await expect(
-    Cert.Certificate.create({
-      certificate: fromHex(SAMPLE_CERT),
-      rootKey: fromHex(IC_ROOT_KEY),
-      canisterId: Principal.managementCanister(),
-      blsVerify,
-    }),
-  ).resolves.not.toThrow();
-});
-
 type FakeCert = {
   tree: Cert.HashTree;
   signature: ArrayBuffer;