diff --git a/.gitignore b/.gitignore index 6da32d7..0e4aaad 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,5 @@ typings/ dist/ lib/ benchmark-reports + +.vscode/ diff --git a/src/blst/privateKey.ts b/src/blst/privateKey.ts index 8716f99..4902473 100644 --- a/src/blst/privateKey.ts +++ b/src/blst/privateKey.ts @@ -1,9 +1,10 @@ import * as blst from "@chainsafe/blst"; -import {bytesToHex, hexToBytes, randomBytes} from "../helpers"; +import {bytesToHex, hexToBytes, isZeroUint8Array, randomBytes} from "../helpers"; import {SECRET_KEY_LENGTH} from "../constants"; import {IPrivateKey} from "../interface"; import {PublicKey} from "./publicKey"; import {Signature} from "./signature"; +import {ZeroPrivateKeyError} from "../errors"; export class PrivateKey implements IPrivateKey { readonly value: blst.SecretKey; @@ -13,6 +14,11 @@ export class PrivateKey implements IPrivateKey { } static fromBytes(bytes: Uint8Array): PrivateKey { + // draft-irtf-cfrg-bls-signature-04 does not allow SK == 0 + if (isZeroUint8Array(bytes)) { + throw new ZeroPrivateKeyError(); + } + const sk = blst.SecretKey.fromBytes(bytes); return new PrivateKey(sk); } diff --git a/src/blst/publicKey.ts b/src/blst/publicKey.ts index 657aef2..39c3684 100644 --- a/src/blst/publicKey.ts +++ b/src/blst/publicKey.ts @@ -1,4 +1,5 @@ import * as blst from "@chainsafe/blst"; +import {EmptyAggregateError, ZeroPublicKeyError} from "../errors"; import {bytesToHex, hexToBytes} from "../helpers"; import {IPublicKey} from "../interface"; @@ -13,6 +14,10 @@ export class PublicKey implements IPublicKey { static fromBytes(bytes: Uint8Array): PublicKey { const affine = blst.PublicKey.fromBytes(bytes); + if (affine.value.is_inf()) { + throw new ZeroPublicKeyError(); + } + const jacobian = blst.AggregatePublicKey.fromPublicKey(affine); return new PublicKey(affine, jacobian); } @@ -22,6 +27,10 @@ export class PublicKey implements IPublicKey { } static aggregate(pubkeys: PublicKey[]): PublicKey { + if (pubkeys.length === 0) { + throw new EmptyAggregateError(); + } + const jacobian = blst.aggregatePubkeys(pubkeys.map((pk) => pk.jacobian)); const affine = jacobian.toPublicKey(); return new PublicKey(affine, jacobian); diff --git a/src/blst/signature.ts b/src/blst/signature.ts index d3a74f4..3a476fd 100644 --- a/src/blst/signature.ts +++ b/src/blst/signature.ts @@ -2,6 +2,7 @@ import * as blst from "@chainsafe/blst"; import {bytesToHex, hexToBytes} from "../helpers"; import {ISignature} from "../interface"; import {PublicKey} from "./publicKey"; +import {EmptyAggregateError, ZeroSignatureError} from "../errors"; export class Signature implements ISignature { readonly affine: blst.Signature; @@ -19,11 +20,20 @@ export class Signature implements ISignature { } static aggregate(signatures: Signature[]): Signature { + if (signatures.length === 0) { + throw new EmptyAggregateError(); + } + const agg = blst.AggregateSignature.fromSignatures(signatures.map((sig) => sig.affine)); return new Signature(agg.toSignature()); } verify(publicKey: PublicKey, message: Uint8Array): boolean { + // Individual infinity signatures are NOT okay. Aggregated signatures MAY be infinity + if (this.affine.value.is_inf()) { + throw new ZeroSignatureError(); + } + return this.aggregateVerify([message], [publicKey.affine]); } diff --git a/src/constants.ts b/src/constants.ts index 9da09d2..2243a76 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -3,5 +3,3 @@ export const SIGNATURE_LENGTH = 96; export const FP_POINT_LENGTH = 48; export const PUBLIC_KEY_LENGTH = FP_POINT_LENGTH; export const G2_HASH_PADDING = 16; -export const EMPTY_PUBLIC_KEY = new Uint8Array(PUBLIC_KEY_LENGTH); -export const EMPTY_SIGNATURE = new Uint8Array(SIGNATURE_LENGTH); diff --git a/src/errors.ts b/src/errors.ts index 8666707..58ac7b2 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,5 +1,45 @@ /** - * Indicate that this error is expected and should not be ignored - * by the functional interface try / catch blocks + * This error should not be ignored by the functional interface + * try / catch blocks, to prevent false negatives */ -export class ExpectedError extends Error {} +export class NotInitializedError extends Error { + constructor(implementation: string) { + super(`NOT_INITIALIZED: ${implementation}`); + } +} + +export class ZeroPrivateKeyError extends Error { + constructor() { + super("ZERO_PRIVATE_KEY"); + } +} + +export class ZeroPublicKeyError extends Error { + constructor() { + super("ZERO_PUBLIC_KEY"); + } +} + +export class ZeroSignatureError extends Error { + constructor() { + super("ZERO_SIGNATURE"); + } +} + +export class EmptyAggregateError extends Error { + constructor() { + super("EMPTY_AGGREGATE_ARRAY"); + } +} + +export class InvalidOrderError extends Error { + constructor() { + super("INVALID_ORDER"); + } +} + +export class InvalidLengthError extends Error { + constructor(arg: string, length: number) { + super(`INVALID_LENGTH: ${arg} must have ${length} bytes`); + } +} diff --git a/src/functional.ts b/src/functional.ts index da7ad41..a731a7d 100644 --- a/src/functional.ts +++ b/src/functional.ts @@ -1,6 +1,6 @@ import {IBls} from "./interface"; import {validateBytes} from "./helpers"; -import {ExpectedError} from "./errors"; +import {NotInitializedError} from "./errors"; // Returned type is enforced at each implementation's index // eslint-disable-next-line @typescript-eslint/explicit-function-return-type @@ -54,7 +54,7 @@ export function functionalInterfaceFactory({ try { return Signature.fromBytes(signature).verify(PublicKey.fromBytes(publicKey), message); } catch (e) { - if (e instanceof ExpectedError) throw e; + if (e instanceof NotInitializedError) throw e; return false; } } @@ -76,7 +76,7 @@ export function functionalInterfaceFactory({ message ); } catch (e) { - if (e instanceof ExpectedError) throw e; + if (e instanceof NotInitializedError) throw e; return false; } } @@ -102,7 +102,7 @@ export function functionalInterfaceFactory({ messages.map((msg) => msg) ); } catch (e) { - if (e instanceof ExpectedError) throw e; + if (e instanceof NotInitializedError) throw e; return false; } } diff --git a/src/helpers/utils.ts b/src/helpers/utils.ts index 6a7c143..114ab7b 100644 --- a/src/helpers/utils.ts +++ b/src/helpers/utils.ts @@ -3,14 +3,6 @@ import randomBytes from "randombytes"; // Single import to ease changing this lib if necessary export {randomBytes}; -export function isEqualBytes(a: Buffer | Uint8Array, b: Buffer | Uint8Array): boolean { - return toBuffer(a).equals(toBuffer(b)); -} - -export function toBuffer(input: Uint8Array): Buffer { - return Buffer.from(input.buffer, input.byteOffset, input.length); -} - /** * Validate bytes to prevent confusing WASM errors downstream if bytes is null */ @@ -24,3 +16,7 @@ export function validateBytes( } } } + +export function isZeroUint8Array(bytes: Uint8Array): boolean { + return bytes.every((byte) => byte === 0); +} diff --git a/src/herumi/context.ts b/src/herumi/context.ts index b4da52e..7780283 100644 --- a/src/herumi/context.ts +++ b/src/herumi/context.ts @@ -1,6 +1,6 @@ /* eslint-disable require-atomic-updates */ import bls from "bls-eth-wasm"; -import {ExpectedError} from "../errors"; +import {NotInitializedError} from "../errors"; type Bls = typeof bls; let blsGlobal: Bls | null = null; @@ -28,7 +28,7 @@ export function destroy(): void { export function getContext(): Bls { if (!blsGlobal) { - throw new ExpectedError("BLS not initialized"); + throw new NotInitializedError("herumi"); } return blsGlobal; } diff --git a/src/herumi/privateKey.ts b/src/herumi/privateKey.ts index 6fcfb7f..7eb9df6 100644 --- a/src/herumi/privateKey.ts +++ b/src/herumi/privateKey.ts @@ -6,17 +6,22 @@ import {PublicKey} from "./publicKey"; import {Signature} from "./signature"; import {bytesToHex, hexToBytes} from "../helpers"; import {IPrivateKey} from "../interface"; +import {InvalidLengthError, ZeroPrivateKeyError} from "../errors"; export class PrivateKey implements IPrivateKey { readonly value: SecretKeyType; constructor(value: SecretKeyType) { + if (value.isZero()) { + throw new ZeroPrivateKeyError(); + } + this.value = value; } static fromBytes(bytes: Uint8Array): PrivateKey { if (bytes.length !== SECRET_KEY_LENGTH) { - throw Error(`Private key should have ${SECRET_KEY_LENGTH} bytes`); + throw new InvalidLengthError("PrivateKey", SECRET_KEY_LENGTH); } const context = getContext(); diff --git a/src/herumi/publicKey.ts b/src/herumi/publicKey.ts index 441193e..34c8338 100644 --- a/src/herumi/publicKey.ts +++ b/src/herumi/publicKey.ts @@ -1,24 +1,29 @@ import {PublicKeyType} from "bls-eth-wasm"; import {getContext} from "./context"; -import {EMPTY_PUBLIC_KEY, PUBLIC_KEY_LENGTH} from "../constants"; -import {bytesToHex, hexToBytes, isEqualBytes} from "../helpers"; +import {PUBLIC_KEY_LENGTH} from "../constants"; +import {bytesToHex, hexToBytes, isZeroUint8Array} from "../helpers"; import {IPublicKey} from "../interface"; +import {EmptyAggregateError, InvalidLengthError, ZeroPublicKeyError} from "../errors"; export class PublicKey implements IPublicKey { readonly value: PublicKeyType; constructor(value: PublicKeyType) { + if (value.isZero()) { + throw new ZeroPublicKeyError(); + } + this.value = value; } static fromBytes(bytes: Uint8Array): PublicKey { if (bytes.length !== PUBLIC_KEY_LENGTH) { - throw Error(`Public key must have ${PUBLIC_KEY_LENGTH} bytes`); + throw new InvalidLengthError("PublicKey", PUBLIC_KEY_LENGTH); } const context = getContext(); const publicKey = new context.PublicKey(); - if (!isEqualBytes(EMPTY_PUBLIC_KEY, bytes)) { + if (!isZeroUint8Array(bytes)) { publicKey.deserialize(bytes); } return new PublicKey(publicKey); @@ -30,7 +35,7 @@ export class PublicKey implements IPublicKey { static aggregate(pubkeys: PublicKey[]): PublicKey { if (pubkeys.length === 0) { - throw Error("EMPTY_AGGREGATE_ARRAY"); + throw new EmptyAggregateError(); } const agg = new PublicKey(pubkeys[0].value.clone()); diff --git a/src/herumi/signature.ts b/src/herumi/signature.ts index b0d9fc2..ad0df9f 100644 --- a/src/herumi/signature.ts +++ b/src/herumi/signature.ts @@ -1,16 +1,17 @@ -import {SIGNATURE_LENGTH, EMPTY_SIGNATURE} from "../constants"; +import {SIGNATURE_LENGTH} from "../constants"; import {SignatureType} from "bls-eth-wasm"; import {getContext} from "./context"; import {PublicKey} from "./publicKey"; -import {bytesToHex, hexToBytes, isEqualBytes} from "../helpers"; +import {bytesToHex, hexToBytes, isZeroUint8Array} from "../helpers"; import {ISignature} from "../interface"; +import {EmptyAggregateError, InvalidLengthError, InvalidOrderError} from "../errors"; export class Signature implements ISignature { readonly value: SignatureType; constructor(value: SignatureType) { if (!value.isValidOrder()) { - throw Error("Signature is not in valid order"); + throw new InvalidOrderError(); } this.value = value; @@ -18,12 +19,12 @@ export class Signature implements ISignature { static fromBytes(bytes: Uint8Array): Signature { if (bytes.length !== SIGNATURE_LENGTH) { - throw Error(`Signature must have ${SIGNATURE_LENGTH} bytes`); + throw new InvalidLengthError("Signature", SIGNATURE_LENGTH); } const context = getContext(); const signature = new context.Signature(); - if (!isEqualBytes(EMPTY_SIGNATURE, bytes)) { + if (!isZeroUint8Array(bytes)) { signature.deserialize(bytes); } return new Signature(signature); @@ -35,7 +36,7 @@ export class Signature implements ISignature { static aggregate(signatures: Signature[]): Signature { if (signatures.length === 0) { - throw Error("EMPTY_AGGREGATE_ARRAY"); + throw new EmptyAggregateError(); } const context = getContext(); diff --git a/test/benchmark/index.ts b/test/benchmark/index.ts index 9136baa..9e0380f 100644 --- a/test/benchmark/index.ts +++ b/test/benchmark/index.ts @@ -3,9 +3,7 @@ import {runForAllImplementations} from "../switch"; import {IPublicKey, ISignature} from "../../src/interface"; import {randomBytes} from "../../src/helpers"; -runForAllImplementations(async (bls, implementation) => { - await bls.init(); - +runForAllImplementations((bls, implementation) => { const aggCount = 30; // verify diff --git a/test/spec/aggregate_sigs.test.ts b/test/spec/aggregate.test.ts similarity index 89% rename from test/spec/aggregate_sigs.test.ts rename to test/spec/aggregate.test.ts index a825e74..41af125 100644 --- a/test/spec/aggregate_sigs.test.ts +++ b/test/spec/aggregate.test.ts @@ -3,6 +3,7 @@ import {describeDirectorySpecTest, InputType} from "@chainsafe/lodestar-spec-tes import {bytesToHex, hexToBytes} from "../../src/helpers"; import {SPEC_TESTS_DIR} from "../params"; import {describeForAllImplementations} from "../switch"; +import {EmptyAggregateError} from "../../src/errors"; interface IAggregateSigsTestCase { data: { @@ -21,7 +22,7 @@ describeForAllImplementations((bls) => { const agg = bls.aggregateSignatures(signatures.map(hexToBytes)); return bytesToHex(agg); } catch (e) { - if (e.message === "EMPTY_AGGREGATE_ARRAY") return null; + if (e instanceof EmptyAggregateError) return null; throw e; } }, diff --git a/test/spec/aggregate_sigs_verify.test.ts b/test/spec/aggregate_verify.test.ts similarity index 100% rename from test/spec/aggregate_sigs_verify.test.ts rename to test/spec/aggregate_verify.test.ts diff --git a/test/spec/sign.test.ts b/test/spec/sign.test.ts index a90f21b..f8a9869 100644 --- a/test/spec/sign.test.ts +++ b/test/spec/sign.test.ts @@ -3,6 +3,7 @@ import {describeDirectorySpecTest, InputType} from "@chainsafe/lodestar-spec-tes import {bytesToHex, hexToBytes} from "../../src/helpers"; import {SPEC_TESTS_DIR} from "../params"; import {describeForAllImplementations} from "../switch"; +import {ZeroPrivateKeyError} from "../../src/errors"; interface ISignMessageTestCase { data: { @@ -19,9 +20,14 @@ describeForAllImplementations((bls) => { "bls/sign/small", path.join(SPEC_TESTS_DIR, "general/phase0/bls/sign/small"), (testCase) => { - const {privkey, message} = testCase.data.input; - const signature = bls.sign(hexToBytes(privkey), hexToBytes(message)); - return bytesToHex(signature); + try { + const {privkey, message} = testCase.data.input; + const signature = bls.sign(hexToBytes(privkey), hexToBytes(message)); + return bytesToHex(signature); + } catch (e) { + if (e instanceof ZeroPrivateKeyError) return null; + else throw e; + } }, { inputTypes: {data: InputType.YAML}, diff --git a/test/switch.ts b/test/switch.ts index 942f6ae..c86f143 100644 --- a/test/switch.ts +++ b/test/switch.ts @@ -15,11 +15,12 @@ export function getBls(implementation: Implementation): IBls { } export async function runForAllImplementations( - callback: (bls: IBls, implementation: Implementation) => Promise | void + callback: (bls: IBls, implementation: Implementation) => void ): Promise { for (const implementation of ["blst", "herumi"] as Implementation[]) { const bls = getBls(implementation); - await callback(bls, implementation); + await bls.init(); + callback(bls, implementation); } } diff --git a/test/unit/helpers/bytes.test.ts b/test/unit/helpers/bytes.test.ts new file mode 100644 index 0000000..1958185 --- /dev/null +++ b/test/unit/helpers/bytes.test.ts @@ -0,0 +1,29 @@ +import {expect} from "chai"; +import {isZeroUint8Array} from "../../../src/helpers/utils"; + +describe("helpers / bytes", () => { + describe("isZeroUint8Array", () => { + const testCases: {isZero: boolean; hex: string}[] = [ + {hex: "0x00", isZero: true}, + {hex: "0x" + "00".repeat(32), isZero: true}, + {hex: "0x" + "00".repeat(96), isZero: true}, + {hex: "0x" + "00".repeat(31) + "01", isZero: false}, + { + hex: "0xb6f21199594b56d77670564bf422cb331d5281ca2c1f9a45588a56881d8287ef8619efa6456d6cd2ef61306aa5b21311", + isZero: false, + }, + ]; + + for (const {hex, isZero} of testCases) { + it(`${hex} isZero = ${isZero}`, () => { + const bytes = hexToBytesNode(hex); + console.log(bytes); + expect(isZeroUint8Array(bytes)).to.equal(isZero); + }); + } + }); +}); + +function hexToBytesNode(hex: string): Buffer { + return Buffer.from(hex.replace("0x", ""), "hex"); +}