From dd020e971b06bea7307a21e7626fa2d13e299034 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Wed, 12 Mar 2025 17:02:16 +0000 Subject: [PATCH 1/2] Rename sesman privilege detection function access_login_mng_allowed() -> access_login_is_admin() --- sesman/libsesman/sesman_access.c | 4 ++-- sesman/libsesman/sesman_access.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/sesman/libsesman/sesman_access.c b/sesman/libsesman/sesman_access.c index defb488f..3e19561e 100644 --- a/sesman/libsesman/sesman_access.c +++ b/sesman/libsesman/sesman_access.c @@ -168,8 +168,8 @@ access_login_allowed(const struct config_security *cfg_sec, const char *user) /******************************************************************************/ int -access_login_mng_allowed(const struct config_security *cfg_sec, - const char *user) +access_login_is_admin(const struct config_security *cfg_sec, + const char *user) { int rv = 0; diff --git a/sesman/libsesman/sesman_access.h b/sesman/libsesman/sesman_access.h index 8287fba9..1a47c2ce 100644 --- a/sesman/libsesman/sesman_access.h +++ b/sesman/libsesman/sesman_access.h @@ -42,13 +42,13 @@ access_login_allowed(const struct config_security *cfg_sec, /** * - * @brief Checks if the user is allowed to access the terminal server for management + * @brief Checks if the user is allowed to admin the terminal server * @param user the user to check - * @return 0 if access is denied, !=0 if allowed + * @return != 0 if the user can perform administrative tasks * */ int -access_login_mng_allowed(const struct config_security *cfg_sec, - const char *user); +access_login_is_admin(const struct config_security *cfg_sec, + const char *user); #endif From 86c7fa63b93fe9840c2d37a110ecc98bfe57686d Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Fri, 20 Dec 2024 16:58:24 +0000 Subject: [PATCH 2/2] Give privilege to users in TerminalServerAdmins Revives the currently unused TerminalServerAdmins group. Users in this group will eventually have special privileges for session management. Currently, members of this group will be allowed to list all sessions with the xrdp-sesadmin command. --- docs/man/sesman.ini.5.in | 17 ++- docs/man/xrdp-sesadmin.8.in | 3 +- sesman/eicp_process.c | 6 + sesman/libsesman/sesman_access.c | 222 +++++++++++++++---------------- sesman/pre_session_list.h | 1 + sesman/scp_process.c | 23 +++- sesman/sesman.ini.in | 3 +- sesman/session_list.c | 15 ++- sesman/session_list.h | 5 +- 9 files changed, 162 insertions(+), 133 deletions(-) diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index 6c44a66f..7f1f2914 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -271,8 +271,9 @@ login for all users is enabled. .TP \fBTerminalServerAdmins\fR=\fIgroup\fR -\fIThis option is currently ignored!\fR Only members of this group can -have session management rights. +Members of this group can use the \fBxrdp-sesadmin\fR command to +administer sessions started by other users. The root user is always +considered to be in this group. .TP \fBRestrictOutboundClipboard\fR=\fI[all|none|text|file|image]\fR @@ -334,8 +335,16 @@ To keep compatibility, the following aliases are also available. .TP \fBAlwaysGroupCheck\fR=\fI[true|false]\fR -If set to \fB1\fR, \fBtrue\fR or \fByes\fR, require group membership even -if the group specified in \fBTerminalServerUsers\fR doesn't exist. +If set to \fB1\fR, \fBtrue\fR or \fByes\fR:- +.RS +.HP 3 +- For normal logins, require group membership even if the group specified +in \fBTerminalServerUsers\fR doesn't exist. +.HP 3 +- An error message may be generated when any user authenticates +if the group specified in \fBTerminalServerAdmins\fR doesn't exist. This is +because the system is unable to check whether the user is an administrator. +.RE .TP \fBAllowAlternateShell\fR=\fI[true|false]\fR diff --git a/docs/man/xrdp-sesadmin.8.in b/docs/man/xrdp-sesadmin.8.in index cba29a37..69c4bca5 100644 --- a/docs/man/xrdp-sesadmin.8.in +++ b/docs/man/xrdp-sesadmin.8.in @@ -37,7 +37,8 @@ Valid commands are: .RS 4 .TP .B list -List active sessions for the current user. +List active sessions for the current user. Members of the +\fBTerminalServerAdmins\fR group can view active sessions for all users. .TP .BI kill: sid Kills the session specified the given \fIsession id\fP. diff --git a/sesman/eicp_process.c b/sesman/eicp_process.c index 6705f7e9..e62939ae 100644 --- a/sesman/eicp_process.c +++ b/sesman/eicp_process.c @@ -36,6 +36,8 @@ #include "pre_session_list.h" #include "scp.h" #include "sesman.h" +#include "sesman_access.h" +#include "sesman_config.h" /******************************************************************************/ @@ -79,6 +81,10 @@ process_sys_login_response(struct pre_session_item *psi) psi->client_trans->callback_data = (void *)psi; psi->login_state = E_PS_LOGIN_SYS; psi->uid = uid; + // For system logins, don't allow admin access + //psi->is_admin = access_login_mng_allowed(&g_cfg->sec, + // psi->username); + psi->is_admin = 0; } } } diff --git a/sesman/libsesman/sesman_access.c b/sesman/libsesman/sesman_access.c index 3e19561e..34937476 100644 --- a/sesman/libsesman/sesman_access.c +++ b/sesman/libsesman/sesman_access.c @@ -39,131 +39,90 @@ /******************************************************************************/ /** - * Root user login check + * user is root * - * @param cfg_sec Sesman security config - * @param user user name - * @return 0 if user is root and root accesses are not allowed + * @param username + * @return 1 if user is UID 0 */ static int -root_login_check(const struct config_security *cfg_sec, - const char *user) +user_is_root(const char *user) { - int rv = 0; - if (cfg_sec->allow_root) - { - rv = 1; - } - else - { - // Check the UID of the user isn't 0 - int uid; - if (g_getuser_info_by_name(user, &uid, NULL, NULL, NULL, NULL) != 0) - { - LOG(LOG_LEVEL_ERROR, "Can't get UID for user %s", user); - } - else if (0 == uid) - { - LOG(LOG_LEVEL_ERROR, - "ROOT login attempted, but root login is disabled"); - } - else - { - rv = 1; - } - } - return rv; -} - -/******************************************************************************/ -/** - * Common access control for group checks - * @param group Group name - * @param param Where group comes from (e.g. "TerminalServerUsers") - * @param always_check_group 0 if a missing group allows a login - * @param user Username - * @return != 0 if the access is allowed */ - -static int -access_login_allowed_common(const char *group, const char *param, - int always_check_group, - const char *user) -{ - int rv = 0; - int gid; - int ok; - - if (group == NULL || group[0] == '\0') - { - /* Group is not defined. Default access depends on whether - * we must have the group or not */ - if (always_check_group) - { - LOG(LOG_LEVEL_ERROR, - "%s group is not defined. Access denied for %s", - param, user); - } - else - { - LOG(LOG_LEVEL_INFO, - "%s group is not defined. Access granted for %s", - param, user); - rv = 1; - } - } - else if (g_getgroup_info(group, &gid) != 0) - { - /* Group is defined but doesn't exist. Default access depends - * on whether we must have the group or not */ - if (always_check_group) - { - LOG(LOG_LEVEL_ERROR, - "%s group %s doesn't exist. Access denied for %s", - param, group, user); - } - else - { - LOG(LOG_LEVEL_INFO, - "%s group %s doesn't exist. Access granted for %s", - param, group, user); - rv = 1; - } - } - else if (0 != g_check_user_in_group(user, gid, &ok)) - { - LOG(LOG_LEVEL_ERROR, "Error checking %s group %s. " - "Access denied for %s", param, group, user); - } - else if (!ok) - { - LOG(LOG_LEVEL_ERROR, "User %s is not in %s group %s. Access denied", - user, param, group); - } - else - { - LOG(LOG_LEVEL_INFO, "User %s is in %s group %s. Access granted", - user, param, group); - rv = 1; - } - - return rv; + int uid = -1; + (void)g_getuser_info_by_name(user, &uid, NULL, NULL, NULL, NULL); + return (uid == 0); } /******************************************************************************/ int access_login_allowed(const struct config_security *cfg_sec, const char *user) { - int rv = 0; + int ok = 0; - if (root_login_check(cfg_sec, user)) + if (!cfg_sec->allow_root && user_is_root(user)) { - rv = access_login_allowed_common(cfg_sec->ts_users, - "TerminalServerUsers", - cfg_sec->ts_always_group_check, - user); + LOG(LOG_LEVEL_ERROR, + "ROOT login attempted, but root login is disabled"); + } + else + { + const char *group = cfg_sec->ts_users; + const char *param = "TerminalServerUsers"; + int gid = -1; + + if (group == NULL || group[0] == '\0') + { + /* Group is not defined. Default access depends on whether + * we must have the group or not */ + if (cfg_sec->ts_always_group_check) + { + LOG(LOG_LEVEL_ERROR, + "%s group is not defined. Access denied for %s", + param, user); + } + else + { + LOG(LOG_LEVEL_INFO, + "%s group is not defined. Access granted for %s", + param, user); + ok = 1; + } + } + else if (g_getgroup_info(group, &gid) != 0) + { + /* Group is defined but doesn't exist. Default access depends + * on whether we must have the group or not */ + if (cfg_sec->ts_always_group_check) + { + LOG(LOG_LEVEL_ERROR, + "%s group %s doesn't exist. Access denied for %s", + param, group, user); + } + else + { + LOG(LOG_LEVEL_INFO, + "%s group %s doesn't exist. Access granted for %s", + param, group, user); + ok = 1; + } + } + else if (0 != g_check_user_in_group(user, gid, &ok)) + { + LOG(LOG_LEVEL_ERROR, "Error checking %s group %s. " + "Access denied for %s", param, group, user); + } + else if (!ok) + { + LOG(LOG_LEVEL_ERROR, "User %s is not in %s group %s. Access denied", + user, param, group); + } + else + { + LOG(LOG_LEVEL_INFO, "User %s is in %s group %s. Access granted", + user, param, group); + } } - return rv; + return ok; } /******************************************************************************/ @@ -171,15 +130,42 @@ int access_login_is_admin(const struct config_security *cfg_sec, const char *user) { - int rv = 0; + int ok = 0; - if (root_login_check(cfg_sec, user)) + const char *group = cfg_sec->ts_admins; + const char *param = "TerminalServerAdmins"; + int gid = -1; + + if (group == NULL || group[0] == '\0') { - rv = access_login_allowed_common(cfg_sec->ts_admins, - "TerminalServerAdmins", - 1, - user); + if (cfg_sec->ts_always_group_check) + { + LOG(LOG_LEVEL_ERROR, "%s group is not defined", param); + } + } + else if (g_getgroup_info(group, &gid) != 0) + { + /* Group is defined but doesn't exist */ + if (cfg_sec->ts_always_group_check) + { + LOG(LOG_LEVEL_ERROR, "%s group %s doesn't exist.", param, group); + } + } + else if (0 != g_check_user_in_group(user, gid, &ok)) + { + LOG(LOG_LEVEL_ERROR, "Error checking %s group %s.", param, group); } - return rv; + // Root always has access. Do other checks and logging first + if (!ok && user_is_root(user)) + { + ok = 1; + } + + if (ok) + { + LOG(LOG_LEVEL_INFO, "User %s is in %s group %s", user, param, group); + } + + return ok; } diff --git a/sesman/pre_session_list.h b/sesman/pre_session_list.h index 5f217731..6be6905c 100644 --- a/sesman/pre_session_list.h +++ b/sesman/pre_session_list.h @@ -77,6 +77,7 @@ struct pre_session_item uid_t uid; ///< User char *username; ///< Username from UID (at time of logon) char start_ip_addr[MAX_PEER_ADDRSTRLEN]; + int is_admin; }; diff --git a/sesman/scp_process.c b/sesman/scp_process.c index 62960b37..addce038 100644 --- a/sesman/scp_process.c +++ b/sesman/scp_process.c @@ -204,11 +204,21 @@ authenticate_and_authorize_uds_connection(struct pre_session_item *psi, } else { - LOG(LOG_LEVEL_INFO, "Access permitted for user: %s", - username); psi->login_state = E_PS_LOGIN_UDS; psi->uid = uid; psi->start_ip_addr[0] = '\0'; + psi->is_admin = access_login_is_admin(&g_cfg->sec, + psi->username); + if (psi->is_admin) + { + LOG(LOG_LEVEL_INFO, "Admin access permitted for user: %s", + username); + } + else + { + LOG(LOG_LEVEL_INFO, "Normal access permitted for user: %s", + username); + } } } @@ -555,7 +565,14 @@ process_list_sessions_request(struct pre_session_item *psi) "Received request from %s to list sessions for user %s", psi->peername, psi->username); - info = session_list_get_byuid(psi->uid, &cnt, 0); + if (psi->is_admin) + { + info = session_list_get_byuid(NULL, &cnt, 0); + } + else + { + info = session_list_get_byuid(&psi->uid, &cnt, 0); + } for (i = 0; rv == 0 && i < cnt; ++i) { diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in index 9983a7cb..d800d3bf 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -18,7 +18,8 @@ XAuthorityInSystemDir=no TerminalServerUsers=tsusers TerminalServerAdmins=tsadmins ; When AlwaysGroupCheck=false access will be permitted -; if the group TerminalServerUsers is not defined. +; if the group TerminalServerUsers is not defined, and the +; non-existence of TerminalServerAdmins will not be reported AlwaysGroupCheck=false ; When RestrictOutboundClipboard=all clipboard from the ; server is not pushed to the client. diff --git a/sesman/session_list.c b/sesman/session_list.c index 090d441e..54392cf5 100644 --- a/sesman/session_list.c +++ b/sesman/session_list.c @@ -438,7 +438,7 @@ session_list_get_bydata(uid_t uid, /******************************************************************************/ struct scp_session_info * -session_list_get_byuid(uid_t uid, unsigned int *cnt, unsigned int flags) +session_list_get_byuid(const uid_t *uid, unsigned int *cnt, unsigned int flags) { int i; struct scp_session_info *sess; @@ -447,13 +447,20 @@ session_list_get_byuid(uid_t uid, unsigned int *cnt, unsigned int flags) count = 0; - LOG(LOG_LEVEL_DEBUG, "searching for session by UID: %d", uid); + if (uid != NULL) + { + LOG(LOG_LEVEL_DEBUG, "searching for session by UID: %d", (int)*uid); + } + else + { + LOG(LOG_LEVEL_DEBUG, "searching for all sessions"); + } for (i = 0 ; i < g_session_list->count ; ++i) { const struct session_item *si; si = (const struct session_item *)list_get_item(g_session_list, i); - if (SESSION_IN_USE(si) && uid == si->uid) + if (SESSION_IN_USE(si) && (uid == NULL || *uid == si->uid)) { count++; } @@ -480,7 +487,7 @@ session_list_get_byuid(uid_t uid, unsigned int *cnt, unsigned int flags) const struct session_item *si; si = (const struct session_item *)list_get_item(g_session_list, i); - if (SESSION_IN_USE(si) && uid == si->uid) + if (SESSION_IN_USE(si) && (uid == NULL || *uid == si->uid)) { (sess[index]).sid = si->sesexec_pid; (sess[index]).display = si->display; diff --git a/sesman/session_list.h b/sesman/session_list.h index d33c56c6..a3e762f7 100644 --- a/sesman/session_list.h +++ b/sesman/session_list.h @@ -140,7 +140,8 @@ session_list_get_bydata(uid_t uid, /** * @brief retrieves session descriptions - * @param uid the UID for the descriptions + * @param uid the UID for the descriptions by reference, or NULL for + * all UIDs * @param[out] cnt The number of sessions returned * @param flags Future expansion * @return A block of session descriptions @@ -149,7 +150,7 @@ session_list_get_bydata(uid_t uid, * */ struct scp_session_info * -session_list_get_byuid(uid_t uid, unsigned int *cnt, unsigned int flags); +session_list_get_byuid(const uid_t *uid, unsigned int *cnt, unsigned int flags); /** *