From 7fd48264de5c828d0898d48875fc6c5a6f292386 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 27 May 2021 12:03:19 +0200 Subject: [PATCH] Fix #76694: native Windows cert verification uses CN as sever name This is not guaranteed to work, since the actual server name may only be given as SAN. Since we're doing the peer verification later anyway (using the respective context options as appropriate), there is no need to even supply a server name when verifying against the Windows cert store. Closes GH-7060. --- NEWS | 4 ++++ ext/openssl/xp_ssl.c | 48 +++----------------------------------------- 2 files changed, 7 insertions(+), 45 deletions(-) diff --git a/NEWS b/NEWS index 75cf470375..5a46ee4664 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,10 @@ PHP NEWS . Fixed bug #81090 (Typed property performance degradation with .= operator). (Nikita) +- OpenSSL: + . Fixed bug #76694 (native Windows cert verification uses CN as sever name). + (cmb) + - Standard: . Fixed bug #81048 (phpinfo(INFO_VARIABLES) "Array to string conversion"). (cmb) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index a390dcb724..9710e44a07 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -718,56 +718,14 @@ static int php_openssl_win_cert_verify_callback(X509_STORE_CTX *x509_store_ctx, LPWSTR server_name = NULL; BOOL verify_result; - { /* This looks ridiculous and it is - but we validate the name ourselves using the peer_name - ctx option, so just use the CN from the cert here */ - - X509_NAME *cert_name; - unsigned char *cert_name_utf8; - int index, cert_name_utf8_len; - DWORD num_wchars; - - cert_name = X509_get_subject_name(cert); - index = X509_NAME_get_index_by_NID(cert_name, NID_commonName, -1); - if (index < 0) { - php_error_docref(NULL, E_WARNING, "Unable to locate certificate CN"); - CertFreeCertificateChain(cert_chain_ctx); - CertFreeCertificateContext(cert_ctx); - RETURN_CERT_VERIFY_FAILURE(SSL_R_CERTIFICATE_VERIFY_FAILED); - } - - cert_name_utf8_len = PHP_X509_NAME_ENTRY_TO_UTF8(cert_name, index, cert_name_utf8); - - num_wchars = MultiByteToWideChar(CP_UTF8, 0, (char*)cert_name_utf8, -1, NULL, 0); - if (num_wchars == 0) { - php_error_docref(NULL, E_WARNING, "Unable to convert %s to wide character string", cert_name_utf8); - OPENSSL_free(cert_name_utf8); - CertFreeCertificateChain(cert_chain_ctx); - CertFreeCertificateContext(cert_ctx); - RETURN_CERT_VERIFY_FAILURE(SSL_R_CERTIFICATE_VERIFY_FAILED); - } - - server_name = emalloc((num_wchars * sizeof(WCHAR)) + sizeof(WCHAR)); - - num_wchars = MultiByteToWideChar(CP_UTF8, 0, (char*)cert_name_utf8, -1, server_name, num_wchars); - if (num_wchars == 0) { - php_error_docref(NULL, E_WARNING, "Unable to convert %s to wide character string", cert_name_utf8); - efree(server_name); - OPENSSL_free(cert_name_utf8); - CertFreeCertificateChain(cert_chain_ctx); - CertFreeCertificateContext(cert_ctx); - RETURN_CERT_VERIFY_FAILURE(SSL_R_CERTIFICATE_VERIFY_FAILED); - } - - OPENSSL_free(cert_name_utf8); - } - ssl_policy_params.dwAuthType = (sslsock->is_client) ? AUTHTYPE_SERVER : AUTHTYPE_CLIENT; - ssl_policy_params.pwszServerName = server_name; + /* we validate the name ourselves using the peer_name + ctx option, so no need to use a server name here */ + ssl_policy_params.pwszServerName = NULL; chain_policy_params.pvExtraPolicyPara = &ssl_policy_params; verify_result = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, cert_chain_ctx, &chain_policy_params, &chain_policy_status); - efree(server_name); CertFreeCertificateChain(cert_chain_ctx); CertFreeCertificateContext(cert_ctx);