Upgrade libgit2 to 1.3.0 #3962

nalimilan · 2021-11-28T21:53:09Z

nalimilan · 2021-11-28T22:11:28Z

L/LibGit2/bundled/patches/libgit2-hostkey.patch

-@@ -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) {


@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.

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:

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.

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.

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.

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.

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.

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://git@A.B.C.D:32800/nalimilan/FreqTables.jl.git

One weird fact is that if I call git clone ssh://git@A.B.C.D: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?

mkitti · 2022-01-07T11:14:22Z

Would it make sense to pin this to MbedTLS 2.28.0?

mkitti · 2022-01-07T11:16:34Z

JuliaLang/julia#42311 will not be able to succeed until we move the pin for LibGit2 to 2.28

LoadError("sysimg.jl", 19, LoadError("/buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.8/LibGit2/src/LibGit2.jl", 3, LoadError("/buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.8/LibGit2/src/utils.jl", 44, ErrorException("could not load library \"libgit2\"\nlibmbedtls.so.13: cannot open shared object file: No such file or directory"))))

ViralBShah · 2022-01-30T18:51:59Z

What do we need to get this merged?

giordano · 2022-01-30T18:58:02Z

Besides the fact it isn't clear to me whether the discussion above finished, we should decide whether to ship MbedTLS 2.28 in julia v1.8: if so, we must link libgit2 to that version (mbedtls breaks the ABI every time....)

ViralBShah · 2022-01-30T19:02:36Z

I do believe we want to ship MbedTLS 2.28 in julia v1.8, and because of the security vulnerabilities, we may even need to backport to 1.7 and 1.6

giordano · 2022-01-30T19:06:35Z

we may even need to backport to 1.7 and 1.6

That's not going to work: since mbedtls breaks the ABI, it'll break any package depending on it, like libcurl, libgit2.

nalimilan · 2022-01-30T19:08:20Z

I think everything is ready, we just need a confirmation by @StefanKarpinski. JuliaLang/julia#43250 bumps libgit2 in Julia and should be merged soon after this one.

ViralBShah · 2022-01-30T19:08:31Z

we may even need to backport to 1.7 and 1.6

That's not going to work: since mbedtls breaks the ABI, it'll break any package depending on it, like libcurl, libgit2.

Right - but if we don't that would mean that we have an LTS with know security vulnerabilities. It appears to me that we will probably need to update that whole bunch.

ViralBShah · 2022-01-30T19:09:36Z

@nalimilan If things are good and passing tests, I suggest merging.

ViralBShah · 2022-01-30T19:12:02Z

I think we still need to pin to MbedTLS 2.28 before merging.

L/LibGit2/build_tarballs.jl

giordano · 2022-01-30T19:19:47Z

Note that you'll first need to update mbedtls 2.28 in julia, and perhaps update all dependent packages together at the same time, otherwise nothing will work at all.

ViralBShah · 2022-01-30T19:26:54Z

I assume that all Ygg PRs need to be merged first, and then Julia 1.8-dev does a single PR to bump them all. We still need an upstream release of libssh2. Going step by step - can we merge this PR now?

nalimilan · 2022-02-07T08:41:35Z

@giordano Could you confirm what steps we should take?

mkitti · 2022-02-07T09:46:00Z

The issue to watch is #4208 for libssh2. It would be preferable to have a version bump upstream, although we need to consider if we should just create a libssh2 1.10.1 ourselves.

giordano · 2022-02-07T10:20:49Z

Yes, libgit2 and libssh2 linking to the new mbedtls must all go together.

nalimilan · 2022-02-07T10:56:05Z

OK. TBH I don't really understand the situation with libssh2: is there anything preventing us from upgrading to 1.10.0 and handling the security fix after the feature freeze?

ViralBShah · 2022-02-07T14:01:41Z

We can certainly after the freeze. It is just a crucial CVE in mbedtls, so sooner the better.

ViralBShah · 2022-02-07T18:33:28Z

@nalimilan We now have libssh2 1.10.1 merged in #4208. Updating this PR to use that version.

L/LibGit2/build_tarballs.jl

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

giordano

This is ok with me. Feel free to merge if there are no more concerns with compatibility on the julia side

ViralBShah · 2022-02-08T14:24:07Z

Only one way to find out.

* Upgrade libgit2 to 1.3.0 * Delete libgit2-continue-zlib.patch * Update libgit2-hostkey.patch * Update libgit2-hostkey.patch * Pin MbedTLS to 2.28 * Update L/LibGit2/build_tarballs.jl * Update build_tarballs.jl * Update build_tarballs.jl * Update L/LibGit2/build_tarballs.jl Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com> Co-authored-by: Viral B. Shah <ViralBShah@users.noreply.github.com> Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

nalimilan added 3 commits November 28, 2021 22:53

Upgrade libgit2 to 1.3.0

a10083b

Delete libgit2-continue-zlib.patch

a54f928

Update libgit2-hostkey.patch

30b1d8e

nalimilan mentioned this pull request Nov 28, 2021

Upgrade libgit2 to 1.3.0, libssh2 to 1.10.2, mbedtls to 2.28 and libcurl to 7.81.0 JuliaLang/julia#43250

Merged

Update libgit2-hostkey.patch

b229cee

nalimilan commented Nov 28, 2021

View reviewed changes

giordano requested a review from StefanKarpinski November 28, 2021 23:48

giordano added the julia 💜 ❤️ 💚 Builders and issues related to Julia and its dependencies label Nov 29, 2021

giordano mentioned this pull request Dec 22, 2021

Re-enable rr for Pkg tests JuliaLang/julia#43521

Closed

Pin MbedTLS to 2.28

99a1e63

giordano reviewed Jan 30, 2022

View reviewed changes

L/LibGit2/build_tarballs.jl Outdated Show resolved Hide resolved

Update L/LibGit2/build_tarballs.jl

e235fa6

ViralBShah added 2 commits February 7, 2022 13:34

Update build_tarballs.jl

2e4276d

Update build_tarballs.jl

b1cb2eb

giordano reviewed Feb 7, 2022

View reviewed changes

L/LibGit2/build_tarballs.jl Outdated Show resolved Hide resolved

Update L/LibGit2/build_tarballs.jl

38702d6

Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>

giordano approved these changes Feb 7, 2022

View reviewed changes

ViralBShah merged commit 4ebbdc2 into master Feb 8, 2022

ViralBShah deleted the nl/libgit2-1.3.0 branch February 8, 2022 14:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade libgit2 to 1.3.0 #3962

Upgrade libgit2 to 1.3.0 #3962

nalimilan commented Nov 28, 2021 •

edited

Loading

nalimilan Nov 28, 2021

StefanKarpinski Nov 29, 2021

StefanKarpinski Nov 29, 2021

nalimilan Nov 29, 2021

StefanKarpinski Nov 29, 2021

nalimilan Dec 19, 2021 •

edited

Loading

nalimilan Jan 4, 2022

mkitti commented Jan 7, 2022

mkitti commented Jan 7, 2022

ViralBShah commented Jan 30, 2022

giordano commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

giordano commented Jan 30, 2022

nalimilan commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

giordano commented Jan 30, 2022

ViralBShah commented Jan 30, 2022 •

edited

Loading

nalimilan commented Feb 7, 2022

mkitti commented Feb 7, 2022

giordano commented Feb 7, 2022 •

edited

Loading

nalimilan commented Feb 7, 2022

ViralBShah commented Feb 7, 2022

ViralBShah commented Feb 7, 2022 •

edited

Loading

giordano left a comment

ViralBShah commented Feb 8, 2022

Upgrade libgit2 to 1.3.0 #3962

Upgrade libgit2 to 1.3.0 #3962

Conversation

nalimilan commented Nov 28, 2021 • edited Loading

nalimilan Nov 28, 2021

Choose a reason for hiding this comment

StefanKarpinski Nov 29, 2021

Choose a reason for hiding this comment

StefanKarpinski Nov 29, 2021

Choose a reason for hiding this comment

nalimilan Nov 29, 2021

Choose a reason for hiding this comment

StefanKarpinski Nov 29, 2021

Choose a reason for hiding this comment

nalimilan Dec 19, 2021 • edited Loading

Choose a reason for hiding this comment

nalimilan Jan 4, 2022

Choose a reason for hiding this comment

mkitti commented Jan 7, 2022

mkitti commented Jan 7, 2022

ViralBShah commented Jan 30, 2022

giordano commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

giordano commented Jan 30, 2022

nalimilan commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

ViralBShah commented Jan 30, 2022

giordano commented Jan 30, 2022

ViralBShah commented Jan 30, 2022 • edited Loading

nalimilan commented Feb 7, 2022

mkitti commented Feb 7, 2022

giordano commented Feb 7, 2022 • edited Loading

nalimilan commented Feb 7, 2022

ViralBShah commented Feb 7, 2022

ViralBShah commented Feb 7, 2022 • edited Loading

giordano left a comment

Choose a reason for hiding this comment

ViralBShah commented Feb 8, 2022

nalimilan commented Nov 28, 2021 •

edited

Loading

nalimilan Dec 19, 2021 •

edited

Loading

ViralBShah commented Jan 30, 2022 •

edited

Loading

giordano commented Feb 7, 2022 •

edited

Loading

ViralBShah commented Feb 7, 2022 •

edited

Loading