Merge pull request #1849 from matt335672/chansrv-on-display-0

Allow DISPLAY=:0 for chansrv (#1849)

common/string_calls.c

@@ -24,6 +24,8 @@
 #include <string.h>
 #include <strings.h>
 #include <stdlib.h>
+#include <ctype.h>
+
 
 #include "log.h"
 #include "os_calls.h"
@@ -140,6 +142,36 @@ g_text2bool(const char *s)
     return 0;
 }
 
+/*****************************************************************************/
+int
+g_get_display_num_from_display(const char *display_text)
+{
+    int rv = -1;
+    const char *p;
+
+    /* Skip over the hostname part of the DISPLAY */
+    if (display_text != NULL && (p = strchr(display_text, ':')) != NULL)
+    {
+        ++p; /* Skip the ':' */
+
+        /* Cater for the (still supported) double-colon. See
+         * https://www.x.org/releases/X11R7.7/doc/libX11/libX11/libX11.html */
+        if (*p == ':')
+        {
+            ++p;
+        }
+
+        /* Check it starts with a digit, to avoid oddities like DISPLAY=":zz.0"
+         * being parsed successfully */
+        if (isdigit(*p))
+        {
+            rv = g_atoi(p);
+        }
+    }
+
+    return rv;
+}
+
 /*****************************************************************************/
 /* returns length of text */
 int
@@ -774,4 +806,3 @@ g_strtrim(char *str, int trim_flags)
     free(text1);
     return 0;
 }
-

common/string_calls.h

@@ -103,6 +103,16 @@ g_text2bool(const char *s);
 char *
 g_bytes_to_hexdump(const char *src, int len);
 
+/**
+ * Extracts the display number from an X11 display string
+ *
+ * @param Display string (i.e. g_getenv("DISPLAY"))
+ *
+ * @result <0 if the string could not be parsed, or >=0 for a display number
+ */
+int
+g_get_display_num_from_display(const char *display_text);
+
 int      g_strlen(const char *text);
 const char *g_strchr(const char *text, int c);
 char    *g_strcpy(char *dest, const char *src);

docs/man/xrdp.ini.5.in

@@ -232,7 +232,7 @@ This option can have one of the following values:
 
 \fBINFO\fR or \fB3\fR \- Logs errors, warnings and informational messages
 
-\fBDEBUG\fR or \fB4\fR \- Log everything. If \fBsesman\fR is compiled in debug mode, this options will output many more low\-level message, useful for developers
+\fBDEBUG\fR or \fB4\fR \- Log everything. If \fBxrdp-sesman\fR is compiled in debug mode, this options will output many more low\-level message, useful for developers
 
 .TP
 \fBEnableSyslog\fR=\fI[true|false]\fR
@@ -338,6 +338,16 @@ values supported for a particular release of \fBxrdp\fR(8) are documented in
 Specifies the session type. The default, \fI0\fR, is Xvnc, \fI10\fR is
 X11rdp, and \fI20\fR is Xorg with xorgxrdp modules.
 
+.TP
+\fBchansrvport\fR=\fBDISPLAY(\fR\fIn\fR\fB)\fR|\fI/path/to/domain-socket\fR
+Asks xrdp to connect to a manually started \fBxrdp-chansrv\fR instance.
+This can be useful if you wish to use to use xrdp to connect to a VNC session
+which has been started other than by \fBxrdp-sesman\fR, as you can then make
+use of \fBxrdp\-chansrv\fR facilities in the VNC session.
+
+The first form of this setting is recommended, replacing \fIn\fR with the
+X11 display number of the session.
+
 .SH "EXAMPLES"
 This is an example \fBxrdp.ini\fR:
 
@@ -369,8 +379,9 @@ password={base64}cGFzc3dvcmQhCg==
 
 .SH "SEE ALSO"
 .BR xrdp (8),
-.BR sesman (8),
-.BR sesrun (8),
+.BR xrdp\-chansrv (8),
+.BR xrdp\-sesman (8),
+.BR xrdp\-sesrun (8),
 .BR sesman.ini (5)
 
 For more info on \fBxrdp\fR see

sesman/chansrv/chansrv.c

@@ -61,7 +61,7 @@ static tbus g_thread_done_event = 0;
 
 struct config_chansrv *g_cfg = NULL;
 
