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

ActiveRecord Encryption initializes with SHA1 and then new Rails 7 default switches to SHA256 #42922

Closed
boomer196 opened this issue Jul 31, 2021 · 5 comments

Comments

@boomer196
Copy link

ActiveRecord::Railtie initialization fires before ActiveSupport::Railtie

In Rails::Application::Configuration#load_defaults(7.0) it sets active_support.key_generator_hash_digest_class = OpenSSL::Digest::SHA256

Then in the ActiveSupport::Railtie it sets ActiveSupport::KeyGenerator.hash_digest_class = active_support.key_generator_hash_digest_class

However, the ActiveRecord::Railtie configures ActiveRecord::Encryption before this configuration change happens.

What this causes is the ActiveRecord::Encryption::Configurable.configure to set the context.key_provider = ActiveRecord::Encryption::DerivedSecretKeyProvider.new(primary_key), which uses the ActiveSupport::KeyGenerator from ActiveRecord::Encryption::KeyGenerator#derive_key_from method. At this point it uses the classes default hash_digest_class of OpenSSL::Digest::SHA1.

After ActiveSupport::Railtie runs the ActiveSupport::KeyGenerator.hash_digest_class changes to OpenSSL::Digest::SHA256

I am not sure what the best fix is and there may be others, but a couple I can think of are:

  • ActiveRecord::Encryption is "explicit" about how it configures/uses the ActiveSupport::KeyGenerator and possibly adds additional options for hash_digest_class and iterations. This removes the dependency. It also allows the database to live past future changes to ActiveSupport defaults.
  • ActiveSupport::KeyGenerator changes it's class default to be OpenSSL::Digest::SHA256 and the new_framework_defaults_7_0.rb file sets it back to OpenSSL::Digest::SHA1

Steps to reproduce

  1. Create a new rails 7 project using all rails 7 defaults (removed new_framework_defaults_7_0.rb file and application.rb changes to config.load_defaults 7.0)
  2. Configure Encryption by running bin/rails db:encryption:init and adding the generated credentials (bin/rails credentials:edit)
Add this entry to the credentials of the target environment:

active_record_encryption:
  primary_key: nYeEQD0EqOKXeRMFcUq7BX6goxGt0hPq
  deterministic_key: aduCGCnZWFtRIN3xc8CiM15kZKt2Az8T
  key_derivation_salt: fidtmePZxQNmR6Gc4Yv7j1lko06ISaBu
  1. Start a rails console (bin/rails console)
ActiveRecord::Encryption.key_provider.encryption_key.secret
primary_key = "nYeEQD0EqOKXeRMFcUq7BX6goxGt0hPq"
key_derivation_salt = "fidtmePZxQNmR6Gc4Yv7j1lko06ISaBu"
ActiveRecord::Encryption::DerivedSecretKeyProvider.new(primary_key).encryption_key.secret
OpenSSL::PKCS5.pbkdf2_hmac(primary_key, key_derivation_salt, 2**16, 32, OpenSSL::Digest::SHA1.new)
OpenSSL::PKCS5.pbkdf2_hmac(primary_key, key_derivation_salt, 2**16, 32, OpenSSL::Digest::SHA256.new)

Expected behavior

ActiveRecord::Encryption.key_provider.encryption_key.secret
\xE547G\xE7\x99?\x95\xAEX\xE1\xEFS\xE9p\x87}\x93\xF8\x8A\x9Ch\x80\x95#Bee\x95\nD\x7F

Actual behavior

ActiveRecord::Encryption.key_provider.encryption_key.secret
\x8B\x17k\xA1\xC9:a\x05>N\x10\xE4\eYD\xCFUh\x10\x9Ep\x89\x10\x15vt\xE0\x94\xAAN

ActiveRecord::Encryption::DerivedSecretKeyProvider.new(primary_key).encryption_key.secret
\xE547G\xE7\x99?\x95\xAEX\xE1\xEFS\xE9p\x87}\x93\xF8\x8A\x9Ch\x80\x95#Bee\x95\nD\x7F

OpenSSL::PKCS5.pbkdf2_hmac(primary_key, key_derivation_salt, 2**16, 32, OpenSSL::Digest::SHA1.new)
\x8B\x17k\xA1\xC9:a\x05>N\x10\xE4\eYD\xCFUh\x10\x9Ep\x89\x10\x15vt\xE0\x94\xAAN

OpenSSL::PKCS5.pbkdf2_hmac(primary_key, key_derivation_salt, 2**16, 32, OpenSSL::Digest::SHA256.new)
\xE547G\xE7\x99?\x95\xAEX\xE1\xEFS\xE9p\x87}\x93\xF8\x8A\x9Ch\x80\x95#Bee\x95\nD\x7F

System configuration

Rails version: 7.0 (gem 'rails', github: 'rails/rails', branch: 'main')

Ruby version: ruby 2.7.2p137

@ghiculescu
Copy link
Member

@jorgemanrubia do you have any immediate thoughts on a good solution here? I have a feeling the fix here might also solve #42414 which I have been a bit stuck on.

@jorgemanrubia
Copy link
Contributor

jorgemanrubia commented Aug 1, 2021

Fantastic report @boomer196. I'll take a stab at fixing this shortly. My first thoughts would be exposing an option to configure the digest class for ActionRecordEncryption, separated from the active support default (but probably OpenSSL::Digest::SHA256 default too for consistency). We don't really want to have this option changing around with the general framework default.

@ghiculescu I'll have a look at #42414 too. Maybe could you follow up there with the problems you are seeing?

@rmm5t
Copy link
Contributor

rmm5t commented Aug 2, 2021

"My first thoughts would be exposing an option to configure the digest class for ActionRecordEncryption, separated from the active support default (but probably OpenSSL::Digest::SHA256 default too for consistency)."

@jorgemanrubia I agree. This is probably the best future proofing fix. This ensures that long-term encrypted persistence is configurable in case future defaults change.

I think the number of iterations should be configurable too, in case the default of 2**16 is changed or someone changes that config option in ActiveSupport.

@p8 p8 added the activerecord label Aug 3, 2021
@rails-bot
Copy link

rails-bot bot commented Nov 1, 2021

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Nov 1, 2021
@rails-bot rails-bot bot removed the stale label Nov 1, 2021
jorgemanrubia added a commit to basecamp/rails that referenced this issue Nov 3, 2021
Rails 7 uses `SHA256` as the new default digest. With this change,
Active Record Encryption will use `SHA256` too. This can be configured
with:

```ruby
config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA1
```

This sets the 6.0 default as `SHA1`, so that early apps that adopted
encryption before the new default changed continued working without
setting any config.

Fix rails#42922
@jorgemanrubia
Copy link
Contributor

@matthewd I think we can close this: it was fixed in #44540.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants