Switch from MbedTLS to OpenSSL #56708

eschnett · 2024-11-28T16:45:34Z

Based on @fxcoudert's openssl branch and pull request at #53891.

diff re-applied to current Julia master (hence the new commit)
LibCURL, LibGit2, LibSSH2, OpenSSL updated to newest version
MbedTLS removed

@fxcoudert

Based on @fxcoudert's openssl branch and pull request at JuliaLang#53891. - diff re-applied to current Julia master (hence the new commit) - LibCURL, LibGit2, LibSSH2, OpenSSL updated to newest version - MbedTSL removed

eschnett · 2024-11-28T21:47:08Z

Current to-do list:

Understand how NetworkOptions and OpenSSL cooperate regarding CA roots. Possibly remove all NetworkOptions stuff from the OpenSSL_jll stdlib.
NetworkOptions has some MbedTLS-specific code. This needs to be switched to OpenSSL.
I added some OpenSSL CA-root work-around into the test cases. That should probably be removed again.
Something is off with LibCURL. We need to decide what to do, at the moment likely just staying at an older version.

giordano · 2024-11-28T22:04:34Z

Something is off with LibCURL. We need to decide what to do, at the moment likely just staying at an older version.

If you mean with libcurl v8.10+ JuliaLang/Downloads.jl#260, JuliaLang/Downloads.jl#260 (comment) should fix it. But there are still other failures: JuliaLang/Downloads.jl#261

eschnett · 2024-11-29T18:13:58Z

This PR requires the two other PRs

Currently these other PRs are hardcoded in stdlib/NetworkOptions.version and Pkg.version. That also needs to be undone.

eschnett · 2024-11-29T19:57:32Z

@giordano There is a likely-spurious test failure. Can you restart test x86_64-unknown-freebsd?

deps/checksums/SuiteSparse-e8285dd13a6d5b5cf52d8124793fc4d622d07554.tar.gz/md5

deps/checksums/clang

deps/checksums/llvm

deps/checksums/suitesparse

deps/checksums/unwind

deps/curl.mk

eschnett · 2024-11-30T18:57:34Z

@IanButterworth I don't see other errors, e.g. this works:

julia> using Downloads

julia> Downloads.download("https://github.com/eschnett/Pkg.jl")
"/tmp/jl_UvWhePmhGr"

giordano · 2024-12-01T18:26:51Z

Ok, now we're back to the point where Julia's own CI passes, but trying to load PyCall wreaks havoc:

$ julia +pr56708 --project -q
julia> using PyCall

julia> py"""
       import urllib.request
       with urllib.request.urlopen('http://python.org/') as response:
          html = response.read()
       """
