Browse Source

Issue 2299 – Better error message when server is using incompatible TLS protocol version

https://winscp.net/tracker/2299

Source commit: 03997d32ab26920b5c4c3fb934e1595db6fe8d38
Martin Prikryl 1 year ago
parent
commit
15112f5277

+ 26 - 1
source/core/FtpFileSystem.cpp

@@ -16,9 +16,11 @@
 #include "HelpCore.h"
 #include "Security.h"
 #include "NeonIntf.h"
+#include "SessionInfo.h"
 #include <StrUtils.hpp>
 #include <DateUtils.hpp>
 #include <openssl/x509_vfy.h>
+#include <openssl/err.h>
 #include <limits>
 //---------------------------------------------------------------------------
 #pragma package(smart_init)
@@ -64,6 +66,7 @@ protected:
   virtual wchar_t * LastSysErrorMessage();
   virtual std::wstring GetClientString();
   virtual void SetupSsl(ssl_st * Ssl);
+  virtual std::wstring CustomReason(int Err);
 
 private:
   TFTPFileSystem * FFileSystem;
@@ -167,7 +170,29 @@ std::wstring TFileZillaImpl::GetClientString()
 //---------------------------------------------------------------------------
 void TFileZillaImpl::SetupSsl(ssl_st * Ssl)
 {
-  ::SetupSsl(Ssl, FFileSystem->FTerminal->SessionData->MinTlsVersion, FFileSystem->FTerminal->SessionData->MaxTlsVersion);
+  TSessionData * SessionData = FFileSystem->FTerminal->SessionData;
+  ::SetupSsl(Ssl, SessionData->MinTlsVersion, SessionData->MaxTlsVersion);
+}
+//---------------------------------------------------------------------------
+std::wstring TFileZillaImpl::CustomReason(int Err)
+{
+  std::wstring Result;
+  int Lib = ERR_GET_LIB(Err);
+  int Reason = ERR_GET_REASON(Err);
+  if ((Lib == ERR_LIB_SSL) &&
+      ((Reason == SSL_R_UNSUPPORTED_PROTOCOL) ||
+       (Reason == SSL_R_TLSV1_ALERT_PROTOCOL_VERSION) ||
+       (Reason == SSL_R_WRONG_SSL_VERSION) ||
+       (Reason == SSL_R_WRONG_VERSION_NUMBER)))
+  {
+    TSessionData * SessionData = FFileSystem->FTerminal->SessionData;
+    Result =
+      FMTLOAD(
+        TLS_UNSUPPORTED, (
+          GetTlsVersionName(SessionData->MinTlsVersion), GetTlsVersionName(SessionData->MaxTlsVersion),
+          GetTlsVersionName(tlsMin), GetTlsVersionName(tlsMax))).c_str();
+  }
+  return Result;
 }
 //---------------------------------------------------------------------------
 //---------------------------------------------------------------------------

+ 19 - 0
source/core/SessionData.cpp

@@ -5937,4 +5937,23 @@ int __fastcall DefaultPort(TFSProtocol FSProtocol, TFtps Ftps)
   }
   return Result;
 }
+//---------------------------------------------------------------------------
+UnicodeString GetTlsVersionName(TTlsVersion TlsVersion)
+{
+  switch (TlsVersion)
+  {
+    default:
+      DebugFail();
+    case ssl2:
+    case ssl3:
+    case tls10:
+      return "TLSv1.0";
+    case tls11:
+      return "TLSv1.1";
+    case tls12:
+      return "TLSv1.2";
+    case tls13:
+      return "TLSv1.3";
+  }
+}
 //---------------------------------------------------------------------

+ 1 - 0
source/core/SessionData.h

@@ -832,5 +832,6 @@ TFSProtocol NormalizeFSProtocol(TFSProtocol FSProtocol);
 bool ParseOpensshDirective(const UnicodeString & ALine, UnicodeString & Directive, UnicodeString & Value);
 UnicodeString CutOpensshToken(UnicodeString & S);
 UnicodeString ConvertPathFromOpenssh(const UnicodeString & Path);
+UnicodeString GetTlsVersionName(TTlsVersion TlsVersion);
 //---------------------------------------------------------------------------
 #endif

+ 0 - 19
source/core/SessionInfo.cpp

@@ -1071,25 +1071,6 @@ void __fastcall TSessionLog::AddStartupInfo(bool System)
   }
 }
 //---------------------------------------------------------------------------
