diff --git a/src/__tests__/api/advancedWalletManager/signMpcTransaction.test.ts b/src/__tests__/api/advancedWalletManager/signMpcTransaction.test.ts index a0055bc1..9d3bede0 100644 --- a/src/__tests__/api/advancedWalletManager/signMpcTransaction.test.ts +++ b/src/__tests__/api/advancedWalletManager/signMpcTransaction.test.ts @@ -186,13 +186,13 @@ describe('signMpcTransaction', () => { .send(rInput); rResponse.status.should.equal(200); - rResponse.body.should.have.property('rShare'); + rResponse.body.rShare.should.have.only.keys('i', 'j', 'r', 'R', 'commitment'); + rResponse.body.rShare.i.should.equal(3); + rResponse.body.rShare.j.should.equal(1); rKeyProviderNock.done(); decryptDataKeyNock.done(); - // Continue with G share test using the returned rShare - const rShare = rResponse.body.rShare; const derivationPath = 'm/0'; const tMessage = 'testMessage'; @@ -227,7 +227,8 @@ describe('signMpcTransaction', () => { source: 'user', pub: 'DSqMPMsMAbEJVNuPKv1ZFdzt6YvJaDPDddfeW7ajtqds', txRequest: mockTxRequest, - userToBitgoRShare: rShare, + encryptedUserToBitgoRShare, + encryptedDataKey, bitgoToUserRShare: signatureShareRec, bitgoToUserCommitment: bitgoToUserCommitmentShare, }; @@ -238,6 +239,10 @@ describe('signMpcTransaction', () => { .query({ source: 'user' }) .reply(200, mockKeyProviderResponse); + const gDecryptDataKeyNock = nock(keyProviderUrl) + .post('/decryptDataKey') + .reply(200, mockDecryptedDataKeyResponse); + const gResponse = await agent .post(`/api/${coin}/mpc/sign/g`) .set('Authorization', `Bearer ${accessToken}`) @@ -251,6 +256,33 @@ describe('signMpcTransaction', () => { gResponse.body.gShare.should.have.property('R'); gKeyProviderNock.done(); + gDecryptDataKeyNock.done(); + }); + + it('should fail G share generation without the encrypted commitment round state', async () => { + const input = { + source: 'user', + pub: 'DSqMPMsMAbEJVNuPKv1ZFdzt6YvJaDPDddfeW7ajtqds', + txRequest: mockTxRequest, + bitgoToUserRShare: { from: 'bitgo', to: 'user', share: 'r-share' }, + bitgoToUserCommitment: { from: 'bitgo', to: 'user', share: 'commitment' }, + }; + + const keyProviderNock = nock(keyProviderUrl) + .get(`/key/${input.pub}`) + .query({ source: 'user' }) + .reply(200, { prv: 'mock-prv', pub: input.pub, source: 'user', type: 'independent' }); + + const response = await agent + .post(`/api/${coin}/mpc/sign/g`) + .set('Authorization', `Bearer ${accessToken}`) + .send(input); + + response.status.should.equal(500); + response.body.details.should.equal( + 'encryptedUserToBitgoRShare is required for G share generation', + ); + keyProviderNock.done(); }); it('should fail when key provider returns no private key', async () => { diff --git a/src/__tests__/api/master/eddsa.test.ts b/src/__tests__/api/master/eddsa.test.ts index eac7bdcc..fdc0dd8a 100644 --- a/src/__tests__/api/master/eddsa.test.ts +++ b/src/__tests__/api/master/eddsa.test.ts @@ -117,6 +117,7 @@ describe('Eddsa Signing Handler', () => { const offerRShareNock = nock(bitgoApiUrl) .post( `/api/v2/wallet/${walletId}/txrequests/test-tx-request-id/transactions/0/signatureshares`, + (body) => body.signatureShare?.share === 'r-shareR-share', ) .matchHeader('any', () => true) .reply(200, { @@ -203,19 +204,18 @@ describe('Eddsa Signing Handler', () => { const signMpcRShareNockAwm = nock(advancedWalletManagerUrl) .post(`/api/${coin}/mpc/sign/r`) .reply(200, { - rShare: { - rShares: [ - { r: 'r-share', R: 'R-share' }, - { r: 'r-share-2', R: 'R-share-2' }, - { r: 'r-share-3', R: 'R-share-3' }, - { r: 'r-share-4', R: 'R-share-4', i: 3, j: 1 }, - ], - }, + rShare: { i: 3, j: 1, r: 'r-share', R: 'R-share', commitment: 'commitment' }, }); // Mock MPC G-share signing const signMpcGShareNockAwm = nock(advancedWalletManagerUrl) - .post(`/api/${coin}/mpc/sign/g`) + .post(`/api/${coin}/mpc/sign/g`, (body) => { + return ( + body.encryptedUserToBitgoRShare?.share === 'encrypted-user-to-bitgo-r-share' && + body.encryptedDataKey === 'test-encrypted-data-key' && + body.userToBitgoRShare === undefined + ); + }) .reply(200, { gShare: { r: 'r', diff --git a/src/__tests__/api/master/sendMany.test.ts b/src/__tests__/api/master/sendMany.test.ts index 50e6e8f1..324e2b40 100644 --- a/src/__tests__/api/master/sendMany.test.ts +++ b/src/__tests__/api/master/sendMany.test.ts @@ -663,18 +663,17 @@ describe('POST /api/v1/:coin/advancedwallet/:walletId/sendMany', () => { const signMpcRShareNockAwm = nock(advancedWalletManagerUrl) .post(`/api/${coin}/mpc/sign/r`) .reply(200, { - rShare: { - rShares: [ - { r: 'r-share', R: 'R-share' }, - { r: 'r-share-2', R: 'R-share-2' }, - { r: 'r-share-3', R: 'R-share-3' }, - { r: 'r-share-4', R: 'R-share-4', i: 3, j: 1 }, - ], - }, + rShare: { i: 3, j: 1, r: 'r-share', R: 'R-share', commitment: 'commitment' }, }); const signMpcGShareNockAwm = nock(advancedWalletManagerUrl) - .post(`/api/${coin}/mpc/sign/g`) + .post(`/api/${coin}/mpc/sign/g`, (body) => { + return ( + body.encryptedUserToBitgoRShare?.share === 'encrypted-user-to-bitgo-r-share' && + body.encryptedDataKey === 'test-encrypted-data-key' && + body.userToBitgoRShare === undefined + ); + }) .reply(200, { gShare: { r: 'r', gamma: 'gamma', i: 1, j: 3, n: 4 }, }); diff --git a/src/__tests__/api/master/signAndSendTxRequest.test.ts b/src/__tests__/api/master/signAndSendTxRequest.test.ts index 88f6d564..9fbbfc9c 100644 --- a/src/__tests__/api/master/signAndSendTxRequest.test.ts +++ b/src/__tests__/api/master/signAndSendTxRequest.test.ts @@ -294,18 +294,17 @@ describe('POST /api/v1/:coin/advancedwallet/:walletId/txrequest/:txRequestId/sig const signMpcRShareNockAwm = nock(advancedWalletManagerUrl) .post(`/api/${eddsaCoin}/mpc/sign/r`) .reply(200, { - rShare: { - rShares: [ - { r: 'r-share', R: 'R-share' }, - { r: 'r-share-2', R: 'R-share-2' }, - { r: 'r-share-3', R: 'R-share-3' }, - { r: 'r-share-4', R: 'R-share-4', i: 3, j: 1 }, - ], - }, + rShare: { i: 3, j: 1, r: 'r-share', R: 'R-share', commitment: 'commitment' }, }); const signMpcGShareNockAwm = nock(advancedWalletManagerUrl) - .post(`/api/${eddsaCoin}/mpc/sign/g`) + .post(`/api/${eddsaCoin}/mpc/sign/g`, (body) => { + return ( + body.encryptedUserToBitgoRShare?.share === 'encrypted-user-to-bitgo-r-share' && + body.encryptedDataKey === 'test-encrypted-data-key' && + body.userToBitgoRShare === undefined + ); + }) .reply(200, { gShare: { r: 'r', gamma: 'gamma', i: 1, j: 3, n: 4 } }); const response = await agent diff --git a/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts b/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts index eb29758b..5ccfe288 100644 --- a/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts +++ b/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts @@ -1,4 +1,7 @@ -import { AwmApiSpecRouteRequest } from '../routers/advancedWalletManagerApiSpec'; +import { + AwmApiSpecRouteRequest, + EddsaUserToBitgoRShare, +} from '../routers/advancedWalletManagerApiSpec'; import { decryptDataKey, generateDataKey, retrieveKeyProviderPrvKey } from './utils/utils'; import logger from '../../shared/logger'; import { @@ -8,6 +11,7 @@ import { EddsaUtils, EncryptedSignerShareRecord, GShare, + ShareKeyPosition, SignatureShareRecord, SignShare, TxRequest, @@ -67,7 +71,6 @@ interface EddsaSigningParams { prv: string; encryptedDataKey?: string; bitgoToUserRShare?: SignatureShareRecord; - userToBitgoRShare?: SignShare; encryptedUserToBitgoRShare?: EncryptedSignerShareRecord; bitgoToUserCommitment?: CommitmentShareRecord; bitgoPublicGpgKey?: string; @@ -112,7 +115,6 @@ export async function signMpcTransaction(req: AwmApiSpecRouteRequest<'v1.mpc.sig prv, encryptedDataKey, bitgoToUserRShare: req.decoded.bitgoToUserRShare, - userToBitgoRShare: req.decoded.userToBitgoRShare, encryptedUserToBitgoRShare: req.decoded.encryptedUserToBitgoRShare, bitgoToUserCommitment: req.decoded.bitgoToUserCommitment, bitgoPublicGpgKey: req.decoded.bitgoPublicGpgKey, @@ -146,7 +148,7 @@ async function handleEddsaSigning( userToBitgoCommitment?: CommitmentShareRecord; encryptedSignerShare?: EncryptedSignerShareRecord; encryptedUserToBitgoRShare?: EncryptedSignerShareRecord; - rShare?: SignShare; + rShare?: EddsaUserToBitgoRShare; gShare?: GShare; encryptedDataKey?: string; }> { @@ -157,7 +159,6 @@ async function handleEddsaSigning( prv, encryptedDataKey, bitgoToUserRShare, - userToBitgoRShare, encryptedUserToBitgoRShare, bitgoToUserCommitment, bitgoPublicGpgKey, @@ -184,32 +185,32 @@ async function handleEddsaSigning( }; } case ShareType.R: { - if (!encryptedUserToBitgoRShare) { - throw new Error('encryptedUserToBitgoRShare is required for R share generation'); - } - if (!encryptedDataKey) { - throw new Error( - 'encryptedDataKey from commitment share generation round is required for R share generation', - ); - } - const plaintextDataKey = await decryptDataKey({ encryptedDataKey, cfg }); - const rShareParams: RShareParams = { + const userSignShare = await decryptUserSignShare(eddsaUtils, cfg, { + shareType: ShareType.R, txRequest, - walletPassphrase: plaintextDataKey, + encryptedDataKey, encryptedUserToBitgoRShare, - }; - return await eddsaUtils.createRShareFromTxRequest(rShareParams); + }); + const rShare = userSignShare.rShares[ShareKeyPosition.BITGO]; + if (!rShare) { + throw new Error('Unable to find userToBitgo rShare in userSignShare'); + } + const { i, j, r, R, commitment } = rShare; + return { rShare: { i, j, r, R, commitment } }; } case ShareType.G: { if (!bitgoToUserRShare) { throw new Error('bitgoToUserRShare is required for G share generation'); } - if (!userToBitgoRShare) { - throw new Error('userToBitgoRShare is required for G share generation'); - } if (!bitgoToUserCommitment) { throw new Error('bitgoToUserCommitment is required for G share generation'); } + const userToBitgoRShare = await decryptUserSignShare(eddsaUtils, cfg, { + shareType: ShareType.G, + txRequest, + encryptedDataKey, + encryptedUserToBitgoRShare, + }); const gShareParams: GShareParams = { txRequest, prv, @@ -227,6 +228,36 @@ async function handleEddsaSigning( } } +async function decryptUserSignShare( + eddsaUtils: EddsaUtils, + cfg: AdvancedWalletManagerConfig, + params: { + shareType: ShareType; + txRequest: TxRequest; + encryptedDataKey?: string; + encryptedUserToBitgoRShare?: EncryptedSignerShareRecord; + }, +): Promise { + const { txRequest, encryptedDataKey, encryptedUserToBitgoRShare } = params; + const shareType = params.shareType.toUpperCase(); + if (!encryptedUserToBitgoRShare) { + throw new Error(`encryptedUserToBitgoRShare is required for ${shareType} share generation`); + } + if (!encryptedDataKey) { + throw new Error( + `encryptedDataKey from commitment share generation round is required for ${shareType} share generation`, + ); + } + const plaintextDataKey = await decryptDataKey({ encryptedDataKey, cfg }); + const rShareParams: RShareParams = { + txRequest, + walletPassphrase: plaintextDataKey, + encryptedUserToBitgoRShare, + }; + const { rShare } = await eddsaUtils.createRShareFromTxRequest(rShareParams); + return rShare; +} + async function handleEcdsaMpcV2Signing( bitgo: BitGoAPI, cfg: AdvancedWalletManagerConfig, diff --git a/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts b/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts index 068bf06b..958ce036 100644 --- a/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts +++ b/src/advancedWalletManager/routers/advancedWalletManagerApiSpec.ts @@ -121,7 +121,6 @@ const SignMpcRequest = { pub: t.string, txRequest: t.any, bitgoToUserRShare: t.union([t.undefined, t.any]), - userToBitgoRShare: t.union([t.undefined, t.any]), encryptedUserToBitgoRShare: t.union([t.undefined, t.any]), bitgoToUserCommitment: t.union([t.undefined, t.any]), bitgoPublicGpgKey: t.union([t.undefined, t.string]), @@ -133,6 +132,15 @@ const SignMpcRequest = { encryptedRound2Session: t.union([t.undefined, t.string]), }; +export const EddsaUserToBitgoRShare = t.type({ + i: t.number, + j: t.number, + r: t.string, + R: t.string, + commitment: t.string, +}); +export type EddsaUserToBitgoRShare = t.TypeOf; + // Response type for /mpc/sign endpoint const SignMpcResponse: HttpResponse = { // Response type for MPC transaction signing @@ -146,7 +154,7 @@ const SignMpcResponse: HttpResponse = { }), // EDDSA R share response t.type({ - rShare: t.any, + rShare: EddsaUserToBitgoRShare, }), // EDDSA G share response t.type({ diff --git a/src/masterBitgoExpress/clients/advancedWalletManagerClient.ts b/src/masterBitgoExpress/clients/advancedWalletManagerClient.ts index a576a31b..3b1a4008 100644 --- a/src/masterBitgoExpress/clients/advancedWalletManagerClient.ts +++ b/src/masterBitgoExpress/clients/advancedWalletManagerClient.ts @@ -14,7 +14,6 @@ import { MPCTxs, SignatureShareRecord, SignedTransaction, - SignShare, TransactionPrebuild, TxRequest, } from '@bitgo-beta/sdk-core'; @@ -32,6 +31,7 @@ import { AdvancedWalletManagerApiSpec } from '../../advancedWalletManager/router import { PingResponseType, VersionResponseType } from '../../types/health'; import { extractTransactionRequestInfo } from '../../shared/transactionUtils'; import { + EddsaUserToBitgoRShare, KeyShareType, MpcFinalizeResponseType, MpcInitializeResponseType, @@ -123,22 +123,24 @@ export interface SignMpcCommitmentResponse { encryptedDataKey: string; } -interface SignMpcRShareParams { +type EddsaMpcCommitmentSigningState = Pick< + SignMpcCommitmentResponse, + 'encryptedUserToBitgoRShare' | 'encryptedDataKey' +>; + +interface SignMpcRShareParams extends EddsaMpcCommitmentSigningState { txRequest: TxRequest; - encryptedUserToBitgoRShare: EncryptedSignerShareRecord; - encryptedDataKey: string; source: 'user' | 'backup'; pub: string; } interface SignMpcRShareResponse { - rShare: SignShare; + rShare: EddsaUserToBitgoRShare; } -interface SignMpcGShareParams { +interface SignMpcGShareParams extends EddsaMpcCommitmentSigningState { txRequest: TxRequest; bitgoToUserRShare: SignatureShareRecord; - userToBitgoRShare: SignShare; bitgoToUserCommitment: CommitmentShareRecord; source: 'user' | 'backup'; pub: string; diff --git a/src/masterBitgoExpress/handlers/eddsa.ts b/src/masterBitgoExpress/handlers/eddsa.ts index 8802d39f..3ea83c85 100644 --- a/src/masterBitgoExpress/handlers/eddsa.ts +++ b/src/masterBitgoExpress/handlers/eddsa.ts @@ -11,6 +11,7 @@ import { CustomCommitmentGeneratingFunction, CustomRShareGeneratingFunction, CustomGShareGeneratingFunction, + ShareKeyPosition, } from '@bitgo-beta/sdk-core'; import { AdvancedWalletManagerClient, @@ -64,12 +65,15 @@ export function createEddsaCustomSigningFunctions( source, pub: commonKeychain, }); - return { rShare: response.rShare }; + const rShare: SignShare = { + xShare: { i: ShareKeyPosition.USER, y: '', u: '', r: '', R: '' }, + rShares: { [ShareKeyPosition.BITGO]: { ...response.rShare, u: '' } }, + }; + return { rShare }; }; const customGShareGenerator: CustomGShareGeneratingFunction = async (params: { txRequest: TxRequest; - userToBitgoRShare: SignShare; bitgoToUserRShare: SignatureShareRecord; bitgoToUserCommitment: CommitmentShareRecord; }) => { @@ -79,7 +83,8 @@ export function createEddsaCustomSigningFunctions( const response = await awmClient.signMpcGShare({ txRequest: params.txRequest, bitgoToUserRShare: params.bitgoToUserRShare, - userToBitgoRShare: params.userToBitgoRShare, + encryptedUserToBitgoRShare: commitmentResponse.encryptedUserToBitgoRShare, + encryptedDataKey: commitmentResponse.encryptedDataKey, bitgoToUserCommitment: params.bitgoToUserCommitment, source, pub: commonKeychain,