From 645ed3da85ae14623f7baf0c38eef23b374a2bf1 Mon Sep 17 00:00:00 2001
From: "Mark S. Lewis" <Mark.S.Lewis@outlook.com>
Date: Sat, 9 Mar 2024 14:46:47 +0000
Subject: [PATCH] Use @noble/curves for Node ECDSA signing

The @noble/hashes packages used by @noble/curves supports big-endian systems since v1.4.0. Switch from asn1.js / bn.js / elliptic to use @noble/curves, since this provides:

- better typing
- smaller dependency footprint
- simpler usage for the required scenarios
- appears more actively maintained

Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>
---
 node/package.json                   |  5 +--
 node/src/README.md                  |  4 +--
 node/src/blockevents.test.ts        |  2 +-
 node/src/checkpointers.test.ts      |  4 +--
 node/src/filecheckpointer.ts        |  2 +-
 node/src/gateway.test.ts            |  2 +-
 node/src/hash/hashes.ts             |  2 +-
 node/src/identity/asn1.ts           | 19 -----------
 node/src/identity/ecdsa.ts          | 52 ++++++-----------------------
 node/src/identity/hsmsigner.test.ts | 30 ++++++++++-------
 node/src/identity/hsmsigner.ts      | 12 +++++--
 node/src/identity/signers.test.ts   |  2 +-
 node/src/signingidentity.test.ts    | 18 +++++-----
 node/src/signingidentity.ts         |  6 ++--
 node/src/testutils.test.ts          |  6 ++--
 node/src/transactioncontext.ts      |  2 +-
 node/src/transactionparser.ts       |  2 +-
 scenario/node/src/customworld.ts    |  6 ++--
 scenario/node/src/fabric.ts         |  6 ++--
 scenario/node/src/utils.ts          |  2 +-
 20 files changed, 71 insertions(+), 113 deletions(-)
 delete mode 100644 node/src/identity/asn1.ts

diff --git a/node/package.json b/node/package.json
index 26832274d..65a0fe86c 100644
--- a/node/package.json
+++ b/node/package.json
@@ -35,9 +35,7 @@
     "dependencies": {
         "@grpc/grpc-js": "^1.10.0",
         "@hyperledger/fabric-protos": "^0.3.0",
-        "asn1.js": "^5.4.0",
-        "bn.js": "^5.2.0",
-        "elliptic": "^6.5.0",
+        "@noble/curves": "^1.4.0",
         "google-protobuf": "^3.21.0"
     },
     "optionalDependencies": {
@@ -46,7 +44,6 @@
     "devDependencies": {
         "@cyclonedx/cyclonedx-npm": "^1.16.1",
         "@tsconfig/node18": "^18.2.2",
-        "@types/elliptic": "^6.4.18",
         "@types/google-protobuf": "^3.15.12",
         "@types/jest": "^29.5.12",
         "@types/node": "^18.19.22",
diff --git a/node/src/README.md b/node/src/README.md
index b92e95f2b..5f4324771 100644
--- a/node/src/README.md
+++ b/node/src/README.md
@@ -18,8 +18,8 @@ The following complete example shows how to connect to a Fabric network, submit
 import * as grpc from '@grpc/grpc-js';
 import * as crypto from 'node:crypto';
 import { connect, Identity, signers } from '@hyperledger/fabric-gateway';
-import { promises as fs } from 'fs';
-import { TextDecoder } from 'util';
+import { promises as fs } from 'node:fs';
+import { TextDecoder } from 'node:util';
 
 const utf8Decoder = new TextDecoder();
 
diff --git a/node/src/blockevents.test.ts b/node/src/blockevents.test.ts
index 4dc5613b1..a3b70e9a2 100644
--- a/node/src/blockevents.test.ts
+++ b/node/src/blockevents.test.ts
@@ -52,7 +52,7 @@ describe('Block Events', () => {
         details: 'DETAILS',
         metadata: new Metadata(),
     });
-    const tlsClientCertificateHash = Uint8Array.from(Buffer.from('TLS_CLIENT_CERTIFICATE_HASH'));
+    const tlsClientCertificateHash = new Uint8Array(Buffer.from('TLS_CLIENT_CERTIFICATE_HASH'));
 
     let defaultOptions: () => CallOptions;
     let client: MockGatewayGrpcClient;
diff --git a/node/src/checkpointers.test.ts b/node/src/checkpointers.test.ts
index 30108e471..7db78dec4 100644
--- a/node/src/checkpointers.test.ts
+++ b/node/src/checkpointers.test.ts
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { promises as fs } from 'fs';
-import * as path from 'path';
+import { promises as fs } from 'node:fs';
+import * as path from 'node:path';
 import { ChaincodeEvent } from './chaincodeevent';
 import { Checkpointer } from './checkpointer';
 import * as checkpointers from './checkpointers';
diff --git a/node/src/filecheckpointer.ts b/node/src/filecheckpointer.ts
index 0ea69653d..371787b5b 100644
--- a/node/src/filecheckpointer.ts
+++ b/node/src/filecheckpointer.ts
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import fs from 'fs';
+import fs from 'node:fs';
 import { ChaincodeEvent } from './chaincodeevent';
 import { Checkpointer } from './checkpointer';
 
diff --git a/node/src/gateway.test.ts b/node/src/gateway.test.ts
index a4f80d48e..4c0aa7303 100644
--- a/node/src/gateway.test.ts
+++ b/node/src/gateway.test.ts
@@ -61,7 +61,7 @@ describe('Gateway', () => {
             const result = gateway.getIdentity();
 
             expect(result.mspId).toEqual(identity.mspId);
-            expect(Uint8Array.from(result.credentials)).toEqual(Uint8Array.from(identity.credentials));
+            expect(new Uint8Array(result.credentials)).toEqual(new Uint8Array(identity.credentials));
         });
     });
 
