Add custom lints covering our CP/CPS profiles - #8485
Conversation
e0879bd to
cbb7695
Compare
mcpherrinm
left a comment
mcpherrinm
left a comment
There was a problem hiding this comment.
I think this looks pretty good structurally.
I think the biggest concern is I think we could issue a cert for which CheckApplies is false on all lints, and we wouldn't lint it as a result.
My secondary concern is that I think we could make more use of some helper functions to validate each field, as the checks are nearly the same across several lints. I left one comment to that effect but it holds in numerous places.
jsha
left a comment
jsha
left a comment
There was a problem hiding this comment.
Partial review; I'm as far as linter/lints/cpcps/lint_cross_certified_subordinate_ca_certificate.go.
|
I've responded to all comments except for the one about |
|
|
||
| // Execute checks that the given certificate meets the "CheckApplies" criteria | ||
| // of exactly one of the five lints which enforce each of our CP/CPS profiles. | ||
| // https://github.com/letsencrypt/cp-cps/blob/TKTK-replace-with-version-tag/CP-CPS.md?plain=1#L990 |
There was a problem hiding this comment.
Do not resolve this comment until these URLs have been updated with the actual CP/CPS version. This comment blocks merge.
| "1080h", | ||
| "160h" | ||
| ], | ||
| "issuerCerts": [ |
There was a problem hiding this comment.
Do not resolve this comment until IN-12938 has been deployed. This comment blocks merge.
5 participants
Add six new custom zlint lints, each covering one of the profiles listed in Version TKTK of our CP/CPS:
Each lint walks down the corresponding table in our CP/CPS, checking each row in order. Every check includes a quote of the relevant statement and a link to the exact commit and line of our CP/CPS. This will allow us to automated enforcement of updates to our lints when our CP/CPS changes, and vice versa. The meta lint ensures that every certificate qualifies for exactly one of the individual lints, ensuring that we don't issue any certificates which dodge being checked by at least one, and ensuring that we never try to count a cert as meeting multiple profiles.
Several of the items in our CP/CPS profiles state that our certificates either match their issuer certificate, or (for Cross-Certified certs) match a pre-existing certificate. In order to support this, abuse zlint's configuration mechanism to allow us to configure the issuer and pre-existing cert before each lint run. Each of the new lints uses the configured issuer to perform several of its checks, and fails the lint if not so configured.
Fixes #5492
An LLM was used to generate three of the lints in the style of the first two hand-written ones, and to generate some test cases for all five.
Warning
DO NOT MERGE until IN-12938 (configuring issuer certs in cert-checker) is complete.
Warning
DO NOT MERGE until we are confident that we are going to publish a new version of our CP/CPS based on letsencrypt/cp-cps#307. Then, update this PR description and the links within each lint to point at a version tag instead of a commit sha1.