Re: openssl_(en|de)crypt missing IV

On Tue, May 18, 2010 at 10:19 AM, Davey Shafik <davey@php.net> wrote: > 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... Don't think this warrants a notice/warning in existing code; for those who are concerned about this, a simple release note stating a fifth parameter has been added to openssl_encrypt should suffice IMO. That said, having a "truly" random and big enough IV (OTP) should keep hackers away for some time. There are also alternatives that already accept IV in their encrypt() methods; mcrypt comes to mind ;-) > > 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 > > > On May 17, 2010, at 9:53 PM, 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) >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > -- -- Tjerk