CONC-527 "SEC_E_ALGORITHM_MISMATCH" connecting Windows client to Ubuntu

The bug happens only when connecting with SSL with client certificates.

Apparently if client certificates are used in TLS handshake,
private keys for cert should be loaded into named persistent
container.This is because AcquireCredentialsHandle is done partically
out-of-process in lsass.exe, and lsass wants to read private keys from disk

See discussion in dotnet/runtime#23749

Schannel has legacy behavior for ephemeral keys, not involving lsass,
and this is why it worked for us so far, however there are limitations.

It appears to only use rsa_sha1 for signature verification, and newer
OpenSSL no longer allows SHA1 for it, and this ends up in
"algorithm mismatch" message from schannel.

The above is just my understanding of how it works, because there is no
real documentation, the conclusion is based on discussion in
dotnet/runtime#23749

The fix:
So storing the key in persistent named container evidently fixes it,
and this is what is done in this patch. Care is takes to destroy
key container after key is no longer needed, to
avoid filling  %AppData%\Roaming\Microsoft\Crypto\RSA with tiny encrypted
key files. Thus the "persistency window" of the key in container on disk
is only for duration of AcquireCredentialsHandle

Author: vaintroub

libmariadb/secure/schannel.c

@@ -206,13 +206,14 @@ void ma_tls_end()
 }
 
 /* {{{ static int ma_tls_set_client_certs(MARIADB_TLS *ctls) */
