From 9b4b9c720211226ab197b05f024b9ba19d9bd0b9 Mon Sep 17 00:00:00 2001 From: Stefan Karpinski Date: Thu, 10 Dec 2020 19:13:23 -0500 Subject: [PATCH] LibGit2: improve error when CA root cert can't be set 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`. --- stdlib/LibGit2/src/LibGit2.jl | 52 +++++++++++++++++++-------- stdlib/LibGit2/test/bad_ca_roots.jl | 53 ++++++++++++++++++++++++++++ stdlib/LibGit2/test/bad_ca_roots.pem | 22 ++++++++++++ stdlib/LibGit2/test/online.jl | 8 +++++ 4 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 stdlib/LibGit2/test/bad_ca_roots.jl create mode 100644 stdlib/LibGit2/test/bad_ca_roots.pem diff --git a/stdlib/LibGit2/src/LibGit2.jl b/stdlib/LibGit2/src/LibGit2.jl index 3decc4dc01608..e2e48fee855f7 100644 --- a/stdlib/LibGit2/src/LibGit2.jl +++ b/stdlib/LibGit2/src/LibGit2.jl @@ -961,13 +961,19 @@ end ## lazy libgit2 initialization +const ENSURE_INITIALIZED_LOCK = ReentrantLock() + function ensure_initialized() - x = Threads.atomic_cas!(REFCOUNT, 0, 1) - if x < 0 - negative_refcount_error(x)::Union{} - end - if x == 0 - initialize() + lock(ENSURE_INITIALIZED_LOCK) do + x = Threads.atomic_cas!(REFCOUNT, 0, 1) + x > 0 && return + x < 0 && negative_refcount_error(x)::Union{} + try initialize() + catch + Threads.atomic_sub!(REFCOUNT, 1) + @assert REFCOUNT[] == 0 + rethrow() + end end return nothing end @@ -979,24 +985,40 @@ end @noinline function initialize() @check ccall((:git_libgit2_init, :libgit2), Cint, ()) + cert_loc = NetworkOptions.ca_roots() + cert_loc !== nothing && set_ssl_cert_locations(cert_loc) + atexit() do # refcount zero, no objects to be finalized if Threads.atomic_sub!(REFCOUNT, 1) == 1 ccall((:git_libgit2_shutdown, :libgit2), Cint, ()) end end - - cert_loc = NetworkOptions.ca_roots() - cert_loc !== nothing && set_ssl_cert_locations(cert_loc) end function set_ssl_cert_locations(cert_loc) - cert_file = isfile(cert_loc) ? cert_loc : Cstring(C_NULL) - cert_dir = isdir(cert_loc) ? cert_loc : Cstring(C_NULL) - cert_file == C_NULL && cert_dir == C_NULL && return - @check ccall((:git_libgit2_opts, :libgit2), Cint, - (Cint, Cstring...), - Cint(Consts.SET_SSL_CERT_LOCATIONS), cert_file, cert_dir) + cert_file = cert_dir = Cstring(C_NULL) + if isdir(cert_loc) # directories + cert_dir = cert_loc + else # files, /dev/null, non-existent paths, etc. + cert_file = cert_loc + end + ret = ccall((:git_libgit2_opts, :libgit2), Cint, (Cint, Cstring...), + Cint(Consts.SET_SSL_CERT_LOCATIONS), cert_file, cert_dir) + ret >= 0 && return ret + err = Error.GitError(ret) + err.class == Error.SSL && + err.msg == "TLS backend doesn't support certificate locations" || + throw(err) + var = nothing + for v in NetworkOptions.CA_ROOTS_VARS + haskey(ENV, v) && (var = v) + end + @assert var !== nothing # otherwise we shouldn't be here + msg = """ + 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. + """ + throw(Error.GitError(err.class, err.code, chomp(msg))) end end # module diff --git a/stdlib/LibGit2/test/bad_ca_roots.jl b/stdlib/LibGit2/test/bad_ca_roots.jl new file mode 100644 index 0000000000000..e4ebdc709637a --- /dev/null +++ b/stdlib/LibGit2/test/bad_ca_roots.jl @@ -0,0 +1,53 @@ +# This file is a part of Julia. License is MIT: https://julialang.org/license + +module Test_LibGit2_https + +using Test, LibGit2, NetworkOptions + +# we currently use system SSL/TLS on macOS and Windows platforms +# and libgit2 cannot set the CA roots path on those systems +# if that changes, this may need to be adjusted +const CAN_SET_CA_ROOTS_PATH = !Sys.isapple() && !Sys.iswindows() + +@testset "empty CA roots file" begin + # these fail for different reasons on different platforms: + # - on Apple & Windows you cannot set the CA roots path location + # - on Linux & FreeBSD you you can but these are invalid files + ENV["JULIA_SSL_CA_ROOTS_PATH"] = "/dev/null" + @test_throws LibGit2.GitError LibGit2.ensure_initialized() + ENV["JULIA_SSL_CA_ROOTS_PATH"] = tempname() + @test_throws LibGit2.GitError LibGit2.ensure_initialized() + # test that it still fails if called a second time + @test_throws LibGit2.GitError LibGit2.ensure_initialized() + if !CAN_SET_CA_ROOTS_PATH + # test that this doesn't work on macOS & Windows + ENV["JULIA_SSL_CA_ROOTS_PATH"] = NetworkOptions.bundled_ca_roots() + @test_throws LibGit2.GitError LibGit2.ensure_initialized() + delete!(ENV, "JULIA_SSL_CA_ROOTS_PATH") + @test LibGit2.ensure_initialized() === nothing + end +end + +if CAN_SET_CA_ROOTS_PATH + @testset "non-empty but bad CA roots file" begin + # should still be possible to initialize + ENV["JULIA_SSL_CA_ROOTS_PATH"] = joinpath(@__DIR__, "bad_ca_roots.pem") + @test LibGit2.ensure_initialized() === nothing + end + mktempdir() do dir + repo_url = "https://github.com/JuliaLang/Example.jl" + @testset "HTTPS clone with bad CA roots fails" begin + repo_path = joinpath(dir, "Example.HTTPS") + c = LibGit2.CredentialPayload(allow_prompt=false, allow_git_helpers=false) + redirect_stderr(devnull) + err = try LibGit2.clone(repo_url, repo_path, credentials=c) + catch err + err + end + @test err isa LibGit2.GitError + @test err.msg == "user rejected certificate for github.com" + end + end +end + +end # module diff --git a/stdlib/LibGit2/test/bad_ca_roots.pem b/stdlib/LibGit2/test/bad_ca_roots.pem new file mode 100644 index 0000000000000..36ca4150efaf0 --- /dev/null +++ b/stdlib/LibGit2/test/bad_ca_roots.pem @@ -0,0 +1,22 @@ +-----BEGIN CERTIFICATE----- +MIIDtDCCApwCCQDeWk9ywtjrpTANBgkqhkiG9w0BAQsFADCBmzELMAkGA1UEBhMC +VVMxETAPBgNVBAgMCE5ldyBZb3JrMREwDwYDVQQHDAhOZXcgWW9yazEnMCUGA1UE +CgweVGhlIEp1bGlhIFByb2dyYW1taW5nIExhbmd1YWdlMRYwFAYDVQQDDA1qdWxp +YWxhbmcub3JnMSUwIwYJKoZIhvcNAQkBFhZzZWN1cml0eUBqdWxpYWxhbmcub3Jn +MB4XDTIwMTIxMTE3NTgxN1oXDTI1MTIxMDE3NTgxN1owgZsxCzAJBgNVBAYTAlVT +MREwDwYDVQQIDAhOZXcgWW9yazERMA8GA1UEBwwITmV3IFlvcmsxJzAlBgNVBAoM +HlRoZSBKdWxpYSBQcm9ncmFtbWluZyBMYW5ndWFnZTEWMBQGA1UEAwwNanVsaWFs +YW5nLm9yZzElMCMGCSqGSIb3DQEJARYWc2VjdXJpdHlAanVsaWFsYW5nLm9yZzCC +ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANCFgRMFlNGIgmZtMzR+Xx+t +cPXpYnw9sZXlGy4y+P+UVW5rnFtf+OL4WkcJykmL3n/iLBKpdrndhzL7zuc6lGVv +G6u+Gvwg5uCZ4RqiFSPP9xK7tl7H+CwrtWL/2vF1wlYC228A+NMpPyQw4XtX1L8G +xAvJbFz8JrJ+WH1wCmVpkxA6pnpK+DZ/QKPVwa/qhB80ur3bYwlHXWwDBf8bq98f +7wDBpJoEc3IG3GYopP6ie5KTENKxbDZjr306ZuxTLjXKrAE/OJkAiGKJ7gPlwT/E +kFI/x/No9Y/fPWFRGiFo2L4fhP2Mohcph3PQswFKfnQlMQzztetDKWCZveB5HisC +AwEAATANBgkqhkiG9w0BAQsFAAOCAQEAqAaFA93Q3VWWKAZBqORT+6N2iHDiOxMu +Ol8Jjqp3Spj552NbyPPpfF2a2Q/Bh2ZAmncCoGTpuXdnowSHyXuxPey6BIvEbq0L +FizTNuIzaA95fO/ce9LNujxliDHhKMJBZtCqBJYJ4dgd9sA4/LeAG/P3ltIY6K8P +22AAx2bzWbeRJSqxeBodm19rOb9Yz2SOaZIam42E+xia+hsUFdGf6Zkfpa02azDm +93EjS+DwapqxAKgkps6JuKqpRFdZd8QsVmgAcapnIt77w8sfBu9eyITF/Tm+MA8k +IRieSypM7TK0jQ6QrNV7FKSI6eEPaqWBMwkLg3S5H6KQMntVRlcc0A== +-----END CERTIFICATE----- diff --git a/stdlib/LibGit2/test/online.jl b/stdlib/LibGit2/test/online.jl index 888af97fe0a69..96b6bf5b22371 100644 --- a/stdlib/LibGit2/test/online.jl +++ b/stdlib/LibGit2/test/online.jl @@ -90,4 +90,12 @@ mktempdir() do dir end end +# needs to be run in separate process so it can re-initialize libgit2 +# with a useless self-signed certificate authority root certificate +file = joinpath(@__DIR__, "bad_ca_roots.jl") +cmd = `$(Base.julia_cmd()) --depwarn=no --startup-file=no $file` +if !success(pipeline(cmd; stdout=stdout, stderr=stderr)) + error("bad CA roots tests failed, cmd : $cmd") +end + end # module