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

Lockbox::DecryptionError even when the old key is provided in previous_versions #35

Closed
eddierubeiz opened this issue Feb 12, 2020 · 6 comments

Comments

@eddierubeiz
Copy link

@eddierubeiz eddierubeiz commented Feb 12, 2020

I install lockbox-0.3.1
I set up a model with encrypts :patron_email
I run the migration add_column :r_and_r_items, :patron_name_ciphertext, :text
I set my master key in dev:

lockbox_master_key: '0000000000000000000000000000000000000000000000000000000000000000'

I persist an item with this key.
It's persisted correctly -- the value in the database is encrypted, and the value as shown is correct.

Let's suppose my key is compromised.
I now change the lockbox_master_key used to

lockbox_master_key: '000000000000000000000000000000000000000000000000000000000000aaaa'

As expected, I get a Lockbox::DecryptionError: Decryption failed if I try to decrypt the encoded patron_email with the new key.

I now change the model to:

encrypts :patron_email, previous_versions: [{key: "0000000000000000000000000000000000000000000000000000000000000000"}]

Expected behavior:

Lockbox attempts to decypher the encrypted patron_email, fails, then tries with the key listed in previous_versions, succeeds, and returns that.

Actual behavior:

Lockbox::DecryptionError: Decryption failed is thrown.

@eddierubeiz eddierubeiz changed the title Lockbox::DecryptionError even when the correct value is provided in previous_versions Lockbox::DecryptionError even when the old key is provided in previous_versions Feb 12, 2020
@ankane

This comment has been minimized.

Copy link
Owner

@ankane ankane commented Feb 12, 2020

Hey @eddierubeiz, key is different from master_key. You can pass the master_key option to previous_versions, or get the actual key with the instructions here: https://github.com/ankane/lockbox#key-separation.

@eddierubeiz

This comment has been minimized.

Copy link
Author

@eddierubeiz eddierubeiz commented Feb 13, 2020

Thanks a lot for your prompt reply! I'll figure it out. In the meantime, it does look like I'm passing master_key to previous_versions wrong:

encrypts : patron_email,  previous_versions: [{master_key: "0000000000000000000000000000000000000000000000000000000000000000"}]

gives me ArgumentError (unknown keyword: master_key) when I try to access the value.

@eddierubeiz

This comment has been minimized.

Copy link
Author

@eddierubeiz eddierubeiz commented Feb 13, 2020

I was able to rotate the key using the clue @ankane provided. Thanks! I'm attaching the recipe that worked for me, in case anyone has this problem later on:

Your master_key has been leaked. Do this:

  1. Retrieve the current column key for each encrypted column as follows:
Lockbox.attribute_key(table: "r_and_r_items", attribute: "patron_name_ciphertext")
Lockbox.attribute_key(table: "r_and_r_items", attribute: "patron_email_ciphertext")
  1. Change the value of master_key to your new, safe master_key.

  2. Change the model so it can use the old column keys:

encrypts :patron_name,  previous_versions: [{key: old_column_key_for_patron_name} ]
encrypts :patron_email, previous_versions: [{key: old_column_key_for_patron_email}]
  1. Re-encrypt the values using the new key:
Admin::RAndRItem.unscoped.find_each { |x| x.update!(patron_email: x.patron_email) }
Admin::RAndRItem.unscoped.find_each { |x| x.update!(patron_name: x.patron_name) }
  1. Switch your model back to:
encrypts :patron_name
encrypts :patron_email
  1. Immolate the old keys.
@ankane

This comment has been minimized.

Copy link
Owner

@ankane ankane commented Feb 13, 2020

Hey @eddierubeiz, big thanks for sharing!

It looks like master_key was only an option for blind index rotation rather than encrypted fields (my bad). I've added support for this on master. 1797767

Your instructions are great! Have been meaning to add better docs for this in the readme. Will do this later today.

With the new option, users should be able to do:

encrypts :patron_name, :patron_email, previous_versions: [{master_key: "..."}]

You can speed up step 4 by updating both at once (saves a DB query).

Admin::RAndRItem.unscoped.find_each { |x| x.update!(patron_name: x.patron_name, patron_email: x.patron_email) }

There's a bit more that can be optimized by batching DB transactions (I've been meaning to do this for Lockbox.migrate), so I'll probably create a method like:

Lockbox.rotate(Admin::RAndRItem, attributes: [:patron_name, :patron_email])

that handles this automatically.

ankane added a commit that referenced this issue Feb 13, 2020
@eddierubeiz

This comment has been minimized.

Copy link
Author

@eddierubeiz eddierubeiz commented Feb 13, 2020

Yeah -- that will be a very useful feature. Thanks for your help; we appreciate it.

ankane added a commit that referenced this issue Feb 14, 2020
@ankane

This comment has been minimized.

Copy link
Owner

@ankane ankane commented Feb 14, 2020

On master, you can now do:

Lockbox.rotate(Admin::RAndRItem, attributes: [:patron_name, :patron_email])

which includes the optimization mentioned above. Will add it to the readme once the next version goes out.

@ankane ankane closed this Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.