-int g_display_num = 0;
+int g_display_num = -1;
 int g_cliprdr_chan_id = -1; /* cliprdr */
 int g_rdpsnd_chan_id = -1;  /* rdpsnd  */
 int g_rdpdr_chan_id = -1;   /* rdpdr   */
@@ -1557,65 +1557,6 @@ x_server_fatal_handler(void)
     exit(0);
 }
 
-/*****************************************************************************/
-static int
-get_display_num_from_display(char *display_text)
-{
-    int index;
-    int mode;
-    int host_index;
-    int disp_index;
-    int scre_index;
-    char host[256];
-    char disp[256];
-    char scre[256];
-
-    g_memset(host, 0, 256);
-    g_memset(disp, 0, 256);
-    g_memset(scre, 0, 256);
-
-    index = 0;
-    host_index = 0;
-    disp_index = 0;
-    scre_index = 0;
-    mode = 0;
-
-    while (display_text[index] != 0)
-    {
-        if (display_text[index] == ':')
-        {
-            mode = 1;
-        }
-        else if (display_text[index] == '.')
-        {
-            mode = 2;
-        }
-        else if (mode == 0)
-        {
-            host[host_index] = display_text[index];
-            host_index++;
-        }
-        else if (mode == 1)
-        {
-            disp[disp_index] = display_text[index];
-            disp_index++;
-        }
-        else if (mode == 2)
-        {
-            scre[scre_index] = display_text[index];
-            scre_index++;
-        }
-
-        index++;
-    }
-
-    host[host_index] = 0;
-    disp[disp_index] = 0;
-    scre[scre_index] = 0;
-    g_display_num = g_atoi(disp);
-    return 0;
-}
-
 /*****************************************************************************/
 int
 main_cleanup(void)
@@ -1728,7 +1669,7 @@ main(int argc, char **argv)
     char text[256];
     const char *config_path;
     char log_path[256];
-    char *display_text;
+    const char *display_text;
     char log_file[256];
     enum logReturns error;
     struct log_config *logconfig;
@@ -1743,6 +1684,21 @@ main(int argc, char **argv)
         return 1;
     }
 
+    display_text = g_getenv("DISPLAY");
+    if (display_text == NULL)
+    {
+        g_writeln("DISPLAY is not set");
+        main_cleanup();
+        return 1;
+    }
+    g_display_num = g_get_display_num_from_display(display_text);
+    if (g_display_num < 0)
+    {
+        g_writeln("Unable to get display from DISPLAY='%s'", display_text);
+        main_cleanup();
+        return 1;
+    }
+
     /*
      * The user is unable at present to override the sysadmin-provided
      * sesman.ini location */
@@ -1755,11 +1711,6 @@ main(int argc, char **argv)
     config_dump(g_cfg);
 
     pid = g_getpid();
-    display_text = g_getenv("DISPLAY");
-    if (display_text != NULL)
-    {
-        get_display_num_from_display(display_text);
-    }
 
     /* starting logging subsystem */
     g_snprintf(log_file, 255, "%s/xrdp-chansrv.%d.log", log_path, g_display_num);
@@ -1813,13 +1764,6 @@ main(int argc, char **argv)
 
     LOG_DEVEL(LOG_LEVEL_INFO, "main: DISPLAY env var set to %s", display_text);
 
-    if (g_display_num == 0)
-    {
-        LOG_DEVEL(LOG_LEVEL_ERROR, "main: error, display is zero");
-        main_cleanup();
-        return 1;
-    }
-
     LOG_DEVEL(LOG_LEVEL_INFO, "main: using DISPLAY %d", g_display_num);
     g_snprintf(text, 255, "xrdp_chansrv_%8.8x_main_term", pid);
     g_term_event = g_create_wait_obj(text);

sesman/chansrv/pcsc/xrdp_pcsc.c

@@ -11,6 +11,8 @@
 #include <sys/un.h>
 #include <sys/stat.h>
 
+#include "string_calls.h"
+
 #define PCSC_API
 
 typedef unsigned char BYTE;
@@ -153,66 +155,6 @@ lhexdump(void *p, int len)
     }
 }
 
-/*****************************************************************************/
-static int
-get_display_num_from_display(const char *display_text)
-{
-    int rv;
-    int index;
-    int mode;
-    int host_index;
-    int disp_index;
-    int scre_index;
-    char host[256];
-    char disp[256];
-    char scre[256];
-
-    memset(host, 0, 256);
-    memset(disp, 0, 256);
-    memset(scre, 0, 256);
-
-    index = 0;
-    host_index = 0;
-    disp_index = 0;
-    scre_index = 0;
-    mode = 0;
-
-    while (display_text[index] != 0)
-    {
-        if (display_text[index] == ':')
-        {
-            mode = 1;
-        }
-        else if (display_text[index] == '.')
-        {
-            mode = 2;
-        }
-        else if (mode == 0)
-        {
-            host[host_index] = display_text[index];
-            host_index++;
-        }
-        else if (mode == 1)
-        {
-            disp[disp_index] = display_text[index];
-            disp_index++;
-        }
-        else if (mode == 2)
-        {
-            scre[scre_index] = display_text[index];
-            scre_index++;
-        }
-        index++;
-    }
-    host[host_index] = 0;
-    disp[disp_index] = 0;
-    scre[scre_index] = 0;
-    LLOGLN(10, ("get_display_num_from_display: host [%s] disp [%s] scre [%s]",
-                host, disp, scre));
-    rv = atoi(disp);
-    return rv;
-}
-
 /*****************************************************************************/
 static int
 connect_to_chansrv(void)
@@ -252,11 +194,11 @@ connect_to_chansrv(void)
         LLOGLN(0, ("connect_to_chansrv: error, home not set"));
         return 1;
     }
-    dis = get_display_num_from_display(xrdp_display);
-    if (dis < 10)
+    dis = g_get_display_num_from_display(xrdp_display);
+    if (dis < 0)
     {
-        /* DISPLAY must be > 9 */
-        LLOGLN(0, ("connect_to_chansrv: error, display not > 9 %d", dis));
+        LLOGLN(0, ("connect_to_chansrv: error, don't understand DISPLAY='%s'",
+               xrdp_display));
         return 1;
     }
     g_sck = socket(PF_LOCAL, SOCK_STREAM, 0);

sesman/tools/Makefile.am

@@ -38,6 +38,9 @@ xrdp_sesadmin_SOURCES = \
 xrdp_dis_SOURCES = \
   dis.c
 
+xrdp_dis_LDADD = \
+  $(top_builddir)/common/libcommon.la
+
 xrdp_xcon_SOURCES = \
   xcon.c
 

sesman/tools/dis.c

@@ -29,6 +29,7 @@
 #include <errno.h>
 
 #include "xrdp_sockets.h"
+#include "string_calls.h"
 
 int main(int argc, char **argv)
 {
@@ -36,7 +37,6 @@ int main(int argc, char **argv)
     int dis;
     struct sockaddr_un sa;
     size_t len;
-    char *p;
     char *display;
 
     if (argc != 1)
@@ -54,7 +54,12 @@ int main(int argc, char **argv)
         return 1;
     }
 
-    dis = strtol(display + 1, &p, 10);
+    dis = g_get_display_num_from_display(display);
+    if (dis < 0)
+    {
+        printf("Can't parse DISPLAY='%s'\n", display);
+        return 1;
+    }
     memset(&sa, 0, sizeof(sa));
     sa.sun_family = AF_UNIX;
     sprintf(sa.sun_path, XRDP_DISCONNECT_STR, dis);

xrdp/xrdp.ini.in

@@ -182,9 +182,6 @@ tcutils=true
 ; for debugging xrdp, in section xrdp1, change port=-1 to this:
 #port=/tmp/.xrdp/xrdp_display_10
 
-; for debugging xrdp, add following line to section xrdp1
-#chansrvport=/tmp/.xrdp/xrdp_chansrv_socket_7210
-
 
 ;
 ; Session types
@@ -214,7 +211,10 @@ port=-1
 ; Disable requested encodings to support buggy VNC servers
 ; (1 = ExtendedDesktopSize)
 #disabled_encodings_mask=0
-
+; Use this to connect to a chansrv instance created outside of sesman
+; (e.g. as part of an x11vnc console session). Replace '0' with the
+; display number of the session
+#chansrvport=DISPLAY(0)
 
 [vnc-any]
 name=vnc-any

xrdp/xrdp_mm.c

@@ -34,6 +34,7 @@
 #include <security/pam_constants.h>
 #endif
 #endif /* USE_PAM */
+#include <ctype.h>
 
 #include "xrdp_encoder.h"
 #include "xrdp_sockets.h"
