Redesigning (and refactoring) host key prompts in anticipation of Bug 2145

Source commit: 178f136f5f65f1352cfa6fe5eb0071d8fefa5f6d

source/core/SecureShell.cpp

@@ -11,6 +11,7 @@
 #include "HelpCore.h"
 #include "CoreMain.h"
 #include <StrUtils.hpp>
+#include <Consts.hpp>
 
 #ifndef AUTO_WINSOCK
 #include <winsock2.h>
@@ -2609,45 +2610,102 @@ void __fastcall TSecureShell::VerifyHostKey(
 
       bool Unknown = StoredKeys.IsEmpty();
 
+      UnicodeString AcceptButton = LoadStr(HOSTKEY_ACCEPT_BUTTON);
+      UnicodeString OnceButton = LoadStr(HOSTKEY_ONCE_BUTTON);
+      UnicodeString CancelButton = Vcl_Consts_SMsgDlgCancel;
+      UnicodeString UpdateButton = LoadStr(UPDATE_KEY_BUTTON);
+      UnicodeString AddButton = LoadStr(ADD_KEY_BUTTON);
       int Answers;
-      int AliasesCount;
-      TQueryButtonAlias Aliases[4];
-      Aliases[0].Button = qaRetry;
-      Aliases[0].Alias = LoadStr(COPY_KEY_BUTTON);
-      Aliases[0].ActionAlias = LoadStr(COPY_KEY_ACTION);
-      Aliases[0].OnSubmit = &ClipboardHandler.Copy;
-      Aliases[1].Button = qaIgnore;
-      Aliases[1].Alias = LoadStr(PASTE_KEY_BUTTON);
-      Aliases[1].OnSubmit = &PasteKeyHandler.Paste;
-      Aliases[1].GroupWith = qaYes;
-      Answers = qaYes | qaCancel | qaRetry | qaIgnore;
-      AliasesCount = 2;
+      std::vector<TQueryButtonAlias> Aliases;
+
+      TQueryButtonAlias CopyAlias;
+      CopyAlias.Button = qaRetry;
+      CopyAlias.Alias = LoadStr(COPY_KEY_BUTTON);
+      CopyAlias.ActionAlias = LoadStr(COPY_KEY_ACTION);
+      CopyAlias.OnSubmit = &ClipboardHandler.Copy;
+      Aliases.push_back(CopyAlias);
+
+      TQueryButtonAlias PasteAlias;
+      PasteAlias.Button = qaIgnore;
+      PasteAlias.Alias = LoadStr(PASTE_KEY_BUTTON);
+      PasteAlias.OnSubmit = &PasteKeyHandler.Paste;
+      PasteAlias.GroupWith = qaYes;
+      Aliases.push_back(PasteAlias);
+
+      TQueryButtonAlias OnceAlias;
+      OnceAlias.Button = qaOK;
+      OnceAlias.Alias = OnceButton;
+      OnceAlias.GroupWith = qaYes;
+      Aliases.push_back(OnceAlias);
+
+      Answers = qaYes | qaOK | qaCancel | qaRetry | qaIgnore;
       if (!Unknown)
       {
-        Aliases[2].Button = qaYes;
-        Aliases[2].Alias = LoadStr(UPDATE_KEY_BUTTON);
-        Aliases[3].Button = qaOK;
-        Aliases[3].Alias = LoadStr(ADD_KEY_BUTTON);
-        AliasesCount += 2;
-        Answers |= qaSkip | qaOK;
+        TQueryButtonAlias UpdateAlias;
+        UpdateAlias.Button = qaYes;
+        UpdateAlias.Alias = UpdateButton;
+        Aliases.push_back(UpdateAlias);
+
+        TQueryButtonAlias AddAlias;
+        AddAlias.Button = qaNo;
+        AddAlias.Alias = AddButton;
+        AddAlias.GroupWith = qaYes;
+        Aliases.push_back(AddAlias);
+
+        Answers |= qaNo;
       }
       else
       {
-        Answers |= qaNo;
+        TQueryButtonAlias AcceptAlias;
+        AcceptAlias.Button = qaYes;
+        AcceptAlias.Alias = AcceptButton;
+        Aliases.push_back(AcceptAlias);
       }
 
       TQueryParams Params(qpWaitInBatch);
-      Params.NoBatchAnswers = qaYes | qaRetry | qaSkip | qaOK;
+      Params.NoBatchAnswers = qaYes | qaNo | qaRetry | qaIgnore | qaOK;
       Params.HelpKeyword = (Unknown ? HELP_UNKNOWN_KEY : HELP_DIFFERENT_KEY);
-      Params.Aliases = Aliases;
-      Params.AliasesCount = AliasesCount;
+      Params.Aliases = &Aliases[0];
+      Params.AliasesCount = Aliases.size();
 
+      UnicodeString NewLine = L"\n";
+      UnicodeString Para = NewLine + NewLine;
+      UnicodeString Message;
+      UnicodeString ServerPara = FMTLOAD(HOSTKEY_SERVER, (Host, Port)) + Para;
       UnicodeString KeyTypeHuman = GetKeyTypeHuman(KeyType);
-      UnicodeString KeyDetails = FMTLOAD(KEY_DETAILS, (SignKeyType, SHA256, MD5));
-      UnicodeString Message = FMTLOAD((Unknown ? UNKNOWN_KEY4 : DIFFERENT_KEY5), (KeyTypeHuman, KeyDetails));
+      UnicodeString Nbsp = L"\xA0";
+      UnicodeString Indent = Nbsp + Nbsp + Nbsp + Nbsp;
+      UnicodeString FingerprintPara =
+        Indent + FMTLOAD(HOSTKEY_FINGERPRINT, (KeyTypeHuman)) + NewLine +
+        Indent + ReplaceStr(FingerprintSHA256, L" ", Nbsp) + Para;
+      if (Unknown)
+      {
+        Message =
+          MainInstructions(LoadStr(HOSTKEY_UNKNOWN)) + Para +
+          LoadStr(HOSTKEY_NOT_CACHED) + NewLine +
+          ServerPara +
+          LoadStr(HOSTKEY_NO_GUARANTEE) + Para +
+          FingerprintPara +
+          FMTLOAD(HOSTKEY_ACCEPT_NEW, (StripHotkey(AcceptButton))) + NewLine +
+          FMTLOAD(HOSTKEY_ONCE_NEW, (StripHotkey(OnceButton))) + NewLine +
+          FMTLOAD(HOSTKEY_CANCEL_NEW, (StripHotkey(CancelButton)));
+      }
+      else
+      {
+        Message =
+          MainInstructions(LoadStr(HOSTKEY_SECURITY_BREACH)) + Para +
+          LoadStr(HOSTKEY_DOESNT_MATCH) + NewLine +
+          ServerPara +
+          FMTLOAD(HOSTKEY_TWO_EXPLANATIONS, (LoadStr(HOSTKEY_ADMINISTRATOR_CHANGED), LoadStr(HOSTKEY_ANOTHER_COMPUTER))) + Para +
+          FingerprintPara +
+          FMTLOAD(HOSTKEY_ACCEPT_CHANGE, (StripHotkey(UpdateButton), StripHotkey(AddButton))) + NewLine +
+          FMTLOAD(HOSTKEY_ONCE_CHANGE, (StripHotkey(OnceButton))) + NewLine +
+          FMTLOAD(HOSTKEY_CANCEL_CHANGE, (StripHotkey(CancelButton), StripHotkey(CancelButton)));
+      }
+
       if (Configuration->Scripting)
       {
-        AddToList(Message, LoadStr(SCRIPTING_USE_HOSTKEY), L"\n");
+        AddToList(Message, LoadStr(SCRIPTING_USE_HOSTKEY), Para);
       }
 
       unsigned int R =
@@ -2655,7 +2713,7 @@ void __fastcall TSecureShell::VerifyHostKey(
       UnicodeString StoreKeyStr = KeyStr;
 
       switch (R) {
-        case qaOK:
+        case qaNo:
           DebugAssert(!Unknown);
           StoreKeyStr = (StoredKeys + HostKeyDelimiter + StoreKeyStr);
           // fall thru

source/resource/TextsCore.h

@@ -1,9 +1,6 @@
 #ifndef TextsCoreH
 #define TextsCoreH
 
-#define UNKNOWN_KEY4    1
-#define DIFFERENT_KEY5  2
-
 #define SCRIPT_HELP_HELP        4
 #define SCRIPT_EXIT_HELP        5
 #define SCRIPT_OPEN_HELP11      6
@@ -347,6 +344,24 @@
 #define PROXY_AUTH_USERNAME_PROMPT 364
 #define PROXY_AUTH_PASSWORD_PROMPT 365
 #define DIRECTORY_OVERWRITE_WARNING 366
+#define HOSTKEY_UNKNOWN         367
+#define HOSTKEY_SERVER          368
+#define HOSTKEY_NOT_CACHED      369
+#define HOSTKEY_NO_GUARANTEE    370
+#define HOSTKEY_SECURITY_BREACH 371
+#define HOSTKEY_TWO_EXPLANATIONS 372
+#define HOSTKEY_ADMINISTRATOR_CHANGED 373
+#define HOSTKEY_ANOTHER_COMPUTER 374
+#define HOSTKEY_FINGERPRINT     375
+#define HOSTKEY_DOESNT_MATCH    376
+#define HOSTKEY_ACCEPT_NEW      377
+#define HOSTKEY_ACCEPT_BUTTON   378
+#define HOSTKEY_ONCE_NEW        379
+#define HOSTKEY_ONCE_BUTTON     380
+#define HOSTKEY_CANCEL_NEW      381
+#define HOSTKEY_ACCEPT_CHANGE   382
+#define HOSTKEY_ONCE_CHANGE     383
+#define HOSTKEY_CANCEL_CHANGE   384
 
 #define CORE_INFORMATION_STRINGS 400
 #define YES_STR                 401
@@ -497,7 +512,6 @@
 #define PASTE_KEY_BUTTON        558
 #define SCRIPT_CP_DESC          559
 #define TIME_UNKNOWN            560
-#define KEY_DETAILS             561
 #define COPY_KEY_ACTION         562
 #define COPY_CERTIFICATE_ACTION 563
 #define COPY_INFO_DONT_ENCRYPT_NEW_FILES 564

source/resource/TextsCore1.rc

@@ -317,6 +317,24 @@ BEGIN
   PROXY_AUTH_USERNAME_PROMPT, "Proxy &username:"
   PROXY_AUTH_PASSWORD_PROMPT, "Proxy &password:"
   DIRECTORY_OVERWRITE_WARNING, "All files in the target directory will be deleted!"
+  HOSTKEY_UNKNOWN, "Continue connecting to an unknown server and add its host key to a cache?"
+  HOSTKEY_SERVER, "%s (port %d)"
+  HOSTKEY_NOT_CACHED, "The host key is not cached for this server:"
+  HOSTKEY_NO_GUARANTEE, "You have no guarantee that the server is the computer you think it is."
+  HOSTKEY_SECURITY_BREACH, "Warning – Potential security breach!"
+  HOSTKEY_TWO_EXPLANATIONS, "This means that either %s, or %s."
+  HOSTKEY_ADMINISTRATOR_CHANGED, "the server administrator has changed the host key"
+  HOSTKEY_ANOTHER_COMPUTER, "you have actually connected to another computer pretending to be the server"
+  HOSTKEY_FINGERPRINT, "The %s key fingerprint is:"
+  HOSTKEY_DOESNT_MATCH, "The host key does not match the one WinSCP has cached for this server:"
+  HOSTKEY_ACCEPT_NEW, "If you trust this host, select %s to add the key to WinSCP's cache and carry on connecting."
+  HOSTKEY_ACCEPT_BUTTON, "&Accept"
+  HOSTKEY_ONCE_NEW, "If you want to carry on connecting just once, without adding the key to the cache, select %s."
+  HOSTKEY_ONCE_BUTTON, "Connect &Once"
+  HOSTKEY_CANCEL_NEW, "If you do not trust this host, select %s to abandon the connection."
+  HOSTKEY_ACCEPT_CHANGE, "If you were expecting this change, trust the new key and want to continue connecting to the server, either select %s to update cache, or select %s to add the new key to the cache while keeping the old one(s)."
+  HOSTKEY_ONCE_CHANGE, "If you want to carry on connecting but without updating the cache, select %s."
+  HOSTKEY_CANCEL_CHANGE, "If you want to abandon the connection completely, select %s to cancel. Selecting %s is the ONLY guaranteed safe choice."
 
   CORE_INFORMATION_STRINGS, "CORE_INFORMATION"
   YES_STR, "Yes"
@@ -467,7 +485,6 @@ BEGIN
   PASTE_KEY_BUTTON, "&Paste key"
   SCRIPT_CP_DESC, "Duplicates remote file"
   TIME_UNKNOWN, "Unknown"
-  KEY_DETAILS, "    Algorithm:\t%s\n    SHA-256:\t%s\n    MD5:\t%s"
   COPY_KEY_ACTION, "&Copy key fingerprints to clipboard"
   COPY_CERTIFICATE_ACTION, "&Copy certificate fingerprint to clipboard"
   COPY_INFO_DONT_ENCRYPT_NEW_FILES, "Do not encrypt new files"

source/resource/TextsCore2.rc

@@ -2,43 +2,6 @@
 
 STRINGTABLE
 BEGIN
-  UNKNOWN_KEY4,
-    "**Continue connecting to an unknown server and "
-    "add its host key to a cache?**\n"
-    "\n"
-    "The server's host key was not found in the cache. You have no guarantee "
-    "that the server is the computer you think it is.\n"
-    "\n"
-    "The server's %s key details are:\n"
-    "\n"
-    "%s\n"
-    "\n"
-    "If you trust this host, press Yes. "
-    "To connect without adding host key to the cache, press No. "
-    "To abandon the connection press Cancel."
-  DIFFERENT_KEY5,
-    "**WARNING – POTENTIAL SECURITY BREACH!**\n"
-    "\n"
-    "The server's host key does not match the one WinSCP has "
-    "in cache. This means that either the "
-    "server administrator has changed the host key, "
-    "the server presents different key under certain circumstance, "
-    "or you have actually connected to another computer pretending "
-    "to be the server.\n"
-    "\n"
-    "The new %s key details are:\n"
-    "\n"
-    "%s\n"
-    "\n"
-    "If you were expecting this change, trust the new key and "
-    "want to continue connecting to the server, "
-    "either press Update to update cache, or "
-    "press Add to add the new key to the cache while keeping the old one(s). "
-    "If you want to carry on connecting but without updating "
-    "the cache, press Skip. "
-    "If you want to abandon the connection completely, press "
-    "Cancel. Pressing Cancel is the ONLY guaranteed safe "
-    "choice."
   SCRIPT_HELP_HELP,
     "help [ <command> [ <command2> ... ] ]\n"
     "  Displays list of commands when no parameters are specified.\n"