On Tue, May 18, 2010 at 10:19 AM, Davey Shafik <da...@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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