@@ -2160,6 +2161,54 @@ getPAMAdditionalErrorInfo(const int pamError, struct xrdp_mm *self)
 }
 #endif
 
+/*************************************************************************//**
+ * Parses a chansrvport string
+ *
+ * This will be in one of the following formats:-
+ * <path>         UNIX path to a domain socket
+ * DISPLAY(<num>) Use chansrv on X Display <num>
+ *
+ * @param value assigned to chansrvport
+ * @param dest Output buffer
+ * @param dest_size Total size of output buffer, including terminator space
+ * @return 0 for success
+ */
+
+static int
+parse_chansrvport(const char *value, char *dest, int dest_size)
+{
+    int rv = 0;
+
+    if (g_strncmp(value, "DISPLAY(", 8) == 0)
+    {
+        const char *p = value + 8;
+        const char *end = p;
+
+        /* Check next chars are digits followed by ')' */
+        while (isdigit(*end))
+        {
+            ++end;
+        }
+
+        if (end == p || *end != ')')
+        {
+            LOG(LOG_LEVEL_WARNING, "Ignoring invalid chansrvport string '%s'",
+                value);
+            rv = -1;
+        }
+        else
+        {
+            g_snprintf(dest, dest_size, XRDP_CHANSRV_STR, g_atoi(p));
+        }
+    }
+    else
+    {
+        g_strncpy(dest, value, dest_size - 1);
+    }
+
+    return rv;
+}
+
 /*****************************************************************************/
 int
 xrdp_mm_connect(struct xrdp_mm *self)
@@ -2238,8 +2287,10 @@ xrdp_mm_connect(struct xrdp_mm *self)
         }
         else if (g_strcasecmp(name, "chansrvport") == 0)
         {
-            g_strncpy(chansrvport, value, 255);
-            self->usechansrv = 1;
+            if (parse_chansrvport(value, chansrvport, sizeof(chansrvport)) == 0)
+            {
+                self->usechansrv = 1;
+            }
         }
     }
 

xrdpapi/xrdpapi.c

@@ -35,6 +35,7 @@
 
 #include "log.h"
 #include "xrdp_sockets.h"
+#include "string_calls.h"
 #include "xrdpapi.h"
 
 struct wts_obj
@@ -45,8 +46,6 @@ struct wts_obj
 
 /* helper functions used by WTSxxx API - do not invoke directly */
 static int
-get_display_num_from_display(char *display_text);
-static int
 can_send(int sck, int millis);
 static int
 can_recv(int sck, int millis);
@@ -93,7 +92,6 @@ WTSVirtualChannelOpenEx(unsigned int SessionId, const char *pVirtualName,
                         unsigned int flags)
 {
     struct wts_obj *wts;
-    char *display_text;
     int bytes;
     unsigned long long1;
     struct sockaddr_un s;
@@ -113,16 +111,10 @@ WTSVirtualChannelOpenEx(unsigned int SessionId, const char *pVirtualName,
         return 0;
     }
     wts->fd = -1;
-    display_text = getenv("DISPLAY");
-
-    if (display_text != 0)
+    wts->display_num = g_get_display_num_from_display(getenv("DISPLAY"));
+    if (wts->display_num < 0)
     {
-        wts->display_num = get_display_num_from_display(display_text);
-    }
-
-    if (wts->display_num <= 0)
-    {
-        LOG(LOG_LEVEL_ERROR, "WTSVirtualChannelOpenEx: fatal error; display is <= 0");
+        LOG(LOG_LEVEL_ERROR, "WTSVirtualChannelOpenEx: fatal error; invalid DISPLAY");
         free(wts);
         return NULL;
     }
@@ -526,38 +518,3 @@ can_recv(int sck, int millis)
 
     return 0;
 }
-
-/*****************************************************************************/
-static int
-get_display_num_from_display(char *display_text)
-{
-    int index;
-    int mode;
-    int disp_index;
-    char disp[256];
-
-    index = 0;
-    disp_index = 0;
-    mode = 0;
-
-    while (display_text[index] != 0)
-    {
-        if (display_text[index] == ':')
-        {
-            mode = 1;
-        }
-        else if (display_text[index] == '.')
-        {
-            mode = 2;
-        }
-        else if (mode == 1)
-        {
-            disp[disp_index] = display_text[index];
-            disp_index++;
-        }
-        index++;
-    }
-
-    disp[disp_index] = 0;
-    return atoi(disp);
-}