decidim / decidim-bulletin-board

GNU Affero General Public License v3.0
5 stars 6 forks source link

Fix issue with OpenSSL 3.0 #294

Closed ahukkanen closed 2 years ago

ahukkanen commented 2 years ago

There is still a compatibility issue in the Bulletin Board with OpenSSL 3.0.

The comment is stating that after jwt/ruby-jwt#375 is merged and released, the extra stuff in the import code is no longer needed which seems to make sense looking at the changes in that PR. These changes became part of jwt-2.2.3 (2021-04-19), so these should no longer be needed.

When running the current code under OpenSSL 3.0, you will see the following exception:

     @bulletin_board ||= Decidim::BulletinBoard::Client.new

     OpenSSL::PKey::PKeyError:
       rsa#set_key= is incompatible with OpenSSL 3.0
     # /home/ahukkanen/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/decidim-bulletin_board-0.24.0/lib/decidim/bulletin_board/jwk_utils.rb:19:in `set_key'
     # /home/ahukkanen/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/decidim-bulletin_board-0.24.0/lib/decidim/bulletin_board/jwk_utils.rb:19:in `import_private_key'
     # /home/ahukkanen/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/decidim-bulletin_board-0.24.0/lib/decidim/bulletin_board/settings.rb:13:in `initialize'
     # /home/ahukkanen/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/decidim-bulletin_board-0.24.0/lib/decidim/bulletin_board/client.rb:26:in `new'
     # /home/ahukkanen/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/decidim-bulletin_board-0.24.0/lib/decidim/bulletin_board/client.rb:26:in `initialize'

Therefore, we should remove this code as stated in the comment and let jwt itself handle this part.

codecov-commenter commented 2 years ago

Codecov Report

Merging #294 (7e53020) into develop (1b4db9e) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #294   +/-   ##
========================================
  Coverage    69.57%   69.57%           
========================================
  Files           96       96           
  Lines         1630     1630           
========================================
  Hits          1134     1134           
  Misses         496      496           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ahukkanen commented 2 years ago

We should probably also test this under OpenSSL 3, so we would catch this kind of issues in the CI.

I think the main problem is that the base Ruby image we are relying on is based on Debian Bullseye: https://github.com/docker-library/ruby/blob/41208aad6764925f0f082a695a076f7f39af1233/3.1/bullseye/Dockerfile#L7

And Bullseye ships with OpenSSL 1.1 by default: https://packages.debian.org/bullseye/openssl

Currently we would either need to install OpenSSL 3 manually to the container when we build it or build an alternative container for testing. It's a bit cumbersome... But Debian Bookworm should be released shortly looking at their release history, so we could just wait for that and then wait the Ruby Docker containers to be released that are based on that release. Then just swap the base Ruby image to e.g. ruby:3.1.2-bookworm (if they'll ever release that, we'll see).

I suggest currently testing it manually and after Bookworm is released, see again what our options are.

ahukkanen commented 2 years ago

The least that I could do is to run the client specs under Ubuntu 22.04 as of 7e53020fb2c6db5aedd450a088ac447dcaf28af5.

Running the E2E tests under OpenSSL 3 would be harder as explained above. I think the "easiest" way around that would be to uninstall the base image's openssl package and build OpenSSL 3 from source but that may have unexpected consequences in other packages or the Ruby native modules that were built using OpenSSL 1.

ahukkanen commented 2 years ago

@andreslucena Forgot to mention in the maintainers meeting but I'd appreciate to get this also merged so that I can release new version of the bulletin board ruby client with OpenSSL 3 support (the current release is broken under OpenSSL).

To test this:

  1. Run the ruby client specs at develop under Ubuntu 22.04 -> Expect to see lots of errors
  2. Run the ruby client specs at this PR branch under Ubuntu 22.04 -> Expect to see green

This is not urgent regarding the 0.27 release but we need this to fix the issue for the next 0.28 release. Just wanted to ping to you that forgot to mention about this one.

andreslucena commented 2 years ago

The comment is stating that after https://github.com/jwt/ruby-jwt/pull/375 is merged and released, the extra stuff in the import code is no longer needed which seems to make sense looking at the changes in that PR.

I found out this TODO last year. I left an issue, so we don't miss it, so this will fix #248