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.

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, libssh2 to 1.10.2, mbedtls to 2.28 and libcurl to 7.81.0 #43250

Merged
merged 12 commits into from
Feb 16, 2022

Conversation

nalimilan
Copy link
Member

@nalimilan nalimilan commented Nov 28, 2021

Goes with JuliaPackaging/Yggdrasil#3962.

Closes #42209
Closes #43249

@nalimilan nalimilan added the libgit2 The libgit2 library or the LibGit2 stdlib module label Nov 28, 2021
@DilumAluthge
Copy link
Member

Perhaps you need this patch? #43249 (comment)

@KristofferC
Copy link
Member

Perhaps you need this patch?

That's already in here I think: https://github.com/JuliaLang/julia/pull/43250/files#diff-ba4b52130833cf4924e96d191202c63e7ddee40545d5c001cc927e77a392f762R233-R235


Closes #42209?

@DilumAluthge
Copy link
Member

Hmmm. Not sure why tests are failing, then.

@DilumAluthge
Copy link
Member

Ah. I guess the Ygg PR needs to be merged first.

deps/libgit2.mk Outdated Show resolved Hide resolved
@mkitti
Copy link
Contributor

mkitti commented Jan 3, 2022

Just reporting in from https://github.com/conda-forge/julia-feedstock. With PR conda-forge/julia-feedstock#157, we deployed Julia 1.7.1 with LibGit2 1.3.

@ViralBShah
Copy link
Member

ViralBShah commented Feb 8, 2022

mbedtls, libssh2 and libgit2 should probably all be upgraded simultaneously, since libssh2 and libgit2 are hard linked to mbedtls 2.28. And of course one hopes everything will work after that!

@nalimilan
Copy link
Member Author

I've updated the PR to also upgrade libssh2 and mbedTLS, stealing commits from #42311. Please double check, as I'm quite confused by the versioning tricks from JuliaPackaging/Yggdrasil#4208. Also, what's going on with the patch at JuliaPackaging/Yggdrasil#4179? Should it be applied in Julia too?

@ViralBShah ViralBShah changed the title Upgrade libgit2 to 1.3.0 Upgrade libgit2 to 1.3.0, libssh2 to 1.10.2 and mbedtls to 2.28 Feb 10, 2022
@mkitti
Copy link
Contributor

mkitti commented Feb 10, 2022

We probably do want to import the patch into JuliaLang/Julia.

The build is still looking for libmbedtls.so.13

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"))))

@mkitti
Copy link
Contributor

mkitti commented Feb 10, 2022

Looks like libssh2 needs to be updated on this branch:
https://github.com/JuliaLang/julia/blob/nl/libgit2-1.3.0/deps/libssh2.version

Currently, it says 1.10.0.

LIBSSH2_BRANCH=libssh2-1.10.0
LIBSSH2_SHA1=635caa90787220ac3773c1d5ba11f1236c22eae8

Shouldn't it be 1.10.2 https://github.com/JuliaBinaryWrappers/LibSSH2_jll.jl/releases/tag/LibSSH2-v1.10.2%2B0 ?

@nalimilan
Copy link
Member Author

The build is still looking for libmbedtls.so.13

Yeah, but AFAICT this is due to libgit2.so (and then libssh2.so and libcurl.so) linking to libmbedtls.so.13. I guess they need to be rebuilt in Yggdrasil? The build works with USE_BINARYBUILDER_*=0 for these three deps.

Shouldn't it be 1.10.2 https://github.com/JuliaBinaryWrappers/LibSSH2_jll.jl/releases/tag/LibSSH2-v1.10.2%2B0 ?

This is one of the things I wondered. Since the variable says "branch", I assumed it has to refer to an existing branch in the repo? 1.10.2 doesn't actually exist upstream.

@mkitti
Copy link
Contributor

mkitti commented Feb 10, 2022

This is one of the things I wondered. Since the variable says "branch", I assumed it has to refer to an existing branch in the repo? 1.10.2 doesn't actually exist upstream.

