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

Read default cert at compile time #219

Merged
merged 1 commit into from
Mar 4, 2020
Merged

Conversation

JackDunnNZ
Copy link
Contributor

Fixes #218

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

@codecov-io
Copy link

codecov-io commented Feb 28, 2020

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%) ⬇️

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.

@quinnj quinnj merged commit d6bb0ba into JuliaLang:master Mar 4, 2020
@KwatMDPhD
Copy link

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

GSEA-MSigDB/GSEA2.jl#59

@JackDunnNZ
Copy link
Contributor Author

@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

@JackDunnNZ JackDunnNZ deleted the jd/cert branch August 5, 2022 13:20
@KwatMDPhD
Copy link

KwatMDPhD commented Aug 5, 2022

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

@KwatMDPhD
Copy link

KwatMDPhD commented Aug 5, 2022

@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
Copy link
Member

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
Copy link

Thanks for the response @KristofferC!

Screen Shot 2022-08-05 at 11 44 24

1.1.3.

@JackDunnNZ
Copy link
Contributor Author

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:

MbedTLS.jl/src/ssl.jl

Lines 803 to 808 in 22f5393

fallback = abspath(joinpath(Sys.BINDIR, "..", "share", "julia", "cert.pem"))
if isfile(MozillaCACerts_jll.cacert)
DEFAULT_CERT[] = read(MozillaCACerts_jll.cacert, String)
else
DEFAULT_CERT[] = read(fallback, String)
end

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
Copy link

I want to help fix this. Can I just revert this? 71bd43a

@JackDunnNZ
Copy link
Contributor Author

@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
Copy link
Member

Yes, that would solve it.

@KwatMDPhD
Copy link

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

KristofferC pushed a commit to JuliaLang/PackageCompiler.jl that referenced this pull request Aug 8, 2022
KristofferC added a commit to JuliaLang/PackageCompiler.jl that referenced this pull request Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SystemError opening cert.pem with PackageCompiler-generated binary
5 participants