Bug #68344 [Com]: MySQLi does not provide way to disable peer certificate validation

From: Date: Mon, 10 Aug 2015 11:42:23 +0000
Subject: Bug #68344 [Com]: MySQLi does not provide way to disable peer certificate validation
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-195081@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=68344&edit=1

 ID:                 68344
 Comment by:         arekm at maven dot pl
 Reported by:        james at jamesreno dot com
 Summary:            MySQLi does not provide way to disable peer
                     certificate validation
 Status:             No Feedback
 Type:               Bug
 Package:            MySQLi related
 Operating System:   NA
 PHP Version:        5.6.2
 Assigned To:        mysql
 Block user comment: N
 Private report:     N

 New Comment:

Previous comment was describing IMO nicest solution (provide an API to copy options and use it) and
in mean time for those who need some workaround, tested on 5.6.12:

; obey few default context options
; https://bugs.php.net/bug.php?id=68344
diff -urbB php-5.6.12/ext/mysqlnd/mysqlnd_net.c php-5.6.12/ext/mysqlnd/mysqlnd_net.c
--- php-5.6.12/ext/mysqlnd/mysqlnd_net.c	2015-08-06 09:55:57.000000000 +0200
+++ php-5.6.12/ext/mysqlnd/mysqlnd_net.c	2015-08-10 13:25:30.187912101 +0200
@@ -29,6 +29,7 @@
 #include "mysqlnd_ext_plugin.h"
 #include "php_network.h"
 #include "zend_ini.h"
+#include "ext/standard/file.h"
 #ifdef MYSQLND_COMPRESSION_ENABLED
 #include <zlib.h>
 #endif
@@ -868,6 +868,21 @@ MYSQLND_METHOD(mysqlnd_net, enable_ssl)(
 		DBG_RETURN(FAIL);
 	}
 
+	if (FG(default_context)) {
+		zval **tmpzval = NULL;
+		int i = 0;
+		/* copy values from default stream settings */
+		char *opts[] = { "allow_self_signed", "cafile", "capath",
"ciphers", "CN_match",
+			"disable_compression", "local_cert", "local_pk",
"no_ticket", "passphrase",
+			"peer_fingerprint", "peer_name", "SNI_enabled",
"SNI_server_certs", "SNI_server_name",
+			"verify_depth", "verify_peer", "verify_peer_name", NULL };
+		while (opts[i]) {
+			if (php_stream_context_get_option(FG(default_context), "ssl", opts[i], &tmpzval)
== SUCCESS)
+				php_stream_context_set_option(context, "ssl", opts[i], *tmpzval);
+			i++;
+		}
+	}
+
 	if (net->data->options.ssl_key) {
 		zval key_zval;
 		ZVAL_STRING(&key_zval, net->data->options.ssl_key, 0);


Previous Comments:
------------------------------------------------------------------------
[2015-08-10 10:50:41] arekm at maven dot pl

Note, this is only to show the idea. It's a ugly patch, no error checking, exposing internal
ext/standard function in unfriendly way.

Anyway with this patch newly created mysqlnd stream inherits all options from default stream thus
obeying what we want - ssl verify_peer, verify_peer_name etc settings.

mysqli then works just fine and with verify_peer_name==false no longer yelds " Peer certificate
CN=... did not match expected CN=..." error.

diff -urbB ../1/php-5.6.12/ext/mysqlnd/mysqlnd_net.c ../php-5.6.12/ext/mysqlnd/mysqlnd_net.c
--- ../1/php-5.6.12/ext/mysqlnd/mysqlnd_net.c	2015-08-06 09:55:57.000000000 +0200
+++ ../php-5.6.12/ext/mysqlnd/mysqlnd_net.c	2015-08-10 12:44:58.377480518 +0200
@@ -29,6 +29,7 @@
 #include "mysqlnd_ext_plugin.h"
 #include "php_network.h"
 #include "zend_ini.h"
+#include "ext/standard/file.h"
 #ifdef MYSQLND_COMPRESSION_ENABLED
 #include <zlib.h>
 #endif
@@ -39,7 +40,7 @@
 #include <winsock.h>
 #endif
 
-
+extern int parse_context_options(php_stream_context *context, zval *options TSRMLS_DC);
 /* {{{ mysqlnd_set_sock_no_delay */
 static int
 mysqlnd_set_sock_no_delay(php_stream * stream TSRMLS_DC)
@@ -858,12 +859,14 @@
 static enum_func_status
 MYSQLND_METHOD(mysqlnd_net, enable_ssl)(MYSQLND_NET * const net TSRMLS_DC)
 {
 #ifdef MYSQLND_SSL_SUPPORTED
 	php_stream_context * context = php_stream_context_alloc(TSRMLS_C);
 	php_stream * net_stream = net->data->m.get_stream(net TSRMLS_CC);
 
+	parse_context_options(context, FG(default_context)->options TSRMLS_CC) ;
+
 	DBG_ENTER("mysqlnd_net::enable_ssl");
 	if (!context) {
 		DBG_RETURN(FAIL);
 	}
 
diff -urbB ../1/php-5.6.12/ext/standard/streamsfuncs.c ../php-5.6.12/ext/standard/streamsfuncs.c
--- ../1/php-5.6.12/ext/standard/streamsfuncs.c	2015-08-06 09:55:57.000000000 +0200
+++ ../php-5.6.12/ext/standard/streamsfuncs.c	2015-08-10 12:44:41.237035776 +0200
@@ -913,7 +913,7 @@
 	}
 }
 
