CA: implement CFSSL/zlint pre-issuance linting.#4378
Conversation
This will unblock pre-issuance linting support by updating the `github.com/cloudflare/cfssl` dependency to the `1.3.4` tag which notably includes the zlint integration developed in cloudflare/cfssl#1015
jsha
left a comment
jsha
left a comment
There was a problem hiding this comment.
Generally looks great. I'm curious - why do the lint errors need to be separately marshalled? Are they not serialized in err.Error()?
|
Working on the
@jsha Correct: https://github.com/cloudflare/cfssl/blob/633726f6bcb7574626ae05ae72ca3c8dbc51810f/signer/local/local.go#L137-L140 I thought slamming all the failures into the |
* The change to the `lints.LintStatus` marshaling means we need to use the human readable version for `lint_error_level`. * Both profiles in each CA config need the linting configured. * The `ct_sct_policy_count_unsatisfied` lint needs to be ignored: we're linting pre-certificates which by definition don't have SCTs.
jsha
left a comment
jsha
left a comment
There was a problem hiding this comment.
I see you've added ct_sct_policy_count_unsatisfied to the ignore list, but I think our integration test config always includes 2 SCTs, which is what the lint expects for our 90-day certs, per zmap/zlint#278. Any idea why that lint was triggering?
@jsha The linting is performed on precertificates. The SCTs from the test CT servers are added at the time the signer signs the final certificate with the precert and the SCTs. There's no linting done at that stage because of CFSSL's design. When linting/signing the TBS precert it's expected there are 0 SCTs and so the lint needs to be disabled or it will always fire. |
|
Got it, thanks. Seems like that lint should consider itself not applicable on precertificates - does that make sense to you? If so I'll file a zlint bug (and meanwhile this is fine to merge). |
@jsha That would be a smart fix 👍 Thanks for the idea. I'll pick up that zlint issue once you've filed it. Next week after the lint fix is merged I can revert the ignore of this lint. |
3 participants
The
test/config-nextCA configs are both updated to usezlintto lint TBS pre-certificates with a throw-away key and treat any lint findings >=lints.Passas an error, blocking the CA from signing the TBS pre-cert with its private key.The CA
issuePrecertificateInnerfunction is updated to specifically catch linting related errors from CFSSL to marshal the linting findings to the audit log. A small unit test for this change is included.The CA
IssueCertificateForPrecertificatefunction remains unchanged: the CFSSL interface that definesSignFromPrecertdoesn't facilitate linting. We still lint final certificates post-issuance withcert-checkerand accept the possibility there may be some compliance issues that could occur between the precertificate passing linting and the final certificate being signed.Resolves #4255