If I'm not mistaken, this first if is meant to include certificates that have no value in their EKU extension, or more precisely that do not have the EKU extension (since this extension cannot be empty). This is correct, in a way, because if a cert does not specify an EKU, it maybe a certificate to which the CABF BRs are applicable (if so, it's likely a CA certificate). However, if this condition is verified in that way (see the above excerpt), sometimes an incorrect result will be obtained, because the ExtKeyUsage field of the Certificate struct does not contain the complete list of keypurposes present in the EKU extension, but only the list of keypurposes that are found in the EKU extension AND are known to the zcrypto/x509 package. If the certificate contains an "unknown" keypurpose in the EKU, this is inserted in a different field of the Certificate: UnknownExtKeyUsage. Consequently, to correctly verify that the certificate under examination does not have the EKU extension, it is not sufficient to check that the length of the ExtKeyUsage field is zero: it is necessary to check that the length of the UnknownExtKeyUsage field is also zero. Alternatively we could simply use util.IsExtInCert(c, util.EkuSynOid) but I suppose this would be slightly more CPU-intensive.
Due to the way the first if is currently constructed, various types of non-TLS certificates (containing EKUs unknown to the zcrypto/x509 package) have so far been incorrectly considered candidates for CABF_BR-based linting. IMO this should be fixed via the proposed change to util/ca.go (and consequently, to config.json) or an equivalent one.
I think the first
ifin the following excerpt is subtly wrong:https://github.com/zmap/zlint/blob/ae8d59405f1926eb418d496cd0415b8a4fa88e04/v3/util/ca.go#L54-L57
If I'm not mistaken, this first
ifis meant to include certificates that have no value in their EKU extension, or more precisely that do not have the EKU extension (since this extension cannot be empty). This is correct, in a way, because if a cert does not specify an EKU, it maybe a certificate to which the CABF BRs are applicable (if so, it's likely a CA certificate). However, if this condition is verified in that way (see the above excerpt), sometimes an incorrect result will be obtained, because the ExtKeyUsage field of the Certificate struct does not contain the complete list of keypurposes present in the EKU extension, but only the list of keypurposes that are found in the EKU extension AND are known to the zcrypto/x509 package. If the certificate contains an "unknown" keypurpose in the EKU, this is inserted in a different field of the Certificate: UnknownExtKeyUsage. Consequently, to correctly verify that the certificate under examination does not have the EKU extension, it is not sufficient to check that the length of the ExtKeyUsage field is zero: it is necessary to check that the length of the UnknownExtKeyUsage field is also zero. Alternatively we could simply use util.IsExtInCert(c, util.EkuSynOid) but I suppose this would be slightly more CPU-intensive.Due to the way the first
ifis currently constructed, various types of non-TLS certificates (containing EKUs unknown to the zcrypto/x509 package) have so far been incorrectly considered candidates for CABF_BR-based linting. IMO this should be fixed via the proposed change to util/ca.go (and consequently, to config.json) or an equivalent one.