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).

git-svn-id: svn://svn.tartarus.org/sgt/putty@1413 cda61777-01e9-0310-a592-d414129be87e

diff --git a/pageant.c b/pageant.c
index 95eb1da..0114804 100644 (file)
--- a/pageant.c
+++ b/pageant.c
@@ -237,6 +237,26 @@ static int CALLBACK PassphraseProc(HWND hwnd, UINT msg,
 }
 
 /*
+ * 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.
  */
 static void keylist_update(void)

diff --git a/plink.c b/plink.c
index c60902b..8d4701a 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 6439de3..6b7ff1d 100644 (file)
--- a/psftp.c
+++ b/psftp.c
@@ -1362,6 +1362,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.
  */
 void fatalbox(char *fmt, ...)

diff --git a/puttygen.c b/puttygen.c
index 4ac0fef..689a383 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 c11f499..db9952e 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 2edfa47..c05c78c 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 65844a9..7022a6e 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 2d1f554..86c9fc0 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 f48051c..a9c1200 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);
+}