Reject mismatched :authority and Host headers

Related to #245, and we may want to merge the work together. Read that issue before tackling this one.

HTTP/2 allows for both the :authority and Host header to be present in a request header block. If they are both present, however, they absolutely should be the same. We should add a stage to the header validation pipeline (possibly the same as the stage in #245) that confirms that, if present, these are both identical. If they are not identical, that's a ProtocolError.

For @python-hyper/contributors, see the message in #245 for guidance on tackling this.

This is partly resolved by #252. We now need only to reject sending headers that are mismatched in this way.

Interesting fact, recording here for posterity: traditionally servers like Apache have been known to dispatch based on the request line / :authority equivalent, while applications like Django have trusted the Host: header, and this has lead to real security bugs: http://www.skeletonscribe.net/2013/05/practical-http-host-header-attacks.html

So thanks to #252, this particular attack is not possible on h2-based servers.

Yup, it's fairly important to police this. It's one of the advantages of the newness of HTTP/2: we can tighten this stuff up pretty easily without breaking too much!

Just to clarify: We can still send mismatched header/:authority values?

For the moment, but we're working on a PR to stop that too.

Hm - that is actually an ability we would like to retain for mitmproxy. Can we cater for this somehow?

Doesn't have to sacrifice the API usability, I'd be fine with us calling h2._dont_check_host_mismatch = True somewhere after import.

@mhils

@mhils

I think (I don’t have a working setup to try) it would be possible to skip this particular validation step by stubbing out the validation function in h2.utilities, something like:
```
import h2
h2.utilities. _check_sent_host_authority_header = lambda headers: headers
```
Which is a fiddly solution, but should work to disable any part of the validation pipeline (ingoing or outgoing) that you wanted to skip.

I don’t know if @Lukasa is interested in offering anything less-fiddly than that. My gut feeling is against it, because it feels like it would add code complexity for what seems like a niche use case – but I don’t feel strongly.

Out of curiosity, what’s the use case for being able to send mismatched :authority/Host headers?
There’s an idea to do more policing of outgoing headers in general – see It’s possible to send header blocks from hyper-as-server that hyper-as-client will reject #262 – because right now it’s possible for h2 to send header blocks that it would reject if it received them. It might be worth checking whether anything there would cause problems for mitmproxy.

Out of curiosity, what’s the use case for being able to send mismatched :authority/Host headers?

We use hyper-h2 in mitmproxy: mitmproxy is an interactive SSL-capable intercepting HTTP proxy for penetration testers and software developers.

Precisely the security issues mentioned above are our use case 😉
We/our users want to send requests with mismatched :authority/Host headers, weird Transfer-Encoding headers, duplicate pseudo-header fields and/or pseudo-header fields coming after an ordinary header field, etc.

As long as validation is happening in isolated functions that we can just monkeypatch, we are good to go. Our use case is certainly the exotic one and I totally agree that this should not sacrifice everone else's API.
I mostly want to avoid a headache where validation is deeply embedded in more complex functions and then gets tricky for us to circumvent. Looking at the problem at hand, this should actually come along with a good coding style, so I don't see any roadblocks coming up here. Just wanted to raise a small flag that this is something we care about. ☺️

I think there's some value in offering something like strict_mode, where if it's disabled we skip a whole lot of checking.

Maybe at this time we should consider having a ConnectionSettings structure that is passed to a H2Connection object and allows setting certain flags to control behaviour. We could then have a setting: validate_outbound_headers, for example. If that's set to False, we just skip this step entirely.

How does that sound? If we pursue it, of course, we'll deprecate the other kwargs to H2Connection in favour of this object and move those kwargs to it.

@Lukasa

@Lukasa: As long as you come up with something we can accomodate for, please go ahead with whatever looks most reasonable to you.
We'd be fine to monkeypatch hyper-h2 for this (we pin the version anyway), but if you think this is useful for others as well, I'm happy to take advantage of a more prominent API. 😃

I'd rather avoid the monkeypatch: these are reasonable switches to want to toggle, and as long as the defaults are sensible I don't mind having them available in a library that's this low level.

Just make sure everyone who wants to do this has to import a bunch of obnoxiously long named constant values in order to do this:

from h2.connection.properties import ConnectionProperties
from h2.settings.hazmat.connection.authority import AUTHORITY_HOST_MISMATCH_ALLOWED
from h2.settings.hazmat.enable import YES_ENABLE_IT
ConnectionProperties({AUTHORITY_HOST_MISMATCH_ALLOWED: YES_ENABLE_IT})

this ought to discourage the civilians :).

Hah, gross! I don't think we need to be that aggressive about it: a stern warning in the docs is probably good enough, especially if it defaults to on.

Resolved.

Navigation Menu

Repository navigation

Reject mismatched :authority and Host headers #246

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Reject mismatched :authority and Host headers #246

Description

Activity

Lukasa commented on Jul 20, 2016

njsmith commented on Jul 24, 2016

Lukasa commented on Jul 24, 2016

mhils commented on Jul 24, 2016

Lukasa commented on Jul 24, 2016

mhils commented on Jul 24, 2016

alexwlchan commented on Jul 24, 2016

mhils commented on Jul 24, 2016

Lukasa commented on Jul 24, 2016

mhils commented on Jul 24, 2016

Lukasa commented on Jul 24, 2016

glyph commented on Jul 24, 2016

Lukasa commented on Jul 24, 2016

Lukasa commented on Aug 11, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions