Incorrect header normalization

[hollow1]

I'm submitting a ...

• [x] bug report
• [ ] feature request
• [ ] question about the decisions made in the repository

What is the current behavior? Server accepts and normalizes invalid HTTP/1.1 headers with a space before the colon, in violation of RFC 7230. If a cherrypy server is used behind an uncommon reverse proxy that accepts and forwards but doesn't normalize such invalid headers, the reverse proxy and the server can interpret the headers differently. This can lead to filter bypasses or request smuggling, the latter if requests from separate clients are multiplexed onto the same upstream connection by the proxy.

If the current behavior is a bug, please provide the steps to reproduce and if possible a screenshots and logs of the problem. If you can, show us your code. Note the space in Transfer-Encoding header. Request:

POST /generate HTTP/1.1
Host: localhost:8888
Content-Type: application/x-www-form-urlencoded
Content-Length: 22
Connection: close
Transfer-Encoding : chunked

8
length=1
0

Response:

HTTP/1.1 200 OK
Content-Type: text/html;charset=utf-8
Server: CherryPy/18.1.2
Date: Tue, 01 Oct 2019 06:42:47 GMT
Content-Length: 1
Connection: close

1

What is the expected behavior?

According to RFC7230:

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).

What is the motivation / use case for changing the behavior?

Prevent smuggling attacks. Maybe it makes sense to leave current behaviour as it is, just implement an option 'strict header parsing' for paranoid users.

Please tell us about your environment:

• Cheroot version: 6.5.6
• CherryPy version: 18.1.2
• Python version: 3.7.4
• OS: Mac OS 10.14.6
• Browser: all

Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, e.g. stackoverflow, gitter, etc.)

Same bug in Go: https://groups.google.com/forum/#!msg/golang-announce/cszieYyuL9Q/g4Z7pKaqAgAJ Lighthttpd: https://redmine.lighttpd.net/issues/2985?issue_count=2&issue_position=1&next_issue_id=2984 More info on smuggling attacks: https://portswigger.net/research/http-desync-attacks-request-smuggling-reborn

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[etsangsplk]

@webknjaz and @hollow1 Can we remove the strip() function? Is this where cherrypy webserver process incoming http request? https://github.com/cherrypy/cheroot/blob/master/cheroot/server.py#L239 Then we can enforce the headers in more proper format. Is there a need you need to support blank spaces in those type of headers?

[webknjaz]

@etsangsplk that sounds reasonable. The change would need to include the corresponding tests that demonstrate the expected behavior in case of both valid header format and different invalid ones.