fix(sdk-core): redact EdDSA MPCv1 R share, bind G round to signer-held envelope

modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts

@@ -1,3 +1,4 @@
+import { randomBytes } from 'crypto';
 import * as assert from 'assert';
 import * as sodium from 'libsodium-wrappers-sumo';
 import * as _ from 'lodash';
@@ -19,10 +20,13 @@ import {
   Ed25519BIP32,
   EDDSAUtils,
   Eddsa,
+  EddsaUtils,
   EncryptedSignerShareType,
   ExchangeCommitmentResponse,
   Keychain,
   KeyShare,
+  bigIntFromBufferLE,
+  bigIntToBufferLE,
   RequestTracer,
   RequestType,
   SignatureShareRecord,
@@ -672,10 +676,34 @@ describe('TSS Utils:', async function () {
       userId: 'userId',
     };
 
-    it('v2 R-share round-trip: encrypt via commitment, verify envelope, decrypt via createRShare', async function () {
-      const passphrase = 'test-passphrase';
-      const prv = JSON.stringify(validUserSigningMaterial);
+    const passphrase = 'test-passphrase';
+    const prv = JSON.stringify(validUserSigningMaterial);
+    const otherSignableTxRequest: TxRequest = {
+      ...signingTxRequest,
+      unsignedTxs: [{ ...signingTxRequest.unsignedTxs[0], signableHex: 'deadbe00' }],
+    };
+    const forgedBitgoRecords = () => {
+      // Production nonce pattern (account-lib signShare): reduce random entropy to a scalar.
+      const rB = Eddsa.curve.scalarReduce(bigIntFromBufferLE(randomBytes(64)));
+      const rBPoint = Eddsa.curve.basePointMult(rB);
+      const rBHex = bigIntToBufferLE(rB, 32).toString('hex');
+      const rBPointHex = bigIntToBufferLE(rBPoint, 32).toString('hex');
+      return {
+        bitgoToUserRShare: {
+          from: SignatureShareType.BITGO,
+          to: SignatureShareType.USER,
+          share: rBHex + rBPointHex,
+        },
+        bitgoToUserCommitment: {
+          from: SignatureShareType.BITGO,
+          to: SignatureShareType.USER,
+          type: CommitmentType.COMMITMENT,
+          share: rBPointHex,
+        },
+      };
+    };
 
+    it('v2 signing round-trip: bound R envelope, redacted R reveal, G consumption', async function () {
       // 1. Encrypt prv with v2
       const encryptedPrv = await bitgo.encrypt({
         input: prv,
@@ -684,7 +712,7 @@ describe('TSS Utils:', async function () {
       });
       JSON.parse(encryptedPrv).v.should.equal(2);
 
-      // 2. Create commitment -- R-share should be a v2 envelope
+      // 2. Create commitment -- R-share should be a v2 envelope bound to the signable
       const commitResult = await tssUtils.createCommitmentShareFromTxRequest({
         txRequest: signingTxRequest,
         prv,
@@ -696,16 +724,132 @@ describe('TSS Utils:', async function () {
       const rShareEnvelope = JSON.parse(commitResult.encryptedUserToBitgoRShare.share);
       rShareEnvelope.v.should.equal(2);
       rShareEnvelope.should.have.property('hkdfSalt');
+      rShareEnvelope.should.have.property('adata');
 
-      // 3. Round-trip: decrypt the v2 R-share
+      // 3. R round only reveals the nonce material offered to BitGo
       const { rShare } = await tssUtils.createRShareFromTxRequest({
         txRequest: signingTxRequest,
         walletPassphrase: passphrase,
         encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare,
       });
+      rShare.should.have.property('r').and.be.a.String();
+      rShare.should.have.property('R').and.be.a.String();
+      Object.keys(rShare).sort().should.deepEqual(['R', 'r']);
+
+      // 4. G round consumes the same envelope and produces the user gamma
+      const { bitgoToUserRShare, bitgoToUserCommitment } = forgedBitgoRecords();
+      const gShare = await tssUtils.createGShareFromTxRequest({
+        txRequest: signingTxRequest,
+        prv,
+        walletPassphrase: passphrase,
+        encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare,
+        bitgoToUserRShare,
+        bitgoToUserCommitment,
+        markConsumed: () => undefined,
+      });
+      gShare.should.have.property('gamma').and.be.a.String();
+    });
+
+    it('commitment rejects a caller-supplied non-BitGo GPG public key in pinned envs', async function () {
+      const testEnvBitgo = TestBitGo.decorate(BitGo, { env: 'test' });
+      const testEnvUtils = new EddsaUtils(testEnvBitgo, testEnvBitgo.coin('tsol'));
+      await testEnvUtils
+        .createCommitmentShareFromTxRequest({
+          txRequest: signingTxRequest,
+          prv,
+          walletPassphrase: passphrase,
+          bitgoGpgPubKey: bitgoGpgKey.publicKey,
+        })
+        .should.be.rejectedWith('Invalid BitGo GPG public key');
+    });
+
+    it('R round rejects an envelope bound to a different signable', async function () {
+      const commitResult = await tssUtils.createCommitmentShareFromTxRequest({
+        txRequest: signingTxRequest,
+        prv,
+        walletPassphrase: passphrase,
+        bitgoGpgPubKey: bitgoGpgKey.publicKey,
+      });
+      await tssUtils
+        .createRShareFromTxRequest({
+          txRequest: otherSignableTxRequest,
+          walletPassphrase: passphrase,
+          encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare,
+        })
+        .should.be.rejectedWith('Adata does not match cyphertext adata');
+    });
+
+    it('G round rejects an envelope bound to a different signable', async function () {
+      const commitResult = await tssUtils.createCommitmentShareFromTxRequest({
+        txRequest: signingTxRequest,
+        prv,
+        walletPassphrase: passphrase,
+        bitgoGpgPubKey: bitgoGpgKey.publicKey,
+      });
+      const { bitgoToUserRShare, bitgoToUserCommitment } = forgedBitgoRecords();
+      await tssUtils
+        .createGShareFromTxRequest({
+          txRequest: otherSignableTxRequest,
+          prv,
+          walletPassphrase: passphrase,
+          encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare,
+          bitgoToUserRShare,
+          bitgoToUserCommitment,
+        })
+        .should.be.rejectedWith('Adata does not match cyphertext adata');
+    });
+
+    it('G round rejects a replayed envelope (second use with a different BitGo nonce)', async function () {
+      const commitResult = await tssUtils.createCommitmentShareFromTxRequest({
+        txRequest: signingTxRequest,
+        prv,
+        walletPassphrase: passphrase,
+        bitgoGpgPubKey: bitgoGpgKey.publicKey,
+      });
+      const records = forgedBitgoRecords();
+      let consumed = false;
+      const markConsumed = () => {
+        if (consumed) {
+          throw new Error('encrypted share has already been consumed; restart the signing session');
+        }
+        consumed = true;
+      };
+      await tssUtils.createGShareFromTxRequest({
+        txRequest: signingTxRequest,
+        prv,
+        walletPassphrase: passphrase,
+        encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare,
+        bitgoToUserRShare: records.bitgoToUserRShare,
+        bitgoToUserCommitment: records.bitgoToUserCommitment,
+        markConsumed,
+      });
+      const secondRecords = forgedBitgoRecords();
+      await tssUtils
+        .createGShareFromTxRequest({
+          txRequest: signingTxRequest,
+          prv,
+          walletPassphrase: passphrase,
+          encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare,
+          bitgoToUserRShare: secondRecords.bitgoToUserRShare,
+          bitgoToUserCommitment: secondRecords.bitgoToUserCommitment,
+          markConsumed,
+        })
+        .should.be.rejectedWith('encrypted share has already been consumed');
+    });
 
-      should.exist(rShare.xShare);
-      should.exist(rShare.rShares);
+    it('R round rejects a legacy envelope without a signable binding', async function () {
+      const legacyEnvelope = await bitgo.encrypt({
+        input: JSON.stringify({ xShare: {}, rShares: {} }),
+        password: passphrase,
+        encryptionVersion: 2,
+      });
+      await tssUtils
+        .createRShareFromTxRequest({
+          txRequest: signingTxRequest,
+          walletPassphrase: passphrase,
+          encryptedUserToBitgoRShare: { from: 'user', to: 'bitgo', share: legacyEnvelope, type: 'encryptedRShare' },
+        })
+        .should.be.rejectedWith('Missing adata on encrypted share envelope; restart the signing session');
     });
   });
 
@@ -819,7 +963,7 @@ describe('TSS Utils:', async function () {
           encryptedSignerShare: tssUtils.createUserToBitgoEncryptedSignerShare('encrypted-signer-share'),
           encryptedUserToBitgoRShare: tssUtils.createUserToBitgoEncryptedSignerShare('encrypted-r-share'),
         }),
-        async () => ({ rShare: validUserSignShare }),
+        async () => ({ rShare: { r: validUserSignShare.rShares[3].r, R: validUserSignShare.rShares[3].R } }),
         async () => ({
           i: 1,
           y: 'user-y-share',

modules/bitgo/test/v2/unit/tss/eddsa.ts

@@ -483,11 +483,16 @@ describe('test tss helper functions', function () {
     });
 
     describe('offerUserToBitgoRShare:', async function () {
-      it('should succeed to send Signature Share', async function () {
+      const reveal = {
+        r: validUserSignShare.rShares[3].r,
+        R: validUserSignShare.rShares[3].R,
+      };
+
+      it('should succeed to send the nonce reveal as the signature share', async function () {
         const signatureShare = {
           from: 'user',
           to: 'bitgo',
-          share: validUserSignShare.rShares[3].r + validUserSignShare.rShares[3].R,
+          share: reveal.r + reveal.R,
         } as SignatureShareRecord;
         const nock = await nockSendSignatureShare({
           walletId: wallet.id(),
@@ -499,51 +504,17 @@ describe('test tss helper functions', function () {
           bitgo,
           wallet.id(),
           txRequest.txRequestId,
-          validUserSignShare,
+          reveal,
           'signerShare'
         ).should.be.fulfilled();
         nock.isDone().should.equal(true);
       });
 
-      it('should fail if no rShare is found', async function () {
-        const invalidUserSignShare = _.cloneDeep(validUserSignShare) as any;
-        delete invalidUserSignShare.rShares[3];
-        await offerUserToBitgoRShare(
-          bitgo,
-          wallet.id(),
-          txRequest.txRequestId,
-          invalidUserSignShare,
-          'signerShare'
-        ).should.be.rejectedWith('userToBitgo RShare not found');
-      });
-
-      it('should fail if the rShare found is invalid', async function () {
-        const invalidUserSignShare = _.cloneDeep(validUserSignShare) as any;
-        invalidUserSignShare.rShares[3].i = 1;
-        await offerUserToBitgoRShare(
-          bitgo,
-          wallet.id(),
-          txRequest.txRequestId,
-          invalidUserSignShare,
-          'signerShare'
-        ).should.be.rejectedWith('Invalid RShare, is not from User to Bitgo');
-
-        const invalidUserSignShare2 = _.cloneDeep(validUserSignShare) as any;
-        invalidUserSignShare2.rShares[3].j = 3;
-        await offerUserToBitgoRShare(
-          bitgo,
-          wallet.id(),
-          txRequest.txRequestId,
-          invalidUserSignShare2,
-          'signerShare'
-        ).should.be.rejectedWith('Invalid RShare, is not from User to Bitgo');
-      });
-
       it('should call setRequestTracer', async function () {
         const signatureShare = {
           from: 'user',
           to: 'bitgo',
-          share: validUserSignShare.rShares[3].r + validUserSignShare.rShares[3].R,
+          share: reveal.r + reveal.R,
         } as SignatureShareRecord;
         const nock = await nockSendSignatureShare({
           walletId: wallet.id(),
@@ -558,7 +529,7 @@ describe('test tss helper functions', function () {
           bitgo,
           wallet.id(),
           txRequest.txRequestId,
-          validUserSignShare,
+          reveal,
           'signerShare',
           undefined,
           reqId

modules/express/src/clientRoutes.ts

@@ -32,7 +32,7 @@ import {
   multisigTypes,
   Safes,
   ShareType,
-  SignShare,
+  UserToBitgoRShareReveal,
   SShare,
   TssEcdsaStep1ReturnMessage,
   TssEcdsaStep2ReturnMessage,
@@ -472,6 +472,19 @@ function markConsumedShareClosure(
     getConsumedShareRegistry(registryFilePath).markConsumed(computeConsumedShareKey(shareType, adata));
 }
 
+/** Share-round failures are caller-input failures; surface them as 400s,
+ * passing through errors that already carry an HTTP status. */
+async function rejectInvalidShareRequest<T>(operation: () => Promise<T>): Promise<T> {
+  try {
+    return await operation();
+  } catch (e) {
+    if (typeof e === 'object' && e !== null && 'status' in e && typeof e.status === 'number') {
+      throw e;
+    }
+    throw new ApiResponseError(e instanceof Error ? e.message : String(e), 400);
+  }
+}
+
 async function decryptPrivKey(bg: BitGo, encryptedPrivKey: string, walletPw: string): Promise<string> {
   try {
     return await bg.decrypt({ password: walletPw, input: encryptedPrivKey });
@@ -540,11 +553,19 @@ export async function handleV2GenerateShareTSS(
         const eddsaUtils = new EddsaUtils(bitgo, coin);
         switch (req.decoded.sharetype) {
           case ShareType.Commitment:
-            return await eddsaUtils.createCommitmentShareFromTxRequest(req.body);
+            return await rejectInvalidShareRequest(() => eddsaUtils.createCommitmentShareFromTxRequest(req.body));
           case ShareType.R:
-            return await eddsaUtils.createRShareFromTxRequest(req.body);
-          case ShareType.G:
-            return await eddsaUtils.createGShareFromTxRequest(req.body);
+            return await rejectInvalidShareRequest(() => eddsaUtils.createRShareFromTxRequest(req.body));
+          case ShareType.G: {
+            // Each R-share envelope may be used exactly once; a replayed envelope is
+            // rejected before the share is used (fail-closed).
+            req.body.markConsumed = markConsumedShareClosure(
+              registryFilePath,
+              req.decoded.sharetype,
+              req.body.encryptedUserToBitgoRShare?.share
+            );
+            return await rejectInvalidShareRequest(() => eddsaUtils.createGShareFromTxRequest(req.body));
+          }
           default:
             throw new Error(
               `Share type ${req.decoded.sharetype} not supported, only commitment, G and R share generation is supported.`
@@ -2124,7 +2145,7 @@ export function createCustomCommitmentGenerator(
 }
 
 export function createCustomRShareGenerator(externalSignerUrl: string, coin: string): CustomRShareGeneratingFunction {
-  return async function (params): Promise<{ rShare: SignShare }> {
+  return async function (params): Promise<{ rShare: UserToBitgoRShareReveal }> {
     const { body: rShare } = await retryPromise(
       () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/R`).type('json').send(params),
       (err, tryCount) => {

modules/express/src/expressApp.ts

@@ -5,6 +5,7 @@ import express from 'express';
 import * as path from 'path';
 import * as _ from 'lodash';
 import debugLib from 'debug';
+import { logger } from '@bitgo/logger';
 import * as https from 'https';
 import * as http from 'http';
 import morgan, { token as morganToken } from 'morgan';
@@ -27,8 +28,10 @@ import {
 } from './errors';
 
 import { Environments } from 'bitgo';
-import * as clientRoutes from './clientRoutes';
+
 import { isSameFilePath } from './consumedSharesRegistry';
+import * as clientRoutes from './clientRoutes';
+import { BitgoMpcGpgPubKeys } from '@bitgo/sdk-core';
 import { getBitGoTlsPolicy } from './tlsPolicy';
 
 /**
@@ -257,6 +260,13 @@ function checkPreconditions(config: Config) {
     );
   }
 
+  if (signerMode && !BitgoMpcGpgPubKeys.envRequiresBitgoPubGpgKeyConfig(env)) {
+    logger.warn(
+      `External signer mode is running in env '${env}', which does not enforce the ` +
+        'BitGo GPG public key pin; share encryption will accept any supplied GPG public key.'
+    );
+  }
+
   if (signerMode !== undefined && lightningSignerFileSystemPath !== undefined) {
     throw new LightningSignerConfigError(
       'signerMode and lightningSignerFileSystemPath cannot be set at the same time.'