Bug fix: Failure when trying to use a file not containing a private key as a client certificate file with WebDAV

Source commit: e0f0381f09526e14ec4f9347e976d6bf30504221

source/core/Common.cpp

@@ -3551,143 +3551,155 @@ void __fastcall ParseCertificate(const UnicodeString & Path,
   Certificate = NULL;
   PrivateKey = NULL;
   WrongPassphrase = false;
-  bool HasPassphrase = !Passphrase.IsEmpty();
-
-  FILE * File;
-
-  // Inspired by neon's ne_ssl_clicert_read
-  File = OpenCertificate(Path);
-  // openssl pkcs12 -inkey cert.pem -in cert.crt -export -out cert.pfx
-  // Binary file
-  PKCS12 * Pkcs12 = d2i_PKCS12_fp(File, NULL);
-  fclose(File);
-
-  if (Pkcs12 != NULL)
-  {
-    UTF8String PassphraseUtf(Passphrase);
-
-    bool Result =
-      (PKCS12_parse(Pkcs12, PassphraseUtf.c_str(), &PrivateKey, &Certificate, NULL) == 1);
-    PKCS12_free(Pkcs12);
-
-    if (!Result)
-    {
-      ThrowTlsCertificateErrorIgnorePassphraseErrors(Path, HasPassphrase);
-      WrongPassphrase = true;
-    }
-  }
-  else
+  bool Discard = false;
+  try
   {
-    ERR_clear_error();
-
-    TPemPasswordCallbackData CallbackUserData;
-    // PemPasswordCallback never writes to the .Passphrase
-    CallbackUserData.Passphrase = const_cast<UnicodeString *>(&Passphrase);
-
-    File = OpenCertificate(Path);
-    // Encrypted:
-    // openssl req -x509 -newkey rsa:2048 -keyout cert.pem -out cert.crt
-    // -----BEGIN ENCRYPTED PRIVATE KEY-----
-    // ...
-    // -----END ENCRYPTED PRIVATE KEY-----
-
-    // Not encrypted (add -nodes):
-    // -----BEGIN PRIVATE KEY-----
-    // ...
-    // -----END PRIVATE KEY-----
-    // Or (openssl genrsa -out client.key 1024   # used for certificate signing request)
-    // -----BEGIN RSA PRIVATE KEY-----
-    // ...
-    // -----END RSA PRIVATE KEY-----
-    PrivateKey = PEM_read_PrivateKey(File, NULL, PemPasswordCallback, &CallbackUserData);
-    fclose(File);
-
     try
     {
-      if (PrivateKey == NULL)
-      {
-        ThrowTlsCertificateErrorIgnorePassphraseErrors(Path, HasPassphrase);
-        WrongPassphrase = true;
-      }
+      bool HasPassphrase = !Passphrase.IsEmpty();
 
+      FILE * File;
+
+      // Inspired by neon's ne_ssl_clicert_read
       File = OpenCertificate(Path);
-      // The file can contain both private and public key
-      // (basically cert.pem and cert.crt appended one to each other)
-      // -----BEGIN ENCRYPTED PRIVATE KEY-----
-      // ...
-      // -----END ENCRYPTED PRIVATE KEY-----
-      // -----BEGIN CERTIFICATE-----
-      // ...
-      // -----END CERTIFICATE-----
-      Certificate = PEM_read_X509(File, NULL, PemPasswordCallback, &CallbackUserData);
+      // openssl pkcs12 -inkey cert.pem -in cert.crt -export -out cert.pfx
+      // Binary file
+      PKCS12 * Pkcs12 = d2i_PKCS12_fp(File, NULL);
       fclose(File);
 
-      if (Certificate == NULL)
+      if (Pkcs12 != NULL)
       {
-        unsigned long Error = ERR_get_error();
-        // unlikely
-        if (IsTlsPassphraseError(Error, HasPassphrase))
+        UTF8String PassphraseUtf(Passphrase);
+
+        bool Result =
+          (PKCS12_parse(Pkcs12, PassphraseUtf.c_str(), &PrivateKey, &Certificate, NULL) == 1);
+        PKCS12_free(Pkcs12);
+
+        if (!Result)
         {
+          ThrowTlsCertificateErrorIgnorePassphraseErrors(Path, HasPassphrase);
           WrongPassphrase = true;
         }
-        else
+        if (!WrongPassphrase && (PrivateKey == NULL))
         {
-          UnicodeString CertificatePath = ChangeFileExt(Path, L".cer");
-          if (!FileExists(CertificatePath))
-          {
-            CertificatePath = ChangeFileExt(Path, L".crt");
-          }
+          throw Exception(FMTLOAD(NO_PRIVATE_KEY, (Path)));
+        }
+      }
+      else
+      {
+        ERR_clear_error();
+
+        TPemPasswordCallbackData CallbackUserData;
+        // PemPasswordCallback never writes to the .Passphrase
+        CallbackUserData.Passphrase = const_cast<UnicodeString *>(&Passphrase);
+
+        File = OpenCertificate(Path);
+        // Encrypted:
+        // openssl req -x509 -newkey rsa:2048 -keyout cert.pem -out cert.crt
+        // -----BEGIN ENCRYPTED PRIVATE KEY-----
+        // ...
+        // -----END ENCRYPTED PRIVATE KEY-----
+
+        // Not encrypted (add -nodes):
+        // -----BEGIN PRIVATE KEY-----
+        // ...
+        // -----END PRIVATE KEY-----
+        // Or (openssl genrsa -out client.key 1024   # used for certificate signing request)
+        // -----BEGIN RSA PRIVATE KEY-----
+        // ...
+        // -----END RSA PRIVATE KEY-----
+        PrivateKey = PEM_read_PrivateKey(File, NULL, PemPasswordCallback, &CallbackUserData);
+        fclose(File);
+
+        if (PrivateKey == NULL)
+        {
+          ThrowTlsCertificateErrorIgnorePassphraseErrors(Path, HasPassphrase);
+          WrongPassphrase = true;
+        }
 
-          if (!FileExists(CertificatePath))
+        File = OpenCertificate(Path);
+        // The file can contain both private and public key
+        // (basically cert.pem and cert.crt appended one to each other)
+        // -----BEGIN ENCRYPTED PRIVATE KEY-----
+        // ...
+        // -----END ENCRYPTED PRIVATE KEY-----
+        // -----BEGIN CERTIFICATE-----
+        // ...
+        // -----END CERTIFICATE-----
+        Certificate = PEM_read_X509(File, NULL, PemPasswordCallback, &CallbackUserData);
+        fclose(File);
+
+        if (Certificate == NULL)
+        {
+          unsigned long Error = ERR_get_error();
+          // unlikely
+          if (IsTlsPassphraseError(Error, HasPassphrase))
           {
-            throw Exception(MainInstructions(FMTLOAD(CERTIFICATE_PUBLIC_KEY_NOT_FOUND, (Path))));
+            WrongPassphrase = true;
           }
           else
           {
-            File = OpenCertificate(CertificatePath);
-            // -----BEGIN CERTIFICATE-----
-            // ...
-            // -----END CERTIFICATE-----
-            Certificate = PEM_read_X509(File, NULL, PemPasswordCallback, &CallbackUserData);
-            fclose(File);
-
-            if (Certificate == NULL)
+            UnicodeString CertificatePath = ChangeFileExt(Path, L".cer");
+            if (!FileExists(CertificatePath))
             {
-              unsigned long Base64Error = ERR_get_error();
+              CertificatePath = ChangeFileExt(Path, L".crt");
+            }
 
+            if (!FileExists(CertificatePath))
+            {
+              throw Exception(MainInstructions(FMTLOAD(CERTIFICATE_PUBLIC_KEY_NOT_FOUND, (Path))));
+            }
+            else
+            {
               File = OpenCertificate(CertificatePath);
-              // Binary DER-encoded certificate
-              // (as above, with BEGIN/END removed, and decoded from Base64 to binary)
-              // openssl x509 -in cert.crt -out client.der.crt -outform DER
-              Certificate = d2i_X509_fp(File, NULL);
+              // -----BEGIN CERTIFICATE-----
+              // ...
+              // -----END CERTIFICATE-----
+              Certificate = PEM_read_X509(File, NULL, PemPasswordCallback, &CallbackUserData);
               fclose(File);
 
               if (Certificate == NULL)
               {
-                unsigned long DERError = ERR_get_error();
-
-                UnicodeString Message = MainInstructions(FMTLOAD(CERTIFICATE_READ_ERROR, (CertificatePath)));
-                UnicodeString MoreMessages =
-                  FORMAT(L"Base64: %s\nDER: %s", (GetTlsErrorStr(Base64Error), GetTlsErrorStr(DERError)));
-                throw ExtException(Message, MoreMessages);
+                unsigned long Base64Error = ERR_get_error();
+
+                File = OpenCertificate(CertificatePath);
+                // Binary DER-encoded certificate
+                // (as above, with BEGIN/END removed, and decoded from Base64 to binary)
+                // openssl x509 -in cert.crt -out client.der.crt -outform DER
+                Certificate = d2i_X509_fp(File, NULL);
+                fclose(File);
+
+                if (Certificate == NULL)
+                {
+                  unsigned long DERError = ERR_get_error();
+
+                  UnicodeString Message = MainInstructions(FMTLOAD(CERTIFICATE_READ_ERROR, (CertificatePath)));
+                  UnicodeString MoreMessages =
+                    FORMAT(L"Base64: %s\nDER: %s", (GetTlsErrorStr(Base64Error), GetTlsErrorStr(DERError)));
+                  throw ExtException(Message, MoreMessages);
+                }
               }
             }
           }
         }
       }
     }
-    __finally
+    catch (...)
+    {
+      Discard = true;
+      throw;
+    }
+  }
+  __finally
+  {
+    if (Discard || WrongPassphrase)
     {
-      // We loaded private key, but failed to load certificate, discard the certificate
-      // (either exception was thrown or WrongPassphrase)
-      if ((PrivateKey != NULL) && (Certificate == NULL))
+      if (PrivateKey != NULL)
       {
         EVP_PKEY_free(PrivateKey);
         PrivateKey = NULL;
       }
-      // Certificate was verified, but passphrase was wrong when loading private key,
-      // so discard the certificate
-      else if ((Certificate != NULL) && (PrivateKey == NULL))
+      if (Certificate != NULL)
       {
         X509_free(Certificate);
         Certificate = NULL;

source/resource/TextsCore.h

@@ -293,6 +293,7 @@
 #define INI_NO_SITES            782
 #define TLS_UNSUPPORTED         783
 #define OPENSSL_INIT_ERROR      784
+#define NO_PRIVATE_KEY          785
 
 #define CORE_CONFIRMATION_STRINGS 300
 #define CONFIRM_PROLONG_TIMEOUT3 301

source/resource/TextsCore1.rc

@@ -269,6 +269,7 @@ BEGIN
   INI_NO_SITES, "No sites found in \"%s\"."
   TLS_UNSUPPORTED, "The server is using unsupported protocol. Your WinSCP session is configured to use %s through %s. It can be configured to use %s through %s. Though, avoid using old insecure protocols, whenever possible."
   OPENSSL_INIT_ERROR, "OpenSSL initialization failed"
+  NO_PRIVATE_KEY, "No private key found in %s file."
 
   CORE_CONFIRMATION_STRINGS, "CORE_CONFIRMATION"
   CONFIRM_PROLONG_TIMEOUT3, "Host is not communicating for %d seconds.\n\nWait for another %0:d seconds?"