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

Using SSL_NO_VERIFY stops sending the SNI header #113

Closed
aviks opened this issue Apr 19, 2021 · 3 comments
Closed

Using SSL_NO_VERIFY stops sending the SNI header #113

aviks opened this issue Apr 19, 2021 · 3 comments

Comments

@aviks
Copy link
Member

aviks commented Apr 19, 2021

When setting JULIA_SSL_NO_VERIFY_HOSTS, Downloads.jl does not send the SNI header. Some hosts fail to properly set up an SSL session when the SNI header is not present. Command line curl seems to present the header when when verification is switched off.

Tested on :
system curl : curl 7.64.1 (x86_64-apple-darwin19.0) libcurl/7.64.1 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.39.2

Downloads.jl: libcurl/7.73.0 SecureTransport zlib/1.2.11 libssh2/1.9.0 nghttp2/1.41.0

Tested with AWS Cloudfront, which shows this behaviour. Error shown from Pkg is:

err = SSL peer handshake failed, the server most likely requires a client certificate to connect while requesting https://.......

cc: @vdayanand

@StefanKarpinski
Copy link
Member

This seems to be a documented limitation of libcurl's CURLOPT_SSL_VERIFYHOST option:

Secure Transport: If verify value is 0, then SNI is also disabled. SNI is a TLS extension that sends the hostname to the server. The server may use that information to do such things as sending back a specific certificate for the hostname, or forwarding the request to a specific origin server. Some hostnames may be inaccessible if SNI is not sent.

We could try only turning the CURLOPT_SSL_VERIFYPEER option off instead of turning both options off. The difference between these two options is a little confusing, but my reading of the docs is that VERIFYHOST is about checking whether the cert the server presents is for the right host or not whereas VERIFYPEER is about checking whether the certificate presented can be verified via CA roots and whatnot. It might be ok to leave VERIFYHOST on since the main purpose of the option for this is as an escape hatch if someone's local CA roots are messed up or they are behind a MITM firewall. I don't think there are a lot of realistic scenarios where someone needs to talk to a server that's presenting a certificate for the wrong host.

My understanding is that what we do now matches the behavior of curl -k so I'd be interested if curl -k can be used to fetch such URLs. It would be helpful to have a specific URL to try downloading.

@StefanKarpinski
Copy link
Member

The "fix" here is just this patch:

diff --git a/src/Curl/Easy.jl b/src/Curl/Easy.jl
index 0ac9fc1..c143fdd 100644
--- a/src/Curl/Easy.jl
+++ b/src/Curl/Easy.jl
@@ -76,7 +76,6 @@ set_url(easy::Easy, url::AbstractString) = set_url(easy, String(url))

 function set_ssl_verify(easy::Easy, verify::Bool)
     setopt(easy, CURLOPT_SSL_VERIFYPEER, verify)
-    setopt(easy, CURLOPT_SSL_VERIFYHOST, verify*2)
 end

 function set_ssh_verify(easy::Easy, verify::Bool)

I would have thought that this would cause download to fail when trying to fetch https://wrong.host.badssl.com with ENV["JULIA_SSL_NO_VERIFY_HOSTS"] = "**.badssl.com" but for some reason it doesn't. I've tried all the various bad hostname pages at badssl.com and they all work with just CURLOPT_SSL_VERIFYPEER set to zero, so it seems fairly safe to stop setting CURLOPT_SSL_VERIFYHOST to zero, which I've verified does cause the client to send SNI.

@vdayanand
Copy link

It would be helpful to have a specific URL to try downloading.

julia> Downloads.download("https://pkg.juliahub.com/ui/index.html")
ERROR: SSL peer handshake failed, the server most likely requires a client certificate to connect while requesting https://pkg.juliahub.com/ui/index.html
Stacktrace:
  [1] (::Downloads.var"#9#18"{IOStream, Base.DevNull, Nothing, Vector{Pair{String, String}}, Float64, Nothing, Bool, Bool, String, Int64, Bool, Bool})(easy::Downloads.Curl.Easy)
    @ Downloads /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Downloads/src/Downloads.jl:356

 [12] top-level scope
    @ REPL[9]:1

julia> run(`curl -k  https://pkg.juliahub.com/ui/index.html`)
<!DOCTYPE html><html lang="en"><head><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=edge"><meta name="viewport" content="width=device-width,initial-scale=0.86,user-scalable=no,maximum-scale=0.86"><title>JuliaHub</title><link rel="shortcut icon" id="favicon" href="/static/favicon.ico"><link rel="stylesheet" href="/ui/Fonts/fonts.css"><link href="/ui/css/AdminPanel.e7842508.css" rel="prefetch"><link href="/ui/css/Applications.f7659dc9.css" rel="prefetch"><link href="/ui/css/Applications~Home~Run.eb837784.css" rel="prefetch"><link href="/ui/css/AuditLogViewer~LogViewer.f6502b22.css" rel="prefetch"><link href="/ui/css/CodeSearch.259755ca.css" rel="prefetch"><link href="/ui/css/CodeViewer.68533e03.css" rel="prefetch"><link href="/ui/css/Dashboard.cd857a9d.css" rel="prefetch"><link href="/ui/css/Datasets.d677a15c.css" rel="prefetch"><link href="/ui/css/DocumentationSearch.8b300cb5.css" rel="prefetch"><link href="/ui/css/Home.6fb944d4.css" rel="prefetch"><link href="/ui/css/JuliaPro.ca8b51d1.css" rel="prefetch"><link href="/ui/css/PackageDetails.3f0d5461.css" rel="prefetch"><link href="/ui/css/Packages.363abd08.css" rel="prefetch"><link href="/ui/css/Payments.d70651eb.css" rel="prefetch"><link href="/ui/css/Registrator.cb017124.css" rel="prefetch"><link href="/ui/css/Run.c95e044b.css" rel="prefetch"><link href="/ui/css/Secrets.379e19f3.css" rel="prefetch"><link href="/ui/css/Unauthorised.7a80c61b.css" rel="prefetch"><link href="/ui/css/_App.b5205a29.css" rel="prefetch"><link href="/ui/css/chunk-ce705a34.cd857a9d.css" rel="prefetch"><link href="/ui/js/AdminPanel.4c879bbf.js" rel="prefetch"><link href="/ui/js/AdminPanel~LogViewer.5700290c.js" rel="prefetch"><link href="/ui/js/Applications.826c78c9.js" rel="prefetch"><link href="/ui/js/Applications~Home~Run.8b6d3e75.js" rel="prefetch"><link href="/ui/js/AuditLogViewer.aefaa48b.js" rel="prefetch"><link href="/u```

StefanKarpinski added a commit that referenced this issue Apr 20, 2021
In https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html under
"Limitations", it is documented that when `CURLOPT_SSL_VERIFYHOST` is
set to zero this also turns off SNI (Server Name Indication):

> Secure Transport: If verify value is 0, then SNI is also disabled. SNI
> is a TLS extension that sends the hostname to the server. The server
> may use that information to do such things as sending back a specific
> certificate for the hostname, or forwarding the request to a specific
> origin server. Some hostnames may be inaccessible if SNI is not sent.

Since SNI is required to make requests to some HTTPS servers, disabling
SNI can break things. This change leaves host verification on and only
turns peer verification off (i.e. CA chain checking). I have yet to find
an example where turning host verification off is necessary.

Closes #113.
StefanKarpinski added a commit that referenced this issue May 4, 2021
In https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html under
"Limitations", it is documented that when `CURLOPT_SSL_VERIFYHOST` is
set to zero this also turns off SNI (Server Name Indication):

> Secure Transport: If verify value is 0, then SNI is also disabled. SNI
> is a TLS extension that sends the hostname to the server. The server
> may use that information to do such things as sending back a specific
> certificate for the hostname, or forwarding the request to a specific
> origin server. Some hostnames may be inaccessible if SNI is not sent.

Since SNI is required to make requests to some HTTPS servers, disabling
SNI can break things. This change leaves host verification on and only
turns peer verification off (i.e. CA chain checking). I have yet to find
an example where turning host verification off is necessary.

Closes #113.

(cherry picked from commit 86e52d7)
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

No branches or pull requests

3 participants