Allow for individual lints to opt-out of the ZLint framework executing pre-flight applicability rules

[christopher-henderson]

Addresses #838

The core issue here was CABF places requirements, by-and-large, on server certificates. Thus, the framework did a kindness to CABF lints by baking in a pre-flight IsServerAuth check for all CABF lints.

This however, precluded that ability to lint certificates that are governed by CABF, but are not themselves server certificates. A prime example is v3/lints/cabf_br/lint_ocsp_id_pkix_ocsp_nocheck_ext_not_included_server_auth.go which is a CABF requirement placed on OCSP signing certificates.

As such, we would like for individual lints to be able to opt out of the framework's pre-flight applicability rules when necessary.w

Thank you @defacto64 for reporting the issue and thank you to @toddgaunt-gs for the cleaner boolean flag idea (I had forgotten that we had concrete structs available to us, rather than just interfaces that lints implement).

[christopher-henderson]

These are tests certs that do not have server auth, so they now apply.

[christopher-henderson]

@zakird howdy sir. Could I solicit a quick glance at this PR? It's not PKI related so it's a bit difficult to garnish interest in a review. 🙏

[zakird]

Offhand, this feels like adding in an exception path to bypass fixing scoping of lints rather than fixing that our lints are currently not scoped correctly. Why not instead change the CheckApplies for subscriber certificates to check util.IsServerAuthCert(cert)?

[defacto64]

This PR seems just fine to me. It adds the capability for a lint to do its own pre-flight applicability check, which sometimes can be useful when the general (framework) applicability pre-flight check would unwantedly exclude a certificate from such lint. Relying on util.IsServerAuthCert(cert) it's not the same thing, because that means relying on util.IsServerAuthCert(cert) doing always the "right" thing, which in fact is not always the case. For instance, as I found out myself, without this PR or an equivalent change, there is currently no way to lint an OCSP responder certificate according to the BR. At the same time, I do not think we shoud focus on just solving this particular annoyance, but rather to address the more general need - that is, increase flexibility.

[christopher-henderson]

@zakird ah yes, here's some added context. As @defacto64 brought up in his own experience, there are handful of times where a governing document declares requirements on a classification of certificates that are otherwise normally outside of the document's domain.

Concretely, in this case, CABF/BRs has an odd handful of requirements against OCSP responder certificates. This would be fine were it not for the fact that the framework applies it's own CheckApplies as a courtesy to lints. That is,

Listen, 99.9% of CABF/BR lints are going to be against server certs. So instead of asking every single lint check for that, we are going to apply this condition to every CABF/BR lint.

This is generally great and avoids a lot of mistakes from being made. It does, however, cause edge case certificates (such as the aforementioned OCSP responder certs) to go silently unloved.

[zakird]

Yeah, I'm with you that we'd have to update a huge number of CABF certs. I guess the question is, do we want this to be explicit and update all the lints or have it be implicit with a bypass mechanism. We'd have to do a lot fewer updates to lints, but it does make it more likely that we'll have this bug again in the future if someone doesn't know what they have to opt out of something. In general, ZLint tries to be pretty explicit about everything to avoid this kind of complication, but I also don't know if I'm putting code cleanliness over all practicality. I don't know if I have too strong an opinion. @dadrian what's your take?

[dadrian]

It seems like Source shouldn't define Scope. Might this make more sense as either as new subdirectory for OCSP lints, or a new property on the lint struct used for the CABF lints? I'm not sure it's good to just have a random opt-out for filtering, especially when the filtering is arbitrarily based on source. Instead, we should either make another source, or not base the filter (IsServerAuth) on source.

[christopher-henderson]

To help bring this conversation back to the ground, I'd like to point out that this issue was raised because of precisely one lint that has existed for over four years and had gone unnoticed until @defacto64 read the code base carefully.

So I am quite allergic to altering the fundamental scoping strategy of the project. If there is no solution that even a plurality of folks are comfortable with, then I would sooner leave the issue unresolved than risk major changes.

[defacto64]

Right.
Let's put it this way: do we think it is OK that Zlint cannot currently be used to lint an OCSP certificate for BR compliance?
IMO it is an ugly limitation, but one that is easy to fix in the proposed way or onother.
Besides, the proposed fix is very light, neat, and apparently does not break anything.
The merits of other possible solutions are hard to judge until one is put forward.

[toddgaunt-gs]

Not that anyone asked, but just to chime in I am fine with this solution. I agree with @defacto64 that this is the simplest fix that impacts the codebase the least and it is easily understandable. While we could introduce another field to determine scoping that isn't Source, that would require each lint currently relying on the Source scoping to be updated and would be very similar to this change.

Alternatively we could just add another check to see if the certificate being linted satisfies util.IsDelegatedOCSPResponderCert(c), however its been long enough that I forget if there was a reason we aren't just doing this.