diff --git a/modules/sdk-core/src/account-lib/mpc/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/account-lib/mpc/tss/ecdsa/ecdsa.ts index 3476eb2d9f..74cc5f9854 100644 --- a/modules/sdk-core/src/account-lib/mpc/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/account-lib/mpc/tss/ecdsa/ecdsa.ts @@ -1357,10 +1357,13 @@ export default class Ecdsa { * @param {OShare} oShare private omicron share of current participant * @param {DShare} dShare delta share received from the other participant * @param {Hash} hash hashing algorithm implementing Node`s standard crypto hash interface - * @param shouldHash if true, we hash the provided buffer before signing + * @param shouldHash if true, we hash the provided buffer before signing. If false, M must be exactly 32 bytes (a hash). * @returns {VAShare} */ sign(M: Buffer, oShare: OShare, dShare: DShare, hash?: Hash, shouldHash = true): VAShare { + if (!shouldHash && M.length !== 32) { + throw new Error(`When shouldHash=false, message must be exactly 32 bytes (a hash), but got ${M.length} bytes`); + } const m = shouldHash ? (hash || createHash('sha256')).update(M).digest() : M; const delta = Ecdsa.curve.scalarAdd(hexToBigInt(oShare.delta), hexToBigInt(dShare.delta)); @@ -1560,10 +1563,15 @@ export default class Ecdsa { * @param {Buffer} message * @param {Signature } signature * @param {Hash} hash hashing algorithm implementing Node`s standard crypto hash interface - * @param {boolean} shouldHash if true, we hash the provided buffer before verifying + * @param {boolean} shouldHash if true, we hash the provided buffer before verifying. If false, message must be exactly 32 bytes (a hash). * @returns {boolean} True if signature is valid; False otherwise */ verify(message: Buffer, signature: Signature, hash?: Hash, shouldHash = true): boolean { + if (!shouldHash && message.length !== 32) { + throw new Error( + `When shouldHash=false, message must be exactly 32 bytes (a hash), but got ${message.length} bytes` + ); + } const messageToVerify = shouldHash ? (hash || createHash('sha256')).update(message).digest() : message; return Ecdsa.curve.verify( messageToVerify, diff --git a/modules/sdk-core/test/unit/account-lib/mpc/tss/ecdsa/ecdsa.ts b/modules/sdk-core/test/unit/account-lib/mpc/tss/ecdsa/ecdsa.ts index 6ff3183b4b..964b4a7511 100644 --- a/modules/sdk-core/test/unit/account-lib/mpc/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/test/unit/account-lib/mpc/tss/ecdsa/ecdsa.ts @@ -26,6 +26,32 @@ describe('ecdsa tss', function () { let signCombine1: SignCombineRT, signCombine2: SignCombineRT; + function signAndVerify(message: Buffer, shouldHash = true): boolean { + const [sign1, sign2] = [ + ecdsa.sign(message, signCombine1.oShare, signCombine2.dShare, undefined, shouldHash), + ecdsa.sign(message, signCombine2.oShare, signCombine1.dShare, undefined, shouldHash), + ]; + + const [signWithProofs1, signWithProofs2] = [ + ecdsa.generateVAProofs(message, sign1), + ecdsa.generateVAProofs(message, sign2), + ]; + + const [UT1, UT2] = [ + ecdsa.verifyVAShares(signWithProofs1, [signWithProofs2]), + ecdsa.verifyVAShares(signWithProofs2, [signWithProofs1]), + ]; + + const [publicUTShares_1, publicUTShares_2] = [UT1, UT2]; + const [signature1, signature2] = [ + ecdsa.verifyUTShares(UT1, [publicUTShares_2]), + ecdsa.verifyUTShares(UT2, [publicUTShares_1]), + ]; + + const signature = ecdsa.constructSignature([signature1, signature2]); + return ecdsa.verify(message, signature, undefined, shouldHash); + } + before('generate key and sign phase 1 to 4', async function () { // In some execution environments (e.g. CI), generateNtilde below may take longer at times. this.timeout('40s'); @@ -158,8 +184,6 @@ describe('ecdsa tss', function () { }); it('sign phase 5 should succeed', async function () { - // TODO(HSM-129): There is a bug signing unhashed message (although this deviates from DSA spec) if the message is a little long. - // Some discrepancy between Ecdsa.sign and secp256k1.recoverPublicKey on handling the message input. const message = Buffer.from('GG18 PHASE 5'); // Starting phase 5 @@ -271,4 +295,41 @@ describe('ecdsa tss', function () { (() => ecdsa.verifyUTShares(UT1, [publicUTShares_2])).should.throw('Sum of all U_i does not match sum of all T_i'); (() => ecdsa.verifyUTShares(UT2, [publicUTShares_1])).should.throw('Sum of all U_i does not match sum of all T_i'); }); + + describe('shouldHash parameter', function () { + it('should validate message length when shouldHash=false', function () { + const hash32 = Buffer.alloc(32, 0x01); + const shortMessage = Buffer.from('Short'); + const longMessage = Buffer.from('This is a longer message that exceeds 32 bytes in length'); + + (() => ecdsa.sign(hash32, signCombine1.oShare, signCombine2.dShare, undefined, false)).should.not.throw(); + + (() => ecdsa.sign(shortMessage, signCombine1.oShare, signCombine2.dShare, undefined, false)).should.throw( + /must be exactly 32 bytes/ + ); + (() => ecdsa.sign(longMessage, signCombine1.oShare, signCombine2.dShare, undefined, false)).should.throw( + /must be exactly 32 bytes/ + ); + + const dummySig = { r: '00', s: '00', recid: 0, y: signCombine1.oShare.y }; + (() => ecdsa.verify(shortMessage, dummySig, undefined, false)).should.throw(/must be exactly 32 bytes/); + (() => ecdsa.verify(longMessage, dummySig, undefined, false)).should.throw(/must be exactly 32 bytes/); + }); + + it('should work correctly with shouldHash=false when message is 32 bytes', async function () { + const { createHash } = require('crypto'); + const message = Buffer.from('Any message'); + const messageHash = createHash('sha256').update(message).digest(); + + signAndVerify(messageHash, false).should.equal(true); + }); + + it('should work correctly with shouldHash=true for any message length', async function () { + const shortMessage = Buffer.from('Short message'); + const longMessage = Buffer.from('This is a longer message that exceeds 32 bytes in length'); + + signAndVerify(shortMessage, true).should.equal(true); + signAndVerify(longMessage, true).should.equal(true); + }); + }); });