Upload / Up1

Client-side encrypted image host web server
MIT License
813 stars 97 forks source link

Key/IV Generation #34

Closed securitygeneration closed 8 years ago

securitygeneration commented 8 years ago

The AES key and IV generation process sounds a bit dubious. I'm no crypto expert, but I think the practice of using a single value (especially one shorter than the length of your output) and passing that through a hash function in order to use subsets of that output as the key and IV is questionable.

You may want to consider using a key derivation function (like PBKDF2) instead of a hash function.

k3d3 commented 8 years ago

but I think the practice of using a single value (especially one shorter than the length of your output) and passing that through a hash function in order to use subsets of that output as the key and IV is questionable.

Why would that be questionable? The randomly generated value has 128 bits of entropy, and the only way to get any specific (key, IV, ident) tuple is to either brute-force the input (128 bits) or to brute-force the output (512 bits). Also, when splitting a value in half, the total entropy doesn't necessarily split in half (in this case, the 512 bits have 128 bits of entropy, and each piece has 128 bits of entropy on its own (depending on the strength of the hash function, of course), or 128 bits of entropy altogether). Hopefully that gives a better understanding as to why we generate the key and IV the way we do.

You may want to consider using a key derivation function (like PBKDF2) instead of a hash function.

Something like PBKDF2 is only really useful when the input could be considered weak (such as a password) and you want to slow down the speed of dictionary or brute-force attacks. We're using a 128-bit randomly generated value, however, which is very strong.

ultramancool commented 8 years ago

@k3d3 There are actually some functions such as HKDF designed to do this kind of expansion, but in this use case I don't think they would provide any benefit.

http://webee.technion.ac.il/~hugo/kdf/

ultramancool commented 8 years ago

@securitygeneration To expand on that, PBKDF2 would not be applicable here as we don't use passwords but long base64'd keys. HKDF could be used here instead, however it doesn't quite seem appropriate as we only need to gain 512 bits of data, which a single iteration of SHA512 can obtain.

If you've got a paper or someone credible who can refute this, I'm all ears, but to my knowledge there's no vulnerability here. Certainly no practical one at least.

ultramancool commented 8 years ago

Hmm, perhaps in our next update we should consider going to HKDF, I'll have to do some more reading on this to see if there's any practical benefit, but based off the fact we only need a small bit of data and that HKDF also appears to rely on hash functions to distribute this I suspect it may be similar...

k3d3 commented 8 years ago

If we want to discuss moving to HKDF, we should open a different issue for it. As far as this question goes, I think it's been adequately answered.

@securitygeneration, I'm going to close this issue for now, however if you have any more questions about how the security works, feel free to respond to and/or re-open this issue, or discuss on IRC with us on freenode in #upload.

Thanks!