Re: Enforce rewriting of Host header when an absolue URI is given

[Jacob Champion]

Yann,

I found this while trying to understand the corner cases for Origin 
header checks for mod_websocket, and I do actually have some thoughts on 
it...


On 03/04/2015 07:21 AM, Yann Ylavic wrote:

(by default, not only with "HttpProtocol strict", which is trunk only btw).

Per RFC7230#section-5.4 :
When a proxy receives a request with an absolute-form of
request-target, the proxy MUST ignore the received Host header field
(if any) and instead replace it with the host information of the
request-target.  A proxy that forwards such a request MUST generate a
new Host field-value based on the received request-target rather than
forward the received Host field-value.

The first part is already honored, but not the forwarding part: the
Host header is not rewritten with the one extracted from the absolute
URI, neither at the protocol (core) level nor at proxy level.

There are PR56718 (and duplicate PR57563) about this.
Still the same question about whether providing the headers to some
CGI or module is a forward or not,


I don't buy this. IMO, CGI/plugins/modules/etc. are implementation 
details of the server.



I think personally it would be sane
to do this at the protocol level (beginning of the request).
I proposed a patch there and refined it a bit (attached), so that
section 5.4 is applied in vhost.c::fix_hostname().

It also implements this part of section 5.4 (second point, _underlined_) :
A server MUST respond with a 400 (Bad Request) status code to any
HTTP/1.1 request message that lacks a Host header field and _to any
request message that contains more than one Host header field_ or a
Host header field with an invalid field-value.

The first point is already handled, and the third is "HttpProtocol
strict" dependent (which is enough I think, but maybe deserve a
backport too).


Thoughts?


I have conflicting feelings on your approach, but I haven't pinned down 
a good argument against it. So here's an alternative for the sake of 
discussion: instead of rewriting the request to try to prevent 
downstream security issues (and allowing a request that is probably 
malicious to stay alive), why not reject the request outright? Any 
non-malicious uses of a mismatched Host will be broken by this patch 
anyway, right?


--Jacob

Re: Enforce rewriting of Host header when an absolue URI is given

[Roy T. Fielding]

> On Oct 26, 2015, at 10:33 AM, Jacob Champion  wrote:
> 
> Yann,
> 
> I found this while trying to understand the corner cases for Origin header 
> checks for mod_websocket, and I do actually have some thoughts on it...
> 
> On 03/04/2015 07:21 AM, Yann Ylavic wrote:
>> (by default, not only with "HttpProtocol strict", which is trunk only btw).
>> 
>> Per RFC7230#section-5.4 :
>>When a proxy receives a request with an absolute-form of
>>request-target, the proxy MUST ignore the received Host header field
>>(if any) and instead replace it with the host information of the
>>request-target.  A proxy that forwards such a request MUST generate a
>>new Host field-value based on the received request-target rather than
>>forward the received Host field-value.
>> 
>> The first part is already honored, but not the forwarding part: the
>> Host header is not rewritten with the one extracted from the absolute
>> URI, neither at the protocol (core) level nor at proxy level.
>> 
>> There are PR56718 (and duplicate PR57563) about this.
>> Still the same question about whether providing the headers to some
>> CGI or module is a forward or not,
> 
> I don't buy this. IMO, CGI/plugins/modules/etc. are implementation details of 
> the server.

Correct.  The word "proxy" in HTTP only applies to to the forwarding of HTTP
messages by a client-selected (forward) proxy.  There was no such thing as
a "reverse proxy" (an idiotic marketing term invented by Netscape) when the
original HTTP specs were written.  Those are gateways (as in Common Gateway 
Interface).

>> I think personally it would be sane
>> to do this at the protocol level (beginning of the request).
>> I proposed a patch there and refined it a bit (attached), so that
>> section 5.4 is applied in vhost.c::fix_hostname().
>> 
>> It also implements this part of section 5.4 (second point, _underlined_) :
>>A server MUST respond with a 400 (Bad Request) status code to any
>>HTTP/1.1 request message that lacks a Host header field and _to any
>>request message that contains more than one Host header field_ or a
>>Host header field with an invalid field-value.
>> 
>> The first point is already handled, and the third is "HttpProtocol
>> strict" dependent (which is enough I think, but maybe deserve a
>> backport too).

We should always be strict on received Host handling because misplaced routing
information is often used to bypass security filters.  That is, we should not 
allow
an invalid Host header field to pass through. It should at least be rejected
by default (if non-reject is configurable).

Roy

Enforce rewriting of Host header when an absolue URI is given

[Yann Ylavic]

(by default, not only with HttpProtocol strict, which is trunk only btw).

Per RFC7230#section-5.4 :
   When a proxy receives a request with an absolute-form of
   request-target, the proxy MUST ignore the received Host header field
   (if any) and instead replace it with the host information of the
   request-target.  A proxy that forwards such a request MUST generate a
   new Host field-value based on the received request-target rather than
   forward the received Host field-value.

The first part is already honored, but not the forwarding part: the
Host header is not rewritten with the one extracted from the absolute
URI, neither at the protocol (core) level nor at proxy level.

There are PR56718 (and duplicate PR57563) about this.
Still the same question about whether providing the headers to some
CGI or module is a forward or not, I think personally it would be sane
to do this at the protocol level (beginning of the request).
I proposed a patch there and refined it a bit (attached), so that
section 5.4 is applied in vhost.c::fix_hostname().

It also implements this part of section 5.4 (second point, _underlined_) :
   A server MUST respond with a 400 (Bad Request) status code to any
   HTTP/1.1 request message that lacks a Host header field and _to any
   request message that contains more than one Host header field_ or a
   Host header field with an invalid field-value.

The first point is already handled, and the third is HttpProtocol
strict dependent (which is enough I think, but maybe deserve a
backport too).


Thoughts?

Regards,
Yann.


httpd-trunk-fix_hostname.patch
Description: application/download