Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Andrey Hristov
 More info about the segfaults? Tests that show the segfaults and thus 
keep us from regressions?


Andrey


Antony Dovgal wrote:

tony2001 Thu, 22 Apr 2010 15:59:44 +

Revision: http://svn.php.net/viewvc?view=revisionrevision=298331

Log:
revert most of the Andrey's patch that causes segfaults
(as agreed with Pierre)

Changed paths:
U   php/php-src/branches/PHP_5_3/ext/openssl/openssl.c
U   php/php-src/trunk/ext/openssl/openssl.c

Modified: php/php-src/branches/PHP_5_3/ext/openssl/openssl.c
===
--- php/php-src/branches/PHP_5_3/ext/openssl/openssl.c  2010-04-22 15:51:03 UTC 
(rev 298330)
+++ php/php-src/branches/PHP_5_3/ext/openssl/openssl.c  2010-04-22 15:59:44 UTC 
(rev 298331)
@@ -4445,7 +4445,6 @@
EVP_PKEY *key = NULL;
SSL *tmpssl;
char resolved_path_buff[MAXPATHLEN];
-   const char * private_key = NULL;

if (VCWD_REALPATH(certfile, resolved_path_buff)) {
/* a certificate to use for authentication */
@@ -4454,10 +4453,8 @@
return NULL;
}

