[PATCH] Re: Behavior of Host: vs. SNI Hostname in proxy CONNECT requests

["William A. Rowe Jr." <wr...@rowe-clan.net>]

On Mon, 25 Nov 2013 15:55:41 -0600
"William A. Rowe Jr." <wr...@rowe-clan.net> wrote:

> It appears that our SNI hostname comparison is invalid for forward
> proxy applications, specifically proxy CONNECT.  RFC 2616 states;
> 
>   14.23 Host
> 
>   The Host request-header field specifies the Internet host and port
>   number of the resource being requested, as obtained from the
> original URI given by the user or referring resource (generally an
> HTTP URL, as described in section 3.2.2). The Host field value MUST
> represent the naming authority of the origin server or gateway given
> by the original URL. This allows the origin server or gateway to
>   differentiate between internally-ambiguous URLs, such as the root
> "/" URL of a server for multiple host names on a single IP address.
> 
> I had read this as proxy host's name in forward proxy applications,
> but reading on, it's clear this is ambiguous;  from RFC 2817 
> 
>   5.2 Requesting a Tunnel with CONNECT
> 
>    A CONNECT method requests that a proxy establish a tunnel
> connection on its behalf. The Request-URI portion of the Request-Line
> is always an 'authority' as defined by URI Generic Syntax [2], which
> is to say the host name and port number destination of the requested
> connection separated by a colon:
> 
>       CONNECT server.example.com:80 HTTP/1.1
>       Host: server.example.com:80
> 
> So we have a clear, RFC-approved example of using the final endpoint
> as the Host: header value, not the intermediate proxy.example.com.
> 
> In this case, we fail the request because the SNI of the proxy machine
> does not match the Host: of the origin server name.
> 
> Ideas for the appropriate patch to httpd?  Scope this fix to CONNECT
> requests alone, or all forward proxy requests?

After reading a rather confused and confusing thread on this question,
I've come to the conclusion that the entire block of code below is
wrong;

167	rpluem	757373	    if ((servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name))) {
168	 	 	        char *host, *scope_id;
169	 	 	        apr_port_t port;
170	 	 	        apr_status_t rv;
171	 	 	
172	 	 	        /*
173	 	 	         * The SNI extension supplied a hostname. So don't accept requests
174	 	 	         * with either no hostname or a different hostname.
175	 	 	         */
176	 	 	        if (!r->hostname) {
177	sf	1209766	            ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, APLOGNO(02031)
178	rpluem	757373	                        "Hostname %s provided via SNI, but no hostname"
179	 	 	                        " provided in HTTP request", servername);
180	 	 	            return HTTP_BAD_REQUEST;
181	 	 	        }

This cannot occur for HTTP/1.1 requests as missing Host: is rejected elsewhere.

We have no particular reason to test for or prohibit HTTP/1.0 or 0.9 requests, 
AFAICT.

182	 	 	        rv = apr_parse_addr_port(&host, &scope_id, &port, r->hostname, r->pool);
183	 	 	        if (rv != APR_SUCCESS || scope_id) {
184	 	 	            return HTTP_BAD_REQUEST;
185	 	 	        }
186	jorton	1082189	        if (strcasecmp(host, servername)) {
187	sf	1209766	            ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, APLOGNO(02032)
188	rpluem	757373	                        "Hostname %s provided via SNI and hostname %s provided"
189	 	 	                        " via HTTP are different", servername, host);
190	 	 	            return HTTP_BAD_REQUEST;
191	 	 	        }

This occurs in all cases for HTTP CONNECT requests.  There seems to be no 
justification to test for this condition, since VirtualHost only establishes
a relationship of the HTTP Host: header.  Any relationship to TLS/SNI hostnames
is a new (unwise) invention, which has broken a number of facilities in httpd.

Validation of the SNI hostname against ServerName/ServerAlias already occurs 
elsewhere.  As Yann is pointing out, validation of SSL session requirements
also already occurs elsewhere.

The definition of SNI hostname and HTTP Host: header values were never equal,
only roughly equivalent, and this attempt to make them synonymous has really
been disruptive to httpd stability.

I'm proposing the following patch to drop this foolishness into the very same
SSLStrictSNIVHostCheck ruleset, for those interesting in adding these new,
non-HTTP compliance requirements as HTTP compliance requirements.

Comments?