Why did you even commit such a change without discussing it? I understand your logic for doing this but there was no RFC or discussion around the impact of this.
OpenSSL has been FIPS certified, your change has changed this contract and it's calling back into a Windows API. Has it been reviewed for correctness? Whats the speed difference between OpenSSL and your version. I know you removed the screen code which was causing a long delay. On Jul 19, 2011, at 4:13 PM, Pierre Joye wrote: > Why did you not ask in the 1st place before reverting it? > > Please don't waste our time with such things. Users expect this > (tested) fix in the next releases. > > Now, openssl has lower minimum windows version support that we do, and > does all possible things to improve the entropy, which is not required > nor necessary for the windows we support. > > The idea in the 1st place was to have a standard set of random > functions instead of this, as you well know. Now it is too late and we > have to live with this function. While the set of random will surely > come at some point too. > > On Wed, Jul 20, 2011 at 1:04 AM, Scott MacVicar <sc...@macvicar.net> wrote: >> Why isn't this fixed upstream? This is a horrible idea to make core changes >> like this without a discussion. >> >> I'll revert this again so we can at least have the opportunity to discuss >> this. >> >> S >> >> On 19 Jul 2011, at 15:55, Pierre Joye <pierre....@gmail.com> wrote: >> >>> Please restore that, now. That's not your cup of tea and it is the way >>> it should have been in the 1st place. >>> >>> On Wed, Jul 20, 2011 at 12:29 AM, Scott MacVicar <scott...@php.net> wrote: >>>> scottmac Tue, 19 Jul 2011 22:29:55 +0000 >>>> >>>> Revision: http://svn.php.net/viewvc?view=revision&revision=313455 >>>> >>>> Log: >>>> Revert change to use a special Windows version of >>>> openssl_random_pseudo_bytes(). >>>> >>>> Lets discuss this on internals first. We're advertising something from the >>>> OpenSSL library >>>> and then subverting it with another Windows OS call. >>>> >>>> What are the implications of this? Should we make this available in >>>> ext/standard/ instead? >>>> >>>> Changed paths: >>>> U php/php-src/branches/PHP_5_4/ext/openssl/openssl.c >>>> U php/php-src/trunk/ext/openssl/openssl.c >>>> >>>> Modified: php/php-src/branches/PHP_5_4/ext/openssl/openssl.c >>>> =================================================================== >>>> --- php/php-src/branches/PHP_5_4/ext/openssl/openssl.c 2011-07-19 >>>> 22:18:08 UTC (rev 313454) >>>> +++ php/php-src/branches/PHP_5_4/ext/openssl/openssl.c 2011-07-19 >>>> 22:29:55 UTC (rev 313455) >>>> @@ -4930,19 +4930,10 @@ >>>> >>>> buffer = emalloc(buffer_length + 1); >>>> >>>> -#ifdef PHP_WIN32 >>>> - strong_result = 1; >>>> - /* random/urandom equivalent on Windows */ >>>> - if (php_win32_get_random_bytes(buffer, (size_t) buffer_length) == >>>> FAILURE){ >>>> - efree(buffer); >>>> - RETURN_FALSE; >>>> - } >>>> -#else >>>> if ((strong_result = RAND_pseudo_bytes(buffer, buffer_length)) < 0) >>>> { >>>> efree(buffer); >>>> RETURN_FALSE; >>>> } >>>> -#endif >>>> >>>> buffer[buffer_length] = 0; >>>> RETVAL_STRINGL((char *)buffer, buffer_length, 0); >>>> >>>> Modified: php/php-src/trunk/ext/openssl/openssl.c >>>> =================================================================== >>>> --- php/php-src/trunk/ext/openssl/openssl.c 2011-07-19 22:18:08 UTC >>>> (rev 313454) >>>> +++ php/php-src/trunk/ext/openssl/openssl.c 2011-07-19 22:29:55 UTC >>>> (rev 313455) >>>> @@ -4926,19 +4926,10 @@ >>>> >>>> buffer = emalloc(buffer_length + 1); >>>> >>>> -#ifdef PHP_WIN32 >>>> - strong_result = 1; >>>> - /* random/urandom equivalent on Windows */ >>>> - if (php_win32_get_random_bytes(buffer, (size_t) buffer_length) == >>>> FAILURE){ >>>> - efree(buffer); >>>> - RETURN_FALSE; >>>> - } >>>> -#else >>>> if ((strong_result = RAND_pseudo_bytes(buffer, buffer_length)) < 0) >>>> { >>>> efree(buffer); >>>> RETURN_FALSE; >>>> } >>>> -#endif >>>> >>>> buffer[buffer_length] = 0; >>>> RETVAL_STRINGL((char *)buffer, buffer_length, 0); >>>> >>>> >>>> -- >>>> PHP CVS Mailing List (http://www.php.net/) >>>> To unsubscribe, visit: http://www.php.net/unsub.php >>>> >>> >>> >>> >>> -- >>> Pierre >>> >>> @pierrejoye | http://blog.thepimp.net | http://www.libgd.org >> > > > > -- > Pierre > > @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
