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

Updates for OpenSSL 3 #2800

Merged
merged 1 commit into from Jan 22, 2022
Merged

Updates for OpenSSL 3 #2800

merged 1 commit into from Jan 22, 2022

Conversation

MSP-Greg
Copy link
Member

@MSP-Greg MSP-Greg commented Jan 16, 2022

Description

This makes a change for OpenSSL 3 which is recommended by OpenSSL, and also removes the deprecation warnings when compiled with OpenSSL 3.

I've only tested Puma with OpenSSL 3 on Windows, if anyone can test on other platforms, that would be appreciated. Make sure to use a Ruby that is also compiled with OpenSSL 3.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@nateberkopec
Copy link
Member

Is there something going up upstream w/2.5 re: our build failures? Kinda random.

@nateberkopec nateberkopec added waiting-for-review Waiting on review from anyone contrib-wanted labels Jan 17, 2022
@nateberkopec
Copy link
Member

Labeling as contrib-wanted, anyone is welcome to checkout this branch, run tests locally against OpenSSL3 and report back if it worked or not.

@dentarg
Copy link
Member

dentarg commented Jan 17, 2022

Is there something going up upstream w/2.5 re: our build failures? Kinda random.

See ruby/setup-ruby#264 (comment)

@dentarg
Copy link
Member

dentarg commented Jan 17, 2022

On macOS 12.1, I installed Ruby 3.2.0-dev using ruby-build. It uses OpenSSL 3.0.1:

ruby 3.2.0dev (2022-01-17T05:56:11Z master b4e362d444) [x86_64-darwin21]
RUBYOPT:

                         Puma::MiniSSL                   Ruby OpenSSL
OPENSSL_LIBRARY_VERSION: OpenSSL 3.0.1 14 Dec 2021       OpenSSL 3.0.1 14 Dec 2021
        OPENSSL_VERSION: OpenSSL 3.0.1 14 Dec 2021       OpenSSL 3.0.1 14 Dec 2021

Current master (5f255fc) compiles (with warnings) and passes our SSL tests (bundle exec m test/test_puma_server_ssl.rb test/test_minissl.rb test/test_integration_ssl.rb).

However, our CI turns compile warnings into errors:

$ MAKE_WARNINGS_INTO_ERRORS=true bundle exec rake compile
cd tmp/x86_64-darwin21/puma_http11/3.2.0
/Users/dentarg/.rubies/3.2.0-dev/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/puma_http11/extconf.rb
using OpenSSL pkgconfig (openssl.pc)
checking for openssl/bio.h... yes
checking for DTLS_method() in openssl/ssl.h... yes
checking for TLS_server_method() in openssl/ssl.h... yes
checking for SSL_CTX_set_min_proto_version(NULL, 0) in openssl/ssl.h... yes
checking for X509_STORE_up_ref()... yes
checking for SSL_CTX_set_ecdh_auto(NULL, 0) in openssl/ssl.h... yes
checking for Random.bytes... yes
checking for whether -Werror is accepted as CFLAGS... yes
checking for whether -Wno-implicit-fallthrough is accepted as CFLAGS... yes
creating Makefile
cd -
cd tmp/x86_64-darwin21/puma_http11/3.2.0
/usr/bin/make
compiling ../../../../ext/puma_http11/mini_ssl.c
../../../../ext/puma_http11/mini_ssl.c:98:8: error: 'DH_new' is deprecated [-Werror,-Wdeprecated-declarations]
  dh = DH_new();
       ^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/dh.h:199:1: note: 'DH_new' has been explicitly marked deprecated here
OSSL_DEPRECATEDIN_3_0 DH *DH_new(void);
^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/macros.h:182:49: note: expanded from macro 'OSSL_DEPRECATEDIN_3_0'
#   define OSSL_DEPRECATEDIN_3_0                OSSL_DEPRECATED(3.0)
                                                ^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/macros.h:62:52: note: expanded from macro 'OSSL_DEPRECATED'
#     define OSSL_DEPRECATED(since) __attribute__((deprecated))
                                                   ^
../../../../ext/puma_http11/mini_ssl.c:112:34: error: 'DH_set0_pqg' is deprecated [-Werror,-Wdeprecated-declarations]
  if (p == NULL || g == NULL || !DH_set0_pqg(dh, p, NULL, g)) {
                                 ^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/dh.h:255:1: note: 'DH_set0_pqg' has been explicitly marked deprecated here
OSSL_DEPRECATEDIN_3_0 int DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g);
^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/macros.h:182:49: note: expanded from macro 'OSSL_DEPRECATEDIN_3_0'
#   define OSSL_DEPRECATEDIN_3_0                OSSL_DEPRECATED(3.0)
                                                ^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/macros.h:62:52: note: expanded from macro 'OSSL_DEPRECATED'
