Host key and certificate fingerprint verification improvements:

- When verifying a host key using an expected fingerprint, the expected fingerprint can contain only the checksum.
- When verifying a host key or a certificate using an expected fingerprint, the comparison is case-insensitive (with an exception of SHA-256 checksum).
- When verifying a host key using an expected fingerprint, a padding in SHA-256 checksum is not required
- TLS/SSL certificate fingerprints in generated URL use dashes (-) instead of colons (:)
- SHA-256 fingerprints in in generated URL use dashes (-) and underscores (_) instead of pluses (+) and slashes (/)

Source commit: 5d82e86b61ddce7d0b6b282764ca61764acf9b63

dotnet/SessionOptions.cs

@@ -438,10 +438,10 @@ namespace WinSCP
         private Protocol _protocol;
 
         private const string _listPattern = @"{0}(;{0})*";
-        private const string _sshHostKeyPattern = @"((ssh-rsa|ssh-dss|ssh-ed25519|ecdsa-sha2-nistp(256|384|521))( |-))?(\d+ )?(([0-9a-f]{2}(:|-)){15}[0-9a-f]{2}|[0-9a-zA-Z+/]{43}=)";
+        private const string _sshHostKeyPattern = @"((ssh-rsa|ssh-dss|ssh-ed25519|ecdsa-sha2-nistp(256|384|521))( |-))?(\d+ )?(([0-9a-fA-F]{2}(:|-)){15}[0-9a-fA-F]{2}|[0-9a-zA-Z+/\-_]{43}=?)";
         private static readonly Regex _sshHostKeyRegex =
             new Regex(string.Format(CultureInfo.InvariantCulture, _listPattern, _sshHostKeyPattern));
-        private const string _tlsCertificatePattern = @"([0-9a-f]{2}:){19}[0-9a-f]{2}";
+        private const string _tlsCertificatePattern = @"([0-9a-fA-F]{2}[:\-]){19}[0-9a-fA-F]{2}";
         private static readonly Regex _tlsCertificateRegex =
             new Regex(string.Format(CultureInfo.InvariantCulture, _listPattern, _tlsCertificatePattern));
     }

source/core/Common.cpp

@@ -444,6 +444,46 @@ bool IsNumber(const UnicodeString Str)
   return TryStrToInt(Str, Value);
 }
 //---------------------------------------------------------------------------
+UnicodeString Base64ToUrlSafe(const UnicodeString & S)
+{
+  UnicodeString Result = S;
+  while (EndsStr(L"=", Result))
+  {
+    Result.SetLength(Result.Length() - 1);
+  }
+  // See https://en.wikipedia.org/wiki/Base64#Implementations_and_history
+  Result = ReplaceChar(Result, L'+', L'-');
+  Result = ReplaceChar(Result, L'/', L'_');
+  return Result;
+}
+//---------------------------------------------------------------------------
+const wchar_t NormalizedFingerprintSeparator = L'-';
+//---------------------------------------------------------------------------
+UnicodeString MD5ToUrlSafe(const UnicodeString & S)
+{
+  return ReplaceChar(S, L':', NormalizedFingerprintSeparator);
+}
+//---------------------------------------------------------------------------
+bool SameChecksum(const UnicodeString & AChecksum1, const UnicodeString & AChecksum2, bool Base64)
+{
+  UnicodeString Checksum1(AChecksum1);
+  UnicodeString Checksum2(AChecksum2);
+  bool Result;
+  if (Base64)
+  {
+    Checksum1 = Base64ToUrlSafe(Checksum1);
+    Checksum2 = Base64ToUrlSafe(Checksum2);
+    Result = SameStr(Checksum1, Checksum2);
+  }
+  else
+  {
+    Checksum1 = MD5ToUrlSafe(Checksum1);
+    Checksum2 = MD5ToUrlSafe(Checksum2);
+    Result = SameText(Checksum1, Checksum2);
+  }
+  return Result;
+}
+//---------------------------------------------------------------------------
 UnicodeString __fastcall SystemTemporaryDirectory()
 {
   UnicodeString TempDir;
@@ -2709,9 +2749,9 @@ UnicodeString __fastcall DoEncodeUrl(UnicodeString S, const UnicodeString & DoNo
   return S;
 }
 //---------------------------------------------------------------------------
-UnicodeString __fastcall EncodeUrlString(UnicodeString S, const UnicodeString & DoNotEncode)
+UnicodeString __fastcall EncodeUrlString(UnicodeString S)
 {
-  return DoEncodeUrl(S, DoNotEncode);
+  return DoEncodeUrl(S, UnicodeString());
 }
 //---------------------------------------------------------------------------
 UnicodeString __fastcall EncodeUrlPath(UnicodeString S)

source/core/Common.h

@@ -69,6 +69,10 @@ UnicodeString RemoveInteractiveMsgTag(UnicodeString S);
 UnicodeString RemoveEmptyLines(const UnicodeString & S);
 UnicodeString NormalizeString(const UnicodeString & S);
 bool IsNumber(const UnicodeString Str);
+extern const wchar_t NormalizedFingerprintSeparator;
+UnicodeString Base64ToUrlSafe(const UnicodeString & S);
+UnicodeString MD5ToUrlSafe(const UnicodeString & S);
+bool SameChecksum(const UnicodeString & AChecksum1, const UnicodeString & AChecksum2, bool Base64);
 UnicodeString __fastcall SystemTemporaryDirectory();
 UnicodeString __fastcall GetShellFolderPath(int CSIdl);
 UnicodeString __fastcall GetPersonalFolder();
@@ -112,7 +116,7 @@ bool __fastcall IsLetter(wchar_t Ch);
 bool __fastcall IsDigit(wchar_t Ch);
 bool __fastcall IsHex(wchar_t Ch);
 UnicodeString __fastcall DecodeUrlChars(UnicodeString S);
-UnicodeString __fastcall EncodeUrlString(UnicodeString S, const UnicodeString & DoNotEncode = UnicodeString());
+UnicodeString __fastcall EncodeUrlString(UnicodeString S);
 UnicodeString __fastcall EncodeUrlPath(UnicodeString S);
 UnicodeString __fastcall AppendUrlParams(UnicodeString URL, UnicodeString Params);
 UnicodeString __fastcall ExtractFileNameFromUrl(const UnicodeString & Url);

source/core/PuttyIntf.cpp

@@ -744,9 +744,8 @@ bool __fastcall HasGSSAPI(UnicodeString CustomPath)
   return (has > 0);
 }
 //---------------------------------------------------------------------------
