Improvements to PuTTYgen error reporting:

 - will now display a reason when it fails to load a key
 - uses existing error return from native keys
 - import.c had a lot of error descriptions which weren't going anywhere;
   since the strings are probably taking up space in the binary, we
   may as well use them

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

diff --git a/cmdgen.c b/cmdgen.c
index 5042aa4..83a158d 100644 (file)
--- a/cmdgen.c
+++ b/cmdgen.c
@@ -739,15 +739,14 @@ int main(int argc, char **argv)
 
          case SSH_KEYTYPE_OPENSSH:
          case SSH_KEYTYPE_SSHCOM:
-           ssh2key = import_ssh2(&infilename, intype, passphrase);
-           if (ssh2key && ssh2key != SSH2_WRONG_PASSPHRASE)
-               error = NULL;
-           else if (!error) {
-               if (ssh2key == SSH2_WRONG_PASSPHRASE)
-                   error = "wrong passphrase";
+           ssh2key = import_ssh2(&infilename, intype, passphrase, &error);
+           if (ssh2key) {
+               if (ssh2key != SSH2_WRONG_PASSPHRASE)
+                   error = NULL;
                else
-                   error = "unknown error";
-           }
+                   error = "wrong passphrase";
+           } else if (!error)
+               error = "unknown error";
            break;
 
          default:

diff --git a/import.c b/import.c
index d1ab751..b6d32cb 100644 (file)
--- a/import.c
+++ b/import.c
@@ -25,12 +25,14 @@
     ((unsigned long)(unsigned char)(cp)[3]))
 
 int openssh_encrypted(const Filename *filename);
-struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase);
+struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase,
+                                 const char **errmsg_p);
 int openssh_write(const Filename *filename, struct ssh2_userkey *key,
                  char *passphrase);
 
 int sshcom_encrypted(const Filename *filename, char **comment);
-struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase);
+struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
+                                const char **errmsg_p);
 int sshcom_write(const Filename *filename, struct ssh2_userkey *key,
                 char *passphrase);
 
@@ -79,7 +81,7 @@ int import_encrypted(const Filename *filename, int type, char **comment)
  * Import an SSH1 key.
  */
 int import_ssh1(const Filename *filename, int type,
-               struct RSAKey *key, char *passphrase)
+               struct RSAKey *key, char *passphrase, const char **errmsg_p)
 {
     return 0;
 }
@@ -88,12 +90,12 @@ int import_ssh1(const Filename *filename, int type,
  * Import an SSH2 key.
  */
 struct ssh2_userkey *import_ssh2(const Filename *filename, int type,
-                                char *passphrase)
+                                char *passphrase, const char **errmsg_p)
 {
     if (type == SSH_KEYTYPE_OPENSSH)
-       return openssh_read(filename, passphrase);
+       return openssh_read(filename, passphrase, errmsg_p);
     if (type == SSH_KEYTYPE_SSHCOM)
-       return sshcom_read(filename, passphrase);
+       return sshcom_read(filename, passphrase, errmsg_p);
     return NULL;
 }
 
@@ -315,7 +317,8 @@ struct openssh_key {
     int keyblob_len, keyblob_size;
 };
 
