Skip to content
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

Add zlint lints based on our specific CP/CPS #5492

Open
aarongable opened this issue Jun 16, 2021 · 9 comments
Open

Add zlint lints based on our specific CP/CPS #5492

aarongable opened this issue Jun 16, 2021 · 9 comments
Labels
starter Ideal issues for folks getting familiar with Boulder

Comments

@aarongable
Copy link
Contributor

We do pre-issuance linting using zlint to ensure that we are compliant with the baseline requirements and various root program requirements. As Bug 1715455 shows, it is possible to still misissue by being in violation of one's own CP/CPS, rather than in violation of root program requirements. We should consider forking zlint to add a new directory of lints based on the contents of our own CP/CPS.

@cpu
Copy link
Contributor

cpu commented Jun 16, 2021

Do you need to fork ZLint for this? As a library user I think it should be possible to compose a lint.Registry with lints of your own from outside the ZLint src tree that you can use with the same linting interface. If it turns out that there's something that prevents that from working well I'd be interested in trying to help fix it upstream in ZLint. Forking is always an option but I expect other CAs have this use-case too and I think it makes sense to support it upstream.

@aarongable
Copy link
Contributor Author

Yeah, I'd prefer to do it without forking as well. In the period of time between coming up with this idea and filing this bug I took a very quick look at the zlint docs. The section on "Lint Sources" didn't mention bringing your own, nor did the section on "Extending Zlint", so I just assumed there weren't affordances for doing so. But I haven't looked any further than that. I'll definitely take a closer look when we actually try to implement this.

@aarongable
Copy link
Contributor Author

Ooh, I wonder if we could actually store the lints in the CP/CPS repo itself, and then vendor that repo into boulder like any other dependency. That's probably too much of a pain from an automation / CI / ease-of-editing perspective but I kinda like the idea of guaranteeing they're in sync.

@cpu
Copy link
Contributor

cpu commented Jun 16, 2021

Providing out-of-tree lints was a use-case I had in mind when I did the refactoring that allowed excluding categories of lints but I don't think it's been tried in practice (or captured in docs 😓). There might be some bumps in the road but I think it should be achievable 🤞

@aarongable
Copy link
Contributor Author

Turns out this is pretty straightforward. I've created a proof-of-concept PR above, adding the very first lints. Tests and additional lints will be forthcoming.

@aarongable aarongable self-assigned this Jun 30, 2021
@aarongable aarongable added this to the Sprint 2021-06-29 milestone Jun 30, 2021
@aarongable aarongable changed the title Consider forking zlint to add lints based on our specific CP/CPS Add zlint lints based on our specific CP/CPS Jun 30, 2021
aarongable added a commit that referenced this issue Jul 13, 2021
Add a collection of custom lints to enforce that our issuance of
Subscriber (via normal Boulder operation) and Root and Intermediate CA
Certificates (via the Ceremony tool) abides by the requirements we
place on ourselves via our CPS. Provide a small collection of useful
constants for these lints to share. Import all of these lints from our
lint package, so that they are automatically registered with zlint's
`GlobalRegistry` and are automatically included in all of our lint
checks.

At this time, only three lints are included, checking that the validity
periods of our various certificate types do not exceed their CPS-set
maximums. Additional lints for key sizes, distinguished names, key
usages, policy OIDs, AIA URLs, and more will be added in the future.

Part of #5492
@aarongable
Copy link
Contributor Author

aarongable commented Jul 13, 2021

The first version of these lints have landed! Next steps are:

  1. Add more lints. Currently there's just lifetime; we need to do policy OIDs, key sizes, key usages, and more.
  2. Add tests for the lints. The zlint tests use a format I don't like very much (a bajillion hard-coded certs, one or more for every lint) so that'll be a little bit interesting.
  3. Look into moving these lints to the cp-cps repo and then vendoring them in here. I think that would be pretty easy, actually, since none of the lints rely on Boulder common utilities.

@sheurich
Copy link
Contributor

Would you consider moving the lints into configuration files? This would promote cleaner re-use of Boulder with other CA certificates.

@aarongable
Copy link
Contributor Author

All lints can be excluded via existing configuration files. If/when we move the lints into our cp-cps repo, they could easily be substituted via a replace directive in go.mod. Zlint doesn't have a way to dynamically load lints at runtime based on e.g. lists of directories that should be searched for lints, so we can't use configuration files as allowlists, only as blocklists. The only possibility I see would be to extend our current configuration file syntax to allow excluding entire lint sources, but that feels like a low priority at the moment when the number of lints is so small.

@sheurich
Copy link
Contributor

Ah didn't realize the allow list limitation. Thanks 😄

@aarongable aarongable removed their assignment Aug 10, 2021
@aarongable aarongable added starter Ideal issues for folks getting familiar with Boulder and removed help wanted labels Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
starter Ideal issues for folks getting familiar with Boulder
Projects
None yet
Development

No branches or pull requests

4 participants