RE: [PHP-DEV] openssl_seal new param

Hi Jakub, > -----Original Message----- > From: jakub.php@gmail.com [mailto:jakub.php@gmail.com] On Behalf Of Jakub > Zelenka > Sent: Sunday, August 30, 2015 9:11 PM > To: PHP internals list <internals@lists.php.net>; Ferenc Kovacs > <tyra3l@gmail.com>; Anatol Belski <anatol.php@belski.net>; Pierre Joye > <pierre.php@gmail.com> > Subject: [PHP-DEV] openssl_seal new param > > Hi, > > I have been looking to https://bugs.php.net/bug.php?id=60632 > which is about > failing (segfaulting) openssl_seal when used with cipher alg that requires IV (e.g. > AES-128-CBC). I think that the patch looks reasonable from the quick look. > > The only question and the reason why I'm sending this here is if everyone (and > mainly Ferenc ) is ok with adding new ref arg to openssl_seal that will return iv > to 5.6? So the definition is: > > int openssl_seal ( string $data , string &$sealed_data , array &$env_keys , array > $pub_key_ids [, string $method[, string &$iv ]] ) > > (the last iv is new). There would be also a new param for openssl_open that > would allow to pass that IV for opening sealed data. > > Alternatively we could just disable IV ciphers in 5.6 to at least prevent the > segfault and add it to 7 if Anatol and Kalle are ok with that or 7.1 if not :) ? > Were it possible to list the ciphers that are affected? IMHO adding the IV arg were fine for 7.0 if there are many ciphers affected (say 1/3 of them), otherwise just checking EVP_CIPHER_iv_length() and returning false were appropriate with a subsequent proper fix flowing into 7.1. > There also is an another thing for TS Win build (probably question for Anatol and > Pierre :) ). The thing is that EVP_SealInit uses internally RAND_bytes. IIRC there is > some locking issue with openssl RAND on TS win ( the reason why > openssl_random_pseudo_bytes uses Win random) so I was wondering if it > should be disabled on win? The thing is that it is already a case for other > functions. One example is generating key params in > openssl_pkey_new: > > openssl_pkey_new(array( 'dh'=> array( 'p' => $bin_prime, > 'g' => '2' ))); > > This will also call RAND_bytes when generating priv key. > If you look into crypto/rand/rand_lib.c within the openssl src - it is not thread safe even in 1.0.2 branch. It is a good catch, but has only to do with the thread safety. And the issue is exactly that there is no locking around RAND_bytes() affected codes in PHP. So the issue is cross platform. A quick solution could be using the mutex functionality exported from TSRM (tsrm_mutex_*() functions) in the thread safe builds for affected functions. Regards Anatol