diff --git a/README.md b/README.md index 874289d3..3fb74f92 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,6 @@ curl -X POST http://localhost:3081/ping/advancedWalletManager | ------------------------------ | ---------------------------------- | ------- | -------- | | `ADVANCED_WALLET_MANAGER_PORT` | Port to listen on | `3080` | ❌ | | `KEY_PROVIDER_URL` | URL to your key provider API implementation | - | ✅ | -| `SIGNING_MODE` | Signing mode (`local` or `external`). Use `external` to delegate key generation and signing to your key provider — the private key never leaves the HSM. | `local` | ❌ | > **Note:** The `KEY_PROVIDER_URL` points to your implementation of the key provider API interface. You must implement this interface to connect your KMS/HSM. See [Prerequisites](#prerequisites) for the specification and examples. diff --git a/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts b/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts index 89adec0e..2948e16f 100644 --- a/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts +++ b/src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts @@ -1,5 +1,6 @@ import { AppMode, AdvancedWalletManagerConfig, TlsMode, SigningMode } from '../../../initConfig'; import { app as expressApp } from '../../../advancedWalletManagerApp'; +import { KeyProviderClient } from '../../../advancedWalletManager/keyProviderClient/keyProviderClient'; import express from 'express'; import nock from 'nock'; @@ -111,3 +112,128 @@ describe('postMpcV2Key', () => { ); }); }); + +describe('KeyProviderClient.generateKey', () => { + const keyProviderUrl = 'http://key-provider.invalid'; + const endPointPath = '/key/generate'; + const params = { coin: 'hteth', source: 'user' as const, type: 'independent' as const }; + const mockResponse = { pub: 'xpub661MyMwAq', coin: 'hteth', source: 'user', type: 'independent' }; + let client: KeyProviderClient; + + before(() => { + nock.disableNetConnect(); + client = new KeyProviderClient({ + appMode: AppMode.ADVANCED_WALLET_MANAGER, + signingMode: SigningMode.LOCAL, + port: 0, + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + keyProviderUrl, + tlsMode: TlsMode.DISABLED, + clientCertAllowSelfSigned: true, + }); + }); + + afterEach(() => nock.cleanAll()); + + it('should call POST /key/generate with correct params and return response', async () => { + const nockMocked = nock(keyProviderUrl).post(endPointPath, params).reply(200, mockResponse); + + const result = await client.generateKey(params); + + result.should.have.property('pub', mockResponse.pub); + result.should.have.property('coin', mockResponse.coin); + result.should.have.property('source', mockResponse.source); + result.should.have.property('type', mockResponse.type); + nockMocked.done(); + }); + + [ + { + url: endPointPath, + statusCode: 400, + mockedError: 'bad request', + expectedError: 'bad request', + }, + { url: endPointPath, statusCode: 404, mockedError: 'not found', expectedError: 'not found' }, + { url: endPointPath, statusCode: 409, mockedError: 'conflict', expectedError: 'conflict' }, + { + url: endPointPath, + statusCode: 500, + mockedError: 'internal error', + expectedError: 'internal error', + }, + ].forEach(({ url, statusCode, mockedError, expectedError }) => { + it(`should bubble up ${statusCode} errors`, async () => { + const nockMocked = nock(keyProviderUrl) + .post(url) + .reply(statusCode, { message: mockedError }) + .persist(); + await client.generateKey(params).should.be.rejectedWith(expectedError); + nockMocked.done(); + }); + }); +}); + +describe('KeyProviderClient.sign', () => { + const keyProviderUrl = 'http://key-provider.invalid'; + const endPointPath = '/sign'; + const params = { + pub: 'xpub661MyMwAq', + source: 'user' as const, + signablePayload: 'deadbeef', + algorithm: 'ecdsa', + }; + const mockResponse = { signature: 'signedpsbt' }; + let client: KeyProviderClient; + + before(() => { + nock.disableNetConnect(); + client = new KeyProviderClient({ + appMode: AppMode.ADVANCED_WALLET_MANAGER, + signingMode: SigningMode.LOCAL, + port: 0, + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + keyProviderUrl, + tlsMode: TlsMode.DISABLED, + clientCertAllowSelfSigned: true, + }); + }); + + afterEach(() => nock.cleanAll()); + + it('should call POST /sign with correct params and return signature', async () => { + const n = nock(keyProviderUrl).post(endPointPath, params).reply(200, mockResponse); + + const result = await client.sign(params); + + result.should.have.property('signature', mockResponse.signature); + n.done(); + }); + + it('should throw if response has no signature', async () => { + nock(keyProviderUrl).post(endPointPath).reply(200, {}); + await client + .sign(params) + .should.be.rejectedWith(/key provider returned unexpected response when signing/); + }); + + [ + { statusCode: 400, mockedError: 'bad request', expectedError: 'bad request' }, + { statusCode: 404, mockedError: 'not found', expectedError: 'not found' }, + { statusCode: 409, mockedError: 'conflict', expectedError: 'conflict' }, + { statusCode: 500, mockedError: 'internal error', expectedError: 'internal error' }, + ].forEach(({ statusCode, mockedError, expectedError }) => { + it(`should bubble up ${statusCode} errors`, async () => { + const nockMocked = nock(keyProviderUrl) + .post(endPointPath) + .reply(statusCode, { message: mockedError }) + .persist(); + await client.sign(params).should.be.rejectedWith(expectedError); + nockMocked.done(); + }); + }); +}); diff --git a/src/__tests__/api/advancedWalletManager/postIndependentKey.test.ts b/src/__tests__/api/advancedWalletManager/postIndependentKey.test.ts index 8b4a5868..318d146f 100644 --- a/src/__tests__/api/advancedWalletManager/postIndependentKey.test.ts +++ b/src/__tests__/api/advancedWalletManager/postIndependentKey.test.ts @@ -9,6 +9,7 @@ import express from 'express'; import * as sinon from 'sinon'; import coinFactory from '../../../shared/coinFactory'; import { BaseCoin } from '@bitgo-beta/sdk-core'; +import { CoinFamily } from '@bitgo-beta/statics'; describe('postIndependentKey', () => { let cfg: AdvancedWalletManagerConfig; @@ -85,6 +86,7 @@ describe('postIndependentKey', () => { 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({ + getFamily: () => CoinFamily.ETH, keychains: () => ({ create: () => ({}), }), @@ -102,3 +104,103 @@ describe('postIndependentKey', () => { coinStub.restore(); }); }); + +describe('postIndependentKey — external signing mode', () => { + let app: express.Application; + let agent: request.SuperAgentTest; + let coinStub: sinon.SinonStub; + + const keyProviderUrl = 'http://key-provider.invalid'; + const coin = 'tbtc'; + const accessToken = 'test-token'; + const mockGenerateKeyResponse = { + pub: 'xpub661MyMwAq', + coin, + source: 'user', + type: 'independent', + }; + + const utxoCoinStub = { + getFamily: () => CoinFamily.BTC, + getFullName: () => 'Test Bitcoin', + keychains: () => ({ create: sinon.stub().returns({ pub: 'xpub...', prv: 'xprv...' }) }), + } as unknown as BaseCoin; + + const nonUtxoCoinStub = { + getFamily: () => CoinFamily.ETH, + getFullName: () => 'Test Ethereum', + keychains: () => ({ create: sinon.stub().returns({ pub: 'xpub...', prv: 'xprv...' }) }), + } as unknown as BaseCoin; + + before(() => { + nock.disableNetConnect(); + nock.enableNetConnect('127.0.0.1'); + + app = advancedWalletManagerApp({ + appMode: AppMode.ADVANCED_WALLET_MANAGER, + signingMode: SigningMode.EXTERNAL, + port: 0, + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + keyProviderUrl, + tlsMode: TlsMode.DISABLED, + clientCertAllowSelfSigned: true, + }); + agent = request.agent(app); + }); + + afterEach(() => { + nock.cleanAll(); + coinStub?.restore(); + }); + + it('should call POST /key/generate for UTXO coin and not call POST /key', async () => { + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoCoinStub); + const externalKeyGeneratorNock = nock(keyProviderUrl) + .post('/key/generate', { coin, source: 'user', type: 'independent' }) + .reply(200, mockGenerateKeyResponse); + const localKeyGeneratorNock = nock(keyProviderUrl).post('/key').reply(200, {}); + + const response = await agent + .post(`/api/${coin}/key/independent`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(200); + response.body.should.have.property('pub', mockGenerateKeyResponse.pub); + externalKeyGeneratorNock.done(); + localKeyGeneratorNock.isDone().should.equal(false); + }); + + it('should not call coin.keychains().create() in external mode for UTXO coin', async () => { + const createSpy = sinon.spy(); + const utxoWithSpy = { + ...utxoCoinStub, + keychains: () => ({ create: createSpy }), + } as unknown as BaseCoin; + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoWithSpy); + nock(keyProviderUrl).post('/key/generate').reply(200, mockGenerateKeyResponse); + + await agent + .post(`/api/${coin}/key/independent`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'backup' }); + + createSpy.called.should.equal(false); + }); + + it('should fall through to local path for non-UTXO coin in external mode', async () => { + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(nonUtxoCoinStub); + const externalKeyGeneratorNock = nock(keyProviderUrl).post('/key/generate').reply(200, {}); + nock(keyProviderUrl).post('/key').reply(200, mockGenerateKeyResponse); + + const response = await agent + .post(`/api/${coin}/key/independent`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user' }); + + response.status.should.equal(200); + externalKeyGeneratorNock.isDone().should.equal(false); + }); +}); diff --git a/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts b/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts index 797d8ea0..c6b971d6 100644 --- a/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts +++ b/src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts @@ -9,7 +9,7 @@ import * as sinon from 'sinon'; import * as configModule from '../../../initConfig'; import { DklsTypes, DklsUtils } from '@bitgo-beta/sdk-lib-mpc'; -describe('recoveryMpcV2', async () => { +describe('recoveryMpcV2', () => { let cfg: AdvancedWalletManagerConfig; let app: express.Application; let agent: request.SuperAgentTest; @@ -23,36 +23,43 @@ describe('recoveryMpcV2', async () => { // sinon stubs let configStub: sinon.SinonStub; - // key provider nocks setup - const [userShare, backupShare] = await DklsUtils.generateDKGKeyShares(); - const userKeyShare = userShare.getKeyShare().toString('base64'); - const backupKeyShare = backupShare.getKeyShare().toString('base64'); - const commonKeychain = DklsTypes.getCommonKeychain(userShare.getKeyShare()); - - const mockKeyProviderUserResponse = { - prv: JSON.stringify(userKeyShare), - pub: commonKeychain, - source: 'user', - type: 'tss', - }; - - const mockKeyProviderBackupResponse = { - prv: JSON.stringify(backupKeyShare), - pub: commonKeychain, - source: 'backup', - type: 'tss', - }; - const input = { - txHex: - '02f6824268018502540be4008504a817c80083030d409443442e403d64d29c4f64065d0c1a0e8edc03d6c88801550f7dca700000823078c0', - pub: commonKeychain, - }; + // key provider nocks setup — initialized in before() + let commonKeychain!: string; + let mockKeyProviderUserResponse: { prv: string; pub: string; source: string; type: string }; + let mockKeyProviderBackupResponse: { prv: string; pub: string; source: string; type: string }; + let input: { txHex: string; pub: string }; before(async () => { // nock config nock.disableNetConnect(); nock.enableNetConnect('127.0.0.1'); + // generate DKG key shares + const [userShare, backupShare] = await DklsUtils.generateDKGKeyShares(); + const userKeyShare = userShare.getKeyShare().toString('base64'); + const backupKeyShare = backupShare.getKeyShare().toString('base64'); + commonKeychain = DklsTypes.getCommonKeychain(userShare.getKeyShare()); + + mockKeyProviderUserResponse = { + prv: JSON.stringify(userKeyShare), + pub: commonKeychain, + source: 'user', + type: 'tss', + }; + + mockKeyProviderBackupResponse = { + prv: JSON.stringify(backupKeyShare), + pub: commonKeychain, + source: 'backup', + type: 'tss', + }; + + input = { + txHex: + '02f6824268018502540be4008504a817c80083030d409443442e403d64d29c4f64065d0c1a0e8edc03d6c88801550f7dca700000823078c0', + pub: commonKeychain, + }; + // app config cfg = { appMode: AppMode.ADVANCED_WALLET_MANAGER, diff --git a/src/__tests__/api/advancedWalletManager/recoveryMultisigTransaction.test.ts b/src/__tests__/api/advancedWalletManager/recoveryMultisigTransaction.test.ts index 15b58419..5e405e5f 100644 --- a/src/__tests__/api/advancedWalletManager/recoveryMultisigTransaction.test.ts +++ b/src/__tests__/api/advancedWalletManager/recoveryMultisigTransaction.test.ts @@ -8,6 +8,9 @@ import * as middleware from '../../../shared/middleware'; import { BitGoRequest } from '../../../types/request'; import { BitGoAPI as BitGo } from '@bitgo-beta/sdk-api'; import * as keyProviderUtils from '../../../advancedWalletManager/handlers/utils/utils'; +import coinFactory from '../../../shared/coinFactory'; +import { BaseCoin } from '@bitgo-beta/sdk-core'; +import { CoinFamily } from '@bitgo-beta/statics'; describe('UTXO recovery', () => { let agent: request.SuperAgentTest; @@ -134,3 +137,147 @@ describe('UTXO recovery', () => { .should.be.true(); }); }); + +describe('UTXO recovery — external signing mode', () => { + let agent: request.SuperAgentTest; + + const keyProviderUrl = 'http://key-provider.invalid'; + const coin = 'tbtc'; + const userPub = + 'xpub661MyMwAqRbcF3g1sUm7T5pN8ViCr9bS6XiQbq7dVXFdPEGYfhGgjjV2AFxTYVWik29y7NHmCZjWYDkt4RGw57HNYpHnoHeeqJV6s8hwcsV'; + const backupPub = + 'xpub661MyMwAqRbcEywGPF6Pg1FDUtHGyxsn7nph8dcy8GFLKvQ8hSCKgUm8sNbJhegDbmLtMpMnGZtrqfRXCjeDtfJ2UGDSzNTkRuvAQ5KNPcH'; + const bitgoPub = + 'xpub661MyMwAqRbcGcBurxn9ptqqKGmMhnKa8D7TeZkaWpfQNTeG4qKEJ67eb6Hy58kZBwPHqjUt5iApUwvFVk9ffQYaV42RRom2p7yU5bcCwpq'; + const unsignedTxHex = '70736274ff01000000'; + const halfSignedTxHex = '70736274ff01000001'; + const fullSignedTxHex = '70736274ff01000002'; + + const config: AdvancedWalletManagerConfig = { + appMode: AppMode.ADVANCED_WALLET_MANAGER, + signingMode: SigningMode.EXTERNAL, + port: 0, + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + tlsMode: TlsMode.DISABLED, + clientCertAllowSelfSigned: true, + keyProviderUrl, + recoveryMode: true, + }; + + const utxoCoinStub = { + getFamily: () => CoinFamily.BTC, + getFullName: () => 'Test Bitcoin', + isEVM: () => false, + } as unknown as BaseCoin; + + beforeEach(() => { + nock.disableNetConnect(); + nock.enableNetConnect('127.0.0.1'); + + const bitgo = new BitGo({ env: 'test', accessToken: 'test_token' }); + + sinon.stub(middleware, 'prepareBitGo').callsFake(() => (req, res, next) => { + (req as BitGoRequest).bitgo = bitgo; + (req as BitGoRequest).config = config; + next(); + }); + + sinon.stub(coinFactory, 'getCoin').resolves(utxoCoinStub); + + const app = expressApp(config); + agent = request.agent(app); + }); + + afterEach(() => { + nock.cleanAll(); + sinon.restore(); + }); + + it('should call POST /sign twice (user then backup) and not call retrieveKeyProviderPrvKey', async () => { + const retrieveStub = sinon.stub(keyProviderUtils, 'retrieveKeyProviderPrvKey'); + + const userSignNock = nock(keyProviderUrl) + .post('/sign', { + pub: userPub, + source: 'user', + signablePayload: unsignedTxHex, + algorithm: 'ecdsa', + }) + .reply(200, { signature: halfSignedTxHex }); + const backupSignNock = nock(keyProviderUrl) + .post('/sign', { + pub: backupPub, + source: 'backup', + signablePayload: halfSignedTxHex, + algorithm: 'ecdsa', + }) + .reply(200, { signature: fullSignedTxHex }); + + const response = await agent.post(`/api/${coin}/multisig/recovery`).send({ + userPub, + backupPub, + bitgoPub, + unsignedSweepPrebuildTx: { txHex: unsignedTxHex }, + walletContractAddress: '', + coin, + }); + + response.status.should.equal(200); + response.body.should.have.property('txHex', fullSignedTxHex); + userSignNock.done(); + backupSignNock.done(); + retrieveStub.called.should.equal(false); + }); + + it('should use half-signed PSBT from user sign as input to backup sign', async () => { + nock(keyProviderUrl) + .post('/sign', { + pub: userPub, + source: 'user', + signablePayload: unsignedTxHex, + algorithm: 'ecdsa', + }) + .reply(200, { signature: halfSignedTxHex }); + // backup receives the half-signed PSBT (output of user sign), not the original unsigned one + const backupNock = nock(keyProviderUrl) + .post('/sign', { + pub: backupPub, + source: 'backup', + signablePayload: halfSignedTxHex, + algorithm: 'ecdsa', + }) + .reply(200, { signature: fullSignedTxHex }); + + const response = await agent.post(`/api/${coin}/multisig/recovery`).send({ + userPub, + backupPub, + bitgoPub, + unsignedSweepPrebuildTx: { txHex: unsignedTxHex }, + walletContractAddress: '', + coin, + }); + + response.status.should.equal(200); + /** Verify backup sign call was made */ + backupNock.done(); + }); + + it('should return 500 if user sign call fails', async () => { + nock(keyProviderUrl) + .post('/sign', (body) => body.source === 'user') + .reply(500, { message: 'HSM error' }); + + const response = await agent.post(`/api/${coin}/multisig/recovery`).send({ + userPub, + backupPub, + bitgoPub, + unsignedSweepPrebuildTx: { txHex: unsignedTxHex }, + walletContractAddress: '', + coin, + }); + + response.status.should.equal(500); + }); +}); diff --git a/src/__tests__/api/advancedWalletManager/signMultisigTransaction.test.ts b/src/__tests__/api/advancedWalletManager/signMultisigTransaction.test.ts index a0d1387b..1127893d 100644 --- a/src/__tests__/api/advancedWalletManager/signMultisigTransaction.test.ts +++ b/src/__tests__/api/advancedWalletManager/signMultisigTransaction.test.ts @@ -8,6 +8,9 @@ import express from 'express'; import * as sinon from 'sinon'; import * as configModule from '../../../initConfig'; +import coinFactory from '../../../shared/coinFactory'; +import { BaseCoin } from '@bitgo-beta/sdk-core'; +import { CoinFamily } from '@bitgo-beta/statics'; describe('signMultisigTransaction', () => { let cfg: AdvancedWalletManagerConfig; @@ -159,3 +162,103 @@ describe('signMultisigTransaction', () => { keyProviderNock.done(); }); }); + +describe('signMultisigTransaction — external signing mode', () => { + let app: express.Application; + let agent: request.SuperAgentTest; + let coinStub: sinon.SinonStub; + + const keyProviderUrl = 'http://key-provider.invalid'; + const accessToken = 'test-token'; + + const txHexPrefix = '70736274ff'; + const txHex = `${txHexPrefix}01000000`; + const userPub = + 'xpub661MyMwAqRbcF3g1sUm7T5pN8ViCr9bS6XiQbq7dVXFdPEGYfhGgjjV2AFxTYVWik29y7NHmCZjWYDkt4RGw57HNYpHnoHeeqJV6s8hwcsV'; + + const utxoCoinStub = { + getFamily: () => CoinFamily.BTC, + getFullName: () => 'Test Bitcoin', + } as unknown as BaseCoin; + + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoCoinStub); + + const nonUtxoCoinStub = { + getFamily: () => CoinFamily.ETH, + getFullName: () => 'Test Ethereum', + } as unknown as BaseCoin; + + before(() => { + nock.disableNetConnect(); + nock.enableNetConnect('127.0.0.1'); + + app = advancedWalletManagerApp({ + appMode: AppMode.ADVANCED_WALLET_MANAGER, + signingMode: SigningMode.EXTERNAL, + port: 0, + bind: 'localhost', + timeout: 60000, + httpLoggerFile: '', + keyProviderUrl, + tlsMode: TlsMode.DISABLED, + clientCertAllowSelfSigned: true, + }); + agent = request.agent(app); + }); + + afterEach(() => { + nock.cleanAll(); + coinStub.restore(); + }); + + it('should call POST /sign for UTXO coin and not call GET /key/:pub', async () => { + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoCoinStub); + const signedPsbt = `${txHexPrefix}signed`; + + const signNock = nock(keyProviderUrl) + .post('/sign', { pub: userPub, source: 'user', signablePayload: txHex, algorithm: 'ecdsa' }) + .reply(200, { signature: signedPsbt }); + const getKeyNock = nock(keyProviderUrl).get(`/key/${userPub}`).reply(200, {}); + + const response = await agent + .post(`/api/tbtc/multisig/sign`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user', pub: userPub, txPrebuild: { txHex } }); + + response.status.should.equal(200); + response.body.should.have.property('txHex', signedPsbt); + signNock.done(); + getKeyNock.isDone().should.equal(false); + }); + + it('should fall through to local path for non-UTXO coin in external mode', async () => { + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(nonUtxoCoinStub); + + const signNock = nock(keyProviderUrl).post('/sign').reply(200, {}); + nock(keyProviderUrl).get(`/key/${userPub}`).query({ source: 'user' }).reply(200, { + prv: 'xprv9s21ZrQH143K2ZbYmTE75wsdaTsiSgsajJnooSi1wBieWRwQ89xSBwAYK1VJR795Y8XFCCXYHHs4sk2Heg6dkX3CHMBq5bw8DwBWByWx883', + pub: userPub, + source: 'user', + type: 'independent', + }); + + await agent + .post(`/api/hteth/multisig/sign`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'user', pub: userPub, txPrebuild: { txHex } }); + + /** assert that /sign endpoint is not called */ + signNock.isDone().should.equal(false); + }); + + it('should return 500 for invalid source in external mode on UTXO coin', async () => { + coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoCoinStub); + + const response = await agent + .post(`/api/tbtc/multisig/sign`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ source: 'bitgo', pub: userPub, txPrebuild: { txHex } }); + + response.status.should.equal(500); + }); +}); diff --git a/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts b/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts index 7264f100..eb29758b 100644 --- a/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts +++ b/src/advancedWalletManager/handlers/ecdsaEddsaSignTransaction.ts @@ -30,7 +30,7 @@ enum ShareType { } // Define MPC algorithm types -enum MPCType { +export enum MPCType { EDDSA = 'eddsa', ECDSA = 'ecdsa', } diff --git a/src/advancedWalletManager/handlers/multisigRecovery.ts b/src/advancedWalletManager/handlers/multisigRecovery.ts index 7dad73b7..ea769e15 100644 --- a/src/advancedWalletManager/handlers/multisigRecovery.ts +++ b/src/advancedWalletManager/handlers/multisigRecovery.ts @@ -3,6 +3,7 @@ import { AbstractUtxoCoin } from '@bitgo-beta/abstract-utxo'; import { HalfSignedUtxoTransaction, MethodNotImplementedError, + MPCType, TransactionRecipient, } from '@bitgo-beta/sdk-core'; import { AwmApiSpecRouteRequest } from '../routers/advancedWalletManagerApiSpec'; @@ -15,8 +16,15 @@ import { getReplayProtectionOptions, } from '../../shared/recoveryUtils'; import { SignedEthLikeRecoveryTx } from '../../types/transaction'; -import { checkRecoveryMode, retrieveKeyProviderPrvKey } from './utils/utils'; +import { + checkRecoveryMode, + retrieveKeyProviderPrvKey, + isExternalSigningEnabledForCoin, +} from './utils/utils'; import coinFactory from '../../shared/coinFactory'; +import { KeyProviderClient } from '../keyProviderClient/keyProviderClient'; +import { SignResponse } from '../keyProviderClient/types/sign'; +import { KeySource } from '../../shared/types'; export async function recoveryMultisigTransaction( req: AwmApiSpecRouteRequest<'v1.multisig.recovery', 'post'>, @@ -26,6 +34,19 @@ export async function recoveryMultisigTransaction( const { userPub, backupPub, bitgoPub, unsignedSweepPrebuildTx, walletContractAddress, coin } = req.decoded; + const bitgo = req.bitgo; + const baseCoin = await coinFactory.getCoin(coin, bitgo); + + if (isExternalSigningEnabledForCoin(req.config, baseCoin)) { + const keyProvider = new KeyProviderClient(req.config); + return recoverTransactionExternally({ + keyProvider, + userPub, + backupPub, + unsignedTxHex: unsignedSweepPrebuildTx.txHex, + }); + } + //fetch prv and check that pub are valid const userPrv = await retrieveKeyProviderPrvKey({ pub: userPub, @@ -44,9 +65,6 @@ export async function recoveryMultisigTransaction( throw new Error(errorMsg); } - const bitgo = req.bitgo; - const baseCoin = await coinFactory.getCoin(coin, bitgo); - // The signed transaction format depends on the coin type so we do this check as a guard // If you check the type of coin before and after the "if", you may see "BaseCoin" vs "AbstractEthLikeCoin" if (baseCoin.isEVM()) { @@ -180,6 +198,49 @@ export async function recoveryMultisigTransaction( } } +async function recoverTransactionExternally({ + keyProvider, + userPub, + backupPub, + unsignedTxHex, +}: { + keyProvider: KeyProviderClient; + userPub: string; + backupPub: string; + unsignedTxHex: string; +}): Promise<{ txHex: string }> { + const errorResponse = (error: any, keySource: string) => ({ + status: error.status || 500, + message: error.message || `Failed to sign recovery transaction for source=${keySource}`, + }); + + /** User Key Signs */ + let halfSignedRes: SignResponse; + try { + halfSignedRes = await keyProvider.sign({ + pub: userPub, + source: KeySource.USER, + signablePayload: unsignedTxHex, + algorithm: MPCType.ECDSA, + }); + } catch (error: any) { + throw errorResponse(error, KeySource.USER); + } + + /** Backup Key Signs */ + try { + const fullSignedRes = await keyProvider.sign({ + pub: backupPub, + source: KeySource.BACKUP, + signablePayload: halfSignedRes.signature, + algorithm: MPCType.ECDSA, + }); + return { txHex: fullSignedRes.signature }; + } catch (error: any) { + throw errorResponse(error, KeySource.BACKUP); + } +} + function checkIfNoRecipients({ recipients, coin, diff --git a/src/advancedWalletManager/handlers/multisigSignTransaction.ts b/src/advancedWalletManager/handlers/multisigSignTransaction.ts index 63cd2131..0765da38 100644 --- a/src/advancedWalletManager/handlers/multisigSignTransaction.ts +++ b/src/advancedWalletManager/handlers/multisigSignTransaction.ts @@ -3,6 +3,9 @@ import { TransactionPrebuild } from '@bitgo-beta/sdk-core'; import logger from '../../shared/logger'; import { AwmApiSpecRouteRequest } from '../routers/advancedWalletManagerApiSpec'; import coinFactory from '../../shared/coinFactory'; +import { isExternalSigningEnabledForCoin, isNonBitgoKeySource } from './utils/utils'; +import { SignResponse } from '../keyProviderClient/types/sign'; +import { MPCType } from './ecdsaEddsaSignTransaction'; export async function signMultisigTransaction( req: AwmApiSpecRouteRequest<'v1.multisig.sign', 'post'>, @@ -17,6 +20,11 @@ export async function signMultisigTransaction( const bitgo = req.bitgo; const keyProvider = new KeyProviderClient(req.config); + const coin = await coinFactory.getCoin(req.params.coin, bitgo); + + if (isExternalSigningEnabledForCoin(req.config, coin)) { + return signTransactionExternally({ keyProvider, pub, source, txPrebuild }); + } // Retrieve the private key from key provider let prv: string; @@ -31,17 +39,52 @@ export async function signMultisigTransaction( } // Sign the transaction using BitGo SDK - const coin = await coinFactory.getCoin(req.params.coin, bitgo); try { const signedTx = await coin.signTransaction({ txPrebuild, prv, ...(walletPubs && { pubs: walletPubs }), }); - // The signed transaction format depends on the coin type return signedTx; } catch (error) { logger.error('error while signing wallet transaction:', error); throw error; } } + +async function signTransactionExternally({ + keyProvider, + pub, + source, + txPrebuild, +}: { + keyProvider: KeyProviderClient; + pub: string; + source: string; + txPrebuild: TransactionPrebuild; +}): Promise<{ txHex: string }> { + if (!isNonBitgoKeySource(source)) { + throw new Error(`Invalid source: ${source}. Must be 'user' or 'backup'.`); + } + + let res: SignResponse; + try { + if (!txPrebuild.txHex) { + throw new Error(`txPrebuild must include txHex for external signing`); + } + + res = await keyProvider.sign({ + pub, + source, + signablePayload: txPrebuild.txHex, + algorithm: MPCType.ECDSA, + }); + } catch (error: any) { + throw { + status: error.status || 500, + message: error.message || 'Failed to sign transaction via key provider', + }; + } + + return { txHex: res.signature }; +} diff --git a/src/advancedWalletManager/handlers/postIndependentKey.ts b/src/advancedWalletManager/handlers/postIndependentKey.ts index 30d98c12..9eac978b 100644 --- a/src/advancedWalletManager/handlers/postIndependentKey.ts +++ b/src/advancedWalletManager/handlers/postIndependentKey.ts @@ -2,18 +2,49 @@ import { BitGoAPI } from '@bitgo-beta/sdk-api'; import { KeyProviderClient } from '../keyProviderClient/keyProviderClient'; import { AwmApiSpecRouteRequest } from '../routers/advancedWalletManagerApiSpec'; import coinFactory from '../../shared/coinFactory'; +import { isExternalSigningEnabledForCoin, isNonBitgoKeySource } from './utils/utils'; +import logger from '../../shared/logger'; +import { GenerateKeyResponse } from '../keyProviderClient/types/generateKey'; +import { PostKeyResponse } from '../keyProviderClient/types/postKey'; + +async function generateKeyViaKeyProvider( + keyGenerator: () => Promise, + keyGeneratorContext: string, +): Promise { + try { + logger.info(keyGeneratorContext); + return await keyGenerator(); + } catch (error: any) { + throw { + status: error.status || 500, + message: error.message || `Failed to generate key via key provider`, + }; + } +} export async function postIndependentKey( req: AwmApiSpecRouteRequest<'v1.key.independent', 'post'>, ) { const { source, seed }: { source: string; seed?: string } = req.decoded; - // setup clients const bitgo: BitGoAPI = req.bitgo; const keyProvider = new KeyProviderClient(req.config); - - // create public and private key pairs on BitGo SDK const coin = await coinFactory.getCoin(req.params.coin, bitgo); + + if (isExternalSigningEnabledForCoin(req.config, coin) && isNonBitgoKeySource(source)) { + logger.info( + `External signing is supported for coin=$${coin.getFullName()}. Generating key via key provider for source=${source}`, + ); + return await generateKeyViaKeyProvider( + () => + keyProvider.generateKey({ + coin: req.params.coin, + source, + type: 'independent', + }), + `postIndependentKey in external signing mode for coin=${coin.getFullName()} and source=${source}`, + ); + } const { pub, prv } = coin.keychains().create(); if (!pub) { @@ -21,19 +52,16 @@ export async function postIndependentKey( } // post key to key provider for encryption and storage - try { - return await keyProvider.postKey({ - pub, - prv, - coin: req.params.coin, - source, - type: 'independent', - seed, - }); - } catch (error: any) { - throw { - status: error.status || 500, - message: error.message || 'Failed to post key to key provider', - }; - } + return await generateKeyViaKeyProvider( + () => + keyProvider.postKey({ + pub, + prv, + coin: req.params.coin, + source, + type: 'independent', + seed, + }), + `postIndependentKey in local mode for coin=${coin.getFullName()} and source=${source}`, + ); } diff --git a/src/advancedWalletManager/handlers/utils/utils.ts b/src/advancedWalletManager/handlers/utils/utils.ts index c95ffdc3..02fd6f11 100644 --- a/src/advancedWalletManager/handlers/utils/utils.ts +++ b/src/advancedWalletManager/handlers/utils/utils.ts @@ -5,7 +5,8 @@ import * as bitgoSdk from '@bitgo-beta/sdk-core'; import { KeyProviderClient } from '../../keyProviderClient/keyProviderClient'; import { GenerateDataKeyResponse } from '../../keyProviderClient/types/dataKey'; -import { AdvancedWalletManagerConfig } from '../../../shared/types'; +import { AdvancedWalletManagerConfig, KeySource, SigningMode } from '../../../shared/types'; +import { isUtxoCoin } from '../../../shared/coinUtils'; export async function retrieveKeyProviderPrvKey({ pub, @@ -116,6 +117,22 @@ export async function decryptDataKey({ } } +export function isExternalSigningModeEnabled(config: AdvancedWalletManagerConfig): boolean { + return config.signingMode === SigningMode.EXTERNAL; +} + +export function isExternalSigningEnabledForCoin( + config: AdvancedWalletManagerConfig, + coin: BaseCoin, +): boolean { + /** Support ETH after COIN-108 */ + return isExternalSigningModeEnabled(config) && isUtxoCoin(coin); +} + +export function isNonBitgoKeySource(source: string): source is KeySource.USER | KeySource.BACKUP { + return source === KeySource.USER || source === KeySource.BACKUP; +} + export function checkRecoveryMode(config: AdvancedWalletManagerConfig) { if (!config.recoveryMode) { throw new Error( diff --git a/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts b/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts index 6ddbeb1b..8e2bbdcf 100644 --- a/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts +++ b/src/advancedWalletManager/keyProviderClient/keyProviderClient.ts @@ -13,6 +13,12 @@ import { GenerateDataKeyParams, GenerateDataKeyResponse, } from './types/generateDataKey'; +import { + GenerateKeyResponseSchema, + GenerateKeyParams, + GenerateKeyResponse, +} from './types/generateKey'; +import { SignResponseSchema, SignParams, SignResponse } from './types/sign'; import https from 'https'; import { BadRequestError, ConflictError, NotFoundError } from '../../shared/errors'; import { URL } from 'url'; @@ -155,6 +161,54 @@ export class KeyProviderClient { return response.body as GetKeyResponse; } + async generateKey(params: GenerateKeyParams): Promise { + logger.info( + 'Generating key via key provider with coin: %s and source: %s', + params.coin, + params.source, + ); + + const response = await this.call('post', `${this.url}/key/generate`, { + body: params, + errorContext: 'Error generating key via key provider', + }); + + try { + GenerateKeyResponseSchema.parse(response.body); + } catch (error: any) { + logger.error('key provider returned unexpected response when generating key', error); + throw new Error( + `key provider returned unexpected response when generating key${ + error.message ? `: ${error.message}` : '' + }`, + ); + } + + return response.body as GenerateKeyResponse; + } + + async sign(params: SignParams): Promise { + logger.info('Signing via key provider with pub: %s and source: %s', params.pub, params.source); + + const response = await this.call('post', `${this.url}/sign`, { + body: params, + errorContext: 'Error signing via key provider', + }); + + try { + SignResponseSchema.parse(response.body); + } catch (error: any) { + logger.error('key provider returned unexpected response when signing', error); + throw new Error( + `key provider returned unexpected response when signing${ + error.message ? `: ${error.message}` : '' + }`, + ); + } + + return response.body as SignResponse; + } + async generateDataKey(params: GenerateDataKeyParams): Promise { logger.info('Generating data key from key provider with type: %s', params.keyType); diff --git a/src/advancedWalletManager/keyProviderClient/types/generateKey.ts b/src/advancedWalletManager/keyProviderClient/types/generateKey.ts new file mode 100644 index 00000000..cfebc4ed --- /dev/null +++ b/src/advancedWalletManager/keyProviderClient/types/generateKey.ts @@ -0,0 +1,15 @@ +import * as z from 'zod'; + +const GenerateKeyBaseSchema = z.object({ + coin: z.string(), + source: z.enum(['user', 'backup']), + type: z.enum(['independent', 'tss']), +}); + +export const GenerateKeyParamsSchema = GenerateKeyBaseSchema; +export const GenerateKeyResponseSchema = GenerateKeyBaseSchema.extend({ + pub: z.string(), +}); + +export type GenerateKeyParams = z.infer; +export type GenerateKeyResponse = z.infer; diff --git a/src/advancedWalletManager/keyProviderClient/types/sign.ts b/src/advancedWalletManager/keyProviderClient/types/sign.ts new file mode 100644 index 00000000..3e9a6ba3 --- /dev/null +++ b/src/advancedWalletManager/keyProviderClient/types/sign.ts @@ -0,0 +1,18 @@ +import * as z from 'zod'; + +const SignBaseSchema = z.object({ + pub: z.string(), + source: z.enum(['user', 'backup']), + algorithm: z.string(), +}); + +export const SignParamsSchema = SignBaseSchema.extend({ + signablePayload: z.string(), +}); + +export const SignResponseSchema = z.object({ + signature: z.string().min(1), +}); + +export type SignParams = z.infer; +export type SignResponse = z.infer; diff --git a/src/shared/types/index.ts b/src/shared/types/index.ts index d07d20da..219b2f6a 100644 --- a/src/shared/types/index.ts +++ b/src/shared/types/index.ts @@ -8,6 +8,12 @@ export enum SigningMode { EXTERNAL = 'external', } +export enum KeySource { + USER = 'user', + BACKUP = 'backup', + BITGO = 'bitgo', +} + export enum AppMode { ADVANCED_WALLET_MANAGER = 'advanced-wallet-manager', MASTER_EXPRESS = 'master-express',