From 86802ea31396b03b1df25b2ec3c9cc78d85140d3 Mon Sep 17 00:00:00 2001 From: Max Tropets Date: Sun, 4 Aug 2024 20:19:52 +0000 Subject: [PATCH 1/3] WIP --- CHANGELOG.md | 12 ++++++++ include/ccf/crypto/rsa_key_pair.h | 13 +++++--- include/ccf/crypto/rsa_public_key.h | 3 +- js/ccf-app/src/global.ts | 9 ++++-- js/ccf-app/src/polyfill.ts | 10 ++++--- js/ccf-app/test/polyfill.test.ts | 22 +++++++------- src/crypto/openssl/rsa_key_pair.cpp | 9 ++++-- src/crypto/openssl/rsa_key_pair.h | 7 +++-- src/crypto/openssl/rsa_public_key.cpp | 5 +++- src/crypto/openssl/rsa_public_key.h | 3 +- src/crypto/test/crypto.cpp | 32 ++++++++++++++++++++ src/js/extensions/ccf/crypto.cpp | 43 +++++++++++++++++++++------ tests/infra/crypto.py | 21 +++++++++++-- tests/npm_tests.py | 4 +-- 14 files changed, 151 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a4d2a19da6a..1a49789ead9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. [5.0.2]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.2 +### Developer API + +#### C++ + +- `RSAKeyPair::sign` and `RSAKeyPair::verify` now use `RSA-PSS` instead of `RSASSA-PKCS1-v1_5`. + - Users can specify `salt_length` (defaulted to `0`). + +#### TypeScript/JavaScript + +- `/app/sign` and `/app/verify` no longer support `RSASSA-PKCS1-v1_5`, instead `RSA-PSS` has been added. + - `SigningAlgorithm` has been extended with optional `saltLength`, defaulted to `0` if not passed. + ### Bug Fixes - The `/tx` endpoint returns more accurate error messages for incorrectly formed transactions ids (#6359). diff --git a/include/ccf/crypto/rsa_key_pair.h b/include/ccf/crypto/rsa_key_pair.h index ac0f9eca6cf1..ffc10c28a4d5 100644 --- a/include/ccf/crypto/rsa_key_pair.h +++ b/include/ccf/crypto/rsa_key_pair.h @@ -55,26 +55,31 @@ namespace ccf::crypto virtual std::vector public_key_der() const = 0; virtual std::vector sign( - std::span d, MDType md_type = MDType::NONE) const = 0; + std::span d, + MDType md_type = MDType::NONE, + size_t salt_length = 0) const = 0; virtual bool verify( const uint8_t* contents, size_t contents_size, const uint8_t* signature, size_t signature_size, - MDType md_type = MDType::NONE) = 0; + MDType md_type = MDType::NONE, + size_t salt_length = 0) = 0; virtual bool verify( const std::vector& contents, const std::vector& signature, - MDType md_type = MDType::NONE) + MDType md_type = MDType::NONE, + size_t salt_length = 0) { return verify( contents.data(), contents.size(), signature.data(), signature.size(), - md_type); + md_type, + salt_length); } virtual JsonWebKeyRSAPrivate private_key_jwk_rsa( diff --git a/include/ccf/crypto/rsa_public_key.h b/include/ccf/crypto/rsa_public_key.h index d37463da9447..cd62eba0e7f4 100644 --- a/include/ccf/crypto/rsa_public_key.h +++ b/include/ccf/crypto/rsa_public_key.h @@ -81,7 +81,8 @@ namespace ccf::crypto size_t contents_size, const uint8_t* signature, size_t signature_size, - MDType md_type = MDType::NONE) = 0; + MDType md_type = MDType::NONE, + size_t salt_legth = 0) = 0; struct Components { diff --git a/js/ccf-app/src/global.ts b/js/ccf-app/src/global.ts index 33db097ba2fe..cf03dfab2f68 100644 --- a/js/ccf-app/src/global.ts +++ b/js/ccf-app/src/global.ts @@ -231,7 +231,7 @@ export interface CryptoKeyPair { publicKey: string; } -export type AlgorithmName = "RSASSA-PKCS1-v1_5" | "ECDSA" | "EdDSA" | "HMAC"; +export type AlgorithmName = "RSA-PSS" | "ECDSA" | "EdDSA" | "HMAC"; export type DigestAlgorithm = "SHA-256" | "SHA-384" | "SHA-512"; @@ -239,9 +239,14 @@ export interface SigningAlgorithm { name: AlgorithmName; /** - * Digest algorithm. It's necessary for "RSASSA-PKCS1-v1_5", "ECDSA", and "HMAC" + * Digest algorithm. It's necessary for "RSA-PSS", "ECDSA", and "HMAC" */ hash?: DigestAlgorithm; + + /** + * Salt length, necessary for "RSA-PSS". + */ + saltLength?: number; } /** diff --git a/js/ccf-app/src/polyfill.ts b/js/ccf-app/src/polyfill.ts index 949ca1e2fdd2..58a8b9de7689 100644 --- a/js/ccf-app/src/polyfill.ts +++ b/js/ccf-app/src/polyfill.ts @@ -142,8 +142,8 @@ class CCFPolyfill implements CCF { let padding = undefined; const privKey = jscrypto.createPrivateKey(key); if (privKey.asymmetricKeyType == "rsa") { - if (algorithm.name === "RSASSA-PKCS1-v1_5") { - padding = jscrypto.constants.RSA_PKCS1_PADDING; + if (algorithm.name === "RSA-PSS") { + padding = jscrypto.constants.RSA_PKCS1_PSS_PADDING; } else { throw new Error("incompatible signing algorithm for given key type"); } @@ -168,6 +168,7 @@ class CCFPolyfill implements CCF { key: privKey, dsaEncoding: "ieee-p1363", padding: padding, + saltLength: algorithm.saltLength ?? 0, }); }, verifySignature( @@ -179,8 +180,8 @@ class CCFPolyfill implements CCF { let padding = undefined; const pubKey = jscrypto.createPublicKey(key); if (pubKey.asymmetricKeyType == "rsa") { - if (algorithm.name === "RSASSA-PKCS1-v1_5") { - padding = jscrypto.constants.RSA_PKCS1_PADDING; + if (algorithm.name === "RSA-PSS") { + padding = jscrypto.constants.RSA_PKCS1_PSS_PADDING; } else { throw new Error("incompatible signing algorithm for given key type"); } @@ -211,6 +212,7 @@ class CCFPolyfill implements CCF { key: pubKey, dsaEncoding: "ieee-p1363", padding: padding, + saltLength: algorithm.saltLength ?? 0, }, new Uint8Array(signature), ); diff --git a/js/ccf-app/test/polyfill.test.ts b/js/ccf-app/test/polyfill.test.ts index ef4ddf430325..5b3cf0259f83 100644 --- a/js/ccf-app/test/polyfill.test.ts +++ b/js/ccf-app/test/polyfill.test.ts @@ -167,7 +167,7 @@ describe("polyfill", function () { }); }); describe("sign", function () { - it("performs RSASSA-PKCS1-v1_5 sign correctly", function () { + it("performs RSA-PSS sign correctly", function () { const { publicKey, privateKey } = crypto.generateKeyPairSync("rsa", { modulusLength: 2048, publicKeyEncoding: { @@ -182,7 +182,7 @@ describe("polyfill", function () { const data = ccf.strToBuf("foo"); const signature = ccf.crypto.sign( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, privateKey, @@ -198,6 +198,7 @@ describe("polyfill", function () { { key: publicKey, dsaEncoding: "ieee-p1363", + padding: crypto.constants.RSA_PKCS1_PSS_PADDING, }, new Uint8Array(signature), ), @@ -208,7 +209,7 @@ describe("polyfill", function () { assert.isTrue( ccf.crypto.verifySignature( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, publicKey, @@ -392,7 +393,7 @@ describe("polyfill", function () { }); }); describe("verifySignature", function () { - it("performs RSASSA-PKCS1-v1_5 validation correctly", function () { + it("performs RSA-PSS validation correctly", function () { const { cert, publicKey, privateKey } = generateSelfSignedCert(); const signer = crypto.createSign("sha256"); const data = ccf.strToBuf("foo"); @@ -400,12 +401,13 @@ describe("polyfill", function () { signer.end(); const signature = signer.sign({ key: crypto.createPrivateKey(privateKey), - padding: crypto.constants.RSA_PKCS1_PADDING, + padding: crypto.constants.RSA_PKCS1_PSS_PADDING, + saltLength: 0, }); assert.isTrue( ccf.crypto.verifySignature( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, cert, @@ -416,7 +418,7 @@ describe("polyfill", function () { assert.isTrue( ccf.crypto.verifySignature( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, publicKey, @@ -427,7 +429,7 @@ describe("polyfill", function () { assert.isNotTrue( ccf.crypto.verifySignature( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, cert, @@ -494,7 +496,7 @@ describe("polyfill", function () { assert.throws(() => ccf.crypto.verifySignature( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, publicKey, @@ -543,7 +545,7 @@ describe("polyfill", function () { assert.throws(() => ccf.crypto.verifySignature( { - name: "RSASSA-PKCS1-v1_5", + name: "RSA-PSS", hash: "SHA-256", }, publicKey, diff --git a/src/crypto/openssl/rsa_key_pair.cpp b/src/crypto/openssl/rsa_key_pair.cpp index 4c31ddcbfd0f..8fd90593943d 100644 --- a/src/crypto/openssl/rsa_key_pair.cpp +++ b/src/crypto/openssl/rsa_key_pair.cpp @@ -205,12 +205,14 @@ namespace ccf::crypto } std::vector RSAKeyPair_OpenSSL::sign( - std::span d, MDType md_type) const + std::span d, MDType md_type, size_t salt_length) const { std::vector r(2048); auto hash = OpenSSLHashProvider().Hash(d.data(), d.size(), md_type); Unique_EVP_PKEY_CTX pctx(key); CHECK1(EVP_PKEY_sign_init(pctx)); + CHECK1(EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING)); + CHECK1(EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, salt_length)); CHECK1(EVP_PKEY_CTX_set_signature_md(pctx, get_md_type(md_type))); size_t olen = r.size(); CHECK1(EVP_PKEY_sign(pctx, r.data(), &olen, hash.data(), hash.size())); @@ -223,10 +225,11 @@ namespace ccf::crypto size_t contents_size, const uint8_t* signature, size_t signature_size, - MDType md_type) + MDType md_type, + size_t salt_length) { return RSAPublicKey_OpenSSL::verify( - contents, contents_size, signature, signature_size, md_type); + contents, contents_size, signature, signature_size, md_type, salt_length); } JsonWebKeyRSAPrivate RSAKeyPair_OpenSSL::private_key_jwk_rsa( diff --git a/src/crypto/openssl/rsa_key_pair.h b/src/crypto/openssl/rsa_key_pair.h index e51b9bf45661..3c5902aaeb89 100644 --- a/src/crypto/openssl/rsa_key_pair.h +++ b/src/crypto/openssl/rsa_key_pair.h @@ -36,14 +36,17 @@ namespace ccf::crypto virtual std::vector public_key_der() const override; virtual std::vector sign( - std::span d, MDType md_type = MDType::NONE) const override; + std::span d, + MDType md_type = MDType::NONE, + size_t salt_length = 0) const override; virtual bool verify( const uint8_t* contents, size_t contents_size, const uint8_t* signature, size_t signature_size, - MDType md_type = MDType::NONE) override; + MDType md_type = MDType::NONE, + size_t salt_length = 0) override; virtual JsonWebKeyRSAPrivate private_key_jwk_rsa( const std::optional& kid = std::nullopt) const override; diff --git a/src/crypto/openssl/rsa_public_key.cpp b/src/crypto/openssl/rsa_public_key.cpp index e508b10e4454..b8fb2f61be58 100644 --- a/src/crypto/openssl/rsa_public_key.cpp +++ b/src/crypto/openssl/rsa_public_key.cpp @@ -195,11 +195,14 @@ namespace ccf::crypto size_t contents_size, const uint8_t* signature, size_t signature_size, - MDType md_type) + MDType md_type, + size_t salt_length) { auto hash = OpenSSLHashProvider().Hash(contents, contents_size, md_type); Unique_EVP_PKEY_CTX pctx(key); CHECK1(EVP_PKEY_verify_init(pctx)); + CHECK1(EVP_PKEY_CTX_set_rsa_padding(pctx, RSA_PKCS1_PSS_PADDING)); + CHECK1(EVP_PKEY_CTX_set_rsa_pss_saltlen(pctx, salt_length)); CHECK1(EVP_PKEY_CTX_set_signature_md(pctx, get_md_type(md_type))); return EVP_PKEY_verify( pctx, signature, signature_size, hash.data(), hash.size()) == 1; diff --git a/src/crypto/openssl/rsa_public_key.h b/src/crypto/openssl/rsa_public_key.h index 2ae58522bce1..061ba053ad80 100644 --- a/src/crypto/openssl/rsa_public_key.h +++ b/src/crypto/openssl/rsa_public_key.h @@ -52,7 +52,8 @@ namespace ccf::crypto size_t contents_size, const uint8_t* signature, size_t signature_size, - MDType md_type = MDType::NONE) override; + MDType md_type = MDType::NONE, + size_t salt_length = 0) override; virtual Components components() const override; diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index 4746f69430bd..d43353474981 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -931,4 +931,36 @@ TEST_CASE("Incremental hash") } } ccf::crypto::openssl_sha256_shutdown(); +} + +TEST_CASE("Sign and verify with RSA key") +{ + const auto kp = ccf::crypto::make_rsa_key_pair(); + const auto pub = ccf::crypto::make_rsa_public_key(kp->public_key_pem()); + const auto mdtype = ccf::crypto::MDType::SHA256; + vector contents(contents_.begin(), contents_.end()); + + { + constexpr size_t salt_length = 0; + const auto sig = kp->sign(contents, mdtype, salt_length); + REQUIRE(pub->verify( + contents.data(), + contents.size(), + sig.data(), + sig.size(), + mdtype, + salt_length)); + } + + { + constexpr size_t sign_salt_length = 0, verify_salt_legth = 32; + const auto sig = kp->sign(contents, mdtype, sign_salt_length); + REQUIRE(!pub->verify( + contents.data(), + contents.size(), + sig.data(), + sig.size(), + mdtype, + verify_salt_legth)); + } } \ No newline at end of file diff --git a/src/js/extensions/ccf/crypto.cpp b/src/js/extensions/ccf/crypto.cpp index abd1d935828e..80f6fd5d4e65 100644 --- a/src/js/extensions/ccf/crypto.cpp +++ b/src/js/extensions/ccf/crypto.cpp @@ -884,10 +884,19 @@ namespace ccf::js::extensions sig_der, key_pair->get_curve_id()); return JS_NewArrayBufferCopy(ctx, sig.data(), sig.size()); } - else if (algo_name == "RSASSA-PKCS1-v1_5") + else if (algo_name == "RSA-PSS") { auto key_pair = ccf::crypto::make_rsa_key_pair(key); - auto sig = key_pair->sign(contents, mdtype); + + int64_t salt_length{}; + std::ignore = JS_ToInt64( + jsctx, + &salt_length, + jsctx.get_property(algorithm, "saltLength").val); + + auto sig = + key_pair->sign(contents, mdtype, static_cast(salt_length)); + return JS_NewArrayBufferCopy(ctx, sig.data(), sig.size()); } else if (algo_name == "HMAC") @@ -900,8 +909,8 @@ namespace ccf::js::extensions { return JS_ThrowRangeError( ctx, - "Unsupported signing algorithm, supported: RSASSA-PKCS1-v1_5, " - "ECDSA, EdDSA, HMAC"); + "Unsupported signing algorithm, supported: RSA-PSS, ECDSA, EdDSA, " + "HMAC"); } } catch (const std::exception& ex) @@ -1010,12 +1019,12 @@ namespace ccf::js::extensions ctx, "Unsupported hash algorithm, supported: SHA-256"); } - if (algo_name != "RSASSA-PKCS1-v1_5" && algo_name != "ECDSA") + if (algo_name != "RSA-PSS" && algo_name != "ECDSA") { return JS_ThrowRangeError( ctx, - "Unsupported signing algorithm, supported: RSASSA-PKCS1-v1_5, " - "ECDSA, EdDSA"); + "Unsupported signing algorithm, supported: RSA-PSS, ECDSA, " + "EdDSA"); } std::vector sig(signature, signature + signature_size); @@ -1035,13 +1044,29 @@ namespace ccf::js::extensions valid = verifier->verify(data, data_size, sig.data(), sig.size(), mdtype); } - else + else if (algo_name == "ECDSA") { auto public_key = ccf::crypto::make_public_key(key); valid = public_key->verify(data, data_size, sig.data(), sig.size(), mdtype); } - + else + { + int64_t salt_length{}; + std::ignore = JS_ToInt64( + jsctx, + &salt_length, + jsctx.get_property(algorithm, "saltLength").val); + + auto public_key = ccf::crypto::make_rsa_public_key(key); + valid = public_key->verify( + data, + data_size, + sig.data(), + sig.size(), + mdtype, + static_cast(salt_length)); + } return JS_NewBool(ctx, valid); } catch (const std::exception& ex) diff --git a/tests/infra/crypto.py b/tests/infra/crypto.py index 6efb24ae267c..c762e28198ad 100644 --- a/tests/infra/crypto.py +++ b/tests/infra/crypto.py @@ -213,8 +213,15 @@ def sign(algorithm: dict, key_pem: str, data: bytes) -> bytes: else: raise ValueError("Unsupported hash algorithm") if isinstance(key, rsa.RSAPrivateKey): - if algorithm["name"] == "RSASSA-PKCS1-v1_5": - return key.sign(data, padding.PKCS1v15(), hash_alg) + if algorithm["name"] == "RSA-PSS": + return key.sign( + data, + padding=padding.PSS( + mgf=padding.MGF1(algorithm=hashes.SHA256()), + salt_length=algorithm.get("saltLength", 0), + ), + algorithm=hash_alg, + ) else: raise ValueError("Unsupported signing algorithm") elif isinstance(key, ec.EllipticCurvePrivateKey): @@ -254,7 +261,15 @@ def verify_signature(algorithm: dict, signature: bytes, data: bytes, key_pub_pem if isinstance(key_pub, rsa.RSAPublicKey): if algorithm["hash"] != "SHA-256": raise ValueError("Unsupported hash algorithm") - key_pub.verify(signature, data, padding.PKCS1v15(), hashes.SHA256()) + key_pub.verify( + signature, + data, + padding.PSS( + mgf=padding.MGF1(algorithm=hashes.SHA256()), + salt_length=algorithm.get("saltLength", 0), + ), + hashes.SHA256(), + ) elif isinstance(key_pub, ec.EllipticCurvePublicKey): if algorithm["hash"] != "SHA-256": raise ValueError("Unsupported hash algorithm") diff --git a/tests/npm_tests.py b/tests/npm_tests.py index 85cb37b5bab7..86880cd39b75 100644 --- a/tests/npm_tests.py +++ b/tests/npm_tests.py @@ -438,7 +438,7 @@ def test_npm_app(network, args): # Test RSA signing + verification key_priv_pem, key_pub_pem = infra.crypto.generate_rsa_keypair(2048) - algorithm = {"name": "RSASSA-PKCS1-v1_5", "hash": "SHA-256"} + algorithm = {"name": "RSA-PSS", "hash": "SHA-256", "saltLength": 32} data = rand_bytes(random.randint(2, 50)) r = c.post( "/app/sign", @@ -551,7 +551,7 @@ def test_npm_app(network, args): pass key_priv_pem, key_pub_pem = infra.crypto.generate_rsa_keypair(2048) - algorithm = {"name": "RSASSA-PKCS1-v1_5", "hash": "SHA-256"} + algorithm = {"name": "RSA-PSS", "hash": "SHA-256", "saltLength": 32} signature = infra.crypto.sign(algorithm, key_priv_pem, data) r = c.post( "/app/verifySignature", From b51dd296a932b1c8fce7fe88f2b5fb40c0918178 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 5 Aug 2024 11:36:29 +0100 Subject: [PATCH 2/3] Update CHANGELOG.md Co-authored-by: Amaury Chamayou --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a49789ead9c..deaddfb2b499 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. #### TypeScript/JavaScript -- `/app/sign` and `/app/verify` no longer support `RSASSA-PKCS1-v1_5`, instead `RSA-PSS` has been added. +- `ccfapp.crypto.sign()` and `ccfapp.crypto.verifySignature()` no longer support `RSASSA-PKCS1-v1_5`, instead `RSA-PSS` has been added. - `SigningAlgorithm` has been extended with optional `saltLength`, defaulted to `0` if not passed. ### Bug Fixes From de24418c6b665d926f33a6631f1494f7a080fcad Mon Sep 17 00:00:00 2001 From: Max Tropets Date: Mon, 5 Aug 2024 10:47:55 +0000 Subject: [PATCH 3/3] Test update --- src/crypto/test/crypto.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index d43353474981..3c6e7132bd58 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -963,4 +963,16 @@ TEST_CASE("Sign and verify with RSA key") mdtype, verify_salt_legth)); } + + { + constexpr size_t sign_salt_length = 32, verify_salt_legth = 32; + const auto sig = kp->sign(contents, mdtype, sign_salt_length); + REQUIRE(pub->verify( + contents.data(), + contents.size(), + sig.data(), + sig.size(), + mdtype, + verify_salt_legth)); + } } \ No newline at end of file