Skip to content

Commit

Permalink
LibGit2: patch to pass hostkey & port to host verify callback [ci skip]
Browse files Browse the repository at this point in the history
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.)
  versus 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 cananot be distinguished with 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.
  • Loading branch information
StefanKarpinski committed Jan 19, 2021
1 parent a3369df commit d5183c3
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 71 deletions.
2 changes: 1 addition & 1 deletion deps/Versions.make
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ CURL_JLL_NAME := LibCURL
LAPACK_VER := 3.9.0

# LibGit2
LIBGIT2_VER := 1.1.0
LIBGIT2_JLL_VER := 1.2.0+0
LIBGIT2_JLL_NAME := LibGit2

# LibSSH2
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
36f71a3f67e73b8af23f148f72714213
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
336e2a44706c200d124f825b7c28438bdb63acb44f7ed8bdf34642220ff2c4bb3526e634f8da3a31da00fb2a879a6d64378522de8109678b5feae9465bdf252e
8 changes: 7 additions & 1 deletion deps/libgit2.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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 $@)
Expand Down
61 changes: 61 additions & 0 deletions deps/patches/libgit2-hostkey.patch
Original file line number Diff line number Diff line change
@@ -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())
109 changes: 45 additions & 64 deletions stdlib/LibGit2/src/callbacks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -465,12 +454,20 @@ struct KnownHost
end

function ssh_knownhost_check(
files :: AbstractVector{<:AbstractString},
host :: AbstractString,
hashes :: KeyHashes,
files :: AbstractVector{<:AbstractString},
host :: AbstractString,
cert :: CertHostKey,
)
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
len = cert.len
data = collect(cert.data)
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},
Expand All @@ -492,46 +489,30 @@ 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
check = @ccall "libssh2".libssh2_knownhost_checkp(
hosts :: Ptr{Cvoid},
host :: Cstring,
port :: Cint,
data :: Ptr{UInt8},
len :: 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
check == Consts.LIBSSH2_KNOWNHOST_CHECK_FAILURE &&
@warn("Error searching SSH known hosts file `$file`")
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`"
Expand Down
10 changes: 5 additions & 5 deletions stdlib/LibGit2/src/consts.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d5183c3

Please sign in to comment.