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

WIP: Add ability to find the system CACert or explicitly define one yourself #57

Closed
wants to merge 5 commits into from

Conversation

fchorney
Copy link
Contributor

@fchorney fchorney commented Mar 7, 2019

This PR defines two new functions, two lists, and two types.

Two consts: cert_files and cert_directories which contain paths to the most common locations for Certificate Authority Certificates on various OS types.

Since LibCURL isn't compiled to naturally know where the certs are on any given machine, I have written find_system_cacert as a way to search for a system cert based on known locations on various OS's/installations

2 new types have been created for this. CACertFile which contains a string that defines a path to a specific pem/crt file, and CACertPath which contains a string that defines a path to a directory that contains pem/crt files. I decided to use two different types as there are 2 different commands in LibCURL to set the file path vs the directory path, and thus by using these types we can leverage multiple dispatch to use the correct function.

find_system_cacert will return a CACertFile or a CACertPath depending on what location it finds the certs in. If it can't seem to find any system certs at all, it will return with nothing, and the user must check against it returning nothing and handle the errors themselves.

On top of this, I have defined enable_cacert which is a function that takes in a curl handle and either a CACertFile or CACertPath and applies the path using the correct LibCURL function to the handle so that any future requests with that handle will use the certificate that was found.

- Find a system CACert if it exists
- If not, you can explicitly set your own CACertFile or CACertPath
- Added test to use system cacert for an ssl query
@fchorney
Copy link
Contributor Author

fchorney commented Mar 7, 2019

Hmm interesting, so on the linux tests, it looks like it's using the system certificate without specifying it, or it can connect to google perfectly fine for some reason.

@codecov-io
Copy link

Codecov Report

Merging #57 into master will decrease coverage by 82.75%.
The diff coverage is 38.46%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #57       +/-   ##
===========================================
- Coverage     100%   17.24%   -82.76%     
===========================================
  Files           2        3        +1     
  Lines           5       87       +82     
===========================================
+ Hits            5       15       +10     
- Misses          0       72       +72
Impacted Files Coverage Δ
src/LibCURL.jl 45.45% <ø> (-54.55%) ⬇️
src/cert.jl 38.46% <38.46%> (ø)
src/lC_curl_h.jl 7.93% <0%> (-92.07%) ⬇️

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 1a3d44f...544d4b9. Read the comment docs.

@iamed2
Copy link

iamed2 commented Mar 7, 2019

https://curl.haxx.se/docs/sslcerts.html

See "Certificate Verification with Schannel and Secure Transport". Seems like libcurl will look in different places on Windows and macOS...

@omus
Copy link
Collaborator

omus commented Mar 7, 2019

The fact that Linux works does point to the problem being just a configuration issue.

@omus
Copy link
Collaborator

omus commented May 21, 2020

As of #75 we now have resolved the certificate issue by using MozillaCACerts_jll. I don't believe this functionality is needed with this addressed.

@omus omus closed this May 21, 2020
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.

4 participants