Merge pull request #8958 from BitGo/zahinmohammad/wcn-266-update-ofcisvalidpub-in-sdk-for-happy-path

feat(sdk-core): accept SPKI secp256k1 pubs in Ofc.isValidPub

Author: zahin-mohammad

modules/bitgo/test/v2/unit/coins/ofc.ts

@@ -1,4 +1,5 @@
 import 'should';
+import { generateKeyPairSync } from 'crypto';
 
 import { TestBitGo } from '@bitgo/sdk-test';
 import { BitGo } from '../../../../src/bitgo';
@@ -24,8 +25,39 @@ describe('OFC:', function () {
     ofcCoin.isValidMofNSetup({ m: 1, n: 1 }).should.be.true();
   });
 
-  it('should validate pub key', () => {
-    const { pub } = ofcCoin.keychains().create();
-    ofcCoin.isValidPub(pub).should.equal(true);
+  describe('isValidPub', () => {
+    it('accepts a BIP-32 xpub (user key)', () => {
+      const { pub } = ofcCoin.keychains().create();
+      ofcCoin.isValidPub(pub).should.equal(true);
+    });
+
+    it('accepts a base64 SPKI secp256k1 public key (BitGo key)', () => {
+      const { publicKey } = generateKeyPairSync('ec', { namedCurve: 'secp256k1' });
+      const spkiBase64 = publicKey.export({ type: 'spki', format: 'der' }).toString('base64');
+      ofcCoin.isValidPub(spkiBase64).should.equal(true);
+    });
+
+    it('rejects a BIP-32 xprv (private key)', () => {
+      const { prv } = ofcCoin.keychains().create();
+      ofcCoin.isValidPub(prv).should.equal(false);
+    });
+
+    it('rejects a SPKI public key on the wrong curve (P-256)', () => {
+      const { publicKey } = generateKeyPairSync('ec', { namedCurve: 'prime256v1' });
+      const spkiBase64 = publicKey.export({ type: 'spki', format: 'der' }).toString('base64');
+      ofcCoin.isValidPub(spkiBase64).should.equal(false);
+    });
+
+    it('rejects garbage base64', () => {
+      ofcCoin.isValidPub(Buffer.from('not a key').toString('base64')).should.equal(false);
+    });
+
+    it('rejects an empty string', () => {
+      ofcCoin.isValidPub('').should.equal(false);
+    });
+
+    it('rejects a raw (non-SPKI) secp256k1 hex public key', () => {
+      ofcCoin.isValidPub('02' + '11'.repeat(32)).should.equal(false);
+    });
   });
 });

modules/sdk-core/src/coins/ofc.ts

@@ -1,7 +1,7 @@
 /**
  * @prettier
  */
-import { randomBytes } from 'crypto';
+import { createPublicKey, randomBytes } from 'crypto';
 import { bip32 } from '@bitgo/utxo-lib';
 import {
   AuditDecryptedKeyParams,
@@ -70,12 +70,26 @@ export class Ofc extends BaseCoin {
   /**
    * Return boolean indicating whether input is valid public key for the coin.
    *
+   * OFC wallets hold keys in two formats: the user key is a BIP-32 base58 xpub,
+   * and the BitGo key (source: 'bitgo') is a base64-encoded SPKI (DER-wrapped)
+   * secp256k1 public key. Both are valid.
+   *
    * @param {String} pub the pub to be checked
    * @returns {Boolean} is it valid?
    */
   isValidPub(pub: string): boolean {
+    // BIP-32 base58 xpub (user key)
+    try {
+      if (bip32.fromBase58(pub).isNeutered()) {
+        return true;
+      }
+    } catch (e) {
+      // not a BIP-32 key; fall through to the SPKI check
+    }
+    // base64-encoded SPKI DER secp256k1 public key (BitGo key)
     try {
-      return bip32.fromBase58(pub).isNeutered();
+      const key = createPublicKey({ key: Buffer.from(pub, 'base64'), format: 'der', type: 'spki' });
+      return key.asymmetricKeyType === 'ec' && key.asymmetricKeyDetails?.namedCurve === 'secp256k1';
     } catch (e) {
       return false;
     }