From afd67a406ba8c0db00415d5d72e8bedfc8996901 Mon Sep 17 00:00:00 2001 From: feri42 Date: Wed, 28 Feb 2024 19:05:42 +0100 Subject: [PATCH 1/7] feat: pass options to secp256k1 signBuffer --- features/keychain/module/crypto/secp256k1.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/features/keychain/module/crypto/secp256k1.js b/features/keychain/module/crypto/secp256k1.js index 1f908f21..56f7c990 100644 --- a/features/keychain/module/crypto/secp256k1.js +++ b/features/keychain/module/crypto/secp256k1.js @@ -6,9 +6,10 @@ export const create = ({ getPrivateHDKey }) => { const curve = new EC('secp256k1') const createInstance = () => ({ - signBuffer: async ({ seedId, keyId, data }) => { + signBuffer: async ({ seedId, keyId, data, options }) => { const { privateKey } = getPrivateHDKey({ seedId, keyId }) - return Buffer.from(curve.sign(data, privateKey).toDER()) + const signature = curve.sign(data, privateKey, options) + return options?.rawSignature ? signature : Buffer.from(signature.toDER()) }, }) From f56bba53fc444b773d81ff9228b390e1f07d601a Mon Sep 17 00:00:00 2001 From: feri42 Date: Wed, 28 Feb 2024 19:13:51 +0100 Subject: [PATCH 2/7] refactor: add test --- .../keychain/module/__tests__/ethsign.test.js | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 features/keychain/module/__tests__/ethsign.test.js diff --git a/features/keychain/module/__tests__/ethsign.test.js b/features/keychain/module/__tests__/ethsign.test.js new file mode 100644 index 00000000..9251b07e --- /dev/null +++ b/features/keychain/module/__tests__/ethsign.test.js @@ -0,0 +1,72 @@ +import { create } from '../crypto/secp256k1' +import Signature from '@exodus/elliptic/lib/elliptic/ec/signature' + +const fixtures = [ + { + buffer: '45819f5903df1754ac9e570f0d5dfecaf9c204bcd52eb23c8850d337a73bf115', + pk: '164122e5d39e9814ca723a749253663bafb07f6af91704d9754c361eb315f0c1', + signature: + 'c71d5e344d0ea6f2a453b10fdd390cfb8ad3ba79147b1d2003946ac315dda26d65183e9008cb732e340e21c7f7fb0c5621ff26bedc584d650894bef2cd555306', + recid: 1, + }, + { + buffer: 'adfbf5a2accc98c5046e8bf42e2e315bc2f81a37cd930674f8ae8bd9345ff93a', + pk: '4646464646464646464646464646464646464646464646464646464646464646', + signature: + 'a9b97cb071ceae096999de5184000c5db2ce394dae436f8bd7fdab2c3f5437b7188f6bdf224529a21b6b915a51002f645196a228b5ec80c237186c0154bf52c5', + recid: 1, + }, + { + buffer: 'c68ddb8e0ffdf28caabdf1f4e6b85476515442fa4b96bbf41ae8ef5da28d76ef', + pk: 'e0a462586887362a18a318b128dbc1e3a0cae6d4b0739f5d0419ec25114bc722', + signature: + '5288e2aeac147ab07a3748d4989ccafc038b4287310a16ae1c65a5bc6e94df463e57d447ebedbe7d2c04be7a68f1fd2ef56b1f18c3c277933e33e8d595bfa736', + recid: 0, + }, + { + buffer: 'aad787b6c7cfb13feab05f6175089c95f0b54839365fab43c7c4245bd32b3d65', + pk: '164122e5d39e9814ca723a749253663bafb07f6af91704d9754c361eb315f0c1', + signature: + '055ba80267103f44de6acd0d2dc69b805c02358bbc7c024b2b9961a3884ac5b473327be0ca0ca0e2ce5775cda73c3a985810869703cbea708c6e8c15b7cddf1d', + recid: 1, + }, + { + buffer: 'f999cebf59f8139dfc6f29538bdb53c82523bb973c529c8ec8f05217f710ff33', + pk: 'e0a462586887362a18a318b128dbc1e3a0cae6d4b0739f5d0419ec25114bc722', + signature: + '57bce6dcfe018f05b4528b62f9f00a6b21060f82cdd29a3db946937b82f933817211ea21c740fa08c313d7c6c074c31fbc5c2d299f39e6f3c9fd8032a650599d', + recid: 1, + }, + { + buffer: 'a3491b82987a48227a915318085d93d709d8650530fa017153f6366d884799d8', + pk: 'e0a462586887362a18a318b128dbc1e3a0cae6d4b0739f5d0419ec25114bc722', + signature: + '85895e1fa45198b1c10e3e6982a5d5cfcf29ac95b6a3386ee42be1af72c2f054362876f8aec05f0bd18fcaffda1a6019198e79da256d848194cedba27b568024', + recid: 1, + }, +] + +describe('ETH Signer', () => { + it('should signBuffer', async () => { + for (const fixture of fixtures) { + const getPrivateHDKey = () => ({ privateKey: Buffer.from(fixture.pk, 'hex') }) + const secp256k1Signer = create({ getPrivateHDKey }) + + const ethSigner = async (buffer) => { + const sig = await secp256k1Signer.signBuffer({ + data: buffer, + options: { canonical: true, rawSignature: true }, + }) + const signature = new Uint8Array(64) + signature.set(sig.r.toArrayLike(Uint8Array, 'be', 32), 0) + signature.set(sig.s.toArrayLike(Uint8Array, 'be', 32), 32) + return { signature, recid: sig.recoveryParam } + } + + const { signature, recid } = await ethSigner(fixture.buffer) + + expect(recid).toBe(fixture.recid) + expect(Buffer.from(signature).toString('hex')).toBe(fixture.signature) + } + }) +}) From a1d4f653dfd71445275e049f2324dd7ca058b57f Mon Sep 17 00:00:00 2001 From: feri42 Date: Wed, 28 Feb 2024 19:17:07 +0100 Subject: [PATCH 3/7] refactor: remove import --- features/keychain/module/__tests__/ethsign.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/features/keychain/module/__tests__/ethsign.test.js b/features/keychain/module/__tests__/ethsign.test.js index 9251b07e..095c8844 100644 --- a/features/keychain/module/__tests__/ethsign.test.js +++ b/features/keychain/module/__tests__/ethsign.test.js @@ -1,5 +1,4 @@ import { create } from '../crypto/secp256k1' -import Signature from '@exodus/elliptic/lib/elliptic/ec/signature' const fixtures = [ { From 7fb32bf7971dd0191014fd4e655ae80c0e9a6034 Mon Sep 17 00:00:00 2001 From: feri42 Date: Wed, 28 Feb 2024 19:19:49 +0100 Subject: [PATCH 4/7] refactor: do not mix options --- features/keychain/module/__tests__/ethsign.test.js | 3 ++- features/keychain/module/crypto/secp256k1.js | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/features/keychain/module/__tests__/ethsign.test.js b/features/keychain/module/__tests__/ethsign.test.js index 095c8844..cf2b3672 100644 --- a/features/keychain/module/__tests__/ethsign.test.js +++ b/features/keychain/module/__tests__/ethsign.test.js @@ -54,7 +54,8 @@ describe('ETH Signer', () => { const ethSigner = async (buffer) => { const sig = await secp256k1Signer.signBuffer({ data: buffer, - options: { canonical: true, rawSignature: true }, + ecOptions: { canonical: true }, + rawSignature: true, }) const signature = new Uint8Array(64) signature.set(sig.r.toArrayLike(Uint8Array, 'be', 32), 0) diff --git a/features/keychain/module/crypto/secp256k1.js b/features/keychain/module/crypto/secp256k1.js index 56f7c990..6b0ffe10 100644 --- a/features/keychain/module/crypto/secp256k1.js +++ b/features/keychain/module/crypto/secp256k1.js @@ -6,10 +6,10 @@ export const create = ({ getPrivateHDKey }) => { const curve = new EC('secp256k1') const createInstance = () => ({ - signBuffer: async ({ seedId, keyId, data, options }) => { + signBuffer: async ({ seedId, keyId, data, ecOptions, rawSignature }) => { const { privateKey } = getPrivateHDKey({ seedId, keyId }) - const signature = curve.sign(data, privateKey, options) - return options?.rawSignature ? signature : Buffer.from(signature.toDER()) + const signature = curve.sign(data, privateKey, ecOptions) + return rawSignature ? signature : Buffer.from(signature.toDER()) }, }) From be1cc8006200ff0eb564c879152ba5c3fc7606c0 Mon Sep 17 00:00:00 2001 From: feri42 Date: Wed, 28 Feb 2024 19:30:34 +0100 Subject: [PATCH 5/7] refactor: do not return internal Signature object --- features/keychain/module/crypto/secp256k1.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/keychain/module/crypto/secp256k1.js b/features/keychain/module/crypto/secp256k1.js index 6b0ffe10..2598e4c4 100644 --- a/features/keychain/module/crypto/secp256k1.js +++ b/features/keychain/module/crypto/secp256k1.js @@ -9,7 +9,7 @@ export const create = ({ getPrivateHDKey }) => { signBuffer: async ({ seedId, keyId, data, ecOptions, rawSignature }) => { const { privateKey } = getPrivateHDKey({ seedId, keyId }) const signature = curve.sign(data, privateKey, ecOptions) - return rawSignature ? signature : Buffer.from(signature.toDER()) + return rawSignature ? { ...signature } : Buffer.from(signature.toDER()) }, }) From d7cae80720225a6dc05645843dbfbbaf2ccceed2 Mon Sep 17 00:00:00 2001 From: feri42 Date: Thu, 29 Feb 2024 08:40:30 +0100 Subject: [PATCH 6/7] refactor: rename arg to `enc` --- features/keychain/module/__tests__/ethsign.test.js | 2 +- features/keychain/module/crypto/secp256k1.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/features/keychain/module/__tests__/ethsign.test.js b/features/keychain/module/__tests__/ethsign.test.js index cf2b3672..a4420fae 100644 --- a/features/keychain/module/__tests__/ethsign.test.js +++ b/features/keychain/module/__tests__/ethsign.test.js @@ -55,7 +55,7 @@ describe('ETH Signer', () => { const sig = await secp256k1Signer.signBuffer({ data: buffer, ecOptions: { canonical: true }, - rawSignature: true, + enc: 'raw', }) const signature = new Uint8Array(64) signature.set(sig.r.toArrayLike(Uint8Array, 'be', 32), 0) diff --git a/features/keychain/module/crypto/secp256k1.js b/features/keychain/module/crypto/secp256k1.js index 2598e4c4..82971872 100644 --- a/features/keychain/module/crypto/secp256k1.js +++ b/features/keychain/module/crypto/secp256k1.js @@ -1,3 +1,4 @@ +import assert from 'minimalistic-assert' import elliptic from '@exodus/elliptic' import { mapValues } from '@exodus/basic-utils' @@ -6,10 +7,11 @@ export const create = ({ getPrivateHDKey }) => { const curve = new EC('secp256k1') const createInstance = () => ({ - signBuffer: async ({ seedId, keyId, data, ecOptions, rawSignature }) => { + signBuffer: async ({ seedId, keyId, data, ecOptions, enc = 'der' }) => { + assert(['der', 'raw'].includes(enc), 'signBuffer: invalid enc') const { privateKey } = getPrivateHDKey({ seedId, keyId }) const signature = curve.sign(data, privateKey, ecOptions) - return rawSignature ? { ...signature } : Buffer.from(signature.toDER()) + return enc === 'der' ? Buffer.from(signature.toDER()) : { ...signature } }, }) From b7bb5889f77a95a092a98d90a2761b36f73d200a Mon Sep 17 00:00:00 2001 From: feri42 Date: Thu, 29 Feb 2024 11:29:26 +0100 Subject: [PATCH 7/7] refactor: only allow `canonical` --- features/keychain/module/crypto/secp256k1.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/features/keychain/module/crypto/secp256k1.js b/features/keychain/module/crypto/secp256k1.js index 82971872..c49b7a0e 100644 --- a/features/keychain/module/crypto/secp256k1.js +++ b/features/keychain/module/crypto/secp256k1.js @@ -1,6 +1,9 @@ import assert from 'minimalistic-assert' import elliptic from '@exodus/elliptic' -import { mapValues } from '@exodus/basic-utils' +import { mapValues, pick } from '@exodus/basic-utils' + +const validEcOptions = (ecOptions) => + !ecOptions || Object.keys(ecOptions).every((key) => ['canonical'].includes(key)) export const create = ({ getPrivateHDKey }) => { const EC = elliptic.ec @@ -9,8 +12,9 @@ export const create = ({ getPrivateHDKey }) => { const createInstance = () => ({ signBuffer: async ({ seedId, keyId, data, ecOptions, enc = 'der' }) => { assert(['der', 'raw'].includes(enc), 'signBuffer: invalid enc') + assert(validEcOptions(ecOptions), 'signBuffer: invalid EC option') const { privateKey } = getPrivateHDKey({ seedId, keyId }) - const signature = curve.sign(data, privateKey, ecOptions) + const signature = curve.sign(data, privateKey, pick(ecOptions, ['canonical'])) return enc === 'der' ? Buffer.from(signature.toDER()) : { ...signature } }, })