fix(sdk-lib-mpc): authenticate signatureR in DKLS DSG round 4 messages

modules/express/test/unit/clientRoutes/externalSign.ts

@@ -1045,17 +1045,25 @@ async function signBitgoMPCv2Round3(
   userShare: SignatureShareRecord,
   userGPGPubKey: string
 ): Promise<{ userMsg4: MPCv2SignatureShareRound3Input }> {
-  const parsedSignatureShare = JSON.parse(userShare.share) as MPCv2SignatureShareRound3Input;
+  const parsedSignatureShare = JSON.parse(userShare.share) as MPCv2SignatureShareRound3Input & {
+    data: { msg4: { signatureRSignature?: string } };
+  };
+  const msg4 = parsedSignatureShare.data.msg4;
+  const signatureRAuthMessage =
+    msg4.signatureR && msg4.signatureRSignature
+      ? { message: msg4.signatureR, signature: msg4.signatureRSignature }
+      : undefined;
   const serializedMessages = await DklsComms.decryptAndVerifyIncomingMessages(
     {
       p2pMessages: [],
       broadcastMessages: [
         {
-          from: parsedSignatureShare.data.msg4.from,
+          from: msg4.from,
           payload: {
-            message: parsedSignatureShare.data.msg4.message,
-            signature: parsedSignatureShare.data.msg4.signature,
+            message: msg4.message,
+            signature: msg4.signature,
           },
+          signatureR: signatureRAuthMessage,
         },
       ],
     },

modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts

@@ -129,17 +129,24 @@ export async function getSignatureShareRoundThree(
   if (!userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR?.message) {
     throw Error('signatureR should be defined');
   }
-  const share: MPCv2SignatureShareRound3Input = {
+  if (!userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR?.signature) {
+    throw Error('signatureR signature should be defined');
+  }
+  // signatureRSignature carries the GPG detached signature over signatureR bytes.
+  // It is appended alongside the existing MPCv2SignatureShareRound3Input fields so
+  // the server can authenticate signatureR before using it in combinePartialSignatures.
+  const share = {
     type: 'round3Input',
     data: {
       msg4: {
         from: userToBitGoEncryptedMsg4.broadcastMessages[0].from,
         message: userToBitGoEncryptedMsg4.broadcastMessages[0].payload.message,
         signature: userToBitGoEncryptedMsg4.broadcastMessages[0].payload.signature,
         signatureR: userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR.message,
+        signatureRSignature: userToBitGoEncryptedMsg4.broadcastMessages[0].signatureR.signature,
       },
     },
-  };
+  } as unknown as MPCv2SignatureShareRound3Input;
   return {
     from: partyIdToSignatureShareType(partyId),
     to: partyIdToSignatureShareType(otherSignerPartyId),

modules/sdk-lib-mpc/src/tss/ecdsa-dkls/commsLayer.ts

@@ -162,9 +162,15 @@ export async function decryptAndVerifyIncomingMessages(
         if (!(await verifySignedData(m.payload, pubGpgKey.gpgKey))) {
           throw Error(`Failed to authenticate broadcast message from party: ${m.from}`);
         }
+        if (m.signatureR !== undefined) {
+          if (!(await verifySignedData(m.signatureR, pubGpgKey.gpgKey))) {
+            throw Error(`Failed to authenticate signatureR in broadcast message from party: ${m.from}`);
+          }
+        }
         return {
           from: m.from,
           payload: m.payload.message,
+          signatureR: m.signatureR?.message,
         };
       })
     ),
@@ -211,10 +217,7 @@ export async function encryptAndAuthOutgoingMessages(
           from: m.from,
           payload: await detachSignData(Buffer.from(m.payload, 'base64'), prvGpgKey.gpgKey),
           signatureR: m.signatureR
-            ? {
-                message: m.signatureR,
-                signature: '',
-              }
+            ? await detachSignData(Buffer.from(m.signatureR, 'base64'), prvGpgKey.gpgKey)
             : undefined,
         };
       })

modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsComms.ts

@@ -1,4 +1,9 @@
-import { decryptAndVerifySignedData, encryptAndDetachSignData } from '../../../../src/tss/ecdsa-dkls/commsLayer';
+import {
+  decryptAndVerifySignedData,
+  encryptAndDetachSignData,
+  encryptAndAuthOutgoingMessages,
+  decryptAndVerifyIncomingMessages,
+} from '../../../../src/tss/ecdsa-dkls/commsLayer';
 import * as openpgp from 'openpgp';
 
 describe('DKLS Communication Layer', function () {
@@ -95,3 +100,129 @@ describe('DKLS Communication Layer', function () {
     );
   });
 });
