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

--tls-max 1.1 should imply --http1.1 #8235

Closed
jhoyla opened this issue Jan 6, 2022 · 4 comments
Closed

--tls-max 1.1 should imply --http1.1 #8235

jhoyla opened this issue Jan 6, 2022 · 4 comments
Assignees

Comments

@jhoyla
Copy link

jhoyla commented Jan 6, 2022

According to the H/2 spec H/2 MUST use TLS 1.2 or higher:

   Implementations of HTTP/2 MUST use TLS version 1.2 [TLS12] or higher
   for HTTP/2 over TLS.

https://datatracker.ietf.org/doc/html/rfc7540#section-9.2

However if I run:

curl https://example.com --tls-max 1.1 -svo /dev/null
*   Trying 93.184.216.34:443...
* Connected to example.com (93.184.216.34) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1

curl offers h2 in the ALPN.

--tls-max 1.1 should imply --http1.1

curl https://example.com --tls-max 1.1 --http1.1 -svo /dev/null
*   Trying 93.184.216.34:443...
* Connected to example.com (93.184.216.34) port 443 (#0)
* ALPN, offering http/1.1
curl -V
curl 7.80.0 (x86_64-pc-linux-gnu) libcurl/7.80.0 OpenSSL/1.1.1m zlib/1.2.11 brotli/1.0.9 zstd/1.5.1 libidn2/2.3.2 libpsl/0.21.1 (+libidn2/2.3.0) libssh2/1.10.0 nghttp2/1.46.0
Release-Date: 2021-11-10
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL TLS-SRP UnixSockets zstd
uname -a
Linux peninsula 5.15.10-arch1-1 #1 SMP PREEMPT Fri, 17 Dec 2021 11:17:37 +0000 x86_64 GNU/Linux

This is something of the inverse of Issue #7980 and TODO 5.7

@bagder
Copy link
Member

bagder commented Jan 6, 2022

I acknowledge that curl allows any TLS version with HTTP/2. It does however usually require that you as a user select to go that route, meaning that you decided to do something differently. I don't see the harm in curl allowing this.

Also, TLS < 1.2 is destined to vanish from the internet soon enough, which might just be another reason why putting effort into enforcing this might not be worth it.

@bagder bagder self-assigned this Jan 8, 2022
@jhoyla
Copy link
Author

jhoyla commented Jan 10, 2022

Yeah, I'm not suggesting that you ignore the --http2 flag if manually specified, just that the default behaviour should be standards compliant.
I'm not quite as hopeful that TLS < 1.2 will disappear in practice any time soon. These things seem to have an unbelievably long tail.

@jhoyla
Copy link
Author

jhoyla commented Jan 10, 2022

A further aspect of this is that if a server configured to only support TLS 1.1 improperly negotiates H/2 (e.g. as the latest version of nginx will) then curl will go along with it. Not sure what the "correct" behaviour here should be, given that the other side is misbehaving, but maybe a warning?
I was looking at this because of your comment "you decided to do something differently", so here, a totally normal curl not restricted to any particular version of TLS results in this non-compliant behaviour.
Even on a secondary connection to the server curl continues to improperly use H/2.

curl -ksv https://localhost:443 https://localhost:443
*   Trying 127.0.0.1:443...
* Connected to localhost (127.0.0.1) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.1 (IN), TLS handshake, Certificate (11):
* TLSv1.1 (IN), TLS handshake, Server key exchange (12):
* TLSv1.1 (IN), TLS handshake, Server finished (14):
* TLSv1.1 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.1 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.1 (OUT), TLS handshake, Finished (20):
* TLSv1.1 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.1 / ECDHE-RSA-AES256-SHA
* ALPN, server accepted to use h2
* Server certificate:
*  subject: C=gb; O=Test; CN=localhost
*  start date: Jan 10 17:39:59 2022 GMT
*  expire date: Feb  9 17:39:59 2022 GMT
*  issuer: C=gb; O=Test; CN=Test Root CA
*  SSL certificate verify result: unable to get local issuer certificate (20), continuing anyway.
* Using HTTP2, server supports multiplexing
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x56100f4842d0)
> GET / HTTP/2
> Host: localhost
> user-agent: curl/7.81.0
> accept: */*
>
* Connection state changed (MAX_CONCURRENT_STREAMS == 128)!
< HTTP/2 200
< server: nginx/1.21.5
< date: Mon, 10 Jan 2022 17:50:08 GMT
< content-type: text/html
< content-length: 13
< last-modified: Mon, 10 Jan 2022 15:40:31 GMT
< etag: "61dc536f-d"
< accept-ranges: bytes
<
Hello World!
* Connection #0 to host localhost left intact
* Found bundle for host localhost: 0x56100f47eaf0 [can multiplex]
* Re-using existing connection! (#0) with host localhost
* Connected to localhost (127.0.0.1) port 443 (#0)
* Using Stream ID: 3 (easy handle 0x56100f4aab90)
> GET / HTTP/2
> Host: localhost
> user-agent: curl/7.81.0
> accept: */*
>
< HTTP/2 200
< server: nginx/1.21.5
< date: Mon, 10 Jan 2022 17:50:08 GMT
< content-type: text/html
< content-length: 13
< last-modified: Mon, 10 Jan 2022 15:40:31 GMT
< etag: "61dc536f-d"
< accept-ranges: bytes
<
Hello World!
* Connection #0 to host localhost left intact

@bagder
Copy link
Member

bagder commented Jan 13, 2022

maybe a warning?

libcurl doesn't really have any warning system. It just has the verbose messages, and in a case like this libcurl is already talking a lot of verbose messages so saying "ALERT: HTTP/2 connection using < TLS 1.2" I doubt many users will care, if even notice at all. I would not object to such a line though. Perhaps that's even the best we can do about this.

We don't have any existing good ways to insist on TLS 1.2 for HTTP/2 in libcurl right now, as the protocols are fairly independent. We don't even know if HTTP/2 is going to be used until after the TLS handshake, so basically the only action we have would be to force-close such a connection and retry it again with a raised minimum TLS requirement. And if it negotiated TLS < 1.2 in the first place, chances are it will fail in a subsequent 1.2 attempt. Is that going to make anyone happier? I mean apart from perhaps someone reading the HTTP/2 spec and checking off compliance check-marks?

Maybe the best "fix" for now is to simply document that we don't enforce this requirement.

We could of course insist on using < HTTP/2 when TLS < 1.2 is asked for. Will this make any users happier?

bagder added a commit that referenced this issue Jan 19, 2022
Both for --http2 and CURLOPT_HTTP_VERSION.

Reported-by: jhoyla on github
Fixes #8235
bagder added a commit that referenced this issue Jan 19, 2022
Both for --http2 and CURLOPT_HTTP_VERSION.

Reported-by: jhoyla on github
Fixes #8235
@bagder bagder closed this as completed in cdb495f Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants