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

bf-cbc isn't a supported cipher in Ruby 3.2 #17

Closed

Conversation

UnderpantsGnome
Copy link
Contributor

No description provided.

@UnderpantsGnome UnderpantsGnome marked this pull request as draft March 11, 2023 21:41
@UnderpantsGnome UnderpantsGnome marked this pull request as ready for review March 11, 2023 21:55
@arianf
Copy link

arianf commented May 7, 2023

@ahoward Any chance of review this?

F, [2023-05-07T02:16:48.271999 #54] FATAL -- : unsupported (OpenSSL::Cipher::CipherError)
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/lib/sekrets.rb:319:in `initialize'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/lib/sekrets.rb:319:in `new'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/lib/sekrets.rb:319:in `cipher'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/lib/sekrets.rb:329:in `decrypt'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/bin/sekrets:228:in `block (2 levels) in run'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/lib/sekrets.rb:209:in `openw'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/bin/sekrets:226:in `block in run'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/lib/sekrets.rb:241:in `openr'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/bin/sekrets:223:in `run'
/usr/lib/ruby/gems/3.1.0/gems/main-6.3.0/lib/main/program/class_methods.rb:168:in `block in run'
/usr/lib/ruby/gems/3.1.0/gems/main-6.3.0/lib/main/program/class_methods.rb:157:in `catch'
/usr/lib/ruby/gems/3.1.0/gems/main-6.3.0/lib/main/program/class_methods.rb:157:in `run'
/usr/lib/ruby/gems/3.1.0/gems/main-6.3.0/lib/main/factories.rb:18:in `run'
/usr/lib/ruby/gems/3.1.0/gems/main-6.3.0/lib/main/factories.rb:25:in `Main'
/usr/lib/ruby/gems/3.1.0/gems/sekrets-1.13.0/bin/sekrets:3:in `<top (required)>'
/usr/bin/sekrets:25:in `load'
/usr/bin/sekrets:25:in `<main>'

Note this has less to do with ruby version than it has to do with OpenSSL 3 marking bf-cbc cipher as deprecated.

Copy link

@bak1an bak1an left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@UnderpantsGnome I think this approach will be problematic as there is no easy way to update (or downgrade) ruby with it.

It looks like old ciphers are still available in modern openssl but they are just disabled with default provider. legacy provider still has them.

openssl 3.2+ added support for providers in ruby/openssl#635

So just adding gem "openssl", "~> 3.2.0" and calling something like:

if defined?(OpenSSL::Provider)
  OpenSSL::Provider.load("legacy")
end

before loading sekrets does the trick.

It also looks like in previous openssl gem versions you can achieve the same with OPENSSL_CONF env variable but I have not explored that path.

@UnderpantsGnome
Copy link
Contributor Author

@bak1an That's a good tip for people coming to this, I have my doubts about this making it to main.

I'm not sure I see the Ruby version change (upgrade/downgrade) issue, 3.0 is already EOL as it is.

If you change your cipher you will need to re-encrypt things, but that shouldn't be happening very often. 🙂

@arianf
Copy link

arianf commented May 8, 2024

It's really a shame that this won't make it to main, as we still use this gem for ruby (non-rails) applications to do encryption

@bak1an
Copy link

bak1an commented May 8, 2024

@bak1an That's a good tip for people coming to this, I have my doubts about this making it to main.

I'm not sure I see the Ruby version change (upgrade/downgrade) issue, 3.0 is already EOL as it is.

If you change your cipher you will need to re-encrypt things, but that shouldn't be happening very often. 🙂

Unfortunately there are still pre ruby 3.0 apps out there.

And you can also build ruby 3.0+ with openssl 1.1 and it will work. So it is not even ruby version issue but your system's openssl version issue:

3.2.2 :008 > RUBY_VERSION
 => "3.2.2"
3.2.2 :009 > OpenSSL::OPENSSL_VERSION
 => "OpenSSL 1.1.1u  30 May 2023"
3.2.2 :010 > OpenSSL::Cipher.new("bf-cbc")
 => #<OpenSSL::Cipher:0x0000000105cee888>

I could also imagine a situation of gradual upgrade when part of the servers in the pool already have new ruby & openssl and other part don't. Having different ciphers for your sekrets file in such case would be slightly hard to manage (imagine someone also trying to modify secrets while this rollout going on :trollface: )

@bak1an
Copy link

bak1an commented May 8, 2024

It's really a shame that this won't make it to main, as we still use this gem for ruby (non-rails) applications to do encryption

@arianf Solution with loading legacy provider seems to be working fine. In case you can't get 3.2+ of openssl gem you can use something like:

[openssl_init]
providers = provider_sect

[provider_sect]
default = default_sect
legacy = legacy_sect

[default_sect]
activate = 1

[legacy_sect]
activate = 1

in your openssl config to make it work without any code modifications.

You can either directly modify system's /etc/ssl/openssl.cnf or copy it, modify and use OPENSSL_CONF=/path/to/new/openssl.cnf env variable to just make things work again.


But overall, BF-CBC does not seem good to be used in 2024. Even in 2017 it was not good #6

So unless there are maintainers in this repo perhaps a fork is required to properly migrate to modern ciphers.

@pyrareae
Copy link

pyrareae commented May 9, 2024

Even if this doesn't get merged to master you can reference this branch directly in your gemfile with git: xxx, branch: yyy or fork the repo. It might be a good opportunity for someone to maintain a new fork.

@pyrareae
Copy link

pyrareae commented May 9, 2024

@UnderpantsGnome I think this approach will be problematic as there is no easy way to update (or downgrade) ruby with it.

It looks like old ciphers are still available in modern openssl but they are just disabled with default provider. legacy provider still has them.

openssl 3.2+ added support for providers in ruby/openssl#635

So just adding gem "openssl", "~> 3.2.0" and calling something like:

if defined?(OpenSSL::Provider)
  OpenSSL::Provider.load("legacy")
end

before loading sekrets does the trick.

It also looks like in previous openssl gem versions you can achieve the same with OPENSSL_CONF env variable but I have not explored that path.

This is helpful 👍

@copiousfreetime
Copy link
Collaborator

FYI - only @ahoward has privileges to release a new version of the gem - I'll ping him and see if I can get privileges and go from there.

@UnderpantsGnome
Copy link
Contributor Author

I'll ping him and see if I can get privileges and go from there.

Thanks Jeremey! Long time no talk 🙂

@ahoward
Copy link
Owner

ahoward commented May 10, 2024

tracking this @UnderpantsGnome and @copiousfreetime -- happy to give perms to whomever but would like to peek just a little to review code and consider some route to entirely eliminate openssl moving fwd ... albatros ! ;-/. thoughts and prayers welcome.

@pyrareae
Copy link

It would definitely be good to have some kind of script for changing/upgrading ciphers built in moving forward, that would make updating pretty painless.

@UnderpantsGnome
Copy link
Contributor Author

@ahoward let me know if you want me to look into simplifying cipher changes.

@ahoward
Copy link
Owner

ahoward commented May 10, 2024

@copiousfreetime
Copy link
Collaborator

@bak1an
Copy link

bak1an commented May 10, 2024

What if openssl is kept (it comes with ruby anyway) but cipher is configurable (with some good modern defaults as suggested in #6 )?

Instructions or helper command for re encrypting secrets into different cipher would also be good.

@ahoward
Copy link
Owner

ahoward commented May 12, 2024

@UnderpantsGnome my concern - and i'd forgotten - is that, in addition to the cipher needing to tweak for ruby versions i think the key would also need to magically double in size? that is to say, the key would need to grow from 16 to 32 and and a re-crypt would need to happen - yes? i really want to roll something out but want to make sure i grok the issue and that the fix would work in the wild as it's difficult to test with certainty. also, we could catch up on phone as it's been a while if faster ...

@ahoward
Copy link
Owner

ahoward commented May 15, 2024

have a look at master will ya @arianf, @bak1an , @copiousfreetime , and @UnderpantsGnome .... if you like i'll push the 1.14.0 gem today

@bak1an
Copy link

bak1an commented May 15, 2024

Looks good @ahoward !

Thanks!

@UnderpantsGnome
Copy link
Contributor Author

@ahoward while this does silence the error when using an old cipher, it doesn't provide a way to move to a better cipher. Any newer projects of mine are using aes-256-cfb.

@bak1an
Copy link

bak1an commented May 15, 2024

@UnderpantsGnome Moving to a new cipher sounds like a separate thing and a bigger release since it will break compatibility for existing projects.

Making it just work on servers with modern openssl is a good start I would say.

@UnderpantsGnome
Copy link
Contributor Author

@bak1an that's fair. @ahoward I'm good with this.

@ahoward
Copy link
Owner

ahoward commented May 15, 2024

@ahoward while this does silence the error when using an old cipher, it doesn't provide a way to move to a better cipher. Any newer projects of mine are using aes-256-cfb.

totally want to do this next and love your feedback. two thoughts on this:

  • the whole reason sekrets came into existence was that ppl give up on security since it's too hard.... anyone used the stupid one-password api on projects in a ci pipeline... OMFG ;=/. so i want to do this but want a good design interface. for example
    • i think a nice way would be to allow keys in yaml format and that those would be (.key.yml vs. .key file) a nice place to hang our hat on simple to manage keys moving fwd. having a binary blob in a file is hard to manage. also could encode meta-data in the filename but.. bletch.... also very lightweight. AND we need to consider ENV configuration in CI because file formats and ENV....

    • due to the above point, this will be a 2.0 release. i also want to delete a ton of deps in that release practically all of them.... main, fattr, and highline, etc. i have lighter approaches now... tl;dr; bundle a tiny cli lib, do things a bit simpler. kill deps. because, truly, this has been one of my most used projects - i like the pattern - and i want to grow the project to be better yet smaller.

@UnderpantsGnome
Copy link
Contributor Author

I could see something like

# .sekrets.yml (.sekrets.yaml)
---
key: this is a secret
cipher: my super fancy (supported) cipher, or the default if not present

I'm not sure if .key.yml is so generic that it might clash with something else. 🤷‍♂️

For CI I could see something like

SEKRETS_KEY="this is a secret"
SEKRETS_CIPHER="my super fancy (supported) cipher, or the default if not present"

I use 1P for my ssh keys, I haven't gone down the path of trying to use it in CI, too many less painful ways to deal with it. 🙂

@ahoward
Copy link
Owner

ahoward commented May 15, 2024

ok everyone, i'll push the 1.14 now..

sekrets on  master ➜ gem push pkg/sekrets-1.14.0.gem
Pushing gem to https://rubygems.org...
Successfully registered gem: sekrets (1.14.0)

lmk if that's working for you and i'm going to extract some stuff out of this pr for the 2.0 release and will close soon. thanks a ton!

@bak1an
Copy link

bak1an commented May 16, 2024

@ahoward Thanks! You can ping me if you need some review or testing for 2.0 whenever it is around.

@ahoward
Copy link
Owner

ahoward commented May 16, 2024

closing as this code has made it into 1.14 gem

@ahoward ahoward closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants