-
Notifications
You must be signed in to change notification settings - Fork 108
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
RFC for Configurable Lints #624
Comments
Just ACKing that I have seen it, I feel bad for not having meaningfully commented yet, and that I'm still noodling this (in part, to think of counterfactuals or edge cases to sort through) |
It's good @sleevi, there's a fair amount here. And regardless I wanted to figure out that the "higher scoped configurations" problem was at least technically feasible without being too filthy with reflection. I think I have some reasonable enough code local that I'm comfortable pitching the idea as something that can actually be delivered reasonably well. |
Howdy folks @sleevi @cpu @robplee @zakird. I've reached a point where I have enough of a complete working example on my local of the above proposal that I am comfortable in taking the My goal is for this to be a non-breaking change, however it is a large change with user impact for both consumers of the CLI tool as well as lint authors. As such, I am eyeing getting this proposal out onto the mailing list by the end of the month so that I may solicit UX feedback from the community'/s stakeholders. I would appreciate a quick scan for anything that is glaringly bad or obtuse before doing so. |
@christopher-henderson Thanks for the long - and detailed - comment on the design! I'll be honest, I'm not too familiar with the Go idioms to be able to usefully comment on the implementation strategy, so this may feel a bit of 'high level' feedback without much concrete actionable, directed change. I definitely think getting feedback from CAs is useful, and I think the most useful feedback from them is from the sort of ergonomic example you mentioned re: #619 . In the W3C, we use the term for this of explainers, to sort of capture "problems to solve" / "possible shapes of solution" and use that to iterate on the understanding, before drilling down into the implementation details. To the topic of ergonomics, I think we should look at existing CA software, to see how it's likely integrated. Far and away, the most common CA software implementation seems to be EJBCA in some variation. EJBCA has a configuration known as post-processing validators. Validators can be configured either for all certificates issued from a given CA, or only for particular certificates for a given profile. So when we think about something like #619, it's likely to imagine that there is a "Subordinate CA Certificate" profile existing under the Root Certificate. And this profile will be used for multiple sub-CAs. As such, I'm a little concerned that a lint design like that outlined would be difficult - you wouldn't want to create a configuration-per-cert you're issuing, but instead want something dynamic. For example, for this lint to be useful, we'd probably want something like a filename of "all (existing) sub-CAs", such that This suggests that we need a signal mechanism for invalid configurations - e.g. what if the file can't be loaded? But, naively, the design I suggest above also seems to have a problem: it fails open (i.e. if no matching certificate is found, it assumes it's not cross-signed) rather than fails closed (i.e. requires explicit confirmation/manual review that the condition holds). Iterating on that design, then, it seems like a lint for #619 would say require that the configuration for this lint might be a list of (encoded) DNs, and an affirmative statement by the CA as to which state the DN is in (i.e. "This DN is not cross-certified" vs "This DN is cross-certified"). Any attempt to issue a new DN would cause the lint to fail (for a non-allowlisted DN), which would then prompt a manual review, and a manual reconfiguration of the lint, to explicitly state which it was. For something like #491 , though, we might do something similar to what you proposed: for example, because we assume responders will only be for the CAs own certificate, have a file that contains every CA certificate from a given CA organization, and then the idea being the responder-validating lint could look up in that file to see all the issuers that match, see if any have the Taking a step back, then, from these detailed cases, I guess my questions are:
|
Thank you @sleevi! The
It sounds like you are referring to semantic validation, right? As in, That is, we have the following lint statuses available to us: const (
// Unused / unset LintStatus
Reserved LintStatus = 0
// Not Applicable
NA LintStatus = 1
// Not Effective
NE LintStatus = 2
Pass LintStatus = 3
Notice LintStatus = 4
Warn LintStatus = 5
Error LintStatus = 6
Fatal LintStatus = 7
) Of which I believe type MyLint struct {
config MyConfiguration
}
type MyConfiguration struct {
subordinate string
}
func (l *MyLint) Execute(cert *x509.Certificate) {
bytes, err := ioutil.ReadAll(l.config.subordinate)
if err != nil {
return &lint.LintResult{Status: lint.Fatal, Details: "I don't think so, Tim"}
}
} This could extend out to pretty much any ad-hoc validation error that could occur (parse failures, certificates that appear to be completely unrelated to the lint, stringly typed validation errors, and so on).
In my mind, it would be easy enough to build configurations that can apply to a whole cache of certificates. [some_lint]
web_pki = false [some_lint]
web_pki = true
You can even express hierarchies by concatenating/merging TOML documents. Let's take the following where some certs are intended for the web PKI but are not cross signed, and some certs are both intended for the web PKI and are cross signed.
Let's say that [some_lint]
web_pki = true ..and that [some_other_lint]
cross_signed_by = "/path/to/cert.crt" Well, the simple concatenation of those two config files is itself a valid config file. [some_lint]
web_pki = true
[some_other_lint]
cross_signed_by = "/path/to/cert.crt" In that way you can achieve a hierarchy through some simple bash scripting. MERGED=$(mktemp)
cat web_pki/config.toml web_pki/cross_signed/config.toml > "${MERGED}"
for cert in web_pki/cross_signed/*.crt; do
zlint --config "${MERGED}" "${cert}"
done Alternatively, zlint --configList confg1.toml,config2.toml,config3.toml cert.crt |
It sounds like you are referring to semantic validation, right? As in, yeah,
you gave me a file path, but I'm telling you it's just not there?
Yes. Just trying to think of the ergonomics for how misconfiguration is
signaled, and minimizing the risk of “fail open” implementation errors. I
suppose dozens of lints failing is a useful signal.
func (l *MyLint) Execute(cert *x509.Certificate) {
bytes, err := ioutil.ReadAll(l.config.subordinate)
if err != nil {
return &lint.LintResult{Status: lint.Fatal, Details: "I don't think so, Tim"}
}
}
Right; the alternative is an explicit method (like Configure or Validate)
that checks that. Not saying I’m opposed, just making sure to discuss the
design and make sure we’re not overlooking anything.
In my mind, it would be easy enough to build configurations that can apply
to a whole cache of certificates.
Is this the right way to be thinking about how linting works? That is, our
two main use cases for linting are CAs doing pre-issuance lints and
observers doing post-issuance lints.
I mentioned the EJBCA design, to make sure we’ve got the right mental model
here: issuance profiles, which are associated with “logical” CAs, and each
logical CA may have many CA certificates (with the same Subject/SPKI).
We have lints configured for all certificates from a given CA, and lints
that may be associated with a given profile. Linters are generally
configured via command-line parameters.
The filesystem layout approach seems very error prone, and implicit
configuration (the existence or absence of a file) rather than explicit
configuration.
Well, the simple concatenation of those two config files is itself a valid
config file.
[some_lint]
web_pki = true
[some_other_lint]
cross_signed_by = "/path/to/cert.crt"
This seems like a dangerous design pattern - for example, if I concatenate
two files, one with web_pki = true, with web_pki = false, the behavior
there is going to determine on the order of concatenation, as well as
whether or not it uses first or last value. JSON libraries at least have
this problem in spades - I would be shocked if TOML doesn’t as well.
Alternatively, zlint can always take in a list of configurations and merge
them internally.
zlint --configList confg1.toml,config2.toml,config3.toml cert.crt
Yeah, my gut is either we explicitly support multiple configurations,
which means defining things predictably (and presumably, the ability to
dump/debug the logical view), or we just KISS and say you need to merge all
of this beforehand.
I’d be useful to hear from some of the CAs if they have a preference. I
lean towards KISS, even though it does mean greater work for CAs to
reconfigure lints; hopefully they have good processes in place by now for
that.
… |
Well perhaps change the word
You are absolutely right in that duplicate keys/sections become very difficult to reason with regard to which one would take affect. If I were using this system I would do what you mention, which seems to be simply writing your different configuration files for different classes of certs, be done with it, and go on vacation. I suppose that I was trying to show was that operators could get clever with this should they find the need, although I would not necessarily recommend doing so. |
Purpose
Today, the only facts that are provided to ZLint are those that are present within the target x509 certificate. That is to say, the only input to the linter is the certificate itself. This works fairly well, in general, due to the fact that most requirements refer to data encoded within the certificate itself.
This is not, however, universally true. A lint may require access to the full certificate chain in order to deterministically come to a conclusion (#491, Ocsp eku check for non tls certificates). Or language may specify that the requirement only takes effect if certain conditions regarding the certificate's cross signer are true (#619, Lint that the EKU values within a SubCA certificate comply with CABF_BR 7.1.2.2 letter g)). And ETSI lints have had a history of being inaccurate due to requiring outside data (#581, Bug: ESI lints are not consistent with ETSI EN 319 412-5 requirements regarding non-qualified certificates).
If the certificate in isolation is in its self insufficient for the accurate execution of a given lint, then it seems that additional context must be shuttled to the lint in some way.
Proposal
This proposal is to introduce a second, optional, input file into the ZLint command line interface which represents a configuration/context for that individual run. A supporting framework is also proposed for lints to declare an interest in arbitrary configurations.
Goals
Code Interface
Lints may implement a new, optional, interface -
Configurable
......where the returned
interface{}
is a pointer to the target struct to deserialize your configuration into.This struct may encode any arbitrary data that may be deserialized from TOML. Examples may include:
...and so on. How stable and/or appropriate a given configuration field is is left as a code review exercise on a per-lint basis.
If a lint is
Configurable
then a new step is injected at the beginning of its lifecycle.Non-Configurable Lifecycle
Configurable Lifecycle
Example Lints
Contrived Examples
The following is a set of complete examples showcasing how one might write lints to utilize this configuration framework. They are contrived and solve no real problem in-and-of themselves. They are merely demonstrative.
The following lint encodes its configuration within the
struct
that is the lint itself. The lint itself is rather silly in that it is asserting that the signing certificate authority must play certain games while on a road trip - information that is clearly not encoded in any certificate ever issued.The following lint takes on a different strategy of aggregating a configurable
struct
rather than being the targetstruct
itself.Given the above lints, operators may then generate a sample configuration which contains the defaults for every lint that is configurable.
Note that comments declared in the provided
structs
ARE present within example configuration generated by for operators. As such, lint authors SHOULD provide comments describing the purposes of their fields as well as the impact that those fields have on their given lint.Higher Scoped Configurations
Lints may embed within theselves either pointers or structs to the following definitions.
Doing so allows for the reuse of common configurations among entire classes of lints.
Lints receive copies of higher scoped configurations (even in the event of embedding a pointer) in order to avoid surprising mutations across many lints.
Live Example
The following example implementation is derived from #619. It serves as real world, and immediate, usecase for these new configuration facilities. There may be, however, a more stable and/or correct implementation - this is merely serving as a motivating example.
This structure generates the following configuration.
With the cross signer certificate filled in...
Example CLI Usages
Operators will be able to generate a configuration file which contains all available configurations with ZLint initialized to their defualt values.
Retrieving an Example Configuration
Running Against a Configuration.
Running against a configuration is as simple as pointing the
-config
flag to a file path.Testing
The linting
test
package provides the convenience functiontest.TestLint(lintName string, testCertFilename string)
.In addition to the above, a new facility shall be added that allows for the execution of a test against an arbitrary TOML string -
test.TestLintWithCtx(lintName string, testCertFilename string, context string)
.Choice of TOML
vs. JSON
JSON does not allow for comments in its documents. This alone is the primary disqualifier.
Comments from lint authors to CAs are very important for conveying the purpose and impact of individual fields. Additionally, it is likely that CAs will version control (and possibly even template) their own configurations, in which case it is an attractive proposition that CAs be able to leave comments in their internal configurations in order to track their own policy decisions.
vs. YAML
YAML allows for comments. However, it is a far richer (and thus far more complex) lanuage than TOML is. It is not uncommon for humans to have difficulty writing and editing YAML by hand.
Should use cases arise such that the complexity of YAML becomes desirable then it may be a consideration for usage.
Noted Dissatisfactions
Long Single Line Comments
Lint authors provide comments in their configurations via the
comment:"<text>"
Go tag. Unfortunately, Go does not allow for multiline tags nor does it allow for usages ofconst
orvar
strings within tags - all tags must be string literals. This means that, as is, it is very likely that the majority of comments are going to break code style rules simply by being even reasonably descriptive. This is seen immediately in the example provided in Live Example.There may be remediations to this. One is to have an implicit, reflective, interface.
<Field>Comment
that returns a string.However, spike work has not yet been done on accomplishing an interface such as this. As such it is not yet known how conducive our TOML framework would be towards being extended in such a way.
Manual Certificate Parsing
The TOML serde framework does not have a notion of x509 certificates and, as such, lint authors are backed into the wall of declaring
string
fields and documenting that they MUST be properly formatted PEMs and then do the parsing and error handling themselves. This is seen in the Live Example.Possible remediations include:
struct
that both knows how to deserialize from TOML and how to convert itself into a*x509.Certificate
.utils
facilites.The text was updated successfully, but these errors were encountered: