-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCSP responder certificates are not actually linted, to date (fix needed) #838
Comments
Good catch @defacto64 Right, so this is tricky for quite a few reasons.
We have hundreds of lints for server certificates and one-to-two (that I saw) lints for OCSP certificates. I tried a quick-and-dirty hack that says if l.Source == CABFBaselineRequirements {
_, ocspCertLint := l.Lint().(*cabf_br.OCSPIDPKIXOCSPNocheckExtNotIncludedServerAuth)
if !util.IsServerAuthCert(cert) && !ocspCertLint {
return &LintResult{Status: NA}
}
} This, however, is an import cycle as the Perhaps I will have to think on it further, but off the top of my head in order to get this lint operational our options include.
I will say that I (personally) have no appetite for #1 as it creates a whole new category for, effectively, a single lint. #2 is somewhat more appetizing as I can see it being used by other lints with similar awkward nuances. Something perhaps akin to... type Overrider interface {
LintInterface
OverrideFrameworkCheck(c *x509.Certificate) *LintResult
}
...
...
...
override, ok := l.Lint().(Overrider)
if ok {
result := override.OverrideFrameworkCheck(cert)
if result != nil {
return result
}
} else {
if l.Source == CABFBaselineRequirements && !util.IsServerAuthCert(cert) {
return &LintResult{Status: NA}
}
if l.Source == CABFSMIMEBaselineRequirements && !((util.IsEmailProtectionCert(cert) && util.HasEmailSAN(cert)) || util.IsSMIMEBRCertificate(cert)) {
return &LintResult{Status: NA}
}
} I've hacked up something (with terrible names and little consideration for implications) at #842. |
I think I see how the thing works. Please correct me if I am wrong. |
That is certainly an option! I admit that I didn't put too much thought into the details just yet, just spit balling on ideas that could accomplish the goal without having to tear up too much just for one (or several) lints. type AGoodAndDescriptiveName interface {
func Override()
}
...
...
...
if _, ok := lint.(AGoodAndDescriptiveName); !ok {
// framework check
}
lint.Execute(cert) |
Hey @christopher-henderson, I'm curious what other cases could be solved by this interface check. I'm wondering if a bool or other flag value in the |
@toddgaunt-gs adding a default-false field to CertificateLint is indeed another option. |
Yes, a default-false booean in
Then, the framework check in base.go could be modified like this:
How about that? |
I think I found a problem in an already existing lint, and a more general problem in a core component of Zlint.
Let me start from the more general problem. I think we all agree that it is (would be) a good thing to be able to also lint OCSP responder certificates, since there are specific requirements for them in the CABF baseline requirements (all of them). For a start, an OCSP responder cert must contain the ocsp-nocheck extension, so I was happy to see that there already exists a lint [1] that checks for this. However, I did some tests with real OCSP responder certs, both good and bad, and I found with dismay that that lint is actually never invoked (and therefore it's as if it didn't exist). After a bit of digging, I understood that that no OCSP responder cert is really linted by Zlint, to date, for the simple reason that a lint that refers to the BRs is considered not applicable (NA) if the certificate to be linted is not a ServerAuthCert. This is the consequence of this code excerpt from lint/base.go:
The problem stems from the fact that, normally, an OCSP responder cert does NOT contain the serverAuth EKU (even less the AnyKeyUsage EKU), which is the test done by
util.IsServerAuthCert()
. And besides, that is forbidden since CABF BR 2.0.0.The same check is re-done (uselessly) in the lint [1], and would cause the lint to be skipped regardless of the above issue in lint/base.go.
I think this problem should be fixed soon.
Does anyone have any suggestions? Otherwise I can see if I can come up with something....
[1] https://github.com/zmap/zlint/blob/master/v3/lints/cabf_br/lint_ocsp_id_pkix_ocsp_nocheck_ext_not_included_server_auth.go
The text was updated successfully, but these errors were encountered: