OCSP responder certificates are not actually linted, to date (fix needed)

[defacto64]

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:

func (l *CertificateLint) Execute(cert *x509.Certificate, config Configuration) *LintResult {
	if l.Source == CABFBaselineRequirements && !util.IsServerAuthCert(cert) {
		return &LintResult{Status: NA}
	}

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

[christopher-henderson]

Good catch @defacto64

Right, so this is tricky for quite a few reasons.

1. CABF/BR, in general, operates on server certificates so we certainly do not want to remove the check from the framework as this would imply that every individual lint would be responsible for performing this check themselves.
2. The requirement is, itself, quite awkward. It is a requirement on an OCSP certificate in a sea of server certificate requirements.
3. Indeed, if I do a case insensitive grep for ocsp I find lint_ocsp_id_pkix_ocsp_nocheck_ext_not_included_server_auth.go and possibly lint_ocsp_id_pkix_ocsp_nocheck_ext_not_included_server_auth.go.

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 okay, fine, they all have to be server certificates EXCEPT this one lint.

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 cabf_br package refers to the lint package for items such as lint.RegisterLint. lint.Metadata, lint.Result, and so on.

---

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.

• Create a sub-category (cabf_br::ocsp).
• Create a new (optional) interface wherein an individual lint could choose to override the framework's effective check.

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.

[defacto64]

I think I see how the thing works. Please correct me if I am wrong.
If a linter implements the Overrider interface, the framework check is skipped.
However, in this case I understand that the linter can perform its job either within the OverrideFrameworkCheck function itself (by returning a LintResult) or return nil and then do its job within the usual Execute function. Right?
If so, what's the use of this double option? Wouldnt it suffice to just check if the linter implements the Overrider interface, skip the the framework check in case, and let the linter do its job via the Execute function alone, as usual?

[christopher-henderson]

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)

[toddgaunt-gs]

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 CertificateLint struct could accomplish the same thing for this one lint. Something to consider I think unless there is good reason as you've mentioned in a previous comment. The main reason is that I am not seeing that this interface accomplishes anything that having a flag to bypass the NA check and modifying CheckApplies can't accomplish. Having an additional interface and optional method just seems more complicated than its worth to me. Let me know if I am missing anything here, since otherwise it seems like your solution will do the trick.

[christopher-henderson]

@toddgaunt-gs adding a default-false field to CertificateLint is indeed another option.

[defacto64]

Yes, a default-false booean in LintMetadata seems simpler. Could be like follows:

SkipFrameworkCheck bool

Then, the framework check in base.go could be modified like this:

func (l *CertificateLint) Execute(cert *x509.Certificate, config Configuration) *LintResult {
    if !l.SkipFrameworkCheck {
        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}
        }
    }
    lint := l.Lint()
    err := config.MaybeConfigure(lint, l.Name)
    if err != nil {
	return &LintResult{
		Status:  Fatal,
		Details: err.Error()}
    }
    ...and so on...

How about that?