ERROR: PyError ($(Expr(:escape, :(ccall(#= /home/mose/.julia/packages/PyCall/1gn3u/src/pyeval.jl:38 =# @pysym(:PyEval_EvalCode), PyPtr, (PyPtr, PyPtr, PyPtr), o, globals, locals))))) <class 'urllib.error.URLE
rror'>
URLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1007)'))

This is where we were stuck with #53891.

stdlib/LibSSH2_jll/test/runtests.jl

giordano · 2024-12-01T18:36:57Z

Should PyCall/PythonCall do something like

    py"""
    import ssl
    def create_julia_https_context():
        return ssl.create_default_context(cafile=$(NetworkOptions.bundled_ca_roots()))

    ssl._create_default_https_context = create_julia_https_context
    """

as suggested by @mkitti at #53891 (comment)? That does seem to work for me:

$ julia +pr56708 --project -q
julia> using PyCall, NetworkOptions

julia> py"""
       import ssl
       def create_julia_https_context():
           return ssl.create_default_context(cafile=$(NetworkOptions.bundled_ca_roots()))
       ssl._create_default_https_context = create_julia_https_context
       import urllib.request
       with urllib.request.urlopen('http://python.org/') as response:
           html = response.read()
       """

julia> py"html"
"<!doctype html>\n<!--[if lt IE 7]>   <html class=\"no-js ie6 lt-ie7 lt-ie8 lt-ie9\">   <![endif]-->\n<!--[if IE 7]>      <html class=\"no-js ie7 lt-ie8 lt-ie9\">          <![endif]-->\n<!--[if IE 8]>      <html class=\"no-js ie8 lt-ie9\">                 <![endif]-->\n<!--[if gt IE " ⋯ 50648 bytes ⋯ "s/IE8-min.8af6e26c7a3b.js\" charset=\"utf-8\"></script>\n    \n    \n    <![endif]-->\n\n    <!--[if lte IE 8]>\n
    <script type=\"text/javascript\" src=\"/static/js/plugins/getComputedStyle-min.d41d8cd98f00.js\" charset=\"utf-8\"></script>\n    \n    \n    <![endif]-->\n\n    \n\n    \n    \n\n</body>\n</html>\n"

CC @cjdoris @stevengj we really need your input here, otherwise this PR will be stuck again for ages and we don't move on.

KristofferC · 2024-12-01T19:13:38Z

Maybe asking for forgiveness is better than asking for permission here. As I understand it, this is quite important from a security standpoint so it if the ecosystem has to catch up for a while then so be it?

DilumAluthge · 2024-12-02T16:11:48Z

Are PyCall/PythonCall the only places in the ecosystem that this PR breaks?

Could we run a PkgEval on this PR, to see if any other parts of the ecosystem are broken by this PR?

giordano · 2024-12-02T16:21:11Z

This is the summary of last time I looked at PkgEval: #53891 (comment)

giordano · 2024-12-02T18:17:54Z

stdlib/Pkg.version

Reminder that before merging this PR the PRs to Pkg and NetworkOptions must be merged, and these files changed back to point to the original repo

Tests on Pkg doesn't pass until this is merged so it is a bit tricky.

Are there any tests that are run only on CI there, or we're good to have all tests passing on this PR (they were passing until yesterday or so)? We can do a coordinated merge of the three PRs, to reduce disruption.

I think it is fine to merge here and then when master updates rerun CI on Pkg and merge it there (and then update the commit again). The activity in the Pkg repo is not super high nowadays.

The alternative is to write the tests so that they pass both here and on master but that might be a bit more effort than it is worth.

The problem of merging this PR first is that it'd make its merge commit not buildable from source if Erik's branch or fork are deleted.

And thus can possibly break git bisect in the future?

We can merge it on Pkg with tests breaking if the manifest is the only test that matters. Or both manifests are checked in and the relevant one is renamed based on the version.

And thus can possibly break git bisect in the future?

Yeah, whenever someone needs to build a specific commit, git bisect is indeed my main concern.

Let me know when is a good time to do so. (After doing so, many things will fail on this branch before these repos are updated. We should do so only shortly before merging.)

For these files to be updated the respective changes to NetworkOptions and Pkg need to be merged first.

mkitti · 2024-12-02T20:38:10Z

Can we package mbedTLS as an independent package for other packages which may still have a hard dependency on it?

quinnj · 2024-12-02T20:44:49Z

Can we package mbedTLS as an independent package for other packages which may still have a hard dependency on it?

Separate from the existing mbedtls_jll and MbedTLS.jl packages that already exist?

giordano · 2024-12-02T22:21:57Z

@nanosoldier runtests()

DilumAluthge · 2024-12-02T22:50:15Z

I think the backticks are mandatory.

DilumAluthge · 2024-12-02T22:50:38Z

@nanosoldier runtests()

nanosoldier · 2024-12-03T11:08:15Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

eschnett · 2024-12-03T14:52:35Z

Some interesting failures: EasyCurl, JLLPrefixes.
Some expected/benign failures: SciMLTutorials (stores Manifest.toml in the repository, fails because MbedTLS_jll isn't there any more)

eschnett · 2024-12-06T16:55:08Z

How do we proceed here?

oscardssmith · 2024-12-06T21:16:46Z

pressing the merge button? :)

eschnett · 2024-12-10T15:20:10Z

If you are worried about merging three PRs simultaneously: With semver, the technically correct path would be to declare compatibilities via version numbers (i.e. bumping the major version number of NetworkOptions.jl and Pkg.jl) and applying the patches there, and then merging this PR here.

mkitti · 2024-12-10T18:25:41Z

How do we make MbedTLS_jll available?

eschnett · 2024-12-10T20:02:54Z

@mkitti MbedTLS_jll is available via the standard package registry. If you add MbedTLS_jll in your project it'll be added in the usual way. Having it in your Project.toml as dependency suffices. See e.g. https://github.com/JuliaLang/MbedTLS.jl/blob/master/Project.toml .

This PR removes MbedTLS_jll from the Project.toml in Julia's stdlib directory because no stdlib depends on it any more.

Julia's build-from-source mechanism can operate in two ways, either building all stdlibs from source, or (and that's the default) downloading jlls from the package registry. This is how MbedTLS_jll is installed by default.

I don't know whether I'm answering your question or saying something trivial.

mkitti · 2024-12-10T20:27:02Z

What's the fix for SciMLTutorials? Could we out together that PR first?

KristofferC · 2024-12-11T11:48:05Z

The Pkg bump should at least be rebased on current Pkg master.

eschnett · 2024-12-11T17:06:50Z

The fix for SciMLTutorials is to update all the Manifest.toml files in the repository. I assume that a using Pkg; Pkg.instantiate() would be sufficient. Pkg.update() would also work.

eschnett · 2024-12-17T00:29:06Z

@KristofferC I updated the Pkg.jl PR.

giordano · 2024-12-18T15:46:40Z

I went through all the new 35 failures reported by PkgEval:

issues to be solved in PyCall/PythonCall
- PyArrow
- Seaborn
- NeidArchive
- CometLogger
- EcRequests
- SunAsAStar
packages that need to be adapted (e.g. simply need to re-resolve an environment, or adapt compat bounds)
packages using libcurl API directly
- EasyCurl
unrelated failures (e.g. flaky tests, or with unreliable numerical tolerances)
didn't understand what's the issue, but likely unrelated (e.g. timeout)

If we're happy to let downstream developers fix the issues in their packages, e.g. for PyCall/PythonCall (the three BinaryBuilder-related ones are fine to ignore, issues should definitely be fixed in the package), then this is good to go from my point of view, as long as we decide what to do with the versions of the external stdlibs that need to be updated (I vote to merge the PRs to the external repos all together and then merge this PR, too, tests here are passing for julia + stdlibs)

eschnett · 2024-12-19T18:39:46Z

I merged in the master branch to resolve conflicts.

eschnett added 9 commits November 28, 2024 11:45

Switch from MbedTLS to OpenSSL

bf4a023

Based on @fxcoudert's openssl branch and pull request at JuliaLang#53891. - diff re-applied to current Julia master (hence the new commit) - LibCURL, LibGit2, LibSSH2, OpenSSL updated to newest version - MbedTSL removed

Update dependency checksums

1361539

Add openssl checksums

6dcf771

stdlib: Update Manifest.toml

2eaf9e7

Use curl 8.9.1

0c71d23

Update Manifests

2dddd78

Add LibCURL checksums

1538fdc

Merge branch 'master' into eschnett/openssl

e42eac4

Wiggle OpenSSL tests

cd9df42

eschnett requested review from giordano and StefanKarpinski November 28, 2024 21:22

eschnett added 2 commits November 28, 2024 20:42

Update sha1 and tests

117e762

Update NetworkOptions

e74bd3c

eschnett mentioned this pull request Nov 29, 2024

Enable OpenSSL certificates JuliaLang/NetworkOptions.jl#36

Open

eschnett added 5 commits November 28, 2024 20:59

Update checksums

5fd5ce1

Add NetworkOptions tarball url

2ab1e7d

Update checksums

2cc9b32

Remove file

f5400d9

Update checksums

a22c365

eschnett mentioned this pull request Nov 29, 2024

Update Manifest to avoid MbedTLS_jll JuliaLang/Pkg.jl#4100

Open

Update Pkg.jl

cef9458

eschnett mentioned this pull request Nov 29, 2024

Try migration to OpenSSL #53891

Open

giordano reviewed Nov 29, 2024

View reviewed changes

giordano added building Build system, or building Julia or its dependencies external dependencies Involves LLVM, OpenBLAS, or other linked libraries stdlib Julia's standard library labels Nov 29, 2024

giordano reviewed Dec 1, 2024

View reviewed changes

stdlib/LibSSH2_jll/test/runtests.jl Show resolved Hide resolved

LibSSH2: Correct expected version number

af33deb

LibSSH2_jll: Correct version number check

f7af55f

giordano requested changes Dec 2, 2024

View reviewed changes

giordano added the needs pkgeval Tests for all registered packages should be run with this change label Dec 2, 2024

Merge branch 'master' into eschnett/openssl

ba14e00

Switch from MbedTLS to OpenSSL #56708

Are you sure you want to change the base?

Switch from MbedTLS to OpenSSL #56708

Conversation

eschnett commented Nov 28, 2024 • edited Loading

eschnett commented Nov 28, 2024 • edited Loading

giordano commented Nov 28, 2024

eschnett commented Nov 29, 2024 • edited Loading

eschnett commented Nov 29, 2024

eschnett commented Nov 30, 2024

giordano commented Dec 1, 2024

giordano commented Dec 1, 2024

KristofferC commented Dec 1, 2024

DilumAluthge commented Dec 2, 2024

giordano commented Dec 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkitti commented Dec 2, 2024

quinnj commented Dec 2, 2024

giordano commented Dec 2, 2024

DilumAluthge commented Dec 2, 2024

DilumAluthge commented Dec 2, 2024

nanosoldier commented Dec 3, 2024

eschnett commented Dec 3, 2024

eschnett commented Dec 6, 2024

oscardssmith commented Dec 6, 2024

eschnett commented Dec 10, 2024

mkitti commented Dec 10, 2024

eschnett commented Dec 10, 2024

mkitti commented Dec 10, 2024 • edited Loading

KristofferC commented Dec 11, 2024

eschnett commented Dec 11, 2024

eschnett commented Dec 17, 2024

giordano commented Dec 18, 2024

eschnett commented Dec 19, 2024

eschnett commented Nov 28, 2024 •

edited

Loading

eschnett commented Nov 28, 2024 •

edited

Loading

eschnett commented Nov 29, 2024 •

edited

Loading

mkitti commented Dec 10, 2024 •

edited

Loading