Skip to content

Commit

Permalink
schannel: verify hostname independent of verify cert
Browse files Browse the repository at this point in the history
Prior to this change when CURLOPT_SSL_VERIFYPEER (verifypeer) was off
and CURLOPT_SSL_VERIFYHOST (verifyhost) was on we did not verify the
hostname in schannel code.

This fixes KNOWN_BUG 2.8 "Schannel disable CURLOPT_SSL_VERIFYPEER and
verify hostname". We discussed a fix several years ago in #3285 but it
went stale.

Assisted-by: Daniel Stenberg

Bug: https://curl.haxx.se/mail/lib-2018-10/0113.html
Reported-by: Martin Galvan

Ref: #3285

Fixes #3284
Closes #10056
  • Loading branch information
jay committed Aug 11, 2023
1 parent c705675 commit 889c071
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 22 deletions.
7 changes: 0 additions & 7 deletions docs/KNOWN_BUGS
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ problems may have been fixed or changed somewhat since this was written.
2.4 Secure Transport will not import PKCS#12 client certificates without a password
2.5 Client cert handling with Issuer DN differs between backends
2.7 Client cert (MTLS) issues with Schannel
2.8 Schannel disable CURLOPT_SSL_VERIFYPEER and verify hostname
2.11 Schannel TLS 1.2 handshake bug in old Windows versions
2.12 FTPS with Schannel times out file list operation
2.13 CURLOPT_CERTINFO results in CURLE_OUT_OF_MEMORY with Schannel
Expand Down Expand Up @@ -163,12 +162,6 @@ problems may have been fixed or changed somewhat since this was written.

See https://github.com/curl/curl/issues/3145

2.8 Schannel disable CURLOPT_SSL_VERIFYPEER and verify hostname

This seems to be a limitation in the underlying Schannel API.

https://github.com/curl/curl/issues/3284

2.11 Schannel TLS 1.2 handshake bug in old Windows versions

In old versions of Windows such as 7 and 8.1 the Schannel TLS 1.2 handshake
Expand Down
12 changes: 9 additions & 3 deletions lib/vtls/schannel.c
Original file line number Diff line number Diff line change
Expand Up @@ -810,9 +810,9 @@ schannel_acquire_credential_handle(struct Curl_cfilter *cf,

SCH_CREDENTIALS credentials = { 0 };
TLS_PARAMETERS tls_parameters = { 0 };
CRYPTO_SETTINGS crypto_settings[4] = { 0 };
UNICODE_STRING blocked_ccm_modes[1] = { 0 };
UNICODE_STRING blocked_gcm_modes[1] = { 0 };
CRYPTO_SETTINGS crypto_settings[4] = { { 0 } };
UNICODE_STRING blocked_ccm_modes[1] = { { 0 } };
UNICODE_STRING blocked_gcm_modes[1] = { { 0 } };

int crypto_settings_idx = 0;

Expand Down Expand Up @@ -1634,10 +1634,16 @@ schannel_connect_step2(struct Curl_cfilter *cf, struct Curl_easy *data)

#ifdef HAS_MANUAL_VERIFY_API
if(conn_config->verifypeer && backend->use_manual_cred_validation) {
/* Certificate verification also verifies the hostname if verifyhost */
return Curl_verify_certificate(cf, data);
}
#endif

/* Verify the hostname manually when certificate verification is disabled,
because in that case Schannel won't verify it. */
if(!conn_config->verifypeer && conn_config->verifyhost)
return Curl_verify_host(cf, data);

return CURLE_OK;
}

Expand Down
3 changes: 3 additions & 0 deletions lib/vtls/schannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@

extern const struct Curl_ssl Curl_ssl_schannel;

CURLcode Curl_verify_host(struct Curl_cfilter *cf,
struct Curl_easy *data);

CURLcode Curl_verify_certificate(struct Curl_cfilter *cf,
struct Curl_easy *data);

Expand Down
52 changes: 52 additions & 0 deletions lib/vtls/schannel_int.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,58 @@
#define HAS_CLIENT_CERT_PATH
#endif

#ifndef CRYPT_DECODE_NOCOPY_FLAG
#define CRYPT_DECODE_NOCOPY_FLAG 0x1
#endif

#ifndef CRYPT_DECODE_ALLOC_FLAG
#define CRYPT_DECODE_ALLOC_FLAG 0x8000
#endif

#ifndef CERT_ALT_NAME_DNS_NAME
#define CERT_ALT_NAME_DNS_NAME 3
#endif

#ifndef CERT_ALT_NAME_IP_ADDRESS
#define CERT_ALT_NAME_IP_ADDRESS 8
#endif


#if defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR)
/* Original mingw is missing CERT structs or they're disabled.
Refer to w32api-5.0.2-mingw32-dev\include\wincrypt.h. */

/* !checksrc! disable TYPEDEFSTRUCT 4 */
typedef struct _CERT_OTHER_NAME {
LPSTR pszObjId;
CRYPT_OBJID_BLOB Value;
} CERT_OTHER_NAME, *PCERT_OTHER_NAME;

typedef struct _CERT_ALT_NAME_ENTRY {
DWORD dwAltNameChoice;
union {
PCERT_OTHER_NAME pOtherName;
LPWSTR pwszRfc822Name;
LPWSTR pwszDNSName;
CERT_NAME_BLOB DirectoryName;
LPWSTR pwszURL;
CRYPT_DATA_BLOB IPAddress;
LPSTR pszRegisteredID;
};
} CERT_ALT_NAME_ENTRY, *PCERT_ALT_NAME_ENTRY;

typedef struct _CERT_ALT_NAME_INFO {
DWORD cAltEntry;
PCERT_ALT_NAME_ENTRY rgAltEntry;
} CERT_ALT_NAME_INFO, *PCERT_ALT_NAME_INFO;

