From 0f32661056b2263bc2686e22f05509ba8b0ba2ad Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 8 Jun 2023 15:57:17 +0100 Subject: [PATCH 01/22] Add g_strsignal() to string_calls module --- common/string_calls.c | 24 ++++++++++++++++++++++++ common/string_calls.h | 6 ++++++ sesman/sesexec/session.c | 2 +- sesman/sesexec/xwait.c | 4 ++-- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/common/string_calls.c b/common/string_calls.c index de74f9e5d2..14448bbfa9 100644 --- a/common/string_calls.c +++ b/common/string_calls.c @@ -422,6 +422,30 @@ g_atoi(const char *str) return atoi(str); } +/*****************************************************************************/ +char * +g_strsignal(int signum) +{ + char *result = strsignal(signum); + + if (result == NULL) + { + // Using a static buffer offers the same guarantees as the + // strsignal() call + static char buff[32]; + unsigned int len; + len = g_snprintf(buff, sizeof(buff), "SIG#%d", signum); + if (len >= sizeof(buff)) + { + // Buffer overflow + g_snprintf(buff, sizeof(buff), "SIG???"); + } + result = buff; + } + + return result; +} + /*****************************************************************************/ /* As g_atoi() but allows for hexadecimal too */ int diff --git a/common/string_calls.h b/common/string_calls.h index 49092c44fb..7403f11bef 100644 --- a/common/string_calls.h +++ b/common/string_calls.h @@ -266,6 +266,12 @@ int g_strncmp_d(const char *c1, const char *c2, const char delim, int len); int g_strcasecmp(const char *c1, const char *c2); int g_strncasecmp(const char *c1, const char *c2, int len); int g_atoi(const char *str); +/** + * Implements POSIX 1003.1 strsignal() + * + * This function never returns NULL + */ +char *g_strsignal(int signum); /** * Extends g_atoi(), Converts decimal and hexadecimal number String to integer * diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 485631d847..2385de6845 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -821,7 +821,7 @@ exit_status_to_str(const struct exit_status *e, char buff[], int bufflen) break; case E_XR_SIGNAL: - g_snprintf(buff, bufflen, "signal %d", e->val); + g_snprintf(buff, bufflen, "signal \"%s\"", g_strsignal(e->val)); break; default: diff --git a/sesman/sesexec/xwait.c b/sesman/sesexec/xwait.c index 271804e68e..375399ce4b 100644 --- a/sesman/sesexec/xwait.c +++ b/sesman/sesexec/xwait.c @@ -156,8 +156,8 @@ wait_for_xserver(uid_t uid, case E_XR_SIGNAL: LOG(LOG_LEVEL_ERROR, - "waitforx failed with unexpected signal %d", - e.val); + "waitforx failed with unexpected signal \"%s\"", + g_strsignal(e.val)); break; default: From cb9a697fa87fbe039bf05c5b35fe20334d12aba0 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 7 Jun 2023 12:35:24 +0100 Subject: [PATCH 02/22] Log xrdp sub-processes which fail due to a signal --- xrdp/xrdp.c | 15 ++++++++++--- xrdp/xrdp.h | 6 ++++++ xrdp/xrdp_listen.c | 32 +++++++++++++++++++++++++++ xrdp/xrdp_main_utils.c | 49 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 98 insertions(+), 4 deletions(-) diff --git a/xrdp/xrdp.c b/xrdp/xrdp.c index 22065982e3..db0535625d 100644 --- a/xrdp/xrdp.c +++ b/xrdp/xrdp.c @@ -102,9 +102,7 @@ xrdp_shutdown(int sig) static void xrdp_child(int sig) { - while (g_waitchild(NULL) > 0) - { - } + g_set_sigchld(1); } /*****************************************************************************/ @@ -562,6 +560,14 @@ main(int argc, char **argv) LOG(LOG_LEVEL_WARNING, "error creating g_term_event"); } + g_snprintf(text, 255, "xrdp_%8.8x_main_sigchld", pid); + g_set_sigchld_event(g_create_wait_obj(text)); + + if (g_get_sigchld() == 0) + { + LOG(LOG_LEVEL_WARNING, "error creating g_sigchld_event"); + } + g_snprintf(text, 255, "xrdp_%8.8x_main_sync", pid); g_set_sync_event(g_create_wait_obj(text)); @@ -583,6 +589,9 @@ main(int argc, char **argv) g_delete_wait_obj(g_get_term()); g_set_term_event(0); + g_delete_wait_obj(g_get_sigchld()); + g_set_sigchld_event(0); + g_delete_wait_obj(g_get_sync_event()); g_set_sync_event(0); diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index 8a85575487..f06eb1a90c 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -55,6 +55,8 @@ g_set_sync1_mutex(long mutex); void g_set_term_event(tbus event); void +g_set_sigchld_event(tbus event); +void g_set_sync_event(tbus event); long g_get_threadid(void); @@ -62,10 +64,14 @@ void g_set_threadid(long id); tbus g_get_term(void); +tbus +g_get_sigchld(void); int g_is_term(void); void g_set_term(int in_val); +void +g_set_sigchld(int in_val); tbus g_get_sync_event(void); void diff --git a/xrdp/xrdp_listen.c b/xrdp/xrdp_listen.c index 3543bac400..d9a6506aeb 100644 --- a/xrdp/xrdp_listen.c +++ b/xrdp/xrdp_listen.c @@ -846,6 +846,29 @@ xrdp_listen_conn_in(struct trans *self, struct trans *new_self) return 0; } +/*****************************************************************************/ +/** + * Process pending SIGCHLD events in the listen process + * + * The main reason for this is to log children which fail + * on a signal. This should be investigated. + */ +static void +process_pending_sigchld_events(void) +{ + struct exit_status e; + int pid; + + while ((pid = g_waitchild(&e)) > 0) + { + if (e.reason == E_XR_SIGNAL) + { + LOG(LOG_LEVEL_ERROR, + "Child %d terminated unexpectedly with signal \"%s\"", + pid, g_strsignal(e.val)); + } + } +} /*****************************************************************************/ /* wait for incoming connections passes through trans_listen_address return value */ @@ -858,6 +881,7 @@ xrdp_listen_main_loop(struct xrdp_listen *self) int timeout; intptr_t robjs[32]; intptr_t term_obj; + intptr_t sigchld_obj; intptr_t sync_obj; intptr_t done_obj; struct trans *ltrans; @@ -876,6 +900,7 @@ xrdp_listen_main_loop(struct xrdp_listen *self) return 1; } term_obj = g_get_term(); /*Global termination event */ + sigchld_obj = g_get_sigchld(); sync_obj = g_get_sync_event(); done_obj = self->pro_done_event; cont = 1; @@ -884,6 +909,7 @@ xrdp_listen_main_loop(struct xrdp_listen *self) /* build the wait obj list */ robjs_count = 0; robjs[robjs_count++] = term_obj; + robjs[robjs_count++] = sigchld_obj; robjs[robjs_count++] = sync_obj; robjs[robjs_count++] = done_obj; timeout = -1; @@ -918,6 +944,12 @@ xrdp_listen_main_loop(struct xrdp_listen *self) break; } + if (g_is_wait_obj_set(sigchld_obj)) /* SIGCHLD caught */ + { + g_set_sigchld(0); + process_pending_sigchld_events(); + } + /* some function must be processed by this thread */ if (g_is_wait_obj_set(sync_obj)) { diff --git a/xrdp/xrdp_main_utils.c b/xrdp/xrdp_main_utils.c index 9c4fd706ed..0b1bc36477 100644 --- a/xrdp/xrdp_main_utils.c +++ b/xrdp/xrdp_main_utils.c @@ -39,6 +39,7 @@ static long g_threadid = 0; /* main threadid */ static long g_sync_mutex = 0; static long g_sync1_mutex = 0; static tbus g_term_event = 0; +static tbus g_sigchld_event = 0; static tbus g_sync_event = 0; /* synchronize stuff */ static int g_sync_command = 0; @@ -104,6 +105,19 @@ g_xrdp_sync(long (*sync_func)(long param1, long param2), long sync_param1, return sync_result; } +/*****************************************************************************/ +/* Signal handler for SIGCHLD in the child + * Note: only signal safe code (eg. setting wait event) should be executed in + * this function. For more details see `man signal-safety` + */ +static void +xrdp_child_sigchld_handler(int sig) +{ + while (g_waitchild(NULL) > 0) + { + } +} + /*****************************************************************************/ /* called in child just after fork */ int @@ -112,12 +126,17 @@ xrdp_child_fork(void) int pid; char text[256]; - /* close, don't delete these */ + /* SIGCHLD in the child is of no interest to us */ + g_signal_child_stop(xrdp_child_sigchld_handler); /* SIGCHLD */ + g_close_wait_obj(g_term_event); + g_close_wait_obj(g_sigchld_event); g_close_wait_obj(g_sync_event); + pid = g_getpid(); g_snprintf(text, 255, "xrdp_%8.8x_main_term", pid); g_term_event = g_create_wait_obj(text); + g_sigchld_event = -1; g_snprintf(text, 255, "xrdp_%8.8x_main_sync", pid); g_sync_event = g_create_wait_obj(text); return 0; @@ -158,6 +177,13 @@ g_set_term_event(tbus event) g_term_event = event; } +/*****************************************************************************/ +void +g_set_sigchld_event(tbus event) +{ + g_sigchld_event = event; +} + /*****************************************************************************/ tbus g_get_sync_event(void) @@ -193,6 +219,13 @@ g_get_term(void) return g_term_event; } +/*****************************************************************************/ +tbus +g_get_sigchld(void) +{ + return g_sigchld_event; +} + /*****************************************************************************/ int g_is_term(void) @@ -214,6 +247,20 @@ g_set_term(int in_val) } } +/*****************************************************************************/ +void +g_set_sigchld(int in_val) +{ + if (in_val) + { + g_set_wait_obj(g_sigchld_event); + } + else + { + g_reset_wait_obj(g_sigchld_event); + } +} + /*****************************************************************************/ /*Some function must be called from the main thread. if g_sync_command==THREAD_WAITING a function is waiting to be processed*/ From 9bf78e4a8094b50c7de1cd7267fc6b16cd391350 Mon Sep 17 00:00:00 2001 From: Daniel Richard G Date: Mon, 22 May 2023 15:53:40 -0400 Subject: [PATCH 03/22] Add syscall filtering to xrdp systemd unit --- instfiles/xrdp.service.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instfiles/xrdp.service.in b/instfiles/xrdp.service.in index db52e31af8..814d4de590 100644 --- a/instfiles/xrdp.service.in +++ b/instfiles/xrdp.service.in @@ -9,6 +9,8 @@ Type=exec EnvironmentFile=-@sysconfdir@/sysconfig/xrdp EnvironmentFile=-@sysconfdir@/default/xrdp ExecStart=@sbindir@/xrdp $XRDP_OPTIONS --nodaemon +SystemCallArchitectures=native +SystemCallFilter=@basic-io @file-system @io-event @ipc @network-io @process @signal ioctl madvise sysinfo uname [Install] WantedBy=multi-user.target From 5a02a3b3d9f88f22acd1cfb917e3cf17624b49cb Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Mon, 29 May 2023 00:07:54 +0900 Subject: [PATCH 04/22] Add issue template: links --- .github/ISSUE_TEMPLATE/config.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/config.yml diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000000..ae365ed8d3 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +contact_links: + - name: Questions + about: If you are new to xrdp and want to ask something, raise it as Q&A in discussion. + url: https://github.com/neutrinolabs/xrdp/discussions/new?category=q-a + From 650dc032ce481ca174ab23f514df878d139b7d1f Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Fri, 9 Jun 2023 21:43:12 +0900 Subject: [PATCH 05/22] Add bug report template --- .github/ISSUE_TEMPLATE/bug_report.yml | 84 +++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000000..82ea4bdf3e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,84 @@ +name: "🕷️ Bug report" +description: fofofof +labels: + - "bug" +body: + - type: input + attributes: + label: xrdp version + placeholder: 0.9.20 + validations: + required: true + - type: textarea + attributes: + label: Detailed xrdp version, build options + description: Copy & paste the result of `xrdp --version`. DO NOT remove `~~~` but paste the result between two `~~~`. + value: | + ~~~ + PASTE HERE + ~~~ + - type: input + attributes: + label: Operating system & version + placeholder: "Ubuntu 22.04 / AlmaLinux 9 / FreeBSD 13.2 / etc" + description: Tell us about your operating system. See PRETTY_NAME in /etc/os-release if you don't know. + validations: + required: true + - type: dropdown + attributes: + label: Installation method + description: How was xrdp installed from? + multiple: true + options: + - dnf / apt / zypper / pkg / etc + - Homebrew / MacPorts + - git clone & make install + - other + validations: + required: true + - type: input + attributes: + label: Which backend do you use? + description: + - type: input + attributes: + label: What's your client? + description: If you issue occurs with specific clients, tell us the client app name, app version client os version and platform. + placeholder: Microsoft's official client from Mac App Store, running on macOS Ventura. + - type: dropdown + attributes: + label: Area(s) with issue? + multiple: true + options: + - Audio redirection + - Authentication + - Crashes such as segfault + - Clipboard + - Compatiblity aginst clients + - Compile error + - File transfer / drive redirection + - Graphic glitches + - Keyboard / Mouse + - Network + - Performance + - Session manager (sesman) + - Smartcard + - Other + - type: textarea + attributes: + label: Steps to reproduce + placeholder: Having detailed steps helps us reproduce the bug. + validations: + required: true + - type: textarea + attributes: + label: ✔️ Expected Behavior + placeholder: What were you expecting? + validations: + required: false + - type: textarea + attributes: + label: ❌ Actual Behavior + placeholder: What happened instead? + validations: + required: false From 07797866e33e9ee106a6d4424c0fab585e47d028 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Sat, 10 Jun 2023 00:07:01 +0900 Subject: [PATCH 06/22] Update bug report template --- .github/ISSUE_TEMPLATE/bug_report.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 82ea4bdf3e..e0bed14c2b 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -1,5 +1,5 @@ name: "🕷️ Bug report" -description: fofofof +description: Report errors or unexpected behavior labels: - "bug" body: @@ -28,11 +28,11 @@ body: attributes: label: Installation method description: How was xrdp installed from? - multiple: true options: - dnf / apt / zypper / pkg / etc - Homebrew / MacPorts - git clone & make install + - Doesn't matter - other validations: required: true @@ -48,6 +48,7 @@ body: - type: dropdown attributes: label: Area(s) with issue? + description: What things had an issue? Check all that apply. multiple: true options: - Audio redirection From afb34f23a4603a529662b6b182270b83b756b849 Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Sat, 10 Jun 2023 00:26:20 +0900 Subject: [PATCH 07/22] Reword descriptions for the link to Q&A --- .github/ISSUE_TEMPLATE/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index ae365ed8d3..57f99e839e 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,5 +1,5 @@ contact_links: - name: Questions - about: If you are new to xrdp and want to ask something, raise it as Q&A in discussion. + about: If you are new to xrdp and want to ask community for help, raise it as Q&A in discussion. url: https://github.com/neutrinolabs/xrdp/discussions/new?category=q-a From ce42e3e12d22e61de2891b75dcc7e00bdb534fb4 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 12 Jun 2023 15:40:22 +0100 Subject: [PATCH 08/22] Replace g_strsignal() with g_sig2text() This call provides a textual representation of a signal number, i.e. SIGHUP is mapped to "SIGHUP" Unit tests are also added. --- common/string_calls.c | 169 ++++++++++++++++++++++++++----- common/string_calls.h | 40 ++++++-- sesman/sesexec/session.c | 8 +- sesman/sesexec/xwait.c | 9 +- tests/common/test_string_calls.c | 80 +++++++++++++++ xrdp/xrdp_listen.c | 5 +- 6 files changed, 274 insertions(+), 37 deletions(-) diff --git a/common/string_calls.c b/common/string_calls.c index 14448bbfa9..1ba35613bd 100644 --- a/common/string_calls.c +++ b/common/string_calls.c @@ -21,6 +21,7 @@ #if defined(HAVE_CONFIG_H) #include "config_ac.h" #endif +#include #include #include #include @@ -422,30 +423,6 @@ g_atoi(const char *str) return atoi(str); } -/*****************************************************************************/ -char * -g_strsignal(int signum) -{ - char *result = strsignal(signum); - - if (result == NULL) - { - // Using a static buffer offers the same guarantees as the - // strsignal() call - static char buff[32]; - unsigned int len; - len = g_snprintf(buff, sizeof(buff), "SIG#%d", signum); - if (len >= sizeof(buff)) - { - // Buffer overflow - g_snprintf(buff, sizeof(buff), "SIG???"); - } - result = buff; - } - - return result; -} - /*****************************************************************************/ /* As g_atoi() but allows for hexadecimal too */ int @@ -1167,3 +1144,147 @@ g_charstr_to_bitmask(const char *str, const struct bitmask_char bitdefs[], return bitmask; } +/*****************************************************************************/ +/* + * Looks for a simple mapping of signal number to name + */ +static const char * +find_sig_name(int signum) +{ + typedef struct + { + int num; + const char *name; + } sig_to_name_type; + + // Map a string 'zzz' to { SIGzzz, "zzz"} for making + // typo-free sig_to_name_type objects +# define DEFSIG(sig) { SIG ## sig, # sig } + + // Entries in this array are taken from + // The Single UNIX ® Specification, Version 2 (1997) + // plus additions from specific operating systems. + // + // The SUS requires these to be positive integer constants with a + // macro definition. Note that SIGRTMIN and SIGRTMAX on Linux are + // NOT constants, so have to be handled separately. + static const sig_to_name_type sigmap[] = + { + // Names from SUS v2, in the order they are listed in that document + // that *should* be defined everywhere + // + // Commented out definitions below are NOT used everywhere + DEFSIG(ABRT), DEFSIG(ALRM), DEFSIG(FPE), DEFSIG(HUP), + DEFSIG(ILL), DEFSIG(INT), DEFSIG(KILL), DEFSIG(PIPE), + DEFSIG(QUIT), DEFSIG(SEGV), DEFSIG(TERM), DEFSIG(USR1), + DEFSIG(USR2), DEFSIG(CHLD), DEFSIG(CONT), DEFSIG(STOP), + DEFSIG(TSTP), DEFSIG(TTIN), DEFSIG(TTOU), DEFSIG(BUS), + /* DEFSIG(POLL), */ /* DEFSIG(PROF), */ DEFSIG(SYS), DEFSIG(TRAP), + DEFSIG(URG), DEFSIG(VTALRM), DEFSIG(XCPU), DEFSIG(XFSZ), + + // SIGPOLL and SIGPROF are marked as obselescent in 1003.1-2017, + // Also SIGPOLL isn't in *BSD operating systems which use SIGIO +#ifdef SIGPOLL + DEFSIG(POLL), +#endif +#ifdef SIGPROF + DEFSIG(PROF), +#endif + + // BSD signals (from FreeBSD/OpenBSD sys/signal.h and + // Darwin/Illumos signal.h) +#ifdef SIGEMT + DEFSIG(EMT), +#endif +#ifdef SIGIO + DEFSIG(IO), +#endif +#ifdef SIGWINCH + DEFSIG(WINCH), +#endif +#ifdef SIGINFO + DEFSIG(INFO), +#endif +#ifdef SIGTHR + DEFSIG(THR), +#endif +#ifdef SIGLIBRT + DEFSIG(LIBRT), +#endif +#ifdef SIGPWR + DEFSIG(PWR), +#endif +#ifdef SIGWAITING + DEFSIG(WAITING), +#endif +#ifdef SIGLWP + DEFSIG(LWP), +#endif + + // Linux additions to *BSD (signal(7)) +#ifdef SIGLOST + DEFSIG(LOST), +#endif +#ifdef SIGSTKFLT + DEFSIG(STKFLT), +#endif + + // Terminator + {0, NULL} +#undef DEFSIG + }; + + const sig_to_name_type *p; + + for (p = &sigmap[0] ; p->name != NULL ; ++p) + { + if (p->num == signum) + { + return p->name; + } + } + + // These aren't constants on Linux +#ifdef SIGRTMIN + if (signum == SIGRTMIN) + { + return "RTMIN"; + } +#endif +#ifdef SIGRTMAX + if (signum == SIGRTMAX) + { + return "RTMAX"; + } +#endif + + return NULL; +} + +/*****************************************************************************/ +char * +g_sig2text(int signum, char sigstr[]) +{ + if (signum >= 0) + { + const char *name = find_sig_name(signum); + + if (name != NULL) + { + g_snprintf(sigstr, MAXSTRSIGLEN, "SIG%s", name); + return sigstr; + } + +#if defined(SIGRTMIN) && defined(SIGRTMAX) + if (signum > SIGRTMIN && signum < SIGRTMAX) + { + g_snprintf(sigstr, MAXSTRSIGLEN, "SIGRTMIN+%d", signum - SIGRTMIN); + return sigstr; + } +#endif + } + + // If all else fails... + g_snprintf(sigstr, MAXSTRSIGLEN, "SIG#%d", signum); + return sigstr; +} diff --git a/common/string_calls.h b/common/string_calls.h index 7403f11bef..b4ab8c9613 100644 --- a/common/string_calls.h +++ b/common/string_calls.h @@ -67,6 +67,26 @@ struct bitmask_char #define BITMASK_CHAR_END_OF_LIST { 0, '\0' } +enum +{ + // See g_sig2text() + // Must be able to hold "SIG#%d" for INT_MIN + // + // ((sizeof(int) * 5 + 1) / 2) provides a very slight overestimate of + // the bytes requires to store a decimal expansion of 'int':- + // sizeof INT_MAX display bytes ((sizeof(int) * 5 + 1) + // (int) needed / 2) + // ------ ------- ------------- --------------------------- + // 1 127 3 3 + // 2 32767 5 5 + // 3 8388607 7 8 + // 4 2147483637 10 10 + // 8 9*(10**18) 19 20 + // 16 2*(10**38) 39 40 + // 32 6*(10**76) 77 80 + MAXSTRSIGLEN = (3 + 1 + 1 + ((sizeof(int) * 5 + 1) / 2) + 1) +}; + /** * Processes a format string for general info * @@ -266,12 +286,7 @@ int g_strncmp_d(const char *c1, const char *c2, const char delim, int len); int g_strcasecmp(const char *c1, const char *c2); int g_strncasecmp(const char *c1, const char *c2, int len); int g_atoi(const char *str); -/** - * Implements POSIX 1003.1 strsignal() - * - * This function never returns NULL - */ -char *g_strsignal(int signum); + /** * Extends g_atoi(), Converts decimal and hexadecimal number String to integer * @@ -289,4 +304,17 @@ char *g_strstr(const char *haystack, const char *needle); int g_mbstowcs(twchar *dest, const char *src, int n); int g_wcstombs(char *dest, const twchar *src, int n); int g_strtrim(char *str, int trim_flags); + +/** + * Maps a signal number to a string, i.e. SIGHUP -> "SIGHUP" + * + * @param signum Signal number + * @param sigstr buffer for result + * @return sigstr, for convenience + * + * Buffer is assumed to be at least MAXSTRSIGLEN + * + * The string "SIG#" is returned for unrecognised signums + */ +char *g_sig2text(int signum, char sigstr[]); #endif diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 2385de6845..6836d5c47d 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -821,8 +821,12 @@ exit_status_to_str(const struct exit_status *e, char buff[], int bufflen) break; case E_XR_SIGNAL: - g_snprintf(buff, bufflen, "signal \"%s\"", g_strsignal(e->val)); - break; + { + char sigstr[MAXSTRSIGLEN]; + g_snprintf(buff, bufflen, "signal %s", + g_sig2text(e->val, sigstr)); + } + break; default: g_snprintf(buff, bufflen, "an unexpected error"); diff --git a/sesman/sesexec/xwait.c b/sesman/sesexec/xwait.c index 375399ce4b..9a7bd376f6 100644 --- a/sesman/sesexec/xwait.c +++ b/sesman/sesexec/xwait.c @@ -155,10 +155,13 @@ wait_for_xserver(uid_t uid, break; case E_XR_SIGNAL: + { + char sigstr[MAXSTRSIGLEN]; LOG(LOG_LEVEL_ERROR, - "waitforx failed with unexpected signal \"%s\"", - g_strsignal(e.val)); - break; + "waitforx failed with unexpected signal %s", + g_sig2text(e.val, sigstr)); + } + break; default: LOG(LOG_LEVEL_ERROR, diff --git a/tests/common/test_string_calls.c b/tests/common/test_string_calls.c index 0f7d626859..0224080906 100644 --- a/tests/common/test_string_calls.c +++ b/tests/common/test_string_calls.c @@ -3,6 +3,10 @@ #include "config_ac.h" #endif +#include +#include + +#include "os_calls.h" #include "string_calls.h" #include "ms-rdpbcgr.h" @@ -1036,6 +1040,76 @@ END_TEST /******************************************************************************/ +START_TEST(test_sigs__common) +{ + char name[MAXSTRSIGLEN]; + char *res; + + // Check some common POSIX signals + res = g_sig2text(SIGHUP, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIGHUP"); + + res = g_sig2text(SIGCHLD, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIGCHLD"); + + res = g_sig2text(SIGXFSZ, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIGXFSZ"); + + res = g_sig2text(SIGRTMIN, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIGRTMIN"); + + res = g_sig2text(SIGRTMIN + 2, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIGRTMIN+2"); + + res = g_sig2text(SIGRTMAX, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIGRTMAX"); + + // Should be invalid + res = g_sig2text(0, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIG#0"); + + res = g_sig2text(65535, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIG#65535"); + + + // POSIX defines signals as ints, but insists they are positive. So + // we ought to trest we get sane behaviour for -ve numbers + res = g_sig2text(-1, name); + ck_assert_ptr_eq(res, name); + ck_assert_str_eq(res, "SIG#-1"); +} +END_TEST + +START_TEST(test_sigs__bigint) +{ + char name[MAXSTRSIGLEN]; + char name2[1024]; + + // Check that big integers aren't being truncated by the definition + // of MAXSTRSIGLEN + int i = INT_MAX; + + g_sig2text(i, name); + g_snprintf(name2, sizeof(name2), "SIG#%d", i); + ck_assert_str_eq(name, name2); + + i = INT_MIN; + g_sig2text(i, name); + g_snprintf(name2, sizeof(name2), "SIG#%d", i); + ck_assert_str_eq(name, name2); +} +END_TEST + +/******************************************************************************/ + Suite * make_suite_test_string(void) { @@ -1046,6 +1120,7 @@ make_suite_test_string(void) TCase *tc_bm2char; TCase *tc_char2bm; TCase *tc_strtrim; + TCase *tc_sigs; s = suite_create("String"); @@ -1118,5 +1193,10 @@ make_suite_test_string(void) tcase_add_test(tc_strtrim, test_strtrim__trim_both); tcase_add_test(tc_strtrim, test_strtrim__trim_through); + tc_sigs = tcase_create("signals"); + suite_add_tcase(s, tc_sigs); + tcase_add_test(tc_sigs, test_sigs__common); + tcase_add_test(tc_sigs, test_sigs__bigint); + return s; } diff --git a/xrdp/xrdp_listen.c b/xrdp/xrdp_listen.c index d9a6506aeb..5dd9c19b56 100644 --- a/xrdp/xrdp_listen.c +++ b/xrdp/xrdp_listen.c @@ -863,9 +863,10 @@ process_pending_sigchld_events(void) { if (e.reason == E_XR_SIGNAL) { + char sigstr[MAXSTRSIGLEN]; LOG(LOG_LEVEL_ERROR, - "Child %d terminated unexpectedly with signal \"%s\"", - pid, g_strsignal(e.val)); + "Child %d terminated unexpectedly with signal %s", + pid, g_sig2text(e.val, sigstr)); } } } From 416615bfb5f43b4dd1e7af5b4d4b1dc7c5bd485d Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Tue, 13 Jun 2023 09:38:54 +0900 Subject: [PATCH 09/22] Add field for desktop environment, backend, VM and anything else in bug report template --- .github/ISSUE_TEMPLATE/bug_report.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index e0bed14c2b..f197c55487 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -39,7 +39,17 @@ body: - type: input attributes: label: Which backend do you use? - description: + description: Tell us about xrdp backend and version you're using. Typically, it would be either Xvnc or xorgxrdp or rarely NeutrionRDP / FreeRDP. + placeholder: Xvnc (tigervnc-1.12.0-13.el9_2) + - type: input + attributes: + label: What desktop environment do you use? + description: Tell us about your desktop (e.g. GNOME / KDE / Xfce / xterm). If you're certain the bug you about to report is not desktop specific, fill "any" here. + placeholder: GNOME + - type: input + attributes: + label: Environment xrdp running on + description: Tell us whether xrdp is running on a VM, or if on a physical machine what graphics cards are installed. - type: input attributes: label: What's your client? @@ -83,3 +93,8 @@ body: placeholder: What happened instead? validations: required: false + - type: textarea + attributes: + label: Anything else? + description: + Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. From 6e192995cb85710ad84b8b302c602e3f3ecd0b2a Mon Sep 17 00:00:00 2001 From: Koichiro IWAO Date: Tue, 13 Jun 2023 09:50:25 +0900 Subject: [PATCH 10/22] Tip is not allowed here anymore https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-issue-forms --- .github/ISSUE_TEMPLATE/bug_report.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index f197c55487..7975033bf6 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -96,5 +96,4 @@ body: - type: textarea attributes: label: Anything else? - description: - Tip: You can attach images or log files by clicking this area to highlight it and then dragging files in. + description: Links? References? Anything that will give us more context about the issue you are encountering! We recommend attaching `xrdp.log`, `xrdp-sesman.log`, `xrdp/xorg.conf` or screenshots to clarify context. From d77b0b3b9d0d07b63c40a910ef6ce1c5c22995ac Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 23 Jun 2023 14:56:41 +0100 Subject: [PATCH 11/22] Bump cppcheck to v2.11 This fixes the following errors:- sesman/tools/authtest.c:64:14: error: syntax error [syntaxError] g_printf("xrdp auth module tester v" PACKAGE_VERSION "\n"); ^ sesman/tools/sesrun.c:165:14: error: syntax error [syntaxError] g_printf("xrdp session starter v" PACKAGE_VERSION "\n"); ^ vrplayer/decoder.h:35:12: error: There is an unknown macro here somewhere. Configuration is required. If slots is a macro then please configure it. [unknownMacro] public slots: ^ vrplayer/playaudio.h:45:12: error: There is an unknown macro here somewhere. Configuration is required. If slots is a macro then please configure it. [unknownMacro] public slots: ^ vrplayer/dlgabout.h:22:13: error: There is an unknown macro here somewhere. Configuration is required. If slots is a macro then please configure it. [unknownMacro] private slots: ^ vrplayer/playvideo.h:49:12: error: There is an unknown macro here somewhere. Configuration is required. If slots is a macro then please configure it. [unknownMacro] public slots: ^ Additionally, cppcheck now makes use of all available CPUs --- .github/workflows/build.yml | 2 +- librfxcodec | 2 +- scripts/run_cppcheck.sh | 8 +++++++- sesman/tools/authtest.c | 5 +++++ sesman/tools/sesrun.c | 5 +++++ vrplayer/decoder.h | 2 +- vrplayer/dlgabout.h | 2 +- vrplayer/playaudio.h | 2 +- vrplayer/playvideo.h | 2 +- 9 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 979a851b1f..949fccabe1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -165,7 +165,7 @@ jobs: CC: gcc # This is required to use a version of cppcheck other than that # supplied with the operating system - CPPCHECK_VER: "2.10" + CPPCHECK_VER: "2.11" CPPCHECK_REPO: https://github.com/danmar/cppcheck.git steps: # Set steps.os.outputs.image to the specific OS (e.g. 'ubuntu20') diff --git a/librfxcodec b/librfxcodec index 30f6ce185c..25f6128026 160000 --- a/librfxcodec +++ b/librfxcodec @@ -1 +1 @@ -Subproject commit 30f6ce185c84edc6350bce0d6eae1a16edd2b1a3 +Subproject commit 25f6128026bddf7d3588f5f5466603c87ac6c0b3 diff --git a/scripts/run_cppcheck.sh b/scripts/run_cppcheck.sh index adad9d488d..545af41b2e 100755 --- a/scripts/run_cppcheck.sh +++ b/scripts/run_cppcheck.sh @@ -45,10 +45,16 @@ if [ -z "$CPPCHECK_FLAGS" ]; then CPPCHECK_FLAGS="--quiet --force --std=c11 --std=c++11 --inline-suppr \ --enable=warning --error-exitcode=1 -i third_party" fi +CPPCHECK_FLAGS="$CPPCHECK_FLAGS -D__cppcheck__" # Any options/directories specified? if [ $# -eq 0 ]; then - set -- -j 2 . + if [ -f /proc/cpuinfo ]; then + cpus=$(grep '^processor' /proc/cpuinfo | wc -l) + else + cpus=2 + fi + set -- -j $cpus . fi # Display the cppcheck version and command for debugging diff --git a/sesman/tools/authtest.c b/sesman/tools/authtest.c index 48d8cca296..9224204b95 100644 --- a/sesman/tools/authtest.c +++ b/sesman/tools/authtest.c @@ -35,6 +35,11 @@ #include "os_calls.h" #include "string_calls.h" +// cppcheck doesn't always set this macro to something in double-quotes +#if defined(__cppcheck__) +#undef PACKAGE_VERSION +#endif + #if !defined(PACKAGE_VERSION) #define PACKAGE_VERSION "???" #endif diff --git a/sesman/tools/sesrun.c b/sesman/tools/sesrun.c index 46df4e4a26..e326b07353 100644 --- a/sesman/tools/sesrun.c +++ b/sesman/tools/sesrun.c @@ -42,6 +42,11 @@ #include "tools_common.h" +// cppcheck doesn't always set this macro to something in double-quotes +#if defined(__cppcheck__) +#undef PACKAGE_VERSION +#endif + #if !defined(PACKAGE_VERSION) #define PACKAGE_VERSION "???" #endif diff --git a/vrplayer/decoder.h b/vrplayer/decoder.h index d66b921496..c295b69887 100644 --- a/vrplayer/decoder.h +++ b/vrplayer/decoder.h @@ -32,7 +32,7 @@ class Decoder : public QObject signals: - public slots: + public slots: // cppcheck-suppress unknownMacro void onGeometryChanged(QRect *geometry); }; diff --git a/vrplayer/dlgabout.h b/vrplayer/dlgabout.h index 1960670a99..9706e3e396 100644 --- a/vrplayer/dlgabout.h +++ b/vrplayer/dlgabout.h @@ -19,7 +19,7 @@ class DlgAbout : public QDialog private: Ui::DlgAbout *ui; - private slots: + private slots: // cppcheck-suppress unknownMacro void onOk(); }; diff --git a/vrplayer/playaudio.h b/vrplayer/playaudio.h index 0a05d41fc0..8e91dcc54f 100644 --- a/vrplayer/playaudio.h +++ b/vrplayer/playaudio.h @@ -42,7 +42,7 @@ class PlayAudio : public QObject void setVcrOp(int op); - public slots: + public slots: // cppcheck-suppress unknownMacro void play(); private: diff --git a/vrplayer/playvideo.h b/vrplayer/playvideo.h index b63bc1801e..e0285d5e01 100644 --- a/vrplayer/playvideo.h +++ b/vrplayer/playvideo.h @@ -46,7 +46,7 @@ class PlayVideo : public QObject //void setVcrOp(int op); //void onMediaRestarted(); - public slots: + public slots: // cppcheck-suppress unknownMacro void play(); //signals: From 1ac6206af68bc8c4a21a7a93c24276347a26a110 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 23 Jun 2023 15:30:24 +0100 Subject: [PATCH 12/22] Use all available CPUs to build cppcheck --- scripts/install_cppcheck.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/scripts/install_cppcheck.sh b/scripts/install_cppcheck.sh index db4f38801d..24ff921057 100755 --- a/scripts/install_cppcheck.sh +++ b/scripts/install_cppcheck.sh @@ -140,6 +140,14 @@ fi fi esac + # Use all available CPUs + if [ -f /proc/cpuinfo ]; then + cpus=`grep ^processor /proc/cpuinfo | wc -l` + if [ -n "$cpus" ]; then + make_args="$make_args -j $cpus" + fi + fi + echo "Making cppcheck..." # CFGDIR is needed for cppcheck before 1.86 call_make $make_args From 90c80ce855dc3b81dd8ce1d53325527bb292bc6a Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 31 Jul 2023 15:08:50 +0100 Subject: [PATCH 13/22] Exclude RHEL from supported operating systems --- .github/ISSUE_TEMPLATE/bug_report.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 7975033bf6..755937124c 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -21,7 +21,12 @@ body: attributes: label: Operating system & version placeholder: "Ubuntu 22.04 / AlmaLinux 9 / FreeBSD 13.2 / etc" - description: Tell us about your operating system. See PRETTY_NAME in /etc/os-release if you don't know. + description: Tell us about your operating system. See PRETTY_NAME + in /etc/os-release if you don't know. + + Note we are currently unable to provide direct support for Red Hat + Enterprise Linux. Either reproduce the issue on CentOS/Alma/Rocky + OS, or contact Red Hat directly for support. validations: required: true - type: dropdown From 45ca9fe09854a976838088de31751d0e2103fd46 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 4 Aug 2023 16:43:42 +0100 Subject: [PATCH 14/22] clipboard: Tell the X11 client if a selection is unavailable --- sesman/chansrv/clipboard.c | 178 ++++++++++++++++++++++++------------- 1 file changed, 118 insertions(+), 60 deletions(-) diff --git a/sesman/chansrv/clipboard.c b/sesman/chansrv/clipboard.c index d5422483bb..ea68223aeb 100644 --- a/sesman/chansrv/clipboard.c +++ b/sesman/chansrv/clipboard.c @@ -1157,12 +1157,15 @@ clipboard_process_data_request(struct stream *s, int clip_msg_status, return 0; } -/*****************************************************************************/ -/* client to server */ -/* sent as a reply to CB_FORMAT_DATA_REQUEST; used to indicate whether - processing of the CB_FORMAT_DATA_REQUEST was successful; if processing - was successful, CB_FORMAT_DATA_RESPONSE includes contents of requested - clipboard data. */ +/**************************************************************************//** + * Process a CB_FORMAT_DATA_RESPONSE for an X client requesting an image + * + * @param s Stream containing CLIPRDR_FILELIST ([MS-RDPECLIP]) + * @param clip_msg_status msgFlags from Clipboard PDU Header + * @param clip_msg_len dataLen from Clipboard PDU Header + * + * @return Status + */ static int clipboard_process_data_response_for_image(struct stream *s, int clip_msg_status, @@ -1289,6 +1292,90 @@ clipboard_process_data_response_for_file(struct stream *s, return rv; } +/**************************************************************************//** + * Process a CB_FORMAT_DATA_RESPONSE for an X client requesting text + * + * @param s Stream containing CLIPRDR_FILELIST ([MS-RDPECLIP]) + * @param clip_msg_status msgFlags from Clipboard PDU Header + * @param clip_msg_len dataLen from Clipboard PDU Header + * + * @return Status + */ +static int +clipboard_process_data_response_for_text(struct stream *s, + int clip_msg_status, + int clip_msg_len) +{ + XSelectionRequestEvent *lxev = &g_saved_selection_req_event; + twchar *wtext; + twchar wchr; + int len; + int index; + int byte_count; + + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_response_for_text: "); + len = (int)(s->end - s->p); + if (len < 1) + { + len = 0; + } + byte_count = ((len / 2) + 1) * sizeof(twchar); + wtext = (twchar *) g_malloc(byte_count, 0); + if (wtext == 0) + { + LOG(LOG_LEVEL_ERROR, "Can't allocate %d bytes for text clip response", + byte_count); + + clipboard_refuse_selection(lxev); + } + else + { + index = 0; + while (s_check_rem(s, 2)) + { + in_uint16_le(s, wchr); + wtext[index] = wchr; + if (wchr == 0) + { + break; + } + index++; + } + wtext[index] = 0; + g_free(g_clip_c2s.data); + g_clip_c2s.data = 0; + g_clip_c2s.total_bytes = 0; + len = g_wcstombs(0, wtext, 0); + if (len < 0) + { + LOG(LOG_LEVEL_ERROR, + "Received malformed Unicode paste text from client"); + clipboard_refuse_selection(lxev); + } + else + { + byte_count = len + 16; + g_clip_c2s.data = (char *) g_malloc(byte_count, 0); + if (g_clip_c2s.data == 0) + { + LOG(LOG_LEVEL_ERROR, + "Can't allocate %d bytes for text clip response", + byte_count); + clipboard_refuse_selection(lxev); + } + else + { + g_wcstombs(g_clip_c2s.data, wtext, len + 1); + g_clip_c2s.total_bytes = g_strlen(g_clip_c2s.data); + g_clip_c2s.read_bytes_done = g_clip_c2s.total_bytes; + clipboard_provide_selection_c2s(lxev, lxev->target); + } + } + g_free(wtext); + } + return 0; +} + /*****************************************************************************/ /* client to server */ /* sent as a reply to CB_FORMAT_DATA_REQUEST; used to indicate whether @@ -1300,75 +1387,46 @@ static int clipboard_process_data_response(struct stream *s, int clip_msg_status, int clip_msg_len) { - XSelectionRequestEvent *lxev; - twchar *wtext; - twchar wchr; - int len; - int index; + int rv = 0; + + XSelectionRequestEvent *lxev = &g_saved_selection_req_event; LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_response:"); - lxev = &g_saved_selection_req_event; g_clip_c2s.in_request = 0; - if (g_clip_c2s.xrdp_clip_type == XRDP_CB_BITMAP) - { - clipboard_process_data_response_for_image(s, clip_msg_status, - clip_msg_len); - return 0; - } - if (g_clip_c2s.xrdp_clip_type == XRDP_CB_FILE) - { - clipboard_process_data_response_for_file(s, clip_msg_status, - clip_msg_len); - return 0; - } - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_response: " - "CLIPRDR_DATA_RESPONSE"); - len = (int)(s->end - s->p); - if (len < 1) + + if ((clip_msg_status & CB_RESPONSE_FAIL) != 0) { - return 0; + /* Requested data was not returned from the client. Most likely + * the client has lost the selection between announcing it and + * responding to our request */ + clipboard_refuse_selection(lxev); } - wtext = (twchar *) g_malloc(((len / 2) + 1) * sizeof(twchar), 0); - if (wtext == 0) + else if ((clip_msg_status & CB_RESPONSE_OK) == 0) { - return 0; + /* One of CB_RESPONSE_FAIL or CB_RESPONSE_OK MUST be set in + * a CLIPRDR_FORMAT_DATA_RESPONSE msg */ + LOG(LOG_LEVEL_ERROR, "CLIPRDR_FORMAT_DATA_RESPONSE is badly formed"); + clipboard_refuse_selection(lxev); } - index = 0; - while (s_check_rem(s, 2)) + else if (g_clip_c2s.xrdp_clip_type == XRDP_CB_BITMAP) { - in_uint16_le(s, wchr); - wtext[index] = wchr; - if (wchr == 0) - { - break; - } - index++; + clipboard_process_data_response_for_image(s, clip_msg_status, + clip_msg_len); } - wtext[index] = 0; - g_free(g_clip_c2s.data); - g_clip_c2s.data = 0; - g_clip_c2s.total_bytes = 0; - len = g_wcstombs(0, wtext, 0); - if (len >= 0) + else if (g_clip_c2s.xrdp_clip_type == XRDP_CB_FILE) { - g_clip_c2s.data = (char *) g_malloc(len + 16, 0); - if (g_clip_c2s.data == 0) - { - g_free(wtext); - return 0; - } - g_wcstombs(g_clip_c2s.data, wtext, len + 1); + clipboard_process_data_response_for_file(s, clip_msg_status, + clip_msg_len); } - if (g_clip_c2s.data != 0) + else { - g_clip_c2s.total_bytes = g_strlen(g_clip_c2s.data); - g_clip_c2s.read_bytes_done = g_clip_c2s.total_bytes; - clipboard_provide_selection_c2s(lxev, lxev->target); + clipboard_process_data_response_for_text(s, clip_msg_status, + clip_msg_len); } - g_free(wtext); - return 0; + return rv; } + /*****************************************************************************/ static int clipboard_process_clip_caps(struct stream *s, int clip_msg_status, From 8eed7a395e24fadf2a5e81edaf36c414216d3a9e Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 4 Aug 2023 16:46:02 +0100 Subject: [PATCH 15/22] clipboard: Only advertise text to X11 clients if it is available --- sesman/chansrv/clipboard.c | 22 +++++++++++++++++----- sesman/chansrv/clipboard_common.h | 1 + 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/sesman/chansrv/clipboard.c b/sesman/chansrv/clipboard.c index ea68223aeb..b3889b0bdb 100644 --- a/sesman/chansrv/clipboard.c +++ b/sesman/chansrv/clipboard.c @@ -2353,12 +2353,21 @@ clipboard_event_selection_request(XEvent *xevent) atom_buf[1] = g_timestamp_atom; atom_buf[2] = g_multiple_atom; atom_count = 3; - if ((g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_TEXT) == 0) + + /* Only announce text if the client is advertising text, or + * a file list */ + if (clipboard_find_format_id(CB_FORMAT_UNICODETEXT) >= 0 || + clipboard_find_format_id(CB_FORMAT_OEMTEXT) >= 0 || + clipboard_find_format_id(CB_FORMAT_TEXT) >= 0 || + clipboard_find_format_id(g_file_format_id) >= 0) { - atom_buf[atom_count] = XA_STRING; - atom_count++; - atom_buf[atom_count] = g_utf8_atom; - atom_count++; + if ((g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_TEXT) == 0) + { + atom_buf[atom_count] = XA_STRING; + atom_count++; + atom_buf[atom_count] = g_utf8_atom; + atom_count++; + } } if (clipboard_find_format_id(CB_FORMAT_DIB) >= 0 && (g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_IMAGE) == 0) @@ -2445,6 +2454,9 @@ clipboard_event_selection_request(XEvent *xevent) } else { + /* The client may have advertised CF_TEXT or CF_OEMTEXT, + * but the Windows clipboard will convert these formats + * to Unicode if asked */ g_memcpy(&g_saved_selection_req_event, lxev, sizeof(g_saved_selection_req_event)); g_clip_c2s.type = lxev->target; diff --git a/sesman/chansrv/clipboard_common.h b/sesman/chansrv/clipboard_common.h index 739fa63bb3..9d56bc4ae7 100644 --- a/sesman/chansrv/clipboard_common.h +++ b/sesman/chansrv/clipboard_common.h @@ -27,6 +27,7 @@ * where possible */ #define CB_FORMAT_RAW 0x0000 #define CB_FORMAT_TEXT 0x0001 +#define CB_FORMAT_OEMTEXT 0x0007 #define CB_FORMAT_DIB 0x0008 #define CB_FORMAT_UNICODETEXT 0x000D #define CB_FORMAT_HTML 0xD010 From 84ae372a58d3ece09e075f11819a2fd748ea6f1c Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 4 Aug 2023 17:08:42 +0100 Subject: [PATCH 16/22] clipboard: Fix TODO action in clipboard_common.h Use the official Windows clipboard format names where appropriate Replace g_file_format_id with g_file_group_descriptor_format_id as the latter name is more descriptive of what is described in [MS-ECLIP] --- sesman/chansrv/clipboard.c | 67 ++++++++++++++++++------------- sesman/chansrv/clipboard_common.h | 14 ------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/sesman/chansrv/clipboard.c b/sesman/chansrv/clipboard.c index b3889b0bdb..4b30152002 100644 --- a/sesman/chansrv/clipboard.c +++ b/sesman/chansrv/clipboard.c @@ -179,6 +179,7 @@ x-special/gnome-copied-files #include "xcommon.h" #include "chansrv_fuse.h" #include "ms-rdpeclip.h" +#include "xrdp_constants.h" static char g_bmp_image_header[] = { @@ -245,10 +246,22 @@ static int g_cliprdr_flags = CB_USE_LONG_FORMAT_NAMES | static int g_formatIds[16]; static int g_num_formatIds = 0; -static int g_file_format_id = -1; +/* Format ID assigned to "FileGroupDescriptorW" by the client */ +static int g_file_group_descriptor_format_id = -1; static char g_last_atom_name[256] = ""; +/* + * Values for the named formats we send to the client in + * a Format List PDU + */ + +enum +{ + CB_FORMAT_FILE_GROUP_DESCRIPTOR = 0xc0bc +}; + + /*****************************************************************************/ static char * get_atom_text(Atom atom) @@ -646,7 +659,7 @@ clipboard_send_format_announce(int xrdp_clip_type) case XRDP_CB_FILE: LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_send_format_announce: XRDP_CB_FILE"); /* canned response for "file" */ - out_uint32_le(s, 0x0000c0bc); + out_uint32_le(s, CB_FORMAT_FILE_GROUP_DESCRIPTOR); clipboard_out_unicode(s, "FileGroupDescriptorW", 21); out_uint32_le(s, 0x0000c0ba); clipboard_out_unicode(s, "FileContents", 13); @@ -690,7 +703,7 @@ clipboard_send_format_announce(int xrdp_clip_type) case XRDP_CB_FILE: LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_send_format_announce: XRDP_CB_FILE"); /* canned response for "file" */ - out_uint32_le(s, 0x0000c0bc); + out_uint32_le(s, CB_FORMAT_FILE_GROUP_DESCRIPTOR); out_uint8p(s, windows_native_format, sizeof(windows_native_format)); out_uint32_le(s, 0x0000c0ba); out_uint8p(s, windows_native_format, sizeof(windows_native_format)); @@ -1028,11 +1041,11 @@ clipboard_process_format_announce(struct stream *s, int clip_msg_status, LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_format_announce: max formats"); } - /* format id for file copy copy seems to keep changing */ - /* seen 0x0000c0c8, 0x0000c0ed */ + /* format id for file copy copy is dynamic and announced by the + * client */ if (g_strcmp(desc, "FileGroupDescriptorW") == 0) { - g_file_format_id = formatId; + g_file_group_descriptor_format_id = formatId; } } @@ -1100,48 +1113,48 @@ clipboard_process_data_request(struct stream *s, int clip_msg_status, in_uint32_le(s, requestedFormatId); switch (requestedFormatId) { - case CB_FORMAT_FILE: /* 0xC0BC */ + case CB_FORMAT_FILE_GROUP_DESCRIPTOR: if ((g_clip_s2c.xrdp_clip_type == XRDP_CB_FILE) && g_clip_s2c.converted) { - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_FILE"); + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_FILE_GROUP_DESCRIPTOR"); clipboard_send_data_response(XRDP_CB_FILE, g_clip_s2c.data, g_clip_s2c.total_bytes); } else { - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_FILE, " + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_FILE_GROUP_DESCRIPTOR, " "calling XConvertSelection to g_utf8_atom"); g_clip_s2c.xrdp_clip_type = XRDP_CB_FILE; XConvertSelection(g_display, g_clipboard_atom, g_clip_s2c.type, g_clip_property_atom, g_wnd, CurrentTime); } break; - case CB_FORMAT_DIB: /* 0x0008 */ + case CF_DIB: if ((g_clip_s2c.xrdp_clip_type == XRDP_CB_BITMAP) && g_clip_s2c.converted) { - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_DIB"); + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CF_DIB"); clipboard_send_data_response(XRDP_CB_BITMAP, g_clip_s2c.data, g_clip_s2c.total_bytes); } else { - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_DIB, " + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CF_DIB, " "calling XConvertSelection to g_image_bmp_atom"); g_clip_s2c.xrdp_clip_type = XRDP_CB_BITMAP; XConvertSelection(g_display, g_clipboard_atom, g_image_bmp_atom, g_clip_property_atom, g_wnd, CurrentTime); } break; - case CB_FORMAT_UNICODETEXT: /* 0x000D */ + case CF_UNICODETEXT: if ((g_clip_s2c.xrdp_clip_type == XRDP_CB_TEXT) && g_clip_s2c.converted) { - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_UNICODETEXT"); + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CF_UNICODETEXT"); clipboard_send_data_response(XRDP_CB_TEXT, g_clip_s2c.data, g_clip_s2c.total_bytes); } else { - LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CB_FORMAT_UNICODETEXT, " + LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_process_data_request: CF_UNICODETEXT, " "calling XConvertSelection to g_utf8_atom"); g_clip_s2c.xrdp_clip_type = XRDP_CB_TEXT; XConvertSelection(g_display, g_clipboard_atom, g_utf8_atom, @@ -2356,10 +2369,10 @@ clipboard_event_selection_request(XEvent *xevent) /* Only announce text if the client is advertising text, or * a file list */ - if (clipboard_find_format_id(CB_FORMAT_UNICODETEXT) >= 0 || - clipboard_find_format_id(CB_FORMAT_OEMTEXT) >= 0 || - clipboard_find_format_id(CB_FORMAT_TEXT) >= 0 || - clipboard_find_format_id(g_file_format_id) >= 0) + if (clipboard_find_format_id(CF_UNICODETEXT) >= 0 || + clipboard_find_format_id(CF_OEMTEXT) >= 0 || + clipboard_find_format_id(CF_TEXT) >= 0 || + clipboard_find_format_id(g_file_group_descriptor_format_id) >= 0) { if ((g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_TEXT) == 0) { @@ -2369,14 +2382,14 @@ clipboard_event_selection_request(XEvent *xevent) atom_count++; } } - if (clipboard_find_format_id(CB_FORMAT_DIB) >= 0 && + if (clipboard_find_format_id(CF_DIB) >= 0 && (g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_IMAGE) == 0) { LOG_DEVEL(LOG_LEVEL_DEBUG, " reporting image/bmp"); atom_buf[atom_count] = g_image_bmp_atom; atom_count++; } - if (clipboard_find_format_id(g_file_format_id) >= 0 && + if (clipboard_find_format_id(g_file_group_descriptor_format_id) >= 0 && (g_cfg->restrict_inbound_clipboard & CLIP_RESTRICT_FILE) == 0) { LOG_DEVEL(LOG_LEVEL_DEBUG, " reporting text/uri-list"); @@ -2420,7 +2433,7 @@ clipboard_event_selection_request(XEvent *xevent) } else if ((lxev->target == XA_STRING) || (lxev->target == g_utf8_atom)) { - if (clipboard_find_format_id(g_file_format_id) >= 0) + if (clipboard_find_format_id(g_file_group_descriptor_format_id) >= 0) { LOG_DEVEL(LOG_LEVEL_DEBUG, "clipboard_event_selection_request: " "text requested when files available"); @@ -2438,7 +2451,7 @@ clipboard_event_selection_request(XEvent *xevent) sizeof(g_saved_selection_req_event)); g_clip_c2s.type = lxev->target; g_clip_c2s.xrdp_clip_type = XRDP_CB_FILE; - clipboard_send_data_request(g_file_format_id); + clipboard_send_data_request(g_file_group_descriptor_format_id); } } @@ -2461,7 +2474,7 @@ clipboard_event_selection_request(XEvent *xevent) sizeof(g_saved_selection_req_event)); g_clip_c2s.type = lxev->target; g_clip_c2s.xrdp_clip_type = XRDP_CB_TEXT; - clipboard_send_data_request(CB_FORMAT_UNICODETEXT); + clipboard_send_data_request(CF_UNICODETEXT); } } @@ -2500,7 +2513,7 @@ clipboard_event_selection_request(XEvent *xevent) sizeof(g_saved_selection_req_event)); g_clip_c2s.type = g_image_bmp_atom; g_clip_c2s.xrdp_clip_type = XRDP_CB_BITMAP; - clipboard_send_data_request(CB_FORMAT_DIB); + clipboard_send_data_request(CF_DIB); } return 0; @@ -2537,7 +2550,7 @@ clipboard_event_selection_request(XEvent *xevent) sizeof(g_saved_selection_req_event)); g_clip_c2s.type = g_file_atom1; g_clip_c2s.xrdp_clip_type = XRDP_CB_FILE; - clipboard_send_data_request(g_file_format_id); + clipboard_send_data_request(g_file_group_descriptor_format_id); return 0; } @@ -2575,7 +2588,7 @@ clipboard_event_selection_request(XEvent *xevent) sizeof(g_saved_selection_req_event)); g_clip_c2s.type = g_file_atom2; g_clip_c2s.xrdp_clip_type = XRDP_CB_FILE; - clipboard_send_data_request(g_file_format_id); + clipboard_send_data_request(g_file_group_descriptor_format_id); return 0; } } diff --git a/sesman/chansrv/clipboard_common.h b/sesman/chansrv/clipboard_common.h index 9d56bc4ae7..3b8c0088c7 100644 --- a/sesman/chansrv/clipboard_common.h +++ b/sesman/chansrv/clipboard_common.h @@ -22,20 +22,6 @@ #include "arch.h" #include "parse.h" -/* - * TODO : Replace these with the definitions in xrdp_constants.h - * where possible */ -#define CB_FORMAT_RAW 0x0000 -#define CB_FORMAT_TEXT 0x0001 -#define CB_FORMAT_OEMTEXT 0x0007 -#define CB_FORMAT_DIB 0x0008 -#define CB_FORMAT_UNICODETEXT 0x000D -#define CB_FORMAT_HTML 0xD010 -#define CB_FORMAT_PNG 0xD011 -#define CB_FORMAT_JPEG 0xD012 -#define CB_FORMAT_GIF 0xD013 -#define CB_FORMAT_FILE 0xC0BC - /* these are the supported general types */ #define XRDP_CB_TEXT 1 #define XRDP_CB_BITMAP 2 From 4b1482b5dfaaae42ba56407d085c27371fe6847a Mon Sep 17 00:00:00 2001 From: Jay Sorg Date: Sun, 28 May 2023 00:52:26 -0700 Subject: [PATCH 17/22] move to posix shm --- xrdp/xrdp.h | 6 +++ xrdp/xrdp_encoder.h | 2 + xrdp/xrdp_mm.c | 14 +++++++ xrdp/xrdp_types.h | 8 +++- xup/xup.c | 99 +++++++++++++++++++++++++++++++++++++++++++++ xup/xup.h | 8 +++- 6 files changed, 135 insertions(+), 2 deletions(-) diff --git a/xrdp/xrdp.h b/xrdp/xrdp.h index f06eb1a90c..03bcd254e3 100644 --- a/xrdp/xrdp.h +++ b/xrdp/xrdp.h @@ -524,6 +524,12 @@ server_set_pointer_large(struct xrdp_mod *mod, int x, int y, char *data, char *mask, int bpp, int width, int height); int +server_paint_rects_ex(struct xrdp_mod *mod, int num_drects, short *drects, + int num_crects, short *crects, + char *data, int left, int top, + int width, int height, + int flags, int frame_id); +int server_palette(struct xrdp_mod *mod, int *palette); int server_msg(struct xrdp_mod *mod, const char *msg, int code); diff --git a/xrdp/xrdp_encoder.h b/xrdp/xrdp_encoder.h index 2598bcb523..ce8c128a06 100644 --- a/xrdp/xrdp_encoder.h +++ b/xrdp/xrdp_encoder.h @@ -38,6 +38,8 @@ struct xrdp_enc_data int num_crects; short *crects; /* 4 * num_crects */ char *data; + int left; + int top; int width; int height; int flags; diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index b43ccf53a9..573c1e3317 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -423,6 +423,7 @@ xrdp_mm_setup_mod1(struct xrdp_mm *self) self->mod->server_paint_rects = server_paint_rects; self->mod->server_session_info = server_session_info; self->mod->server_set_pointer_large = server_set_pointer_large; + self->mod->server_paint_rects_ex = server_paint_rects_ex; self->mod->si = &(self->wm->session->si); } } @@ -3357,6 +3358,17 @@ int server_paint_rects(struct xrdp_mod *mod, int num_drects, short *drects, int num_crects, short *crects, char *data, int width, int height, int flags, int frame_id) +{ + return server_paint_rects_ex(mod, num_drects, drects, num_crects, crects, + data, 0, 0, width, height, flags, frame_id); +} + +/*****************************************************************************/ +int +server_paint_rects_ex(struct xrdp_mod *mod, int num_drects, short *drects, + int num_crects, short *crects, char *data, + int left, int top, int width, int height, + int flags, int frame_id) { struct xrdp_wm *wm; struct xrdp_mm *mm; @@ -3404,6 +3416,8 @@ server_paint_rects(struct xrdp_mod *mod, int num_drects, short *drects, enc_data->num_drects = num_drects; enc_data->num_crects = num_crects; enc_data->data = data; + enc_data->left = left; + enc_data->top = top; enc_data->width = width; enc_data->height = height; enc_data->flags = flags; diff --git a/xrdp/xrdp_types.h b/xrdp/xrdp_types.h index 55677fcf35..f75193460a 100644 --- a/xrdp/xrdp_types.h +++ b/xrdp/xrdp_types.h @@ -174,7 +174,13 @@ struct xrdp_mod int (*server_set_pointer_large)(struct xrdp_mod *v, int x, int y, char *data, char *mask, int bpp, int width, int height); - tintptr server_dumby[100 - 47]; /* align, 100 minus the number of server + int (*server_paint_rects_ex)(struct xrdp_mod *v, + int num_drects, short *drects, + int num_crects, short *crects, + char *data, int left, int top, + int width, int height, + int flags, int frame_id); + tintptr server_dumby[100 - 48]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as int */ diff --git a/xup/xup.c b/xup/xup.c index 3fb1a421ad..cbd912c850 100644 --- a/xup/xup.c +++ b/xup/xup.c @@ -1304,6 +1304,102 @@ process_server_set_pointer_shmfd(struct mod *amod, struct stream *s) return rv; } +/******************************************************************************/ +/* return error */ +static int +process_server_paint_rect_shmfd(struct mod *amod, struct stream *s) +{ + int num_drects; + int num_crects; + int flags; + int frame_id; + int shmem_bytes; + int shmem_offset; + int left; + int top; + int width; + int height; + int index; + int rv; + int16_t *ldrects; + int16_t *ldrects1; + int16_t *lcrects; + int16_t *lcrects1; + char *bmpdata; + int fd; + int recv_bytes; + unsigned int num_fds; + void *shmem_ptr; + char msg[4]; + + /* dirty pixels */ + in_uint16_le(s, num_drects); + ldrects = g_new(int16_t, 2 * 4 * num_drects); + ldrects1 = ldrects; + for (index = 0; index < num_drects; index++) + { + in_sint16_le(s, ldrects1[0]); + in_sint16_le(s, ldrects1[1]); + in_sint16_le(s, ldrects1[2]); + in_sint16_le(s, ldrects1[3]); + ldrects1 += 4; + } + + /* copied pixels */ + in_uint16_le(s, num_crects); + lcrects = g_new(int16_t, 2 * 4 * num_crects); + lcrects1 = lcrects; + for (index = 0; index < num_crects; index++) + { + in_sint16_le(s, lcrects1[0]); + in_sint16_le(s, lcrects1[1]); + in_sint16_le(s, lcrects1[2]); + in_sint16_le(s, lcrects1[3]); + lcrects1 += 4; + } + + in_uint32_le(s, flags); + in_uint32_le(s, frame_id); + in_uint32_le(s, shmem_bytes); + in_uint32_le(s, shmem_offset); + + in_uint16_le(s, left); + in_uint16_le(s, top); + in_uint16_le(s, width); + in_uint16_le(s, height); + + if (g_tcp_can_recv(amod->trans->sck, 5000) == 0) + { + g_free(ldrects); + g_free(lcrects); + return 1; + } + rv = 1; + recv_bytes = g_sck_recv_fd_set(amod->trans->sck, msg, 4, &fd, 1, &num_fds); + LOG_DEVEL(LOG_LEVEL_DEBUG, "process_server_paint_rect_shmfd: " + "g_sck_recv_fd_set rv %d fd %d", recv_bytes, fd); + if (recv_bytes == 4) + { + if (num_fds == 1) + { + if (g_file_map(fd, 1, 1, shmem_bytes, &shmem_ptr) == 0) + { + bmpdata = (char *)shmem_ptr; + bmpdata += shmem_offset; + rv = amod->server_paint_rects_ex(amod, num_drects, ldrects, + num_crects, lcrects, bmpdata, + left, top, width, height, + flags, frame_id); + g_munmap(shmem_ptr, shmem_bytes); + } + g_file_close(fd); + } + } + g_free(ldrects); + g_free(lcrects); + return rv; +} + /******************************************************************************/ /* return error */ static int @@ -1532,6 +1628,9 @@ lib_mod_process_orders(struct mod *mod, int type, struct stream *s) case 63: /* server_set_pointer_shmfd */ rv = process_server_set_pointer_shmfd(mod, s); break; + case 64: /* server_paint_rect_shmfd */ + rv = process_server_paint_rect_shmfd(mod, s); + break; default: LOG_DEVEL(LOG_LEVEL_WARNING, "lib_mod_process_orders: unknown order type %d", type); diff --git a/xup/xup.h b/xup/xup.h index 6a14206aa3..fc3e8ef507 100644 --- a/xup/xup.h +++ b/xup/xup.h @@ -160,7 +160,13 @@ struct mod int (*server_set_pointer_large)(struct mod *v, int x, int y, char *data, char *mask, int bpp, int width, int height); - tintptr server_dumby[100 - 47]; /* align, 100 minus the number of server + int (*server_paint_rects_ex)(struct mod *v, + int num_drects, short *drects, + int num_crects, short *crects, + char *data, int left, int top, + int width, int height, + int flags, int frame_id); + tintptr server_dumby[100 - 48]; /* align, 100 minus the number of server functions above */ /* common */ tintptr handle; /* pointer to self as long */ From 25a1fab5b6c5ef2a8bb109232b765cb8b332ce5e Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Sat, 19 Aug 2023 13:35:26 +0100 Subject: [PATCH 18/22] Check auth_start_session() result --- sesman/libsesman/verify_user_pam.c | 24 +++++++++++++++++++-- sesman/libsesman/verify_user_pam_userpass.c | 24 +++++++++++++++++++-- sesman/sesexec/session.c | 7 +++++- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/sesman/libsesman/verify_user_pam.c b/sesman/libsesman/verify_user_pam.c index b400bb5e2c..ca5136688d 100644 --- a/sesman/libsesman/verify_user_pam.c +++ b/sesman/libsesman/verify_user_pam.c @@ -397,8 +397,8 @@ auth_uds(const char *user, enum scp_login_status *errorcode) /******************************************************************************/ /* returns error */ -int -auth_start_session(struct auth_info *auth_info, int display_num) +static int +auth_start_session_private(struct auth_info *auth_info, int display_num) { int error; char display[256]; @@ -436,6 +436,26 @@ auth_start_session(struct auth_info *auth_info, int display_num) return 0; } +/******************************************************************************/ +/** + * Main routine to start a session + * + * Calls the private routine and logs an additional error if the private + * routine fails + */ +int +auth_start_session(struct auth_info *auth_info, int display_num) +{ + int result = auth_start_session_private(auth_info, display_num); + if (result != 0) + { + LOG(LOG_LEVEL_ERROR, + "Can't start PAM session. See PAM logging for more info"); + } + + return result; +} + /******************************************************************************/ /* returns error */ static int diff --git a/sesman/libsesman/verify_user_pam_userpass.c b/sesman/libsesman/verify_user_pam_userpass.c index 349d811145..9a6f657c2c 100644 --- a/sesman/libsesman/verify_user_pam_userpass.c +++ b/sesman/libsesman/verify_user_pam_userpass.c @@ -207,8 +207,8 @@ auth_uds(const char *user, enum scp_login_status *errorcode) /******************************************************************************/ /* returns error */ -int -auth_start_session(struct auth_info *auth_info, int display_num) +static int +auth_start_session_private(struct auth_info *auth_info, int display_num) { int error; char display[256]; @@ -246,6 +246,26 @@ auth_start_session(struct auth_info *auth_info, int display_num) return 0; } +/******************************************************************************/ +/** + * Main routine to start a session + * + * Calls the private routine and logs an additional error if the private + * routine fails + */ +int +auth_start_session(struct auth_info *auth_info, int display_num) +{ + int result = auth_start_session_private(auth_info, display_num); + if (result != 0) + { + LOG(LOG_LEVEL_ERROR, + "Can't start PAM session. See PAM logging for more info"); + } + + return result; +} + /******************************************************************************/ /* returns error */ static int diff --git a/sesman/sesexec/session.c b/sesman/sesexec/session.c index 6836d5c47d..900b394333 100644 --- a/sesman/sesexec/session.c +++ b/sesman/sesexec/session.c @@ -568,7 +568,12 @@ session_start_wrapped(struct login_info *login_info, int window_manager_pid; enum scp_screate_status status = E_SCP_SCREATE_GENERAL_ERROR; - auth_start_session(login_info->auth_info, s->display); + if (auth_start_session(login_info->auth_info, s->display) != 0) + { + // Errors are logged by the auth module, as they are + // specific to that module + return E_SCP_SCREATE_GENERAL_ERROR; + } #ifdef USE_BSD_SETLOGIN /** * Create a new session and process group since the 4.4BSD From 8fb5bd9096ac6373427c78660fc0e13053acd675 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 30 Aug 2023 12:37:44 +0100 Subject: [PATCH 19/22] Add keyring support for Debian and Arch Adds optional calls to GNOME and KDE keyrings for Debian and Arch. Also upstreams a current Debian patch to call pam_env.so --- instfiles/pam.d/xrdp-sesman.arch | 15 +++++++++++---- instfiles/pam.d/xrdp-sesman.debian | 11 ++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/instfiles/pam.d/xrdp-sesman.arch b/instfiles/pam.d/xrdp-sesman.arch index d303ab79e9..2763612e9c 100644 --- a/instfiles/pam.d/xrdp-sesman.arch +++ b/instfiles/pam.d/xrdp-sesman.arch @@ -1,5 +1,12 @@ #%PAM-1.0 -auth include system-remote-login -account include system-remote-login -password include system-remote-login -session include system-remote-login +auth include system-remote-login +-auth optional pam_gnome_keyring.so +-auth optional pam_kwallet5.so + +account include system-remote-login + +password include system-remote-login + +session include system-remote-login +-session optional pam_gnome_keyring.so auto_start +-session optional pam_kwallet5.so auto_start diff --git a/instfiles/pam.d/xrdp-sesman.debian b/instfiles/pam.d/xrdp-sesman.debian index 789ce8f7cf..bab1e57201 100644 --- a/instfiles/pam.d/xrdp-sesman.debian +++ b/instfiles/pam.d/xrdp-sesman.debian @@ -1,5 +1,14 @@ #%PAM-1.0 +auth required pam_env.so readenv=1 +auth required pam_env.so readenv=1 envfile=/etc/default/locale @include common-auth +-auth optional pam_gnome_keyring.so +-auth optional pam_kwallet5.so + @include common-account -@include common-session + @include common-password + +@include common-session +-session optional pam_gnome_keyring.so auto_start +-session optional pam_kwallet5.so auto_start From c37ce6fa47a18261a79e748443de1dbcef034632 Mon Sep 17 00:00:00 2001 From: firewave Date: Mon, 4 Sep 2023 09:27:21 +0200 Subject: [PATCH 20/22] Cppcheck 2.8 removed the dependency on z3 --- .github/workflows/build.yml | 2 +- scripts/install_cppcheck.sh | 8 +++++- .../install_cppcheck_dependencies_with_apt.sh | 26 +++++++++++++++++-- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 949fccabe1..1d0d653e7c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -181,7 +181,7 @@ jobs: with: path: ~/cppcheck.local key: ${{ steps.os.outputs.image }}-build-${{ env.cache-name }}-${{ env.CPPCHECK_VER }} - - run: sudo scripts/install_cppcheck_dependencies_with_apt.sh + - run: sudo scripts/install_cppcheck_dependencies_with_apt.sh $CPPCHECK_VER - run: ./bootstrap - run: scripts/install_cppcheck.sh $CPPCHECK_REPO $CPPCHECK_VER - run: scripts/run_cppcheck.sh -v $CPPCHECK_VER diff --git a/scripts/install_cppcheck.sh b/scripts/install_cppcheck.sh index 24ff921057..94600b239a 100755 --- a/scripts/install_cppcheck.sh +++ b/scripts/install_cppcheck.sh @@ -128,7 +128,12 @@ fi # CFGDIR is needed for cppcheck before 1.86 make_args="FILESDIR=$FILESDIR PREFIX=$FILESDIR CFGDIR=$FILESDIR" ;; - *) make_args="FILESDIR=$FILESDIR PREFIX=$FILESDIR USE_Z3=yes" + 2.8 | 2.9 | 2.1*) + # Cppcheck 2.8 removed the dependency on z3 + make_args="FILESDIR=$FILESDIR PREFIX=$FILESDIR" + ;; + 2.*) + make_args="FILESDIR=$FILESDIR PREFIX=$FILESDIR USE_Z3=yes" # Check that the Z3 development files appear to be installed # before trying to create z3_version.h. Otherwise we may # mislead the user as to what needs to be done. @@ -138,6 +143,7 @@ fi if [ ! -f /usr/include/z3_version.h ]; then create_z3_version_h fi + ;; esac # Use all available CPUs diff --git a/scripts/install_cppcheck_dependencies_with_apt.sh b/scripts/install_cppcheck_dependencies_with_apt.sh index 610fb0ed22..95887e78ba 100755 --- a/scripts/install_cppcheck_dependencies_with_apt.sh +++ b/scripts/install_cppcheck_dependencies_with_apt.sh @@ -3,5 +3,27 @@ set -eufx PACKAGES="libz3-dev z3" -apt-get update -apt-get -yq --no-install-suggests --no-install-recommends install $PACKAGES +usage() +{ + echo "** Usage: $0 " + echo " e.g. $0 1.90" +} >&2 + +if [ $# -ne 1 ]; then + usage + exit 1 +fi +CPPCHECK_VER="$1" + +case "$CPPCHECK_VER" in + 1.*) + # no dependencies + ;; + 2.8 | 2.9 | 2.1*) + # Cppcheck 2.8 removed the dependency on z3 + ;; + 2.*) + apt-get update + apt-get -yq --no-install-suggests --no-install-recommends install $PACKAGES + ;; +esac From a978b58f11c56ca8f06cf22785362317b4db2c00 Mon Sep 17 00:00:00 2001 From: Koichiro Iwao Date: Mon, 4 Sep 2023 17:33:27 +0900 Subject: [PATCH 21/22] waitforx: fix build --- waitforx --- CCLD waitforx ld: error: unable to find library -lX11 ld: error: unable to find library -lXrandr cc: error: linker command failed with exit code 1 (use -v to see invocation) *** [waitforx] Error code 1 --- waitforx/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/waitforx/Makefile.am b/waitforx/Makefile.am index 5b96f6614d..d4ddaa45ab 100644 --- a/waitforx/Makefile.am +++ b/waitforx/Makefile.am @@ -1,7 +1,7 @@ pkglibexec_PROGRAMS = \ waitforx -AM_LDFLAGS = -lX11 -lXrandr +AM_LDFLAGS = $(X_LIBS) -lX11 -lXrandr AM_CPPFLAGS = \ -I$(top_srcdir)/sesman/sesexec \ From 27d34e784d912b876753e79d02e1d94cf8bcbb5d Mon Sep 17 00:00:00 2001 From: firewave Date: Mon, 4 Sep 2023 23:47:56 +0200 Subject: [PATCH 22/22] fixed Cppcheck `unusedVariable` warnings --- fontutils/windows/fontdump.c | 2 -- libxrdp/xrdp_surface.c | 3 --- sesman/chansrv/pcsc/wrapper/winscard.c | 1 - tools/devel/gtcp_proxy/gtcp.c | 2 -- xrdpapi/vrplayer.c | 5 ----- 5 files changed, 13 deletions(-) diff --git a/fontutils/windows/fontdump.c b/fontutils/windows/fontdump.c index 286bcb8503..79214ec699 100644 --- a/fontutils/windows/fontdump.c +++ b/fontutils/windows/fontdump.c @@ -466,8 +466,6 @@ create_window(void) { WNDCLASS wc; DWORD style; - HDC dc; - int height; int left; int top; diff --git a/libxrdp/xrdp_surface.c b/libxrdp/xrdp_surface.c index 7b49c9e20c..b41379e6f2 100644 --- a/libxrdp/xrdp_surface.c +++ b/libxrdp/xrdp_surface.c @@ -94,10 +94,7 @@ xrdp_surface_send_surface_bits(struct xrdp_surface *self, int bpp, char *data, int x, int y, int cx, int cy) { RFX_RECT rect; - char *buf; int Bpp; - int i; - int j; int codecId; uint32 bitmapDataLength; STREAM *s; diff --git a/sesman/chansrv/pcsc/wrapper/winscard.c b/sesman/chansrv/pcsc/wrapper/winscard.c index a8287010ca..62556d5f8f 100644 --- a/sesman/chansrv/pcsc/wrapper/winscard.c +++ b/sesman/chansrv/pcsc/wrapper/winscard.c @@ -723,7 +723,6 @@ SCardControl(SCARDHANDLE hCard, DWORD dwControlCode, LPCVOID lpInBuffer, DWORD nOutBufferSize, LPDWORD lpBytesReturned) { LONG rv; - char text[8148]; LLOGLN(10, ("SCardControl:")); LLOGLN(10, (" hCard %p", hCard)); diff --git a/tools/devel/gtcp_proxy/gtcp.c b/tools/devel/gtcp_proxy/gtcp.c index 6e52babce2..0ea8e2c860 100644 --- a/tools/devel/gtcp_proxy/gtcp.c +++ b/tools/devel/gtcp_proxy/gtcp.c @@ -131,8 +131,6 @@ int tcp_listen(int skt) int tcp_accept(int skt) { - int ret ; - char ipAddr[256] ; struct sockaddr_in s; socklen_t i; diff --git a/xrdpapi/vrplayer.c b/xrdpapi/vrplayer.c index 5e4fbde796..de20c303fc 100644 --- a/xrdpapi/vrplayer.c +++ b/xrdpapi/vrplayer.c @@ -56,14 +56,9 @@ extract_32(uint32_t *value, char *buf) int main(int argc, char **argv) { - char *data; - char *data1; void *channel; int written = 0; - int rv; int first_time = 1; - int length; - int bytes_read; if (argc < 2) {