INCOMPATIBLE CHANGE to the SSH2 private key file format. There is

now a passphrase-keyed MAC covering _all_ important data in the
file, including the public blob and the key comment. Should
conclusively scupper any attacks based on nobbling the key file in
an attempt to sucker the machine that decrypts it. MACing the
comment field also protects against a key-substitution attack (if
someone's worked out a way past our DSA protections and can extract
the private key from a signature, swapping key files and
substituting comments might just enable them to get the signature
they need to do this. Paranoid, but might as well).

[originally from svn r1413]

diff --git a/pageant.c b/pageant.c
index 95eb1da65230d73027ee75603919f4a8ba53c24a..0114804772d800f39dc6613089f2d730b4ec090e 100644 (file)
--- a/pageant.c
+++ b/pageant.c
@@ -236,6 +236,26 @@ static int CALLBACK PassphraseProc(HWND hwnd, UINT msg,
     return 0;
 }
 
+/*
+ * Warn about the obsolescent key file format.
+ */
+void old_keyfile_warning(void)
+{
+    static const char mbtitle[] = "PuTTY Key File Warning";
+    static const char message[] =
+       "You are loading an SSH 2 private key which has an\n"
+       "old version of the file format. This means your key\n"
+       "file is not fully tamperproof. Future versions of\n"
+       "PuTTY may stop supporting this private key format,\n"
+       "so we recommend you convert your key to the new\n"
+       "format.\n"
+       "\n"
+       "You can perform this conversion by loading the key\n"
+       "into PuTTYgen and then saving it again.";
+
+    MessageBox(NULL, message, mbtitle, MB_OK);
+}
+
 /*
  * Update the visible key list.
  */

diff --git a/plink.c b/plink.c
index c60902b548915c4dc9f239d6ffae4440daff846e..8d4701a9217708b76f33588c1d305969d9a2bd9d 100644 (file)
--- a/plink.c
+++ b/plink.c
@@ -162,6 +162,25 @@ void askcipher(char *ciphername, int cs)
     }
 }
 
+/*
+ * Warn about the obsolescent key file format.
+ */
+void old_keyfile_warning(void)
+{
+    static const char message[] =
+       "You are loading an SSH 2 private key which has an\n"
+       "old version of the file format. This means your key\n"
+       "file is not fully tamperproof. Future versions of\n"
+       "PuTTY may stop supporting this private key format,\n"
+       "so we recommend you convert your key to the new\n"
+       "format.\n"
+       "\n"
+       "Once the key is loaded into PuTTYgen, you can perform\n"
+       "this conversion simply by saving it again.\n";
+
+    fputs(message, stderr);
+}
+
 HANDLE inhandle, outhandle, errhandle;
 DWORD orig_console_mode;
 

diff --git a/psftp.c b/psftp.c
index 6439de3ff607ef2362e77b88b606ce64ca261f13..6b7ff1d1a34813b4f536d06da25b3f31d695baca 100644 (file)
--- a/psftp.c
+++ b/psftp.c
@@ -1361,6 +1361,25 @@ void askcipher(char *ciphername, int cs)
     }
 }
 
+/*
+ * Warn about the obsolescent key file format.
+ */
+void old_keyfile_warning(void)
+{
+    static const char message[] =
+       "You are loading an SSH 2 private key which has an\n"
+       "old version of the file format. This means your key\n"
+       "file is not fully tamperproof. Future versions of\n"
+       "PuTTY may stop supporting this private key format,\n"
+       "so we recommend you convert your key to the new\n"
+       "format.\n"
+       "\n"
+       "Once the key is loaded into PuTTYgen, you can perform\n"
+       "this conversion simply by saving it again.\n";
+
+    fputs(message, stderr);
+}
+
 /*
  *  Print an error message and perform a fatal exit.
  */

diff --git a/puttygen.c b/puttygen.c
index 4ac0fef9c7853610c12e5d1488759894c8608f97..689a38353af7f32b350d3b326660e5f9e36b09aa 100644 (file)
--- a/puttygen.c
+++ b/puttygen.c
@@ -406,6 +406,26 @@ static int save_ssh1_pubkey(char *filename, struct RSAKey *key)
     return 1;
 }
 
