diff --git a/src/__tests__/api/enclaved/postIndependentKey.test.ts b/src/__tests__/api/enclaved/postIndependentKey.test.ts index 34a1f1a8..0be22196 100644 --- a/src/__tests__/api/enclaved/postIndependentKey.test.ts +++ b/src/__tests__/api/enclaved/postIndependentKey.test.ts @@ -6,6 +6,10 @@ import { app as enclavedApp } from '../../../enclavedApp'; import { AppMode, EnclavedConfig, TlsMode } from '../../../shared/types'; import express from 'express'; +import * as sinon from 'sinon'; +import coinFactory from '../../../shared/coinFactory'; +import { BaseCoin } from '@bitgo-beta/sdk-core'; + describe('postIndependentKey', () => { let cfg: EnclavedConfig; let app: express.Application; @@ -76,4 +80,24 @@ describe('postIndependentKey', () => { response.status.should.equal(400); }); + + it('should fail if there is an error in creating the public and private key pairs', async () => { + const coinStub = sinon.stub(coinFactory, 'getCoin').returns( + Promise.resolve({ + keychains: () => ({ + create: () => ({}), + }), + } as unknown as BaseCoin), + ); + + const response = await agent + .post(`/api/${coin}/key/independent`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(500); + response.body.should.have.property('details', 'BitGo SDK failed to create public key'); + + coinStub.restore(); + }); }); diff --git a/src/__tests__/api/enclaved/postMpcV2Key.test.ts b/src/__tests__/api/enclaved/postMpcV2Key.test.ts index 071a5c8f..01c1b2d1 100644 --- a/src/__tests__/api/enclaved/postMpcV2Key.test.ts +++ b/src/__tests__/api/enclaved/postMpcV2Key.test.ts @@ -18,7 +18,7 @@ describe('postMpcV2Key', () => { // test config const kmsUrl = 'http://kms.invalid'; - const coin = 'tsol'; + const coin = 'hteth'; const accessToken = 'test-token'; // sinon stubs @@ -48,15 +48,7 @@ describe('postMpcV2Key', () => { agent = request.agent(app); }); - afterEach(() => { - nock.cleanAll(); - }); - - after(() => { - configStub.restore(); - }); - - it('should be able to create a new MPC V2 wallet', async () => { + beforeEach(() => { // nocks for KMS responses nock(kmsUrl) .post(`/generateDataKey`) @@ -93,7 +85,17 @@ describe('postMpcV2Key', () => { .persist(); nock(kmsUrl).post(`/postKey`).reply(200, {}).persist(); + }); + afterEach(() => { + nock.cleanAll(); + }); + + after(() => { + configStub.restore(); + }); + + it('should be able to create a new MPC V2 wallet', async () => { // mocking bitgo's GPG key generation session const bitgoGpgKey = await bitgoSdk.generateGPGKeyPair('secp256k1'); const bitgoGpgPub = { @@ -470,4 +472,255 @@ describe('postMpcV2Key', () => { ); userFinalizeResponse.body.commonKeychain.should.equal(bitgoCommonKeychain); }); + + it('should throw an error if round number is invalid', async () => { + const response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: '', + encryptedDataKey: '', + round: 5, + p2pMessages: { + bitgo: {}, + counterParty: {}, + }, + }); + + response.status.should.equal(400); + response.body.should.have.property('details', 'Round must be between 1 and 4'); + }); + + it('should throw error if round number is not sequential', async () => { + const userInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + const backupInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'backup' }); + + // round 1 + const userRound1Response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userInitResponse.body.encryptedData, + encryptedDataKey: userInitResponse.body.encryptedDataKey, + round: 1, + bitgoGpgPub: userInitResponse.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + }); + + // round 3 without round 2 + const skipRoundResponse = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userRound1Response.body.encryptedData, + encryptedDataKey: userRound1Response.body.encryptedDataKey, + round: 3, + bitgoGpgPub: userRound1Response.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + p2pMessages: { + bitgo: {}, + counterParty: {}, + }, + }); + + skipRoundResponse.status.should.equal(422); + skipRoundResponse.body.should.have.property('details', 'Round mismatch: expected 2, got 3'); + + // repeating round 1 + const repeatRoundResponse = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userRound1Response.body.encryptedData, + encryptedDataKey: userRound1Response.body.encryptedDataKey, + round: 1, + bitgoGpgPub: userRound1Response.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + }); + + repeatRoundResponse.status.should.equal(422); + repeatRoundResponse.body.should.have.property('details', 'Round mismatch: expected 2, got 1'); + }); + + it('should throw error if broadcastMessages or p2pMessages does not contain all required messages', async () => { + const userInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + const backupInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'backup' }); + + // round 1 + const userRound1Response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userInitResponse.body.encryptedData, + encryptedDataKey: userInitResponse.body.encryptedDataKey, + round: 1, + bitgoGpgPub: userInitResponse.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + }); + + // round 2 with missing broadcastMessages + const response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userInitResponse.body.encryptedData, + encryptedDataKey: userInitResponse.body.encryptedDataKey, + round: 2, + }); + + response.status.should.equal(400); + response.body.should.have.property( + 'details', + 'At least one of broadcastMessages or p2pMessages must be provided', + ); + + // round 2 with missing p2pMessages + const response2 = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userRound1Response.body.encryptedData, + encryptedDataKey: userRound1Response.body.encryptedDataKey, + round: 2, + p2pMessages: {}, + }); + + response2.status.should.equal(400); + response2.body.should.have.property( + 'details', + 'p2pMessages did not contain all required messages', + ); + + // round 2 with missing broadcastMessages + const response3 = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userRound1Response.body.encryptedData, + encryptedDataKey: userRound1Response.body.encryptedDataKey, + round: 2, + broadcastMessages: {}, + }); + + response3.status.should.equal(400); + response3.body.should.have.property( + 'details', + 'broadcastMessages did not contain all required messages', + ); + }); + + it('should throw error if counterPartyGpgPub does not match expected value', async () => { + const userInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + const backupInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'backup' }); + + // round 1 + const userRound1Response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userInitResponse.body.encryptedData, + encryptedDataKey: userInitResponse.body.encryptedDataKey, + round: 1, + bitgoGpgPub: userInitResponse.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + }); + + // round 2 with incorrect counterPartyGpgPub + const response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userRound1Response.body.encryptedData, + encryptedDataKey: userRound1Response.body.encryptedDataKey, + round: 2, + bitgoGpgPub: userRound1Response.body.gpgPub, + counterPartyGpgPub: 'incorrect-gpg-pub', + broadcastMessages: { + bitgo: {}, + counterParty: {}, + }, + }); + + response.status.should.equal(422); + response.body.should.have.property( + 'details', + `Counterparty GPG public key mismatch: expected ${backupInitResponse.body.gpgPub}, got incorrect-gpg-pub`, + ); + }); + + it('should throw error if encrypted state misses the required messages', async () => { + const userInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + const backupInitResponse = await agent + .post(`/api/${coin}/mpcv2/initialize`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'backup' }); + + // round 1 + const userRound1Response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: userInitResponse.body.encryptedData, + encryptedDataKey: userInitResponse.body.encryptedDataKey, + round: 1, + bitgoGpgPub: userInitResponse.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + }); + + // round 2 with missing p2pMessages + const response = await agent + .post(`/api/${coin}/mpcv2/round`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + encryptedData: '', + encryptedDataKey: userRound1Response.body.encryptedDataKey, + round: 2, + bitgoGpgPub: userRound1Response.body.gpgPub, + counterPartyGpgPub: backupInitResponse.body.gpgPub, + p2pMessages: {}, + }); + + response.status.should.equal(400); + response.body.should.have.property( + 'details', + 'p2pMessages did not contain all required messages', + ); + }); }); diff --git a/src/__tests__/api/master/generateWallet.test.ts b/src/__tests__/api/master/generateWallet.test.ts index 4fec691a..1d281828 100644 --- a/src/__tests__/api/master/generateWallet.test.ts +++ b/src/__tests__/api/master/generateWallet.test.ts @@ -1174,4 +1174,18 @@ describe('POST /api/:coin/wallet/generate', () => { response2.status.should.equal(400); }); + + it('should fail when coin does not support TSS', async () => { + const response = await agent + .post(`/api/tbtc/wallet/generate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + label: 'test_wallet', + enterprise: 'test_enterprise', + multisigType: 'tss', + }); + + response.status.should.equal(400); + response.body.details.should.equal('MPC wallet generation is not supported for coin tbtc'); + }); }); diff --git a/src/api/enclaved/handlers/ecdsaMPCv2Round.ts b/src/api/enclaved/handlers/ecdsaMPCv2Round.ts index c2a9dd6a..4d088af4 100644 --- a/src/api/enclaved/handlers/ecdsaMPCv2Round.ts +++ b/src/api/enclaved/handlers/ecdsaMPCv2Round.ts @@ -7,6 +7,7 @@ import { import { MPCv2PartiesEnum } from '@bitgo-beta/sdk-core/dist/src/bitgo/utils/tss/ecdsa'; import { KmsClient } from '../../../kms/kmsClient'; import logger from '../../../logger'; +import { BadRequestError, ValidationError } from '../../../shared/errors'; export async function ecdsaMPCv2Round( req: EnclavedApiSpecRouteRequest<'v1.mpcv2.round', 'post'>, @@ -21,19 +22,19 @@ export async function ecdsaMPCv2Round( // sanity checks if (round < 1 || round > 4) { - throw new Error('Round must be between 1 and 4'); + throw new BadRequestError('Round must be between 1 and 4'); } if (!broadcastMessages && !p2pMessages && round > 1) { - throw new Error('At least one of broadcastMessages or p2pMessages must be provided'); + throw new BadRequestError('At least one of broadcastMessages or p2pMessages must be provided'); } - if (broadcastMessages && Object.keys(broadcastMessages).length === 0) { - throw new Error('broadcastMessages did not contain all required messages'); + if (broadcastMessages && (!broadcastMessages.bitgo || !broadcastMessages.counterParty)) { + throw new BadRequestError('broadcastMessages did not contain all required messages'); } - if (p2pMessages && Object.keys(p2pMessages).length === 0) { - throw new Error('p2pMessages did not contain all required messages'); + if (p2pMessages && (!p2pMessages.bitgo || !p2pMessages.counterParty)) { + throw new BadRequestError('p2pMessages did not contain all required messages'); } // fetch previous state of execution @@ -52,8 +53,8 @@ export async function ecdsaMPCv2Round( partyId: MPCv2PartiesEnum.BITGO, }; } else if (bitgoGpgPubInput && state.bitgoGpgPub.gpgKey !== bitgoGpgPubInput) { - throw new Error( - `BitGo GPG public key mismatch: expected ${state.bitgoGpgPub}, got ${bitgoGpgPubInput}`, + throw new ValidationError( + `BitGo GPG public key mismatch: expected ${state.bitgoGpgPub.gpgKey}, got ${bitgoGpgPubInput}`, ); } @@ -66,19 +67,19 @@ export async function ecdsaMPCv2Round( counterPartyGpgPubInput && state.counterPartyGpgPub.gpgKey !== counterPartyGpgPubInput ) { - throw new Error( - `Counterparty GPG public key mismatch: expected ${state.counterPartyGpgPub}, got ${counterPartyGpgPubInput}`, + throw new ValidationError( + `Counterparty GPG public key mismatch: expected ${state.counterPartyGpgPub.gpgKey}, got ${counterPartyGpgPubInput}`, ); } if (state.round !== round) { - throw new Error(`Round mismatch: expected ${state.round}, got ${round}`); + throw new ValidationError(`Round mismatch: expected ${state.round}, got ${round}`); } const { sourceGpgPrv, bitgoGpgPub, counterPartyGpgPub, sessionData } = state; // restore session data and cast necessary fields into Uint8Array if (!sessionData && round > 1) { - throw new Error('Session data is missing for round greater than 1'); + throw new ValidationError('Session data is missing for round greater than 1'); } else if (sessionData) { sessionData.dkgSessionBytes = new Uint8Array(Object.values(sessionData.dkgSessionBytes)); sessionData.chainCodeCommitment = new Uint8Array( diff --git a/src/api/enclaved/mpcFinalize.ts b/src/api/enclaved/mpcFinalize.ts index ed6e4afd..dad86daf 100644 --- a/src/api/enclaved/mpcFinalize.ts +++ b/src/api/enclaved/mpcFinalize.ts @@ -78,7 +78,6 @@ export async function eddsaFinalize( chaincode: bitgoToSourcePrivateShare.slice(64), }; - // TOOD: clean up, probably doign unnecessary transformations const counterPartyToSourcePrivateShare = await gpgDecrypt( counterPartyToSourceKeyShare.privateShare, sourceGpgPrv, diff --git a/src/api/enclaved/mpcInitialize.ts b/src/api/enclaved/mpcInitialize.ts index bc7126ca..887e7bbd 100644 --- a/src/api/enclaved/mpcInitialize.ts +++ b/src/api/enclaved/mpcInitialize.ts @@ -32,7 +32,7 @@ export async function eddsaInitialize( const sourceIndex = source === 'user' ? 1 : 2; const counterPartyIndex = source === 'user' ? 2 : 1; const bitgoIndex = 3; - const keyShare: bitgoSdk.KeyShare = MPC.keyShare(sourceIndex, m, n); // encrypt this and send it in payload? + const keyShare: bitgoSdk.KeyShare = MPC.keyShare(sourceIndex, m, n); const sourceGpgKey = await bitgoSdk.generateGPGKeyPair('secp256k1'); // source to source private share diff --git a/src/api/master/handlers/generateWallet.ts b/src/api/master/handlers/generateWallet.ts index 4e7f803e..66837e83 100644 --- a/src/api/master/handlers/generateWallet.ts +++ b/src/api/master/handlers/generateWallet.ts @@ -2,7 +2,6 @@ import { AddKeychainOptions, Keychain, KeychainsTriplet, - NotImplementedError, promiseProps, RequestTracer, SupplementGenerateWalletOptions, @@ -14,6 +13,7 @@ import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec'; import { orchestrateEcdsaKeyGen } from './ecdsaMPCv2'; import { orchestrateEddsaKeyGen } from './eddsa'; import coinFactory from '../../../shared/coinFactory'; +import { BadRequestError } from '../../../shared/errors'; /** * Request handler for generating a wallet on-premises. @@ -144,7 +144,7 @@ async function handleGenerateOnPremMpcWallet( const enclavedExpressClient = req.enclavedExpressClient; if (!baseCoin.supportsTss()) { - throw new NotImplementedError( + throw new BadRequestError( `MPC wallet generation is not supported for coin ${req.decoded.coin}`, ); } @@ -168,7 +168,7 @@ async function handleGenerateOnPremMpcWallet( if (!_.isUndefined(enterprise)) { if (!_.isString(enterprise)) { - throw new Error('invalid enterprise argument, expecting string'); + throw new BadRequestError('invalid enterprise argument, expecting string'); } walletParams.enterprise = enterprise; } diff --git a/src/api/master/routers/masterApiSpec.ts b/src/api/master/routers/masterApiSpec.ts index 02ee6e1b..c6e95886 100644 --- a/src/api/master/routers/masterApiSpec.ts +++ b/src/api/master/routers/masterApiSpec.ts @@ -233,6 +233,7 @@ const GenerateWalletResponse: HttpResponse = { // TODO: Get type from public types repo 200: t.any, ...InternalServerErrorResponse, + ...BadRequestResponse, }; // Request type for /generate endpoint diff --git a/src/enclavedBitgoExpress/routers/enclavedApiSpec.ts b/src/enclavedBitgoExpress/routers/enclavedApiSpec.ts index a3585ba0..55d2f43d 100644 --- a/src/enclavedBitgoExpress/routers/enclavedApiSpec.ts +++ b/src/enclavedBitgoExpress/routers/enclavedApiSpec.ts @@ -21,6 +21,7 @@ import { BadRequestResponse, InternalServerErrorResponse, NotImplementedResponse, + UnprocessableEntityResponse, } from '../../shared/errors'; import { postIndependentKey } from '../../api/enclaved/handlers/postIndependentKey'; @@ -449,10 +450,9 @@ export const EnclavedAPiSpec = apiSpec({ }), response: { 200: MpcV2RoundResponseType, - 500: t.type({ - error: t.string, - details: t.string, - }), + ...InternalServerErrorResponse, + ...BadRequestResponse, + ...UnprocessableEntityResponse, }, description: 'Perform a round in the MPC protocol', }),