From e7cb5bd64ad49c4c25f662983db78de03df87542 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 31 Mar 2021 11:31:41 +0100 Subject: [PATCH 1/3] Allow DISPLAY=:0 for chansrv and in other modules --- sesman/chansrv/chansrv.c | 96 ++++++++++++--------------------- sesman/chansrv/pcsc/xrdp_pcsc.c | 12 +++-- xrdpapi/xrdpapi.c | 10 ++-- 3 files changed, 50 insertions(+), 68 deletions(-) diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c index 4bca525e..5a96f9b8 100644 --- a/sesman/chansrv/chansrv.c +++ b/sesman/chansrv/chansrv.c @@ -17,6 +17,8 @@ * limitations under the License. */ +#include + #if defined(HAVE_CONFIG_H) #include #endif @@ -61,7 +63,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 */ @@ -1559,61 +1561,30 @@ x_server_fatal_handler(void) /*****************************************************************************/ static int -get_display_num_from_display(char *display_text) +get_display_num_from_display(const 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]; + int rv = -1; + const char *p; - 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) + /* Skip over the hostname part of the DISPLAY */ + if ((p = g_strchr(display_text, ':')) != NULL) { - if (display_text[index] == ':') + ++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 == ':') { - 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++; + ++p; } - index++; + if (isdigit(*p)) + { + rv = g_atoi(p); + } } - host[host_index] = 0; - disp[disp_index] = 0; - scre[scre_index] = 0; - g_display_num = g_atoi(disp); - return 0; + return rv; } /*****************************************************************************/ @@ -1728,7 +1699,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 +1714,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 = 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 +1741,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 +1794,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); diff --git a/sesman/chansrv/pcsc/xrdp_pcsc.c b/sesman/chansrv/pcsc/xrdp_pcsc.c index a6ea874a..cf958dbb 100644 --- a/sesman/chansrv/pcsc/xrdp_pcsc.c +++ b/sesman/chansrv/pcsc/xrdp_pcsc.c @@ -194,6 +194,10 @@ get_display_num_from_display(const char *display_text) } else if (mode == 1) { + if (display_text[index] < '0' || display_text[index] > '9') + { + return -1; + } disp[disp_index] = display_text[index]; disp_index++; } @@ -209,7 +213,7 @@ get_display_num_from_display(const char *display_text) scre[scre_index] = 0; LLOGLN(10, ("get_display_num_from_display: host [%s] disp [%s] scre [%s]", host, disp, scre)); - rv = atoi(disp); + rv = (disp_index== 0) ? -1 : atoi(disp); return rv; } @@ -253,10 +257,10 @@ connect_to_chansrv(void) return 1; } dis = get_display_num_from_display(xrdp_display); - if (dis < 10) + 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); diff --git a/xrdpapi/xrdpapi.c b/xrdpapi/xrdpapi.c index db1a1a7d..11198e28 100644 --- a/xrdpapi/xrdpapi.c +++ b/xrdpapi/xrdpapi.c @@ -120,9 +120,9 @@ WTSVirtualChannelOpenEx(unsigned int SessionId, const char *pVirtualName, wts->display_num = get_display_num_from_display(display_text); } - if (wts->display_num <= 0) + 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; } @@ -552,6 +552,10 @@ get_display_num_from_display(char *display_text) } else if (mode == 1) { + if (display_text[index] < '0' || display_text[index] > '9') + { + return -1; + } disp[disp_index] = display_text[index]; disp_index++; } @@ -559,5 +563,5 @@ get_display_num_from_display(char *display_text) } disp[disp_index] = 0; - return atoi(disp); + return (disp_index== 0) ? -1 : atoi(disp); } From 86c87b6f159a426efd3c4ee75cd993937460ae36 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 13 Apr 2021 12:11:19 +0100 Subject: [PATCH 2/3] Move get_display_num_from_display to string_calls module --- common/string_calls.c | 33 +++++++++++++++- common/string_calls.h | 10 +++++ sesman/chansrv/chansrv.c | 32 +--------------- sesman/chansrv/pcsc/xrdp_pcsc.c | 68 ++------------------------------- sesman/tools/Makefile.am | 3 ++ sesman/tools/dis.c | 9 ++++- xrdpapi/xrdpapi.c | 51 +------------------------ 7 files changed, 58 insertions(+), 148 deletions(-) diff --git a/common/string_calls.c b/common/string_calls.c index 18b6acf4..a53a4d9d 100644 --- a/common/string_calls.c +++ b/common/string_calls.c @@ -24,6 +24,8 @@ #include #include #include +#include + #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; } - diff --git a/common/string_calls.h b/common/string_calls.h index 4918b840..02876fca 100644 --- a/common/string_calls.h +++ b/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); diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c index 5a96f9b8..4452d998 100644 --- a/sesman/chansrv/chansrv.c +++ b/sesman/chansrv/chansrv.c @@ -17,8 +17,6 @@ * limitations under the License. */ -#include - #if defined(HAVE_CONFIG_H) #include #endif @@ -1559,34 +1557,6 @@ x_server_fatal_handler(void) exit(0); } -/*****************************************************************************/ -static int -get_display_num_from_display(const char *display_text) -{ - int rv = -1; - const char *p; - - /* Skip over the hostname part of the DISPLAY */ - if ((p = g_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; - } - - if (isdigit(*p)) - { - rv = g_atoi(p); - } - } - - return rv; -} - /*****************************************************************************/ int main_cleanup(void) @@ -1721,7 +1691,7 @@ main(int argc, char **argv) main_cleanup(); return 1; } - g_display_num = get_display_num_from_display(display_text); + 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); diff --git a/sesman/chansrv/pcsc/xrdp_pcsc.c b/sesman/chansrv/pcsc/xrdp_pcsc.c index cf958dbb..252212c0 100644 --- a/sesman/chansrv/pcsc/xrdp_pcsc.c +++ b/sesman/chansrv/pcsc/xrdp_pcsc.c @@ -11,6 +11,8 @@ #include #include +#include "string_calls.h" + #define PCSC_API typedef unsigned char BYTE; @@ -153,70 +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) - { - if (display_text[index] < '0' || display_text[index] > '9') - { - return -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 = (disp_index== 0) ? -1 : atoi(disp); - return rv; -} - /*****************************************************************************/ static int connect_to_chansrv(void) @@ -256,7 +194,7 @@ connect_to_chansrv(void) LLOGLN(0, ("connect_to_chansrv: error, home not set")); return 1; } - dis = get_display_num_from_display(xrdp_display); + dis = g_get_display_num_from_display(xrdp_display); if (dis < 0) { LLOGLN(0, ("connect_to_chansrv: error, don't understand DISPLAY='%s'", diff --git a/sesman/tools/Makefile.am b/sesman/tools/Makefile.am index 2f8be440..ae5d80cd 100644 --- a/sesman/tools/Makefile.am +++ b/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 diff --git a/sesman/tools/dis.c b/sesman/tools/dis.c index 728be4a9..cc06e870 100644 --- a/sesman/tools/dis.c +++ b/sesman/tools/dis.c @@ -29,6 +29,7 @@ #include #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); diff --git a/xrdpapi/xrdpapi.c b/xrdpapi/xrdpapi.c index 11198e28..1eeabdac 100644 --- a/xrdpapi/xrdpapi.c +++ b/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,13 +111,7 @@ WTSVirtualChannelOpenEx(unsigned int SessionId, const char *pVirtualName, return 0; } wts->fd = -1; - display_text = getenv("DISPLAY"); - - if (display_text != 0) - { - wts->display_num = get_display_num_from_display(display_text); - } - + wts->display_num = g_get_display_num_from_display(getenv("DISPLAY")); if (wts->display_num < 0) { LOG(LOG_LEVEL_ERROR, "WTSVirtualChannelOpenEx: fatal error; invalid DISPLAY"); @@ -526,42 +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) - { - if (display_text[index] < '0' || display_text[index] > '9') - { - return -1; - } - disp[disp_index] = display_text[index]; - disp_index++; - } - index++; - } - - disp[disp_index] = 0; - return (disp_index== 0) ? -1 : atoi(disp); -} From 32b676472a90ae62d438325d6ff3d4eb3890dfbb Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 13 Apr 2021 12:12:08 +0100 Subject: [PATCH 3/3] Add DISPLAY(n) ass a valid form of chansrvport --- docs/man/xrdp.ini.5.in | 17 ++++++++++--- xrdp/xrdp.ini.in | 8 +++--- xrdp/xrdp_mm.c | 55 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/docs/man/xrdp.ini.5.in b/docs/man/xrdp.ini.5.in index da5b7c06..581fcac9 100644 --- a/docs/man/xrdp.ini.5.in +++ b/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 diff --git a/xrdp/xrdp.ini.in b/xrdp/xrdp.ini.in index 699b9911..d6bb0595 100644 --- a/xrdp/xrdp.ini.in +++ b/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 diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 4679bd6b..915c3b32 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -34,6 +34,7 @@ #include #endif #endif /* USE_PAM */ +#include #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:- + * UNIX path to a domain socket + * DISPLAY() Use chansrv on X Display + * + * @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; + } } }