Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions modules/express/src/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ parser.addArgument(['--signerFileSystemPath'], {
help: 'Local path specifying where an Express signer machine keeps encrypted user private keys.',
});

parser.addArgument(['--signerAuthToken'], {
help: 'Shared bearer token required to call external-signer routes (also BITGO_SIGNER_AUTH_TOKEN). Required in signerMode and when externalSignerUrl is set.',
});

parser.addArgument(['--lightningSignerFileSystemPath'], {
help: 'Local path specifying where an Express machine keeps lightning signer urls.',
});
Expand Down
249 changes: 186 additions & 63 deletions modules/express/src/clientRoutes.ts

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions modules/express/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ export interface Config {
externalSignerUrl?: string;
signerMode?: boolean;
signerFileSystemPath?: string;
/**
* Shared secret required for all external-signer HTTP routes when signerMode is enabled,
* and sent by generator Express instances when calling externalSignerUrl.
* Configure via --signerAuthToken or BITGO_SIGNER_AUTH_TOKEN.
*/
signerAuthToken?: string;
lightningSignerFileSystemPath?: string;
keepAliveTimeout?: number;
headersTimeout?: number;
Expand All @@ -66,6 +72,7 @@ export const ArgConfig = (args): Partial<Config> => ({
externalSignerUrl: args.externalSignerUrl,
signerMode: args.signerMode,
signerFileSystemPath: args.signerFileSystemPath,
signerAuthToken: args.signerAuthToken,
lightningSignerFileSystemPath: args.lightningSignerFileSystemPath,
keepAliveTimeout: args.keepalivetimeout,
headersTimeout: args.headerstimeout,
Expand All @@ -92,6 +99,7 @@ export const EnvConfig = (): Partial<Config> => ({
externalSignerUrl: readEnvVar('BITGO_EXTERNAL_SIGNER_URL'),
signerMode: readEnvVar('BITGO_SIGNER_MODE') ? true : undefined,
signerFileSystemPath: readEnvVar('BITGO_SIGNER_FILE_SYSTEM_PATH'),
signerAuthToken: readEnvVar('BITGO_SIGNER_AUTH_TOKEN'),
lightningSignerFileSystemPath: readEnvVar('BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH'),
keepAliveTimeout: Number(readEnvVar('BITGO_KEEP_ALIVE_TIMEOUT')),
headersTimeout: Number(readEnvVar('BITGO_HEADERS_TIMEOUT')),
Expand Down Expand Up @@ -178,6 +186,7 @@ function mergeConfigs(...configs: Partial<Config>[]): Config {
externalSignerUrl,
signerMode: get('signerMode'),
signerFileSystemPath: get('signerFileSystemPath'),
signerAuthToken: get('signerAuthToken'),
lightningSignerFileSystemPath: get('lightningSignerFileSystemPath'),
keepAliveTimeout: get('keepAliveTimeout'),
headersTimeout: get('headersTimeout'),
Expand Down
9 changes: 9 additions & 0 deletions modules/express/src/expressApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ function checkPreconditions(config: Config) {
externalSignerUrl,
signerMode,
signerFileSystemPath,
signerAuthToken,
lightningSignerFileSystemPath,
} = config;

Expand Down Expand Up @@ -260,6 +261,14 @@ function checkPreconditions(config: Config) {
);
}

// External signing routes hold user private keys. Require a shared secret so unauthenticated
// network clients cannot use the machine as a signing oracle (CWE-306).
if ((signerMode !== undefined || externalSignerUrl !== undefined) && !signerAuthToken) {
throw new ExternalSignerConfigError(
'signerAuthToken must be set when running in external signing mode or when externalSignerUrl is configured. Set --signerAuthToken or BITGO_SIGNER_AUTH_TOKEN.'
);
}

if (signerFileSystemPath !== undefined) {
checkJsonFilePath(signerFileSystemPath);
}
Expand Down
1 change: 1 addition & 0 deletions modules/express/src/typedRoutes/api/v2/coinSign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export const CoinSignResponse = {
*
* **Configuration Requirements:**
* - `signerFileSystemPath`: Path to JSON file containing encrypted private keys
* - `signerAuthToken` / `BITGO_SIGNER_AUTH_TOKEN`: required bearer token for all signer routes
* - Environment variable: `WALLET_{walletId}_PASSPHRASE` for each wallet
*
* **Request Body:**
Expand Down
2 changes: 2 additions & 0 deletions modules/express/test/integration/externalSigner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('Custom signing function', () => {
debug: true,
env: 'test',
externalSignerUrl,
signerAuthToken: 'test-signer-auth-token',
timeout: 60000,
};

Expand All @@ -42,6 +43,7 @@ describe('Custom signing function', () => {
// setup nock to external signer
const signernock = nock(externalSignerUrl)
.post('/api/v2/btc/sign')
.matchHeader('authorization', 'Bearer test-signer-auth-token')
.reply(200, { externalSigner: 'external signer response' });

// setup nock to wallet platform GET /wallet/fakeid
Expand Down
21 changes: 21 additions & 0 deletions modules/express/test/unit/bitgoExpress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ describe('Bitgo Express', function () {
env: 'test',
signerMode: 'signerMode',
signerFileSystemPath: 'signerFileSystemPath',
signerAuthToken: 'test-signer-auth-token',
};

app(args);
Expand Down Expand Up @@ -505,16 +506,35 @@ describe('Bitgo Express', function () {

const readFileStub = sinon.stub(fs, 'readFileSync').returns(validPrvJSON);
args.signerMode = 'signerMode';
args.signerAuthToken = 'test-signer-auth-token';
(() => expressApp(args)).should.not.throw();

readFileStub.restore();
});

it('should require signerAuthToken when running in signer mode', function () {
const readFileStub = sinon.stub(fs, 'readFileSync').returns(validPrvJSON);
const args: any = {
env: 'test',
signerMode: 'signerMode',
signerFileSystemPath: 'signerFileSystemPath',
};
(() => expressApp(args)).should.throw({
name: 'ExternalSignerConfigError',
message:
'signerAuthToken must be set when running in external signing mode or when externalSignerUrl is configured. Set --signerAuthToken or BITGO_SIGNER_AUTH_TOKEN.',
});
args.signerAuthToken = 'test-signer-auth-token';
(() => expressApp(args)).should.not.throw();
readFileStub.restore();
});

it('should require that an externalSignerUrl and signerMode are not both set', function () {
const args: any = {
env: 'test',
signerMode: 'signerMode',
externalSignerUrl: 'externalSignerUrl',
signerAuthToken: 'test-signer-auth-token',
};
(() => expressApp(args)).should.throw({
name: 'ExternalSignerConfigError',
Expand Down Expand Up @@ -549,6 +569,7 @@ describe('Bitgo Express', function () {
env: 'test',
signerMode: 'signerMode',
signerFileSystemPath: 'invalidSignerFileSystemPath',
signerAuthToken: 'test-signer-auth-token',
};
(() => expressApp(args)).should.throw();

Expand Down
3 changes: 3 additions & 0 deletions modules/express/test/unit/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ describe('Config:', () => {
BITGO_EXTERNAL_SIGNER_URL: 'envexternalSignerUrl',
BITGO_SIGNER_MODE: 'envsignerMode',
BITGO_SIGNER_FILE_SYSTEM_PATH: 'envsignerFileSystemPath',
BITGO_SIGNER_AUTH_TOKEN: 'envsignerAuthToken',
BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH: 'envlightningSignerFileSystemPath',
BITGO_KEEP_ALIVE_TIMETOUT: 'envkeepalivetimeout',
BITGO_HEADERS_TIMETOUT: 'envheaderstimeout',
Expand Down Expand Up @@ -153,6 +154,7 @@ describe('Config:', () => {
enclavedExpressSSLCert: 'argenclavedExpressSSLCert',
signerMode: 'argsignerMode',
signerFileSystemPath: 'argsignerFileSystemPath',
signerAuthToken: 'argsignerAuthToken',
lightningSignerFileSystemPath: 'arglightningSignerFileSystemPath',
keepalivetimeout: 'argkeepalivetimeout',
headerstimeout: 'argheaderstimeout',
Expand Down Expand Up @@ -181,6 +183,7 @@ describe('Config:', () => {
externalSignerUrl: 'https://argexternalSignerUrl',
signerMode: 'argsignerMode',
signerFileSystemPath: 'argsignerFileSystemPath',
signerAuthToken: 'argsignerAuthToken',
lightningSignerFileSystemPath: 'arglightningSignerFileSystemPath',
keepAliveTimeout: 'argkeepalivetimeout',
headersTimeout: 'argheaderstimeout',
Expand Down
37 changes: 37 additions & 0 deletions modules/express/test/unit/typedRoutes/coinSign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () {
agent = setupAgent({
signerMode: true,
signerFileSystemPath: signerFilePath,
signerAuthToken: 'test_access_token_12345',
});
});

Expand Down Expand Up @@ -149,6 +150,42 @@ describe('CoinSign codec tests (External Signer Mode)', function () {
assert.strictEqual(signTxCall.isLastSignature, true);
});

it('should reject unauthenticated sign requests in external signer mode', async function () {
const requestBody = {
txPrebuild: {
walletId: walletId,
txHex:
'0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
},
isLastSignature: true,
};

const result = await agent.post(`/api/v2/${coin}/sign`).set('Content-Type', 'application/json').send(requestBody);

assert.strictEqual(result.status, 401);
result.body.should.have.property('message', 'Unauthorized');
});

it('should reject sign requests with an incorrect signer auth token', async function () {
const requestBody = {
txPrebuild: {
walletId: walletId,
txHex:
'0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
},
isLastSignature: true,
};

const result = await agent
.post(`/api/v2/${coin}/sign`)
.set('Authorization', 'Bearer wrong-token')
.set('Content-Type', 'application/json')
.send(requestBody);

assert.strictEqual(result.status, 401);
result.body.should.have.property('message', 'Unauthorized');
});

it('should successfully sign with derivationSeed', async function () {
const derivationSeed = 'test-derivation-seed-123';
const derivedKey =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () {
agent = setupAgent({
signerMode: true,
signerFileSystemPath: signerFilePath,
signerAuthToken: 'test_access_token_12345',
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () {
agent = setupAgent({
signerMode: true,
signerFileSystemPath: signerFilePath,
signerAuthToken: 'test_access_token_12345',
});
});

Expand Down