From bd6eadd196d11648e76b0ce3402a236df2317c24 Mon Sep 17 00:00:00 2001 From: Jacob Nevins Date: Sun, 27 Feb 2005 23:01:11 +0000 Subject: [PATCH] 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 [originally from svn r5408] --- cmdgen.c | 15 ++++---- import.c | 94 ++++++++++++++++++++++++++--------------------- ssh.h | 4 +- windows/winpgen.c | 20 +++++----- 4 files changed, 73 insertions(+), 60 deletions(-) diff --git a/cmdgen.c b/cmdgen.c index 5042aa4e..83a158d4 100644 --- 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 d1ab7511..b6d32cb5 100644 --- 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 0886d450..a557cf09 100644 --- 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 7ba88d2a..dae9f93f 100644 --- 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