search

JuliaLang / MbedTLS.jl

Wrapper around mbedtls
Other
41 stars 50 forks source link

Read default cert at compile time #219

Closed JackDunnNZ closed 4 years ago

JackDunnNZ commented 4 years ago

Fixes #218

@kmsquire can you check if this fixes the problem for you?

codecov-io commented 4 years ago

Codecov Report

Merging #219 into master will decrease coverage by 0.04%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #219      +/-   ##
==========================================
- Coverage   72.63%   72.58%   -0.05%     
==========================================
  Files          12       12              
  Lines         570      569       -1     
==========================================
- Hits          414      413       -1     
  Misses        156      156
Impacted Files Coverage Δ
src/ssl.jl 66.66% <ø> (-0.15%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c419ee2...5b30311. Read the comment docs.

KwatMDPhD commented 2 years ago

Hi @JackDunnNZ , it looks like I still see this error where cert.pem is missing. Could you please help?

https://github.com/KwatMDPhD/GSEA.jl/issues/59

JackDunnNZ commented 2 years ago

@KwatMDPhD The problem seems to be that PackageCompiler doesn't bundle share/julia/cert.pem when creating an application, whereas MbedTLS assumes this file is always present. PackageCompiler should probably be changed to bundle this file into compiled applications

KwatMDPhD commented 2 years ago

Thanks for the explanation. @KristofferC could you please help?

KwatMDPhD commented 2 years ago

@JackDunnNZ , it looks like PackageCompiler upgraded with some breaking changes. Does MbedTLS take these into account these changes? For example the changes about artifacts.

https://github.com/JuliaLang/PackageCompiler.jl#upgrading-from-packagecompiler-10

KristofferC commented 2 years ago

What version of MbedTLS is used? The point of this PR is to read the cert at compile time and not runtime so I don't see why PackageCompiler has to bundle anything.

KwatMDPhD commented 2 years ago

Thanks for the response @KristofferC!

Screen Shot 2022-08-05 at 11 44 24

1.1.3.

JackDunnNZ commented 2 years ago

Looks like this code has since been changed, most recently in #246, so now the cert is being read from share/julia at init time:

https://github.com/JuliaLang/MbedTLS.jl/blob/22f5393176a1a470317fa831c2db2392a9ccfb1e/src/ssl.jl#L803-L808

So an alternative to PackageCompiler bundling it is to go back to reading the fallback cert at compile time, as this PR originally changed

KwatMDPhD commented 2 years ago

I want to help fix this. Can I just revert this? https://github.com/JuliaLang/MbedTLS.jl/pull/246/commits/71bd43a05575fa332e952cea49ef50f4091c479b

JackDunnNZ commented 2 years ago

@KristofferC I was looking at how the bundled cert is used in base, and it seems that MozillaCACerts_jll assumes it is always present at init time:

https://github.com/JuliaLang/julia/blob/fa986d9e0525eedd7706929d549c61179e6c6090/stdlib/MozillaCACerts_jll/src/MozillaCACerts_jll.jl#L20

If I understand correctly, this would mean a compiled app can currently fail if it depends on MozillaCACerts_jll, since cacert will point to a file that doesn't exist. If that's the case, it seems like it would be a reason to bundle the cert file in PackageCompiler?

KristofferC commented 2 years ago

Yes, that would solve it.

KwatMDPhD commented 2 years ago

I know you are busy, but it would be amazing if you could update PackageCompiler @KristofferC 🙏