[prev in list] [next in list] [prev in thread] [next in thread]
List: apache-httpd-dev
Subject: Re: svn commit: r1769965 - /httpd/httpd/trunk/server/vhost.c
From: William A Rowe Jr <wrowe () rowe-clan ! net>
Date: 2016-11-16 13:04:06
Message-ID: CACsi251+RvifPNaFRDnPcGRa-RbdsSQFsXX5CC8sF5pMx4OK3w () mail ! gmail ! com
[Download RAW message or body]
On Wed, Nov 16, 2016 at 1:52 PM, Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 11/16/2016 01:05 PM, wrowe@apache.org wrote:
> > Author: wrowe
> > Date: Wed Nov 16 12:05:53 2016
> > New Revision: 1769965
> >
> > URL: http://svn.apache.org/viewvc?rev=1769965&view=rev
> > Log:
> > Actually cause the Host header to be overridden, as noted by rpluem,
> > and simplify now that there isn't a log-only mode.
> >
> > I believe this logic to be busted. Given this request;
> >
> > GET http://distant-host.com/ HTTP/1.1
> > Host: proxy-host
> >
> > we would now fail to evaluate the proxy-host virtual host rules.
> >
> > This seems like a breaking change to our config. mod_proxy already
> > follows this rule of 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.
> >
> > Section 5.5 of RFC7230 has this to say;
> >
> > Once the effective request URI has been constructed, an origin server
> > needs to decide whether or not to provide service for that URI via
> > the connection in which the request was received. For example, the
> > request might have been misdirected, deliberately or accidentally,
> > such that the information within a received request-target or Host
> > header field differs from the host or port upon which the connection
> > has been made. If the connection is from a trusted gateway, that
> > inconsistency might be expected; otherwise, it might indicate an
> > attempt to bypass security filters, trick the server into delivering
> > non-public content, or poison a cache. See Section 9 for security
> > considerations regarding message routing.
> >
> > Section 5.3.1 states;
> >
> > To allow for transition to the absolute-form for all requests in some
> > future version of HTTP, a server MUST accept the absolute-form in
> > requests, even though HTTP/1.1 clients will only send them in
> > requests to proxies.
> >
> > It seems to me we should simply trust the Host: header and dump this
> whole
> > mess. If we want to reject requests in absolute form after the proxy
> modules
> > have had a chance to accept them, that wouldn't be a bad solution.
> >
> > Modified:
> > httpd/httpd/trunk/server/vhost.c
> >
> > Modified: httpd/httpd/trunk/server/vhost.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/
> vhost.c?rev=1769965&r1=1769964&r2=1769965&view=diff
> > ============================================================
> ==================
> > --- httpd/httpd/trunk/server/vhost.c (original)
> > +++ httpd/httpd/trunk/server/vhost.c Wed Nov 16 12:05:53 2016
> > @@ -1165,13 +1165,11 @@ AP_DECLARE(void) ap_update_vhost_from_he
> > * request line.
> > */
> > if (have_hostname_from_url && host_header != NULL) {
> > - const char *info = "Would replace";
> > - const char *new = construct_host_header(r, is_v6literal);
> > - apr_table_set(r->headers_in, "Host", r->hostname);
>
> IMHO the old code was wrong because r->hostname misses the surrounding []
> in case of IPV6 literals,
> but otherwise I see no change in logic here: Host part of the request
> still takes precedence over Host header.
>
Ok, I misread your original post, I thought you were pointing out that
r->hostname
is the Host: header value.
> > - info = "Replacing";
> > + const char *repl = construct_host_header(r, is_v6literal);
> > + apr_table_set(r->headers_in, "Host", repl);
> > ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02417)
> > - "%s Host header '%s' with host from request
> uri: "
> > - "'%s'", info, host_header, new);
> > + "Replacing host header '%s' with host '%s'
> given "
> > + "in the request uri", host_header, repl);
> > }
> > }
> >
> >
> >
> >
>
> Doesn't this need to get added to the large conformance backport proposal?
Added, or discarded entirely, let's resume on the other discussion thread.
[Attachment #3 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 16, 2016 \
at 1:52 PM, Ruediger Pluem <span dir="ltr"><<a href="mailto:rpluem@apache.org" \
target="_blank">rpluem@apache.org</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br> On 11/16/2016 01:05 \
PM, <a href="mailto:wrowe@apache.org">wrowe@apache.org</a> wrote:<br> > Author: \
wrowe<br> > Date: Wed Nov 16 12:05:53 2016<br>
> New Revision: 1769965<br>
><br>
> URL: <a href="http://svn.apache.org/viewvc?rev=1769965&view=rev" \
rel="noreferrer" target="_blank">http://svn.apache.org/viewvc?<wbr>rev=1769965&view=rev</a><br>
> Log:<br>
> Actually cause the Host header to be overridden, as noted by rpluem,<br>
> and simplify now that there isn't a log-only mode.<br>
><br>
> I believe this logic to be busted. Given this request;<br>
><br>
> GET <a href="http://distant-host.com/" rel="noreferrer" \
target="_blank">http://distant-host.com/</a> HTTP/1.1<br> > Host: proxy-host<br>
><br>
> we would now fail to evaluate the proxy-host virtual host rules.<br>
><br>
> This seems like a breaking change to our config. mod_proxy already<br>
> follows this rule of RFC7230 section 5.4;<br>
><br>
> When a proxy receives a request with an absolute-form of<br>
> request-target, the proxy MUST ignore the received Host header field<br>
> (if any) and instead replace it with the host information of the<br>
> request-target. A proxy that forwards such a request MUST generate a<br>
> new Host field-value based on the received request-target rather than<br>
> forward the received Host field-value.<br>
><br>
> Section 5.5 of RFC7230 has this to say;<br>
><br>
> Once the effective request URI has been constructed, an origin server<br>
> needs to decide whether or not to provide service for that URI via<br>
> the connection in which the request was received. For example, the<br>
> request might have been misdirected, deliberately or accidentally,<br>
> such that the information within a received request-target or Host<br>
> header field differs from the host or port upon which the connection<br>
> has been made. If the connection is from a trusted gateway, that<br>
> inconsistency might be expected; otherwise, it might indicate an<br>
> attempt to bypass security filters, trick the server into delivering<br>
> non-public content, or poison a cache. See Section 9 for security<br>
> considerations regarding message routing.<br>
><br>
> Section 5.3.1 states;<br>
><br>
> To allow for transition to the absolute-form for all requests in some<br>
> future version of HTTP, a server MUST accept the absolute-form in<br>
> requests, even though HTTP/1.1 clients will only send them in<br>
> requests to proxies.<br>
><br>
> It seems to me we should simply trust the Host: header and dump this whole<br>
> mess. If we want to reject requests in absolute form after the proxy modules<br>
> have had a chance to accept them, that wouldn't be a bad \
solution.<br>><br> > Modified:<br>
> httpd/httpd/trunk/server/<wbr>vhost.c<br>
><br>
> Modified: httpd/httpd/trunk/server/<wbr>vhost.c<br>
> URL: <a href="http://svn.apache.org/viewvc/httpd/httpd/trunk/server/vhost.c?rev=1769965&r1=1769964&r2=1769965&view=diff" \
rel="noreferrer" target="_blank">http://svn.apache.org/viewvc/<wbr>httpd/httpd/trunk/s \
erver/<wbr>vhost.c?rev=1769965&r1=<wbr>1769964&r2=1769965&view=diff</a><br>
> ==============================<wbr>==============================<wbr>==================<br>
> --- httpd/httpd/trunk/server/<wbr>vhost.c (original)<br>
> +++ httpd/httpd/trunk/server/<wbr>vhost.c Wed Nov 16 12:05:53 2016<br>
> @@ -1165,13 +1165,11 @@ AP_DECLARE(void) ap_update_vhost_from_he<br>
> * request line.<br>
> */<br>
> if (have_hostname_from_url && host_header != NULL) {<br>
> - const char *info = "Would replace";<br>
> - const char *new = construct_host_header(r, is_v6literal);<br>
> - apr_table_set(r->headers_in, "Host", \
r->hostname);<br> <br>
</div></div>IMHO the old code was wrong because r->hostname misses the surrounding \
[] in case of IPV6 literals,<br> but otherwise I see no change in logic here: Host \
part of the request still takes precedence over Host \
header.<br></blockquote><div><br></div><div>Ok, I misread your original post, I \
thought you were pointing out that r->hostname</div><div>is the Host: header \
value.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> - \
info = "Replacing";<br> > + const char *repl = \
construct_host_header(r, is_v6literal);<br> > + \
apr_table_set(r->headers_in, "Host", repl);<br> > \
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02417)<br> > - \
"%s Host header '%s' with host from request uri: "<br> > - \
"'%s'", info, host_header, new);<br> > + \
"Replacing host header '%s' with host '%s' given "<br> > \
+ "in the request uri", host_header, \
repl);<br> > }<br>
> }<br>
><br>
><br>
><br>
><br>
<br>
</span>Doesn't this need to get added to the large conformance backport \
proposal?</blockquote><div><br></div><div>Added, or discarded entirely, let's \
resume on the other discussion thread. </div></div><br></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic