From 547c619c2ff9f27d3688e95130a4ee924fcaa939 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 14 Jun 2023 10:17:19 +0100 Subject: [PATCH 1/4] Move g_mk_socket_path() to sesman The sockdir is only used when sesman is active. The call g_mk_socket_path() is removed from os_calls and moved to sesman. We also change the permissions on this directory to 0755 rather than 01777 (01000 is the 'sticky bit', S_ISVTX). The behaviour of g_create_dir() has been modified to not set S_ISVTX on Linux directories. This is implementation-defined behaviour according to 1003.1, and is no longer required for the sockdir. --- common/Makefile.am | 3 +-- common/os_calls.c | 24 +----------------------- common/os_calls.h | 1 - sesman/sesman.c | 40 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 41 insertions(+), 27 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index c929baab1..2c5fc5633 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -27,8 +27,7 @@ AM_CPPFLAGS = \ -DXRDP_SBIN_PATH=\"${sbindir}\" \ -DXRDP_SHARE_PATH=\"${datadir}/xrdp\" \ -DXRDP_PID_PATH=\"${localstatedir}/run\" \ - -DXRDP_LOG_PATH=\"${localstatedir}/log\" \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" + -DXRDP_LOG_PATH=\"${localstatedir}/log\" # -no-suppress is an automake-specific flag which is needed # to prevent us missing compiler errors in some circumstances diff --git a/common/os_calls.c b/common/os_calls.c index 6739a647c..1194c0486 100644 --- a/common/os_calls.c +++ b/common/os_calls.c @@ -150,28 +150,6 @@ g_rm_temp_dir(void) return 0; } -/*****************************************************************************/ -int -g_mk_socket_path(void) -{ - if (!g_directory_exist(XRDP_SOCKET_PATH)) - { - if (!g_create_path(XRDP_SOCKET_PATH"/")) - { - /* if failed, still check if it got created by someone else */ - if (!g_directory_exist(XRDP_SOCKET_PATH)) - { - LOG(LOG_LEVEL_ERROR, - "g_mk_socket_path: g_create_path(%s) failed", - XRDP_SOCKET_PATH); - return 1; - } - } - g_chmod_hex(XRDP_SOCKET_PATH, 0x1777); - } - return 0; -} - /*****************************************************************************/ void g_init(const char *app_name) @@ -2666,7 +2644,7 @@ g_create_dir(const char *dirname) #if defined(_WIN32) return CreateDirectoryA(dirname, 0); // test this #else - return mkdir(dirname, (mode_t) - 1) == 0; + return mkdir(dirname, 0777) == 0; #endif } diff --git a/common/os_calls.h b/common/os_calls.h index 6579d1163..a9cf8f5d2 100644 --- a/common/os_calls.h +++ b/common/os_calls.h @@ -53,7 +53,6 @@ struct list; #define g_close_wait_obj g_delete_wait_obj int g_rm_temp_dir(void); -int g_mk_socket_path(void); void g_init(const char *app_name); void g_deinit(void); void g_printf(const char *format, ...) printflike(1, 2); diff --git a/sesman/sesman.c b/sesman/sesman.c index 0f780bd85..f5d0b9cf2 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -682,6 +682,44 @@ read_pid_file(const char *pid_file, int *pid) return rv; } +/******************************************************************************/ +/** Creates the socket path for sesman and session sockets +*/ +static int +create_xrdp_socket_root_path(void) +{ +#ifndef XRDP_SOCKET_PATH +# error "XRDP_SOCKET_PATH must be defined" +#endif + int uid = g_getuid(); + int gid = g_getgid(); + + /* Create the path using 0755 permissions */ + int old_umask = g_umask_hex(0x22); + (void)g_create_path(XRDP_SOCKET_PATH"/"); + (void)g_umask_hex(old_umask); + + /* Check the ownership and permissions on the last path element + * are as expected */ + if (g_chown(XRDP_SOCKET_PATH, uid, gid) != 0) + { + LOG(LOG_LEVEL_ERROR, + "create_xrdp_socket_root_path: Can't set owner of %s to %d:%d", + XRDP_SOCKET_PATH, uid, gid); + return 1; + } + + if (g_chmod_hex(XRDP_SOCKET_PATH, 0x755) != 0) + { + LOG(LOG_LEVEL_ERROR, + "create_xrdp_socket_root_path: Can't set perms of %s to 0x755", + XRDP_SOCKET_PATH); + return 1; + } + + return 0; +} + /******************************************************************************/ int main(int argc, char **argv) @@ -928,7 +966,7 @@ main(int argc, char **argv) "starting xrdp-sesman with pid %d", g_pid); /* make sure the socket directory exists */ - g_mk_socket_path(); + create_xrdp_socket_root_path(); /* make sure the /tmp/.X11-unix directory exists */ if (!g_directory_exist("/tmp/.X11-unix")) From 63235eaafd1710e0ee835d19cb60ad3347a7c84a Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 14 Jun 2023 10:33:44 +0100 Subject: [PATCH 2/4] Fix typo in error message --- sesman/scp_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sesman/scp_process.c b/sesman/scp_process.c index f2d0b9769..7fab3d46e 100644 --- a/sesman/scp_process.c +++ b/sesman/scp_process.c @@ -389,7 +389,7 @@ process_create_session_request(struct pre_session_item *psi) else if (psi->sesexec_trans == NULL && sesexec_start(psi) != 0) { LOG(LOG_LEVEL_ERROR, - "Can't start sesexec to authenticate user"); + "Can't start sesexec to manage session"); status = E_SCP_SCREATE_GENERAL_ERROR; } else From 675dd77807063b0eec407cb51184dc74812657a6 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 14 Jun 2023 13:59:55 +0100 Subject: [PATCH 3/4] Parameterise the sockdir with the UID of the user The top level socket directory is now called XRDP_SOCKET_ROOT_PATH. Below that are user-specific directories referred to with the XRDP_SOCKET_PATH macro - this name is hard-coded into xorgxrdp and the audio modules as an environment variable. XRDP_SOCKET_PATH now looks like $XRDP_SOCKET_ROOT_PATH/ XRDP_SOCKET_PATH is only writeable by the user, and readable by the user and the xrdp process. --- common/xrdp_sockets.h | 29 +++++++++-- docs/man/sesman.ini.5.in | 5 ++ libipm/Makefile.am | 1 + libipm/scp.c | 25 +++++++--- libipm/scp.h | 11 ++++- sesman/Makefile.am | 2 +- sesman/chansrv/Makefile.am | 2 +- sesman/chansrv/chansrv.c | 8 +-- sesman/chansrv/sound.c | 8 +-- sesman/libsesman/sesman_config.c | 30 +++++++----- sesman/libsesman/sesman_config.h | 6 +++ sesman/scp_process.c | 83 ++++++++++++++++++++++++++++---- sesman/sesexec/Makefile.am | 2 +- sesman/sesexec/env.c | 5 +- sesman/sesexec/login_info.c | 3 +- sesman/sesexec/session.c | 25 +++++----- sesman/sesman.c | 4 +- sesman/sesman.ini.in | 6 +++ sesman/session_list.c | 35 -------------- sesman/tools/Makefile.am | 2 +- sesman/tools/dis.c | 3 +- sesman/tools/sesadmin.c | 2 +- sesman/tools/sesrun.c | 2 +- xrdp/Makefile.am | 2 +- xrdp/xrdp_mm.c | 29 +++++++---- xrdp/xrdp_types.h | 1 + xrdpapi/Makefile.am | 2 +- xrdpapi/xrdpapi.c | 2 +- 28 files changed, 219 insertions(+), 116 deletions(-) diff --git a/common/xrdp_sockets.h b/common/xrdp_sockets.h index 6402798c2..ebf044fff 100644 --- a/common/xrdp_sockets.h +++ b/common/xrdp_sockets.h @@ -21,16 +21,39 @@ #if !defined(XRDP_SOCKETS_H) #define XRDP_SOCKETS_H -/* basename of socket files */ +/* XRDP_SOCKET_ROOT_PATH must be defined to include this file */ +#ifdef __cppcheck__ +/* avoid syntax errors */ +# define XRDP_SOCKET_ROOT_PATH "/dummy" +#elif !defined(XRDP_SOCKET_ROOT_PATH) +# error "XRDP_SOCKET_ROOT_PATH must be defined" +#endif + +/* Buffer size for code for fullpath declarations + * + * This needs to fit in the sun_path field of a sockaddr_un. POSIX + * does not define this size, so the value below is the lower of + * the FreeBSD/OpenBSD/NetBSD(104) and Linux(108) values */ +#define XRDP_SOCKETS_MAXPATH 104 + +/* The socketdir is rooted at XRDP_SOCKET_ROOT_PATH. User-specific + * sockets live in a user-specific sub-directory of this called + * XRDP_SOCKET_PATH. The sub-directory is the UID of the user */ +#define XRDP_SOCKET_PATH XRDP_SOCKET_ROOT_PATH "/%d" + +/* Sockets in XRDP_SOCKET_ROOT_PATH */ +#define SCP_LISTEN_PORT_BASE_STR "sesman.socket" + +/* names of socket files within XRDP_SOCKET_PATH, qualified by + * display number */ #define XRDP_CHANSRV_BASE_STR "xrdp_chansrv_socket_%d" #define CHANSRV_PORT_OUT_BASE_STR "xrdp_chansrv_audio_out_socket_%d" #define CHANSRV_PORT_IN_BASE_STR "xrdp_chansrv_audio_in_socket_%d" #define CHANSRV_API_BASE_STR "xrdpapi_%d" #define XRDP_X11RDP_BASE_STR "xrdp_display_%d" #define XRDP_DISCONNECT_BASE_STR "xrdp_disconnect_display_%d" -#define SCP_LISTEN_PORT_BASE_STR "sesman.socket" -/* fullpath of sockets */ +/* fullpath declarations */ #define XRDP_CHANSRV_STR XRDP_SOCKET_PATH "/" XRDP_CHANSRV_BASE_STR #define CHANSRV_PORT_OUT_STR XRDP_SOCKET_PATH "/" CHANSRV_PORT_OUT_BASE_STR #define CHANSRV_PORT_IN_STR XRDP_SOCKET_PATH "/" CHANSRV_PORT_IN_BASE_STR diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index 1933893a5..0d4f5c907 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -312,6 +312,11 @@ to a setuid Xorg executable. However, if a kernel security module (such as AppArmor) is used to confine xrdp, \fIno_new_privs\fR may interfere with transitions between confinement domains. +.TP +\fBSessionSockdirGroup\fR=\fIgroup\fR +Sets the group owner of the directories containing session sockets. This +is normally the GID of the xrdp process so xrdp can connect to user sessions. + .SH "X11 SERVER" Following parameters can be used in the \fB[Xvnc]\fR and \fB[Xorg]\fR sections. diff --git a/libipm/Makefile.am b/libipm/Makefile.am index 84ea09482..4770f4dd1 100644 --- a/libipm/Makefile.am +++ b/libipm/Makefile.am @@ -1,6 +1,7 @@ AM_CPPFLAGS = \ -DSESMAN_RUNTIME_PATH=\"${sesmanruntimedir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_srcdir)/common module_LTLIBRARIES = \ diff --git a/libipm/scp.c b/libipm/scp.c index de0028d7e..eaeac344a 100644 --- a/libipm/scp.c +++ b/libipm/scp.c @@ -332,14 +332,16 @@ scp_get_sys_login_request(struct trans *trans, int scp_send_login_response(struct trans *trans, enum scp_login_status login_result, - int server_closed) + int server_closed, + int uid) { return libipm_msg_out_simple_send( trans, (int)E_SCP_LOGIN_RESPONSE, - "ib", + "ibi", login_result, - (server_closed != 0)); /* Convert to 0/1 */ + (server_closed != 0), /* Convert to 0/1 */ + uid); } /*****************************************************************************/ @@ -347,22 +349,33 @@ scp_send_login_response(struct trans *trans, int scp_get_login_response(struct trans *trans, enum scp_login_status *login_result, - int *server_closed) + int *server_closed, + int *uid) { int32_t i_login_result = 0; + int32_t i_uid = 0; int dummy; + /* User can pass in NULL for server_closed if they're trying an - * login method like UDS for which all fails are fatal */ + * login method like UDS for which all fails are fatal. Likewise + * they may be uninterested in the uid */ if (server_closed == NULL) { server_closed = &dummy; } + if (uid == NULL) + { + uid = &dummy; + } - int rv = libipm_msg_in_parse(trans, "ib", &i_login_result, server_closed); + int rv = libipm_msg_in_parse(trans, "ibi", + &i_login_result, server_closed, &i_uid); if (rv == 0) { *login_result = (enum scp_login_status)i_login_result; + *uid = i_uid; } + return rv; } diff --git a/libipm/scp.h b/libipm/scp.h index eec86c4f8..ff54fd6a7 100644 --- a/libipm/scp.h +++ b/libipm/scp.h @@ -284,12 +284,14 @@ scp_get_sys_login_request(struct trans *trans, * @param login_result What happened to the login * @param server_closed If login fails, whether server has closed connection. * If not, a retry can be made. + * @param uid UID for a successful login * @return != 0 for error */ int scp_send_login_response(struct trans *trans, enum scp_login_status login_result, - int server_closed); + int server_closed, + int uid); /** * Parses an incoming E_SCP_LOGIN_RESPONSE (SCP client) @@ -298,12 +300,17 @@ scp_send_login_response(struct trans *trans, * @param[out] login_result 0 for success, PAM error code otherwise * @param[out] server_closed If login fails, whether server has closed * connection. If not a retry can be made. + * @param[out] uid UID for a successful login + * + * server_closed and uid can be passed NULL if the caller isn't interested. + * * @return != 0 for error */ int scp_get_login_response(struct trans *trans, enum scp_login_status *login_result, - int *server_closed); + int *server_closed, + int *uid); /** * Send an E_SCP_LOGOUT_REQUEST (SCP client) diff --git a/sesman/Makefile.am b/sesman/Makefile.am index 542186d87..638ca94ec 100644 --- a/sesman/Makefile.am +++ b/sesman/Makefile.am @@ -6,7 +6,7 @@ AM_CPPFLAGS = \ -DXRDP_SBIN_PATH=\"${sbindir}\" \ -DXRDP_LIBEXEC_PATH=\"${libexecdir}/xrdp\" \ -DXRDP_PID_PATH=\"${localstatedir}/run\" \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -DSESMAN_RUNTIME_PATH=\"${sesmanruntimedir}\" \ -I$(top_srcdir)/sesman/libsesman \ -I$(top_srcdir)/common \ diff --git a/sesman/chansrv/Makefile.am b/sesman/chansrv/Makefile.am index 75eee8d78..959cdb52f 100644 --- a/sesman/chansrv/Makefile.am +++ b/sesman/chansrv/Makefile.am @@ -8,7 +8,7 @@ AM_CPPFLAGS = \ -DXRDP_SBIN_PATH=\"${sbindir}\" \ -DXRDP_SHARE_PATH=\"${datadir}/xrdp\" \ -DXRDP_PID_PATH=\"${localstatedir}/run\" \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_srcdir)/sesman/libsesman \ -I$(top_srcdir)/common diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c index b83c1057f..31daff4c0 100644 --- a/sesman/chansrv/chansrv.c +++ b/sesman/chansrv/chansrv.c @@ -1238,7 +1238,7 @@ my_api_trans_conn_in(struct trans *trans, struct trans *new_trans) static int setup_listen(void) { - char port[256]; + char port[XRDP_SOCKETS_MAXPATH]; int error = 0; if (g_lis_trans != 0) @@ -1248,7 +1248,7 @@ setup_listen(void) g_lis_trans = trans_create(TRANS_MODE_UNIX, 8192, 8192); g_lis_trans->is_term = g_is_term; - g_snprintf(port, 255, XRDP_CHANSRV_STR, g_display_num); + g_snprintf(port, sizeof(port), XRDP_CHANSRV_STR, g_getuid(), g_display_num); g_lis_trans->trans_conn_in = my_trans_conn_in; error = trans_listen(g_lis_trans, port); @@ -1267,12 +1267,12 @@ setup_listen(void) static int setup_api_listen(void) { - char port[256]; + char port[XRDP_SOCKETS_MAXPATH]; int error = 0; g_api_lis_trans = trans_create(TRANS_MODE_UNIX, 8192 * 4, 8192 * 4); g_api_lis_trans->is_term = g_is_term; - g_snprintf(port, 255, CHANSRV_API_STR, g_display_num); + g_snprintf(port, sizeof(port), CHANSRV_API_STR, g_getuid(), g_display_num); g_api_lis_trans->trans_conn_in = my_api_trans_conn_in; error = trans_listen(g_api_lis_trans, port); diff --git a/sesman/chansrv/sound.c b/sesman/chansrv/sound.c index 7e03b598e..2c862f99b 100644 --- a/sesman/chansrv/sound.c +++ b/sesman/chansrv/sound.c @@ -1894,11 +1894,11 @@ sound_sndsrvr_source_data_in(struct trans *trans) static int sound_start_source_listener(void) { - char port[1024]; + char port[XRDP_SOCKETS_MAXPATH]; g_audio_l_trans_in = trans_create(TRANS_MODE_UNIX, 128 * 1024, 8192); g_audio_l_trans_in->is_term = g_is_term; - g_snprintf(port, 255, CHANSRV_PORT_IN_STR, g_display_num); + g_snprintf(port, sizeof(port), CHANSRV_PORT_IN_STR, g_getuid(), g_display_num); g_audio_l_trans_in->trans_conn_in = sound_sndsrvr_source_conn_in; if (trans_listen(g_audio_l_trans_in, port) != 0) { @@ -1913,11 +1913,11 @@ sound_start_source_listener(void) static int sound_start_sink_listener(void) { - char port[1024]; + char port[XRDP_SOCKETS_MAXPATH]; g_audio_l_trans_out = trans_create(TRANS_MODE_UNIX, 128 * 1024, 8192); g_audio_l_trans_out->is_term = g_is_term; - g_snprintf(port, 255, CHANSRV_PORT_OUT_STR, g_display_num); + g_snprintf(port, sizeof(port), CHANSRV_PORT_OUT_STR, g_getuid(), g_display_num); g_audio_l_trans_out->trans_conn_in = sound_sndsrvr_sink_conn_in; if (trans_listen(g_audio_l_trans_out, port) != 0) { diff --git a/sesman/libsesman/sesman_config.c b/sesman/libsesman/sesman_config.c index e47a85499..9e3d9e728 100644 --- a/sesman/libsesman/sesman_config.c +++ b/sesman/libsesman/sesman_config.c @@ -71,6 +71,7 @@ #define SESMAN_CFG_SEC_RESTRICT_INBOUND_CLIPBOARD "RestrictInboundClipboard" #define SESMAN_CFG_SEC_ALLOW_ALTERNATE_SHELL "AllowAlternateShell" #define SESMAN_CFG_SEC_XORG_NO_NEW_PRIVILEGES "XorgNoNewPrivileges" +#define SESMAN_CFG_SEC_SESSION_SOCKDIR_GROUP "SessionSockdirGroup" #define SESMAN_CFG_SESSIONS "Sessions" #define SESMAN_CFG_SESS_MAX "MaxSessions" @@ -312,6 +313,7 @@ config_read_security(int file, struct config_security *sc, sc->xorg_no_new_privileges = 1; sc->ts_users = g_strdup(""); sc->ts_admins = g_strdup(""); + sc->session_sockdir_group = g_strdup(""); file_read_section(file, SESMAN_CFG_SECURITY, param_n, param_v); @@ -324,29 +326,25 @@ config_read_security(int file, struct config_security *sc, { sc->allow_root = g_text2bool(value); } - - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_LOGIN_RETRY)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_LOGIN_RETRY)) { sc->login_retry = g_atoi(value); } - - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_USR_GROUP)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_USR_GROUP)) { g_free(sc->ts_users); sc->ts_users = g_strdup(value); } - - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_ADM_GROUP)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_ADM_GROUP)) { g_free(sc->ts_admins); sc->ts_admins = g_strdup(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_ALWAYSGROUPCHECK)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_ALWAYSGROUPCHECK)) { sc->ts_always_group_check = g_text2bool(value); } - - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_RESTRICT_OUTBOUND_CLIPBOARD)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_RESTRICT_OUTBOUND_CLIPBOARD)) { char unrecognised[256]; sc->restrict_outbound_clipboard = @@ -360,7 +358,7 @@ config_read_security(int file, struct config_security *sc, unrecognised); } } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_RESTRICT_INBOUND_CLIPBOARD)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_RESTRICT_INBOUND_CLIPBOARD)) { char unrecognised[256]; sc->restrict_inbound_clipboard = @@ -374,17 +372,21 @@ config_read_security(int file, struct config_security *sc, unrecognised); } } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_ALLOW_ALTERNATE_SHELL)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_ALLOW_ALTERNATE_SHELL)) { sc->allow_alternate_shell = g_text2bool(value); } - - if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_XORG_NO_NEW_PRIVILEGES)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_XORG_NO_NEW_PRIVILEGES)) { sc->xorg_no_new_privileges = g_text2bool(value); } + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SEC_SESSION_SOCKDIR_GROUP)) + { + g_free(sc->session_sockdir_group); + sc->session_sockdir_group = g_strdup(value); + } } return 0; @@ -684,6 +686,7 @@ config_dump(struct config_sesman *config) g_writeln(" RestrictInboundClipboard: %s", restrict_s); g_writeln(" TSUsersGroup: %s", sc->ts_users); g_writeln(" TSAdminsGroup: %s", sc->ts_admins); + g_writeln(" SessionSockdirGroup: %s", sc->session_sockdir_group); /* Xorg */ @@ -741,6 +744,7 @@ config_free(struct config_sesman *cs) list_delete(cs->env_values); g_free(cs->sec.ts_users); g_free(cs->sec.ts_admins); + g_free(cs->sec.session_sockdir_group); g_free(cs); } } diff --git a/sesman/libsesman/sesman_config.h b/sesman/libsesman/sesman_config.h index 9dd6672b2..5f403fa15 100644 --- a/sesman/libsesman/sesman_config.h +++ b/sesman/libsesman/sesman_config.h @@ -107,6 +107,12 @@ struct config_security * @brief if the Xorg X11 server should be started with no_new_privs (Linux only) */ int xorg_no_new_privileges; + + /* + * @var session_sockdir_group + * @brief Group to have read access to the session sockdirs + */ + char *session_sockdir_group; }; /** diff --git a/sesman/scp_process.c b/sesman/scp_process.c index 7fab3d46e..3f0c36307 100644 --- a/sesman/scp_process.c +++ b/sesman/scp_process.c @@ -44,6 +44,7 @@ #include "session_list.h" #include "sesexec_control.h" #include "string_calls.h" +#include "xrdp_sockets.h" /******************************************************************************/ @@ -139,7 +140,7 @@ process_sys_login_request(struct pre_session_item *psi) { /* We only get here if something has gone * wrong with the handover to sesexec */ - rv = scp_send_login_response(psi->client_trans, errorcode, 1); + rv = scp_send_login_response(psi->client_trans, errorcode, 1, -1); psi->dispatcher_action = E_PSD_TERMINATE_PRE_SESSION; } } @@ -219,15 +220,15 @@ process_uds_login_request(struct pre_session_item *psi) int uid; int pid; char *username = NULL; - int server_closed = 1; + int server_closed; rv = g_sck_get_peer_cred(psi->client_trans->sck, &pid, &uid, NULL); if (rv != 0) { + errorcode = E_SCP_LOGIN_GENERAL_ERROR; LOG(LOG_LEVEL_INFO, "Unable to get peer credentials for socket %d", (int)psi->client_trans->sck); - errorcode = E_SCP_LOGIN_GENERAL_ERROR; } else { @@ -252,21 +253,26 @@ process_uds_login_request(struct pre_session_item *psi) errorcode = authenticate_and_authorize_uds_connection( psi, uid, username); g_free(username); - - if (errorcode == E_SCP_LOGIN_OK) - { - server_closed = 0; - } } } - if (server_closed) + if (errorcode == E_SCP_LOGIN_OK) { + server_closed = 0; + } + else + { + server_closed = 1; + /* Close the connection after returning from this callback */ psi->dispatcher_action = E_PSD_TERMINATE_PRE_SESSION; + + /* Never return the UID if the server is closing */ + uid = -1; } - return scp_send_login_response(psi->client_trans, errorcode, server_closed); + return scp_send_login_response(psi->client_trans, errorcode, + server_closed, uid); } /******************************************************************************/ @@ -302,6 +308,58 @@ process_logout_request(struct pre_session_item *psi) return 0; } +/******************************************************************************/ +/** + * Create xrdp socket path for the user + * + * We do this here rather than in sesexec as we're single-threaded here + * and so don't have to worry about race conditions + * + * Directory is owned by UID of session, but can be accessed by + * the group specified in the config. + * + * Errors are logged so the caller doesn't have to + */ +static int +create_xrdp_socket_path(uid_t uid) +{ + int rv = 1; + const char *sockdir_group = g_cfg->sec.session_sockdir_group; + int gid = 0; // Default if no group specified + + char sockdir[XRDP_SOCKETS_MAXPATH]; + g_snprintf(sockdir, sizeof(sockdir), XRDP_SOCKET_PATH, (int)uid); + + // Create directory permissions 0x750, if it doesn't exist already. + int old_umask = g_umask_hex(0x750 ^ 0x777); + if (!g_directory_exist(sockdir) && !g_create_dir(sockdir)) + { + LOG(LOG_LEVEL_ERROR, + "create_xrdp_socket_path: Can't create %s [%s]", + sockdir, g_get_strerror()); + } + else if (sockdir_group != NULL && sockdir_group[0] != '\0' && + g_getgroup_info(sockdir_group, &gid) != 0) + { + LOG(LOG_LEVEL_ERROR, + "create_xrdp_socket_path: Can't get GID of group %s [%s]", + sockdir_group, g_get_strerror()); + } + else if (g_chown(sockdir, uid, gid) != 0) + { + LOG(LOG_LEVEL_ERROR, + "create_xrdp_socket_path: Can't set owner of %s to %d:%d [%s]", + sockdir, uid, gid, g_get_strerror()); + } + else + { + rv = 0; + } + (void)g_umask_hex(old_umask); + + return rv; +} + /******************************************************************************/ static int @@ -385,6 +443,11 @@ process_create_session_request(struct pre_session_item *psi) { status = E_SCP_SCREATE_NO_MEMORY; } + // Create a socket dir for this user + else if (create_xrdp_socket_path(psi->uid) != 0) + { + status = E_SCP_SCREATE_GENERAL_ERROR; + } // Create a sesexec process if we don't have one (UDS login) else if (psi->sesexec_trans == NULL && sesexec_start(psi) != 0) { diff --git a/sesman/sesexec/Makefile.am b/sesman/sesexec/Makefile.am index f2bac24ab..b7d5abb60 100644 --- a/sesman/sesexec/Makefile.am +++ b/sesman/sesexec/Makefile.am @@ -2,7 +2,7 @@ AM_CPPFLAGS = \ -DXRDP_CFG_PATH=\"${sysconfdir}/xrdp\" \ -DXRDP_SBIN_PATH=\"${sbindir}\" \ -DXRDP_LIBEXEC_PATH=\"${libexecdir}/xrdp\" \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_srcdir)/sesman/libsesman \ -I$(top_srcdir)/libipm \ -I$(top_srcdir)/common diff --git a/sesman/sesexec/env.c b/sesman/sesexec/env.c index a7e556595..f56a93d44 100644 --- a/sesman/sesexec/env.c +++ b/sesman/sesexec/env.c @@ -152,9 +152,10 @@ env_set_user(int uid, char **passwd_file, int display, // Use our PID as the XRDP_SESSION value g_snprintf(text, sizeof(text), "%d", g_pid); g_setenv("XRDP_SESSION", text, 1); - /* XRDP_SOCKET_PATH should be set even here. It's used by + /* XRDP_SOCKET_PATH should be set here. It's used by * xorgxrdp and the pulseaudio plugin */ - g_setenv("XRDP_SOCKET_PATH", XRDP_SOCKET_PATH, 1); + g_snprintf(text, sizeof(text), XRDP_SOCKET_PATH, uid); + g_setenv("XRDP_SOCKET_PATH", text, 1); /* pulse sink socket */ g_snprintf(text, sizeof(text), CHANSRV_PORT_OUT_BASE_STR, display); g_setenv("XRDP_PULSE_SINK_SOCKET", text, 1); diff --git a/sesman/sesexec/login_info.c b/sesman/sesexec/login_info.c index a8ccda81b..adb8ef1ad 100644 --- a/sesman/sesexec/login_info.c +++ b/sesman/sesexec/login_info.c @@ -280,7 +280,8 @@ login_info_sys_login_user(struct trans *scp_trans, } } - if (scp_send_login_response(scp_trans, status, server_closed) != 0) + if (scp_send_login_response(scp_trans, status, + server_closed, result->uid) != 0) { status = E_SCP_LOGIN_GENERAL_ERROR; break; diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 900b39433..a49509732 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -370,8 +370,6 @@ prepare_xorg_xserver_params(const struct session_parameters *s, g_snprintf(text, sizeof(text), "%d", g_cfg->sess.kill_disconnected); g_setenv("XRDP_SESMAN_KILL_DISCONNECTED", text, 1); - g_setenv("XRDP_SOCKET_PATH", XRDP_SOCKET_PATH, 1); - /* get path of Xorg from config */ xserver = (const char *)list_get_item(g_cfg->xorg_params, 0); @@ -716,15 +714,14 @@ session_start(struct login_info *login_info, /******************************************************************************/ static int -cleanup_sockets(int display) +cleanup_sockets(int uid, int display) { - LOG(LOG_LEVEL_INFO, "cleanup_sockets:"); - char file[256]; - int error; + LOG_DEVEL(LOG_LEVEL_INFO, "cleanup_sockets:"); - error = 0; + char file[XRDP_SOCKETS_MAXPATH]; + int error = 0; - g_snprintf(file, 255, CHANSRV_PORT_OUT_STR, display); + g_snprintf(file, sizeof(file), CHANSRV_PORT_OUT_STR, uid, display); if (g_file_exist(file)) { LOG(LOG_LEVEL_DEBUG, "cleanup_sockets: deleting %s", file); @@ -737,7 +734,7 @@ cleanup_sockets(int display) } } - g_snprintf(file, 255, CHANSRV_PORT_IN_STR, display); + g_snprintf(file, sizeof(file), CHANSRV_PORT_IN_STR, uid, display); if (g_file_exist(file)) { LOG(LOG_LEVEL_DEBUG, "cleanup_sockets: deleting %s", file); @@ -750,7 +747,7 @@ cleanup_sockets(int display) } } - g_snprintf(file, 255, XRDP_CHANSRV_STR, display); + g_snprintf(file, sizeof(file), XRDP_CHANSRV_STR, uid, display); if (g_file_exist(file)) { LOG(LOG_LEVEL_DEBUG, "cleanup_sockets: deleting %s", file); @@ -763,7 +760,7 @@ cleanup_sockets(int display) } } - g_snprintf(file, 255, CHANSRV_API_STR, display); + g_snprintf(file, sizeof(file), CHANSRV_API_STR, uid, display); if (g_file_exist(file)) { LOG(LOG_LEVEL_DEBUG, "cleanup_sockets: deleting %s", file); @@ -779,7 +776,7 @@ cleanup_sockets(int display) /* the following files should be deleted by xorgxrdp * but just in case the deletion failed */ - g_snprintf(file, 255, XRDP_X11RDP_STR, display); + g_snprintf(file, sizeof(file), XRDP_X11RDP_STR, uid, display); if (g_file_exist(file)) { LOG(LOG_LEVEL_DEBUG, "cleanup_sockets: deleting %s", file); @@ -792,7 +789,7 @@ cleanup_sockets(int display) } } - g_snprintf(file, 255, XRDP_DISCONNECT_STR, display); + g_snprintf(file, sizeof(file), XRDP_DISCONNECT_STR, uid, display); if (g_file_exist(file)) { LOG(LOG_LEVEL_DEBUG, "cleanup_sockets: deleting %s", file); @@ -908,7 +905,7 @@ session_process_child_exit(struct session_data *sd, if (!session_active(sd)) { - cleanup_sockets(sd->params.display); + cleanup_sockets(g_login_info->uid, sd->params.display); } } diff --git a/sesman/sesman.c b/sesman/sesman.c index f5d0b9cf2..1c1cb6191 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -50,6 +50,7 @@ #include "string_calls.h" #include "trans.h" #include "xrdp_configure_options.h" +#include "xrdp_sockets.h" /** * Maximum number of pre-session items @@ -688,9 +689,6 @@ read_pid_file(const char *pid_file, int *pid) static int create_xrdp_socket_root_path(void) { -#ifndef XRDP_SOCKET_PATH -# error "XRDP_SOCKET_PATH must be defined" -#endif int uid = g_getuid(); int gid = g_getgid(); diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in index 01a1aa5b7..685af28b7 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -44,6 +44,12 @@ RestrictInboundClipboard=none ; however, interfere with the use of security modules such as AppArmor. ; Leave this unset unless you need to disable it. #XorgNoNewPrivileges=true +; Specify the group which is to have read access to the directory where +; local sockets for the session are created. This is normally the GID +; which the xrdp process runs as. +; Default is 'root' +#SessionSockdirGroup=root + [Sessions] ;; X11DisplayOffset - x11 display number offset diff --git a/sesman/session_list.c b/sesman/session_list.c index 497c6eb86..a3a333dda 100644 --- a/sesman/session_list.c +++ b/sesman/session_list.c @@ -198,41 +198,6 @@ x_server_running_check_ports(int display) } } - if (!x_running) - { - LOG(LOG_LEVEL_DEBUG, "Did not find a running X server at %s", text); - g_sprintf(text, XRDP_CHANSRV_STR, display); - x_running = g_file_exist(text); - } - - if (!x_running) - { - LOG(LOG_LEVEL_DEBUG, "Did not find a running X server at %s", text); - g_sprintf(text, CHANSRV_PORT_OUT_STR, display); - x_running = g_file_exist(text); - } - - if (!x_running) - { - LOG(LOG_LEVEL_DEBUG, "Did not find a running X server at %s", text); - g_sprintf(text, CHANSRV_PORT_IN_STR, display); - x_running = g_file_exist(text); - } - - if (!x_running) - { - LOG(LOG_LEVEL_DEBUG, "Did not find a running X server at %s", text); - g_sprintf(text, CHANSRV_API_STR, display); - x_running = g_file_exist(text); - } - - if (!x_running) - { - LOG(LOG_LEVEL_DEBUG, "Did not find a running X server at %s", text); - g_sprintf(text, XRDP_X11RDP_STR, display); - x_running = g_file_exist(text); - } - if (x_running) { LOG(LOG_LEVEL_INFO, "Found X server running at %s", text); diff --git a/sesman/tools/Makefile.am b/sesman/tools/Makefile.am index a6803bb3f..73f1a3b96 100644 --- a/sesman/tools/Makefile.am +++ b/sesman/tools/Makefile.am @@ -1,6 +1,6 @@ AM_CPPFLAGS = \ -DXRDP_CFG_PATH=\"${sysconfdir}/xrdp\" \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_srcdir)/sesman/libsesman \ -I$(top_srcdir)/common \ -I$(top_srcdir)/libipm diff --git a/sesman/tools/dis.c b/sesman/tools/dis.c index cc06e8707..2e744ef71 100644 --- a/sesman/tools/dis.c +++ b/sesman/tools/dis.c @@ -29,6 +29,7 @@ #include #include "xrdp_sockets.h" +#include "os_calls.h" #include "string_calls.h" int main(int argc, char **argv) @@ -62,7 +63,7 @@ int main(int argc, char **argv) } memset(&sa, 0, sizeof(sa)); sa.sun_family = AF_UNIX; - sprintf(sa.sun_path, XRDP_DISCONNECT_STR, dis); + sprintf(sa.sun_path, XRDP_DISCONNECT_STR, g_getuid(), dis); if (access(sa.sun_path, F_OK) != 0) { diff --git a/sesman/tools/sesadmin.c b/sesman/tools/sesadmin.c index 8c9157bac..e41c8fed4 100644 --- a/sesman/tools/sesadmin.c +++ b/sesman/tools/sesadmin.c @@ -95,7 +95,7 @@ int main(int argc, char **argv) if ((rv = scp_send_uds_login_request(t)) == 0 && (rv = wait_for_sesman_reply(t, E_SCP_LOGIN_RESPONSE)) == 0) { - rv = scp_get_login_response(t, &login_result, NULL); + rv = scp_get_login_response(t, &login_result, NULL, NULL); if (rv == 0) { if (login_result != E_SCP_LOGIN_OK) diff --git a/sesman/tools/sesrun.c b/sesman/tools/sesrun.c index e326b0735..affc78356 100644 --- a/sesman/tools/sesrun.c +++ b/sesman/tools/sesrun.c @@ -460,7 +460,7 @@ handle_login_response(struct trans *t, int *server_closed) } else { - rv = scp_get_login_response(t, &login_result, server_closed); + rv = scp_get_login_response(t, &login_result, server_closed, NULL); if (rv == 0) { if (login_result != E_SCP_LOGIN_OK) diff --git a/xrdp/Makefile.am b/xrdp/Makefile.am index 432fdf2da..3e11f4eeb 100644 --- a/xrdp/Makefile.am +++ b/xrdp/Makefile.am @@ -9,7 +9,7 @@ AM_CPPFLAGS = \ -DXRDP_SHARE_PATH=\"${datadir}/xrdp\" \ -DXRDP_PID_PATH=\"${localstatedir}/run\" \ -DXRDP_MODULE_PATH=\"${moduledir}\" \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_builddir) \ -I$(top_srcdir)/common \ -I$(top_srcdir)/libipm \ diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 04e646a80..3f1db0afe 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -61,6 +61,8 @@ xrdp_mm_create(struct xrdp_wm *owner) self->resize_queue = list_create(); self->resize_queue->auto_free = 1; + self->uid = -1; /* Never good to default UIDs to 0 */ + pid = g_getpid(); /* setup wait objects for signalling */ g_snprintf(buf, sizeof(buf), "xrdp_%8.8x_resize_ready", pid); @@ -469,11 +471,12 @@ xrdp_mm_setup_mod2(struct xrdp_mm *self) { if (self->code == XVNC_SESSION_CODE) { - g_snprintf(text, 255, "%d", 5900 + self->display); + g_snprintf(text, sizeof(text), "%d", 5900 + self->display); } else if (self->code == XORG_SESSION_CODE) { - g_snprintf(text, 255, XRDP_X11RDP_STR, self->display); + g_snprintf(text, sizeof(text), XRDP_X11RDP_STR, + self->uid, self->display); } else { @@ -2178,7 +2181,8 @@ xrdp_mm_process_login_response(struct xrdp_mm *self) self->mmcs_expecting_msg = 0; - rv = scp_get_login_response(self->sesman_trans, &login_result, &server_closed); + rv = scp_get_login_response(self->sesman_trans, &login_result, + &server_closed, &self->uid); if (rv == 0) { if (login_result != E_SCP_LOGIN_OK) @@ -2346,11 +2350,16 @@ cleanup_states(struct xrdp_mm *self) * @param value assigned to chansrvport * @param dest Output buffer * @param dest_size Total size of output buffer, including terminator space + * @param uid of destination + * + * Pass in dest of NULL, dest_size of 0 and uid of -1 to simply see if + * the string parses OK. + * * @return 0 for success */ static int -parse_chansrvport(const char *value, char *dest, int dest_size) +parse_chansrvport(const char *value, char *dest, int dest_size, int uid) { int rv = 0; @@ -2373,7 +2382,7 @@ parse_chansrvport(const char *value, char *dest, int dest_size) } else { - g_snprintf(dest, dest_size, XRDP_CHANSRV_STR, g_atoi(p)); + g_snprintf(dest, dest_size, XRDP_CHANSRV_STR, uid, g_atoi(p)); } } else @@ -2561,7 +2570,7 @@ xrdp_mm_connect(struct xrdp_mm *self) { const char *csp = xrdp_mm_get_value(self, "chansrvport"); /* It's defined, but is it a valid string? */ - if (csp != NULL && parse_chansrvport(csp, NULL, 0) == 0) + if (csp != NULL && parse_chansrvport(csp, NULL, 0, -1) == 0) { self->use_chansrv = 1; } @@ -2652,6 +2661,7 @@ xrdp_mm_connect_sm(struct xrdp_mm *self) "access control check was successful"); // No reply needed for this one status = scp_send_logout_request(self->sesman_trans); + self->uid = -1; } if (status == 0 && self->use_sesman) @@ -2723,20 +2733,21 @@ xrdp_mm_connect_sm(struct xrdp_mm *self) { if (self->use_chansrv) { - char portbuff[256]; + char portbuff[XRDP_SOCKETS_MAXPATH]; xrdp_wm_log_msg(self->wm, LOG_LEVEL_INFO, "Connecting to chansrv"); if (self->use_sesman) { g_snprintf(portbuff, sizeof(portbuff), - XRDP_CHANSRV_STR, self->display); + XRDP_CHANSRV_STR, self->uid, self->display); } else { const char *cp = xrdp_mm_get_value(self, "chansrvport"); portbuff[0] = '\0'; - parse_chansrvport(cp, portbuff, sizeof(portbuff)); + parse_chansrvport(cp, portbuff, sizeof(portbuff), + self->uid); } xrdp_mm_update_allowed_channels(self); diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index e379ff504..58af848c2 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -401,6 +401,7 @@ struct xrdp_mm int (*mod_exit)(struct xrdp_mod *); struct xrdp_mod *mod; /* module interface */ int display; /* 10 for :10.0, 11 for :11.0, etc */ + int uid; /* UID for a successful login, -1 otherwise */ struct guid guid; /* GUID for the session, or all zeros */ int code; /* 0=Xvnc session, 20=xorg driver mode */ struct xrdp_encoder *encoder; diff --git a/xrdpapi/Makefile.am b/xrdpapi/Makefile.am index 55014bc5f..266b07a52 100644 --- a/xrdpapi/Makefile.am +++ b/xrdpapi/Makefile.am @@ -4,7 +4,7 @@ EXTRA_DIST = \ vrplayer.mk AM_CPPFLAGS = \ - -DXRDP_SOCKET_PATH=\"${socketdir}\" \ + -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_srcdir)/common module_LTLIBRARIES = \ diff --git a/xrdpapi/xrdpapi.c b/xrdpapi/xrdpapi.c index 1b72339e5..4288b3552 100644 --- a/xrdpapi/xrdpapi.c +++ b/xrdpapi/xrdpapi.c @@ -140,7 +140,7 @@ WTSVirtualChannelOpenEx(unsigned int SessionId, const char *pVirtualName, memset(&s, 0, sizeof(struct sockaddr_un)); s.sun_family = AF_UNIX; bytes = sizeof(s.sun_path); - snprintf(s.sun_path, bytes - 1, CHANSRV_API_STR, wts->display_num); + snprintf(s.sun_path, bytes - 1, CHANSRV_API_STR, getuid(), wts->display_num); s.sun_path[bytes - 1] = 0; bytes = sizeof(struct sockaddr_un); From c51ec2e8e9691653ec17b20deeff3bce98008068 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 14 Jun 2023 14:23:07 +0100 Subject: [PATCH 4/4] Remove sesmanruntimedir Now we've made the XRDP_SOCKET_PATH only writeable by root, it's safe to move the sesman socket back into this directory. We no longer need a separate sesmanruntimedir --- configure.ac | 7 ----- docs/man/Makefile.am | 1 - docs/man/sesman.ini.5.in | 2 +- docs/man/xrdp-sesadmin.8.in | 2 +- docs/man/xrdp-sesman.8.in | 2 +- libipm/Makefile.am | 1 - libipm/scp.c | 23 +++++++++++++-- sesman/Makefile.am | 1 - sesman/sesman.c | 56 +++++-------------------------------- 9 files changed, 31 insertions(+), 64 deletions(-) diff --git a/configure.ac b/configure.ac index 2f3d69384..adabcc6bc 100644 --- a/configure.ac +++ b/configure.ac @@ -62,12 +62,6 @@ AC_ARG_WITH([socketdir], [], [with_socketdir="$runstatedir/xrdp"]) AC_SUBST([socketdir], [$with_socketdir]) -AC_ARG_WITH([sesmanruntimedir], - [AS_HELP_STRING([--with-sesmanruntimedir=DIR], - [Use directory for sesman runtime data (default: RUNSTATEDIR/xrdp-sesman)])], - [], [with_sesmanruntimedir="$runstatedir/xrdp-sesman"]) -AC_SUBST([sesmanruntimedir], [$with_sesmanruntimedir]) - AC_ARG_WITH([systemdsystemunitdir], AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd service files, no to disable]), [], [ @@ -655,7 +649,6 @@ echo " pamconfdir $pamconfdir" echo " localstatedir $localstatedir" echo " runstatedir $runstatedir" echo " socketdir $socketdir" -echo " sesmanruntimedir $sesmanruntimedir" echo "" echo " unit tests performable $perform_unit_tests" echo "" diff --git a/docs/man/Makefile.am b/docs/man/Makefile.am index 8da6cd967..8dbd71422 100644 --- a/docs/man/Makefile.am +++ b/docs/man/Makefile.am @@ -27,7 +27,6 @@ SUBST_VARS = sed \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ -e 's|@sysconfdir[@]|$(sysconfdir)|g' \ -e 's|@socketdir[@]|$(socketdir)|g' \ - -e 's|@sesmanruntimedir[@]|$(sesmanruntimedir)|g' \ -e 's|@xrdpconfdir[@]|$(sysconfdir)/xrdp|g' \ -e 's|@xrdpdatadir[@]|$(datadir)/xrdp|g' \ -e 's|@xrdphomeurl[@]|http://www.xrdp.org/|g' diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index 0d4f5c907..d2ca98324 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -57,7 +57,7 @@ In this instance, the system administrator is responsible for ensuring the socket can only be created by a suitably privileged process. .PP If the parameter does not start with a '/', a name within -@sesmanruntimedir@ is used. +@socketdir@ is used. .RE .TP diff --git a/docs/man/xrdp-sesadmin.8.in b/docs/man/xrdp-sesadmin.8.in index 953f3f672..cba29a37e 100644 --- a/docs/man/xrdp-sesadmin.8.in +++ b/docs/man/xrdp-sesadmin.8.in @@ -28,7 +28,7 @@ Retained for compatibility, but ignored. .BI \-i= port The sesman \fIUNIX domain socket\fP to connect to. Defaults to \fBsesman.socket\fP. -If no path is specified for the socket, a default of @sesmanruntimedir@ is used. +If no path is specified for the socket, a default of @socketdir@ is used. .TP .BI \-c= command diff --git a/docs/man/xrdp-sesman.8.in b/docs/man/xrdp-sesman.8.in index e676548e9..379b92b56 100644 --- a/docs/man/xrdp-sesman.8.in +++ b/docs/man/xrdp-sesman.8.in @@ -70,7 +70,7 @@ not running \fBxrdp\-sesman\fR as a daemon. .br @localstatedir@/run/xrdp\-sesman.pid .br -@sesmanruntimedir@/sesman.socket +@socketdir@/sesman.socket .SH "AUTHORS" Jay Sorg diff --git a/libipm/Makefile.am b/libipm/Makefile.am index 4770f4dd1..a1a6edcda 100644 --- a/libipm/Makefile.am +++ b/libipm/Makefile.am @@ -1,6 +1,5 @@ AM_CPPFLAGS = \ - -DSESMAN_RUNTIME_PATH=\"${sesmanruntimedir}\" \ -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ -I$(top_srcdir)/common diff --git a/libipm/scp.c b/libipm/scp.c index eaeac344a..c090bc500 100644 --- a/libipm/scp.c +++ b/libipm/scp.c @@ -27,6 +27,8 @@ #include #endif +#include + #include "scp.h" #include "libipm.h" #include "guid.h" @@ -76,6 +78,23 @@ scp_msgno_to_str(enum scp_msg_code n, char *buff, unsigned int buff_size) return buff; } +/*****************************************************************************/ +/** + * Helper function returning 1 if the passed-in string is an integer >= 0 + */ +static int is_positive_int(const char *s) +{ + for ( ; *s != '\0' ; ++s) + { + if (!isdigit(*s)) + { + return 0; + } + } + + return 1; +} + /*****************************************************************************/ int scp_port_to_unix_domain_path(const char *port, char *buff, @@ -111,7 +130,7 @@ scp_port_to_unix_domain_path(const char *port, char *buff, { port = SCP_LISTEN_PORT_BASE_STR; } - else if (g_strcmp(port, "3350") == 0) + else if (is_positive_int(port)) { /* Version v0.9.x and earlier of xrdp used a TCP port * number. If we come across this, we'll ignore it for @@ -121,7 +140,7 @@ scp_port_to_unix_domain_path(const char *port, char *buff, port = SCP_LISTEN_PORT_BASE_STR; } - result = g_snprintf(buff, bufflen, SESMAN_RUNTIME_PATH "/%s", port); + result = g_snprintf(buff, bufflen, XRDP_SOCKET_ROOT_PATH "/%s", port); } return result; diff --git a/sesman/Makefile.am b/sesman/Makefile.am index 638ca94ec..7b34470c5 100644 --- a/sesman/Makefile.am +++ b/sesman/Makefile.am @@ -7,7 +7,6 @@ AM_CPPFLAGS = \ -DXRDP_LIBEXEC_PATH=\"${libexecdir}/xrdp\" \ -DXRDP_PID_PATH=\"${localstatedir}/run\" \ -DXRDP_SOCKET_ROOT_PATH=\"${socketdir}\" \ - -DSESMAN_RUNTIME_PATH=\"${sesmanruntimedir}\" \ -I$(top_srcdir)/sesman/libsesman \ -I$(top_srcdir)/common \ -I$(top_srcdir)/libipm diff --git a/sesman/sesman.c b/sesman/sesman.c index 1c1cb6191..21409c192 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -193,45 +193,6 @@ sesman_process_params(int argc, char **argv, return 0; } -/******************************************************************************/ -static int -create_sesman_runtime_dir(void) -{ - int rv = -1; - /* Make sure if we create the directory, there's no gap where it - * may have the wrong permissions */ - int entry_umask = g_umask_hex(0x755); - - if (!g_directory_exist(SESMAN_RUNTIME_PATH) && - !g_create_dir(SESMAN_RUNTIME_PATH)) - { - LOG(LOG_LEVEL_ERROR, - "Can't create runtime directory '" - SESMAN_RUNTIME_PATH "' [%s]", g_get_strerror()); - } - else if (g_chown(SESMAN_RUNTIME_PATH, g_getuid(), g_getuid()) != 0) - { - LOG(LOG_LEVEL_ERROR, - "Can't set ownership of sesman runtime directory [%s]", - g_get_strerror()); - } - else if (g_chmod_hex(SESMAN_RUNTIME_PATH, 0x755) != 0) - { - /* This might seem redundant, but there's a chance the - * directory already exists */ - LOG(LOG_LEVEL_ERROR, - "Can't set permissions of sesman runtime directory [%s]", - g_get_strerror()); - } - else - { - rv = 0; - } - g_umask_hex(entry_umask); - - return rv; -} - /******************************************************************************/ static int sesman_listen_test(struct config_sesman *cfg) { @@ -694,24 +655,24 @@ create_xrdp_socket_root_path(void) /* Create the path using 0755 permissions */ int old_umask = g_umask_hex(0x22); - (void)g_create_path(XRDP_SOCKET_PATH"/"); + (void)g_create_path(XRDP_SOCKET_ROOT_PATH"/"); (void)g_umask_hex(old_umask); /* Check the ownership and permissions on the last path element * are as expected */ - if (g_chown(XRDP_SOCKET_PATH, uid, gid) != 0) + if (g_chown(XRDP_SOCKET_ROOT_PATH, uid, gid) != 0) { LOG(LOG_LEVEL_ERROR, "create_xrdp_socket_root_path: Can't set owner of %s to %d:%d", - XRDP_SOCKET_PATH, uid, gid); + XRDP_SOCKET_ROOT_PATH, uid, gid); return 1; } - if (g_chmod_hex(XRDP_SOCKET_PATH, 0x755) != 0) + if (g_chmod_hex(XRDP_SOCKET_ROOT_PATH, 0x755) != 0) { LOG(LOG_LEVEL_ERROR, "create_xrdp_socket_root_path: Can't set perms of %s to 0x755", - XRDP_SOCKET_PATH); + XRDP_SOCKET_ROOT_PATH); return 1; } @@ -887,9 +848,9 @@ main(int argc, char **argv) } } - /* Create the runtime directory before we try to listen (or + /* Create the socket directory before we try to listen (or * test-listen), so there's somewhere for the default socket to live */ - if (create_sesman_runtime_dir() != 0) + if (create_xrdp_socket_root_path() != 0) { config_free(g_cfg); log_end(); @@ -963,9 +924,6 @@ main(int argc, char **argv) LOG(LOG_LEVEL_INFO, "starting xrdp-sesman with pid %d", g_pid); - /* make sure the socket directory exists */ - create_xrdp_socket_root_path(); - /* make sure the /tmp/.X11-unix directory exists */ if (!g_directory_exist("/tmp/.X11-unix")) {