Opened 7 years ago
Closed 16 months ago
#1014 closed enhancement (fixed)
RFC 7230 Compliance: Err 400 on space+colon for header field separator
|Reported by:||regilero||Owned by:|
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(...)
# 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 188.8.131.52 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 184.108.40.206 80 # => 200 full response instead of 400
Change History (5)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
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).
comment:3 by , 7 years ago
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.
comment:4 by , 7 years ago
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.
comment:5 by , 16 months ago
|Status:||new → closed|
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.
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.