Improve MiniSSL for users without OpenSSL available

[JuanitoFatas]

Description

SSL constants removed in 3a127f7. And for people who did not install from OpenSSL extension, this change will end up here.

puma/ext/puma_http11/mini_ssl.c

Line 12 in 91e57f4

#ifdef HAVE_OPENSSL_BIO_H

which HAVE_OPENSSL_BIO_H is false, then arrived at this code path

puma/ext/puma_http11/mini_ssl.c

Lines 555 to 562 in 91e57f4

	void Init_mini_ssl(VALUE puma) {
	VALUE mod;
	mod = rb_define_module_under(puma, "MiniSSL");
	rb_define_class_under(mod, "SSLError", rb_eStandardError);
	rb_define_singleton_method(mod, "check", raise_error, 0);
	}

resulted in SSLError.

Check if the OPENSSL_VERSION constant is defined to fix this.

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] the pull request title.
• I have added 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.

[MSP-Greg]

@JuanitoFatas

Thanks. I started working on this, but got distracted. Might it be better to not even load the definition of MiniSSL in minissl.rb if Puma is compiled without SSL support? Same for the ssl methods in Binder and loading of Puma::MiniSSL::ContextBuilder.

Because MiniSSL is compiled in puma_http11, some of the 'requires' need their order changed. I wanted to add at least one Actions job with SSL support. Also, I'm not that familiar with the other OpenSSL variants, like libressl.

[sj26]

Skipping minissl entirely would also work, but felt like a bigger change. Absolutely do it, if you think that's best! We were opting for the light touch just to fix this loading problem introduced by the 5 beta for demonstration purposes.

[sj26]

(To prevent future regressions like, perhaps introducing compile options into the build matrix would work? Not sure if it's worthwhile.)

[MSP-Greg]

@sj26

I try to help with SSL issues, and apologies, I never thought about compiling without SSL support. But, it is a valid need, and should probably be tested.

The other issue that is somewhat mixed with this is, if one compiles with SSL support, then uses Puma without binding to an SSL connection, can it be setup to not load all the SSL related code, including not loading the OpenSSL system libraries. I kind of stopped when I was trying to see if the code changes could be separated. A long weekend is a good time, so...

[sj26]

I think openssl might be loaded by other aspects of ruby anyway in almost all applications which use puma, we just don't have openssl headers available in production for the bundle to build puma with ssl bindings, so it's probably a pretty uncommon case! We appreciate the attention. Even if this was merged as-is for a fix, and then you considered avoiding minissl entirely for puma without ssl — either way. :-)

(We use unix sockets from puma to another layer which handles ssl.)

[MSP-Greg]

I think openssl might be loaded by other aspects of ruby anyway in almost all applications which use puma

Now that I'm being reminded, I think nio4r always loads Ruby OpenSSL. Also, not all the tests that use SSL are properly guarded, so that's more code if we add 'no SSL' to CI.

Let me get the code for fixing building/compiling with ENV["DISABLE_SSL"], and the other issues can be addressed later.

By chance, can you test it?

[sj26]

By chance, can you test it?

Absolutely! We already have a copy of puma running in pre-production using the patch on this branch. But we'd be happy to test anything you'd like to ship here.

I am about to head to bed, but can follow up in the morning [~9am utc+10].

[MSP-Greg]

@sj26

Sorry for the delay. See PR #2305. If you can try that, it would be appreciated. I haven't added a 'no SSL' build to CI yet. Soon, hopefully tomorrow.

I was mistaken about NIO. I believe nio4r 2.5.2 needs OpenSSL for JRuby & Windows, but it doesn't load it. That was my mistake, and it's been corrected in master, so next release it should be fixed.

[JuanitoFatas]

Closing this in favor of #2305. Thanks everyone!