From df086188d7a47471cbb0240031c3de0418e9e252 Mon Sep 17 00:00:00 2001 From: Dmitrii Date: Tue, 14 Jul 2026 19:28:01 +0300 Subject: [PATCH] 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 Resolves: #751 --- NEWS | 1 + doc/requirements/internal/authentication.md | 19 ++++++++++-- src/auth/radius.c | 19 ++++++++++++ src/auth/radius.h | 1 + src/sec-mod-auth.c | 1 + src/sec-mod.h | 1 + tests/radius | 34 +++++++++++++++++++++ 7 files changed, 73 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 634ad438..fb1a0788 100644 --- a/NEWS +++ b/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 diff --git a/doc/requirements/internal/authentication.md b/doc/requirements/internal/authentication.md index 8f2db711..b981f4a4 100644 --- a/doc/requirements/internal/authentication.md +++ b/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 -`nas-identifier` appears as `NAS-Identifier`. +attributes are present with expected values, that a configured +`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 diff --git a/src/auth/radius.c b/src/auth/radius.c index 49fd3dbf..86745317 100644 --- a/src/auth/radius.c +++ b/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) { diff --git a/src/auth/radius.h b/src/auth/radius.h index cc4b334f..1c8dac74 100644 --- a/src/auth/radius.h +++ b/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; diff --git a/src/sec-mod-auth.c b/src/sec-mod-auth.c index d3c4e3cb..2908299a 100644 --- a/src/sec-mod-auth.c +++ b/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, diff --git a/src/sec-mod.h b/src/sec-mod.h index 1da7c66a..3e47d148 100644 --- a/src/sec-mod.h +++ b/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; diff --git a/tests/radius b/tests/radius index d0c8321a..42e97232 100755 --- a/tests/radius +++ b/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