Use noble-ed25519 over tweetnacl for signature verification (#16)

Much faster than tweetnacl, and no constant-timeness required. We are not using v2 for now, despite being smaller, because it relies on bigint literals, and it requires polyfilling the WebCrypto lib manually in Node < 19.
ProtonMail · Jul 5, 2024 · 7f396a5 · 7f396a5
1 parent dcf4096
commit 7f396a5
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 9 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -63,6 +63,7 @@
   },
   "devDependencies": {
     "@noble/curves": "^1.4.0",
+    "@noble/ed25519": "^1.7.3",
     "@noble/hashes": "^1.4.0",
     "@openpgp/asmcrypto.js": "^3.1.0",
     "@openpgp/jsdoc": "^3.6.11",

diff --git a/rollup.config.js b/rollup.config.js
@@ -79,6 +79,13 @@ export default Object.assign([
       commonjs({
         ignore: nodeBuiltinModules.concat(nodeDependencies)
       }),
+      replace({
+        include: 'node_modules/@noble/ed25519/**',
+        // Rollup ignores the `browser: { crypto: false }` directive in package.json, since `exports` are present,
+        // hence we need to manually drop it.
+        "import * as nodeCrypto from 'crypto'": 'const nodeCrypto = null',
+        delimiters: ['', '']
+      }),
       replace({
         'OpenPGP.js VERSION': `OpenPGP.js ${pkg.version}`,
         "import { createRequire } from 'module';": 'const createRequire = () => () => {}',
@@ -128,6 +135,13 @@ export default Object.assign([
       commonjs({
         ignore: nodeBuiltinModules.concat(nodeDependencies)
       }),
+      replace({
+        include: 'node_modules/@noble/ed25519/**',
+        // Rollup ignores the `browser: { crypto: false }` directive in package.json, since `exports` are present,
+        // hence we need to manually drop it.
+        "import * as nodeCrypto from 'crypto'": 'const nodeCrypto = null',
+        delimiters: ['', '']
+      }),
       replace({
         'OpenPGP.js VERSION': `OpenPGP.js ${pkg.version}`,
         "import { createRequire } from 'module';": 'const createRequire = () => () => {}',
@@ -158,6 +172,13 @@ export default Object.assign([
         ignore: nodeBuiltinModules.concat(nodeDependencies),
         requireReturnsDefault: 'preferred'
       }),
+      replace({
+        include: 'node_modules/@noble/ed25519/**',
+        // Rollup ignores the `browser: { crypto: false }` directive in package.json, since `exports` are present,
+        // hence we need to manually drop it.
+        "import * as nodeCrypto from 'crypto'": 'const nodeCrypto = null',
+        delimiters: ['', '']
+      }),
       replace({
         "import { createRequire } from 'module';": 'const createRequire = () => () => {}',
         delimiters: ['', '']

diff --git a/src/crypto/public_key/elliptic/eddsa.js b/src/crypto/public_key/elliptic/eddsa.js
@@ -20,7 +20,9 @@
  * @module crypto/public_key/elliptic/eddsa
  */
 
-import ed25519 from '@openpgp/tweetnacl';
+import naclEd25519 from '@openpgp/tweetnacl'; // better constant-timeness as it uses Uint8Arrays over BigInts
+import { verify as nobleEd25519Verify } from '@noble/ed25519';
+
 import util from '../../../util';
 import enums from '../../../enums';
 import hash from '../../hash';
@@ -36,7 +38,7 @@ export async function generate(algo) {
   switch (algo) {
     case enums.publicKey.ed25519: {
       const seed = getRandomBytes(getPayloadSize(algo));
-      const { publicKey: A } = ed25519.sign.keyPair.fromSeed(seed);
+      const { publicKey: A } = naclEd25519.sign.keyPair.fromSeed(seed);
       return { A, seed };
     }
     case enums.publicKey.ed448: {
@@ -70,7 +72,7 @@ export async function sign(algo, hashAlgo, message, publicKey, privateKey, hashe
   switch (algo) {
     case enums.publicKey.ed25519: {
       const secretKey = util.concatUint8Array([privateKey, publicKey]);
-      const signature = ed25519.sign.detached(hashed, secretKey);
+      const signature = naclEd25519.sign.detached(hashed, secretKey);
       return { RS: signature };
     }
     case enums.publicKey.ed448: {
@@ -101,7 +103,7 @@ export async function verify(algo, hashAlgo, { RS }, m, publicKey, hashed) {
   }
   switch (algo) {
     case enums.publicKey.ed25519:
-      return ed25519.sign.detached.verify(hashed, RS, publicKey);
+      return nobleEd25519Verify(RS, hashed, publicKey);
     case enums.publicKey.ed448: {
       const ed448 = await util.getNobleCurve(enums.publicKey.ed448);
       return ed448.verify(RS, hashed, publicKey);
@@ -126,7 +128,7 @@ export async function validateParams(algo, A, seed) {
        * Derive public point A' from private key
        * and expect A == A'
        */
-      const { publicKey } = ed25519.sign.keyPair.fromSeed(seed);
+      const { publicKey } = naclEd25519.sign.keyPair.fromSeed(seed);
       return util.equalsUint8Array(A, publicKey);
     }
 

diff --git a/src/crypto/public_key/elliptic/eddsa_legacy.js b/src/crypto/public_key/elliptic/eddsa_legacy.js
@@ -21,7 +21,8 @@
  * @module crypto/public_key/elliptic/eddsa_legacy
  */
 
-import nacl from '@openpgp/tweetnacl';
+import naclEd25519 from '@openpgp/tweetnacl'; // better constant-timeness as it uses Uint8Arrays over BigInts
+import { verify as nobleEd25519Verify } from '@noble/ed25519';
 import util from '../../../util';
 import enums from '../../../enums';
 import hash from '../../hash';
@@ -49,7 +50,7 @@ export async function sign(oid, hashAlgo, message, publicKey, privateKey, hashed
     throw new Error('Hash algorithm too weak for EdDSA.');
   }
   const secretKey = util.concatUint8Array([privateKey, publicKey.subarray(1)]);
-  const signature = nacl.sign.detached(hashed, secretKey);
+  const signature = naclEd25519.sign.detached(hashed, secretKey);
   // EdDSA signature params are returned in little-endian format
   return {
     r: signature.subarray(0, 32),
@@ -76,7 +77,7 @@ export async function verify(oid, hashAlgo, { r, s }, m, publicKey, hashed) {
     throw new Error('Hash algorithm too weak for EdDSA.');
   }
   const signature = util.concatUint8Array([r, s]);
-  return nacl.sign.detached.verify(hashed, signature, publicKey.subarray(1));
+  return nobleEd25519Verify(signature, hashed, publicKey.subarray(1));
 }
 /**
  * Validate legacy EdDSA parameters
@@ -96,7 +97,7 @@ export async function validateParams(oid, Q, k) {
    * Derive public point Q' = dG from private key
    * and expect Q == Q'
    */
-  const { publicKey } = nacl.sign.keyPair.fromSeed(k);
+  const { publicKey } = naclEd25519.sign.keyPair.fromSeed(k);
   const dG = new Uint8Array([0x40, ...publicKey]); // Add public key prefix
   return util.equalsUint8Array(Q, dG);