On 27/07/18 16:18, Alex Rousskov wrote: > On 07/26/2018 09:15 PM, Amos Jeffries wrote: >> 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. > > Agreed. > > >> 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. > > At TLS level, yes. However, one could argue that Squid should honor a > (higher level) client and server assumption that they are talking to > each other (at HTTP+ level). One could, but that is dependent on what the admin policy is. Configuring persistence on/off is the way to control that, not pinning. > We will probably break fewer transactions > that way. With that idea in mind, the "first point" becomes establishing > a TCP tunnel with the server, even if no client Hello pieces are forwarded. > HTTP(S) being stateless, at that level any endpoint relying on implicit state is non-compliant and buggy. HTTP(S) agents are explicitly required to include any necessary state references in each message generated. It is *nice* not to result in visible errors, but not a requirement we should stick to at cost of proper behaviour. HTTP and extensions also specify required error handling in most cases where it matters. > >> 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. > > True for TLS level. Not necessarily true for higher levels as discussed > above. > The client accepted proof that Squid *was* that origin (false or not) order to reach said higher levels. Pinning at a later time due to higher level stateful situation is not relevant to the bumping code actions and not something we need to consider at the present C2 use of S after C1 should have pinned it. The only cross-level requirement is to provide the surety that pinning is a one-way action. Any level setting it makes the connections fate shared. > >>>> 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 > >>> 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. > > Your earlier "Sending the client handshake to a server" and "Bumping at > step1" statements contradict this statement AFAICT because both C1 and > C2 were bumped at step1 in "Case 1". > "after C1 has pinned" - if no pinning happens at all the whole statement is irrelevant. Sorry for confusion. > FWIW, I would default to honor the tunnel until somebody presents a > convincing argument for making the behavior configurable. That is, like > you said, treat C2 reusing S (or, more precisely, not closing S when C1 > is closed) as a Squid bug. > What I'm most confused about here is why S is closed when C1 dies with non-pinning. _unless_ there is pinning between them they should not be that closely fate sharing. Is the HTTPS message on both C1 and S saying "Connection: close" perhapse? If so the closure relationship is an illusion. But then, why would C2 be using a connection currently in-use by C1 in a way that starts *after* C1 closure? (so not collapsed forwarding AFAIK) Something smells fishy. > >> * 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. > > I agree that pinned connections should not be pooled. Bugs > notwithstanding, they are not pooled today IIRC. > > I am not sure you meant that, but I doubt a hypothetical feature of > pooling before-sending-handshake/before-pinning connections is a good > idea, but it does not contradict "pinned connections are not pooled" > principle. In practice, once the TCP connection is established, the > requester pins it immediately (if needed) so there is no opportunity or > need for pooling. > Off-topic; TLS has connections that do not use handshakes at all, which are becoming more common in TLS/1.3. So I believe such a feature may be coming one day, but irrelevant right now. > >> * 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. > > Agreed. > > >> 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. > > The open question is whether S should be pinned in the case where C1 is > bumped at step1 (i.e., "Case 1" in Vishali's email). If S should be > pinned, then I agree with the above statement. If S should not be > pinned, then S is just a regular Squid-to-server connection that can be > pooled. > IMO in *that* specific case S should not be pinned. Because pinning would prevent the very reasonable actions of handling server failures by opening new Sn connections to finish incomplete responses to the client, or retaining a server connection for the quick_abort features. Squid should instead be ensuring that the server handshake at TLS level matches what it would have used for C2 clean handshake. I may have missed it, but that is not yet being done (just TCP equivalence, not TLS). Amos _______________________________________________ squid-users mailing list squid-users@xxxxxxxxxxxxxxxxxxxxx http://lists.squid-cache.org/listinfo/squid-users
