Skip to content

Don't error when initializing LibGit2 with CA roots path #56924

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
Mar 25, 2025

Conversation

visr
Copy link
Contributor

@visr visr commented Dec 31, 2024

When SSL_CERT_FILE or SSL_CERT_DIR is set, it is impossible to set this location in LibGit2_jll on Apple and Windows because it isn't built with support for that. Until now we've errored out with a message telling users to set JULIA_SSL_CA_ROOTS_PATH to an empty string, which is a somewhat problematic workaround because the Windows environment variables UI doesn't allow empty values, and setting it to an empty string from PowerShell unsets it. This PR changes the behavior to allow this expected error.

Variables like SSL_CERT_FILE are for instance set by the Conda OpenSSL package on environment activation used by e.g. Python, ensuring many people cannot use Pkg operations that use LibGit2, like dev Example, add Example#master. See more user reports on Discourse.

Together with JuliaLang/NetworkOptions.jl#37 this should improve the experience of users trying out Julia from a Conda environment. This should also be fine to backport.

@LilithHafner LilithHafner added the security System security concerns and vulnerabilities label Dec 31, 2024
@nsajko nsajko added libgit2 The libgit2 library or the LibGit2 stdlib module stdlib Julia's standard library labels Dec 31, 2024
@visr visr force-pushed the allow-cert-file branch 2 times, most recently from 8be0eee to e33618a Compare January 7, 2025 20:52
@visr
Copy link
Contributor Author

visr commented Jan 9, 2025

Test failures are unrelated. I rebased this now on top of the LibGit2 1.9 update and switch to OpenSSL that have merged since. Would appreciate a review on this and its companion JuliaLang/NetworkOptions.jl#37. Tagging @StefanKarpinski who originally contributed this code in #38827.

@visr
Copy link
Contributor Author

visr commented Jan 29, 2025

Bump, hoping to get a review for this and JuliaLang/NetworkOptions.jl#37.

@DilumAluthge
Copy link
Member

I don't think I have the necessary knowledge to review this PR, but I can try to find someone to review it.

@visr
Copy link
Contributor Author

visr commented Feb 7, 2025

Thanks for the review of JuliaLang/NetworkOptions.jl#37 @aviks. Would you be able to take a look at this one as well?

When e.g. SSL_CERT_FILE is set, we cannot set this location in LibGit2_jll because it isn't built with support for that. Until now we've errored out with a message telling users to set JULIA_SSL_CA_ROOTS_PATH to an empty string.

This changes the behavior to allow this expected error. Variables like SSL_CERT_FILE are for instance set by Conda, ensuring many people running into this, see e.g. https://discourse.julialang.org/search?q=JULIA_SSL_CA_ROOTS_PATH.

The other part, and some more context for this, is here: JuliaLang/NetworkOptions.jl#37 (comment)
@visr
Copy link
Contributor Author

visr commented Feb 14, 2025

Rebased and rewrote the top post with a lot of references for easier reviewing.

@visr
Copy link
Contributor Author

visr commented Feb 25, 2025

Bump :)

@aviks
Copy link
Member

aviks commented Feb 26, 2025

Sorry, really not trying to ignore this, but every time I've looked at this unfamiliar code, it's looked too complicated for a quick look, and I've bounced, not having had the mental bandwidth to sit and think through the flow. Finally managed to sit down and focus on this today.

@visr
Copy link
Contributor Author

visr commented Mar 11, 2025

I'd be happy to walk you through it if that helps.

@visr
Copy link
Contributor Author

visr commented Mar 25, 2025

bump

@StefanKarpinski
Copy link
Member

Sure, I guess this is ok. It's unfortunate for the behavior of LibGit2 and Downloads to be different and use different certificates, but I'm not sure what else we can do.

@StefanKarpinski StefanKarpinski merged commit 7fa969a into JuliaLang:master Mar 25, 2025
9 checks passed
@visr visr deleted the allow-cert-file branch March 25, 2025 15:06
@visr
Copy link
Contributor Author

visr commented Mar 25, 2025

Thanks! Can this get a backport label? 1.12 for sure, earlier should also be fine as far as I can see.

