Try migration to OpenSSL #53891
Try migration to OpenSSL #53891
Conversation
inkydragon
added
building
|
OK I'm learning about stdlib as I go along, feedback is very welcome. I think I've done what is needed to add OpenSSL_jl to stdlib. It allows me to build locally. There is one thing I think I missed, but I don't know how to fix it: I can't import OpenSSL_jll. I think it's built (because I had a bug in it at some point, and it was complaining which meant it tried to compile it, and now it doesn't complain anymore)… but I'm not sure. |
|
OK, somehow the OpenSSL_jll sources are not getting copied to the right directory. I see this message (not an error, but worrying): |
|
I think you need to add it to |
fxcoudert
force-pushed
the
openssl
branch
2 times, most recently
from
df3f08b
to
bbc918d
Compare
Thanks, that works. |
|
Once we finish JuliaPackaging/Yggdrasil#8377 (and JuliaPackaging/Yggdrasil#8386) and the corresponding builds are updated here, we can remove mbedTLS_jll as stdlib from here, since it isn't used for anything else, I think. |
|
Build works for me locally, but on CI it fails with: |
|
I'm not sure, but maybe that's during documentation building? |
|
Yes, it's during docs building, which I'm not doing locally (not sure what the |
|
@IanButterworth @KristofferC do I understand correctly we need to update |
|
@giordano yeah any manifests may need to be re-resolved |
May I ask how this is done? I couldn't figure it out from the doc. |
fxcoudert
force-pushed
the
openssl
branch
2 times, most recently
from
7c1c2de
to
44859e1
Compare
|
Manually updated |
|
It usually requires Pkg to resolve it to be accurate I just pushed the result of that. I also checked for other manifests in this repo that need updating and there doesn't appear to be any. Surprisingly there's only one in Pkg https://github.com/IanButterworth/Pkg.jl/blob/205309092c861d58bf787628ccbff4f5a41a0840/test/test_packages/ShouldPreserveSemver/Manifest.toml#L90 |
|
Thanks @IanButterworth That does not seem to pass CI (https://buildkite.com/julialang/julia-master/builds/35131#018e94a6-3c00-4b01-8b2a-8797e5b2ddf0), but… I can now run that command from my branch. I don't get the same output as you, here is the diff between your version and mine: Some versions and commit ids are different. But maybe more importantly, it does remove the |
|
OK, that makes the build pass on FreeBSD and macOS (both Intel and ARM). It's also failing on Windows. It's failing on My guess is that it's because in So: should I got back to the OpenSSL package and make it put libraries in On Windows, the naming convention is I'll let testing finish here, but I think Windows can be handled with: diff --git a/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl b/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl
index 26eb3d243e..bba9a0a299 100644
--- a/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl
+++ b/stdlib/OpenSSL_jll/src/OpenSSL_jll.jl
@@ -3,7 +3,7 @@
## dummy stub for https://github.com/JuliaBinaryWrappers/OpenSSL_jll.jl
baremodule OpenSSL_jll
-using Base, Libdl
+using Base, Libdl, Base.BinaryPlatforms
const PATH_list = String[]
const LIBPATH_list = String[]
@@ -20,8 +20,13 @@ libssl_handle::Ptr{Cvoid} = C_NULL
libssl_path::String = ""
if Sys.iswindows()
- const libcrypto = "libcrypto.dll"
- const libssl = "libssl.dll"
+ if arch(HostPlatform()) == "x86_64"
+ const libcrypto = "libcrypto-3-x64.dll"
+ const libssl = "libssl-3-x64.dll"
+ else
+ const libcrypto = "libcrypto-3.dll"
+ const libssl = "libssl-3.dll"
+ end
elseif Sys.isapple()
const libcrypto = "@rpath/libcrypto.3.dylib"
const libssl = "@rpath/libssl.3.dylib" |
|
Status:
I have to dig to understand why this used to work and doesn't anymore. It is working for me on a full source build, but not with binarybuilder. Go figure… |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Yes, we stumbled upon that already: JuliaPackaging/Yggdrasil#7576 (comment) |
It works locally for me with BinaryBuilder binaries: but it's possible libgit2/openssl is picking up from my system whatever certificates it needs. Maybe we somehow need to inform openssl/libgit2 where the certificates are ( |
| # If not already done, set the environment variable `SSL_CERT_FILE` when necessary. | ||
| cert_loc = ca_roots() | ||
| if cert_loc isa String | ||
| get!(ENV, "SSL_CERT_FILE", cert_loc) | ||
| end |
There was a problem hiding this comment.
@StefanKarpinski can you please check this makes sense? I followed the same logic as in
julia/stdlib/LibGit2/src/LibGit2.jl
Lines 1027 to 1028 in 5006312
There was a problem hiding this comment.
Shouldn't this be done in NetworkOptions though, so that everyone gets it?
|
Ok, setting the environment variable doesn't work well because there are tests explicitly checking that |
|
I got some bad ideas. One is that we mangle the binary in the same way as others. The other is that we modify the source code to read the location from somewhere. |
|
Ugh, we should probably schedule a call about this to figure out what to do... |
|
I'm kind of tempted to use a simpler TLS library that is better isolated. |
|
I just read through this, and as far as I can tell, the only thing that is failing is teaching OpenSSL about the certificate location, is that correct? |
|
Since mbedtls-2.28 is EOL'd at the end of 2024, any way we could nudge Julia to OpenSSL 3.X as distros, like Fedora, would like to drop 2.28. |
|
We should modify the BinaryBuilder build of OpenSSL such that these hardcoded paths are a longer placeholder path: For example, the conda-forge builds set this as follows: conda, mamba, or pixi then replaces these with environment specific locations. For example, I'm not suggesting that Julia do the mangling directly. Rather I suggest that rather we just use an extended placeholder path so that someone could easily replace it in the BinaryBuilder openssl binaries if needed. |
The construction of the placeholder path is as follows in Rust: let placeholder_template = "_placehold";
let mut placeholder = String::new();
let placeholder_length: usize = 255;
while placeholder.len() < placeholder_length {
placeholder.push_str(placeholder_template);
}
let placeholder = placeholder
[0..placeholder_length - build_dir.join("host_env").as_os_str().len()]
.to_string();
build_dir.join(format!("host_env{}", placeholder))A similar Julia function might be as follows. julia> function gen_placehold_path(
destdir = "/workspace/destdir";
placeholder_template = "_placehold",
placeholder_length = 255
)
len = 0
iob = IOBuffer()
while len < placeholder_length
len += write(iob, placeholder_template)
end
seekstart(iob)
placeholder = String(read(iob, placeholder_length - length(destdir) - 1))
return joinpath(destdir, placeholder)
end
gen_placehold_path (generic function with 2 methods)
julia> gen_placehold_path()
"/workspace/destdir/_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place" |
|
Not sure if anyone saw my comment at JuliaPackaging/Yggdrasil#9371 to I'll repeat it here: I think we can ensure OpenSSL finds its certificates and more by setting environment variables: That is, ensure The devil is of course in the details: do we wan the user to override these env vars, too? In that case the code checking them should perhaps only set them if they aren't already set. But to me this sounds like a luxury problem to be worried about if it affects someone. |
I recommend that Julia works with a SSL Context and create the necessary APIs to make one globally accessible: Environment variables can be quite fragile, especially on Windows. I do not think we should rely on them for the standard configuration. JuliaPackaging/Yggdrasil#9371 is not meant to implement a Julia configuration. Rather it is intended for particular interoperability scenarios where other software (e.g. Python) may depend on the default configuration of the library at build time. By providing a long path name in the Julia bundled openssl libraries, we provide the opportunity for the system integrator to mangle our openssl build rather than trying to make Julia work with third-party openssl libraries. |
I don't think we want integrators to do that at all. This is completely anathema to what JLLs do. We want them to be immutable |
To be clear, I do not particularly want people to do this either, but
If you would like to spend more time thinking about how software that is not Julia uses OpenSSL when interoperating with Julia, please do so. I'm tired of doing this, so I just settled on proposing the solution that others outside of Julia settled on. To be honest, I'm not really sure why people are so concerned about making this one useless string long. |
|
So looking at this it doesn't look like there's really a super satisifying path forward. Though it seems @mkitti has dealt with this on the conda side and honestly we should just follow that IMO. It works well for julia and has some solutions for interop. |
|
For Julia, we should just do what OpenSSL.jl is currently doing. That is we should use using OpenSSL
ctx = OpenSSL.SSLContext(OpenSSL.TLSClientMethod())The optional second argument to SSLContext is @assert ccall(
(:SSL_CTX_load_verify_locations, libssl),
Cint,
(SSLContext, Ptr{Cchar}, Ptr{Cchar}),
ssl_context,
verify_file,
C_NULL) == 1edit: To be consistent with the prior discussion, this should change to be |
|
For the PyCall / PythonCall based packages that we can control, we should modify them to use a SSL context as I mentioned earlier. |
|
On macOS: |
|
Here's a fix for ScikitLearn.jl: using OpenSSL
using ScikitLearn
using NetworkOptions
using PyCall
if length(ARGS) > 0 && ARGS[1] == "--fix"
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
"""
end
@sk_import datasets: (fetch_california_housing, clear_data_home)
clear_data_home()
println(length(fetch_california_housing()))credit to https://stackoverflow.com/a/54385130/10313003 for the inspiration. Without With |
|
I was trying to test SSL under the scenario of Python loading Julia and OpenSSL.jl via However, |
This is a test of migrating the source build to OpenSSL.