Skip to content

Conversation

@dolmen
Copy link
Contributor

@dolmen dolmen commented Feb 22, 2025

Add Go tests in package embedded to check:

  • validity of each certificate in 1 month (failure)
  • validity of each certificate in 3 months (warning)
  • certificate declared usage (warning)

GH Actions workflow is updated to run the tests.

Note: I had written an earlier version of those checks for gocertifi in certifi/gocertifi#28.

Add checks of validity of the certificates in the near future (1 month from now).
Add checks of the certificates declared usage.
@breml
Copy link
Owner

breml commented Feb 24, 2025

@dolmen thanks for your PR.

I am not yet convinced, that this test is actually helpful. This repo has a recurring job (runs weekly, https://github.com/breml/rootcerts/blob/master/.github/workflows/update.yml#L21), that checks, if there has been a change in the upstream certificates, that are embedded, which are served by the Mozilla Foundation. If the certificates provided by the Mozilla Foundation are no longer up to date (or do contain certificates, that are out of validity), then the wider ecco system does have a serious problem anyway with all the Firefox, Thunderbrid, etc. installations in the wild. We have to trust the Mozilla Foundation to do the right thing. If we don't, the validity is our least problem.

I made sure, that this GH action is kept active and it has run for the past 4 years.

Do I miss something? Where exactly do you see the additional value of this test?

@dolmen
Copy link
Contributor Author

dolmen commented Feb 28, 2025

  1. Those checks add our own level of trust. They include some checks of certificates options (such as usage bits) and we could add more.
  2. The checks include validity checks. If the Mozilla URL stops being updated (this happened in the past when Mozilla switched to Mercurial and their older repo was still online but not updated), the lack of update will trigger a failure.
  3. The checks verify that the certificates bundle works (parsing by crypto/x509 works) with the Go version being tested. We could add a workflow that runs the test with the development build (tip) of Go to detect future failures earlier. We could also add workflows that run the test with various GODEBUG options (ex: x509usepolicies=0).
  4. Last but not least, I have a patch ready to switch the storage from PEM (text) to PER (binary) for a smaller footprint. These tests are an obvious requirements for regression testing.

@breml
Copy link
Owner

breml commented Mar 18, 2025

I looked at this PR and I am fine with merging it.
I have sent you some changes in a PR (looks like you have not allowed the maintainer to update your branch).

@dolmen
Copy link
Contributor Author

dolmen commented Mar 19, 2025

dolmen-go#1

@dolmen
Copy link
Contributor Author

dolmen commented Mar 19, 2025

@breml I've accepted and pushed your changes.

Copy link
Owner

@breml breml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@breml breml merged commit 6cb83c8 into breml:master Mar 19, 2025
1 check passed
@breml
Copy link
Owner

breml commented Apr 14, 2025

@dolmen The check now fails in https://github.com/breml/rootcerts/actions/runs/14425557364/job/40453882801, but the certificate is still valid until May. Can you please have a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants