From 27294d37653bdfa46b4a1bae01215581e83af31c Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Tue, 10 Nov 2026 16:21:04 +0530 Subject: [PATCH 1/5] refactor(sdk-core): hoist offline signing adata helpers for TSS reuse Moves the offline-signing adata binding helpers (signable/path core, envelope formatting and validation, max age) from EcdsaUtils into a shared module so signing protocols enforce an identical binding format. Behavior-preserving; EcdsaUtils delegates to the shared helpers. References: WAL-2456 --- .../src/bitgo/utils/tss/ecdsa/ecdsa.ts | 124 +++--------------- .../bitgo/utils/tss/offlineSigningAdata.ts | 110 ++++++++++++++++ 2 files changed, 125 insertions(+), 109 deletions(-) create mode 100644 modules/sdk-core/src/bitgo/utils/tss/offlineSigningAdata.ts 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 30576599b4..12daf62ba2 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, createHash } from 'crypto'; +import { Hash } from 'crypto'; import { EcdsaPaillierProof, EcdsaRangeProof, EcdsaTypes, hexToBigInt, minModulusBitLength } from '@bitgo/sdk-lib-mpc'; import { bip32 } from '@bitgo/utxo-lib'; @@ -33,7 +33,6 @@ import { TssSignTxRequestParamsWithPrv, TssTxRecipientSource, TxRequest, - UnsignedTransactionTss, } from '../baseTypes'; import { getTxRequest } from '../../../tss'; import { AShare, DShare, EncryptedNShare, SendShareType, SShare, WShare, OShare } from '../../../tss/ecdsa/types'; @@ -53,11 +52,16 @@ import { BaseEcdsaUtils } from './base'; import { EncryptionVersion, IRequestTracer } from '../../../../api'; import { resolveEffectiveTxParams } from '../recipientUtils'; import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify'; +import { + formatOfflineSigningAdata, + getOfflineSigningAdata, + OFFLINE_SIGNING_SHARE_MAX_AGE_MS, + validateOfflineSigningAdata, +} from '../offlineSigningAdata'; const encryptNShare = ECDSAMethods.encryptNShare; -/** Encrypted signing-share envelopes older than this are rejected along with their registry entries. */ -export const OFFLINE_SIGNING_SHARE_MAX_AGE_MS = 30 * 24 * 3600 * 1000; // 30 days +export { OFFLINE_SIGNING_SHARE_MAX_AGE_MS }; /** @inheritdoc */ export class EcdsaUtils extends BaseEcdsaUtils { @@ -535,7 +539,7 @@ export class EcdsaUtils extends BaseEcdsaUtils { password: params.walletPassphrase, encryptionVersion: params.encryptionVersion, adata: params.signableAdata - ? this.formatOfflineSigningAdata(EcdsaUtils.GG18_SIGNING_WSHARE_STATE, params.signableAdata) + ? formatOfflineSigningAdata(EcdsaUtils.GG18_SIGNING_WSHARE_STATE, params.signableAdata) : undefined, }) : userSignShare.wShare, @@ -574,7 +578,7 @@ export class EcdsaUtils extends BaseEcdsaUtils { password: params.walletPassphrase, encryptionVersion: params.encryptionVersion, adata: params.signableAdata - ? this.formatOfflineSigningAdata(EcdsaUtils.GG18_SIGNING_OSHARE_STATE, params.signableAdata) + ? formatOfflineSigningAdata(EcdsaUtils.GG18_SIGNING_OSHARE_STATE, params.signableAdata) : undefined, }) : userOmicronAndDeltaShare.oShare, @@ -587,57 +591,6 @@ 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) @@ -649,53 +602,6 @@ export class EcdsaUtils extends BaseEcdsaUtils { 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): string { - 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'); - } - const now = Date.now(); - assert( - Number.isFinite(created) && created <= now, - 'Encrypted share envelope has a future timestamp; restart the signing session' - ); - assert( - now - created <= OFFLINE_SIGNING_SHARE_MAX_AGE_MS, - 'Encrypted share envelope has expired; restart the signing session' - ); - return stored; - } - async createOfflineKShare(params: { tssParams: TSSParams | TSSParamsForMessage; challenges: { @@ -739,7 +645,7 @@ export class EcdsaUtils extends BaseEcdsaUtils { derivationPath: derivationPath, walletPassphrase: params.walletPassphrase, encryptionVersion: params.encryptionVersion, - signableAdata: this.getOfflineSigningAdata(txRequest, requestType, { bufferToSign }), + signableAdata: getOfflineSigningAdata(txRequest, requestType, { bufferToSign }), }); } @@ -778,10 +684,10 @@ export class EcdsaUtils extends BaseEcdsaUtils { // 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; + const expectedCore = txRequest ? getOfflineSigningAdata(txRequest, requestType, { bufferToSign }) : undefined; let signableAdata: string; if (expectedCore) { - validatedAdata = this.validateOfflineSigningAdata( + validatedAdata = validateOfflineSigningAdata( params.encryptedWShare, EcdsaUtils.GG18_SIGNING_WSHARE_STATE, expectedCore @@ -875,10 +781,10 @@ export class EcdsaUtils extends BaseEcdsaUtils { } const bufferToSign = params.tssParams && 'bufferToSign' in params.tssParams ? params.tssParams.bufferToSign : undefined; - const validatedAdata = this.validateOfflineSigningAdata( + const validatedAdata = validateOfflineSigningAdata( encryptedOShare, EcdsaUtils.GG18_SIGNING_OSHARE_STATE, - this.getOfflineSigningAdata(txRequest, requestType, { bufferToSign }) + getOfflineSigningAdata(txRequest, requestType, { bufferToSign }) ); let hash: Hash | undefined; try { diff --git a/modules/sdk-core/src/bitgo/utils/tss/offlineSigningAdata.ts b/modules/sdk-core/src/bitgo/utils/tss/offlineSigningAdata.ts new file mode 100644 index 0000000000..588b03048f --- /dev/null +++ b/modules/sdk-core/src/bitgo/utils/tss/offlineSigningAdata.ts @@ -0,0 +1,110 @@ +/** + * @prettier + */ +import assert from 'assert'; +import { createHash } from 'crypto'; + +import { RequestType, TxRequest, UnsignedTransactionTss } from './baseTypes'; + +/** + * Encrypted signing-share envelopes older than this are rejected along with their registry entries. + */ +export const OFFLINE_SIGNING_SHARE_MAX_AGE_MS = 30 * 24 * 3600 * 1000; // 30 days + +/** + * 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. + */ +export function 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}`; +} + +/** Envelope adata format: `:::`. */ +export function 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. + */ +export function validateOfflineSigningAdata(cyphertext: string, state: string, expectedCore: string): string { + 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'); + } + const now = Date.now(); + assert( + Number.isFinite(created) && created <= now, + 'Encrypted share envelope has a future timestamp; restart the signing session' + ); + assert( + now - created <= OFFLINE_SIGNING_SHARE_MAX_AGE_MS, + 'Encrypted share envelope has expired; restart the signing session' + ); + return stored; +} From 774bbb9944ac898df9f5fde67216f0e3058cdc2a Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Tue, 10 Nov 2026 16:23:47 +0530 Subject: [PATCH 2/5] fix(sdk-core): redact EdDSA MPCv1 R share and bind G round to signer-held envelope The R round now returns only the nonce reveal (r and R). The R-share envelope is bound to the signable via AEAD adata and validated at the R and G rounds, and the G round consumes the signer-created envelope once via a markConsumed callback instead of caller-supplied share values. The commitment round accepts only the pinned BitGo MPCv1 GPG public key in environments that require BitGo public key config. References: WAL-2456 --- .../test/v2/unit/internal/tssUtils/eddsa.ts | 160 +++++++++++++++++- modules/bitgo/test/v2/unit/tss/eddsa.ts | 49 ++---- modules/sdk-core/src/bitgo/tss/eddsa/eddsa.ts | 14 +- .../src/bitgo/utils/tss/baseTSSUtils.ts | 20 ++- .../sdk-core/src/bitgo/utils/tss/baseTypes.ts | 18 +- .../src/bitgo/utils/tss/eddsa/eddsa.ts | 74 ++++++-- 6 files changed, 251 insertions(+), 84 deletions(-) diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts index 477cdee351..9a927dc07e 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts @@ -1,3 +1,4 @@ +import { randomBytes } from 'crypto'; import * as assert from 'assert'; import * as sodium from 'libsodium-wrappers-sumo'; import * as _ from 'lodash'; @@ -19,10 +20,13 @@ import { Ed25519BIP32, EDDSAUtils, Eddsa, + EddsaUtils, EncryptedSignerShareType, ExchangeCommitmentResponse, Keychain, KeyShare, + bigIntFromBufferLE, + bigIntToBufferLE, RequestTracer, RequestType, SignatureShareRecord, @@ -672,10 +676,34 @@ describe('TSS Utils:', async function () { userId: 'userId', }; - it('v2 R-share round-trip: encrypt via commitment, verify envelope, decrypt via createRShare', async function () { - const passphrase = 'test-passphrase'; - const prv = JSON.stringify(validUserSigningMaterial); + const passphrase = 'test-passphrase'; + const prv = JSON.stringify(validUserSigningMaterial); + const otherSignableTxRequest: TxRequest = { + ...signingTxRequest, + unsignedTxs: [{ ...signingTxRequest.unsignedTxs[0], signableHex: 'deadbe00' }], + }; + const forgedBitgoRecords = () => { + // Production nonce pattern (account-lib signShare): reduce random entropy to a scalar. + const rB = Eddsa.curve.scalarReduce(bigIntFromBufferLE(randomBytes(64))); + const rBPoint = Eddsa.curve.basePointMult(rB); + const rBHex = bigIntToBufferLE(rB, 32).toString('hex'); + const rBPointHex = bigIntToBufferLE(rBPoint, 32).toString('hex'); + return { + bitgoToUserRShare: { + from: SignatureShareType.BITGO, + to: SignatureShareType.USER, + share: rBHex + rBPointHex, + }, + bitgoToUserCommitment: { + from: SignatureShareType.BITGO, + to: SignatureShareType.USER, + type: CommitmentType.COMMITMENT, + share: rBPointHex, + }, + }; + }; + it('v2 signing round-trip: bound R envelope, redacted R reveal, G consumption', async function () { // 1. Encrypt prv with v2 const encryptedPrv = await bitgo.encrypt({ input: prv, @@ -684,7 +712,7 @@ describe('TSS Utils:', async function () { }); JSON.parse(encryptedPrv).v.should.equal(2); - // 2. Create commitment -- R-share should be a v2 envelope + // 2. Create commitment -- R-share should be a v2 envelope bound to the signable const commitResult = await tssUtils.createCommitmentShareFromTxRequest({ txRequest: signingTxRequest, prv, @@ -696,16 +724,132 @@ describe('TSS Utils:', async function () { const rShareEnvelope = JSON.parse(commitResult.encryptedUserToBitgoRShare.share); rShareEnvelope.v.should.equal(2); rShareEnvelope.should.have.property('hkdfSalt'); + rShareEnvelope.should.have.property('adata'); - // 3. Round-trip: decrypt the v2 R-share + // 3. R round only reveals the nonce material offered to BitGo const { rShare } = await tssUtils.createRShareFromTxRequest({ txRequest: signingTxRequest, walletPassphrase: passphrase, encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, }); + rShare.should.have.property('r').and.be.a.String(); + rShare.should.have.property('R').and.be.a.String(); + Object.keys(rShare).sort().should.deepEqual(['R', 'r']); + + // 4. G round consumes the same envelope and produces the user gamma + const { bitgoToUserRShare, bitgoToUserCommitment } = forgedBitgoRecords(); + const gShare = await tssUtils.createGShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, + bitgoToUserRShare, + bitgoToUserCommitment, + markConsumed: () => undefined, + }); + gShare.should.have.property('gamma').and.be.a.String(); + }); + + it('commitment rejects a caller-supplied non-BitGo GPG public key in pinned envs', async function () { + const testEnvBitgo = TestBitGo.decorate(BitGo, { env: 'test' }); + const testEnvUtils = new EddsaUtils(testEnvBitgo, testEnvBitgo.coin('tsol')); + await testEnvUtils + .createCommitmentShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + bitgoGpgPubKey: bitgoGpgKey.publicKey, + }) + .should.be.rejectedWith('Invalid BitGo GPG public key'); + }); + + it('R round rejects an envelope bound to a different signable', async function () { + const commitResult = await tssUtils.createCommitmentShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + bitgoGpgPubKey: bitgoGpgKey.publicKey, + }); + await tssUtils + .createRShareFromTxRequest({ + txRequest: otherSignableTxRequest, + walletPassphrase: passphrase, + encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, + }) + .should.be.rejectedWith('Adata does not match cyphertext adata'); + }); + + it('G round rejects an envelope bound to a different signable', async function () { + const commitResult = await tssUtils.createCommitmentShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + bitgoGpgPubKey: bitgoGpgKey.publicKey, + }); + const { bitgoToUserRShare, bitgoToUserCommitment } = forgedBitgoRecords(); + await tssUtils + .createGShareFromTxRequest({ + txRequest: otherSignableTxRequest, + prv, + walletPassphrase: passphrase, + encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, + bitgoToUserRShare, + bitgoToUserCommitment, + }) + .should.be.rejectedWith('Adata does not match cyphertext adata'); + }); + + it('G round rejects a replayed envelope (second use with a different BitGo nonce)', async function () { + const commitResult = await tssUtils.createCommitmentShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + bitgoGpgPubKey: bitgoGpgKey.publicKey, + }); + const records = forgedBitgoRecords(); + let consumed = false; + const markConsumed = () => { + if (consumed) { + throw new Error('encrypted share has already been consumed; restart the signing session'); + } + consumed = true; + }; + await tssUtils.createGShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, + bitgoToUserRShare: records.bitgoToUserRShare, + bitgoToUserCommitment: records.bitgoToUserCommitment, + markConsumed, + }); + const secondRecords = forgedBitgoRecords(); + await tssUtils + .createGShareFromTxRequest({ + txRequest: signingTxRequest, + prv, + walletPassphrase: passphrase, + encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, + bitgoToUserRShare: secondRecords.bitgoToUserRShare, + bitgoToUserCommitment: secondRecords.bitgoToUserCommitment, + markConsumed, + }) + .should.be.rejectedWith('encrypted share has already been consumed'); + }); - should.exist(rShare.xShare); - should.exist(rShare.rShares); + it('R round rejects a legacy envelope without a signable binding', async function () { + const legacyEnvelope = await bitgo.encrypt({ + input: JSON.stringify({ xShare: {}, rShares: {} }), + password: passphrase, + encryptionVersion: 2, + }); + await tssUtils + .createRShareFromTxRequest({ + txRequest: signingTxRequest, + walletPassphrase: passphrase, + encryptedUserToBitgoRShare: { from: 'user', to: 'bitgo', share: legacyEnvelope, type: 'encryptedRShare' }, + }) + .should.be.rejectedWith('Missing adata on encrypted share envelope; restart the signing session'); }); }); @@ -819,7 +963,7 @@ describe('TSS Utils:', async function () { encryptedSignerShare: tssUtils.createUserToBitgoEncryptedSignerShare('encrypted-signer-share'), encryptedUserToBitgoRShare: tssUtils.createUserToBitgoEncryptedSignerShare('encrypted-r-share'), }), - async () => ({ rShare: validUserSignShare }), + async () => ({ rShare: { r: validUserSignShare.rShares[3].r, R: validUserSignShare.rShares[3].R } }), async () => ({ i: 1, y: 'user-y-share', diff --git a/modules/bitgo/test/v2/unit/tss/eddsa.ts b/modules/bitgo/test/v2/unit/tss/eddsa.ts index 1fadbaff89..c82b69e196 100644 --- a/modules/bitgo/test/v2/unit/tss/eddsa.ts +++ b/modules/bitgo/test/v2/unit/tss/eddsa.ts @@ -483,11 +483,16 @@ describe('test tss helper functions', function () { }); describe('offerUserToBitgoRShare:', async function () { - it('should succeed to send Signature Share', async function () { + const reveal = { + r: validUserSignShare.rShares[3].r, + R: validUserSignShare.rShares[3].R, + }; + + it('should succeed to send the nonce reveal as the signature share', async function () { const signatureShare = { from: 'user', to: 'bitgo', - share: validUserSignShare.rShares[3].r + validUserSignShare.rShares[3].R, + share: reveal.r + reveal.R, } as SignatureShareRecord; const nock = await nockSendSignatureShare({ walletId: wallet.id(), @@ -499,51 +504,17 @@ describe('test tss helper functions', function () { bitgo, wallet.id(), txRequest.txRequestId, - validUserSignShare, + reveal, 'signerShare' ).should.be.fulfilled(); nock.isDone().should.equal(true); }); - it('should fail if no rShare is found', async function () { - const invalidUserSignShare = _.cloneDeep(validUserSignShare) as any; - delete invalidUserSignShare.rShares[3]; - await offerUserToBitgoRShare( - bitgo, - wallet.id(), - txRequest.txRequestId, - invalidUserSignShare, - 'signerShare' - ).should.be.rejectedWith('userToBitgo RShare not found'); - }); - - it('should fail if the rShare found is invalid', async function () { - const invalidUserSignShare = _.cloneDeep(validUserSignShare) as any; - invalidUserSignShare.rShares[3].i = 1; - await offerUserToBitgoRShare( - bitgo, - wallet.id(), - txRequest.txRequestId, - invalidUserSignShare, - 'signerShare' - ).should.be.rejectedWith('Invalid RShare, is not from User to Bitgo'); - - const invalidUserSignShare2 = _.cloneDeep(validUserSignShare) as any; - invalidUserSignShare2.rShares[3].j = 3; - await offerUserToBitgoRShare( - bitgo, - wallet.id(), - txRequest.txRequestId, - invalidUserSignShare2, - 'signerShare' - ).should.be.rejectedWith('Invalid RShare, is not from User to Bitgo'); - }); - it('should call setRequestTracer', async function () { const signatureShare = { from: 'user', to: 'bitgo', - share: validUserSignShare.rShares[3].r + validUserSignShare.rShares[3].R, + share: reveal.r + reveal.R, } as SignatureShareRecord; const nock = await nockSendSignatureShare({ walletId: wallet.id(), @@ -558,7 +529,7 @@ describe('test tss helper functions', function () { bitgo, wallet.id(), txRequest.txRequestId, - validUserSignShare, + reveal, 'signerShare', undefined, reqId diff --git a/modules/sdk-core/src/bitgo/tss/eddsa/eddsa.ts b/modules/sdk-core/src/bitgo/tss/eddsa/eddsa.ts index 30a89b547f..4fcf91dfe1 100644 --- a/modules/sdk-core/src/bitgo/tss/eddsa/eddsa.ts +++ b/modules/sdk-core/src/bitgo/tss/eddsa/eddsa.ts @@ -21,6 +21,7 @@ import { RequestType, CommitmentShareRecord, CommitmentType, + UserToBitgoRShareReveal, } from '../../utils'; import { BaseTransaction } from '../../../account-lib'; import { Ed25519Bip32HdTree } from '@bitgo/sdk-lib-mpc'; @@ -189,11 +190,11 @@ export async function createUserToBitGoGShare( } /** - * Sends the User to Bitgo RShare to Bitgo + * Sends the User to Bitgo RShare reveal to Bitgo * @param {BitGoBase} bitgo - the bitgo instance * @param {String} walletId - the wallet id * @param {String} txRequestId - the txRequest Id - * @param {SignShare} userSignShare - the user Sign Share + * @param {UserToBitgoRShareReveal} rShare - the user nonce reveal (r and R only) * @param {String} encryptedSignerShare - signer share encrypted to bitgo key * @param {String} apiMode - the api mode, defaults to 'lite' * @returns {Promise} @@ -204,20 +205,13 @@ export async function offerUserToBitgoRShare( bitgo: BitGoBase, walletId: string, txRequestId: string, - userSignShare: SignShare, + rShare: UserToBitgoRShareReveal, encryptedSignerShare: string, apiMode: 'full' | 'lite' = 'lite', reqId?: IRequestTracer, requestType: RequestType = RequestType.tx, attestation?: AttestationPayload ): Promise { - const rShare: RShare = userSignShare.rShares[ShareKeyPosition.BITGO]; - if (_.isNil(rShare)) { - throw new Error('userToBitgo RShare not found'); - } - if (rShare.i !== ShareKeyPosition.BITGO || rShare.j !== ShareKeyPosition.USER) { - throw new Error('Invalid RShare, is not from User to Bitgo'); - } const signatureShare: SignatureShareRecord = { from: SignatureShareType.USER, to: SignatureShareType.BITGO, diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts index 78cc176570..3f62425fb1 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts @@ -42,8 +42,9 @@ import { TssSignTxRequestParamsWithPrv, TxRequest, TxRequestVersion, + UserToBitgoRShareReveal, } from './baseTypes'; -import { GShare, SignShare } from '../../../account-lib/mpc/tss'; +import { GShare } from '../../../account-lib/mpc/tss'; import { RequestTracer } from '../util'; import { envRequiresBitgoPubGpgKeyConfig, getBitgoMpcGpgPubKey } from '../../tss/bitgoPubKeys'; import { getBitgoGpgPubKey } from '../opengpgUtils'; @@ -371,16 +372,15 @@ export default class BaseTssUtils extends MpcUtils implements ITssUtil * * @param {Object} params - params object * @param {TxRequest} params.txRequest - transaction request with unsigned transaction - * @param {string} params.prv - user signing material - * @param {string} [params.walletPassphrase] - wallet passphrase - * @param {EncryptedSignerShareRecord} [params.encryptedUserToBitgoRShare] - encrypted user to bitgo R share generated in the commitment phase - * @returns {Promise<{ rShare: SignShare }>} - R Share to BitGo + * @param {string} params.walletPassphrase - wallet passphrase + * @param {EncryptedSignerShareRecord} params.encryptedUserToBitgoRShare - encrypted user to bitgo R share generated in the commitment phase + * @returns {Promise<{ rShare: UserToBitgoRShareReveal }>} - nonce reveal (r and R) to offer to BitGo */ createRShareFromTxRequest(params: { txRequest: TxRequest; walletPassphrase: string; encryptedUserToBitgoRShare: EncryptedSignerShareRecord; - }): Promise<{ rShare: SignShare }> { + }): Promise<{ rShare: UserToBitgoRShareReveal }> { throw new Error('Method not implemented.'); } @@ -390,17 +390,21 @@ export default class BaseTssUtils extends MpcUtils implements ITssUtil * @param {Object} params - params object * @param {TxRequest} params.txRequest - transaction request with unsigned transaction * @param {string} params.prv - user signing material + * @param {string} params.walletPassphrase - wallet passphrase + * @param {EncryptedSignerShareRecord} params.encryptedUserToBitgoRShare - encrypted user to bitgo R share generated in the commitment phase * @param {SignatureShareRecord} params.bitgoToUserRShare - BitGo to User R Share - * @param {SignShare} params.userToBitgoRShare - User to BitGo R Share * @param {CommitmentShareRecord} params.bitgoToUserCommitment - BitGo to User Commitment + * @param {(adata: string) => void} [params.markConsumed] - marks the R-share envelope consumed via its validated adata after it authenticates * @returns {Promise} - GShare from User to BitGo */ createGShareFromTxRequest(params: { txRequest: TxRequest; prv: string; + walletPassphrase: string; + encryptedUserToBitgoRShare: EncryptedSignerShareRecord; bitgoToUserRShare: SignatureShareRecord; - userToBitgoRShare: SignShare; bitgoToUserCommitment: CommitmentShareRecord; + markConsumed?: (adata: string) => void; }): Promise { throw new Error('Method not implemented.'); } diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index b51ab61b47..7092806b27 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -4,7 +4,7 @@ import { type ITransactionRecipient, KeychainsTriplet, ParsedTransaction, Transa import { ApiKeyShare, Keychain, WebauthnKeyEncryptionInfo } from '../../keychain'; import { ApiVersion, Memo, WalletType, type PrebuildTransactionFeeInfo } from '../../wallet'; import type { AttestationPayload } from '../../wallet/BuildParams'; -import { EDDSA, GShare, Signature, SignShare } from '../../../account-lib/mpc/tss'; +import { EDDSA, GShare, Signature } from '../../../account-lib/mpc/tss'; import { Signature as EcdsaSignature } from '../../../account-lib/mpc/tss/ecdsa/types'; import { KeyShare } from './ecdsa'; import { EcdsaTypes } from '@bitgo/sdk-lib-mpc'; @@ -196,16 +196,22 @@ export interface CustomCommitmentGeneratingFunction { }>; } +/** The only R-round material that leaves the signer: the nonce reveal offered to BitGo. */ +export interface UserToBitgoRShareReveal { + r: string; + R: string; +} + export interface CustomRShareGeneratingFunction { (params: { txRequest: TxRequest; encryptedUserToBitgoRShare: EncryptedSignerShareRecord }): Promise<{ - rShare: SignShare; + rShare: UserToBitgoRShareReveal; }>; } export interface CustomGShareGeneratingFunction { (params: { txRequest: TxRequest; - userToBitgoRShare: SignShare; + encryptedUserToBitgoRShare: EncryptedSignerShareRecord; bitgoToUserRShare: SignatureShareRecord; bitgoToUserCommitment: CommitmentShareRecord; }): Promise; @@ -1004,13 +1010,15 @@ export interface ITssUtils { txRequest: TxRequest; walletPassphrase: string; encryptedUserToBitgoRShare: EncryptedSignerShareRecord; - }): Promise<{ rShare: SignShare }>; + }): Promise<{ rShare: UserToBitgoRShareReveal }>; createGShareFromTxRequest(params: { txRequest: TxRequest; prv: string; + walletPassphrase: string; + encryptedUserToBitgoRShare: EncryptedSignerShareRecord; bitgoToUserRShare: SignatureShareRecord; - userToBitgoRShare: SignShare; bitgoToUserCommitment: CommitmentShareRecord; + markConsumed?: (adata: string) => void; }): Promise; prebuildTxWithIntent( params: PrebuildTransactionWithIntentOptions, diff --git a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts index 2a678a1d0a..d686942af3 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts @@ -33,9 +33,11 @@ import { TssSignTxRequestParamsWithPrv, TssTxRecipientSource, TxRequest, + UserToBitgoRShareReveal, UnsignedTransactionTss, isV2Envelope, } from '../baseTypes'; +import { formatOfflineSigningAdata, getOfflineSigningAdata, validateOfflineSigningAdata } from '../offlineSigningAdata'; import { InvalidTransactionError } from '../../../errors'; import { CreateEddsaBitGoKeychainParams, CreateEddsaKeychainParams, KeyShare, YShare } from './types'; import baseTSSUtils from '../baseTSSUtils'; @@ -55,6 +57,7 @@ import type { EddsaKeyGenCallbacks } from '../../../wallet/iWallets'; */ export class EddsaUtils extends baseTSSUtils { + private static readonly EDDSA_SIGNING_RSHARE_STATE = 'EDDSA_SIGNING_RSHARE_STATE'; async verifyWalletSignatures( userGpgPub: string, backupGpgPub: string, @@ -536,6 +539,10 @@ export class EddsaUtils extends baseTSSUtils { }> { const bitgoIndex = ShareKeyPosition.BITGO; const { txRequest, prv } = params; + + if (envRequiresBitgoPubGpgKeyConfig(this.bitgo.getEnv())) { + assert(isBitgoMpcPubKey(params.bitgoGpgPubKey, 'mpcv1'), 'Invalid BitGo GPG public key'); + } const txRequestResolved: TxRequest = txRequest; const hdTree = await Ed25519Bip32HdTree.initialize(); @@ -579,11 +586,15 @@ export class EddsaUtils extends baseTSSUtils { const encryptedSignerShare = this.createUserToBitgoEncryptedSignerShare(userToBitgoEncryptedSignerShare); const stringifiedRShare = JSON.stringify(userSignShare); + // Bind the R-share envelope to this exact signable so later rounds reject replays + // against any other transaction or derivation path. + const signableAdata = getOfflineSigningAdata(txRequestResolved, RequestType.tx); + const rShareAdata = formatOfflineSigningAdata(EddsaUtils.EDDSA_SIGNING_RSHARE_STATE, signableAdata); let encryptedRShare: string; if (params.encryptedPrv && isV2Envelope(params.encryptedPrv)) { const session = await this.bitgo.createEncryptionSession(params.walletPassphrase); try { - encryptedRShare = await session.encrypt(stringifiedRShare); + encryptedRShare = await session.encrypt(stringifiedRShare, rShareAdata); } finally { session.destroy(); } @@ -592,6 +603,7 @@ export class EddsaUtils extends baseTSSUtils { input: stringifiedRShare, password: params.walletPassphrase, encryptionVersion: 1, + adata: rShareAdata, }); } const encryptedUserToBitgoRShare = this.createUserToBitgoEncryptedRShare(encryptedRShare); @@ -603,30 +615,45 @@ export class EddsaUtils extends baseTSSUtils { txRequest: TxRequest; walletPassphrase: string; encryptedUserToBitgoRShare: EncryptedSignerShareRecord; - }): Promise<{ rShare: SignShare }> { - const { walletPassphrase, encryptedUserToBitgoRShare } = params; + }): Promise<{ rShare: UserToBitgoRShareReveal }> { + const { txRequest, walletPassphrase, encryptedUserToBitgoRShare } = params; + + // The envelope must belong to this exact signable; a captured envelope from + // another transaction (or derivation path) is rejected before decryption. + validateOfflineSigningAdata( + encryptedUserToBitgoRShare.share, + EddsaUtils.EDDSA_SIGNING_RSHARE_STATE, + getOfflineSigningAdata(txRequest, RequestType.tx) + ); const decryptedRShare = await this.bitgo.decrypt({ input: encryptedUserToBitgoRShare.share, password: walletPassphrase, }); - const rShare = JSON.parse(decryptedRShare); - assert(rShare.xShare, 'Unable to find xShare in decryptedRShare'); - assert(rShare.rShares, 'Unable to find rShares in decryptedRShare'); - - return { rShare }; + const userSignShare: SignShare = JSON.parse(decryptedRShare); + assert(userSignShare.xShare, 'Unable to find xShare in decryptedRShare'); + assert(userSignShare.rShares, 'Unable to find rShares in decryptedRShare'); + const bitgoRShare = userSignShare.rShares[ShareKeyPosition.BITGO]; + assert(bitgoRShare, 'Unable to find bitgo rShare in decryptedRShare'); + + // Reveal only the nonce material BitGo needs. The full SignShare (key share + // and nonce points) never leaves the signer in plaintext. + return { rShare: { r: bitgoRShare.r, R: bitgoRShare.R } }; } async createGShareFromTxRequest(params: { txRequest: string | TxRequest; prv: string; + walletPassphrase: string; + encryptedUserToBitgoRShare: EncryptedSignerShareRecord; bitgoToUserRShare: SignatureShareRecord; - userToBitgoRShare: SignShare; bitgoToUserCommitment: CommitmentShareRecord; + /** Marks the R-share envelope consumed via its validated adata; invoked only after the envelope fully authenticates. */ + markConsumed?: (adata: string) => void; }): Promise { let txRequestResolved: TxRequest; - const { txRequest, prv, bitgoToUserCommitment, bitgoToUserRShare, userToBitgoRShare } = params; + const { txRequest, prv, walletPassphrase, encryptedUserToBitgoRShare, markConsumed } = params; if (typeof txRequest === 'string') { txRequestResolved = await getTxRequest(this.bitgo, this.wallet.id(), txRequest); @@ -652,13 +679,29 @@ export class EddsaUtils extends baseTSSUtils { const signablePayload = Buffer.from(unsignedTx.signableHex, 'hex'); + // The G round only consumes signer-created state: the R-share envelope bound + // to this exact signable. A forged, modified or replayed envelope is rejected. + const validatedAdata = validateOfflineSigningAdata( + encryptedUserToBitgoRShare.share, + EddsaUtils.EDDSA_SIGNING_RSHARE_STATE, + getOfflineSigningAdata(txRequestResolved, RequestType.tx) + ); + const decryptedRShare = await this.bitgo.decrypt({ + input: encryptedUserToBitgoRShare.share, + password: walletPassphrase, + }); + // The envelope has fully authenticated (AEAD tag verified); only now is it + // marked consumed, so unauthenticated junk never reaches the registry. + markConsumed?.(validatedAdata); + const userToBitgoRShare: SignShare = JSON.parse(decryptedRShare); + const userToBitGoGShare = await createUserToBitGoGShare( userToBitgoRShare, - bitgoToUserRShare, + params.bitgoToUserRShare, userSigningMaterial.backupYShare, userSigningMaterial.bitgoYShare, signablePayload, - bitgoToUserCommitment + params.bitgoToUserCommitment ); return userToBitGoGShare; } @@ -717,7 +760,7 @@ export class EddsaUtils extends baseTSSUtils { const gSignShareTransactionParams = { txRequest: txRequestResolved, bitgoToUserRShare: bitgoToUserRShare, - userToBitgoRShare: rShare, + encryptedUserToBitgoRShare, bitgoToUserCommitment, }; const gShare = await externalSignerGShareGenerator(gSignShareTransactionParams); @@ -829,7 +872,10 @@ export class EddsaUtils extends baseTSSUtils { this.bitgo, this.wallet.id(), txRequestId, - userSignShare, + { + r: userSignShare.rShares[bitgoIndex].r, + R: userSignShare.rShares[bitgoIndex].R, + }, userToBitgoEncryptedSignerShare, apiVersion, params.reqId, From 06ae129f40dc9b65ef9e972ebf32f41965215117 Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Tue, 10 Nov 2026 16:25:16 +0530 Subject: [PATCH 3/5] fix(express): enforce single-use EdDSA MPCv1 R envelopes on signer Wires the consumed-shares registry into the EdDSA MPCv1 G round, removes the plaintext user share from the G request codec, and redacts the R response codec to the nonce reveal. References: WAL-2456 --- modules/express/src/clientRoutes.ts | 14 +- .../typedRoutes/api/v2/generateShareTSS.ts | 49 +-- .../test/unit/clientRoutes/externalSign.ts | 378 ++++++++++-------- .../test/unit/typedRoutes/generateShareTSS.ts | 61 +-- 4 files changed, 246 insertions(+), 256 deletions(-) diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 8e3c082ba7..a2a49723cc 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -32,7 +32,7 @@ import { multisigTypes, Safes, ShareType, - SignShare, + UserToBitgoRShareReveal, SShare, TssEcdsaStep1ReturnMessage, TssEcdsaStep2ReturnMessage, @@ -543,8 +543,16 @@ export async function handleV2GenerateShareTSS( return await eddsaUtils.createCommitmentShareFromTxRequest(req.body); case ShareType.R: return await eddsaUtils.createRShareFromTxRequest(req.body); - case ShareType.G: + case ShareType.G: { + // Each R-share envelope may be used exactly once; a replayed envelope is + // rejected before the share is used (fail-closed). + req.body.markConsumed = markConsumedShareClosure( + registryFilePath, + req.decoded.sharetype, + req.body.encryptedUserToBitgoRShare?.share + ); return await eddsaUtils.createGShareFromTxRequest(req.body); + } default: throw new Error( `Share type ${req.decoded.sharetype} not supported, only commitment, G and R share generation is supported.` @@ -2124,7 +2132,7 @@ export function createCustomCommitmentGenerator( } export function createCustomRShareGenerator(externalSignerUrl: string, coin: string): CustomRShareGeneratingFunction { - return async function (params): Promise<{ rShare: SignShare }> { + return async function (params): Promise<{ rShare: UserToBitgoRShareReveal }> { const { body: rShare } = await retryPromise( () => superagent.post(`${externalSignerUrl}/api/v2/${coin}/tssshare/R`).type('json').send(params), (err, tryCount) => { diff --git a/modules/express/src/typedRoutes/api/v2/generateShareTSS.ts b/modules/express/src/typedRoutes/api/v2/generateShareTSS.ts index 3e35233950..aaeee4cc44 100644 --- a/modules/express/src/typedRoutes/api/v2/generateShareTSS.ts +++ b/modules/express/src/typedRoutes/api/v2/generateShareTSS.ts @@ -135,27 +135,6 @@ const DShare = t.type({ Gamma: t.string, }); -/** - * RShare structure for EDDSA (individual R share) - * Based on: modules/sdk-core/src/account-lib/mpc/tss/eddsa/types.ts:57-65 - */ -const RShareStructure = t.type({ - /** Participant index i */ - i: t.number, - /** Participant index j */ - j: t.number, - /** U value (hex string) */ - u: t.string, - /** V value (hex string) */ - v: t.string, - /** r value (hex string) */ - r: t.string, - /** R value (hex string) */ - R: t.string, - /** Commitment value (hex string) */ - commitment: t.string, -}); - /** * Request body for generating TSS shares in external signer mode * @@ -216,15 +195,6 @@ export const GenerateShareTSSBody = { share: t.string, }) ), - /** User's R share sent to BitGo containing cryptographic commitments for EDDSA G share generation */ - userToBitgoRShare: optional( - t.partial({ - /** Participant index in the signing protocol */ - i: t.number, - /** Mapping of participant indices to their R share structures (commitment, u, v, r, R values) */ - rShares: t.record(t.string, RShareStructure), - }) - ), /** BitGo's commitment share sent to user for EDDSA G share generation */ bitgoToUserCommitment: optional( t.partial({ @@ -324,14 +294,6 @@ const EncryptedSignerShareRecord = t.type({ type: t.string, }); -/** EDDSA signing share containing R share commitments indexed by participant */ -const SignShare = t.type({ - /** Participant index in signing protocol */ - i: t.number, - /** Mapping of participant IDs to R share structures with cryptographic commitments */ - rShares: t.record(t.string, RShareStructure), -}); - /** EDDSA G share containing signature components (participant index, public values, and R point) */ const GShare = t.type({ /** Participant index */ @@ -428,10 +390,15 @@ export const EddsaCommitmentShareResponse = t.type({ encryptedUserToBitgoRShare: EncryptedSignerShareRecord, }); -/** EDDSA R share generation response containing R shares for signature */ +/** EDDSA R share generation response containing the nonce reveal offered to BitGo */ export const EddsaRShareResponse = t.type({ - /** R share containing participant index and share commitments */ - rShare: SignShare, + /** Nonce reveal (r and R only); the full SignShare never leaves the signer */ + rShare: t.type({ + /** Nonce share value */ + r: t.string, + /** Nonce point */ + R: t.string, + }), }); /** EDDSA G share generation response with final signature share components */ diff --git a/modules/express/test/unit/clientRoutes/externalSign.ts b/modules/express/test/unit/clientRoutes/externalSign.ts index fc9e6a1ed3..a5069042de 100644 --- a/modules/express/test/unit/clientRoutes/externalSign.ts +++ b/modules/express/test/unit/clientRoutes/externalSign.ts @@ -3,6 +3,7 @@ */ import { + BitgoMpcGpgPubKeys, common, Ed25519BIP32, Eddsa, @@ -672,7 +673,10 @@ describe('External signer', () => { } }); - it('should read an encrypted prv from signerFileSystemPath and pass it to commitment, R and G share generators', async () => { + // Runs a full commitment -> R -> G external-signing session for an EdDSA MPCv1 key + // through handleV2GenerateShareTSS and returns the requests/results so replay tests + // can re-send them against the real consumed-shares registry. + async function runEddsaMPCv1SigningSession(signableHex = 'testMessage') { const walletID = '62fe536a6b4cf70007acb48c0e7bb0b0'; const user = MPC.keyShare(1, 2, 3); const backup = MPC.keyShare(2, 2, 3); @@ -685,200 +689,232 @@ describe('External signer', () => { const bg = new BitGo({ env: 'test' }); const walletPassphrase = 'testPass'; const validPrv = await bg.encrypt({ input: JSON.stringify(userSigningMaterial), password: walletPassphrase }); - const output: Output = {}; - output[walletID] = validPrv; - const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(JSON.stringify(output)); + const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(JSON.stringify({ [walletID]: validPrv })); const envStub = sinon .stub(process, 'env') .value({ WALLET_62fe536a6b4cf70007acb48c0e7bb0b0_PASSPHRASE: walletPassphrase }); - const tMessage = 'testMessage'; + resetConsumedShareRegistriesForTesting(); + // 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-eddsa-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}` + ); const bgTest = new BitGo({ env: 'test' }); const derivationPath = 'm/0'; + const txRequest = { + apiVersion: 'full', + walletId: walletID, + transactions: [{ unsignedTx: { derivationPath, signableHex } }], + }; + const buildReq = (sharetype: string, body: Record) => + ({ + bitgo: bgTest, + body, + decoded: { coin: 'tsol', sharetype }, + params: { coin: 'tsol', sharetype }, + config: { signerFileSystemPath: signerTmpPath }, + } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>); - const reqCommitment = { - bitgo: bgTest, - body: { - bitgoGpgPubKey: bitgoGpgKey.public, - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, - }, - }, - ], - }, - }, - decoded: { - coin: 'tsol', - sharetype: 'commitment', - bitgoGpgPubKey: bitgoGpgKey.public, - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, - }, - }, - ], - }, - }, - params: { - coin: 'tsol', - sharetype: 'commitment', - }, - config: { - signerFileSystemPath: 'signerFileSystemPath', - }, - } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; - const cResult = await handleV2GenerateShareTSS(reqCommitment); - cResult.should.have.property('userToBitgoCommitment'); - cResult.should.have.property('encryptedSignerShare'); - cResult.should.have.property('encryptedUserToBitgoRShare'); - const encryptedUserToBitgoRShare = cResult.encryptedUserToBitgoRShare; - const reqR = { - bitgo: bgTest, - body: { - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, - }, - }, - ], - }, - encryptedUserToBitgoRShare, - }, - decoded: { - coin: 'tsol', - sharetype: 'R', - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, - }, - }, - ], - }, - encryptedUserToBitgoRShare, - }, - params: { - coin: 'tsol', - sharetype: 'R', - }, - config: { - signerFileSystemPath: 'signerFileSystemPath', - }, - } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; - const rResult = await handleV2GenerateShareTSS(reqR); - rResult.should.have.property('rShare'); - + // Real BitGo co-signer material: a genuine nonce pair for the G round. const signingKey = MPC.keyDerive( userSigningMaterial.uShare, [userSigningMaterial.bitgoYShare, userSigningMaterial.backupYShare], derivationPath ); - const bitgoCombine = MPC.keyCombine(bitgo.uShare, [signingKey.yShares[3], backup.yShares[3]]); - const bitgoSignShare = await MPC.signShare(Buffer.from(tMessage, 'hex'), bitgoCombine.pShare, [ + const bitgoSignShare = await MPC.signShare(Buffer.from(signableHex, 'hex'), bitgoCombine.pShare, [ bitgoCombine.jShares[1], ]); - const signatureShareRec = { + const bitgoToUserRShare = { from: SignatureShareType.BITGO, to: SignatureShareType.USER, share: bitgoSignShare.rShares[1].r + bitgoSignShare.rShares[1].R, }; - const bitgoToUserCommitmentShare = { + const bitgoToUserCommitment = { from: SignatureShareType.BITGO, to: SignatureShareType.USER, share: bitgoSignShare.rShares[1].commitment, type: 'commitment', }; - const reqG = { - bitgo: bgTest, - body: { - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, - }, - }, - ], - }, - userToBitgoRShare: rResult.rShare, - bitgoToUserRShare: signatureShareRec, - bitgoToUserCommitment: bitgoToUserCommitmentShare, - }, - decoded: { - coin: 'tsol', - sharetype: 'G', - txRequest: { - apiVersion: 'full', - walletId: walletID, - transactions: [ - { - unsignedTx: { - derivationPath, - signableHex: tMessage, - }, - }, - ], - }, - userToBitgoRShare: rResult.rShare, - bitgoToUserRShare: signatureShareRec, - bitgoToUserCommitment: bitgoToUserCommitmentShare, - }, - params: { - coin: 'tsol', - sharetype: 'G', - }, - config: { - signerFileSystemPath: 'signerFileSystemPath', - }, - } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; - const userGShare = await handleV2GenerateShareTSS(reqG); - userGShare.should.have.property('i'); - userGShare.should.have.property('y'); - userGShare.should.have.property('gamma'); - userGShare.should.have.property('R'); - const userToBitgoRShare = { - i: ShareKeyPosition.BITGO, - j: ShareKeyPosition.USER, - u: signingKey.yShares[3].u, - v: rResult.rShare.rShares[3].v, - r: rResult.rShare.rShares[3].r, - R: rResult.rShare.rShares[3].R, - commitment: rResult.rShare.rShares[3].commitment, + + const cleanup = () => { + readFileStub.restore(); + envStub.restore(); + fs.promises.unlink(`${signerTmpPath}.consumedShares.json`).catch(() => undefined); }; - const bitgoGShare = MPC.sign( - Buffer.from(tMessage, 'hex'), - bitgoSignShare.xShare, - [userToBitgoRShare], - [backup.yShares[3]] - ); - const signature = MPC.signCombine([userGShare, bitgoGShare]); - const veriResult = MPC.verify(Buffer.from(tMessage, 'hex'), signature); - veriResult.should.be.true(); - readFileStub.restore(); - envStub.restore(); + try { + const commitResult = await handleV2GenerateShareTSS( + buildReq('commitment', { txRequest, bitgoGpgPubKey: BitgoMpcGpgPubKeys.bitgoMpcGpgPubKeys.mpcv1.onprem.test }) + ); + const rResult = await handleV2GenerateShareTSS( + buildReq('R', { txRequest, encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare }) + ); + const buildGRequest = ( + tx: Record = txRequest, + records: Record<'bitgoToUserRShare' | 'bitgoToUserCommitment', unknown> = { + bitgoToUserRShare, + bitgoToUserCommitment, + } + ) => + buildReq('G', { + txRequest: tx, + encryptedUserToBitgoRShare: commitResult.encryptedUserToBitgoRShare, + bitgoToUserRShare: records.bitgoToUserRShare, + bitgoToUserCommitment: records.bitgoToUserCommitment, + }); + const gResult = await handleV2GenerateShareTSS(buildGRequest()); + return { + walletID, + walletPassphrase, + bgTest, + signingKey, + userSigningMaterial, + backup, + bitgoSignShare, + commitResult, + rResult, + gResult, + buildReq, + buildGRequest, + bitgoToUserRShare, + bitgoToUserCommitment, + cleanup, + }; + } catch (e) { + cleanup(); + throw e; + } + } + + it('should read an encrypted prv from signerFileSystemPath and pass it to commitment, R and G share generators', async () => { + const session = await runEddsaMPCv1SigningSession(); + try { + session.commitResult.should.have.property('userToBitgoCommitment'); + session.commitResult.should.have.property('encryptedSignerShare'); + session.commitResult.should.have.property('encryptedUserToBitgoRShare'); + // R round reveals only the nonce material; the full SignShare never leaves the signer + session.rResult.should.have.property('rShare'); + session.rResult.rShare.should.have.property('r').and.be.a.String(); + session.rResult.rShare.should.have.property('R').and.be.a.String(); + should.not.exist(session.rResult.rShare.xShare); + should.not.exist(session.rResult.rShare.rShares); + // G round consumes the signer-held envelope and produces a valid user gamma + session.gResult.should.have.property('i'); + session.gResult.should.have.property('y'); + session.gResult.should.have.property('gamma'); + session.gResult.should.have.property('R'); + + // Complete the ceremony as BitGo and verify the combined signature + const userSignShare = JSON.parse( + await session.bgTest.decrypt({ + input: session.commitResult.encryptedUserToBitgoRShare.share, + password: session.walletPassphrase, + }) + ); + const userToBitgoRShare = { + i: ShareKeyPosition.BITGO, + j: ShareKeyPosition.USER, + u: session.signingKey.yShares[3].u, + v: userSignShare.rShares[3].v, + r: userSignShare.rShares[3].r, + R: userSignShare.rShares[3].R, + commitment: userSignShare.rShares[3].commitment, + }; + const bitgoGShare = MPC.sign( + Buffer.from('testMessage', 'hex'), + session.bitgoSignShare.xShare, + [userToBitgoRShare], + [session.backup.yShares[3]] + ); + const signature = MPC.signCombine([session.gResult, bitgoGShare]); + MPC.verify(Buffer.from('testMessage', 'hex'), signature).should.be.true(); + } finally { + session.cleanup(); + } + }); + + it('should reject a replayed EdDSA MPCv1 R-share envelope', async function () { + const session = await runEddsaMPCv1SigningSession(); + try { + await handleV2GenerateShareTSS(session.buildGRequest()).should.be.rejectedWith( + 'encrypted share has already been consumed; restart the signing session' + ); + } finally { + session.cleanup(); + } + }); + + it('should reject an EdDSA MPCv1 G replay with a different BitGo nonce', async function () { + const session = await runEddsaMPCv1SigningSession(); + try { + await handleV2GenerateShareTSS( + session.buildGRequest(session.buildReq('G', {}).body.txRequest, { + bitgoToUserRShare: session.bitgoToUserRShare, + bitgoToUserCommitment: { + from: SignatureShareType.BITGO, + to: SignatureShareType.USER, + share: '00'.repeat(32), + type: 'commitment', + }, + }) + ).should.be.rejectedWith('encrypted share has already been consumed; restart the signing session'); + } finally { + session.cleanup(); + } + }); + + it('should reject an EdDSA MPCv1 G envelope bound to a different signable', async function () { + const session = await runEddsaMPCv1SigningSession('deadbeef'); + try { + const otherTxRequest = { + apiVersion: 'full', + walletId: session.walletID, + transactions: [{ unsignedTx: { derivationPath: 'm/0', signableHex: 'deadbe00' } }], + }; + await handleV2GenerateShareTSS(session.buildGRequest(otherTxRequest)).should.be.rejectedWith( + 'Adata does not match cyphertext adata' + ); + } finally { + session.cleanup(); + } + }); + + it('should reject a non-BitGo GPG public key for the EdDSA MPCv1 commitment round', async function () { + const walletID = '62fe536a6b4cf70007acb48c0e7bb0b0'; + const user = MPC.keyShare(1, 2, 3); + const backup = MPC.keyShare(2, 2, 3); + const bitgo = MPC.keyShare(3, 2, 3); + const userSigningMaterial = { + uShare: user.uShare, + bitgoYShare: bitgo.yShares[1], + backupYShare: backup.yShares[1], + }; + const bg = new BitGo({ env: 'test' }); + const validPrv = await bg.encrypt({ input: JSON.stringify(userSigningMaterial), password: 'testPass' }); + const readFileStub = sinon.stub(fs.promises, 'readFile').resolves(JSON.stringify({ [walletID]: validPrv })); + const envStub = sinon + .stub(process, 'env') + .value({ WALLET_62fe536a6b4cf70007acb48c0e7bb0b0_PASSPHRASE: 'testPass' }); + try { + const reqCommitment = { + bitgo: new BitGo({ env: 'test' }), + body: { + bitgoGpgPubKey: bitgoGpgKey.public, + txRequest: { + apiVersion: 'full', + walletId: walletID, + transactions: [{ unsignedTx: { derivationPath: 'm/0', signableHex: 'testMessage' } }], + }, + }, + decoded: { coin: 'tsol', sharetype: 'commitment' }, + params: { coin: 'tsol', sharetype: 'commitment' }, + config: { signerFileSystemPath: path.join(os.tmpdir(), `bitgojs-test-signer-eddsa-${Date.now()}`) }, + } as unknown as ExpressApiRouteRequest<'express.v2.tssshare.generate', 'post'>; + await handleV2GenerateShareTSS(reqCommitment).should.be.rejectedWith('Invalid BitGo GPG public key'); + } finally { + readFileStub.restore(); + envStub.restore(); + } }); it('should route EdDSA MPCv2 share types to offline round share generators', async function () { diff --git a/modules/express/test/unit/typedRoutes/generateShareTSS.ts b/modules/express/test/unit/typedRoutes/generateShareTSS.ts index 5c45f50713..239ba75d5c 100644 --- a/modules/express/test/unit/typedRoutes/generateShareTSS.ts +++ b/modules/express/test/unit/typedRoutes/generateShareTSS.ts @@ -215,23 +215,13 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { it('should validate EDDSA R share response', function () { const validResponse = { rShare: { - i: 1, - rShares: { - 2: { - i: 1, - j: 2, - u: 'u-value', - v: 'v-value', - r: 'r-value', - R: 'R-value', - commitment: 'commitment-value', - }, - }, + r: 'r-value', + R: 'R-value', }, }; const decoded = assertDecode(EddsaRShareResponse, validResponse); - assert.strictEqual(decoded.rShare.i, 1); - assert.ok(decoded.rShare.rShares); + assert.strictEqual(decoded.rShare.r, 'r-value'); + assert.strictEqual(decoded.rShare.R, 'R-value'); }); it('should validate EDDSA G share response', function () { @@ -514,18 +504,8 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { const mockRShareResponse = { rShare: { - i: 1, - rShares: { - 2: { - i: 1, - j: 2, - u: 'u-value', - v: 'v-value', - r: 'r-value', - R: 'R-value', - commitment: 'commitment-value', - }, - }, + r: 'r-value', + R: 'R-value', }, }; @@ -559,8 +539,8 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { // Validate response type const decodedResponse = assertDecode(EddsaRShareResponse, result.body); - assert.strictEqual(decodedResponse.rShare.i, 1); - assert.ok(decodedResponse.rShare.rShares); + assert.strictEqual(decodedResponse.rShare.r, 'r-value'); + assert.strictEqual(decodedResponse.rShare.R, 'R-value'); }); it('should successfully generate EDDSA G share via API', async function () { @@ -576,19 +556,18 @@ describe('GenerateShareTSS codec tests (External Signer Mode)', function () { to: 'user', share: 'bitgo-r-share', }, - userToBitgoRShare: { - i: 1, - rShares: { - 2: { - i: 1, - j: 2, - u: 'u-value', - v: 'v-value', - r: 'r-value', - R: 'R-value', - commitment: 'commitment-value', - }, - }, + encryptedUserToBitgoRShare: { + from: 'user', + to: 'bitgo', + // The G round registers the R-share envelope for single-use consumption, + // so the mock must carry a parseable v1 envelope with iv/ct. + share: JSON.stringify({ + v: 1, + iv: Buffer.alloc(16, 3).toString('base64'), + ct: Buffer.alloc(32, 4).toString('base64'), + ts: 64, + }), + type: 'encryptedRShare', }, bitgoToUserCommitment: { from: 'bitgo', From e1439fc5a5ac4f4b6da963f362a720752f8ff271 Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Tue, 10 Nov 2026 21:21:41 +0530 Subject: [PATCH 4/5] fix(express): return 400 for invalid EdDSA MPCv1 signer share requests Share-generation failures on the EdDSA MPCv1 rounds (GPG key pin, envelope binding, expiry, single use, undecryptable envelope) are caller-input failures; surface them as 400 client errors instead of 500. The original error is still logged by the handler. References: WAL-2456 --- modules/express/src/clientRoutes.ts | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index a2a49723cc..ce395ed8ba 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -472,6 +472,19 @@ function markConsumedShareClosure( getConsumedShareRegistry(registryFilePath).markConsumed(computeConsumedShareKey(shareType, adata)); } +/** Share-round failures are caller-input failures; surface them as 400s, + * passing through errors that already carry an HTTP status. */ +async function rejectInvalidShareRequest(operation: () => Promise): Promise { + try { + return await operation(); + } catch (e) { + if (typeof e === 'object' && e !== null && 'status' in e && typeof e.status === 'number') { + throw e; + } + throw new ApiResponseError(e instanceof Error ? e.message : String(e), 400); + } +} + async function decryptPrivKey(bg: BitGo, encryptedPrivKey: string, walletPw: string): Promise { try { return await bg.decrypt({ password: walletPw, input: encryptedPrivKey }); @@ -540,9 +553,9 @@ export async function handleV2GenerateShareTSS( const eddsaUtils = new EddsaUtils(bitgo, coin); switch (req.decoded.sharetype) { case ShareType.Commitment: - return await eddsaUtils.createCommitmentShareFromTxRequest(req.body); + return await rejectInvalidShareRequest(() => eddsaUtils.createCommitmentShareFromTxRequest(req.body)); case ShareType.R: - return await eddsaUtils.createRShareFromTxRequest(req.body); + return await rejectInvalidShareRequest(() => eddsaUtils.createRShareFromTxRequest(req.body)); case ShareType.G: { // Each R-share envelope may be used exactly once; a replayed envelope is // rejected before the share is used (fail-closed). @@ -551,7 +564,7 @@ export async function handleV2GenerateShareTSS( req.decoded.sharetype, req.body.encryptedUserToBitgoRShare?.share ); - return await eddsaUtils.createGShareFromTxRequest(req.body); + return await rejectInvalidShareRequest(() => eddsaUtils.createGShareFromTxRequest(req.body)); } default: throw new Error( From 8dfed06e6d2f70fc08ca4d63d8eab595f26a8bdb Mon Sep 17 00:00:00 2001 From: Marzooqa Naeema Kather Date: Wed, 11 Nov 2026 19:16:29 +0530 Subject: [PATCH 5/5] fix(express): warn when signer mode runs without GPG key pinning In environments that do not enforce the BitGo GPG public key config (mock, custom, local dev), the signer share is encrypted to any caller-supplied GPG public key. Log a warning at startup so operators of unpinned environments know share encryption accepts any key. References: WAL-2456 --- modules/express/src/expressApp.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/modules/express/src/expressApp.ts b/modules/express/src/expressApp.ts index 90e0bce48e..e7e8314afc 100644 --- a/modules/express/src/expressApp.ts +++ b/modules/express/src/expressApp.ts @@ -5,6 +5,7 @@ import express from 'express'; import * as path from 'path'; import * as _ from 'lodash'; import debugLib from 'debug'; +import { logger } from '@bitgo/logger'; import * as https from 'https'; import * as http from 'http'; import morgan, { token as morganToken } from 'morgan'; @@ -27,8 +28,10 @@ import { } from './errors'; import { Environments } from 'bitgo'; -import * as clientRoutes from './clientRoutes'; + import { isSameFilePath } from './consumedSharesRegistry'; +import * as clientRoutes from './clientRoutes'; +import { BitgoMpcGpgPubKeys } from '@bitgo/sdk-core'; import { getBitGoTlsPolicy } from './tlsPolicy'; /** @@ -257,6 +260,13 @@ function checkPreconditions(config: Config) { ); } + if (signerMode && !BitgoMpcGpgPubKeys.envRequiresBitgoPubGpgKeyConfig(env)) { + logger.warn( + `External signer mode is running in env '${env}', which does not enforce the ` + + 'BitGo GPG public key pin; share encryption will accept any supplied GPG public key.' + ); + } + if (signerMode !== undefined && lightningSignerFileSystemPath !== undefined) { throw new LightningSignerConfigError( 'signerMode and lightningSignerFileSystemPath cannot be set at the same time.'