-static int ma_tls_set_client_certs(MARIADB_TLS *ctls,const CERT_CONTEXT **cert_ctx)
+static int ma_tls_set_client_certs(MARIADB_TLS *ctls, client_cert_handle *cert_handle)
 {
   MYSQL *mysql= ctls->pvio->mysql;
   char *certfile= mysql->options.ssl_cert,
        *keyfile= mysql->options.ssl_key;
   MARIADB_PVIO *pvio= ctls->pvio;
   char errmsg[256];
+  SECURITY_STATUS status;
 
   if (!certfile && keyfile)
     certfile= keyfile;
@@ -222,8 +223,8 @@ static int ma_tls_set_client_certs(MARIADB_TLS *ctls,const CERT_CONTEXT **cert_c
   if (!certfile)
     return 0;
 
-  *cert_ctx = schannel_create_cert_context(certfile, keyfile, errmsg, sizeof(errmsg));
-  if (!*cert_ctx)
+  status = schannel_create_cert_context(certfile, keyfile, cert_handle, errmsg, sizeof(errmsg));
+  if (status)
   {
     pvio->set_error(pvio->mysql, CR_SSL_CONNECTION_ERROR, SQLSTATE_UNKNOWN, 0, errmsg);
     return 1;
@@ -372,7 +373,7 @@ my_bool ma_tls_connect(MARIADB_TLS *ctls)
   size_t i;
   DWORD protocol = 0;
   int verify_certs;
-  const CERT_CONTEXT* cert_context = NULL;
+  client_cert_handle cert_handle= {0};
 
   if (!ctls)
     return 1;
@@ -429,16 +430,18 @@ my_bool ma_tls_connect(MARIADB_TLS *ctls)
     Cred.grbitEnabledProtocols = SP_PROT_TLS1_0_CLIENT | SP_PROT_TLS1_1_CLIENT | SP_PROT_TLS1_2_CLIENT;
 
 
-  if (ma_tls_set_client_certs(ctls, &cert_context))
+  if (ma_tls_set_client_certs(ctls, &cert_handle))
     goto end;
 
-  if (cert_context)
+  if (cert_handle.cert)
   {
     Cred.cCreds = 1;
-    Cred.paCred = &cert_context;
+    Cred.paCred = &cert_handle.cert;
   }
   sRet= AcquireCredentialsHandleA(NULL, UNISP_NAME_A, SECPKG_CRED_OUTBOUND,
                                        NULL, &Cred, NULL, NULL, &sctx->CredHdl, NULL);
+  /* We do not need to keep certificates after this point */
+  schannel_free_cert_context(&cert_handle);
   if (sRet)
   {
     ma_schannel_set_sec_error(pvio, sRet);
@@ -459,8 +462,8 @@ my_bool ma_tls_connect(MARIADB_TLS *ctls)
   rc = 0;
 
 end:
-  if (cert_context)
-    schannel_free_cert_context(cert_context);
+  if (cert_handle.cert)
+    schannel_free_cert_context(&cert_handle);
   return rc;
 }
 

libmariadb/secure/schannel_certs.c

@@ -636,16 +636,29 @@ SECURITY_STATUS schannel_verify_server_certificate(
   return status;
 }
 
+/*
+  Generate a random key container name.
+  Used to store private key in Windows key store.
+*/
+#define KEY_CONTAINER_NAME_PREFIX L"MariaDB-Connector-C-"
+static void generate_key_container_name(wchar_t* key_container_name, size_t key_container_name_len)
+{
+   LARGE_INTEGER now;
+   QueryPerformanceCounter(&now);
+   swprintf_s(key_container_name, key_container_name_len,
+              L"MariaDB-Connector-C-%u-%u-%lld",GetCurrentProcessId(),GetCurrentThreadId(),now.QuadPart);
+}
 
 /* Attach private key (in PEM format) to client certificate */
-static SECURITY_STATUS load_private_key(CERT_CONTEXT* cert, char* private_key_str, size_t len, char* errmsg, size_t errmsg_len)
+static SECURITY_STATUS load_private_key(client_cert_handle *cert_handle, char *private_key_str,
+                                        size_t len, char *errmsg,
+                                        size_t errmsg_len)
 {
   DWORD derlen = (DWORD)len;
   BYTE* derbuf = NULL;
   DWORD keyblob_len = 0;
   BYTE* keyblob = NULL;
-  HCRYPTPROV hProv = 0;
-  HCRYPTKEY hKey = 0;
+
   CERT_KEY_CONTEXT cert_key_context = { 0 };
   PCRYPT_PRIVATE_KEY_INFO  pki = NULL;
   DWORD pki_len = 0;
@@ -695,24 +708,30 @@ static SECURITY_STATUS load_private_key(CERT_CONTEXT* cert, char* private_key_st
   {
     FAIL("Failed to parse private key");
   }
+  generate_key_container_name(cert_handle->key_container_name, sizeof(cert_handle->key_container_name) / sizeof(wchar_t));
 
-  if (!CryptAcquireContext(&hProv, NULL, MS_ENHANCED_PROV, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
+  if (!CryptAcquireContextW(&cert_handle->prov,
+                            cert_handle->key_container_name, MS_ENHANCED_PROV_W,
+                            PROV_RSA_FULL, CRYPT_NEWKEYSET))
   {
     FAIL("CryptAcquireContext failed");
   }
 
-  if (!CryptImportKey(hProv, keyblob, keyblob_len, 0, 0, (HCRYPTKEY*)&hKey))
+  if (!CryptImportKey(cert_handle->prov, keyblob, keyblob_len, 0, 0,
+                      &cert_handle->key))
   {
     FAIL("CryptImportKey failed");
   }
-  cert_key_context.hCryptProv = hProv;
-  cert_key_context.dwKeySpec = AT_KEYEXCHANGE;
-  cert_key_context.cbSize = sizeof(cert_key_context);
-
-  /* assign private key to certificate context */
-  if (!CertSetCertificateContextProperty(cert, CERT_KEY_CONTEXT_PROP_ID,
-                                         CERT_STORE_NO_CRYPT_RELEASE_FLAG,
-                                         &cert_key_context))
+  // Link the private key to the certificate
+  CRYPT_KEY_PROV_INFO keyProvInfo= {0};
+  keyProvInfo.pwszContainerName= cert_handle->key_container_name;
+  keyProvInfo.pwszProvName= NULL;
+  keyProvInfo.dwProvType= PROV_RSA_FULL;
+  keyProvInfo.dwFlags= 0;
+  keyProvInfo.cProvParam= 0;
+  keyProvInfo.rgProvParam= NULL;
+  keyProvInfo.dwKeySpec= AT_KEYEXCHANGE;
+  if (!CertSetCertificateContextProperty(cert_handle->cert, CERT_KEY_PROV_INFO_PROP_ID, 0, &keyProvInfo))
   {
     FAIL("CertSetCertificateContextProperty failed");
   }
@@ -721,23 +740,17 @@ static SECURITY_STATUS load_private_key(CERT_CONTEXT* cert, char* private_key_st
   LocalFree(derbuf);
   LocalFree(keyblob);
   LocalFree(pki);
-  if (hKey)
-    CryptDestroyKey(hKey);
-  if (status)
-  {
-    if (hProv)
-      CryptReleaseContext(hProv, 0);
-  }
   return status;
 }
 
 /*
  Given PEM strings for certificate and private key,
  create a client certificate*
 */
-static CERT_CONTEXT* create_client_certificate_mem(
+static SECURITY_STATUS create_client_certificate_mem(
   char* cert_file_content,
   char* key_file_content,
+  client_cert_handle* cert_handle,
   char* errmsg,
   size_t errmsg_len)
 {
@@ -764,7 +777,7 @@ static CERT_CONTEXT* create_client_certificate_mem(
     CERT_QUERY_OBJECT_BLOB, &cert_blob,
     CERT_QUERY_CONTENT_FLAG_CERT,
     CERT_QUERY_FORMAT_FLAG_ALL, 0, NULL, &actual_content_type,
-    NULL, NULL, NULL, (const void**)&ctx))
+    NULL, NULL, NULL, (const void**)&cert_handle->cert))
   {
     FAIL("Can't parse client certficate");
   }
@@ -777,7 +790,7 @@ static CERT_CONTEXT* create_client_certificate_mem(
     if (begin && end)
     {
       /* Assign key to certificate.*/
-      status = load_private_key(ctx, begin, (end - begin), errmsg, errmsg_len);
+      status = load_private_key(cert_handle, begin, (end - begin), errmsg, errmsg_len);
       goto cleanup;
     }
   }
@@ -789,21 +802,20 @@ static CERT_CONTEXT* create_client_certificate_mem(
   }
 
 cleanup:
-  if (status && ctx)
-  {
-    CertFreeCertificateContext(ctx);
-    ctx = NULL;
-  }
-  return ctx;
+  return status;
 }
 
 
 /* Given cert and key, as PEM file names, create a client certificate */
-CERT_CONTEXT* schannel_create_cert_context(char* cert_file, char* key_file, char* errmsg, size_t errmsg_len)
+SECURITY_STATUS schannel_create_cert_context(char* cert_file, char* key_file,
+    client_cert_handle *cert_handle,
+    char* errmsg, size_t errmsg_len)
 {
-  CERT_CONTEXT* ctx = NULL;
   char* key_file_content = NULL;
   char* cert_file_content = NULL;
+  SECURITY_STATUS status= SEC_E_INTERNAL_ERROR;
+
+  memset(cert_handle, 0, sizeof(client_cert_handle));
 
   cert_file_content = pem_file_to_string(cert_file, errmsg, errmsg_len);
 
@@ -821,34 +833,45 @@ CERT_CONTEXT* schannel_create_cert_context(char* cert_file, char* key_file, char
       goto cleanup;
   }
 
-  ctx = create_client_certificate_mem(cert_file_content, key_file_content, errmsg, errmsg_len);
+  status = create_client_certificate_mem(cert_file_content, key_file_content, cert_handle, errmsg, errmsg_len);
 
 cleanup:
   LocalFree(cert_file_content);
   if (cert_file != key_file)
     LocalFree(key_file_content);
-
-  return ctx;
+  if (status)
+    schannel_free_cert_context(cert_handle);
+  return status;
 }
 
 /*
   Free certificate, and all resources, created by schannel_create_cert_context()
 */
-void schannel_free_cert_context(const CERT_CONTEXT* cert)
+void schannel_free_cert_context(client_cert_handle* cert_handle)
 {
-  /* release provider handle which was acquires in load_private_key() */
-  CERT_KEY_CONTEXT cert_key_context = { 0 };
-  cert_key_context.cbSize = sizeof(cert_key_context);
-  DWORD cbData = sizeof(CERT_KEY_CONTEXT);
-  HCRYPTPROV hProv = 0;
-
-  if (CertGetCertificateContextProperty(cert, CERT_KEY_CONTEXT_PROP_ID, &cert_key_context, &cbData))
+  if (cert_handle->key)
   {
-    hProv = cert_key_context.hCryptProv;
+    CryptDestroyKey(cert_handle->key);
+    cert_handle->key = 0;
   }
-  CertFreeCertificateContext(cert);
-  if (hProv)
+  if (cert_handle->prov)
   {
-    CryptReleaseContext(cert_key_context.hCryptProv, 0);
+    CryptReleaseContext(cert_handle->prov, 0);
+    cert_handle->prov = 0;
+  }
+  if (cert_handle->cert)
+  {
+    CertFreeCertificateContext(cert_handle->cert);
+    cert_handle->cert = 0;
+  }
+  if (cert_handle->key_container_name[0])
+  {
+    if (!CryptAcquireContextW(&cert_handle->prov, cert_handle->key_container_name,
+                              MS_ENHANCED_PROV_W, PROV_RSA_FULL,
+                              CRYPT_DELETEKEYSET))
+    {
+      assert(GetLastError() == NTE_BAD_KEYSET);
+    }
+    cert_handle->key_container_name[0] = 0;
   }
 }

libmariadb/secure/schannel_certs.h

@@ -21,6 +21,15 @@
 #pragma once
 #include <windows.h>
 #include <wincrypt.h>
+typedef struct _client_cert_handle {
+  HCRYPTPROV prov; /* provider */
+  HCRYPTKEY  key;  /* private key */
+  PCCERT_CONTEXT cert; /* client certificate */
+  wchar_t key_container_name[64]; /* key container name */
+} client_cert_handle;
+
+
+void schannel_destroy_client_cert_handle(client_cert_handle *cert);
 
 extern SECURITY_STATUS schannel_create_store(
   const char* CAFile,
@@ -43,11 +52,12 @@ extern SECURITY_STATUS schannel_verify_server_certificate(
 
 extern void schannel_free_store(HCERTSTORE store);
 
-extern CERT_CONTEXT* schannel_create_cert_context(
+extern SECURITY_STATUS schannel_create_cert_context(
   char* cert_file,
   char* key_file,
+  client_cert_handle* cert_handle,
   char* errmsg,
   size_t errmsg_len);
 
-extern void schannel_free_cert_context(const CERT_CONTEXT* cert);
+extern void schannel_free_cert_context(client_cert_handle *cert_handle);