/curl

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

Error 35 when running curl with --tlsv1.2 option (Curl version 7.62.0) #3261

Closed
lucacanella opened this Issue Nov 12, 2018 · 10 comments

Comments

Assignees
No one assigned
Labels
SSL/TLS regression
Projects
None yet
Milestone
No milestone
4 participants
@lucacanella @danielgustafsson @bagder @pghmcfc
@lucacanella

lucacanella commented Nov 12, 2018

I did this

Run curl from command line with --tlsv1.2 option.
curl "https://github.com/curl/curl/issues/new" --tlsv1.2

results in error 35:
curl: (35) SSL version range is not valid.

I expected the following

I expected the same result as running curl on the same url without "--tlsv1.2" option, which means:

<html><body>You are being <a href="https://github.com/login?return_to=https%3A%2F%2Fgithub.
com%2Fcurl%2Fcurl%2Fissues%2Fnew">redirected</a>.</body></html>

curl/libcurl version

curl 7.62.0 (x86_64-redhat-linux-gnu) libcurl/7.62.0 NSS/3.36 zlib/1.2.7 libpsl/0.7.0 (+libicu/50.1.2) libssh2/1.8.0 nghttp2/1.31.1
Release-Date: 2018-10-31
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy PSL Metalink

operating system

CentOS Linux release 7.5.1804 (Core)

other info

This doesn't apply to curl version 7.61.0. Downgrading curl to 7.61 recovers functionality.

@bagder bagder added the SSL/TLS label Nov 12, 2018

@danielgustafsson

This comment has been minimized.

Sign in to view
Member

danielgustafsson commented Nov 12, 2018

Odd, I don't see any obvious changes since release but testing this on todays HEAD seems to work.

 $ ./src/.libs/curl --version
curl 7.63.0-DEV (x86_64-apple-darwin17.7.0) libcurl/7.63.0-DEV SecureTransport zlib/1.2.11
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS Debug TrackMemory IPv6 Largefile NTLM NTLM_WB SSL libz UnixSockets
$ ./src/.libs/curl "https://github.com/curl/curl/issues/new" --tlsv1.2
<html><body>You are being <a href="https://github.com/login?return_to=https%3A%2F%2Fgithub.com%2Fcurl%2Fcurl%2Fissues%2Fnew">redirected</a>.</body></html>
@bagder

This comment has been minimized.

Sign in to view
Member

bagder commented Nov 12, 2018

I think NSS/3.36 is a clue. We need to double-check with the NSS backend...

bagder added a commit that referenced this issue Nov 12, 2018

@bagder
nss: fix max-tls to be 1.3/1.2 
Fixes #3261
Verified
This commit was signed with a verified signature.
@bagder bagder Daniel Stenberg
GPG key ID: 5CC908FDB71E12C2 Learn about signing commits
Loading status checks…
a629c8a

@bagder bagder referenced this issue Nov 12, 2018

Merged

nss: fix max-tls to be 1.3/1.2 #3262

@bagder bagder added the regression label Nov 12, 2018

bagder added a commit that referenced this issue Nov 13, 2018

@bagder
nss: set default max-tls to 1.3/1.2 
Fixes #3261
Verified
This commit was signed with a verified signature.
@bagder bagder Daniel Stenberg
GPG key ID: 5CC908FDB71E12C2 Learn about signing commits
0c44809

@bagder bagder closed this in #3262 Nov 14, 2018

@pghmcfc

This comment has been minimized.

Sign in to view

pghmcfc commented Nov 30, 2018

This is still broken in the current nightly build:

$ curl "https://github.com/curl/curl/issues/new" --tlsv1.2
curl: (35) SSL version range is not valid.
$ curl --version
curl 7.63.0-20181130 (x86_64-redhat-linux-gnu) libcurl/7.63.0-20181130 NSS/3.36 zlib/1.2.7 libpsl/0.7.0 (+libicu/50.1.2) libssh2/1.8.0 nghttp2/1.31.1
Release-Date: 2018-11-30
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy PSL Metalink

Anything I can try to debug this?
@bagder

This comment has been minimized.

Sign in to view
Member

bagder commented Nov 30, 2018

As a comparison, here's my local NSS build:

curl 7.63.0-DEV (x86_64-pc-linux-gnu) libcurl/7.63.0-DEV NSS/3.39 zlib/1.2.11 brotli/1.0.7 c-ares/1.14.0 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.5) nghttp2/1.34.0 librtmp/2.3

$ curl "https://github.com/curl/curl/issues/new" --tlsv1.2
<html><body>You are being <a href="https://github.com/login?return_to ...

Anything I can try to debug this?

Probably by single-stepping through lib/vtls/nss.c:nss_setup_connect() and see where things go wrong. The calls to nss_init_sslver() and SSL_VersionRangeSet() are the prime suspects.
@pghmcfc

This comment has been minimized.

Sign in to view

pghmcfc commented Nov 30, 2018

It's failing in SSL_VersionRangeSet():

(gdb) print sslver
$1 = {min = 769, max = 772}
(gdb) s
nss_sslver_from_curl (nssver=nssver@entry=0x7fffffffd850, version=6) at ../../lib/vtls/nss.c:1640
1640	  switch(version) {
(gdb) s
1663	    *nssver = SSL_LIBRARY_VERSION_TLS_1_2;
(gdb) s
1664	    return CURLE_OK;
(gdb) s
1663	    *nssver = SSL_LIBRARY_VERSION_TLS_1_2;
(gdb) s
1664	    return CURLE_OK;
(gdb) s
nss_init_sslver (conn=0x63e640, data=0x63ae50, sslver=0x7fffffffd850) at ../../lib/vtls/nss.c:1706
1706	    if(result) {
(gdb) s
1712	  switch(max) {
(gdb) s
nss_setup_connect (conn=conn@entry=0x63e640, sockindex=sockindex@entry=0) at ../../lib/vtls/nss.c:1835
1835	  if(SSL_VersionRangeSet(model, &sslver) != SECSuccess)
(gdb) s
1812	  result = CURLE_SSL_CONNECT_ERROR;
(gdb) s
2048	  if(model)
(gdb) s
2049	    PR_Close(model);
(gdb) s
2051	  return nss_fail_connect(connssl, data, result);
(gdb) print sslver
$3 = {min = 771, max = 772}

It works for me with 7.61.1 by the way.
@bagder

This comment has been minimized.

Sign in to view
Member

bagder commented Nov 30, 2018

@pghmcfc is that perhaps because your version doesn't like TLS 1.3 as "max" ? Can you check if that's the case with a patch like:

--- a/lib/vtls/nss.c
+++ b/lib/vtls/nss.c
@@ -1777,11 +1777,11 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)
   CURLcode result;
   bool second_layer = FALSE;
 
   SSLVersionRange sslver = {
     SSL_LIBRARY_VERSION_TLS_1_0,  /* min */
-#ifdef SSL_LIBRARY_VERSION_TLS_1_3
+#ifdef xxxSSL_LIBRARY_VERSION_TLS_1_3
     SSL_LIBRARY_VERSION_TLS_1_3   /* max */
 #else
     SSL_LIBRARY_VERSION_TLS_1_2
 #endif
   };
@pghmcfc

This comment has been minimized.

Sign in to view

pghmcfc commented Dec 1, 2018

@bagder yes that's it. NSS in RHEL-7 is built with TLS 1.3 disabled:
https://git.centos.org/blob/rpms!nss.git/56f564f123836b3283dd0ec5e76ba22ae2e42072/SPECS!nss.spec#L356

$ curl --version
curl 7.62.0 (x86_64-redhat-linux-gnu) libcurl/7.62.0 NSS/3.36 zlib/1.2.7 libpsl/0.7.0 (+libicu/50.1.2) libssh2/1.8.0 nghttp2/1.31.1
Release-Date: 2018-10-31
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy PSL Metalink 
$ curl "https://github.com/curl/curl/issues/new" --tlsv1.2
<html><body>You are being <a href="https://github.com/login?return_to=https%3A%2F%2Fgithub.com%2Fcurl%2Fcurl%2Fissues%2Fnew">redirected</a>.</body></html>
@bagder

This comment has been minimized.

Sign in to view
Member

bagder commented Dec 2, 2018

Of course they do! :-(

Ok so we need to add some logic and see if we can make NSS agree to a lesser max version.

@bagder bagder reopened this Dec 2, 2018

bagder added a commit that referenced this issue Dec 2, 2018

@bagder
nss: if built with disabled TLS 1.3, do an extra TLS version range check 
Reported-by: Paul Howarth
Fixes #3261
Verified
This commit was signed with a verified signature.
@bagder bagder Daniel Stenberg
GPG key ID: 5CC908FDB71E12C2 Learn about signing commits
Loading status checks…
61ce482

@bagder bagder referenced this issue Dec 2, 2018

Closed

nss: if built with disabled TLS 1.3, do an extra TLS version range check #3334

@bagder

This comment has been minimized.

Sign in to view
Member

bagder commented Dec 2, 2018

@pghmcfc please try my suggested patch in the #3334 pull request. It tries to lower the max to 1.2 if the minimum allows that and the 1.3 attempt returned a "bad range". I figure that could work?

pghmcfc pushed a commit to pghmcfc/curl that referenced this issue Dec 3, 2018

Paul Howarth
nss: Fall back to latest supported SSL version 
NSS may be built without support for the latest SSL/TLS versions,
leading to "SSL version range is not valid" errors when the library
code supports a recent version (e.g. TLS v1.3) but it has explicitly
been disabled.

This change adjusts the maximum SSL version requested by libcurl to
be the maximum supported version at runtime, as long as that version
is at least as high as the minimum version required by libcurl.

Fixes curl#3261
109abb8

@pghmcfc pghmcfc referenced this issue Dec 3, 2018

Closed

nss: Fall back to latest supported SSL version #3337

@pghmcfc

This comment has been minimized.

Sign in to view

pghmcfc commented Dec 3, 2018

@badger, that didn't work (for reasons I can't quite fathom from looking at the code) but see #3337 for what I think is a cleaner fix, which works for me. This gets the highest supported version from NSS and uses that if it's lower than the current maximum and not less than the minimum required version.

pghmcfc pushed a commit to pghmcfc/curl that referenced this issue Dec 4, 2018

Paul Howarth
nss: Fall back to latest supported SSL version 
NSS may be built without support for the latest SSL/TLS versions,
leading to "SSL version range is not valid" errors when the library
code supports a recent version (e.g. TLS v1.3) but it has explicitly
been disabled.

This change adjusts the maximum SSL version requested by libcurl to
be the maximum supported version at runtime, as long as that version
is at least as high as the minimum version required by libcurl.

Fixes curl#3261
0751db5

@bagder bagder closed this in 6848ea5 Dec 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment