hi,

On Wed, Jul 20, 2011 at 1:50 AM, Scott MacVicar <sc...@macvicar.net> 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 <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
>
>



-- 
Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to