From 7bd1823ceb2fb84d840c7ded737ce1bb60c7e3ba Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Tue, 13 Dec 2016 13:24:15 +0100 Subject: Add xauth support to get more security for all backends --- sesman/session.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'sesman/session.c') diff --git a/sesman/session.c b/sesman/session.c index 783665cf..ea3a7ee8 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -676,6 +676,20 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) g_snprintf(text, 255, "%d", g_cfg->sess.kill_disconnected); g_setenv("XRDP_SESMAN_KILL_DISCONNECTED", text, 1); + /* now the Xauthority stuff */ + char cookie[33] = ""; + char authfile[255] = ".Xauthority"; + + if (g_getenv("XAUTHORITY") !=NULL) + g_sprintf(authfile, "%s", g_getenv("XAUTHORITY")); + /* Create the cookie */ + srand((unsigned int) time(0)); + for (i = 0; i < 32; i += 2) + sprintf(&cookie[i], "%02X", rand() % 16); + + /* Add the entry in XAUTORITY file */ + env_add_xauth_user(display, cookie, NULL); + if (type == SESMAN_SESSION_TYPE_XORG) { #ifdef HAVE_SYS_PRCTL_H @@ -702,6 +716,8 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) /* these are the must have parameters */ list_add_item(xserver_params, (tintptr) g_strdup(xserver)); list_add_item(xserver_params, (tintptr) g_strdup(screen)); + list_add_item(xserver_params, (tintptr) g_strdup("-auth")); + list_add_item(xserver_params, (tintptr) g_strdup(authfile)); /* additional parameters from sesman.ini file */ list_append_list_strdup(g_cfg->xorg_params, xserver_params, 1); @@ -737,6 +753,8 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) /* these are the must have parameters */ list_add_item(xserver_params, (tintptr)g_strdup(xserver)); list_add_item(xserver_params, (tintptr)g_strdup(screen)); + list_add_item(xserver_params, (tintptr)g_strdup("-auth")); + list_add_item(xserver_params, (tintptr)g_strdup(authfile)); list_add_item(xserver_params, (tintptr)g_strdup("-geometry")); list_add_item(xserver_params, (tintptr)g_strdup(geometry)); list_add_item(xserver_params, (tintptr)g_strdup("-depth")); @@ -768,6 +786,8 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) /* these are the must have parameters */ list_add_item(xserver_params, (tintptr)g_strdup(xserver)); list_add_item(xserver_params, (tintptr)g_strdup(screen)); + list_add_item(xserver_params, (tintptr)g_strdup("-auth")); + list_add_item(xserver_params, (tintptr)g_strdup(authfile)); list_add_item(xserver_params, (tintptr)g_strdup("-geometry")); list_add_item(xserver_params, (tintptr)g_strdup(geometry)); list_add_item(xserver_params, (tintptr)g_strdup("-depth")); -- cgit v1.2.3 From 0aa4b85f817ac97ab8916c6aaae3066149902ba6 Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Wed, 14 Dec 2016 00:29:22 +0100 Subject: Xauth: use snprintf for setting the filename and adjust the value of computed cookie --- sesman/session.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'sesman/session.c') diff --git a/sesman/session.c b/sesman/session.c index ea3a7ee8..f057c93d 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -441,6 +441,9 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) struct list *xserver_params = (struct list *)NULL; struct tm stime; time_t ltime; + char cookie[33]; /* the cookie which will be used for xauth */ + char cookie_tmpval; /* Used to fill the cookie with random values */ + char authfile[255]; /* The filename for storing xauth informations */ /* initialize (zero out) local variables: */ g_memset(<ime, 0, sizeof(time_t)); @@ -676,16 +679,23 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) g_snprintf(text, 255, "%d", g_cfg->sess.kill_disconnected); g_setenv("XRDP_SESMAN_KILL_DISCONNECTED", text, 1); - /* now the Xauthority stuff */ - char cookie[33] = ""; - char authfile[255] = ".Xauthority"; - + /* prepare the Xauthority stuff */ if (g_getenv("XAUTHORITY") !=NULL) - g_sprintf(authfile, "%s", g_getenv("XAUTHORITY")); + { + g_snprintf(authfile, 255, "%s", g_getenv("XAUTHORITY")); + } + else + { + g_snprintf(authfile, 11, "%s", ".Xauthority"); + } + /* Create the cookie */ - srand((unsigned int) time(0)); - for (i = 0; i < 32; i += 2) - sprintf(&cookie[i], "%02X", rand() % 16); + for (i = 0; i < 32; i++) + { + g_random((char *) &cookie_tmpval, 1); + sprintf(&cookie[i], "%02X", cookie_tmpval & 0xff); + } + cookie[32]='\0'; /* Add the entry in XAUTORITY file */ env_add_xauth_user(display, cookie, NULL); -- cgit v1.2.3 From 16b6471d880df9185a87bac983685ff8fd1c959f Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Wed, 14 Dec 2016 07:16:06 +0100 Subject: use the correct size for snprintf --- sesman/session.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sesman/session.c') diff --git a/sesman/session.c b/sesman/session.c index f057c93d..81276a91 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -443,7 +443,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) time_t ltime; char cookie[33]; /* the cookie which will be used for xauth */ char cookie_tmpval; /* Used to fill the cookie with random values */ - char authfile[255]; /* The filename for storing xauth informations */ + char authfile[256]; /* The filename for storing xauth informations */ /* initialize (zero out) local variables: */ g_memset(<ime, 0, sizeof(time_t)); @@ -686,7 +686,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) } else { - g_snprintf(authfile, 11, "%s", ".Xauthority"); + g_snprintf(authfile, 12, "%s", ".Xauthority"); } /* Create the cookie */ -- cgit v1.2.3 From 7d1fdd04b56ab2c2849596f52f6976d9e3c4175a Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Wed, 14 Dec 2016 08:28:25 +0100 Subject: Cosmetic change: follow coding standard --- sesman/env.c | 8 ++++---- sesman/session.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'sesman/session.c') diff --git a/sesman/env.c b/sesman/env.c index 5bc4e17d..625f8345 100644 --- a/sesman/env.c +++ b/sesman/env.c @@ -229,12 +229,12 @@ env_add_xauth_user(int display, char *cookie, char *file) g_file_close(fd); } - g_sprintf(xauth_str, "xauth -q -f %s add :%d . %s", file, display, cookie); + g_sprintf(xauth_str, "xauth -q -f %s add :%d . %s", + file, display, cookie); } - log_message(LOG_LEVEL_DEBUG, - "xauth command: %s", xauth_str); + log_message(LOG_LEVEL_DEBUG, "xauth command: %s", xauth_str); - if ( (dp = popen(xauth_str,"r")) == NULL ) { + if ((dp = popen(xauth_str, "r")) == NULL) { log_message(LOG_LEVEL_INFO, "xauth failed, no X security"); return 1; } diff --git a/sesman/session.c b/sesman/session.c index 81276a91..6c4b8d1a 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -680,7 +680,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) g_setenv("XRDP_SESMAN_KILL_DISCONNECTED", text, 1); /* prepare the Xauthority stuff */ - if (g_getenv("XAUTHORITY") !=NULL) + if (g_getenv("XAUTHORITY") != NULL) { g_snprintf(authfile, 255, "%s", g_getenv("XAUTHORITY")); } @@ -695,7 +695,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) g_random((char *) &cookie_tmpval, 1); sprintf(&cookie[i], "%02X", cookie_tmpval & 0xff); } - cookie[32]='\0'; + cookie[32] = '\0'; /* Add the entry in XAUTORITY file */ env_add_xauth_user(display, cookie, NULL); -- cgit v1.2.3 From e72957b7c96c10f8b1e5ea989f676581d6991fe5 Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Wed, 14 Dec 2016 10:55:45 +0100 Subject: xauth: use the authfile if not using default value --- sesman/session.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sesman/session.c') diff --git a/sesman/session.c b/sesman/session.c index 6c4b8d1a..0f5f6fe2 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -698,7 +698,7 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) cookie[32] = '\0'; /* Add the entry in XAUTORITY file */ - env_add_xauth_user(display, cookie, NULL); + env_add_xauth_user(display, cookie, authfile); if (type == SESMAN_SESSION_TYPE_XORG) { -- cgit v1.2.3 From b2f4f68ab8d43283a73aa9de099e73108129c374 Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Thu, 15 Dec 2016 18:06:35 +0100 Subject: - move function related to xauth in own file - use of g_bytes_to_hexstr() - correct typos and coding syntax - don't create auth file, xauth can do that if needed --- sesman/Makefile.am | 2 ++ sesman/env.c | 42 ------------------------------- sesman/env.h | 13 ---------- sesman/sesman.h | 1 - sesman/session.c | 17 +++---------- sesman/xauth.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ sesman/xauth.h | 42 +++++++++++++++++++++++++++++++ 7 files changed, 121 insertions(+), 69 deletions(-) create mode 100644 sesman/xauth.c create mode 100644 sesman/xauth.h (limited to 'sesman/session.c') diff --git a/sesman/Makefile.am b/sesman/Makefile.am index 68dba28e..b5e3f138 100644 --- a/sesman/Makefile.am +++ b/sesman/Makefile.am @@ -54,6 +54,8 @@ xrdp_sesman_SOURCES = \ session.h \ sig.c \ sig.h \ + xauth.c \ + xauth.h \ $(AUTH_C) xrdp_sesman_LDADD = \ diff --git a/sesman/env.c b/sesman/env.c index 625f8345..1ea1a948 100644 --- a/sesman/env.c +++ b/sesman/env.c @@ -201,45 +201,3 @@ env_set_user(const char *username, char **passwd_file, int display, return error; } - - -/******************************************************************************/ -int DEFAULT_CC -env_add_xauth_user(int display, char *cookie, char *file) -{ - FILE *dp; - char xauth_str[256]; - int fd; - - if ( file == NULL ) - { - if (!g_file_exist(".Xauthority")) - { - fd = g_file_open(".Xauthority"); - g_file_close(fd); - } - - g_sprintf(xauth_str, "xauth -q add :%d . %s", display, cookie); - } - else - { - if (!g_file_exist(file)) - { - fd = g_file_open(file); - g_file_close(fd); - } - - g_sprintf(xauth_str, "xauth -q -f %s add :%d . %s", - file, display, cookie); - } - log_message(LOG_LEVEL_DEBUG, "xauth command: %s", xauth_str); - - if ((dp = popen(xauth_str, "r")) == NULL) { - log_message(LOG_LEVEL_INFO, "xauth failed, no X security"); - return 1; - } - - pclose(dp); - - return 0; -} diff --git a/sesman/env.h b/sesman/env.h index 15920512..a7156508 100644 --- a/sesman/env.h +++ b/sesman/env.h @@ -53,17 +53,4 @@ int DEFAULT_CC env_set_user(const char *username, char **passwd_file, int display, const struct list *env_names, const struct list *env_values); -/** - * - * @brief create the XAUTORITY file for the user according to the display and the cookie - * xauth uses XAUTORITY if defined, ~/.Xauthority otherwise - * @param display The session display - * @param cookie The cookie - * @param file If not NULL, write the autorization in the file instead of default location - * @return 0 if adding the cookie is ok - */ - -int DEFAULT_CC -env_add_xauth_user(int display, char *cookie, char * file); - #endif diff --git a/sesman/sesman.h b/sesman/sesman.h index 9abf866e..09b781bc 100644 --- a/sesman/sesman.h +++ b/sesman/sesman.h @@ -30,7 +30,6 @@ #if defined(HAVE_CONFIG_H) #include "config_ac.h" #endif -#include #include "arch.h" #include "parse.h" #include "os_calls.h" diff --git a/sesman/session.c b/sesman/session.c index 0f5f6fe2..0540bc11 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -39,6 +39,7 @@ #include "sesman.h" #include "libscp_types.h" +#include "xauth.h" #ifndef PR_SET_NO_NEW_PRIVS #define PR_SET_NO_NEW_PRIVS 38 @@ -441,8 +442,6 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) struct list *xserver_params = (struct list *)NULL; struct tm stime; time_t ltime; - char cookie[33]; /* the cookie which will be used for xauth */ - char cookie_tmpval; /* Used to fill the cookie with random values */ char authfile[256]; /* The filename for storing xauth informations */ /* initialize (zero out) local variables: */ @@ -686,19 +685,11 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) } else { - g_snprintf(authfile, 12, "%s", ".Xauthority"); + g_snprintf(authfile, 255, "%s", ".Xauthority"); } - /* Create the cookie */ - for (i = 0; i < 32; i++) - { - g_random((char *) &cookie_tmpval, 1); - sprintf(&cookie[i], "%02X", cookie_tmpval & 0xff); - } - cookie[32] = '\0'; - - /* Add the entry in XAUTORITY file */ - env_add_xauth_user(display, cookie, authfile); + /* Add the entry in XAUTHORITY file */ + add_xauth_cookie(display, authfile); if (type == SESMAN_SESSION_TYPE_XORG) { diff --git a/sesman/xauth.c b/sesman/xauth.c new file mode 100644 index 00000000..948d3f0f --- /dev/null +++ b/sesman/xauth.c @@ -0,0 +1,73 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Jay Sorg 2004-2013 + * + * 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 xauth.c + * @brief XAUTHORITY handling code + * @author Emmaunel Blindauer + * + */ + +#include +#include "sesman.h" +// #include "grp.h" +// #include "ssl_calls.h" +#include "os_calls.h" + + +/******************************************************************************/ +int DEFAULT_CC +add_xauth_cookie(int display, const char *file) +{ + FILE *dp; + char cookie[33]; + char char_cookie[16]; + char xauth_str[256]; + int ret; + + g_random(char_cookie, 16); + g_bytes_to_hexstr(char_cookie, 16, cookie, 33); + cookie[32] = '\0'; + + if (file == NULL) + { + g_sprintf(xauth_str, "xauth -q add :%d . %s", display, cookie); + } + else + { + g_sprintf(xauth_str, "xauth -q -f %s add :%d . %s", + file, display, cookie); + } + + dp = popen(xauth_str, "r"); + if (dp == NULL) + { + log_message(LOG_LEVEL_ERROR, "Unable to launch xauth"); + return 1; + } + + ret = pclose(dp); + if (ret < 0) + { + log_message(LOG_LEVEL_ERROR, "An error occured while running xauth"); + return 1; + } + + return 0; +} diff --git a/sesman/xauth.h b/sesman/xauth.h new file mode 100644 index 00000000..2bc98420 --- /dev/null +++ b/sesman/xauth.h @@ -0,0 +1,42 @@ +/** + * xrdp: A Remote Desktop Protocol server. + * + * Copyright (C) Jay Sorg 2004-2013 + * + * 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 xauth.c + * @brief XAUTHORITY handling code + * @author Emmaunel Blindauer + * + */ + +#ifndef XAUTH_H +#define XAUTH_H + +/** + * + * @brief create the XAUTHORITY file for the user according to the display and the cookie + * xauth uses XAUTHORITY if defined, ~/.Xauthority otherwise + * @param display The session display + * @param file If not NULL, write the authorization in the file instead of default location + * @return 0 if adding the cookie is ok + */ + +int DEFAULT_CC +add_xauth_cookie(int display, const char *file); + +#endif -- cgit v1.2.3 From 2927eed74c882322dc34b91e656c13de424a1418 Mon Sep 17 00:00:00 2001 From: BLINDAUER Emmanuel Date: Thu, 15 Dec 2016 22:45:12 +0100 Subject: - Update copyright - remove test on filename for xauth as we know what we send - better names for variables in xauth - if xauth fails, exit sesman - g_bytes_to_hexstr returns a null-teminated string, don't set it twice. --- sesman/session.c | 7 +++++-- sesman/xauth.c | 24 ++++++++---------------- 2 files changed, 13 insertions(+), 18 deletions(-) (limited to 'sesman/session.c') diff --git a/sesman/session.c b/sesman/session.c index 0540bc11..4e51867f 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -688,8 +688,11 @@ session_start_fork(tbus data, tui8 type, struct SCP_SESSION *s) g_snprintf(authfile, 255, "%s", ".Xauthority"); } - /* Add the entry in XAUTHORITY file */ - add_xauth_cookie(display, authfile); + /* Add the entry in XAUTHORITY file or exit if error */ + if (add_xauth_cookie(display, authfile) != 0) + { + g_exit(1); + } if (type == SESMAN_SESSION_TYPE_XORG) { diff --git a/sesman/xauth.c b/sesman/xauth.c index b4375b4f..c019b782 100644 --- a/sesman/xauth.c +++ b/sesman/xauth.c @@ -1,7 +1,8 @@ /** * xrdp: A Remote Desktop Protocol server. * - * Copyright (C) Jay Sorg 2004-2013 + * Copyright (C) Jay Sorg 2004-2017 + * Copyright (C) Emmanuel Blindauer 2004-2017 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +21,6 @@ * * @file xauth.c * @brief XAUTHORITY handling code - * @author Emmaunel Blindauer * */ @@ -34,24 +34,16 @@ int DEFAULT_CC add_xauth_cookie(int display, const char *file) { FILE *dp; - char cookie[33]; - char char_cookie[16]; + char cookie_str[33]; + char cookie_bin[16]; char xauth_str[256]; int ret; - g_random(char_cookie, 16); - g_bytes_to_hexstr(char_cookie, 16, cookie, 33); - cookie[32] = '\0'; + g_random(cookie_bin, 16); + g_bytes_to_hexstr(cookie_bin, 16, cookie_str, 33); - if (file == NULL) - { - g_sprintf(xauth_str, "xauth -q add :%d . %s", display, cookie); - } - else - { - g_sprintf(xauth_str, "xauth -q -f %s add :%d . %s", - file, display, cookie); - } + g_sprintf(xauth_str, "xauth -q -f %s add :%d . %s", + file, display, cookie_str); dp = popen(xauth_str, "r"); if (dp == NULL) -- cgit v1.2.3