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

Ensure TLS 1.3 works with GnuTLS #5223

Closed
wants to merge 1 commit into from
Closed

Conversation

@dbussink
Copy link
Contributor

dbussink commented Apr 12, 2020

When SRP is requested in the priority string, GnuTLS will disable support for TLS 1.3. Before this change, curl would always add +SRP to the priority list, effectively always disabling TLS 1.3 support.

With this change, +SRP is only added to the priority list when SRP authentication is also requested. This also allows updating the error handling here to not have to retry without SRP. This is because SRP is only added when requested and in that case a retry is not needed.

When SRP is requested in the priority string, GnuTLS will disable
support for TLS 1.3. Before this change, curl would always add +SRP to
the priority list, effectively always disabling TLS 1.3 support.

With this change, +SRP is only added to the priority list when SRP
authentication is also requested. This also allows updating the error
handling here to not have to retry without SRP. This is because SRP is
only added when requested and in that case a retry is not needed.
Copy link
Contributor Author

dbussink left a comment

I originally discovered this problem while trying to debug why git on the upcoming Ubuntu 20.04 release was only connecting over TLS 1.2 to GitHub.com, even though GitHub.com supports TLS 1.3.

Ubuntu compiles with OpenSSL normally for the command line curl where TLS 1.3 worked fine, but it didn't for the GnuTLS library version that is also provided and used by git.

rc = gnutls_priority_set_direct(session, prioritysrp, &err);
free(prioritysrp);

if((rc == GNUTLS_E_INVALID_REQUEST) && err) {
infof(data, "This GnuTLS does not support SRP\n");

This comment has been minimized.

Copy link
@dbussink

dbussink Apr 12, 2020

Author Contributor

I've kept this message here so it's shown in the same circumstances, but now only when SRP is explicitly requested.

@dbussink
Copy link
Contributor Author

dbussink commented Apr 12, 2020

Running this locally against a TLS 1.3 only site:

master

dirkjan@x:~/curl$ ./src/curl -v https://tls13.1d.pw/
*   Trying 178.128.250.95:443...
* Connected to tls13.1d.pw (178.128.250.95) port 443 (#0)
* found 128 certificates in /etc/ssl/certs/ca-certificates.crt
* ALPN, offering h2
* ALPN, offering http/1.1
* gnutls_handshake() failed: Error in protocol version
* Closing connection 0
curl: (35) gnutls_handshake() failed: Error in protocol version

fix-gnutls-tls13

dirkjan@x:~/curl$ ./src/curl -v https://tls13.1d.pw/
*   Trying 178.128.250.95:443...
* Connected to tls13.1d.pw (178.128.250.95) port 443 (#0)
* found 128 certificates in /etc/ssl/certs/ca-certificates.crt
* ALPN, offering h2
* ALPN, offering http/1.1
* SSL connection using TLS1.3 / ECDHE_RSA_AES_128_GCM_SHA256
* 	 server certificate verification OK
* 	 server certificate status verification SKIPPED
* 	 common name: tls13.1d.pw (matched)
* 	 server certificate expiration date OK
* 	 server certificate activation date OK
* 	 certificate public key: EC/ECDSA
* 	 certificate version: #3
* 	 subject: CN=tls13.1d.pw
* 	 start date: Wed, 18 Dec 2019 00:00:00 GMT
* 	 expire date: Fri, 17 Dec 2021 23:59:59 GMT
* 	 issuer: C=GB,ST=Greater Manchester,L=Salford,O=Sectigo Limited,CN=Sectigo ECC Domain Validation Secure Server CA
* ALPN, server did not agree to a protocol
> GET / HTTP/1.1
> Host: tls13.1d.pw
> User-Agent: curl/7.70.0-DEV
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: Z/pZ v.1.11.beta
< Public-Key-Pins: pin-sha256="LOip4wo3g8bntfGValXzDEmzTG23JtAZZ5zECX2rOlw="; pin-sha256="As6pWcwmAbUdO56S2HsZWl+kuLXR+ffow9nrkMHhrAQ="; pin-sha256="fec87drmn0s0sf95Tf2ltYSCesjFl43xCX++/laTvTs="; max-age=259201;
< Strict-Transport-Security: max-age=15552000
< X-Recent-Code-Change: desc="Updated URL handling"
< X-TLS-ClientRandom-Challenge: try="0xDEADDEADDEADC0DE0[0...]-in-Random"
< Content-Type: text/html; charset=UTF-8
< Connection: close
@dbussink
Copy link
Contributor Author

dbussink commented Apr 12, 2020

Missed adding a link to the GnuTLS documentation where it's stated that requesting SRP will disable TLS 1.3:

https://www.gnutls.org/manual/gnutls.html#Authentication-using-SRP

The implementation in GnuTLS is based on [TLSSRP]. The supported key exchange
methods are shown below. Enabling any of these key exchange methods in a session
disables support for TLS1.3. 
@bagder bagder added the SSL/TLS label Apr 12, 2020
@bagder
bagder approved these changes Apr 12, 2020
@bagder bagder closed this in d590908 Apr 12, 2020
@bagder
Copy link
Member

bagder commented Apr 12, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.