-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
curl using SChannel fails to validate TLS cert by IP SAN #15149
Comments
I spent a few minutes on this issue and came up with this proof of concept.
BOOL hostname_is_ip(PCRYPT_DATA_BLOB ipblob, LPCSTR hostname)
{
IN_ADDR ipv4addr;
IN6_ADDR ipv6addr;
BOOL result = FALSE;
INT iret = InetPton(AF_INET, hostname, &ipv4addr);
if (iret == 1)
{
ipblob->cbData = sizeof(IN_ADDR);
memcpy(&ipblob->pbData, &ipv4addr, sizeof(IN_ADDR));
result = TRUE;
}
else
{
iret = InetPton(AF_INET6, hostname, &ipv6addr);
if (iret == 1)
{
ipblob->cbData = sizeof(IN6_ADDR);
memcpy(&ipblob->pbData, &ipv6addr, sizeof(IN6_ADDR));
result = TRUE;
}
}
return result;
}
CURLcode Curl_verify_san_ip(PCCERT_CONTEXT ctx, PCRYPT_DATA_BLOB ipblob)
{
PCERT_INFO cert_info = NULL;
PCERT_EXTENSION extension = NULL;
CRYPT_DECODE_PARA decode_para = { sizeof(CRYPT_DECODE_PARA) };
PCERT_ALT_NAME_INFO alt_name_info = NULL;
DWORD alt_name_info_size = 0;
CURLcode result = CURLE_PEER_FAILED_VERIFICATION;
cert_info = ctx->pCertInfo;
if (!cert_info)
{
return result;
}
extension = CertFindExtension(szOID_SUBJECT_ALT_NAME2,
cert_info->cExtension,
cert_info->rgExtension);
if (!extension)
{
return result;
}
if (!CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
szOID_SUBJECT_ALT_NAME2,
extension->Value.pbData,
extension->Value.cbData,
CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG,
&decode_para,
&alt_name_info,
&alt_name_info_size))
{
return result;
}
for (DWORD i = 0; i < alt_name_info->cAltEntry; ++i)
{
PCERT_ALT_NAME_ENTRY entry = &alt_name_info->rgAltEntry[i];
if (entry->dwAltNameChoice == CERT_ALT_NAME_IP_ADDRESS)
{
if (entry->IPAddress.cbData == ipblob->cbData)
{
//
if (!memcmp(entry->IPAddress.pbData, &ipblob->pbData, entry->IPAddress.cbData))
{
result = CURLE_OK;
printf("Verification was successful ...\n");
break;
}
}
}
}
return result;
} You can add the function call to line 497: BYTE ipblob[32] = { 0 };
if (hostname_is_ip(ipblob, conn_hostname))
{
result = Curl_verify_san_ip(pCertContextServer, ipblob);
}
else
{ // rest of the code } curl/lib/vtls/schannel_verify.c Lines 497 to 499 in 5ee43bb
Note: This (pseudo) code isn't even real code, it's unfinished, very crude and requires further development/testing/debugging/analysis. |
|
Hi Daniel, I haven't had time to review the code yet and it's hodgepodge of different formats and whatnot. I'm attaching the entire file, feel free to take a look. I'll submit a PR later (probably after some discussion).
|
@elvinasp Could you rebuild the curl using the following patch in your lab environment? diff --git a/lib/vtls/schannel_verify.c b/lib/vtls/schannel_verify.c
index 9a166c281..fe755f1d0 100644
--- a/lib/vtls/schannel_verify.c
+++ b/lib/vtls/schannel_verify.c
@@ -343,7 +343,9 @@ cleanup:
static DWORD cert_get_name_string(struct Curl_easy *data,
CERT_CONTEXT *cert_context,
LPTSTR host_names,
- DWORD length)
+ DWORD length,
+ PCERT_ALT_NAME_INFO alt_name_info,
+ BOOL Win8_compat)
{
DWORD actual_length = 0;
#if defined(CURL_WINDOWS_UWP)
@@ -351,21 +353,16 @@ static DWORD cert_get_name_string(struct Curl_easy *data,
(void)cert_context;
(void)host_names;
(void)length;
+ (void)alt_name_info;
+ (void)Win8_compat;
#else
BOOL compute_content = FALSE;
- CERT_INFO *cert_info = NULL;
- CERT_EXTENSION *extension = NULL;
- CRYPT_DECODE_PARA decode_para = {0, 0, 0};
- CERT_ALT_NAME_INFO *alt_name_info = NULL;
- DWORD alt_name_info_size = 0;
- BOOL ret_val = FALSE;
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)) {
+ if(Win8_compat) {
/* CertGetNameString will provide the 8-bit character string without
* any decoding */
DWORD name_flags =
@@ -388,43 +385,6 @@ static DWORD cert_get_name_string(struct Curl_easy *data,
*host_names = '\0';
}
- if(!cert_context) {
- failf(data, "schannel: Null certificate context.");
- return actual_length;
- }
-
- cert_info = cert_context->pCertInfo;
- if(!cert_info) {
- failf(data, "schannel: Null certificate info.");
- return actual_length;
- }
-
- extension = CertFindExtension(szOID_SUBJECT_ALT_NAME2,
- cert_info->cExtension,
- cert_info->rgExtension);
- if(!extension) {
- failf(data, "schannel: CertFindExtension() returned no extension.");
- return actual_length;
- }
-
- decode_para.cbSize = sizeof(CRYPT_DECODE_PARA);
-
- ret_val =
- CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
- szOID_SUBJECT_ALT_NAME2,
- extension->Value.pbData,
- extension->Value.cbData,
- CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG,
- &decode_para,
- &alt_name_info,
- &alt_name_info_size);
- if(!ret_val) {
- failf(data,
- "schannel: CryptDecodeObjectEx() returned no alternate name "
- "information.");
- return actual_length;
- }
-
current_pos = host_names;
/* Iterate over the alternate names and populate host_names. */
@@ -467,6 +427,83 @@ static DWORD cert_get_name_string(struct Curl_easy *data,
return actual_length;
}
+BOOL hostname_is_ip(PCRYPT_DATA_BLOB ip_blob,
+ LPCSTR hostname)
+{
+ IN_ADDR ia;
+ IN6_ADDR ia6;
+ BOOL result = FALSE;
+
+ INT res = InetPton(AF_INET, hostname, &ia);
+ if(res == 1) {
+ ip_blob->cbData = sizeof(IN_ADDR);
+ memcpy(&ip_blob->pbData, &ia, sizeof(IN_ADDR));
+ result = TRUE;
+ }
+ else {
+ res = InetPton(AF_INET6, hostname, &ia6);
+ if(res == 1) {
+ ip_blob->cbData = sizeof(IN6_ADDR);
+ memcpy(&ip_blob->pbData, &ia6, sizeof(IN6_ADDR));
+ result = TRUE;
+ }
+ }
+ return result;
+}
+
+BOOL get_alt_name_info(struct Curl_easy *data,
+ PCCERT_CONTEXT ctx,
+ PCERT_ALT_NAME_INFO *alt_name_info,
+ LPDWORD alt_name_info_size)
+{
+ BOOL result = FALSE;
+#if defined(CURL_WINDOWS_UWP)
+ (void)data;
+ (void)ctx;
+ (void)alt_name_info;
+ (void)alt_name_info_size;
+#else
+ PCERT_INFO cert_info = NULL;
+ PCERT_EXTENSION extension = NULL;
+ CRYPT_DECODE_PARA decode_para = { sizeof(CRYPT_DECODE_PARA) };
+
+ if(!ctx) {
+ failf(data, "schannel: Null certificate context.");
+ return result;
+ }
+
+ cert_info = ctx->pCertInfo;
+ if(!cert_info) {
+ failf(data, "schannel: Null certificate info.");
+ return result;
+ }
+
+ extension = CertFindExtension(szOID_SUBJECT_ALT_NAME2,
+ cert_info->cExtension,
+ cert_info->rgExtension);
+ if(!extension) {
+ failf(data, "schannel: CertFindExtension() returned no extension.");
+ return result;
+ }
+
+ if(!CryptDecodeObjectEx(X509_ASN_ENCODING | PKCS_7_ASN_ENCODING,
+ szOID_SUBJECT_ALT_NAME2,
+ extension->Value.pbData,
+ extension->Value.cbData,
+ CRYPT_DECODE_ALLOC_FLAG | CRYPT_DECODE_NOCOPY_FLAG,
+ &decode_para,
+ alt_name_info,
+ alt_name_info_size)) {
+ failf(data,
+ "schannel: CryptDecodeObjectEx() returned no alternate name "
+ "information.");
+ return result;
+ }
+ result = TRUE;
+#endif
+ return result;
+}
+
/* Verify the server's hostname */
CURLcode Curl_verify_host(struct Curl_cfilter *cf,
struct Curl_easy *data)
@@ -481,6 +518,12 @@ CURLcode Curl_verify_host(struct Curl_cfilter *cf,
size_t hostlen = strlen(conn_hostname);
DWORD len = 0;
DWORD actual_len = 0;
+ PCERT_ALT_NAME_INFO alt_name_info = NULL;
+ DWORD alt_name_info_size = 0;
+ BYTE ip_blob[32] = { 0 };
+ BOOL Win8_compat;
+ PCRYPT_DATA_BLOB p = (PCRYPT_DATA_BLOB)ip_blob;
+ DWORD i;
sspi_status =
Curl_pSecFn->QueryContextAttributes(&BACKEND->ctxt->ctxt_handle,
@@ -491,97 +534,123 @@ CURLcode Curl_verify_host(struct Curl_cfilter *cf,
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) {
- failf(data,
- "schannel: CertGetNameString() returned no "
- "certificate name information");
- result = CURLE_PEER_FAILED_VERIFICATION;
- goto cleanup;
+ Win8_compat = curlx_verify_windows_version(6, 2, 0, PLATFORM_WINNT,
+ VERSION_GREATER_THAN_EQUAL);
+ if(hostname_is_ip(p, conn_hostname) || !Win8_compat) {
+ if(!get_alt_name_info(data, pCertContextServer,
+ &alt_name_info, &alt_name_info_size)) {
+ goto cleanup;
+ }
}
- /* CertGetNameString guarantees that the returned name will not contain
- * embedded null bytes. This appears to be undocumented behavior.
- */
- cert_hostname_buff = (LPTSTR)malloc(len * sizeof(TCHAR));
- if(!cert_hostname_buff) {
- result = CURLE_OUT_OF_MEMORY;
- goto cleanup;
+ if(p->cbData) {
+ for(i = 0; i < alt_name_info->cAltEntry; ++i) {
+ PCERT_ALT_NAME_ENTRY entry = &alt_name_info->rgAltEntry[i];
+ if(entry->dwAltNameChoice == CERT_ALT_NAME_IP_ADDRESS) {
+ if(entry->IPAddress.cbData == p->cbData) {
+ if(!memcmp(entry->IPAddress.pbData, &p->pbData,
+ entry->IPAddress.cbData)) {
+ result = CURLE_OK;
+ infof(data,
+ "schannel: connection hostname (%s) matched cert's IP address!",
+ conn_hostname);
+ break;
+ }
+ }
+ }
+ }
}
- actual_len = cert_get_name_string(
- data, pCertContextServer, (LPTSTR)cert_hostname_buff, len);
- /* Sanity check */
- if(actual_len != len) {
- failf(data,
- "schannel: CertGetNameString() returned certificate "
- "name information of unexpected size");
- result = CURLE_PEER_FAILED_VERIFICATION;
- goto cleanup;
- }
+ else {
+ /* Determine the size of the string needed for the cert hostname */
+ len = cert_get_name_string(data, pCertContextServer,
+ NULL, 0, alt_name_info, Win8_compat);
+ if(len == 0) {
+ failf(data,
+ "schannel: CertGetNameString() returned no "
+ "certificate name information");
+ goto cleanup;
+ }
- /* 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 &&
- cert_hostname_buff[cert_hostname_buff_index] != TEXT('\0') &&
- result == CURLE_PEER_FAILED_VERIFICATION) {
+ /* CertGetNameString guarantees that the returned name will not contain
+ * embedded null bytes. This appears to be undocumented behavior.
+ */
+ cert_hostname_buff = (LPTSTR)malloc(len * sizeof(TCHAR));
+ if(!cert_hostname_buff) {
+ result = CURLE_OUT_OF_MEMORY;
+ goto cleanup;
+ }
+ actual_len = cert_get_name_string(data, pCertContextServer,
+ (LPTSTR)cert_hostname_buff, len, alt_name_info, Win8_compat);
- char *cert_hostname;
+ /* Sanity check */
+ if(actual_len != len) {
+ failf(data,
+ "schannel: CertGetNameString() returned certificate "
+ "name information of unexpected size");
+ goto cleanup;
+ }
- /* 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_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.
*/
- cert_hostname = curlx_convert_tchar_to_UTF8(
+ while(cert_hostname_buff_index < len &&
+ cert_hostname_buff[cert_hostname_buff_index] != TEXT('\0') &&
+ result == CURLE_PEER_FAILED_VERIFICATION) {
+
+ 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 = curlx_convert_tchar_to_UTF8(
&cert_hostname_buff[cert_hostname_buff_index]);
- if(!cert_hostname) {
- result = CURLE_OUT_OF_MEMORY;
- }
- else {
- if(Curl_cert_hostcheck(cert_hostname, strlen(cert_hostname),
- conn_hostname, hostlen)) {
- infof(data,
- "schannel: connection hostname (%s) validated "
- "against certificate name (%s)",
- conn_hostname, cert_hostname);
- result = CURLE_OK;
+ if(!cert_hostname) {
+ result = CURLE_OUT_OF_MEMORY;
}
else {
- size_t cert_hostname_len;
+ if(Curl_cert_hostcheck(cert_hostname, strlen(cert_hostname),
+ conn_hostname, hostlen)) {
+ infof(data,
+ "schannel: connection hostname (%s) validated "
+ "against certificate name (%s)",
+ conn_hostname, cert_hostname);
+ result = CURLE_OK;
+ }
+ else {
+ size_t cert_hostname_len;
- infof(data,
- "schannel: connection hostname (%s) did not match "
- "against certificate name (%s)",
- conn_hostname, cert_hostname);
+ infof(data,
+ "schannel: connection hostname (%s) did not match "
+ "against certificate name (%s)",
+ conn_hostname, cert_hostname);
- cert_hostname_len =
- _tcslen(&cert_hostname_buff[cert_hostname_buff_index]);
+ cert_hostname_len =
+ _tcslen(&cert_hostname_buff[cert_hostname_buff_index]);
- /* Move on to next cert name */
- cert_hostname_buff_index += cert_hostname_len + 1;
+ /* Move on to next cert name */
+ cert_hostname_buff_index += cert_hostname_len + 1;
- result = CURLE_PEER_FAILED_VERIFICATION;
+ result = CURLE_PEER_FAILED_VERIFICATION;
+ }
+ curlx_unicodefree(cert_hostname);
}
- curlx_unicodefree(cert_hostname);
}
- }
- if(result == CURLE_PEER_FAILED_VERIFICATION) {
- failf(data,
- "schannel: CertGetNameString() failed to match "
- "connection hostname (%s) against server certificate names",
- conn_hostname);
+ if(result == CURLE_PEER_FAILED_VERIFICATION) {
+ failf(data,
+ "schannel: CertGetNameString() failed to match "
+ "connection hostname (%s) against server certificate names",
+ conn_hostname);
+ }
+ else if(result != CURLE_OK)
+ failf(data, "schannel: server certificate name verification failed");
}
- else if(result != CURLE_OK)
- failf(data, "schannel: server certificate name verification failed");
cleanup:
Curl_safefree(cert_hostname_buff);
@@ -592,7 +661,6 @@ cleanup:
return result;
}
-
#ifdef HAS_MANUAL_VERIFY_API
/* Verify the server's certificate and hostname */
CURLcode Curl_verify_certificate(struct Curl_cfilter *cf, |
Discussed in #15139
Originally posted by elvinasp October 3, 2024
Hello,
I was making some networking labs and one of the examples was to connect via curl using various hosntname combinations and seeing how SNI behaves. When I connected to demo server IP address using windows build-in curl, it failed to validate certificate. Linux curl from WSL environment on same machine successfully connects to service via IP and validated TLS cert.
Has anything changed since v7 to v8?
Attaching certs generated with cfssl: certs.zip
Windows curl:
curl 8.7.1 (Windows) libcurl/8.7.1 Schannel zlib/1.3 WinIDN Release-Date: 2024-03-27 Protocols: dict file ftp ftps http https imap imaps ipfs ipns mqtt pop3 pop3s smb smbs smtp smtps telnet tftp Features: alt-svc AsynchDNS HSTS HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM SPNEGO SSL SSPI threadsafe Unicode UnixSockets
Output:
TLS certificate:
Properly lists all SANS:
The text was updated successfully, but these errors were encountered: