diff --git a/packages/contracts/source/contracts/crypto/transactions.ts b/packages/contracts/source/contracts/crypto/transactions.ts index 1ac989be1..861e6f850 100644 --- a/packages/contracts/source/contracts/crypto/transactions.ts +++ b/packages/contracts/source/contracts/crypto/transactions.ts @@ -81,7 +81,7 @@ export interface TransactionVerifier { ): Promise>; verifySchemaSigned(data: TransactionSerializable): Promise>; verifySchemaStrict(data: TransactionData): Promise>; - verifyLegacySecondSignature(data: TransactionSerializable, legacySecondPublicKey: string): Promise; + verifyLegacySecondSignature(data: TransactionData, legacySecondPublicKey: string): Promise; } export interface TransactionSigner { @@ -90,11 +90,7 @@ export interface TransactionSigner { keys: KeyPair, options?: SerializeOptions, ): Promise; - legacySecondSign( - transaction: TransactionUnsignedSerializable, - keys: KeyPair, - options?: SerializeOptions, - ): Promise; + legacySecondSign(transaction: TransactionSerializable, keys: KeyPair, options?: SerializeOptions): Promise; } export interface TransactionSerializer { @@ -119,6 +115,7 @@ export interface TransactionFactory { export interface TransactionHashFactory { toHashUnsigned(transaction: TransactionUnsignedSerializable): Promise; toHash(transaction: TransactionSerializable): Promise; + toLegacySecondSignatureHash(transactionHash: Buffer): Buffer; } export type TransactionSchema = Record; diff --git a/packages/crypto-signature-ecdsa/source/signature.test.ts b/packages/crypto-signature-ecdsa/source/signature.test.ts index eafd18a88..d227974ca 100644 --- a/packages/crypto-signature-ecdsa/source/signature.test.ts +++ b/packages/crypto-signature-ecdsa/source/signature.test.ts @@ -35,4 +35,71 @@ describe("Signature", ({ assert, it }) => { assert.false(await new Signature().verifyRecoverable(signature, message, publicKey)); }); + + it("#verifyRecoverable should return false for the high s form of a signature", async () => { + const signature = { + r: "66f1c6d9fe13834f6e348aae40426060339ed8cba7d9b2f105c8220be095877c", + s: "1368fffd8294f1e22086703d33511fc8bb25231d6e9dc64d6449035003184bdd", + v: 1, + }; + + const message = Buffer.from("64726e3da8", "hex"); + const publicKey = Buffer.from("03e84093c072af70004a38dd95e34def119d2348d5261228175d032e5f2070e19f", "hex"); + + assert.true(await new Signature().verifyRecoverable(signature, message, publicKey)); + assert.false( + await new Signature().verifyRecoverable( + { ...signature, s: "ec9700027d6b0e1ddf798fc2ccaee035ff89b9c940aad9ee5b895b3ccd1df564", v: 0 }, + message, + publicKey, + ), + ); + }); + + it("#verifyRecoverable should return false if v is not the recovery id", async () => { + const signature = { + r: "66f1c6d9fe13834f6e348aae40426060339ed8cba7d9b2f105c8220be095877c", + s: "1368fffd8294f1e22086703d33511fc8bb25231d6e9dc64d6449035003184bdd", + v: 1, + }; + + const message = Buffer.from("64726e3da8", "hex"); + const publicKey = Buffer.from("03e84093c072af70004a38dd95e34def119d2348d5261228175d032e5f2070e19f", "hex"); + + assert.true(await new Signature().verifyRecoverable(signature, message, publicKey)); + + for (const v of [0, 2, 3, 27]) { + assert.false(await new Signature().verifyRecoverable({ ...signature, v }, message, publicKey)); + } + }); + + it("#verifyRecoverable should return false if no public key is recovered", async () => { + const signature = { + r: "66f1c6d9fe13834f6e348aae40426060339ed8cba7d9b2f105c8220be095877c", + s: "1368fffd8294f1e22086703d33511fc8bb25231d6e9dc64d6449035003184bdd", + v: 1, + }; + + const message = Buffer.from("64726e3da8", "hex"); + const publicKey = Buffer.from("03e84093c072af70004a38dd95e34def119d2348d5261228175d032e5f2070e19f", "hex"); + + assert.true(await new Signature().verifyRecoverable(signature, message, publicKey)); + assert.false(await new Signature().verifyRecoverable({ ...signature, r: "00".repeat(32) }, message, publicKey)); + }); + + it("#verifyRecoverable should verify an uncompressed public key", async () => { + const signature = { + r: "66f1c6d9fe13834f6e348aae40426060339ed8cba7d9b2f105c8220be095877c", + s: "1368fffd8294f1e22086703d33511fc8bb25231d6e9dc64d6449035003184bdd", + v: 1, + }; + + const message = Buffer.from("64726e3da8", "hex"); + const publicKey = Buffer.from( + "04e84093c072af70004a38dd95e34def119d2348d5261228175d032e5f2070e19f9e02d3df88d4bb625c9a207438037f2ab6a820c143db43baa552bb980dbd8095", + "hex", + ); + + assert.true(await new Signature().verifyRecoverable(signature, message, publicKey)); + }); }); diff --git a/packages/crypto-signature-ecdsa/source/signature.ts b/packages/crypto-signature-ecdsa/source/signature.ts index 1db086cae..be8a818a3 100644 --- a/packages/crypto-signature-ecdsa/source/signature.ts +++ b/packages/crypto-signature-ecdsa/source/signature.ts @@ -3,6 +3,8 @@ import type { Contracts } from "@mainsail/contracts"; import { injectable } from "@mainsail/container"; import { secp256k1 } from "bcrypto"; +const RECOVERY_IDS = new Set([0, 1]); + @injectable() export class Signature implements Contracts.Crypto.SignatureEcdsa { public async signRecoverable(message: Buffer, privateKey: Buffer): Promise { @@ -20,11 +22,18 @@ export class Signature implements Contracts.Crypto.SignatureEcdsa { message: Buffer, publicKey: Buffer, ): Promise { - if (!this.isLowS(signature)) { + if (!this.isLowS(signature) || !RECOVERY_IDS.has(signature.v)) { return false; } - return secp256k1.verify(message, Buffer.from(signature.r + signature.s, "hex"), publicKey); + const recovered = secp256k1.recover( + message, + Buffer.from(signature.r + signature.s, "hex"), + signature.v, + publicKey.length === 33, + ); + + return recovered !== null && recovered.equals(publicKey); } public isLowS(signature: Contracts.Crypto.EcdsaSignature): boolean { diff --git a/packages/crypto-transaction/source/builder.test.ts b/packages/crypto-transaction/source/builder.test.ts index 8bb7d44a1..b91d321a2 100644 --- a/packages/crypto-transaction/source/builder.test.ts +++ b/packages/crypto-transaction/source/builder.test.ts @@ -87,35 +87,35 @@ describe<{ r: "8f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2", s: "273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ce", legacySecondSignature: - "8f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ce01", + "73a17206c6ec232c8041de9241573c4cfe8bd629c32d02618ee1aec550af3b1e66798e7e356e1f11a60552646ffbee9df960ec847cdcc9e3e04d54c0fffdfcf900", }; const serialized = - "f8968085012a05f200830f4240808080824e44a08f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2a0273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ceb8418f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ce01"; + "f8968085012a05f200830f4240808080824e44a08f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2a0273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ceb84173a17206c6ec232c8041de9241573c4cfe8bd629c32d02618ee1aec550af3b1e66798e7e356e1f11a60552646ffbee9df960ec847cdcc9e3e04d54c0fffdfcf900"; let builder = app.resolve(TransactionBuilder); - await builder.legacySecondSign(wallet.passphrase); await builder.sign(wallet.passphrase); + await builder.legacySecondSign(wallet.passphrase); const txSignedWithPassphrase = await builder.build(); assert.equal(txSignedWithPassphrase.toData(), transaction); assert.equal(txSignedWithPassphrase.serialized.toString("hex"), serialized); assert.equal(await builder.getStruct(), transaction); builder = app.resolve(TransactionBuilder); + await builder.sign(wallet.passphrase); await builder.legacySecondSignWithKeyPair({ publicKey: wallet.publicKey, privateKey: wallet.privateKey, compressed: false, }); - await builder.sign(wallet.passphrase); const txSignedWithKeyPair = await builder.build(); assert.equal(txSignedWithKeyPair.toData(), transaction); assert.equal(txSignedWithKeyPair.serialized.toString("hex"), serialized); assert.equal(await builder.getStruct(), transaction); builder = app.resolve(TransactionBuilder); - await builder.legacySecondSignWithWif(wallet.WIF); await builder.sign(wallet.passphrase); + await builder.legacySecondSignWithWif(wallet.WIF); const txSignedWithWif = await builder.build(); assert.equal(txSignedWithWif.toData(), transaction); assert.equal(txSignedWithWif.serialized.toString("hex"), serialized); @@ -132,6 +132,10 @@ describe<{ await assert.rejects(() => builder.nonce("-1").legacySecondSign(wallet.passphrase), ValidationFailed); }); + it("#legacySecondSign - should throw if not signed", async ({ builder }) => { + await assert.rejects(() => builder.legacySecondSign(wallet.passphrase), MissingTransactionSignatureError); + }); + it("#getStruct - should throw on missing data", async ({ app }) => { let builder = app.resolve(TransactionBuilder); await assert.rejects(() => builder.getStruct(), MissingTransactionSignatureError); diff --git a/packages/crypto-transaction/source/builder.ts b/packages/crypto-transaction/source/builder.ts index cef4091db..86630dd45 100644 --- a/packages/crypto-transaction/source/builder.ts +++ b/packages/crypto-transaction/source/builder.ts @@ -185,7 +185,11 @@ export class TransactionBuilder { throw new ValidationFailed(error); } - const signature = await this.signer.legacySecondSign(data, keys); + if (!this.data.r || !this.data.s) { + throw new MissingTransactionSignatureError(); + } + + const signature = await this.signer.legacySecondSign(this.data, keys); this.data.legacySecondSignature = signature; diff --git a/packages/crypto-transaction/source/hash.factory.test.ts b/packages/crypto-transaction/source/hash.factory.test.ts index 959321038..6247a947e 100644 --- a/packages/crypto-transaction/source/hash.factory.test.ts +++ b/packages/crypto-transaction/source/hash.factory.test.ts @@ -33,6 +33,14 @@ describe<{ } }); + it("#toLegacySecondSignatureHash - should be ok", ({ hasher }) => { + const hash = hasher.toLegacySecondSignatureHash( + Buffer.from("64ff1ac71a06deb6a2d544786f8751b534c1c38284ccfa5faf2565d19b9f5151", "hex"), + ); + + assert.equal(hash.toString("hex"), "b5c9157468c8204f03389b61407f3caa91b76b28bcc158c9ab347984feb4c331"); + }); + it("#toHashUnsigned - should be ok", async ({ hasher }) => { for (const [tx, expectedHash] of [ [Deserialized.transactionContractCall, "a037cecd15ad24cbccd89b7610f6d7e80482b935798230d3b22c897c91bddc51"], diff --git a/packages/crypto-transaction/source/hash.factory.ts b/packages/crypto-transaction/source/hash.factory.ts index 97caf7bd6..f8babec6c 100644 --- a/packages/crypto-transaction/source/hash.factory.ts +++ b/packages/crypto-transaction/source/hash.factory.ts @@ -3,6 +3,9 @@ import type { Contracts } from "@mainsail/contracts"; import { Identifiers } from "@mainsail/constants"; import { inject, injectable } from "@mainsail/container"; +// 0x8f cannot start a transaction or signed-message preimage (0x00-0x7f, 0xc0-0xff) +const LEGACY_SECOND_SIGNATURE_DOMAIN = Buffer.concat([Buffer.from([0x8f]), Buffer.from("MAINSAIL_LSS_V1")]); + @injectable() export class HashFactory implements Contracts.Crypto.TransactionHashFactory { @inject(Identifiers.Cryptography.Transaction.Serializer) @@ -14,7 +17,7 @@ export class HashFactory implements Contracts.Crypto.TransactionHashFactory { public async toHash(transaction: Contracts.Crypto.TransactionSerializable): Promise { const serialized = await this.serializer.serialize({ ...transaction, - legacySecondSignature: undefined, // TODO: Decide if legacySecondSignature should be part of the hash or not. For now, we exclude it to maintain compatibility with existing hashes. + legacySecondSignature: undefined, // excluded, the legacy second signature signs this hash }); return this.hashFactory.keccak256(serialized); } @@ -23,4 +26,8 @@ export class HashFactory implements Contracts.Crypto.TransactionHashFactory { const serialized = await this.serializer.serializeUnsigned(transaction); return this.hashFactory.keccak256(serialized); } + + public toLegacySecondSignatureHash(transactionHash: Buffer): Buffer { + return this.hashFactory.keccak256([LEGACY_SECOND_SIGNATURE_DOMAIN, transactionHash]); + } } diff --git a/packages/crypto-transaction/source/signer.test.ts b/packages/crypto-transaction/source/signer.test.ts index 4d2274de0..5e7f099b7 100644 --- a/packages/crypto-transaction/source/signer.test.ts +++ b/packages/crypto-transaction/source/signer.test.ts @@ -4,11 +4,13 @@ import { Identifiers } from "@mainsail/constants"; import { TransactionBuilder } from "../source/builder.js"; import { Application } from "@mainsail/kernel"; import { describe } from "@mainsail/test-runner"; +import { wallet } from "../test/fixtures/index"; import { prepareSandbox } from "../test/helpers/prepare-sandbox"; describe<{ app: Application; signer: Contracts.Crypto.TransactionSigner; + factory: Contracts.Crypto.TransactionFactory; keyPair: Contracts.Crypto.KeyPair; transaction: Contracts.Crypto.Transaction; }>("Signer", ({ it, beforeEach, assert }) => { @@ -18,6 +20,9 @@ describe<{ context.signer = context.app.get( Identifiers.Cryptography.Transaction.Signer, ); + context.factory = context.app.get( + Identifiers.Cryptography.Transaction.Factory, + ); context.keyPair = await context.app .getTagged( @@ -53,7 +58,28 @@ describe<{ assert.equal( signature, - "295ffb1befa5259bba46d532affa13f52f1e50f9418a2579982b121b4ef3553a1fe13d077cbcd6f2293d41c66eb5e5dee4e2bf3b8f8eb3e0556304befbbb69bb01", + "963e726eb3bc5f68a1bbf1c25c4324a3ff93c7853e70fc4a890665982c05ae877270a93df25d9ffdb1df2f3bd5a8816287bb9a10f85428970ac25f9ce6dacf4f00", ); }); + + it("should sign legacy signature that is not a primary signature of the second key", async ({ + factory, + signer, + transaction, + }) => { + const legacySecondSignature = await signer.legacySecondSign(transaction, { + compressed: false, + privateKey: wallet.privateKey, + publicKey: wallet.publicKey, + }); + + const replayed = await factory.fromData({ + ...transaction.toData(), + r: legacySecondSignature.slice(0, 64), + s: legacySecondSignature.slice(64, 128), + v: Number.parseInt(legacySecondSignature.slice(128), 16), + }); + + assert.not.equal(replayed.senderPublicKey, wallet.publicKey); + }); }); diff --git a/packages/crypto-transaction/source/signer.ts b/packages/crypto-transaction/source/signer.ts index 26926d45a..2908612c5 100644 --- a/packages/crypto-transaction/source/signer.ts +++ b/packages/crypto-transaction/source/signer.ts @@ -22,10 +22,10 @@ export class Signer implements Contracts.Crypto.TransactionSigner { } public async legacySecondSign( - transaction: Contracts.Crypto.TransactionUnsignedSerializable, + transaction: Contracts.Crypto.TransactionSerializable, keys: Contracts.Crypto.KeyPair, ): Promise { - const hash: Buffer = await this.hashFactory.toHashUnsigned(transaction); + const hash: Buffer = this.hashFactory.toLegacySecondSignatureHash(await this.hashFactory.toHash(transaction)); const { r, s, v } = await this.signatureFactory.signRecoverable(hash, Buffer.from(keys.privateKey, "hex")); return formatEcdsaSignature(r, s, v); diff --git a/packages/crypto-transaction/source/verifier.test.ts b/packages/crypto-transaction/source/verifier.test.ts index cbdd546f6..79845e7fd 100644 --- a/packages/crypto-transaction/source/verifier.test.ts +++ b/packages/crypto-transaction/source/verifier.test.ts @@ -2,7 +2,8 @@ import type { Contracts } from "@mainsail/contracts"; import { Identifiers } from "@mainsail/constants"; import { Application } from "@mainsail/kernel"; import { describe } from "@mainsail/test-runner"; -import { Serialized } from "../test/fixtures/index"; +import { formatEcdsaSignature } from "@mainsail/utils"; +import { Serialized, wallet } from "../test/fixtures/index"; import { prepareSandbox } from "../test/helpers/prepare-sandbox"; import { InvalidLegacySecondSignatureError, MissingLegacySecondSignatureError } from "@mainsail/exceptions"; @@ -10,7 +11,11 @@ describe<{ app: Application; verifier: Contracts.Crypto.TransactionVerifier; factory: Contracts.Crypto.TransactionFactory; + signer: Contracts.Crypto.TransactionSigner; + keyPairFactory: Contracts.Crypto.KeyPairFactory; }>("Verifier", ({ it, beforeEach, assert }) => { + const secondKeyPair = { compressed: false, privateKey: wallet.privateKey, publicKey: wallet.publicKey }; + const txData = { hash: "3a5823fe8f498b2e509974b3939584bd1200ad32fa32bc8a1a778b608f79f780", network: 10000, @@ -37,6 +42,14 @@ describe<{ context.verifier = context.app.get( Identifiers.Cryptography.Transaction.Verifier, ); + context.signer = context.app.get( + Identifiers.Cryptography.Transaction.Signer, + ); + context.keyPairFactory = context.app.getTagged( + Identifiers.Cryptography.Identity.KeyPair.Factory, + "type", + "wallet", + ); }); it("verifySchema - should be ok", async ({ factory, verifier }) => { @@ -107,4 +120,66 @@ describe<{ MissingLegacySecondSignatureError, ); }); + + it("verifyLegacySecondSignature - should throw if the recovery id is changed", async ({ factory, verifier }) => { + const serialized = Serialized.transactionTransferWithSecondSignature; + + assert.true( + await verifier.verifyLegacySecondSignature((await factory.fromHex(serialized)).toData(), wallet.publicKey), + ); + + for (const recoveryId of ["01", "02", "1b", "ff"]) { + const transaction = await factory.fromHex(serialized.slice(0, -2) + recoveryId); + + await assert.rejects( + () => verifier.verifyLegacySecondSignature(transaction.toData(), wallet.publicKey), + InvalidLegacySecondSignatureError, + ); + } + }); + + it("verifyLegacySecondSignature - should throw if signed over the unsigned transaction", async ({ + factory, + signer, + verifier, + }) => { + const transaction = (await factory.fromHex(Serialized.transactionTransfer)).toData(); + const { r, s, v } = await signer.sign(transaction, secondKeyPair); + + const signed = await factory.fromData({ + ...transaction, + legacySecondSignature: await signer.legacySecondSign(transaction, secondKeyPair), + }); + const lifted = await factory.fromData({ ...transaction, legacySecondSignature: formatEcdsaSignature(r, s, v) }); + + assert.true(await verifier.verifyLegacySecondSignature(signed.toData(), wallet.publicKey)); + await assert.rejects( + () => verifier.verifyLegacySecondSignature(lifted.toData(), wallet.publicKey), + InvalidLegacySecondSignatureError, + ); + }); + + it("verifyLegacySecondSignature - should throw if lifted to another sender", async ({ + factory, + keyPairFactory, + signer, + verifier, + }) => { + const transaction = (await factory.fromHex(Serialized.transactionTransfer)).toData(); + const legacySecondSignature = await signer.legacySecondSign(transaction, secondKeyPair); + const otherSender = await keyPairFactory.fromMnemonic("secret"); + + const signed = await factory.fromData({ ...transaction, legacySecondSignature }); + const lifted = await factory.fromData({ + ...transaction, + ...(await signer.sign(transaction, otherSender)), + legacySecondSignature, + }); + + assert.true(await verifier.verifyLegacySecondSignature(signed.toData(), wallet.publicKey)); + await assert.rejects( + () => verifier.verifyLegacySecondSignature(lifted.toData(), wallet.publicKey), + InvalidLegacySecondSignatureError, + ); + }); }); diff --git a/packages/crypto-transaction/source/verifier.ts b/packages/crypto-transaction/source/verifier.ts index bfdd0689a..376d7fff0 100644 --- a/packages/crypto-transaction/source/verifier.ts +++ b/packages/crypto-transaction/source/verifier.ts @@ -35,7 +35,7 @@ export class Verifier implements Contracts.Crypto.TransactionVerifier { } public async verifyLegacySecondSignature( - data: Contracts.Crypto.TransactionSerializable, + data: Contracts.Crypto.TransactionData, legacySecondPublicKey: string, ): Promise { const { legacySecondSignature } = data; @@ -48,7 +48,7 @@ export class Verifier implements Contracts.Crypto.TransactionVerifier { const s = legacySecondSignature.slice(64, 128); const v = Number.parseInt(legacySecondSignature.slice(128, 130), 16); - const hash: Buffer = await this.hashFactory.toHashUnsigned(data); + const hash: Buffer = this.hashFactory.toLegacySecondSignatureHash(Buffer.from(data.hash, "hex")); const verified = await this.signatureFactory.verifyRecoverable( { r, s, v }, diff --git a/packages/crypto-transaction/test/fixtures/serialized.ts b/packages/crypto-transaction/test/fixtures/serialized.ts index d3cfdffb0..47256f65c 100644 --- a/packages/crypto-transaction/test/fixtures/serialized.ts +++ b/packages/crypto-transaction/test/fixtures/serialized.ts @@ -2,7 +2,7 @@ export const transactionTransfer = "f8678085012a05f200830f424094be89811e15f611c1db12e59679b6f3dc1f4301550180824e43a0921101a4583fb153ec00e501f3c2e2636114e1c8c58d2df8a19426cc066a6768a022db4bce1e0ace485ce0838d178b4d5bcfa9f69b315a14c580d9b01e5c980bdd"; export const transactionTransferWithSecondSignature = - "f8968085012a05f200830f4240808080824e44a08f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2a0273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ceb8418f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ce01"; + "f8968085012a05f200830f4240808080824e44a08f0145edea568df2dd39db91be0bff4ebf5b1e54cae49bf2090bf84fa0dd45a2a0273f828aaa99a54e31f8f3e316acc573c6f2490fb903052accddb979647fa5ceb84173a17206c6ec232c8041de9241573c4cfe8bd629c32d02618ee1aec550af3b1e66798e7e356e1f11a60552646ffbee9df960ec847cdcc9e3e04d54c0fffdfcf900"; export const transactionContractCall = "f88b0285012a05f200830f424094535b3d7a252fa034ed71f0c53ec0c6f784cb64e180a46dd7d8ea0000000000000000000000008233f6df6449d7655f4643d2e752dc8d2283fad5824e43a0b75a1acce8cb056addf031c1a5f5de1012a091b93b0889dba35e2cb6e38d5ec1a07971d23217d5df660070737d345758b06267379f962470125c5c2ac3ff393c23";