+
+describe('DKLS encryptAndAuthOutgoingMessages / decryptAndVerifyIncomingMessages', function () {
+  let partyAKey: { publicKey: string; privateKey: string };
+  let partyBKey: { publicKey: string; privateKey: string };
+  let tampererKey: { publicKey: string; privateKey: string };
+
+  before(async function () {
+    openpgp.config.rejectCurves = new Set();
+    partyAKey = await openpgp.generateKey({
+      userIDs: [{ name: 'partyA', email: 'a@test.com' }],
+      curve: 'secp256k1',
+    });
+    partyBKey = await openpgp.generateKey({
+      userIDs: [{ name: 'partyB', email: 'b@test.com' }],
+      curve: 'secp256k1',
+    });
+    tampererKey = await openpgp.generateKey({
+      userIDs: [{ name: 'tamperer', email: 'evil@test.com' }],
+      curve: 'secp256k1',
+    });
+  });
+
+  const partyAId = 0;
+  const partyBId = 2;
+
+  it('should sign signatureR and verify it on receive', async function () {
+    const payload = Buffer.from('deadbeef', 'hex').toString('base64');
+    const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
+
+    const encrypted = await encryptAndAuthOutgoingMessages(
+      {
+        p2pMessages: [],
+        broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
+      },
+      [{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
+      [{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
+    );
+
+    const broadcastMsg = encrypted.broadcastMessages[0];
+    broadcastMsg.signatureR!.message.should.equal(signatureRBytes);
+    broadcastMsg.signatureR!.signature.should.not.equal('');
+
+    const decrypted = await decryptAndVerifyIncomingMessages(
+      {
+        p2pMessages: [],
+        broadcastMessages: [broadcastMsg],
+      },
+      [{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
+      [{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
+    );
+
+    decrypted.broadcastMessages[0].signatureR!.should.equal(signatureRBytes);
+  });
+
+  it('should reject a tampered signatureR on receive', async function () {
+    const payload = Buffer.from('deadbeef', 'hex').toString('base64');
+    const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
+
+    const encrypted = await encryptAndAuthOutgoingMessages(
+      {
+        p2pMessages: [],
+        broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
+      },
+      [{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
+      [{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
+    );
+
+    // MITM substitutes signatureR bytes while keeping the original signature
+    const tampered = {
+      ...encrypted.broadcastMessages[0],
+      signatureR: {
+        message: Buffer.from('00000000', 'hex').toString('base64'),
+        signature: encrypted.broadcastMessages[0].signatureR!.signature,
+      },
+    };
+
+    await decryptAndVerifyIncomingMessages(
+      { p2pMessages: [], broadcastMessages: [tampered] },
+      [{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
+      [{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
+    ).should.be.rejectedWith(`Failed to authenticate signatureR in broadcast message from party: ${partyAId}`);
+  });
+
+  it('should reject a signatureR signed by a different key', async function () {
+    const payload = Buffer.from('deadbeef', 'hex').toString('base64');
+    const signatureRBytes = Buffer.from('cafebabe', 'hex').toString('base64');
+
+    const encrypted = await encryptAndAuthOutgoingMessages(
+      {
+        p2pMessages: [],
+        broadcastMessages: [{ from: partyAId, payload, signatureR: signatureRBytes }],
+      },
+      [{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
+      [{ partyId: partyAId, gpgKey: tampererKey.privateKey }] // tampered: signed by wrong key
+    );
+
+    await decryptAndVerifyIncomingMessages(
+      { p2pMessages: [], broadcastMessages: [encrypted.broadcastMessages[0]] },
+      [{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
+      [{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
+    ).should.be.rejectedWith(`Failed to authenticate broadcast message from party: ${partyAId}`);
+  });
+
+  it('should handle broadcast messages without signatureR unchanged', async function () {
+    const payload = Buffer.from('deadbeef', 'hex').toString('base64');
+
+    const encrypted = await encryptAndAuthOutgoingMessages(
+      {
+        p2pMessages: [],
+        broadcastMessages: [{ from: partyAId, payload }],
+      },
+      [{ partyId: partyBId, gpgKey: partyBKey.publicKey }],
+      [{ partyId: partyAId, gpgKey: partyAKey.privateKey }]
+    );
+
+    (encrypted.broadcastMessages[0].signatureR === undefined).should.equal(true);
+
+    const decrypted = await decryptAndVerifyIncomingMessages(
+      { p2pMessages: [], broadcastMessages: [encrypted.broadcastMessages[0]] },
+      [{ partyId: partyAId, gpgKey: partyAKey.publicKey }],
+      [{ partyId: partyBId, gpgKey: partyBKey.privateKey }]
+    );
+
+    (decrypted.broadcastMessages[0].signatureR === undefined).should.equal(true);
+  });
+});