From 01d6e02e2d4ea946654d44e3296565aab6e9bc16 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Mon, 9 May 2022 14:03:09 +0000 Subject: [PATCH 1/3] cryptomb: add PKCS#1 v1.5 padding. Signed-off-by: Ismo Puustinen --- .../source/cryptomb_private_key_provider.cc | 71 +++++++++++++++---- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc index 3379d6a4a317..27f5ba25ffa4 100644 --- a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc +++ b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc @@ -169,24 +169,65 @@ ssl_private_key_result_t rsaPrivateKeySignInternal(CryptoMbPrivateKeyConnection* return status; } - // Add RSA padding to the the hash. Only `PSS` padding is supported. - // TODO(ipuustin): add PKCS #1 v1.5 padding scheme later if requested. - if (!SSL_is_signature_algorithm_rsa_pss(signature_algorithm)) { - ops->logDebugMsg("non-supported RSA padding"); - return status; - } - uint8_t* msg; size_t msg_len; - msg_len = RSA_size(rsa.get()); - msg = static_cast(OPENSSL_malloc(msg_len)); - if (msg == nullptr) { - return status; - } - if (!RSA_padding_add_PKCS1_PSS_mgf1(rsa.get(), msg, hash, md, nullptr, -1)) { - OPENSSL_free(msg); - return status; + // Add RSA padding to the the hash. `PSS` and `PKCS#1` v1.5 padding schemes are supported. + if (SSL_is_signature_algorithm_rsa_pss(signature_algorithm)) { + msg_len = RSA_size(rsa.get()); + msg = static_cast(OPENSSL_malloc(msg_len)); + if (msg == nullptr) { + return status; + } + + if (!RSA_padding_add_PKCS1_PSS_mgf1(rsa.get(), msg, hash, md, nullptr, -1)) { + OPENSSL_free(msg); + return status; + } + } else { + // PKCS#1 1.5 + int prefix_allocated = 0; + if (!RSA_add_pkcs1_prefix(&msg, &msg_len, &prefix_allocated, EVP_MD_type(md), hash, hash_len)) { + if (prefix_allocated) { + OPENSSL_free(msg); + } + return status; + } + + // RFC 8017 section 9.2 + + unsigned long rsa_len = RSA_size(rsa.get()); + // Header is 3 bytes, padding is min 8 bytes + unsigned long header_len = 3; + unsigned long padding_len = rsa_len - msg_len - header_len; + + if (padding_len < 8) { + OPENSSL_free(msg); + return status; + } + + uint8_t* full_msg = static_cast(OPENSSL_malloc(rsa_len)); + if (full_msg == nullptr) { + if (prefix_allocated) { + OPENSSL_free(msg); + } + return status; + } + + int idx = 0; + full_msg[idx++] = 0x0; // first header byte + full_msg[idx++] = 0x1; // second header byte + memset(full_msg + idx, 0xff, padding_len); // padding + idx += padding_len; + full_msg[idx++] = 0x0; // third header byte + memcpy(full_msg + idx, msg, msg_len); // NOLINT(safe-memcpy) + + if (prefix_allocated) { + OPENSSL_free(msg); + } + + msg = full_msg; + msg_len = rsa_len; } // Create MB context which will be used for this particular From c80a53927025a2e85c02344b9de496d06e8d3b89 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Fri, 13 May 2022 14:47:29 +0300 Subject: [PATCH 2/3] cryptomb: add tests for RSA padding schemes. Signed-off-by: Ismo Puustinen --- .../test/fake_factory.cc | 4 -- .../private_key_providers/test/ops_test.cc | 58 +++++++++++++++++-- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/contrib/cryptomb/private_key_providers/test/fake_factory.cc b/contrib/cryptomb/private_key_providers/test/fake_factory.cc index 4c36ef9e657b..7ae205f52f9e 100644 --- a/contrib/cryptomb/private_key_providers/test/fake_factory.cc +++ b/contrib/cryptomb/private_key_providers/test/fake_factory.cc @@ -78,8 +78,6 @@ uint32_t FakeIppCryptoImpl::mbxRsaPrivateCrtSslMb8( status = mbxSetSts(status, i, inject_errors_ ? !ret : ret); } - UNREFERENCED_PARAMETER(expected_rsa_bitsize); - return status; } @@ -110,8 +108,6 @@ uint32_t FakeIppCryptoImpl::mbxRsaPublicSslMb8(const uint8_t* const from_pa[8], status = mbxSetSts(status, i, inject_errors_ ? !ret : ret); } - UNREFERENCED_PARAMETER(expected_rsa_bitsize); - return status; } diff --git a/contrib/cryptomb/private_key_providers/test/ops_test.cc b/contrib/cryptomb/private_key_providers/test/ops_test.cc index 950c7b59bdce..54484d3fa041 100644 --- a/contrib/cryptomb/private_key_providers/test/ops_test.cc +++ b/contrib/cryptomb/private_key_providers/test/ops_test.cc @@ -1,3 +1,4 @@ +#include #include #include #include @@ -12,6 +13,8 @@ #include "contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.h" #include "fake_factory.h" #include "gtest/gtest.h" +#include "openssl/evp.h" +#include "openssl/rsa.h" namespace Envoy { namespace Extensions { @@ -133,13 +136,41 @@ TEST_F(CryptoMbProviderEcdsaTest, TestEcdsaSigning) { } TEST_F(CryptoMbProviderRsaTest, TestRsaPkcs1Signing) { - // PKCS #1 v1.5 is not supported. - TestCallbacks cb; - CryptoMbPrivateKeyConnection op(cb, *dispatcher_, bssl::UpRef(pkey_), queue_); + // Initialize connections. + TestCallbacks cbs[CryptoMbQueue::MULTIBUFF_BATCH]; + std::vector> connections; + for (auto& cb : cbs) { + connections.push_back(std::make_unique( + cb, *dispatcher_, bssl::UpRef(pkey_), queue_)); + } - res_ = rsaPrivateKeySignForTest(&op, nullptr, nullptr, max_out_len_, SSL_SIGN_RSA_PKCS1_SHA256, - in_, in_len_); - EXPECT_EQ(res_, ssl_private_key_failure); + // Create MULTIBUFF_BATCH amount of signing operations. + for (uint32_t i = 0; i < CryptoMbQueue::MULTIBUFF_BATCH; i++) { + // Create request. + res_ = rsaPrivateKeySignForTest(connections[i].get(), nullptr, nullptr, max_out_len_, + SSL_SIGN_RSA_PKCS1_SHA256, in_, in_len_); + EXPECT_EQ(res_, ssl_private_key_retry); + + // No processing done after first requests. + // After the last request, the status is set only from the event loop which is not run. This + // should still be "retry", the cryptographic result is present anyway. + res_ = privateKeyCompleteForTest(connections[i].get(), nullptr, nullptr, max_out_len_); + EXPECT_EQ(res_, ssl_private_key_retry); + } + + // Timeout does not have to be triggered when queue is at maximum size. + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + res_ = privateKeyCompleteForTest(connections[0].get(), out_, &out_len_, max_out_len_); + EXPECT_EQ(res_, ssl_private_key_success); + EXPECT_NE(out_len_, 0); + + // Check the signature in out_. + RSA* rsa = EVP_PKEY_get0_RSA(pkey_.get()); + + uint8_t buf[max_out_len_] = {0}; + size_t buf_len = 0; + EXPECT_EQ(RSA_verify_raw(rsa, &buf_len, buf, max_out_len_, out_, out_len_, RSA_PKCS1_PADDING), 1); } TEST_F(CryptoMbProviderRsaTest, TestRsaPssSigning) { @@ -171,6 +202,21 @@ TEST_F(CryptoMbProviderRsaTest, TestRsaPssSigning) { res_ = privateKeyCompleteForTest(connections[0].get(), out_, &out_len_, max_out_len_); EXPECT_EQ(res_, ssl_private_key_success); EXPECT_NE(out_len_, 0); + + // Check the signature in out_. + RSA* rsa = EVP_PKEY_get0_RSA(pkey_.get()); + + uint8_t buf[max_out_len_] = {0}; + unsigned int buf_len = 0; + const EVP_MD* md = SSL_get_signature_algorithm_digest(SSL_SIGN_RSA_PSS_SHA256); + EXPECT_NE(md, nullptr); + bssl::ScopedEVP_MD_CTX ctx; + // Calculate the message digest (so that we can be sure that it has been signed). + EXPECT_EQ(EVP_DigestInit_ex(ctx.get(), md, nullptr), 1); + EXPECT_EQ(EVP_DigestUpdate(ctx.get(), in_, in_len_), 1); + EXPECT_EQ(EVP_DigestFinal_ex(ctx.get(), buf, &buf_len), 1); + + EXPECT_EQ(RSA_verify_pss_mgf1(rsa, buf, buf_len, md, nullptr, -1, out_, out_len_), 1); } TEST_F(CryptoMbProviderRsaTest, TestRsaDecrypt) { From 0b6a4f67aa00b2a561fbc484a493268c64eb65a9 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Sat, 14 May 2022 00:54:07 +0300 Subject: [PATCH 3/3] Fix formatting. Signed-off-by: Ismo Puustinen --- .../source/cryptomb_private_key_provider.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc index 27f5ba25ffa4..e1a7fc96d4ae 100644 --- a/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc +++ b/contrib/cryptomb/private_key_providers/source/cryptomb_private_key_provider.cc @@ -215,11 +215,11 @@ ssl_private_key_result_t rsaPrivateKeySignInternal(CryptoMbPrivateKeyConnection* } int idx = 0; - full_msg[idx++] = 0x0; // first header byte - full_msg[idx++] = 0x1; // second header byte + full_msg[idx++] = 0x0; // first header byte + full_msg[idx++] = 0x1; // second header byte memset(full_msg + idx, 0xff, padding_len); // padding idx += padding_len; - full_msg[idx++] = 0x0; // third header byte + full_msg[idx++] = 0x0; // third header byte memcpy(full_msg + idx, msg, msg_len); // NOLINT(safe-memcpy) if (prefix_allocated) {