From 0354eed41085baa5ba8777019ebf5e9ef32c001d Mon Sep 17 00:00:00 2001 From: Dan McNulty Date: Fri, 9 Sep 2016 16:56:02 -0500 Subject: schannel: fix wildcard cert name validation on Win CE Fixes a few issues in manual wildcard cert name validation in schannel support code for Win32 CE: - when comparing the wildcard name to the hostname, the wildcard character was removed from the cert name and the hostname was checked to see if it ended with the modified cert name. This allowed cert names like *.com to match the connection hostname. This violates recommendations from RFC 6125. - when the wildcard name in the certificate is longer than the connection hostname, a buffer overread of the connection hostname buffer would occur during the comparison of the certificate name and the connection hostname. --- lib/hostcheck.c | 9 ++++--- lib/vtls/schannel.c | 69 ++++++++++++++++++++++++++++++++--------------------- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/lib/hostcheck.c b/lib/hostcheck.c index f545254f3..cbd089360 100644 --- a/lib/hostcheck.c +++ b/lib/hostcheck.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2015, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2016, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -22,7 +22,10 @@ #include "curl_setup.h" -#if defined(USE_OPENSSL) || defined(USE_AXTLS) || defined(USE_GSKIT) +#if defined(USE_OPENSSL) \ + || defined(USE_AXTLS) \ + || defined(USE_GSKIT) \ + || (defined(USE_SCHANNEL) && defined(_WIN32_WCE)) /* these backends use functions from this file */ #ifdef HAVE_NETINET_IN_H @@ -144,4 +147,4 @@ int Curl_cert_hostcheck(const char *match_pattern, const char *hostname) return res; } -#endif /* OPENSSL or AXTLS or GSKIT */ +#endif /* OPENSSL, AXTLS, GSKIT or schannel+wince */ diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c index a72753e9a..ac8b70556 100644 --- a/lib/vtls/schannel.c +++ b/lib/vtls/schannel.c @@ -59,6 +59,7 @@ #include "x509asn1.h" #include "curl_printf.h" #include "system_win32.h" +#include "hostcheck.h" /* The last #include file should be: */ #include "curl_memory.h" @@ -1602,14 +1603,9 @@ static CURLcode verify_certificate(struct connectdata *conn, int sockindex) if(result == CURLE_OK) { if(conn->ssl_config.verifyhost) { - TCHAR cert_hostname_buff[128]; - xcharp_u hostname; - xcharp_u cert_hostname; + TCHAR cert_hostname_buff[256]; DWORD len; - cert_hostname.const_tchar_ptr = cert_hostname_buff; - hostname.tchar_ptr = Curl_convert_UTF8_to_tchar(conn_hostname); - /* TODO: Fix this for certificates with multiple alternative names. Right now we're only asking for the first preferred alternative name. Instead we'd need to do all via CERT_NAME_SEARCH_ALL_NAMES_FLAG @@ -1620,31 +1616,50 @@ static CURLcode verify_certificate(struct connectdata *conn, int sockindex) */ len = CertGetNameString(pCertContextServer, CERT_NAME_DNS_TYPE, - 0, + CERT_NAME_DISABLE_IE4_UTF8_FLAG, NULL, - cert_hostname.tchar_ptr, - 128); - if(len > 0 && *cert_hostname.tchar_ptr == '*') { - /* this is a wildcard cert. try matching the last len - 1 chars */ - int hostname_len = strlen(conn_hostname); - cert_hostname.tchar_ptr++; - if(_tcsicmp(cert_hostname.const_tchar_ptr, - hostname.const_tchar_ptr + hostname_len - len + 2) != 0) - result = CURLE_PEER_FAILED_VERIFICATION; + cert_hostname_buff, + 256); + if(len > 0) { + const char *cert_hostname; + + /* Comparing the cert name and the connection hostname encoded as UTF-8 + * is acceptable since both values are assumed to use ASCII + * (or some equivalent) encoding + */ + cert_hostname = Curl_convert_tchar_to_UTF8(cert_hostname_buff); + if(!cert_hostname) { + result = CURLE_OUT_OF_MEMORY; + } + else{ + int match_result; + + match_result = Curl_cert_hostcheck(cert_hostname, conn->host.name); + if(match_result == CURL_HOST_MATCH) { + infof(data, + "schannel: connection hostname (%s) validated " + "against certificate name (%s)\n", + conn->host.name, + cert_hostname); + result = CURLE_OK; + } + else{ + failf(data, + "schannel: connection hostname (%s) " + "does not match certificate name (%s)", + conn->host.name, + cert_hostname); + result = CURLE_PEER_FAILED_VERIFICATION; + } + Curl_unicodefree(cert_hostname); + } } - else if(len == 0 || _tcsicmp(hostname.const_tchar_ptr, - cert_hostname.const_tchar_ptr) != 0) { + else { + failf(data, + "schannel: CertGetNameString did not provide any " + "certificate name information"); result = CURLE_PEER_FAILED_VERIFICATION; } - if(result == CURLE_PEER_FAILED_VERIFICATION) { - char *_cert_hostname; - _cert_hostname = Curl_convert_tchar_to_UTF8(cert_hostname.tchar_ptr); - failf(data, "schannel: CertGetNameString() certificate hostname " - "(%s) did not match connection (%s)", - _cert_hostname, conn_hostname); - Curl_unicodefree(_cert_hostname); - } - Curl_unicodefree(hostname.tchar_ptr); } } -- cgit v1.2.3