Re: [PATCH] Truncate HTTP response bodies to match clen

Alex Rousskov wrote:
> On 06/27/2009 06:32 AM, Amos Jeffries wrote:
>> Alex Rousskov wrote:
>>> Truncate too-long HTTP response bodies to match their Content-Length
>>> header.
>>>
>>> Sometimes a broken server sends more than Content-Length bytes in the
>>> response. For example, a 302 redirect message with "Content-Length: 0"
>>> header may include an HTML body. Squid used to send "everything" it read
>>> to the client, even if it read more than the Content-Length bytes. That
>>> may have helped in some cases, but I think we should be more
>>> conservative when dealing with broken servers to combat message
>>> smuggling attacks and other bad side-effects for clients.
>>>
>>> We now do not forward more than the advertised content length and
>>> declare the connection with a broken server non-persistent.
>>>
>>> Chunked responses (that Squid should not receive and that must not have
>>> a Content-Length header) are not truncated because RFC 2616 says we MUST
>>> ignore their Content-Length header.
>>>
>>> TODO: simply truncating read content would not work for pipelined
>>> responses. We should preserve extra content for the next transaction on
>>> a pconn.
>>>
>>> ---------------
>>>
>>> The attached patch is against Squid 3.0 and has been tested in
>>> production. More testing is welcomed. I will port to trunk if needed if
>>> the change is accepted.
>>>
>>> Thank you,
>>>
>>> Alex.
>>>
>> Trouble...
>>
>> I was of the understanding that Squid already was doing these truncation
>> + non-pconn flagging :( I've written off many bugs in the last few
>> years to this behavior...
>>
>>
>> Is this some side case you found internal to Squid? or are the current
>> Squid-3 actually not protecting people from message-appending?
>
> The problem was triggered at the production site running Squid 3p0-plus
> that was accelerating a broken server application. The application was
> including bodies in 302 redirects that had Content-Length set to zero.
> That, in turn, broke some clients. Neither the application nor the
> clients could be fixed, of course.
>
> Besides, it is probably an HTTP violation to forward trailing garbage
> anyway, unless we switch to some kind of "tunneling mode" and become a
> TCP proxy, but then you get problems with message smuggling and such.
>
> Do you think this change should go in?
>

I do. I'm not the best for RFC compliance issues, but its a stuffing
attack. And we should be protecting from those.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE6 or 3.0.STABLE16
   Current Beta Squid 3.1.0.9