1
0
Эх сурвалжийг харах

Bug 2212: winscp.net root certificate is always trusted when checking for updates, even when (corporate managed) Windows certificate store does not trust it

https://winscp.net/tracker/2212

UpdatesRootCA.pem is DigiCert Global Root CA (of winscp.net) exported from Chrome (export from Firefox is identical). The certificate expires on 10 Nov 2031.

Source commit: 2959e517589dc8cafcd94d3c44d165c1443c04f3
Martin Prikryl 2 жил өмнө
parent
commit
b7f119487f

+ 2 - 0
source/core/Configuration.cpp

@@ -250,6 +250,7 @@ void __fastcall TConfiguration::Default()
   FSshHostCAList->Default();
   RefreshPuttySshHostCAList();
   FSshHostCAsFromPuTTY = false;
+  FHttpsCertificateValidation = 0;
   CollectUsage = FDefaultCollectUsage;
 
   FLogging = false;
@@ -389,6 +390,7 @@ UnicodeString __fastcall TConfiguration::PropertyToKey(const UnicodeString & Pro
     KEY(Integer,  ParallelTransferThreshold); \
     KEY(Integer,  KeyVersion); \
     KEY(Bool,     SshHostCAsFromPuTTY); \
+    KEY(Integer,  HttpsCertificateValidation); \
     KEY(Bool,     CollectUsage); \
     KEY(String,   CertificateStorage); \
     KEY(String,   AWSMetadataService); \

+ 2 - 0
source/core/Configuration.h

@@ -125,6 +125,7 @@ private:
   std::unique_ptr<TSshHostCAList> FSshHostCAList;
   std::unique_ptr<TSshHostCAList> FPuttySshHostCAList;
   bool FSshHostCAsFromPuTTY;
+  int FHttpsCertificateValidation;
 
   bool FDisablePasswordStoring;
   bool FForceBanners;
@@ -400,6 +401,7 @@ public:
   __property TSshHostCAList * PuttySshHostCAList = { read = GetPuttySshHostCAList };
   __property TSshHostCAList * ActiveSshHostCAList = { read = GetActiveSshHostCAList };
   __property bool SshHostCAsFromPuTTY = { read = FSshHostCAsFromPuTTY, write = FSshHostCAsFromPuTTY };
+  __property int HttpsCertificateValidation = { read = FHttpsCertificateValidation, write = FHttpsCertificateValidation };
 
   __property UnicodeString TimeFormat = { read = GetTimeFormat };
   __property TStorage Storage  = { read=GetStorage };

+ 31 - 3
source/core/Http.cpp

@@ -226,12 +226,14 @@ int THttp::NeonServerSSLCallback(void * UserData, int Failures, const ne_ssl_cer
   return Http->NeonServerSSLCallbackImpl(Failures, Certificate);
 }
 //---------------------------------------------------------------------------
-int THttp::NeonServerSSLCallbackImpl(int Failures, const ne_ssl_certificate * Certificate)
+enum { hcvNoWindows = 0x01, hcvNoKnown = 0x02 };
+//---------------------------------------------------------------------------
+int THttp::NeonServerSSLCallbackImpl(int Failures, const ne_ssl_certificate * ACertificate)
 {
-  AnsiString AsciiCert = NeonExportCertificate(Certificate);
+  AnsiString AsciiCert = NeonExportCertificate(ACertificate);
 
   UnicodeString WindowsCertificateError;
-  if (Failures != 0)
+  if ((Failures != 0) && FLAGCLEAR(Configuration->HttpsCertificateValidation, hcvNoWindows))
   {
     AppLogFmt(L"TLS failure: %s (%d)", (NeonCertificateFailuresErrorStr(Failures, FHostName), Failures));
     AppLogFmt(L"Hostname: %s, Certificate: %s", (FHostName, AsciiCert, AsciiCert));
@@ -245,6 +247,32 @@ int THttp::NeonServerSSLCallbackImpl(int Failures, const ne_ssl_certificate * Ce
     }
   }
 
+  if ((Failures != 0) && FLAGSET(Failures, NE_SSL_UNTRUSTED) && FLAGCLEAR(Configuration->HttpsCertificateValidation, hcvNoKnown) &&
+      !Certificate.IsEmpty())
+  {
+    const ne_ssl_certificate * RootCertificate = ACertificate;
+    do
+    {
+      const ne_ssl_certificate * Issuer = ne_ssl_cert_signedby(RootCertificate);
+      if (Issuer != NULL)
+      {
+        RootCertificate = Issuer;
+      }
+      else
+      {
+        break;
+      }
+    }
+    while (true);
+
+    UnicodeString RootCert = UnicodeString(NeonExportCertificate(RootCertificate));
+    if (RootCert == Certificate)
+    {
+      Failures &= ~NE_SSL_UNTRUSTED;
+      AppLogFmt(L"Certificate is known (%d)", (Failures));
+    }
+  }
+
   if (Failures != 0)
   {
     FCertificateError = NeonCertificateFailuresErrorStr(Failures, FHostName);

+ 2 - 0
source/core/Http.h

@@ -36,6 +36,7 @@ public:
   __property __int64 ResponseLimit = { read = FResponseLimit, write = FResponseLimit };
   __property THttpDownloadEvent OnDownload = { read = FOnDownload, write = FOnDownload };
   __property THttpErrorEvent OnError = { read = FOnError, write = FOnError };
+  __property UnicodeString Certificate = { read = FCertificate, write = FCertificate };
 
 private:
   UnicodeString FURL;
@@ -50,6 +51,7 @@ private:
   UnicodeString FCertificateError;
   TStrings * FRequestHeaders;
   TStrings * FResponseHeaders;
+  UnicodeString FCertificate;
 
   static int NeonBodyReader(void * UserData, const char * Buf, size_t Len);
   int NeonBodyReaderImpl(const char * Buf, size_t Len);

+ 22 - 0
source/resource/UpdatesRootCA.pem

@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIDrzCCApegAwIBAgIQCDvgVpBCRrGhdWrJWZHHSjANBgkqhkiG9w0BAQUFADBh
+MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3
+d3cuZGlnaWNlcnQuY29tMSAwHgYDVQQDExdEaWdpQ2VydCBHbG9iYWwgUm9vdCBD
+QTAeFw0wNjExMTAwMDAwMDBaFw0zMTExMTAwMDAwMDBaMGExCzAJBgNVBAYTAlVT
+MRUwEwYDVQQKEwxEaWdpQ2VydCBJbmMxGTAXBgNVBAsTEHd3dy5kaWdpY2VydC5j
+b20xIDAeBgNVBAMTF0RpZ2lDZXJ0IEdsb2JhbCBSb290IENBMIIBIjANBgkqhkiG
+9w0BAQEFAAOCAQ8AMIIBCgKCAQEA4jvhEXLeqKTTo1eqUKKPC3eQyaKl7hLOllsB
+CSDMAZOnTjC3U/dDxGkAV53ijSLdhwZAAIEJzs4bg7/fzTtxRuLWZscFs3YnFo97
+nh6Vfe63SKMI2tavegw5BmV/Sl0fvBf4q77uKNd0f3p4mVmFaG5cIzJLv07A6Fpt
+43C/dxC//AH2hdmoRBBYMql1GNXRor5H4idq9Joz+EkIYIvUX7Q6hL+hqkpMfT7P
+T19sdl6gSzeRntwi5m3OFBqOasv+zbMUZBfHWymeMr/y7vrTC0LUq7dBMtoM1O/4
+gdW7jVg/tRvoSSiicNoxBN33shbyTApOB6jtSj1etX+jkMOvJwIDAQABo2MwYTAO
+BgNVHQ8BAf8EBAMCAYYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUA95QNVbR
+TLtm8KPiGxvDl7I90VUwHwYDVR0jBBgwFoAUA95QNVbRTLtm8KPiGxvDl7I90VUw
+DQYJKoZIhvcNAQEFBQADggEBAMucN6pIExIK+t1EnE9SsPTfrgT1eXkIoyQY/Esr
+hMAtudXH/vTBH1jLuG2cenTnmCmrEbXjcKChzUyImZOMkXDiqw8cvpOp/2PV5Adg
+06O/nVsJ8dWO41P0jmP6P6fbtGbfYmbW0W5BjfIttep3Sp+dWOIrWcBAI+0tKIJF
+PnlUkiaY4IBIqDfv8NZ5YBberOgOzW6sRBc4L0na4UU+Krk2U886UAb3LujEV0ls
+YSEY1QSteDwsOoBrp+uvFRTp2InBuThs4pFsiv9kuXclVzDAGySj4dzp30d8tbQk
+CAUw7C29C79Fv1C5qfPrmAESrciIxpg0X40KPMbp1ZWVbd4=
+-----END CERTIFICATE-----

+ 11 - 0
source/windows/Setup.cpp

@@ -890,6 +890,16 @@ THttp * __fastcall CreateHttp()
   return CreateHttp(WinConfiguration->Updates);
 }
 //---------------------------------------------------------------------------
+UnicodeString GetUpdatesCertificate()
+{
+  UnicodeString Result = ReadResource(L"UPDATES_ROOT_CA");
+  Result = ReplaceStr(Result, L"-----BEGIN CERTIFICATE-----", EmptyStr);
+  Result = ReplaceStr(Result, L"-----END CERTIFICATE-----", EmptyStr);
+  Result = ReplaceStr(Result, L"\n", EmptyStr);
+  Result = ReplaceStr(Result, L"\r", EmptyStr);
+  return Result;
+}
+//---------------------------------------------------------------------------
 static bool __fastcall DoQueryUpdates(TUpdatesConfiguration & Updates, bool CollectUsage)
 {
   bool Complete = false;
@@ -931,6 +941,7 @@ static bool __fastcall DoQueryUpdates(TUpdatesConfiguration & Updates, bool Coll
     CheckForUpdatesHTTP->URL = URL;
     // sanity check
     CheckForUpdatesHTTP->ResponseLimit = BasicHttpResponseLimit;
+    CheckForUpdatesHTTP->Certificate = GetUpdatesCertificate();
     try
     {
       if (CollectUsage)

+ 1 - 0
source/windows/Windows.rc

@@ -9,3 +9,4 @@ Z_3_WORKSPACE ICON "..\\resource\\Workspace.ico"
 
 LICENSE RCDATA "License.txt"
 LICENSE_EXPAT RCDATA "LicenseExpat.txt"
+UPDATES_ROOT_CA RCDATA "UpdatesRootCA.pem"