-static void __fastcall DoNormalizeFingerprint(UnicodeString & Fingerprint, UnicodeString & KeyType)
+static void __fastcall DoNormalizeFingerprint(UnicodeString & Fingerprint, UnicodeString & KeyName, UnicodeString & KeyType)
 {
-  const wchar_t NormalizedSeparator = L'-';
   const int MaxCount = 10;
   const ssh_keyalg * SignKeys[MaxCount];
   int Count = LENOF(SignKeys);
@@ -760,40 +759,42 @@ static void __fastcall DoNormalizeFingerprint(UnicodeString & Fingerprint, Unico
     UnicodeString Name = UnicodeString(SignKey->ssh_id);
     if (StartsStr(Name + L" ", Fingerprint))
     {
-      int LenStart = Name.Length() + 1;
-      Fingerprint[LenStart] = NormalizedSeparator;
-      int Space = Fingerprint.Pos(L" ");
+      UnicodeString Rest = Fingerprint.SubString(Name.Length() + 2, Fingerprint.Length() - Name.Length() - 1);
+      int Space = Rest.Pos(L" ");
       // If not a number, it's an invalid input,
       // either something completelly wrong, or it can be OpenSSH base64 public key,
       // that got here from TPasteKeyHandler::Paste
-      if (IsNumber(Fingerprint.SubString(LenStart + 1, Space - LenStart - 1)))
+      if (IsNumber(Rest.SubString(1, Space - 1)))
       {
-        Fingerprint.Delete(LenStart + 1, Space - LenStart);
-        // noop for SHA256 fingerprints
-        Fingerprint = ReplaceChar(Fingerprint, L':', NormalizedSeparator);
+        KeyName = Name;
+        Fingerprint = Rest.SubString(Space + 1, Fingerprint.Length() - Space);
+        Fingerprint = Base64ToUrlSafe(Fingerprint);
+        Fingerprint = MD5ToUrlSafe(Fingerprint);
         KeyType = UnicodeString(SignKey->cache_id);
         return;
       }
     }
-    else if (StartsStr(Name + NormalizedSeparator, Fingerprint))
+    else if (StartsStr(Name + NormalizedFingerprintSeparator, Fingerprint))
     {
       KeyType = UnicodeString(SignKey->cache_id);
+      KeyName = Name;
+      Fingerprint.Delete(1, Name.Length() + 1);
       return;
     }
   }
 }
 //---------------------------------------------------------------------------
-UnicodeString __fastcall NormalizeFingerprint(UnicodeString Fingerprint)
+void __fastcall NormalizeFingerprint(UnicodeString & Fingerprint, UnicodeString & KeyName)
 {
-  UnicodeString KeyType; // unused
-  DoNormalizeFingerprint(Fingerprint, KeyType);
-  return Fingerprint;
+  UnicodeString KeyType;
+  DoNormalizeFingerprint(Fingerprint, KeyName, KeyType);
 }
 //---------------------------------------------------------------------------
 UnicodeString __fastcall KeyTypeFromFingerprint(UnicodeString Fingerprint)
 {
   UnicodeString KeyType;
-  DoNormalizeFingerprint(Fingerprint, KeyType);
+  UnicodeString KeyName; // unused
+  DoNormalizeFingerprint(Fingerprint, KeyName, KeyType);
   return KeyType;
 }
 //---------------------------------------------------------------------------

source/core/PuttyTools.h

@@ -25,7 +25,7 @@ bool __fastcall HasGSSAPI(UnicodeString CustomPath);
 void __fastcall AES256EncodeWithMAC(char * Data, size_t Len, const char * Password,
   size_t PasswordLen, const char * Salt);
 //---------------------------------------------------------------------------
-UnicodeString __fastcall NormalizeFingerprint(UnicodeString Fingerprint);
+void __fastcall NormalizeFingerprint(UnicodeString & Fingerprint, UnicodeString & KeyName);
 UnicodeString __fastcall KeyTypeFromFingerprint(UnicodeString Fingerprint);
 //---------------------------------------------------------------------------
 UnicodeString __fastcall GetPuTTYVersion();

source/core/SecureShell.cpp

@@ -2200,11 +2200,69 @@ UnicodeString __fastcall TSecureShell::RetrieveHostKey(UnicodeString Host, int P
   return Result;
 }
 //---------------------------------------------------------------------------