#     define OSSL_DEPRECATED(since) __attribute__((deprecated))
                                                   ^
../../../../ext/puma_http11/mini_ssl.c:113:5: error: 'DH_free' is deprecated [-Werror,-Wdeprecated-declarations]
    DH_free(dh);
    ^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/dh.h:200:1: note: 'DH_free' has been explicitly marked deprecated here
OSSL_DEPRECATEDIN_3_0 void DH_free(DH *dh);
^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/macros.h:182:49: note: expanded from macro 'OSSL_DEPRECATEDIN_3_0'
#   define OSSL_DEPRECATEDIN_3_0                OSSL_DEPRECATED(3.0)
                                                ^
/Users/dentarg/.rubies/3.2.0-dev/openssl/include/openssl/macros.h:62:52: note: expanded from macro 'OSSL_DEPRECATED'
#     define OSSL_DEPRECATED(since) __attribute__((deprecated))
                                                   ^
3 errors generated.
make: *** [mini_ssl.o] Error 1
rake aborted!
Command failed with status (2): [/usr/bin/make...]
/Users/dentarg/.gem/ruby/3.2.0/gems/rake-compiler-1.1.7/lib/rake/extensiontask.rb:181:in `block (2 levels) in define_compile_tasks'
/Users/dentarg/.gem/ruby/3.2.0/gems/rake-compiler-1.1.7/lib/rake/extensiontask.rb:180:in `block in define_compile_tasks'
/Users/dentarg/.rubies/3.2.0-dev/bin/bundle:25:in `load'
/Users/dentarg/.rubies/3.2.0-dev/bin/bundle:25:in `<main>'
Tasks: TOP => compile => compile:x86_64-darwin21 => compile:puma_http11:x86_64-darwin21 => copy:puma_http11:x86_64-darwin21:3.2.0 => tmp/x86_64-darwin21/puma_http11/3.2.0/puma_http11.bundle
(See full trace by running task with --trace)

Using this branch there's no warnings/errors:

$ gh pr checkout 2800
Switched to branch 'openssl-3'

$ MAKE_WARNINGS_INTO_ERRORS=true bundle exec rake compile
cd tmp/x86_64-darwin21/puma_http11/3.2.0
/Users/dentarg/.rubies/3.2.0-dev/bin/ruby -I. -r.rake-compiler-siteconf.rb ../../../../ext/puma_http11/extconf.rb
using OpenSSL pkgconfig (openssl.pc)
checking for openssl/bio.h... yes
checking for DTLS_method() in openssl/ssl.h... yes
checking for TLS_server_method() in openssl/ssl.h... yes
checking for SSL_CTX_set_min_proto_version(NULL, 0) in openssl/ssl.h... yes
checking for X509_STORE_up_ref()... yes
checking for SSL_CTX_set_ecdh_auto(NULL, 0) in openssl/ssl.h... yes
checking for SSL_get1_peer_certificate() in openssl/ssl.h... yes
checking for Random.bytes... yes
checking for whether -Werror is accepted as CFLAGS... yes
checking for whether -Wno-implicit-fallthrough is accepted as CFLAGS... yes
creating Makefile
cd -
cd tmp/x86_64-darwin21/puma_http11/3.2.0
/usr/bin/make
compiling ../../../../ext/puma_http11/mini_ssl.c
linking shared-object puma/puma_http11.bundle
cd -
cp ext/puma_http11/extconf.rb tmp/x86_64-darwin21/stage/ext/puma_http11/extconf.rb
cp ext/puma_http11/mini_ssl.c tmp/x86_64-darwin21/stage/ext/puma_http11/mini_ssl.c
/usr/bin/make install target_prefix=
/usr/local/bin/ginstall -c -m 0755 puma_http11.bundle /Users/dentarg/src/dup/puma_ruby-3.2-openssl-3/lib/puma
cp tmp/x86_64-darwin21/puma_http11/3.2.0/puma_http11.bundle tmp/x86_64-darwin21/stage/lib/puma/puma_http11.bundle

👍

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Jan 17, 2022

@nateberkopec

Is there something going up upstream w/2.5 re: our build failures? Kinda random.

