ssh-keygen segfault (was: Re: Call for testing: OpenSSH 8.2)

[Gabriel Kihlman]

Damien Miller <djm at mindrot.org> writes:

> OpenSSH 8.2p1 is almost ready for release, so we would appreciate testing
> on as many platforms and systems as possible. This is a feature release.

[ .. ]

> openssh-unix-dev at mindrot.org. Security bugs should be reported
> directly to openssh at openssh.com.
>

PIN check when enrolling a key segfaults after three attempts for me,
also if the last entry is actually correct since it asks for the pin at
the end of the for-loop but then exists the for-loop before attempting
to use it:

$ ssh-keygen -t ecdsa-sk -vvvv
Generating public/private ecdsa-sk key pair.
You may need to touch your security key to authorize key generation.
[ .. ]
debug1: ssh_sk_enroll: using device /dev/fido/0
debug1: ssh_sk_enroll: fido_dev_make_cred: FIDO_ERR_PIN_REQUIRED
debug1: sshsk_enroll: provider "internal" returned failure -3
debug1: ssh-sk-helper: Enrollment failed: incorrect passphrase supplied to decrypt private key
[ .. ]
Enter PIN for security key:
[ .. ]
debug1: ssh_sk_enroll: using device /dev/fido/0
debug1: ssh_sk_enroll: fido_dev_make_cred: FIDO_ERR_PIN_INVALID
debug1: sshsk_enroll: provider "internal" returned failure -3
debug1: ssh-sk-helper: Enrollment failed: incorrect passphrase supplied to decrypt private key
[ .. ]
Enter PIN for security key:
[ .. ]
debug1: ssh_sk_enroll: using device /dev/fido/0
debug1: ssh_sk_enroll: fido_dev_make_cred: FIDO_ERR_PIN_INVALID
debug1: sshsk_enroll: provider "internal" returned failure -3
debug1: ssh-sk-helper: Enrollment failed: incorrect passphrase supplied to decrypt private key
[ .. ]
Enter PIN for security key:
Segmentation fault (core dumped)
$

$ gdb --quiet /usr/bin/ssh-keygen ssh-keygen.core 
[ .. ]
#0  sshkey_from_private (k=0x0, pkp=0x7f7ffffe9070) at /usr/src/usr.bin/ssh/ssh-keygen/../sshkey.c:1813
1813            if ((n = sshkey_new(k->type)) == NULL) {
(gdb) p k
$1 = (const struct sshkey *) 0x0
Current language:  auto; currently minimal
(gdb) bt
#0  sshkey_from_private (k=0x0, pkp=0x7f7ffffe9070) at /usr/src/usr.bin/ssh/ssh-keygen/../sshkey.c:1813
#1  0x00000054b0a507aa in main (argc=0, argv=0x7f7f0000000a) at /usr/src/usr.bin/ssh/ssh-keygen/../ssh-keygen.c:3591
(gdb) 

Here is a diff which fixes the segfault and allows you to to have
three pin attempts instead of just two as it was previously:

Index: ssh-keygen.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.c,v
retrieving revision 1.396
diff -u -p -u -r1.396 ssh-keygen.c
--- ssh-keygen.c	4 Feb 2020 09:58:04 -0000	1.396
+++ ssh-keygen.c	6 Feb 2020 17:05:37 -0000
@@ -3563,25 +3563,23 @@ main(int argc, char **argv)
 		passphrase = NULL;
 		if ((attest = sshbuf_new()) == NULL)
 			fatal("sshbuf_new failed");
-		for (i = 0 ; i < 3; i++) {
+		for (i = 0 ; i < 4; i++) {
 			fflush(stdout);
 			r = sshsk_enroll(type, sk_provider, sk_device,
 			    sk_application == NULL ? "ssh:" : sk_application,
 			    sk_user, sk_flags, passphrase, challenge,
 			    &private, attest);
+			if (passphrase != NULL)
+				freezero(passphrase, strlen(passphrase));
 			if (r == 0)
 				break;
 			if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
 				fatal("Key enrollment failed: %s", ssh_err(r));
-			if (passphrase != NULL)
-				freezero(passphrase, strlen(passphrase));
+			if (i == 3)
+				fatal("Too many incorrect PINs");
 			passphrase = read_passphrase("Enter PIN for security "
 			    "key: ", RP_ALLOW_STDIN);
 		}
-		if (passphrase != NULL)
-			freezero(passphrase, strlen(passphrase));
-		if (i > 3)
-			fatal("Too many incorrect PINs");
 		break;
 	default:
 		if ((r = sshkey_generate(type, bits, &private)) != 0)


/gabriel