Allow creating x25519 key pairs from JS (microsoft#5846)

takuro-sato · Nov 28, 2023 · 3882284 · 3882284
1 parent cd069ab
commit 3882284
Show file tree

Hide file tree

Showing 11 changed files with 183 additions and 64 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [5.0.0-dev8]
+
+NOTE: UNRELEASED
+
+- `ccf.crypto.generateEddsaKeyPair`, `pubEddsaPemToJwk` and `eddsaPemToJwk` now support `x25519` as well as `curve25519` (#5846).
+
+[5.0.0-dev8]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev8
+
+- `POST /recovery/members/{memberId}:recover` is now authenticated by COSE Sign1, making it consistent with the other `POST` endpoints in governance, and avoiding a potential denial of service where un-authenticated and un-authorised clients could submit invalid shares repeatedly. The `submit_recovery_share.sh` script has been amended accordingly, and now takes a `--member-id-privk` and `--member-id-cert` (#5821).
+
 ## [5.0.0-dev7]
 
 [5.0.0-dev7]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev7

diff --git a/include/ccf/crypto/curve.h b/include/ccf/crypto/curve.h
@@ -25,7 +25,8 @@ namespace crypto
     /// The SECP256K1 curve
     SECP256K1,
     /// The CURVE25519 curve
-    CURVE25519
+    CURVE25519,
+    X25519
   };
 
   DECLARE_JSON_ENUM(
@@ -34,7 +35,8 @@ namespace crypto
      {CurveID::SECP384R1, "Secp384R1"},
      {CurveID::SECP256R1, "Secp256R1"},
      {CurveID::SECP256K1, "Secp256K1"},
-     {CurveID::CURVE25519, "Curve25519"}});
+     {CurveID::CURVE25519, "Curve25519"},
+     {CurveID::X25519, "X25519"}});
 
   static constexpr CurveID service_identity_curve_choice = CurveID::SECP384R1;
   // SNIPPET_END: supported_curves

