Key validation: use WebCrypto API when available for curve25519

For Ed25519/Ed25519Legacy native validation code does a sign-verify check over random data.
This is faster than re-deriving the public point using tweetnacl.
If the native implementation is not available, we fall back to re-deriving
the public point only.

For X25519/Curve25519Legacy, both the native and fallback flows do an ecdh exchange;
in the fallback case, this results in slower performance compared to the existing check,
but encryption subkeys are hardly ever validated directly (only in case of gnu-dummy keys),
and this solution keeps the code simpler.

Separately, all validation tests have been updated to use valid params from a different
key, rather than corrupted parameters.

src/crypto/public_key/elliptic/ecdh_x.js

@@ -82,14 +82,19 @@ export async function generate(algo) {
 */
 export async function validateParams(algo, A, k) {
   switch (algo) {
-    case enums.publicKey.x25519: {
+    case enums.publicKey.x25519:
-      /**
+      // Validation is typically not run for ECDH, since encryption subkeys are only validated
-       * Derive public point A' from private key
+      // for gnu-dummy keys.
-       * and expect A == A'
+      // So, for simplicity, we do an encrypt-decrypt round even if WebCrypto support is not available
-       */
+      try {
-      const { publicKey } = x25519.box.keyPair.fromSecretKey(k);
+        const { ephemeralPublicKey, sharedSecret } = await generateEphemeralEncryptionMaterial(algo, A);
-      return util.equalsUint8Array(A, publicKey);
+        const recomputedSharedSecret = await recomputeSharedSecret(algo, ephemeralPublicKey, A, k);
-    }
+
+        return util.equalsUint8Array(sharedSecret, recomputedSharedSecret);
+      } catch (_) {
+        return false;
+      }
+
     case enums.publicKey.x448: {
       const x448 = await util.getNobleCurve(enums.publicKey.x448);
       /**

src/crypto/public_key/elliptic/eddsa.js

@@ -23,7 +23,7 @@
 import ed25519 from '@openpgp/tweetnacl';
 import util from '../../../util';
 import enums from '../../../enums';
-import { getHashByteLength } from '../../hash';
+import { computeDigest, getHashByteLength } from '../../hash';
 import { getRandomBytes } from '../../random';
 import { b64ToUint8Array, uint8ArrayToB64 } from '../../../encoding/base64';
 
@@ -179,15 +179,34 @@ export async function verify(algo, hashAlgo, { RS }, m, publicKey, hashed) {
  */
 export async function validateParams(algo, A, seed) {
   switch (algo) {
-    case enums.publicKey.ed25519: {
+    case enums.publicKey.ed25519:
-      /**
+      // If webcrypto support is available, we sign-verify random data, as the import-export
-       * Derive public point A' from private key
+      // functions might not implement validity checks.
-       * and expect A == A'
+      // If we need to fallback to JS, we instead only re-derive the public key,
-       * TODO: move to sign-verify using WebCrypto (same as ECDSA) when curve is more widely implemented
+      // as this is much faster than sign-verify.
-       */
+      try {
-      const { publicKey } = ed25519.sign.keyPair.fromSeed(seed);
+        const webCrypto = util.getWebCrypto();
-      return util.equalsUint8Array(A, publicKey);
+        const jwkPrivate = privateKeyToJWK(algo, A, seed);
-    }
+        const jwkPublic = publicKeyToJWK(algo, A);
+
+        const privateCryptoKey = await webCrypto.importKey('jwk', jwkPrivate, 'Ed25519', false, ['sign']);
+        const publicCryptoKey = await webCrypto.importKey('jwk', jwkPublic, 'Ed25519', false, ['verify']);
+
+        const randomData = getRandomBytes(8);
+        const signature = new Uint8Array(
+          await webCrypto.sign('Ed25519', privateCryptoKey, randomData)
+        );
+
+        const verified = await webCrypto.verify('Ed25519', publicCryptoKey, signature, randomData);
+        return verified;
+      } catch (err) {
+        if (err.name !== 'NotSupportedError') {
+          return false;
+        }
+
+        const { publicKey } = ed25519.sign.keyPair.fromSeed(seed);
+        return util.equalsUint8Array(A, publicKey);
+      }
 
     case enums.publicKey.ed448: {
       const ed448 = await util.getNobleCurve(enums.publicKey.ed448);

src/crypto/public_key/elliptic/eddsa_legacy.js

@@ -21,12 +21,11 @@
  * @module crypto/public_key/elliptic/eddsa_legacy
  */
 
-import nacl from '@openpgp/tweetnacl';
 import util from '../../../util';
 import enums from '../../../enums';
 import { getHashByteLength } from '../../hash';
 import { CurveWithOID, checkPublicPointEnconding } from './oid_curves';
-import { sign as eddsaSign, verify as eddsaVerify } from './eddsa';
+import { sign as eddsaSign, verify as eddsaVerify, validateParams as eddsaValidateParams } from './eddsa';
 
 /**
  * Sign a message using the provided legacy EdDSA key
@@ -97,12 +96,9 @@ export async function validateParams(oid, Q, k) {
     return false;
   }
 
-  /**
+  // First byte is relevant for encoding purposes only
-   * Derive public point Q' = dG from private key
+  if (Q.length < 1 || Q[0] !== 0x40) {
-   * and expect Q == Q'
+    return false;
-   */
+  }
-  const { publicKey } = nacl.sign.keyPair.fromSeed(k);
+  return eddsaValidateParams(enums.publicKey.ed25519, Q.subarray(1), k);
-  const dG = new Uint8Array([0x40, ...publicKey]); // Add public key prefix
-  return util.equalsUint8Array(Q, dG);
-
 }

src/crypto/public_key/elliptic/oid_curves.js

@@ -26,7 +26,7 @@ import { uint8ArrayToB64, b64ToUint8Array } from '../../../encoding/base64';
 import OID from '../../../type/oid';
 import { UnsupportedError } from '../../../packet/packet';
 import { generate as eddsaGenerate } from './eddsa';
-import { generate as ecdhXGenerate } from './ecdh_x';
+import { generate as ecdhXGenerate, validateParams as ecdhXValidateParams } from './ecdh_x';
 
 const webCrypto = util.getWebCrypto();
 const nodeCrypto = util.getNodeCrypto();
@@ -252,17 +252,12 @@ async function validateStandardParams(algo, oid, Q, d) {
   }
 
   if (curveName === enums.curve.curve25519Legacy) {
-    d = d.slice().reverse();
+    const dLittleEndian = d.slice().reverse();
-    // Re-derive public point Q'
+    // First byte is relevant for encoding purposes only
-    const { publicKey } = nacl.box.keyPair.fromSecretKey(d);
+    if (Q.length < 1 || Q[0] !== 0x40) {
-
-    Q = new Uint8Array(Q);
-    const dG = new Uint8Array([0x40, ...publicKey]); // Add public key prefix
-    if (!util.equalsUint8Array(dG, Q)) {
       return false;
     }
-
+    return ecdhXValidateParams(enums.publicKey.x25519, Q.subarray(1), dLittleEndian);
-    return true;
   }
 
   const nobleCurve = await util.getNobleCurve(enums.publicKey.ecdsa, curveName); // excluding curve25519Legacy, ecdh and ecdsa use the same curves

test/crypto/validate.js

@@ -90,8 +90,10 @@ async function generatePrivateKeyObject(options) {
 export default () => {
   describe('EdDSA parameter validation (legacy format)', function() {
     let eddsaKey;
+    let anotherEddsaKey;
     before(async () => {
       eddsaKey = await generatePrivateKeyObject({ curve: 'ed25519Legacy' });
+      anotherEddsaKey = await generatePrivateKeyObject({ curve: 'ed25519Legacy' });
     });
 
     it('EdDSA params should be valid', async function() {
@@ -100,11 +102,10 @@ export default () => {
 
     it('detect invalid edDSA Q', async function() {
       const eddsaKeyPacket = await cloneKeyPacket(eddsaKey);
-      const Q = eddsaKeyPacket.publicParams.Q;
+      eddsaKeyPacket.publicParams.Q = anotherEddsaKey.keyPacket.publicParams.Q;
-      Q[0]++;
       await expect(eddsaKeyPacket.validate()).to.be.rejectedWith('Key is invalid');
 
-      const infQ = new Uint8Array(Q.length);
+      const infQ = new Uint8Array(eddsaKeyPacket.publicParams.Q.length);
       eddsaKeyPacket.publicParams.Q = infQ;
       await expect(eddsaKeyPacket.validate()).to.be.rejectedWith('Key is invalid');
     });
@@ -198,13 +199,19 @@ export default () => {
     describe(`ECC ${curve} parameter validation`, () => {
       let ecdsaKey;
       let ecdhKey;
+      let anotherEcdsaKey;
+      let anotherEcdhKey;
       before(async () => {
         if (curve !== 'curve25519Legacy') {
           ecdsaKey = await generatePrivateKeyObject({ curve });
           ecdhKey = ecdsaKey.subkeys[0];
+          anotherEcdsaKey = await generatePrivateKeyObject({ curve });
+          anotherEcdhKey = anotherEcdsaKey.subkeys[0];
         } else {
           const eddsaKey = await generatePrivateKeyObject({ curve: 'ed25519Legacy' });
           ecdhKey = eddsaKey.subkeys[0];
+          const anotherEddsaKey = await generatePrivateKeyObject({ curve: 'ed25519Legacy' });
+          anotherEcdhKey = anotherEddsaKey.subkeys[0];
         }
       });
 
@@ -220,10 +227,9 @@ export default () => {
           this.skip();
         }
         const keyPacket = await cloneKeyPacket(ecdsaKey);
-        const Q = keyPacket.publicParams.Q;
+        keyPacket.publicParams.Q = anotherEcdsaKey.keyPacket.publicParams.Q;
-        Q[16]++;
         await expect(keyPacket.validate()).to.be.rejectedWith('Key is invalid');
-        const infQ = new Uint8Array(Q.length);
+        const infQ = new Uint8Array(anotherEcdsaKey.keyPacket.publicParams.Q.length);
         infQ[0] = 4;
         keyPacket.publicParams.Q = infQ;
         await expect(keyPacket.validate()).to.be.rejectedWith('Key is invalid');
@@ -235,11 +241,10 @@ export default () => {
 
       it(`ECDH ${curve} - detect invalid Q`, async function() {
         const keyPacket = await cloneKeyPacket(ecdhKey);
-        const Q = keyPacket.publicParams.Q;
+        keyPacket.publicParams.Q = anotherEcdhKey.keyPacket.publicParams.Q;
-        Q[16]++;
         await expect(keyPacket.validate()).to.be.rejectedWith('Key is invalid');
 
-        const infQ = new Uint8Array(Q.length);
+        const infQ = new Uint8Array(keyPacket.publicParams.Q.length);
         keyPacket.publicParams.Q = infQ;
         infQ[0] = 4;
         await expect(keyPacket.validate()).to.be.rejectedWith('Key is invalid');
@@ -252,9 +257,13 @@ export default () => {
     describe(`Ed${curveID}/X${curveID} parameter validation`, function() {
       let eddsaKey;
       let ecdhXKey;
+      let anotherEddsaKey;
+      let anotherEcdhXKey;
       before(async () => {
         eddsaKey = await generatePrivateKeyObject({ type: `curve${curveID}` });
         ecdhXKey = eddsaKey.subkeys[0];
+        anotherEddsaKey = await generatePrivateKeyObject({ type: `curve${curveID}` });
+        anotherEcdhXKey = anotherEddsaKey.subkeys[0];
       });
 
       it(`Ed${curveID} params should be valid`, async function() {
@@ -263,11 +272,10 @@ export default () => {
 
       it(`detect invalid Ed${curveID} public point`, async function() {
         const eddsaKeyPacket = await cloneKeyPacket(eddsaKey);
-        const A = eddsaKeyPacket.publicParams.A;
+        eddsaKeyPacket.publicParams.A = anotherEddsaKey.keyPacket.publicParams.A;
-        A[0]++;
         await expect(eddsaKeyPacket.validate()).to.be.rejectedWith('Key is invalid');
 
-        const infA = new Uint8Array(A.length);
+        const infA = new Uint8Array(eddsaKeyPacket.publicParams.A.length);
         eddsaKeyPacket.publicParams.A = infA;
         await expect(eddsaKeyPacket.validate()).to.be.rejectedWith('Key is invalid');
       });
@@ -278,11 +286,10 @@ export default () => {
 
       it(`detect invalid X${curveID} public point`, async function() {
         const ecdhXKeyPacket = await cloneKeyPacket(ecdhXKey);
-        const A = ecdhXKeyPacket.publicParams.A;
+        ecdhXKeyPacket.publicParams.A = anotherEcdhXKey.keyPacket.publicParams.A;
-        A[0]++;
         await expect(ecdhXKeyPacket.validate()).to.be.rejectedWith('Key is invalid');
 
-        const infA = new Uint8Array(A.length);
+        const infA = new Uint8Array(ecdhXKeyPacket.publicParams.A.length);
         ecdhXKeyPacket.publicParams.A = infA;
         await expect(ecdhXKeyPacket.validate()).to.be.rejectedWith('Key is invalid');
       });
@@ -291,8 +298,10 @@ export default () => {
 
   describe('RSA parameter validation', function() {
     let rsaKey;
+    let anotherRsaKey;
     before(async () => {
       rsaKey = await generatePrivateKeyObject({ type: 'rsa', rsaBits: 2048 });
+      anotherRsaKey = await generatePrivateKeyObject({ type: 'rsa', rsaBits: 2048 });
     });
 
     it('generated RSA params are valid', async function() {
@@ -301,15 +310,14 @@ export default () => {
 
     it('detect invalid RSA n', async function() {
       const keyPacket = await cloneKeyPacket(rsaKey);
-      const n = keyPacket.publicParams.n;
+      keyPacket.publicParams.n = anotherRsaKey.keyPacket.publicParams.n;
-      n[0]++;
       await expect(keyPacket.validate()).to.be.rejectedWith('Key is invalid');
     });
 
     it('detect invalid RSA e', async function() {
       const keyPacket = await cloneKeyPacket(rsaKey);
       const e = keyPacket.publicParams.e;
-      e[0]++;
+      e[0]++; // e is hard-coded so we don't take it from `anotherRsaKey`
       await expect(keyPacket.validate()).to.be.rejectedWith('Key is invalid');
     });
   });