Work towards wish `keyfile-diagnostic'. Many sshpubk.c keyfile-loading

functions have sprouted `**errorstr' arguments, which if non-NULL can
return a textual error message. The interface additions are patchy and
ad-hoc since this seemed to suit the style of the existing interfaces.

I've since realised that most of this is masked by sanity-checking that
gets done before these functions are called, but it will at least report
MAC failures and the like (tested on Unix), which was the original point
of the exercise.

Note that not everyone who could be using this information is at the
moment.

[originally from svn r3430]

diff --git a/pageant.c b/pageant.c
index 0ca6b950854ef016320a3bc1d835ee7e7880ffcb..4d69cb5e554e00b329c529bf8b1d6f3545617aa5 100644 (file)
--- a/pageant.c
+++ b/pageant.c
@@ -416,7 +416,7 @@ static void add_keyfile(Filename filename)
        int i, nkeys, bloblen;
 
        if (type == SSH_KEYTYPE_SSH1) {
-           if (!rsakey_pubblob(&filename, &blob, &bloblen)) {
+           if (!rsakey_pubblob(&filename, &blob, &bloblen, NULL)) {
                MessageBox(NULL, "Couldn't load private key.", APPNAME,
                           MB_OK | MB_ICONERROR);
                return;
@@ -424,7 +424,7 @@ static void add_keyfile(Filename filename)
            keylist = get_keylist1();
        } else {
            unsigned char *blob2;
-           blob = ssh2_userkey_loadpub(&filename, NULL, &bloblen);
+           blob = ssh2_userkey_loadpub(&filename, NULL, &bloblen, NULL);
            if (!blob) {
                MessageBox(NULL, "Couldn't load private key.", APPNAME,
                           MB_OK | MB_ICONERROR);
@@ -498,9 +498,9 @@ static void add_keyfile(Filename filename)
        } else
            *passphrase = '\0';
        if (type == SSH_KEYTYPE_SSH1)
-           ret = loadrsakey(&filename, rkey, passphrase);
+           ret = loadrsakey(&filename, rkey, passphrase, NULL);
        else {
-           skey = ssh2_load_userkey(&filename, passphrase);
+           skey = ssh2_load_userkey(&filename, passphrase, NULL);
            if (skey == SSH2_WRONG_PASSPHRASE)
                ret = -1;
            else if (!skey)

diff --git a/puttygen.c b/puttygen.c
index fce91cf55d69a52bf4d35e245f5080137a36076c..3e43b052085e9db87db05ef324680e406006dcdd 100644 (file)
--- a/puttygen.c
+++ b/puttygen.c
@@ -681,14 +681,14 @@ void load_key_file(HWND hwnd, struct MainDlgState *state,
        if (type == SSH_KEYTYPE_SSH1) {
            if (realtype == type)
                ret = loadrsakey(&filename, &newkey1,
-                                passphrase);
+                                passphrase, NULL);
            else
                ret = import_ssh1(&filename, realtype,
                                  &newkey1, passphrase);
        } else {
            if (realtype == type)
                newkey2 = ssh2_load_userkey(&filename,
-                                           passphrase);
+                                           passphrase, NULL);
            else
                newkey2 = import_ssh2(&filename, realtype,
                                      passphrase);

diff --git a/ssh.c b/ssh.c
index 23602f6d5438da1bd9ca41dcef9353eee404afe6..33da9930d2ae2b5e090d75c01a03c21cb12a7253 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -2602,7 +2602,7 @@ static int do_ssh1_login(Ssh ssh, unsigned char *in, int inlen, int ispkt)
     /* Load the public half of ssh->cfg.keyfile so we notice if it's in Pageant */
     if (!filename_is_null(ssh->cfg.keyfile)) {
        if (!rsakey_pubblob(&ssh->cfg.keyfile,
-                           &s->publickey_blob, &s->publickey_bloblen))
+                           &s->publickey_blob, &s->publickey_bloblen, NULL))
            s->publickey_blob = NULL;
     } else
        s->publickey_blob = NULL;
@@ -2888,11 +2888,15 @@ static int do_ssh1_login(Ssh ssh, unsigned char *in, int inlen, int ispkt)
            s->tried_publickey = 1;
            
            {
-               int ret = loadrsakey(&ssh->cfg.keyfile, &s->key, s->password);
+               const char *error = NULL;
+               int ret = loadrsakey(&ssh->cfg.keyfile, &s->key, s->password,
+                                    &error);
                if (ret == 0) {
                    c_write_str(ssh, "Couldn't load private key from ");
                    c_write_str(ssh, filename_to_str(&ssh->cfg.keyfile));
-                   c_write_str(ssh, ".\r\n");
+                   c_write_str(ssh, " (");
+                   c_write_str(ssh, error);
+                   c_write_str(ssh, ").\r\n");
                    continue;          /* go and try password */
                }
                if (ret == -1) {
@@ -4586,7 +4590,7 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt)
            if (keytype == SSH_KEYTYPE_SSH2) {
                s->publickey_blob =
                    ssh2_userkey_loadpub(&ssh->cfg.keyfile, NULL,
-                                        &s->publickey_bloblen);
+                                        &s->publickey_bloblen, NULL);
            } else {
                char *msgbuf;
                logeventf(ssh, "Unable to use this key file (%s)",
@@ -4916,7 +4920,8 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt)
                pub_blob =
                    (unsigned char *)ssh2_userkey_loadpub(&ssh->cfg.keyfile,
                                                          &algorithm,
-                                                         &pub_blob_len);
+                                                         &pub_blob_len,
+                                                         NULL);
                if (pub_blob) {
                    ssh2_pkt_init(ssh, SSH2_MSG_USERAUTH_REQUEST);
                    ssh2_pkt_addstring(ssh, s->username);
@@ -5093,14 +5098,18 @@ static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen, int ispkt)
                 * We have our passphrase. Now try the actual authentication.
                 */
                struct ssh2_userkey *key;
+               const char *error = NULL;
 
-               key = ssh2_load_userkey(&ssh->cfg.keyfile, s->password);
+               key = ssh2_load_userkey(&ssh->cfg.keyfile, s->password,
+                                       &error);
                if (key == SSH2_WRONG_PASSPHRASE || key == NULL) {
                    if (key == SSH2_WRONG_PASSPHRASE) {
                        c_write_str(ssh, "Wrong passphrase\r\n");
                        s->tried_pubkey_config = FALSE;
                    } else {
-                       c_write_str(ssh, "Unable to load private key\r\n");
+                       c_write_str(ssh, "Unable to load private key (");
+                       c_write_str(ssh, error);
+                       c_write_str(ssh, ")\r\n");
                        s->tried_pubkey_config = TRUE;
                    }
                    /* Send a spurious AUTH_NONE to return to the top. */

diff --git a/ssh.h b/ssh.h
index 11f2d19e6afbb4b70bab7315b6a40319d63aba31..3320098bce44f29015e38baeafbb1b79be83e4f4 100644 (file)
--- a/ssh.h
+++ b/ssh.h
@@ -327,9 +327,10 @@ Bignum dh_create_e(void *, int nbits);
 Bignum dh_find_K(void *, Bignum f);
 
 int loadrsakey(const Filename *filename, struct RSAKey *key,
-              char *passphrase);
+              char *passphrase, const char **errorstr);
 int rsakey_encrypted(const Filename *filename, char **comment);
-int rsakey_pubblob(const Filename *filename, void **blob, int *bloblen);
+int rsakey_pubblob(const Filename *filename, void **blob, int *bloblen,
+                  const char **errorstr);
 
 int saversakey(const Filename *filename, struct RSAKey *key, char *passphrase);
 
@@ -344,9 +345,9 @@ extern struct ssh2_userkey ssh2_wrong_passphrase;
 
 int ssh2_userkey_encrypted(const Filename *filename, char **comment);
 struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
-                                      char *passphrase);
+                                      char *passphrase, const char **errorstr);
 char *ssh2_userkey_loadpub(const Filename *filename, char **algorithm,
-                          int *pub_blob_len);
+                          int *pub_blob_len, const char **errorstr);
 int ssh2_save_userkey(const Filename *filename, struct ssh2_userkey *key,
                      char *passphrase);
 

