Skip to content

Commit

Permalink
js: require IEEE P1363 ECDSA signatures in `ccf.crypto.verifySignatur…
Browse files Browse the repository at this point in the history
…e` (#2737)
  • Loading branch information
letmaik authored Jul 6, 2021
1 parent 545a88a commit ce877fc
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 6 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ 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).

[Unreleased]
## [Unreleased]

### Changed

- `ccf.crypto.verifySignature()` previously required DER-encoded ECDSA signatures and now requires IEEE P1363 encoded signatures, aligning with the behavior of the Web Crypto API (#2735).

### Added

Expand Down
11 changes: 11 additions & 0 deletions js/ccf-app/src/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,22 @@ export interface CryptoKeyPair {
publicKey: string;
}

/**
* RSASSA-PKCS1-v1_5 signature algorithm parameters.
*/
export interface RsaPkcsParams {
name: "RSASSA-PKCS1-v1_5";
hash: DigestAlgorithm;
}

/**
* ECDSA signature algorithm parameters.
*
* Note: ECDSA signatures are assumed to be encoded according
* to the Web Crypto API specification, which is the same
* format used in JSON Web Tokens and more generally known
* as IEEE P1363 encoding.
*/
export interface EcdsaParams {
name: "ECDSA";
hash: DigestAlgorithm;
Expand Down
1 change: 1 addition & 0 deletions js/ccf-app/src/polyfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ class CCFPolyfill implements CCF {
return verifier.verify(
{
key: pubKey,
dsaEncoding: "ieee-p1363",
padding: padding,
},
new Uint8Array(signature)
Expand Down
3 changes: 2 additions & 1 deletion js/ccf-app/test/polyfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe("polyfill", function () {
// Not validating EC with certs here as node-forge used in
// generateSelfSignedCert() does not support EC keys.
const { publicKey, privateKey } = crypto.generateKeyPairSync("ec", {
namedCurve: "secp256r1",
namedCurve: "P-256",
publicKeyEncoding: {
type: "spki",
format: "pem",
Expand All @@ -173,6 +173,7 @@ describe("polyfill", function () {
signer.end();
const signature = signer.sign({
key: crypto.createPrivateKey(privateKey),
dsaEncoding: "ieee-p1363",
});
assert.isTrue(
ccf.crypto.verifySignature(
Expand Down
37 changes: 37 additions & 0 deletions src/crypto/ecdsa.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the Apache 2.0 License.
#pragma once

#include "crypto/openssl/openssl_wrappers.h"

#include <openssl/bn.h>
#include <openssl/ecdsa.h>
#include <vector>

namespace crypto
{
/** Converts an ECDSA signature in IEEE P1363 encoding to RFC 3279 DER
* encoding.
* @param signature The signature in IEEE P1363 encoding
*/
static std::vector<uint8_t> ecdsa_sig_p1363_to_der(
const std::vector<uint8_t>& signature)
{
auto signature_size = signature.size();
auto half_size = signature_size / 2;
OpenSSL::Unique_BIGNUM r;
OpenSSL::Unique_BIGNUM s;
OpenSSL::CHECKNULL(BN_bin2bn(signature.data(), half_size, r));
OpenSSL::CHECKNULL(BN_bin2bn(signature.data() + half_size, half_size, s));
OpenSSL::Unique_ECDSA_SIG sig;
OpenSSL::CHECK1(ECDSA_SIG_set0(sig, r, s));
r.release();
s.release();
auto der_size = i2d_ECDSA_SIG(sig, nullptr);
OpenSSL::CHECK0(der_size);
std::vector<uint8_t> der_sig(der_size);
auto der_sig_buf = der_sig.data();
OpenSSL::CHECK0(i2d_ECDSA_SIG(sig, &der_sig_buf));
return der_sig;
}
}
44 changes: 44 additions & 0 deletions src/crypto/openssl/openssl_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ namespace crypto
}
}

inline void CHECK0(int rc)
{
unsigned long ec = ERR_get_error();
if (rc == 0 && ec != 0)
{
throw std::runtime_error(
fmt::format("OpenSSL error: {}", ERR_error_string(ec, NULL)));
}
}

inline void CHECKNULL(void* ptr)
{
if (ptr == NULL)
Expand Down Expand Up @@ -190,6 +200,40 @@ namespace crypto
}
};

class Unique_ECDSA_SIG
{
std::unique_ptr<ECDSA_SIG, void (*)(ECDSA_SIG*)> p;

public:
Unique_ECDSA_SIG() : p(ECDSA_SIG_new(), ECDSA_SIG_free)
{
OpenSSL::CHECKNULL(p.get());
}
operator ECDSA_SIG*()
{
return p.get();
}
};

class Unique_BIGNUM
{
std::unique_ptr<BIGNUM, void (*)(BIGNUM*)> p;

public:
Unique_BIGNUM() : p(BN_new(), BN_free)
{
OpenSSL::CHECKNULL(p.get());
}
operator BIGNUM*()
{
return p.get();
}
void release()
{
p.release();
}
};

inline std::string error_string(int ec)
{
return ERR_error_string((unsigned long)ec, NULL);
Expand Down
14 changes: 11 additions & 3 deletions src/js/crypto.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the Apache 2.0 License.
#include "crypto/ecdsa.h"
#include "crypto/entropy.h"
#include "crypto/key_wrap.h"
#include "crypto/rsa_key_pair.h"
Expand Down Expand Up @@ -448,6 +449,13 @@ namespace js
return JS_EXCEPTION;
}

std::vector<uint8_t> sig(signature, signature + signature_size);

if (algo_name == "ECDSA")
{
sig = crypto::ecdsa_sig_p1363_to_der(sig);
}

auto is_cert = nonstd::starts_with(key, "-----BEGIN CERTIFICATE");

bool valid = false;
Expand All @@ -456,13 +464,13 @@ namespace js
{
auto verifier = crypto::make_unique_verifier(key);
valid =
verifier->verify(data, data_size, signature, signature_size, mdtype);
verifier->verify(data, data_size, sig.data(), sig.size(), mdtype);
}
else
{
auto public_key = crypto::make_public_key(key);
valid = public_key->verify(
data, data_size, signature, signature_size, mdtype);
valid =
public_key->verify(data, data_size, sig.data(), sig.size(), mdtype);
}

return JS_NewBool(ctx, valid);
Expand Down
24 changes: 23 additions & 1 deletion tests/infra/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
load_der_x509_certificate,
)
from cryptography.hazmat.primitives.asymmetric import ec, rsa, padding
from cryptography.hazmat.primitives.asymmetric.utils import decode_dss_signature
from cryptography.hazmat.primitives.serialization import (
load_pem_private_key,
load_pem_public_key,
Expand Down Expand Up @@ -189,13 +190,34 @@ def sign(algorithm: dict, key_pem: str, data: bytes) -> bytes:
elif isinstance(key, ec.EllipticCurvePrivateKey):
if algorithm["name"] == "ECDSA":
# pylint: disable=no-value-for-parameter
return key.sign(data, ec.ECDSA(hash_alg))
signature = key.sign(data, ec.ECDSA(hash_alg))
encoding = algorithm.get("encoding", "ieee-p1363")
if encoding == "der":
pass
elif encoding == "ieee-p1363":
key_size_bits = key.key_size
signature = convert_ecdsa_signature_from_der_to_p1363(
signature, key_size_bits
)
else:
raise ValueError(f"Unknown encoding: {encoding}")
return signature
else:
raise ValueError("Unsupported signing algorithm")
else:
raise ValueError("Unsupported key type")


def convert_ecdsa_signature_from_der_to_p1363(
signature_der: bytes, key_size_bits: int
) -> bytes:
(r, s) = decode_dss_signature(signature_der)
assert key_size_bits % 8 == 0
n = key_size_bits // 8
signature_p1363 = r.to_bytes(n, byteorder="big") + s.to_bytes(n, byteorder="big")
return signature_p1363


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)
Expand Down
3 changes: 3 additions & 0 deletions tests/js-modules/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@ def test_npm_app(network, args):
ccf_pkg_dir = os.path.join(PARENT_DIR, "..", "js", "ccf-app")
subprocess.run(["npm", "install", "--no-package-lock"], cwd=ccf_pkg_dir, check=True)

LOG.info("Running ccf-app unit tests")
subprocess.run(["npm", "test"], cwd=ccf_pkg_dir, check=True)

LOG.info("Building npm app")
app_dir = os.path.join(PARENT_DIR, "npm-app")
subprocess.run(["npm", "install"], cwd=app_dir, check=True)
Expand Down

0 comments on commit ce877fc

Please sign in to comment.