Add support for OpenSSL 3

[ClearlyClaire]

Starting form OpenSSL 3, PKey aren't mutable anymore, so we have to build them instead of separately setting private_key and public_key.

[donv]

Hi!

Any response on getting this merged?

[MBM1607]

lgtm, @ClearlyClaire any response from @zaru on getting this merged?

[collimarco]

+1 for this, it would be useful to have this update merged (and most importantly having this merged before support for previous openssl versions is dropped...)

[collimarco]

I am looking at the code line by line and in general seems good to me. However I have some doubts about the use of OpenSSL::ASN1::Sequence in VapidKey#to_pem and VapidKey#set_keys! , because it seems very low-level. I wonder if there's a better alternative.

For example here's another implementation (less code, high-level, widely used in production) of VapidKey#to_pem:
pushpad/web-push@20f40d9#diff-e8e36e8a5282b2fbf5503e699e222f3d5413c86cdbb94ecdd13148ea0c59e5f5

I wonder if we can keep something like that in OpenSSL 3 - I would feel more confident.

[collimarco]

Are these changes necessary at all?

Please try to use this https://github.com/pushpad/web-push (v2.0.0) and simply update the openssl version in the .gemspec to v3:

spec.add_dependency 'openssl', '~> 3.0'

Then run bundle update and bundle exec rspec... all tests are passing!

Maybe is it because the openssl gem v3 is still using the "OpenSSL library v1.1" on my machine? Or gem v3 always uses the C library v3?

[collimarco]

Ok, the gem version (e.g. v3) can be different from the underlying C library.

For example openssl gem v3 is still compatible with C library v1.1.

The breaking change seems related to updating the underlying C library to v3, not the gem, which can be updated to v3.

For the breaking changes caused by the C library, it seems that the Ruby core team is working on some workarounds at the gem level:

ruby/openssl#480

Maybe that will make all these changes to this gem unnecessary (???)

[ClearlyClaire]

I am looking at the code line by line and in general seems good to me. However I have some doubts about the use of OpenSSL::ASN1::Sequence in VapidKey#to_pem and VapidKey#set_keys! , because it seems very low-level. I wonder if there's a better alternative.

For example here's another implementation (less code, high-level, widely used in production) of VapidKey#to_pem: pushpad/web-push@20f40d9#diff-e8e36e8a5282b2fbf5503e699e222f3d5413c86cdbb94ecdd13148ea0c59e5f5

I wonder if we can keep something like that in OpenSSL 3 - I would feel more confident.

Good catch. The result is the same and it works on OpenSSL 3. Pushed the change.

[collimarco]

@ClearlyClaire Great! I wonder if it's possible to use a similar, high-level approach also for set_keys!

[alecslupu]

@collimarco, We have switched to web-push, version 2.1.0 and the error still persists.
Is there any chance this fix could be applied in a 2.1.1 version ?

https://github.com/decidim/decidim/actions/runs/3648589265/jobs/6162187603

[collimarco]

@alecslupu the Pushpad fork already supports openssl v3 (the ruby gem), but still uses openssl v1.1 (the C library). I also want to add support for OpenSSL v3 (C library) soon, but at the moment the official Ruby docker images still use C library v1.1, so we are ok with that.

My main doubt with the code in this pull request is if we really need to go low-level (e.g. using OpenSSL::ASN1::Sequence) or if there is any other simpler solution.

[alecslupu]

I totally understand the concern. I can confirm that the patch proposed in this PR, works okay for apps using OpenSSL v3 (C library).

I can confirm that Pushpad version is working fine with OpenSSL 3 gem having OpenSSL v1.1 (the C library), yet it fails when used with V3 C library.

[xfalcox]

@collimarco are you considering merging this in your fork? That would be a reason, at least for me, to move to it.

[collimarco]

I have just released web-push v3.0 which is compatible with both OpenSSL v1.1 and OpenSSL v3 🎉
https://github.com/pushpad/web-push

Note: in order to use that gem you need to use gem 'web-push' and rename Webpush to WebPush.

[Sjors]

I replaced the webpush gem with web-push for ForkMonitor (changeWebpush to WebPush in the code) and it no longer throws OpenSSL::PKey::PKeyError when trying to send a push message. Didn't look at the code changes though.

[timokleemann]

I can also confirm that renaming the gem from webpush to web-push fixes the OpenSSL::PKey::PKeyError (pkeys are immutable on OpenSSL 3.0) that bugged me all day. Thanks for the fix!