-static int parse_context_options(php_stream_context *context, zval *options TSRMLS_DC)
+int parse_context_options(php_stream_context *context, zval *options TSRMLS_DC)
 {
 	HashPosition pos, opos;
 	zval **wval, **oval;

------------------------------------------------------------------------
[2015-08-10 10:23:13] arekm at maven dot pl

More, mysqli internally uses mysqlnd, which creates new context in mysqlnd_net::enable_ssl thus
ignoring default context.

Whit patch below I'm getting default context being used.

diff --git a/ext/mysqlnd/mysqlnd_net.c b/ext/mysqlnd/mysqlnd_net.c
index 8683248..2f63961 100644
--- a/ext/mysqlnd/mysqlnd_net.c
+++ b/ext/mysqlnd/mysqlnd_net.c
@@ -29,6 +29,7 @@
 #include "mysqlnd_ext_plugin.h"
 #include "php_network.h"
 #include "zend_ini.h"
+#include "ext/standard/file.h"
 #ifdef MYSQLND_COMPRESSION_ENABLED
 #include <zlib.h>
 #endif
@@ -859,7 +860,7 @@ static enum_func_status
 MYSQLND_METHOD(mysqlnd_net, enable_ssl)(MYSQLND_NET * const net TSRMLS_DC)
 {
 #ifdef MYSQLND_SSL_SUPPORTED
-       php_stream_context * context = php_stream_context_alloc(TSRMLS_C);
+       php_stream_context * context = FG(default_context) ? FG(default_context) :
php_stream_context_alloc(TSRMLS_C);
        php_stream * net_stream = net->data->m.get_stream(net TSRMLS_CC);

        DBG_ENTER("mysqlnd_net::enable_ssl");


Unfortunately above method means that mysqlnd will change some settings
in default context. What we need to do is to leave creation of new context
but then copy all options from default context to our new context, pseudocode:

php_stream_context * context = php_stream_context_alloc(TSRMLS_C);

copy_all_options_from_default_context_to_new_context(context, default_context)


Don't see any internal function that could do that, so someone with code knowledge would have
to write it.

------------------------------------------------------------------------
[2015-08-10 09:12:48] arekm at maven dot pl

"nicely sets GET_VER_OPT("verify_peer") and
GET_VER_OPT("verify_peer_name")) to 0, which is fine."

or rather these are 0 by default.... so default context options are actually not being passed here
:-/

------------------------------------------------------------------------
[2015-08-10 07:51:01] arekm at maven dot pl

Ok, the problem comes from generic openssl code:

ext/openssl/xp_ssl.c, apply_peer_verification_policy() function

    must_verify_peer = GET_VER_OPT("verify_peer")
        ? zend_is_true(*val)
        : sslsock->is_client;

    has_cnmatch_ctx_opt = GET_VER_OPT("CN_match");
    must_verify_peer_name = (has_cnmatch_ctx_opt || GET_VER_OPT("verify_peer_name"))
        ? zend_is_true(*val)
        : sslsock->is_client;


Now code:

$opts = array('ssl'=>array('verify_peer'=> false,
'verify_peer_name' => false));
stream_context_set_default($opts);

nicely sets GET_VER_OPT("verify_peer") and GET_VER_OPT("verify_peer_name")) to
0, which is fine.

Unfortunately above code fallback to sslsock->is_client which is 1. That means that if we are
client (is_client==1) then openssl extension ignores our verify_peer and verify_peer_name settings.
Why is that? No idea.


If I'm looking correctly then this commit changed behaviour:

commit ce8dc0ede2e8084beef1e7b03c8960e938c8399f
Author: Daniel Lowrey <rdlowrey@php.net>
Date:   Fri Feb 14 15:17:30 2014 -0700

    Bug #47030 (separate host and peer verification)

Previously it behaved differently for is_client == 1.

------------------------------------------------------------------------
[2015-08-09 20:03:26] arekm at maven dot pl

"Warning: mysqli_real_connect(): Peer certificate CN=.... did not match expected CN" comes
from openssl/xp_ssl.c

That code uses php stream functions like:

 stream = php_stream_alloc_rel(&php_openssl_socket_ops, sslsock, persistent_id, "r+");

...

then php_openssl_socket_ops structure has php_openssl_sockop_set_option function which then call few
functions and is some cases raises above error.

apply_peer_verification_policy actually checks verify_peer:

   must_verify_peer = GET_VER_OPT("verify_peer")
        ? zend_is_true(*val)
        : sslsock->is_client;


so it should be possible to switch this check off. Unfortunately for me php 5.6.12 is ignoring
setting for this:

$opts = array('ssl'=>array('verify_peer'=> false,
'verify_peer_name' => false));
stream_context_set_default($opts);

$cb = mysqli_init();
mysqli_ssl_set($cb, null, null, null, null, null);
mysqli_real_connect($cb,$db_host, $db_user, $db_pass, $db_name, false, false, MYSQLI_CLIENT_SSL)

and yet I'm getting
"Warning: mysqli_real_connect(): Peer certificate CN=.... did not match expected CN"


So the question is - why openssl code ignores verify_peer setting from default context?

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=68344


--
Edit this bug report at https://bugs.php.net/bug.php?id=68344&edit=1


Thread (57 messages)

« previous php.bugs (#195081) next »