diff --git a/sshpubk.c b/sshpubk.c
index d0594b772eec22468b7707dbada33b2e153968ee..1659aceca8cf5f82cfe639a8766be049b3adfaa2 100644 (file)
--- a/sshpubk.c
+++ b/sshpubk.c
@@ -34,7 +34,8 @@
                           (x)=='/' ? 63 : 0 )
 
 static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
-                          char **commentptr, char *passphrase)
+                          char **commentptr, char *passphrase,
+                          const char **error)
 {
     unsigned char buf[16384];
     unsigned char keybuf[16];
@@ -44,13 +45,18 @@ static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
     struct MD5Context md5c;
     char *comment;
 
+    *error = NULL;
+
     /* Slurp the whole file (minus the header) into a buffer. */
     len = fread(buf, 1, sizeof(buf), fp);
     fclose(fp);
-    if (len < 0 || len == sizeof(buf))
+    if (len < 0 || len == sizeof(buf)) {
+       *error = "error reading file";
        goto end;                      /* file too big or not read */
+    }
 
     i = 0;
+    *error = "file format error";
 
     /*
      * A zero byte. (The signature includes a terminating NUL.)
@@ -98,7 +104,9 @@ static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
     if (key)
        key->comment = comment;
     if (!key) {
-       return ciphertype != 0;
+       ret = ciphertype != 0;
+       *error = NULL;
+       goto end;
     }
 
     /*
@@ -119,6 +127,7 @@ static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
     if (len - i < 4)
        goto end;
     if (buf[i] != buf[i + 2] || buf[i + 1] != buf[i + 3]) {
+       *error = "wrong passphrase";
        ret = -1;
        goto end;
     }
@@ -143,6 +152,7 @@ static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
        goto end;
 
     if (!rsa_verify(key)) {
+       *error = "rsa_verify failed";
        freersakey(key);
        ret = 0;
     } else
@@ -153,28 +163,39 @@ static int loadrsakey_main(FILE * fp, struct RSAKey *key, int pub_only,
     return ret;
 }
 
-int loadrsakey(const Filename *filename, struct RSAKey *key, char *passphrase)
+int loadrsakey(const Filename *filename, struct RSAKey *key, char *passphrase,
+              const char **errorstr)
 {
     FILE *fp;
     char buf[64];
+    int ret = 0;
+    const char *error = NULL;
 
     fp = f_open(*filename, "rb");
-    if (!fp)
-       return 0;                      /* doesn't even exist */
+    if (!fp) {
+       error = "can't open file";
+       goto end;
+    }
 
     /*
      * Read the first line of the file and see if it's a v1 private
      * key file.
      */
     if (fgets(buf, sizeof(buf), fp) && !strcmp(buf, rsa_signature)) {
-       return loadrsakey_main(fp, key, FALSE, NULL, passphrase);
+       ret = loadrsakey_main(fp, key, FALSE, NULL, passphrase, &error);
+       goto end;
     }
 
     /*
      * Otherwise, we have nothing. Return empty-handed.
      */
     fclose(fp);
-    return 0;
+    error = "not an SSH-1 RSA file";
+
+  end:
+    if ((ret != 1) && errorstr)
+       *errorstr = error;
+    return ret;
 }
 
 /*
@@ -195,7 +216,8 @@ int rsakey_encrypted(const Filename *filename, char **comment)
      * key file.
      */
     if (fgets(buf, sizeof(buf), fp) && !strcmp(buf, rsa_signature)) {
-       return loadrsakey_main(fp, NULL, FALSE, comment, NULL);
+       const char *dummy;
+       return loadrsakey_main(fp, NULL, FALSE, comment, NULL, &dummy);
     }
     fclose(fp);
     return 0;                         /* wasn't the right kind of file */