-static struct openssh_key *load_openssh_key(const Filename *filename)
+static struct openssh_key *load_openssh_key(const Filename *filename,
+                                           const char **errmsg_p)
 {
     struct openssh_key *ret;
     FILE *fp;
@@ -333,13 +336,13 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
 
     fp = f_open(*filename, "r");
     if (!fp) {
-       errmsg = "Unable to open key file";
+       errmsg = "unable to open key file";
        goto error;
     }
     if (!fgets(buffer, sizeof(buffer), fp) ||
        0 != strncmp(buffer, "-----BEGIN ", 11) ||
        0 != strcmp(buffer+strlen(buffer)-17, "PRIVATE KEY-----\n")) {
-       errmsg = "File does not begin with OpenSSH key header";
+       errmsg = "file does not begin with OpenSSH key header";
        goto error;
     }
     if (!strcmp(buffer, "-----BEGIN RSA PRIVATE KEY-----\n"))
@@ -347,14 +350,14 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
     else if (!strcmp(buffer, "-----BEGIN DSA PRIVATE KEY-----\n"))
        ret->type = OSSH_DSA;
     else {
-       errmsg = "Unrecognised key type";
+       errmsg = "unrecognised key type";
        goto error;
     }
 
     headers_done = 0;
     while (1) {
        if (!fgets(buffer, sizeof(buffer), fp)) {
-           errmsg = "Unexpected end of file";
+           errmsg = "unexpected end of file";
            goto error;
        }
        if (0 == strncmp(buffer, "-----END ", 9) &&
@@ -362,7 +365,7 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
            break;                     /* done */
        if ((p = strchr(buffer, ':')) != NULL) {
            if (headers_done) {
-               errmsg = "Header found in body of key data";
+               errmsg = "header found in body of key data";
                goto error;
            }
            *p++ = '\0';
@@ -379,7 +382,7 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
                int i, j;
 
                if (strncmp(p, "DES-EDE3-CBC,", 13)) {
-                   errmsg = "Ciphers other than DES-EDE3-CBC not supported";
+                   errmsg = "ciphers other than DES-EDE3-CBC not supported";
                    goto error;
                }
                p += 13;
@@ -390,7 +393,7 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
                    p += 2;
                }
                if (i < 8) {
-                   errmsg = "Expected 16-digit iv in DEK-Info";
+                   errmsg = "expected 16-digit iv in DEK-Info";
                    goto error;
                }
            }
@@ -409,7 +412,7 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
                     len = base64_decode_atom(base64_bit, out);
 
                     if (len <= 0) {
-                        errmsg = "Invalid base64 encoding";
+                        errmsg = "invalid base64 encoding";
                         goto error;
                     }
 
@@ -431,17 +434,18 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
     }
 
     if (ret->keyblob_len == 0 || !ret->keyblob) {
-       errmsg = "Key body not present";
+       errmsg = "key body not present";
        goto error;
     }
 
     if (ret->encrypted && ret->keyblob_len % 8 != 0) {
-       errmsg = "Encrypted key blob is not a multiple of cipher block size";
+       errmsg = "encrypted key blob is not a multiple of cipher block size";
        goto error;
     }
 
     memset(buffer, 0, sizeof(buffer));
     memset(base64_bit, 0, sizeof(base64_bit));
+    if (errmsg_p) *errmsg_p = NULL;
     return ret;
 
     error:
@@ -455,12 +459,13 @@ static struct openssh_key *load_openssh_key(const Filename *filename)
         memset(&ret, 0, sizeof(ret));
        sfree(ret);
     }
+    if (errmsg_p) *errmsg_p = errmsg;
     return NULL;
 }
 
 int openssh_encrypted(const Filename *filename)
 {
-    struct openssh_key *key = load_openssh_key(filename);
+    struct openssh_key *key = load_openssh_key(filename, NULL);
     int ret;
 
     if (!key)
@@ -473,9 +478,10 @@ int openssh_encrypted(const Filename *filename)
     return ret;
 }
 
-struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase)
+struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase,
+                                 const char **errmsg_p)
 {
-    struct openssh_key *key = load_openssh_key(filename);
+    struct openssh_key *key = load_openssh_key(filename, errmsg_p);
     struct ssh2_userkey *retkey;
     unsigned char *p;
     int ret, id, len, flags;
@@ -592,7 +598,7 @@ struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase)
             * this is some sort of version indication).
             */
            if (len != 1 || p[0] != 0) {
-               errmsg = "Version number mismatch";
+               errmsg = "version number mismatch";
                goto error;
            }
        } else if (key->type == OSSH_RSA) {
@@ -662,6 +668,7 @@ struct ssh2_userkey *openssh_read(const Filename *filename, char *passphrase)
     sfree(key->keyblob);
     memset(&key, 0, sizeof(key));
     sfree(key);
+    if (errmsg_p) *errmsg_p = errmsg;
     return retval;
 }
 
@@ -988,7 +995,8 @@ struct sshcom_key {
     int keyblob_len, keyblob_size;
 };
 
-static struct sshcom_key *load_sshcom_key(const Filename *filename)
+static struct sshcom_key *load_sshcom_key(const Filename *filename,
+                                         const char **errmsg_p)
 {
     struct sshcom_key *ret;
     FILE *fp;
@@ -1006,26 +1014,26 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename)
 
     fp = f_open(*filename, "r");
     if (!fp) {
-       errmsg = "Unable to open key file";
+       errmsg = "unable to open key file";
        goto error;
     }
     if (!fgets(buffer, sizeof(buffer), fp) ||
        0 != strcmp(buffer, "---- BEGIN SSH2 ENCRYPTED PRIVATE KEY ----\n")) {
-       errmsg = "File does not begin with ssh.com key header";
+       errmsg = "file does not begin with ssh.com key header";
        goto error;
     }
 
     headers_done = 0;
     while (1) {
        if (!fgets(buffer, sizeof(buffer), fp)) {
-           errmsg = "Unexpected end of file";
+           errmsg = "unexpected end of file";
            goto error;
        }
         if (!strcmp(buffer, "---- END SSH2 ENCRYPTED PRIVATE KEY ----\n"))
             break;                     /* done */
        if ((p = strchr(buffer, ':')) != NULL) {
            if (headers_done) {
-               errmsg = "Header found in body of key data";
+               errmsg = "header found in body of key data";
                goto error;
            }
            *p++ = '\0';
@@ -1037,11 +1045,11 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename)
             while ((len = strlen(p)) > (int)(sizeof(buffer) - (p-buffer) -1) ||
                    p[len-1] != '\n' || p[len-2] == '\\') {
                 if (len > (int)((p-buffer) + sizeof(buffer)-2)) {
-                    errmsg = "Header line too long to deal with";
+                    errmsg = "header line too long to deal with";
                     goto error;
                 }
                 if (!fgets(p+len-2, sizeof(buffer)-(p-buffer)-(len-2), fp)) {
-                    errmsg = "Unexpected end of file";
+                    errmsg = "unexpected end of file";
                     goto error;
                 }
             }
@@ -1069,7 +1077,7 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename)
                     len = base64_decode_atom(base64_bit, out);
 
                     if (len <= 0) {
-                        errmsg = "Invalid base64 encoding";
+                        errmsg = "invalid base64 encoding";
                         goto error;
                     }
 
@@ -1089,10 +1097,11 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename)
     }
 
     if (ret->keyblob_len == 0 || !ret->keyblob) {
-       errmsg = "Key body not present";
+       errmsg = "key body not present";
        goto error;
     }
 
+    if (errmsg_p) *errmsg_p = NULL;
     return ret;
 
     error:
@@ -1104,12 +1113,13 @@ static struct sshcom_key *load_sshcom_key(const Filename *filename)
         memset(&ret, 0, sizeof(ret));
        sfree(ret);
     }
+    if (errmsg_p) *errmsg_p = errmsg;
     return NULL;
 }
 
 int sshcom_encrypted(const Filename *filename, char **comment)
 {
-    struct sshcom_key *key = load_sshcom_key(filename);
+    struct sshcom_key *key = load_sshcom_key(filename, NULL);
     int pos, len, answer;
 
     *comment = NULL;
@@ -1189,9 +1199,10 @@ static int sshcom_put_mpint(void *target, void *data, int len)
     return len+4;
 }
 
-struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
+struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase,
+                                const char **errmsg_p)
 {
-    struct sshcom_key *key = load_sshcom_key(filename);
+    struct sshcom_key *key = load_sshcom_key(filename, errmsg_p);
     char *errmsg;
     int pos, len;
     const char prefix_rsa[] = "if-modn{sign{rsa";
@@ -1212,7 +1223,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
      * Check magic number.
      */
     if (GET_32BIT(key->keyblob) != SSHCOM_MAGIC_NUMBER) {
-        errmsg = "Key does not begin with magic number";
+        errmsg = "key does not begin with magic number";
         goto error;
     }
 
@@ -1222,7 +1233,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
     pos = 8;
     if (key->keyblob_len < pos+4 ||
         (len = GET_32BIT(key->keyblob + pos)) > key->keyblob_len - pos - 4) {
-        errmsg = "Key blob does not contain a key type string";
+        errmsg = "key blob does not contain a key type string";
         goto error;
     }
     if (len > sizeof(prefix_rsa) - 1 &&
@@ -1232,7 +1243,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
         !memcmp(key->keyblob+pos+4, prefix_dsa, sizeof(prefix_dsa) - 1)) {
         type = DSA;
     } else {
-        errmsg = "Key is of unknown type";
+        errmsg = "key is of unknown type";
         goto error;
     }
     pos += 4+len;
@@ -1242,7 +1253,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
      */
     if (key->keyblob_len < pos+4 ||
         (len = GET_32BIT(key->keyblob + pos)) > key->keyblob_len - pos - 4) {
-        errmsg = "Key blob does not contain a cipher type string";
+        errmsg = "key blob does not contain a cipher type string";
         goto error;
     }
     if (len == 4 && !memcmp(key->keyblob+pos+4, "none", 4))
@@ -1250,7 +1261,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
     else if (len == 8 && !memcmp(key->keyblob+pos+4, "3des-cbc", 8))
         encrypted = 1;
     else {
-        errmsg = "Key encryption is of unknown type";
+        errmsg = "key encryption is of unknown type";
         goto error;
     }
     pos += 4+len;
@@ -1260,13 +1271,13 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
      */
     if (key->keyblob_len < pos+4 ||
         (len = GET_32BIT(key->keyblob + pos)) > key->keyblob_len - pos - 4) {
-        errmsg = "Key blob does not contain actual key data";
+        errmsg = "key blob does not contain actual key data";
         goto error;
     }
     ciphertext = (char *)key->keyblob + pos + 4;
     cipherlen = len;
     if (cipherlen == 0) {
-        errmsg = "Length of key data is zero";
+        errmsg = "length of key data is zero";
         goto error;
     }
 
@@ -1286,7 +1297,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
        unsigned char keybuf[32], iv[8];
 
         if (cipherlen % 8 != 0) {
-            errmsg = "Encrypted part of key is not a multiple of cipher block"
+            errmsg = "encrypted part of key is not a multiple of cipher block"
                 " size";
             goto error;
         }
@@ -1418,6 +1429,7 @@ struct ssh2_userkey *sshcom_read(const Filename *filename, char *passphrase)
     sfree(key->keyblob);
     memset(&key, 0, sizeof(key));
     sfree(key);
+    if (errmsg_p) *errmsg_p = errmsg;
     return ret;
 }
 

diff --git a/ssh.h b/ssh.h
index 0886d45..a557cf0 100644 (file)
--- a/ssh.h
+++ b/ssh.h
@@ -387,9 +387,9 @@ int import_possible(int type);
 int import_target_type(int type);
 int import_encrypted(const Filename *filename, int type, char **comment);
 int import_ssh1(const Filename *filename, int type,
-               struct RSAKey *key, char *passphrase);
+               struct RSAKey *key, char *passphrase, const char **errmsg_p);
 struct ssh2_userkey *import_ssh2(const Filename *filename, int type,
-                                char *passphrase);
+                                char *passphrase, const char **errmsg_p);
 int export_ssh1(const Filename *filename, int type,
                struct RSAKey *key, char *passphrase);
 int export_ssh2(const Filename *filename, int type,

diff --git a/windows/winpgen.c b/windows/winpgen.c
index 7ba88d2..dae9f93 100644 (file)
--- a/windows/winpgen.c
+++ b/windows/winpgen.c
@@ -632,6 +632,7 @@ void load_key_file(HWND hwnd, struct MainDlgState *state,
     int needs_pass;
     int type, realtype;
     int ret;
+    const char *errmsg = NULL;
     char *comment;
     struct PassphraseProcStruct pps;
     struct RSAKey newkey1;
@@ -641,11 +642,11 @@ void load_key_file(HWND hwnd, struct MainDlgState *state,
     if (type != SSH_KEYTYPE_SSH1 &&
        type != SSH_KEYTYPE_SSH2 &&
        !import_possible(type)) {
-       char msg[256];
-       sprintf(msg, "Couldn't load private key (%s)",
-               key_type_to_str(type));
+       char *msg = dupprintf("Couldn't load private key (%s)",
+                             key_type_to_str(type));
        MessageBox(NULL, msg,
                   "PuTTYgen Error", MB_OK | MB_ICONERROR);
+       sfree(msg);
        return;
     }
 
@@ -682,17 +683,17 @@ void load_key_file(HWND hwnd, struct MainDlgState *state,
        if (type == SSH_KEYTYPE_SSH1) {
            if (realtype == type)
                ret = loadrsakey(&filename, &newkey1,
-                                passphrase, NULL);
+                                passphrase, &errmsg);
            else
                ret = import_ssh1(&filename, realtype,
-                                 &newkey1, passphrase);
+                                 &newkey1, passphrase, &errmsg);
        } else {
            if (realtype == type)
                newkey2 = ssh2_load_userkey(&filename,
-                                           passphrase, NULL);
+                                           passphrase, &errmsg);
            else
                newkey2 = import_ssh2(&filename, realtype,
-                                     passphrase);
+                                     passphrase, &errmsg);
            if (newkey2 == SSH2_WRONG_PASSPHRASE)
                ret = -1;
            else if (!newkey2)
@@ -704,8 +705,9 @@ void load_key_file(HWND hwnd, struct MainDlgState *state,
     if (comment)
        sfree(comment);
     if (ret == 0) {
-       MessageBox(NULL, "Couldn't load private key.",
-                  "PuTTYgen Error", MB_OK | MB_ICONERROR);
+       char *msg = dupprintf("Couldn't load private key (%s)", errmsg);
+       MessageBox(NULL, msg, "PuTTYgen Error", MB_OK | MB_ICONERROR);
+       sfree(msg);
     } else if (ret == 1) {
        /*
         * Now update the key controls with all the