From: Pierre Ossman <ossman@cendio.se>
Date: Mon, 8 Oct 2018 13:59:02 +0000 (+0200)
Subject: Move access rights and closing to SConnection object
X-Git-Tag: v1.9.90~66^2~9
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7d64b33b5b96a20216a2e1425ff412e027bbebcd;p=tigervnc.git

Move access rights and closing to SConnection object

This makes it generally useful and other code doesn't have to
know of the specific sub-class.
---

diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx
index efc26acf..6b4a5c4f 100644
--- a/common/rfb/SConnection.cxx
+++ b/common/rfb/SConnection.cxx
@@ -269,6 +269,16 @@ void SConnection::writeConnFailedFromScratch(const char* msg,
   os->flush();
 }
 
+void SConnection::setAccessRights(AccessRights ar)
+{
+  accessRights = ar;
+}
+
+bool SConnection::accessCheck(AccessRights ar) const
+{
+  return (accessRights & ar) == ar;
+}
+
 void SConnection::setEncodings(int nEncodings, const rdr::S32* encodings)
 {
   int i;
@@ -342,6 +352,11 @@ void SConnection::clientInit(bool shared)
   state_ = RFBSTATE_NORMAL;
 }
 
+void SConnection::close(const char* reason)
+{
+  state_ = RFBSTATE_CLOSING;
+}
+
 void SConnection::setPixelFormat(const PixelFormat& pf)
 {
   SMsgHandler::setPixelFormat(pf);
diff --git a/common/rfb/SConnection.h b/common/rfb/SConnection.h
index 47092e35..7148294b 100644
--- a/common/rfb/SConnection.h
+++ b/common/rfb/SConnection.h
@@ -69,6 +69,13 @@ namespace rfb {
     void approveConnection(bool accept, const char* reason=0);
 
 
+    // Methods to terminate the connection
+
+    // close() shuts down the connection to the client and awaits
+    // cleanup of the SConnection object by the server
+    virtual void close(const char* reason);
+
+
     // Overridden from SMsgHandler
 
     virtual void setEncodings(int nEncodings, const rdr::S32* encodings);
@@ -118,8 +125,10 @@ namespace rfb {
     virtual void enableContinuousUpdates(bool enable,
                                          int x, int y, int w, int h);
 
+    // Other methods
+
     // setAccessRights() allows a security package to limit the access rights
-    // of a VNCSConnectionST to the server.  How the access rights are treated
+    // of a SConnection to the server.  How the access rights are treated
     // is up to the derived class.
 
     typedef rdr::U16 AccessRights;
@@ -132,9 +141,8 @@ namespace rfb {
     static const AccessRights AccessDefault;        // The default rights, INCLUDING FUTURE ONES
     static const AccessRights AccessNoQuery;        // Connect without local user accepting
     static const AccessRights AccessFull;           // All of the available AND FUTURE rights
-    virtual void setAccessRights(AccessRights ar) = 0;
-
-    // Other methods
+    virtual void setAccessRights(AccessRights ar);
+    virtual bool accessCheck(AccessRights ar) const;
 
     // authenticated() returns true if the client has authenticated
     // successfully.
@@ -201,6 +209,7 @@ namespace rfb {
     SSecurity* ssecurity;
     stateEnum state_;
     rdr::S32 preferredEncoding;
+    AccessRights accessRights;
   };
 }
 #endif
diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index b5f81c61..d9e276a9 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -52,8 +52,7 @@ VNCSConnectionST::VNCSConnectionST(VNCServerST* server_, network::Socket *s,
     losslessTimer(this), server(server_), updates(false),
     updateRenderedCursor(false), removeRenderedCursor(false),
     continuousUpdates(false), encodeManager(this), pointerEventTime(0),
-    clientHasCursor(false),
-    accessRights(AccessDefault), startTime(time(0))
+    clientHasCursor(false), startTime(time(0))
 {
   setStreams(&sock->inStream(), &sock->outStream());
   peerEndpoint.buf = sock->getPeerEndpoint();
@@ -94,17 +93,16 @@ VNCSConnectionST::~VNCSConnectionST()
 }
 
 
-// Methods called from VNCServerST
+// SConnection methods
 
-bool VNCSConnectionST::init()
+bool VNCSConnectionST::accessCheck(AccessRights ar) const
 {
-  try {
-    initialiseProtocol();
-  } catch (rdr::Exception& e) {
-    close(e.str());
-    return false;
-  }
-  return true;
+  // Reverse connections are user initiated, so they are implicitly
+  // allowed to bypass the query
+  if (reverseConnection)
+    ar &= ~AccessNoQuery;
+
+  return SConnection::accessCheck(ar);
 }
 
 void VNCSConnectionST::close(const char* reason)
@@ -123,7 +121,22 @@ void VNCSConnectionST::close(const char* reason)
   // calling code will call VNCServerST's removeSocket() method causing us to
   // be deleted.
   sock->shutdown();
-  setState(RFBSTATE_CLOSING);
+
+  SConnection::close(reason);
+}
+
+
+// Methods called from VNCServerST
+
+bool VNCSConnectionST::init()
+{
+  try {
+    initialiseProtocol();
+  } catch (rdr::Exception& e) {
+    close(e.str());
+    return false;
+  }
+  return true;
 }
 
 
@@ -267,7 +280,7 @@ void VNCSConnectionST::bellOrClose()
 void VNCSConnectionST::serverCutTextOrClose(const char *str, int len)
 {
   try {
-    if (!(accessRights & AccessCutText)) return;
+    if (!accessCheck(AccessCutText)) return;
     if (!rfb::Server::sendCutText) return;
     if (state() == RFBSTATE_NORMAL)
       writer()->writeServerCutText(str, len);
@@ -447,9 +460,8 @@ void VNCSConnectionST::queryConnection(const char* userName)
   }
 
   // - Does the client have the right to bypass the query?
-  if (reverseConnection ||
-      !(rfb::Server::queryConnect || sock->requiresQuery()) ||
-      (accessRights & AccessNoQuery))
+  if (!(rfb::Server::queryConnect || sock->requiresQuery()) ||
+      accessCheck(AccessNoQuery))
   {
     approveConnection(true);
     return;
@@ -463,10 +475,10 @@ void VNCSConnectionST::clientInit(bool shared)
 {
   lastEventTime = time(0);
   if (rfb::Server::alwaysShared || reverseConnection) shared = true;
-  if (!(accessRights & AccessNonShared)) shared = true;
+  if (!accessCheck(AccessNonShared)) shared = true;
   if (rfb::Server::neverShared) shared = false;
   if (!shared) {
-    if (rfb::Server::disconnectClients && (accessRights & AccessNonShared)) {
+    if (rfb::Server::disconnectClients && accessCheck(AccessNonShared)) {
       // - Close all the other connected clients
       vlog.debug("non-shared connection - closing clients");
       server->closeClients("Non-shared connection requested", getSock());
@@ -494,7 +506,7 @@ void VNCSConnectionST::setPixelFormat(const PixelFormat& pf)
 void VNCSConnectionST::pointerEvent(const Point& pos, int buttonMask)
 {
   pointerEventTime = lastEventTime = time(0);
-  if (!(accessRights & AccessPtrEvents)) return;
+  if (!accessCheck(AccessPtrEvents)) return;
   if (!rfb::Server::acceptPointerEvents) return;
   pointerEventPos = pos;
   server->pointerEvent(this, pointerEventPos, buttonMask);
@@ -526,7 +538,7 @@ void VNCSConnectionST::keyEvent(rdr::U32 keysym, rdr::U32 keycode, bool down) {
   rdr::U32 lookup;
 
   lastEventTime = time(0);
-  if (!(accessRights & AccessKeyEvents)) return;
+  if (!accessCheck(AccessKeyEvents)) return;
   if (!rfb::Server::acceptKeyEvents) return;
 
   if (down)
@@ -636,7 +648,7 @@ void VNCSConnectionST::keyEvent(rdr::U32 keysym, rdr::U32 keycode, bool down) {
 
 void VNCSConnectionST::clientCutText(const char* str, int len)
 {
-  if (!(accessRights & AccessCutText)) return;
+  if (!accessCheck(AccessCutText)) return;
   if (!rfb::Server::acceptCutText) return;
   server->clientCutText(str, len);
 }
@@ -645,7 +657,7 @@ void VNCSConnectionST::framebufferUpdateRequest(const Rect& r,bool incremental)
 {
   Rect safeRect;
 
-  if (!(accessRights & AccessView)) return;
+  if (!accessCheck(AccessView)) return;
 
   SConnection::framebufferUpdateRequest(r, incremental);
 
@@ -684,7 +696,7 @@ void VNCSConnectionST::setDesktopSize(int fb_width, int fb_height,
 {
   unsigned int result;
 
-  if (!(accessRights & AccessSetDesktopSize)) return;
+  if (!accessCheck(AccessSetDesktopSize)) return;
   if (!rfb::Server::acceptSetDesktopSize) return;
 
   result = server->setDesktopSize(this, fb_width, fb_height, layout);
@@ -1158,27 +1170,34 @@ char* VNCSConnectionST::getStartTime()
 
 void VNCSConnectionST::setStatus(int status)
 {
+  AccessRights ar;
+
+  ar = AccessDefault;
+
   switch (status) {
   case 0:
-    accessRights = accessRights | AccessPtrEvents | AccessKeyEvents | AccessView;
+    ar |= AccessPtrEvents | AccessKeyEvents | AccessView;
     break;
   case 1:
-    accessRights = (accessRights & ~(AccessPtrEvents | AccessKeyEvents)) | AccessView;
+    ar |= rfb::SConnection::AccessView;
+    ar &= ~(AccessPtrEvents | AccessKeyEvents);
     break;
   case 2:
-    accessRights = accessRights & ~(AccessPtrEvents | AccessKeyEvents | AccessView);
+    ar &= ~(AccessPtrEvents | AccessKeyEvents | AccessView);
     break;
   }
+
+  setAccessRights(ar);
+
   framebufferUpdateRequest(server->getPixelBuffer()->getRect(), false);
 }
 int VNCSConnectionST::getStatus()
 {
-  if ((accessRights & (AccessPtrEvents | AccessKeyEvents | AccessView)) == 0x0007)
+  if (accessCheck(AccessPtrEvents | AccessKeyEvents | AccessView))
     return 0;
-  if ((accessRights & (AccessPtrEvents | AccessKeyEvents | AccessView)) == 0x0001)
+  else if (accessCheck(AccessView))
     return 1;
-  if ((accessRights & (AccessPtrEvents | AccessKeyEvents | AccessView)) == 0x0000)
+  else
     return 2;
-  return 4;
 }
 
diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h
index faa04793..121ebcc1 100644
--- a/common/rfb/VNCSConnectionST.h
+++ b/common/rfb/VNCSConnectionST.h
@@ -43,6 +43,11 @@ namespace rfb {
     VNCSConnectionST(VNCServerST* server_, network::Socket* s, bool reverse);
     virtual ~VNCSConnectionST();
 
+    // SConnection methods
+
+    virtual bool accessCheck(AccessRights ar) const;
+    virtual void close(const char* reason);
+
     // Methods called from VNCServerST.  None of these methods ever knowingly
     // throw an exception.
 
@@ -54,10 +59,6 @@ namespace rfb {
     // returns false, and close() will have been called.
     bool init();
 
-    // close() shuts down the socket to the client and deletes the
-    // SConnectionST object.
-    void close(const char* reason);
-
     // processMessages() processes incoming messages from the client, invoking
     // various callbacks as a result.  It continues to process messages until
     // reading might block.  shutdown() will be called on the connection's
@@ -141,12 +142,6 @@ namespace rfb {
     virtual void supportsContinuousUpdates();
     virtual void supportsLEDState();
 
-    // setAccessRights() allows a security package to limit the access rights
-    // of a VNCSConnectioST to the server.  These access rights are applied
-    // such that the actual rights granted are the minimum of the server's
-    // default access settings and the connection's access settings.
-    virtual void setAccessRights(AccessRights ar) {accessRights=ar;}
-
     // Timer callbacks
     virtual bool handleTimeout(Timer* t);
 
@@ -202,8 +197,6 @@ namespace rfb {
     Point pointerEventPos;
     bool clientHasCursor;
 
-    AccessRights accessRights;
-
     CharArray closeReason;
     time_t startTime;
   };
diff --git a/common/rfb/VNCServer.h b/common/rfb/VNCServer.h
index 2987ec0e..5bb7c0bf 100644
--- a/common/rfb/VNCServer.h
+++ b/common/rfb/VNCServer.h
@@ -73,6 +73,10 @@ namespace rfb {
     //   their close() method with the supplied reason.
     virtual void closeClients(const char* reason) = 0;
 
+    // getConnection() gets the SConnection for a particular Socket.  If
+    // the Socket is not recognised then null is returned.
+    virtual SConnection* getConnection(network::Socket* sock) = 0;
+
     // setCursor() tells the server that the cursor has changed.  The
     // cursorData argument contains width*height rgba quadruplets with
     // non-premultiplied alpha.
diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx
index 31ac5d1c..5e166119 100644
--- a/common/rfb/VNCServerST.cxx
+++ b/common/rfb/VNCServerST.cxx
@@ -583,7 +583,7 @@ void VNCServerST::getSockets(std::list<network::Socket*>* sockets)
   }
 }
 
-SConnection* VNCServerST::getSConnection(network::Socket* sock) {
+SConnection* VNCServerST::getConnection(network::Socket* sock) {
   std::list<VNCSConnectionST*>::iterator ci;
   for (ci = clients.begin(); ci != clients.end(); ci++) {
     if ((*ci)->getSock() == sock)
diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h
index 54443e1f..0fa2c875 100644
--- a/common/rfb/VNCServerST.h
+++ b/common/rfb/VNCServerST.h
@@ -99,6 +99,7 @@ namespace rfb {
     virtual void approveConnection(network::Socket* sock, bool accept,
                                    const char* reason);
     virtual void closeClients(const char* reason) {closeClients(reason, 0);}
+    virtual SConnection* getConnection(network::Socket* sock);
 
     virtual void add_changed(const Region &region);
     virtual void add_copied(const Region &dest, const Point &delta);
@@ -133,11 +134,6 @@ namespace rfb {
     // any), and logs the specified reason for closure.
     void closeClients(const char* reason, network::Socket* sock);
 
-    // getSConnection() gets the SConnection for a particular Socket.  If
-    // the Socket is not recognised then null is returned.
-
-    SConnection* getSConnection(network::Socket* sock);
-
     // queryConnection() is called when a connection has been
     // successfully authenticated.  The sock and userName arguments identify
     // the socket and the name of the authenticated user, if any.