Update API to support OpenSSL v3 while keeping support for older API

[timlegge]

This is very much a work in progress and is more changes that I would like. I suspect I can further reduce the changes as I clean up. I would welcome assistance if anyone would like to send a pull request against https://github.com/timlegge/Crypt-OpenSSL-RSA/tree/openssl3-tim

ToDo:

• [x] More cleanups and fix the checks in the modules style
• [x] Fix the two remaining deprecated functions MD5 and RIPEMD160
• [x] Clean tests from the current github actions
• [x] Add additional OSs on the github actions
• [ ] Actually verify the OPENSSL Versions - Likely just go with OPENSSL_VERSION_NUMBER >= 0x30000000L
• [x] Figure out the remaining test failures
• [x] - bignum.t - older openssl seemed to fill in missing values when a key was generated from data values
• [x] - format - opensslv3 public key format is not PKCS1 format
• [ ] Performance testing
• [ ] Create some compatibility tests - test values generated by old API that can be validated on the new API
• [ ] other things

Issues:

1. Need to be able to verify that data generated by the old module can be reproduced/validated by the new version
2. Performance needs to be tested as openSSL 3 uses multiple calls to do things that the older version did. I chose not to keet the EVP_PKEY_CTX around which might be a performance mistake (but easily rectified) and OpenSSL does talk about digests and their impact on performance so having some tests would likely help

[timlegge]

The padding issues I saw are related to some openssl restrictions on padding:

https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_CTX_set_rsa_padding.html states:

EVP_PKEY_CTX_set_rsa_padding() sets the RSA padding mode for ctx. The pad parameter can take the value RSA_PKCS1_PADDING for PKCS#1 padding, RSA_NO_PADDING for no padding, RSA_PKCS1_OAEP_PADDING for OAEP padding (encrypt and decrypt only), RSA_X931_PADDING for X9.31 padding (signature operations only), RSA_PKCS1_PSS_PADDING (sign and verify only) and RSA_PKCS1_WITH_TLS_PADDING for TLS RSA ClientKeyExchange message padding (decryption only).

Two RSA padding modes behave differently if EVP_PKEY_CTX_set_signature_md() is used. If this function is called for PKCS#1 padding the plaintext buffer is an actual digest value and is encapsulated in a DigestInfo structure according to PKCS#1 when signing and this structure is expected (and stripped off) when verifying. If this control is not used with RSA and PKCS#1 padding then the supplied data is used directly and not encapsulated. In the case of X9.31 padding for RSA the algorithm identifier byte is added or checked and removed if this control is called. If it is not called then the first byte of the plaintext buffer is expected to be the algorithm identifier byte.

The following patch would cause the code to croak if the an unsupported padding is requested. This does however break the existing tests.

diff --git a/RSA.xs b/RSA.xs
index 1cbf068..8a9fb70 100644
--- a/RSA.xs
+++ b/RSA.xs
@@ -1007,8 +1007,7 @@ sign(p_rsa, text_SV)
     ctx = EVP_PKEY_CTX_new(p_rsa->rsa, NULL /* no engine */);
     CHECK_OPEN_SSL(ctx);
     CHECK_OPEN_SSL(EVP_PKEY_sign_init(ctx));
-    /* FIXME: Issue setting padding in some cases */
-    EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding);
+    CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding));

     EVP_MD* md = get_md_bynid(p_rsa->hashMode);
     CHECK_OPEN_SSL(md != NULL);
@@ -1063,8 +1062,7 @@ PPCODE:
     ctx = EVP_PKEY_CTX_new(p_rsa->rsa, NULL /* no engine */);
     CHECK_OPEN_SSL(ctx);
     CHECK_OPEN_SSL(EVP_PKEY_verify_init(ctx) == 1);
-    /* FIXME: Issue setting padding in some cases */
-    EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding);
+    CHECK_OPEN_SSL(EVP_PKEY_CTX_set_rsa_padding(ctx, p_rsa->padding));

     EVP_MD* md = get_md_bynid(p_rsa->hashMode);
     CHECK_OPEN_SSL(md != NULL);
diff --git a/t/rsa.t b/t/rsa.t
index d3e7f0b..676baa9 100644
--- a/t/rsa.t
+++ b/t/rsa.t
@@ -119,6 +119,8 @@ _check_for_croak(
 $plaintext .= $plaintext x 5;

 # check signature algorithms
+$rsa->use_pkcs1_padding();
+$rsa_pub->use_pkcs1_padding();
 $rsa->use_md5_hash();
 $rsa_pub->use_md5_hash();
 _Test_Sign_And_Verify( $plaintext, $rsa, $rsa_pub );

[toddr]

Thanks for your submission. I'll take a look soon.

[timlegge]

Thanks for your submission. I'll take a look soon.

Thanks. I will leave the DRAFT in the title for now. Code-wise its in pretty good shape - well it works :-). Commit wise I may squash some of the commits when I look at it again tonight or tomorrow.

If you were receptive to a bump in version of ExtUtils::ParseXS to 3.51 it would improve the no blank lines between #if and #endif nonsense that is really annoying to look at and find.

I am working on some tests that will have base64 encoded encrypted values that can be compared across versions to ensure that everything works the same as in the past.

I think this needs some performance tests as well so I will see what I can do for it.

Happy to make changes as you see fit.

[toddr]

If you were receptive to a bump in version of ExtUtils::ParseXS to 3.51 it would improve the no blank lines between #if and #endif nonsense that is really annoying to look at and find.

I don't see that that modules is a dependency. Should it be?

[timlegge]

Hi

It is a part of the build process for an XS module. It's a core module since 5.8 but it's only upgraded if the new one is needed. It's not a big issue, I just ran into an issue where the version I have installed with 5.38 is patched to allow it to deal with blank lines before or after them.

So I was surprised when the PR failed on the older github actions...

The blank lines requirement has always annoyed me... :-)

Tim Timothy Legge @. @.

On Mon, May 6, 2024 at 2:26 PM Todd Rinaldo @.***> wrote:

If you were receptive to a bump in version of ExtUtils::ParseXS to 3.51 it would improve the no blank lines between #if and #endif nonsense that is really annoying to look at and find.

I don't see that that modules is a dependency. Should it be?

— Reply to this email directly, view it on GitHub https://github.com/toddr/Crypt-OpenSSL-RSA/pull/43#issuecomment-2096546824, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH3N625WSHIOLXVS6Q3DGTZA64L7AVCNFSM6AAAAABHG5XXTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJWGU2DMOBSGQ . You are receiving this because you authored the thread.Message ID: @.***>

[timlegge]

@toddr I have been busy on https://github.com/dsully/perl-crypt-openssl-pkcs12/pull/44 but I should be able to get back to this this week.

I believe I wrote the compatibility tests already and did some initial performance tests. OpenSSL 3.x is a little slower than older versions but did not seem significant.

[toddr]

thanks!

[timlegge]

Back again... I was stuck in a loop testing/troubleshooting on HPUX - API calls that worked fine everywhere else failed there. It came down to a Crypt::OpenSSL::Random issue where it was linked to SSL 1.x and Crypt::OpenSSL::RSA was linked to OpenSSL 3.x. Since t/rsa.t loads Crypt::OpenSSL::Random before Crypt::OpenSSL::RSA it seemed to interfere with the calls. I sent a PR to Crypt::OpenSSL::Random with assistance from @tux to link properly to the default openssl ssl and crypto libraries. If anyone else runs into it, load Crypt::OpenSSL::RSA first in the test and it should work fine.

Okay, back to work on cleaning up this PR