On 22/01/2013 3:04 a.m., Steve Hill wrote:
On 18.01.13 18:37, Alex Rousskov wrote:
Further to this...
* Use expectNoForwarding() instead of adding forwardToBitbucket().
* In ConnStateData::noteBodyConsumerAborted() do not stop reading if
flags.readMore is set. Just keep auto-consuming. This may prevent
authenticating connections closures even though expectNoForwarding() is
used. Needs to be tested: Will we correctly stop and switch to the new
request when the body finally ends?
I have changed to using expectNoForwarding() and removed the
stopReceiving() call from noteBodyConsumerAborted() and this appears
to work correctly:
1. Making a request which results in an error, with "Connection:
close" produces a response to the client, and then once the request
body has been fully received, Squid closes the connection.
Does this work with a 70,000 clients all doing 100 byte request bodies
sent 1 byte per minute (header sent normally)?
The DoS attack PoC expectation is that the proxy becomes unresponsive
for about an hour while the whole TCP available socket range has
ESTALISHED connections.
The normal Squid operation expects that no TCP sockets held in
ESTABLISHED state past the first second, and resumes full availability
within 15 minutes. Although some requests may fail if the sockets stay
in TIME_WAIT for long. If possible the TIME_WAIT is avoided and full
availability is never lost.
2. Making a request which results in an error, with "Connection:
keep-alive" produces a response to the client, and then once the
request body has been fully received Squid reads the next request.
3. Making a request which does *NOT* result in an error, with
"Connection: keep-alive" works as expected and then waits for the next
request.
4. Making a request which does *NOT* result in an error, with
"Connection: close" works as expected and then closes the connection.
5. Making a request which results in an ICAP error (I shut down a
non-bypassable ICAP service), with "Connection: keep-alive" produces a
500 error response and then waits for the next request.
6. Making a request which results in an ICAP error, with "Connection:
close" produces a 500 error response and then Squid closes the
connection.
I don't really understand the purpose of the readMore flag, can you
explain it?
It's a hack due to some internals of Squid needing direct read-access to
the client socket to perform their actions. SSL handshake and CONNECT
tunnel body handling being the big remaining ones these days. It signals
the ConnStateData object which is normally responsible for the I/O read
handler that it is *not* to process any incoming data on the socket
until the flags is set back to true.
It is/was very likely also being abused to signal that the error cases
have occured and the input is to be dropped by TCP shutdown.
* Discuss and possibly remove readNextRequest setting from
ClientHttpRequest::handleAdaptationFailure(), to remove inconsistency
discussed in #2 above.
As mentioned above, I don't understand what readMore is actually for.
However, as I mentioned in my previous email, the only time I can see
us wanting to blindly close the connection for adaptation failures is
when we've already started sending a response to the client and the
adaptation service blows up in the middle of the response body. It
doesn't _look_ like handleAdaptationFailure() handles that event
though, unless I'm mistaken?
* Discuss whether we are OK with clients tying up Squid after an
[authentication] error. Perhaps we need another timeout there, perhaps
there are already mechanisms to address that, or perhaps we do not
really care.
There are 4 possible ways I can see of a client tieing up Squid:
1. The client connects but doesn't send anything.
2. The client connects, makes a keep-alive request that results in an
error, then doesn't send any new request.
3. The client makes a request that generates an error, but doesn't
send the complete request body.
4. The client makes a request that generates an error, uses chunked
encoding and sends an infinitely long body.
(1) and (2) are basically the same thing. I've tested this and Squid
times out the connections and closes them after a few minutes.
(3) similarly gets timed out after a while.
I haven't tested (4), but I have no reason to believe there's anything
that currently stops it from happening. I'm not sure whether or not
we should do something to prevent it - that's one for discussion.
If the data is being bropped on arrival by Squid we should be watching
for these cases and aborting client connections which go on too long. I
suspect the read_timout timer should be kept ticking on these reads and
not reset to 0. That would cap the abuse time at 15 minutes per client
rather than infinite.
Amos
