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

[matthewd]

Instead of pre-validating the configuration, we now check for the required values when they're used.

This is a light variation on #42958 -- I was nervous about exposing both a nil-returning accessor and a raise-on-nil credential method, so I've moved the raise onto the accessors themselves.

Fixes #42385
Closes #42958

[georgeclaghorn]

FYI, this change broke decryption in an existing app of mine. The cause appears to be that key derivation via ActiveSupport::KeyGenerator now happens after ActiveSupport::KeyGenerator.hash_digest_class gets set to OpenSSL::Digest::SHA256 rather than before.

Not sure what, if anything, should be done about this, but wanted to raise it now anyway. It seems like a possible bug that we were deriving keys before ActiveSupport.hash_digest_class got configured, but one that would obviously be difficult to fix without breaking existing apps.

It’s also possible that this is my fault for causing AR to load too early, but I haven’t found where I might be doing that yet. I’m trying to reproduce in a minimal sample app and haven’t managed to do that yet, either.

[georgeclaghorn]

I was able to reproduce this in a new app.

I generated a new app and pinned it to b6d4269 (d49f956’s parent). I set up development encryption credentials via bin/rails db:encryption:init, generated a Post model with a string title attribute, and added encrypts :title to the Post model. I opened a console and created a post with a title.

I upgraded the app to latest Rails main. I opened the console and ran Post.last.title. The result was a decryption error:

Loading development environment (Rails 7.1.0.alpha)
irb(main):001:0> Post.first.title
   (1.1ms)  SELECT sqlite_version(*)
  Post Load (0.3ms)  SELECT "posts".* FROM "posts" ORDER BY "posts"."id" ASC LIMIT ?  [["LIMIT", 1]]
/Users/george/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/bundler/gems/rails-2c21a39493b3/activerecord/lib/active_record/encryption/encryptor.rb:58:in `rescue in decrypt': ActiveRecord::Encryption::Errors::Decryption (ActiveRecord::Encryption::Errors::Decryption)
/Users/george/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/bundler/gems/rails-2c21a39493b3/activerecord/lib/active_record/encryption/cipher/aes256_gcm.rb:79:in `rescue in decrypt': ActiveRecord::Encryption::Errors::Decryption (ActiveRecord::Encryption::Errors::Decryption)
/Users/george/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/bundler/gems/rails-2c21a39493b3/activerecord/lib/active_record/encryption/cipher/aes256_gcm.rb:75:in `final': OpenSSL::Cipher::CipherError

If I run byebug bin/rails c on the old and new revisions and set a breakpoint at ActiveRecord::Encryption::KeyGenerator#derive_key_from, the difference is:

• On the old revision, #derive_key_from is called during boot when ActiveSupport::KeyGenerator.hash_digest_class is OpenSSL::Digest::SHA1.
• On the new revision, #derive_key_from isn’t called until Post.last.title is run, when ActiveSupport::KeyGenerator.hash_digest_class is OpenSSL::Digest::SHA256.

[EmCousin]

Adding my 2 cents here. This change broke decryption in an existing Rails app of mine as well, although deterministic encryption does not seem to be affected on my end.

Basically every content that was "non-deterministically" encrypted (especially encrypted rich text via ActionText) in my app before this change is no longer decrypted ; while all new content is encrypted and decrypted properly.

Symmetrically, reverting this change restores decryption of encrypted content that was posted before, but breaks decryption of encrypted content that was posted afterwards.

[matthewd]

I just realised I've had this open and unreplied for several weeks, sorry @georgeclaghorn and @EmCousin. 😞

Thanks for the detailed report -- I agree that it seems like this change has accidentally fixed a bug, but in doing so it's introduced an incompatibility.

@jorgemanrubia do you have thoughts on this? On one hand, it seems we could introduce compatibility to decrypt using SHA1-derived keys as well as hash_digest_class. On the other, that still leaves inaccessible data if someone upgrades and then reverts, say. (Do we have an existing mechanism to support an application changing hash_digest_class over time, for which this would just be a special case?)

[jorgemanrubia]

@matthewd key providers can serve multiple keys via #decryption_keys and they will be used in sequence to decrypt (so if one key fails, it will try the next one), while it will always the last one to encrypt. We can leverage this in DerivedSecretKeyProvider to inject an additional password using SHA1 as a digest algorithm.

But I am not sure we need to support both hashes at the same time. Back in the day I created this PR #42929 to make the encryption digest algorithm configurable. That would offer a path for apps to control the used algorithm and would help with this problem, as long as apps don't have encrypted data with both digest algorithms in production of course. Until this patch, it was effectively using SHA1, so we can default to that until the next Rails release.

I can prepare a PR before the week ends. I'm tentatively going with the configurable digest key path, please let me know if you think we need to support both digest algorithms at the same time (I'm not sure about Rails policy to deal with breaking changes in main between official releases).

Great work on the patch that exposed this bug @matthewd and great report @georgeclaghorn.

[jorgemanrubia]

I didn't get to this last week. Aiming to get that PR ready this one.

[jorgemanrubia]

PR to configure the digest algorithm for Active Record Encryption.