[squid-dev] [PATCH] Splice to origin cache_peer

[Amos Jeffries]

On 24/06/2015 2:54 a.m., Tsantilas Christos wrote:
> Currently, Squid cannot redirect intercepted connections that are
> subject to SslBump rules to _originserver_ cache_peer. For example,
> consider Squid that enforces "safe search" by redirecting clients to
> forcesafesearch.example.com. Consider a TLS client that tries to connect
> to www.example.com. Squid needs to send that client to
> forcesafesearch.example.com (without changing the host header and SNI
> information; those would still point to www.example.com for safe search
> to work as intended!).
> 
> The admin may configure Squid to send intercepted clients to an
> originserver cache_peer with the forcesafesearch.example.com address.
> Such a configuration does not currently work together with ssl_bump
> peek/splice rules.
> 
> This patch:
> 
> * Fixes src/neighbors.cc bug which prevented CONNECT requests from going
> to originserver cache peers. This bug affects both true CONNECT requests
> and intercepted SSL/TLS connections (with fake CONNECT requests). Squid
> use the CachePeer::in_addr.port which is not meant to be used for the
> HTTP port, apparently. HTTP checks should use CachePeer::http_port instead.
> 
> * Changes Squid to not initiate SSL/TLS connection to cache_peer for
> true CONNECT requests.
> 
> * Allows forwarding being-peeked (or stared) at connections to
> originserver cache_peers.
> 
> 
> This is a Measurement Factory project.
> 

General comment: remember that SSL (all versions) are now deprecated and
target is to kill all use of SSL (and references if we can). Please use
"TLS" for naming and documenting new things that are generic TLS/SSL and
not explicitly part of SSLv2 or SSLv3 protocols.


in src/FwdState.cc:

* Took me ages to figure out why sslToPeer contains
!userWillSslToPeerForUs. Please either rename sslToPeer  as
needTlsToPeer OR add code comments to document those logics more clearly.
 - please add comment that userWillSslToPeerForUs assumes CONNECT ==
HTTPS (which is not always true in reality).


+1. Other than that bit of polish this looks fine. The updated patch can
go in without another review.

Amos