Skip to content

Commit 96d177f

Browse files
StefanKarpinskiKristofferC
authored andcommitted
LibGit2: improve error when CA root cert can't be set (#38827)
This also fixes an insecure behavior: even if `set_ssl_cert_locations` failed, `REFCOUNT` was still incremented, which meant that subsequent calls to `ensure_initialized` didn't call `initialize` and so there was never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities and that is insecure. To prevent this, this patch locks on `ensure_initialized` and decrements `REFCOUNT` if initialize throws an error, ensuring that `initialize` succeeds at least once, including the call to `set_ssl_cert_locations`. (cherry picked from commit 4dede6d)
1 parent 22fa5a1 commit 96d177f

File tree

4 files changed

+120
-15
lines changed

4 files changed

+120
-15
lines changed

stdlib/LibGit2/src/LibGit2.jl

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -961,13 +961,19 @@ end
961961

962962
## lazy libgit2 initialization
963963

964+
const ENSURE_INITIALIZED_LOCK = ReentrantLock()
965+
964966
function ensure_initialized()
965-
x = Threads.atomic_cas!(REFCOUNT, 0, 1)
966-
if x < 0
967-
negative_refcount_error(x)::Union{}
968-
end
969-
if x == 0
970-
initialize()
967+
lock(ENSURE_INITIALIZED_LOCK) do
968+
x = Threads.atomic_cas!(REFCOUNT, 0, 1)
969+
x > 0 && return
970+
x < 0 && negative_refcount_error(x)::Union{}
971+
try initialize()
972+
catch
973+
Threads.atomic_sub!(REFCOUNT, 1)
974+
@assert REFCOUNT[] == 0
975+
rethrow()
976+
end
971977
end
972978
return nothing
973979
end
@@ -979,24 +985,40 @@ end
979985
@noinline function initialize()
980986
@check ccall((:git_libgit2_init, :libgit2), Cint, ())
981987

988+
cert_loc = NetworkOptions.ca_roots()
989+
cert_loc !== nothing && set_ssl_cert_locations(cert_loc)
990+
982991
atexit() do
983992
# refcount zero, no objects to be finalized
984993
if Threads.atomic_sub!(REFCOUNT, 1) == 1
985994
ccall((:git_libgit2_shutdown, :libgit2), Cint, ())
986995
end
987996
end
988-
989-
cert_loc = NetworkOptions.ca_roots()
990-
cert_loc !== nothing && set_ssl_cert_locations(cert_loc)
991997
end
992998

993999
function set_ssl_cert_locations(cert_loc)
994-
cert_file = isfile(cert_loc) ? cert_loc : Cstring(C_NULL)
995-
cert_dir = isdir(cert_loc) ? cert_loc : Cstring(C_NULL)
996-
cert_file == C_NULL && cert_dir == C_NULL && return
997-
@check ccall((:git_libgit2_opts, :libgit2), Cint,
998-
(Cint, Cstring...),
999-
Cint(Consts.SET_SSL_CERT_LOCATIONS), cert_file, cert_dir)
1000+
cert_file = cert_dir = Cstring(C_NULL)
1001+
if isdir(cert_loc) # directories
1002+
cert_dir = cert_loc
1003+
else # files, /dev/null, non-existent paths, etc.
1004+
cert_file = cert_loc
1005+
end
1006+
ret = ccall((:git_libgit2_opts, :libgit2), Cint, (Cint, Cstring...),
1007+
Cint(Consts.SET_SSL_CERT_LOCATIONS), cert_file, cert_dir)
1008+
ret >= 0 && return ret
1009+
err = Error.GitError(ret)
1010+
err.class == Error.SSL &&
1011+
err.msg == "TLS backend doesn't support certificate locations" ||
1012+
throw(err)
1013+
var = nothing
1014+
for v in NetworkOptions.CA_ROOTS_VARS
1015+
haskey(ENV, v) && (var = v)
1016+
end
1017+
@assert var !== nothing # otherwise we shouldn't be here
1018+
msg = """
1019+
Your Julia is built with a SSL/TLS engine that libgit2 doesn't know how to configure to use a file or directory of certificate authority roots, but your environment specifies one via the $var variable. If you believe your system's root certificates are safe to use, you can `export JULIA_SSL_CA_ROOTS_PATH=""` in your environment to use those instead.
1020+
"""
1021+
throw(Error.GitError(err.class, err.code, chomp(msg)))
10001022
end
10011023