This should be the same as this:
https://github.com/JuliaPackaging/Yggdrasil/blob/dd0bfdabbaef3493841f54bce34539b270ebab8c/L/LibSSH2/build_tarballs.jl#L7-L11

@nalimilan
Copy link
Member Author

OK, turns out I hadn't updated JLLs for all of these! But we'll need to update Curl for build to work: JuliaPackaging/Yggdrasil#4200

@nalimilan
Copy link
Member Author

This should be the same as this: https://github.com/JuliaPackaging/Yggdrasil/blob/dd0bfdabbaef3493841f54bce34539b270ebab8c/L/LibSSH2/build_tarballs.jl#L7-L11

So AFAICT that's what this PR currently does. version is 1.10.0 at that point of the file, and the SHA256 sum of the file matches.

@ViralBShah ViralBShah reopened this Feb 11, 2022
@giordano
Copy link
Contributor

Is the issue that the ABI breakage will break everything that links to mbedtls?

Correct: if a library has embedded that it requires libmbedtls.so.13 (which is mbedtls 2.24), there is no way it can be made to work with mbedtls which has a different soname (one could suggest to use patchelf, but that's begging for troubles elsewhere, you just shift the failure from loading time to runtime).

I suppose these packages need to be looked at and rebuilt? juliahub.com/ui/Packages/MbedTLS_jll/u5NEn/2.28.0+0?page=2

The fact is that you can't "just rebuild them", if you don't change version number and julia compat version correctly you'll break users on older versions of julia.

What I'm trying to say is that there is no easy to way to retroactively change binary dependencies of julia without lots of disruption which I'm not going to deal with if there isn't a robust discussion about how to deal with it. I could understand backporting stuff like libgit2 and libssh2 only which have a stable ABI (and I'd still be wary of doing that blindly), but mbedtls is a serious mess.

@nalimilan
Copy link
Member Author

It's probably better (both in terms of avoiding breakage and in terms of required work) to only backport patches that are needed to fix security issues. Note that Linux distributions have to do the same thing (notably Ubuntu LTS and CentOS, but that also applies to others over their shorter lifetimes), so with some luck we could just reuse the patches they backport. That's maybe something to keep in mind for the next LTS: using the same versions as a major LTS distribution would ensure that we can do that easily.

@nalimilan
Copy link
Member Author

Merge?

@mkitti
Copy link
Contributor

mkitti commented Feb 14, 2022

Why is buildbot/tester_win32 pending?

@ViralBShah
Copy link
Member

We have the CI straight flush. Haven't seen that in a while. Good to merge?

@ViralBShah ViralBShah added building Build system, or building Julia or its dependencies and removed backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Feb 15, 2022
@ViralBShah
Copy link
Member

I've removed the backport labels, since we may want to do a very limited backport and not this whole PR.

@nalimilan
Copy link
Member Author

64-bit Linux CI failed, but that doesn't seem related.

@ViralBShah
Copy link
Member

@nalimilan Merge?

@nalimilan nalimilan merged commit 3e5cd3c into master Feb 16, 2022
@nalimilan nalimilan deleted the nl/libgit2-1.3.0 branch February 16, 2022 15:09
@nalimilan nalimilan added the backport 1.8 Change should be backported to release-1.8 label Feb 16, 2022
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Feb 18, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
@Seelengrab
Copy link
Contributor

We may want to backport the libssh2 version bump to LTS: https://cve.circl.lu/cve/CVE-2019-17498

@giordano
Copy link
Contributor

That's not really doable without breaking some thousands of packages indirectly depending on mbedtls along the way

@Seelengrab
Copy link
Contributor

I'm not sure I understand - is libssh2 deeply tied to mbedtls? I think I'm missing some context here, since just some messages up you mentioned that libssh2 may be possible:

I could understand backporting stuff like libgit2 and libssh2 only which have a stable ABI (and I'd still be wary of doing that blindly), but mbedtls is a serious mess.

Though I of course understand that the impact of that CVE could be considered low enough to not be an issues. I definitely don't want to imply that it has to be done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LoadError: IOError: stream is closed or unusable with libgit2 1.3.0
8 participants