Re: [RFC] CONNECT and use_ssl cache_peer

On 9/09/2012 3:54 a.m., Alex Rousskov wrote:
> On 09/07/2012 10:38 PM, Amos Jeffries wrote:
>> 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.
> Hi Amos,
>
> Can you clarify how tunneling is related to chunking, keep-alive and
> pipelined single-packet requests? It seems to me that all those HTTP
> features are not compatible with CONNECT tunnels because once the tunnel
> is established, the connection stops having any HTTP semantics. Are you
> thinking about error responses to CONNECT requests? A successful CONNECT
> response cannot be chunked (no body), kept alive (no way to determine
> the end of the tunnel other than by closing the connection), or handle
> more requests (same reason).
>
> As for 1xx messages and error responses, I agree that we should handle
> them better in response to Squid's CONNECT request, but we do not handle
> them well now, and option #3 will share any future handling improvements
> with tunnel.cc and forward.cc code.

Henrik and I seem to disagree on this being a good idea being plugged
into BodyPipe. But as I see it BodyPipe is where Upgrade, WebSockets and
SSL-Bump should be happening in a node which internally "receives" the
tunnel connection and acts like a ConnStateData reading out of the tunnel.

When going DIRECT we have to assume the CONNECT is infinite length and
handle as per now with closures.

But ... I see a potential gap in the RFC texts that when communicating
between two proxies we can do one of several methods (Expect-100 or peer
OPTIONS) to negotiate chunking support for any data the CONNECT has
following it. Thus keeping the connection between the peers (only)
re-usable after the CONNECT is finished. So long as the transformation
(chunked encoding) is stripped away at the recipient proxy this does not
introduce problems to the transported end-to-end data stream.

I have one client VERY interested in such otimizations across a
multi-layer Squid cluster.

Amos
Received on Sun Sep 09 2012 - 09:34:19 MDT