search

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 enforcing the restrictions on subject DN fields for mailbox validated SMIME certificates #713

Closed robplee closed 1 year ago

robplee commented 1 year ago

This PR is the first to attempt to address something from the big todo list in #712. It's a simple lint enforcing the field MAY/SHALL NOT table for subject DN fields in mailbox validated certificates. I think the changes are fairly straightforward so we should be able to discover if there are any extra things needed for us to start supporting SMIME BR linting.

My current wonder is about the TestCorpus for integration testing as I assume it won't have any applicable certs in it but I guess that's something we can try to work out now?

christopher-henderson commented 1 year ago

My current wonder is about the TestCorpus for integration testing as I assume it won't have any applicable certs in it but I guess that's something we can try to work out now?

@robplee I'll take on this investigation as it's an interesting question. The test corpus from a query against the censys.io database, so I am as yet unsure of whether-or-not these certificates will be available to us. My guess would be no since it's my understanding that Censys get its certificate database by crawling the open internet.

cardonator commented 1 year ago

I believe this PR sets some basic precedents and expectations required to start plowing through #712, so what else needs to be done here do you think? Does this need further review or just testing against a large corpus of SMIME certs? I could try to help run against an internal corpus here to at least get some initial testing results if that's needed before we feel comfortable merging this PR.

christopher-henderson commented 1 year ago

@cardonator

Does this need further review or just testing against a large corpus of SMIME certs?

I have actually been pecking away at this over the past several weeks. We got a new batch of certs 100k+ large from Censys, but it's been a lot of work to format the corpus, deduplicate, sanitize, and smoke that the new failures make sense.

...before we feel comfortable merging this PR.

In the interest of time I think that we would be fine merging this now. If things go truly south in the immediate absence of more test certs then it will be easy to soft "back out" changes by simply not running SMIME checks,

christopher-henderson commented 1 year ago

@cardonator

Does this need further review or just testing against a large corpus of SMIME certs?

I have actually been pecking away at this over the past several weeks. We got a new batch of certs 100k+ large from Censys, but it's been a lot of work to format the corpus, deduplicate, sanitize, and smoke that the new failures make sense.

...before we feel comfortable merging this PR.

In the interest of time I think that we would be fine merging this now. If things go truly south in the immediate absence of more test certs then it will be easy to soft "back out" changes by simply not running SMIME checks,

mtgag commented 1 year ago

Would it be meaningful to change

https://github.com/zmap/zlint/blob/master/v3/lints/cabf_smime_br/mailbox_validated_enforce_subject_field_restrictions.go#L80

return util.IsMailboxValidatedCertificate(c)

to:

return util.IsMailboxValidatedCertificate(c) && util.IsSubscriberCert(c)

because this is under Section 7.1.4.2 which regards subscriber certificates?