radius: add group-separator option for OU= Class attributes

Freeradius deployments may send groups in the Class attribute using a
comma as separator (e.g. "OU=group1,group2") rather than the semicolon
that ocserv expects by default. Add a group-separator option to the
radius auth configuration to allow this to be changed:

  auth = "radius[config=...,group-separator=comma]"

Accepted values are 'semicolon' (default) and 'comma'. The literal
character is not accepted in the config syntax as it conflicts with the
key=value pair delimiter.

Resolves: #428

Signed-off-by: Nikos Mavrogiannopoulos <n.mavrogiannopoulos@gmail.com>

NEWS

@@ -1,4 +1,7 @@
 * Version 1.4.2 (unreleased)
+- radius: added group-separator option to auth configuration, allowing
+  the separator used in OU= Class attributes to be set to 'semicolon'
+  (default) or 'comma', to support Freeradius deployments (#428)
 - The bundled llhttp was updated to 9.3.1.
 - occtl: Added 'terminate user', 'terminate id', and 'terminate session'
   commands that disconnect users and invalidate their session cookies,

doc/README-radius.md

@@ -106,6 +106,11 @@ ATTRIBUTE	Framed-Route		22	string
 # format "OU=group1;group2" or by a single group name
 # in the attribute. It is possible to specify multiple
 # groups in separate class attributes.
+# The separator used in the OU= format defaults to ';' and can
+# be changed via the group-separator option (accepted values:
+# 'semicolon', 'comma'). For example, to use comma-separated
+# groups as sent by Freeradius:
+#   auth = "radius[config=...,group-separator=comma]"
 # Note that this works only when groupconfig is set to
 # true, and if the groups sent by the server are made known
 # to ocserv, via the select-group config variable.

doc/sample.config

@@ -28,11 +28,13 @@
 # an oath password file to be used for one time passwords; the format of
 # the file is described in https://github.com/archiecobbs/mod-authn-otp/wiki/UsersFile
 #
-# radius[config=/etc/radiusclient/radiusclient.conf,groupconfig=true,nas-identifier=name]:
+# radius[config=/etc/radiusclient/radiusclient.conf,groupconfig=true,nas-identifier=name,group-separator=semicolon]:
 #  The radius option requires specifying freeradius-client configuration
 # file. If the groupconfig option is set, then config-per-user/group will be overridden,
 # and all configuration will be read from radius. That also includes the
-# Acct-Interim-Interval, and Session-Timeout values.
+# Acct-Interim-Interval, and Session-Timeout values. The group-separator option
+# sets the separator used in the OU= Class attribute; accepted values are
+# 'semicolon' (default) and 'comma'.
 #
 # See doc/README-radius.md for the supported radius configuration attributes.
 #

src/auth/radius.c

@@ -98,6 +98,14 @@ static void radius_vhost_init(void **_vctx, void *pool, void *additional)
 		vctx->nas_identifier[0] = 0;
 	}
 
+	if (config->group_separator) {
+		strlcpy(vctx->group_separator, config->group_separator,
+			sizeof(vctx->group_separator));
+	} else {
+		strlcpy(vctx->group_separator, ";",
+			sizeof(vctx->group_separator));
+	}
+
 	if (rc_read_dictionary(vctx->rh, rc_conf_str(vctx->rh, "dictionary")) !=
 	    0) {
 		fprintf(stderr, "error reading the radius dictionary\n");
@@ -233,10 +241,11 @@ static void append_route(struct radius_ctx_st *pctx, const char *route,
 	}
 }
 
-/* Parses group of format "OU=group1;group2;group3" */
+/* Parses group of format "OU=group1<sep>group2<sep>group3" */
 static void parse_groupnames(struct radius_ctx_st *pctx, const char *full)
 {
 	char *p, *p2;
+	const char *sep = pctx->vctx->group_separator;
 
 	if (pctx->groupnames_size >= MAX_GROUPS) {
 		oc_syslog(
@@ -252,13 +261,13 @@ static void parse_groupnames(struct radius_ctx_st *pctx, const char *full)
 		if (p == NULL)
 			return;
 
-		p2 = strsep(&p, ";");
+		p2 = strsep(&p, sep);
 		while (p2 != NULL) {
 			pctx->groupnames[pctx->groupnames_size++] = p2;
 
 			oc_syslog(LOG_DEBUG, "radius-auth: found group %s", p2);
 
-			p2 = strsep(&p, ";");
+			p2 = strsep(&p, sep);
 
 			if (pctx->groupnames_size == MAX_GROUPS) {
 				if (p2)

src/auth/radius.h

@@ -35,6 +35,7 @@
 struct radius_vhost_ctx {
 	rc_handle *rh;
 	char nas_identifier[64];
+	char group_separator[2]; /* separator used in OU= Class attributes */
 };
 
 struct radius_ctx_st {

src/common-config.h

@@ -48,6 +48,8 @@ typedef struct gssapi_cfg_st {
 typedef struct radius_cfg_st {
 	char *config;
 	char *nas_identifier;
+	/* separator character(s) used in OU= Class attributes */
+	const char *group_separator;
 } radius_cfg_st;
 
 typedef struct plain_cfg_st {

src/subconfig.c

@@ -250,6 +250,19 @@ void *radius_get_brackets_string(void *pool, struct perm_cfg_st *config,
 				   0) {
 				additional->nas_identifier = vals[i].value;
 				vals[i].value = NULL;
+			} else if (strcasecmp(vals[i].name,
+					      "group-separator") == 0) {
+				if (strcasecmp(vals[i].value, "comma") == 0)
+					additional->group_separator = ",";
+				else if (strcasecmp(vals[i].value,
+						    "semicolon") == 0)
+					additional->group_separator = ";";
+				else {
+					fprintf(stderr,
+						"unknown group-separator value '%s'; use 'semicolon' or 'comma'\n",
+						vals[i].value);
+					exit(EXIT_FAILURE);
+				}
 			} else if (strcasecmp(vals[i].name, "groupconfig") ==
 				   0) {
 				if (CHECK_TRUE(vals[i].value))