search

Netflix / dynomite

A generic dynamo implementation for different k-v storage engines
Apache License 2.0
4.19k stars 530 forks source link

Use of AES key as IV will leak the key #591

Open blueben opened 6 years ago

blueben commented 6 years ago

https://github.com/Netflix/dynomite/blob/f59986bac5e6c7ec81f5b92209f5d65e5b7ab2a3/src/dyn_crypto.c#L265

EVP_EncryptInit_ex is being called with arg_aes_key as the IV. This is a problem as an IV must be both unique (used only once) and random when used in a CBC mode (as is the case here). This means that the IV is easily recovered and because the IV is the key, it means that attackers can recover the key and decrypt any data being transported. In short, as implemented today the crypto scheme for dynomite is completely ineffective.

References: https://en.wikipedia.org/wiki/Initialization_vector https://defuse.ca/blog/recovering-cbc-mode-iv-chosen-ciphertext.html http://www.cryptofails.com/post/70059594911/cakephp-using-the-iv-as-the-key https://nsamteladze.wordpress.com/2016/05/27/initialization-vector-iv-in-aes-cbc/

numberten commented 5 years ago

Openssl isn't appending the iv to the ciphertext for use in decryption, which is why it's required for decryption. Since the iv isn't being sent over the wire, the only side effect of making the iv equal to aes_key is that encryption is now deterministic.

Since there is no encryption oracle, an attacker isn't able to guess plaintexts either (the advantage gained by determinism).

smukil commented 5 years ago

@numberten You bring up a fair point, however, even though there isn't an encryption oracle, snooping traffic for long periods of time could give an attacker some insight into the data due to the determinism. It definitely would be a hard attack to carry out, but we should still fix it nonetheless.

numberten commented 5 years ago

@smukil

It definitely would be a hard attack to carry out, but we should still fix it nonetheless.

I agree!

I tried to keep my original comment succinct with the intention of explaining why I didn't think dynomite's crypto scheme was "completely ineffective". I'm definitely not advocating for deterministic encryption!

blueben commented 5 years ago

Fair enough. Completely ineffective is a bit hyperbolic. Still, I'm glad this is on the docket to fix.

numberten commented 5 years ago

https://github.com/Netflix/dynomite/pull/618 is my proposed solution for this issue. Probably worth keeping this ticket open, or creating a new one making note of the deterministic encryption, until a fix (#618 or otherwise) is merged into a stable branch.

ipapapa commented 5 years ago

@numberten see my comments on the PR on why #618 has not moved forward. I also do not feel we should be closing this issue until it is fixed.