Re: svn: /php/php-src/ branches/PHP_5_4/ext/openssl/openssl.c trunk/ext/openssl/openssl.c
hi,
On Wed, Jul 20, 2011 at 1:50 AM, Scott MacVicar <[email protected]> wrote:
> Why did you even commit such a change without discussing it?
I maintain this part of the OpenSsl extension and the php windows
port. There was a horrible bug and slowdown with this code and I fixed
it. Users tested it and are happy with the fix.
> I understand your logic for doing this but there was no RFC or discussion around the impact of
> this.
I do not need to do a RFC for a fix and even less for an extension I maintain.
> 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?
Be serious two and half second. I gave you the explanation in my reply.
> Whats the speed difference between OpenSSL and your version. I know you removed the screen code
> which was causing a long delay.
300%
Now move on, we have other things to do that arguing about this little
broken function introduced back then.
>
> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
>
>
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
Thread (5 messages)