@StefanKarpinski
Copy link
Member

@KristofferC, I'll defer to you on backporting but it seems like 1.10, 1.11 and 1.12 would be the candidates for backports.

@StefanKarpinski StefanKarpinski added the backport 1.12 Change should be backported to release-1.12 label Mar 25, 2025
KristofferC pushed a commit that referenced this pull request Mar 26, 2025
When SSL_CERT_FILE or SSL_CERT_DIR is set, it is [impossible to set this
location](https://github.com/libgit2/libgit2/blob/4dcdb64c6844d76776745cdc25071a72c1af84d6/src/libgit2/settings.c#L206-L222)
in LibGit2_jll on Apple and Windows because [it isn't built with support
for
that](https://github.com/JuliaPackaging/Yggdrasil/blob/7123a60a68102ba6cd953e13a4e45845dc37fd82/L/LibGit2/build_tarballs.jl#L67).
Until now we've errored out with a message telling users to set
JULIA_SSL_CA_ROOTS_PATH to an empty string, which is a somewhat
problematic workaround because the Windows environment variables UI
doesn't allow empty values, and [setting it to an empty string from
PowerShell unsets
it](https://discourse.julialang.org/t/how-to-fix-ssl-cert-issues-in-pkg/115495/7?u=visr).
This PR changes the behavior to allow this expected error.

Variables like SSL_CERT_FILE are for instance [set by the Conda OpenSSL
package on environment
activation](https://github.com/conda-forge/openssl-feedstock/blob/83b5e2a793bc95d19e6cc2d9d28068f1a6ff6b79/recipe/activate-win.ps1)
used by e.g. Python, ensuring many people cannot use Pkg operations that
use LibGit2, like `dev Example`, `add Example#master`. See more user
reports [on
Discourse](https://discourse.julialang.org/search?q=JULIA_SSL_CA_ROOTS_PATH).

Together with JuliaLang/NetworkOptions.jl#37
this should improve the experience of users trying out Julia from a
Conda environment. This should also be fine to backport.

(cherry picked from commit 7fa969a)
@KristofferC KristofferC added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 and removed backport 1.12 Change should be backported to release-1.12 labels Mar 31, 2025
@KristofferC KristofferC mentioned this pull request Apr 4, 2025
51 tasks
KristofferC pushed a commit that referenced this pull request Apr 4, 2025
When SSL_CERT_FILE or SSL_CERT_DIR is set, it is [impossible to set this
location](https://github.com/libgit2/libgit2/blob/4dcdb64c6844d76776745cdc25071a72c1af84d6/src/libgit2/settings.c#L206-L222)
in LibGit2_jll on Apple and Windows because [it isn't built with support
for
that](https://github.com/JuliaPackaging/Yggdrasil/blob/7123a60a68102ba6cd953e13a4e45845dc37fd82/L/LibGit2/build_tarballs.jl#L67).
Until now we've errored out with a message telling users to set
JULIA_SSL_CA_ROOTS_PATH to an empty string, which is a somewhat
problematic workaround because the Windows environment variables UI
doesn't allow empty values, and [setting it to an empty string from
PowerShell unsets
it](https://discourse.julialang.org/t/how-to-fix-ssl-cert-issues-in-pkg/115495/7?u=visr).
This PR changes the behavior to allow this expected error.

Variables like SSL_CERT_FILE are for instance [set by the Conda OpenSSL
package on environment
activation](https://github.com/conda-forge/openssl-feedstock/blob/83b5e2a793bc95d19e6cc2d9d28068f1a6ff6b79/recipe/activate-win.ps1)
used by e.g. Python, ensuring many people cannot use Pkg operations that
use LibGit2, like `dev Example`, `add Example#master`. See more user
reports [on
Discourse](https://discourse.julialang.org/search?q=JULIA_SSL_CA_ROOTS_PATH).

Together with JuliaLang/NetworkOptions.jl#37
this should improve the experience of users trying out Julia from a
Conda environment. This should also be fine to backport.

(cherry picked from commit 7fa969a)
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Apr 10, 2025
@visr
Copy link
Contributor Author

visr commented Apr 14, 2025

The backport 1.12 label can be removed. It was backported weeks ago in 145ff43 but keeps popping up as "Need manual backport" because the label is still on.

@KristofferC
Copy link
Member

I remove the backport label once the PR is merged, typically. It's ok if it pops up there.

@KristofferC KristofferC mentioned this pull request Apr 29, 2025
53 tasks
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label May 5, 2025
KristofferC pushed a commit that referenced this pull request Jun 4, 2025
When SSL_CERT_FILE or SSL_CERT_DIR is set, it is [impossible to set this
location](https://github.com/libgit2/libgit2/blob/4dcdb64c6844d76776745cdc25071a72c1af84d6/src/libgit2/settings.c#L206-L222)
in LibGit2_jll on Apple and Windows because [it isn't built with support
for
that](https://github.com/JuliaPackaging/Yggdrasil/blob/7123a60a68102ba6cd953e13a4e45845dc37fd82/L/LibGit2/build_tarballs.jl#L67).
Until now we've errored out with a message telling users to set
JULIA_SSL_CA_ROOTS_PATH to an empty string, which is a somewhat
problematic workaround because the Windows environment variables UI
doesn't allow empty values, and [setting it to an empty string from
PowerShell unsets
it](https://discourse.julialang.org/t/how-to-fix-ssl-cert-issues-in-pkg/115495/7?u=visr).
This PR changes the behavior to allow this expected error.

Variables like SSL_CERT_FILE are for instance [set by the Conda OpenSSL
package on environment
activation](https://github.com/conda-forge/openssl-feedstock/blob/83b5e2a793bc95d19e6cc2d9d28068f1a6ff6b79/recipe/activate-win.ps1)
used by e.g. Python, ensuring many people cannot use Pkg operations that
use LibGit2, like `dev Example`, `add Example#master`. See more user
reports [on
Discourse](https://discourse.julialang.org/search?q=JULIA_SSL_CA_ROOTS_PATH).

Together with JuliaLang/NetworkOptions.jl#37
this should improve the experience of users trying out Julia from a
Conda environment. This should also be fine to backport.

(cherry picked from commit 7fa969a)
@KristofferC KristofferC mentioned this pull request Jun 4, 2025
75 tasks
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
When SSL_CERT_FILE or SSL_CERT_DIR is set, it is [impossible to set this
location](https://github.com/libgit2/libgit2/blob/4dcdb64c6844d76776745cdc25071a72c1af84d6/src/libgit2/settings.c#L206-L222)
in LibGit2_jll on Apple and Windows because [it isn't built with support
for
that](https://github.com/JuliaPackaging/Yggdrasil/blob/7123a60a68102ba6cd953e13a4e45845dc37fd82/L/LibGit2/build_tarballs.jl#L67).
Until now we've errored out with a message telling users to set
JULIA_SSL_CA_ROOTS_PATH to an empty string, which is a somewhat
problematic workaround because the Windows environment variables UI
doesn't allow empty values, and [setting it to an empty string from
PowerShell unsets
it](https://discourse.julialang.org/t/how-to-fix-ssl-cert-issues-in-pkg/115495/7?u=visr).
This PR changes the behavior to allow this expected error.

Variables like SSL_CERT_FILE are for instance [set by the Conda OpenSSL
package on environment
activation](https://github.com/conda-forge/openssl-feedstock/blob/83b5e2a793bc95d19e6cc2d9d28068f1a6ff6b79/recipe/activate-win.ps1)
used by e.g. Python, ensuring many people cannot use Pkg operations that
use LibGit2, like `dev Example`, `add Example#master`. See more user
reports [on
Discourse](https://discourse.julialang.org/search?q=JULIA_SSL_CA_ROOTS_PATH).

Together with JuliaLang/NetworkOptions.jl#37
this should improve the experience of users trying out Julia from a
Conda environment. This should also be fine to backport.

(cherry picked from commit 7fa969a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release libgit2 The libgit2 library or the LibGit2 stdlib module security System security concerns and vulnerabilities stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants