New option to drop headers that contain underscores about cheroot HOT 6 CLOSED

Hello Martin,

I like your idea overall. I'm currently not sure about the config, perhaps I'll figure this out later.
You may proceed with PR and decide about the feature toggle later during review.

from cheroot.

Maybe 'drop_underscore_headers' is a better option name than 'underscores_in_headers'.
Default should probably be False for now to avoid surprises. (But if I am not mistaken, Apache and nginx seem to have this enabled by default.)

The implementation is trivial, however I don't know what is the best way to configure it.
Configuration should be possible when Cheroot is used directly, but we should also offer to configure this through the CherryPy framework

We could

• configure it as attribute of server.HTTPServer
  Don't know if this a good place, but max_request_header_sizelives there for example.
  But how could a user configure it using th config.ini or using@cherrypy.config(**{...})?
• Configuration-wise I think that :
  @cherrypy.config(**{'server.drop_underscore_headers': True})
  would be intuitive for a CherryPy user.
  But how can this setting be accessed from Cheroot code?

Any advice?

from cheroot.

Can you update the original post to describe (or link to) the original problem description (explaining why someone would care about this issue)?

I can't help but feel like this issue is a problem with the WSGI spec, since it's WSGI that's informing the choice to translate lowercase and dash-separated names to uppercase, underscore-separated names. It seems to me the issue doesn't apply only to the dash/underscore translation but also with the case sensitivity. This translation doesn't happen in the base HTTPServer.

This finding makes me wonder if the wsgi spec has anything to say about the handling of conflated headers as you describe.

from cheroot.

Reading PEP 3333 gives me little insight.

from cheroot.

I think it may have security implications:

Consider a CherryPy application running behind a proxy that does NTLM or SAML
authentication.
If the user is authenticated, a header field will be injected, otherwise this header is removed or emptied.
Since the app is only reachable through the proxy it will trust this field.

Assume the field is named X-Auth-User-Name.

An attacker that is not authorized or authorized with a restricted account could issue an ajax request with additional headers, e.g.
X_Auth-User_name: admin, X_Auth_User-name: admin, …
(Easy with browser plugins, for example.)

With a bit of luck, CherryPy will replace the protected field with the
elevated variant.

I don’t think that this is strictly a bug in Cheroot (since the proxy could filter this out), but the proposed change would make it easier to harden the application.

from cheroot.

We actually fell over this, in that a client could overwrite the HTTP headers of a proxy. We discovered this ticket and the solution.

But using DropUnderscoreHeaderReader actually breaks the service completely as it expects a native str, but the header is passed as a byte string:

Traceback (most recent call last):
    File ".../python3.7/site-packages/cheroot/server.py", line 1274, in communicate
    req.parse_request()
  File ".../python3.7/site-packages/cheroot/server.py", line 716, in parse_request
    success = self.read_request_headers()
  File ".../python3.7/site-packages/cheroot/server.py", line 971, in read_request_headers
    self.header_reader(self.rfile, self.inheaders)
  File ".../python3.7/site-packages/cheroot/server.py", line 226, in __call__
    if not self._allow_header(k):
  File ".../python3.7/site-packages/cheroot/server.py", line 250, in _allow_header
    return orig and '_' not in key_name
TypeError: a bytes-like object is required, not 'str'

This tells me

a) this class did never work
b) everybody using cheroot so far is prone to header spoofing attacks

I'd really like cheroot to be secure by default...

Greetings, Torsten

from cheroot.