[squid-users] server persistent connections and cache

[Amos Jeffries]

On 27/07/18 13:31, Alex Rousskov wrote:
> On 07/26/2018 05:47 PM, Vishali Somaskanthan wrote:
> 
>> By re-use I meant to say that the server-connection S (TCP + SSL) is
>> re-used across 2 client connections (C1 and C2), from the same client
>> one after the other is torn down. I, presume that
>> “/server_persistent_connection on/” allows for such a use-case.
> 
> server_persistent_connection controls whether a single Squid-to-server
> connection can carry more than one request. It does not (or should not)
> control re-pinning.
> 

Also, pinned connections should never be added to the persistent pool
after pinning. Doing so is a bug IMO. See the sequence at the end of
this mail.

> 
>> Is my
>> understanding that Pinning means binding C1 to S and then if C1 is
>> closed, we unpin and then later if C2 is created, we can pin it again to S?
> 
> IIRC, a lot of code assumes that pinning ties C1 and S connection
> lifetimes together. I do not know whether all code assumes that. I do
> not know whether there is consensus that "same lifetimes" is the correct
> approach for all pinned connections.
> 

Correct (Alex). Pinning to us/Squid means the two connections have done
something stateful - which means shared fate is required from that point
onwards to avoid nasties outside Squid screwing with that statefulness.

Today I work to:
* If one dies they both do.
* A pinned server connection must not be usable by any other client
*after* being pinned.
* A connection not pinned may only be used by *one* client at a time.

There may be old code which does not check pinned state when it should
or makes wrong assumptions about a pinned connection usability. Please
fix as and when found.


> 
>> There is some confusion in my understanding this statement – “pinning
>> _all_ SslBump connections is easier than pinning some of them”, because
>> I see different behaviors when I bump at Step 1 (case 1) vs bump at Step
>> 2 (case 2).
> 
> Just because something is easier in retrospect, does not mean it was
> easy or even clear to the developers writing bits and pieces of that
> code. There are a lot of inconsistencies (and bugs) that we are slowly
> weeding out.
> 

Sending the client handshake to a server makes the TLS end-to-end
stateful between the client<->server. So *that* is the first point at
which pinning is required by Squid.

Peeking at step1 alone does not add statefulness to the client
connection being peeked.

Peeking at step2 requires client data sent to the server. So pinning is
required.

Bumping at step1 does not involve any server information. So no pinning
required. The client is talking to *Squid* over TLS independent of how
the response is fetched.

Bumping at step2 or step3 the client is talking to the specific server
over TLS.

Splice at any time requires tunneling data both ways. So pinning is
required in all cases.

Stare AFAIK does send *some* client state (filtered) to the server so
pinning is probably always required here - but may not be depending on
what the filtering was.


> 
>> Case 1: We see that no pinning happens i.e. pinConnection() is not
>> called at all. C1->S gets established, C1 is closed and then C2 re-uses S
>>
>> Case 2: We see that pinning happens i.e. httpsPeeked() calls
>> pinConnection(). Here, C1->S gets established. Closing C1 from the
>> client brings down S. Later, opening C2 opens a new server-connection S.
>>
>> Is this the expected behavior?
> 
> The code just happens to work this way (evidently). It is not something
> I would rely on until the matter is discussed and settled.
> 


I think the described behaviour of C2 using S after C1 has pinned the
connection is a bug.

* S should not be pooled as persistent until the client which triggered
its TCP open has finished with it (eg after the TLS handshake completes).

* Once the handshake begins sending client data, whichever client was
opening it should pin it.

* The sequence of the above two should result in S being pinned before
it is ever considered for the persistence pool. Which should cause it to
be excluded from the servers pool.

So IMO when these things are working properly, C2 server selection
should never even see that S exists let alone be able to use it.

Amos