From 4fe68272ec5e507cedd22852cdd65ee5e1d2c572 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] 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 | 16 ++- docs/man/xrdp-sesadmin.8.in | 3 +- sesman/eicp_process.c | 4 + sesman/libsesman/sesman_access.c | 191 ++++++++++++++----------------- sesman/pre_session_list.h | 1 + sesman/scp_process.c | 11 +- sesman/sesman.ini.in | 3 +- sesman/session_list.c | 15 ++- sesman/session_list.h | 5 +- 9 files changed, 134 insertions(+), 115 deletions(-) diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index b73fd880da..57f9b0c4ac 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -259,8 +259,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 have session management rights, and can view +session information for other users. The root user is always considered +to be in this group. .TP \fBRestrictOutboundClipboard\fR=\fI[all|none|text|file|image]\fR @@ -322,8 +323,15 @@ 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 +- For admin logins, log an error if the group specified in +\fBTerminalServerAdmins\fR doesn't exist. +.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 cba29a37e9..69c4bca5b7 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 6705f7e929..ecdf8d067c 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,8 @@ 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; + psi->is_admin = access_login_mng_allowed(&g_cfg->sec, + psi->username); } } } diff --git a/sesman/libsesman/sesman_access.c b/sesman/libsesman/sesman_access.c index defb488f34..2c334fc4a9 100644 --- a/sesman/libsesman/sesman_access.c +++ b/sesman/libsesman/sesman_access.c @@ -39,147 +39,134 @@ /******************************************************************************/ /** - * 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) + 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 ok = 0; + + if (!cfg_sec->allow_root && user_is_root(user)) { - rv = 1; + LOG(LOG_LEVEL_ERROR, + "ROOT login attempted, but root login is disabled"); } 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) + 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) { - LOG(LOG_LEVEL_ERROR, "Can't get UID for user %s", user); + /* 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 == uid) + else if (0 != g_check_user_in_group(user, gid, &ok)) { - LOG(LOG_LEVEL_ERROR, - "ROOT login attempted, but root login is disabled"); + 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 { - rv = 1; + LOG(LOG_LEVEL_INFO, "User %s is in %s group %s. Access granted", + user, param, group); } } - return rv; + + return ok; } /******************************************************************************/ -/** - * 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 +access_login_mng_allowed(const struct config_security *cfg_sec, + const char *user) { - int rv = 0; - int gid; - int ok; + int ok = 0; + + const char *group = cfg_sec->ts_admins; + const char *param = "TerminalServerAdmins"; + 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 (always_check_group) - { - LOG(LOG_LEVEL_ERROR, - "%s group is not defined. Access denied for %s", - param, user); - } - else + if (cfg_sec->ts_always_group_check) { - LOG(LOG_LEVEL_INFO, - "%s group is not defined. Access granted for %s", - param, user); - rv = 1; + 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. Default access depends - * on whether we must have the group or not */ - if (always_check_group) + /* Group is defined but doesn't exist */ + 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); - rv = 1; + 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. " - "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; + LOG(LOG_LEVEL_ERROR, "Error checking %s group %s.", param, group); } - return rv; -} - -/******************************************************************************/ -int -access_login_allowed(const struct config_security *cfg_sec, const char *user) -{ - int rv = 0; - - if (root_login_check(cfg_sec, user)) + // Root always has access. Do other checks and logging first + if (!ok && user_is_root(user)) { - rv = access_login_allowed_common(cfg_sec->ts_users, - "TerminalServerUsers", - cfg_sec->ts_always_group_check, - user); + ok = 1; } - return rv; -} - -/******************************************************************************/ -int -access_login_mng_allowed(const struct config_security *cfg_sec, - const char *user) -{ - int rv = 0; - - if (root_login_check(cfg_sec, user)) + if (ok) { - rv = access_login_allowed_common(cfg_sec->ts_admins, - "TerminalServerAdmins", - 1, - user); + LOG(LOG_LEVEL_INFO, "User %s is in %s group %s. Admin access granted", + user, param, group); } - return rv; + return ok; } diff --git a/sesman/pre_session_list.h b/sesman/pre_session_list.h index 5f21773181..6be6905c6f 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 62960b370d..e6eb00753e 100644 --- a/sesman/scp_process.c +++ b/sesman/scp_process.c @@ -209,6 +209,8 @@ authenticate_and_authorize_uds_connection(struct pre_session_item *psi, psi->login_state = E_PS_LOGIN_UDS; psi->uid = uid; psi->start_ip_addr[0] = '\0'; + psi->is_admin = access_login_mng_allowed(&g_cfg->sec, + psi->username); } } @@ -555,7 +557,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 cd49a148ec..12e5ac81ed 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -17,7 +17,8 @@ MaxLoginRetry=4 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 b2c0ea59de..3b5f448344 100644 --- a/sesman/session_list.c +++ b/sesman/session_list.c @@ -420,7 +420,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; @@ -429,13 +429,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++; } @@ -462,7 +469,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 b018afad97..c7fb2845fa 100644 --- a/sesman/session_list.h +++ b/sesman/session_list.h @@ -132,7 +132,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 @@ -141,7 +142,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); /** *