Rotating master_key as default

[danleyden]

Hi

I'm trying to figure out master key rotation.

I'd prefer not to have to alter all my models each time I need to rotate the key, or have the same master key settings everywhere I need an encrypted field.

I've seen that altering the model to have encrypts :my_field, previous_versions: [{master_key: old_master_key}] works, but with dozens of models, with encrypted fields spread throughout, I'd much prefer to have

Lockbox.default_options[:previous_versions] = [{ master_key: old_master_key }]

set in one place for my application.

#38 suggests that this should be possible, but on trying it (version 0.6.2) I'm getting ArgumentError (unknown keyword: :master_key) when trying to access the field with the below trace:

ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/box.rb:3:in `initialize'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/encryptor.rb:12:in `new'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/encryptor.rb:12:in `block in initialize'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/encryptor.rb:12:in `map'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/encryptor.rb:12:in `initialize'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox.rb:104:in `new'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox.rb:104:in `new'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/utils.rb:50:in `build_box'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/model.rb:461:in `block (3 levels) in encrypts'
ruby-2.7.2/gems/lockbox-0.6.2/lib/lockbox/model.rb:374:in `block (3 levels) in encrypts'

A test case with ActiveRecord would look something like:

original_key = Lockbox.generate_key
Lockbox.master_key = original_key

class MyModel < ActiveRecord::Base
  encrypts :my_field
end

instance = MyModel.create(my_field: 'plaintext')

Lockbox.master_key = Lockbox.generate_key
Lockbox.default_options[:previous_versions] = [{ master_key: original_key }]

assert_equal 'plaintext', MyModel.find(instance.id).my_field # raises ArgumentError

[ankane]

Hey @danleyden, agree it should be easier to rotate the master key. Let me think on the best way to do that.

For now, you could add a method to ApplicationRecord:

class ApplicationRecord < ActiveRecord::Base
  def self.lockbox_options
    {previous_versions: [{master_key: old_master_key}]}
  end
end

And pass it to every encrypts method.

class User < ApplicationRecord
  encrypts :email, **lockbox_options
end

[danleyden]

Thanks, I'll give that a try. Appreciate any further thoughts or improvements ;)

[swiknaba]

Using

• lockbox 0.6.8
• rails 6.0.3.5
• ruby 2.7.5

I, too, get

.rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/lockbox-0.6.8/lib/lockbox/box.rb:3:

in `initialize':

unknown keyword: :master_key (ArgumentError)

as an error, when I try to set previous_versions. I've tried this approach:

class ApplicationRecord < ActiveRecord::Base
  def self.lockbox_options
    {previous_versions: [{master_key: ENV['LOCKBOX_PREVIOUS_MASTER_KEY']}]}
  end
end

class MyModel < ApplicationRecord
  encrypts :my_secret, **lockbox_options
end

and just for sanity also the more explicit syntax

class MyModel < ApplicationRecord
  encrypts :my_secret, previous_versions: [{master_key: ENV['LOCKBOX_PREVIOUS_MASTER_KEY']}]
end

I also tested setting the previous key in the initializer

# config/initializers/lockbox.rb

Lockbox.master_key = Rails.application.credentials.lockbox[:master_key]
Lockbox.default_options[:previous_versions] = [{ master_key: ENV['LOCKBOX_PREVIOUS_MASTER_KEY'] }]

but that approach failed with the same error.

💡 from further digging, this appears to happen, if the previous key (in my case ENV['LOCKBOX_PREVIOUS_MASTER_KEY']) is blank. This seems non-obvious to me at first sight. I was expecting that I can always keep a placeholder env var for the previous key which would allow me to rotate the key by updating ENV vars rather than deploying the app. The ArgumentError is at least irritating here.

The culprit is around here. https://github.com/ankane/lockbox/blob/75dae4f797136237a76a8bdf3333fbe3e80e4cd5/lib/lockbox/utils.rb#L34

# utils.rb

      if options[:previous_versions].is_a?(Array)
        # dup previous versions array (with map) since elements are updated
        # dup each version (with dup) since keys are sometimes deleted
        options[:previous_versions] = options[:previous_versions].map(&:dup)
        options[:previous_versions].each_with_index do |version, i|
          if !(version[:key] || version[:encryption_key] || version[:decryption_key]) && (version[:master_key] || version[:key_table] || version[:key_attribute])
            # could also use key_table and key_attribute from options
            # when specified, but keep simple for now
            # also, this change isn't backward compatible

You can try

options = {previous_versions: [{ master_key: nil }]}
version = options[:previous_versions].first

!(version[:key] || version[:encryption_key] || version[:decryption_key]) && (version[:master_key] || version[:key_table] || version[:key_attribute])
=> nil

so we see the condition here is never met and we end in a state that is not handled. Probably having a else case here to raise/log a meaningful error would be nice (if it makes sense in the context of this method, I'm not deep into the codebase here) 🙂

checked out the code, set the previous master key to nil in the active record spec, blows up the tests (I've verified, the tests are all passing on current master branch without that change)

[capps]

This has worked for quite a while -- feels cleaner than controller solution encrypts :account1,:account2, :account3, Rails.application.config.lockbox_params

[porterbayne]

Would a conditional help minimize the code change efforts during rotation?

Assume a lockbox[:previous_key] stored in Rails 6+ credentials:

class ApplicationRecord < ActiveRecord::Base
  def self.lockbox_options
    if Rails.application.credentials.lockbox[:previous_key].present?
      {previous_versions: [{master_key: old_master_key}]}
    else
      {}
    end
  end
end

Then rotation might just be:

• update keys in credits
• run rotation
• remove previous_key

Haven't tested this, I'm coding in my head while reading through all of this. 😄

[ankane]

With the commit above, you can now use:

Lockbox.default_options[:previous_versions] = [{master_key: previous_key}]

Sorry this took so long to resolve.