Re: [RFC] CONNECT and use_ssl cache_peer

On 8/09/2012 3:54 p.m., Alex Rousskov wrote:
> Hello,
>
> I came upon a Squid proxy configured with "cache_peer ssl" (i.e.,
> peer->use_ssl is true). The proxy forwards GETs to the peer using an SSL
> connection because forward.cc code knows that "SSL peers" need an SSL
> connection and establishes such a connection. This works well.
>
> However, when the proxy receives a CONNECT request, it diverts the
> request to tunnel.cc. Tunnel.cc is aware of peer selection logic but
> lacks the code that establishes an SSL connection to SSL peers. Squid
> establishes a regular TCP connection to the SSL peer. The SSL peer
> receives a plain CONNECT request from Squid instead of the expected SSL
> handshake and closes the connection with an error.
>
>
> Here are my top three choices for the fix:
>
> 1) Teach tunnel.cc code to establish SSL connections to SSL peers, like
> forward.cc does. This design is straightforward, but duplicates
> significant (and sometimes rather complex) portions of forward.cc code.
> For example, FwdState::initiateSSL() and FwdState::negotiateSSL() would
> need to be duplicated (with the exception of SslBump code).
>
>
> 2) Remove peer selection (and SSL peer connection) code from tunnel.cc.
> Pass CONNECT requests to forward.cc and allow forward.cc to call
> tunnelStart() after selecting (and connecting to) the peer as usual.
> This probably requires rewriting tunnel.cc to work with
> clientStreams/storeEntries because forward.cc kind of relies on that
> API. While I would like to see that kind of design eventually, I do not
> want to rewrite so much just to fix this bug. And these more complex
> APIs are likely to also make tunneling less efficient.
>
>
> 3) Extract [SSL] peer connection establishment code from tunnel.cc and
> forward.cc into a new class. Allow tunnel.cc and forward.cc code to use
> that new class to establish the SSL connection as needed, returning
> control back to the caller (either tunnel.cc or forward.cc). This avoids
> code duplication but SslBump and other code specific to forward.cc will
> complicate a clean extraction of the forward.cc code into a stand-alone
> peer-connection-establishing class.
>
>
> My current plan is to try #3 to avoid code duplication but if it becomes
> too complex, go for #1 and duplicate what is necessary. What do you
> think? Any better ideas?

My vote is for #2 if you can do it. That needs to be done long-term
anyways and will result in less time trying to upkeep separate pathways
in tunnel.cc and forward.cc. The tunnel.cc pathway is already way behind
on features like 1xx status, chunking, keep-alive and pipelined
single-packet requests.

After #2 is done ssl-bump should be able to become something like just
another BodyPipe component in the chain handling the CONNECT request.

Amos
Received on Sat Sep 08 2012 - 04:38:22 MDT