typedef struct _CRYPT_DECODE_PARA {
DWORD cbSize;
PFN_CRYPT_ALLOC pfnAlloc;
PFN_CRYPT_FREE pfnFree;
} CRYPT_DECODE_PARA, *PCRYPT_DECODE_PARA;
#endif

#ifndef SCH_CREDENTIALS_VERSION

#define SCH_CREDENTIALS_VERSION 0x00000005
Expand Down
49 changes: 37 additions & 12 deletions lib/vtls/schannel_verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
#include "schannel.h"
#include "schannel_int.h"

#ifdef HAS_MANUAL_VERIFY_API

#include "vtls.h"
#include "vtls_int.h"
#include "sendf.h"
Expand All @@ -56,6 +54,9 @@

#define BACKEND ((struct schannel_ssl_backend_data *)connssl->backend)


#ifdef HAS_MANUAL_VERIFY_API

#define MAX_CAFILE_SIZE 1048576 /* 1 MiB */
#define BEGIN_CERT "-----BEGIN CERTIFICATE-----"
#define END_CERT "\n-----END CERTIFICATE-----"
Expand Down Expand Up @@ -330,6 +331,8 @@ static CURLcode add_certs_file_to_store(HCERTSTORE trust_store,
return result;
}

#endif /* HAS_MANUAL_VERIFY_API */

/*
* Returns the number of characters necessary to populate all the host_names.
* If host_names is not NULL, populate it with all the host names. Each string
Expand All @@ -353,10 +356,10 @@ static DWORD cert_get_name_string(struct Curl_easy *data,
LPTSTR current_pos = NULL;
DWORD i;

#ifdef CERT_NAME_SEARCH_ALL_NAMES_FLAG
/* CERT_NAME_SEARCH_ALL_NAMES_FLAG is available from Windows 8 onwards. */
if(curlx_verify_windows_version(6, 2, 0, PLATFORM_WINNT,
VERSION_GREATER_THAN_EQUAL)) {
#ifdef CERT_NAME_SEARCH_ALL_NAMES_FLAG
/* CertGetNameString will provide the 8-bit character string without
* any decoding */
DWORD name_flags =
Expand All @@ -368,8 +371,8 @@ static DWORD cert_get_name_string(struct Curl_easy *data,
host_names,
length);
return actual_length;
#endif
}
#endif

compute_content = host_names != NULL && length != 0;

Expand Down Expand Up @@ -457,17 +460,34 @@ static DWORD cert_get_name_string(struct Curl_easy *data,
return actual_length;
}

static CURLcode verify_host(struct Curl_easy *data,
CERT_CONTEXT *pCertContextServer,
const char *conn_hostname)
/* Verify the server's hostname */
CURLcode Curl_verify_host(struct Curl_cfilter *cf,
struct Curl_easy *data)
{
struct ssl_connect_data *connssl = cf->ctx;
SECURITY_STATUS sspi_status;
CURLcode result = CURLE_PEER_FAILED_VERIFICATION;
CERT_CONTEXT *pCertContextServer = NULL;
TCHAR *cert_hostname_buff = NULL;
size_t cert_hostname_buff_index = 0;
const char *conn_hostname = connssl->hostname;
size_t hostlen = strlen(conn_hostname);
DWORD len = 0;
DWORD actual_len = 0;

sspi_status =
s_pSecFn->QueryContextAttributes(&BACKEND->ctxt->ctxt_handle,
SECPKG_ATTR_REMOTE_CERT_CONTEXT,
&pCertContextServer);

if((sspi_status != SEC_E_OK) || !pCertContextServer) {
char buffer[STRERROR_LEN];
failf(data, "schannel: Failed to read remote certificate context: %s",
Curl_sspi_strerror(sspi_status, buffer, sizeof(buffer)));
result = CURLE_PEER_FAILED_VERIFICATION;
goto cleanup;
}

/* Determine the size of the string needed for the cert hostname */
len = cert_get_name_string(data, pCertContextServer, NULL, 0);
if(len == 0) {
Expand Down Expand Up @@ -498,10 +518,9 @@ static CURLcode verify_host(struct Curl_easy *data,
goto cleanup;
}

/* If HAVE_CERT_NAME_SEARCH_ALL_NAMES is available, the output
* will contain all DNS names, where each name is null-terminated
* and the last DNS name is double null-terminated. Due to this
* encoding, use the length of the buffer to iterate over all names.
/* cert_hostname_buff contains all DNS names, where each name is
* null-terminated and the last DNS name is double null-terminated. Due to
* this encoding, use the length of the buffer to iterate over all names.
*/
result = CURLE_PEER_FAILED_VERIFICATION;
while(cert_hostname_buff_index < len &&
Expand Down Expand Up @@ -560,9 +579,15 @@ static CURLcode verify_host(struct Curl_easy *data,
cleanup:
Curl_safefree(cert_hostname_buff);

if(pCertContextServer)
CertFreeCertificateContext(pCertContextServer);

return result;
}


#ifdef HAS_MANUAL_VERIFY_API
/* Verify the server's certificate and hostname */
CURLcode Curl_verify_certificate(struct Curl_cfilter *cf,
struct Curl_easy *data)
{
Expand Down Expand Up @@ -721,7 +746,7 @@ CURLcode Curl_verify_certificate(struct Curl_cfilter *cf,

if(result == CURLE_OK) {
if(conn_config->verifyhost) {
result = verify_host(data, pCertContextServer, connssl->hostname);
result = Curl_verify_host(cf, data);
}
}

Expand Down

0 comments on commit 889c071

Please sign in to comment.