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
