From e50c6bab52476c8cec9c3a2a266969c2d9a9a108 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 4 Dec 2024 10:12:53 +0000 Subject: [PATCH 01/19] Remove g_time1() call This is not year 2038 compliant on systems with 32-bit integers. The call can be replaced with the standard C time() call. On POSIX systems, time_t is guaranteed to be an integer type. --- common/os_calls.c | 29 ----------------------------- common/os_calls.h | 1 - sesman/chansrv/clipboard_file.c | 2 +- sesman/libsesman/verify_user.c | 6 +++--- sesman/sesexec/login_info.c | 4 ++-- sesman/sesexec/session.c | 4 ++-- 6 files changed, 8 insertions(+), 38 deletions(-) diff --git a/common/os_calls.c b/common/os_calls.c index afd5a95858..d81cf0f126 100644 --- a/common/os_calls.c +++ b/common/os_calls.c @@ -2045,18 +2045,6 @@ g_obj_wait(tintptr *read_objs, int rcount, tintptr *write_objs, int wcount, void g_random(char *data, int len) { -#if defined(_WIN32) - int index; - - srand(g_time1()); - - for (index = 0; index < len; index++) - { - data[index] = (char)rand(); /* rand returns a number between 0 and - RAND_MAX */ - } - -#else int fd; memset(data, 0x44, len); @@ -2075,8 +2063,6 @@ g_random(char *data, int len) close(fd); } - -#endif } /*****************************************************************************/ @@ -3777,21 +3763,6 @@ g_check_user_in_group(const char *username, int gid, int *ok) } #endif // HAVE_GETGROUPLIST -/*****************************************************************************/ -/* returns the time since the Epoch (00:00:00 UTC, January 1, 1970), - measured in seconds. - for windows, returns the number of seconds since the machine was - started. */ -int -g_time1(void) -{ -#if defined(_WIN32) - return GetTickCount() / 1000; -#else - return time(0); -#endif -} - /*****************************************************************************/ /* returns the number of milliseconds since the machine was started. */ diff --git a/common/os_calls.h b/common/os_calls.h index be06b07b45..8de07bf356 100644 --- a/common/os_calls.h +++ b/common/os_calls.h @@ -393,7 +393,6 @@ int g_getgroup_info(const char *groupname, int *gid); * Primary group of username is also checked */ int g_check_user_in_group(const char *username, int gid, int *ok); -int g_time1(void); int g_time2(void); int g_time3(void); int g_save_to_bmp(const char *filename, char *data, int stride_bytes, diff --git a/sesman/chansrv/clipboard_file.c b/sesman/chansrv/clipboard_file.c index 90fed048af..0be2f6e68e 100644 --- a/sesman/chansrv/clipboard_file.c +++ b/sesman/chansrv/clipboard_file.c @@ -274,7 +274,7 @@ clipboard_get_file(const char *file, int bytes) list_add_item(g_files_list, (tintptr)cfi); cfi->size = g_file_get_size(full_fn); cfi->flags = CB_FILE_ATTRIBUTE_ARCHIVE; - cfi->time = (g_time1() + CB_EPOCH_DIFF) * 10000000LL; + cfi->time = (time(NULL) + CB_EPOCH_DIFF) * 10000000LL; LOG_DEVEL(LOG_LEVEL_DEBUG, "ok filename [%s] pathname [%s] size [%d]", cfi->filename, cfi->pathname, cfi->size); result = 0; diff --git a/sesman/libsesman/verify_user.c b/sesman/libsesman/verify_user.c index ccd289d238..fad83b422a 100644 --- a/sesman/libsesman/verify_user.c +++ b/sesman/libsesman/verify_user.c @@ -225,7 +225,7 @@ auth_check_pwd_chg(const char *user) } /* check if we need a pwd change */ - now = g_time1(); + now = time(NULL); today = now / SECS_PER_DAY; if (stp->sp_expire == -1) @@ -306,7 +306,7 @@ auth_change_pwd(const char *user, const char *newpwd) } stp->sp_pwdp = g_strdup(hash); - today = g_time1() / SECS_PER_DAY; + today = time(NULL) / SECS_PER_DAY; stp->sp_lstchg = today; stp->sp_expire = today + stp->sp_max + stp->sp_inact; fd = fopen("/etc/shadow", "rw"); @@ -377,7 +377,7 @@ auth_account_disabled(struct spwd *stp) return 1; } - today = g_time1() / SECS_PER_DAY; + today = time(NULL) / SECS_PER_DAY; LOG_DEVEL(LOG_LEVEL_DEBUG, "last %ld", stp->sp_lstchg); LOG_DEVEL(LOG_LEVEL_DEBUG, "min %ld", stp->sp_min); diff --git a/sesman/sesexec/login_info.c b/sesman/sesexec/login_info.c index adb8ef1add..bb9d3f9eb7 100644 --- a/sesman/sesexec/login_info.c +++ b/sesman/sesexec/login_info.c @@ -57,8 +57,8 @@ log_authfail_message(const char *username, const char *ip_addr) { ip_addr = "unknown"; } - LOG(LOG_LEVEL_INFO, "AUTHFAIL: user=%s ip=%s time=%d", - username, ip_addr, g_time1()); + LOG(LOG_LEVEL_INFO, "AUTHFAIL: user=%s ip=%s time=%ld", + username, ip_addr, (long)time(NULL)); } /******************************************************************************/ diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 11ec979d95..13ac7f3596 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -674,7 +674,7 @@ session_start_wrapped(struct login_info *login_info, sd->win_mgr = window_manager_pid; sd->x_server = display_pid; sd->chansrv = chansrv_pid; - sd->start_time = g_time1(); + sd->start_time = time(NULL); status = E_SCP_SCREATE_OK; } } @@ -860,7 +860,7 @@ session_process_child_exit(struct session_data *sd, } else if (pid == sd->win_mgr) { - int wm_wait_time = g_time1() - sd->start_time; + int wm_wait_time = time(NULL) - sd->start_time; if (e->reason == E_PXR_STATUS_CODE && e->val == 0) { From 7bf35a5ce27d662546cc700936ed3c618ee36cb2 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 4 Dec 2024 10:21:09 +0000 Subject: [PATCH 02/19] Remove g_time2() call This is currently unused in xrdp --- common/os_calls.c | 17 ----------------- common/os_calls.h | 1 - sesman/chansrv/xcommon.c | 4 +--- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/common/os_calls.c b/common/os_calls.c index d81cf0f126..5d65fd4caf 100644 --- a/common/os_calls.c +++ b/common/os_calls.c @@ -3763,23 +3763,6 @@ g_check_user_in_group(const char *username, int gid, int *ok) } #endif // HAVE_GETGROUPLIST -/*****************************************************************************/ -/* returns the number of milliseconds since the machine was - started. */ -int -g_time2(void) -{ -#if defined(_WIN32) - return (int)GetTickCount(); -#else - struct tms tm; - clock_t num_ticks = 0; - g_memset(&tm, 0, sizeof(struct tms)); - num_ticks = times(&tm); - return (int)(num_ticks * 10); -#endif -} - /*****************************************************************************/ /* returns time in milliseconds, uses gettimeofday does not work in win32 */ diff --git a/common/os_calls.h b/common/os_calls.h index 8de07bf356..0ef5cdba06 100644 --- a/common/os_calls.h +++ b/common/os_calls.h @@ -393,7 +393,6 @@ int g_getgroup_info(const char *groupname, int *gid); * Primary group of username is also checked */ int g_check_user_in_group(const char *username, int gid, int *ok); -int g_time2(void); int g_time3(void); int g_save_to_bmp(const char *filename, char *data, int stride_bytes, int width, int height, int depth, int bits_per_pixel); diff --git a/sesman/chansrv/xcommon.c b/sesman/chansrv/xcommon.c index 2e444e76cc..5655176772 100644 --- a/sesman/chansrv/xcommon.c +++ b/sesman/chansrv/xcommon.c @@ -86,9 +86,7 @@ xcommon_fatal_handler(Display *dis) } /*****************************************************************************/ -/* returns time in milliseconds - this is like g_time2 in os_calls, but not milliseconds since machine was - up, something else +/* returns time in milliseconds since a point in the past this is a time value similar to what the xserver uses */ int xcommon_get_local_time(void) From 52178eeaaa79ebf2fec892b74e8d0e15eabb8439 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 4 Dec 2024 11:13:29 +0000 Subject: [PATCH 03/19] Replace g_time3() with g_get_elapsed_ms() The function as specified used gettimeofday() which is susceptible to manual time changes, and is obsoleted in POSIX.1-2008. The replacement uses clock_gettime(CLOCK_MONOTONIC, ) which is not susceptible to manual time changes (at least on Linux) and cannot run backwards. Also, on systems with 32-bit integers, the value returned by this function wraps around every 49.7 days. To cope with a wraparound in a way compliant with the C standard, this value needs to return an unsigned integer type rather than a signed integer type. --- common/os_calls.c | 24 +++++++++++++----------- common/os_calls.h | 21 ++++++++++++++++++++- common/trans.c | 12 ++++++------ sesman/chansrv/chansrv.c | 6 +++--- sesman/chansrv/sound.c | 24 ++++++++++++------------ sesman/chansrv/xcommon.c | 9 --------- sesman/chansrv/xcommon.h | 2 -- xrdp/xrdp.h | 4 ++-- xrdp/xrdp_mm.c | 23 ++++++++++++----------- xrdp/xrdp_types.h | 2 +- 10 files changed, 69 insertions(+), 58 deletions(-) diff --git a/common/os_calls.c b/common/os_calls.c index 5d65fd4caf..03c57b1fd2 100644 --- a/common/os_calls.c +++ b/common/os_calls.c @@ -3764,19 +3764,21 @@ g_check_user_in_group(const char *username, int gid, int *ok) #endif // HAVE_GETGROUPLIST /*****************************************************************************/ -/* returns time in milliseconds, uses gettimeofday - does not work in win32 */ -int -g_time3(void) +unsigned int +g_get_elapsed_ms(void) { -#if defined(_WIN32) - return 0; -#else - struct timeval tp; + unsigned int result = 0; + struct timespec tp; - gettimeofday(&tp, 0); - return (tp.tv_sec * 1000) + (tp.tv_usec / 1000); -#endif + if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0) + { + result = (unsigned int)tp.tv_sec * 1000; + // POSIX 1003.1-2004 specifies that tv_nsec is a long (i.e. a + // signed type), but can only contain [0..999,999,999] + result += tp.tv_nsec / 1000000; + } + + return result; } /******************************************************************************/ diff --git a/common/os_calls.h b/common/os_calls.h index 0ef5cdba06..31e8933ba2 100644 --- a/common/os_calls.h +++ b/common/os_calls.h @@ -393,7 +393,26 @@ int g_getgroup_info(const char *groupname, int *gid); * Primary group of username is also checked */ int g_check_user_in_group(const char *username, int gid, int *ok); -int g_time3(void); + +/** + * Gets elapsed milliseconds since some arbitrary point in the past + * + * The returned value is unaffected by leap-seconds or time zone changes. + * + * @return elaped ms since some arbitrary point + * + * Calculate the duration of a task by calling this routine before and + * after the task, and subtracting the two values. + * + * The value wraps every so often (every 49.7 days on a 32-bit system), + * but as we are using unsigned arithmetic, the difference of any of these + * two values can be used to calculate elapsed time, whether-or-not a wrap + * occurs during the interval - provided of course the time being measured + * is less than the total wrap-around interval. + */ +unsigned int +g_get_elapsed_ms(void); + int g_save_to_bmp(const char *filename, char *data, int stride_bytes, int width, int height, int depth, int bits_per_pixel); void *g_shmat(int shmid); diff --git a/common/trans.c b/common/trans.c index 780b4c52a7..ceaf7802df 100644 --- a/common/trans.c +++ b/common/trans.c @@ -696,7 +696,7 @@ local_connect_shim(int fd, const char *server, const char *port) /**************************************************************************//** * Waits for an asynchronous connect to complete. * @param self - Transport object - * @param start_time Start time of connect (from g_time3()) + * @param start_time Start time of connect (from g_get_elapsed_ms()) * @param timeout Total wait timeout * @return 0 - connect succeeded, 1 - Connect failed * @@ -704,10 +704,10 @@ local_connect_shim(int fd, const char *server, const char *port) * on a regular basis. */ static int -poll_for_async_connect(struct trans *self, int start_time, int timeout) +poll_for_async_connect(struct trans *self, unsigned int start_time, int timeout) { int rv = 1; - int ms_remaining = timeout - (g_time3() - start_time); + int ms_remaining = timeout - (int)(g_get_elapsed_ms() - start_time); while (ms_remaining > 0) { @@ -736,7 +736,7 @@ poll_for_async_connect(struct trans *self, int start_time, int timeout) break; } - ms_remaining = timeout - (g_time3() - start_time); + ms_remaining = timeout - (int)(g_get_elapsed_ms() - start_time); } return rv; } @@ -747,7 +747,7 @@ int trans_connect(struct trans *self, const char *server, const char *port, int timeout) { - int start_time = g_time3(); + unsigned int start_time = g_get_elapsed_ms(); int error; int ms_before_next_connect; @@ -826,7 +826,7 @@ trans_connect(struct trans *self, const char *server, const char *port, } /* Have we reached the total timeout yet? */ - int ms_left = timeout - (g_time3() - start_time); + int ms_left = timeout - (int)(g_get_elapsed_ms() - start_time); if (ms_left <= 0) { error = 1; diff --git a/sesman/chansrv/chansrv.c b/sesman/chansrv/chansrv.c index f19346bc41..495d270c37 100644 --- a/sesman/chansrv/chansrv.c +++ b/sesman/chansrv/chansrv.c @@ -132,7 +132,7 @@ add_timeout(int msoffset, void (*callback)(void *data), void *data) tui32 now; LOG_DEVEL(LOG_LEVEL_DEBUG, "add_timeout:"); - now = g_time3(); + now = g_get_elapsed_ms(); tobj = g_new0(struct timeout_obj, 1); tobj->mstime = now + msoffset; tobj->callback = callback; @@ -167,7 +167,7 @@ get_timeout(int *timeout) tobj = g_timeout_head; if (tobj != 0) { - now = g_time3(); + now = g_get_elapsed_ms(); while (tobj != 0) { LOG_DEVEL(LOG_LEVEL_DEBUG, " now %u tobj->mstime %u", now, tobj->mstime); @@ -215,7 +215,7 @@ check_timeout(void) while (tobj != 0) { count++; - now = g_time3(); + now = g_get_elapsed_ms(); if (now >= tobj->mstime) { tobj->callback(tobj->data); diff --git a/sesman/chansrv/sound.c b/sesman/chansrv/sound.c index 2c862f99bd..ba3685ae79 100644 --- a/sesman/chansrv/sound.c +++ b/sesman/chansrv/sound.c @@ -71,7 +71,7 @@ static struct trans *g_audio_c_trans_out = 0; /* connection */ static struct trans *g_audio_l_trans_in = 0; /* listener */ static struct trans *g_audio_c_trans_in = 0; /* connection */ -static int g_training_sent_time = 0; +static unsigned int g_training_sent_time = 0; static int g_cBlockNo = 0; static int g_bytes_in_stream = 0; struct fifo *g_in_fifo; @@ -86,7 +86,7 @@ static struct stream *g_stream_incoming_packet = NULL; #define MAX_BBUF_SIZE (1024 * 16) static char g_buffer[MAX_BBUF_SIZE]; static int g_buf_index = 0; -static int g_sent_time[256]; +static unsigned int g_sent_time[256]; static int g_bbuf_size = 1024 * 8; /* may change later */ @@ -341,7 +341,7 @@ sound_send_training(void) { struct stream *s; int bytes; - int time; + unsigned int time; char *size_ptr; make_stream(s); @@ -349,7 +349,7 @@ sound_send_training(void) out_uint16_le(s, SNDC_TRAINING); size_ptr = s->p; out_uint16_le(s, 0); /* size, set later */ - time = g_time3(); + time = g_get_elapsed_ms(); g_training_sent_time = time; out_uint16_le(s, time); out_uint16_le(s, 1024); @@ -885,7 +885,7 @@ sound_send_wave_data_chunk(char *data, int data_bytes) { struct stream *s; int bytes; - int time; + unsigned int time; int format_index; char *size_ptr; @@ -912,7 +912,7 @@ sound_send_wave_data_chunk(char *data, int data_bytes) out_uint16_le(s, SNDC_WAVE); size_ptr = s->p; out_uint16_le(s, 0); /* size, set later */ - time = g_time3(); + time = g_get_elapsed_ms(); out_uint16_le(s, time); out_uint16_le(s, format_index); /* wFormatNo */ g_cBlockNo++; @@ -1040,7 +1040,7 @@ sound_process_training(struct stream *s, int size) { int time_diff; - time_diff = g_time3() - g_training_sent_time; + time_diff = g_get_elapsed_ms() - g_training_sent_time; LOG(LOG_LEVEL_INFO, "sound_process_training: round trip time %u", time_diff); return 0; } @@ -1052,12 +1052,12 @@ sound_process_wave_confirm(struct stream *s, int size) { int wTimeStamp; int cConfirmedBlockNo; - int time; + unsigned int time; int time_diff; int index; int acc; - time = g_time3(); + time = g_get_elapsed_ms(); in_uint16_le(s, wTimeStamp); in_uint8(s, cConfirmedBlockNo); time_diff = time - g_sent_time[cConfirmedBlockNo & 0xff]; @@ -1095,13 +1095,13 @@ static int process_pcm_message(int id, int size, struct stream *s) { static int sending_silence = 0; - static int silence_start_time = 0; + static unsigned int silence_start_time = 0; switch (id) { case 0: if ((g_client_does_fdk_aac || g_client_does_mp3lame) && sending_silence) { - if ((g_time3() - silence_start_time) < (int)g_cfg->msec_do_not_send) + if ((g_get_elapsed_ms() - silence_start_time) < g_cfg->msec_do_not_send) { /* do not send data within msec_do_not_send msec after SNDC_CLOSE is sent, to avoid stutter. setting from sesman.ini */ return 0; @@ -1119,7 +1119,7 @@ process_pcm_message(int id, int size, struct stream *s) if (buf != NULL) { int i; - silence_start_time = g_time3(); + silence_start_time = g_get_elapsed_ms(); sending_silence = 1; for (i = 0; i < send_silence_times; i++) { diff --git a/sesman/chansrv/xcommon.c b/sesman/chansrv/xcommon.c index 5655176772..2067bbd6f4 100644 --- a/sesman/chansrv/xcommon.c +++ b/sesman/chansrv/xcommon.c @@ -85,15 +85,6 @@ xcommon_fatal_handler(Display *dis) return 0; } -/*****************************************************************************/ -/* returns time in milliseconds since a point in the past - this is a time value similar to what the xserver uses */ -int -xcommon_get_local_time(void) -{ - return g_time3(); -} - /******************************************************************************/ /* this should be called first */ int diff --git a/sesman/chansrv/xcommon.h b/sesman/chansrv/xcommon.h index 75e93c0957..3195eda2a9 100644 --- a/sesman/chansrv/xcommon.h +++ b/sesman/chansrv/xcommon.h @@ -28,8 +28,6 @@ typedef void (*x_server_fatal_cb_type)(void); -int -xcommon_get_local_time(void); int xcommon_init(void); int diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index 072df9cc0a..96ee93c222 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -489,8 +489,8 @@ struct display_control_monitor_layout_data { struct display_size_description description; enum display_resize_state state; - int last_state_update_timestamp; - int start_time; + unsigned int last_state_update_timestamp; + unsigned int start_time; /// This flag is set if the state machine needs to /// shutdown/startup EGFX int using_egfx; diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index 5d276c3b88..6bccd952ba 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -1275,14 +1275,14 @@ advance_resize_state_machine(struct xrdp_mm *mm, "advance_resize_state_machine:" " Processing resize to: %d x %d." " Advancing state from %s to %s." - " Previous state took %d MS.", + " Previous state took %u MS.", description->description.session_width, description->description.session_height, XRDP_DISPLAY_RESIZE_STATE_TO_STR(description->state), XRDP_DISPLAY_RESIZE_STATE_TO_STR(new_state), - g_time3() - description->last_state_update_timestamp); + g_get_elapsed_ms() - description->last_state_update_timestamp); description->state = new_state; - description->last_state_update_timestamp = g_time3(); + description->last_state_update_timestamp = g_get_elapsed_ms(); g_set_wait_obj(mm->resize_ready); return 0; } @@ -1829,7 +1829,7 @@ process_display_control_monitor_layout_data(struct xrdp_wm *wm) // ever is, advance the state machine! if (chan->drdynvcs[mm->egfx->channel_id].status == XRDP_DRDYNVC_STATUS_CLOSED - || (g_time3() - description->last_state_update_timestamp) > 100) + || (g_get_elapsed_ms() - description->last_state_update_timestamp) > 100) { advance_resize_state_machine(mm, WMRZ_EGFX_CONN_CLOSED); break; @@ -2045,7 +2045,7 @@ dynamic_monitor_process_queue(struct xrdp_mm *self) g_malloc(LAYOUT_DATA_SIZE, 1); g_memcpy(&(self->resize_data->description), queue_head, sizeof(struct display_size_description)); - const int time = g_time3(); + const unsigned int time = g_get_elapsed_ms(); self->resize_data->start_time = time; self->resize_data->last_state_update_timestamp = time; self->resize_data->using_egfx = (self->egfx != NULL); @@ -2072,10 +2072,10 @@ dynamic_monitor_process_queue(struct xrdp_mm *self) if (self->resize_data->state == WMRZ_COMPLETE) { LOG(LOG_LEVEL_INFO, "dynamic_monitor_process_queue: Clearing" - " completed resize (w: %d x h: %d). It took %d milliseconds.", + " completed resize (w: %d x h: %d). It took %u milliseconds.", self->resize_data->description.session_width, self->resize_data->description.session_height, - g_time3() - self->resize_data->start_time); + g_get_elapsed_ms() - self->resize_data->start_time); g_set_wait_obj(self->resize_ready); } else if (self->resize_data->state == WMRZ_ERROR) @@ -3532,9 +3532,10 @@ xrdp_mm_get_wait_objs(struct xrdp_mm *self, { if (xrdp_region_not_empty(self->wm->screen_dirty_region)) { - int now = g_time3(); - int next_screen_draw_time = self->wm->last_screen_draw_time + - MIN_MS_BETWEEN_FRAMES; + unsigned int now = g_get_elapsed_ms(); + unsigned int next_screen_draw_time = + self->wm->last_screen_draw_time + + MIN_MS_BETWEEN_FRAMES; int diff = next_screen_draw_time - now; int ltimeout = *timeout; diff = MAX(diff, MIN_MS_TO_WAIT_FOR_MORE_UPDATES); @@ -3924,7 +3925,7 @@ xrdp_mm_check_wait_objs(struct xrdp_mm *self) { if (xrdp_region_not_empty(self->wm->screen_dirty_region)) { - int now = g_time3(); + unsigned int now = g_get_elapsed_ms(); int diff = now - self->wm->last_screen_draw_time; LOG_DEVEL(LOG_LEVEL_TRACE, "xrdp_mm_check_wait_objs: not empty diff %d", diff); if ((diff < 0) || (diff >= 40)) diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index d972e02fd3..f4ef5cd143 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -589,7 +589,7 @@ struct xrdp_wm struct xrdp_tconfig_gfx *gfx_config; struct xrdp_region *screen_dirty_region; - int last_screen_draw_time; + unsigned int last_screen_draw_time; }; /* rdp process */ From 6bf2fe744554e36ac4c82c7c4b66959d42e0912c Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 27 Nov 2024 11:19:36 +0000 Subject: [PATCH 04/19] Restrict scope of sesman_close_all() This function does not need to have global scope. --- sesman/sesman.c | 13 ++++++++++++- sesman/sesman.h | 14 -------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/sesman/sesman.c b/sesman/sesman.c index 18cd765f0c..d3e950026b 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -202,7 +202,18 @@ static int sesman_listen_test(struct config_sesman *cfg) } /******************************************************************************/ -int +/** + * Close all file descriptors used by sesman. + * + * This is generally used after forking, to make sure the + * file descriptors used by the main process are not disturbed + * + * This call will also :- + * - release all trans objects held by sesman + * - Delete sesman wait objects + * - Call sesman_delete_listening_transport() + */ +static int sesman_close_all(void) { LOG_DEVEL(LOG_LEVEL_TRACE, "sesman_close_all:"); diff --git a/sesman/sesman.h b/sesman/sesman.h index 5f7c06d285..ba5a595bf6 100644 --- a/sesman/sesman.h +++ b/sesman/sesman.h @@ -33,20 +33,6 @@ struct trans; /* Globals */ extern struct config_sesman *g_cfg; -/** - * Close all file descriptors used by sesman. - * - * This is generally used after forking, to make sure the - * file descriptors used by the main process are not disturbed - * - * This call will also :- - * - release all trans objects held by sesman - * - Delete sesman wait objects - * - Call sesman_delete_listening_transport() - */ -int -sesman_close_all(void); - /* * Remove the listening transport * From 6b1c90b82d6bb97513cbee2427a6450da5354a52 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 28 Nov 2024 15:38:11 +0000 Subject: [PATCH 05/19] Remove unused variable g_con_list --- sesman/sesman.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/sesman/sesman.c b/sesman/sesman.c index d3e950026b..aac7785be2 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -87,7 +87,6 @@ static struct trans *g_list_trans; /* Variables used to lock g_list_trans */ static struct lock_uds *g_list_trans_lock; -static struct list *g_con_list = NULL; static int g_pid; /*****************************************************************************/ @@ -470,17 +469,10 @@ sesman_main_loop(void) int robjs_count; intptr_t robjs[1024]; - g_con_list = list_create(); - if (g_con_list == NULL) - { - LOG(LOG_LEVEL_ERROR, "sesman_main_loop: list_create failed"); - return 1; - } if (sesman_create_listening_transport(g_cfg) != 0) { LOG(LOG_LEVEL_ERROR, "sesman_main_loop: sesman_create_listening_transport failed"); - list_delete(g_con_list); return 1; } LOG(LOG_LEVEL_INFO, "Sesman now listening on %s", g_cfg->listen_port); From a5ea687f22b8cc049906864c3ec84870940c0ff4 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:17:13 +0000 Subject: [PATCH 06/19] Remove commented-out code The function sesexce_scp_data_in in sesexec.c is a development artefact and can safely be removed --- sesman/sesexec/sesexec.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/sesman/sesexec/sesexec.c b/sesman/sesexec/sesexec.c index 67b493e8d2..5fe52630c0 100644 --- a/sesman/sesexec/sesexec.c +++ b/sesman/sesexec/sesexec.c @@ -119,30 +119,6 @@ process_params(int argc, char **argv, return 0; } -/******************************************************************************/ -#if 0 -static int -sesexec_scp_data_in(struct trans *self) -{ - int rv; - int available; - - rv = scp_msg_in_check_available(self, &available); - - if (rv == 0 && available) - { - struct sesman_con *sc = (struct sesman_con *)self->callback_data; - //if ((rv = scp_process(sc)) != 0) - { - LOG(LOG_LEVEL_ERROR, "%s: scp_process failed", __func__); - } - scp_msg_in_reset(self); - } - - return rv; -} -#endif - /******************************************************************************/ static int sesexec_eicp_data_in(struct trans *self) From 6806489e1eb79928be54c4acda38413410cc0461 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:14:41 +0000 Subject: [PATCH 07/19] Add g_readdir_entries() to OS calls --- common/os_calls.c | 59 ++++++++++++++++++++++++++++ common/os_calls.h | 13 +++++++ tests/common/test_os_calls.c | 74 ++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) diff --git a/common/os_calls.c b/common/os_calls.c index 03c57b1fd2..9bb87c7fcb 100644 --- a/common/os_calls.c +++ b/common/os_calls.c @@ -37,6 +37,7 @@ #define ctid_t id_t #endif #include +#include #include #include #include @@ -4199,3 +4200,61 @@ g_qsort(void *base, size_t nitems, size_t size, { qsort(base, nitems, size, compar); } + +/*****************************************************************************/ +struct list * +g_readdir(const char *dir) +{ + DIR *handle; + struct list *result = NULL; + struct dirent *dent; + int saved_errno; + + errno = 0; // See readdir(3) + if ((handle = opendir(dir)) != NULL && + (result = list_create()) != NULL) + { + result->auto_free = 1; + while (1) + { + errno = 0; + dent = readdir(handle); + if (dent == NULL) + { + break; // errno = 0 for end-of-dir, or != 0 for error + } + + // Ignore '.' and '..' + if (dent->d_name[0] == '.' && dent->d_name[1] == '\0') + { + continue; + } + if (dent->d_name[0] == '.' && dent->d_name[1] == '.' && + dent->d_name[2] == '\0') + { + continue; + } + + if (!list_add_strdup(result, dent->d_name)) + { + // Memory allocation failure + errno = ENOMEM; + break; + } + } + } + + saved_errno = errno; + if (errno != 0) + { + list_delete(result); + result = NULL; + } + if (handle != NULL) + { + closedir(handle); + } + errno = saved_errno; + + return result; +} diff --git a/common/os_calls.h b/common/os_calls.h index 31e8933ba2..189f07a488 100644 --- a/common/os_calls.h +++ b/common/os_calls.h @@ -428,6 +428,19 @@ void g_qsort(void *base, size_t nitems, size_t size, int (*compar)(const void *, const void *)); +/** + * Returns a list of the filenames contained within a directory + * + * @param dir Name of directory + * @return list of directory entry names + * + * If NULL is returned, further information may be available in errno. No + * other errors are specifically logged. + * The special files '.' and '..' are not returned. + */ +struct list * +g_readdir(const char *dir); + /* glib-style wrappers */ #define g_new(struct_type, n_structs) \ (struct_type *) malloc(sizeof(struct_type) * (n_structs)) diff --git a/tests/common/test_os_calls.c b/tests/common/test_os_calls.c index c2a3e09e8b..b1c6b8f87d 100644 --- a/tests/common/test_os_calls.c +++ b/tests/common/test_os_calls.c @@ -7,6 +7,7 @@ #include #include #include +#include #include "os_calls.h" #include "list.h" @@ -20,6 +21,9 @@ // File for testing ro/rw opens #define RO_RW_FILE "./test_ro_rw" +// Directory for testing files +#define TEST_READDIR "./test_readdir" + /******************************************************************************/ /*** * Gets the number of open file descriptors for the current process */ @@ -487,6 +491,74 @@ START_TEST(test_g_sck_fd_overflow) } END_TEST +/******************************************************************************/ +static int qsort_func_strlist(const void *a, const void *b) +{ + return strcmp(*(const char **)a, *(const char **)b); +} + +START_TEST(test_g_readdir) +{ + int status; + const char *entries[] = { "one", "two", "three", "four", "five", 0}; + unsigned int count = 0; + const char **name; + + // Don't check results of create dir, as we may be re-running this + // after a fail + (void)g_mkdir(TEST_READDIR); + // This should work though + status = g_set_current_dir(TEST_READDIR); + ck_assert_int_eq(status, 0); + + // Create some files in the directory + for (name = entries; *name != NULL; ++name, ++count) + { + int fd = g_file_open_rw(*name); + ck_assert(fd >= 0); + g_file_close(fd); + } + + // Can we read them, and are there the expected number? + struct list *dentries = g_readdir("."); + ck_assert_ptr_ne(dentries, NULL); + ck_assert_ptr_ne(dentries, NULL); + ck_assert_int_eq(dentries->count, count); + + // Sort both lists according to the current locale + qsort(entries, count, sizeof(entries[0]), qsort_func_strlist); + qsort(dentries->items, count, sizeof(dentries->items[0]), + qsort_func_strlist); + + // Check both lists are identical + int i; + for (i = 0 ; i < count; ++i) + { + ck_assert_str_eq(entries[i], (const char *)dentries->items[i]); + } + + // Clean up + list_delete(dentries); + for (i = 0 ; i < count ; ++i) + { + g_file_delete(entries[i]); + } + g_set_current_dir(".."); + status = g_remove_dir(TEST_READDIR); // Returns a boolean(?) + ck_assert_int_ne(status, 0); + + +} +END_TEST + +START_TEST(test_g_readdir_not_dir) +{ + struct list *dentries = g_readdir("NoSuchDirectory"); + ck_assert_ptr_eq(dentries, 0); + ck_assert_int_eq(errno, ENOENT); +} +END_TEST + /******************************************************************************/ Suite * make_suite_test_os_calls(void) @@ -512,6 +584,8 @@ make_suite_test_os_calls(void) tcase_add_test(tc_os_calls, test_g_file_is_open); tcase_add_test(tc_os_calls, test_g_sck_fd_passing); tcase_add_test(tc_os_calls, test_g_sck_fd_overflow); + tcase_add_test(tc_os_calls, test_g_readdir); + tcase_add_test(tc_os_calls, test_g_readdir_not_dir); // Add other test cases in other files suite_add_tcase(s, make_tcase_test_os_calls_signals()); From ec711f4844dc277b78dd0a15ea07e951bf51c7ee Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 2 Dec 2024 18:14:36 +0000 Subject: [PATCH 08/19] Add g_socket_exist() to OS calls --- common/os_calls.c | 17 +++++++++++++++++ common/os_calls.h | 1 + 2 files changed, 18 insertions(+) diff --git a/common/os_calls.c b/common/os_calls.c index 9bb87c7fcb..e479368e60 100644 --- a/common/os_calls.c +++ b/common/os_calls.c @@ -2588,6 +2588,23 @@ g_executable_exist(const char *exename) return access(exename, R_OK | X_OK) == 0; } +/*****************************************************************************/ +/* returns boolean, non zero if the socket exists */ +int +g_socket_exist(const char *sockname) +{ + struct stat st; + + if (stat(sockname, &st) == 0) + { + return S_ISSOCK(st.st_mode); + } + else + { + return 0; + } +} + /*****************************************************************************/ /* returns boolean */ int diff --git a/common/os_calls.h b/common/os_calls.h index 189f07a488..ad54ddca7e 100644 --- a/common/os_calls.h +++ b/common/os_calls.h @@ -258,6 +258,7 @@ int g_file_exist(const char *filename); int g_file_readable(const char *filename); int g_directory_exist(const char *dirname); int g_executable_exist(const char *dirname); +int g_socket_exist(const char *dirname); int g_create_dir(const char *dirname); int g_create_path(const char *path); int g_remove_dir(const char *dirname); From cb0e1d5629016f2e72f52cf79643116382e159b3 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 2 Dec 2024 18:15:08 +0000 Subject: [PATCH 09/19] Add ercp_connect() call to ERCP interface This is used by sesman to connect to sockets in the restart directory. --- libipm/ercp.c | 26 ++++++++++++++++++++++++++ libipm/ercp.h | 10 ++++++++++ 2 files changed, 36 insertions(+) diff --git a/libipm/ercp.c b/libipm/ercp.c index a5ba61c9ea..e048f5fb8f 100644 --- a/libipm/ercp.c +++ b/libipm/ercp.c @@ -81,6 +81,32 @@ ercp_trans_from_eicp_trans(struct trans *trans, /*****************************************************************************/ +struct trans * +ercp_connect(const char *port, int (*term_func)(void)) +{ + struct trans *t; + + if ((t = trans_create(TRANS_MODE_UNIX, 128, 128)) != NULL) + { + t->is_term = term_func; + + if (trans_connect(t, NULL, port, 3000) != 0) + { + trans_delete(t); + t = NULL; + } + else if (ercp_init_trans(t) != 0) + { + trans_delete(t); + t = NULL; + } + } + + return t; +} + +/*****************************************************************************/ + int ercp_msg_in_check_available(struct trans *trans, int *available) { diff --git a/libipm/ercp.h b/libipm/ercp.h index aac0a8d681..c77bf984f1 100644 --- a/libipm/ercp.h +++ b/libipm/ercp.h @@ -88,6 +88,16 @@ ercp_trans_from_eicp_trans(struct trans *trans, ttrans_data_in callback_func, void *callback_data); +/** + * Connects to an ERCP endpoint. + * + * @param port Path to UDS object in filesystem + * @param term_func Function to poll during connection for program + * termination, or NULL for none. + */ +struct trans * +ercp_connect(const char *port, + int (*term_func)(void)); /** * Checks an ERCP transport to see if a complete message is From 41aa9aeb0766bf163baa60b619239d4e49719c83 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Tue, 3 Dec 2024 12:35:42 +0000 Subject: [PATCH 10/19] Add session_list_get_count_by_state() to session list --- sesman/session_list.c | 18 ++++++++++++++++++ sesman/session_list.h | 8 ++++++++ 2 files changed, 26 insertions(+) diff --git a/sesman/session_list.c b/sesman/session_list.c index b2c0ea59de..090d441eb8 100644 --- a/sesman/session_list.c +++ b/sesman/session_list.c @@ -122,6 +122,24 @@ session_list_get_count(void) return g_session_list->count; } +/******************************************************************************/ +unsigned int +session_list_get_count_by_state(enum session_state state) +{ + unsigned int result = 0; + int i; + for (i = 0 ; i < g_session_list->count ; ++i) + { + struct session_item *si; + si = (struct session_item *)list_get_item(g_session_list, i); + if (si->state == state) + { + ++result; + } + } + return result; +} + /******************************************************************************/ struct session_item * session_list_new(void) diff --git a/sesman/session_list.h b/sesman/session_list.h index b018afad97..d33c56c65c 100644 --- a/sesman/session_list.h +++ b/sesman/session_list.h @@ -90,6 +90,14 @@ session_list_cleanup(void); unsigned int session_list_get_count(void); +/** + * @brief Get the number of sessions in a particular state + * @param state to count + * @return session count + */ +unsigned int +session_list_get_count_by_state(enum session_state state); + /** * Allocates a new session on the list * From c503f17547d10e2431b2dda9ce0ef796a4c813be Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Sat, 7 Dec 2024 17:37:35 +0000 Subject: [PATCH 11/19] Make listen_port smaller than XRDP_SOCKETS_MAXPATH --- sesman/libsesman/sesman_config.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sesman/libsesman/sesman_config.h b/sesman/libsesman/sesman_config.h index aa8d43dcfb..26800cb473 100644 --- a/sesman/libsesman/sesman_config.h +++ b/sesman/libsesman/sesman_config.h @@ -183,8 +183,11 @@ struct config_sesman /** * @var listen_port * @brief Listening port + * + * This string is used to form the restart directory name, so + * can't be the full XRDP_SOCKETS_MAXPATH length. */ - char listen_port[XRDP_SOCKETS_MAXPATH]; + char listen_port[XRDP_SOCKETS_MAXPATH - 10]; /** * @var enable_user_wm * @brief Flag that enables user specific wm From 44bd2a29470669c31f01531ce61f89cb77898aff Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 28 Nov 2024 15:55:37 +0000 Subject: [PATCH 12/19] Add warning if listen_port changes --- sesman/sig.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sesman/sig.c b/sesman/sig.c index 8cdd508a6a..c7151d8e22 100644 --- a/sesman/sig.c +++ b/sesman/sig.c @@ -58,6 +58,8 @@ sig_sesman_reload_cfg(void) { LOG(LOG_LEVEL_INFO, "sesman listen port changed to %s", cfg->listen_port); + LOG(LOG_LEVEL_WARNING, + "Restarting sesman will now lose active sessions"); /* We have to delete the old port before listening to the new one * in case they overlap in scope */ From b6ed47c316b78edb3e3df94e5d10fc5c71671b1d Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 2 Dec 2024 11:23:02 +0000 Subject: [PATCH 13/19] Add sesman restart module Adds a module which can be used when sesman is restarting. This is initially used to rediscover sessions from a previous run. --- sesman/Makefile.am | 2 + sesman/sesman.c | 6 +- sesman/sesman.h | 1 + sesman/sesman_restart.c | 288 ++++++++++++++++++++++++++++++++++++++++ sesman/sesman_restart.h | 40 ++++++ 5 files changed, 335 insertions(+), 2 deletions(-) create mode 100644 sesman/sesman_restart.c create mode 100644 sesman/sesman_restart.h diff --git a/sesman/Makefile.am b/sesman/Makefile.am index 8d9d31022c..8744405baa 100644 --- a/sesman/Makefile.am +++ b/sesman/Makefile.am @@ -32,6 +32,8 @@ xrdp_sesman_SOURCES = \ sesexec_control.h \ session_list.c \ session_list.h \ + sesman_restart.c \ + sesman_restart.h \ sig.c \ sig.h diff --git a/sesman/sesman.c b/sesman/sesman.c index aac7785be2..4fbd94e8ed 100644 --- a/sesman/sesman.c +++ b/sesman/sesman.c @@ -46,6 +46,7 @@ #include "scp.h" #include "scp_process.h" #include "sesexec_control.h" +#include "sesman_restart.h" #include "sig.h" #include "string_calls.h" #include "trans.h" @@ -78,7 +79,7 @@ struct sesman_startup_params }; struct config_sesman *g_cfg; -static tintptr g_term_event = 0; +tintptr g_term_event = 0; static tintptr g_sigchld_event = 0; static tintptr g_reload_event = 0; @@ -940,7 +941,8 @@ main(int argc, char **argv) } if ((error = pre_session_list_init(MAX_PRE_SESSION_ITEMS)) == 0 && - (error = session_list_init()) == 0) + (error = session_list_init()) == 0 && + (error = sesman_restart_discover_sessions()) == 0) { error = sesman_main_loop(); } diff --git a/sesman/sesman.h b/sesman/sesman.h index ba5a595bf6..6c60de738c 100644 --- a/sesman/sesman.h +++ b/sesman/sesman.h @@ -32,6 +32,7 @@ struct trans; /* Globals */ extern struct config_sesman *g_cfg; +extern tintptr g_term_event; /* * Remove the listening transport diff --git a/sesman/sesman_restart.c b/sesman/sesman_restart.c new file mode 100644 index 0000000000..ff7e0cae5f --- /dev/null +++ b/sesman/sesman_restart.c @@ -0,0 +1,288 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Matt Burt 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * + * @file sesman_restart.c + * @brief Sesman restart definitions + * @author Matt Burt + * + */ + +#if defined(HAVE_CONFIG_H) +#include "config_ac.h" +#endif + +#include + +#include "os_calls.h" +#include "sesman.h" +#include "sesman_config.h" +#include "session_list.h" + +#include "ercp.h" + +#include "sesman_restart.h" +#include "xrdp_sockets.h" + +// Result of calling init_restart_directory() +enum init_restart_dir_status +{ + E_RESTART_DIR_CREATED_OK, ///< All good. Dir created + E_RESTART_DIR_ALREADY_EXISTS, ///< All good. Dir already existed + E_RESTART_DIR_ERROR ///< Not good +}; + +enum +{ + MAX_DISCOVERY_WAIT_TIME = 5000 // Milli-seconds +}; + +/******************************************************************************/ +static enum init_restart_dir_status +init_restart_directory(const char *restart_dir) +{ + enum init_restart_dir_status rv; + + if (g_directory_exist(restart_dir)) + { + rv = E_RESTART_DIR_ALREADY_EXISTS; + } + else + { + // Create the restart directory for the next run + if (g_mkdir(restart_dir) != 0) + { + LOG(LOG_LEVEL_ERROR, "Can't create restart directory %s [%s]", + restart_dir, g_get_strerror()); + rv = E_RESTART_DIR_ERROR; + } + else + { + rv = E_RESTART_DIR_CREATED_OK; + } + } + + if (rv != E_RESTART_DIR_ERROR) + { + // Always set the permissions on the restart directory, whether + // or not we created it + if (g_chown(restart_dir, g_getuid(), g_getuid()) != 0) + { + LOG(LOG_LEVEL_ERROR, "Can't set ownership of '%s' [%s]", + restart_dir, g_get_strerror()); + rv = E_RESTART_DIR_ERROR; + } + else if (g_chmod_hex(restart_dir, 0x700) != 0) + { + LOG(LOG_LEVEL_ERROR, "Can't set permissions on '%s' [%s]", + restart_dir, g_get_strerror()); + rv = E_RESTART_DIR_ERROR; + } + } + return rv; +} + +/******************************************************************************/ +/** + * Attempts to add a sesexec Unix Domain Socket to the process_list + * @param filename Name of UDS + * @return Boolean for success + * + * The credentials of the process on the other end are checked. + * We don't take the max session limit into account when adding sessions here, + * as this could result in orphaned sessions. + */ +static int +add_sesexec_fd_to_session_list(const char *filename) +{ + int status = 0; + struct trans *t = NULL; + + // Check filename is a socket + if (g_socket_exist(filename)) + { + // Try to connect to the session + if ((t = ercp_connect(filename, sesman_is_term)) != NULL) + { + int sesexec_pid; + int sesexec_uid; + int sesexec_gid; + + // Find the credentials of the sesexec process on the other end + if (g_sck_get_peer_cred(t->sck, &sesexec_pid, + &sesexec_uid, &sesexec_gid) == 0) + { + // Don't talk to unprivileged processes. It's a big concern + // if we find one. + if (sesexec_uid != 0 || sesexec_gid != 0) + { + LOG(LOG_LEVEL_ALWAYS, + "Unexpected sesexec owner %d:%d" + " for PID %d listening on %s", + sesexec_uid, sesexec_gid, sesexec_pid, filename); + } + else + { + struct session_item *s_item; + if ((s_item = session_list_new()) != NULL) + { + // Finalise the session for I/O + t->trans_data_in = sesman_ercp_data_in; + t->callback_data = (void *)s_item; + + // Complete the session fields for the + // E_SESSION_STARTING state + s_item->sesexec_trans = t; + s_item->sesexec_pid = sesexec_pid; + s_item->display = -1; + + // Tell the caller we've added one + status = 1; + } + } + } + } + } + + // Clean up an unused transport + if (status == 0) + { + trans_delete(t); + } + + return 1; +} + +/******************************************************************************/ +static int +discover_sessions(const char *restart_dir) +{ + int rv = 0; + struct list *dirnames = g_readdir(restart_dir); + unsigned int start_time = g_get_elapsed_ms(); + unsigned int session_count; + unsigned int elapsed; + int robjs_count; + intptr_t robjs[1024]; + int timeout; + + if (dirnames == NULL) + { + LOG(LOG_LEVEL_ERROR, + "Can't read restart directory to discover sessions [%s]", + g_get_strerror()); + } + else + { + // Iterate over the restart directory, and add any sesexec + // processes we discover to the session list, in + // E_SESSION_STARTING state. + char filename[XRDP_SOCKETS_MAXPATH]; + int i; + + for (i = 0 ; i < dirnames->count; ++i) + { + g_snprintf(filename, sizeof(filename), "%s/%s", + restart_dir, (const char *)dirnames->items[i]); + + (void)add_sesexec_fd_to_session_list(filename); + } + } + + // Process session list messages until either all sessions have + // started (or failed), or we hit a timeout. + while (1) + { + session_count = session_list_get_count_by_state(E_SESSION_STARTING); + elapsed = (g_get_elapsed_ms() - start_time); + if (session_count == 0 || elapsed >= MAX_DISCOVERY_WAIT_TIME) + { + break; + } + + robjs_count = 0; + robjs[robjs_count++] = g_term_event; + (void)session_list_get_wait_objs(robjs, &robjs_count); + + timeout = MAX_DISCOVERY_WAIT_TIME - elapsed; // > 0 + if (g_obj_wait(robjs, robjs_count, NULL, 0, timeout) != 0) + { + /* should not get here */ + g_sleep(100); + continue; + } + + if (g_is_wait_obj_set(g_term_event)) /* term */ + { + LOG(LOG_LEVEL_INFO, "discover_sessions: sesman asked to terminate"); + rv = 1; + break; + } + + (void)session_list_check_wait_objs(); + } + + if (rv == 0) + { + LOG(LOG_LEVEL_INFO, + "Session discovery took %d secs and loaded %u sessions", + elapsed, session_list_get_count_by_state(E_SESSION_RUNNING)); + + if (session_count > 0) + { + LOG(LOG_LEVEL_WARNING, + "%u sessions have not responded at end of discovery", + session_count); + } + } + + return rv; +} + +/******************************************************************************/ +int +sesman_restart_discover_sessions(void) +{ + int rv = 1; + // The restart directory contains Unix Domain sockets, so can't + // exceed XRDP_SOCKETS_MAXPATH in length + char restart_dir[XRDP_SOCKETS_MAXPATH]; + + // sizeof(g_cfg->listen_port) is guaranteed to be smaller than + // XRDP_SOCKETS_MAXPATH + g_snprintf(restart_dir, sizeof(restart_dir), + "%s.r", g_cfg->listen_port); + switch (init_restart_directory(restart_dir)) + { + case E_RESTART_DIR_CREATED_OK: + // Nothing to discover + rv = 0; + break; + + case E_RESTART_DIR_ALREADY_EXISTS: + // Look for sessions from previous run + rv = discover_sessions(restart_dir); + break; + + default: + ; + } + + return rv; +} diff --git a/sesman/sesman_restart.h b/sesman/sesman_restart.h new file mode 100644 index 0000000000..bbfa11b49c --- /dev/null +++ b/sesman/sesman_restart.h @@ -0,0 +1,40 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Matt Burt 2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * + * @file sesman_restart.h + * @brief Sesman restart declarations + * @author Matt Burt + * + */ + + +#ifndef SESMAN_RESTART_H +#define SESMAN_RESTART_H + +/** + * Discover sessions from a previous sesman run + * @return 0 for success + * + * Errors are logged + */ +int +sesman_restart_discover_sessions(void); + +#endif // SESMAN_RESTART_H From 24cb2745996c501f8635b8f3cdc93aba5832a421 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Sat, 7 Dec 2024 17:38:54 +0000 Subject: [PATCH 14/19] Add session_get_parameters() Also add useful comment to session_send_term() --- sesman/sesexec/session.c | 10 ++++++++++ sesman/sesexec/session.h | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 13ac7f3596..6641ac8ea9 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -929,12 +929,22 @@ session_get_start_time(const struct session_data *sd) return (sd == NULL) ? 0 : sd->start_time; } +/******************************************************************************/ +const struct session_parameters * +session_get_parameters(const struct session_data *sd) +{ + return (sd == NULL) ? NULL : &sd->params; +} + /******************************************************************************/ void session_send_term(struct session_data *sd) { if (sd != NULL && sd->win_mgr > 0) { + // Killing the window manager only is appropriate here. + // When we process SIGCHLD for the windowe manager, we + // will kill other processes as appropriate g_sigterm(sd->win_mgr); } } diff --git a/sesman/sesexec/session.h b/sesman/sesexec/session.h index 5cfab1409f..a1fdd4abf1 100644 --- a/sesman/sesexec/session.h +++ b/sesman/sesexec/session.h @@ -103,10 +103,23 @@ session_active(const struct session_data *sd); * Returns the start time for an active session * * @param sd session_data for this session + * @return session start time */ time_t session_get_start_time(const struct session_data *sd); +/** + * Returns the parameters used to start the session + * + * @param sd session_data for this session + * @return Pointer to parameters + * + * The pointed-to data returned must not be modified in + * any way. + */ +const struct session_parameters * +session_get_parameters(const struct session_data *sd); + /*** * Ask a session to terminate by signalling the window manager * From 371bc8028096a669a5b65989b8826260e245c729 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:12:24 +0000 Subject: [PATCH 15/19] Add sesexec discover module The module is a dummy to be filled in later Other structural changes to sesexec:- 1) A failure of sesman needs to be detected and handled without causing sesexec to exit 2) If sesexec exits, the session can never be rediscovered. sesexec must be robust enough to stay up for the lifetime of the session so that the discovery function always works. 3) There is a mechanism for sesexec to terminate the session, but it doesn't work, as SIGCHLD is not processed while we are waiting for the session to finish. This needs fixing. --- sesman/sesexec/Makefile.am | 2 + sesman/sesexec/eicp_server.c | 87 ++++++++++++++------- sesman/sesexec/sesexec.c | 125 +++++++++++++++++++++++++----- sesman/sesexec/sesexec_discover.c | 63 +++++++++++++++ sesman/sesexec/sesexec_discover.h | 77 ++++++++++++++++++ 5 files changed, 308 insertions(+), 46 deletions(-) create mode 100644 sesman/sesexec/sesexec_discover.c create mode 100644 sesman/sesexec/sesexec_discover.h diff --git a/sesman/sesexec/Makefile.am b/sesman/sesexec/Makefile.am index 7adb959150..c48d1e6c5f 100644 --- a/sesman/sesexec/Makefile.am +++ b/sesman/sesexec/Makefile.am @@ -29,6 +29,8 @@ xrdp_sesexec_SOURCES = \ env.h \ login_info.c \ login_info.h \ + sesexec_discover.c \ + sesexec_discover.h \ sessionrecord.c \ sessionrecord.h \ xauth.c \ diff --git a/sesman/sesexec/eicp_server.c b/sesman/sesexec/eicp_server.c index 80255fdf04..210d21b757 100644 --- a/sesman/sesexec/eicp_server.c +++ b/sesman/sesexec/eicp_server.c @@ -37,6 +37,7 @@ #include "ercp.h" #include "scp.h" #include "sesexec.h" +#include "sesexec_discover.h" #include "session.h" /******************************************************************************/ @@ -98,12 +99,13 @@ handle_create_session_request(struct trans *self) { int scp_fd; struct session_parameters sp = {0}; - int rv; + int status; - rv = eicp_get_create_session_request(self, &scp_fd, &sp.display, - &sp.type, &sp.width, &sp.height, - &sp.bpp, &sp.shell, &sp.directory); - if (rv == 0) + status = eicp_get_create_session_request( + self, &scp_fd, &sp.display, + &sp.type, &sp.width, &sp.height, + &sp.bpp, &sp.shell, &sp.directory); + if (status == 0) { // Need to talk to the SCP client struct trans *scp_trans; @@ -113,7 +115,7 @@ handle_create_session_request(struct trans *self) { LOG(LOG_LEVEL_ERROR, "Can't create SCP trans"); g_file_close(scp_fd); - rv = 1; + status = 1; } else { @@ -134,39 +136,68 @@ handle_create_session_request(struct trans *self) scp_status = session_start(g_login_info, &sp, &g_session_data); } - // Return the status to the SCP client - rv = scp_send_create_session_response(scp_trans, scp_status, - sp.display, &sp.guid); - trans_delete(scp_trans); - - // Further comms from sesexec is sent over the ERCP protocol + // Further comms to sesman is sent over the ERCP protocol ercp_trans_from_eicp_trans(self, sesexec_ercp_data_in, (void *)self); - if (scp_status == E_SCP_SCREATE_OK) + if (scp_status != E_SCP_SCREATE_OK) { - rv = ercp_send_session_announce_event( - self, - sp.display, - g_login_info->uid, - sp.type, - sp.width, - sp.height, - sp.bpp, - &sp.guid, - g_login_info->ip_addr, - session_get_start_time(g_session_data)); + // Tell sesman the session hasn't started + (void)ercp_send_session_finished_event(self); } - else + else if ((status = ercp_send_session_announce_event( + self, + sp.display, + g_login_info->uid, + sp.type, + sp.width, + sp.height, + sp.bpp, + &sp.guid, + g_login_info->ip_addr, + session_get_start_time(g_session_data))) != 0) + { + // We failed to tell sesman about the new session. This + // probably means sesman has exited in the time between + // asking us to start a session, and our reply. This + // could be many seconds, and a new sesman may well + // have started. + // If we enable the restart functionality at + // this point, we have a race condition that could + // result in a session which sesman doesn't know + // about. The simplest thing to do in this rare situation + // is to abort the session - the user can create a + // new one + LOG(LOG_LEVEL_ERROR, + "sesman appears to have failed - stopping session"); + scp_status = E_SCP_SCREATE_GENERAL_ERROR; + } + else if ((status = sesexec_discover_enable()) != 0) { - rv = ercp_send_session_finished_event(self); - sesexec_terminate_main_loop(1); + // Equally regrettable - we can't make the session + // discoverable, so we'll stop it. We can tell sesman + // about this one however. + LOG(LOG_LEVEL_ERROR, + "unable to make session discoverable" + " - stopping session"); + (void)ercp_send_session_finished_event(self); + scp_status = E_SCP_SCREATE_GENERAL_ERROR; } + + // Return the status to the SCP client. + (void)scp_send_create_session_response(scp_trans, scp_status, + sp.display, &sp.guid); + trans_delete(scp_trans); } + } + if (status != 0) + { + // Kill sesexec, and any active session + sesexec_terminate_main_loop(status); } - return rv; + return 0; } /******************************************************************************/ diff --git a/sesman/sesexec/sesexec.c b/sesman/sesexec/sesexec.c index 5fe52630c0..752a770964 100644 --- a/sesman/sesexec/sesexec.c +++ b/sesman/sesexec/sesexec.c @@ -38,6 +38,7 @@ #include "ercp_server.h" #include "login_info.h" #include "sesexec.h" +#include "sesexec_discover.h" #include "sesman_config.h" #include "log.h" #include "os_calls.h" @@ -209,8 +210,12 @@ sesexec_is_term(void) void sesexec_terminate_main_loop(int status) { - g_terminate_loop = 1; - g_terminate_status = status; + // Only take the first request to terminate the loop + if (!g_terminate_loop) + { + g_terminate_loop = 1; + g_terminate_status = status; + } } /******************************************************************************/ @@ -227,6 +232,45 @@ process_sigchld_event(void) } } +/******************************************************************************/ +static void +sesexec_terminate_session_and_wait(void) +{ + session_send_term(g_session_data); + do + { + g_sleep(1000); + // Process SIGCHLD events while waiting for the session + // to exit. + if (g_is_wait_obj_set(g_sigchld_event)) + { + g_reset_wait_obj(g_sigchld_event); + process_sigchld_event(); + } + } + while (session_active(g_session_data)); +} + +/******************************************************************************/ +static void +sesexec_main_loop_cleanup(void) +{ + login_info_free(g_login_info); + + /* This session is no longer discoverable */ + sesexec_discover_disable(); + + /* Don't allow sesexec to terminate with an active + session, as we can't connect to such a session */ + if (session_active(g_session_data)) + { + LOG(LOG_LEVEL_INFO, + "Stopping session on xrdp-sesexec exit"); + sesexec_terminate_session_and_wait(); + } + session_data_free(g_session_data); +} + /******************************************************************************/ /** * @@ -238,7 +282,8 @@ sesexec_main_loop(void) { int error = 0; int robjs_count; - intptr_t robjs[32]; +#define MAX_ROBJS 32 + intptr_t robjs[MAX_ROBJS]; g_terminate_loop = 0; g_terminate_status = 0; @@ -250,11 +295,25 @@ sesexec_main_loop(void) robjs[robjs_count++] = g_term_event; robjs[robjs_count++] = g_sigchld_event; - error = trans_get_wait_objs(g_ecp_trans, robjs, &robjs_count); + // ECP transport may be null if sesman has gone away + if (g_ecp_trans != NULL) + { + error = trans_get_wait_objs(g_ecp_trans, robjs, &robjs_count); + if (error != 0) + { + LOG(LOG_LEVEL_ERROR, "sesexec_main_loop: " + "trans_get_wait_objs(ECP) failed"); + sesexec_terminate_main_loop(error); + continue; + } + } + + // Add any objects from the discover module + error = sesexec_discover_get_wait_objs(robjs, &robjs_count, MAX_ROBJS); if (error != 0) { LOG(LOG_LEVEL_ERROR, "sesexec_main_loop: " - "trans_get_wait_objs(ECP) failed"); + "sesexec_discover_get_wait_objs() failed"); sesexec_terminate_main_loop(error); continue; } @@ -273,21 +332,17 @@ sesexec_main_loop(void) g_reset_wait_obj(g_term_event); if (session_active(g_session_data)) { - // Ask the active session to terminate LOG(LOG_LEVEL_INFO, "sesexec_main_loop: " - "sesexec asked to terminate. " - "Terminating active session"); - session_send_term(g_session_data); + "sesexec asked to terminate with active session."); } else { - // Terminate immediately LOG(LOG_LEVEL_INFO, "sesexec_main_loop: " "sesexec asked to terminate. " "No session is active"); - sesexec_terminate_main_loop(0); - continue; } + sesexec_terminate_main_loop(0); + continue; } if (g_is_wait_obj_set(g_sigchld_event)) /* SIGCHLD */ @@ -302,7 +357,10 @@ sesexec_main_loop(void) { // We've finished the session. Tell sesman and // finish up. - (void)ercp_send_session_finished_event(g_ecp_trans); + if (g_ecp_trans != NULL) + { + (void)ercp_send_session_finished_event(g_ecp_trans); + } session_data_free(g_session_data); g_session_data = NULL; @@ -311,19 +369,51 @@ sesexec_main_loop(void) } } - error = trans_check_wait_objs(g_ecp_trans); + if (g_ecp_trans != NULL) + { + error = trans_check_wait_objs(g_ecp_trans); + if (error != 0) + { + if (g_ecp_trans->status != TRANS_STATUS_UP && + session_active(g_session_data)) + { + // sesman has gone away. We have an active session + // to keep track of, so sesman can pick it up when it + // restarts + LOG(LOG_LEVEL_INFO, "sesexec_main_loop: " + "sesman has exited"); + trans_delete(g_ecp_trans); + g_ecp_trans = NULL; + } + else + { + // A callback has failed, or sesman has gone away and + // we have no active session + LOG(LOG_LEVEL_ERROR, "sesexec_main_loop: " + "trans_check_wait_objs failed for ECP transport"); + sesexec_terminate_main_loop(error); + } + continue; + } + } + + error = sesexec_discover_check_wait_objs(); if (error != 0) { LOG(LOG_LEVEL_ERROR, "sesexec_main_loop: " - "trans_check_wait_objs failed for ECP transport"); + "sesexec_discover_check_wait_objs failed"); sesexec_terminate_main_loop(error); continue; } + } - login_info_free(g_login_info); + /* close sesman communications immediately */ + trans_delete(g_ecp_trans); + g_ecp_trans = NULL; return g_terminate_status; +#undef MAX_ROBJS } /******************************************************************************/ @@ -483,9 +573,8 @@ main(int argc, char **argv) /* start program main loop */ LOG(LOG_LEVEL_INFO, "starting xrdp-sesexec with pid %d", g_pid); error = sesexec_main_loop(); - trans_delete(g_ecp_trans); + sesexec_main_loop_cleanup(); } - g_delete_wait_obj(g_term_event); } config_free(g_cfg); diff --git a/sesman/sesexec/sesexec_discover.c b/sesman/sesexec/sesexec_discover.c new file mode 100644 index 0000000000..d4a2ea7f8b --- /dev/null +++ b/sesman/sesexec/sesexec_discover.c @@ -0,0 +1,63 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Jay Sorg 2004-2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * + * @file sesexec_discover.c + * @brief Declare functionality associated with sesman restart support + * @author Matt Burt + * + */ + +#if defined(HAVE_CONFIG_H) +#include +#endif + +#include "sesexec_discover.h" + +/******************************************************************************/ +int +sesexec_discover_enable(void) +{ + return 0; +} + +/******************************************************************************/ + +int +sesexec_discover_disable(void) +{ + return 0; +} + +/******************************************************************************/ + +int +sesexec_discover_get_wait_objs(intptr_t robjs[], int *robjs_count, + int max_count) +{ + return 0; +} + +/******************************************************************************/ + +int +sesexec_discover_check_wait_objs(void) +{ + return 0; +} diff --git a/sesman/sesexec/sesexec_discover.h b/sesman/sesexec/sesexec_discover.h new file mode 100644 index 0000000000..e29fea4027 --- /dev/null +++ b/sesman/sesexec/sesexec_discover.h @@ -0,0 +1,77 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Jay Sorg 2004-2024 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * + * @file sesexec_discover.h + * @brief Declare functionality associated with sesman restart support + * for sesexec + * + * @author Matt Burt + * + */ + +#ifndef SESEXEC_DISCOVER_H +#define SESEXEC_DISCOVER_H + +#include + +/** + * Start the listening object used when sesman restarts + * + * @return != 0 for error + */ +int +sesexec_discover_enable(void); + +/** + * Stop the listening object used when sesman restarts, and deallocate all + * module resources. + * + * @return != 0 for error + */ +int +sesexec_discover_disable(void); + +/** + * Add any file descriptors in use by the module to an array + * + * @param robjs Array to add fds to + * @param[in,out] robjs_count Index where elements are to be added + * @param max_count Max value of robjs_count + * @return != 0 for error + * + * This function can be called before sesexec_discover_enable(), + * in which case it does nothing. + */ +int +sesexec_discover_get_wait_objs(intptr_t robjs[], int *robjs_count, + int max_count); + + +/** + * Check any file descriptors in use by the module for actionable events + * @return != 0 for error + * + * This function can be called before sesexec_discover_enable(), + * in which case it does nothing. + */ +int +sesexec_discover_check_wait_objs(void); + +#endif // SESEXEC_DISCOVER_H From 6554832dbafc8a67e5985ff9cae7c592eb86557f Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:04:11 +0000 Subject: [PATCH 16/19] Add sesexec_set_ecp_transport/sesexec_is_ecp_active() These sesexec functions are needed for the discovery module to function. --- sesman/sesexec/sesexec.c | 63 +++++++++++++++++++++++++++++++++++----- sesman/sesexec/sesexec.h | 20 +++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/sesman/sesexec/sesexec.c b/sesman/sesexec/sesexec.c index 752a770964..0d9e440881 100644 --- a/sesman/sesexec/sesexec.c +++ b/sesman/sesexec/sesexec.c @@ -68,6 +68,7 @@ pid_t g_pid; * Module-scope globals */ static struct trans *g_ecp_trans; +static pid_t g_ecp_pid; static int g_terminate_loop = 0; static int g_terminate_status = 0; @@ -218,6 +219,53 @@ sesexec_terminate_main_loop(int status) } } +/******************************************************************************/ +int +sesexec_set_ecp_transport(struct trans *t) +{ + int rv; + int pid; + int uid; + int gid; + + if (t == NULL) + { + trans_delete(g_ecp_trans); + g_ecp_trans = NULL; + g_ecp_pid = 0; + rv = 0; + } + else if ((rv = g_sck_get_peer_cred(t->sck, &pid, &uid, &gid)) != 0) + { + LOG(LOG_LEVEL_ERROR, "Can't get credentials of sesman socket [%s]", + g_get_strerror()); + } + else if (uid != 0 || gid != 0) + { + LOG(LOG_LEVEL_ERROR, "sesman PID %d is running as UID:GID %d:%d", + pid, uid, gid); + rv = 1; + } + else + { + trans_delete(g_ecp_trans); + g_ecp_trans = t; + g_ecp_pid = pid; + rv = 0; + } + + return rv; +} + +/******************************************************************************/ +int +sesexec_is_ecp_active(void) +{ + return (g_ecp_trans != NULL && + g_ecp_pid != 0 && g_pid_is_active(g_ecp_pid)); + +} + /******************************************************************************/ static void process_sigchld_event(void) @@ -382,8 +430,7 @@ sesexec_main_loop(void) // restarts LOG(LOG_LEVEL_INFO, "sesexec_main_loop: " "sesman has exited"); - trans_delete(g_ecp_trans); - g_ecp_trans = NULL; + sesexec_set_ecp_transport(NULL); } else { @@ -409,8 +456,7 @@ sesexec_main_loop(void) } /* close sesman communications immediately */ - trans_delete(g_ecp_trans); - g_ecp_trans = NULL; + sesexec_set_ecp_transport(NULL); return g_terminate_status; #undef MAX_ROBJS @@ -542,6 +588,7 @@ main(int argc, char **argv) else { char text[128]; + struct trans *t; g_pid = g_getpid(); @@ -562,10 +609,10 @@ main(int argc, char **argv) /* Set up an EICP process handler * Errors are logged by this call if necessary */ - g_ecp_trans = eicp_init_trans_from_fd(eicp_fd, - TRANS_TYPE_SERVER, - sesexec_is_term); - if (g_ecp_trans != NULL) + t = eicp_init_trans_from_fd(eicp_fd, + TRANS_TYPE_SERVER, + sesexec_is_term); + if (t != NULL && sesexec_set_ecp_transport(t) == 0) { g_ecp_trans->trans_data_in = sesexec_eicp_data_in; g_ecp_trans->callback_data = NULL; diff --git a/sesman/sesexec/sesexec.h b/sesman/sesexec/sesexec.h index ff6fe7e0fc..7d476b2c23 100644 --- a/sesman/sesexec/sesexec.h +++ b/sesman/sesexec/sesexec.h @@ -67,4 +67,24 @@ sesexec_is_term(void); void sesexec_terminate_main_loop(int status); +/** + * Sets the ECP (sesman) transport + * + * @param t Transport supposedly connected to the ECP protocol + * provider (sesman), or NULL to clear the transport + * @return 0 for success + * + * Peer credentials are checked for root:root + */ +int +sesexec_set_ecp_transport(struct trans *t); + +/* + * Is the ECP transport still active? + * + * @result boolean + */ +int +sesexec_is_ecp_active(void); + #endif // SESEXEC_H From f18bebdf316c1b982a3d550e153008bf4143586f Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:06:51 +0000 Subject: [PATCH 17/19] Fill in discovery module Add functionality to sesexec discovery module to enable sesman restarts. --- sesman/sesexec/sesexec_discover.c | 135 ++++++++++++++++++++++++++++-- 1 file changed, 130 insertions(+), 5 deletions(-) diff --git a/sesman/sesexec/sesexec_discover.c b/sesman/sesexec/sesexec_discover.c index d4a2ea7f8b..952b327c33 100644 --- a/sesman/sesexec/sesexec_discover.c +++ b/sesman/sesexec/sesexec_discover.c @@ -28,20 +28,130 @@ #include #endif +#include + #include "sesexec_discover.h" +#include "trans.h" +#include "sesexec.h" +#include "session.h" +#include "sesman_config.h" +#include "os_calls.h" +#include "ercp.h" +#include "login_info.h" + +/* + * Module-scope globals + */ +static struct trans *g_discover_trans = NULL; + +/*****************************************************************************/ +static int +discover_trans_conn_in(struct trans *trans, struct trans *new_trans) +{ + const struct session_parameters *sp; + int rv = 0; + + if (trans == NULL || new_trans == NULL || trans != g_discover_trans) + { + return 1; + } + + LOG_DEVEL(LOG_LEVEL_DEBUG, "discover_trans_conn_in:"); + + if (sesexec_is_ecp_active()) + { + int pid = 0; + (void)g_sck_get_peer_cred(new_trans->sck, &pid, 0, 0); + + LOG(LOG_LEVEL_WARNING, + "Connection attempt to sesexec PID %d from PID %d" + " while ECP is still active", + g_pid, pid); + + trans_delete(new_trans); + } + else if ((sp = session_get_parameters(g_session_data)) == NULL) + { + // If we haven't got session parameters, we shouldn't be here + LOG(LOG_LEVEL_ERROR, "Bugcheck: Can't get active session params"); + trans_delete(new_trans); + rv = 1; + } + else + { + // Reconnect to sesman and tell it about our existing + // session + ercp_init_trans(new_trans); + new_trans->trans_data_in = sesexec_ercp_data_in; + new_trans->callback_data = (void *)new_trans; + + // Note, this call makes further privilege checks that may still + // fail. If they do however, we wish to carry on running. These + // failed checks will be logged. + if (sesexec_set_ecp_transport(new_trans) == 0) + { + (void)ercp_send_session_announce_event( + new_trans, + sp->display, + g_login_info->uid, + sp->type, + sp->width, + sp->height, + sp->bpp, + &sp->guid, + g_login_info->ip_addr, + session_get_start_time(g_session_data)); + } + } + return rv; +} /******************************************************************************/ int sesexec_discover_enable(void) { - return 0; + int rv = 1; + if (g_session_data == NULL) + { + LOG(LOG_LEVEL_ERROR, "Cant enable discovery without an active session"); + } + else if (g_discover_trans != NULL) + { + LOG(LOG_LEVEL_ERROR, "Logic error: discovery is already active"); + } + else if ((g_discover_trans = + trans_create(TRANS_MODE_UNIX, 8192, 8192)) == NULL) + { + LOG(LOG_LEVEL_ERROR, "Out of memory enabling discovery"); + } + else + { + char discover_port[XRDP_SOCKETS_MAXPATH]; + + snprintf(discover_port, sizeof(discover_port), "%s.r/%u", + g_cfg->listen_port, + session_get_parameters(g_session_data)->display); + g_discover_trans->is_term = sesexec_is_term; + g_discover_trans->trans_conn_in = discover_trans_conn_in; + if ((rv = trans_listen(g_discover_trans, discover_port)) != 0) + { + LOG(LOG_LEVEL_ERROR, "Transport error enabling discovery [%s]", + g_get_strerror()); + trans_delete(g_discover_trans); + g_discover_trans = NULL; + } + } + + return rv; } /******************************************************************************/ - int sesexec_discover_disable(void) { + trans_delete(g_discover_trans); + g_discover_trans = NULL; + return 0; } @@ -51,13 +161,28 @@ int sesexec_discover_get_wait_objs(intptr_t robjs[], int *robjs_count, int max_count) { - return 0; + int rv; + if (g_discover_trans == NULL) + { + rv = 0; + } + else if (*robjs_count >= max_count) + { + rv = 1; + } + else + { + rv = trans_get_wait_objs(g_discover_trans, robjs, robjs_count); + } + + return rv; } /******************************************************************************/ - int sesexec_discover_check_wait_objs(void) { - return 0; + return (g_discover_trans == NULL) + ? 0 + : trans_check_wait_objs(g_discover_trans); } From 63c2e185c39009e791bdfb33e233d78882a76a0b Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 18 Dec 2024 12:42:11 +0000 Subject: [PATCH 18/19] Fix missing displays on sesman restart --- libipm/ercp.c | 5 +---- libipm/ercp.h | 1 - sesman/ercp_process.c | 17 ++++++++++++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/libipm/ercp.c b/libipm/ercp.c index e048f5fb8f..0085838789 100644 --- a/libipm/ercp.c +++ b/libipm/ercp.c @@ -208,10 +208,7 @@ ercp_get_session_announce_event(struct trans *trans, if (rv == 0) { - if (display != NULL) - { - *display = i_display; - } + *display = i_display; *uid = (uid_t)i_uid; *type = (enum scp_session_type)i_type; *start_width = i_width; diff --git a/libipm/ercp.h b/libipm/ercp.h index c77bf984f1..0f608fd32e 100644 --- a/libipm/ercp.h +++ b/libipm/ercp.h @@ -188,7 +188,6 @@ ercp_send_session_announce_event(struct trans *trans, * * @param trans EICP transport * @param[out] display Display used by session. - * Pointer can be NULL if this is already known. * @param[out] uid UID of user logged in to session * @param[out] type Session type * @param[out] start_width Starting width of seenio diff --git a/sesman/ercp_process.c b/sesman/ercp_process.c index 4ac95990e4..8569b23b75 100644 --- a/sesman/ercp_process.c +++ b/sesman/ercp_process.c @@ -42,9 +42,10 @@ process_session_announce_event(struct session_item *si) { int rv; const char *start_ip_addr; + unsigned int display; rv = ercp_get_session_announce_event(si->sesexec_trans, - NULL, + &display, &si->uid, &si->type, &si->start_width, @@ -53,10 +54,24 @@ process_session_announce_event(struct session_item *si) &si->guid, &start_ip_addr, &si->start_time); + if (rv == 0) + { + // We may already know the display we sent sesexec. If we do, + // check sesexec sent the same value back. + if (si->display >= 0 && display != (unsigned int)si->display) + { + LOG(LOG_LEVEL_ERROR, "Bugcheck: sesman expected display %d, got %u", + si->display, display); + rv = 1; + } + } + if (rv == 0) { snprintf(si->start_ip_addr, sizeof(si->start_ip_addr), "%s", start_ip_addr); + si->display = display; + si->state = E_SESSION_RUNNING; } From 64b6d99740843a67648cf5df3a9ac41ada14d56f Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:34:04 +0000 Subject: [PATCH 19/19] Add commented out KillMode=process to xrdp-service The KillMode of process allows an xrdp connection to survive a reatart of the xrdp process. This is of minimal benefit, as a user can always simply reconnect - the session will survive the restart. The downside is that xrdp will not benefit from any security fixes, and may well end up out-of-sync with sesman. --- instfiles/xrdp.service.in | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/instfiles/xrdp.service.in b/instfiles/xrdp.service.in index 52216cc81d..72bc2e99f3 100644 --- a/instfiles/xrdp.service.in +++ b/instfiles/xrdp.service.in @@ -11,6 +11,13 @@ EnvironmentFile=-@sysconfdir@/default/xrdp ExecStart=@sbindir@/xrdp $XRDP_OPTIONS --nodaemon SystemCallArchitectures=native SystemCallFilter=@system-service +# Uncomment the following line if you wish xrdp connections to survive +# an xrdp restart. +# +# This is not recommended, as the xrdp and xrdp-sesman processes can +# end up with API differences if the restart was caused by an upgrade. It +# may however be useful for some use cases. +#KillMode=process [Install] WantedBy=multi-user.target