Improved error messages if you use the wrong key type: you should

now be told that the key is the wrong type, _and_ what type it is,
rather than being given a blanket `unable to read key file' message.

[originally from svn r1662]

pageant.c

@@ -390,13 +390,14 @@ static void add_keyfile(char *filename)
     int attempts;
     char *comment;
     struct PassphraseProcStruct pps;
-    int ver;
+    int type;
     int original_pass;
 	
-    ver = keyfile_version(filename);
+    type = key_type(filename);
-    if (ver == 0) {
+    if (type != SSH_KEYTYPE_SSH1 && type != SSH_KEYTYPE_SSH2) {
-	MessageBox(NULL, "Couldn't load private key.", APPNAME,
+	char msg[256];
-		   MB_OK | MB_ICONERROR);
+	sprintf(msg, "Couldn't load this key (%s)", key_type_to_str(type));
+	MessageBox(NULL, msg, APPNAME, MB_OK | MB_ICONERROR);
 	return;
     }
 
@@ -409,7 +410,7 @@ static void add_keyfile(char *filename)
 	unsigned char *keylist, *p;
 	int i, nkeys, bloblen;
 
-	if (ver == 1) {
+	if (type == SSH_KEYTYPE_SSH1) {
 	    if (!rsakey_pubblob(filename, &blob, &bloblen)) {
 		MessageBox(NULL, "Couldn't load private key.", APPNAME,
 			   MB_OK | MB_ICONERROR);
@@ -445,7 +446,7 @@ static void add_keyfile(char *filename)
 		    return;
 		}
 		/* Now skip over public blob */
-		if (ver == 1)
+		if (type == SSH_KEYTYPE_SSH1)
 		    p += rsa_public_blob_len(p);
 		else
 		    p += 4 + GET_32BIT(p);
@@ -459,12 +460,12 @@ static void add_keyfile(char *filename)
 	sfree(blob);
     }
 
-    if (ver == 1)
+    if (type == SSH_KEYTYPE_SSH1)
 	needs_pass = rsakey_encrypted(filename, &comment);
     else
 	needs_pass = ssh2_userkey_encrypted(filename, &comment);
     attempts = 0;
-    if (ver == 1)
+    if (type == SSH_KEYTYPE_SSH1)
 	rkey = smalloc(sizeof(*rkey));
     pps.passphrase = passphrase;
     pps.comment = comment;
@@ -484,14 +485,14 @@ static void add_keyfile(char *filename)
 		if (!dlgret) {
 		    if (comment)
 			sfree(comment);
-		    if (ver == 1)
+		    if (type == SSH_KEYTYPE_SSH1)
 			sfree(rkey);
 		    return;		       /* operation cancelled */
 		}
 	    }
 	} else
 	    *passphrase = '\0';
-	if (ver == 1)
+	if (type == SSH_KEYTYPE_SSH1)
 	    ret = loadrsakey(filename, rkey, passphrase);
 	else {
 	    skey = ssh2_load_userkey(filename, passphrase);
@@ -516,11 +517,11 @@ static void add_keyfile(char *filename)
     if (ret == 0) {
 	MessageBox(NULL, "Couldn't load private key.", APPNAME,
 		   MB_OK | MB_ICONERROR);
-	if (ver == 1)
+	if (type == SSH_KEYTYPE_SSH1)
 	    sfree(rkey);
 	return;
     }
-    if (ver == 1) {
+    if (type == SSH_KEYTYPE_SSH1) {
 	if (already_running) {
 	    unsigned char *request, *response;
 	    void *vresponse;

puttygen.c

@@ -846,22 +846,25 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
 		if (prompt_keyfile(hwnd, "Load private key:", filename, 0)) {
 		    char passphrase[PASSPHRASE_MAXLEN];
 		    int needs_pass;
-		    int ver;
+		    int type;
 		    int ret;
 		    char *comment;
 		    struct PassphraseProcStruct pps;
 		    struct RSAKey newkey1;
 		    struct ssh2_userkey *newkey2 = NULL;
 
-		    ver = keyfile_version(filename);
+		    type = key_type(filename);
-		    if (ver == 0) {
+		    if (type != SSH_KEYTYPE_SSH1 && type != SSH_KEYTYPE_SSH2) {
-			MessageBox(NULL, "Couldn't load private key.",
+			char msg[256];
+			sprintf(msg, "Couldn't load private key (%s)",
+				key_type_to_str(type));
+			MessageBox(NULL, msg,
 				   "PuTTYgen Error", MB_OK | MB_ICONERROR);
 			break;
 		    }
 
 		    comment = NULL;
-		    if (ver == 1)
+		    if (type == SSH_KEYTYPE_SSH1)
 			needs_pass = rsakey_encrypted(filename, &comment);
 		    else
 			needs_pass =
@@ -881,7 +884,7 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
 			    }
 			} else
 			    *passphrase = '\0';
-			if (ver == 1)
+			if (type == SSH_KEYTYPE_SSH1)
 			    ret =
 				loadrsakey(filename, &newkey1, passphrase);
 			else {
@@ -918,7 +921,7 @@ static int CALLBACK MainDlgProc(HWND hwnd, UINT msg,
 					   passphrase);
 			    SetDlgItemText(hwnd, IDC_PASSPHRASE2EDIT,
 					   passphrase);
-			    if (ver == 1) {
+			    if (type == SSH_KEYTYPE_SSH1) {
 				char buf[128];
 				char *savecomment;
 

ssh.c

@@ -2430,8 +2430,22 @@ static int do_ssh1_login(unsigned char *in, int inlen, int ispkt)
 	}
 	if (pwpkt_type == SSH1_CMSG_AUTH_RSA) {
 	    char *comment = NULL;
+	    int type;
+	    char msgbuf[256];
 	    if (flags & FLAG_VERBOSE)
 		c_write_str("Trying public key authentication.\r\n");
+	    sprintf(msgbuf, "Trying public key \"%.200s\"", cfg.keyfile);
+	    logevent(msgbuf);
+	    type = key_type(cfg.keyfile);
+	    if (type != SSH_KEYTYPE_SSH1) {
+		sprintf(msgbuf, "Key is of wrong type (%s)",
+			key_type_to_str(type));
+		logevent(msgbuf);
+		c_write_str(msgbuf);
+		c_write_str("\r\n");
+		tried_publickey = 1;
+		continue;
+	    }
 	    if (!rsakey_encrypted(cfg.keyfile, &comment)) {
 		if (flags & FLAG_VERBOSE)
 		    c_write_str("No passphrase required.\r\n");
@@ -4085,8 +4099,21 @@ static void do_ssh2_authconn(unsigned char *in, int inlen, int ispkt)
 	kbd_inter_running = FALSE;
 	/* Load the pub half of cfg.keyfile so we notice if it's in Pageant */
 	if (*cfg.keyfile) {
+	    int keytype;
+	    logeventf("Reading private key file \"%.150s\"", cfg.keyfile);
+	    keytype = key_type(cfg.keyfile);
+	    if (keytype == SSH_KEYTYPE_SSH2)
 		publickey_blob = ssh2_userkey_loadpub(cfg.keyfile, NULL,
 						      &publickey_bloblen);
+	    else {
+		char msgbuf[256];
+		logeventf("Unable to use this key file (%s)",
+			key_type_to_str(keytype));
+		sprintf(msgbuf, "Unable to use key file \"%.150s\" (%s)\r\n",
+			cfg.keyfile, key_type_to_str(keytype));
+		c_write_str(msgbuf);
+		publickey_blob = NULL;
+	    }
 	} else
 	    publickey_blob = NULL;
 
@@ -4345,7 +4372,7 @@ static void do_ssh2_authconn(unsigned char *in, int inlen, int ispkt)
 		}
 	    }
 
-	    if (!method && can_pubkey && *cfg.keyfile
+	    if (!method && can_pubkey && publickey_blob
 		&& !tried_pubkey_config) {
 		unsigned char *pub_blob;
 		char *algorithm, *comment;

ssh.h

@@ -297,7 +297,14 @@ char *ssh2_userkey_loadpub(char *filename, char **algorithm,
 int ssh2_save_userkey(char *filename, struct ssh2_userkey *key,
 		      char *passphrase);
 
-int keyfile_version(char *filename);
+enum {
+    SSH_KEYTYPE_UNOPENABLE,
+    SSH_KEYTYPE_UNKNOWN,
+    SSH_KEYTYPE_SSH1, SSH_KEYTYPE_SSH2,
+    SSH_KEYTYPE_OPENSSH, SSH_KEYTYPE_SSHCOM
+};
+int key_type(char *filename);
+char *key_type_to_str(int type);
 
 void des3_decrypt_pubkey(unsigned char *key, unsigned char *blk, int len);
 void des3_encrypt_pubkey(unsigned char *key, unsigned char *blk, int len);

sshpubk.c

@@ -1122,22 +1122,51 @@ int ssh2_save_userkey(char *filename, struct ssh2_userkey *key,
 }
 
 /* ----------------------------------------------------------------------
- * A function to determine which version of SSH to try on a private
+ * A function to determine the type of a private key file. Returns
- * key file. Returns 0 on failure, 1 or 2 on success.
+ * 0 on failure, 1 or 2 on success.
  */
-int keyfile_version(char *filename)
+int key_type(char *filename)
 {
     FILE *fp;
+    char buf[32];
+    const char putty2_sig[] = "PuTTY-User-Key-File-";
+    const char sshcom_sig[] = "---- BEGIN SSH2 ENCRYPTED PRIVAT";
+    const char openssh_sig[] = "-----BEGIN ";
     int i;
 
     fp = fopen(filename, "r");
     if (!fp)
-	return 0;
+	return SSH_KEYTYPE_UNOPENABLE;
-    i = fgetc(fp);
+    i = fread(buf, 1, sizeof(buf), fp);
     fclose(fp);
-    if (i == 'S')
+    if (i < 0)
-	return 1;		       /* "SSH PRIVATE KEY FORMAT" etc */
+	return SSH_KEYTYPE_UNOPENABLE;
-    if (i == 'P')		       /* "PuTTY-User-Key-File" etc */
+    if (i < 32)
-	return 2;
+	return SSH_KEYTYPE_UNKNOWN;
-    return 0;			       /* unrecognised or EOF */
+    if (!memcmp(buf, rsa_signature, sizeof(rsa_signature)-1))
+	return SSH_KEYTYPE_SSH1;
+    if (!memcmp(buf, putty2_sig, sizeof(putty2_sig)-1))
+	return SSH_KEYTYPE_SSH2;
+    if (!memcmp(buf, openssh_sig, sizeof(openssh_sig)-1))
+	return SSH_KEYTYPE_OPENSSH;
+    if (!memcmp(buf, sshcom_sig, sizeof(sshcom_sig)-1))
+	return SSH_KEYTYPE_SSHCOM;
+    return SSH_KEYTYPE_UNKNOWN;	       /* unrecognised or EOF */
+}
+
+/*
+ * Convert the type word to a string, for `wrong type' error
+ * messages.
+ */
+char *key_type_to_str(int type)
+{
+    switch (type) {
+      case SSH_KEYTYPE_UNOPENABLE: return "unable to open file"; break;
+      case SSH_KEYTYPE_UNKNOWN: return "not a private key"; break;
+      case SSH_KEYTYPE_SSH1: return "SSH1 private key"; break;
+      case SSH_KEYTYPE_SSH2: return "PuTTY SSH2 private key"; break;
+      case SSH_KEYTYPE_OPENSSH: return "OpenSSH SSH2 private key"; break;
+      case SSH_KEYTYPE_SSHCOM: return "ssh.com SSH2 private key"; break;
+      default: return "INTERNAL ERROR"; break;
+    }
 }