Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:48304
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <D0.2B.05421.113F1FB4@pb1.pair.com>
To: internals@lists.php.net
Date: Mon, 17 May 2010 18:53:18 -0700
User-Agent: Thunderbird 2.0.0.21 (Macintosh/20090302)
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: openssl_(en|de)crypt missing IV
From: pollita@php.net (Sara Golemon)

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)