Skip to content

Commit

Permalink
LibGit2: improve error when CA root cert can't be set
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
StefanKarpinski committed Dec 11, 2020
1 parent 99402b4 commit f48bb3d
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 16 deletions.
52 changes: 37 additions & 15 deletions stdlib/LibGit2/src/LibGit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
25 changes: 25 additions & 0 deletions stdlib/LibGit2/test/bad_ca_roots.jl
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions stdlib/LibGit2/test/bad_ca_roots.pem
Original file line number Diff line number Diff line change
@@ -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-----
29 changes: 28 additions & 1 deletion stdlib/LibGit2/test/libgit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions stdlib/LibGit2/test/online.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

0 comments on commit f48bb3d

Please sign in to comment.