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.

Author: matt335672

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 <User,BitPerPixel>
-.br
-\fBUBD\fR - session per <User,BitPerPixel,DisplaySize>
-.br
-\fBUBI\fR - session per <User,BitPerPixel,IPAddr>
-.br
-\fBUBC\fR - session per <User,BitPerPixel,Connection>
-.br
-\fBUBDI\fR - session per <User,BitPerPixel,DisplaySize,IPAddr>
-.br
-\fBUBDC\fR - session per <User,BitPerPixel,DisplaySize,Connection>
-.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.

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:");

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

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 <User,BitPerPixel>
-; "UBD" session per <User,BitPerPixel,DisplaySize>
-; "UBI" session per <User,BitPerPixel,IPAddr>
-; "UBC" session per <User,BitPerPixel,Connection>
-; "UBDI" session per <User,BitPerPixel,DisplaySize,IPAddr>
-; "UBDC" session per <User,BitPerPixel,DisplaySize,Connection>
+;
+;  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]