From e82f212f34abee9e09383bd2908e98f7359f7ca9 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Thu, 14 Jun 2018 11:59:27 +0900 Subject: [PATCH 1/6] sesman: accept full path for DefaultWindowManager Solves: #1143 Also, this idea is inspired by Fedora's patch [1]. Some distro wants to put all scripts in libexec directory due to SELinux. This enables distros to put such scripts anywhere. [1] https://src.fedoraproject.org/cgit/rpms/xrdp.git/tree/xrdp-0.9.6-scripts-libexec.patch?id=02f845c1b8cea781313cf3e9efcd6d7d50341824 --- sesman/config.c | 24 ++++++++++++++++++++---- sesman/config.h | 2 +- sesman/session.c | 3 +-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/sesman/config.c b/sesman/config.c index 8e5b096d..0fa8c86b 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -115,7 +115,7 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, cf->listen_port[0] = '\0'; cf->enable_user_wm = 0; cf->user_wm[0] = '\0'; - cf->default_wm[0] = '\0'; + cf->default_wm = 0; cf->auth_file_path = 0; file_read_section(file, SESMAN_CFG_GLOBALS, param_n, param_v); @@ -126,7 +126,7 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, if (0 == g_strcasecmp(buf, SESMAN_CFG_DEFWM)) { - g_strncpy(cf->default_wm, (char *)list_get_item(param_v, i), 31); + cf->default_wm = g_strdup((char *)list_get_item(param_v, i)); } else if (0 == g_strcasecmp(buf, SESMAN_CFG_USERWM)) { @@ -166,9 +166,24 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, cf->enable_user_wm = 0; } - if ('\0' == cf->default_wm[0]) + if (cf->default_wm == 0) { - g_strncpy(cf->default_wm, "startwm.sh", 11); + cf->default_wm = g_strdup("startwm.sh"); + } + else if (g_strlen(cf->default_wm) == 0) + { + g_free(cf->default_wm); + cf->default_wm = g_strdup("startwm.sh"); + } + + /* if default_wm doesn't begin with '/', it's a relative path from XRDP_CFG_PATH */ + if (cf->default_wm[0] != '/') + { + buf = (char *)g_malloc(1024, 0); + g_sprintf(buf, "%s/%s", XRDP_CFG_PATH, g_cfg->default_wm); + g_free(g_cfg->default_wm); + g_cfg->default_wm = g_strdup(buf); + g_free(buf); } return 0; @@ -530,6 +545,7 @@ config_dump(struct config_sesman *config) void config_free(struct config_sesman *cs) { + g_free(cs->default_wm); g_free(cs->auth_file_path); list_delete(cs->rdp_params); list_delete(cs->vnc_params); diff --git a/sesman/config.h b/sesman/config.h index c7cb50fd..fc964a60 100644 --- a/sesman/config.h +++ b/sesman/config.h @@ -198,7 +198,7 @@ struct config_sesman * @var default_wm * @brief Default window manager */ - char default_wm[32]; + char *default_wm; /** * @var user_wm * @brief Default window manager diff --git a/sesman/session.c b/sesman/session.c index d33f2f5f..3c128ebb 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -575,8 +575,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_CONNECTION *c, } /* if we're here something happened to g_execlp3 so we try running the default window manager */ - g_sprintf(text, "%s/%s", XRDP_CFG_PATH, g_cfg->default_wm); - g_execlp3(text, g_cfg->default_wm, 0); + g_execlp3(g_cfg->default_wm, g_cfg->default_wm, 0); log_message(LOG_LEVEL_ALWAYS, "error starting default " "wm for user %s - pid %d", s->username, g_getpid()); From a39b4137463841c364f8a08d8b9b28fc80a69739 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Thu, 14 Jun 2018 15:31:12 +0900 Subject: [PATCH 2/6] sesman: make the path of reconnect script configurable --- sesman/config.c | 26 ++++++++++++++++++++++++++ sesman/config.h | 6 ++++++ sesman/sesman.ini.in | 1 + sesman/session.c | 6 ++---- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/sesman/config.c b/sesman/config.c index 0fa8c86b..28b66cda 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -117,6 +117,7 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, cf->user_wm[0] = '\0'; cf->default_wm = 0; cf->auth_file_path = 0; + cf->reconnect_sh = 0; file_read_section(file, SESMAN_CFG_GLOBALS, param_n, param_v); @@ -148,6 +149,10 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, { cf->auth_file_path = g_strdup((char *)list_get_item(param_v, i)); } + else if (g_strcasecmp(buf, SESMAN_CFG_RECONNECT_SH) == 0) + { + cf->reconnect_sh = g_strdup((char *)list_get_item(param_v, i)); + } } /* checking for missing required parameters */ @@ -186,6 +191,25 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, g_free(buf); } + if (cf->reconnect_sh == 0) + { + cf->reconnect_sh = g_strdup("reconnectwm.sh"); + } + else if (g_strlen(cf->reconnect_sh) == 0) + { + g_free(cf->reconnect_sh); + cf->reconnect_sh = g_strdup("reconnectwm.sh"); + } + if (cf->reconnect_sh[0] != '/') + { + buf = (char *)g_malloc(1024, 0); + g_sprintf(buf, "%s/%s", XRDP_CFG_PATH, g_cfg->reconnect_sh); + g_free(g_cfg->reconnect_sh); + g_cfg->reconnect_sh = g_strdup(buf); + g_free(buf); + } + + return 0; } @@ -451,6 +475,7 @@ config_dump(struct config_sesman *config) g_writeln(" EnableUserWindowManager: %d", config->enable_user_wm); g_writeln(" UserWindowManager: %s", config->user_wm); g_writeln(" DefaultWindowManager: %s", config->default_wm); + g_writeln(" ReconnectScript: %s", config->reconnect_sh); g_writeln(" AuthFilePath: %s", ((config->auth_file_path) ? (config->auth_file_path) : ("disabled"))); @@ -546,6 +571,7 @@ void config_free(struct config_sesman *cs) { g_free(cs->default_wm); + g_free(cs->reconnect_sh); g_free(cs->auth_file_path); list_delete(cs->rdp_params); list_delete(cs->vnc_params); diff --git a/sesman/config.h b/sesman/config.h index fc964a60..6e63fcef 100644 --- a/sesman/config.h +++ b/sesman/config.h @@ -39,6 +39,7 @@ #define SESMAN_CFG_USERWM "UserWindowManager" #define SESMAN_CFG_MAX_SESSION "MaxSessions" #define SESMAN_CFG_AUTH_FILE_PATH "AuthFilePath" +#define SESMAN_CFG_RECONNECT_SH "ReconnectScript" #define SESMAN_CFG_RDP_PARAMS "X11rdp" #define SESMAN_CFG_XORG_PARAMS "Xorg" @@ -204,6 +205,11 @@ struct config_sesman * @brief Default window manager */ char user_wm[32]; + /** + * @var reconnect_sh + * @brief Script executed when reconnected + */ + char *reconnect_sh; /** * @var auth_file_path * @brief Auth file path diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in index e3609da2..f3cdc7f7 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -4,6 +4,7 @@ ListenPort=3350 EnableUserWindowManager=true UserWindowManager=startwm.sh DefaultWindowManager=startwm.sh +ReconnectScript=reconnectwm.sh [Security] AllowRootLogin=true diff --git a/sesman/session.c b/sesman/session.c index 3c128ebb..0877c542 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -862,7 +862,6 @@ static int session_reconnect_fork(int display, char *username, long data) { int pid; - char text[256]; pid = g_fork(); @@ -877,11 +876,10 @@ session_reconnect_fork(int display, char *username, long data) g_cfg->env_names, g_cfg->env_values); auth_set_env(data); - g_snprintf(text, 255, "%s/%s", XRDP_CFG_PATH, "reconnectwm.sh"); - if (g_file_exist(text)) + if (g_file_exist(g_cfg->reconnect_sh)) { - g_execlp3(text, g_cfg->default_wm, 0); + g_execlp3(g_cfg->reconnect_sh, g_cfg->reconnect_sh, 0); } g_exit(0); From 6fb18cd5faa82013de8dcc39ffb67eb614681394 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Thu, 14 Jun 2018 16:04:02 +0900 Subject: [PATCH 3/6] docs: document configurable reconnect script path --- docs/man/Makefile.am | 3 ++- docs/man/sesman.ini.5.in | 15 ++++++++++++--- sesman/sesman.ini.in | 5 +++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/man/Makefile.am b/docs/man/Makefile.am index 841e13e1..f138673d 100644 --- a/docs/man/Makefile.am +++ b/docs/man/Makefile.am @@ -17,7 +17,8 @@ SUBST_VARS = sed \ -e 's|@bindir[@]|$(bindir)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ -e 's|@sysconfdir[@]|$(sysconfdir)|g' \ - -e 's|@socketdir[@]|$(socketdir)|g' + -e 's|@socketdir[@]|$(socketdir)|g' \ + -e 's|@xrdpconfdir[@]|$(sysconfdir)/xrdp|g' subst_verbose = $(subst_verbose_@AM_V@) subst_verbose_ = $(subst_verbose_@AM_DEFAULT_V@) diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index 9cd05932..4fad8eb4 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -62,14 +62,23 @@ specified by \fBUserWindowManager\fR if it exists. .TP \fBUserWindowManager\fR=\fIfilename\fR -Name of the startup script relative to the user's home directory. If +Path of the startup script relative to the user's home directory. If present and enabled by \fBEnableUserWindowManager\fR, that script is executed instead of \fBDefaultWindowManager\fR. .TP \fBDefaultWindowManager\fR=\fIfilename\fR -Full path to the default startup script used by xrdp-sesman to start a -session if the user script is disabled or missing. +Full path or relative path of the default startup script used by xrdp-sesman +to start a session. If the path is not a full path, it will be resolved as +relative path to \fI@xrdpconfdir@\fR. If not specified, defaults to +\fI@xrdpconfdir@/startwm.sh\fR. + +.TP +\fBReconnectScript\fR=\fIfilename\fR +Full path or relative path if the script which executed when users reconnects +to the existing session. If the path is not a full path, it will be resolved as +relative path to \fI@xrdpconfdir@\fR. If not specified, defaults to +\fI@xrdpconfdir@/reconnectwm.sh\fR. .SH "LOGGING" Following parameters can be used in the \fB[Logging]\fR section. diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in index f3cdc7f7..68ef7795 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -1,9 +1,14 @@ +;; See `man 5 sesman.ini` for details + [Globals] ListenAddress=127.0.0.1 ListenPort=3350 EnableUserWindowManager=true +; Give in relative path to user's home directory UserWindowManager=startwm.sh +; Give in full path or relative path to @sesmansysconfdir@ DefaultWindowManager=startwm.sh +; Give in full path or relative path to @sesmansysconfdir@ ReconnectScript=reconnectwm.sh [Security] From 9192e95c969a697f70e8f15e0f2c640297ba636e Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Fri, 15 Jun 2018 13:46:23 +0900 Subject: [PATCH 4/6] sesman: fix logging after default_wm change --- sesman/session.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sesman/session.c b/sesman/session.c index 0877c542..0d9fdc70 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -584,7 +584,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_CONNECTION *c, "%s", g_get_errno(), g_get_strerror()); log_message(LOG_LEVEL_DEBUG, "execlp3 parameter list:"); log_message(LOG_LEVEL_DEBUG, " argv[0] = %s", - text); + g_cfg->default_wm); log_message(LOG_LEVEL_DEBUG, " argv[1] = %s", g_cfg->default_wm); From 6e16b38ecc109bc889dcc1ea79a9ad436abd463a Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Fri, 15 Jun 2018 14:10:25 +0900 Subject: [PATCH 5/6] sesman: fix potential buffer over flow --- sesman/config.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sesman/config.c b/sesman/config.c index 28b66cda..796b9511 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -105,6 +105,7 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, struct list *param_v) { int i; + int length; char *buf; list_clear(param_v); @@ -180,11 +181,11 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, g_free(cf->default_wm); cf->default_wm = g_strdup("startwm.sh"); } - - /* if default_wm doesn't begin with '/', it's a relative path from XRDP_CFG_PATH */ + /* if default_wm doesn't begin with '/', it's a relative path to XRDP_CFG_PATH */ if (cf->default_wm[0] != '/') { - buf = (char *)g_malloc(1024, 0); + length = sizeof(XRDP_CFG_PATH) + g_strlen(g_cfg->default_wm) + 1; /* '/' */ + buf = (char *)g_malloc(length, 0); g_sprintf(buf, "%s/%s", XRDP_CFG_PATH, g_cfg->default_wm); g_free(g_cfg->default_wm); g_cfg->default_wm = g_strdup(buf); @@ -200,16 +201,17 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, g_free(cf->reconnect_sh); cf->reconnect_sh = g_strdup("reconnectwm.sh"); } + /* if reconnect_sh doesn't begin with '/', it's a relative path to XRDP_CFG_PATH */ if (cf->reconnect_sh[0] != '/') { - buf = (char *)g_malloc(1024, 0); + length = sizeof(XRDP_CFG_PATH) + g_strlen(g_cfg->reconnect_sh) + 1; /* '/' */ + buf = (char *)g_malloc(length, 0); g_sprintf(buf, "%s/%s", XRDP_CFG_PATH, g_cfg->reconnect_sh); g_free(g_cfg->reconnect_sh); g_cfg->reconnect_sh = g_strdup(buf); g_free(buf); } - return 0; } From eda18428252aa5b12532387d236d06530c0d866d Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Tue, 19 Jun 2018 12:57:30 +0900 Subject: [PATCH 6/6] sesman: add comments, no logic change --- sesman/config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sesman/config.c b/sesman/config.c index 796b9511..78949995 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -184,6 +184,7 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, /* if default_wm doesn't begin with '/', it's a relative path to XRDP_CFG_PATH */ if (cf->default_wm[0] != '/') { + /* sizeof operator returns string length including null terminator */ length = sizeof(XRDP_CFG_PATH) + g_strlen(g_cfg->default_wm) + 1; /* '/' */ buf = (char *)g_malloc(length, 0); g_sprintf(buf, "%s/%s", XRDP_CFG_PATH, g_cfg->default_wm); @@ -204,6 +205,7 @@ config_read_globals(int file, struct config_sesman *cf, struct list *param_n, /* if reconnect_sh doesn't begin with '/', it's a relative path to XRDP_CFG_PATH */ if (cf->reconnect_sh[0] != '/') { + /* sizeof operator returns string length including null terminator */ length = sizeof(XRDP_CFG_PATH) + g_strlen(g_cfg->reconnect_sh) + 1; /* '/' */ buf = (char *)g_malloc(length, 0); g_sprintf(buf, "%s/%s", XRDP_CFG_PATH, g_cfg->reconnect_sh);