-UnicodeString __fastcall TSessionLog::GetTlsVersionName(TTlsVersion TlsVersion)
-{
-  switch (TlsVersion)
-  {
-    default:
-      DebugFail();
-    case ssl2:
-    case ssl3:
-    case tls10:
-      return "TLSv1.0";
-    case tls11:
-      return "TLSv1.1";
-    case tls12:
-      return "TLSv1.2";
-    case tls13:
-      return "TLSv1.3";
-  }
-}
-//---------------------------------------------------------------------------
 UnicodeString __fastcall TSessionLog::LogSensitive(const UnicodeString & Str)
 {
   if (FConfiguration->LogSensitive && !Str.IsEmpty())

+ 0 - 1
source/core/SessionInfo.h

@@ -301,7 +301,6 @@ private:
   void __fastcall DoAddToSelf(TLogLineType aType, const UnicodeString & aLine);
   void __fastcall AddStartupInfo(bool System);
   void __fastcall DoAddStartupInfo(TSessionData * Data);
-  UnicodeString __fastcall GetTlsVersionName(TTlsVersion TlsVersion);
   UnicodeString __fastcall LogSensitive(const UnicodeString & Str);
   static UnicodeString __fastcall GetCmdLineLog(TConfiguration * AConfiguration);
   void __fastcall CheckSize(__int64 Addition);

+ 20 - 6
source/filezilla/AsyncSslSocketLayer.cpp

@@ -816,7 +816,8 @@ int CAsyncSslSocketLayer::InitSSLConnection(bool clientMode,
     return SSL_FAILURE_INITSSL;
   }
 
-  tools->SetupSsl(m_ssl);
+  m_Tools = tools;
+  m_Tools->SetupSsl(m_ssl);
 
   //Init SSL connection
   void *ssl_sessionid = NULL;
@@ -1193,6 +1194,7 @@ void CAsyncSslSocketLayer::apps_ssl_info_callback(const SSL *s, int where, int r
       if (error != SSL_ERROR_WANT_READ && error != SSL_ERROR_WANT_WRITE)
       {
         pLayer->LogSslError(s, str, "%s: error in %s", FZ_LOG_WARNING);
+        pLayer->PrintLastErrorMsg();
         SendFailure = true;
       }
     }
@@ -1837,14 +1839,26 @@ void CAsyncSslSocketLayer::PrintLastErrorMsg()
   int err = ERR_get_error();
   while (err)
   {
-    char *buffer = new char[512];
-    const char *reason = ERR_reason_error_string(err);
-    ERR_error_string(err, buffer);
-    err = ERR_get_error();
     USES_CONVERSION;
+
+    int aerr = err;
+    err = ERR_get_error();
+
+    char *buffer = new char[512];
+    ERR_error_string(aerr, buffer);
     LogSocketMessageRaw(FZ_LOG_PROGRESS, A2T(buffer));
-    LogSocketMessageRaw(FZ_LOG_WARNING, A2T(reason));
     delete [] buffer;
+
+    std::wstring CustomReason = m_Tools->CustomReason(aerr);
+    if (!CustomReason.empty())
+    {
+      LogSocketMessageRaw(FZ_LOG_WARNING, CustomReason.c_str());
+    }
+    else
+    {
+      const char * reason = ERR_reason_error_string(aerr);
+      LogSocketMessageRaw(FZ_LOG_WARNING, A2T(reason));
+    }
   }
 }
 

+ 1 - 0
source/filezilla/AsyncSslSocketLayer.h

@@ -232,6 +232,7 @@ private:
 
   X509 * FCertificate;
   EVP_PKEY * FPrivateKey;
+  CFileZillaTools * m_Tools;
 };
 //---------------------------------------------------------------------------
 #define SSL_INFO 0

+ 1 - 0
source/filezilla/FilezillaTools.h

@@ -13,6 +13,7 @@ public:
   virtual wchar_t * LastSysErrorMessage() = 0;
   virtual std::wstring GetClientString() = 0;
   virtual void SetupSsl(ssl_st * Ssl) = 0;
+  virtual std::wstring CustomReason(int Err) = 0;
 };
 //---------------------------------------------------------------------------
 #endif // FileZillaToolsH

+ 1 - 0
source/resource/TextsCore.h

@@ -291,6 +291,7 @@
 #define S3_ASSUME_ROLE_ERROR    780
 #define S3_ASSUME_ROLE_RESPONSE_ERROR 781
 #define INI_NO_SITES            782
+#define TLS_UNSUPPORTED         783
 
 #define CORE_CONFIRMATION_STRINGS 300
 #define CONFIRM_PROLONG_TIMEOUT3 301

+ 1 - 0
source/resource/TextsCore1.rc

@@ -267,6 +267,7 @@ BEGIN
   S3_ASSUME_ROLE_ERROR, "Error assuming role '%s'."
   S3_ASSUME_ROLE_RESPONSE_ERROR, "Unexpected response to assume role request (%s)."
   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."
 
   CORE_CONFIRMATION_STRINGS, "CORE_CONFIRMATION"
   CONFIRM_PROLONG_TIMEOUT3, "Host is not communicating for %d seconds.\n\nWait for another %0:d seconds?"