QPID-4021: Badly behaved clients can still clog up the broker

Ameliorate the problem by only turning timeout off after receiving 3 frames
from the sender. This avoids an unauthenticed client causing a DoS by just
hanging before completing authentication in most cases.
This is far from a good fix, but should mostly avoid the issue until it can
be fixed in a neat way.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1355142 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 12 insertions, 2 deletions

diff --git a/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp b/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp
index 8e83c9b32d..7231fa9dfd 100644
--- a/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp
+++ b/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp
@@ -65,12 +65,15 @@ AsynchIOHandler::AsynchIOHandler(const std::string& id, ConnectionCodec::Factory
     aio(0),
     factory(f),
     codec(0),
+    reads(0),
     readError(false),
     isClient(false),
     readCredit(InfiniteCredit)
 {}
 
 AsynchIOHandler::~AsynchIOHandler() {
+    if (timeoutTimerTask)
+        timeoutTimerTask->cancel();
     if (codec)
         codec->closed();
     if (timeoutTimerTask)
@@ -154,10 +157,18 @@ void AsynchIOHandler::readbuff(AsynchIO& , AsynchIO::BufferBase* buff) {
         }
     }
 
+    ++reads;
     size_t decoded = 0;
     if (codec) {                // Already initiated
         try {
             decoded = codec->decode(buff->bytes+buff->dataStart, buff->dataCount);
+            // When we've decoded 3 reads (probably frames) we will have authenticated and
+            // started heartbeats, if specified, in many (but not all) cases so now we will cancel
+            // the idle connection timeout - this is really hacky, and would be better implemented
+            // in the connection, but that isn't actually created until the first decode.
+            if (reads == 3) {
+                timeoutTimerTask->cancel();
+            }
         }catch(const std::exception& e){
             QPID_LOG(error, e.what());
             readError = true;
@@ -168,8 +179,6 @@ void AsynchIOHandler::readbuff(AsynchIO& , AsynchIO::BufferBase* buff) {
         framing::ProtocolInitiation protocolInit;
         if (protocolInit.decode(in)) {
             decoded = in.getPosition();
-            // We've just got the protocol negotiation so we can cancel the timeout for that
-            timeoutTimerTask->cancel();
 
             QPID_LOG(debug, "RECV [" << identifier << "]: INIT(" << protocolInit << ")");
             try {
diff --git a/qpid/cpp/src/qpid/sys/AsynchIOHandler.h b/qpid/cpp/src/qpid/sys/AsynchIOHandler.h
index 2ddd5c9a90..307aad5b85 100644
--- a/qpid/cpp/src/qpid/sys/AsynchIOHandler.h
+++ b/qpid/cpp/src/qpid/sys/AsynchIOHandler.h
@@ -48,6 +48,7 @@ class AsynchIOHandler : public OutputControl {
     AsynchIO* aio;
     ConnectionCodec::Factory* factory;
     ConnectionCodec* codec;
+    uint32_t reads;
     bool readError;
     bool isClient;
     AtomicValue<int32_t> readCredit;