Use elliptic instead of @noble/curves for Node crypto (#629)

@noble/curves makes use of @noble/hashes, which does not function correctly on big-endian systems, and also throws an error on module load if a big-endian system is detected. Revert to using elliptic as the ECDSA signing implementation. Signed-off-by: Mark S. Lewis <[email protected]>
hyperledger · Sep 11, 2023 · f75f440 · f75f440
1 parent 5fe4c4f
commit f75f440
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 36 deletions.
diff --git a/node/package.json b/node/package.json
@@ -33,27 +33,30 @@
     "dependencies": {
         "@grpc/grpc-js": "^1.9.0",
         "@hyperledger/fabric-protos": "^0.2.0",
-        "@noble/curves": "^1.1.0",
+        "asn1.js": "^5.4.1",
+        "bn.js": "^5.2.1",
+        "elliptic": "^6.5.4",
         "google-protobuf": "^3.21.0"
     },
     "optionalDependencies": {
         "pkcs11js": "^1.3.0"
     },
     "devDependencies": {
-        "@cyclonedx/cyclonedx-npm": "^1.13.0",
-        "@tsconfig/node16": "^16.1.0",
+        "@cyclonedx/cyclonedx-npm": "^1.14.0",
+        "@tsconfig/node16": "^16.1.1",
+        "@types/elliptic": "^6.4.14",
         "@types/google-protobuf": "^3.15.6",
-        "@types/jest": "^29.5.3",
-        "@types/node": "^16.18.39",
-        "@typescript-eslint/eslint-plugin": "^6.2.1",
-        "@typescript-eslint/parser": "^6.2.1",
-        "eslint": "^8.46.0",
+        "@types/jest": "^29.5.4",
+        "@types/node": "^16.18.48",
+        "@typescript-eslint/eslint-plugin": "^6.6.0",
+        "@typescript-eslint/parser": "^6.6.0",
+        "eslint": "^8.49.0",
         "eslint-plugin-jest": "^27.2.3",
         "eslint-plugin-tsdoc": "^0.2.17",
-        "jest": "^29.6.2",
+        "jest": "^29.6.4",
         "npm-run-all": "^4.1.5",
         "ts-jest": "^29.1.1",
-        "typedoc": "^0.24.8",
-        "typescript": "~5.1.6"
+        "typedoc": "^0.25.1",
+        "typescript": "~5.2.2"
     }
 }
