Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace RSASSA-PKCS1-v1_5 with RSA-PSS in crypto API #6415

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

- `/app/sign` and `/app/verify` no longer support `RSASSA-PKCS1-v1_5`, instead `RSA-PSS` has been added.
maxtropets marked this conversation as resolved.
Show resolved Hide resolved
- `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;
}
maxtropets marked this conversation as resolved.
Show resolved Hide resolved

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));
maxtropets marked this conversation as resolved.
Show resolved Hide resolved
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
32 changes: 32 additions & 0 deletions src/crypto/test/crypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<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;
maxtropets marked this conversation as resolved.
Show resolved Hide resolved
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
Loading