RFC 7230 Compliance: Err 400 on space+colon for header field separator

[regilero]

​https://tools.ietf.org/html/rfc7230#section-3.2.4

No whitespace is allowed between the header field-name and colon. In
the past, differences in the handling of such whitespace have led to
security vulnerabilities in request routing and response handling. A
server MUST reject any received request message that contains
whitespace between a header field-name and colon with a response code
of 400 (Bad Request). A proxy MUST remove any such whitespace from a
response message before forwarding the message downstream.

Currently, sending such header, like:

    Dummy : header
         ^
         bad space

Nginx is not generating an err400. Nginx as a safe behavior:

• ignore the header value
• does not transmit it when used as a reverse proxy

But that's not the official way: (...) A server MUST reject any received request message(...)

Easy test:

# valid range header
printf 'GET / HTTP/1.1\r\n'\
'Host: nginx.org\r\n'\
'range: bytes=2-4\r\n''\r\n' | nc -q 3 95.211.80.227 80
# => 206 partial content

# invalid range header
printf 'GET / HTTP/1.1\r\n'\
'Host: nginx.org\r\n'\
'range : bytes=2-4\r\n''\r\n' | nc -q 3 95.211.80.227 80
# => 200 full response instead of 400

[Maxim Dounin]

Current nginx behaviour predates RFC 7230 by years and universally applies to multiple unusual characters which can be used in various exploits. I don't see how strictly following RFC 7230 and returning 400 in a very special case of a space character will improve things.

[regilero]

Well, at least it would prevent innocent errors from bad http client developpers.

Having an error 400 is always better than having an header silently removed.

With more HTTP client failing on this error, we'll maybe get more of them fixed, and every other actor would maybe stop accepting this sort of bad headers.

Having an invalid header following a chain of http agents, sometimes interpreted, sometimes not, is bad. With an error 400 the chain is stopped, no matter how much mistakes where made in previous front actors. With a silent removal we let a small door opened for attacks on such badly implemented reverse proxy (interpreting and transmitting this header without a fix).

So I think I understand why the RFC stated for an error 400, that would make the whole ecosystem more robust, not just nginx (which is fine on this subject, I agree).

[Maxim Dounin]

There are 256 different 1-byte characters, and only about 70 of them are more or less safe in HTTP headers. Everything else - including many explicitly allowed by RFC 7230 - are unsafe from security point of view. Returning 400 for just one of them isn't something which looks justified by an attempt to prevent innocent errors.

[regilero]

OK, but the problem is not this specific character. It's more about not being silent on invalid headers, but throwing an error 400 instead.

Well, it's a little about throwing error 400 on characters that may have been considered as legal in the past.

The main difference between silent removal of headers and an error is that no pipeline could occurs after an error 400. And this is a good extra step on security.

[Maxim Dounin]

Starting with 41f4bd4c51f1 (nginx 1.21.1), space and control characters are no longer allowed in header names, and requests with such headers are now unconditionally rejected.