Put DH group1-sha1 KEX below 'warn' by default.

Also try to upgrade the settings of people who haven't changed the
defaults; but anyone who has, or anyone who's used the pre-release
snapshots with elliptic-curve support, will have to review their
settings manually.

diff --git a/doc/config.but b/doc/config.but
index 94626ab0be64f65f954cd904b9f8bab53e9a4fe4..3558f4ebdb16c5dcc83469e413f2fe31f45d758b 100644 (file)
--- a/doc/config.but
+++ b/doc/config.but
@@ -2394,15 +2394,16 @@ PuTTY currently supports the following key exchange methods:
 2048-bit group.
 
 \b \q{Group 1}: Diffie-Hellman key exchange with a well-known
-1024-bit group. This is less secure \#{FIXME better words} than
-group 14, but may be faster with slow client or server machines,
-and may be the only method supported by older server software.
+1024-bit group. We no longer recommend using this method, and it's
+not used by default in new installations; however, it may be the
+only method supported by very old server software.
 
 \b \q{\ii{Group exchange}}: with this method, instead of using a fixed
 group, PuTTY requests that the server suggest a group to use for key
 exchange; the server can avoid groups known to be weak, and possibly
 invent new ones over time, without any changes required to PuTTY's
-configuration. We recommend use of this method, if possible.
+configuration. We recommend use of this method instead of the
+well-known groups, if possible.
 
 \b \q{\i{RSA key exchange}}: this requires much less computational
 effort on the part of the client, and somewhat less on the part of

diff --git a/settings.c b/settings.c
index a5d29748165b0f8619e39dd71e698d5b9ade0256..0df7bb87a7304cc4733030d178be311c3ad77d6d 100644 (file)
--- a/settings.c
+++ b/settings.c
@@ -19,11 +19,15 @@ static const struct keyvalwhere ciphernames[] = {
     { "des",        CIPHER_DES,             -1, -1 }
 };
 
+/* The default order here is sometimes overridden by the backward-
+ * compatibility warts in load_open_settings(), and should be kept
+ * in sync with those. */
 static const struct keyvalwhere kexnames[] = {
     { "ecdh",               KEX_ECDH,       -1, +1 },
+    /* This name is misleading: it covers both SHA-256 and SHA-1 variants */
     { "dh-gex-sha1",        KEX_DHGEX,      -1, -1 },
     { "dh-group14-sha1",    KEX_DHGROUP14,  -1, -1 },
-    { "dh-group1-sha1",     KEX_DHGROUP1,   -1, -1 },
+    { "dh-group1-sha1",     KEX_DHGROUP1,   KEX_WARN, +1 },
     { "rsa",                KEX_RSA,        KEX_WARN, -1 },
     { "WARN",               KEX_WARN,       -1, -1 }
 };
@@ -309,20 +313,15 @@ static const char *val2key(const struct keyvalwhere *mapping,
  * to the end and duplicates are weeded.
  * XXX: assumes vals in 'mapping' are small +ve integers
  */
-static void gprefs(void *sesskey, const char *name, const char *def,
-                  const struct keyvalwhere *mapping, int nvals,
-                  Conf *conf, int primary)
+static void gprefs_from_str(const char *str,
+                           const struct keyvalwhere *mapping, int nvals,
+                           Conf *conf, int primary)
 {
-    char *commalist;
+    char *commalist = dupstr(str);
     char *p, *q;
     int i, j, n, v, pos;
     unsigned long seen = 0;           /* bitmap for weeding dups etc */
 
-    /*
-     * Fetch the string which we'll parse as a comma-separated list.
-     */
-    commalist = gpps_raw(sesskey, name, def);
-
     /*
      * Go through that list and convert it into values.
      */
@@ -393,6 +392,21 @@ static void gprefs(void *sesskey, const char *name, const char *def,
     }
 }
 
+/*
+ * Read a preference list.
+ */
+static void gprefs(void *sesskey, const char *name, const char *def,
+                  const struct keyvalwhere *mapping, int nvals,
+                  Conf *conf, int primary)
+{
+    /*
+     * Fetch the string which we'll parse as a comma-separated list.
+     */
+    char *value = gpps_raw(sesskey, name, def);
+    gprefs_from_str(value, mapping, nvals, conf, primary);
+    sfree(value);
+}
+
 /* 
  * Write out a preference list.
  */
@@ -784,20 +798,44 @@ void load_open_settings(void *sesskey, Conf *conf)
     gprefs(sesskey, "Cipher", "\0",
           ciphernames, CIPHER_MAX, conf, CONF_ssh_cipherlist);
     {
-       /* Backward-compatibility: we used to have an option to
+       /* Backward-compatibility: before 0.58 (when the "KEX"
+        * preference was first added), we had an option to
         * disable gex under the "bugs" panel after one report of
         * a server which offered it then choked, but we never got
         * a server version string or any other reports. */
-       const char *default_kexes;
+       const char *default_kexes,
+                  *normal_default = "ecdh,dh-gex-sha1,dh-group14-sha1,rsa,"
+                      "WARN,dh-group1-sha1",
+                  *bugdhgex2_default = "ecdh,dh-group14-sha1,rsa,"
+                      "WARN,dh-group1-sha1,dh-gex-sha1";
+       char *raw;
        i = 2 - gppi_raw(sesskey, "BugDHGEx2", 0);
        if (i == FORCE_ON)
-            default_kexes = "ecdh,dh-group14-sha1,dh-group1-sha1,rsa,"
-                "WARN,dh-gex-sha1";
+            default_kexes = bugdhgex2_default;
        else
-            default_kexes = "ecdh,dh-gex-sha1,dh-group14-sha1,"
-                "dh-group1-sha1,rsa,WARN";
-       gprefs(sesskey, "KEX", default_kexes,
-              kexnames, KEX_MAX, conf, CONF_ssh_kexlist);
+            default_kexes = normal_default;
+       /* Migration: after 0.67 we decided we didn't like
+        * dh-group1-sha1. If it looks like the user never changed
+        * the defaults, quietly upgrade their settings to demote it.
+        * (If they did, they're on their own.) */
+       raw = gpps_raw(sesskey, "KEX", default_kexes);
+       assert(raw != NULL);
+       /* Lack of 'ecdh' tells us this was saved by 0.58-0.67
+        * inclusive. If it was saved by a later version, we need
+        * to leave it alone. */
+       if (strcmp(raw, "dh-group14-sha1,dh-group1-sha1,rsa,"
+                  "WARN,dh-gex-sha1") == 0) {
+           /* Previously migrated from BugDHGEx2. */
+           sfree(raw);
+           raw = dupstr(bugdhgex2_default);
+       } else if (strcmp(raw, "dh-gex-sha1,dh-group14-sha1,"
+                         "dh-group1-sha1,rsa,WARN") == 0) {
+           /* Untouched old default setting. */
+           sfree(raw);
+           raw = dupstr(normal_default);
+       }
+       gprefs_from_str(raw, kexnames, KEX_MAX, conf, CONF_ssh_kexlist);
+       sfree(raw);
     }
     gprefs(sesskey, "HostKey", "ed25519,ecdsa,rsa,dsa,WARN",
            hknames, HK_MAX, conf, CONF_ssh_hklist);