From 7b881e43160b51986e5f1ca49866e4de65aa7db2 Mon Sep 17 00:00:00 2001 From: Vibhav Simha G Date: Thu, 5 Nov 2026 14:55:26 +0530 Subject: [PATCH] fix(sdk-core): bind GG18 offline shares to signable, enforce single use An attacker reaching the signer's tssshare port could replay one encryptedOShare with different signables or D-shares: since sign() computes s = m*k + r*omicron with r derived from oShare.Gamma + dShare.Gamma, two signatures solve for k and omicron, and with an attacker-supplied Paillier modulus yield the user's key share. wShare/oShare envelopes now carry adata binding ::: , validated (binding and age, with v1 escape decoding) before decryption; counterparty Paillier moduli (challenge n, A-share n) must match the signer's own bitgoNShare.n; the express signer marks MuDelta/S ciphertexts consumed exactly once in a file-backed registry that survives restarts, keyed on the canonical decoded bytes that decryption consumes (v1 iv truncated to the effective CCM nonce), with consumption recorded only after the envelope fully authenticates so junk requests never reach the registry. Legacy relay bodies and legacy direct callers keep working; the oShare inherits the wShare's binding when no txRequest is available. References: WAL-2459 --- .../test/v2/unit/internal/tssUtils/ecdsa.ts | 499 +++++++++++++++++- modules/express/.env.example | 1 + modules/express/README.md | 3 + modules/express/src/args.ts | 4 + modules/express/src/clientRoutes.ts | 43 +- modules/express/src/config.ts | 5 + modules/express/src/consumedSharesRegistry.ts | 171 ++++++ .../test/unit/clientRoutes/externalSign.ts | 265 +++++++++- modules/express/test/unit/config.ts | 2 + .../sdk-core/src/bitgo/utils/tss/baseTypes.ts | 8 +- .../src/bitgo/utils/tss/ecdsa/ecdsa.ts | 218 +++++++- 11 files changed, 1181 insertions(+), 38 deletions(-) create mode 100644 modules/express/src/consumedSharesRegistry.ts diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index a5c5b36e8b2..5e0f7b7b764 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts @@ -46,7 +46,7 @@ import { nockGetTxRequest, } from '../../tss/helpers'; import { bip32, ecc } from '@bitgo/utxo-lib'; -import { Hash } from 'crypto'; +import { Hash, createHash } from 'crypto'; import { mockChallengeA, mockChallengeB, mockChallengeC } from './mocks/ecdsaNtilde'; import { loadWebAssembly } from '@bitgo/sdk-opensslbytes'; @@ -558,6 +558,18 @@ describe('TSS Ecdsa Utils:', async function () { }); }); + // adata strings bound into the wShare/oShare envelopes, mirroring EcdsaUtils' offline signing binding + const WSHARE_STATE = 'GG18_SIGNING_WSHARE_STATE'; + const OSHARE_STATE = 'GG18_SIGNING_OSHARE_STATE'; + const offlineShareAdataCore = (txr: TxRequest) => + `${createHash('sha256').update(Buffer.from(txr.unsignedTxs[0].signableHex, 'hex')).digest('hex')}:${ + txr.unsignedTxs[0].derivationPath + }`; + const offlineShareAdata = (txr: TxRequest, state: string = WSHARE_STATE) => + `${state}:${Date.now()}:${offlineShareAdataCore(txr)}`; + const expiredOfflineShareAdata = (txr: TxRequest, state: string = WSHARE_STATE) => + `${state}:${Date.now() - 31 * 24 * 3600 * 1000}:${offlineShareAdataCore(txr)}`; + describe('signTxRequest:', () => { const txRequestId = 'randomidEcdsa'; const txRequest: TxRequest = { @@ -1104,6 +1116,8 @@ describe('TSS Ecdsa Utils:', async function () { it('createOfflineKShare should succeed', async function () { const mockPassword = 'password'; + // pre-condition: BitGo challenge modulus must match the signer's bitgoNShare + assert.strictEqual(bitgoChallenges.n, bitgoKeyShare.nShares[1].n); const step1SigningMaterial = await tssUtils.createOfflineKShare({ tssParams: { txRequest, @@ -1129,7 +1143,15 @@ describe('TSS Ecdsa Utils:', async function () { step1SigningMaterial.encryptedSignerOffsetShare.should.startWith('-----BEGIN PGP MESSAGE-----'); step1SigningMaterial.userPublicGpgKey.should.startWith('-----BEGIN PGP PUBLIC KEY BLOCK-----'); step1SigningMaterial.kShare.n.should.equal(userKeyShare.pShare.n); - step1SigningMaterial.wShare.should.startWith('{"iv":'); + const wShareEnvelope = step1SigningMaterial.wShare; + // wShare is an encrypted JSON envelope string when a walletPassphrase is provided + if (typeof wShareEnvelope !== 'string') { + throw new Error('wShare should be an encrypted envelope string'); + } + // adata format: `:::` + const adata = JSON.parse(wShareEnvelope).adata as string; + adata.should.startWith(`${WSHARE_STATE}:`); + adata.should.endWith(`:${offlineShareAdataCore(txRequest)}`); }); it('createOfflineKShare should fail with txId passed', async function () { @@ -1162,9 +1184,23 @@ describe('TSS Ecdsa Utils:', async function () { const deltaLength = 64; const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); const step2SigningMaterial = await tssUtils.createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), aShareFromBitgo: aShare, bitgoChallenge: bitgoChallenges, - encryptedWShare: await bitgo.encrypt({ input: JSON.stringify(wShare), password: mockPassword }), + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), walletPassphrase: mockPassword, }); step2SigningMaterial.muDShare.muShare.alpha.length.should.equal(alphaLength); @@ -1177,14 +1213,426 @@ describe('TSS Ecdsa Utils:', async function () { const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); await tssUtils .createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), aShareFromBitgo: aShare, bitgoChallenge: bitgoChallenges, - encryptedWShare: await bitgo.encrypt({ input: JSON.stringify(wShare), password: mockPassword }), + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), walletPassphrase: 'password1', }) .should.be.rejectedWith('incorrect password'); }); + it('createOfflineMuDeltaShare should fail with wShare adata bound to a different signable', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + const otherTxRequest = { + ...txRequest, + unsignedTxs: [{ ...txRequest.unsignedTxs[0], signableHex: 'deadbeef' }], + }; + await tssUtils + .createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(otherTxRequest), + }), + walletPassphrase: mockPassword, + }) + .should.be.rejectedWith('Adata does not match cyphertext adata'); + }); + + it('createOfflineKShare should fail when BitGo challenge modulus does not match the signer key material', async function () { + const mockPassword = 'password'; + await tssUtils + .createOfflineKShare({ + tssParams: { + txRequest, + reqId, + }, + challenges: { + enterpriseChallenge: enterpriseChallenges, + bitgoChallenge: { ...bitgoChallenges, n: '1234' }, + }, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + requestType: RequestType.tx, + walletPassphrase: mockPassword, + }) + .should.be.rejectedWith("BitGo challenge Paillier modulus does not match the signer's BitGo key material"); + }); + + it('createOfflineMuDeltaShare should fail when BitGo challenge modulus does not match the signer key material', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + await tssUtils + .createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: { ...bitgoChallenges, n: '1234' }, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + }) + .should.be.rejectedWith("BitGo challenge Paillier modulus does not match the signer's BitGo key material"); + }); + + it('createOfflineMuDeltaShare should fail when the A-share modulus does not match the signer key material', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + await tssUtils + .createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: { ...aShare, n: '1234' }, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + }) + .should.be.rejectedWith('A-share from BitGo has an unexpected Paillier modulus'); + }); + + it('createOfflineMuDeltaShare should succeed with a legacy relay body: top-level txRequest and route-injected prv', async function () { + const mockPassword = 'password'; + const alphaLength = 1536; + const deltaLength = 64; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + const step2SigningMaterial = await tssUtils.createOfflineMuDeltaShare({ + txRequest, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + }); + step2SigningMaterial.muDShare.muShare.alpha.length.should.equal(alphaLength); + step2SigningMaterial.muDShare.dShare.delta.length.should.equal(deltaLength); + }); + + it('createOfflineMuDeltaShare should keep legacy behavior for direct callers without txRequest/prv', async function () { + const mockPassword = 'password'; + const alphaLength = 1536; + const deltaLength = 64; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + // Legacy direct callers omit txRequest, so MuDelta propagates the wShare envelope's + // own binding (re-prefixed for the oShare) instead of recomputing it. + const step2SigningMaterial = await tssUtils.createOfflineMuDeltaShare({ + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + }); + step2SigningMaterial.muDShare.muShare.alpha.length.should.equal(alphaLength); + step2SigningMaterial.muDShare.dShare.delta.length.should.equal(deltaLength); + // The propagated binding is re-prefixed for the oShare with a fresh creation timestamp. + const oShareAdata = JSON.parse(step2SigningMaterial.oShare as string).adata as string; + oShareAdata.should.startWith(`${OSHARE_STATE}:`); + oShareAdata.should.endWith(`:${offlineShareAdataCore(txRequest)}`); + }); + + it('createOfflineMuDeltaShare should reject a wShare without a binding', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + await tssUtils + .createOfflineMuDeltaShare({ + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ input: JSON.stringify(wShare), password: mockPassword }), + walletPassphrase: mockPassword, + }) + .should.be.rejectedWith('wShare envelope is missing a binding; restart the signing session'); + }); + + it('createOfflineMuDeltaShare marks the wShare consumed only after authentication', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + let marks = 0; + const markConsumed = () => { + marks++; + }; + await tssUtils.createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + markConsumed, + }); + marks.should.equal(1); + }); + + it('createOfflineMuDeltaShare does not mark a wShare that fails adata validation', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + let marks = 0; + const markConsumed = () => { + marks++; + }; + const otherTxRequest = { + ...txRequest, + unsignedTxs: [{ ...txRequest.unsignedTxs[0], signableHex: 'deadbeef' }], + }; + await tssUtils + .createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(otherTxRequest), + }), + walletPassphrase: mockPassword, + markConsumed, + }) + .should.be.rejectedWith('Adata does not match cyphertext adata'); + marks.should.equal(0); + }); + + it('createOfflineSShare should reject an oShare without adata with a distinct error', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + let marks = 0; + const markConsumed = () => { + marks++; + }; + await tssUtils + .createOfflineSShare({ + tssParams: { + txRequest, + reqId, + }, + dShareFromBitgo: dShare, + // an oShare envelope with no adata at all must not pass validation + encryptedOShare: await bitgo.encrypt({ input: JSON.stringify(oShare), password: mockPassword }), + walletPassphrase: mockPassword, + requestType: RequestType.tx, + markConsumed, + }) + .should.be.rejectedWith('Missing adata on encrypted share envelope; restart the signing session'); + marks.should.equal(0); + }); + + it('createOfflineSShare marks the oShare consumed only after authentication', async function () { + const mockPassword = 'password'; + const pubKeyLength = 66; + const privKeyLength = 64; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + let marks = 0; + const markConsumed = () => { + marks++; + }; + const step3SigningMaterial = await tssUtils.createOfflineSShare({ + tssParams: { + txRequest, + reqId, + }, + dShareFromBitgo: dShare, + encryptedOShare: await bitgo.encrypt({ + input: JSON.stringify(oShare), + password: mockPassword, + adata: offlineShareAdata(txRequest, OSHARE_STATE), + }), + walletPassphrase: mockPassword, + requestType: RequestType.tx, + markConsumed, + }); + marks.should.equal(1); + step3SigningMaterial.R.length.should.equal(pubKeyLength); + step3SigningMaterial.y.length.should.equal(pubKeyLength); + step3SigningMaterial.s.length.should.equal(privKeyLength); + }); + + it('createOfflineSShare should complete a v1 (SJCL) envelope flow with encryptionVersion 1', async function () { + const mockPassword = 'password'; + const pubKeyLength = 66; + const privKeyLength = 64; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + // v1 (SJCL) envelopes escape() the adata (':' -> '%3A'); the whole K -> MuDelta -> S + // chain must still bind and validate correctly. + const encryptedWShare = await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + encryptionVersion: 1, + }); + const muDeltaResult = await tssUtils.createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare, + walletPassphrase: mockPassword, + encryptionVersion: 1, + }); + const step3SigningMaterial = await tssUtils.createOfflineSShare({ + tssParams: { + txRequest, + reqId, + }, + dShareFromBitgo: dShare, + encryptedOShare: muDeltaResult.oShare as string, + walletPassphrase: mockPassword, + requestType: RequestType.tx, + }); + step3SigningMaterial.R.length.should.equal(pubKeyLength); + step3SigningMaterial.y.length.should.equal(pubKeyLength); + step3SigningMaterial.s.length.should.equal(privKeyLength); + }); + + it('createOfflineSShare should complete a legacy direct-caller flow (propagated oShare binding)', async function () { + const mockPassword = 'password'; + const pubKeyLength = 66; + const privKeyLength = 64; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + // Legacy direct callers omit txRequest, so MuDelta propagates the wShare envelope's + // own binding (re-prefixed for the oShare) instead of recomputing it. + const muDeltaResult = await tssUtils.createOfflineMuDeltaShare({ + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: offlineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + }); + // The S round always has tssParams, so it validates the propagated binding strictly. + const step3SigningMaterial = await tssUtils.createOfflineSShare({ + tssParams: { + txRequest, + reqId, + }, + dShareFromBitgo: dShare, + encryptedOShare: muDeltaResult.oShare as string, + walletPassphrase: mockPassword, + requestType: RequestType.tx, + }); + step3SigningMaterial.R.length.should.equal(pubKeyLength); + step3SigningMaterial.y.length.should.equal(pubKeyLength); + step3SigningMaterial.s.length.should.equal(privKeyLength); + }); + + it('createOfflineMuDeltaShare should reject an expired wShare envelope', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + await tssUtils + .createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + aShareFromBitgo: aShare, + bitgoChallenge: bitgoChallenges, + encryptedWShare: await bitgo.encrypt({ + input: JSON.stringify(wShare), + password: mockPassword, + adata: expiredOfflineShareAdata(txRequest), + }), + walletPassphrase: mockPassword, + }) + .should.be.rejectedWith('Encrypted share envelope has expired; restart the signing session'); + }); + it('createOfflineMuDeltaShare should succeed with v2-encrypted wShare', async function () { const mockPassword = 'password'; const alphaLength = 1536; @@ -1194,9 +1642,20 @@ describe('TSS Ecdsa Utils:', async function () { input: JSON.stringify(wShare), password: mockPassword, encryptionVersion: 2, + adata: offlineShareAdata(txRequest), }); assert.strictEqual(JSON.parse(encryptedWShare).v, 2, 'pre-condition: wShare must be v2-encrypted'); const step2SigningMaterial = await tssUtils.createOfflineMuDeltaShare({ + tssParams: { + txRequest, + reqId, + }, + requestType: RequestType.tx, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), aShareFromBitgo: aShare, bitgoChallenge: bitgoChallenges, encryptedWShare, @@ -1217,7 +1676,11 @@ describe('TSS Ecdsa Utils:', async function () { reqId: reqId, }, dShareFromBitgo: dShare, - encryptedOShare: await bitgo.encrypt({ input: JSON.stringify(oShare), password: mockPassword }), + encryptedOShare: await bitgo.encrypt({ + input: JSON.stringify(oShare), + password: mockPassword, + adata: offlineShareAdata(txRequest, OSHARE_STATE), + }), walletPassphrase: mockPassword, requestType: RequestType.tx, }); @@ -1226,6 +1689,31 @@ describe('TSS Ecdsa Utils:', async function () { step3SigningMaterial.s.length.should.equal(privKeyLength); }); + it('createOfflineSShare should fail with oShare adata bound to a different signable', async function () { + const mockPassword = 'password'; + const bitgo = TestBitGo.decorate(BitGo, { env: 'mock' }); + const otherTxRequest = { + ...txRequest, + unsignedTxs: [{ ...txRequest.unsignedTxs[0], signableHex: 'deadbeef' }], + }; + await tssUtils + .createOfflineSShare({ + tssParams: { + txRequest, + reqId, + }, + dShareFromBitgo: dShare, + encryptedOShare: await bitgo.encrypt({ + input: JSON.stringify(oShare), + password: mockPassword, + adata: offlineShareAdata(otherTxRequest, OSHARE_STATE), + }), + walletPassphrase: mockPassword, + requestType: RequestType.tx, + }) + .should.be.rejectedWith('Adata does not match cyphertext adata'); + }); + it('createOfflineSShare should succeed with v2-encrypted oShare', async function () { const mockPassword = 'password'; const pubKeyLength = 66; @@ -1235,6 +1723,7 @@ describe('TSS Ecdsa Utils:', async function () { input: JSON.stringify(oShare), password: mockPassword, encryptionVersion: 2, + adata: offlineShareAdata(txRequest, OSHARE_STATE), }); assert.strictEqual(JSON.parse(encryptedOShare).v, 2, 'pre-condition: oShare must be v2-encrypted'); const step3SigningMaterial = await tssUtils.createOfflineSShare({ diff --git a/modules/express/.env.example b/modules/express/.env.example index c4a05b7e52e..f29a04654f8 100644 --- a/modules/express/.env.example +++ b/modules/express/.env.example @@ -16,6 +16,7 @@ BITGO_AUTH_VERSION=2 BITGO_EXTERNAL_SIGNER_URL= BITGO_SIGNER_MODE= BITGO_SIGNER_FILE_SYSTEM_PATH= +BITGO_SIGNER_REGISTRY_FILE_PATH= BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH= BITGO_KEEP_ALIVE_TIMEOUT= BITGO_HEADERS_TIMEOUT= diff --git a/modules/express/README.md b/modules/express/README.md index 612b3577a17..357a49bba66 100644 --- a/modules/express/README.md +++ b/modules/express/README.md @@ -225,6 +225,8 @@ This may be preferable for users who would like to apply their signature to thei For more information please see our [External Signing Mode Documentation](EXTERNAL_SIGNER.md). +When running express in `--signerMode`, one signer process must own each `signerFileSystemPath`: the consumed-share registry file (`.consumedShares.json`) is read once per process and rewritten by the process that owns it, so sharing one registry file across replicas can lose replay-protection records. A relay retry after a lost signer response restarts the signing session from the K round (fail-closed). The registry location defaults to `.consumedShares.json`; the signer never assumes write access there — if it is not writable, a warning is logged and replay protection degrades to in-memory only (lost on restart). Point `--signerRegistryFilePath` (or `BITGO_SIGNER_REGISTRY_FILE_PATH`) at a writable location for full restart protection. + ### KeepAliveTimeout/HeadersTimeout If the deployment destination is configured with a load balancer such as ELB, a 504 Gateway Timeout may occur. In such cases, it might be resolved by configuring the settings so that LB keepAliveTimeout < keepAliveTimeout < headersTimeout. This can be achieved by specifying the keepAliveTimeout and headersTimeout options. @@ -258,6 +260,7 @@ 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 | --signerRegistryFilePath | `BITGO_SIGNER_REGISTRY_FILE_PATH` | N/A | Local path specifying where an Express signer machine keeps the encrypted user private keys. Required when signerMode is set. | | 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..b97e3f84e65 100644 --- a/modules/express/src/args.ts +++ b/modules/express/src/args.ts @@ -112,6 +112,10 @@ parser.addArgument(['--signerFileSystemPath'], { help: 'Local path specifying where an Express signer machine keeps encrypted user private keys.', }); +parser.addArgument(['--signerRegistryFilePath'], { + help: 'Optional path for the consumed-share registry file (defaults to .consumedShares.json). Useful when the key-file directory is read-only.', +}); + 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..339abb70f09 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -56,6 +56,7 @@ import { Config } from './config'; import { ApiResponseError, BitGoExpressError } from './errors'; import { promises as fs } from 'fs'; import { retryPromise } from './retryPromise'; +import { computeConsumedShareKey, getConsumedShareRegistry } from './consumedSharesRegistry'; import { SENSITIVE_REQUEST_KEYS } from './sensitiveRequestKeys'; import { handleCreateSignerMacaroon, @@ -439,6 +440,31 @@ async function getEncryptedPrivKey(path: string, walletId: string): Promise void { + if (typeof encryptedShare !== 'string' || !encryptedShare) { + throw new ApiResponseError('Missing encrypted share in request', 400); + } + // Cheap pre-checks before the helper runs: an unparseable body or one without string + // iv/ct would fail decryption anyway, and rejecting it early with a 400 keeps junk + // requests from ever reaching the registry. + let envelope: Record; + try { + envelope = JSON.parse(encryptedShare) as Record; + } catch { + throw new ApiResponseError('Invalid encrypted share envelope', 400); + } + if (typeof envelope.iv !== 'string' || typeof envelope.ct !== 'string') { + throw new ApiResponseError('Invalid encrypted share envelope', 400); + } + return () => + getConsumedShareRegistry(registryFilePath).markConsumed(computeConsumedShareKey(shareType, encryptedShare)); +} + async function decryptPrivKey(bg: BitGo, encryptedPrivKey: string, walletPw: string): Promise { try { return await bg.decrypt({ password: walletPw, input: encryptedPrivKey }); @@ -467,6 +493,7 @@ export async function handleV2GenerateShareTSS( if (!signerFileSystemPath) { throw new Error('Missing required configuration: signerFileSystemPath'); } + const registryFilePath = req.config.signerRegistryFilePath ?? `${signerFileSystemPath}.consumedShares.json`; const encryptedPrivKey = await getEncryptedPrivKey(signerFileSystemPath, walletId); const bitgo = req.bitgo; @@ -540,10 +567,22 @@ export async function handleV2GenerateShareTSS( return ecdsaUtils.getOfflineSignerPaillierModulus(req.body); case ShareType.K: return await ecdsaUtils.createOfflineKShare(req.body); - case ShareType.MuDelta: + case ShareType.MuDelta: { + req.body.markConsumed = markConsumedShareClosure( + registryFilePath, + req.decoded.sharetype, + req.body.encryptedWShare + ); return await ecdsaUtils.createOfflineMuDeltaShare(req.body); - case ShareType.S: + } + case ShareType.S: { + req.body.markConsumed = markConsumedShareClosure( + registryFilePath, + req.decoded.sharetype, + req.body.encryptedOShare + ); return await ecdsaUtils.createOfflineSShare(req.body); + } default: throw new Error( `Share type ${req.decoded.sharetype} not supported, only PaillierModulus, K, MUDelta, and S share generation is supported.` diff --git a/modules/express/src/config.ts b/modules/express/src/config.ts index ba1e65bd8b8..cc3d01efafb 100644 --- a/modules/express/src/config.ts +++ b/modules/express/src/config.ts @@ -49,6 +49,8 @@ export interface Config { externalSignerUrl?: string; signerMode?: boolean; signerFileSystemPath?: string; + /** Overrides the consumed-share registry location (defaults to `.consumedShares.json`). */ + signerRegistryFilePath?: string; lightningSignerFileSystemPath?: string; keepAliveTimeout?: number; headersTimeout?: number; @@ -76,6 +78,7 @@ export const ArgConfig = (args): Partial => ({ externalSignerUrl: args.externalSignerUrl, signerMode: args.signerMode, signerFileSystemPath: args.signerFileSystemPath, + signerRegistryFilePath: args.signerRegistryFilePath, lightningSignerFileSystemPath: args.lightningSignerFileSystemPath, keepAliveTimeout: args.keepalivetimeout, headersTimeout: args.headerstimeout, @@ -103,6 +106,7 @@ export const EnvConfig = (): Partial => ({ externalSignerUrl: readEnvVar('BITGO_EXTERNAL_SIGNER_URL'), signerMode: readEnvVar('BITGO_SIGNER_MODE') ? true : undefined, signerFileSystemPath: readEnvVar('BITGO_SIGNER_FILE_SYSTEM_PATH'), + signerRegistryFilePath: readEnvVar('BITGO_SIGNER_REGISTRY_FILE_PATH'), lightningSignerFileSystemPath: readEnvVar('BITGO_LIGHTNING_SIGNER_FILE_SYSTEM_PATH'), keepAliveTimeout: Number(readEnvVar('BITGO_KEEP_ALIVE_TIMEOUT')), headersTimeout: Number(readEnvVar('BITGO_HEADERS_TIMEOUT')), @@ -191,6 +195,7 @@ function mergeConfigs(...configs: Partial[]): Config { externalSignerUrl, signerMode: get('signerMode'), signerFileSystemPath: get('signerFileSystemPath'), + signerRegistryFilePath: get('signerRegistryFilePath'), lightningSignerFileSystemPath: get('lightningSignerFileSystemPath'), keepAliveTimeout: get('keepAliveTimeout'), headersTimeout: get('headersTimeout'), diff --git a/modules/express/src/consumedSharesRegistry.ts b/modules/express/src/consumedSharesRegistry.ts new file mode 100644 index 00000000000..fcbf3cd1655 --- /dev/null +++ b/modules/express/src/consumedSharesRegistry.ts @@ -0,0 +1,171 @@ +import { createHash } from 'crypto'; +import * as fs from 'fs'; + +import { logger } from '@bitgo/logger'; +import { BitGoJsError } from '@bitgo/sdk-core'; + +import assert from 'assert'; + +const CONSUMED_SHARES_MAX_AGE_MS = 30 * 24 * 3600 * 1000; + +/** + * Mirrors @bitgo/sdk-api decryptV1.ts `ccmNonceLength`: v1 (SJCL/CCM) decryption truncates + * the iv to the nonce length derived from the plaintext size, so trailing iv bytes do not + * affect decryption. Keep in sync with decryptV1.ts. + */ +function v1CcmNonceLength(plaintextLen: number): number { + let L = 2; + while (L < 4 && plaintextLen >= 2 ** (8 * L)) L++; + return 15 - L; +} + +/** + * Key excludes attacker-controlled fields (e.g. walletId) so a relabeled replay is still + * rejected. The key covers the canonical decoded bytes that decryption actually consumes: + * for v1 (CCM) the iv is truncated to the effective nonce length (trailing iv bytes are + * ignored by decryption, so any iv longer than the nonce fails closed), and for v2 (GCM) the + * full iv is authenticated. Re-serializing the envelope (key order, whitespace, base64 + * padding) still collides with the original key and is rejected. + */ +export function computeConsumedShareKey(shareType: string, encryptedShare: string): string { + let envelope: Record; + try { + envelope = JSON.parse(encryptedShare) as Record; + } catch { + return createHash('sha256').update(`${shareType}:${encryptedShare}`).digest('hex'); + } + assert( + typeof envelope.iv === 'string' && typeof envelope.ct === 'string', + 'encrypted share envelope is missing iv/ct' + ); + const iv = Buffer.from(envelope.iv, 'base64'); + const ct = Buffer.from(envelope.ct, 'base64'); + // v2 (GCM) authenticates the full iv. v1 (CCM) truncates the iv to the nonce length + // (real SJCL envelopes carry a 16-byte iv for a 13-byte nonce), so hash only the + // effective nonce bytes — trailing iv bytes can neither change the key nor affect + // decryption. + let effectiveIv = iv; + if (envelope.v !== 2) { + const tagBytes = typeof envelope.ts === 'number' ? envelope.ts / 8 : 0; + const nonceLen = v1CcmNonceLength(ct.length - tagBytes); + if (iv.length < nonceLen) { + // decryption would reject this envelope anyway; hash the raw string for uniqueness + return createHash('sha256').update(`${shareType}:${encryptedShare}`).digest('hex'); + } + effectiveIv = iv.subarray(0, nonceLen); + } + const hash = createHash('sha256'); + hash.update(`${shareType}:`); + hash.update(effectiveIv); + hash.update(ct); + return hash.digest('hex'); +} + +/** + * One-time consumption registry: marks wShare/oShare ciphertexts as used before the share runs; + * a second attempt with the same ciphertext throws (fail-closed against replay). + */ +export class ConsumedShareRegistry { + private readonly consumed = new Map(); + private loaded = false; + + constructor(private readonly filePath: string) {} + + private load(): void { + if (this.loaded) { + return; + } + let raw: string; + try { + raw = fs.readFileSync(this.filePath, 'utf8'); + } catch (e) { + if (typeof e === 'object' && e !== null && 'code' in e && e.code === 'ENOENT') { + // No registry file yet (first run): nothing to load. + this.loaded = true; + return; + } + // An unreadable registry must not silently forget consumed shares: fail closed and + // retry the load on the next call. + const detail = e instanceof Error ? e.message : String(e); + throw new BitGoJsError(`failed to read consumed share registry from ${this.filePath}: ${detail}`); + } + try { + const parsed = JSON.parse(raw) as Record; + for (const [key, consumedAt] of Object.entries(parsed)) { + if (typeof consumedAt === 'number') { + this.consumed.set(key, consumedAt); + } + } + this.loaded = true; + } catch { + // A corrupt registry must not silently forget consumed shares: fail closed and retry + // the load on the next call. + throw new BitGoJsError(`failed to parse consumed share registry from ${this.filePath}`); + } + } + + markConsumed(key: string): void { + // Check-then-set is race-free only because the whole path is synchronous; do not add an + // await between the check and the set. + this.load(); + if (this.consumed.has(key)) { + throw new BitGoJsError('encrypted share has already been consumed; restart the signing session'); + } + this.consumed.set(key, Date.now()); + this.persist(); + } + + /** Drops expired entries so the in-memory map and the persisted file stay bounded. */ + private prune(): void { + const now = Date.now(); + for (const [key, consumedAt] of this.consumed) { + if (now - consumedAt >= CONSUMED_SHARES_MAX_AGE_MS) { + this.consumed.delete(key); + } + } + } + + private persist(): void { + let tempPath = ''; + // Atomic write: a crash mid-write cannot truncate the live registry (a truncated file + // would fail closed on load and lock out all signing). + try { + this.prune(); + tempPath = `${this.filePath}.${process.pid}.${Math.random().toString(36).slice(2)}.tmp`; + fs.writeFileSync(tempPath, JSON.stringify(Object.fromEntries(this.consumed))); + fs.renameSync(tempPath, this.filePath); + } catch (e) { + try { + fs.unlinkSync(tempPath); + } catch { + // nothing to clean up + } + // The signer process never assumes file write access (only the separate + // fetchEncryptedPrivKeys provisioning step writes there). If the registry location + // is not writable, degrade to in-memory-only protection: the share stays consumed + // for this process, but persistence (and restart protection) is lost until the + // operator relocates the registry via --signerRegistryFilePath. + const detail = e instanceof Error ? e.message : String(e); + logger.warn( + `failed to persist consumed share registry (${detail}); ` + + `replay protection is in-memory only until ${this.filePath} is writable ` + + '(relocate via --signerRegistryFilePath if needed)' + ); + } + } +} + +const registries = new Map(); + +export function getConsumedShareRegistry(registryFilePath: string): ConsumedShareRegistry { + let registry = registries.get(registryFilePath); + if (!registry) { + registry = new ConsumedShareRegistry(registryFilePath); + registries.set(registryFilePath, registry); + } + return registry; +} + +export function resetConsumedShareRegistriesForTesting(): void { + registries.clear(); +} diff --git a/modules/express/test/unit/clientRoutes/externalSign.ts b/modules/express/test/unit/clientRoutes/externalSign.ts index bdebf72471c..a960ff754b3 100644 --- a/modules/express/test/unit/clientRoutes/externalSign.ts +++ b/modules/express/test/unit/clientRoutes/externalSign.ts @@ -50,6 +50,8 @@ import { import * as assert from 'assert'; import nock from 'nock'; import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; import 'should-http'; import 'should-sinon'; @@ -64,6 +66,11 @@ import { handleV2SignTSSWalletTx, } from '../../../src/clientRoutes'; import { fetchKeys } from '../../../src/fetchEncryptedPrivKeys'; +import { + ConsumedShareRegistry, + computeConsumedShareKey, + resetConsumedShareRegistriesForTesting, +} from '../../../src/consumedSharesRegistry'; import { mockChallengeA, mockChallengeB } from './mocks/ecdsaNtilde'; import { ExpressApiRouteRequest } from '../../../src/typedRoutes/api'; import { Coin, BitGo, SignedTransaction } from 'bitgo'; @@ -169,9 +176,9 @@ describe('External signer', () => { envStub.restore(); }); - it('should read an encrypted prv from signerFileSystemPath and pass it to PaillierModulus, K, MuDelta, and S share generators', async function () { - // This test performs multiple MPC ECDSA rounds and can exceed Mocha's default 60s timeout on CI - this.timeout(180000); + // Runs a full PaillierModulus -> K -> MuDelta -> S external-signing session through + // handleV2GenerateShareTSS and returns the requests/results so replay tests can re-send them. + async function runEcdsaSigningSession(opts: { signerRegistryFilePath?: string } = {}) { const walletID = '62fe536a6b4cf70007acb48c0e7bb0b0'; const user = keyShareOneEcdsa; // await mpcEcdsa.keyShare(1, 2, 3); const backup = keyShareTwoEcdsa; // await mpcEcdsa.keyShare(2, 2, 3); @@ -190,9 +197,15 @@ describe('External signer', () => { const output: Output = {}; output[walletID] = validPrv; const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(JSON.stringify(output)); + // Unique temp path per session so the real consumed-shares persistence writes to a throwaway file + const signerTmpPath = path.join( + os.tmpdir(), + `bitgojs-test-signer-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}` + ); const envStub = sinon .stub(process, 'env') .value({ WALLET_62fe536a6b4cf70007acb48c0e7bb0b0_PASSPHRASE: walletPassphrase }); + resetConsumedShareRegistriesForTesting(); const tMessage = 'testMessage'; const bgTest = new BitGo({ env: 'test' }); const derivationPath = ''; @@ -233,7 +246,8 @@ describe('External signer', () => { sharetype: 'PaillierModulus', }, config: { - signerFileSystemPath: 'signerFileSystemPath', + signerFileSystemPath: signerTmpPath, + signerRegistryFilePath: opts.signerRegistryFilePath, }, } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; const paillierResult = await handleV2GenerateShareTSS(reqPaillierModulus); @@ -316,7 +330,8 @@ describe('External signer', () => { sharetype: 'K', }, config: { - signerFileSystemPath: 'signerFileSystemPath', + signerFileSystemPath: signerTmpPath, + signerRegistryFilePath: opts.signerRegistryFilePath, }, } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; const kResult = await handleV2GenerateShareTSS(reqK); @@ -334,18 +349,21 @@ describe('External signer', () => { const reqMuDelta = { bitgo: bgTest, body: { - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, + tssParams: { + txRequest: { + apiVersion: 'full', + walletId: walletID, + transactions: [ + { + unsignedTx: { + derivationPath, + signableHex: tMessage, + }, }, - }, - ], + ], + }, }, + requestType: 0, aShareFromBitgo: aShareFromBitgo.aShare, bitgoChallenge: { ntilde: bitgoChallenge.ntilde, @@ -359,18 +377,21 @@ describe('External signer', () => { decoded: { coin: 'tbsc', sharetype: 'MuDelta', - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, + tssParams: { + txRequest: { + apiVersion: 'full', + walletId: walletID, + transactions: [ + { + unsignedTx: { + derivationPath, + signableHex: tMessage, + }, }, - }, - ], + ], + }, }, + requestType: 0, aShareFromBitgo: aShareFromBitgo.aShare, bitgoChallenge: { ntilde: bitgoChallenge.ntilde, @@ -386,7 +407,8 @@ describe('External signer', () => { sharetype: 'MuDelta', }, config: { - signerFileSystemPath: 'signerFileSystemPath', + signerFileSystemPath: signerTmpPath, + signerRegistryFilePath: opts.signerRegistryFilePath, }, } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; const muDeltaResult = await handleV2GenerateShareTSS(reqMuDelta); @@ -450,7 +472,8 @@ describe('External signer', () => { sharetype: 'S', }, config: { - signerFileSystemPath: 'signerFileSystemPath', + signerFileSystemPath: signerTmpPath, + signerRegistryFilePath: opts.signerRegistryFilePath, }, } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; const sResult = await handleV2GenerateShareTSS(reqS); @@ -465,8 +488,192 @@ describe('External signer', () => { ); const signature = mpcEcdsa.constructSignature([bitGoSShare, sResult]); mpcEcdsa.verify(Buffer.from(tMessage, 'hex'), signature, createKeccakHash('keccak256') as Hash).should.be.true; - readFileStub.restore(); - envStub.restore(); + fs.promises.unlink(`${signerTmpPath}.consumedShares.json`).catch(() => undefined); + const cleanup = () => { + readFileStub.restore(); + envStub.restore(); + }; + return { reqMuDelta, reqS, kResult, muDeltaResult, sResult, cleanup }; + } + + it('should read an encrypted prv from signerFileSystemPath and pass it to PaillierModulus, K, MuDelta, and S share generators', async function () { + // This test performs multiple MPC ECDSA rounds and can exceed Mocha's default 60s timeout on CI + this.timeout(180000); + const { sResult, cleanup } = await runEcdsaSigningSession(); + try { + sResult.should.have.property('R'); + sResult.should.have.property('s'); + sResult.should.have.property('y'); + } finally { + cleanup(); + } + }); + + it('should reject a replayed MuDelta wShare ciphertext', async function () { + this.timeout(180000); + const { reqMuDelta, cleanup } = await runEcdsaSigningSession(); + try { + await handleV2GenerateShareTSS(reqMuDelta).should.be.rejectedWith( + 'encrypted share has already been consumed; restart the signing session' + ); + } finally { + cleanup(); + } + }); + + it('should reject a replayed S oShare ciphertext', async function () { + this.timeout(180000); + const { reqS, cleanup } = await runEcdsaSigningSession(); + try { + await handleV2GenerateShareTSS(reqS).should.be.rejectedWith( + 'encrypted share has already been consumed; restart the signing session' + ); + } finally { + cleanup(); + } + }); + + it('should reject an S replay with a different dShareFromBitgo (nonce-reuse attempt)', async function () { + this.timeout(180000); + const { reqS, cleanup } = await runEcdsaSigningSession(); + try { + // same oShare ciphertext, different dShare: must be rejected by the registry before signing + const tamperDelta = (share: { delta: string } | undefined) => { + assert.ok(share && typeof share.delta === 'string', 'S request must carry dShareFromBitgo with a delta'); + share.delta = 'ff' + share.delta.slice(2); + }; + tamperDelta('dShareFromBitgo' in reqS.body ? reqS.body.dShareFromBitgo : undefined); + tamperDelta('dShareFromBitgo' in reqS.decoded ? reqS.decoded.dShareFromBitgo : undefined); + await handleV2GenerateShareTSS(reqS).should.be.rejectedWith( + 'encrypted share has already been consumed; restart the signing session' + ); + } finally { + cleanup(); + } + }); + + it('should complete a fresh signing session after a rejected replay', async function () { + this.timeout(180000); + const first = await runEcdsaSigningSession(); // first session consumes its shares + first.cleanup(); + const second = await runEcdsaSigningSession(); // fresh K -> new ciphertexts + try { + second.sResult.should.have.property('R'); + second.sResult.should.have.property('s'); + second.sResult.should.have.property('y'); + } finally { + second.cleanup(); + } + }); + + describe('ConsumedShareRegistry', () => { + let tempDir: string; + // v1-shaped envelope like real SJCL output: 16-byte iv, ts=64 (8-byte tag), 32-byte ct. + // CCM truncates the iv to a 13-byte nonce for this plaintext size. + const ivB64 = Buffer.alloc(16, 1).toString('base64'); + const ctB64 = Buffer.alloc(32, 2).toString('base64'); + const envelope = JSON.stringify({ ts: 64, iv: ivB64, ct: ctB64 }); + const key = computeConsumedShareKey('S', envelope); + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'bitgojs-registry-test-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it('should reject a replay after a restart (state survives in the file)', () => { + const registryFile = path.join(tempDir, 'registry.json'); + new ConsumedShareRegistry(registryFile).markConsumed(key); + (() => new ConsumedShareRegistry(registryFile).markConsumed(key)).should.throw( + 'encrypted share has already been consumed; restart the signing session' + ); + }); + + it('should reject a re-serialized (malleated) replay of a consumed envelope', () => { + const registryFile = path.join(tempDir, 'registry.json'); + const registry = new ConsumedShareRegistry(registryFile); + registry.markConsumed(key); + // key reorder + whitespace + extra field: decrypts identically, must still collide + const malleated = JSON.stringify({ extra: 'field', ct: ctB64, iv: ivB64 }, null, 2); + (() => registry.markConsumed(computeConsumedShareKey('S', malleated))).should.throw( + 'encrypted share has already been consumed; restart the signing session' + ); + }); + + it('should reject an appended-iv v1 envelope (trailing bytes do not change the key)', () => { + const registryFile = path.join(tempDir, 'registry.json'); + const registry = new ConsumedShareRegistry(registryFile); + registry.markConsumed(key); + // v1 decryption truncates the iv to the nonce length, so appending iv bytes changes + // nothing for decryption; the key must still collide with the original. + const appendedIv = JSON.stringify({ ts: 64, iv: Buffer.alloc(17, 1).toString('base64'), ct: ctB64 }); + (() => registry.markConsumed(computeConsumedShareKey('S', appendedIv))).should.throw( + 'encrypted share has already been consumed; restart the signing session' + ); + }); + + it('should fail closed when the registry file is corrupt', () => { + const registryFile = path.join(tempDir, 'registry.json'); + fs.writeFileSync(registryFile, 'not json'); + (() => new ConsumedShareRegistry(registryFile).markConsumed(key)).should.throw( + /failed to parse consumed share registry/ + ); + }); + + it('should fail closed when the registry file is unreadable', () => { + // a directory at the registry path: reads fail with EISDIR, not ENOENT + const registryPath = path.join(tempDir, 'registry-dir'); + fs.mkdirSync(registryPath); + (() => new ConsumedShareRegistry(registryPath).markConsumed(key)).should.throw( + /failed to read consumed share registry/ + ); + }); + + it('should degrade to in-memory protection when persisting fails', () => { + const registryFile = path.join(tempDir, 'registry.json'); + const registry = new ConsumedShareRegistry(registryFile); + registry.markConsumed(key); + // Replace the registry file with a directory: the atomic rename (temp file -> + // registry) fails with EISDIR regardless of process privileges — root ignores + // permission bits, so a chmod-based failure would not reproduce for root runs. + fs.rmSync(registryFile); + fs.mkdirSync(registryFile); + // The signer never assumes file write access: a persist failure must not fail the + // request — protection degrades to in-memory only (lost on restart). + const newKey = computeConsumedShareKey( + 'S', + JSON.stringify({ + ts: 64, + iv: Buffer.alloc(16, 3).toString('base64'), + ct: Buffer.alloc(32, 4).toString('base64'), + }) + ); + registry.markConsumed(newKey); + // the failed-persist entry is still recorded in memory: replaying it is rejected + (() => registry.markConsumed(newKey)).should.throw( + 'encrypted share has already been consumed; restart the signing session' + ); + // the first share is still considered consumed in memory + (() => registry.markConsumed(key)).should.throw( + 'encrypted share has already been consumed; restart the signing session' + ); + }); + }); + + it('should honor a signerRegistryFilePath override for the consumed-share registry', async function () { + this.timeout(180000); + const overridePath = path.join(os.tmpdir(), `bitgojs-registry-override-${process.pid}-${Date.now()}.json`); + const { cleanup } = await runEcdsaSigningSession({ signerRegistryFilePath: overridePath }); + try { + fs.existsSync(overridePath).should.be.true('registry must be persisted at the override path'); + const entries = JSON.parse(fs.readFileSync(overridePath, 'utf8')); + Object.keys(entries).length.should.be.greaterThan(0); + } finally { + cleanup(); + fs.promises.unlink(overridePath).catch(() => undefined); + } }); it('should read an encrypted prv from signerFileSystemPath and pass it to commitment, R and G share generators', async () => { diff --git a/modules/express/test/unit/config.ts b/modules/express/test/unit/config.ts index 4916c5e30d3..5304738735d 100644 --- a/modules/express/test/unit/config.ts +++ b/modules/express/test/unit/config.ts @@ -155,6 +155,7 @@ describe('Config:', () => { enclavedExpressSSLCert: 'argenclavedExpressSSLCert', signerMode: 'argsignerMode', signerFileSystemPath: 'argsignerFileSystemPath', + signerRegistryFilePath: 'argsignerRegistryFilePath', lightningSignerFileSystemPath: 'arglightningSignerFileSystemPath', keepalivetimeout: 'argkeepalivetimeout', headerstimeout: 'argheaderstimeout', @@ -184,6 +185,7 @@ describe('Config:', () => { externalSignerUrl: 'https://argexternalSignerUrl', signerMode: 'argsignerMode', signerFileSystemPath: 'argsignerFileSystemPath', + signerRegistryFilePath: 'argsignerRegistryFilePath', lightningSignerFileSystemPath: 'arglightningSignerFileSystemPath', keepAliveTimeout: 'argkeepalivetimeout', headersTimeout: 'argheaderstimeout', diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index 0473a7ee397..38595a1f172 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -166,7 +166,13 @@ export interface CustomKShareGeneratingFunction { export interface CustomMuDeltaShareGeneratingFunction { (params: { - txRequest: TxRequest; + /** Bound signable context; enables adata validation. */ + tssParams?: TSSParams | TSSParamsForMessage; + requestType?: RequestType; + /** Legacy relays pass txRequest at top level; still adata-validated. */ + txRequest?: TxRequest; + /** Injected by the express signer route; enables modulus validation. */ + prv?: string; aShareFromBitgo: Omit; bitgoChallenge: TxRequestChallengeResponse; encryptedWShare: string; diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts index fece8b88d0c..e07e38e0d4c 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -2,7 +2,7 @@ import assert from 'assert'; import { Buffer } from 'buffer'; import * as openpgp from 'openpgp'; import { Key, SerializedKeyPair } from 'openpgp'; -import { Hash } from 'crypto'; +import { Hash, createHash } from 'crypto'; import { EcdsaPaillierProof, EcdsaRangeProof, EcdsaTypes, hexToBigInt, minModulusBitLength } from '@bitgo/sdk-lib-mpc'; import { bip32 } from '@bitgo/utxo-lib'; @@ -33,6 +33,7 @@ import { TssSignTxRequestParamsWithPrv, TssTxRecipientSource, TxRequest, + UnsignedTransactionTss, } from '../baseTypes'; import { getTxRequest } from '../../../tss'; import { AShare, DShare, EncryptedNShare, SendShareType, SShare, WShare, OShare } from '../../../tss/ecdsa/types'; @@ -55,8 +56,14 @@ import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify'; const encryptNShare = ECDSAMethods.encryptNShare; +/** Encrypted signing-share envelopes older than this are rejected along with their registry entries. */ +const OFFLINE_SIGNING_SHARE_MAX_AGE_MS = 30 * 24 * 3600 * 1000; // 30 days + /** @inheritdoc */ export class EcdsaUtils extends BaseEcdsaUtils { + private static readonly GG18_SIGNING_WSHARE_STATE = 'GG18_SIGNING_WSHARE_STATE'; + private static readonly GG18_SIGNING_OSHARE_STATE = 'GG18_SIGNING_OSHARE_STATE'; + async finalizeBitgoHeldBackupKeyShare( keyId: string, commonKeychain: string, @@ -459,6 +466,7 @@ export class EcdsaUtils extends BaseEcdsaUtils { derivationPath: string; walletPassphrase?: string; encryptionVersion?: EncryptionVersion; + signableAdata?: string; }): Promise { const { challenges, derivationPath, prv } = params; const userSigningMaterial: ECDSAMethodTypes.SigningMaterial = JSON.parse(prv); @@ -526,6 +534,9 @@ export class EcdsaUtils extends BaseEcdsaUtils { input: JSON.stringify(userSignShare.wShare), password: params.walletPassphrase, encryptionVersion: params.encryptionVersion, + adata: params.signableAdata + ? this.formatOfflineSigningAdata(EcdsaUtils.GG18_SIGNING_WSHARE_STATE, params.signableAdata) + : undefined, }) : userSignShare.wShare, }; @@ -537,6 +548,7 @@ export class EcdsaUtils extends BaseEcdsaUtils { aShareFromBitgo: Omit; walletPassphrase?: string; encryptionVersion?: EncryptionVersion; + signableAdata?: string; }): Promise { // Append the BitGo challenge to the Ashare to be used in subsequent proofs const bitgoToUserAShareWithNtilde: AShare = { @@ -561,6 +573,9 @@ export class EcdsaUtils extends BaseEcdsaUtils { input: JSON.stringify(userOmicronAndDeltaShare.oShare), password: params.walletPassphrase, encryptionVersion: params.encryptionVersion, + adata: params.signableAdata + ? this.formatOfflineSigningAdata(EcdsaUtils.GG18_SIGNING_OSHARE_STATE, params.signableAdata) + : undefined, }) : userOmicronAndDeltaShare.oShare, }; @@ -572,6 +587,109 @@ export class EcdsaUtils extends BaseEcdsaUtils { return { userPaillierModulus: userSigningMaterial.pShare.n }; } + /** + * Computes the adata binding string for offline (external-signer) signing: the + * sha256 of the signable payload, plus the derivation path (empty for message + * signing). Encrypted share envelopes created from the signer's own key + * material carry this adata and later rounds validate it, so an envelope can + * only be replayed against the exact signable it was created for. + */ + private getOfflineSigningAdata( + txRequest: TxRequest, + requestType: RequestType, + params: { bufferToSign?: unknown } = {} + ): string { + let signablePayload: Buffer; + let derivationPath: string; + if (requestType === RequestType.tx) { + let unsignedTx: UnsignedTransactionTss | undefined; + if (txRequest.apiVersion === 'full') { + assert(txRequest.transactions && txRequest.transactions.length > 0, 'Missing transactions in txRequest'); + unsignedTx = txRequest.transactions[0].unsignedTx; + } else { + assert(txRequest.unsignedTxs && txRequest.unsignedTxs.length > 0, 'Missing unsignedTxs in txRequest'); + unsignedTx = txRequest.unsignedTxs[0]; + } + assert(unsignedTx, 'Missing unsignedTx in txRequest'); + assert(unsignedTx.signableHex, 'Missing signableHex in txRequest'); + signablePayload = Buffer.from(unsignedTx.signableHex, 'hex'); + derivationPath = unsignedTx.derivationPath ?? ''; + } else if (requestType === RequestType.message) { + // Declared as Buffer, but over HTTP it may arrive as the JSON round-trip form `{ type: 'Buffer', data: number[] }` + const bufferToSign: unknown = params.bufferToSign; + assert(bufferToSign, 'Missing bufferToSign for message signing'); + if (Buffer.isBuffer(bufferToSign)) { + signablePayload = bufferToSign; + } else if ( + typeof bufferToSign === 'object' && + bufferToSign !== null && + 'data' in bufferToSign && + Array.isArray(bufferToSign.data) + ) { + // JSON round-trip of a Buffer arrives as `{ type: 'Buffer', data: number[] }` + signablePayload = Buffer.from(bufferToSign.data); + } else { + throw new Error('Invalid bufferToSign type'); + } + derivationPath = ''; + } else { + throw new Error(`Invalid requestType: ${requestType}`); + } + return `${createHash('sha256').update(signablePayload).digest('hex')}:${derivationPath}`; + } + + /** + * Returns the Paillier modulus BitGo used at keygen time, as recorded in the + * signer's own signing material. Counterparty inputs (challenges, A-shares) + * must carry the same modulus. + */ + private getBitgoPaillierModulusFromSigningMaterial(prv: string): string { + const userSigningMaterial: ECDSAMethodTypes.SigningMaterial = JSON.parse(prv); + assert(userSigningMaterial.bitgoNShare?.n, 'Signer signing material is missing the BitGo Paillier modulus'); + return userSigningMaterial.bitgoNShare.n; + } + + /** Envelope adata format: `:::`. */ + private formatOfflineSigningAdata(state: string, core: string): string { + return `${state}:${Date.now()}:${core}`; + } + + /** + * Validates an encrypted share envelope's adata against the expected signable binding. + * adata is AEAD-authenticated (v1 CCM / v2 GCM), so it cannot be stripped or altered + * without breaking decryption. The creation timestamp is checked separately from the + * binding, so the two concerns stay independent. + */ + private validateOfflineSigningAdata(cyphertext: string, state: string, expectedCore: string): void { + let stored: string | undefined; + try { + const raw = JSON.parse(cyphertext).adata; + // v1 (SJCL) envelopes escape() the adata (':' -> '%3A'); decode before parsing, + // matching baseTSSUtils.validateAdata. + stored = typeof raw === 'string' ? decodeURIComponent(raw) : undefined; + } catch { + throw new Error('Failed to parse cyphertext to JSON'); + } + if (typeof stored !== 'string') { + throw new Error('Missing adata on encrypted share envelope; restart the signing session'); + } + if (!stored.startsWith(`${state}:`)) { + throw new Error('Adata does not match cyphertext adata'); + } + const remainder = stored.slice(state.length + 1); + const separator = remainder.indexOf(':'); + assert(separator > 0, 'Adata does not match cyphertext adata'); + const created = Number(remainder.slice(0, separator)); + const core = remainder.slice(separator + 1); + if (core !== expectedCore) { + throw new Error('Adata does not match cyphertext adata'); + } + assert( + Number.isFinite(created) && Date.now() - created <= OFFLINE_SIGNING_SHARE_MAX_AGE_MS, + 'Encrypted share envelope has expired; restart the signing session' + ); + } + async createOfflineKShare(params: { tssParams: TSSParams | TSSParamsForMessage; challenges: { @@ -602,32 +720,113 @@ export class EcdsaUtils extends BaseEcdsaUtils { // TODO BG-67299 Message signing with derivation path derivationPath = ''; } + const bufferToSign = tssParams && 'bufferToSign' in tssParams ? tssParams.bufferToSign : undefined; + assert( + challenges.bitgoChallenge?.n && + challenges.bitgoChallenge.n.toLowerCase() === + this.getBitgoPaillierModulusFromSigningMaterial(prv).toLowerCase(), + "BitGo challenge Paillier modulus does not match the signer's BitGo key material" + ); return this.createTssEcdsaStep1SigningMaterial({ prv: prv, challenges: challenges, derivationPath: derivationPath, walletPassphrase: params.walletPassphrase, encryptionVersion: params.encryptionVersion, + signableAdata: this.getOfflineSigningAdata(txRequest, requestType, { bufferToSign }), }); } async createOfflineMuDeltaShare(params: { + /** Bound signable context; enables wShare adata validation. */ + tssParams?: TSSParams | TSSParamsForMessage; + requestType?: RequestType; + /** Legacy relays pass txRequest at top level; still fully validated. */ + txRequest?: TxRequest; + /** Injected by the express signer route; enables Paillier-modulus validation. */ + prv?: string; + /** Marks the wShare ciphertext consumed; invoked only after the envelope fully authenticates. */ + markConsumed?: () => void; aShareFromBitgo: Omit; bitgoChallenge: TxRequestChallengeResponse; encryptedWShare: string; walletPassphrase: string; encryptionVersion?: EncryptionVersion; }): Promise { + // Resolve txRequest/requestType from the current shape (tssParams/requestType) or the + // legacy relay shape (top-level txRequest; relays only sign transactions). + const tssParams = params.tssParams; + let txRequest: TxRequest | undefined; + if (tssParams) { + // fail loudly on a malformed (string) txRequest rather than skipping validation + assert(typeof tssParams.txRequest !== 'string', 'Invalid txRequest type'); + txRequest = tssParams.txRequest; + } else { + txRequest = params.txRequest; + } + const requestType = params.requestType ?? RequestType.tx; + const bufferToSign = tssParams && 'bufferToSign' in tssParams ? tssParams.bufferToSign : undefined; + // With txRequest: validate the wShare binding and re-bind the oShare to the same signable. + // Without txRequest (legacy direct callers): propagate the wShare envelope's own binding; + // this path does not itself check the signable — the S round enforces that binding + // (re-prefixed for the oShare), so the S round still validates strictly. adata is + // AEAD-authenticated, so a captured envelope's binding cannot be stripped or altered. + const expectedCore = txRequest ? this.getOfflineSigningAdata(txRequest, requestType, { bufferToSign }) : undefined; + let signableAdata: string; + if (expectedCore) { + this.validateOfflineSigningAdata(params.encryptedWShare, EcdsaUtils.GG18_SIGNING_WSHARE_STATE, expectedCore); + signableAdata = expectedCore; + } else { + let wAdata: unknown; + try { + // v1 (SJCL) envelopes escape() the adata (':' -> '%3A'); decode before parsing, + // matching baseTSSUtils.validateAdata. + wAdata = decodeURIComponent(JSON.parse(params.encryptedWShare).adata ?? ''); + } catch { + throw new Error('Failed to parse encryptedWShare to JSON'); + } + const wPrefix = `${EcdsaUtils.GG18_SIGNING_WSHARE_STATE}:`; + assert( + typeof wAdata === 'string' && wAdata.startsWith(wPrefix), + 'wShare envelope is missing a binding; restart the signing session' + ); + // Re-prefix the wShare binding for the oShare: strip the original timestamp (checking + // its age), then let the builder stamp the oShare's own creation time. + const wCore = wAdata.slice(wPrefix.length); + const wSeparator = wCore.indexOf(':'); + assert(wSeparator > 0, 'Adata does not match cyphertext adata'); + const wCreated = Number(wCore.slice(0, wSeparator)); + assert( + Number.isFinite(wCreated) && Date.now() - wCreated <= OFFLINE_SIGNING_SHARE_MAX_AGE_MS, + 'Encrypted share envelope has expired; restart the signing session' + ); + signableAdata = wCore.slice(wSeparator + 1); + } + if (params.prv) { + const bitgoPaillierModulus = this.getBitgoPaillierModulusFromSigningMaterial(params.prv).toLowerCase(); + assert( + params.bitgoChallenge?.n && params.bitgoChallenge.n.toLowerCase() === bitgoPaillierModulus, + "BitGo challenge Paillier modulus does not match the signer's BitGo key material" + ); + assert( + params.aShareFromBitgo?.n && params.aShareFromBitgo.n.toLowerCase() === bitgoPaillierModulus, + 'A-share from BitGo has an unexpected Paillier modulus' + ); + } const decryptedWShare = await this.bitgo.decrypt({ input: params.encryptedWShare, password: params.walletPassphrase, }); + // The envelope has fully authenticated (AEAD tag verified); only now is it marked + // consumed, so unauthenticated junk never reaches the registry. + params.markConsumed?.(); return await this.createTssEcdsaStep2SigningMaterial({ aShareFromBitgo: params.aShareFromBitgo, bitgoChallenge: params.bitgoChallenge, wShare: JSON.parse(decryptedWShare), walletPassphrase: params.walletPassphrase, encryptionVersion: params.encryptionVersion, + signableAdata: signableAdata, }); } @@ -637,6 +836,8 @@ export class EcdsaUtils extends BaseEcdsaUtils { requestType: RequestType; encryptedOShare: string; walletPassphrase: string; + /** Marks the oShare ciphertext consumed; invoked only after the envelope fully authenticates. */ + markConsumed?: () => void; }): Promise { const { tssParams, requestType, dShareFromBitgo, encryptedOShare, walletPassphrase } = params; assert(typeof tssParams.txRequest !== 'string', 'Invalid txRequest type'); @@ -655,6 +856,13 @@ export class EcdsaUtils extends BaseEcdsaUtils { } else if (requestType === RequestType.message) { signablePayload = (params.tssParams as TSSParamsForMessage).bufferToSign; } + const bufferToSign = + params.tssParams && 'bufferToSign' in params.tssParams ? params.tssParams.bufferToSign : undefined; + this.validateOfflineSigningAdata( + encryptedOShare, + EcdsaUtils.GG18_SIGNING_OSHARE_STATE, + this.getOfflineSigningAdata(txRequest, requestType, { bufferToSign }) + ); let hash: Hash | undefined; try { hash = this.baseCoin.getHashFunction(); @@ -662,6 +870,9 @@ export class EcdsaUtils extends BaseEcdsaUtils { hash = undefined; } const decryptedOShare = await this.bitgo.decrypt({ input: encryptedOShare, password: walletPassphrase }); + // The envelope has fully authenticated (AEAD tag verified); only now is it marked + // consumed, so unauthenticated junk never reaches the registry. + params.markConsumed?.(); const { i, R, s, y } = await ECDSAMethods.createUserSignatureShare( JSON.parse(decryptedOShare), dShareFromBitgo, @@ -726,6 +937,11 @@ export class EcdsaUtils extends BaseEcdsaUtils { )) as Omit; // WP/HSM does not return the initial challenge const step2Return = await externalSignerMuDeltaShareGenerator({ txRequest: txRequestObj, + tssParams: { + ...params, + txRequest: txRequestObj, + }, + requestType: requestType, aShareFromBitgo: bitgoToUserAShare, bitgoChallenge: bitgoChallenge, encryptedWShare: step1SigningMaterial.wShare as string,