neilmiddleton / attr_secure

MIT License
11 stars 1 forks source link

Support key rotation #10

Closed msakrejda closed 10 years ago

msakrejda commented 10 years ago

Have you considered how key rotation fits into this? Keys may need to be rotated (e.g., due to a credential leak), and without some support from the plugin, managing this would be a nightmare (especially if someone is using the custom secret with a lambda).

Any thoughts on this? I think the simplest would be to support multiple keys (in some well-defined order) and prefer encrypting with the "newest" one (and decrypting with each in sequence). I suppose one could do this in the lambda directly, but it may be useful to have a better mechanism in place to try to idiot-proof this.

Also /cc @tmaher this exists and seems like exactly what we talked about!

neilmiddleton commented 10 years ago

Honestly, it's not something I've really thought about, but seems obvious now you mention it. I'm not going to get a chance to look at this any time soon, so I might need to lean on someone else to create a PR for it.

tmaher commented 10 years ago

+1 to key rotation. It's one of those things where when you need it, it saves a tremendous amount of dev and ops work.

neilmiddleton commented 10 years ago

So I'm understanding this properly (and before screwing up the code) am I right in thinking that you're talking about being able to store a bunch of keys via an Array, and Fernet's #valid? will tell you if you're using the right key to decrypt (and that new encryptions should be done with the newest key)

tmaher commented 10 years ago

I believe you'll have to loop over the array of keys yourself, but yes.

msakrejda commented 10 years ago

You know, I was thinking about this, and I believe the Go version of Fernet already supports multiple keys precisely for the rotation use case. Basically, you always encrypt with one key, but you decrypt with a list of keys and the right thing automagically happens.

All in all, getting that working in the Ruby version of Fernet may be a better approach than trying to manage this in attr_secure (since key rotation is useful in more contexts than just this). I think getting Fernet.verifier to optionally accept the "token" argument as a list should do what we want...

neilmiddleton commented 10 years ago

Agreed, however, given that this doesn't yet support Ruby Fernet 2.0 we're limited. I don't know what the roadmap plans are for Ruby Fernet, but this PR should add key rotation support for apps already using Fernet 1.6

msakrejda commented 10 years ago

Yeah, good point. Code looks good except for a small think-o. Unfortunately, Shogun is basically on Fernet master, so we may need 2.0 support. I'll see what that would take (using your current approach, without mucking about with multiple keys in Fernet). Thanks for taking a stab at this.

neilmiddleton commented 10 years ago

Prelim method (above) supported in 0.3.0. +1 for making this part of Fernet somehow