Closed asummers closed 7 years ago
Here's my current diff to give you an idea of what I'm thinking: https://github.com/danielberkompas/cloak/compare/master...asummers:allow-multiple-concurrent-ciphers
It seems like the encryption_version
column would need to be rethought a bit in this world of having different encryption keys for different fields. Maybe being able to override the field used in the cloak
migration step would achieve it. That way you could express first_name
is version 1, but last_name
could be version 2.
Hello @danielberkompas just curious if you've had an opportunity to look at the above issue and/or the associated PR.
@asummers sorry, I've been swamped with work, so this is the first time I've seen this. Can you give me a tl;dr version of what config.exs
would look like with your changes?
@danielberkompas No worries whatsoever!
It'd look the same as current. So borrowing the one from docs you could say:
config :cloak, Cloak.AES.CTR,
tag: "AES",
default: true,
keys: [
%{tag: <<1>>, key: :base64.decode("..."), default: true}
]
config :cloak, Cloak.AES.CTR,
tag: "AES_second_column",
default: false,
keys: [
%{tag: <<1>>, key: :base64.decode("..."), default: true}
]
config :cloak,My.Special.Cipher
tag: "MyCipher",
default: false,
keys: [
%{tag: <<1>>, key: :base64.decode("..."), default: true},
%{tag: <<2>>, key: :base64.decode("..."), default: false}
]
And if you then said Cloak.encrypt("plaintext")
, it would then go through and use the AES cipher due to default: true
, exactly as it does now. However, then you could then say Cloak.encrypt("plaintext", "AES")
to mimic that, or Cloak.encrypt("plaintext", "AES_second_column")
and Cloak.encrypt("plaintext", "MyCipher")
to have it go through the alternate ciphers, with the same default key mechanics as currently exist. This is all in service of both allowing multiple default keys within the same cipher (say encrypting columns from table A with one key and table B with another) as well as allowing multiple types of ciphers simultaneously with keys (say sha256 hash with key for search field AND AES for the symmetric crypto).
Looking through the code, there seems to be a series of assumptions that the default encryption algorithm will be the only one with a key. There exists things like the
Cloak.Sha256Field
that allow you to put in values without keys, but the way that theencrypt
function works, it looks like an additional key will be hard to get at currently. The tests seem to support this state of affairs, withCloakTest
having empty tests for encrypting with configs not having the default tag.Proposals: In
cloak/config.ex
: Add functioncipher/1
that takes in a tag and returns the cipher fromCloak.Config.all/0
that has the tag passed into the arguments.In
cloak.ex
: Add functionsversion/1
,cipher/1
,encrypt/2
that each take in a tag as the lattermost argument. These would defer to the aforementioned function inCloak.Config
and do the same things as the default versions do.Add tests/docs for the new behavior.
Another thing I'm noticing is that many functions do not have typespecs. I was planning on adding those as I dig through the code.
Depending on the extent of the necessary changes I might pull central, private functions out to util files. (See e.g.
Cloak.AES.CTR.decode_key/2
and really most non callback functions in there).Additionally, and I'm not sure if this is possible already, but we should also be able to encrypt with a specific key within a cipher. For example, I might have a key for a User's Email column and want a separate key for my User's First Name column. This might be already possible, but I haven't seen a path towards that. I plan on adding that if it's not already possible. My recollection of reading
encrypt/1
incloak.ex
was that it fell back to default.--
Wanted to get your thoughts before I go down the rabbit hole. Additionally, how would you like me to divide the work up in the PR process? The extent of changes should be mostly additive, and the scope shouldn't be large in line magnitude, outside new docs and such.