Add option to configure digest algorithm in Active Record Encryption #44873

jorgemanrubia · 2022-04-11T15:35:32Z

This adds a new option to configure the digest algorithm in Active Record Encryption. It sets SHA-256 as the new default starting in 7.1 and SHA-1 for previous versions.

This is a second take on this problem (see #42929). I forgot about the first PR and it went unmerged 🙄.

Rails 7 uses SHA256 as the new default digest. However, due to this bug , Active Record Encryption kept using SHA-1 until it was unintentionally fixed by this change. Because that fix is recent, I think setting SHA-1 as the pre-7.1 default makes sense.

This shows why a mechanism to fix the digest algorithm in AR encryption is positive. Changing the digest prevents existing encrypted contents from decrypting, so we don't want the digest to change inadvertently when picking a new one in Rails.

To configure the new option:

config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256

Hat tip to @boomer196 for the original report, to @georgeclaghorn for the great troubleshooting here, and to @matthewd for the change that exposed this bug.

cc @matthewd

guides/source/configuring.md

matthewd · 2022-04-13T16:53:14Z

Thanks!

How do we feel about leaving apps that started in an older version on SHA1 for the long term? Is our recommendation that for this use case it's really fine either way, despite their differing security profiles... or is this a situation where using SHA1 really is less-advisable, and we're just warning that it's hard to change once you have data stored?

Regardless of how strongly we encourage it, if people do want to switch, is there a story for how they can go about it? How much of A Thing would it be for us to provide read-old-write-new compatibility, as [I believe?] we do for key transitions?

jorgemanrubia · 2022-04-14T14:52:42Z

How do we feel about leaving apps that started in an older version on SHA1 for the long term? Is our recommendation that for this use case it's really fine either way, despite their differing security profiles... or is this a situation where using SHA1 really is less-advisable, and we're just warning that it's hard to change once you have data stored?

I don't think using SHA1 represents any security flaw here. Active Record encryption uses the digest algorithm to derive keys with the size expected by the cipher (AES256-GCM) from the configured secrets. The generated hashes are calculated on the fly and aren't exposed anywhere, so an attacker can't really exploit collision-related flaws in the algorithm to guess keys. Of course, as long as the original secrets remain private.

Regardless of how strongly we encourage it, if people do want to switch, is there a story for how they can go about it? How much of A Thing would it be for us to provide read-old-write-new compatibility, as [I believe?] we do for key transitions?

There is a path, but right now it's not directly supported. The system is prepared to work with multiple decryption keys, so it would be a matter of injecting keys with multiple hash digest algorithms (in ActiveRecord::Encryption::DerivedSecretKeyProvider). Possibly as an option, as we don't want to add more keys than necessary in general. I can explore this in a separated PR, but I don't think it's a pressing issue right now.

ghiculescu · 2022-12-05T17:44:33Z

...ib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt

+# Active Record Encryption now uses SHA-256 as its hash digest algorithm. Important: If you have data encrypted
+# with previous versions, you should not set the new default or the existing data will fail to decrypt.
+# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256


I don't think this is sufficient. If you have data encrypted with previous versions, then you should set the old version as a config. That way if you later do load_defaults 7.1 it won't change this under the hood.

Suggested change

# Active Record Encryption now uses SHA-256 as its hash digest algorithm. Important: If you have data encrypted

# with previous versions, you should not set the new default or the existing data will fail to decrypt.

# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256

# Active Record Encryption hash digest algorithm: 2 options

#

# The new default hash digest algorithm is SHA-256. If you haven't used Active Record Encryption

# yet, it is safe to upgrade.

#

# **Important**: If you have data encrypted already, it is likely encrypted using SHA-1. You should keep the old default or the existing data will fail to decrypt.

#

# New default:

# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256

#

# Old default:

# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA1

Good point @ghiculescu. I'll review this and rebase the branch shortly.

@ghiculescu I have reviewed the template for the new 7.1 defaults explaining things better. If someone decides to skip the "upgrade" path and just loads the new 7.1 defaults, decryption will fail, but that's what happens with other similar changes (e.g: with cookie serialization back in the day), so I think that's OK. Do you have a different suggestion here?

I think your changes look good 👍

ghiculescu · 2022-12-05T17:46:24Z

@matthewd @jorgemanrubia can we add this to the 7.1 milestone? Unless I'm misreading, #44540 introduced a fairly bad bug into Rails main which this fixes, but we shouldn't launch 7.1 with it un-fixed.

activerecord/test/cases/encryption/key_generator_test.rb

jorgemanrubia · 2022-12-05T19:57:28Z

...ib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt

+# Active Record Encryption now uses SHA-256 as its hash digest algorithm. Important: If you have data encrypted
+# with previous versions, you should not set the new default or the existing data will fail to decrypt.
+# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256


Good point @ghiculescu. I'll review this and rebase the branch shortly.

ghiculescu · 2022-12-05T21:56:00Z

activerecord/test/cases/encryption/key_generator_test.rb

+      ActiveRecord::Encryption.config.hash_digest_class = other_digest_class
+      derived_with_other_digest_class = @generator.derive_key_from("some password", length: 12)
+
+      assert_not_equal derived_with_digest_class, derived_with_other_digest_class


(I'm not an encryption expert at all...) beyond asserting a difference, is there a way of asserting that a specific string was encrypted with a specific digest?

👍 I changed the test to actually check that it's deriving the expected key.

This commit changes the active_record_encryption.configuration to be set on an after initialize block. After PR rails#44873, we can configure now hash_digest_class that will be used on ActiveRecord::Encryption. But the docs said that we could change it via an initializer by setting Rails.application.config.active_record.encryption.hash_digest_class. This wasn't working because the railtie "active_record_encryption.configuration" was being set before the initializer was loaded. This changes this behave to configure the active_record encryption after the initialize was loaded.

…ally with a hash digest of SHA1 There is currently a problem with Active Record encryption for users updating from 7.0 to 7.1 Before rails#44873, data encrypted with non-deterministic encryption was always using SHA-1. The reason is that `ActiveSupport::KeyGenerator.hash_digest_class` is set in an after_initialize block in the railtie config, but encryption config was running before that, so it was effectively using the previous default SHA1. That means that existing users are using SHA256 for non deterministic encryption, and SHA1 for deterministic encryption. This adds a new option `use_sha1_digest_for_non_deterministic_data` that users can enable to support for SHA1 and SHA256 when decrypting existing data.

…inistically with a SHA1 hash digest (#48530) * Make sure active record encryption configuration happens after initializers have run Co-authored-by: Cadu Ribeiro <mail@cadu.dev> * Add a new option to support previous data encrypted non-deterministically with a hash digest of SHA1 There is currently a problem with Active Record encryption for users updating from 7.0 to 7.1 Before #44873, data encrypted with non-deterministic encryption was always using SHA-1. The reason is that `ActiveSupport::KeyGenerator.hash_digest_class` is set in an after_initialize block in the railtie config, but encryption config was running before that, so it was effectively using the previous default SHA1. That means that existing users are using SHA256 for non deterministic encryption, and SHA1 for deterministic encryption. This adds a new option `use_sha1_digest_for_non_deterministic_data` that users can enable to support for SHA1 and SHA256 when decrypting existing data. * Set a default value of true for `support_sha1_for_non_deterministic_encryption` and proper initializer values. We want to enable the flag existing versions (< 7.1), and we want it to be false moving by default moving forward. * Make sure the system to auto-filter params supports different initialization orders This reworks the system to auto-filter params so that it works when encrypted attributes are declared before the encryption configuration logic runs. Co-authored-by: Cadu Ribeiro <mail@cadu.dev> --------- Co-authored-by: Cadu Ribeiro <mail@cadu.dev>

ClaytonPassmore · 2023-12-18T19:40:27Z

We introduced some deterministic encrypted columns while using Rails 7.0. These were encrypted with SHA256. It must have been the default.

When we upgraded to Rails 7.1 while still using the 7.0 framework defaults, the encryption algorithm changed to SHA1. This resulted in decryption errors and could have resulted in mixed values had new records been inserted. Fortunately we realized what was going on before that could happen.

It's possible that I missed something during the upgrade but it seems like upgrading changed the default algorithm even though we were still using the 7.0 defaults. Apologies if this isn't the right spot for this, just thought I would post it here for visibility.

Edit: Looks like this might be tracked in #48204

rails-bot bot added activerecord docs railties labels Apr 11, 2022

jorgemanrubia mentioned this pull request Apr 11, 2022

Don't require encryption credentials when using a custom key provider #44540

Merged

jorgemanrubia force-pushed the configure-digest-class-in-are branch 5 times, most recently from 635b8f4 to b3150e0 Compare April 11, 2022 15:53

skipkayhil reviewed Apr 11, 2022

View reviewed changes

guides/source/configuring.md Outdated Show resolved Hide resolved

jorgemanrubia force-pushed the configure-digest-class-in-are branch 3 times, most recently from 5622b0c to cc5b2b8 Compare April 13, 2022 16:34

georgeclaghorn mentioned this pull request Sep 28, 2022

Encrypted Rails fields failing to decrypt after upgrading to latest ref #46150

Closed

ghiculescu reviewed Dec 5, 2022

View reviewed changes

ghiculescu mentioned this pull request Dec 5, 2022

ActiveRecord eagerly validates encryption configuration unnecessarily #46647

Closed

jorgemanrubia commented Dec 5, 2022

View reviewed changes

jorgemanrubia force-pushed the configure-digest-class-in-are branch 4 times, most recently from c453211 to 1034c6c Compare December 5, 2022 21:53

ghiculescu reviewed Dec 5, 2022

View reviewed changes

jorgemanrubia force-pushed the configure-digest-class-in-are branch 5 times, most recently from fa385e1 to df4f651 Compare December 5, 2022 22:16

dhh merged commit 5d7b6d8 into rails:main Feb 27, 2023

dhh deleted the configure-digest-class-in-are branch February 27, 2023 09:16

duduribeiro mentioned this pull request Jun 16, 2023

Change new_framework_defaults_7_1.rb to use the right config for hash_digest_class #48508

Closed

4 tasks

duduribeiro mentioned this pull request Jun 19, 2023

Wrap active_record_encryption.configuration on an after initialize #48520

Closed

4 tasks

jorgemanrubia mentioned this pull request Jun 19, 2023

Upgrade to Rails 7.1 alpha breaks ActiveRecord attribute encryption (deterministic) #48204

Closed

jorgemanrubia mentioned this pull request Jun 20, 2023

Add a encryption option to support previous data encrypted non-deterministically with a SHA1 hash digest #48530

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to configure digest algorithm in Active Record Encryption #44873

Add option to configure digest algorithm in Active Record Encryption #44873

jorgemanrubia commented Apr 11, 2022 •

edited

matthewd commented Apr 13, 2022

jorgemanrubia commented Apr 14, 2022

ghiculescu Dec 5, 2022

jorgemanrubia Dec 5, 2022

jorgemanrubia Dec 5, 2022

ghiculescu Dec 5, 2022

ghiculescu commented Dec 5, 2022 •

edited

jorgemanrubia Dec 5, 2022

ghiculescu Dec 5, 2022

jorgemanrubia Dec 5, 2022

ClaytonPassmore commented Dec 18, 2023 •

edited

-# Active Record Encryption now uses SHA-256 as its hash digest algorithm. Important: If you have data encrypted
-# with previous versions, you should not set the new default or the existing data will fail to decrypt.
-# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256
+# Active Record Encryption hash digest algorithm: 2 options
+#
+# The new default hash digest algorithm is SHA-256. If you haven't used Active Record Encryption
+# yet, it is safe to upgrade.
+#
+# **Important**: If you have data encrypted already, it is likely encrypted using SHA-1. You should keep the old default or the existing data will fail to decrypt.
+#
+# New default:
+# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA256
+#
+# Old default:
+# Rails.application.config.active_record.encryption.hash_digest_class = OpenSSL::Digest::SHA1

Add option to configure digest algorithm in Active Record Encryption #44873

Add option to configure digest algorithm in Active Record Encryption #44873

Conversation

jorgemanrubia commented Apr 11, 2022 • edited

matthewd commented Apr 13, 2022

jorgemanrubia commented Apr 14, 2022

ghiculescu Dec 5, 2022

Choose a reason for hiding this comment

jorgemanrubia Dec 5, 2022

Choose a reason for hiding this comment

jorgemanrubia Dec 5, 2022

Choose a reason for hiding this comment

ghiculescu Dec 5, 2022

Choose a reason for hiding this comment

ghiculescu commented Dec 5, 2022 • edited

jorgemanrubia Dec 5, 2022

Choose a reason for hiding this comment

ghiculescu Dec 5, 2022

Choose a reason for hiding this comment

jorgemanrubia Dec 5, 2022

Choose a reason for hiding this comment

ClaytonPassmore commented Dec 18, 2023 • edited

jorgemanrubia commented Apr 11, 2022 •

edited

ghiculescu commented Dec 5, 2022 •

edited

ClaytonPassmore commented Dec 18, 2023 •

edited