riboseinc / transcryptor

Assists your everyday re-encryption needs, in Rails.
MIT License
2 stars 1 forks source link

Zero downtime 2.0 #35

Closed nattfodd closed 6 years ago

nattfodd commented 6 years ago

This pull request addresses the following issues: https://github.com/riboseinc/transcryptor/issues/33, https://github.com/riboseinc/transcryptor/issues/24

Idea

To make this happen you need to create a column, that contains current version of the encrypted field. For example, if you have attr_encrypted :ssn, you need to create in your table another column: ssn_version, with some default value - let it be 20180401000000 for example.

Assuming, you want to change encryption now. You create a new initializer file config/initializers/transcryptor.rb. And describe there the history of your changes:

Transcryptor::Migration.draw do
  define_encryption User,
    field: :ssn,
    options: {
      key:       '67c3800d1572d9d964a6ff3bd821ed02',
      algorithm: 'aes-256-gcm'
    },
    version: 20180401000000,
    version_field: :encrypted_ssn_version # optional

  define_encryption User,
    field: :ssn,
    options: {
      key:       '0726c4d149fa59523bc47d592151584b',
      algorithm: 'id-aes192-GCM'
    },
    version: 20180401000001

  define_encryption User,
    field: :ssn,
    options: {
      key:       '94dd7e2c40a3d51a8dd0a9137356a18e',
      algorithm: 'RC2-64-CBC'
    },
    version: 20180401000002
end

Complete example of usage can be found here: https://github.com/nattfodd/transcryptor_sample_app/pull/1/files

Pros:

TODOs:

nattfodd commented 6 years ago

@nattfodd looks great indeed — in fact it might be more useful if we separate the data migration scheme and the attr_encryption “processor”, for example, so when one day we switch to another non-attr_encryption gem, we can still take advantage of the data migration scheme!

Do you think it fits the scope of Transcryptor gem? Maybe it makes sense to extract migration logic into small separate gem and use it in transcryptor?

ronaldtse commented 6 years ago

Yes that’s what I meant too. Feel free to proceed.

ronaldtse commented 6 years ago

@nattfodd I believe these enhancements can be implemented after this PR is merged:

nattfodd commented 6 years ago

I believe the PR is ready to be merged (besides those nasty hound comments, which is difficult to fix).

Having direction from unencrypted to encrypted and vice versa isn't still clear for me, since encrypted field is called encrypted_ssn in the database, and it would be weird to have raw unencrypted value in the column with such name. Same for universal migrations.

Let's discuss it in a different Pull Request when it will be subject to discuss.

ronaldtse commented 6 years ago

@nattfodd sounds good, please feel free to merge it with rebase.

Regarding unencrypted to encrypted, I suspect the following naming pairs make better sense?

Data column Version column
ssn ssn_version
encrypted_ssn encrypted_ssn_version

So the database will:

  1. Start with an :ssn column
  2. Add a database migration to add :encrypted_ssn, :encrypted_ssn_version columns (and other necessary attr_encrypted columns) (and run it)
  3. Add a data migration to migrate :ssn data to :encrypted_ssn

Perhaps something like this is reasonable?

nattfodd commented 6 years ago

@nattfodd sounds good, please feel free to merge it with rebase.

I don't have access to write to the repository :)

ronaldtse commented 6 years ago

Hmm.. I checked settings and it shouldn't be the case though. Anyway I'll merge it here.

ronaldtse commented 6 years ago

Ha, just found out why 😉 Fixed.