From 3e488773d7d59e9aa94a0940130a032e631d53cd Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 2 May 2022 12:54:40 +0100 Subject: [PATCH] Updated session allocation policy for sesman Made session allocation policies more readable and maintainable. The 'C' policy which was confusing before has been replaced with the 'Separate' keyword. This is a public interface change, but is unlikely to affect many users. The logging in session_get_bydata() is substantially improved, making it far easier to spot why sessions are getting matched or not matched. --- docs/man/sesman.ini.5.in | 38 ++++++----- sesman/config.c | 138 +++++++++++++++++++++++++++------------ sesman/config.h | 41 ++++++------ sesman/sesman.ini.in | 21 ++++-- sesman/session.c | 62 +++++++++++++----- 5 files changed, 198 insertions(+), 102 deletions(-) diff --git a/docs/man/sesman.ini.5.in b/docs/man/sesman.ini.5.in index af3be2a726..366ecfadc6 100644 --- a/docs/man/sesman.ini.5.in +++ b/docs/man/sesman.ini.5.in @@ -176,30 +176,34 @@ If set to \fI0\fR, idle sessions will never be disconnected by timeout. This works only with xorgxrdp sessions. Moreover, xorgxrdp must be v0.2.9 or later. .TP -\fBPolicy\fR=\fI[Default|UBD|UBI|UBC|UBDI|UBDC]\fR +\fBPolicy\fR=\fI[Default|Separate|{UBDI}]\fR Session allocation policy. Used to decide when to allocate a new session. Set to one of the following values: .br -.br -\fBDefault\fR - session per -.br -\fBUBD\fR - session per -.br -\fBUBI\fR - session per -.br -\fBUBC\fR - session per -.br -\fBUBDI\fR - session per -.br -\fBUBDC\fR - session per -.br +.RS +.HP 12 +\fBDefault\fR - Currently the same as "UB" for all session types +.HP 12 +\fBSeparate\fR - All sessions are separate. Sessions can never be rejoined, +and will need to be cleaned up manually, or automatically by setting other +sesman options. +.P +Alternatively combine one-or-more of the following options +.HP 4 +\fBU\fR - Sessions are separated per user +.HP 4 +\fBB\fR - Sessions are separated by bits-per-pixel +.HP 4 +\fBD\fR - Sessions are separated by initial display size +.HP 4 +\fBI\fR - Sessions are separated by IP address +.RE -.br -Note that the \fBUser\fR and \fBBitPerPixel\fR criteria cannot be turned +.IP +Note that the \fBU\fR and \fBB\fR criteria cannot be turned off. \fBDisplaySize\fR refers to the initial geometry of a connection, as actual display sizes can change dynamically. -.br .SH "SECURITY" Following parameters can be used in the \fB[Security]\fR section. diff --git a/sesman/config.c b/sesman/config.c index 4eabdd4687..23e7ff3715 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -37,6 +37,81 @@ #include "chansrv/chansrv_common.h" #include "scp.h" +static const struct bitmask_char policy_bits[] = +{ + { SESMAN_CFG_SESS_POLICY_U, 'U' }, + { SESMAN_CFG_SESS_POLICY_B, 'B' }, + { SESMAN_CFG_SESS_POLICY_D, 'D' }, + { SESMAN_CFG_SESS_POLICY_I, 'I' }, + BITMASK_CHAR_END_OF_LIST +}; + +/***************************************************************************//** + * Parse a session allocation policy string + */ +static unsigned int +parse_policy_string(const char *value) +{ + unsigned int rv; + char unrecognised[16]; + + if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_DFLT_S)) + { + rv = SESMAN_CFG_SESS_POLICY_DEFAULT; + } + else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_SEP_S)) + { + rv = SESMAN_CFG_SESS_POLICY_SEPARATE; + } + else + { + unrecognised[0] = '\0'; + rv = g_charstr_to_bitmask(value, policy_bits, unrecognised, + sizeof(unrecognised)); + if (unrecognised[0] != '\0') + { + LOG(LOG_LEVEL_WARNING, "Character(s) '%s' in the session" + " allocation policy are not recognised", unrecognised); + + if (g_strchr(unrecognised, 'C') != NULL || + g_strchr(unrecognised, 'c') != NULL) + { + /* Change from xrdp v0.9.x */ + LOG(LOG_LEVEL_WARNING, "Character 'C' is no longer used" + " in session allocation policies - use '%s'", + SESMAN_CFG_SESS_POLICY_SEP_S); + } + } + } + + return rv; +} + +/******************************************************************************/ +int +config_output_policy_string(unsigned int value, + char *buff, unsigned int bufflen) +{ + int rv = 0; + if (bufflen > 0) + { + if (value & SESMAN_CFG_SESS_POLICY_DEFAULT) + { + rv = g_snprintf(buff, bufflen, "Default"); + } + else if (value & SESMAN_CFG_SESS_POLICY_SEPARATE) + { + rv = g_snprintf(buff, bufflen, "Separate"); + } + else + { + rv = g_bitmask_to_charstr(value, policy_bits, buff, bufflen, NULL); + } + } + + return rv; +} + /***************************************************************************//** * * @brief Reads sesman [global] configuration section @@ -295,7 +370,8 @@ config_read_sessions(int file, struct config_sessions *se, struct list *param_n, struct list *param_v) { int i; - char *buf; + const char *buf; + const char *value; list_clear(param_v); list_clear(param_n); @@ -306,70 +382,43 @@ config_read_sessions(int file, struct config_sessions *se, struct list *param_n, se->max_idle_time = 0; se->max_disc_time = 0; se->kill_disconnected = 0; - se->policy = SESMAN_CFG_SESS_POLICY_DFLT; + se->policy = SESMAN_CFG_SESS_POLICY_DEFAULT; file_read_section(file, SESMAN_CFG_SESSIONS, param_n, param_v); for (i = 0; i < param_n->count; i++) { - buf = (char *)list_get_item(param_n, i); + buf = (const char *)list_get_item(param_n, i); + value = (const char *)list_get_item(param_v, i); if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_X11DISPLAYOFFSET)) { - se->x11_display_offset = g_atoi((char *)list_get_item(param_v, i)); + se->x11_display_offset = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_MAX)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_MAX)) { - se->max_sessions = g_atoi((char *)list_get_item(param_v, i)); + se->max_sessions = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_KILL_DISC)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_KILL_DISC)) { - se->kill_disconnected = g_text2bool((char *)list_get_item(param_v, i)); + se->kill_disconnected = g_text2bool(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_IDLE_LIMIT)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_IDLE_LIMIT)) { - se->max_idle_time = g_atoi((char *)list_get_item(param_v, i)); + se->max_idle_time = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_DISC_LIMIT)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_DISC_LIMIT)) { - se->max_disc_time = g_atoi((char *)list_get_item(param_v, i)); + se->max_disc_time = g_atoi(value); } - if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_POLICY_S)) + else if (0 == g_strcasecmp(buf, SESMAN_CFG_SESS_POLICY_S)) { - char *value = (char *)list_get_item(param_v, i); - if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_DFLT_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_DFLT; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBD_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBD; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBI_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBI; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBC_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBC; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBDI_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBDI; - } - else if (0 == g_strcasecmp(value, SESMAN_CFG_SESS_POLICY_UBDC_S)) - { - se->policy = SESMAN_CFG_SESS_POLICY_UBDC; - } - else /* silently ignore typos */ - { - se->policy = SESMAN_CFG_SESS_POLICY_DFLT; - } + se->policy = parse_policy_string(value); } } @@ -570,6 +619,7 @@ config_dump(struct config_sesman *config) struct config_security *sc; se = &(config->sess); sc = &(config->sec); + char policy_s[64]; /* Global sesman configuration */ g_writeln("Filename: %s", config->sesman_ini); @@ -583,13 +633,15 @@ config_dump(struct config_sesman *config) (config->auth_file_path ? config->auth_file_path : "disabled")); /* Session configuration */ + config_output_policy_string(se->policy, policy_s, sizeof(policy_s)); + g_writeln("Session configuration:"); g_writeln(" MaxSessions: %d", se->max_sessions); g_writeln(" X11DisplayOffset: %d", se->x11_display_offset); g_writeln(" KillDisconnected: %d", se->kill_disconnected); g_writeln(" IdleTimeLimit: %d", se->max_idle_time); g_writeln(" DisconnectedTimeLimit: %d", se->max_disc_time); - g_writeln(" Policy: %d", se->policy); + g_writeln(" Policy: %s", policy_s); /* Security configuration */ g_writeln("Security configuration:"); diff --git a/sesman/config.h b/sesman/config.h index b73ff46c7a..87698c5520 100644 --- a/sesman/config.h +++ b/sesman/config.h @@ -72,27 +72,18 @@ #define SESMAN_CFG_SESS_POLICY_S "Policy" #define SESMAN_CFG_SESS_POLICY_DFLT_S "Default" -#define SESMAN_CFG_SESS_POLICY_UBD_S "UBD" -#define SESMAN_CFG_SESS_POLICY_UBI_S "UBI" -#define SESMAN_CFG_SESS_POLICY_UBC_S "UBC" -#define SESMAN_CFG_SESS_POLICY_UBDI_S "UBDI" -#define SESMAN_CFG_SESS_POLICY_UBDC_S "UBDC" +#define SESMAN_CFG_SESS_POLICY_SEP_S "Separate" enum SESMAN_CFG_SESS_POLICY_BITS { - SESMAN_CFG_SESS_POLICY_D = 0x01, - SESMAN_CFG_SESS_POLICY_I = 0x02, - SESMAN_CFG_SESS_POLICY_C = 0x04 -}; - -enum SESMAN_CFG_SESS_POLICY -{ - SESMAN_CFG_SESS_POLICY_DFLT = 0, - SESMAN_CFG_SESS_POLICY_UBD = SESMAN_CFG_SESS_POLICY_D, - SESMAN_CFG_SESS_POLICY_UBI = SESMAN_CFG_SESS_POLICY_I, - SESMAN_CFG_SESS_POLICY_UBC = SESMAN_CFG_SESS_POLICY_C, - SESMAN_CFG_SESS_POLICY_UBDI = SESMAN_CFG_SESS_POLICY_D | SESMAN_CFG_SESS_POLICY_I, - SESMAN_CFG_SESS_POLICY_UBDC = SESMAN_CFG_SESS_POLICY_D | SESMAN_CFG_SESS_POLICY_C + /* If these two are set, they override everything else */ + SESMAN_CFG_SESS_POLICY_DEFAULT = (1 << 0), + SESMAN_CFG_SESS_POLICY_SEPARATE = (1 << 1), + /* Configuration bits */ + SESMAN_CFG_SESS_POLICY_U = (1 << 2), + SESMAN_CFG_SESS_POLICY_B = (1 << 3), + SESMAN_CFG_SESS_POLICY_D = (1 << 4), + SESMAN_CFG_SESS_POLICY_I = (1 << 5) }; /** @@ -180,7 +171,7 @@ struct config_sessions * @var policy * @brief session allocation policy */ - enum SESMAN_CFG_SESS_POLICY policy; + unsigned int policy; }; /** @@ -304,4 +295,16 @@ config_dump(struct config_sesman *config); void config_free(struct config_sesman *cs); +/** + * Converts a session allocation Policy value to a strin + * @param value - Session allocation policy value + * @param buff - Buffer for result + * @param bufflen - Length of buffer + * @return Length of string that would be required without a terminator + * to write the whole output (like snprintf()) + */ +int +config_output_policy_string(unsigned int value, + char *buff, unsigned int bufflen); + #endif diff --git a/sesman/sesman.ini.in b/sesman/sesman.ini.in index 15ef50bad2..278224a19d 100644 --- a/sesman/sesman.ini.in +++ b/sesman/sesman.ini.in @@ -69,13 +69,20 @@ DisconnectedTimeLimit=0 IdleTimeLimit=0 ;; Policy - session allocation policy -; Type: enum [ "Default" | "UBD" | "UBI" | "UBC" | "UBDI" | "UBDC" ] -; "Default" session per -; "UBD" session per -; "UBI" session per -; "UBC" session per -; "UBDI" session per -; "UBDC" session per +; +; Type: enum [ "Default" | "Separate" | Combination from {UBDI} ] +; "Default" Currently same as "UB" +; "Separate" All sessions are separate. Sessions can never be rejoined, +; and will need to be cleaned up manually, or automatically +; by setting other sesman options. +; +; Combination options:- +; U Sessions are separated per user +; B Sessions are separated by bits-per-pixel +; D Sessions are separated by initial display size +; I Sessions are separated by IP address +; +; The options U and B are always active, and cannot be de-selected. Policy=Default [Logging] diff --git a/sesman/session.c b/sesman/session.c index b59be93d13..f95a5e234c 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -88,17 +88,31 @@ dumpItemsToString(struct list *self, char *outstr, int len) struct session_item * session_get_bydata(const struct session_parameters *sp) { + char policy_str[64]; struct session_chain *tmp; - enum SESMAN_CFG_SESS_POLICY policy = g_cfg->sess.policy; + int policy = g_cfg->sess.policy; - LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: search policy %d U %s W %d H %d bpp %d T %d IP %s", - policy, sp->username, sp->width, sp->height, sp->bpp, - sp->type, sp->ip_addr); + if ((policy & SESMAN_CFG_SESS_POLICY_DEFAULT) != 0) + { + /* In the past (i.e. xrdp before v0.9.14), the default + * session policy varied by sp->type. If this is needed again + * in the future, here is the place to add it */ + policy = SESMAN_CFG_SESS_POLICY_U | SESMAN_CFG_SESS_POLICY_B; + } + + config_output_policy_string(policy, policy_str, sizeof(policy_str)); - if ((policy & SESMAN_CFG_SESS_POLICY_C) != 0) + LOG(LOG_LEVEL_DEBUG, + "%s: search policy=%s type=%s U=%s B=%d D=(%dx%d) I=%s", + __func__, + policy_str, SCP_SESSION_TYPE_TO_STR(sp->type), + sp->username, sp->bpp, sp->width, sp->height, + sp->ip_addr); + + /* 'Separate' policy never matches */ + if (policy & SESMAN_CFG_SESS_POLICY_SEPARATE) { - /* Never matches */ + LOG(LOG_LEVEL_DEBUG, "%s: No matches possible", __func__); return NULL; } @@ -107,19 +121,33 @@ session_get_bydata(const struct session_parameters *sp) struct session_item *item = tmp->item; LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: try %p U %s W %d H %d bpp %d T %d IP %s", + "%s: try %p type=%s U=%s B=%d D=(%dx%d) I=%s", + __func__, item, + SCP_SESSION_TYPE_TO_STR(item->type), item->name, + item->bpp, item->width, item->height, - item->bpp, item->type, item->start_ip_addr); - if (g_strncmp(sp->username, item->name, 255) != 0 || - item->bpp != sp->bpp || - item->type != sp->type) + if (item->type != sp->type) + { + LOG(LOG_LEVEL_DEBUG, "%s: Type doesn't match", __func__); + continue; + } + + if ((policy & SESMAN_CFG_SESS_POLICY_U) && + g_strncmp(sp->username, item->name, sizeof(item->name) - 1) != 0) { LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: Basic parameters don't match"); + "%s: Username doesn't match for 'U' policy", __func__); + continue; + } + + if ((policy & SESMAN_CFG_SESS_POLICY_B) && item->bpp != sp->bpp) + { + LOG(LOG_LEVEL_DEBUG, + "%s: bpp doesn't match for 'B' policy", __func__); continue; } @@ -127,7 +155,7 @@ session_get_bydata(const struct session_parameters *sp) (item->width != sp->width || item->height != sp->height)) { LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: Dimensions don't match for 'D' policy"); + "%s: Dimensions don't match for 'D' policy", __func__); continue; } @@ -135,14 +163,16 @@ session_get_bydata(const struct session_parameters *sp) g_strcmp(item->start_ip_addr, sp->ip_addr) != 0) { LOG(LOG_LEVEL_DEBUG, - "session_get_bydata: IPs don't match for 'I' policy"); + "%s: IPs don't match for 'I' policy", __func__); continue; } - LOG(LOG_LEVEL_DEBUG, "session_get_bydata: Got match"); + LOG(LOG_LEVEL_DEBUG, + "%s: Got match, display=%d", __func__, item->display); return item; } + LOG(LOG_LEVEL_DEBUG, "%s: No matches found", __func__); return 0; }