+static bool DoVerifyFingerprint(const UnicodeString & AFingerprintFromUser, const UnicodeString & AFingerprintFromHost, bool Base64)
+{
+  UnicodeString FingerprintFromUser = AFingerprintFromUser;
+  UnicodeString FingerprintFromHost = AFingerprintFromHost;
+  UnicodeString FingerprintFromUserName, FingerprintFromHostName;
+  NormalizeFingerprint(FingerprintFromUser, FingerprintFromUserName);
+  NormalizeFingerprint(FingerprintFromHost, FingerprintFromHostName);
+  bool Result;
+  if (DebugAlwaysFalse(FingerprintFromHostName.IsEmpty()))
+  {
+    Result = false;
+  }
+  else
+  {
+    // In all below three formats, the checksum can be any of these formats:
+    // MD5 (case insensitive):
+    // xx:xx:xx
+    // xx-xx-xx
+    // SHA-256 (case sensitive):
+    // xxxx+xx/xxx=
+    // xxxx+xx/xxx
+    // xxxx-xx_xxx=
+    // xxxx-xx_xxx
+
+    // Full fingerprint format "type bits checksum"
+    if (!FingerprintFromUserName.IsEmpty())
+    {
+      Result =
+        SameText(FingerprintFromUserName, FingerprintFromHostName) &&
+        SameChecksum(FingerprintFromUser, FingerprintFromHost, Base64);
+    }
+    else
+    {
+      // normalized format "type-checksum"
+      UnicodeString NormalizedPrefix = FingerprintFromHost + NormalizedFingerprintSeparator;
+      if (StartsText(NormalizedPrefix, FingerprintFromUser))
+      {
+        FingerprintFromUser.Delete(1, NormalizedPrefix.Length());
+        Result = SameChecksum(FingerprintFromUser, FingerprintFromHost, Base64);
+      }
+      else
+      {
+        // last resort: "checksum" only
+        Result = SameChecksum(FingerprintFromUser, FingerprintFromHost, Base64);
+      }
+    }
+  }
+  return Result;
+}
+//---------------------------------------------------------------------------
+static bool VerifyFingerprint(
+  const UnicodeString & FingerprintFromUser, const UnicodeString & FingerprintFromHostMD5, const UnicodeString & FingerprintFromHostSHA256)
+{
+  return
+    DoVerifyFingerprint(FingerprintFromUser, FingerprintFromHostMD5, false) ||
+    DoVerifyFingerprint(FingerprintFromUser, FingerprintFromHostSHA256, true);
+}
+//---------------------------------------------------------------------------
 struct TPasteKeyHandler
 {
   UnicodeString KeyStr;
-  UnicodeString NormalizedFingerprintMD5;
-  UnicodeString NormalizedFingerprintSHA256;
+  UnicodeString FingerprintMD5;
+  UnicodeString FingerprintSHA256;
   TSessionUI * UI;
 
   void __fastcall Paste(TObject * Sender, unsigned int & Answer);
@@ -2215,10 +2273,7 @@ void __fastcall TPasteKeyHandler::Paste(TObject * /*Sender*/, unsigned int & Ans
   UnicodeString ClipboardText;
   if (TextFromClipboard(ClipboardText, true))
   {
-    UnicodeString NormalizedClipboardFingerprint = NormalizeFingerprint(ClipboardText);
-    // case insensitive comparison, contrary to VerifyHostKey (we should change to insesitive there too)
-    if (SameText(NormalizedClipboardFingerprint, NormalizedFingerprintMD5) ||
-        SameText(NormalizedClipboardFingerprint, NormalizedFingerprintSHA256) ||
+    if (VerifyFingerprint(ClipboardText, FingerprintMD5, FingerprintSHA256) ||
         SameText(ClipboardText, KeyStr))
     {
       Answer = qaYes;
@@ -2271,8 +2326,6 @@ void __fastcall TSecureShell::VerifyHostKey(
   UnicodeString FingerprintMD5 = SignKeyType + L' ' + MD5;
   UnicodeString SHA256 = Buf;
   UnicodeString FingerprintSHA256 = SignKeyType + L' ' + SHA256;
-  UnicodeString NormalizedFingerprintMD5 = NormalizeFingerprint(FingerprintMD5);
-  UnicodeString NormalizedFingerprintSHA256 = NormalizeFingerprint(FingerprintSHA256);
 
   FSessionInfo.HostKeyFingerprintSHA256 = FingerprintSHA256;
   FSessionInfo.HostKeyFingerprintMD5 = FingerprintMD5;
@@ -2291,18 +2344,17 @@ void __fastcall TSecureShell::VerifyHostKey(
     UnicodeString StoredKey = CutToChar(Buf, HostKeyDelimiter, false);
     // skip leading ECDH subtype identification
     int P = StoredKey.Pos(L",");
-    // start from beginning or after the comma, if there's any
+    // Start from beginning or after the comma, if there's any.
+    // If it does not start with 0x, it's probably a fingerprint (stored by TSessionData::CacheHostKey).
     bool Fingerprint = (StoredKey.SubString(P + 1, 2) != L"0x");
-    // it's probably a fingerprint (stored by TSessionData::CacheHostKey)
-    UnicodeString NormalizedExpectedKey;
-    if (Fingerprint)
+    if (!Fingerprint && (StoredKey == KeyStr))
     {
-      NormalizedExpectedKey = NormalizeFingerprint(StoredKey);
+      LogEvent(L"Host key matches cached key");
+      Result = true;
     }
-    if ((!Fingerprint && (StoredKey == KeyStr)) ||
-        (Fingerprint && ((NormalizedExpectedKey == NormalizedFingerprintMD5) || (NormalizedExpectedKey == NormalizedFingerprintSHA256))))
+    else if (Fingerprint && VerifyFingerprint(StoredKey, FingerprintMD5, FingerprintSHA256))
     {
-      LogEvent(L"Host key matches cached key");
+      LogEvent(L"Host key matches cached key fingerprint");
       Result = true;
     }
     else
@@ -2328,7 +2380,6 @@ void __fastcall TSecureShell::VerifyHostKey(
     while (!Result && !Buf.IsEmpty())
     {
       UnicodeString ExpectedKey = CutToChar(Buf, HostKeyDelimiter, false);
-      UnicodeString NormalizedExpectedKey = NormalizeFingerprint(ExpectedKey);
       if (ExpectedKey == L"*")
       {
         UnicodeString Message = LoadStr(ANY_HOSTKEY);
@@ -2336,14 +2387,14 @@ void __fastcall TSecureShell::VerifyHostKey(
         FLog->Add(llException, Message);
         Result = true;
       }
-      else if ((NormalizedExpectedKey == NormalizedFingerprintMD5) || (NormalizedExpectedKey == NormalizedFingerprintSHA256))
+      else if (VerifyFingerprint(ExpectedKey, FingerprintMD5, FingerprintSHA256))
       {
-        LogEvent(L"Host key matches configured key");
+        LogEvent(L"Host key matches configured key fingerprint");
         Result = true;
       }
       else
       {
-        LogEvent(FORMAT(L"Host key does not match configured key %s", (ExpectedKey)));
+        LogEvent(FORMAT(L"Host key does not match configured key fingerprint %s", (ExpectedKey)));
       }
     }
 
@@ -2374,8 +2425,8 @@ void __fastcall TSecureShell::VerifyHostKey(
       ClipboardHandler.Text = FingerprintSHA256 + L"\n" + FingerprintMD5;
       TPasteKeyHandler PasteKeyHandler;
       PasteKeyHandler.KeyStr = KeyStr;
-      PasteKeyHandler.NormalizedFingerprintMD5 = NormalizedFingerprintMD5;
-      PasteKeyHandler.NormalizedFingerprintSHA256 = NormalizedFingerprintSHA256;
+      PasteKeyHandler.FingerprintMD5 = FingerprintMD5;
+      PasteKeyHandler.FingerprintSHA256 = FingerprintSHA256;
       PasteKeyHandler.UI = FUI;
 
       bool Unknown = StoredKeys.IsEmpty();

source/core/SessionData.cpp

@@ -1683,7 +1683,7 @@ void __fastcall TSessionData::CacheHostKeyIfNotCached()
     UnicodeString HostKeyName = PuttyMungeStr(FORMAT(L"%s@%d:%s", (KeyType, PortNumber, HostName)));
     if (!Storage->ValueExists(HostKeyName))
     {
-      // fingerprint is MD5 of host key, so it cannot be translated back to host key,
+      // fingerprint is a checksum of a host key, so it cannot be translated back to host key,
       // so we store fingerprint and TSecureShell::VerifyHostKey was
       // modified to accept also fingerprint
       Storage->WriteString(HostKeyName, HostKey);
@@ -3072,13 +3072,22 @@ UnicodeString __fastcall TSessionData::GenerateSessionUrl(unsigned int Flags)
 
     if (FLAGSET(Flags, sufHostKey) && !HostKey.IsEmpty())
     {
-      // Many SHA-256 fingeprints end with an equal sign and we do not really need it to be encoded, so avoid that.
-      // Also colons in TLS/SSL fingerprint do not really need encoding.
-      UnicodeString S = EncodeUrlString(NormalizeFingerprint(HostKey), L"=:");
+      UnicodeString KeyName;
+      UnicodeString Fingerprint = HostKey;
+      NormalizeFingerprint(Fingerprint, KeyName);
+      UnicodeString S = Fingerprint;
+      if (!KeyName.IsEmpty())
+      {
+        S = KeyName + NormalizedFingerprintSeparator + S;
+      }
+      S = Base64ToUrlSafe(S); // Noop for MD5 (both in SSH host keys and TLS/SSL)
+      S = MD5ToUrlSafe(S); // TLS/SSL fingerprints
+      UnicodeString S2 = EncodeUrlString(S);
+      DebugAssert(S2 == S2); // There should be nothing left for encoding
 
       Url +=
         UnicodeString(UrlParamSeparator) + UrlHostKeyParamName +
-        UnicodeString(UrlParamValueSeparator) + S;
+        UnicodeString(UrlParamValueSeparator) + S2;
     }
 
     if (FLAGSET(Flags, sufRawSettings))

source/core/Terminal.cpp

@@ -7629,7 +7629,7 @@ bool  __fastcall TTerminal::VerifyCertificate(
     if (Storage->ValueExists(SiteKey))
     {
       UnicodeString CachedCertificateData = Storage->ReadString(SiteKey, L"");
-      if (CertificateData == CachedCertificateData)
+      if (SameChecksum(CertificateData, CachedCertificateData, false))
       {
         LogEvent(FORMAT(L"Certificate for \"%s\" matches cached fingerprint and failures", (CertificateSubject)));
         Result = true;
@@ -7655,7 +7655,7 @@ bool  __fastcall TTerminal::VerifyCertificate(
         Log->Add(llException, Message);
         Result = true;
       }
-      else if (ExpectedKey == Fingerprint)
+      else if (SameChecksum(ExpectedKey, Fingerprint, false))
       {
         LogEvent(FORMAT(L"Certificate for \"%s\" matches configured fingerprint", (CertificateSubject)));
         Result = true;

source/resource/TextsCore1.rc

@@ -201,7 +201,7 @@ BEGIN
   REDIRECT_LOOP, "Redirect loop detected."
   INVALID_URL, "Invalid URL \"%s\"."
   PROXY_AUTHENTICATION_FAILED, "Proxy authentication failed."
-  CONFIGURED_KEY_NOT_MATCH, "Host key does not match configured key \"%s\"!"
+  CONFIGURED_KEY_NOT_MATCH, "Host key does not match configured key fingerprint \"%s\"!"
   SFTP_STATUS_OWNER_INVALID, "The name specified can not be assigned as an owner of a file."
   SFTP_STATUS_GROUP_INVALID, "The name specified can not be assigned as the primary group of a file."
   SFTP_STATUS_NO_MATCHING_BYTE_RANGE_LOCK, "The requested operation could not be completed because the specified byte range lock has not been granted."