From 0ac60d605567f3274c0db93afb5fb9c04ebf3743 Mon Sep 17 00:00:00 2001 From: twosee Date: Fri, 3 Sep 2021 09:37:42 +0800 Subject: [PATCH] Micro optimizations for xp_ssl.c (#7447) If certfile/private_key points to a file that doesn't exist, it throw a warning and return failure now. Also fixed sni_server tests. Co-authored-by: Nikita Popov --- ext/openssl/tests/sni_server.phpt | 1 - ext/openssl/tests/sni_server_key_cert.phpt | 1 - ext/openssl/xp_ssl.c | 67 ++++++++++------------ 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/ext/openssl/tests/sni_server.phpt b/ext/openssl/tests/sni_server.phpt index cb3b3d77168..ecfcf7a8172 100644 --- a/ext/openssl/tests/sni_server.phpt +++ b/ext/openssl/tests/sni_server.phpt @@ -11,7 +11,6 @@ if (!function_exists("proc_open")) die("skip no proc_open"); $serverCode = <<<'CODE' $flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN; $ctx = stream_context_create(['ssl' => [ - 'local_cert' => __DIR__ . '/domain1.pem', 'SNI_server_certs' => [ "cs.php.net" => __DIR__ . "/sni_server_cs.pem", "uk.php.net" => __DIR__ . "/sni_server_uk.pem", diff --git a/ext/openssl/tests/sni_server_key_cert.phpt b/ext/openssl/tests/sni_server_key_cert.phpt index 2d0ef8c1944..a4e1f5e42a3 100644 --- a/ext/openssl/tests/sni_server_key_cert.phpt +++ b/ext/openssl/tests/sni_server_key_cert.phpt @@ -11,7 +11,6 @@ if (!function_exists("proc_open")) die("skip no proc_open"); $serverCode = <<<'CODE' $flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN; $ctx = stream_context_create(['ssl' => [ - 'local_cert' => __DIR__ . '/domain1.pem', 'SNI_server_certs' => [ "cs.php.net" => [ 'local_cert' => __DIR__ . "/sni_server_cs_cert.pem", diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 15d11e3b9b6..5564bf6f086 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -897,35 +897,30 @@ static int php_openssl_set_local_cert(SSL_CTX *ctx, php_stream *stream) /* {{{ * char resolved_path_buff[MAXPATHLEN]; const char *private_key = NULL; - if (VCWD_REALPATH(certfile, resolved_path_buff)) { - /* a certificate to use for authentication */ - if (SSL_CTX_use_certificate_chain_file(ctx, resolved_path_buff) != 1) { - php_error_docref(NULL, E_WARNING, - "Unable to set local cert chain file `%s'; Check that your cafile/capath " - "settings include details of your certificate and its issuer", - certfile); - return FAILURE; - } - GET_VER_OPT_STRING("local_pk", private_key); + if (!VCWD_REALPATH(certfile, resolved_path_buff)) { + php_error_docref(NULL, E_WARNING, "Unable to get real path of certificate file `%s'", certfile); + return FAILURE; + } + /* a certificate to use for authentication */ + if (SSL_CTX_use_certificate_chain_file(ctx, resolved_path_buff) != 1) { + php_error_docref(NULL, E_WARNING, + "Unable to set local cert chain file `%s'; Check that your cafile/capath " + "settings include details of your certificate and its issuer", + certfile); + return FAILURE; + } - if (private_key) { - char resolved_path_buff_pk[MAXPATHLEN]; - if (VCWD_REALPATH(private_key, resolved_path_buff_pk)) { - if (SSL_CTX_use_PrivateKey_file(ctx, resolved_path_buff_pk, SSL_FILETYPE_PEM) != 1) { - php_error_docref(NULL, E_WARNING, "Unable to set private key file `%s'", resolved_path_buff_pk); - return FAILURE; - } - } - } else { - if (SSL_CTX_use_PrivateKey_file(ctx, resolved_path_buff, SSL_FILETYPE_PEM) != 1) { - php_error_docref(NULL, E_WARNING, "Unable to set private key file `%s'", resolved_path_buff); - return FAILURE; - } - } - - if (!SSL_CTX_check_private_key(ctx)) { - php_error_docref(NULL, E_WARNING, "Private key does not match certificate!"); - } + GET_VER_OPT_STRING("local_pk", private_key); + if (private_key && !VCWD_REALPATH(private_key, resolved_path_buff)) { + php_error_docref(NULL, E_WARNING, "Unable to get real path of private key file `%s'", private_key); + return FAILURE; + } + if (SSL_CTX_use_PrivateKey_file(ctx, resolved_path_buff, SSL_FILETYPE_PEM) != 1) { + php_error_docref(NULL, E_WARNING, "Unable to set private key file `%s'", resolved_path_buff); + return FAILURE; + } + if (!SSL_CTX_check_private_key(ctx)) { + php_error_docref(NULL, E_WARNING, "Private key does not match certificate!"); } } @@ -1614,11 +1609,16 @@ int php_openssl_setup_crypto(php_stream *stream, /* We need to do slightly different things based on client/server method * so lets remember which method was selected */ sslsock->is_client = cparam->inputs.method & STREAM_CRYPTO_IS_CLIENT; - method_flags = ((cparam->inputs.method >> 1) << 1); + method_flags = cparam->inputs.method & ~STREAM_CRYPTO_IS_CLIENT; method = sslsock->is_client ? SSLv23_client_method() : SSLv23_server_method(); sslsock->ctx = SSL_CTX_new(method); + if (sslsock->ctx == NULL) { + php_error_docref(NULL, E_WARNING, "SSL context creation failure"); + return FAILURE; + } + GET_VER_OPT_LONG("min_proto_version", min_version); GET_VER_OPT_LONG("max_proto_version", max_version); method_flags = php_openssl_get_proto_version_flags(method_flags, min_version, max_version); @@ -1628,11 +1628,6 @@ int php_openssl_setup_crypto(php_stream *stream, ssl_ctx_options = SSL_OP_ALL; #endif - if (sslsock->ctx == NULL) { - php_error_docref(NULL, E_WARNING, "SSL context creation failure"); - return FAILURE; - } - if (GET_VER_OPT("no_ticket") && zend_is_true(val)) { ssl_ctx_options |= SSL_OP_NO_TICKET; } @@ -2293,9 +2288,7 @@ static inline int php_openssl_tcp_sockop_accept(php_stream *stream, php_openssl_ if (xparam->outputs.client && sock->enable_on_connect) { /* remove the client bit */ - if (sock->method & STREAM_CRYPTO_IS_CLIENT) { - sock->method = ((sock->method >> 1) << 1); - } + sock->method &= ~STREAM_CRYPTO_IS_CLIENT; clisockdata->method = sock->method;