openssl
openssl copied to clipboard
ruby
[WIP] Support OpenSSL 3.0
A collection of patches to support OpenSSL 3.0 API.
Currently, as of 2021-05-25, it compiles with OpenSSL's master branch (3.0.0-alpha17), but many things are known to be broken and test suite does not pass.
See also the tracking issue #369 (Support OpenSSL 3.0) for the summary.
These test failures are as of 2020-08, which may or may not be relevant today.
Test suite does not pass yet - some are due to unresolved issues in OpenSSL, which have open issues on openssl/openssl:
- OpenSSL::PKey.generate_key with HMAC pkey
- OpenSSL::PKey.read with a passphrase with a null character in the middle
I haven't dug into others, yet:
- OpenSSL::PKey::PKey.sign_raw against RSA with 'none' padding
- test_connect_certificate_verify_failed_exception_message(OpenSSL::TestSSL)
- test_post_connection_check(OpenSSL::TestSSL)
- test_ciphers(OpenSSL::TestCipher)
- test_reset(OpenSSL::TestCipher)
- test_verify_callback(OpenSSL::TestX509Store)
I've tried to apply this PR on top of Ruby 3.0.1, and I'm experiencing a segfault on running test suite (failed when reading key). Any ideas how to fix that? Is that still expected?
I'll appreciate any help.
I assume that's why this PR is WIP. I think it's better to share why you want to run Ruby on the OpenSSL 3.0 "right now" without waiting the stable version if it is possible. That's helpful for people to know the proper context. As your colleague working in the same company, I know you are trying to build Ruby with OpenSSL 3.0 "right now". I think a possible solution might be to apply a patch to skip related logic by #ifdef or something.
I think the steps to support OpenSSL 3.0 for Ruby is like this. I think what you can do to accelerate the steps is to contribute to step 1 first.
- Test ruby/openssl repo with OpenSSL 3.0.
- Test ruby/ruby repo porting the lateted ruby/openssl version supporting it.
As a reference, this repo still does not have an openssl 3.0 case.
https://github.com/ruby/openssl/blob/05e1c015d64cc2a2f99d8d85586e6d48c725e30c/.github/workflows/test.yml#L74-L76
In my humble opinion, it might be also helpful for you to provide a reproductive script such as by using this repo's CI adding the OpenSSL 3.0 or using a container. Otherwise there might not be many things for people to help you here.
The segfault is likely due to OSSL_STORE_attach()'s function signature having been changed in OpenSSL master in the last month. I expect other parts of OpenSSL::PKey can also explode, as the underlying EVP_PKEY structure was made immutable while we've provided "destructive" methods on it. The change happened earlier this year.
Yes, this is a work-in-progress and is not really ready. Please note that OpenSSL 3.0 is still in an alpha release and has not reached its feature freeze.
I assume that's why this PR is WIP. I think it's better to share why you want to run Ruby on the OpenSSL 3.0 "right now" without waiting the stable version if it is possible. That's helpful for people to know the proper context. As your colleague working in the same company, I know you are trying to build Ruby with OpenSSL 3.0 "right now". I think a possible solution might be to apply a patch to skip related logic by
#ifdefor something.
It's not that I need it working "right now" (I'm not sure where did you get that impression). I'm trying to apply it now, for testing purposes, WRT Centos Stream 9.
You're right, the PR is WIP, but that does not mean it's "development behind closed doors", right? I was trying to get the actual (usability) status, using combination of several PRs, and additional modifications, applied it to currently released Ruby (not a development version), as we plan to use it in the future.
I may have applied the patch in a wrong way as well, so that's the part I'd be glad to get any feedback for.
I think the steps to support OpenSSL 3.0 for Ruby is like this. I think what you can do to accelerate the steps is to contribute to step 1 first.
1. Test ruby/openssl repo with OpenSSL 3.0. 2. Test ruby/ruby repo porting the lateted ruby/openssl version supporting it.As a reference, this repo still does not have an openssl 3.0 case.
https://github.com/ruby/openssl/blob/05e1c015d64cc2a2f99d8d85586e6d48c725e30c/.github/workflows/test.yml#L74-L76
Jun, if You think it's good to engage in upstream development, feel free to do it. I do not think it's mandatory to be able to have a conversation (give + receive feedback). I'll probably be able to engage more in upstream development in the future, but I'm afraid it's out of scope for me, currently.
In my humble opinion, it might be also helpful for you to provide a reproductive script such as by using this repo's CI adding the OpenSSL 3.0 or using a container. Otherwise there might not be many things for people to help you here.
The segfault is likely due to
OSSL_STORE_attach()'s function signature having been changed in OpenSSL master in the last month. I expect other parts of OpenSSL::PKey can also explode, as the underlying EVP_PKEY structure was made immutable while we've provided "destructive" methods on it. The change happened earlier this year.
Thank you!
Yes, this is a work-in-progress and is not really ready. Please note that OpenSSL 3.0 is still in an alpha release and has not reached its feature freeze.
Yes, AFAICT this is part of the "alpha" compatibility / readiness early testing (not directly by me), as we're encouraged to give feedback as well.
You're right, the PR is WIP, but that does not mean it's "development behind closed doors", right?
Right. But I meant if you explain more context, it's better to open the door, not to surprise people. People are busy. It's better to think how to do for people to answer your question easily.
I do not think it's mandatory to be able to have a conversation (give + receive feedback). I'll probably be able to engage more in upstream development in the future, but I'm afraid it's out of scope for me, currently.
I just suggested a possible way for you to do. There is no pressure.
OSSL_STORE_attach()
The commit using this function (https://github.com/ruby/openssl/commit/506be78d53d4393f2ff6acb710f2b97fb137aeb0) is now replaced by commit https://github.com/ruby/openssl/commit/01e123051d774355bdda1947d77a832216e5b595. Some additional necessary changes for OpenSSL 3.0 support are also pushed to GitHub.
It compiles with OpenSSL master (if all the warnings are ignored), but more work is still needed for the other part of OpenSSL::PKey. Also, as you can see in the GitHub Actions' output, many test cases are currently failing due to various reasons.
Yes, AFAICT this is part of the "alpha" compatibility / readiness early testing (not directly by me), as we're encouraged to give feedback as well.
Thanks, this is helpful.
OSSL_STORE_attach()The commit using this function (506be78) is now replaced by commit 01e1230. Some additional necessary changes for OpenSSL 3.0 support are also pushed to GitHub.
Thanks! I've replaced the commit in my ruby package patch, but I'm still getting a segfault.
I might by able to try with newer openssl (from the package maintainer) later this week. Additionaly, I will try updating the ruby openssl gem to latest github state, to see whether I can achieve better result.
It compiles with OpenSSL master (if all the warnings are ignored), but more work is still needed for the other part of OpenSSL::PKey. Also, as you can see in the GitHub Actions' output, many test cases are currently failing due to various reasons.
Yes, AFAICT this is part of the "alpha" compatibility / readiness early testing (not directly by me), as we're encouraged to give feedback as well.
Thanks, this is helpful.
I'm glad to hear that, and I'll be happy to test further.
Please note that OpenSSL 3.0 is still in an alpha release and has not reached its feature freeze.
Since Beta1 is already out, can I gently ask for an update? Thx a lot.
A test failure due to a behavior change in OpenSSL 3.0.0 (https://github.com/openssl/openssl/issues/16321):
1) Failure:
OpenSSL::TestHMAC#test_hmac [test/openssl/test_hmac.rb:12]:
<"9294727a3638bb1c13f48ef8158bfc9d"> expected but was
<"47de897f45806cf1a5a78277cd74b5dc">.
A repro for convenience:
require "openssl"
instance = OpenSSL::HMAC.new('key', 'SHA1')
a = instance
b = instance
exit 1 if a != b
I wonder, will the OpenSSL 3.x support be API compatible with older versions? Going through several PRs, it seems that it probably will, but there are so many changes (for better, I like what I have seen so far).
@rhenium Thanks for all your work on this. Running on Windows mingw Ruby master with this branch, OpenSSL 3.0.0 had two failures and a few more errors. If you need any help with Windows testing...
Trying to backport this patch set to Ruby 3.0.2, I am struggling with these test failures:
1) Failure:
TestGemRemoteFetcher#test_do_not_allow_invalid_client_cert_auth_connection [/builddir/build/BUILD/ruby-3.0.2/test/rubygems/test_gem_remote_fetcher.rb:954]:
[Gem::RemoteFetcher::FetchError] exception expected, not #<OpenSSL::PKey::RSAError: Neither PUB key nor PRIV key>.
2) Error:
TestGemRemoteFetcher#test_ssl_client_cert_auth_connection:
OpenSSL::PKey::RSAError: Neither PUB key nor PRIV key
/builddir/build/BUILD/ruby-3.0.2/.ext/common/openssl/pkey.rb:318:in `initialize'
/builddir/build/BUILD/ruby-3.0.2/.ext/common/openssl/pkey.rb:318:in `new'
/builddir/build/BUILD/ruby-3.0.2/.ext/common/openssl/pkey.rb:318:in `new'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request.rb:57:in `configure_connection_for_https'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request/https_pool.rb:6:in `setup_connection'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request/http_pool.rb:39:in `make_connection'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request/http_pool.rb:20:in `checkout'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request.rb:127:in `connection_for'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request.rb:186:in `perform_request'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/request.rb:152:in `fetch'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/remote_fetcher.rb:316:in `request'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/remote_fetcher.rb:216:in `fetch_http'
/builddir/build/BUILD/ruby-3.0.2/lib/rubygems/remote_fetcher.rb:255:in `fetch_path'
/builddir/build/BUILD/ruby-3.0.2/test/rubygems/test_gem_remote_fetcher.rb:939:in `block in test_ssl_client_cert_auth_connection'
/builddir/build/BUILD/ruby-3.0.2/test/rubygems/test_gem_remote_fetcher.rb:1011:in `with_configured_fetcher'
/builddir/build/BUILD/ruby-3.0.2/test/rubygems/test_gem_remote_fetcher.rb:936:in `test_ssl_client_cert_auth_connection'
Consulting with RH OpenSSL folks, the issue might be that the client.pem as well as invalid_client.pem contain certificate and private key in the same file. Is there a chance to have additional test case covering this situation?
Just FTR, this can't be reproduced by openssl utility, which can read the appropriate parts of the file just fine. But this is the approach it uses:
https://github.com/openssl/openssl/blob/af5e63e1e3300f784f302a5d3309bf673cc08894/apps/pkey.c#L217-L220 https://github.com/openssl/openssl/blob/af5e63e1e3300f784f302a5d3309bf673cc08894/apps/lib/apps.c#L978-L992
Consulting with RH OpenSSL folks, the issue might be that the client.pem as well as invalid_client.pem contain certificate and private key in the same file. Is there a chance to have additional test case covering this situation?
Commit https://github.com/ruby/openssl/pull/399/commits/1672668e07d037e92a6e643906910ef038d9596b ("pkey: use OSSL_STORE to load encrypted PEM on OpenSSL 3.0") was using OSSL_DECODER_from_bio() incorrectly and was apparently stopping after encountering the first PEM block in the string.
Commit https://github.com/ruby/openssl/pull/399/commits/6f5a47f1e24ee403181c11a58e35f8c8535fd615 ("test/openssl/test_pkey_rsa: test concatenated PEM parsing") adds a test case and commit https://github.com/ruby/openssl/pull/399/commits/de5277e57f6487b53959645055807e6913d52aa3 ("pkey: use OSSL_DECODER to load encrypted PEM on OpenSSL 3.0") replaces the problematic change.
Commit 6f5a47f ("test/openssl/test_pkey_rsa: test concatenated PEM parsing") adds a test case and commit de5277e ("pkey: use OSSL_DECODER to load encrypted PEM on OpenSSL 3.0") replaces the problematic change.
Thx a lot :clap: I'll give them a try.
During testing, I have randomly hit the following error:
1) Failure:
OpenSSL::TestPKey#test_s_generate_parameters [/builddir/build/BUILD/ruby-3.0.2/test/openssl/test_pkey.rb:44]:
RuntimeError expected but nothing was raised.
I cannot reproduce the issue, though :/
Testing Puma against OpenSSL 3.x, I observer the following error:
7) Failure:
TestPumaControlCli#test_control_ssl
[/builddir/build/BUILD/puma-4.3.6/usr/share/gems/gems/puma-4.3.6/test/test_pumactl.rb:170]:
Expected /Command\ stop\ sent\ success/ to match "SSL_read: unexpected
eof while reading\n".
It seems that this is caused by OpenSSL changing its behavior in EOF handling in SSL_read and this is the affected code:
https://github.com/ruby/openssl/blob/8193b7322ece2548eaf3d8acd1aec32bfc2cfdb9/ext/openssl/ossl_ssl.c#L1834-L1835
The issue is that now SSL_ERROR_SSL error is raised instead of SSL_ERROR_SYSCALL. This was changed by the following commit:
https://github.com/openssl/openssl/commit/d924dbf4ae127c68463bcbece04b6e06abc58928
Checking this with @beldmit, I was given openssl/openssl/pull/11735 as an example to fix this issue. If I understand it correctly, it should be enough to set the SSL_OP_IGNORE_UNEXPECTED_EOF if available and this should make the SSL_read to return the SSL_ERROR_ZERO_RETURN instead, which is already handled as EOF.
I am not sure it's the best possible fix, but I had to introduce this option for backward compatibility with IIS, if I'm not mistaken.
See also https://github.com/openssl/openssl/issues/11209#issuecomment-623597166
This naive patch fixes the Puma issue:
$ git diff
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index 3b425ca..40e748c 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1812,6 +1812,10 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
if (!ssl_started(ssl))
rb_raise(eSSLError, "SSL session is not started yet");
+#ifdef SSL_OP_IGNORE_UNEXPECTED_EOF
+ SSL_set_options(ssl, SSL_OP_IGNORE_UNEXPECTED_EOF);
+#endif
+
ilen = NUM2INT(len);
if (NIL_P(str))
str = rb_str_new(0, ilen);
@beldmit since you have mentioned IIS, do you think this should be better handled on server side and therefore should I report this issue to Puma?
The issue is that now
SSL_ERROR_SSLerror is raised instead ofSSL_ERROR_SYSCALL. This was changed by the following commit:
I wasn't aware of the behavior change. (This seems to be another case test code was missing.)
Personally I've never been a fan of the current behavior of ruby/openssl, to treat unexpected EOF the same as a proper shutdown. This is for backwards compatibility, but it's a protocol error and potentially makes applications vulnerable to TLS truncation attack. IMO, this was something we needed to get rid of sooner or later.
If OpenSSL now natively provides an option to turn on/off the behavior, it may make sense to let user decide.
SSL_OP_IGNORE_UNEXPECTED_EOF
In any case, we should provide a matching constant under OpenSSL::SSL namespace, just as we do for all other SSL_OP_* flags.
https://github.com/ruby/openssl/blob/8193b7322ece2548eaf3d8acd1aec32bfc2cfdb9/ext/openssl/ossl_pkey_dh.c#L67-L69
Should the example above be updated to not use the deprecated #generate_key!?
Most of the patches I posted in this draft PR have been merged to master with more polishing:
- #468
- #478
- #479
- #480
- #481
Our test suite now runs successfully with OpenSSL 3.0.1. However, we still use some deprecated functions in OpenSSL 3.0 and you will see a number of compilation warnings.
The 3 commits remaining in this PR are wrappers for EVP_PKEY_todo() family. These would be nice to have on OpenSSL 3.0.
How is the progress on this? I'm starting to test Jammy for server deployments and with libssl-dev 3.0.2 it won't compile as it's limited to OpenSSL >= 1.0.1, < 3.0.0
Closing this PR, changes already landed on master branch two years ago.
The openssl gem version 3.0.0 (released 2021-12) or later is compatible with OpenSSL 3.0.