ahoward / sekrets

sekrets is a command line tool and library used to securely manage encrypted files and settings in your rails' applications and git repositories.
BSD 2-Clause "Simplified" License
268 stars 28 forks source link

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

Closed UnderpantsGnome closed 6 months ago

arianf commented 1 year ago

@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.

UnderpantsGnome commented 6 months ago

@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 commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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 https://github.com/ahoward/sekrets/issues/6

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

pyrareae commented 6 months ago

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 commented 6 months ago

@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 :+1:

copiousfreetime commented 6 months ago

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 commented 6 months ago

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

Thanks Jeremey! Long time no talk 🙂

ahoward commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

ahoward commented 6 months ago

@UnderpantsGnome i am peeking at

now and, yes ;-)

copiousfreetime commented 6 months ago
bak1an commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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 commented 6 months ago

Looks good @ahoward !

Thanks!

UnderpantsGnome commented 6 months ago

@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 commented 6 months ago

@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 commented 6 months ago

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

ahoward commented 6 months ago

@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:

UnderpantsGnome commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

ahoward commented 6 months ago

closing as this code has made it into 1.14 gem