Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade libgit2 to 1.3.0 #3962

Merged
merged 9 commits into from
Feb 8, 2022
Merged

Upgrade libgit2 to 1.3.0 #3962

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a10083b
Upgrade libgit2 to 1.3.0
nalimilan Nov 28, 2021
a54f928
Delete libgit2-continue-zlib.patch
nalimilan Nov 28, 2021
30b1d8e
Update libgit2-hostkey.patch
nalimilan Nov 28, 2021
b229cee
Update libgit2-hostkey.patch
nalimilan Nov 28, 2021
99a1e63
Pin MbedTLS to 2.28
ViralBShah Jan 30, 2022
e235fa6
Update L/LibGit2/build_tarballs.jl
giordano Jan 30, 2022
2e4276d
Update build_tarballs.jl
ViralBShah Feb 7, 2022
b1cb2eb
Update build_tarballs.jl
ViralBShah Feb 7, 2022
38702d6
Update L/LibGit2/build_tarballs.jl
ViralBShah Feb 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions L/LibGit2/build_tarballs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
using BinaryBuilder

name = "LibGit2"
version = v"1.2.3"
version = v"1.3.0"

# Collection of sources required to build libgit2
sources = [
GitSource("https://github.com/libgit2/libgit2.git",
"7f4fa178629d559c037a1f72f79f79af9c1ef8ce"),
"b7bad55e4bb0a285b073ba5e02b01d3f522fc95d"),
DirectorySource("./bundled"),
]

Expand All @@ -16,7 +16,6 @@ cd $WORKSPACE/srcdir/libgit2*/

atomic_patch -p1 $WORKSPACE/srcdir/patches/libgit2-agent-nonfatal.patch
atomic_patch -p1 $WORKSPACE/srcdir/patches/libgit2-hostkey.patch
atomic_patch -p1 $WORKSPACE/srcdir/patches/libgit2-continue-zlib.patch

BUILD_FLAGS=(
-DCMAKE_BUILD_TYPE=Release
Expand Down Expand Up @@ -50,7 +49,7 @@ make install

# These are the platforms we will build for by default, unless further
# platforms are passed in on the command line
platforms = supported_platforms(;experimental=true)
platforms = supported_platforms()

# The products that we will ensure are always built
products = [
Expand All @@ -59,10 +58,10 @@ products = [

# Dependencies that must be installed before this package can be built
dependencies = [
Dependency("MbedTLS_jll", v"2.24.0"),
Dependency("LibSSH2_jll"),
Dependency("MbedTLS_jll"; compat="~2.28.0"),
Dependency("LibSSH2_jll"; compat="~1.10.1"),
]

# Build the tarballs, and possibly a `build.jl` as well.
build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies; julia_compat="1.6")
build_tarballs(ARGS, name, version, sources, script, platforms, products, dependencies; julia_compat="1.8")

43 changes: 0 additions & 43 deletions L/LibGit2/bundled/patches/libgit2-continue-zlib.patch
View file
Open in desktop

This file was deleted.

42 changes: 5 additions & 37 deletions L/LibGit2/bundled/patches/libgit2-hostkey.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,48 +1,16 @@
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..ec6366a5f 100644
index 471c3273ed..32189d0979 100644
--- a/src/transports/ssh.c
+++ b/src/transports/ssh.c
@@ -523,6 +523,7 @@ static int _git_ssh_setup_conn(
@@ -525,6 +525,7 @@ static int _git_ssh_setup_conn(
git_credential *cred = NULL;
LIBSSH2_SESSION* session=NULL;
LIBSSH2_CHANNEL* channel=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) {
Comment on lines -5 to -44
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StefanKarpinski This part of the patch seems to have been replaced by libgit2/libgit2@29fe5f6. I'm not sure whether it's perfectly equivalent though. x-ref #2419.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun. I guess we need to make sure that the certificate_callback and ssh_knownhost_check stuff in LibGit2 still works with the new libgit2 version. The patch was required, IIRC, because the old version of libgit2 was doing several broken things:

  1. It was failing to pass the port number with the host, which is necessary when connecting over SSH using different ports and using the known hosts file.
  2. It didn't pass the actual hostkey info from the connection to the callback, which made it impossible to do hostkey verification.

It's unclear how (or if) anyone was actually using SSH hostkey checking previously.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the first thing is still broken but the second thing got fixed but in an incompatible way because they stuck that raw_type field in there instead of just appending the hostkey data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. So AFAIK I kept the patch for the first thing so it should be OK. For the second one, tests added by JuliaLang/julia#39324 cover it, right? At least they pass with libgit2 1.3 and this branch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that tests the port part but it looks like it tests the hostkey part. I was testing by actually connecting to a host that I was proxying through to github.

Copy link
Member Author

@nalimilan nalimilan Dec 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I've just done this, and it seems to work!

For the record, here's what I did to forward port 22 on my machine to github.com:

sudo iptables -t nat -A PREROUTING -i wlo1 -p tcp --dport 32800  -j DNAT --to 38.140.156.5:22
sudo iptables -A FORWARD -i wlo1 -p tcp --dport 22 -d 38.140.156.5 -j ACCEPT

Then I had to go through my router as trying to connect from the same machine avoids the forwarding.

Then I installed a package via (after manually adding A.B.C.D to ~/.ssh/known_hosts with the signature for github.com):

]add ssh://[email protected]:32800/nalimilan/FreqTables.jl.git

One weird fact is that if I call git clone ssh://[email protected]:32800/nalimilan/FreqTables.jl.git, an entry for [A.B.C.D]:32800 is added to ~/.ssh/known_hosts, but that entry isn't used by ]add ... to work. I need to add the signature without the port. I guess that's expected since the same happens on Julia 1.6.

Anything I missed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump.

@@ -597,7 +604,15 @@ post_extract:
@@ -636,7 +637,15 @@ post_extract:

cert_ptr = &cert;

Expand Down