curl not functioning properly after updating to libnghttp2 1.49.0

[p-linnane]

Originally found in Homebrew/homebrew-core#108833

After upgrading to libnghttp2 1.49.0 I am not able to utilize curl without the error HTTP/2 stream 0 was not closed cleanly: PROTOCOL_ERROR (err 1).

curl -vv https://better-mouse.com
*   Trying 192.0.78.138:443...
* Connected to better-mouse.com (192.0.78.138) port 443 (#0)
* ALPN: offers h2
* ALPN: offers http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=tls.automattic.com
*  start date: Aug 20 16:38:12 2022 GMT
*  expire date: Nov 18 16:38:11 2022 GMT
*  subjectAltName: host "better-mouse.com" matched cert's "better-mouse.com"
*  issuer: C=US; O=Let's Encrypt; CN=R3
*  SSL certificate verify ok.
* Using HTTP2, server supports multiplexing
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* h2h3 [:method: GET]
* h2h3 [:path: /]
* h2h3 [:scheme: https]
* h2h3 [:authority: better-mouse.com]
* h2h3 [user-agent: curl/7.84.0]
* h2h3 [accept: */*]
* Using Stream ID: 1 (easy handle 0x137812400)
> GET / HTTP/2
> Host: better-mouse.com
> user-agent: curl/7.84.0
> accept: */*
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Connection state changed (MAX_CONCURRENT_STREAMS == 128)!
* HTTP/2 stream 0 was not closed cleanly: PROTOCOL_ERROR (err 1)
* Connection #0 to host better-mouse.com left intact
curl: (92) HTTP/2 stream 0 was not closed cleanly: PROTOCOL_ERROR (err 1)

Same command using the macOS system curl (truncated for relevancy):

/usr/bin/curl -vv https://better-mouse.com
*   Trying 192.0.78.138:443...
* Connected to better-mouse.com (192.0.78.138) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: /etc/ssl/cert.pem
*  CApath: none
* (304) (OUT), TLS handshake, Client hello (1):
* (304) (IN), TLS handshake, Server hello (2):
* (304) (IN), TLS handshake, Unknown (8):
* (304) (IN), TLS handshake, Certificate (11):
* (304) (IN), TLS handshake, CERT verify (15):
* (304) (IN), TLS handshake, Finished (20):
* (304) (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / AEAD-CHACHA20-POLY1305-SHA256
* ALPN, server accepted to use h2
* Server certificate:
*  subject: CN=tls.automattic.com
*  start date: Aug 20 16:38:12 2022 GMT
*  expire date: Nov 18 16:38:11 2022 GMT
*  subjectAltName: host "better-mouse.com" matched cert's "better-mouse.com"
*  issuer: C=US; O=Let's Encrypt; CN=R3
*  SSL certificate verify ok.
* 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 0x155011a00)
> GET / HTTP/2
> Host: better-mouse.com
> user-agent: curl/7.79.1
> accept: */*
>
* Connection state changed (MAX_CONCURRENT_STREAMS == 128)!
< HTTP/2 200
< server: nginx
< date: Thu, 25 Aug 2022 00:23:15 GMT
< content-type: text/html; charset=UTF-8
< strict-transport-security: max-age=31536000
< vary: Accept-Encoding
< last-modified: Thu, 25 Aug 2022 00:18:31 GMT
---

[tatsuhiro-t]

The web site returns malformed header field whose value ends with a white space. It is explicitly marked malformed by RFC 9113. RFC 9113 is a new revision of RFC 7445 HTTP/2. It says client must not accept malformed response.

Historically, nghttp2 follows RFC and treats this as a stream error. Application which needs accept such malformed header fields can specify nghttp2_on_invalid_header_callback2.

[bagder]

It seems a bit strict since the browsers don't fault on those headers so they will be used and this will only punish users of nghttp2...

[icing]

Interesting. Hmm, I assume that will affect my proxy module as well.

[bagder]

Further: as this most certainly is going to happen to more (curl) users very soon: is there a way we can figure out (and report) that this happened because of trailing space in a header field?

[icing]

I like the checks by nghttp2. When registering the callback hook, I will basically no longer use them as I am forced to have my own validation code run. I mean, we have header validation code in the server for the http/1.1 parts. I just have to make sure that nothing falls between the cracks when I switch...

[tatsuhiro-t]

If a browser-like behavior is needed, just returning 0 from nghttp2_on_invalid_header_callback2 would make most people happy.

We have no indication of which validation failed for the particular header field, so you can check the existence of trailing spaces in header field value, but there are other errors in name or value.

[icing]

Just to confirm my understanding of the nghttp2 code: returning 0 from a registered callback makes the header disappear without error. But there is no way that a header with trailing spaces might be accepted (other than the application passing it internally).

[tatsuhiro-t]

Yes. If you really want to treat trailing white spaces specially, you need to run some kind of validation by yourself.

Note that https://datatracker.ietf.org/doc/html/rfc9113#section-8.1.1

   These requirements are intended to protect against several types of
   common attacks against HTTP; they are deliberately strict because
   being permissive can expose implementations to these vulnerabilities.

The RFC authors know this is deliberately strict.

[bagder]

RFC authors also don't have users breathing down their neck... 😏

[icing]

That's what we get when we use a library from someone who follows standards. 😌

[tatsuhiro-t]

The code paths that lead to invalid header callback are quite limited.
In general:

1. nghttp2_check_header_name returns 0; or
2. nghttp2_check_header_value_rfc9113 returns 0.

'host' header field adds some complexity because it is validated by nghttp2_check_authority instead of nghttp2_check_header_value_rfc9113 if it is in request or PUSH_PROMISE.
But host is already required not to contain white spaces.

In on_invalid_header_callback, return error if nghttp2_check_header_name returns 0. And if you get host header field, then return error.
Otherwise, run nghttp2_check_header_value. If it returns nonzero, the header field is sent here because of trailing white spaces.

[tatsuhiro-t]

Alright, I will write HTTP/4 library without reading RFC.

[p-linnane]

This is very informative. Appreciate everyone's input here and glad to know this is deliberate and not a bug.

[Glandos]

A lot of Wordpress/Automattic website are returning x-ca headers with value ending with a white space. This is kind of a nightmare. I already contacted them using https://automattic.com/contact/ but if you have more "personal" contact, I think it's time to use it.

EDIT: same thing on https://data.opendatasoft.com/pages/home/ (contacted using https://hello.opendatasoft.com/contact)