Skip to content

Commit fda6f15

Browse files
authored
Merge pull request #199 from BitGo/WCN-397/implementation
feat: implement external signing - phase 1
2 parents 020ec0d + dcee4fc commit fda6f15

15 files changed

Lines changed: 778 additions & 52 deletions

README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,6 @@ curl -X POST http://localhost:3081/ping/advancedWalletManager
180180
| ------------------------------ | ---------------------------------- | ------- | -------- |
181181
| `ADVANCED_WALLET_MANAGER_PORT` | Port to listen on | `3080` ||
182182
| `KEY_PROVIDER_URL` | URL to your key provider API implementation | - ||
183-
| `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` ||
184183

185184
> **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.
186185

src/__tests__/api/advancedWalletManager/keyProviderClient.test.ts

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { AppMode, AdvancedWalletManagerConfig, TlsMode, SigningMode } from '../../../initConfig';
22
import { app as expressApp } from '../../../advancedWalletManagerApp';
3+
import { KeyProviderClient } from '../../../advancedWalletManager/keyProviderClient/keyProviderClient';
34

45
import express from 'express';
56
import nock from 'nock';
@@ -111,3 +112,128 @@ describe('postMpcV2Key', () => {
111112
);
112113
});
113114
});
115+
116+
describe('KeyProviderClient.generateKey', () => {
117+
const keyProviderUrl = 'http://key-provider.invalid';
118+
const endPointPath = '/key/generate';
119+
const params = { coin: 'hteth', source: 'user' as const, type: 'independent' as const };
120+
const mockResponse = { pub: 'xpub661MyMwAq', coin: 'hteth', source: 'user', type: 'independent' };
121+
let client: KeyProviderClient;
122+
123+
before(() => {
124+
nock.disableNetConnect();
125+
client = new KeyProviderClient({
126+
appMode: AppMode.ADVANCED_WALLET_MANAGER,
127+
signingMode: SigningMode.LOCAL,
128+
port: 0,
129+
bind: 'localhost',
130+
timeout: 60000,
131+
httpLoggerFile: '',
132+
keyProviderUrl,
133+
tlsMode: TlsMode.DISABLED,
134+
clientCertAllowSelfSigned: true,
135+
});
136+
});
137+
138+
afterEach(() => nock.cleanAll());
139+
140+
it('should call POST /key/generate with correct params and return response', async () => {
141+
const nockMocked = nock(keyProviderUrl).post(endPointPath, params).reply(200, mockResponse);
142+
143+
const result = await client.generateKey(params);
144+
145+
result.should.have.property('pub', mockResponse.pub);
146+
result.should.have.property('coin', mockResponse.coin);
147+
result.should.have.property('source', mockResponse.source);
148+
result.should.have.property('type', mockResponse.type);
149+
nockMocked.done();
150+
});
151+
152+
[
153+
{
154+
url: endPointPath,
155+
statusCode: 400,
156+
mockedError: 'bad request',
157+
expectedError: 'bad request',
158+
},
159+
{ url: endPointPath, statusCode: 404, mockedError: 'not found', expectedError: 'not found' },
160+
{ url: endPointPath, statusCode: 409, mockedError: 'conflict', expectedError: 'conflict' },
161+
{
162+
url: endPointPath,
163+
statusCode: 500,
164+
mockedError: 'internal error',
165+
expectedError: 'internal error',
166+
},
167+
].forEach(({ url, statusCode, mockedError, expectedError }) => {
168+
it(`should bubble up ${statusCode} errors`, async () => {
169+
const nockMocked = nock(keyProviderUrl)
170+
.post(url)
171+
.reply(statusCode, { message: mockedError })
172+
.persist();
173+
await client.generateKey(params).should.be.rejectedWith(expectedError);
174+
nockMocked.done();
175+
});
176+
});
177+
});
178+
179+
describe('KeyProviderClient.sign', () => {
180+
const keyProviderUrl = 'http://key-provider.invalid';
181+
const endPointPath = '/sign';
182+
const params = {
183+
pub: 'xpub661MyMwAq',
184+
source: 'user' as const,
185+
signablePayload: 'deadbeef',
186+
algorithm: 'ecdsa',
187+
};
188+
const mockResponse = { signature: 'signedpsbt' };
189+
let client: KeyProviderClient;
190+
191+
before(() => {
192+
nock.disableNetConnect();
193+
client = new KeyProviderClient({
194+
appMode: AppMode.ADVANCED_WALLET_MANAGER,
195+
signingMode: SigningMode.LOCAL,
196+
port: 0,
197+
bind: 'localhost',
198+
timeout: 60000,
199+
httpLoggerFile: '',
200+
keyProviderUrl,
201+
tlsMode: TlsMode.DISABLED,
202+
clientCertAllowSelfSigned: true,
203+
});
204+
});
205+
206+
afterEach(() => nock.cleanAll());
207+
208+
it('should call POST /sign with correct params and return signature', async () => {
209+
const n = nock(keyProviderUrl).post(endPointPath, params).reply(200, mockResponse);
210+
211+
const result = await client.sign(params);
212+
213+
result.should.have.property('signature', mockResponse.signature);
214+
n.done();
215+
});
216+
217+
it('should throw if response has no signature', async () => {
218+
nock(keyProviderUrl).post(endPointPath).reply(200, {});
219+
await client
220+
.sign(params)
221+
.should.be.rejectedWith(/key provider returned unexpected response when signing/);
222+
});
223+
224+
[
225+
{ statusCode: 400, mockedError: 'bad request', expectedError: 'bad request' },
226+
{ statusCode: 404, mockedError: 'not found', expectedError: 'not found' },
227+
{ statusCode: 409, mockedError: 'conflict', expectedError: 'conflict' },
228+
{ statusCode: 500, mockedError: 'internal error', expectedError: 'internal error' },
229+
].forEach(({ statusCode, mockedError, expectedError }) => {
230+
it(`should bubble up ${statusCode} errors`, async () => {
231+
const nockMocked = nock(keyProviderUrl)
232+
.post(endPointPath)
233+
.reply(statusCode, { message: mockedError })
234+
.persist();
235+
await client.sign(params).should.be.rejectedWith(expectedError);
236+
nockMocked.done();
237+
});
238+
});
239+
});

src/__tests__/api/advancedWalletManager/postIndependentKey.test.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import express from 'express';
99
import * as sinon from 'sinon';
1010
import coinFactory from '../../../shared/coinFactory';
1111
import { BaseCoin } from '@bitgo-beta/sdk-core';
12+
import { CoinFamily } from '@bitgo-beta/statics';
1213

1314
describe('postIndependentKey', () => {
1415
let cfg: AdvancedWalletManagerConfig;
@@ -85,6 +86,7 @@ describe('postIndependentKey', () => {
8586
it('should fail if there is an error in creating the public and private key pairs', async () => {
8687
const coinStub = sinon.stub(coinFactory, 'getCoin').returns(
8788
Promise.resolve({
89+
getFamily: () => CoinFamily.ETH,
8890
keychains: () => ({
8991
create: () => ({}),
9092
}),
@@ -102,3 +104,103 @@ describe('postIndependentKey', () => {
102104
coinStub.restore();
103105
});
104106
});
107+
108+
describe('postIndependentKey — external signing mode', () => {
109+
let app: express.Application;
110+
let agent: request.SuperAgentTest;
111+
let coinStub: sinon.SinonStub;
112+
113+
const keyProviderUrl = 'http://key-provider.invalid';
114+
const coin = 'tbtc';
115+
const accessToken = 'test-token';
116+
const mockGenerateKeyResponse = {
117+
pub: 'xpub661MyMwAq',
118+
coin,
119+
source: 'user',
120+
type: 'independent',
121+
};
122+
123+
const utxoCoinStub = {
124+
getFamily: () => CoinFamily.BTC,
125+
getFullName: () => 'Test Bitcoin',
126+
keychains: () => ({ create: sinon.stub().returns({ pub: 'xpub...', prv: 'xprv...' }) }),
127+
} as unknown as BaseCoin;
128+
129+
const nonUtxoCoinStub = {
130+
getFamily: () => CoinFamily.ETH,
131+
getFullName: () => 'Test Ethereum',
132+
keychains: () => ({ create: sinon.stub().returns({ pub: 'xpub...', prv: 'xprv...' }) }),
133+
} as unknown as BaseCoin;
134+
135+
before(() => {
136+
nock.disableNetConnect();
137+
nock.enableNetConnect('127.0.0.1');
138+
139+
app = advancedWalletManagerApp({
140+
appMode: AppMode.ADVANCED_WALLET_MANAGER,
141+
signingMode: SigningMode.EXTERNAL,
142+
port: 0,
143+
bind: 'localhost',
144+
timeout: 60000,
145+
httpLoggerFile: '',
146+
keyProviderUrl,
147+
tlsMode: TlsMode.DISABLED,
148+
clientCertAllowSelfSigned: true,
149+
});
150+
agent = request.agent(app);
151+
});
152+
153+
afterEach(() => {
154+
nock.cleanAll();
155+
coinStub?.restore();
156+
});
157+
158+
it('should call POST /key/generate for UTXO coin and not call POST /key', async () => {
159+
coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoCoinStub);
160+
const externalKeyGeneratorNock = nock(keyProviderUrl)
161+
.post('/key/generate', { coin, source: 'user', type: 'independent' })
162+
.reply(200, mockGenerateKeyResponse);
163+
const localKeyGeneratorNock = nock(keyProviderUrl).post('/key').reply(200, {});
164+
165+
const response = await agent
166+
.post(`/api/${coin}/key/independent`)
167+
.set('Authorization', `Bearer ${accessToken}`)
168+
.send({ source: 'user' });
169+
170+
response.status.should.equal(200);
171+
response.body.should.have.property('pub', mockGenerateKeyResponse.pub);
172+
externalKeyGeneratorNock.done();
173+
localKeyGeneratorNock.isDone().should.equal(false);
174+
});
175+
176+
it('should not call coin.keychains().create() in external mode for UTXO coin', async () => {
177+
const createSpy = sinon.spy();
178+
const utxoWithSpy = {
179+
...utxoCoinStub,
180+
keychains: () => ({ create: createSpy }),
181+
} as unknown as BaseCoin;
182+
coinStub = sinon.stub(coinFactory, 'getCoin').resolves(utxoWithSpy);
183+
nock(keyProviderUrl).post('/key/generate').reply(200, mockGenerateKeyResponse);
184+
185+
await agent
186+
.post(`/api/${coin}/key/independent`)
187+
.set('Authorization', `Bearer ${accessToken}`)
188+
.send({ source: 'backup' });
189+
190+
createSpy.called.should.equal(false);
191+
});
192+
193+
it('should fall through to local path for non-UTXO coin in external mode', async () => {
194+
coinStub = sinon.stub(coinFactory, 'getCoin').resolves(nonUtxoCoinStub);
195+
const externalKeyGeneratorNock = nock(keyProviderUrl).post('/key/generate').reply(200, {});
196+
nock(keyProviderUrl).post('/key').reply(200, mockGenerateKeyResponse);
197+
198+
const response = await agent
199+
.post(`/api/${coin}/key/independent`)
200+
.set('Authorization', `Bearer ${accessToken}`)
201+
.send({ source: 'user' });
202+
203+
response.status.should.equal(200);
204+
externalKeyGeneratorNock.isDone().should.equal(false);
205+
});
206+
});

src/__tests__/api/advancedWalletManager/recoveryMpcV2.test.ts

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import * as sinon from 'sinon';
99
import * as configModule from '../../../initConfig';
1010
import { DklsTypes, DklsUtils } from '@bitgo-beta/sdk-lib-mpc';
1111

12-
describe('recoveryMpcV2', async () => {
12+
describe('recoveryMpcV2', () => {
1313
let cfg: AdvancedWalletManagerConfig;
1414
let app: express.Application;
1515
let agent: request.SuperAgentTest;
@@ -23,36 +23,43 @@ describe('recoveryMpcV2', async () => {
2323
// sinon stubs
2424
let configStub: sinon.SinonStub;
2525

26-
// key provider nocks setup
27-
const [userShare, backupShare] = await DklsUtils.generateDKGKeyShares();
28-
const userKeyShare = userShare.getKeyShare().toString('base64');
29-
const backupKeyShare = backupShare.getKeyShare().toString('base64');
30-
const commonKeychain = DklsTypes.getCommonKeychain(userShare.getKeyShare());
31-
32-
const mockKeyProviderUserResponse = {
33-
prv: JSON.stringify(userKeyShare),
34-
pub: commonKeychain,
35-
source: 'user',
36-
type: 'tss',
37-
};
38-
39-
const mockKeyProviderBackupResponse = {
40-
prv: JSON.stringify(backupKeyShare),
41-
pub: commonKeychain,
42-
source: 'backup',
43-
type: 'tss',
44-
};
45-
const input = {
46-
txHex:
47-
'02f6824268018502540be4008504a817c80083030d409443442e403d64d29c4f64065d0c1a0e8edc03d6c88801550f7dca700000823078c0',
48-
pub: commonKeychain,
49-
};
26+
// key provider nocks setup — initialized in before()
27+
let commonKeychain!: string;
28+
let mockKeyProviderUserResponse: { prv: string; pub: string; source: string; type: string };
29+
let mockKeyProviderBackupResponse: { prv: string; pub: string; source: string; type: string };
30+
let input: { txHex: string; pub: string };
5031

5132
before(async () => {
5233
// nock config
5334
nock.disableNetConnect();
5435
nock.enableNetConnect('127.0.0.1');
5536

37+
// generate DKG key shares
38+
const [userShare, backupShare] = await DklsUtils.generateDKGKeyShares();
39+
const userKeyShare = userShare.getKeyShare().toString('base64');
40+
const backupKeyShare = backupShare.getKeyShare().toString('base64');
41+
commonKeychain = DklsTypes.getCommonKeychain(userShare.getKeyShare());
42+
43+
mockKeyProviderUserResponse = {
44+
prv: JSON.stringify(userKeyShare),
45+
pub: commonKeychain,
46+
source: 'user',
47+
type: 'tss',
48+
};
49+
50+
mockKeyProviderBackupResponse = {
51+
prv: JSON.stringify(backupKeyShare),
52+
pub: commonKeychain,
53+
source: 'backup',
54+
type: 'tss',
55+
};
56+
57+
input = {
58+
txHex:
59+
'02f6824268018502540be4008504a817c80083030d409443442e403d64d29c4f64065d0c1a0e8edc03d6c88801550f7dca700000823078c0',
60+
pub: commonKeychain,
61+
};
62+
5663
// app config
5764
cfg = {
5865
appMode: AppMode.ADVANCED_WALLET_MANAGER,

0 commit comments

Comments
 (0)