openssl_(en|de)crypt missing IV

The least disruptive change would be to have it as the last arg, and default to the current all-null value.

Perhaps you could do this and add a warning akin to the date.timezone if none is passed?

Having said that, I don't think the disruption would be too bad, I haven't seen much use of the openssl stuff in the
wild; it's pretty much pointless anyways, everybody knows you can decrypt anything with a modem coupler and
any 15 year old kid...

Another option, is an openssl_set_iv($iv); method, that would setup for openssl_encrypt/decrypt to use, otherwise
use the current all-null option....

This has the added benefit of being as global as you like, and no need to keep passing the IV to encrypt/decrypt
all over the place. It has the potential of clashing when you bring multiple codebases together however (i.e a framework).
Possible to limit to a single namespace?

• Davey

I was just looking through the implementation of openssl_encrypt() (and openssl_decrypt()) today because I need to make some encrypted payloads, but the prototype didn't have anywhere to place an initialization vector.

On opening ext/openssl/openssl.c, I noticed line 4620 which simply hardcodes IV as a string of NULL bytes.

This is a bad idea roughly equivalent to hashing passwords without salt; Worse, it prevents interoperability at the application layer by preventing the decryption of a data stream where the generator used an IV other than all-null.

Fixing this is a simple matter, but I wanted to bounce approaches for BC (or lack thereof) off everyone else since this version of openssl_encrypt() is already "in the wild".

The most direct and obvious solution is to add a fifth, optional parameter to openssl_encrypt() and openssl_decrypt() to take IV as a string. The problems with this are that it: (A) Places the IV in an odd argument location, and (B) Does not enforce the passing of an IV (since raw is already optional). As stated above, IV really should be enforced, given what it does to soften the security normally offered by a chaining block method.

That said, I'd like to propose something unpopular; Change the signature for these methods entirely, deliberately breaking BC. I know, I know.... spare me your rotten tomatoes. I think it's justified in this case because, as they are now, these functions are useless at best, and possibly dangerous in terms of encouraging unsafe practices with regards to cryptography.

I think it's worth a BC break. Comments?

-Sara

P.S. - Here's the signature I'd go with: openssl_encrypt($data, $method, $iv, $key, $raw=false)

hi Sara,

I was just looking through the implementation of openssl_encrypt() (and
openssl_decrypt()) today because I need to make some encrypted payloads, but
the prototype didn't have anywhere to place an initialization vector.

On opening ext/openssl/openssl.c, I noticed line 4620 which simply hardcodes
IV as a string of NULL bytes.

This is a bad idea roughly equivalent to hashing passwords without salt;
Worse, it prevents interoperability at the application layer by preventing
the decryption of a data stream where the generator used an IV other than
all-null.

Fixing this is a simple matter, but I wanted to bounce approaches for BC (or
lack thereof) off everyone else since this version of openssl_encrypt() is
already "in the wild".

I think it's worth a BC break.  Comments?

To break BC is a no go, even if your arguments are appealing (even in
a major version).

I would suggest a new function:

openssl_encrypt_iv($data, $method, $key, $iv, $raw=false);

Which will use the same internal implementations internally but with a
different entry point. please note that I moved iv to the 4th position
as well.

Cheers,

Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

Sara Golemon wrote:

I was just looking through the implementation of openssl_encrypt()
(and openssl_decrypt()) today because I need to make some encrypted
payloads, but the prototype didn't have anywhere to place an
initialization vector.

On opening ext/openssl/openssl.c, I noticed line 4620 which simply
hardcodes IV as a string of NULL bytes.

This is a bad idea roughly equivalent to hashing passwords without
salt; Worse, it prevents interoperability at the application layer by
preventing the decryption of a data stream where the generator used an
IV other than all-null.

Fixing this is a simple matter, but I wanted to bounce approaches for
BC (or lack thereof) off everyone else since this version of
openssl_encrypt() is already "in the wild".

The most direct and obvious solution is to add a fifth, optional
parameter to openssl_encrypt() and openssl_decrypt() to take IV as a
string. The problems with this are that it: (A) Places the IV in an
odd argument location, and (B) Does not enforce the passing of an IV
(since raw is already optional). As stated above, IV really should
be enforced, given what it does to soften the security normally
offered by a chaining block method.

That said, I'd like to propose something unpopular; Change the
signature for these methods entirely, deliberately breaking BC. I
know, I know.... spare me your rotten tomatoes. I think it's
justified in this case because, as they are now, these functions are
useless at best, and possibly dangerous in terms of encouraging unsafe
practices with regards to cryptography.

I think it's worth a BC break. Comments?

-Sara

P.S. - Here's the signature I'd go with: openssl_encrypt($data,
$method, $iv, $key, $raw=false)

Personally I would like to see the signature changed to be even more
aligned with the ones from mcrypt - since thats what people are most
used to using. Currently without the iv, these methods are useless imo.
The biggest downfall from changing the signature where the parameter is
not optional, is that it cant be done until a major release, which will
drag out the time where the functionality will be useful to anyone.

When the iv parameter is added (however it is done), also add
functionality for creating an iv and getting iv length based on method.
These methods are crucial for the function to be useful in real world apps.

Rob