Merge pull request #5330 from BitGo/BTC-1708.verify-descriptor-signat…

…ures feat(abstract-utxo): validate descriptor signatures
BitGo · Jan 3, 2025 · 0cbf14a · 0cbf14a
2 parents c82a4f3 + a19606a
commit 0cbf14a
Show file tree

Hide file tree

Showing 5 changed files with 164 additions and 47 deletions.
diff --git a/modules/abstract-utxo/src/core/descriptor/DescriptorMap.ts b/modules/abstract-utxo/src/core/descriptor/DescriptorMap.ts
@@ -4,6 +4,11 @@ import { Descriptor } from '@bitgo/wasm-miniscript';
 export type DescriptorMap = Map<string, Descriptor>;
 
 /** Convert an array of descriptor name-value pairs to a descriptor map */
-export function toDescriptorMap(descriptors: { name: string; value: string }[]): DescriptorMap {
-  return new Map(descriptors.map((d) => [d.name, Descriptor.fromString(d.value, 'derivable')]));
+export function toDescriptorMap(descriptors: { name: string; value: Descriptor | string }[]): DescriptorMap {
+  return new Map(
+    descriptors.map((d) => [
+      d.name,
+      d.value instanceof Descriptor ? d.value : Descriptor.fromString(d.value, 'derivable'),
+    ])
+  );
 }
diff --git a/modules/abstract-utxo/src/descriptor/NamedDescriptor.ts b/modules/abstract-utxo/src/descriptor/NamedDescriptor.ts
@@ -16,7 +16,11 @@ export const NamedDescriptor = t.intersection(
   'NamedDescriptor'
 );
 
-export type NamedDescriptor = t.TypeOf<typeof NamedDescriptor>;
+export type NamedDescriptor<T = string> = {
+  name: string;
+  value: T;
+  signatures?: string[];
+};
 
 export function createNamedDescriptorWithSignature(
   name: string,
@@ -28,14 +32,19 @@ export function createNamedDescriptorWithSignature(
   return { name, value, signatures: [signature] };
 }
 
-export function assertHasValidSignature(namedDescriptor: NamedDescriptor, key: BIP32Interface): void {
-  if (namedDescriptor.signatures === undefined) {
-    throw new Error(`Descriptor ${namedDescriptor.name} does not have a signature`);
+export function hasValidSignature(descriptor: string | Descriptor, key: BIP32Interface, signatures: string[]): boolean {
+  if (typeof descriptor === 'string') {
+    descriptor = Descriptor.fromString(descriptor, 'derivable');
   }
-  const isValid = namedDescriptor.signatures.some((signature) => {
-    return verifyMessage(namedDescriptor.value, key, Buffer.from(signature, 'hex'), networks.bitcoin);
+
+  const message = descriptor.toString();
+  return signatures.some((signature) => {
+    return verifyMessage(message, key, Buffer.from(signature, 'hex'), networks.bitcoin);
   });
-  if (!isValid) {
+}
+
+export function assertHasValidSignature(namedDescriptor: NamedDescriptor, key: BIP32Interface): void {
+  if (!hasValidSignature(namedDescriptor.value, key, namedDescriptor.signatures ?? [])) {
     throw new Error(`Descriptor ${namedDescriptor.name} does not have a valid signature (key=${key.toBase58()})`);
   }
 }
diff --git a/modules/abstract-utxo/src/descriptor/validatePolicy.ts b/modules/abstract-utxo/src/descriptor/validatePolicy.ts
@@ -4,67 +4,109 @@ import * as utxolib from '@bitgo/utxo-lib';
 
 import { DescriptorMap, toDescriptorMap } from '../core/descriptor';
 
-import { DescriptorBuilder, parseDescriptor } from './builder';
-import { NamedDescriptor } from './NamedDescriptor';
-
-export type DescriptorValidationPolicy = { allowedTemplates: DescriptorBuilder['name'][] } | 'allowAll';
+import { parseDescriptor } from './builder';
+import { hasValidSignature, NamedDescriptor } from './NamedDescriptor';
 
 export type KeyTriple = Triple<utxolib.BIP32Interface>;
 
-function isDescriptorWithTemplate(
-  d: Descriptor,
-  name: DescriptorBuilder['name'],
-  walletKeys: Triple<utxolib.BIP32Interface>
-): boolean {
-  const parsed = parseDescriptor(d);
-  if (parsed.name !== name) {
-    return false;
-  }
-  if (parsed.keys.length !== walletKeys.length) {
-    return false;
+export interface DescriptorValidationPolicy {
+  name: string;
+  validate(d: Descriptor, walletKeys: KeyTriple, signatures: string[]): boolean;
+}
+
+export const policyAllowAll: DescriptorValidationPolicy = {
+  name: 'allowAll',
+  validate: () => true,
+};
+
+export function getValidatorDescriptorTemplate(name: string): DescriptorValidationPolicy {
+  return {
+    name: 'descriptorTemplate(' + name + ')',
+    validate(d: Descriptor, walletKeys: KeyTriple): boolean {
+      const parsed = parseDescriptor(d);
+      return (
+        parsed.name === name &&
+        parsed.keys.length === walletKeys.length &&
+        parsed.keys.every((k, i) => k.toBase58() === walletKeys[i].neutered().toBase58())
+      );
+    },
+  };
+}
+
+export function getValidatorEvery(validators: DescriptorValidationPolicy[]): DescriptorValidationPolicy {
+  return {
+    name: 'every(' + validators.map((v) => v.name).join(',') + ')',
+    validate(d: Descriptor, walletKeys: KeyTriple, signatures: string[]): boolean {
+      return validators.every((v) => v.validate(d, walletKeys, signatures));
+    },
+  };
+}
+
+export function getValidatorSome(validators: DescriptorValidationPolicy[]): DescriptorValidationPolicy {
+  return {
+    name: 'some(' + validators.map((v) => v.name).join(',') + ')',
+    validate(d: Descriptor, walletKeys: KeyTriple, signatures: string[]): boolean {
+      return validators.some((v) => v.validate(d, walletKeys, signatures));
+    },
+  };
+}
+
+export function getValidatorOneOfTemplates(names: string[]): DescriptorValidationPolicy {
+  return getValidatorSome(names.map(getValidatorDescriptorTemplate));
+}
+
+export function getValidatorSignedByUserKey(): DescriptorValidationPolicy {
+  return {
+    name: 'signedByUser',
+    validate(d: Descriptor, walletKeys: KeyTriple, signatures: string[]): boolean {
+      // the first key is the user key, by convention
+      return hasValidSignature(d, walletKeys[0], signatures);
+    },
+  };
+}
+
+export class DescriptorPolicyValidationError extends Error {
+  constructor(descriptor: Descriptor, policy: DescriptorValidationPolicy) {
+    super(`Descriptor ${descriptor.toString()} does not match policy ${policy.name}`);
   }
-  return parsed.keys.every((k, i) => k.toBase58() === walletKeys[i].toBase58());
 }
 
 export function assertDescriptorPolicy(
   descriptor: Descriptor,
   policy: DescriptorValidationPolicy,
-  walletKeys: Triple<utxolib.BIP32Interface>
+  walletKeys: KeyTriple,
+  signatures: string[]
 ): void {
-  if (policy === 'allowAll') {
-    return;
-  }
-
-  if ('allowedTemplates' in policy) {
-    const allowed = policy.allowedTemplates;
-    if (!allowed.some((t) => isDescriptorWithTemplate(descriptor, t, walletKeys))) {
-      throw new Error(`Descriptor ${descriptor.toString()} does not match any allowed template`);
-    }
+  if (!policy.validate(descriptor, walletKeys, signatures)) {
+    throw new DescriptorPolicyValidationError(descriptor, policy);
   }
-
-  throw new Error(`Unknown descriptor validation policy: ${policy}`);
 }
 
 export function toDescriptorMapValidate(
   descriptors: NamedDescriptor[],
   walletKeys: KeyTriple,
   policy: DescriptorValidationPolicy
 ): DescriptorMap {
-  const map = toDescriptorMap(descriptors);
-  for (const descriptor of map.values()) {
-    assertDescriptorPolicy(descriptor, policy, walletKeys);
-  }
-  return map;
+  return toDescriptorMap(
+    descriptors.map((namedDescriptor) => {
+      const d = Descriptor.fromString(namedDescriptor.value, 'derivable');
+      assertDescriptorPolicy(d, policy, walletKeys, namedDescriptor.signatures ?? []);
+      return { name: namedDescriptor.name, value: d };
+    })
+  );
 }
 
 export function getPolicyForEnv(env: EnvironmentName): DescriptorValidationPolicy {
   switch (env) {
     case 'adminProd':
     case 'prod':
-      return {
-        allowedTemplates: ['Wsh2Of3', 'Wsh2Of3CltvDrop', 'ShWsh2Of3CltvDrop'],
-      };
+      return getValidatorSome([
+        // allow all 2-of-3-ish descriptors where the keys match the wallet keys
+        getValidatorOneOfTemplates(['Wsh2Of3', 'Wsh2Of3CltvDrop', 'ShWsh2Of3CltvDrop']),
+        // allow all descriptors signed by the user key
+        getValidatorSignedByUserKey(),
+      ]);
     default:
-      return 'allowAll';
+      return policyAllowAll;
   }
 }
diff --git a/modules/abstract-utxo/test/descriptor/descriptorWallet.ts b/modules/abstract-utxo/test/descriptor/descriptorWallet.ts
@@ -4,6 +4,7 @@ import { getDescriptorMapFromWallet, isDescriptorWallet } from '../../src/descri
 import { UtxoWallet } from '../../src/wallet';
 import { getDefaultXPubs, getDescriptorMap } from '../core/descriptor/descriptor.utils';
 import { toBip32Triple } from '../../src/keychains';
+import { policyAllowAll } from '../../src/descriptor/validatePolicy';
 
 describe('isDescriptorWalletData', function () {
   const descriptorMap = getDescriptorMap('Wsh2Of3');
@@ -21,7 +22,7 @@ describe('isDescriptorWalletData', function () {
 
     assert(isDescriptorWallet(wallet));
     assert.strictEqual(
-      getDescriptorMapFromWallet(wallet, toBip32Triple(getDefaultXPubs()), 'allowAll').size,
+      getDescriptorMapFromWallet(wallet, toBip32Triple(getDefaultXPubs()), policyAllowAll).size,
       descriptorMap.size
     );
   });

diff --git a/modules/abstract-utxo/test/transaction/descriptor/validatePolicy.ts b/modules/abstract-utxo/test/transaction/descriptor/validatePolicy.ts
@@ -0,0 +1,60 @@
+import assert from 'assert';
+
+import { Descriptor } from '@bitgo/wasm-miniscript';
+import { Triple } from '@bitgo/sdk-core';
+import { BIP32Interface } from '@bitgo/utxo-lib';
+
+import {
+  assertDescriptorPolicy,
+  DescriptorPolicyValidationError,
+  DescriptorValidationPolicy,
+  getPolicyForEnv,
+  getValidatorDescriptorTemplate,
+} from '../../../src/descriptor/validatePolicy';
+import { DescriptorTemplate, getDescriptor } from '../../core/descriptor/descriptor.utils';
+import { getKeyTriple } from '../../core/key.utils';
+import { NamedDescriptor } from '../../../src/descriptor';
+import { createNamedDescriptorWithSignature } from '../../../src/descriptor/NamedDescriptor';
+
+function testAssertDescriptorPolicy(
+  d: NamedDescriptor<string>,
+  p: DescriptorValidationPolicy,
+  k: Triple<BIP32Interface>,
+  expectedError: DescriptorPolicyValidationError | null
+) {
+  const f = () => assertDescriptorPolicy(Descriptor.fromString(d.value, 'derivable'), p, k, d.signatures ?? []);
+  if (expectedError) {
+    assert.throws(f);
+  } else {
+    assert.doesNotThrow(f);
+  }
+}
+
+describe('assertDescriptorPolicy', function () {
+  const keys = getKeyTriple();
+  function getNamedDescriptor(name: DescriptorTemplate): NamedDescriptor {
+    return createNamedDescriptorWithSignature(name, getDescriptor(name), keys[0]);
+  }
+  function stripSignature(d: NamedDescriptor): NamedDescriptor {
+    return { ...d, signatures: undefined };
+  }
+
+  it('has expected result', function () {
+    testAssertDescriptorPolicy(getNamedDescriptor('Wsh2Of3'), getValidatorDescriptorTemplate('Wsh2Of3'), keys, null);
+
+    // prod does only allow Wsh2Of3-ish descriptors
+    testAssertDescriptorPolicy(getNamedDescriptor('Wsh2Of3'), getPolicyForEnv('prod'), keys, null);
+
+    // prod only allows other descriptors if they are signed by the user key
+    testAssertDescriptorPolicy(getNamedDescriptor('Wsh2Of2'), getPolicyForEnv('prod'), keys, null);
+    testAssertDescriptorPolicy(
+      stripSignature(getNamedDescriptor('Wsh2Of2')),
+      getPolicyForEnv('prod'),
+      keys,
+      new DescriptorPolicyValidationError(getDescriptor('Wsh2Of2'), getPolicyForEnv('prod'))
+    );
+
+    // test is very permissive by default
+    testAssertDescriptorPolicy(stripSignature(getNamedDescriptor('Wsh2Of2')), getPolicyForEnv('test'), keys, null);
+  });
+});