Return to Answer

There ARE a scenarios where adding the pbkdf2_hmac_sha1 step might be damaging to security. That includes an adversary having instrumented that step to log the input as part of another attack; or a successful side-channel attack on that step (e.g. using a less-privileged process on the same CPU).

Multiplying the crypto algorithms increases the attack surface.

It also increases the difficulty there is to try to ascertain that the code is correct, because more code is involved: now we also need to review pbkdf2_hmac_sha1, and make sure that it uses the whole entropy in its first parameter (that's at least plausible given what I know of PKKDF2 and aes-256-cbc), and outputs something that is acceptable whenever a OpenSSL::Cipher::Cipher.new('aes-256-cbc').random_key is (that's plausible, but I would be more hesitant with 3DES instead of AES, because it was customary, or even sometime required, that (3)DES keys use odd parity).

Also: if there is reason to fear that the initial key is bad, you should worry about that; and there is little reason to believe that OpenSSL::Random.random_bytes would give better entropy.

In summary: Yes it is overkill, don't do it. If you have concern about some crypto code, reapir that code; or use another code base; or try to workaround the issue, but only as a last resort, and after having understood what the issue really is.

As an aside: when I run the first two code lines in my head, my model of what happens is that two objects Cipher suitable for aes-256-cbc are created (each complete with key, IV, and other state variables), and destroyed after their respective key and IV have been copied; that seems like a lot of pointless work. If that's what happens, I would qualify that as a coding mistake, even if it had no adverse effect beyond requiring way too much work.

Also, that whole code would not work if the key of the Cipher object only had opaque access methods.

There ARE a scenarios where adding the pbkdf2_hmac_sha1 step might be damaging to security. That includes an adversary having instrumented that step to log the input as part of another attack; or a successful side-channel attack on that step (e.g. using a less-privileged process on the same CPU).

Multiplying the crypto algorithms increases the attack surface.

It also increases the difficulty there is to try to ascertain that the code is correct, because more code is involved: now we also need to review pbkdf2_hmac_sha1, and make sure that it spits out most of the entropy in its first parameter (that's at least plausible given what I know of PKKDF2, HMAC and SHA-1, and what I guess of aes-256-cbc), and outputs something that is acceptable whenever a OpenSSL::Cipher::Cipher.new('aes-256-cbc').random_key is (that's plausible, but I would be more hesitant with 3DES instead of AES, because it was customary, or even sometime required, that (3)DES keys use odd parity).

Also: if there is reason to fear that the initial key is bad, you should worry about that; and there is little reason to believe that OpenSSL::Random.random_bytes would give better entropy.

In summary: Yes it is overkill, don't do it. If you have concern about some crypto code, repair that code; or use another code base; or try to workaround the issue, but only as a last resort, and after having understood what the issue really is.

As an aside: when I run the first two code lines in my head, my model of what happens is that two objects Cipher suitable for aes-256-cbc are created (each complete with key, IV, and other state variables), and destroyed after their respective key and IV have been copied; that seems like a lot of pointless work. If that's what happens, I would qualify that as a coding mistake, even if it had no adverse effect beyond requiring way too much work.

Also, that whole code would not work if the key of the Cipher object only had opaque access methods.

There ARE a scenarios where adding the pbkdf2_hmac_sha1 step might be damaging to security. That includes an adversary having instrumented that step to log the input as part of another attack; or a successful side-channel attack on that step (e.g. using a less-privileged process on the same CPU).

Multiplying the crypto algorithms increases the attack surface.

It also increases the difficulty there is to try to ascertain that the code is correct, because more code is involved: now we also need to review pbkdf2_hmac_sha1, and make sure that it uses the whole entropy in its first parameter (that's at least plausible given what I know of PKKDF2 and aes-256-cbc), and outputs something that is acceptable whenever a OpenSSL::Cipher::Cipher.new('aes-256-cbc').random_key is (that's plausible, but I would be more hesitant with 3DES instead of AES, because it was customary, or even sometime required, that (3)DES keys use odd parity).

Also: if there is reason to fear that the initial key is bad, you should worry about that; and there is little reason to believe that OpenSSL::Random.random_bytes would give better entropy.

In summary: Yes it is overkill, don't do it.

