diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index a5c5b36e8b..5e0f7b7b76 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 c4a05b7e52..f29a04654f 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 612b3577a1..357a49bba6 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 180498a62f..b97e3f84e6 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 8f8c6b8791..339abb70f0 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 ba1e65bd8b..cc3d01efaf 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 0000000000..fcbf3cd165 --- /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 bdebf72471..a960ff754b 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 4916c5e30d..5304738735 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 0473a7ee39..38595a1f17 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 fece8b88d0..e07e38e0d4 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,