diff --git a/node/src/hash/hashes.ts b/node/src/hash/hashes.ts
index a3ad003b8..2cdc7967b 100644
--- a/node/src/hash/hashes.ts
+++ b/node/src/hash/hashes.ts
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { createHash } from 'crypto';
+import { createHash } from 'node:crypto';
 import { Hash } from './hash';
 
 /**
diff --git a/node/src/identity/asn1.ts b/node/src/identity/asn1.ts
deleted file mode 100644
index 2879fe0d7..000000000
--- a/node/src/identity/asn1.ts
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * Copyright IBM Corp. All Rights Reserved.
- *
- * SPDX-License-Identifier: Apache-2.0
- */
-
-/* eslint-disable */
-// @ts-nocheck
-
-import { define } from 'asn1.js';
-import BN from 'bn.js';
-
-const ECSignature = define('ECSignature', function () {
-    return this.seq().obj(this.key('r').int(), this.key('s').int());
-});
-
-export function ecSignatureAsDER(r: BN, s: BN): Uint8Array {
-    return new Uint8Array(ECSignature.encode({ r, s }, 'der'));
-}
diff --git a/node/src/identity/ecdsa.ts b/node/src/identity/ecdsa.ts
index 19ee140e9..ba8ad95e8 100644
--- a/node/src/identity/ecdsa.ts
+++ b/node/src/identity/ecdsa.ts
@@ -4,15 +4,15 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import BN from 'bn.js';
-import { KeyObject } from 'crypto';
-import { ec as EC } from 'elliptic';
-import { ecSignatureAsDER } from './asn1';
+import { CurveFn } from '@noble/curves/abstract/weierstrass';
+import { p256 } from '@noble/curves/p256';
+import { p384 } from '@noble/curves/p384';
+import { KeyObject } from 'node:crypto';
 import { Signer } from './signer';
 
