diff --git a/modules/express/test/unit/clientRoutes/externalSign.ts b/modules/express/test/unit/clientRoutes/externalSign.ts index 9cbc2b18c2..f51e300d55 100644 --- a/modules/express/test/unit/clientRoutes/externalSign.ts +++ b/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, }, ], }, diff --git a/modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts index f813823c36..d65fdc5b9c 100644 --- a/modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/tss/ecdsa/ecdsaMPCv2.ts @@ -129,7 +129,13 @@ 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: { @@ -137,9 +143,10 @@ export async function getSignatureShareRoundThree( 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), diff --git a/modules/sdk-lib-mpc/src/tss/ecdsa-dkls/commsLayer.ts b/modules/sdk-lib-mpc/src/tss/ecdsa-dkls/commsLayer.ts index 3ec85a2906..9116a8cc7a 100644 --- a/modules/sdk-lib-mpc/src/tss/ecdsa-dkls/commsLayer.ts +++ b/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, }; }) diff --git a/modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsComms.ts b/modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsComms.ts index 1190f27caf..9848be695a 100644 --- a/modules/sdk-lib-mpc/test/unit/tss/ecdsa/dklsComms.ts +++ b/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); + }); +});