+/*
+ * Warn about the obsolescent key file format.
+ */
+void old_keyfile_warning(void)
+{
+    static const char mbtitle[] = "PuTTY Key File Warning";
+    static const char message[] =
+       "You are loading an SSH 2 private key which has an\n"
+       "old version of the file format. This means your key\n"
+       "file is not fully tamperproof. Future versions of\n"
+       "PuTTY may stop supporting this private key format,\n"
+       "so we recommend you convert your key to the new\n"
+       "format.\n"
+       "\n"
+       "Once the key is loaded into PuTTYgen, you can perform\n"
+       "this conversion simply by saving it again.";
+
+    MessageBox(NULL, message, mbtitle, MB_OK);
+}
+
 static int save_ssh2_pubkey(char *filename, struct ssh2_userkey *key)
 {
     unsigned char *pub_blob;

diff --git a/scp.c b/scp.c
index c11f499e92c5cfa5647187c05f343aa1a1ebd74f..db9952efbc8ba9cfa815ec66b10f325c4e300e0f 100644 (file)
--- a/scp.c
+++ b/scp.c
@@ -226,6 +226,25 @@ void askcipher(char *ciphername, int cs)
     }
 }
 
+/*
+ * Warn about the obsolescent key file format.
+ */
+void old_keyfile_warning(void)
+{
+    static const char message[] =
+       "You are loading an SSH 2 private key which has an\n"
+       "old version of the file format. This means your key\n"
+       "file is not fully tamperproof. Future versions of\n"
+       "PuTTY may stop supporting this private key format,\n"
+       "so we recommend you convert your key to the new\n"
+       "format.\n"
+       "\n"
+       "Once the key is loaded into PuTTYgen, you can perform\n"
+       "this conversion simply by saving it again.\n";
+
+    fputs(message, stderr);
+}
+
 /* GUI Adaptation - Sept 2000 */
 static void send_msg(HWND h, UINT message, WPARAM wParam)
 {

diff --git a/ssh.h b/ssh.h
index 2edfa47a4c54c232c27e4841c43d7a9646139bd0..c05c78c8f013ab436b01254f1dd3722aa37ba97f 100644 (file)
--- a/ssh.h
+++ b/ssh.h
@@ -355,3 +355,9 @@ int zlib_decompress_block(unsigned char *block, int len,
 #define SSH2_AGENTC_ADD_IDENTITY                17
 #define SSH2_AGENTC_REMOVE_IDENTITY             18
 #define SSH2_AGENTC_REMOVE_ALL_IDENTITIES       19
+
+/*
+ * Need this to warn about support for the original SSH2 keyfile
+ * format.
+ */
+void old_keyfile_warning(void);

diff --git a/sshdss.c b/sshdss.c
index 65844a9bcda2ec90a4830485e216d80afbc0a802..7022a6e2a9bc173de1c796c646caa5efb04d34ea 100644 (file)
--- a/sshdss.c
+++ b/sshdss.c
@@ -361,25 +361,15 @@ static unsigned char *dss_private_blob(void *key, int *len)
     xlen = (bignum_bitcount(dss->x) + 8) / 8;
 
     /*
-     * mpint x, string[20] the SHA of p||q||g. Total 28 + xlen.
-     * (two length fields and twenty bytes, 20+8=28).
+     * mpint x, string[20] the SHA of p||q||g. Total 4 + xlen.
      */
-    bloblen = 28 + xlen;
+    bloblen = 4 + xlen;
     blob = smalloc(bloblen);
     p = blob;
     PUT_32BIT(p, xlen);
     p += 4;
     for (i = xlen; i--;)
        *p++ = bignum_byte(dss->x, i);
-    PUT_32BIT(p, 20);
-    SHA_Init(&s);
-    sha_mpint(&s, dss->p);
-    sha_mpint(&s, dss->q);
-    sha_mpint(&s, dss->g);
-    SHA_Final(&s, digest);
-    p += 4;
-    for (i = 0; i < 20; i++)
-       *p++ = digest[i];
     assert(p == blob + bloblen);
     *len = bloblen;
     return blob;
@@ -398,24 +388,22 @@ static void *dss_createkey(unsigned char *pub_blob, int pub_len,
 
     dss = dss_newkey((char *) pub_blob, pub_len);
     dss->x = getmp(&pb, &priv_len);
-    getstring(&pb, &priv_len, &hash, &hashlen);
 
     /*
-     * Verify details of the key. First check that the hash is
-     * indeed a hash of p||q||g.
+     * Check the obsolete hash in the old DSS key format.
      */
-    if (hashlen != 20) {
-       dss_freekey(dss);
-       return NULL;
-    }
-    SHA_Init(&s);
-    sha_mpint(&s, dss->p);
-    sha_mpint(&s, dss->q);
-    sha_mpint(&s, dss->g);
-    SHA_Final(&s, digest);
-    if (0 != memcmp(hash, digest, 20)) {
-       dss_freekey(dss);
-       return NULL;
+    hashlen = -1;
+    getstring(&pb, &priv_len, &hash, &hashlen);
+    if (hashlen == 20) {
+       SHA_Init(&s);
+       sha_mpint(&s, dss->p);
+       sha_mpint(&s, dss->q);
+       sha_mpint(&s, dss->g);
+       SHA_Final(&s, digest);
+       if (0 != memcmp(hash, digest, 20)) {
+           dss_freekey(dss);
+           return NULL;
+       }
     }
 
     /*

diff --git a/sshpubk.c b/sshpubk.c
index 2d1f55409fb7694c81dac4e76c31e271dd88b71a..86c9fc03a42849c2dc522622fc9b1e6854ba177c 100644 (file)
--- a/sshpubk.c
+++ b/sshpubk.c
@@ -10,6 +10,7 @@
 #include <assert.h>
 
 #include "ssh.h"
+#include "misc.h"
 
 #define PUT_32BIT(cp, value) do { \
   (cp)[3] = (value); \
@@ -306,7 +307,7 @@ int saversakey(char *filename, struct RSAKey *key, char *passphrase)
  * The file is text. Lines are terminated by CRLF, although CR-only
  * and LF-only are tolerated on input.
  *
- * The first line says "PuTTY-User-Key-File-1: " plus the name of the
+ * The first line says "PuTTY-User-Key-File-2: " plus the name of the
  * algorithm ("ssh-dss", "ssh-rsa" etc).
  *
  * The next line says "Encryption: " plus an encryption type.
@@ -339,19 +340,18 @@ int saversakey(char *filename, struct RSAKey *key, char *passphrase)
  * And for "ssh-dss", it will be composed of
  *
  *    mpint  x                  (the private key parameter)
- *    string hash               (20-byte hash of mpints p || q || g)
- *
- * Finally, there is a line saying _either_
- *
- *  - "Private-Hash: " plus a hex representation of a SHA-1 hash of
- *    the plaintext version of the private part, including the
- *    final padding.
+ *  [ string hash   20-byte hash of mpints p || q || g   only in old format ]
  * 
- * or
- * 
- *  - "Private-MAC: " plus a hex representation of a HMAC-SHA-1 of
- *    the plaintext version of the private part, including the
- *    final padding.
+ * Finally, there is a line saying "Private-MAC: " plus a hex
+ * representation of a HMAC-SHA-1 of:
+ *
+ *    string  name of algorithm ("ssh-dss", "ssh-rsa")
+ *    string  encryption type
+ *    string  comment
+ *    string  public-blob
+ *    string  private-plaintext (the plaintext version of the
+ *                               private part, including the final
+ *                               padding)
  * 
  * The key to the MAC is itself a SHA-1 hash of:
  * 
@@ -371,16 +371,15 @@ int saversakey(char *filename, struct RSAKey *key, char *passphrase)
  * where the sequence-number increases from zero. As many of these
  * hashes are used as necessary.
  *
- * NOTE! It is important that all _public_ data can be verified
- * with reference to the _private_ data. There exist attacks based
- * on modifying the public key but leaving the private section
- * intact.
- *
- * With RSA, this is easy: verify that n = p*q, and also verify
- * that e*d == 1 modulo (p-1)(q-1). With DSA, we need to store
- * extra data in the private section other than just x, namely a
- * hash of p||q||g. (It's then easy to verify that y is equal to
- * g^x mod p.)
+ * For backwards compatibility with snapshots between 0.51 and
+ * 0.52, we also support the older key file format, which begins
+ * with "PuTTY-User-Key-File-1" (version number differs). In this
+ * format the Private-MAC: field only covers the private-plaintext
+ * field and nothing else (and without the 4-byte string length on
+ * the front too). Moreover, for RSA keys the Private-MAC: field
+ * can be replaced with a Private-Hash: field which is a plain
+ * SHA-1 hash instead of an HMAC. This is not allowable in DSA
+ * keys. (Yes, the old format was a mess. Guess why it changed :-)
  */
 
 static int read_header(FILE * fp, char *header)
@@ -535,13 +534,13 @@ struct ssh2_userkey ssh2_wrong_passphrase = {
 struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
 {
     FILE *fp;
-    char header[40], *b, *comment, *mac;
+    char header[40], *b, *encryption, *comment, *mac;
     const struct ssh_signkey *alg;
     struct ssh2_userkey *ret;
     int cipher, cipherblk;
     unsigned char *public_blob, *private_blob;
     int public_blob_len, private_blob_len;
-    int i, is_mac;
+    int i, is_mac, old_fmt;
     int passlen = passphrase ? strlen(passphrase) : 0;
 
     ret = NULL;                               /* return NULL for most errors */
@@ -553,8 +552,15 @@ struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
        goto error;
 
     /* Read the first header line which contains the key type. */
-    if (!read_header(fp, header)
-       || 0 != strcmp(header, "PuTTY-User-Key-File-1"))
+    if (!read_header(fp, header))
+       goto error;
+    if (0 == strcmp(header, "PuTTY-User-Key-File-2")) {
+       old_fmt = 0;
+    } else if (0 == strcmp(header, "PuTTY-User-Key-File-1")) {
+       /* this is an old key file; warn and then continue */
+       old_keyfile_warning();
+       old_fmt = 1;
+    } else
        goto error;
     if ((b = read_body(fp)) == NULL)
        goto error;
@@ -572,19 +578,18 @@ struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
     /* Read the Encryption header line. */
     if (!read_header(fp, header) || 0 != strcmp(header, "Encryption"))
        goto error;
-    if ((b = read_body(fp)) == NULL)
+    if ((encryption = read_body(fp)) == NULL)
        goto error;
-    if (!strcmp(b, "aes256-cbc")) {
+    if (!strcmp(encryption, "aes256-cbc")) {
        cipher = 1;
        cipherblk = 16;
-    } else if (!strcmp(b, "none")) {
+    } else if (!strcmp(encryption, "none")) {
        cipher = 0;
        cipherblk = 1;
     } else {
-       sfree(b);
+       sfree(encryption);
        goto error;
     }
-    sfree(b);
 
     /* Read the Comment header line. */
     if (!read_header(fp, header) || 0 != strcmp(header, "Comment"))
@@ -619,7 +624,8 @@ struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
        if ((mac = read_body(fp)) == NULL)
            goto error;
        is_mac = 1;
-    } else if (0 == strcmp(header, "Private-Hash")) {
+    } else if (0 == strcmp(header, "Private-Hash") &&
+                          alg == &ssh_rsa && old_fmt) {
        if ((mac = read_body(fp)) == NULL)
            goto error;
        is_mac = 0;
@@ -653,33 +659,66 @@ struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
     }
 
     /*
-     * Verify the private hash.
+     * Verify the MAC.
      */
     {
        char realmac[41];
        unsigned char binary[20];
+       unsigned char *macdata;
+       int maclen;
+       int free_macdata;
+
+       if (old_fmt) {
+           /* MAC (or hash) only covers the private blob. */
+           macdata = private_blob;
+           maclen = private_blob_len;
+           free_macdata = 0;
+       } else {
+           unsigned char *p;
+           int namelen = strlen(alg->name);
+           int enclen = strlen(encryption);
+           int commlen = strlen(comment);
+           maclen = (4 + namelen +
+                     4 + enclen +
+                     4 + commlen +
+                     4 + public_blob_len +
+                     4 + private_blob_len);
+           macdata = smalloc(maclen);
+           p = macdata;
+#define DO_STR(s,len) PUT_32BIT(p,(len));memcpy(p+4,(s),(len));p+=4+(len)
+           DO_STR(alg->name, namelen);
+           DO_STR(encryption, enclen);
+           DO_STR(comment, commlen);
+           DO_STR(public_blob, public_blob_len);
+           DO_STR(private_blob, private_blob_len);
+
+           free_macdata = 1;
+       }
 
        if (is_mac) {
            SHA_State s;
            unsigned char mackey[20];
            char header[] = "putty-private-key-file-mac-key";
 
-           if (!passphrase)           /* can't have MAC in unencrypted key */
-               goto error;
-
            SHA_Init(&s);
            SHA_Bytes(&s, header, sizeof(header)-1);
-           SHA_Bytes(&s, passphrase, passlen);
+           if (passphrase)
+               SHA_Bytes(&s, passphrase, passlen);
            SHA_Final(&s, mackey);
 
-           hmac_sha1_simple(mackey, 20, private_blob, private_blob_len,
-                            binary);
+           hmac_sha1_simple(mackey, 20, macdata, maclen, binary);
 
            memset(mackey, 0, sizeof(mackey));
            memset(&s, 0, sizeof(s));
        } else {
-           SHA_Simple(private_blob, private_blob_len, binary);
+           SHA_Simple(macdata, maclen, binary);
        }
+
+       if (free_macdata) {
+           memset(macdata, 0, maclen);
+           sfree(macdata);
+       }
+
        for (i = 0; i < 20; i++)
            sprintf(realmac + 2 * i, "%02x", binary[i]);
 
@@ -707,6 +746,7 @@ struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
     }
     sfree(public_blob);
     sfree(private_blob);
+    sfree(encryption);
     return ret;
 
     /*
@@ -717,6 +757,8 @@ struct ssh2_userkey *ssh2_load_userkey(char *filename, char *passphrase)
        fclose(fp);
     if (comment)
        sfree(comment);
+    if (encryption)
+       sfree(encryption);
     if (mac)
        sfree(mac);
     if (public_blob)
@@ -744,7 +786,8 @@ char *ssh2_userkey_loadpub(char *filename, char **algorithm,
 
     /* Read the first header line which contains the key type. */
     if (!read_header(fp, header)
-       || 0 != strcmp(header, "PuTTY-User-Key-File-1"))
+       || (0 != strcmp(header, "PuTTY-User-Key-File-2") &&
+           0 != strcmp(header, "PuTTY-User-Key-File-1")))
        goto error;
     if ((b = read_body(fp)) == NULL)
        goto error;
@@ -812,7 +855,8 @@ int ssh2_userkey_encrypted(char *filename, char **commentptr)
     if (!fp)
        return 0;
     if (!read_header(fp, header)
-       || 0 != strcmp(header, "PuTTY-User-Key-File-1")) {
+       || (0 != strcmp(header, "PuTTY-User-Key-File-2") &&
+           0 != strcmp(header, "PuTTY-User-Key-File-1"))) {
        fclose(fp);
        return 0;
     }
@@ -914,7 +958,7 @@ int ssh2_save_userkey(char *filename, struct ssh2_userkey *key,
     int pub_blob_len, priv_blob_len, priv_encrypted_len;
     int passlen;
     int cipherblk;
-    int i, is_mac;
+    int i;
     char *cipherstr;
     unsigned char priv_mac[20];
 
@@ -951,29 +995,42 @@ int ssh2_save_userkey(char *filename, struct ssh2_userkey *key,
     memcpy(priv_blob_encrypted + priv_blob_len, priv_mac,
           priv_encrypted_len - priv_blob_len);
 
-    /* Now create the private MAC. */
-    if (passphrase) {
+    /* Now create the MAC. */
+    {
+       unsigned char *macdata;
+       int maclen;
+       unsigned char *p;
+       int namelen = strlen(key->alg->name);
+       int enclen = strlen(cipherstr);
+       int commlen = strlen(key->comment);
        SHA_State s;
        unsigned char mackey[20];
        char header[] = "putty-private-key-file-mac-key";
 
-       passlen = strlen(passphrase);
+       maclen = (4 + namelen +
+                 4 + enclen +
+                 4 + commlen +
+                 4 + pub_blob_len +
+                 4 + priv_encrypted_len);
+       macdata = smalloc(maclen);
+       p = macdata;
+#define DO_STR(s,len) PUT_32BIT(p,(len));memcpy(p+4,(s),(len));p+=4+(len)
+       DO_STR(key->alg->name, namelen);
+       DO_STR(cipherstr, enclen);
+       DO_STR(key->comment, commlen);
+       DO_STR(pub_blob, pub_blob_len);
+       DO_STR(priv_blob_encrypted, priv_encrypted_len);
 
        SHA_Init(&s);
        SHA_Bytes(&s, header, sizeof(header)-1);
-       SHA_Bytes(&s, passphrase, passlen);
+       if (passphrase)
+           SHA_Bytes(&s, passphrase, strlen(passphrase));
        SHA_Final(&s, mackey);
-
-       hmac_sha1_simple(mackey, 20,
-                        priv_blob_encrypted, priv_encrypted_len,
-                        priv_mac);
-       is_mac = 1;
-
+       hmac_sha1_simple(mackey, 20, macdata, maclen, priv_mac);
+       memset(macdata, 0, maclen);
+       sfree(macdata);
        memset(mackey, 0, sizeof(mackey));
        memset(&s, 0, sizeof(s));
-    } else {
-       SHA_Simple(priv_blob_encrypted, priv_encrypted_len, priv_mac);
-       is_mac = 0;
     }
 
     if (passphrase) {
@@ -1000,21 +1057,23 @@ int ssh2_save_userkey(char *filename, struct ssh2_userkey *key,
     fp = fopen(filename, "w");
     if (!fp)
        return 0;
-    fprintf(fp, "PuTTY-User-Key-File-1: %s\n", key->alg->name);
+    fprintf(fp, "PuTTY-User-Key-File-2: %s\n", key->alg->name);
     fprintf(fp, "Encryption: %s\n", cipherstr);
     fprintf(fp, "Comment: %s\n", key->comment);
     fprintf(fp, "Public-Lines: %d\n", base64_lines(pub_blob_len));
     base64_encode(fp, pub_blob, pub_blob_len);
     fprintf(fp, "Private-Lines: %d\n", base64_lines(priv_encrypted_len));
     base64_encode(fp, priv_blob_encrypted, priv_encrypted_len);
-    if (is_mac)
-       fprintf(fp, "Private-MAC: ");
-    else
-       fprintf(fp, "Private-Hash: ");
+    fprintf(fp, "Private-MAC: ");
     for (i = 0; i < 20; i++)
        fprintf(fp, "%02x", priv_mac[i]);
     fprintf(fp, "\n");
     fclose(fp);
+
+    sfree(pub_blob);
+    memset(priv_blob, 0, priv_blob_len);
+    sfree(priv_blob);
+    sfree(priv_blob_encrypted);
     return 1;
 }
 

diff --git a/windlg.c b/windlg.c
index f48051c71ac7b39ef1d0ccee257b900db9fc98a8..a9c1200a4488af5938a55e9ea9407b61d440c212 100644 (file)
--- a/windlg.c
+++ b/windlg.c
@@ -2952,3 +2952,23 @@ int askappend(char *filename)
     else
        return 0;
 }
+
+/*
+ * Warn about the obsolescent key file format.
+ */
+void old_keyfile_warning(void)
+{
+    static const char mbtitle[] = "PuTTY Key File Warning";
+    static const char message[] =
+       "You are loading an SSH 2 private key which has an\n"
+       "old version of the file format. This means your key\n"
+       "file is not fully tamperproof. Future versions of\n"
+       "PuTTY may stop supporting this private key format,\n"
+       "so we recommend you convert your key to the new\n"
+       "format.\n"
+       "\n"
+       "You can perform this conversion by loading the key\n"
+       "into PuTTYgen and then saving it again.";
+
+    MessageBox(NULL, message, mbtitle, MB_OK);
+}