As an aside: when I run the first two code lines in my head, my model of what happens is that two objects Cipher suitable for aes-256-cbc are created (each complete with key, IV, and other state variables), and destroyed after their respective key and IV have been copied; that seems like a lot of pointless work. If that's what happens, I would qualify that as a coding mistake, even if it had no adverse effect beyond requiring way too much work.

Also, that whole code would not work if the key of the Cipher object only had opaque access methods.

There ARE a scenarios where adding the pbkdf2_hmac_sha1 step might be damaging to security. That includes an adversary having instrumented that step to log the input as part of another attack; or a successful side-channel attack on that step (e.g. using a less-privileged process on the same CPU).

Multiplying the crypto algorithms increases the attack surface.

It also increases the difficulty there is to try to ascertain that the code is correct, because more code is involved: now we also need to review pbkdf2_hmac_sha1, and make sure that it uses the whole entropy in its first parameter (that's at least plausible given what I know of PKKDF2 and aes-256-cbc), and outputs something that is acceptable whenever a OpenSSL::Cipher::Cipher.new('aes-256-cbc').random_key is (that's plausible, but I would be more hesitant with 3DES instead of AES, because it was customary, or even sometime required, that (3)DES keys use odd parity).

Also: if there is reason to fear that the initial key is bad, you should worry about that; and there is little reason to believe that OpenSSL::Random.random_bytes would give better entropy.

In summary: Yes it is overkill, don't do it. If you have concern about some crypto code, reapir that code; or use another code base; or try to workaround the issue, but only as a last resort, and after having understood what the issue really is.

As an aside: when I run the first two code lines in my head, my model of what happens is that two objects Cipher suitable for aes-256-cbc are created (each complete with key, IV, and other state variables), and destroyed after their respective key and IV have been copied; that seems like a lot of pointless work. If that's what happens, I would qualify that as a coding mistake, even if it had no adverse effect beyond requiring way too much work.

Also, that whole code would not work if the key of the Cipher object only had opaque access methods.

There ARE a scenarios where adding the pbkdf2_hmac_sha1 step might be damaging to security. That includes an adversary having instrumented that step to log the input as part of another attack; or a successful side-channel attack on that step (e.g. using a less-privileged process on the same CPU).

Multiplying the crypto algorithms increases the attack surface.

It also increases the difficulty there is to try to ascertain that the code is correct, because more code is involved: now we also need to review pbkdf2_hmac_sha1, and make sure that it uses the whole entropy in its first parameter (that's at least plausible given what I know of PKKDF2 and aes-256-cbc), and outputs something that is acceptable whenever a OpenSSL::Cipher::Cipher.new('aes-256-cbc').random_key is (that's plausible, but I would be more hesitant with 3DES instead of AES, because it was customary, or even sometime required, that (3)DES keys use odd parity).

In summary: don't do it.

There ARE a scenarios where adding the pbkdf2_hmac_sha1 step might be damaging to security. That includes an adversary having instrumented that step to log the input as part of another attack; or a successful side-channel attack on that step (e.g. using a less-privileged process on the same CPU).

Multiplying the crypto algorithms increases the attack surface.

It also increases the difficulty there is to try to ascertain that the code is correct, because more code is involved: now we also need to review pbkdf2_hmac_sha1, and make sure that it uses the whole entropy in its first parameter (that's at least plausible given what I know of PKKDF2 and aes-256-cbc), and outputs something that is acceptable whenever a OpenSSL::Cipher::Cipher.new('aes-256-cbc').random_key is (that's plausible, but I would be more hesitant with 3DES instead of AES, because it was customary, or even sometime required, that (3)DES keys use odd parity).

Also: if there is reason to fear that the initial key is bad, you should worry about that; and there is little reason to believe that OpenSSL::Random.random_bytes would give better entropy.

In summary: Yes it is overkill, don't do it.

As an aside: when I run the first two code lines in my head, my model of what happens is that two objects Cipher suitable for aes-256-cbc are created (each complete with key, IV, and other state variables), and destroyed after their respective key and IV have been copied; that seems like a lot of pointless work. If that's what happens, I would qualify that as a coding mistake, even if it had no adverse effect beyond requiring way too much work.

Also, that whole code would not work if the key of the Cipher object only had opaque access methods.