Skip to content

Update all test certificates and add maintainence documentation #190

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

Merged
merged 7 commits into from
Aug 13, 2025

Conversation

complexspaces
Copy link
Collaborator

This PR does several things, with the goal of both unbreaking CI today and making future maintenance easier. These include:

  • Reducing the number of chains we need to maintain in the test data
  • Simplifying tests with less constants
  • Adding documentation for the test certificate update process
  • Switching away from 1Password-related certificate chains to ones for AWS themselves. I did this for two reasons:
    • Trying to special-case 1Password less in the repository since its now under the rustls org instead.
    • Earlier this year I learned that we (1Password) have not yet enabled TLS 1.3 support for our production servers, so the certificate update script wasn't able to communicate with my.1password.com because it only supports TLS 1.3 by default. I could have just enabled the tls12 feature but it seemed cleaner to just part with it and use a web server that has its TLS configurations updated more often.
    • Updating the real world and mock certificates, as both have since expired this summer.

Commit-by-commit review is recommended.

Going forwards we will just use another pre-existing
chain for unrelated name tests to reduce the maintainence burden.
Given that, we don't need to keep this chain around or test
that its valid for itself anymore.
@complexspaces complexspaces requested a review from djc August 13, 2025 19:47
@complexspaces complexspaces force-pushed the cert-tests-maintainence branch from 33d916f to b329fa6 Compare August 13, 2025 19:54
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for getting this done!

@complexspaces
Copy link
Collaborator Author

Thanks for the fast approval! Please bear with me as I get the various platform-specific compilation failures fixed 😅.

@complexspaces complexspaces force-pushed the cert-tests-maintainence branch from b329fa6 to 52c9270 Compare August 13, 2025 20:06
@complexspaces complexspaces force-pushed the cert-tests-maintainence branch from c1f1b3b to ade5bb0 Compare August 13, 2025 20:50
@complexspaces
Copy link
Collaborator Author

@djc Do you mind double checking my approach in ade5bb0? I'm not super happy with it but we haven't yet worked out #179 and we need to do something to get CI passing again at minimum.

@djc
Copy link
Member

djc commented Aug 13, 2025

It seems reasonable-ish at least!

@complexspaces complexspaces merged commit 042330d into main Aug 13, 2025
21 checks passed
@complexspaces complexspaces deleted the cert-tests-maintainence branch August 13, 2025 21:16
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.

3 participants