ankane/blind_index

Add info about keys and env variables

david-a-wheeler opened this issue · 14 comments

The current documentation shows a less than great practice involving keys. Storing keys in environment variables is fine, but loading them directly as the key is a problem. Environment variables cannot have byte zero. I suspect many people are using hexadecimal numbers as their key, but in fact each digit only has 4 bits of entropy. In that case, you really have a hundred and twenty eight bit key instead of a 256-bit key. I suggest the documentation show how to use pack and such so that you can get the best possible keys.

I intend to submit this as a pull request. But if it's a terrible idea, please let me know. I also intend to submit the same comment to sister gems which have the same issue.

Was thinking about this the other day. Love this idea.

rbnacl raises an error when strings aren't encoded in binary. This is one idea to strictly enforce it. https://github.com/crypto-rb/rbnacl/blob/e4d5811b5b75122fade3177a1196f696830309f7/lib/rbnacl/util.rb#L169

That's a clever idea. I prefer APIs that make it easy to do the right thing, and also easy to detect the wrong thing.

That said, showing in the documentation what you should do is still a good idea. Something like:

[ENV('EMAIL_BLIND_INDEX_KEY')].pack('H*')

Makes sense. The gem can do it automatically if it detects a 64 length hex string, but users would manually have to do it for attr_encrypted. I feel like pack code can be intimidating, so thinking about the best way to abstract that.

Where does hex_to_bytes come from? I do not have it. It is clear, but perhaps unavailable. If that's not a standard part of Ruby, it might be better to simply use pack. I don't see why pack is scary, it's a standard part of Ruby.

An alternative: a hex_to_bytes gem. Perhaps the world's tiniest gem? :-)

It appears there is already such a gem, hex_string. I haven't used it or reviewed it yet, but looks promising: https://www.rubydoc.info/gems/hex_string/frames
Since keys must be handled in other cases, it'd be better to have a tiny gem for the purpose that can be recommended.

Blind index will provide the method as a core extension:

# not ideal, but ActiveSupport has many

Also, blind index can handle the conversion internally if it detects a non-binary string that consists of all hex characters, so we really only need a solution for attr_encrypted. I'll file an issue detailing the approach rbnacl takes and what we're thinking and get their feedback.

Okay. I think method for handling Keys should be the same between this Gem and attr_encrypted. I think that if you're going to add a method to string, it should be in a completely separate gem so that people can use it regardless of whether or not they use blind_index. I don't even mind if you automatically include that other gem when you include this one, but it doesn't seem right to add a method to string as a built in.

Went with pack recommendation for now just to get this out. May iterate on it in the future. Thanks for driving this.

I think that is a good choice. You can always do more later. Thank you again for this gem, it is really a thyme savor :-).