radius: send the session ID as Acct-Session-Id in the Access-Request

RFC 2866 (5.5) allows an Access-Request to carry Acct-Session-Id and
requires the same value in the session's Accounting-Requests. Sending it
already at authentication time lets the RADIUS server correlate the two
exchanges by a single per-session key (e.g. for rlm_ippool, so concurrent
sessions of the same user from the same client do not collide on one IP
lease).

Documented as REQ-AUTH-AUTH-025, with a positive check in tests/radius
that the id sent as Acct-Session-Id in the Access-Request matches the
Accounting-Request.

Signed-off-by: Dmitrii <dimmispencer@gmail.com>
Resolves: #751

NEWS

@@ -1,4 +1,5 @@
 * Version 1.5.1 (unreleased)
+- Send the RADIUS session ID as Acct-Session-Id in the Access-Request (#751)
 - Hardened per-user/group configuration file lookup against path-traversal;
   a malformed per-user/group or default configuration file now causes the
   affected session to be rejected rather than silently ignored. A configured

doc/requirements/internal/authentication.md

@@ -734,18 +734,31 @@ containing: `User-Name` (REQ-AUTH-AUTH-024), `User-Password` (the submitted
 password), `NAS-IP-Address` or `NAS-IPv6-Address` (from `e->our_ip`, whichever
 family applies), `NAS-Identifier` if `nas-identifier=` is configured
 (REQ-AUTH-AUTH-022), `Calling-Station-Id` (the client's `remote_ip`),
-`Connect-Info` (the client's User-Agent string), `Service-Type =
+`Connect-Info` (the client's User-Agent string), `Acct-Session-Id` (the
+session identifier, when available — see below), `Service-Type =
 Authenticate-Only`, `NAS-Port-Type = Async`, and — when continuing a
 challenge (REQ-AUTH-AUTH-027) — the `State` attribute echoed back from the
 prior `Access-Challenge`.
+
+The `Acct-Session-Id` carries the same session identifier
+(`acct_info.safe_id`) that is later sent in the Accounting-Request packets
+(REQ-AUTH-ACCT-*). RFC 2866 (section 5.5) allows an Access-Request to carry
+`Acct-Session-Id` ("An Access-Request packet MAY have an Acct-Session-Id"),
+and requires that if it does, the NAS MUST use the same value in the
+Accounting-Request packets for that session — so the two exchanges correlate
+on one key. Emitting it during authentication gives the RADIUS server a
+per-session key already at Access-Request time (e.g. for `rlm_ippool`, so
+concurrent sessions of the same user from the same client do not collide on a
+single IP lease).
 **Strength:** MUST
 **Status:** DERIVED
 **Source:** src/auth/radius.c:293-430 (request construction, before
 `rc_auth()`/`rc_send_server()`)
 **Acceptance:** unit, local — capture the RADIUS request (e.g. via a test
 FreeRADIUS server with `auth_log`) for a normal login; confirm all listed
-attributes are present with expected values, and that a configured
+attributes are present with expected values, that a configured
-`nas-identifier` appears as `NAS-Identifier`.
+`nas-identifier` appears as `NAS-Identifier`, and that `Acct-Session-Id` is
+present and equals the session id reported in the matching Accounting-Request.
 **Links:** REQ-AUTH-AUTH-024, REQ-AUTH-AUTH-026, REQ-AUTH-AUTH-027
 
 ### REQ-AUTH-AUTH-026 — `Access-Accept` attributes populate group membership and per-session network configuration

src/auth/radius.c

@@ -167,6 +167,9 @@ static int radius_auth_init(void **ctx, void *pool, void *_vctx,
 		strlcpy(pctx->user_agent, info->user_agent,
 			sizeof(pctx->user_agent));
 
+	if (info->sid)
+		strlcpy(pctx->sid, info->sid, sizeof(pctx->sid));
+
 	*ctx = pctx;
 
 	return ERR_AUTH_CONTINUE;
@@ -375,6 +378,22 @@ static int radius_auth_pass(void *ctx, const char *pass, unsigned int pass_len)
 		goto cleanup;
 	}
 
+	/* RFC 2866 (5.5) allows an Access-Request to carry Acct-Session-Id and
+	 * requires the same value in the session's Accounting-Requests. Sending
+	 * it already at authentication time lets the RADIUS server correlate the
+	 * two exchanges by a single per-session key (#751). */
+	if (pctx->sid[0] != 0) {
+		if (rc_avpair_add(pctx->vctx->rh, &send, PW_ACCT_SESSION_ID,
+				  pctx->sid, -1, 0) == NULL) {
+			oc_syslog(
+				LOG_ERR,
+				"%s:%u: error in constructing radius message for user '%s'",
+				__func__, __LINE__, pctx->username);
+			ret = ERR_AUTH_FAIL;
+			goto cleanup;
+		}
+	}
+
 	if (pctx->user_agent[0] != 0) {
 		if (rc_avpair_add(pctx->vctx->rh, &send, PW_CONNECT_INFO,
 				  pctx->user_agent, -1, 0) == NULL) {

src/auth/radius.h

@@ -41,6 +41,7 @@ struct radius_vhost_ctx {
 struct radius_ctx_st {
 	char username[MAX_USERNAME_SIZE * 2];
 	char user_agent[MAX_AGENT_NAME];
+	char sid[SAFE_ID_SIZE]; /* session id, sent as Acct-Session-Id */
 
 	char *groupnames[MAX_GROUPS];
 	unsigned int groupnames_size;

src/sec-mod-auth.c

@@ -942,6 +942,7 @@ int handle_sec_auth_init(int cfd, sec_mod_st *sec, const SecAuthInitMsg *req,
 		st.ip = req->remote_ip;
 		st.our_ip = req->our_ip;
 		st.user_agent = req->user_agent;
+		st.sid = e->acct_info.safe_id;
 		st.id = pid;
 
 		ret = e->module->auth_init(&e->auth_ctx, e, e->vhost_auth_ctx,

src/sec-mod.h

@@ -63,6 +63,7 @@ typedef struct common_auth_init_st {
 	const char *ip;
 	const char *our_ip;
 	const char *user_agent;
+	const char *sid; /* printable session id (== acct_info.safe_id) */
 	unsigned int id;
 } common_auth_init_st;
 

tests/radius

@@ -204,4 +204,38 @@ if test -z "$OCTETS" || test "$OCTETS" = 0;then
 fi
 echo "Transferred ${OCTETS} bytes"
 
+# REQ-AUTH-AUTH-025: the session id must be sent as Acct-Session-Id already in
+# the Access-Request, using the same value that later appears in the
+# Accounting-Request so the two exchanges correlate.
+echo " * Verifying Acct-Session-Id is sent during authentication..."
+
+# Session id reported by the accounting exchange of the connected session.
+ACCT_SID=$(awk '
+	/Received Accounting-Request/ { in_acct=1 }
+	/Received Access-Request/ { in_acct=0 }
+	in_acct && /Acct-Session-Id/ {
+		v=$0; sub(/.*Acct-Session-Id = /, "", v); gsub(/"/, "", v); last=v
+	}
+	END { print last }' "${RADIUSLOG}")
+
+if test -z "${ACCT_SID}";then
+	cat ${RADIUSLOG}
+	echo "No Acct-Session-Id present in the Accounting-Request!"
+	exit 1
+fi
+
+# The same identifier must have been present in an Access-Request.
+FOUND=$(awk -v sid="${ACCT_SID}" '
+	/Received Access-Request/ { in_access=1 }
+	/Received Accounting-Request/ { in_access=0 }
+	in_access && /Acct-Session-Id/ && index($0, sid) { print "yes"; exit }
+	' "${RADIUSLOG}")
+
+if test "${FOUND}" != "yes";then
+	cat ${RADIUSLOG}
+	echo "Acct-Session-Id ${ACCT_SID} was not sent in any Access-Request!"
+	exit 1
+fi
+echo "Acct-Session-Id ${ACCT_SID} present in both Access-Request and Accounting-Request"
+
 exit 0