zmap / zlint

X.509 Certificate Linter focused on Web PKI standards and requirements.
https://zmap.io
Apache License 2.0
353 stars 107 forks source link

Add lint to check for incorrect 'unused' bit encoding in KeyUsages #684

Closed christopher-henderson closed 2 years ago

christopher-henderson commented 2 years ago

At-mentions for @aarongable and @CBonnell since you both helped with #682 so I would appreciate a perfunctory glance at this follow-on lint if you happen have the time and energy.


This lint is a continuation of #682 wherein KeyUsages were found to be incorrectly padded (that is, there are more trailing 0 bits than is declared in the BitString).

As per ITU X.690-0207...

11.2.2 Where ITU-T Rec. X.680 | ISO/IEC 8824-1, 21.7, applies, the bitstring shall have all trailing 0 bits removed before it is encoded.

In this context, what it means for a bit to be "removed" is to declare it as being "unused". For example, the bit field 01011000 has three trailing 0 bits that should be "removed" (that is, declared to be unused).

Test Corpus

There is a large number of failures in the test corpus (over 6500) so it took some digging to confirm that this lint is reasonable.

Let us take the chose text cert as an example (whose ASN1 can be found on crt.sh)

The KU for this certificate is the following... 0x03 0x02 0x05 0x80

Or, in binary... 00000011 00000010 00000101 10000000

What this is saying is: 0x03: This is a bitstring 0x02: That is two bytes long 0x05: And has five unused bits 0x80: Whose contents are 0x80

However, we can see clearly that 0x80 does not have five unused bits... 100 00000 ...rather, it has seven.

Indeed, after a bit of digging through the zcrypto/x509 extension construction logic I did indeed find an old bug wherein the unused bits would always be 0. This was fixed in CL 168990043 and, indeed, the fixed Go stdlib agrees with our notions on what the encoding should be.

As such, it seems as though that there was likely and ASN1 implementation that is widely used that at some point had this KU encoding bug.