Merge branch 'allowoverride' of https://github.com/michalsrb/tigervnc

4 files changed, 69 insertions, 8 deletions

diff --git a/unix/xserver/hw/vnc/Xvnc.man b/unix/xserver/hw/vnc/Xvnc.man
index 4a833157..a4d9f8d3 100644
--- a/unix/xserver/hw/vnc/Xvnc.man
+++ b/unix/xserver/hw/vnc/Xvnc.man
@@ -300,6 +300,21 @@ Key affected by NumLock often require a fake Shift to be inserted in order
 for the correct symbol to be generated. Turning on this option avoids these
 extra fake Shift events but may result in a slightly different symbol
 (e.g. a Return instead of a keypad Enter).
+.
+.TP
+.B \-AllowOverride
+Comma separated list of parameters that can be modified using VNC extension.
+Parameters can be modified for example using \fBvncconfig\fP(1) program from
+inside a running session.
+
+Allowing override of parameters such as \fBPAMService\fP or \fBPasswordFile\fP
+can negatively impact security if Xvnc runs under different user than the
+programs allowed to override the parameters.
+
+When \fBNoClipboard\fP parameter is set, allowing override of \fBSendCutText\fP
+and \fBAcceptCutText\fP has no effect.
+
+Default is \fBdesktop,AcceptPointerEvents,SendCutText,AcceptCutText\fP.
 
 .SH USAGE WITH INETD
 By configuring the \fBinetd\fP(1) service appropriately, Xvnc can be launched
diff --git a/unix/xserver/hw/vnc/vncExt.c b/unix/xserver/hw/vnc/vncExt.c
index 43794dad..b27115f6 100644
--- a/unix/xserver/hw/vnc/vncExt.c
+++ b/unix/xserver/hw/vnc/vncExt.c
@@ -182,17 +182,16 @@ static int ProcVncExtSetParam(ClientPtr client)
   rep.sequenceNumber = client->sequence;
 
   /*
-   * Allow to change only certain parameters.
-   * Changing other parameters (for example PAM service name)
-   * could have negative security impact.
+   * Prevent change of clipboard related parameters if clipboard is disabled.
    */
-  if (strncasecmp(param, "desktop", 7) != 0 &&
-      strncasecmp(param, "AcceptPointerEvents", 19) != 0 &&
-      (vncNoClipboard || strncasecmp(param, "SendCutText", 11) != 0) &&
-      (vncNoClipboard || strncasecmp(param, "AcceptCutText", 13) != 0))
+  if (vncNoClipboard &&
+      (strncasecmp(param, "SendCutText", 11) == 0 ||
+       strncasecmp(param, "AcceptCutText", 13) == 0))
+    goto deny;
+
+  if (!vncOverrideParam(param))
     goto deny;
 
-  vncSetParamSimple(param);
   rep.success = 1;
 
   // Send DesktopName update if desktop name has been changed
diff --git a/unix/xserver/hw/vnc/vncExtInit.cc b/unix/xserver/hw/vnc/vncExtInit.cc
index 863cd36b..1d374938 100644
--- a/unix/xserver/hw/vnc/vncExtInit.cc
+++ b/unix/xserver/hw/vnc/vncExtInit.cc
@@ -20,6 +20,9 @@
 #include <stdio.h>
 #include <errno.h>
 
+#include <set>
+#include <string>
+
 #include <rfb/Configuration.h>
 #include <rfb/Logger_stdio.h>
 #include <rfb/LogWriter.h>
@@ -52,6 +55,15 @@ int vncFbstride[MAXSCREENS];
 
 int vncInetdSock = -1;
 
+struct CaseInsensitiveCompare {
+  bool operator() (const std::string &a, const std::string &b) const {
+    return strcasecmp(a.c_str(), b.c_str()) < 0;
+  }
+};
+
+typedef std::set<std::string, CaseInsensitiveCompare> ParamSet;
+static ParamSet allowOverrideSet;
+
 rfb::StringParameter httpDir("httpd",
                              "Directory containing files to serve via HTTP",
                              "");
@@ -69,6 +81,9 @@ rfb::StringParameter interface("interface",
 rfb::BoolParameter avoidShiftNumLock("AvoidShiftNumLock",
                                      "Avoid fake Shift presses for keys affected by NumLock.",
                                      true);
+rfb::StringParameter allowOverride("AllowOverride",
+                                   "Comma separated list of parameters that can be modified using VNC extension.",
+                                   "desktop,AcceptPointerEvents,SendCutText,AcceptCutText");
 
 static PixelFormat vncGetPixelFormat(int scrIdx)
 {
@@ -99,6 +114,19 @@ static PixelFormat vncGetPixelFormat(int scrIdx)
                      redShift, greenShift, blueShift);
 }
 
+static void parseOverrideList(const char *text, ParamSet &out)
+{
+  for (const char* iter = text; ; ++iter) {
+    if (*iter == ',' || *iter == '\0') {
+      out.insert(std::string(text, iter));
+      text = iter + 1;
+
+      if (*iter == '\0')
+        break;
+    }
+  }
+}
+
 void vncExtensionInit(void)
 {
   int ret;
@@ -128,6 +156,10 @@ void vncExtensionInit(void)
   try {
     if (!initialised) {
       rfb::initStdIOLoggers();
+
+      parseOverrideList(allowOverride, allowOverrideSet);
+      allowOverride.setImmutable();
+
       initialised = true;
     }
 
@@ -379,3 +411,16 @@ void vncRefreshScreenLayout(int scrIdx)
 {
   desktop[scrIdx]->refreshScreenLayout();
 }
+
+int vncOverrideParam(const char *nameAndValue)
+{
+  const char* equalSign = strchr(nameAndValue, '=');
+  if (!equalSign)
+    return 0;
+
+  std::string key(nameAndValue, equalSign);
+  if (allowOverrideSet.find(key) == allowOverrideSet.end())
+    return 0;
+
+  return rfb::Configuration::setParam(nameAndValue);
+}
diff --git a/unix/xserver/hw/vnc/vncExtInit.h b/unix/xserver/hw/vnc/vncExtInit.h
index 6430ac05..be6487c8 100644
--- a/unix/xserver/hw/vnc/vncExtInit.h
+++ b/unix/xserver/hw/vnc/vncExtInit.h
@@ -90,6 +90,8 @@ void vncPreScreenResize(int scrIdx);
 void vncPostScreenResize(int scrIdx, int success, int width, int height);
 void vncRefreshScreenLayout(int scrIdx);
 
+int vncOverrideParam(const char *nameAndValue);
+
 #ifdef __cplusplus
 }
 #endif