From 170e9992e959d1ade81d28ffe5866a689d2b2263 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 9 Aug 2023 14:32:31 +0200 Subject: [PATCH 01/14] Check the secret key Ensure that the secret key generated using the `SecretKey.fromBytes` function is non-zero modulo the order of the elliptic curve. --- elements/lisk-cryptography/src/bls_lib/lib.ts | 42 ++++++++++++++-- .../test/bls_lib/lib.spec.ts | 49 +++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/elements/lisk-cryptography/src/bls_lib/lib.ts b/elements/lisk-cryptography/src/bls_lib/lib.ts index d8abf596057..90c08e2b1db 100644 --- a/elements/lisk-cryptography/src/bls_lib/lib.ts +++ b/elements/lisk-cryptography/src/bls_lib/lib.ts @@ -45,9 +45,31 @@ export const blsKeyValidate = (pk: Buffer): boolean => { // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.3 export const blsKeyGen = (ikm: Buffer): Buffer => Buffer.from(SecretKey.fromKeygen(ikm).toBytes()); +// check that the secret key generated is non-zero modulo the order of the elliptic curve. +export const isSecretKeyNonZeroModEC = (secretKey: SecretKey): boolean => { + // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-pairing-friendly-curves-10#section-4.2.1 + const curveOrder = '0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001'; + + const skBigInt = BigInt(`0x${Buffer.from(secretKey.toBytes()).toString('hex')}`); + + // check if secret key is non-zero modulo the order of the elliptic curve. + if (skBigInt % BigInt(curveOrder) === BigInt(0)) { + throw new Error('Secret key is not valid.'); + } + + return true; +}; + // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.4 -export const blsSkToPk = (sk: Buffer): Buffer => - Buffer.from(SecretKey.fromBytes(sk).toPublicKey().toBytes()); +export const blsSkToPk = (sk: Buffer): Buffer => { + const secretKey = SecretKey.fromBytes(sk); + + if (!isSecretKeyNonZeroModEC(secretKey)) { + throw new Error('Secret key is not valid.'); + } + + return Buffer.from(secretKey.toPublicKey().toBytes()); +}; // https://tools.ietf.org/html/draft-irtf-cfrg-bls-signature-04#section-2.8 export const blsAggregate = (signatures: Buffer[]): Buffer | false => { @@ -66,7 +88,13 @@ export const blsSign = (sk: Buffer, message: Buffer): Buffer => { return Buffer.concat([Buffer.from([192]), Buffer.alloc(95)]); } - return Buffer.from(SecretKey.fromBytes(sk).sign(message).toBytes()); + const secretKey = SecretKey.fromBytes(sk); + + if (!isSecretKeyNonZeroModEC(secretKey)) { + throw new Error('Secret key is not valid.'); + } + + return Buffer.from(secretKey.sign(message).toBytes()); }; // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.7 @@ -123,8 +151,14 @@ export const blsPopProve = (sk: Buffer): Buffer => { const message = blsSkToPk(sk); const sig = new blst.P2(); + const secretKey = SecretKey.fromBytes(sk); + + if (!isSecretKeyNonZeroModEC(secretKey)) { + throw new Error('Secret key is not valid.'); + } + return Buffer.from( - new Signature(sig.hash_to(message, DST_POP).sign_with(SecretKey.fromBytes(sk).value)).toBytes(), + new Signature(sig.hash_to(message, DST_POP).sign_with(secretKey.value)).toBytes(), ); }; diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 70bea252e6e..05dda6fe444 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -13,6 +13,8 @@ * */ +import { ErrorBLST, SecretKey } from '@chainsafe/blst'; +import { isSecretKeyNonZeroModEC } from '../../src/bls_lib/lib'; import { blsAggregate, blsAggregateVerify, @@ -58,6 +60,29 @@ interface EthFastAggrVerifySpec { } describe('bls_lib', () => { + const curveOrder = '0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001'; + + describe('isSecretKeyNonZeroModEC', () => { + it('should return true when given a valid secret key', () => { + const secretKey = SecretKey.fromBytes(Buffer.alloc(32, 1)); + expect(isSecretKeyNonZeroModEC(secretKey)).toBe(true); + }); + + it('should return true when given a non-zero modulo secret key', () => { + const secretKey = SecretKey.fromBytes( + Buffer.from('0000000000000000000000000000000000000000000000000000000000000001', 'hex'), + ); + expect(isSecretKeyNonZeroModEC(secretKey)).toBe(true); + }); + + it('should throw an error when given a zero modulo secret key', () => { + const secretKey = SecretKey.fromBytes( + Buffer.from('73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001', 'hex'), + ); + expect(() => isSecretKeyNonZeroModEC(secretKey)).toThrow('Secret key is not valid.'); + }); + }); + describe('blsSkToPk', () => { describe.each(getAllFiles(['bls_specs/sk_to_pk']))('%s', ({ path }) => { it('should convert to valid pk', () => { @@ -68,6 +93,30 @@ describe('bls_lib', () => { ); }); }); + + it('should return a non-empty buffer when given a valid input buffer', () => { + const sk = Buffer.alloc(32, 1); + const pk = blsSkToPk(sk); + + expect(pk).toBeDefined(); + expect(pk.length).toBeGreaterThan(0); + }); + + it('should throw an error when given an input buffer with value equal to the curve order', () => { + const sk = Buffer.from(curveOrder.slice(2), 'hex'); + + expect(() => blsSkToPk(sk)).toThrow('Secret key is not valid.'); + }); + + it('should throw an error if the input buffer is zero', () => { + const sk = Buffer.alloc(32, 0); + expect(() => blsSkToPk(sk)).toThrow('ZERO_SECRET_KEY'); + }); + + it('should throw an error if the input buffer is not 32 bytes long', () => { + const sk = Buffer.alloc(31, 1); + expect(() => blsSkToPk(sk)).toThrow(ErrorBLST); + }); }); describe('blsSign', () => { From 65bf2a0b541181fcde8177f677ff913e8f1a123b Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 9 Aug 2023 17:41:14 +0200 Subject: [PATCH 02/14] Add unit tests --- .../lisk-cryptography/test/bls_lib/lib.spec.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 05dda6fe444..1a13df1b92e 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -135,6 +135,13 @@ describe('bls_lib', () => { }); }, ); + + it('should throw an error when given an input buffer with value equal to the curve order', () => { + const sk = Buffer.from(curveOrder.slice(2), 'hex'); + const message = Buffer.from('hello world'); + + expect(() => blsSign(sk, message)).toThrow('Secret key is not valid.'); + }); }); describe('blsVerify', () => { @@ -230,6 +237,14 @@ describe('bls_lib', () => { ); }); }); + + it('should throw an error when given an input buffer with value equal to the curve order', () => { + const sk = Buffer.from(curveOrder.slice(2), 'hex'); + + expect(() => { + blsPopProve(sk); + }).toThrow('Secret key is not valid.'); + }); }); describe('blsPopVerify', () => { From 8a1aaa1a22d1fd398be55d7acb6ecba312ead6ee Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 9 Aug 2023 17:52:45 +0200 Subject: [PATCH 03/14] Refactor the isSecretKeyNonZeroModEC function logic --- elements/lisk-cryptography/src/bls_lib/lib.ts | 2 +- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/elements/lisk-cryptography/src/bls_lib/lib.ts b/elements/lisk-cryptography/src/bls_lib/lib.ts index 90c08e2b1db..2ba0af53b88 100644 --- a/elements/lisk-cryptography/src/bls_lib/lib.ts +++ b/elements/lisk-cryptography/src/bls_lib/lib.ts @@ -54,7 +54,7 @@ export const isSecretKeyNonZeroModEC = (secretKey: SecretKey): boolean => { // check if secret key is non-zero modulo the order of the elliptic curve. if (skBigInt % BigInt(curveOrder) === BigInt(0)) { - throw new Error('Secret key is not valid.'); + return false; } return true; diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 1a13df1b92e..1da3a601c04 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -75,11 +75,11 @@ describe('bls_lib', () => { expect(isSecretKeyNonZeroModEC(secretKey)).toBe(true); }); - it('should throw an error when given a zero modulo secret key', () => { + it('should return false for a secret key that is a multiple of the order of the elliptic curve', () => { const secretKey = SecretKey.fromBytes( Buffer.from('73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001', 'hex'), ); - expect(() => isSecretKeyNonZeroModEC(secretKey)).toThrow('Secret key is not valid.'); + expect(isSecretKeyNonZeroModEC(secretKey)).toBe(false); }); }); From ace7eb24d2fa14631fd21720fe3dea288e96186d Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Tue, 15 Aug 2023 16:43:32 +0200 Subject: [PATCH 04/14] Add unit test Case for a secret key that is non-zero but zero modulo the group order must hold --- .../lisk-cryptography/test/bls_lib/lib.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 1da3a601c04..3d1d3d8f0dc 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -142,6 +142,20 @@ describe('bls_lib', () => { expect(() => blsSign(sk, message)).toThrow('Secret key is not valid.'); }); + + it('should throw an error when a secret key that is non-zero but zero modulo the group order', () => { + // sk equals 2*r where r is order of the groups G1 and G2 + const sk = Buffer.from( + 'e7db4ea6533afa906673b0101343b00aa77b4805fffcb7fdfffffffe00000002', + 'hex', + ); + const message = Buffer.from( + 'abababababababababababababababababababababababababababababababab', + 'hex', + ); + + expect(() => blsSign(sk, message)).toThrow('Secret key is not valid.'); + }); }); describe('blsVerify', () => { From b3e9f228f80270883e8d1cf02945d11409f3e65c Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 16 Aug 2023 11:04:24 +0200 Subject: [PATCH 05/14] Refactor the blsSign function - Update the blsSign function - Add and update unit tests --- elements/lisk-cryptography/src/bls_lib/lib.ts | 8 +--- .../test/bls_lib/lib.spec.ts | 38 ++++++++++++++----- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/elements/lisk-cryptography/src/bls_lib/lib.ts b/elements/lisk-cryptography/src/bls_lib/lib.ts index 2ba0af53b88..2ccc358b71a 100644 --- a/elements/lisk-cryptography/src/bls_lib/lib.ts +++ b/elements/lisk-cryptography/src/bls_lib/lib.ts @@ -82,15 +82,9 @@ export const blsAggregate = (signatures: Buffer[]): Buffer | false => { // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.6 export const blsSign = (sk: Buffer, message: Buffer): Buffer => { - // In case of zero private key, it should return particular output regardless of message. - // elements/lisk-cryptography/test/protocol_specs/bls_specs/sign/zero_private_key.yml - if (timingSafeEqual(sk, Buffer.alloc(32))) { - return Buffer.concat([Buffer.from([192]), Buffer.alloc(95)]); - } - const secretKey = SecretKey.fromBytes(sk); - if (!isSecretKeyNonZeroModEC(secretKey)) { + if (timingSafeEqual(sk, Buffer.alloc(32)) || !isSecretKeyNonZeroModEC(secretKey)) { throw new Error('Secret key is not valid.'); } diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 3d1d3d8f0dc..124703954d3 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -120,22 +120,40 @@ describe('bls_lib', () => { }); describe('blsSign', () => { - // Signing with the zero private key is not a use case according to the BLS specifications describe.each(getAllFiles(['eth2_bls_specs/sign', 'bls_specs/sign'], /sign_case_zero_privkey/))( '%s', ({ path }) => { - it('should generate valid signature', () => { - const { - input: { privkey, message }, - output, - } = loadSpecFile(path); - const signature = blsSign(hexToBuffer(privkey), hexToBuffer(message)); - - expect(signature.toString('hex')).toEqual(hexToBuffer(output).toString('hex')); - }); + const { + input: { privkey, message }, + output, + } = loadSpecFile(path); + + if (privkey !== `0x${'00'.repeat(32)}`) { + it('should generate valid signature if private key is non zero', () => { + const signature = blsSign(hexToBuffer(privkey), hexToBuffer(message)); + expect(signature.toString('hex')).toEqual(hexToBuffer(output).toString('hex')); + }); + } }, ); + it('should sign a message with a valid secret key', () => { + const sk = Buffer.alloc(32, 1); + const message = Buffer.from('hello world'); + + const signature = blsSign(sk, message); + + expect(signature).toBeDefined(); + expect(signature.length).toBeGreaterThan(0); + }); + + it('should throw an error if the secret key is all zeros', () => { + const sk = Buffer.alloc(32, 0); + const message = Buffer.from('hello world'); + + expect(() => blsSign(sk, message)).toThrow('ZERO_SECRET_KEY'); + }); + it('should throw an error when given an input buffer with value equal to the curve order', () => { const sk = Buffer.from(curveOrder.slice(2), 'hex'); const message = Buffer.from('hello world'); From bac6e364c5f945f816dd965012aa5fdb92eea665 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 16 Aug 2023 11:26:09 +0200 Subject: [PATCH 06/14] Update unit test --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 124703954d3..110f62a1ec3 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -133,6 +133,12 @@ describe('bls_lib', () => { const signature = blsSign(hexToBuffer(privkey), hexToBuffer(message)); expect(signature.toString('hex')).toEqual(hexToBuffer(output).toString('hex')); }); + } else { + it('should throw an error if the private key is all zeros', () => { + expect(() => blsSign(hexToBuffer(privkey), hexToBuffer(message))).toThrow( + 'ZERO_SECRET_KEY', + ); + }); } }, ); From a8e6ffb6da35beb5558232d7aa7393b8596cc811 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 16 Aug 2023 11:37:42 +0200 Subject: [PATCH 07/14] Remove already tested case --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 110f62a1ec3..12ccd142718 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -153,13 +153,6 @@ describe('bls_lib', () => { expect(signature.length).toBeGreaterThan(0); }); - it('should throw an error if the secret key is all zeros', () => { - const sk = Buffer.alloc(32, 0); - const message = Buffer.from('hello world'); - - expect(() => blsSign(sk, message)).toThrow('ZERO_SECRET_KEY'); - }); - it('should throw an error when given an input buffer with value equal to the curve order', () => { const sk = Buffer.from(curveOrder.slice(2), 'hex'); const message = Buffer.from('hello world'); From 879f9bdbd494ae44cef3cf07c608d9bd9523c43f Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Wed, 16 Aug 2023 11:40:10 +0200 Subject: [PATCH 08/14] Remove already tested case --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 12ccd142718..e0c9c0a9f1e 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -143,16 +143,6 @@ describe('bls_lib', () => { }, ); - it('should sign a message with a valid secret key', () => { - const sk = Buffer.alloc(32, 1); - const message = Buffer.from('hello world'); - - const signature = blsSign(sk, message); - - expect(signature).toBeDefined(); - expect(signature.length).toBeGreaterThan(0); - }); - it('should throw an error when given an input buffer with value equal to the curve order', () => { const sk = Buffer.from(curveOrder.slice(2), 'hex'); const message = Buffer.from('hello world'); From b9db0ecf9a69c0e5df6f112c86cd199f230c0359 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Thu, 17 Aug 2023 11:45:59 +0200 Subject: [PATCH 09/14] Update unit test Remove unnecessary checks Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com> --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index e0c9c0a9f1e..f203c22bd4f 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -120,7 +120,7 @@ describe('bls_lib', () => { }); describe('blsSign', () => { - describe.each(getAllFiles(['eth2_bls_specs/sign', 'bls_specs/sign'], /sign_case_zero_privkey/))( + describe.each(getAllFiles(['eth2_bls_specs/sign', 'bls_specs/sign']))( '%s', ({ path }) => { const { From f00485fc59d5f8ed248ae82308fbc3a31f13ea95 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Thu, 17 Aug 2023 11:46:40 +0200 Subject: [PATCH 10/14] Update test description Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com> --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index f203c22bd4f..f7202f2c12c 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -75,7 +75,7 @@ describe('bls_lib', () => { expect(isSecretKeyNonZeroModEC(secretKey)).toBe(true); }); - it('should return false for a secret key that is a multiple of the order of the elliptic curve', () => { + it('should return false for a secret key that is a multiple of the order of the group', () => { const secretKey = SecretKey.fromBytes( Buffer.from('73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001', 'hex'), ); From e229ec29b6f09f7d95c188e4743aa8e344e02f3f Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Thu, 17 Aug 2023 11:47:49 +0200 Subject: [PATCH 11/14] Update test desc Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com> --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index f7202f2c12c..46bc0a9451a 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -143,7 +143,7 @@ describe('bls_lib', () => { }, ); - it('should throw an error when given an input buffer with value equal to the curve order', () => { + it('should throw an error when given an input buffer with value equal to the group order', () => { const sk = Buffer.from(curveOrder.slice(2), 'hex'); const message = Buffer.from('hello world'); From cf6866c7415ba82253774b64effbcfe940f79e85 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Thu, 17 Aug 2023 11:48:10 +0200 Subject: [PATCH 12/14] Update test desc Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com> --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index 46bc0a9451a..4ef8a981679 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -259,7 +259,7 @@ describe('bls_lib', () => { }); }); - it('should throw an error when given an input buffer with value equal to the curve order', () => { + it('should throw an error when given an input buffer with value equal to the group order', () => { const sk = Buffer.from(curveOrder.slice(2), 'hex'); expect(() => { From 5498c1d52a129b3e3b5de55bbbed4913bf798ee4 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Thu, 17 Aug 2023 12:20:28 +0200 Subject: [PATCH 13/14] Refactor the secret key checks and tests --- elements/lisk-cryptography/src/bls_lib/lib.ts | 16 ++-- .../test/bls_lib/lib.spec.ts | 77 +++++++++---------- 2 files changed, 42 insertions(+), 51 deletions(-) diff --git a/elements/lisk-cryptography/src/bls_lib/lib.ts b/elements/lisk-cryptography/src/bls_lib/lib.ts index 2ccc358b71a..c8b6f8876ee 100644 --- a/elements/lisk-cryptography/src/bls_lib/lib.ts +++ b/elements/lisk-cryptography/src/bls_lib/lib.ts @@ -13,8 +13,6 @@ * */ -import { timingSafeEqual } from 'crypto'; - import { aggregateSignatures, aggregateVerify, @@ -45,15 +43,15 @@ export const blsKeyValidate = (pk: Buffer): boolean => { // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.3 export const blsKeyGen = (ikm: Buffer): Buffer => Buffer.from(SecretKey.fromKeygen(ikm).toBytes()); -// check that the secret key generated is non-zero modulo the order of the elliptic curve. -export const isSecretKeyNonZeroModEC = (secretKey: SecretKey): boolean => { +// check that the secret key generated is non-zero modulo the group order. +export const isSecretKeyNonZeroModOrder = (secretKey: SecretKey): boolean => { // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-pairing-friendly-curves-10#section-4.2.1 - const curveOrder = '0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001'; + const groupOrder = '0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001'; const skBigInt = BigInt(`0x${Buffer.from(secretKey.toBytes()).toString('hex')}`); // check if secret key is non-zero modulo the order of the elliptic curve. - if (skBigInt % BigInt(curveOrder) === BigInt(0)) { + if (skBigInt % BigInt(groupOrder) === BigInt(0)) { return false; } @@ -64,7 +62,7 @@ export const isSecretKeyNonZeroModEC = (secretKey: SecretKey): boolean => { export const blsSkToPk = (sk: Buffer): Buffer => { const secretKey = SecretKey.fromBytes(sk); - if (!isSecretKeyNonZeroModEC(secretKey)) { + if (!isSecretKeyNonZeroModOrder(secretKey)) { throw new Error('Secret key is not valid.'); } @@ -84,7 +82,7 @@ export const blsAggregate = (signatures: Buffer[]): Buffer | false => { export const blsSign = (sk: Buffer, message: Buffer): Buffer => { const secretKey = SecretKey.fromBytes(sk); - if (timingSafeEqual(sk, Buffer.alloc(32)) || !isSecretKeyNonZeroModEC(secretKey)) { + if (!isSecretKeyNonZeroModOrder(secretKey)) { throw new Error('Secret key is not valid.'); } @@ -147,7 +145,7 @@ export const blsPopProve = (sk: Buffer): Buffer => { const secretKey = SecretKey.fromBytes(sk); - if (!isSecretKeyNonZeroModEC(secretKey)) { + if (!isSecretKeyNonZeroModOrder(secretKey)) { throw new Error('Secret key is not valid.'); } diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index e0c9c0a9f1e..ceb96ac3853 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -14,7 +14,7 @@ */ import { ErrorBLST, SecretKey } from '@chainsafe/blst'; -import { isSecretKeyNonZeroModEC } from '../../src/bls_lib/lib'; +import { isSecretKeyNonZeroModOrder } from '../../src/bls_lib/lib'; import { blsAggregate, blsAggregateVerify, @@ -60,26 +60,25 @@ interface EthFastAggrVerifySpec { } describe('bls_lib', () => { - const curveOrder = '0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001'; + const groupOrder = '0x73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001'; - describe('isSecretKeyNonZeroModEC', () => { + describe('isSecretKeyNonZeroModOrder', () => { it('should return true when given a valid secret key', () => { const secretKey = SecretKey.fromBytes(Buffer.alloc(32, 1)); - expect(isSecretKeyNonZeroModEC(secretKey)).toBe(true); + expect(isSecretKeyNonZeroModOrder(secretKey)).toBe(true); }); - it('should return true when given a non-zero modulo secret key', () => { - const secretKey = SecretKey.fromBytes( - Buffer.from('0000000000000000000000000000000000000000000000000000000000000001', 'hex'), + it('should throw error when given a zero secret key', () => { + expect(() => isSecretKeyNonZeroModOrder(SecretKey.fromBytes(Buffer.alloc(32, 0)))).toThrow( + 'ZERO_SECRET_KEY', ); - expect(isSecretKeyNonZeroModEC(secretKey)).toBe(true); }); - it('should return false for a secret key that is a multiple of the order of the elliptic curve', () => { + it('should return false for a secret key that is a multiple of the order of the group', () => { const secretKey = SecretKey.fromBytes( Buffer.from('73eda753299d7d483339d80809a1d80553bda402fffe5bfeffffffff00000001', 'hex'), ); - expect(isSecretKeyNonZeroModEC(secretKey)).toBe(false); + expect(isSecretKeyNonZeroModOrder(secretKey)).toBe(false); }); }); @@ -102,8 +101,8 @@ describe('bls_lib', () => { expect(pk.length).toBeGreaterThan(0); }); - it('should throw an error when given an input buffer with value equal to the curve order', () => { - const sk = Buffer.from(curveOrder.slice(2), 'hex'); + it('should throw an error when given an input buffer with value equal to the group order', () => { + const sk = Buffer.from(groupOrder.slice(2), 'hex'); expect(() => blsSkToPk(sk)).toThrow('Secret key is not valid.'); }); @@ -120,31 +119,28 @@ describe('bls_lib', () => { }); describe('blsSign', () => { - describe.each(getAllFiles(['eth2_bls_specs/sign', 'bls_specs/sign'], /sign_case_zero_privkey/))( - '%s', - ({ path }) => { - const { - input: { privkey, message }, - output, - } = loadSpecFile(path); - - if (privkey !== `0x${'00'.repeat(32)}`) { - it('should generate valid signature if private key is non zero', () => { - const signature = blsSign(hexToBuffer(privkey), hexToBuffer(message)); - expect(signature.toString('hex')).toEqual(hexToBuffer(output).toString('hex')); - }); - } else { - it('should throw an error if the private key is all zeros', () => { - expect(() => blsSign(hexToBuffer(privkey), hexToBuffer(message))).toThrow( - 'ZERO_SECRET_KEY', - ); - }); - } - }, - ); + describe.each(getAllFiles(['eth2_bls_specs/sign', 'bls_specs/sign']))('%s', ({ path }) => { + const { + input: { privkey, message }, + output, + } = loadSpecFile(path); + + if (privkey !== `0x${'00'.repeat(32)}`) { + it('should generate valid signature if private key is non zero', () => { + const signature = blsSign(hexToBuffer(privkey), hexToBuffer(message)); + expect(signature.toString('hex')).toEqual(hexToBuffer(output).toString('hex')); + }); + } else { + it('should throw an error if the private key is all zeros', () => { + expect(() => blsSign(hexToBuffer(privkey), hexToBuffer(message))).toThrow( + 'ZERO_SECRET_KEY', + ); + }); + } + }); - it('should throw an error when given an input buffer with value equal to the curve order', () => { - const sk = Buffer.from(curveOrder.slice(2), 'hex'); + it('should throw an error when given an input buffer with value equal to the group order', () => { + const sk = Buffer.from(groupOrder.slice(2), 'hex'); const message = Buffer.from('hello world'); expect(() => blsSign(sk, message)).toThrow('Secret key is not valid.'); @@ -156,10 +152,7 @@ describe('bls_lib', () => { 'e7db4ea6533afa906673b0101343b00aa77b4805fffcb7fdfffffffe00000002', 'hex', ); - const message = Buffer.from( - 'abababababababababababababababababababababababababababababababab', - 'hex', - ); + const message = Buffer.from('hello world', 'hex'); expect(() => blsSign(sk, message)).toThrow('Secret key is not valid.'); }); @@ -259,8 +252,8 @@ describe('bls_lib', () => { }); }); - it('should throw an error when given an input buffer with value equal to the curve order', () => { - const sk = Buffer.from(curveOrder.slice(2), 'hex'); + it('should throw an error when given an input buffer with value equal to the group order', () => { + const sk = Buffer.from(groupOrder.slice(2), 'hex'); expect(() => { blsPopProve(sk); From 98b14dc07d219e3db52df08ade0b4a52ec03c394 Mon Sep 17 00:00:00 2001 From: Martin Macharia Date: Thu, 17 Aug 2023 14:14:59 +0200 Subject: [PATCH 14/14] Add a unit test Test that `blsPopProve` function throws an an error when the sk is zero --- elements/lisk-cryptography/test/bls_lib/lib.spec.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts index dd3e82ec380..0bd7b8c62ce 100644 --- a/elements/lisk-cryptography/test/bls_lib/lib.spec.ts +++ b/elements/lisk-cryptography/test/bls_lib/lib.spec.ts @@ -265,6 +265,14 @@ describe('bls_lib', () => { blsPopProve(sk); }).toThrow('Secret key is not valid.'); }); + + it('should throw an error when a secret key is zero', () => { + const sk = Buffer.alloc(32, 0); + + expect(() => { + blsPopProve(sk); + }).toThrow('ZERO_SECRET_KEY'); + }); }); describe('blsPopVerify', () => {