-
Notifications
You must be signed in to change notification settings - Fork 174
add OpenSSL Provider support #635
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
Conversation
056a641
to
efc2ed1
Compare
efc2ed1
to
6c46c41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this!
This is something we definitely should have.
@rhenium |
I wonder why CI is failing on Windows. |
With Windows and Ruby over 3.2, OpenSSL 3 will be used. refs: |
I've got msys2 and vcpkg tools on my systems, and they have legacy.dll in different locations. These dll's are bundled with Windows Rubies. Where should they be located? Or, what is the 'default search path'? |
I've got Windows ucrt fixed locally, the mingw & ucrt builds are being built for GitHub Actions, should be about 40 minutes, as the full ruby/ruby test suite is run. The mswin build isn't done yet, that may take till tomorrow. Where the This is due to MSYS2 patching OpenSSL to assign paths for misc OpenSSL folders relative to where the main OpenSSL dll's are located, which is in Pinging @larskanis for Ruby 3.2. |
The Windows head builds (mingw, ucrt, & mswin) have been fixed. I patched test.yml in my fork with needed changes. See Actions here, the patch is here. It removes the Note that the existing Actions steps (or something equivalent to them) will probably be needed in ruby/ruby,.. |
@MSP-Greg |
.github/workflows/test.yml
Outdated
@@ -130,7 +136,7 @@ jobs: | |||
run: echo "OPENSSL_CONF=$(pwd)/test/openssl/fixtures/ssl/openssl_fips.cnf" >> $GITHUB_ENV | |||
if: matrix.fips-enabled | |||
|
|||
- name: set fips enviornment variable for testing. | |||
- name: set fips environment variable for testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding and fixing a typo. However, I don't think this is related to this PR's main topic. Maybe better to send another PR for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened the PR #641 for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm...
Should I change the commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I think so. I already merged the commit only fixing the typo. So, I think after you will finish the review and fix everything in this PR, I think you (or a maintainer) needs to squash the commits in this PR into 1 commit on the latest master branch. We call it rebasing the PR on master branch. Could you do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typo fix came from my commit, which modified the Actions workflow file, which isn't part of the gem, or copied to ruby/ruby, etc.
Fixing a pair of transposed comment characters in a file that is already being modified seemed trivial...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you squash the commits in this PR to the 1 commit, it's nice to have the following line in the bottom of the commit message. You can check the examples in the past commits by git log
on the master
branch.
Co-authored-by: MSP-Greg <Greg.mpls@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a constant that can be added, similar to These locations are coded in the OpenSSL files, and may be patched by the build system (as in MSYS2, vcpkg, etc). MSYS2 does so and they have changed the locations in the past. I think the current locations are unlikely to change. |
There seems to be an API in OpenSSL master to get the default load path, but it does not seem to exist in 3.1 or 3.0 https://github.com/openssl/openssl/blob/openssl-3.1/include/openssl/provider.h |
Co-authored-by: MSP-Greg <Greg.mpls@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready! Thanks so much for your patience.
Let me squash commits and rebase on top of the current master, which has since then updated .github/workflows/test.yaml. |
90c4c7c
to
189c167
Compare
Merged now. Thank you so much! |
Thank you, too! |
@rhenium @QWYNG mswin and vs2019 environment is broken at https://github.com/ruby/ruby/actions/runs/5309762723/jobs/9610845792#step:21:951 Can you look this? |
Thanks for informing me. I will look into it. It's odd, though, because the same workflow passed on my rhenium/ruby fork, with both vs2019 and vs2022. https://github.com/rhenium/ruby/actions/runs/5304896653/jobs/9601511128 |
They are using different versions of OpenSSL from vcpkg. I wasn't able to pinpoint the change from https://github.com/microsoft/vcpkg/commits/master/ports/openssl, but I suppose something fixed the installation paths for providers. As an easy workaround, I will try clearing the vcpkg caches. Perhaps should the workflow attempt to update the packages? On ruby/ruby:
On rhenium/ruby:
|
I suspect that ruby/ruby will need MSYS2 has a patch in their OpenSSL package ('relocation.patch') that makes several (all?) of the OpenSSL default file system locations relative to the OpenSSL dll's. MSFT vcpkg does not have an equivalent patch, and the values are hard references to various directories. MSFT vcpkg is broken right now. See microsoft/vcpkg#31908. |
Yes, that appears to be the root cause. I installed OpenSSL with |
OpenSSL::Provider not loading for me. Ruby version: 3.2.2
Any help, please? |
What is your used OpenSSL version ( Lines 65 to 67 in f948e6b
|
have you solved the problem @junaruga ? I get the same problem with you |
It works now. we need to make sure that we use correct native OpenSSL version |
The provider feature only works with OpenSSL 3 or later versions. Perhaps the error message in OpenSSL 1.0/1.1 can be better. |
Hi! Thank you for great gem.
This PR fix #567
Currently Ruby does not seem to have an API to load providers
To add a provider, you need to set the following file in OPENSSL_CONF
This change provides an API to add a provider using
OSSL_PROVIDER_load
I am just a beginner to Ruby C extensions. I would love to get a review!
ref:
https://www.openssl.org/docs/manmaster/man3/OSSL_PROVIDER_load.html