search

zmap / zlint

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

Add lint to check for a valid business category in EV certificates #830

Open defacto64 opened 2 months ago

defacto64 commented 2 months ago

Please add this lint to check for a valid business category in EV certificates, as per EVG 9.2.3

defacto64 commented 2 months ago

Cannot understand why "integration-test" fails....

XolphinMartijn commented 2 months ago

Cannot understand why "integration-test" fails....

It looks like 11306 certs in the test data-set are returning on error on this linter. If that number of certs hitting this error is indeed correct, config.json needs to be updated to reflect this expected error count

defacto64 commented 2 months ago

That's quite strange and unexpected. I have checked one of the offending certficates (whose fingerprint I found in the integration-test log) and it makes zlint crash: https://crt.sh/?q=4712f1b2a94994b55626ecba2104bbf23d39c05e7a2751e5af8a923bac23fd8f I mean, even the current official version of zlint.... I am quite perplexed....

defacto64 commented 2 months ago

Look at this: how come? image

robstradling commented 2 months ago

@defacto64 Running that certificate through pkilint's lint_cabf_serverauth_cert linter sheds some light on this "crash":

DecodingValidator @ certificate.tbsCertificate.extensions.9.extnValue.qCStatements.2
    itu.invalid_asn1_syntax (FATAL): ASN.1 decoding failure occurred at "certificate.tbsCertificate.extensions.9.extnValue.qCStatements.2" with schema "QcType", OID 0.4.0.1862.1.6: Value node is absent, but the ASN.1 schema specifies that it must be present

Zlint doesn't "crash" if the 0.4.0.1862.1.6 statement is removed from the cert. (I tried this just now with the help of der-ascii).

defacto64 commented 2 months ago

Thanks Rob, that's interesting, but I don't understand what this has to do with my lint...and the fact that "integration-test" fails....

XolphinMartijn commented 2 months ago

@defacto64 I don't think it is. https://github.com/zmap/zlint/actions/runs/8613740876/job/23605682791 flags the same certificate as FATAL, but that is just a warning in the log, just as during the run on your PR.

I just ran the lint locally against a modified zlint version, which spits out the offending thumbprints. It seems to believe https://crt.sh/?q=1de8b31449944cdd6f5657af900a488c517246a474e6500832621dc058e9aea0 has a bad businessCategory

defacto64 commented 2 months ago

Hmmm... I fixed that typo, but there must still be something wrong somewhere.

XolphinMartijn commented 2 months ago

Hmmm... I fixed that typo, but there must still be something wrong somewhere.

Correct, I think my second comment went away. Some of the test certs have different capitalization on the strings. I'm not sure if that actually is a violation or not though

defacto64 commented 2 months ago

Oh, I assumed that capitalization is meaningful. The EVGs at 9.2.3 are quite clear on this, IMO: This field MUST contain one of the following strings: “Private Organization”, “Government Entity”, “Business Entity”, or “Non‐Commercial Entity

defacto64 commented 2 months ago

And, if I am not mistaken, also pkilint only allows the case-sensitive versions of those four strings....

XolphinMartijn commented 2 months ago

Makes sense! But than you'll need to update config.json to incorporate the error count.

I can make a suggestion once back at my machine

XolphinMartijn commented 2 months ago

@defacto64 I suspect https://github.com/defacto64/zlint/pull/1 will solve this

cardonator commented 2 months ago

Thanks for creating this lint!

defacto64 commented 2 months ago

This morning I positively verified that pkilint reports ERROR on a businessCategory that has one of the values expected by EVG 9.2.3 but in all capital letters (e.g. "PRIVATE ORGANIZATION"), so I think we are quite confident in the interpretation of 9.2.3. At this point I suppose that in the corpus of test certificates used in the 'integration-test' there are EV certificates with businessCategory in capital letters and for which, however, an error is not expected. Right now I am not sure how to fix this problem, maybe I need a hand from Chris? Any suggestions welcome.

XolphinMartijn commented 2 months ago

This morning I positively verified that pkilint reports ERROR on a businessCategory that has one of the values expected by EVG 9.2.3 but in all capital letters (e.g. "PRIVATE ORGANIZATION"), so I think we are quite confident in the interpretation of 9.2.3. At this point I suppose that in the corpus of test certificates used in the 'integration-test' there are EV certificates with businessCategory in capital letters and for which, however, an error is not expected. Right now I am not sure how to fix this problem, maybe I need a hand from Chris? Any suggestions welcome.

@defacto64 Please see https://github.com/defacto64/zlint/pull/1 which I filed against your repo. It should solve this I would imagine

defacto64 commented 2 months ago

Unfortunately, it seems not, Martijn.

defacto64 commented 2 months ago

Εύρηκα!

defacto64 commented 1 month ago

A recent incident on https://bugzilla.mozilla.org corroborates that businessCategory in EV certificates is a case-sensitive attribute at least from one major certificate consumer's point ot view, and based on what is by far the commonest practice in the industry, therefore I think it would be good to have this lint - to mitigate the risk that similar incidents re-occur.