-const namedCurves: Record<string, EC> = {
-    'P-256': new EC('p256'),
-    'P-384': new EC('p384'),
+const namedCurves: Record<string, CurveFn> = {
+    'P-256': p256,
+    'P-384': p384,
 };
 
 export function newECPrivateKeySigner(key: KeyObject): Signer {
@@ -28,13 +28,12 @@ export function newECPrivateKeySigner(key: KeyObject): Signer {
     const privateKey = Buffer.from(d, 'base64url');
 
     return (digest) => {
-        const signature = curve.sign(digest, privateKey, { canonical: true });
-        const signatureBytes = new Uint8Array(signature.toDER());
-        return Promise.resolve(signatureBytes);
+        const signature = curve.sign(digest, privateKey, { lowS: true });
+        return Promise.resolve(signature.toDERRawBytes());
     };
 }
 
-function getCurve(name: string): EC {
+function getCurve(name: string): CurveFn {
     const curve = namedCurves[name];
     if (!curve) {
         throw new Error(`Unsupported curve: ${name}`);
@@ -42,34 +41,3 @@ function getCurve(name: string): EC {
 
     return curve;
 }
-
-export class ECSignature {
-    readonly #curve: EC;
-    readonly #r: BN;
-    #s: BN;
-
-    constructor(curveName: string, compactSignature: Uint8Array) {
-        this.#curve = getCurve(curveName);
-
-        const sIndex = compactSignature.length / 2;
-        const r = compactSignature.slice(0, sIndex);
-        const s = compactSignature.slice(sIndex);
-        this.#r = new BN(r);
-        this.#s = new BN(s);
-    }
-
-    normalise(): this {
-        const n = this.#curve.n!;
-        const halfOrder = n.divn(2);
-
-        if (this.#s.gt(halfOrder)) {
-            this.#s = n.sub(this.#s);
-        }
-
-        return this;
-    }
-
-    toDER(): Uint8Array {
-        return ecSignatureAsDER(this.#r, this.#s);
-    }
-}
diff --git a/node/src/identity/hsmsigner.test.ts b/node/src/identity/hsmsigner.test.ts
index c0a2bcafc..067b43e59 100644
--- a/node/src/identity/hsmsigner.test.ts
+++ b/node/src/identity/hsmsigner.test.ts
@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { p256 } from '@noble/curves/p256';
+import { createHash } from 'node:crypto';
 import { Mechanism, Pkcs11Error, SessionInfo, SlotInfo, Template, TokenInfo } from 'pkcs11js';
 import { HSMSignerOptions } from './hsmsigner';
 import { newHSMSignerFactory } from './signers';
@@ -188,12 +190,11 @@ describe('When using an HSM Signer', () => {
     const mockSession = Buffer.from('mockSession');
     const mockPrivateKeyHandle = Buffer.from('someobject');
 
-    const HSMSignature =
-        'a5f6e5dd8c46ee4094ebb908b719572022f64ed4bbc21f1f5aa4e49163f4f56c4c6ca8b0393836c79045b1be2f25b1cd2b2b253a213fc9248b7e18574c4170b4';
-    const DERSignature =
-        '3045022100a5f6e5dd8c46ee4094ebb908b719572022f64ed4bbc21f1f5aa4e49163f4f56c02204c6ca8b0393836c79045b1be2f25b1cd2b2b253a213fc9248b7e18574c4170b4';
     const hsmSignerFactory = newHSMSignerFactory('somelibrary');
 
+    const privateKey = p256.utils.randomPrivateKey();
+    const publicKey = p256.getPublicKey(privateKey);
+
     beforeEach(() => {
         resetPkcs11Stub();
         pkcs11Stub.C_GetTokenInfo = mockTokenInfo;
@@ -206,6 +207,13 @@ describe('When using an HSM Signer', () => {
         pkcs11Stub.C_FindObjects = jest.fn(() => {
             return [mockPrivateKeyHandle];
         });
+        pkcs11Stub.C_SignInit = jest.fn();
+        pkcs11Stub.C_Sign = jest.fn((session, digest, buffer) => {
+            const signature = p256.sign(digest, privateKey).toCompactRawBytes();
+            signature.forEach((b, i) => buffer.writeUInt8(b, i));
+            // Return buffer of exactly signature length regardless of supplied buffer size
+            return buffer.subarray(0, signature.length);
+        });
     });
 
     it('throws if label, pin or identifier are blank or not provided', () => {
@@ -354,16 +362,14 @@ describe('When using an HSM Signer', () => {
     });
 
     it('signs using the HSM', async () => {
-        pkcs11Stub.C_SignInit = jest.fn();
-        pkcs11Stub.C_Sign = jest.fn(() => {
-            return Buffer.from(HSMSignature, 'hex');
-        });
-
-        const digest = Buffer.from('some digest');
+        const message = Buffer.from('A quick brown fox jumps over the lazy dog');
+        const digest = createHash('sha256').update(message).digest();
 
         const { signer } = hsmSignerFactory.newSigner(hsmOptions);
-        const signed = await signer(digest);
-        expect(signed).toEqual(new Uint8Array(Buffer.from(DERSignature, 'hex')));
+        const signature = await signer(digest);
+
+        const valid = p256.verify(signature, digest, publicKey);
+        expect(valid).toBe(true);
 
         expect(pkcs11Stub.C_SignInit).toHaveBeenCalledWith(mockSession, { mechanism: CKM_ECDSA }, mockPrivateKeyHandle);
         expect(pkcs11Stub.C_Sign).toHaveBeenCalledWith(mockSession, digest, expect.anything());
diff --git a/node/src/identity/hsmsigner.ts b/node/src/identity/hsmsigner.ts
index 2df0d73f4..304af274a 100644
--- a/node/src/identity/hsmsigner.ts
+++ b/node/src/identity/hsmsigner.ts
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { p256 } from '@noble/curves/p256';
 import * as pkcs11js from 'pkcs11js';
-import { ECSignature } from './ecdsa';
 import { Signer } from './signer';
 
 export interface HSMSignerOptions {
@@ -96,8 +96,14 @@ export class HSMSignerFactoryImpl implements HSMSignerFactory {
         return {
             signer: (digest) => {
                 pkcs11.C_SignInit(session, { mechanism: pkcs11js.CKM_ECDSA }, privateKeyHandle);
-                const compactSignature = pkcs11.C_Sign(session, Buffer.from(digest), Buffer.alloc(256));
-                const signature = new ECSignature('P-256', compactSignature).normalise().toDER();
+                const compactSignature = pkcs11.C_Sign(
+                    session,
+                    Buffer.from(digest),
+                    // EC signatures have length of 2n according to the PKCS11 spec:
+                    // https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/pkcs11-spec-v3.1.html
+                    Buffer.alloc(p256.CURVE.nByteLength * 2),
+                );
+                const signature = p256.Signature.fromCompact(compactSignature).normalizeS().toDERRawBytes();
                 return Promise.resolve(signature);
             },
             close: () => pkcs11.C_CloseSession(session),
diff --git a/node/src/identity/signers.test.ts b/node/src/identity/signers.test.ts
index 73b0a63a2..1f46ac0f5 100644
--- a/node/src/identity/signers.test.ts
+++ b/node/src/identity/signers.test.ts
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { createHash, generateKeyPairSync, verify } from 'crypto';
+import { createHash, generateKeyPairSync, verify } from 'node:crypto';
 import { newPrivateKeySigner } from './signers';
 
 describe('signers', () => {
diff --git a/node/src/signingidentity.test.ts b/node/src/signingidentity.test.ts
index d1c555c0e..b9f0fd37d 100644
--- a/node/src/signingidentity.test.ts
+++ b/node/src/signingidentity.test.ts
@@ -15,14 +15,14 @@ describe('SigningIdentity', () => {
     beforeEach(() => {
         identity = {
             mspId: 'MSP_ID',
-            credentials: Uint8Array.from(Buffer.from('CREDENTIALS')),
+            credentials: new Uint8Array(Buffer.from('CREDENTIALS')),
         };
     });
 
     describe('identity', () => {
         it('changes to returned identity do not modify signing identity', () => {
             const expectedMspId = identity.mspId;
-            const expectedCredentials = Uint8Array.from(identity.credentials); // Copy
+            const expectedCredentials = new Uint8Array(identity.credentials); // Copy
             const signingIdentity = new SigningIdentity({ identity });
 
             const output = signingIdentity.getIdentity();
@@ -31,13 +31,13 @@ describe('SigningIdentity', () => {
 
             const actual = signingIdentity.getIdentity();
             expect(actual.mspId).toBe(expectedMspId);
-            const actualCredentials = Uint8Array.from(actual.credentials); // Ensure it's really a Uint8Array
+            const actualCredentials = new Uint8Array(actual.credentials); // Ensure it's really a Uint8Array
             expect(actualCredentials).toEqual(expectedCredentials);
         });
 
         it('changes to supplied identity do not modify signing identity', () => {
             const expectedMspId = identity.mspId;
-            const expectedCredentials = Uint8Array.from(identity.credentials); // Copy
+            const expectedCredentials = new Uint8Array(identity.credentials); // Copy
 
             const signingIdentity = new SigningIdentity({ identity });
             identity.mspId = 'wrong';
@@ -45,7 +45,7 @@ describe('SigningIdentity', () => {
 
             const actual = signingIdentity.getIdentity();
             expect(actual.mspId).toBe(expectedMspId);
-            const actualCredentials = Uint8Array.from(actual.credentials); // Ensure it's really a Uint8Array
+            const actualCredentials = new Uint8Array(actual.credentials); // Ensure it's really a Uint8Array
             expect(actualCredentials).toEqual(expectedCredentials);
         });
     });
@@ -58,18 +58,18 @@ describe('SigningIdentity', () => {
 
             const actual = msp.SerializedIdentity.deserializeBinary(creator);
             expect(actual.getMspid()).toBe(identity.mspId);
-            const credentials = Uint8Array.from(actual.getIdBytes_asU8()); // Ensure it's really a Uint8Array
+            const credentials = new Uint8Array(actual.getIdBytes_asU8()); // Ensure it's really a Uint8Array
             expect(credentials).toEqual(identity.credentials);
         });
 
         it('changes to returned creator do not modify signing identity', () => {
             const signingIdentity = new SigningIdentity({ identity });
-            const expected = Uint8Array.from(signingIdentity.getCreator()); // Ensure it's really a Uint8Array
+            const expected = new Uint8Array(signingIdentity.getCreator()); // Ensure it's really a Uint8Array
 
             const creator = signingIdentity.getCreator();
             creator.fill(0);
 
-            const actual = Uint8Array.from(signingIdentity.getCreator()); // Ensure it's really a Uint8Array
+            const actual = new Uint8Array(signingIdentity.getCreator()); // Ensure it's really a Uint8Array
             expect(actual).toEqual(expected);
         });
     });
@@ -83,7 +83,7 @@ describe('SigningIdentity', () => {
         });
 
         it('uses supplied signer', async () => {
-            const expected = Uint8Array.from(Buffer.from('SIGNATURE'));
+            const expected = new Uint8Array(Buffer.from('SIGNATURE'));
             const signer: Signer = async () => Promise.resolve(expected);
             const digest = Buffer.from('DIGEST');
             const signingIdentity = new SigningIdentity({ identity, signer });
diff --git a/node/src/signingidentity.ts b/node/src/signingidentity.ts
index e7164194c..a70f05aa7 100644
--- a/node/src/signingidentity.ts
+++ b/node/src/signingidentity.ts
@@ -28,7 +28,7 @@ export class SigningIdentity {
     constructor(options: Readonly<SigningIdentityOptions>) {
         this.#identity = {
             mspId: options.identity.mspId,
-            credentials: Uint8Array.from(options.identity.credentials),
+            credentials: new Uint8Array(options.identity.credentials),
         };
 
         const serializedIdentity = new msp.SerializedIdentity();
@@ -43,12 +43,12 @@ export class SigningIdentity {
     getIdentity(): Identity {
         return {
             mspId: this.#identity.mspId,
-            credentials: Uint8Array.from(this.#identity.credentials),
+            credentials: new Uint8Array(this.#identity.credentials),
         };
     }
 
     getCreator(): Uint8Array {
-        return Uint8Array.from(this.#creator);
+        return new Uint8Array(this.#creator);
     }
 
     hash(message: Uint8Array): Uint8Array {
diff --git a/node/src/testutils.test.ts b/node/src/testutils.test.ts
index ea072798f..8cd2238da 100644
--- a/node/src/testutils.test.ts
+++ b/node/src/testutils.test.ts
@@ -6,9 +6,9 @@
 
 import * as grpc from '@grpc/grpc-js';
 import { common, gateway, peer } from '@hyperledger/fabric-protos';
-import fs from 'fs';
-import os from 'os';
-import path from 'path';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
 import {
     CloseableAsyncIterable,
     DuplexStreamResponse,
diff --git a/node/src/transactioncontext.ts b/node/src/transactioncontext.ts
index 982b2d378..408cd0d2e 100644
--- a/node/src/transactioncontext.ts
+++ b/node/src/transactioncontext.ts
@@ -5,7 +5,7 @@
  */
 
 import { common } from '@hyperledger/fabric-protos';
-import { randomBytes } from 'crypto';
+import { randomBytes } from 'node:crypto';
 import { sha256 } from './hash/hashes';
 import { SigningIdentity } from './signingidentity';
 
diff --git a/node/src/transactionparser.ts b/node/src/transactionparser.ts
index 5b96e36e3..a06d9babc 100644
--- a/node/src/transactionparser.ts
+++ b/node/src/transactionparser.ts
@@ -5,7 +5,7 @@
  */
 
 import { common, peer } from '@hyperledger/fabric-protos';
-import { inspect } from 'util';
+import { inspect } from 'node:util';
 import { assertDefined } from './gateway';
 
 export function parseTransactionEnvelope(envelope: common.Envelope): {
diff --git a/scenario/node/src/customworld.ts b/scenario/node/src/customworld.ts
index 29625f895..b2d447889 100644
--- a/scenario/node/src/customworld.ts
+++ b/scenario/node/src/customworld.ts
@@ -15,9 +15,9 @@ import {
     Signer,
     signers,
 } from '@hyperledger/fabric-gateway';
-import { createPrivateKey, KeyObject, X509Certificate } from 'crypto';
-import { promises as fs } from 'fs';
-import * as path from 'path';
+import { createPrivateKey, KeyObject, X509Certificate } from 'node:crypto';
+import { promises as fs } from 'node:fs';
+import * as path from 'node:path';
 import { findSoftHSMPKCS11Lib, fixturesDir, getOrgForMsp } from './fabric';
 import { getSKIFromCertificate } from './fabricski';
 import { GatewayContext } from './gatewaycontext';
diff --git a/scenario/node/src/fabric.ts b/scenario/node/src/fabric.ts
index 61492c438..0bd2f6505 100644
--- a/scenario/node/src/fabric.ts
+++ b/scenario/node/src/fabric.ts
@@ -4,9 +4,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { execFileSync, spawnSync } from 'child_process';
-import * as path from 'path';
-import * as fs from 'fs';
+import { execFileSync, spawnSync } from 'node:child_process';
+import * as path from 'node:path';
+import * as fs from 'node:fs';
 
 export const fixturesDir = path.resolve(__dirname, '..', '..', 'fixtures');
 
diff --git a/scenario/node/src/utils.ts b/scenario/node/src/utils.ts
index 2c05a4ba0..3a6318c57 100644
--- a/scenario/node/src/utils.ts
+++ b/scenario/node/src/utils.ts
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { inspect, TextDecoder } from 'util';
+import { inspect, TextDecoder } from 'node:util';
 
 const utf8Decoder = new TextDecoder();