Skip to content

Read System.keychain as well as SystemRootCertificates.keychain for MacOS CA Bundle #22701

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 1 commit into from
Feb 9, 2025
Merged

Conversation

dreilly1982
Copy link
Contributor

This adds /Library/Keychains/System.keychain as a keychain to look for trusted CA certficates. Fixes #22700

@alexrp alexrp added this to the 0.14.0 milestone Feb 3, 2025
@alexrp
Copy link
Member

alexrp commented Feb 3, 2025

@FnControlOption would you mind reviewing this since you contributed #14325?

@FnControlOption
Copy link
Contributor

Ran Certificate.Bundle test; seems to work as intended 👍

Amended the commit for a cleaner diff: FnControlOption@e2bbdee

@dreilly1982
Copy link
Contributor Author

I appreciate you cleaning up the diff on that one, I didn't realize just how ugly that looked until the PR was already made. As far as CA Bundles on MacOS are concerned, I think it should eventually read off of any keychain the user has loaded, and I have code that can pull those, but that requires linking against the MacOS frameworks, and I'm not familiar enough with the zig build system (yet) to do that in this context, nor do I know if that is a good idea.

@alexrp alexrp merged commit b3a1101 into ziglang:master Feb 9, 2025
10 checks passed
@squeek502
Copy link
Collaborator

Is it intentional that both keychain files are required after these changes (i.e. if either one can't be read, the function fails)?

Context: https://ziggit.dev/t/zig-fetch-failing-with-discover-remote-git-server-capabilities-certificatebundleloadfailure/8608

@schmee
Copy link
Contributor

schmee commented Feb 16, 2025

@squeek502 possibly related: #22870

@alexrp alexrp mentioned this pull request Feb 17, 2025
@alexrp alexrp removed this from the 0.14.0 milestone Feb 21, 2025
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.

Zig only reads CA Certificates from SystemRootCertificates.keychain and not from System.keychain
5 participants