Bug in KRL signature verification

Damien Miller djm at mindrot.org
Thu Dec 31 11:30:21 AEDT 2015


On Tue, 29 Dec 2015, Carl Jackson wrote:

> There seems to be at least one more bug with KRL signature verification:
> the first pass loop that gathers signatures terminates after the first
> signature [0]. I think removing the "break" at the end of the loop is
> sufficient to fix this behavior for KRLs signed multiple times.

Thanks for the detailed reports. Here are the fixes that I made - 
I'll commit them shortly. Please let me know if you end up making
any more test KRLs for your work on the Go implementation, I might
add them to our test suite.

-d

Index: krl.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/krl.c,v
retrieving revision 1.36
diff -u -p -r1.36 krl.c
--- krl.c	11 Dec 2015 04:21:12 -0000	1.36
+++ krl.c	30 Dec 2015 23:41:06 -0000
@@ -1013,7 +1013,7 @@ ssh_krl_from_blob(struct sshbuf *buf, st
 		}
 		/* Check signature over entire KRL up to this point */
 		if ((r = sshkey_verify(key, blob, blen,
-		    sshbuf_ptr(buf), sshbuf_len(buf) - sig_off, 0)) != 0)
+		    sshbuf_ptr(buf), sig_off, 0)) != 0)
 			goto out;
 		/* Check if this key has already signed this KRL */
 		for (i = 0; i < nca_used; i++) {
@@ -1034,7 +1034,6 @@ ssh_krl_from_blob(struct sshbuf *buf, st
 		ca_used = tmp_ca_used;
 		ca_used[nca_used++] = key;
 		key = NULL;
-		break;
 	}
 
 	if (sshbuf_len(copy) != 0) {
@@ -1099,7 +1098,7 @@ ssh_krl_from_blob(struct sshbuf *buf, st
 			r = SSH_ERR_INVALID_FORMAT;
 			goto out;
 		}
-		if (sshbuf_len(sect) > 0) {
+		if (sect != NULL && sshbuf_len(sect) > 0) {
 			error("KRL section contains unparsed data");
 			r = SSH_ERR_INVALID_FORMAT;
 			goto out;


More information about the openssh-unix-dev mailing list