Don't ask. Fixed by #2801. Really squirrelly issue when a Gemfile includes Psych (either directly or as a dependency) and bundle install --jobs X is used, I think only Ruby 2.5. Using --jobs seems to activate the Psych in the bundle, and the RubyGems shipped with Ruby 2.5 crashes...

@dentarg

Thanks for building/testing. I suspect it should also work fine with Ruby 3.1.0. JFYI, I was starting to lose my patience with the OpenSSL 3.0 issue. Not.pretty.

EDIT: Just built Ruby master with OpenSSL 3.0.1 on Ubuntu 20.04. Puma compiled without warnings, tests passed. This seems to work on all three OS's...

Also, re the odd upstream bug. The current Psych gem is compatible with Rubies 2.4 and later. By compatible, its test suite passes with those Rubies. But, RubyGems and RDoc use Psych, and the breaking changes in Psych 4.x cause errors in RubyGems.

So, in Actions, if one's Gemfile.lock includes Psych, older Rubies also need to have RubyGems updated. The current ruby/setup-ruby action does not do that, which is what caused the job failures.

@MSP-Greg MSP-Greg marked this pull request as ready for review January 18, 2022 19:20
@MSP-Greg
Copy link
Member Author

Earlier today I ran several hundred thousand requests with this using Ruby master/OpenSSL 3.0.1 on WSL2/Ubuntu 20.04. Also, the PR makes no changes to the code used for OpenSSL 1.1.1 and earlier. Hence, removing the draft designation...

@voxik
Copy link
Contributor

voxik commented Jan 20, 2022

Testing on Fedora Rawhide with:

$ rpm -q ruby
ruby-3.1.0-160.fc36.x86_64

$ rpm -q openssl-libs
openssl-libs-3.0.0-1.fc36.x86_64

The build warnings are gone and the tests keep passing:

--- old
+++ new
@@ -94,6 +94,7 @@
 checking for SSL_CTX_set_min_proto_version(NULL, 0) in openssl/ssl.h... yes
 checking for X509_STORE_up_ref()... yes
 checking for SSL_CTX_set_ecdh_auto(NULL, 0) in openssl/ssl.h... yes
+checking for SSL_get1_peer_certificate() in openssl/ssl.h... yes
 checking for Random.bytes... yes
 creating Makefile
 current directory: /builddir/build/BUILD/puma-5.5.2/usr/share/gems/gems/puma-5.5.2/ext/puma_http11
@@ -102,32 +103,9 @@
 rm -fr puma_http11.so false *.o  *.bak mkmf.log .*.time
 current directory: /builddir/build/BUILD/puma-5.5.2/usr/share/gems/gems/puma-5.5.2/ext/puma_http11
 ["make", "DESTDIR="]
