Use of AES key as IV will leak the key

[blueben]

dynomite/src/dyn_crypto.c

Line 265 in f59986b

if(!EVP_EncryptInit_ex(aes_encrypt_ctx, aes_cipher, NULL, arg_aes_key, arg_aes_key)) {

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]

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]

@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]

@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]

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

[numberten]

#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]

@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.