From f48bb3da7c36dd4dc1f225e3c3b612236add5c8b 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, so subsequent calls to `ensure_initialized` didn't call `initialize` and so there is never a successful call to `set_ssl_cert_locations`. Without this libgit2 defaults to not verifying host identities, which 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 | 25 +++++++++++++ stdlib/LibGit2/test/bad_ca_roots.pem | 22 ++++++++++++ stdlib/LibGit2/test/libgit2.jl | 29 +++++++++++++++- stdlib/LibGit2/test/online.jl | 15 ++++++++ 5 files changed, 127 insertions(+), 16 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..eb944492b7721 --- /dev/null +++ b/stdlib/LibGit2/test/bad_ca_roots.jl @@ -0,0 +1,25 @@ +# This file is a part of Julia. License is MIT: https://julialang.org/license + +module Test_LibGit2_https + +using LibGit2, Test + +ENV["JULIA_SSL_CA_ROOTS_PATH"] = joinpath(@__DIR__, "bad_ca_roots.pem") + +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 # 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/libgit2.jl b/stdlib/LibGit2/test/libgit2.jl index 1e5f0ef2c3547..f86471667ff46 100644 --- a/stdlib/LibGit2/test/libgit2.jl +++ b/stdlib/LibGit2/test/libgit2.jl @@ -4,12 +4,17 @@ module LibGit2Tests import LibGit2 using Test -using Random, Serialization, Sockets +using Random, Serialization, Sockets, NetworkOptions const BASE_TEST_PATH = joinpath(Sys.BINDIR, "..", "share", "julia", "test") isdefined(Main, :FakePTYs) || @eval Main include(joinpath($(BASE_TEST_PATH), "testhelpers", "FakePTYs.jl")) import .Main.FakePTYs: with_fake_pty +# 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() + function challenge_prompt(code::Expr, challenges; timeout::Integer=60, debug::Bool=true) input_code = tempname() open(input_code, "w") do fp @@ -168,6 +173,28 @@ end @test findfirst(isequal(LibGit2.Consts.FEATURE_HTTPS), f) !== nothing end +@testset "SSL/TLS initialization" begin + withenv("JULIA_SSL_CA_ROOTS_PATH" => nothing) do + # 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 + # this would work on Linux & FreeBSD, but we don't want it + # see `ca_roots.jl` for tests that don't affect behavior + ENV["JULIA_SSL_CA_ROOTS_PATH"] = NetworkOptions.bundled_ca_roots() + @test_throws LibGit2.GitError LibGit2.ensure_initialized() + end + end + # should still be possible to initialize + @test LibGit2.ensure_initialized() === nothing +end + @testset "OID" begin z = LibGit2.GitHash() @test LibGit2.iszero(z) diff --git a/stdlib/LibGit2/test/online.jl b/stdlib/LibGit2/test/online.jl index 888af97fe0a69..9f03a60b20883 100644 --- a/stdlib/LibGit2/test/online.jl +++ b/stdlib/LibGit2/test/online.jl @@ -6,6 +6,11 @@ using Test import LibGit2 using Random +# 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() + function transfer_progress(progress::Ptr{LibGit2.TransferProgress}, payload::Dict) status = payload[:transfer_progress] progress = unsafe_load(progress) @@ -90,4 +95,14 @@ mktempdir() do dir end end +if CAN_SET_CA_ROOTS_PATH + # 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 + end # module