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 connections advertise and fall back to HTTP/1.1 despite CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE #7980

Closed
akontsevoy opened this issue Nov 9, 2021 · 6 comments

Comments

@akontsevoy
Copy link

I did this

curl_easy_setopt(curl, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE); for an https URL.
The resulting ALPN extension still contained both h2 and http/1.1, and for servers not supporting HTTP/2, the handle falls back to HTTP/1.1 (both as if CURL_HTTP_VERSION_2 was used).

I expected the following

CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE should mean "HTTP/2 or fail" (TLS or no TLS). For TLS connections, this means ALPN extension should contain h2 only (no http/1.1), and if the server does not agree, then the transfer can fail immediately; or ALPN/NPN negotiation results can be ignored, HTTP/2 used anyway (as is over TCP with CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE), and transfer can fail at that stage. TBD which behavior is better; in theory since HTTP/2 servers are required to support either ALPN or NPN, then if they don't agree to h2 it can be assumed that HTTP/2 will not work; but in practice servers may be misconfigured at the transport layer, e.g. be behind a reverse proxy terminating TLS, but support HTTP/2 despite not advertising it.

curl/libcurl version

curl 7.79.1 (i386-pc-win32) libcurl/7.79.1 OpenSSL/1.1.1l zlib/1.2.11 nghttp2/1.46.0

operating system

Windows 10 21H1 amd64

@bagder
Copy link
Member

bagder commented Nov 9, 2021

CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE should mean "HTTP/2 or fail" (TLS or no TLS).

This doesn't seem to be what the current documentation says and neither does the code. Strictly speaking, "prior knowledge" isn't even a thing over HTTPS.

There is no "h2 or fail" option for libcurl when using HTTPS.

@akontsevoy
Copy link
Author

akontsevoy commented Nov 9, 2021

Actually, the standard leave it open to interpretation what "prior knowledge" means for HTTPS (https://datatracker.ietf.org/doc/html/rfc7540#section-3.4). It only specifies that ALPN must be used for negotiation; it does not say which protocols must be present in ALPN (i.e. which protocol versions the client must be willing to negotiate in the first place); so this decision is left to the implementation. To me, the most natural interpretation is one that is consistent with TCP, i.e. "http/2 or fail".

There is no "h2 or fail" option for libcurl when using HTTPS.

That's the point -- there probably should be. There are protocols that mandate the usage of HTTP/2, for example gRPC, but libcurl offers no way to force HTTP/2 over TLS without a hack (which is to disable ALPN and NPN in curl altogether, set a TLS context callback and manually stuff an h2-only ALPN header from there).

@akontsevoy
Copy link
Author

So if we're not willing to change the current behavior of CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE, perhaps a new option (CURL_HTTP_VERSION_2_ONLY) is in order?

@bagder
Copy link
Member

bagder commented Nov 11, 2021

There are protocols that mandate the usage of HTTP/2, for example gRPC

True, but servers that mandate HTTP/2 are also less likely to accept HTTP/1.1 because why would they?

So if we're not willing to change the current behavior of CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE, perhaps a new option (CURL_HTTP_VERSION_2_ONLY) is in order?

I'm not willing to change the existing option as there's a real risk it is actually used and dependent on as currently documented and working. It has been around for over five years already.

I would be fine with a CURL_HTTP_VERSION_2_ONLY option. The question is then perhaps how open ended it should be for future protocol versions, "2 plus" (it can't be version 3, but maybe version 4?)

@akontsevoy
Copy link
Author

True, but servers that mandate HTTP/2 are also less likely to accept HTTP/1.1 because why would they?

Mainly it's the reverse proxies that I'm concerned about. If it's a standalone HTTPS server, then sure -- why should it even accept/offer http/1.1 in ALPN/NPN if it's http/2 only. But a reverse proxy may have several back-ends (some of them http/2-only, some http/1.1 only, some both), and it may not have a good way to distinguish where it should accept which protocol by ALPN and where not, not even with SNI (because that's only host name, whereas reverse proxy routing may be based on URL or headers). Or it may be a straight transport-layer proxy without any knowledge of the HTTP layer... In other words, things get complicated, and the potential for misconfiguration is high (what we get for breaking the OSI model and putting application-layer features such as ALPN and SNI at the transport layer, where they don't belong). For robustness, if the application protocol is known upfront on the client side, it's nice to be able to force the specific HTTP version regardless of the transport layer (mis)configuration.

I'm not willing to change the existing option as there's a real risk it is actually used and dependent on as currently documented and working. It has been around for over five years already.

Agreed -- it's better not to repeat "fixing" CURLOPT_SHARE without CURLOPT_COOKIEFILE.

I would be fine with a CURL_HTTP_VERSION_2_ONLY option. The question is then perhaps how open ended it should be for future protocol versions, "2 plus" (it can't be version 3, but maybe version 4?)

I don't think the new option should be "2 plus" -- all existing CURLOPT_HTTP_VERSION options mean "that version or older" or "that version alone"; no options mean "that version or newer", and I find no reason to make CURL_HTTP_VERSION_2_ONLY different. Even if later HTTP revisions go back from UDP to TCP and provide the means to upgrade from or negotiate earlier HTTP versions over existing transport -- such cases can always be covered with additional "that version or older" options.

@bagder
Copy link
Member

bagder commented Dec 6, 2021

I'm adding this idea to the TODO document for future work by someone interested enough to see it made reality.

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

No branches or pull requests

2 participants