Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87956
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error)
To: "'Jakub Zelenka'" <bukka@php.net>,
	"'PHP internals list'" <internals@lists.php.net>,
	"'Ferenc Kovacs'" <tyra3l@gmail.com>,
	"'Pierre Joye'" <pierre.php@gmail.com>
References: <CAEKnhAF4DZ45_nku8GoRo1s=1ni1M-8krVyqc2FT0-6macH0Mw@mail.gmail.com>
In-Reply-To: <CAEKnhAF4DZ45_nku8GoRo1s=1ni1M-8krVyqc2FT0-6macH0Mw@mail.gmail.com>
Date: Mon, 31 Aug 2015 13:17:41 +0200
Message-ID: <020801d0e3de$a8be40f0$fa3ac2d0$@belski.net>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQKlTIs5g27BLw3cmuRcFI770qdri5x9Giqg
Content-Language: en-us
Subject: RE: [PHP-DEV] openssl_seal new param
From: anatol.php@belski.net ("Anatol Belski")

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
>=20
> Hi,
>=20
> I have been looking to https://bugs.php.net/bug.php?id=3D60632 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.
>=20
> 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:
>=20
> int openssl_seal ( string $data , string &$sealed_data , array =
&$env_keys , array
> $pub_key_ids [, string $method[, string &$iv ]] )
>=20
> (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.
>=20
> 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 :) ?
>=20
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:
>=20
> openssl_pkey_new(array( 'dh'=3D> array( 'p' =3D> $bin_prime, 'g' =3D> =
'2' )));
>=20
> This will also call RAND_bytes when generating priv key.
>=20
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.=20

Regards

Anatol