diff --git a/include/ccf/crypto/jwk.h b/include/ccf/crypto/jwk.h
@@ -82,17 +82,22 @@ namespace crypto
 
   enum class JsonWebKeyEdDSACurve
   {
-    ED25519 = 0
+    ED25519 = 0,
+    X25519 = 1
   };
   DECLARE_JSON_ENUM(
-    JsonWebKeyEdDSACurve, {{JsonWebKeyEdDSACurve::ED25519, "Ed25519"}});
+    JsonWebKeyEdDSACurve,
+    {{JsonWebKeyEdDSACurve::ED25519, "Ed25519"},
+     {JsonWebKeyEdDSACurve::X25519, "X25519"}});
 
   static JsonWebKeyEdDSACurve curve_id_to_jwk_eddsa_curve(CurveID curve_id)
   {
     switch (curve_id)
     {
       case CurveID::CURVE25519:
         return JsonWebKeyEdDSACurve::ED25519;
+      case CurveID::X25519:
+        return JsonWebKeyEdDSACurve::X25519;
       default:
         throw std::logic_error(fmt::format("Unknown EdDSA curve {}", curve_id));
     }

diff --git a/js/ccf-app/src/global.ts b/js/ccf-app/src/global.ts
@@ -371,7 +371,7 @@ export interface CCFCrypto {
   /**
    * Generate an EdDSA key pair.
    *
-   * @param curve The name of the curve. Currently only "curve25519" is supported.
+   * @param curve The name of the curve. Only "curve25519" and "x25519" are supported.
    */
   generateEddsaKeyPair(curve: string): CryptoKeyPair;
 
@@ -453,7 +453,7 @@ export interface CCFCrypto {
 
   /**
    * Converts an EdDSA public key as PEM to JSON Web Key (JWK) object.
-   * Currently only Curve25519 is supported.
+   * Only Curve25519 and X25519 are supported.
    *
    * @param pem EdDSA public key as PEM
    * @param kid Key identifier (optional)
@@ -462,7 +462,7 @@ export interface CCFCrypto {
 
   /**
    * Converts an EdDSA private key as PEM to JSON Web Key (JWK) object.
-   * Currently only Curve25519 is supported.
+   * Only Curve25519 and X25519 are supported.
    *
    * @param pem EdDSA private key as PEM
    * @param kid Key identifier (optional)

diff --git a/js/ccf-app/src/polyfill.ts b/js/ccf-app/src/polyfill.ts
@@ -252,19 +252,33 @@ class CCFPolyfill implements CCF {
       return ecdsaKeyPair;
     },
     generateEddsaKeyPair(curve: string): CryptoKeyPair {
-      // `type` is always "ed25519" because currently only "curve25519" is supported for `curve`.
-      const type = "ed25519";
-      const ecdsaKeyPair = jscrypto.generateKeyPairSync(type, {
-        publicKeyEncoding: {
-          type: "spki",
-          format: "pem",
-        },
-        privateKeyEncoding: {
-          type: "pkcs8",
-          format: "pem",
-        },
-      });
-      return ecdsaKeyPair;
+      if (curve === "curve25519") {
+        return jscrypto.generateKeyPairSync("ed25519", {
+          publicKeyEncoding: {
+            type: "spki",
+            format: "pem",
+          },
+          privateKeyEncoding: {
+            type: "pkcs8",
+            format: "pem",
+          },
+        });
+      } else {
+        if (curve !== "x25519")
+          throw new Error(
+            "Unsupported curve for EdDSA key pair generation: " + curve,
+          );
+        return jscrypto.generateKeyPairSync("x25519", {
+          publicKeyEncoding: {
+            type: "spki",
+            format: "pem",
+          },
+          privateKeyEncoding: {
+            type: "pkcs8",
+            format: "pem",
+          },
+        });
+      }
     },
     wrapKey(
       key: ArrayBuffer,

diff --git a/js/ccf-app/test/polyfill.test.ts b/js/ccf-app/test/polyfill.test.ts
@@ -76,6 +76,13 @@ describe("polyfill", function () {
       assert.isTrue(pair.privateKey.startsWith("-----BEGIN PRIVATE KEY-----"));
     });
   });
+  describe("generateEddsaKeyPair/X25519", function () {
+    it("generates a random EdDSA X25519 key pair", function () {
+      const pair = ccf.crypto.generateEddsaKeyPair("x25519");
+      assert.isTrue(pair.publicKey.startsWith("-----BEGIN PUBLIC KEY-----"));
+      assert.isTrue(pair.privateKey.startsWith("-----BEGIN PRIVATE KEY-----"));
+    });
+  });
   describe("wrapKey", function () {
     it("performs RSA-OAEP wrapping correctly", function () {
       const key = ccf.crypto.generateAesKey(128);
@@ -608,7 +615,7 @@ describe("polyfill", function () {
         assert.equal(pem, pair.privateKey);
       }
     });
-    it("EdDSA", function () {
+    it("Ed25119", function () {
       const my_kid = "my_kid";
       const pair = ccf.crypto.generateEddsaKeyPair("curve25519");
       {
@@ -640,6 +647,38 @@ describe("polyfill", function () {
         assert.equal(pem, pair.privateKey);
       }
     });
+    it("X25119", function () {
+      const my_kid = "my_kid";
+      const pair = ccf.crypto.generateEddsaKeyPair("x25519");
+      {
+        const jwk = ccf.crypto.pubEddsaPemToJwk(pair.publicKey);
+        assert.equal(jwk.kty, "OKP");
+        assert.notEqual(jwk.kid, my_kid);
+        const pem = ccf.crypto.pubEddsaJwkToPem(jwk);
+        assert.equal(pem, pair.publicKey);
+      }
+      {
+        const jwk = ccf.crypto.pubEddsaPemToJwk(pair.publicKey, my_kid);
+        assert.equal(jwk.kty, "OKP");
+        assert.equal(jwk.kid, my_kid);
+        const pem = ccf.crypto.pubEddsaJwkToPem(jwk);
+        assert.equal(pem, pair.publicKey);
+      }
+      {
+        const jwk = ccf.crypto.eddsaPemToJwk(pair.privateKey);
+        assert.equal(jwk.kty, "OKP");
+        assert.notEqual(jwk.kid, my_kid);
+        const pem = ccf.crypto.eddsaJwkToPem(jwk);
+        assert.equal(pem, pair.privateKey);
+      }
+      {
+        const jwk = ccf.crypto.eddsaPemToJwk(pair.privateKey, my_kid);
+        assert.equal(jwk.kty, "OKP");
+        assert.equal(jwk.kid, my_kid);
+        const pem = ccf.crypto.eddsaJwkToPem(jwk);
+        assert.equal(pem, pair.privateKey);
+      }
+    });
   });
   describe("kv", function () {
     it("basic", function () {

diff --git a/src/crypto/openssl/eddsa_key_pair.cpp b/src/crypto/openssl/eddsa_key_pair.cpp
@@ -34,16 +34,26 @@ namespace crypto
         "Cannot construct EdDSA key pair from non-OKP JWK");
     }
 
-    if (jwk.crv != JsonWebKeyEdDSACurve::ED25519)
+    int curve = 0;
+    if (jwk.crv == JsonWebKeyEdDSACurve::ED25519)
+    {
+      curve = EVP_PKEY_ED25519;
+    }
+    else if (jwk.crv == JsonWebKeyEdDSACurve::X25519)
+    {
+      curve = EVP_PKEY_X25519;
+    }
+    else
     {
       throw std::logic_error(
-        "Cannot construct EdDSA key pair from non-Ed25519 JWK");
+        "Cannot construct EdDSA key pair from JWK that is neither Ed25519 nor "
+        "X25519");
     }
 
     auto d_raw = raw_from_b64url(jwk.d);
     OpenSSL::CHECKNULL(
       key = EVP_PKEY_new_raw_private_key(
-        EVP_PKEY_ED25519, nullptr, d_raw.data(), d_raw.size()));
+        curve, nullptr, d_raw.data(), d_raw.size()));
   }
 
   Pem EdDSAKeyPair_OpenSSL::private_key_pem() const

diff --git a/src/crypto/openssl/eddsa_public_key.cpp b/src/crypto/openssl/eddsa_public_key.cpp
@@ -28,15 +28,25 @@ namespace crypto
         "Cannot construct EdDSA public key from non-OKP JWK");
     }
 
-    if (jwk.crv != JsonWebKeyEdDSACurve::ED25519)
+    int curve = 0;
+    if (jwk.crv == JsonWebKeyEdDSACurve::ED25519)
+    {
+      curve = EVP_PKEY_ED25519;
+    }
+    else if (jwk.crv == JsonWebKeyEdDSACurve::X25519)
+    {
+      curve = EVP_PKEY_X25519;
+    }
+    else
     {
       throw std::logic_error(
-        "Cannot construct EdDSA public key from non-Ed25519 JWK");
+        "Cannot construct EdDSA key pair from JWK that is neither Ed25519 nor "
+        "X25519");
     }
 
     auto x_raw = raw_from_b64url(jwk.x);
-    key = EVP_PKEY_new_raw_public_key(
-      EVP_PKEY_ED25519, nullptr, x_raw.data(), x_raw.size());
+    key =
+      EVP_PKEY_new_raw_public_key(curve, nullptr, x_raw.data(), x_raw.size());
     if (key == nullptr)
     {
       throw std::logic_error("Error constructing EdDSA public key from JWK");
@@ -83,6 +93,8 @@ namespace crypto
     {
       case CurveID::CURVE25519:
         return EVP_PKEY_ED25519;
+      case CurveID::X25519:
+        return EVP_PKEY_X25519;
       default:
         throw std::logic_error(
           fmt::format("unsupported OpenSSL CurveID {}", gid));
@@ -97,6 +109,8 @@ namespace crypto
     {
       case NID_ED25519:
         return CurveID::CURVE25519;
+      case NID_X25519:
+        return CurveID::X25519;
       default:
         throw std::runtime_error(fmt::format("Unknown OpenSSL curve {}", nid));
     }

diff --git a/src/js/crypto.cpp b/src/js/crypto.cpp
@@ -186,10 +186,14 @@ namespace ccf::js
     {
       cid = crypto::CurveID::CURVE25519;
     }
+    else if (curve == "x25519")
+    {
+      cid = crypto::CurveID::X25519;
+    }
     else
     {
       return JS_ThrowRangeError(
-        ctx, "Unsupported curve id, supported: curve25519");
+        ctx, "Unsupported curve id, supported: curve25519, x25519");
     }
 
     try

diff --git a/tests/infra/crypto.py b/tests/infra/crypto.py
@@ -16,7 +16,7 @@
     load_pem_x509_certificate,
     load_der_x509_certificate,
 )
-from cryptography.hazmat.primitives.asymmetric import ec, rsa, padding, ed25519
+from cryptography.hazmat.primitives.asymmetric import ec, rsa, padding, ed25519, x25519
 from cryptography.hazmat.primitives.asymmetric.utils import (
     decode_dss_signature,
     encode_dss_signature,
@@ -100,9 +100,14 @@ def generate_ec_keypair(curve: ec.EllipticCurve = ec.SECP256R1) -> Tuple[str, st
     return priv_pem, pub_pem
 
 
-def generate_eddsa_keypair() -> Tuple[str, str]:
-    # Currently only Curve25519 is supported
-    priv = ed25519.Ed25519PrivateKey.generate()
+def generate_eddsa_keypair(curve: str) -> Tuple[str, str]:
+    key_class = {
+        "curve25519": ed25519.Ed25519PrivateKey,
+        "x25519": x25519.X25519PrivateKey,
+    }
+    if curve not in key_class:
+        raise ValueError(f"Unsupported curve: {curve}")
+    priv = key_class[curve].generate()
     pub = priv.public_key()
     priv_pem = priv.private_bytes(
         Encoding.PEM, PrivateFormat.PKCS8, NoEncryption()