From cae6726b3e029042e5f65e22d42d9aca5e7ba746 Mon Sep 17 00:00:00 2001 From: Stefan Karpinski Date: Thu, 21 Jan 2021 11:09:34 -0500 Subject: [PATCH] LibGit2: patch to pass hostkey & port to host verify callback (#39324) It seems that no one actually verifies SSH host identity with libgit2 because the callback doesn't give enough information do so correctly: - It doesn't give the actual host key fingerprint, but rather three different hashes thereof. This means we cannot distinguish a known hosts entry that has a different type (`ssh-rsa`, `ssh-dsa`, etc.) from an entry with a matching type and a fingerprint mismatch: the former should be treated as an unknown host whereas the latter is a host key mismatch; they cannot be distinguished without this patch. - If the user connects on a non-default port (i.e. not 22), this is not passed to the callback in any way. Since there can be different known host entries for different ports and they should be treated as distinct, this also means the current API cannot be used to verify hosts serving SSH on non-standard ports. This patch passes the port. I will try to upstream some version of this patch to libgit2. The same patch has already been applied to the LibGit2 JLL. Fixes #38777. Might fix https://github.com/JuliaLang/Pkg.jl/issues/2334. --- deps/Versions.make | 2 +- .../md5 | 1 + .../sha512 | 1 + deps/libgit2.mk | 8 +- deps/patches/libgit2-hostkey.patch | 61 ++++++++ stdlib/LibGit2/src/callbacks.jl | 143 +++++++----------- stdlib/LibGit2/src/consts.jl | 10 +- stdlib/LibGit2/test/libgit2.jl | 83 ++++------ 8 files changed, 158 insertions(+), 151 deletions(-) create mode 100644 deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/md5 create mode 100644 deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/sha512 create mode 100644 deps/patches/libgit2-hostkey.patch diff --git a/deps/Versions.make b/deps/Versions.make index 9fd9608fc1fa6..22a6c4d744135 100644 --- a/deps/Versions.make +++ b/deps/Versions.make @@ -33,7 +33,7 @@ CURL_JLL_NAME := LibCURL LAPACK_VER := 3.9.0 # LibGit2 -LIBGIT2_VER := 1.1.0 +LIBGIT2_JLL_VER := 1.2.1+0 LIBGIT2_JLL_NAME := LibGit2 # LibSSH2 diff --git a/deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/md5 b/deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/md5 new file mode 100644 index 0000000000000..3e66eb33f76f3 --- /dev/null +++ b/deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/md5 @@ -0,0 +1 @@ +48b3eb5811566f1cc70a9581b8f702f4 diff --git a/deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/sha512 b/deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/sha512 new file mode 100644 index 0000000000000..d9a0fef30537c --- /dev/null +++ b/deps/checksums/LibGit2.v1.2.1+0.x86_64-apple-darwin.tar.gz/sha512 @@ -0,0 +1 @@ +46af2fbe9c96a18a97531aefc79e710abd8e12eca64ddcb2a0ddc8bc675dbaed0723ddbd4401d870eddcae04d99c4306cc6bdaa54b063de36d7fc0981ba86587 diff --git a/deps/libgit2.mk b/deps/libgit2.mk index 301e73b454c5e..82e9817134454 100644 --- a/deps/libgit2.mk +++ b/deps/libgit2.mk @@ -46,9 +46,15 @@ $(LIBGIT2_SRC_PATH)/libgit2-mbedtls-incdir.patch-applied: $(LIBGIT2_SRC_PATH)/li patch -p1 -f < $(SRCDIR)/patches/libgit2-mbedtls-incdir.patch echo 1 > $@ +$(LIBGIT2_SRC_PATH)/libgit2-hostkey.patch-applied: $(LIBGIT2_SRC_PATH)/libgit2-mbedtls-incdir.patch-applied + cd $(LIBGIT2_SRC_PATH) && \ + patch -p1 -f < $(SRCDIR)/patches/libgit2-hostkey.patch + echo 1 > $@ + $(BUILDDIR)/$(LIBGIT2_SRC_DIR)/build-configured: \ $(LIBGIT2_SRC_PATH)/libgit2-agent-nonfatal.patch-applied \ - $(LIBGIT2_SRC_PATH)/libgit2-mbedtls-incdir.patch-applied + $(LIBGIT2_SRC_PATH)/libgit2-mbedtls-incdir.patch-applied \ + $(LIBGIT2_SRC_PATH)/libgit2-hostkey.patch-applied $(BUILDDIR)/$(LIBGIT2_SRC_DIR)/build-configured: $(LIBGIT2_SRC_PATH)/source-extracted mkdir -p $(dir $@) diff --git a/deps/patches/libgit2-hostkey.patch b/deps/patches/libgit2-hostkey.patch new file mode 100644 index 0000000000000..16c0f3b13f621 --- /dev/null +++ b/deps/patches/libgit2-hostkey.patch @@ -0,0 +1,61 @@ +diff --git a/include/git2/cert.h b/include/git2/cert.h +index e8cd2d180..54293cd31 100644 +--- a/include/git2/cert.h ++++ b/include/git2/cert.h +@@ -111,6 +111,14 @@ typedef struct { + * have the SHA-256 hash of the hostkey. + */ + unsigned char hash_sha256[32]; ++ ++ /** ++ * Hostkey itself. ++ */ ++ int hostkey_type; ++ size_t hostkey_len; ++ unsigned char hostkey[1024]; ++ + } git_cert_hostkey; + + /** +diff --git a/src/transports/ssh.c b/src/transports/ssh.c +index f4ed05bb1..049697796 100644 +--- a/src/transports/ssh.c ++++ b/src/transports/ssh.c +@@ -523,6 +523,7 @@ static int _git_ssh_setup_conn( + git_credential *cred = NULL; + LIBSSH2_SESSION* session=NULL; + LIBSSH2_CHANNEL* channel=NULL; ++ char *host_and_port; + + t->current_stream = NULL; + +@@ -566,6 +567,12 @@ post_extract: + + cert.parent.cert_type = GIT_CERT_HOSTKEY_LIBSSH2; + ++ key = libssh2_session_hostkey(session, &cert.hostkey_len, &cert.hostkey_type); ++ bzero(&cert.hostkey, sizeof(cert.hostkey)); ++ if (cert.hostkey_len > sizeof(cert.hostkey)) ++ cert.hostkey_len = sizeof(cert.hostkey); ++ memcpy(&cert.hostkey, key, cert.hostkey_len); ++ + #ifdef LIBSSH2_HOSTKEY_HASH_SHA256 + key = libssh2_hostkey_hash(session, LIBSSH2_HOSTKEY_HASH_SHA256); + if (key != NULL) { +@@ -597,7 +604,15 @@ post_extract: + + cert_ptr = &cert; + +- error = t->owner->certificate_check_cb((git_cert *) cert_ptr, 0, urldata.host, t->owner->message_cb_payload); ++ if (git_net_url_is_default_port(&urldata)) { ++ host_and_port = urldata.host; ++ } else { ++ size_t n = strlen(urldata.host) + strlen(urldata.port) + 2; ++ host_and_port = alloca(n); ++ sprintf(host_and_port, "%s:%s", urldata.host, urldata.port); ++ } ++ ++ error = t->owner->certificate_check_cb((git_cert *) cert_ptr, 0, host_and_port, t->owner->message_cb_payload); + + if (error < 0 && error != GIT_PASSTHROUGH) { + if (!git_error_last()) diff --git a/stdlib/LibGit2/src/callbacks.jl b/stdlib/LibGit2/src/callbacks.jl index 604014bbea7b2..8eddb8c864644 100644 --- a/stdlib/LibGit2/src/callbacks.jl +++ b/stdlib/LibGit2/src/callbacks.jl @@ -360,24 +360,14 @@ function fetchhead_foreach_callback(ref_name::Cstring, remote_url::Cstring, end struct CertHostKey - parent :: Cint - mask :: Cint - md5 :: NTuple{16,UInt8} - sha1 :: NTuple{20,UInt8} - sha256 :: NTuple{32,UInt8} -end - -struct KeyHashes - sha1 :: Union{NTuple{20,UInt8}, Nothing} - sha256 :: Union{NTuple{32,UInt8}, Nothing} -end - -function KeyHashes(cert_p::Ptr{CertHostKey}) - cert = unsafe_load(cert_p) - return KeyHashes( - cert.mask & Consts.CERT_SSH_SHA1 != 0 ? cert.sha1 : nothing, - cert.mask & Consts.CERT_SSH_SHA256 != 0 ? cert.sha256 : nothing, - ) + parent :: Cint + mask :: Cint + md5 :: NTuple{16,UInt8} + sha1 :: NTuple{20,UInt8} + sha256 :: NTuple{32,UInt8} + type :: Cint + len :: Csize_t + data :: NTuple{1024,UInt8} end function verify_host_error(message::AbstractString) @@ -406,22 +396,21 @@ function certificate_callback( return Consts.CERT_REJECT elseif transport == "SSH" # SSH verification has to be done here - files = [joinpath(homedir(), ".ssh", "known_hosts")] - check = ssh_knownhost_check(files, host, KeyHashes(cert_p)) + files = NetworkOptions.ssh_known_hosts_files() + cert = unsafe_load(cert_p) + check = ssh_knownhost_check(files, host, cert) valid = false - if check == Consts.SSH_HOST_KNOWN + if check == Consts.LIBSSH2_KNOWNHOST_CHECK_MATCH valid = true - elseif check == Consts.SSH_HOST_UNKNOWN + elseif check == Consts.LIBSSH2_KNOWNHOST_CHECK_NOTFOUND if Sys.which("ssh-keyscan") !== nothing msg = "Please run `ssh-keyscan $host >> $(files[1])` in order to add the server to your known hosts file and then try again." else msg = "Please connect once using `ssh $host` in order to add the server to your known hosts file and then try again. You may not be allowed to log in (wrong user and/or no login allowed), but ssh will prompt you to add a host key for the server which will allow libgit2 to verify the server." end verify_host_error("SSH host verification: the server `$host` is not a known host. $msg") - elseif check == Consts.SSH_HOST_MISMATCH + elseif check == Consts.LIBSSH2_KNOWNHOST_CHECK_MISMATCH verify_host_error("SSH host verification: the identity of the server `$host` does not match its known hosts record. Someone could be trying to man-in-the-middle your connection. It is also possible that the server has changed its key, in which case you should check with the server administrator and if they confirm that the key has been changed, update your known hosts file.") - elseif check == Consts.SSH_HOST_BAD_HASH - verify_host_error("SSH host verification: no secure certificate hash available for `$host`, cannot verify server identity.") else @error("unexpected SSH known host check result", check) end @@ -431,31 +420,6 @@ function certificate_callback( return Consts.CERT_REJECT end -## SSH known host checking -# -# We can't use libssh2_knownhost_check because libgit2, for no good reason, -# doesn't give us a host fingerprint that we can use for that and instead gives -# us multiple hashes of that fingerprint instead. Moreover, since a host can -# have multiple fingerprints in the known hosts file with different encryption -# types (gitlab.com does this, for example), we need to iterate through all the -# known hosts entries and manually check if any of them is a match. -# -# The fact that libgit2 won't give us a fingerprint also means that we cannot, -# even if we wanted to, prompt the user for whether to add the fingerprint to -# the known hosts file, since we don't have the fingerprint that should be -# added. The only option is to instruct the user how to add it themselves. -# -# Check logic: if a host appears in a known hosts file at all then one of the -# keys in that file must match or we declare a mismatch; if the host name -# doesn't appear in the file at all, however, we will continue searching files. -# -# This allows adding a host to the system known hosts file to fully override -# that host appearing in a bundled known hosts file. It is necessary to allow -# any of multiple entries in a single file to match, however, to allow for the -# possiblity that the file contains multiple fingerprints for the same host. If -# libgit2 gave us the fucking fingerprint then we could search for only an entry -# with the correct type, but we can't do that without the actual fingerprint. - struct KnownHost magic :: Cuint node :: Ptr{Cvoid} @@ -465,12 +429,27 @@ struct KnownHost end function ssh_knownhost_check( - files :: AbstractVector{<:AbstractString}, - host :: AbstractString, - hashes :: KeyHashes, + files :: AbstractVector{<:AbstractString}, + host :: AbstractString, + cert :: CertHostKey, +) + key = collect(cert.data)[1:cert.len] + return ssh_knownhost_check(files, host, key) +end + +function ssh_knownhost_check( + files :: AbstractVector{<:AbstractString}, + host :: AbstractString, + key :: String, ) - hashes.sha1 === hashes.sha256 === nothing && - return Consts.SSH_HOST_BAD_HASH + if (m = match(r"^(.+):(\d+)$", host)) !== nothing + host = m.captures[1] + port = parse(Int, m.captures[2]) + else + port = 22 # default SSH port + end + mask = Consts.LIBSSH2_KNOWNHOST_TYPE_PLAIN | + Consts.LIBSSH2_KNOWNHOST_KEYENC_RAW session = @ccall "libssh2".libssh2_session_init_ex( C_NULL :: Ptr{Cvoid}, C_NULL :: Ptr{Cvoid}, @@ -492,46 +471,32 @@ function ssh_knownhost_check( @ccall "libssh2".libssh2_knownhost_free(hosts::Ptr{Cvoid})::Cvoid continue end - name_match = false - prev = Ptr{KnownHost}(0) - store = Ref{Ptr{KnownHost}}() - while true - get = @ccall "libssh2".libssh2_knownhost_get( - hosts :: Ptr{Cvoid}, - store :: Ptr{Ptr{KnownHost}}, - prev :: Ptr{KnownHost}, - ) :: Cint - get < 0 && @warn("Error searching SSH known hosts file `$file`") - get == 0 || break # end of file or error - # got a known hosts record for host, now check its key hash - prev = store[] - known_host = unsafe_load(prev) - known_host.name == C_NULL && continue - host == unsafe_string(known_host.name) || continue - name_match = true # we've found some entry in this file - key_match = true # all available hashes must match - key = base64decode(unsafe_string(known_host.key)) - if hashes.sha1 !== nothing - key_match &= sha1(key) == collect(hashes.sha1) - end - if hashes.sha256 !== nothing - key_match &= sha256(key) == collect(hashes.sha256) - end - key_match || continue - # name and key match found + size = ncodeunits(key) + check = @ccall "libssh2".libssh2_knownhost_checkp( + hosts :: Ptr{Cvoid}, + host :: Cstring, + port :: Cint, + key :: Ptr{UInt8}, + size :: Csize_t, + mask :: Cint, + C_NULL :: Ptr{Ptr{KnownHost}}, + ) :: Cint + if check == Consts.LIBSSH2_KNOWNHOST_CHECK_MATCH || + check == Consts.LIBSSH2_KNOWNHOST_CHECK_MISMATCH @ccall "libssh2".libssh2_knownhost_free(hosts::Ptr{Cvoid})::Cvoid @assert 0 == @ccall "libssh2".libssh2_session_free(session::Ptr{Cvoid})::Cint - return Consts.SSH_HOST_KNOWN + return check + else + @ccall "libssh2".libssh2_knownhost_free(hosts::Ptr{Cvoid})::Cvoid + if check == Consts.LIBSSH2_KNOWNHOST_CHECK_FAILURE + @warn("Error searching SSH known hosts file `$file`") + end + continue end - @ccall "libssh2".libssh2_knownhost_free(hosts::Ptr{Cvoid})::Cvoid - name_match || continue # no name match, search more files - # name match but no key match => host mismatch - @assert 0 == @ccall "libssh2".libssh2_session_free(session::Ptr{Cvoid})::Cint - return Consts.SSH_HOST_MISMATCH end # name not found in any known hosts files @assert 0 == @ccall "libssh2".libssh2_session_free(session::Ptr{Cvoid})::Cint - return Consts.SSH_HOST_UNKNOWN + return Consts.LIBSSH2_KNOWNHOST_CHECK_NOTFOUND end "C function pointer for `mirror_callback`" diff --git a/stdlib/LibGit2/src/consts.jl b/stdlib/LibGit2/src/consts.jl index 7658b2d47d779..2bc9edaf8950b 100644 --- a/stdlib/LibGit2/src/consts.jl +++ b/stdlib/LibGit2/src/consts.jl @@ -330,11 +330,11 @@ const LIBSSH2_KNOWNHOST_TYPE_CUSTOM = 3 const LIBSSH2_KNOWNHOST_KEYENC_RAW = 1 << 16 const LIBSSH2_KNOWNHOST_KEYENC_BASE64 = 2 << 16 -# internal constants for SSH host verification outcomes -const SSH_HOST_KNOWN = 0 -const SSH_HOST_UNKNOWN = 1 -const SSH_HOST_MISMATCH = 2 -const SSH_HOST_BAD_HASH = 3 +# libssh2 host check return values +const LIBSSH2_KNOWNHOST_CHECK_MATCH = 0 +const LIBSSH2_KNOWNHOST_CHECK_MISMATCH = 1 +const LIBSSH2_KNOWNHOST_CHECK_NOTFOUND = 2 +const LIBSSH2_KNOWNHOST_CHECK_FAILURE = 3 @enum(GIT_SUBMODULE_IGNORE, SUBMODULE_IGNORE_UNSPECIFIED = -1, # use the submodule's configuration SUBMODULE_IGNORE_NONE = 1, # any change or untracked == dirty diff --git a/stdlib/LibGit2/test/libgit2.jl b/stdlib/LibGit2/test/libgit2.jl index 736d1459293c9..2479deeb31b6c 100644 --- a/stdlib/LibGit2/test/libgit2.jl +++ b/stdlib/LibGit2/test/libgit2.jl @@ -2406,35 +2406,17 @@ mktempdir() do dir end @testset "SSH known host checking" begin - key_hashes(sha1::String, sha256::String) = LibGit2.KeyHashes( - Tuple(hex2bytes(sha1)), - Tuple(hex2bytes(sha256)), - ) + CHECK_MATCH = LibGit2.Consts.LIBSSH2_KNOWNHOST_CHECK_MATCH + CHECK_MISMATCH = LibGit2.Consts.LIBSSH2_KNOWNHOST_CHECK_MISMATCH + CHECK_NOTFOUND = LibGit2.Consts.LIBSSH2_KNOWNHOST_CHECK_NOTFOUND + CHECK_FAILURE = LibGit2.Consts.LIBSSH2_KNOWNHOST_CHECK_FAILURE + # randomly generated hashes matching no hosts - random_key_hashes = key_hashes( - "a9971372d02a67bdfea82e2b4808b4cf478b49c0", - "45aac5c20d5c7f8b998fee12fa9b75086c0d3ed6e33063f7ce940409ff4efbbc" - ) + random_key = "\0\0\0\assh-rsa\0\0\0\x01#\0\0\0\x81\0¿\x95\xbe9\xfc9g\n:\xcf&\x06YA\xb5`\x97\xc13A\xbf;T+C\xc9Ut J>\xc5ҍ\xc4_S\x8a \xc1S\xeb\x15FH\xd2a\x04.D\xeeb\xac\x8f\xdb\xcc\xef\xc4l G\x9bR\xafp\x17s<=\x12\xab\x04ڳif\\A\x9ba0\xde%\xdei\x04\xc3\r\xb3\x81w\x88\xec\xc0f\x15A;AÝ\xc0r\xa1\u5fe\xd3\xf6)8\x8e\xa3\xcbc\xee\xdd\$\x04\x0f\xc1\xb4\x1f\xcc\xecK\xe0\x99" # hashes of the unique github.com fingerprint - github_key_hashes = key_hashes( - "bf6b6825d2977c511a475bbefb88aad54a92ac73", - "9d385b83a9175292561a5ec4d4818e0aca51a264f17420112ef88ac3a139498f" - ) + github_key = "\0\0\0\assh-rsa\0\0\0\x01#\0\0\x01\x01\0\xab`;\x85\x11\xa6vy\xbd\xb5@\xdb;\xd2\x03K\0J\xe96\xd0k\xe3\xd7`\xf0\x8f˪\xdbN\xb4\xedóǑ\xc7\n\xae\x9at\xc9Xi\xe4wD!«\xea\x92\xe5T0_8\xb5\xfdAK2\b\xe5t\xc37\xe3 \x93e\x18F,vRɋ1\xe1n}\xa6R;\xd2\0t*dD\xd8?\xcd^\x172\xd06sǷ\x81\x15UH{U\xf0\xc4IO8)\xec\xe6\x0f\x94%Z\x95˚\xf57\xd7\xfc\x8c\x7f\xe4\x9e\xf3\x18GN\xf2\x92\t\x92\x05\"e\xb0\xa0n\xa6mJ\x16\x7f\xd9\xf3\xa4\x8a\x1aJ0~\xc1\xea\xaaQI\xa9i\xa6\xac]V\xa5\xefb~Q}\x81\xfbdO[t\\OG\x8e\xcd\b*\x94\x92\xf7D\xaa\xd3&\xf7l\x8cM\xc9\x10\vƫyF\x1d&W\xcbo\x06\xde\xc9.kd\xa6V/\xf0\xe3 \x84\xea\x06\xce\x0e\xa9\xd3ZX;\xfb\0\xbaӌ\x9d\x19p github_hashes, - "gitlab.com" => gitlab_hashes, - ], hash in hashes + for (host, key) in [ + "github.com" => github_key, + "gitlab.com" => gitlab_key, + ] for files in [[no_file], [empty_file]] - check = LibGit2.ssh_knownhost_check(files, host, hash) - @test check == LibGit2.Consts.SSH_HOST_UNKNOWN + check = LibGit2.ssh_knownhost_check(files, host, key) + @test check == CHECK_NOTFOUND end for files in [ [known_hosts], - [empty_file; known_hosts], - [known_hosts; empty_file], - [known_hosts; wrong_hosts], + [empty_file, known_hosts], + [known_hosts, empty_file], + [known_hosts, wrong_hosts], ] - check = LibGit2.ssh_knownhost_check(files, host, hash) - @test check == LibGit2.Consts.SSH_HOST_KNOWN + check = LibGit2.ssh_knownhost_check(files, host, key) + @test check == CHECK_MATCH end for files in [ [wrong_hosts], - [empty_file; wrong_hosts], - [wrong_hosts; empty_file], - [wrong_hosts; known_hosts], + [empty_file, wrong_hosts], + [wrong_hosts, empty_file], + [wrong_hosts, known_hosts], ] - check = LibGit2.ssh_knownhost_check(files, host, hash) - @test check == LibGit2.Consts.SSH_HOST_MISMATCH + check = LibGit2.ssh_knownhost_check(files, host, key) + @test check == CHECK_MISMATCH end end end