zmap / zlint

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

Handle BMPString properly in e_subject_dn_not_printable_characters #819

Closed cardonator closed 7 months ago

cardonator commented 7 months ago

Looking at #818 , other string types should be allowable as printable in subject fields to conform to RFC 5280.

cardonator commented 7 months ago

I need to get/create certs with matching field data types to add additional tests for this update. However, this does pass the cert referenced in #818 as well as the test certs we created for the original lint a few years ago.

cardonator commented 7 months ago

I noticed in the Integration Test run that the number of certs in the test corpus that fail this lint has dropped from 541 to 34. That seems concerning to me but I'm not sure if it's something to be concerned about. One noteworthy comment here is that it increases the efficacy of this lint for RFC-compliant use cases, but decreases the efficacy for public PKI.

I'll look for feedback from others before deciding how to proceed.

zakird commented 7 months ago

Part of me wonders if we should make it a "community best practice" webpki lint? It doesn't seem to have tripped up any CAs in the last five years? I don't have a strong opinion, but it's an option.

CBonnell commented 7 months ago

SC-62 (the "profiles" ballot) added restrictions on the allowed encodings for DirectoryString attribute value types: https://cabforum.org/working-groups/server/baseline-requirements/requirements/#7142-subject-attribute-encoding.

Specifically, the deprecated BMPString, UniversalString, and TeletexString types are no longer allowed. It's likely more expedient to add this prohibition in a separate lint and forego trying to get the lints for these legacy types correct. Lints for BMPString and UniversalString might be not be too bad to implement, but there will be pain with TeletexString.

zakird commented 7 months ago

@CBonnell's suggestion makes sense to me.

cardonator commented 7 months ago

Does that mean we should add the restrictions and linting from the lint in question to the BR lints and remove it from the RFC lints?

zakird commented 7 months ago

Yes, I think that moving them to a lint that points to the BRs is probably the right move (if one doesn't already exist.)

mtgag commented 7 months ago

I could contribute on the BR lints about allowed encodings and also provide certificates with the disallowed encodings. If that is fine and you are not already on it, I could start working on it.

mtgag commented 7 months ago

I could contribute on the BR lints about allowed encodings and also provide certificates with the disallowed encodings. If that is fine and you are not already on it, I could start working on it.

This is the corresponding PR: https://github.com/zmap/zlint/pull/824

cardonator commented 7 months ago

With #824, I'm thinking we should close this PR, right @christopher-henderson ?