Hi Jakub,

> -----Original Message-----
> From: jakub....@gmail.com [mailto:jakub....@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
> <tyr...@gmail.com>; Anatol Belski <anatol....@belski.net>; Pierre Joye
> <pierre....@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


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to