From 88a94ed13ac4574d762cbf1486a701cde2ba1f9b Mon Sep 17 00:00:00 2001
From: Pierre Ossman <ossman@cendio.se>
Date: Mon, 1 Apr 2019 14:22:01 +0200
Subject: [PATCH] Add delay on authentication failures

This provides some basic rate limiting that will make it difficult
for an attacker to brute force passwords. Only relevant when the
blacklist is disabled as otherwise the attacker only gets a very
limited number of attempts.
---
 common/rfb/SConnection.cxx      | 20 +++++++++++++++-----
 common/rfb/SConnection.h        |  5 +++++
 common/rfb/VNCSConnectionST.cxx | 22 +++++++++++++++++++++-
 common/rfb/VNCSConnectionST.h   |  4 ++++
 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx
index 690653a2..4e224aa1 100644
--- a/common/rfb/SConnection.cxx
+++ b/common/rfb/SConnection.cxx
@@ -236,11 +236,8 @@ void SConnection::processSecurityMsg()
     }
   } catch (AuthFailureException& e) {
     vlog.error("AuthFailureException: %s", e.str());
-    os->writeU32(secResultFailed);
-    if (!client.beforeVersion(3,8)) // 3.8 onwards have failure message
-      os->writeString(e.str());
-    os->flush();
-    throw;
+    state_ = RFBSTATE_SECURITY_FAILURE;
+    authFailure(e.str());
   }
 }
 
@@ -315,6 +312,19 @@ void SConnection::authSuccess()
 {
 }
 
+void SConnection::authFailure(const char* reason)
+{
+  if (state_ != RFBSTATE_SECURITY_FAILURE)
+    throw Exception("SConnection::authFailure: invalid state");
+
+  os->writeU32(secResultFailed);
+  if (!client.beforeVersion(3,8)) // 3.8 onwards have failure message
+    os->writeString(reason);
+  os->flush();
+
+  throw AuthFailureException(reason);
+}
+
 void SConnection::queryConnection(const char* userName)
 {
   approveConnection(true);
diff --git a/common/rfb/SConnection.h b/common/rfb/SConnection.h
index 26302403..31d1cb2e 100644
--- a/common/rfb/SConnection.h
+++ b/common/rfb/SConnection.h
@@ -92,6 +92,10 @@ namespace rfb {
     // authSuccess() is called when authentication has succeeded.
     virtual void authSuccess();
 
+    // authFailure() is called when authentication has failed. The default
+    // implementation will inform the client and throw a AuthFailureException.
+    virtual void authFailure(const char* reason);
+
     // queryConnection() is called when authentication has succeeded, but
     // before informing the client.  It can be overridden to query a local user
     // to accept the incoming connection, for example.  The userName argument
@@ -160,6 +164,7 @@ namespace rfb {
       RFBSTATE_PROTOCOL_VERSION,
       RFBSTATE_SECURITY_TYPE,
       RFBSTATE_SECURITY,
+      RFBSTATE_SECURITY_FAILURE,
       RFBSTATE_QUERYING,
       RFBSTATE_INITIALISATION,
       RFBSTATE_NORMAL,
diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index ea5c52aa..fe00dab6 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -52,7 +52,8 @@ VNCSConnectionST::VNCSConnectionST(VNCServerST* server_, network::Socket *s,
     losslessTimer(this), server(server_),
     updateRenderedCursor(false), removeRenderedCursor(false),
     continuousUpdates(false), encodeManager(this), idleTimer(this),
-    pointerEventTime(0), clientHasCursor(false)
+    pointerEventTime(0), clientHasCursor(false),
+    authFailureTimer(this)
 {
   setStreams(&sock->inStream(), &sock->outStream());
   peerEndpoint.buf = sock->getPeerEndpoint();
@@ -151,6 +152,15 @@ void VNCSConnectionST::processMessages()
     sock->cork(true);
 
     while (getInStream()->checkNoWait(1)) {
+      // Silently drop any data if we are currently delaying an
+      // authentication failure response as otherwise we would close
+      // the connection on unexpected data, and an attacker could use
+      // that to detect our delayed state.
+      if (state() == RFBSTATE_SECURITY_FAILURE) {
+        getInStream()->skip(1);
+        continue;
+      }
+
       if (pendingSyncFence) {
         syncFence = true;
         pendingSyncFence = false;
@@ -407,6 +417,14 @@ void VNCSConnectionST::authSuccess()
   updates.add_changed(server->getPixelBuffer()->getRect());
 }
 
+void VNCSConnectionST::authFailure(const char* reason)
+{
+  // Introduce a slight delay of the authentication failure response
+  // to make it difficult to brute force a password
+  authFailureMsg.replaceBuf(strDup(reason));
+  authFailureTimer.start(100);
+}
+
 void VNCSConnectionST::queryConnection(const char* userName)
 {
   server->queryConnection(this, userName);
@@ -745,6 +763,8 @@ bool VNCSConnectionST::handleTimeout(Timer* t)
     if ((t == &congestionTimer) ||
         (t == &losslessTimer))
       writeFramebufferUpdate();
+    else if (t == &authFailureTimer)
+      SConnection::authFailure(authFailureMsg.buf);
   } catch (rdr::Exception& e) {
     close(e.str());
   }
diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h
index 662d9f34..a9a8d3a4 100644
--- a/common/rfb/VNCSConnectionST.h
+++ b/common/rfb/VNCSConnectionST.h
@@ -109,6 +109,7 @@ namespace rfb {
     // These methods are invoked as callbacks from processMsg()
 
     virtual void authSuccess();
+    virtual void authFailure(const char* reason);
     virtual void queryConnection(const char* userName);
     virtual void clientInit(bool shared);
     virtual void setPixelFormat(const PixelFormat& pf);
@@ -184,6 +185,9 @@ namespace rfb {
     Point pointerEventPos;
     bool clientHasCursor;
 
+    Timer authFailureTimer;
+    CharArray authFailureMsg;
+
     CharArray closeReason;
   };
 }
-- 
2.39.5