Check that the secret key is non-zero modulo the order of the elliptic curve

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)) {
+		return false;
+	}
+
+	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 => {
@@ -60,13 +82,13 @@ 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 (timingSafeEqual(sk, Buffer.alloc(32)) || !isSecretKeyNonZeroModEC(secretKey)) {
+		throw new Error('Secret key is not valid.');
 	}
 
-	return Buffer.from(SecretKey.fromBytes(sk).sign(message).toBytes());
+	return Buffer.from(secretKey.sign(message).toBytes());
 };
 
 // https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-04#section-2.7
@@ -123,8 +145,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(),
 	);
 };
 

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 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)).toBe(false);
+		});
+	});
+
 	describe('blsSkToPk', () => {
 		describe.each(getAllFiles(['bls_specs/sk_to_pk']))('%s', ({ path }) => {
 			it('should convert to valid pk', () => {
@@ -68,24 +93,76 @@ 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', () => {
-		// 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<EthSignSpec>(path);
-					const signature = blsSign(hexToBuffer(privkey), hexToBuffer(message));
-
-					expect(signature.toString('hex')).toEqual(hexToBuffer(output).toString('hex'));
-				});
+				const {
+					input: { privkey, message },
+					output,
+				} = loadSpecFile<EthSignSpec>(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');
+			const message = Buffer.from('hello world');
+
+			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', () => {
@@ -181,6 +258,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', () => {