-gcc -I. -I/usr/include -I/usr/include/ruby/backward -I/usr/include -I.  -DHAVE_OPENSSL_BIO_H -DHAVE_DTLS_METHOD -DHAVE_TLS_SERVER_METHOD -DHAVE_SSL_CTX_SET_MIN_PROTO_VERSION -DHAVE_X509_STORE_UP_REF -DHAVE_SSL_CTX_SET_ECDH_AUTO -DHAVE_RANDOM_BYTES    -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -m64 -o http11_parser.o -c http11_parser.c
-gcc -I. -I/usr/include -I/usr/include/ruby/backward -I/usr/include -I.  -DHAVE_OPENSSL_BIO_H -DHAVE_DTLS_METHOD -DHAVE_TLS_SERVER_METHOD -DHAVE_SSL_CTX_SET_MIN_PROTO_VERSION -DHAVE_X509_STORE_UP_REF -DHAVE_SSL_CTX_SET_ECDH_AUTO -DHAVE_RANDOM_BYTES    -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -m64 -o mini_ssl.o -c mini_ssl.c
-mini_ssl.c: In function 'get_dh2048':
-mini_ssl.c:98:3: warning: 'DH_new' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
-   98 |   dh = DH_new();
-      |   ^~
-In file included from /usr/include/openssl/dsa.h:51,
-                 from /usr/include/openssl/x509.h:37,
-                 from /usr/include/openssl/ssl.h:31,
-                 from mini_ssl.c:10:
-/usr/include/openssl/dh.h:199:27: note: declared here
-  199 | OSSL_DEPRECATEDIN_3_0 DH *DH_new(void);
-      |                           ^~~~~~
-mini_ssl.c:112:3: warning: 'DH_set0_pqg' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
-  112 |   if (p == NULL || g == NULL || !DH_set0_pqg(dh, p, NULL, g)) {
-      |   ^~
-/usr/include/openssl/dh.h:255:27: note: declared here
-  255 | OSSL_DEPRECATEDIN_3_0 int DH_set0_pqg(DH *dh, BIGNUM *p, BIGNUM *q, BIGNUM *g);
-      |                           ^~~~~~~~~~~
-mini_ssl.c:113:5: warning: 'DH_free' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
-  113 |     DH_free(dh);
-      |     ^~~~~~~
-/usr/include/openssl/dh.h:200:28: note: declared here
-  200 | OSSL_DEPRECATEDIN_3_0 void DH_free(DH *dh);
-      |                            ^~~~~~~
-gcc -I. -I/usr/include -I/usr/include/ruby/backward -I/usr/include -I.  -DHAVE_OPENSSL_BIO_H -DHAVE_DTLS_METHOD -DHAVE_TLS_SERVER_METHOD -DHAVE_SSL_CTX_SET_MIN_PROTO_VERSION -DHAVE_X509_STORE_UP_REF -DHAVE_SSL_CTX_SET_ECDH_AUTO -DHAVE_RANDOM_BYTES    -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -m64 -o puma_http11.o -c puma_http11.c
+gcc -I. -I/usr/include -I/usr/include/ruby/backward -I/usr/include -I.  -DHAVE_OPENSSL_BIO_H -DHAVE_DTLS_METHOD -DHAVE_TLS_SERVER_METHOD -DHAVE_SSL_CTX_SET_MIN_PROTO_VERSION -DHAVE_X509_STORE_UP_REF -DHAVE_SSL_CTX_SET_ECDH_AUTO -DHAVE_SSL_GET1_PEER_CERTIFICATE -DHAVE_RANDOM_BYTES    -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -m64 -o http11_parser.o -c http11_parser.c
+gcc -I. -I/usr/include -I/usr/include/ruby/backward -I/usr/include -I.  -DHAVE_OPENSSL_BIO_H -DHAVE_DTLS_METHOD -DHAVE_TLS_SERVER_METHOD -DHAVE_SSL_CTX_SET_MIN_PROTO_VERSION -DHAVE_X509_STORE_UP_REF -DHAVE_SSL_CTX_SET_ECDH_AUTO -DHAVE_SSL_GET1_PEER_CERTIFICATE -DHAVE_RANDOM_BYTES    -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -m64 -o mini_ssl.o -c mini_ssl.c
+gcc -I. -I/usr/include -I/usr/include/ruby/backward -I/usr/include -I.  -DHAVE_OPENSSL_BIO_H -DHAVE_DTLS_METHOD -DHAVE_TLS_SERVER_METHOD -DHAVE_SSL_CTX_SET_MIN_PROTO_VERSION -DHAVE_X509_STORE_UP_REF -DHAVE_SSL_CTX_SET_ECDH_AUTO -DHAVE_SSL_GET1_PEER_CERTIFICATE -DHAVE_RANDOM_BYTES    -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  -m64 -o puma_http11.o -c puma_http11.c
 rm -f puma_http11.so
 gcc -shared -o puma_http11.so http11_parser.o mini_ssl.o puma_http11.o -L. -L/usr/lib64 -Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1 -Wl,-dT,/builddir/build/BUILD/.package_note-rubygem-puma-5.5.2-1.fc36.x86_64.ld  -Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1   -m64  -lruby  -lssl -lcrypto -lm  -lc
 current directory: /builddir/build/BUILD/puma-5.5.2/usr/share/gems/gems/puma-5.5.2/ext/puma_http11

But I don't think I can judge the patch beyond that.

@voxik
Copy link
Contributor

voxik commented Jan 20, 2022

tests keep passing:

Actually, some test reported by @pvalena might be disabled, because they were flaky.

I have tested also against Ruby 3.0.3 with backported OpenSSL patches and no issues there.

@MSP-Greg
Copy link
Member Author

@voxik

See my comment #2804 (comment). Sorry, auto-pilot, I always run tests locally with bundle exec. As mentioned, we should skip tests that assume they were started via bundle exec.

@MSP-Greg MSP-Greg merged commit aa732fd into puma:master Jan 22, 2022
@MSP-Greg MSP-Greg deleted the openssl-3 branch January 22, 2022 15:05
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contrib-wanted ssl waiting-for-review Waiting on review from anyone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants