schannel: when importing PFX, disable key persistence

By default, the PFXImportCertStore API persists the key in the user's
key store (as though the certificate was being imported for permanent,
ongoing use.)

The documentation specifies that keys that are not to be persisted
should be imported with the flag PKCS12_NO_PERSIST_KEY.
NOTE: this flag is only supported on versions of Windows newer than XP
and Server 2003.

--

This is take 2 of the original fix. It extends the lifetime of the
client certificate store to that of the credential handle. The original
fix which landed in 70d010d and was later reverted in aec8d30 failed to
work properly because it did not do that.

Minor changes were made to the schannel credential context to support
closing the client certificate store handle at the end of an SSL session.

--

Reported-by: ShadowZzj@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/9300
Supersedes https://github.com/curl/curl/pull/9363
Closes https://github.com/curl/curl/pull/9460

diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c
index 2653f5dfc475c7bcc88ad8671cdff9cc912be7c8..134726f91d2b1aa823c5c516ff0202be73a0f1c1 100644 (file)
--- a/lib/vtls/schannel.c
+++ b/lib/vtls/schannel.c
@@ -115,11 +115,6 @@
 #define ARRAYSIZE(A) (sizeof(A)/sizeof((A)[0]))
 #endif
 
-#if defined(CryptStringToBinary) && defined(CRYPT_STRING_HEX)   \
-  && !defined(DISABLE_SCHANNEL_CLIENT_CERT)
-#define HAS_CLIENT_CERT_PATH
-#endif
-
 #ifdef HAS_CLIENT_CERT_PATH
 #ifdef UNICODE
 #define CURL_CERT_STORE_PROV_SYSTEM CERT_STORE_PROV_SYSTEM_W
@@ -186,6 +181,10 @@
 #define ALG_CLASS_DHASH ALG_CLASS_HASH
 #endif
 
+#ifndef PKCS12_NO_PERSIST_KEY
+#define PKCS12_NO_PERSIST_KEY 0x00008000
+#endif
+
 static Curl_recv schannel_recv;
 static Curl_send schannel_send;
 
@@ -486,6 +485,7 @@ schannel_acquire_credential_handle(struct Curl_easy *data,
 
 #ifdef HAS_CLIENT_CERT_PATH
   PCCERT_CONTEXT client_certs[1] = { NULL };
+  HCERTSTORE client_cert_store = NULL;
 #endif
   SECURITY_STATUS sspi_status = SEC_E_OK;
   CURLcode result;
@@ -676,7 +676,13 @@ schannel_acquire_credential_handle(struct Curl_easy *data,
         else
           pszPassword[0] = 0;
 
-        cert_store = PFXImportCertStore(&datablob, pszPassword, 0);
+        if(curlx_verify_windows_version(6, 0, 0, PLATFORM_WINNT,
+                                        VERSION_GREATER_THAN_EQUAL))
+          cert_store = PFXImportCertStore(&datablob, pszPassword,
+                                          PKCS12_NO_PERSIST_KEY);
+        else
+          cert_store = PFXImportCertStore(&datablob, pszPassword, 0);
+
         free(pszPassword);
       }
       if(!blob)
@@ -748,7 +754,7 @@ schannel_acquire_credential_handle(struct Curl_easy *data,
         return CURLE_SSL_CERTPROBLEM;
       }
     }
-    CertCloseStore(cert_store, 0);
+    client_cert_store = cert_store;
   }
 #else
   if(data->set.ssl.primary.clientcert || data->set.ssl.primary.cert_blob) {
@@ -766,12 +772,21 @@ schannel_acquire_credential_handle(struct Curl_easy *data,
 #ifdef HAS_CLIENT_CERT_PATH
     if(client_certs[0])
       CertFreeCertificateContext(client_certs[0]);
+    if(client_cert_store)
+      CertCloseStore(client_cert_store, 0);
 #endif
 
     return CURLE_OUT_OF_MEMORY;
   }
   backend->cred->refcount = 1;
 
+#ifdef HAS_CLIENT_CERT_PATH
+  /* Since we did not persist the key, we need to extend the store's
+   * lifetime until the end of the connection
+   */
+  backend->cred->client_cert_store = client_cert_store;
+#endif
+
   /* Windows 10, 1809 (a.k.a. Windows 10 build 17763) */
   if(curlx_verify_windows_version(10, 0, 17763, PLATFORM_WINNT,
                                   VERSION_GREATER_THAN_EQUAL)) {
@@ -2464,6 +2479,12 @@ static void schannel_session_free(void *ptr)
     if(cred->refcount == 0) {
       s_pSecFn->FreeCredentialsHandle(&cred->cred_handle);
       curlx_unicodefree(cred->sni_hostname);
+#ifdef HAS_CLIENT_CERT_PATH
+      if(cred->client_cert_store) {
+        CertCloseStore(cred->client_cert_store, 0);
+        cred->client_cert_store = NULL;
+      }
+#endif
       Curl_safefree(cred);
     }
   }

diff --git a/lib/vtls/schannel.h b/lib/vtls/schannel.h
index 000d1e7b3052c82184bd42c686eac0b3015c6b33..24d7eff25b9b0ed46257d63668c76698e74680b7 100644 (file)
--- a/lib/vtls/schannel.h
+++ b/lib/vtls/schannel.h
@@ -83,17 +83,23 @@ CURLcode Curl_verify_certificate(struct Curl_easy *data,
 /* structs to expose only in schannel.c and schannel_verify.c */
 #ifdef EXPOSE_SCHANNEL_INTERNAL_STRUCTS
 
+#include <wincrypt.h>
+
 #ifdef __MINGW32__
 #ifdef __MINGW64_VERSION_MAJOR
 #define HAS_MANUAL_VERIFY_API
 #endif
 #else
-#include <wincrypt.h>
 #ifdef CERT_CHAIN_REVOCATION_CHECK_CHAIN
 #define HAS_MANUAL_VERIFY_API
 #endif
 #endif
 
+#if defined(CryptStringToBinary) && defined(CRYPT_STRING_HEX)   \
+  && !defined(DISABLE_SCHANNEL_CLIENT_CERT)
+#define HAS_CLIENT_CERT_PATH
+#endif
+
 #ifndef SCH_CREDENTIALS_VERSION
 
 #define SCH_CREDENTIALS_VERSION  0x00000005
@@ -155,6 +161,9 @@ struct Curl_schannel_cred {
   CredHandle cred_handle;
   TimeStamp time_stamp;
   TCHAR *sni_hostname;
+#ifdef HAS_CLIENT_CERT_PATH
+  HCERTSTORE client_cert_store;
+#endif
   int refcount;
 };