@@ -206,12 +228,14 @@ int rsakey_encrypted(const Filename *filename, char **comment)
  * an RSA key, as given in the agent protocol (modulus bits,
  * exponent, modulus).
  */
-int rsakey_pubblob(const Filename *filename, void **blob, int *bloblen)
+int rsakey_pubblob(const Filename *filename, void **blob, int *bloblen,
+                  const char **errorstr)
 {
     FILE *fp;
     char buf[64];
     struct RSAKey key;
     int ret;
+    const char *error = NULL;
 
     /* Default return if we fail. */
     *blob = NULL;
@@ -219,8 +243,10 @@ int rsakey_pubblob(const Filename *filename, void **blob, int *bloblen)
     ret = 0;
 
     fp = f_open(*filename, "rb");
-    if (!fp)
-       return 0;                      /* doesn't even exist */
+    if (!fp) {
+       error = "can't open file";
+       goto end;
+    }
 
     /*
      * Read the first line of the file and see if it's a v1 private
@@ -228,13 +254,19 @@ int rsakey_pubblob(const Filename *filename, void **blob, int *bloblen)
      */
     if (fgets(buf, sizeof(buf), fp) && !strcmp(buf, rsa_signature)) {
        memset(&key, 0, sizeof(key));
-       if (loadrsakey_main(fp, &key, TRUE, NULL, NULL)) {
+       if (loadrsakey_main(fp, &key, TRUE, NULL, NULL, &error)) {
            *blob = rsa_public_blob(&key, bloblen);
            freersakey(&key);
            ret = 1;
        }
+    } else {
+       error = "not an SSH-1 RSA file";
+        fclose(fp);
     }
-    fclose(fp);
+
+  end:
+    if ((ret != 1) && errorstr)
+       *errorstr = error;
     return ret;
 }
 
@@ -575,7 +607,7 @@ struct ssh2_userkey ssh2_wrong_passphrase = {
 };
 
 struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
-                                      char *passphrase)
+                                      char *passphrase, const char **errorstr)
 {
     FILE *fp;
     char header[40], *b, *encryption, *comment, *mac;
@@ -586,14 +618,17 @@ struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
     int public_blob_len, private_blob_len;
     int i, is_mac, old_fmt;
     int passlen = passphrase ? strlen(passphrase) : 0;
+    const char *error = NULL;
 
     ret = NULL;                               /* return NULL for most errors */
     encryption = comment = mac = NULL;
     public_blob = private_blob = NULL;
 
     fp = f_open(*filename, "rb");
-    if (!fp)
+    if (!fp) {
+       error = "can't open file";
        goto error;
+    }
 
     /* Read the first header line which contains the key type. */
     if (!read_header(fp, header))
@@ -604,8 +639,11 @@ struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
        /* this is an old key file; warn and then continue */
        old_keyfile_warning();
        old_fmt = 1;
-    } else
+    } else {
+       error = "not a PuTTY SSH-2 private key";
        goto error;
+    }
+    error = "file format error";
     if ((b = read_body(fp)) == NULL)
        goto error;
     /* Select key algorithm structure. */