-   GET_VER_OPT_STRING(local_pk, private_key);
-
-   if (private_key  SSL_CTX_use_PrivateKey_file(ctx, 
private_key, SSL_FILETYPE_PEM) != 1) {
-   php_error_docref(NULL TSRMLS_CC, E_WARNING, Unable 
to set private key file `%s', private_key);
+   if (SSL_CTX_use_PrivateKey_file(ctx, 
resolved_path_buff, SSL_FILETYPE_PEM) != 1) {
+   php_error_docref(NULL TSRMLS_CC, E_WARNING, Unable 
to set private key file `%s', resolved_path_buff);
return NULL;
}


Modified: php/php-src/trunk/ext/openssl/openssl.c
===
--- php/php-src/trunk/ext/openssl/openssl.c 2010-04-22 15:51:03 UTC (rev 
298330)
+++ php/php-src/trunk/ext/openssl/openssl.c 2010-04-22 15:59:44 UTC (rev 
298331)
@@ -4443,7 +4443,6 @@
EVP_PKEY *key = NULL;
SSL *tmpssl;
char resolved_path_buff[MAXPATHLEN];
-   const char * private_key = NULL;

if (VCWD_REALPATH(certfile, resolved_path_buff)) {
/* a certificate to use for authentication */
@@ -4452,10 +4451,8 @@
return NULL;
}

-   GET_VER_OPT_STRING(local_pk, private_key);
-
-   if (private_key  SSL_CTX_use_PrivateKey_file(ctx, 
private_key, SSL_FILETYPE_PEM) != 1) {
-   php_error_docref(NULL TSRMLS_CC, E_WARNING, Unable 
to set private key file `%s', private_key);
+   if (SSL_CTX_use_PrivateKey_file(ctx, reso, 
SSL_FILETYPE_PEM) != 1) {
+   php_error_docref(NULL TSRMLS_CC, E_WARNING, Unable 
to set private key file `%s', resolved_path_buff);
return NULL;
}






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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Pierre Joye
hi,

On Fri, Apr 23, 2010 at 12:14 PM, Andrey Hristov p...@hristov.com wrote:
  More info about the segfaults? Tests that show the segfaults and thus keep
 us from regressions?

The tests we have in ext/openssl/tests crash.

However we were wondering why you did these changes and I did not see
any relation between the commit msg and this change. If there is a bug
in this code, please provide a reproduce case and a patch so we can be
sure it won't break ssl functions.

Cheers,
-- 
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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Andrey Hristov

 Pierre, Pierre,
+	if (SSL_CTX_use_PrivateKey_file(ctx, resolved_path_buff, 
SSL_FILETYPE_PEM) != 1) {


this is what the revert gives back, if you go and check what this 
function does:


The SSL_CTX_use_PrivateKey_file function loads the private key for use 
with Secure Sockets Layer (SSL) sessions using a specific context (CTX) 
structure.


However, what gets passed is path to a certificate, not to a private 
key. So you reintroduce a bug, that is.


And locally I reverted the patch that was reverting my changes, thus 
introducing them again, and I got :

Number of tests :   4138
Tests skipped   :3 (  7.3%) 
Tests warned:0 (  0.0%) (  0.0%)
Tests failed:0 (  0.0%) (  0.0%)
Expected fail   :0 (  0.0%) (  0.0%)
Tests passed:   38 ( 92.7%) (100.0%)
-
Time taken  :3 seconds
=


So, I am going to revert the revert and reintroduce the code that fixes 
a bug.


Have a nice day!

Andrey


Pierre Joye wrote:

hi,

On Fri, Apr 23, 2010 at 12:14 PM, Andrey Hristov p...@hristov.com wrote:

 More info about the segfaults? Tests that show the segfaults and thus keep
us from regressions?


The tests we have in ext/openssl/tests crash.

However we were wondering why you did these changes and I did not see
any relation between the commit msg and this change. If there is a bug
in this code, please provide a reproduce case and a patch so we can be
sure it won't break ssl functions.

Cheers,



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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Pierre Joye
On Fri, Apr 23, 2010 at 1:05 PM, Andrey Hristov p...@hristov.com wrote:


 So, I am going to revert the revert and reintroduce the code that fixes a
 bug.

No. Don't do that.

If you have found a bug in SSL please report a bug with a SSL specific
test case and a patch if you have one.

Thanks,
-- 
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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Johannes Schlüter
On Fri, 2010-04-23 at 13:05 +0200, Andrey Hristov wrote:
 Pierre, Pierre,
 + if (SSL_CTX_use_PrivateKey_file(ctx, resolved_path_buff, 
 SSL_FILETYPE_PEM) != 1) {
 
 this is what the revert gives back, if you go and check what this 
 function does:
 
 The SSL_CTX_use_PrivateKey_file function loads the private key for use 
 with Secure Sockets Layer (SSL) sessions using a specific context (CTX) 
 structure.
 
 However, what gets passed is path to a certificate, not to a private 
 key. So you reintroduce a bug, that is.
 
 And locally I reverted the patch that was reverting my changes, thus 
 introducing them again, and I got :
 Number of tests :   4138
 Tests skipped   :3 (  7.3%) 
 Tests warned:0 (  0.0%) (  0.0%)
 Tests failed:0 (  0.0%) (  0.0%)
 Expected fail   :0 (  0.0%) (  0.0%)
 Tests passed:   38 ( 92.7%) (100.0%)
 -
 Time taken  :3 seconds
 =

The interesting question is: What's the difference between the systems?
Andrey's system doesn't show an issue, gcov's last run was fine
http://gcov.php.net/viewer.php?version=PHP_5_3func=tests (ok, this is
5.3, but I don't see difference between trunk and 5.3)

What openssl versions yre you using? Any special compiler flags?

The patch itself is needed to so login using SSL certificate to an MySQL
server works under mysqlnd.

Maybe somebody who can reproduce the crash could take a deeper look
what's wrong?

thanks,
johannes



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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Antony Dovgal
On 23.04.2010 15:05, Andrey Hristov wrote:
 The SSL_CTX_use_PrivateKey_file function loads the private key for use 
 with Secure Sockets Layer (SSL) sessions using a specific context (CTX) 
 structure.
 
 However, what gets passed is path to a certificate, not to a private 
 key. So you reintroduce a bug, that is.

AFAIK the certificate may contain several items, including the private key.
At least that worked fine for me.

 And locally I reverted the patch that was reverting my changes, thus 
 introducing them again, and I got :
 Number of tests :   4138
 Tests skipped   :3 (  7.3%) 
 Tests warned:0 (  0.0%) (  0.0%)
 Tests failed:0 (  0.0%) (  0.0%)
 Expected fail   :0 (  0.0%) (  0.0%)
 Tests passed:   38 ( 92.7%) (100.0%)
 -
 Time taken  :3 seconds
 =

Oh, nice!
Try to run ext/openssl/tests/bug46127.phpt with valgrind now.

 So, I am going to revert the revert and reintroduce the code that fixes 
 a bug.

Your fix fixes nothing, please don't reintroduce the segfaults.
If you're unable to reproduce them, I'm ready to do it for you: 
http://pastebin.com/TPCd7WUU

-- 
Wbr,
Antony Dovgal
---
http://pinba.org - realtime statistics for PHP

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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Andrey Hristov

Antony Dovgal wrote:

On 23.04.2010 15:05, Andrey Hristov wrote:
The SSL_CTX_use_PrivateKey_file function loads the private key for use 
with Secure Sockets Layer (SSL) sessions using a specific context (CTX) 
structure.


However, what gets passed is path to a certificate, not to a private 
key. So you reintroduce a bug, that is.


AFAIK the certificate may contain several items, including the private key.
At least that worked fine for me.


This is my certificate file : http://hristov.com/tmp/client-cert.pem
and here is the private key : http://hristov.com/tmp/client-key.pem

And locally I reverted the patch that was reverting my changes, thus 
introducing them again, and I got :

Number of tests :   4138
Tests skipped   :3 (  7.3%) 
Tests warned:0 (  0.0%) (  0.0%)
Tests failed:0 (  0.0%) (  0.0%)
Expected fail   :0 (  0.0%) (  0.0%)
Tests passed:   38 ( 92.7%) (100.0%)
-
Time taken  :3 seconds
=


Oh, nice!
Try to run ext/openssl/tests/bug46127.phpt with valgrind now.

So, I am going to revert the revert and reintroduce the code that fixes 
a bug.


Your fix fixes nothing, please don't reintroduce the segfaults.
If you're unable to reproduce them, I'm ready to do it for you: 
http://pastebin.com/TPCd7WUU



Andrey

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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Andrey Hristov

Tony,

Antony Dovgal wrote:

On 23.04.2010 15:05, Andrey Hristov wrote:
The SSL_CTX_use_PrivateKey_file function loads the private key for use 
with Secure Sockets Layer (SSL) sessions using a specific context (CTX) 
structure.


However, what gets passed is path to a certificate, not to a private 
key. So you reintroduce a bug, that is.


AFAIK the certificate may contain several items, including the private key.
At least that worked fine for me.


after I checked this matter with a guy who knows a lot more about crypto 
than me, it seems that the pem file can, but not always the case, 
include the private key next to the public key. The original SSL code 
does not support pem files which don't include the private key but the 
private key is separate. Having the private key in a separate file is 
not a bad decision but is not always the case, as we see.


I have prepared a patch that doesn't segfault PHP when bug46127.phpt is 
ran but allows one to use separate public and private key files.


http://hristov.com/tmp/new_ssl_patch.txt

And locally I reverted the patch that was reverting my changes, thus 
introducing them again, and I got :

Number of tests :   4138
Tests skipped   :3 (  7.3%) 
Tests warned:0 (  0.0%) (  0.0%)
Tests failed:0 (  0.0%) (  0.0%)
Expected fail   :0 (  0.0%) (  0.0%)
Tests passed:   38 ( 92.7%) (100.0%)
-
Time taken  :3 seconds
=


Oh, nice!
Try to run ext/openssl/tests/bug46127.phpt with valgrind now.

So, I am going to revert the revert and reintroduce the code that fixes 
a bug.


Your fix fixes nothing, please don't reintroduce the segfaults.


My fix fixes the situation described above.


If you're unable to reproduce them, I'm ready to do it for you: 
http://pastebin.com/TPCd7WUU



Andrey

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



Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_3/ext/openssl/openssl.c trunk/ext/openssl/openssl.c

2010-04-23 Thread Pierre Joye
Please open a new bug with the details + reproduce script. Thanks.

On Fri, Apr 23, 2010 at 2:42 PM, Andrey Hristov p...@hristov.com wrote:
 Tony,

 Antony Dovgal wrote:

 On 23.04.2010 15:05, Andrey Hristov wrote:

 The SSL_CTX_use_PrivateKey_file function loads the private key for use
 with Secure Sockets Layer (SSL) sessions using a specific context (CTX)
 structure.

 However, what gets passed is path to a certificate, not to a private key.
 So you reintroduce a bug, that is.

 AFAIK the certificate may contain several items, including the private
 key.
 At least that worked fine for me.

 after I checked this matter with a guy who knows a lot more about crypto
 than me, it seems that the pem file can, but not always the case, include
 the private key next to the public key. The original SSL code does not
 support pem files which don't include the private key but the private key is
 separate. Having the private key in a separate file is not a bad decision
 but is not always the case, as we see.

 I have prepared a patch that doesn't segfault PHP when bug46127.phpt is ran
 but allows one to use separate public and private key files.

 http://hristov.com/tmp/new_ssl_patch.txt

 And locally I reverted the patch that was reverting my changes, thus
 introducing them again, and I got :
 Number of tests :   41                38
 Tests skipped   :    3 (  7.3%) 
 Tests warned    :    0 (  0.0%) (  0.0%)
 Tests failed    :    0 (  0.0%) (  0.0%)
 Expected fail   :    0 (  0.0%) (  0.0%)
 Tests passed    :   38 ( 92.7%) (100.0%)
 -
 Time taken      :    3 seconds
 =

 Oh, nice!
 Try to run ext/openssl/tests/bug46127.phpt with valgrind now.

 So, I am going to revert the revert and reintroduce the code that fixes a
 bug.

 Your fix fixes nothing, please don't reintroduce the segfaults.

 My fix fixes the situation described above.

 If you're unable to reproduce them, I'm ready to do it for you:
 http://pastebin.com/TPCd7WUU


 Andrey




-- 
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