Bug in KRL signature verification

Discussion:

(too old to reply)

Carl Jackson

2015-12-29 23:23:52 UTC

I believe there has been a bug in KRL signature verification that has been
present since the KRL feature was first introduced. It prevents signed KRLs
from being loaded by OpenSSH [0]. I believe this bug applies to all
versions of OpenSSH, although the majority of my effort has been devoted to
(and all of my code snippets come from) openssl-portable.

The bug is that an offset is incorrectly treated as a length [2]:

/* 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)
goto out;

"sshbuf_len(buf) - sig_off" should read "sig_off". The result of this bug
is that the number of unparsed bytes after our current parse cursor, rather
than the number of parsed bytes before the cursor, is used as the length of
the data to be verified. I don't believe this bug has any security
implications, though, since both lengths are necessarily smaller than the
length of buf.

Fixing this bug uncovers another bug in ssh_krl_from_blob [3]: "if
(sshbuf_len(sect) > 0)" should read "if (sect != NULL && sshbuf_len(sect) >
0)" (or similar), since a KRL_SECTION_SIGNATURE above might cause sect to
be set to NULL. This bug results in a segmentation fault, but I don't
believe it can be triggered without first fixing the above bug.

In case anyone is interested in testing this behavior out, I believe the
following hex-encoded string to be a spec-compliant [1] signed KRL:

5353484b524c0a00000000010000000043b9a3550000000043b9a3550000000000000000000000000000000001000000ac00000097000000077373682d727361000000030100010000008100aa28507872c5898cc95419d513f42a4b705fa0a212c5c8684d520375d88c5cb0d29cedb572cba9b07475e96e363511e7b0fb585ee9a3cc58db411abc3cd6e7c26d7d042a2ce842bd7af40522798680b9f1a54fdb598a80a170153995c8e3e63d411af5d436fdf2b9675c2e4bb3cc235a03e7f1955386aa5211ba919ecfb4137700000000200000000800000000000024520400000097000000077373682d727361000000030100010000008100aa28507872c5898cc95419d513f42a4b705fa0a212c5c8684d520375d88c5cb0d29cedb572cba9b07475e96e363511e7b0fb585ee9a3cc58db411abc3cd6e7c26d7d042a2ce842bd7af40522798680b9f1a54fdb598a80a170153995c8e3e63d411af5d436fdf2b9675c2e4bb3cc235a03e7f1955386aa5211ba919ecfb413770000008f000000077373682d727361000000801f3e023a97e606f90085bf09c11bc97ac1ef5ae2a8838d294c182f4d5201c3c9ed30d50c7f010a3f3b7aafb01d4b41b3b7afc33769638841c191d6661be995b310db6d4b90f4291a8a69d079d95bd6e9687ae96b4be58154a13f56680439ce4165170!
e219168db
0ade9f4623b674dc4ff99498388b8344628d9662be3b4c75c0

It revokes a single certificate with serial 9298 issued by the following
authority:

ssh-rsa

AAAAB3NzaC1yc2EAAAADAQABAAAAgQCqKFB4csWJjMlUGdUT9CpLcF+gohLFyGhNUgN12IxcsNKc7bVyy6mwdHXpbjY1Eeew+1he6aPMWNtBGrw81ufCbX0EKizoQr169AUieYaAufGlT9tZioChcBU5lcjj5j1BGvXUNv3yuWdcLkuzzCNaA+fxlVOGqlIRupGez7QTdw==

The KRL also has a single signature from this authority.

Carl

[0]: OpenSSH itself never produces signed KRLs, and so far as I've been
able to tell nobody else does either. I found this while adding KRL support
based on the spec [1] to Go (golang)'s x/crypto/ssh package.
[1]:
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/PROTOCOL.krl
[2]:
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/krl.c#L1010-L1019
[3]:
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/krl.c#L1095-L1104

Carl Jackson

2015-12-30 02:57:50 UTC

Permalink

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.

Carl

[0]:
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/krl.c#L1039

Post by Carl Jackson
I believe there has been a bug in KRL signature verification that has been
present since the KRL feature was first introduced. It prevents signed KRLs
from being loaded by OpenSSH [0]. I believe this bug applies to all
versions of OpenSSH, although the majority of my effort has been devoted to
(and all of my code snippets come from) openssl-portable.
/* 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)
goto out;

5353484b524c0a00000000010000000043b9a3550000000043b9a3550000000000000000000000000000000001000000ac00000097000000077373682d727361000000030100010000008100aa28507872c5898cc95419d513f42a4b705fa0a212c5c8684d520375d88c5cb0d29cedb572cba9b07475e96e363511e7b0fb585ee9a3cc58db411abc3cd6e7c26d7d042a2ce842bd7af40522798680b9f1a54fdb598a80a170153995c8e3e63d411af5d436fdf2b9675c2e4bb3cc235a03e7f1955386aa5211ba919ecfb4137700000000200000000800000000000024520400000097000000077373682d727361000000030100010000008100aa28507872c5898cc95419d513f42a4b705fa0a212c5c8684d520375d88c5cb0d29cedb572cba9b07475e96e363511e7b0fb585ee9a3cc58db411abc3cd6e7c26d7d042a2ce842bd7af40522798680b9f1a54fdb598a80a170153995c8e3e63d411af5d436fdf2b9675c2e4bb3cc235a03e7f1955386aa5211ba919ecfb413770000008f000000077373682d727361000000801f3e023a97e606f90085bf09c11bc97ac1ef5ae2a8838d294c182f4d5201c3c9ed30d50c7f010a3f3b7aafb01d4b41b3b7afc33769638841c191d6661be995b310db6d4b90f4291a8a69d079d95bd6e9687ae96b4be58154a13f56680439ce4165!

170e21916
8db0ade9f4623b674dc4ff99498388b8344628d9662be3b4c75c0

Post by Carl Jackson
It revokes a single certificate with serial 9298 issued by the following
ssh-rsa

The KRL also has a single signature from this authority.
Carl
[0]: OpenSSH itself never produces signed KRLs, and so far as I've been
able to tell nobody else does either. I found this while adding KRL support
based on the spec [1] to Go (golang)'s x/crypto/ssh package.
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/PROTOCOL.krl
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/krl.c#L1010-L1019
https://github.com/openssh/openssh-portable/blob/271df8185d9689b3fb0523f58514481b858f6843/krl.c#L1095-L1104

Damien Miller

2015-12-31 00:30:21 UTC

Permalink

Post by Carl Jackson
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;