diff --git a/node/src/dependency.test.ts b/node/src/dependency.test.ts
@@ -4,12 +4,14 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { generateKeyPairSync } from 'crypto';
+import { generateKeyPairSync } from 'node:crypto';
+import { dirname, sep as pathSeparator } from 'node:path';
 import type { signers as SignersType } from '.';
 
 function isLoaded(moduleName: string): boolean {
     const moduleFile = require.resolve(moduleName);
-    return !!Object.values(require.cache).find(m => m?.filename === moduleFile);
+    const moduleDir = dirname(moduleFile) + pathSeparator;
+    return !!Object.values(require.cache).find(m => m?.filename.startsWith(moduleDir));
 }
 
 describe('optional pkcs11js dependency', () => {

diff --git a/node/src/identity/asn1.ts b/node/src/identity/asn1.ts
@@ -0,0 +1,22 @@
+/*
+ * 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
@@ -4,19 +4,22 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { CurveFn } from '@noble/curves/abstract/weierstrass';
-import { p256 } from '@noble/curves/p256';
-import { p384 } from '@noble/curves/p384';
+import BN from 'bn.js';
 import { KeyObject } from 'crypto';
+import { ec as EC } from 'elliptic';
+import { ecSignatureAsDER } from './asn1';
 import { Signer } from './signer';
 
-const namedCurves: Record<string, CurveFn> = {
-    'P-256': p256,
-    'P-384': p384,
+const namedCurves: Record<string, EC> = {
+    'P-256': new EC('p256'),
+    'P-384': new EC('p384'),
 };
 
 export function newECPrivateKeySigner(key: KeyObject): Signer {
     const { crv, d } = key.export({ format: 'jwk' });
+    if (!crv) {
+        throw new Error('Missing EC curve name');
+    }
     if (!d) {
         throw new Error('Missing EC private key value');
     }
@@ -25,16 +28,48 @@ export function newECPrivateKeySigner(key: KeyObject): Signer {
     const privateKey = Buffer.from(d, 'base64url');
 
     return (digest) => {
-        const signature = curve.sign(digest, privateKey, { lowS: true }).toDERRawBytes();
-        return Promise.resolve(signature);
+        const signature = curve.sign(digest, privateKey, { canonical: true });
+        const signatureBytes = new Uint8Array(signature.toDER());
+        return Promise.resolve(signatureBytes);
     };
 }
 
-function getCurve(name = String(undefined)): CurveFn {
+function getCurve(name: string): EC {
     const curve = namedCurves[name];
     if (!curve) {
         throw new Error(`Unsupported curve: ${name}`);
     }
 
     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.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 {
@@ -94,13 +94,11 @@ export class HSMSignerFactoryImpl implements HSMSignerFactory {
             throw err;
         }
 
-        const compactSignatureLength = p256.CURVE.nByteLength * 2;
-
         return {
             signer: (digest) => {
                 pkcs11.C_SignInit(session, { mechanism: pkcs11js.CKM_ECDSA }, privateKeyHandle);
-                const compactSignature = pkcs11.C_Sign(session, Buffer.from(digest), Buffer.alloc(compactSignatureLength));
-                const signature = p256.Signature.fromCompact(compactSignature).normalizeS().toDERRawBytes();
+                const compactSignature = pkcs11.C_Sign(session, Buffer.from(digest), Buffer.alloc(256));
+                const signature = new ECSignature('P-256', compactSignature).normalise().toDER();
                 return Promise.resolve(signature);
             },
             close: () => pkcs11.C_CloseSession(session),

diff --git a/node/src/identity/signers.ts b/node/src/identity/signers.ts
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { KeyObject, sign } from 'crypto';
+import { KeyObject, sign } from 'node:crypto';
 import { newECPrivateKeySigner } from './ecdsa';
 import { HSMSignerFactory, type HSMSignerFactoryImpl as HSMSignerFactoryImplType } from './hsmsigner';
 import { Signer } from './signer';
@@ -24,6 +24,7 @@ import { Signer } from './signer';
  *
  * The Ed25519 signer operates on the full message content, and should be combined with a `none` (or no-op) hash
  * implementation to ensure the complete message is passed to the signer.
+ *
  * @param key - A private key.
  * @returns A signing implementation.
  */

diff --git a/scenario/node/package.json b/scenario/node/package.json
@@ -23,15 +23,15 @@
         "@hyperledger/fabric-protos": "^0.2.0"
     },
     "devDependencies": {
-        "@cucumber/cucumber": "^9.3.0",
-        "@tsconfig/node16": "^16.1.0",
-        "@types/node": "^16.18.39",
-        "@typescript-eslint/eslint-plugin": "^6.2.1",
-        "@typescript-eslint/parser": "^6.2.1",
+        "@cucumber/cucumber": "^9.5.1",
+        "@tsconfig/node16": "^16.1.1",
+        "@types/node": "^16.18.48",
+        "@typescript-eslint/eslint-plugin": "^6.6.0",
+        "@typescript-eslint/parser": "^6.6.0",
         "cucumber-console-formatter": "^1.0.0",
-        "eslint": "^8.46.0",
-        "expect": "^29.6.2",
+        "eslint": "^8.49.0",
+        "expect": "^29.6.4",
         "npm-run-all": "^4.1.5",
-        "typescript": "~5.1.6"
+        "typescript": "~5.2.2"
     }
 }