Re: Enforce rewriting of Host header when an absolue URI is given
> 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
Re: Enforce rewriting of Host header when an absolue URI is given
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
Enforce rewriting of Host header when an absolue URI is given
(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