[patch] Make passphrase-protected SSHv1 keys work again

[Joachim Schipper]

ssh-add on OpenBSD current (with malloc -S enabled) crashes ("chunk is
already free") when loading my password-protected SSHv1 key (used only
for testing). "ssh-add ~/.ssh/identity" also fails to format the prompt
properly ("Enter passphrase for :").

The issue is as follows:

Starting at ssh-add.c:158 in add_file(ac, filename = "~/.ssh/identity"),
we call
key_load_private(filename = "~/.ssh/identity", passphrase = "", commentp = &comment)
key_parse_private_type(blob = <contents of ~/.ssh/identity>, KEY_RSA1, passphrase = "", commentp = &comment)
key_parse_private_rsa1(blob, passphrase = "", commentp = &comment).

In key_parse_private_rsa1, at authfile.c:423-424, we execute

	if (commentp)
		*commentp = buffer_get_string(blob, NULL);

However, the empty passphrase is not correct (recall that my
~/.ssh/identity file has a passphrase), we fail to load the key and
"goto fail" at authfile.c:455?, and execute

fail:
	if (commentp)
		xfree(*commentp);

so when key_parse_private_rsa1 returns NULL (and key_parse_private_type
and key_load_private return the same value), commentp points into
deallocated space. Unfortunately, since commentp != NULL, add_file
assumes that it is valid, uses it and calls xfree at ssh-add.c:462.

Also note that *commentp as allocated at authfile.c:687 leaks.

The patch below reverts a small part of authfile.c r1.86 ("Refactor
internals of private key loading and saving to work on memory buffers
rather than directly on files."), and solves this issue.

		Joachim

Index: authfile.c
===================================================================
RCS file: /usr/cvs/src/src/usr.bin/ssh/authfile.c,v
retrieving revision 1.86
diff -u -p -r1.86 authfile.c
--- authfile.c	21 Nov 2010 10:57:07 -0000	1.86
+++ authfile.c	27 Nov 2010 13:28:27 -0000
@@ -695,7 +695,7 @@ key_load_private(const char *filename, c
 	} else {
 		key_free(pub);
 		prv = key_parse_private_type(&buffer, KEY_RSA1, passphrase,
-		    commentp);
+		    NULL);
 	}
 	buffer_free(&buffer);
 	return prv;