10021024
end # module
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# This file is a part of Julia. License is MIT: https://julialang.org/license
2+
3+
module Test_LibGit2_https
4+
5+
using Test, LibGit2, NetworkOptions
6+
7+
# we currently use system SSL/TLS on macOS and Windows platforms
8+
# and libgit2 cannot set the CA roots path on those systems
9+
# if that changes, this may need to be adjusted
10+
const CAN_SET_CA_ROOTS_PATH = !Sys.isapple() && !Sys.iswindows()
11+
12+
@testset "empty CA roots file" begin
13+
# these fail for different reasons on different platforms:
14+
# - on Apple & Windows you cannot set the CA roots path location
15+
# - on Linux & FreeBSD you you can but these are invalid files
16+
ENV["JULIA_SSL_CA_ROOTS_PATH"] = "/dev/null"
17+
@test_throws LibGit2.GitError LibGit2.ensure_initialized()
18+
ENV["JULIA_SSL_CA_ROOTS_PATH"] = tempname()
19+
@test_throws LibGit2.GitError LibGit2.ensure_initialized()
20+
# test that it still fails if called a second time
21+
@test_throws LibGit2.GitError LibGit2.ensure_initialized()
22+
if !CAN_SET_CA_ROOTS_PATH
23+
# test that this doesn't work on macOS & Windows
24+
ENV["JULIA_SSL_CA_ROOTS_PATH"] = NetworkOptions.bundled_ca_roots()
25+
@test_throws LibGit2.GitError LibGit2.ensure_initialized()
26+
delete!(ENV, "JULIA_SSL_CA_ROOTS_PATH")
27+
@test LibGit2.ensure_initialized() === nothing
28+
end
29+
end
30+
31+
if CAN_SET_CA_ROOTS_PATH
32+
@testset "non-empty but bad CA roots file" begin
33+
# should still be possible to initialize
34+
ENV["JULIA_SSL_CA_ROOTS_PATH"] = joinpath(@__DIR__, "bad_ca_roots.pem")
35+
@test LibGit2.ensure_initialized() === nothing
36+
end
37+
mktempdir() do dir
38+
repo_url = "https://github.com/JuliaLang/Example.jl"
39+
@testset "HTTPS clone with bad CA roots fails" begin
40+
repo_path = joinpath(dir, "Example.HTTPS")
41+
c = LibGit2.CredentialPayload(allow_prompt=false, allow_git_helpers=false)
42+
redirect_stderr(devnull)
43+
err = try LibGit2.clone(repo_url, repo_path, credentials=c)
44+
catch err
45+
err
46+
end
47+
@test err isa LibGit2.GitError
48+
@test err.msg == "user rejected certificate for github.com"
49+
end
50+
end
51+
end
52+
53+
end # module
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDtDCCApwCCQDeWk9ywtjrpTANBgkqhkiG9w0BAQsFADCBmzELMAkGA1UEBhMC
3+
VVMxETAPBgNVBAgMCE5ldyBZb3JrMREwDwYDVQQHDAhOZXcgWW9yazEnMCUGA1UE
4+
CgweVGhlIEp1bGlhIFByb2dyYW1taW5nIExhbmd1YWdlMRYwFAYDVQQDDA1qdWxp
5+
YWxhbmcub3JnMSUwIwYJKoZIhvcNAQkBFhZzZWN1cml0eUBqdWxpYWxhbmcub3Jn
6+
MB4XDTIwMTIxMTE3NTgxN1oXDTI1MTIxMDE3NTgxN1owgZsxCzAJBgNVBAYTAlVT
7+
MREwDwYDVQQIDAhOZXcgWW9yazERMA8GA1UEBwwITmV3IFlvcmsxJzAlBgNVBAoM
8+
HlRoZSBKdWxpYSBQcm9ncmFtbWluZyBMYW5ndWFnZTEWMBQGA1UEAwwNanVsaWFs
9+
YW5nLm9yZzElMCMGCSqGSIb3DQEJARYWc2VjdXJpdHlAanVsaWFsYW5nLm9yZzCC
10+
ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANCFgRMFlNGIgmZtMzR+Xx+t
11+
cPXpYnw9sZXlGy4y+P+UVW5rnFtf+OL4WkcJykmL3n/iLBKpdrndhzL7zuc6lGVv
12+
G6u+Gvwg5uCZ4RqiFSPP9xK7tl7H+CwrtWL/2vF1wlYC228A+NMpPyQw4XtX1L8G
13+
xAvJbFz8JrJ+WH1wCmVpkxA6pnpK+DZ/QKPVwa/qhB80ur3bYwlHXWwDBf8bq98f
14+
7wDBpJoEc3IG3GYopP6ie5KTENKxbDZjr306ZuxTLjXKrAE/OJkAiGKJ7gPlwT/E
15+
kFI/x/No9Y/fPWFRGiFo2L4fhP2Mohcph3PQswFKfnQlMQzztetDKWCZveB5HisC
16+
AwEAATANBgkqhkiG9w0BAQsFAAOCAQEAqAaFA93Q3VWWKAZBqORT+6N2iHDiOxMu
17+
Ol8Jjqp3Spj552NbyPPpfF2a2Q/Bh2ZAmncCoGTpuXdnowSHyXuxPey6BIvEbq0L
18+
FizTNuIzaA95fO/ce9LNujxliDHhKMJBZtCqBJYJ4dgd9sA4/LeAG/P3ltIY6K8P
19+
22AAx2bzWbeRJSqxeBodm19rOb9Yz2SOaZIam42E+xia+hsUFdGf6Zkfpa02azDm
20+
93EjS+DwapqxAKgkps6JuKqpRFdZd8QsVmgAcapnIt77w8sfBu9eyITF/Tm+MA8k
21+
IRieSypM7TK0jQ6QrNV7FKSI6eEPaqWBMwkLg3S5H6KQMntVRlcc0A==
22+
-----END CERTIFICATE-----

stdlib/LibGit2/test/online.jl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,12 @@ mktempdir() do dir
9090
end
9191
end
9292

93+
# needs to be run in separate process so it can re-initialize libgit2
94+
# with a useless self-signed certificate authority root certificate
95+
file = joinpath(@__DIR__, "bad_ca_roots.jl")
96+
cmd = `$(Base.julia_cmd()) --depwarn=no --startup-file=no $file`
97+
if !success(pipeline(cmd; stdout=stdout, stderr=stderr))
98+
error("bad CA roots tests failed, cmd : $cmd")
99+
end
100+
93101
end # module

0 commit comments

Comments
 (0)