Use p1363 encoding for sign() JS API with ECDSA (#4829)

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 - `ccf::RpcContext::set_response()` has been renamed to `ccf::RpcContext::set_response_json()` (#4813).
 - The built-in authentication policies for JWTs and certs will now enforce expiry times, based on the current time received from the host. JWTs must contain "nbf" and "exp" claims, and if those are outside the current time then the request will get an authentication error (#4786).
+- `ccf.crypto.sign()` previously returned DER-encoded ECDSA signatures and now returns IEEE P1363 encoded signatures, aligning with the behavior of the Web Crypto API and `ccf.crypto.verifySignature()` (#4829).
 
 ## [4.0.0-dev3]
 

include/ccf/crypto/ecdsa.h

@@ -2,6 +2,8 @@
 // Licensed under the Apache 2.0 License.
 #pragma once
 
+#include "ccf/crypto/curve.h"
+
 #include <vector>
 
 namespace crypto
@@ -27,4 +29,7 @@ namespace crypto
    */
   std::vector<uint8_t> ecdsa_sig_p1363_to_der(
     const std::vector<uint8_t>& signature);
+
+  std::vector<uint8_t> ecdsa_sig_der_to_p1363(
+    const std::vector<uint8_t>& signature, CurveID curveId);
 }

js/ccf-app/test/polyfill.test.ts

@@ -166,6 +166,19 @@ describe("polyfill", function () {
         );
       }
 
+      // Also `signature` should be verified successfully with the JS API
+      assert.isTrue(
+        ccf.crypto.verifySignature(
+          {
+            name: "RSASSA-PKCS1-v1_5",
+            hash: "SHA-256",
+          },
+          publicKey,
+          signature,
+          data
+        )
+      );
+
       {
         const verifier = crypto.createVerify("SHA256");
         verifier.update("bar");
@@ -218,6 +231,19 @@ describe("polyfill", function () {
         );
       }
 
+      // Also `signature` should be verified successfully with the JS API
+      assert.isTrue(
+        ccf.crypto.verifySignature(
+          {
+            name: "ECDSA",
+            hash: "SHA-256",
+          },
+          publicKey,
+          signature,
+          data
+        )
+      );
+
       {
         const verifier = crypto.createVerify("SHA256");
         verifier.update("bar");
@@ -262,6 +288,18 @@ describe("polyfill", function () {
         )
       );
 
+      // Also `signature` should be verified successfully with the JS API
+      assert.isTrue(
+        ccf.crypto.verifySignature(
+          {
+            name: "EdDSA",
+          },
+          publicKey,
+          signature,
+          data
+        )
+      );
+
       assert.isFalse(
         crypto.verify(
           null,

src/crypto/ecdsa.cpp

@@ -3,6 +3,7 @@
 #include "ccf/crypto/ecdsa.h"
 
 #include "crypto/openssl/openssl_wrappers.h"
+#include "crypto/openssl/public_key.h"
 
 #include <openssl/bn.h>
 #include <openssl/ecdsa.h>
@@ -50,4 +51,25 @@ namespace crypto
     return ecdsa_sig_from_r_s(
       signature.data(), half_size, signature.data() + half_size, half_size);
   }
+
+  std::vector<uint8_t> ecdsa_sig_der_to_p1363(
+    const std::vector<uint8_t>& signature, CurveID curveId)
+  {
+    auto sig_ptr = signature.data();
+    OpenSSL::Unique_ECDSA_SIG ecdsa_sig(
+      d2i_ECDSA_SIG(NULL, &sig_ptr, signature.size()));
+    // r and s are managed by Unique_ECDSA_SIG object, so we shouldn't use
+    // Unique_BIGNUM for them
+    const BIGNUM* r = ECDSA_SIG_get0_r(ecdsa_sig);
+    const BIGNUM* s = ECDSA_SIG_get0_s(ecdsa_sig);
+    const int nid = PublicKey_OpenSSL::get_openssl_group_id(curveId);
+    OpenSSL::Unique_EC_GROUP ec_group(nid);
+    const int group_order_bits = EC_GROUP_order_bits(ec_group);
+    OpenSSL::CHECKPOSITIVE(group_order_bits);
+    const size_t n = (group_order_bits + 7) / 8;
+    std::vector<uint8_t> sig_p1363(n * 2);
+    OpenSSL::CHECKEQUAL(n, BN_bn2binpad(r, sig_p1363.data(), n));
+    OpenSSL::CHECKEQUAL(n, BN_bn2binpad(s, sig_p1363.data() + n, n));
+    return sig_p1363;
+  }
 }

src/crypto/openssl/openssl_wrappers.h

@@ -78,6 +78,26 @@ namespace crypto
       }
     }
 
+    // Throws if values are not equal
+    inline void CHECKEQUAL(int expect, int actual)
+    {
+      if (expect != actual)
+      {
+        unsigned long ec = ERR_get_error();
+        throw std::runtime_error(
+          fmt::format("OpenSSL error: {}", error_string(ec)));
+      }
+    }
+
+    // Throws if value is not positive
+    inline void CHECKPOSITIVE(int val)
+    {
+      if (val <= 0)
+      {
+        throw std::runtime_error("OpenSSL error: expected positive value");
+      }
+    }
+
     /*
      * Unique pointer wrappers for SSL objects, with SSL' specific constructors
      * and destructors. Some objects need special functionality, others are just
@@ -281,6 +301,9 @@ namespace crypto
       : public Unique_SSL_OBJECT<ECDSA_SIG, ECDSA_SIG_new, ECDSA_SIG_free>
     {
       using Unique_SSL_OBJECT::Unique_SSL_OBJECT;
+      Unique_ECDSA_SIG(ECDSA_SIG* ecdsa_sig) :
+        Unique_SSL_OBJECT(ecdsa_sig, ECDSA_SIG_free)
+      {}
     };
 
     struct Unique_BIGNUM : public Unique_SSL_OBJECT<BIGNUM, BN_new, BN_free>

src/js/crypto.cpp

@@ -601,7 +601,9 @@ namespace ccf::js
       {
         std::vector<uint8_t> contents(data, data + data_size);
         auto key_pair = crypto::make_key_pair(key);
-        auto sig = key_pair->sign(contents, mdtype);
+        auto sig_der = key_pair->sign(contents, mdtype);
+        auto sig =
+          crypto::ecdsa_sig_der_to_p1363(sig_der, key_pair->get_curve_id());
         return JS_NewArrayBufferCopy(ctx, sig.data(), sig.size());
       }
       else if (algo_name == "RSASSA-PKCS1-v1_5")

tests/infra/crypto.py

@@ -17,7 +17,10 @@
     load_der_x509_certificate,
 )
 from cryptography.hazmat.primitives.asymmetric import ec, rsa, padding, ed25519
-from cryptography.hazmat.primitives.asymmetric.utils import decode_dss_signature
+from cryptography.hazmat.primitives.asymmetric.utils import (
+    decode_dss_signature,
+    encode_dss_signature,
+)
 from cryptography.hazmat.primitives.serialization import (
     load_pem_private_key,
     load_pem_public_key,
@@ -241,24 +244,38 @@ def convert_ecdsa_signature_from_der_to_p1363(
     return signature_p1363
 
 
-def verify_signature(
-    signature: bytes, data: bytes, key_pub_pem: str, hash_alg: Optional[str] = None
-):
+def verify_signature(algorithm: dict, signature: bytes, data: bytes, key_pub_pem: str):
     key_pub = load_pem_public_key(key_pub_pem.encode())
     if isinstance(key_pub, rsa.RSAPublicKey):
-        if hash_alg != "SHA-256":
+        if algorithm["hash"] != "SHA-256":
             raise ValueError("Unsupported hash algorithm")
         key_pub.verify(signature, data, padding.PKCS1v15(), hashes.SHA256())
     elif isinstance(key_pub, ec.EllipticCurvePublicKey):
-        if hash_alg != "SHA-256":
+        if algorithm["hash"] != "SHA-256":
             raise ValueError("Unsupported hash algorithm")
+        encoding = algorithm.get("encoding", "ieee-p1363")
+        if encoding == "der":
+            pass
+        elif encoding == "ieee-p1363":
+            signature = convert_ecdsa_signature_from_p1363_der(signature)
+        else:
+            raise ValueError(f"Unknown encoding: {encoding}")
         key_pub.verify(signature, data, ec.ECDSA(hashes.SHA256()))
     elif isinstance(key_pub, ed25519.Ed25519PublicKey):
         return key_pub.verify(signature, data)
     else:
         raise ValueError("Unsupported key type")
 
 
+def convert_ecdsa_signature_from_p1363_der(signature_p1363: bytes) -> bytes:
+    assert len(signature_p1363) % 2 == 0
+    n = len(signature_p1363) // 2
+    r = int.from_bytes(signature_p1363[:n], "big")
+    s = int.from_bytes(signature_p1363[n:], "big")
+    signature_der = encode_dss_signature(r, s)
+    return signature_der
+
+
 def pub_key_pem_to_der(pem: str) -> bytes:
     cert = load_pem_public_key(pem.encode("ascii"), default_backend())
     return cert.public_bytes(Encoding.DER, PublicFormat.SubjectPublicKeyInfo)

tests/js-modules/modules.py

@@ -575,11 +575,24 @@ def test_npm_app(network, args):
         assert r.status_code == http.HTTPStatus.OK, r.status_code
 
         signature = r.body.data()
-        infra.crypto.verify_signature(signature, data, key_pub_pem, algorithm["hash"])
+        infra.crypto.verify_signature(algorithm, signature, data, key_pub_pem)
+
+        # Also verify with the JS API
+        r = c.post(
+            "/app/verifySignature",
+            {
+                "algorithm": algorithm,
+                "key": key_pub_pem,
+                "signature": b64encode(signature).decode(),
+                "data": b64encode(data).decode(),
+            },
+        )
+        assert r.status_code == http.HTTPStatus.OK, r.status_code
+        assert r.body.json() == True, r.body
 
         try:
             infra.crypto.verify_signature(
-                signature, "bar".encode(), key_pub_pem, algorithm["hash"]
+                algorithm, signature, "bar".encode(), key_pub_pem
             )
             assert False, "verify_signature() should throw"
         except InvalidSignature:
@@ -601,13 +614,24 @@ def test_npm_app(network, args):
             assert r.status_code == http.HTTPStatus.OK, r.status_code
 
             signature = r.body.data()
-            infra.crypto.verify_signature(
-                signature, data, key_pub_pem, algorithm["hash"]
+            infra.crypto.verify_signature(algorithm, signature, data, key_pub_pem)
+
+            # Also verify with the JS API
+            r = c.post(
+                "/app/verifySignature",
+                {
+                    "algorithm": algorithm,
+                    "key": key_pub_pem,
+                    "signature": b64encode(signature).decode(),
+                    "data": b64encode(data).decode(),
+                },
             )
+            assert r.status_code == http.HTTPStatus.OK, r.status_code
+            assert r.body.json() == True, r.body
 
             try:
                 infra.crypto.verify_signature(
-                    signature, "bar".encode(), key_pub_pem, algorithm["hash"]
+                    algorithm, signature, "bar".encode(), key_pub_pem
                 )
                 assert False, "verify_signature() should throw"
             except InvalidSignature:
@@ -627,10 +651,25 @@ def test_npm_app(network, args):
         assert r.status_code == http.HTTPStatus.OK, r.status_code
 
         signature = r.body.data()
-        infra.crypto.verify_signature(signature, data, key_pub_pem)
+        infra.crypto.verify_signature(algorithm, signature, data, key_pub_pem)
+
+        # Also verify with the JS API
+        r = c.post(
+            "/app/verifySignature",
+            {
+                "algorithm": algorithm,
+                "key": key_pub_pem,
+                "signature": b64encode(signature).decode(),
+                "data": b64encode(data).decode(),
+            },
+        )
+        assert r.status_code == http.HTTPStatus.OK, r.status_code
+        assert r.body.json() == True, r.body
 
         try:
-            infra.crypto.verify_signature(signature, "bar".encode(), key_pub_pem)
+            infra.crypto.verify_signature(
+                algorithm, signature, "bar".encode(), key_pub_pem
+            )
             assert False, "verify_signature() should throw"
         except InvalidSignature:
             pass