diff --git a/modules/express/EXTERNAL_SIGNER.md b/modules/express/EXTERNAL_SIGNER.md index af2539e268f..8cd2e9b135d 100644 --- a/modules/express/EXTERNAL_SIGNER.md +++ b/modules/express/EXTERNAL_SIGNER.md @@ -9,8 +9,32 @@ This may be preferable for users who would like to apply their signature to thei To set up BitGo Express with an external signer, a url to the external signer instance of BitGo Express must be provided using the `externalSignerUrl` configuration option. The corresponding external signer instance of BitGo Express must have `signerMode` set, and `signerFileSystemPath` set to the path of a json containing the private key. +Both instances must be configured with the same shared secret via `signerAuthSecret`; see [Signer authentication](#signer-authentication) below. +A signer-mode instance must either listen on loopback (the default `localhost` bind, or `--ipc`) or serve TLS with client authentication; see [Signer authentication](#signer-authentication). Note that if BitGo Express encounters an `ECONNREFUSED` error when requesting the external signer for a signature, it will retry the request for up to 15 seconds. +### Signer authentication + +Signer-mode routes have access to the wallet keys, so every request to them must be authenticated. Both the online instance (configured with `externalSignerUrl`) and the signer instance (configured with `signerMode`) must be configured with the same shared secret of at least 16 printable non-whitespace ASCII characters: + +- CLI: `--signerauthsecret=` +- Environment variable: `BITGO_SIGNER_AUTH_SECRET` + +The online instance sends the secret in the `X-Auth-Signer` header on every request to the signer. The signer compares it in constant time and rejects requests without a matching header with a `401` before any wallet key material or passphrase is accessed. + +A signer-mode instance listening on anything other than loopback (or an IPC socket) must additionally be configured with TLS client authentication, so the shared secret cannot be intercepted on the network: + +- `--sslca=` together with `--keypath`/`--crtpath` (or `--sslkey`/`--sslcert`) + +The online instance must present a client certificate to such a signer. Configure it on the instance using `--externalSignerUrl`: + +- `--signerclientkey=` and `--signerclientcert=` (or `BITGO_SIGNER_CLIENT_KEY` / `BITGO_SIGNER_CLIENT_CERT`), provided together +- optionally `--signerclientca=` (or `BITGO_SIGNER_CLIENT_CA`) when the signer's server certificate is issued by a private CA + +The client certificate, key and CA are attached to every outbound request to the signer. Startup fails if `--signerclientkey` or `--signerclientcert` is set without the other, or if any of the configured files is unreadable. + +Startup fails if `signerMode` is set without a secret, if `externalSignerUrl` is set without a secret, or if a signer listens on external interfaces without TLS client authentication. + ### Encrypted private key format The JSON file containing the unencrypted private key(s) must be in the format `"": ""`. Note that the `encryptedPrivateKey` must contain escape characters for the double quotations(`"`) in order for it to be parsed correctly. @@ -61,13 +85,8 @@ yarn tsx ``` ### Wallet passphrase -In order for the external signer instance of BitGo Express to decrypt the private key, the wallet passphrase is required. This can be supplied by one of the below methods. - -#### Sending as a body parameter (recommended) -Set the parameter `walletPassphrase: ` (without <>) in POST requests to the endpoint `/api/v2/ofc/signPayload`. - -#### Set as environment variable -Set as an environment variable in the format `WALLET__PASSPHRASE`. Note that the wallet passphrase must be set for each wallet. +In order for the external signer instance of BitGo Express to decrypt the private key, the wallet passphrase is required. +Set it as an environment variable in the format `WALLET__PASSPHRASE`. Note that the wallet passphrase must be set for each wallet. The environment variable can be set using `export`. For example, the wallet passphrases for the private keys above can be set with the following: ``` @@ -75,6 +94,8 @@ export WALLET__PASSPHRASE= export WALLET__PASSPHRASE= ``` +Signer-mode instances never accept a wallet passphrase in the request body; `walletPassphrase` body parameters are only honored by non-signer instances for local signing. Wrong passphrases and unknown wallet IDs are indistinguishable in error responses, and repeated failures lock a wallet ID temporarily. + ### External signer mode configuration values BitGo Express is able to take configuration options from either command line arguments, or via environment variables. @@ -84,8 +105,13 @@ BitGo Express is able to take configuration options from either command line arg | N/A | --externalSignerUrl | `BITGO_EXTERNAL_SIGNER_URL` | N/A | URL specifying the external API to call for remote signing. | | N/A | --signerMode | `BITGO_SIGNER_MODE ` | N/A | If set, run Express as a remote signer. | | N/A | --signerFileSystemPath | `BITGO_SIGNER_FILE_SYSTEM_PATH ` | N/A | Local path specifying where an Express signer machine keeps the encrypted user private keys. Required when signerMode is set. | +| N/A | --signerauthsecret | `BITGO_SIGNER_AUTH_SECRET` | N/A | Shared secret used to authenticate requests to a signer-mode instance. Required when `signerMode` or `externalSignerUrl` is set. | +| N/A | --sslca | `BITGO_SSL_CA` | N/A | Path to a CA certificate for TLS client authentication. Required when `signerMode` listens on external interfaces. | +| N/A | --signerclientkey | `BITGO_SIGNER_CLIENT_KEY` | N/A | Path to the PEM key presented to a signer-mode instance for TLS client authentication. Use together with --signerclientcert. | +| N/A | --signerclientcert | `BITGO_SIGNER_CLIENT_CERT` | N/A | Path to the PEM certificate presented to a signer-mode instance for TLS client authentication. Use together with --signerclientkey. | +| N/A | --signerclientca | `BITGO_SIGNER_CLIENT_CA` | N/A | Path to the CA certificate that a signer-mode instance's server certificate is verified against. | #### Example -To start up an instance of BitGo Express that will use an external signer, start BitGo Express with `--externalSignerUrl=`. -To start up an external signing instance of BitGo Express, which will have access to the encrypted private keys and wallet passphrases, start BitGo Express with `--signerMode --signerFileSystemPath=`. +To start up an instance of BitGo Express that will use an external signer, start BitGo Express with `--externalSignerUrl= --signerauthsecret=` (plus `--signerclientkey`/`--signerclientcert` and, if needed, `--signerclientca` when the signer requires TLS client authentication). +To start up an external signing instance of BitGo Express, which will have access to the encrypted private keys and wallet passphrases, start BitGo Express with `--signerMode --signerFileSystemPath= --signerauthsecret=` (and, when listening on external interfaces, TLS client authentication via `--sslca` together with the server certificate options). diff --git a/modules/express/README.md b/modules/express/README.md index 612b3577a17..42a58a94ac8 100644 --- a/modules/express/README.md +++ b/modules/express/README.md @@ -258,6 +258,11 @@ BitGo Express is able to take configuration options from either command line arg | N/A | --externalSignerUrl | `BITGO_EXTERNAL_SIGNER_URL` | N/A | URL specifying the external API to call for remote signing. | | N/A | --signerMode | `BITGO_SIGNER_MODE ` | N/A | If set, run Express as a remote signer. | | N/A | --signerFileSystemPath | `BITGO_SIGNER_FILE_SYSTEM_PATH ` | N/A | Local path specifying where an Express signer machine keeps the encrypted user private keys. Required when signerMode is set. | +| N/A | --signerauthsecret | `BITGO_SIGNER_AUTH_SECRET` | N/A | Shared secret used to authenticate requests to a signer-mode instance. Required when `signerMode` or `externalSignerUrl` is set. | +| N/A | --sslca | `BITGO_SSL_CA` | N/A | Path to a CA certificate for TLS client authentication. Required when `signerMode` listens on external interfaces. | +| N/A | --signerclientkey | `BITGO_SIGNER_CLIENT_KEY` | N/A | Path to the PEM key presented to a signer-mode instance for TLS client authentication. Use together with --signerclientcert. | +| N/A | --signerclientcert | `BITGO_SIGNER_CLIENT_CERT` | N/A | Path to the PEM certificate presented to a signer-mode instance for TLS client authentication. Use together with --signerclientkey. | +| N/A | --signerclientca | `BITGO_SIGNER_CLIENT_CA` | N/A | Path to the CA certificate that a signer-mode instance's server certificate is verified against. | | N/A | --lightningSignerFileSystemPath | `BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH` | N/A | Local path specifying how to contact the lightning signer node. | | N/A | --keepalivetimeout | `BITGO_KEEP_ALIVE_TIMEOUT` | N/A | BitGo Express server KeepAliveTimeout node. | | N/A | --headerstimeout | `BITGO_HEADERS_TIMEOUT` | N/A | BitGo Express server HeadersTimeout node. | diff --git a/modules/express/src/args.ts b/modules/express/src/args.ts index 180498a62f6..3bd5f2d6385 100644 --- a/modules/express/src/args.ts +++ b/modules/express/src/args.ts @@ -112,6 +112,26 @@ parser.addArgument(['--signerFileSystemPath'], { help: 'Local path specifying where an Express signer machine keeps encrypted user private keys.', }); +parser.addArgument(['--signerauthsecret'], { + help: 'Shared secret used to authenticate requests to a signer-mode instance. Required when signerMode or externalSignerUrl is set.', +}); + +parser.addArgument(['--sslca'], { + help: 'Path to a CA certificate for TLS client authentication. Required when signerMode listens on external interfaces.', +}); + +parser.addArgument(['--signerclientkey'], { + help: 'Path to the PEM key presented to a signer-mode instance for TLS client authentication. Use together with --signerclientcert on an instance configured with --externalSignerUrl.', +}); + +parser.addArgument(['--signerclientcert'], { + help: 'Path to the PEM certificate presented to a signer-mode instance for TLS client authentication. Use together with --signerclientkey.', +}); + +parser.addArgument(['--signerclientca'], { + help: "Path to the CA certificate that a signer-mode instance's server certificate is verified against.", +}); + parser.addArgument(['--lightningSignerFileSystemPath'], { help: 'Local path specifying where an Express machine keeps lightning signer urls.', }); diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 8f8c6b87918..c00a5503de3 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -47,6 +47,7 @@ import type { ParamsDictionary } from 'express-serve-static-core'; import * as _ from 'lodash'; import * as url from 'url'; import * as superagent from 'superagent'; +import * as crypto from 'crypto'; // RequestTracer should be extracted into a separate npm package (along with // the rest of the BitGoJS HTTP request machinery) @@ -55,6 +56,7 @@ import { RequestTracer } from 'bitgo/dist/src/v2/internal/util'; import { Config } from './config'; import { ApiResponseError, BitGoExpressError } from './errors'; import { promises as fs } from 'fs'; +import * as fsSync from 'fs'; import { retryPromise } from './retryPromise'; import { SENSITIVE_REQUEST_KEYS } from './sensitiveRequestKeys'; import { @@ -85,6 +87,100 @@ const BITGOEXPRESS_USER_AGENT = `BitGoExpress/${pjson.version} BitGoJS/${version */ const FORWARDED_HEADERS = ['x-bitgo-otp']; +/** + * Header used to authenticate requests between an online Express instance and + * a signer-mode Express instance. Deliberately not `Authorization`: + * `prepareBitGo` copies the `Authorization` header's token into the upstream + * BitGo object, so the signer secret must travel on its own header. + */ +const SIGNER_AUTH_HEADER = 'x-auth-signer'; + +/** + * Uniform error message for signer-mode wallet failures. Must not reveal + * whether a walletId is known to this signer, whether a passphrase is + * configured, or whether decryption failed - otherwise the signer routes + * become a wallet enumeration and passphrase guessing oracle. + */ +const WALLET_SIGN_FAILURE_MESSAGE = 'unable to sign for the specified wallet'; + +/** Consecutive wallet lookup/decrypt failures before a walletId is locked out. */ +const WALLET_SIGN_FAILURE_LOCKOUT_THRESHOLD = 5; + +/** + * How long a locked-out walletId is refused for, and after which a walletId + * without recent failures becomes prunable. + */ +const WALLET_SIGN_FAILURE_LOCKOUT_MS = 15 * 60 * 1000; + +/** + * Maximum number of walletIds tracked for failures. walletIds are attacker + * influenced (any client of the online instance can make the signer track + * arbitrary strings), so the map must not grow without limit: stale entries + * are pruned on access and the oldest entry is evicted once the map is full. + */ +const MAX_WALLET_FAILURE_ENTRIES = 1024; + +/** WalletIds longer than this are never real wallets and are not tracked. */ +const MAX_TRACKED_WALLET_ID_LENGTH = 512; + +/** Connection options for requests from an online instance to a signer-mode instance. */ +export interface SignerClientOptions { + /** Shared secret sent in the X-Auth-Signer header. */ + signerAuthSecret?: string; + /** Path to the PEM key presented to the signer for TLS client authentication. */ + signerClientKeyPath?: string; + /** Path to the PEM certificate presented to the signer for TLS client authentication. */ + signerClientCertPath?: string; + /** Path to the CA certificate the signer's server certificate is verified against. */ + signerClientCaPath?: string; +} + +/** + * Contents of signer TLS material by file path, cached so that repeated + * requests do not re-read the files. + */ +const signerTlsFileContents = new Map(); + +function readSignerTlsFile(path: string): string { + const cached = signerTlsFileContents.get(path); + if (cached !== undefined) { + return cached; + } + const contents = fsSync.readFileSync(path, 'utf8'); + signerTlsFileContents.set(path, contents); + return contents; +} + +/** + * Attach the signer authentication header and TLS client material to an + * outbound request to a signer-mode Express instance, for whatever is + * configured. + */ +function withSignerAuth(request: superagent.Request, options?: SignerClientOptions): superagent.Request { + let result = request; + if (options?.signerAuthSecret) { + result = result.set(SIGNER_AUTH_HEADER, options.signerAuthSecret); + } + if (options?.signerClientKeyPath && options?.signerClientCertPath) { + result = result + .key(readSignerTlsFile(options.signerClientKeyPath)) + .cert(readSignerTlsFile(options.signerClientCertPath)); + } + if (options?.signerClientCaPath) { + result = result.ca(readSignerTlsFile(options.signerClientCaPath)); + } + return result; +} + +function signerClientOptionsFromConfig(config?: Config): SignerClientOptions { + return { + signerAuthSecret: config?.signerAuthSecret, + signerClientKeyPath: config?.signerClientKeyPath, + signerClientCertPath: config?.signerClientCertPath, + signerClientCaPath: config?.signerClientCaPath, + }; +} + function handlePing( req: ExpressApiRouteRequest<'express.v1.ping' | 'express.ping', 'get'>, res: express.Response, @@ -451,6 +547,86 @@ async function decryptPrivKey(bg: BitGo, encryptedPrivKey: string, walletPw: str } } +interface WalletSignFailureEntry { + count: number; + lockedUntil: number; + lastFailureAt: number; +} + +const walletSignFailures = new Map(); + +/** + * Drop entries whose lockout has expired and whose most recent failure is + * older than the lockout window, so walletIds that never succeed (attacker + * chosen) age out instead of accumulating forever. + */ +function pruneExpiredWalletSignFailures(now: number): void { + for (const [walletId, entry] of walletSignFailures) { + if (entry.lockedUntil <= now && now - entry.lastFailureAt >= WALLET_SIGN_FAILURE_LOCKOUT_MS) { + walletSignFailures.delete(walletId); + } + } +} + +function assertWalletNotLocked(walletId: string): void { + const now = Date.now(); + pruneExpiredWalletSignFailures(now); + const entry = walletSignFailures.get(walletId); + if (entry && entry.lockedUntil > now) { + throw new ApiResponseError(WALLET_SIGN_FAILURE_MESSAGE, 400); + } +} + +function recordWalletSignFailure(walletId: string): void { + // walletIds of absurd length are never real wallets: do not let them + // consume the bounded map + if (walletId.length > MAX_TRACKED_WALLET_ID_LENGTH) { + return; + } + const now = Date.now(); + pruneExpiredWalletSignFailures(now); + const entry = walletSignFailures.get(walletId) ?? { count: 0, lockedUntil: 0, lastFailureAt: 0 }; + entry.count += 1; + entry.lastFailureAt = now; + if (entry.count >= WALLET_SIGN_FAILURE_LOCKOUT_THRESHOLD) { + entry.lockedUntil = now + WALLET_SIGN_FAILURE_LOCKOUT_MS; + entry.count = 0; + } + if (!walletSignFailures.has(walletId) && walletSignFailures.size >= MAX_WALLET_FAILURE_ENTRIES) { + // the map is full after pruning: evict the oldest tracked entry to keep + // memory bounded, and continue throttling best-effort + const oldest = walletSignFailures.keys().next().value; + if (oldest !== undefined) { + walletSignFailures.delete(oldest); + } + } + walletSignFailures.set(walletId, entry); +} + +/** + * Look up the wallet passphrase and encrypted private key for a signer-mode + * wallet and decrypt it, with per-walletId failure lockout and uniform errors: + * unknown walletId, missing passphrase, unreadable key file and failed + * decryption are all indistinguishable to the caller. + */ +async function getSignerWalletCredentials( + bitgo: BitGo, + signerFileSystemPath: string, + walletId: string +): Promise<{ privKey: string; walletPassphrase: string }> { + assertWalletNotLocked(walletId); + try { + const walletPassphrase = getWalletPwFromEnv(walletId); + const encryptedPrivKey = await getEncryptedPrivKey(signerFileSystemPath, walletId); + const privKey = await decryptPrivKey(bitgo, encryptedPrivKey, walletPassphrase); + walletSignFailures.delete(walletId); + return { privKey, walletPassphrase }; + } catch (e) { + recordWalletSignFailure(walletId); + throw new ApiResponseError(WALLET_SIGN_FAILURE_MESSAGE, 400); + } +} + export async function handleV2GenerateShareTSS( req: ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'> ): Promise { @@ -461,16 +637,18 @@ export async function handleV2GenerateShareTSS( throw new ApiResponseError('Missing required field: walletId', 400); } - const walletPw = getWalletPwFromEnv(walletId); const { signerFileSystemPath } = req.config; if (!signerFileSystemPath) { throw new Error('Missing required configuration: signerFileSystemPath'); } - const encryptedPrivKey = await getEncryptedPrivKey(signerFileSystemPath, walletId); const bitgo = req.bitgo; - const privKey = await decryptPrivKey(bitgo, encryptedPrivKey, walletPw); + const { privKey, walletPassphrase: walletPw } = await getSignerWalletCredentials( + bitgo, + signerFileSystemPath, + walletId + ); const coin = bitgo.coin(req.decoded.coin); req.body.prv = privKey; req.body.walletPassphrase = walletPw; @@ -583,16 +761,14 @@ export async function handleV2Sign(req: ExpressApiRouteRequest<'express.v2.coin. throw new Error('Missing required field: walletId'); } - const walletPw = getWalletPwFromEnv(walletId); const { signerFileSystemPath } = req.config; if (!signerFileSystemPath) { throw new Error('Missing required configuration: signerFileSystemPath'); } - const encryptedPrivKey = await getEncryptedPrivKey(signerFileSystemPath, walletId); const bitgo = req.bitgo; - let privKey = await decryptPrivKey(bitgo, encryptedPrivKey, walletPw); + let { privKey } = await getSignerWalletCredentials(bitgo, signerFileSystemPath, walletId); const coin = bitgo.coin(req.decoded.coin); if (req.body.derivationSeed) { privKey = coin.deriveKeyWithSeed({ key: privKey, seed: req.body.derivationSeed }).key; @@ -610,7 +786,6 @@ export async function handleV2OFCSignPayloadInExtSigningMode( ): Promise<{ payload: string; signature: string }> { const walletId = req.body.walletId; const payload = req.body.payload; - const bodyWalletPassphrase = req.body.walletPassphrase; const ofcCoinName = 'ofc'; if (!payload) { @@ -621,20 +796,17 @@ export async function handleV2OFCSignPayloadInExtSigningMode( throw new ApiResponseError('Missing required field: walletId', 400); } - // fetch the password for the given walletId from the body or the env. This is required for decrypting the private key that belongs to that wallet. - const walletPw = bodyWalletPassphrase || getWalletPwFromEnv(walletId); - const { signerFileSystemPath } = req.config; if (!signerFileSystemPath) { throw new ApiResponseError('Missing required configuration: signerFileSystemPath', 500); } - // get the encrypted private key from the local JSON file (encryptedPrivKeys.json) (populated using fetchEncryptedPrivateKeys.ts) - const encryptedPrivKey = await getEncryptedPrivKey(signerFileSystemPath, walletId); const bitgo = req.bitgo; - // decrypt the encrypted private key using the wallet pwd - const privKey = await decryptPrivKey(bitgo, encryptedPrivKey, walletPw); + // The wallet passphrase always comes from the signer's environment + // (WALLET__PASSPHRASE). Accepting a passphrase from the request + // body would let callers probe for the correct wallet passphrase. + const { privKey } = await getSignerWalletCredentials(bitgo, signerFileSystemPath, walletId); // create a BaseCoin instance for 'ofc' const coin = bitgo.coin(ofcCoinName); @@ -666,8 +838,10 @@ export async function handleV2OFCSignPayload( if (externalSignerUrl) { const { body: payloadWithSignature } = await retryPromise( () => - superagent - .post(`${externalSignerUrl}/api/v2/ofc/signPayload`) + withSignerAuth( + superagent.post(`${externalSignerUrl}/api/v2/ofc/signPayload`), + signerClientOptionsFromConfig(req.config) + ) .type('json') .send({ walletId: walletId, payload: payload }), (err, tryCount) => { @@ -1174,7 +1348,10 @@ function createSendParams(req: express.Request) { if (req.config?.externalSignerUrl !== undefined) { return { ...req.body, - customSigningFunction: createCustomSigningFunction(req.config.externalSignerUrl), + customSigningFunction: createCustomSigningFunction( + req.config.externalSignerUrl, + signerClientOptionsFromConfig(req.config) + ), }; } else { return cloneRequestBody(req); @@ -1190,15 +1367,18 @@ function createTSSSendParams(req: express.Request, wallet: Wallet) { ...req.body, customEddsaMPCv2SigningRound1GenerationFunction: createCustomEddsaMPCv2SigningRound1Generator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) ), customEddsaMPCv2SigningRound2GenerationFunction: createCustomEddsaMPCv2SigningRound2Generator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) ), customEddsaMPCv2SigningRound3GenerationFunction: createCustomEddsaMPCv2SigningRound3Generator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) ), }; } else { @@ -1206,10 +1386,19 @@ function createTSSSendParams(req: express.Request, wallet: Wallet) { ...req.body, customCommitmentGeneratingFunction: createCustomCommitmentGenerator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) + ), + customRShareGeneratingFunction: createCustomRShareGenerator( + req.config.externalSignerUrl, + req.params.coin, + signerClientOptionsFromConfig(req.config) + ), + customGShareGeneratingFunction: createCustomGShareGenerator( + req.config.externalSignerUrl, + req.params.coin, + signerClientOptionsFromConfig(req.config) ), - customRShareGeneratingFunction: createCustomRShareGenerator(req.config.externalSignerUrl, req.params.coin), - customGShareGeneratingFunction: createCustomGShareGenerator(req.config.externalSignerUrl, req.params.coin), }; } } else if (coin.getMPCAlgorithm() === MPCType.ECDSA) { @@ -1218,15 +1407,18 @@ function createTSSSendParams(req: express.Request, wallet: Wallet) { ...req.body, customMPCv2SigningRound1GenerationFunction: createCustomMPCv2SigningRound1Generator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) ), customMPCv2SigningRound2GenerationFunction: createCustomMPCv2SigningRound2Generator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) ), customMPCv2SigningRound3GenerationFunction: createCustomMPCv2SigningRound3Generator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) ), }; } else { @@ -1234,14 +1426,24 @@ function createTSSSendParams(req: express.Request, wallet: Wallet) { ...req.body, customPaillierModulusGeneratingFunction: createCustomPaillierModulusGetter( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) + ), + customKShareGeneratingFunction: createCustomKShareGenerator( + req.config.externalSignerUrl, + req.params.coin, + signerClientOptionsFromConfig(req.config) ), - customKShareGeneratingFunction: createCustomKShareGenerator(req.config.externalSignerUrl, req.params.coin), customMuDeltaShareGeneratingFunction: createCustomMuDeltaShareGenerator( req.config.externalSignerUrl, - req.params.coin + req.params.coin, + signerClientOptionsFromConfig(req.config) + ), + customSShareGeneratingFunction: createCustomSShareGenerator( + req.config.externalSignerUrl, + req.params.coin, + signerClientOptionsFromConfig(req.config) ), - customSShareGeneratingFunction: createCustomSShareGenerator(req.config.externalSignerUrl, req.params.coin), }; } } else { @@ -1804,6 +2006,42 @@ function parseBody(req: express.Request, res: express.Response, next: express.Ne return expressJSONParser(req, res, next); } +/** + * Middleware which requires callers of signer-mode routes to present the + * deployment's shared secret via the X-Auth-Signer header. Runs before + * prepareBitGo and before any handler, so unauthenticated requests never + * reach wallet key material. + * + * The secret deliberately does not travel in the Authorization header: + * prepareBitGo copies that header's token into the upstream BitGo object. + */ +export function requireSignerAuth(config: Config): express.RequestHandler { + const { signerAuthSecret } = config; + const secretDigest = signerAuthSecret ? crypto.createHash('sha256').update(signerAuthSecret).digest() : undefined; + + return function requireSignerAuthMiddleware(req: express.Request, res: express.Response, next: express.NextFunction) { + // Fail closed: without a configured secret no request is authorized. + // (checkPreconditions refuses signerMode startup without one; this only + // guards against configuration mistakes.) + if (!secretDigest) { + res.status(401).send({ error: 'unauthorized' }); + return; + } + const headerValue = req.headers[SIGNER_AUTH_HEADER]; + const header = Array.isArray(headerValue) ? headerValue[0] : headerValue; + if (typeof header !== 'string' || header.length === 0) { + res.status(401).send({ error: 'unauthorized' }); + return; + } + const headerDigest = crypto.createHash('sha256').update(header).digest(); + if (!crypto.timingSafeEqual(secretDigest, headerDigest)) { + res.status(401).send({ error: 'unauthorized' }); + return; + } + next(); + }; +} + /** * Create the bitgo object in the request * @param config @@ -1980,16 +2218,24 @@ export function typedPromiseWrapper(promiseRequestHandler: TypedRequestHandler) }; } -export function createCustomSigningFunction(externalSignerUrl: string): CustomSigningFunction { +export function createCustomSigningFunction( + externalSignerUrl: string, + signerClientOptions?: SignerClientOptions +): CustomSigningFunction { return async function (params): Promise { const { body: signedTx } = await retryPromise( () => - superagent.post(`${externalSignerUrl}/api/v2/${params.coin.getChain()}/sign`).type('json').send({ - txPrebuild: params.txPrebuild, - pubs: params.pubs, - derivationSeed: params.derivationSeed, - signingStep: params.signingStep, - }), + withSignerAuth( + superagent.post(`${externalSignerUrl}/api/v2/${params.coin.getChain()}/sign`), + signerClientOptions + ) + .type('json') + .send({ + txPrebuild: params.txPrebuild, + pubs: params.pubs, + derivationSeed: params.derivationSeed, + signingStep: params.signingStep, + }), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -1999,13 +2245,20 @@ export function createCustomSigningFunction(externalSignerUrl: string): CustomSi } export function createCustomPaillierModulusGetter( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomPaillierModulusGetterFunction { return async function (params): Promise<{ userPaillierModulus: string; }> { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/PaillierModulus`).type('json').send(params), + () => + withSignerAuth( + superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/PaillierModulus`), + signerClientOptions + ) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2014,10 +2267,17 @@ export function createCustomPaillierModulusGetter( }; } -export function createCustomKShareGenerator(externalSignerUrl: string, coin: string): CustomKShareGeneratingFunction { +export function createCustomKShareGenerator( + externalSignerUrl: string, + coin: string, + signerClientOptions?: SignerClientOptions +): CustomKShareGeneratingFunction { return async function (params): Promise { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/K`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/K`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2028,11 +2288,15 @@ export function createCustomKShareGenerator(externalSignerUrl: string, coin: str export function createCustomMuDeltaShareGenerator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomMuDeltaShareGeneratingFunction { return async function (params): Promise { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MuDelta`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MuDelta`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2041,10 +2305,17 @@ export function createCustomMuDeltaShareGenerator( }; } -export function createCustomSShareGenerator(externalSignerUrl: string, coin: string): CustomSShareGeneratingFunction { +export function createCustomSShareGenerator( + externalSignerUrl: string, + coin: string, + signerClientOptions?: SignerClientOptions +): CustomSShareGeneratingFunction { return async function (params): Promise { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/S`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/S`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2055,7 +2326,8 @@ export function createCustomSShareGenerator(externalSignerUrl: string, coin: str export function createCustomCommitmentGenerator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomCommitmentGeneratingFunction { return async function (params): Promise<{ userToBitgoCommitment: CommitmentShareRecord; @@ -2063,7 +2335,10 @@ export function createCustomCommitmentGenerator( encryptedUserToBitgoRShare: EncryptedSignerShareRecord; }> { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/commitment`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/commitment`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2072,10 +2347,17 @@ export function createCustomCommitmentGenerator( }; } -export function createCustomRShareGenerator(externalSignerUrl: string, coin: string): CustomRShareGeneratingFunction { +export function createCustomRShareGenerator( + externalSignerUrl: string, + coin: string, + signerClientOptions?: SignerClientOptions +): CustomRShareGeneratingFunction { return async function (params): Promise<{ rShare: SignShare }> { const { body: rShare } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/R`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/R`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2084,10 +2366,17 @@ export function createCustomRShareGenerator(externalSignerUrl: string, coin: str }; } -export function createCustomGShareGenerator(externalSignerUrl: string, coin: string): CustomGShareGeneratingFunction { +export function createCustomGShareGenerator( + externalSignerUrl: string, + coin: string, + signerClientOptions?: SignerClientOptions +): CustomGShareGeneratingFunction { return async function (params): Promise { const { body: signedTx } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/G`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/G`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2098,11 +2387,15 @@ export function createCustomGShareGenerator(externalSignerUrl: string, coin: str export function createCustomMPCv2SigningRound1Generator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomMPCv2SigningRound1GeneratingFunction { return async function (params) { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MPCv2Round1`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MPCv2Round1`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2113,11 +2406,15 @@ export function createCustomMPCv2SigningRound1Generator( export function createCustomMPCv2SigningRound2Generator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomMPCv2SigningRound2GeneratingFunction { return async function (params) { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MPCv2Round2`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MPCv2Round2`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2128,11 +2425,15 @@ export function createCustomMPCv2SigningRound2Generator( export function createCustomMPCv2SigningRound3Generator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomMPCv2SigningRound3GeneratingFunction { return async function (params) { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MPCv2Round3`).type('json').send(params), + () => + withSignerAuth(superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/MPCv2Round3`), signerClientOptions) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2143,11 +2444,18 @@ export function createCustomMPCv2SigningRound3Generator( export function createCustomEddsaMPCv2SigningRound1Generator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomEddsaMPCv2SigningRound1GeneratingFunction { return async function (params) { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/EddsaMPCv2Round1`).type('json').send(params), + () => + withSignerAuth( + superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/EddsaMPCv2Round1`), + signerClientOptions + ) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2158,11 +2466,18 @@ export function createCustomEddsaMPCv2SigningRound1Generator( export function createCustomEddsaMPCv2SigningRound2Generator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomEddsaMPCv2SigningRound2GeneratingFunction { return async function (params) { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/EddsaMPCv2Round2`).type('json').send(params), + () => + withSignerAuth( + superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/EddsaMPCv2Round2`), + signerClientOptions + ) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2173,11 +2488,18 @@ export function createCustomEddsaMPCv2SigningRound2Generator( export function createCustomEddsaMPCv2SigningRound3Generator( externalSignerUrl: string, - coin: string + coin: string, + signerClientOptions?: SignerClientOptions ): CustomEddsaMPCv2SigningRound3GeneratingFunction { return async function (params) { const { body: result } = await retryPromise( - () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/EddsaMPCv2Round3`).type('json').send(params), + () => + withSignerAuth( + superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/EddsaMPCv2Round3`), + signerClientOptions + ) + .type('json') + .send(params), (err, tryCount) => { debug(`failed to connect to external signer (attempt ${tryCount}, error: ${err.message})`); } @@ -2403,9 +2725,18 @@ export function setupSigningRoutes(app: express.Application, config: Config): vo const router = createExpressRouter(); app.use(router); - router.post('express.v2.coin.sign', [prepareBitGo(config), typedPromiseWrapper(handleV2Sign)]); - router.post('express.v2.tssshare.generate', [prepareBitGo(config), typedPromiseWrapper(handleV2GenerateShareTSS)]); + router.post('express.v2.coin.sign', [ + requireSignerAuth(config), + prepareBitGo(config), + typedPromiseWrapper(handleV2Sign), + ]); + router.post('express.v2.tssshare.generate', [ + requireSignerAuth(config), + prepareBitGo(config), + typedPromiseWrapper(handleV2GenerateShareTSS), + ]); router.post('express.v2.ofc.extSignPayload', [ + requireSignerAuth(config), prepareBitGo(config), typedPromiseWrapper(handleV2OFCSignPayloadInExtSigningMode), ]); diff --git a/modules/express/src/config.ts b/modules/express/src/config.ts index ba1e65bd8b8..247849a26ba 100644 --- a/modules/express/src/config.ts +++ b/modules/express/src/config.ts @@ -49,6 +49,11 @@ export interface Config { externalSignerUrl?: string; signerMode?: boolean; signerFileSystemPath?: string; + signerAuthSecret?: string; + sslCa?: string; + signerClientKeyPath?: string; + signerClientCertPath?: string; + signerClientCaPath?: string; lightningSignerFileSystemPath?: string; keepAliveTimeout?: number; headersTimeout?: number; @@ -76,6 +81,11 @@ export const ArgConfig = (args): Partial => ({ externalSignerUrl: args.externalSignerUrl, signerMode: args.signerMode, signerFileSystemPath: args.signerFileSystemPath, + signerAuthSecret: args.signerauthsecret, + sslCa: args.sslca, + signerClientKeyPath: args.signerclientkey, + signerClientCertPath: args.signerclientcert, + signerClientCaPath: args.signerclientca, lightningSignerFileSystemPath: args.lightningSignerFileSystemPath, keepAliveTimeout: args.keepalivetimeout, headersTimeout: args.headerstimeout, @@ -103,6 +113,11 @@ export const EnvConfig = (): Partial => ({ externalSignerUrl: readEnvVar('BITGO_EXTERNAL_SIGNER_URL'), signerMode: readEnvVar('BITGO_SIGNER_MODE') ? true : undefined, signerFileSystemPath: readEnvVar('BITGO_SIGNER_FILE_SYSTEM_PATH'), + signerAuthSecret: readEnvVar('BITGO_SIGNER_AUTH_SECRET'), + sslCa: readEnvVar('BITGO_SSL_CA'), + signerClientKeyPath: readEnvVar('BITGO_SIGNER_CLIENT_KEY'), + signerClientCertPath: readEnvVar('BITGO_SIGNER_CLIENT_CERT'), + signerClientCaPath: readEnvVar('BITGO_SIGNER_CLIENT_CA'), lightningSignerFileSystemPath: readEnvVar('BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH'), keepAliveTimeout: Number(readEnvVar('BITGO_KEEP_ALIVE_TIMEOUT')), headersTimeout: Number(readEnvVar('BITGO_HEADERS_TIMEOUT')), @@ -191,6 +206,11 @@ function mergeConfigs(...configs: Partial[]): Config { externalSignerUrl, signerMode: get('signerMode'), signerFileSystemPath: get('signerFileSystemPath'), + signerAuthSecret: get('signerAuthSecret'), + sslCa: get('sslCa'), + signerClientKeyPath: get('signerClientKeyPath'), + signerClientCertPath: get('signerClientCertPath'), + signerClientCaPath: get('signerClientCaPath'), lightningSignerFileSystemPath: get('lightningSignerFileSystemPath'), keepAliveTimeout: get('keepAliveTimeout'), headersTimeout: get('headersTimeout'), diff --git a/modules/express/src/expressApp.ts b/modules/express/src/expressApp.ts index 71130354dd5..a50cfb78f5b 100644 --- a/modules/express/src/expressApp.ts +++ b/modules/express/src/expressApp.ts @@ -30,6 +30,32 @@ import { Environments } from 'bitgo'; import * as clientRoutes from './clientRoutes'; import { getBitGoTlsPolicy } from './tlsPolicy'; +/** Minimum length of the shared secret used to authenticate signer-mode requests. */ +const MIN_SIGNER_AUTH_SECRET_LENGTH = 16; + +/** + * A signer auth secret must be a valid X-Auth-Signer header value on both sides + * of the connection: printable, non-whitespace ASCII only. A trailing newline + * makes Node refuse to send the header, and surrounding whitespace is silently + * stripped by header parsing on the signer, so in either case every request + * would fail authentication at runtime. Invalid secrets are rejected at startup + * instead, on both the signer and the online instance. + */ +const SIGNER_AUTH_SECRET_PATTERN = /^[\x21-\x7e]+$/; + +function isValidSignerAuthSecret(secret: string | undefined): boolean { + return ( + secret !== undefined && secret.length >= MIN_SIGNER_AUTH_SECRET_LENGTH && SIGNER_AUTH_SECRET_PATTERN.test(secret) + ); +} + +/** Bind addresses considered loopback; signer mode may listen there without client-auth TLS. */ +const LOOPBACK_BIND_ADDRESSES: Record = { + localhost: true, + '127.0.0.1': true, + '::1': true, +}; + /** * Set up the logging middleware provided by morgan * @@ -81,7 +107,7 @@ function configureEnvironment(config: Config): void { * @return {Server} */ async function createHttpsServer(app: express.Application, config: Config): Promise { - const { keyPath, crtPath, sslKey, sslCert } = config; + const { keyPath, crtPath, sslKey, sslCert, sslCa } = config; let key: string; let cert: string; if (sslKey && sslCert) { @@ -96,7 +122,15 @@ async function createHttpsServer(app: express.Application, config: Config): Prom throw new Error('Failed to get ssl key and certificate'); } - return https.createServer({ secureOptions: SSL_OP_NO_TLSv1, key, cert }, app); + const serverOptions: https.ServerOptions = { secureOptions: SSL_OP_NO_TLSv1, key, cert }; + if (sslCa) { + // Require clients to present a certificate signed by the configured CA. + // Required for signer-mode instances listening on external interfaces. + serverOptions.ca = await fs.promises.readFile(sslCa, 'utf8'); + serverOptions.requestCert = true; + serverOptions.rejectUnauthorized = true; + } + return https.createServer(serverOptions, app); } /** @@ -211,6 +245,11 @@ function checkPreconditions(config: Config) { externalSignerUrl, signerMode, signerFileSystemPath, + signerAuthSecret, + sslCa, + signerClientKeyPath, + signerClientCertPath, + signerClientCaPath, lightningSignerFileSystemPath, } = config; @@ -261,6 +300,63 @@ function checkPreconditions(config: Config) { ); } + // A signer-mode instance holds wallet keys and must not accept unauthenticated requests: + // it requires a shared secret, and (unless listening on loopback/IPC) TLS with client + // authentication so the secret cannot be intercepted on the network. + if (signerMode !== undefined && !isValidSignerAuthSecret(signerAuthSecret)) { + throw new ExternalSignerConfigError( + `signerMode requires a signer auth secret of at least ${MIN_SIGNER_AUTH_SECRET_LENGTH} ` + + 'printable non-whitespace ASCII characters (--signerauthsecret / BITGO_SIGNER_AUTH_SECRET).' + ); + } + + // An instance using an external signer must be able to authenticate to it. + if (externalSignerUrl !== undefined && !isValidSignerAuthSecret(signerAuthSecret)) { + throw new ExternalSignerConfigError( + `externalSignerUrl requires a signer auth secret of at least ${MIN_SIGNER_AUTH_SECRET_LENGTH} ` + + 'printable non-whitespace ASCII characters (--signerauthsecret / BITGO_SIGNER_AUTH_SECRET) ' + + 'so requests to the signer can be authenticated.' + ); + } + + if (signerMode !== undefined) { + const isLoopbackListener = Boolean(ipc) || LOOPBACK_BIND_ADDRESSES[bind] === true; + const hasTlsMaterial = Boolean((keyPath && crtPath) || (sslKey && sslCert)); + if (!isLoopbackListener && !(sslCa && hasTlsMaterial)) { + throw new TlsConfigurationError( + 'signerMode listening on external interfaces requires TLS with client authentication: ' + + 'configure --sslca together with --keypath/--crtpath (or --sslkey/--sslcert), ' + + 'or bind to localhost (--bind localhost) or an IPC socket (--ipc).' + ); + } + } + + if (sslCa !== undefined && !Boolean((keyPath && crtPath) || (sslKey && sslCert))) { + throw new TlsConfigurationError('sslCa requires TLS to be enabled (--keypath/--crtpath or --sslkey/--sslcert).'); + } + + // The online instance presents a client certificate to signers that require + // TLS client authentication. The key and certificate must come as a pair, + // and all configured files must be readable. + if (Boolean(signerClientKeyPath) !== Boolean(signerClientCertPath)) { + throw new TlsConfigurationError( + 'signerclientkey and signerclientcert must be provided together for TLS client authentication.' + ); + } + for (const { name, path } of [ + { name: 'signerclientkey', path: signerClientKeyPath }, + { name: 'signerclientcert', path: signerClientCertPath }, + { name: 'signerclientca', path: signerClientCaPath }, + ]) { + if (path !== undefined) { + try { + fs.readFileSync(path, 'utf8'); + } catch (e) { + throw new TlsConfigurationError(`Failed to read ${name} file at ${path}`); + } + } + } + if (signerFileSystemPath !== undefined) { checkJsonFilePath(signerFileSystemPath); } diff --git a/modules/express/test/integration/externalSigner.ts b/modules/express/test/integration/externalSigner.ts index 3400cf5ed63..d480301481f 100644 --- a/modules/express/test/integration/externalSigner.ts +++ b/modules/express/test/integration/externalSigner.ts @@ -18,6 +18,7 @@ describe('Custom signing function', () => { debug: true, env: 'test', externalSignerUrl, + signerAuthSecret: 'integrationtestsignerauthsecret', timeout: 60000, }; diff --git a/modules/express/test/lib/testutil.ts b/modules/express/test/lib/testutil.ts index 426cc6d0c88..5b46841945d 100644 --- a/modules/express/test/lib/testutil.ts +++ b/modules/express/test/lib/testutil.ts @@ -3,6 +3,14 @@ */ import * as request from 'supertest'; import { app as expressApp } from '../../src/expressApp'; +import { Config, DefaultConfig } from '../../src/config'; + +/** + * Shared secret used by signer-mode test instances. When a test configures + * `signerMode` or `externalSignerUrl`, setupAgent wires this secret in and + * requests must present it via the X-Auth-Signer header. + */ +export const SIGNER_AUTH_SECRET = 'test-signer-auth-secret-0123456789'; // helper function to unlock a token for a specified time export function unlockToken(agent, accessToken, seconds) { @@ -15,13 +23,13 @@ export function unlockToken(agent, accessToken, seconds) { }); } -export function setupAgent(config?: any): request.SuperAgentTest { - const args: any = { - debug: false, - env: 'test', - logfile: '/dev/null', - ...config, - }; +export function setupAgent(config?: Partial): request.SuperAgentTest { + const args: Config = { ...DefaultConfig, ...config }; + + // Signer-mode instances refuse to start without a shared secret. + if ((args.signerMode !== undefined || args.externalSignerUrl !== undefined) && args.signerAuthSecret === undefined) { + args.signerAuthSecret = SIGNER_AUTH_SECRET; + } const app = expressApp(args); return request.agent(app); diff --git a/modules/express/test/unit/bitgoExpress.ts b/modules/express/test/unit/bitgoExpress.ts index d12a60b12ac..14edc7bed88 100644 --- a/modules/express/test/unit/bitgoExpress.ts +++ b/modules/express/test/unit/bitgoExpress.ts @@ -475,8 +475,10 @@ describe('Bitgo Express', function () { const args: any = { env: 'test', + bind: 'localhost', signerMode: 'signerMode', signerFileSystemPath: 'signerFileSystemPath', + signerAuthSecret: 'signerauthsecret0123', }; app(args); @@ -505,6 +507,8 @@ describe('Bitgo Express', function () { const readFileStub = sinon.stub(fs, 'readFileSync').returns(validPrvJSON); args.signerMode = 'signerMode'; + args.signerAuthSecret = 'signerauthsecret0123'; + args.bind = 'localhost'; (() => expressApp(args)).should.not.throw(); readFileStub.restore(); @@ -522,6 +526,7 @@ describe('Bitgo Express', function () { }); args.signerMode = undefined; + args.signerAuthSecret = 'signerauthsecret0123'; (() => expressApp(args)).should.not.throw(); }); @@ -547,8 +552,10 @@ describe('Bitgo Express', function () { it('should require that an signerFileSystemPath contains a parsable json', function () { const args: any = { env: 'test', + bind: 'localhost', signerMode: 'signerMode', signerFileSystemPath: 'invalidSignerFileSystemPath', + signerAuthSecret: 'signerauthsecret0123', }; (() => expressApp(args)).should.throw(); @@ -562,6 +569,154 @@ describe('Bitgo Express', function () { readValidStub.restore(); }); + it('should require a signer auth secret when running in signer mode', function () { + const readFileStub = sinon.stub(fs, 'readFileSync').returns(validPrvJSON); + const args: any = { + env: 'test', + bind: 'localhost', + signerMode: 'signerMode', + signerFileSystemPath: 'signerFileSystemPath', + }; + + (() => expressApp(args)).should.throw({ + name: 'ExternalSignerConfigError', + message: + 'signerMode requires a signer auth secret of at least 16 printable non-whitespace ASCII characters ' + + '(--signerauthsecret / BITGO_SIGNER_AUTH_SECRET).', + }); + + args.signerAuthSecret = 'short'; + (() => expressApp(args)).should.throw({ + name: 'ExternalSignerConfigError', + message: + 'signerMode requires a signer auth secret of at least 16 printable non-whitespace ASCII characters ' + + '(--signerauthsecret / BITGO_SIGNER_AUTH_SECRET).', + }); + + // the secret must survive HTTP header transport on both sides: a newline + // makes Node refuse to send the header, and whitespace is stripped by + // header parsing, so every request would fail authentication at runtime + for (const invalidSecret of [ + 'signerauthsecret0123\n', + ' signerauthsecret0123', + 'signerauthsecret0123 ', + 'signer auth secret 0123', + 'signerauthsecret\u00e90123', + 'signerauthsecret0123\u007f', + ]) { + args.signerAuthSecret = invalidSecret; + (() => expressApp(args)).should.throw({ name: 'ExternalSignerConfigError' }); + } + + args.signerAuthSecret = 'signerauthsecret0123'; + (() => expressApp(args)).should.not.throw(); + + readFileStub.restore(); + }); + + it('should require a signer auth secret when using an external signer', function () { + const args: any = { + env: 'test', + externalSignerUrl: 'https://external-signer.example', + }; + + (() => expressApp(args)).should.throw({ + name: 'ExternalSignerConfigError', + message: + 'externalSignerUrl requires a signer auth secret of at least 16 printable non-whitespace ASCII characters ' + + '(--signerauthsecret / BITGO_SIGNER_AUTH_SECRET) so requests to the signer can be authenticated.', + }); + + args.signerAuthSecret = 'signerauthsecret0123'; + (() => expressApp(args)).should.not.throw(); + }); + + it('should refuse signer mode on external interfaces without TLS client authentication, regardless of env', function () { + const readFileStub = sinon.stub(fs, 'readFileSync').returns(validPrvJSON); + const args: any = { + env: 'test', + bind: '0.0.0.0', + signerMode: 'signerMode', + signerFileSystemPath: 'signerFileSystemPath', + signerAuthSecret: 'signerauthsecret0123', + }; + + (() => expressApp(args)).should.throw({ + name: 'TlsConfigurationError', + message: + 'signerMode listening on external interfaces requires TLS with client authentication: ' + + 'configure --sslca together with --keypath/--crtpath (or --sslkey/--sslcert), ' + + 'or bind to localhost (--bind localhost) or an IPC socket (--ipc).', + }); + + // Server-only TLS (no client CA) is not enough + args.sslKey = 'ssl-key'; + args.sslCert = 'ssl-cert'; + (() => expressApp(args)).should.throw({ name: 'TlsConfigurationError' }); + + // TLS client authentication satisfies the gate, even in a non-prod env + args.sslCa = 'sslca'; + (() => expressApp(args)).should.not.throw(); + + // Loopback and IPC listeners are fine without TLS + args.sslCa = undefined; + args.sslKey = undefined; + args.sslCert = undefined; + args.bind = 'localhost'; + (() => expressApp(args)).should.not.throw(); + + args.bind = '0.0.0.0'; + args.ipc = 'expressIPC'; + (() => expressApp(args)).should.not.throw(); + + readFileStub.restore(); + }); + + it('should refuse sslCa without TLS server material', function () { + const args: any = { + env: 'test', + bind: 'localhost', + signerMode: 'signerMode', + signerFileSystemPath: 'signerFileSystemPath', + signerAuthSecret: 'signerauthsecret0123', + sslCa: 'sslca', + }; + + (() => expressApp(args)).should.throw({ + name: 'TlsConfigurationError', + message: 'sslCa requires TLS to be enabled (--keypath/--crtpath or --sslkey/--sslcert).', + }); + }); + + it('should require signerclientkey and signerclientcert together', function () { + const args: any = { + env: 'test', + signerClientKeyPath: 'client-key.pem', + }; + + (() => expressApp(args)).should.throw({ + name: 'TlsConfigurationError', + message: 'signerclientkey and signerclientcert must be provided together for TLS client authentication.', + }); + + args.signerClientKeyPath = undefined; + args.signerClientCertPath = 'client-cert.pem'; + (() => expressApp(args)).should.throw({ name: 'TlsConfigurationError' }); + }); + + it('should require signer client TLS files to be readable', function () { + const args: any = { + env: 'test', + signerClientKeyPath: '/nonexistent/client-key.pem', + signerClientCertPath: '/nonexistent/client-cert.pem', + }; + + (() => expressApp(args)).should.throw({ + name: 'TlsConfigurationError', + message: 'Failed to read signerclientkey file at /nonexistent/client-key.pem', + }); + }); + it('should require that an lightningSignerFileSystemPath contains a parsable json', function () { const args: any = { env: 'test', diff --git a/modules/express/test/unit/clientRoutes/signPayload.ts b/modules/express/test/unit/clientRoutes/signPayload.ts index b8fb7992fec..02058a79063 100644 --- a/modules/express/test/unit/clientRoutes/signPayload.ts +++ b/modules/express/test/unit/clientRoutes/signPayload.ts @@ -207,19 +207,9 @@ describe('With the handler to sign an arbitrary payload in external signing mode signMessageStub.restore(); }); - it('should use wallet passphrase from request body', async () => { - const stubbedSignature = Buffer.from('mysign'); + it('should ignore a wallet passphrase supplied in the request body', async () => { const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(validPrv); - const signMessageStub = sinon.stub(Coin.Ofc.prototype, 'signMessage').resolves(stubbedSignature); - - const stubbedSigHex = stubbedSignature.toString('hex'); - - const expectedResponse = { - payload: JSON.stringify(payload), - signature: stubbedSigHex, - }; - const req = { bitgo, body: { @@ -237,18 +227,14 @@ describe('With the handler to sign an arbitrary payload in external signing mode }, } as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>; - await handleV2OFCSignPayloadInExtSigningMode(req).should.be.resolvedWith(expectedResponse); - readFileStub.should.be.calledOnceWith('signerFileSystemPath'); - signMessageStub.should.be.calledOnceWith( - sinon.match({ - prv: secret, - }) - ); + // no WALLET__PASSPHRASE in env: the body passphrase must not be + // used as a fallback, and the response must not reveal the cause + await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith('unable to sign for the specified wallet'); + sinon.assert.notCalled(readFileStub); readFileStub.restore(); - signMessageStub.restore(); }); - it('should prioritize request body passphrase over environment variable', async () => { + it('should use the environment passphrase and ignore a different passphrase in the request body', async () => { const stubbedSignature = Buffer.from('mysign'); const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(validPrv); const envStub = sinon.stub(process, 'env').value({ [`WALLET_${walletId}_PASSPHRASE`]: walletPassword }); @@ -261,17 +247,18 @@ describe('With the handler to sign an arbitrary payload in external signing mode payload: JSON.stringify(payload), signature: stubbedSigHex, }; + const req = { bitgo, body: { walletId, payload, - walletPassphrase: walletPassword, + walletPassphrase: 'not-the-real-passphrase', }, decoded: { walletId, payload, - walletPassphrase: walletPassword, + walletPassphrase: 'not-the-real-passphrase', }, config: { signerFileSystemPath: 'signerFileSystemPath', @@ -293,7 +280,7 @@ describe('With the handler to sign an arbitrary payload in external signing mode describe('With invalid setup', () => { const invalidPrv = JSON.stringify({ [walletId]: 'invalid' }); - it('should throw an error with missing wallet passphrase in env', async () => { + it('should throw a uniform error with missing wallet passphrase in env', async () => { const req = { bitgo, body: { @@ -304,10 +291,13 @@ describe('With the handler to sign an arbitrary payload in external signing mode walletId, payload, }, + config: { + signerFileSystemPath: 'signerFileSystemPath', + }, } as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>; await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith( - `Could not find wallet passphrase WALLET_${walletId}_PASSPHRASE in environment` + 'unable to sign for the specified wallet' ); }); @@ -335,7 +325,7 @@ describe('With the handler to sign an arbitrary payload in external signing mode envStub.restore(); }); - it('should throw error when trying to decrypt with invalid private key', async () => { + it('should throw a uniform error when trying to decrypt with invalid private key', async () => { const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(invalidPrv); const envStub = sinon.stub(process, 'env').value({ [`WALLET_${walletId}_PASSPHRASE`]: walletPassword }); @@ -355,14 +345,14 @@ describe('With the handler to sign an arbitrary payload in external signing mode } as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>; await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith( - 'Error when trying to decrypt private key: decrypt: ciphertext is not valid JSON' + 'unable to sign for the specified wallet' ); readFileStub.restore(); envStub.restore(); }); - it('should throw error when trying to decrypt with invalid wallet passphrase key', async () => { + it('should throw a uniform error when the env wallet passphrase is wrong', async () => { const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(validPrv); const envStub = sinon.stub(process, 'env').value({ [`WALLET_${walletId}_PASSPHRASE`]: 'invalidPassphrase' }); @@ -382,14 +372,14 @@ describe('With the handler to sign an arbitrary payload in external signing mode } as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>; await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith( - 'unable to decrypt keychain with the given wallet passphrase' + 'unable to sign for the specified wallet' ); readFileStub.restore(); envStub.restore(); }); - it('should throw error when trying to decrypt with invalid wallet passphrase in body', async () => { + it('should throw a uniform error when the request body supplies a wallet passphrase', async () => { const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(validPrv); const req = { @@ -410,7 +400,7 @@ describe('With the handler to sign an arbitrary payload in external signing mode } as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>; await handleV2OFCSignPayloadInExtSigningMode(req).should.be.rejectedWith( - 'unable to decrypt keychain with the given wallet passphrase' + 'unable to sign for the specified wallet' ); readFileStub.restore(); diff --git a/modules/express/test/unit/clientRoutes/signerAuth.ts b/modules/express/test/unit/clientRoutes/signerAuth.ts new file mode 100644 index 00000000000..6ef29d646a6 --- /dev/null +++ b/modules/express/test/unit/clientRoutes/signerAuth.ts @@ -0,0 +1,394 @@ +/** + * Tests for signer-mode route authentication, uniform wallet errors and the + * per-walletId failure lockout (WAL-2458). + */ +import 'should'; +import 'should-http'; +import 'should-sinon'; +import '../../lib/asserts'; +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import nock from 'nock'; +import * as fsSync from 'fs'; +import { promises as fs } from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import * as express from 'express'; +import * as request from 'supertest'; + +import { BitGo, CustomSigningFunction } from 'bitgo'; +import { DefaultConfig } from '../../../src/config'; +import { + createCustomSigningFunction, + handleV2OFCSignPayloadInExtSigningMode, + requireSignerAuth, +} from '../../../src/clientRoutes'; +import { ApiResponseError } from '../../../src/errors'; +import { setupAgent, SIGNER_AUTH_SECRET } from '../../lib/testutil'; +import { ExpressApiRouteRequest } from '../../../src/typedRoutes/api'; + +describe('Signer route authentication (WAL-2458)', function () { + describe('requireSignerAuth middleware', function () { + const config = { ...DefaultConfig, signerAuthSecret: SIGNER_AUTH_SECRET }; + + // test doubles standing in for express objects + interface ResponseDouble { + status: sinon.SinonStub; + send: sinon.SinonStub; + } + const makeRes = (): ResponseDouble => ({ status: sinon.stub().returnsThis(), send: sinon.stub().returnsThis() }); + const makeReq = (headers: Record) => ({ headers } as unknown as express.Request); + + it('rejects requests without the signer auth header with a generic 401', function () { + const res = makeRes(); + const next = sinon.spy(); + requireSignerAuth(config)(makeReq({}), res as unknown as express.Response, next); + assert.strictEqual(res.status.calledWith(401), true); + assert.strictEqual(res.send.calledWith({ error: 'unauthorized' }), true); + sinon.assert.notCalled(next); + }); + + it('rejects requests with a wrong secret with a generic 401', function () { + const res = makeRes(); + const next = sinon.spy(); + requireSignerAuth(config)( + makeReq({ 'x-auth-signer': 'not-the-secret' }), + res as unknown as express.Response, + next + ); + assert.strictEqual(res.status.calledWith(401), true); + sinon.assert.notCalled(next); + }); + + it('passes requests presenting the shared secret', function () { + const res = makeRes(); + const next = sinon.spy(); + requireSignerAuth(config)( + makeReq({ 'x-auth-signer': SIGNER_AUTH_SECRET }), + res as unknown as express.Response, + next + ); + assert.strictEqual(res.status.called, false); + sinon.assert.calledOnce(next); + }); + + it('fails closed when no secret is configured', function () { + const res = makeRes(); + const next = sinon.spy(); + requireSignerAuth({ ...DefaultConfig, signerAuthSecret: undefined })( + makeReq({ 'x-auth-signer': 'anything' }), + res as unknown as express.Response, + next + ); + assert.strictEqual(res.status.calledWith(401), true); + sinon.assert.notCalled(next); + }); + }); + + describe('signer-mode routes', function () { + const signerFilePath = path.join(os.tmpdir(), `signerAuth-${process.pid}-${Date.now()}.json`); + let agent: request.SuperAgentTest; + + before(function () { + fsSync.writeFileSync(signerFilePath, JSON.stringify({ someWalletId: 'encrypted' })); + agent = setupAgent({ signerMode: true, signerFileSystemPath: signerFilePath }); + }); + + after(function () { + try { + fsSync.unlinkSync(signerFilePath); + } catch { + // ignore + } + }); + + // body passing codec validation, so the auth middleware is what rejects it + const validSignBody = { + txPrebuild: { + walletId: 'someWalletId', + txHex: + '0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000', + }, + }; + + it('returns 401 for requests without the signer auth header, before any handler runs', async function () { + const res = await agent.post('/api/v2/btc/sign').set('Content-Type', 'application/json').send(validSignBody); + assert.strictEqual(res.status, 401); + assert.deepStrictEqual(res.body, { error: 'unauthorized' }); + }); + + it('returns 401 for requests with a wrong secret', async function () { + const res = await agent + .post('/api/v2/btc/sign') + .set('Content-Type', 'application/json') + .set('X-Auth-Signer', 'wrong-secret') + .send(validSignBody); + assert.strictEqual(res.status, 401); + }); + + it('lets authenticated requests reach the route handler', async function () { + const res = await agent + .post('/api/v2/btc/sign') + .set('Content-Type', 'application/json') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) + .send({}); + // reaching the handler means the request is no longer rejected by the + // auth middleware; the invalid body fails validation instead + assert.notStrictEqual(res.status, 401); + }); + }); + + describe('uniform wallet errors and lockout', function () { + const bitgo = new BitGo({ env: 'test' }); + const uniformityWalletId = `uniformity${process.pid}${Date.now()}`; + const lockoutWalletId = `lockout${process.pid}${Date.now()}`; + const junkLengthWalletId = `${'x'.repeat(600)}${Date.now()}`; + let validPrv: string; + + before(async function () { + const encryptedPrvByWallet: [string, string][] = [ + [uniformityWalletId, 'signer-auth-test-priv-1'], + [lockoutWalletId, 'signer-auth-test-priv-2'], + [junkLengthWalletId, 'signer-auth-test-priv-3'], + ]; + const entries: [string, string][] = []; + for (const [walletId, input] of encryptedPrvByWallet) { + const encryptedPrv = await bitgo.encrypt({ + password: 'correct-wallet-passphrase', + input, + encryptionVersion: 1, + }); + entries.push([walletId, encryptedPrv]); + } + validPrv = JSON.stringify(Object.fromEntries(entries)); + }); + + const makeReq = (walletId: string) => + ({ + bitgo, + body: { walletId, payload: { any: 'payload' } }, + config: { signerFileSystemPath: 'signerFileSystemPath' }, + } as unknown as ExpressApiRouteRequest<'express.v2.ofc.extSignPayload', 'post'>); + + it('returns the same error for an unknown wallet and a wrong passphrase', async function () { + const readFileStub = sinon.stub(fs, 'readFile').resolves(validPrv); + + // unknown walletId: not in the key file and no env passphrase configured + const noEnvStub = sinon.stub(process, 'env').value({}); + let unknownWalletError: unknown; + try { + await handleV2OFCSignPayloadInExtSigningMode(makeReq('unknownwalletid0000000000000000')); + } catch (e) { + unknownWalletError = e; + } + noEnvStub.restore(); + + // wrong passphrase: wallet present in the key file, env passphrase set but wrong + const wrongPassphraseEnvStub = sinon + .stub(process, 'env') + .value({ [`WALLET_${uniformityWalletId}_PASSPHRASE`]: 'wrong-passphrase' }); + let wrongPassphraseError: unknown; + try { + await handleV2OFCSignPayloadInExtSigningMode(makeReq(uniformityWalletId)); + } catch (e) { + wrongPassphraseError = e; + } + wrongPassphraseEnvStub.restore(); + readFileStub.restore(); + + const unknownWallet = unknownWalletError as ApiResponseError; + const wrongPassphrase = wrongPassphraseError as ApiResponseError; + assert.ok(unknownWallet instanceof ApiResponseError); + assert.ok(wrongPassphrase instanceof ApiResponseError); + assert.strictEqual(unknownWallet.status, 400); + assert.strictEqual(wrongPassphrase.status, 400); + assert.strictEqual(unknownWallet.message, wrongPassphrase.message); + }); + + it('locks a walletId out after repeated failures without further key access', async function () { + const readFileStub = sinon.stub(fs, 'readFile').resolves(validPrv); + const envStub = sinon + .stub(process, 'env') + .value({ [`WALLET_${lockoutWalletId}_PASSPHRASE`]: 'wrong-passphrase' }); + + for (let attempt = 0; attempt < 5; attempt++) { + await handleV2OFCSignPayloadInExtSigningMode(makeReq(lockoutWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + } + + // locked out: same uniform error, but the key file is not read and the + // key is not decrypted anymore + const decryptSpy = sinon.spy(bitgo, 'decrypt'); + await handleV2OFCSignPayloadInExtSigningMode(makeReq(lockoutWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + sinon.assert.notCalled(decryptSpy); + assert.strictEqual(readFileStub.callCount, 5); + decryptSpy.restore(); + + readFileStub.restore(); + envStub.restore(); + }); + + it('expires lockouts after the lockout window', async function () { + const readFileStub = sinon.stub(fs, 'readFile').resolves(validPrv); + const envStub = sinon + .stub(process, 'env') + .value({ [`WALLET_${lockoutWalletId}_PASSPHRASE`]: 'wrong-passphrase' }); + + for (let attempt = 0; attempt < 5; attempt++) { + await handleV2OFCSignPayloadInExtSigningMode(makeReq(lockoutWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + } + + const decryptSpy = sinon.spy(bitgo, 'decrypt'); + const clock = sinon.useFakeTimers(Date.now()); + clock.tick(15 * 60 * 1000 + 1000); + + // the lockout and the failure history have expired: the failure path + // runs again and reaches decryption + await handleV2OFCSignPayloadInExtSigningMode(makeReq(lockoutWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + sinon.assert.calledOnce(decryptSpy); + clock.restore(); + decryptSpy.restore(); + + readFileStub.restore(); + envStub.restore(); + }); + + it('never locks out absurdly long walletIds, and does not track them', async function () { + const readFileStub = sinon.stub(fs, 'readFile').resolves(validPrv); + const envStub = sinon + .stub(process, 'env') + .value({ [`WALLET_${junkLengthWalletId}_PASSPHRASE`]: 'wrong-passphrase' }); + + const decryptSpy = sinon.spy(bitgo, 'decrypt'); + for (let attempt = 0; attempt < 6; attempt++) { + await handleV2OFCSignPayloadInExtSigningMode(makeReq(junkLengthWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + } + // untracked, so never locked out: every attempt reached decryption + sinon.assert.callCount(decryptSpy, 6); + decryptSpy.restore(); + + readFileStub.restore(); + envStub.restore(); + }); + + it('keeps throttling real wallets after floods of unknown walletIds', async function () { + const readFileStub = sinon.stub(fs, 'readFile').resolves(validPrv); + const envStub = sinon.stub(process, 'env').value({}); + + // 2000 attacker-chosen unknown walletIds: the failure map is capped, so + // this must neither grow memory without limit nor break throttling + for (let attempt = 0; attempt < 2000; attempt++) { + await handleV2OFCSignPayloadInExtSigningMode(makeReq(`floodwallet${attempt}`)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + } + + envStub.value({ [`WALLET_${lockoutWalletId}_PASSPHRASE`]: 'wrong-passphrase' }); + for (let attempt = 0; attempt < 5; attempt++) { + await handleV2OFCSignPayloadInExtSigningMode(makeReq(lockoutWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + } + const decryptSpy = sinon.spy(bitgo, 'decrypt'); + await handleV2OFCSignPayloadInExtSigningMode(makeReq(lockoutWalletId)).should.be.rejectedWith( + 'unable to sign for the specified wallet' + ); + sinon.assert.notCalled(decryptSpy); + decryptSpy.restore(); + + readFileStub.restore(); + envStub.restore(); + }); + }); + + describe('online express client forwards', function () { + const signerUrl = 'http://external-signer.example'; + + // other suites in this directory toggle the global nock interceptor; use it + // for these tests, then leave it exactly as it was found + let nockWasActive = false; + before(function () { + nockWasActive = nock.isActive(); + if (!nockWasActive) { + nock.activate(); + } + }); + + after(function () { + nock.cleanAll(); + if (!nockWasActive) { + nock.restore(); + } + }); + + // test double for the signing params shape + const makeSigningParams = () => + ({ + coin: { getChain: () => 'btc' }, + txPrebuild: { walletId: 'walletid', txHex: 'tx' }, + pubs: [], + } as unknown as Parameters[0]); + it('attaches the signer auth header when a secret is configured', async function () { + const nockScope = nock(signerUrl) + .post('/api/v2/btc/sign') + .matchHeader('x-auth-signer', SIGNER_AUTH_SECRET) + .reply(200, { txHex: 'signed-tx' }); + + const signingFunction = createCustomSigningFunction(signerUrl, { signerAuthSecret: SIGNER_AUTH_SECRET }); + const result = await signingFunction(makeSigningParams()); + assert.deepStrictEqual(result, { txHex: 'signed-tx' }); + nockScope.done(); + }); + + it('omits the header when no secret is configured', async function () { + const nockScope = nock(signerUrl) + .post('/api/v2/btc/sign') + .matchHeader('x-auth-signer', (value: string | undefined) => value === undefined) + .reply(200, { txHex: 'signed-tx' }); + + const signingFunction = createCustomSigningFunction(signerUrl); + const result = await signingFunction(makeSigningParams()); + assert.deepStrictEqual(result, { txHex: 'signed-tx' }); + nockScope.done(); + }); + + it('applies TLS client material when configured', async function () { + const pemDir = fsSync.mkdtempSync(path.join(os.tmpdir(), 'signer-tls-')); + const keyPath = path.join(pemDir, 'client-key.pem'); + const certPath = path.join(pemDir, 'client-cert.pem'); + const caPath = path.join(pemDir, 'ca.pem'); + fsSync.writeFileSync(keyPath, 'not-a-real-key'); + fsSync.writeFileSync(certPath, 'not-a-real-cert'); + fsSync.writeFileSync(caPath, 'not-a-real-ca'); + + const nockScope = nock(signerUrl) + .post('/api/v2/btc/sign') + .matchHeader('x-auth-signer', SIGNER_AUTH_SECRET) + .reply(200, { txHex: 'signed-tx' }); + + // nock intercepts below the TLS layer, so the request still flows: this + // exercises reading the configured PEM files and attaching them to the + // outbound request without breaking it + const signingFunction = createCustomSigningFunction(signerUrl, { + signerAuthSecret: SIGNER_AUTH_SECRET, + signerClientKeyPath: keyPath, + signerClientCertPath: certPath, + signerClientCaPath: caPath, + }); + const result = await signingFunction(makeSigningParams()); + assert.deepStrictEqual(result, { txHex: 'signed-tx' }); + nockScope.done(); + + fsSync.rmSync(pemDir, { recursive: true, force: true }); + }); + }); +}); diff --git a/modules/express/test/unit/config.ts b/modules/express/test/unit/config.ts index 4916c5e30d3..5b252e455fa 100644 --- a/modules/express/test/unit/config.ts +++ b/modules/express/test/unit/config.ts @@ -126,6 +126,11 @@ describe('Config:', () => { BITGO_EXTERNAL_SIGNER_URL: 'envexternalSignerUrl', BITGO_SIGNER_MODE: 'envsignerMode', BITGO_SIGNER_FILE_SYSTEM_PATH: 'envsignerFileSystemPath', + BITGO_SIGNER_AUTH_SECRET: 'envsignerauthsecret', + BITGO_SSL_CA: 'envsslca', + BITGO_SIGNER_CLIENT_KEY: 'envsignerClientKeyPath', + BITGO_SIGNER_CLIENT_CERT: 'envsignerClientCertPath', + BITGO_SIGNER_CLIENT_CA: 'envsignerClientCaPath', BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH: 'envlightningSignerFileSystemPath', BITGO_KEEP_ALIVE_TIMETOUT: 'envkeepalivetimeout', BITGO_HEADERS_TIMETOUT: 'envheaderstimeout', @@ -155,6 +160,11 @@ describe('Config:', () => { enclavedExpressSSLCert: 'argenclavedExpressSSLCert', signerMode: 'argsignerMode', signerFileSystemPath: 'argsignerFileSystemPath', + signerauthsecret: 'argsignerauthsecret', + sslca: 'argsslca', + signerclientkey: 'argsignerClientKeyPath', + signerclientcert: 'argsignerClientCertPath', + signerclientca: 'argsignerClientCaPath', lightningSignerFileSystemPath: 'arglightningSignerFileSystemPath', keepalivetimeout: 'argkeepalivetimeout', headerstimeout: 'argheaderstimeout', @@ -184,6 +194,11 @@ describe('Config:', () => { externalSignerUrl: 'https://argexternalSignerUrl', signerMode: 'argsignerMode', signerFileSystemPath: 'argsignerFileSystemPath', + signerAuthSecret: 'argsignerauthsecret', + sslCa: 'argsslca', + signerClientKeyPath: 'argsignerClientKeyPath', + signerClientCertPath: 'argsignerClientCertPath', + signerClientCaPath: 'argsignerClientCaPath', lightningSignerFileSystemPath: 'arglightningSignerFileSystemPath', keepAliveTimeout: 'argkeepalivetimeout', headersTimeout: 'argheaderstimeout', diff --git a/modules/express/test/unit/typedRoutes/coinSign.ts b/modules/express/test/unit/typedRoutes/coinSign.ts index 762968d02f1..384123daaf0 100644 --- a/modules/express/test/unit/typedRoutes/coinSign.ts +++ b/modules/express/test/unit/typedRoutes/coinSign.ts @@ -19,7 +19,7 @@ import * as sinon from 'sinon'; import { BitGo } from 'bitgo'; import { promises as fs } from 'fs'; import * as fsSync from 'fs'; -import { setupAgent } from '../../lib/testutil'; +import { SIGNER_AUTH_SECRET, setupAgent } from '../../lib/testutil'; describe('CoinSign codec tests (External Signer Mode)', function () { describe('coinSign', function () { @@ -113,6 +113,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -177,6 +178,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -234,6 +236,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -280,6 +283,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -324,6 +328,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -395,6 +400,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -433,6 +439,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -452,6 +459,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -477,11 +485,12 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - runtime errors return 500 - assert.strictEqual(result.status, 500); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -501,11 +510,12 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - runtime errors return 500 - assert.strictEqual(result.status, 500); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -534,11 +544,12 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - runtime errors return 500 - assert.strictEqual(result.status, 500); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); // Cleanup @@ -568,11 +579,12 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - wrong passphrase returns 401 - assert.strictEqual(result.status, 401); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -603,6 +615,7 @@ describe('CoinSign codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/sign`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); diff --git a/modules/express/test/unit/typedRoutes/generateShareTSS.ts b/modules/express/test/unit/typedRoutes/generateShareTSS.ts index 5c45f507137..34aa3c97dd8 100644 --- a/modules/express/test/unit/typedRoutes/generateShareTSS.ts +++ b/modules/express/test/unit/typedRoutes/generateShareTSS.ts @@ -26,7 +26,7 @@ import { BitGo } from 'bitgo'; import { EddsaUtils, EcdsaUtils } from '@bitgo/sdk-core'; import { promises as fs } from 'fs'; import * as fsSync from 'fs'; -import { setupAgent } from '../../lib/testutil'; +import { SIGNER_AUTH_SECRET, setupAgent } from '../../lib/testutil'; describe('GenerateShareTSS codec tests (External Signer Mode)', function () { describe('generateShareTSS', function () { @@ -472,6 +472,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/commitment`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -550,6 +551,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/R`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -626,6 +628,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/G`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -679,6 +682,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${ecdsaCoin}/tssshare/PaillierModulus`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -755,6 +759,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${ecdsaCoin}/tssshare/K`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -785,6 +790,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/commitment`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -802,6 +808,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/commitment`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -827,11 +834,12 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/commitment`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - runtime errors return 500 - assert.strictEqual(result.status, 500); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -852,11 +860,12 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/R`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - runtime errors return 500 - assert.strictEqual(result.status, 500); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -886,11 +895,12 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/commitment`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - runtime errors return 500 - assert.strictEqual(result.status, 500); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); // Cleanup @@ -921,11 +931,12 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/G`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - // Verify error response - wrong passphrase returns 401 - assert.strictEqual(result.status, 401); + // Verify error response - wallet lookup failures return a uniform 400 + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -954,6 +965,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/commitment`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -987,6 +999,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${coin}/tssshare/K`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -1020,6 +1033,7 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const result = await agent .post(`/api/v2/${ecdsaCoin}/tssshare/invalidShareType`) .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); diff --git a/modules/express/test/unit/typedRoutes/ofcExtSignPayload.ts b/modules/express/test/unit/typedRoutes/ofcExtSignPayload.ts index 3164bd9c2fb..078c5c77e4c 100644 --- a/modules/express/test/unit/typedRoutes/ofcExtSignPayload.ts +++ b/modules/express/test/unit/typedRoutes/ofcExtSignPayload.ts @@ -9,7 +9,7 @@ import * as sinon from 'sinon'; import { BitGo } from 'bitgo'; import { promises as fs } from 'fs'; import * as fsSync from 'fs'; -import { setupAgent } from '../../lib/testutil'; +import { SIGNER_AUTH_SECRET, setupAgent } from '../../lib/testutil'; describe('OfcExtSignPayload External Signer Mode Tests', function () { describe('ofcExtSignPayload', function () { @@ -85,6 +85,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -129,6 +130,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -164,6 +166,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -189,10 +192,11 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - assert.strictEqual(result.status, 500); + assert.strictEqual(result.status, 400); result.body.should.have.property('error'); }); @@ -216,10 +220,11 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - assert.strictEqual(result.status, 500); + assert.strictEqual(result.status, 400); result.body.should.have.property('error'); delete process.env[`WALLET_${differentWalletId}_PASSPHRASE`]; @@ -236,10 +241,11 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - assert.strictEqual(result.status, 500); + assert.strictEqual(result.status, 400); assert.ok(result.body); }); @@ -261,10 +267,11 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); - assert.strictEqual(result.status, 401); + assert.strictEqual(result.status, 400); result.body.should.have.property('error'); }); @@ -291,6 +298,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -307,6 +315,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody); @@ -322,6 +331,7 @@ describe('OfcExtSignPayload External Signer Mode Tests', function () { const result = await agent .post('/api/v2/ofc/signPayload') .set('Authorization', 'Bearer test_access_token_12345') + .set('X-Auth-Signer', SIGNER_AUTH_SECRET) .set('Content-Type', 'application/json') .send(requestBody);