Skip to content

Commit 7fd4826

Browse files
committed
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.
1 parent 82f6f6d commit 7fd4826

File tree

2 files changed

+7
-45
lines changed

2 files changed

+7
-45
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ PHP NEWS
88
. Fixed bug #81090 (Typed property performance degradation with .= operator).
99
(Nikita)
1010

11+
- OpenSSL:
12+
. Fixed bug #76694 (native Windows cert verification uses CN as sever name).
13+
(cmb)
14+
1115
- Standard:
1216
. Fixed bug #81048 (phpinfo(INFO_VARIABLES) "Array to string conversion").
1317
(cmb)

ext/openssl/xp_ssl.c

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -718,56 +718,14 @@ static int php_openssl_win_cert_verify_callback(X509_STORE_CTX *x509_store_ctx,
718718
LPWSTR server_name = NULL;
719719
BOOL verify_result;
720720

721-
{ /* This looks ridiculous and it is - but we validate the name ourselves using the peer_name
722-
ctx option, so just use the CN from the cert here */
723-
724-
X509_NAME *cert_name;
725-
unsigned char *cert_name_utf8;
726-
int index, cert_name_utf8_len;
727-
DWORD num_wchars;
728-
729-
cert_name = X509_get_subject_name(cert);
730-
index = X509_NAME_get_index_by_NID(cert_name, NID_commonName, -1);
731-
if (index < 0) {
732-
php_error_docref(NULL, E_WARNING, "Unable to locate certificate CN");
733-
CertFreeCertificateChain(cert_chain_ctx);
734-
CertFreeCertificateContext(cert_ctx);
735-
RETURN_CERT_VERIFY_FAILURE(SSL_R_CERTIFICATE_VERIFY_FAILED);
736-
}
737-
738-
cert_name_utf8_len = PHP_X509_NAME_ENTRY_TO_UTF8(cert_name, index, cert_name_utf8);
739-
740-
num_wchars = MultiByteToWideChar(CP_UTF8, 0, (char*)cert_name_utf8, -1, NULL, 0);
741-
if (num_wchars == 0) {
742-
php_error_docref(NULL, E_WARNING, "Unable to convert %s to wide character string", cert_name_utf8);
743-
OPENSSL_free(cert_name_utf8);
744-
CertFreeCertificateChain(cert_chain_ctx);
745-
CertFreeCertificateContext(cert_ctx);
746-
RETURN_CERT_VERIFY_FAILURE(SSL_R_CERTIFICATE_VERIFY_FAILED);
747-
}
748-
749-
server_name = emalloc((num_wchars * sizeof(WCHAR)) + sizeof(WCHAR));
750-
751-
num_wchars = MultiByteToWideChar(CP_UTF8, 0, (char*)cert_name_utf8, -1, server_name, num_wchars);
752-
if (num_wchars == 0) {
753-
php_error_docref(NULL, E_WARNING, "Unable to convert %s to wide character string", cert_name_utf8);
754-
efree(server_name);
755-
OPENSSL_free(cert_name_utf8);
756-
CertFreeCertificateChain(cert_chain_ctx);
757-
CertFreeCertificateContext(cert_ctx);
758-
RETURN_CERT_VERIFY_FAILURE(SSL_R_CERTIFICATE_VERIFY_FAILED);
759-
}
760-
761-
OPENSSL_free(cert_name_utf8);
762-
}
763-
764721
ssl_policy_params.dwAuthType = (sslsock->is_client) ? AUTHTYPE_SERVER : AUTHTYPE_CLIENT;
765-
ssl_policy_params.pwszServerName = server_name;
722+
/* we validate the name ourselves using the peer_name
723+
ctx option, so no need to use a server name here */
724+
ssl_policy_params.pwszServerName = NULL;
766725
chain_policy_params.pvExtraPolicyPara = &ssl_policy_params;
767726

768727
verify_result = CertVerifyCertificateChainPolicy(CERT_CHAIN_POLICY_SSL, cert_chain_ctx, &chain_policy_params, &chain_policy_status);
769728

770-
efree(server_name);
771729
CertFreeCertificateChain(cert_chain_ctx);
772730
CertFreeCertificateContext(cert_ctx);
773731

0 commit comments

Comments
 (0)