Skip to content

Commit

Permalink
Replace RSASSA-PKCS1-v1_5 with RSA-PSS in crypto API (#6415)
Browse files Browse the repository at this point in the history
Co-authored-by: Amaury Chamayou <[email protected]>
  • Loading branch information
maxtropets and achamayou authored Aug 5, 2024
1 parent 253518b commit 33cb6d4
Show file tree
Hide file tree
Showing 14 changed files with 163 additions and 42 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

- `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

- The `/tx` endpoint returns more accurate error messages for incorrectly formed transactions ids (#6359).
Expand Down
13 changes: 9 additions & 4 deletions include/ccf/crypto/rsa_key_pair.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,31 @@ namespace ccf::crypto
virtual std::vector<uint8_t> public_key_der() const = 0;

virtual std::vector<uint8_t> sign(
std::span<const uint8_t> d, MDType md_type = MDType::NONE) const = 0;
std::span<const uint8_t> 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<uint8_t>& contents,
const std::vector<uint8_t>& 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(
Expand Down
3 changes: 2 additions & 1 deletion include/ccf/crypto/rsa_public_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
9 changes: 7 additions & 2 deletions js/ccf-app/src/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,22 @@ 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";

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;
}

/**
Expand Down
10 changes: 6 additions & 4 deletions js/ccf-app/src/polyfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand All @@ -168,6 +168,7 @@ class CCFPolyfill implements CCF {
key: privKey,
dsaEncoding: "ieee-p1363",
padding: padding,
saltLength: algorithm.saltLength ?? 0,
});
},
verifySignature(
Expand All @@ -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");
}
Expand Down Expand Up @@ -211,6 +212,7 @@ class CCFPolyfill implements CCF {
key: pubKey,
dsaEncoding: "ieee-p1363",
padding: padding,
saltLength: algorithm.saltLength ?? 0,
},
new Uint8Array(signature),
);
Expand Down
22 changes: 12 additions & 10 deletions js/ccf-app/test/polyfill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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,
Expand All @@ -198,6 +198,7 @@ describe("polyfill", function () {
{
key: publicKey,
dsaEncoding: "ieee-p1363",
padding: crypto.constants.RSA_PKCS1_PSS_PADDING,
},
new Uint8Array(signature),
),
Expand All @@ -208,7 +209,7 @@ describe("polyfill", function () {
assert.isTrue(
ccf.crypto.verifySignature(
{
name: "RSASSA-PKCS1-v1_5",
name: "RSA-PSS",
hash: "SHA-256",
},
publicKey,
Expand Down Expand Up @@ -392,20 +393,21 @@ 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");
signer.update(new Uint8Array(data));
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,
Expand All @@ -416,7 +418,7 @@ describe("polyfill", function () {
assert.isTrue(
ccf.crypto.verifySignature(
{
name: "RSASSA-PKCS1-v1_5",
name: "RSA-PSS",
hash: "SHA-256",
},
publicKey,
Expand All @@ -427,7 +429,7 @@ describe("polyfill", function () {
assert.isNotTrue(
ccf.crypto.verifySignature(
{
name: "RSASSA-PKCS1-v1_5",
name: "RSA-PSS",
hash: "SHA-256",
},
cert,
Expand Down Expand Up @@ -494,7 +496,7 @@ describe("polyfill", function () {
assert.throws(() =>
ccf.crypto.verifySignature(
{
name: "RSASSA-PKCS1-v1_5",
name: "RSA-PSS",
hash: "SHA-256",
},
publicKey,
Expand Down Expand Up @@ -543,7 +545,7 @@ describe("polyfill", function () {
assert.throws(() =>
ccf.crypto.verifySignature(
{
name: "RSASSA-PKCS1-v1_5",
name: "RSA-PSS",
hash: "SHA-256",
},
publicKey,
Expand Down
9 changes: 6 additions & 3 deletions src/crypto/openssl/rsa_key_pair.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,14 @@ namespace ccf::crypto
}

std::vector<uint8_t> RSAKeyPair_OpenSSL::sign(
std::span<const uint8_t> d, MDType md_type) const
std::span<const uint8_t> d, MDType md_type, size_t salt_length) const
{
std::vector<uint8_t> 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()));
Expand All @@ -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(
Expand Down
7 changes: 5 additions & 2 deletions src/crypto/openssl/rsa_key_pair.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,17 @@ namespace ccf::crypto
virtual std::vector<uint8_t> public_key_der() const override;

virtual std::vector<uint8_t> sign(
std::span<const uint8_t> d, MDType md_type = MDType::NONE) const override;
std::span<const uint8_t> 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<std::string>& kid = std::nullopt) const override;
Expand Down
5 changes: 4 additions & 1 deletion src/crypto/openssl/rsa_public_key.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/crypto/openssl/rsa_public_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
44 changes: 44 additions & 0 deletions src/crypto/test/crypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -931,4 +931,48 @@ 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<uint8_t> 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));
}

{
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));
}
}
Loading

0 comments on commit 33cb6d4

Please sign in to comment.