<div dir="ltr"><div><div><div><div><div>Hi there,<br><br></div>While I agree that precise host comparison is suboptimal, I think that in this case it's better to chose the strictest (and simplest) approach for the sake of security.<br>
<br></div>Here is a slightly revised version of the patch:<br><a href="https://gist.github.com/gregvish/6572002/raw/4664ac0d3a81473086f185075a1f67c1e02b5877/v3_default_keepalive_patch">https://gist.github.com/gregvish/6572002/raw/4664ac0d3a81473086f185075a1f67c1e02b5877/v3_default_keepalive_patch</a><br>
<br></div>I've attempted to think of a nice way to decouple the code, but I couldn't think of anything pretty. At this point I've put some ifdefs around the code that references the keepalive module from the proxy and upstream modules. <br>
</div>This coupling is similar to the existing coupling of the "ngx_http_upstream_round_robin" module with the upstream module. In that case, the round robin balancer is used as a default for "un-resolved" upstreams. <br>
</div><div>I guess the right solution for the problem at hand should involve configuring both the balancer and keepalive (and other upstream options) for the default case.<br></div><br><div>Thanks,<br></div><div>Greg<br></div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Sep 11, 2013 at 7:32 PM, Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello!<br>
<div class="im"><br>
On Wed, Sep 11, 2013 at 06:54:00PM +0300, Greg Vishnepolsky wrote:<br>
<br>
[...]<br>
<br>
</div><div class="im">> However, in case of SSL connections, it is insufficient to identify a peer<br>
> according to the sockaddr. The hostname is important. For examlple:<br>
> <a href="https://a.host.com" target="_blank">https://a.host.com</a> resolves to <a href="http://1.1.1.1:443" target="_blank">1.1.1.1:443</a><br>
> <a href="https://b.host.com" target="_blank">https://b.host.com</a> also resoves to <a href="http://1.1.1.1:443" target="_blank">1.1.1.1:443</a><br>
> If the server at 1.1.1.1 holds an SSL cert _only_ for <a href="http://a.host.com" target="_blank">a.host.com</a>, it would<br>
> be wrong to use keepalive connections that were opened to this sockaddr for<br>
> requests for <a href="http://b.host.com" target="_blank">b.host.com</a>. If a connection will not be reused, during SSL<br>
> handshake the host cert can be properly verified for each new host.<br>
> The solution that I implemented for this is to add a "host" field to<br>
> "ngx_http_upstream_keepalive_cache_t" and<br>
> "ngx_http_upstream_keepalive_peer_data_t". The function<br>
> "ngx_http_upstream_get_keepalive_peer" now also checks that the host<br>
> matches, as well as the sockaddr to reuse a keepalive connection.<br>
<br>
</div>As of now, there is no SSL certificate verification in proxy, and<br>
hence there is no need for a check here.<br>
<br>
With ceritificate verification introduction some check will be<br>
needed, but just a host equality check might be suboptimal - e.g., a<br>
certificate might be for *.<a href="http://example.com" target="_blank">example.com</a>, and both <a href="http://a.example.com" target="_blank">a.example.com</a> and<br>
<a href="http://b.example.com" target="_blank">b.example.com</a> are valid hostnames for a connection, but a host<br>
check won't allow a connection reuse.   Possible solution would be<br>
to check SSL peer name on an already established connection.<br>
<br>
SNI will also complicate things once introduced.  But much like<br>
the certificate verification, it's a separate problem.<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
Maxim Dounin<br>
<a href="http://nginx.org/en/donation.html" target="_blank">http://nginx.org/en/donation.html</a><br>
<br>
_______________________________________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
</div></div></blockquote></div><br></div>