@@ -769,7 +807,12 @@ struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
        if (strcmp(mac, realmac)) {
            /* An incorrect MAC is an unconditional Error if the key is
             * unencrypted. Otherwise, it means Wrong Passphrase. */
-           ret = cipher ? SSH2_WRONG_PASSPHRASE : NULL;
+           if (cipher) {
+               ret = SSH2_WRONG_PASSPHRASE;
+           } else {
+               error = "MAC failed";
+               ret = NULL;
+           }
            goto error;
        }
     }
@@ -787,10 +830,13 @@ struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
        sfree(ret->comment);
        sfree(ret);
        ret = NULL;
+       error = "createkey failed";
+       goto error;
     }
     sfree(public_blob);
     sfree(private_blob);
     sfree(encryption);
+    *errorstr = NULL;
     return ret;
 
     /*
@@ -809,11 +855,13 @@ struct ssh2_userkey *ssh2_load_userkey(const Filename *filename,
        sfree(public_blob);
     if (private_blob)
        sfree(private_blob);
+    if (errorstr)
+       *errorstr = error;
     return ret;
 }
 
 char *ssh2_userkey_loadpub(const Filename *filename, char **algorithm,
-                          int *pub_blob_len)
+                          int *pub_blob_len, const char **errorstr)
 {
     FILE *fp;
     char header[40], *b;
@@ -821,18 +869,24 @@ char *ssh2_userkey_loadpub(const Filename *filename, char **algorithm,
     unsigned char *public_blob;
     int public_blob_len;
     int i;
+    const char *error = NULL;
 
     public_blob = NULL;
 
     fp = f_open(*filename, "rb");
-    if (!fp)
+    if (!fp) {
+       error = "can't open file";
        goto error;
+    }
 
     /* Read the first header line which contains the key type. */
     if (!read_header(fp, header)
        || (0 != strcmp(header, "PuTTY-User-Key-File-2") &&
-           0 != strcmp(header, "PuTTY-User-Key-File-1")))
+           0 != strcmp(header, "PuTTY-User-Key-File-1"))) {
+       error = "not a PuTTY SSH-2 private key";
        goto error;
+    }
+    error = "file format error";
     if ((b = read_body(fp)) == NULL)
        goto error;
     /* Select key algorithm structure. Currently only ssh-rsa. */
@@ -885,6 +939,8 @@ char *ssh2_userkey_loadpub(const Filename *filename, char **algorithm,
        fclose(fp);
     if (public_blob)
        sfree(public_blob);
+    if (errorstr)
+       *errorstr = error;
     return NULL;
 }