[squid-dev] [PATCH] Do not blindly forward cache peer CONNECT responses

[Amos Jeffries]

On 29/06/2015 12:12 a.m., Amos Jeffries wrote:
> On 24/06/2015 3:26 a.m., Alex Rousskov wrote:
>> On 06/19/2015 06:46 PM, Amos Jeffries wrote:
>>> On 20/06/2015 4:54 a.m., Alex Rousskov wrote:
>>>> Hello,
>>>>
>>>>     The attached trunk patch fixes a rare but nasty problem by removing
>>>> a very old hack which shielded Squid from parsing most CONNECT responses.
>>>>
>>>> Currently, Squid blindly forwards cache peer CONNECT responses to
>>>> clients when possible. This may break things if the peer responds with
>>>> something like HTTP 403 (Forbidden) and keeps the connection with Squid
>>>> open:
>>>>
>>>>   -  The client application issues a CONNECT request.
>>>>   -  Squid forwards this request to a cache peer.
>>>>   -  Cache peer correctly responds back with a "403 Forbidden".
>>>>   -  Squid does not parse cache peer response and
>>>>      just forwards it as if it was a Squid response to the client.
>>>>   -  The TCP connections are not closed.
>>>>
>>>> At this stage, Squid is unaware that the CONNECT request has failed. All
>>>> subsequent requests on the user agent TCP connection are treated as
>>>> tunnelled traffic. Squid is forwarding these requests to the peer on the
>>>> TCP connection previously used for the 403-ed CONNECT request, without
>>>> proper processing. The additional headers which should have been applied
>>>> by Squid to these requests are not applied, and the requests are being
>>>> forwarded to the cache peer even though the Squid configuration may
>>>> state that these requests must go directly to the origin server.
>>>>
>>>> This patch fixes Squid to parse cache peer responses, and if an error
>>>> response found, respond with "502 Bad Gateway" to the client and close
>>>> the connections.
>>>>
>>>
>>> Thank you. I've had this on my todo list for a long time.
>>>
>>> One problem though...
>>>
>>> The parse operations documented as being copied from http.cc
>>> processReplyHeader() do not match what trunk currently does for HTTP
>>> replies in that function.
>>
>>
>> How is that problem related to the proposed fix?
> 
> Ah. Sorry, You are right.
> 
> +1 from me. Please apply.

On second thoughts that traffic behaviour after errors is extremely
nasty. Applying immediately so I can backport into 3.5.6 without
incurring extra delays.

Amos