Bug #14118
open
freeRadius "Amount of Time" setting is not accurately tracked for Stop/Start settings in Caaptive Portal
0%
Description
Re: tested on 23.01 plus mid Feb release: Correct time accounting error in captiveportal.inc Stop/Start routines for freeRadius. The Stop/Start freeRadius routine at lines 690 thru 693 forces the interval to 60 seconds. freeRadius is expecting a duration interval since the last accounting update and as a result, 60 seconds is subtracted from the “allowed time” setting in the freeRadius GIU in pfSense, which is one of the reasons Stop/Start freeRadius works for tracking “Amount of Time” and Stop/Start doesn’t. The Stop/Start routine at lines 693 thru 696 sends an increment from the start of the session to the current time resulting in the cumulation of time at an exponential rate and premature logout of that freeRadius user. Unfortunately once a minute accounting intervals do not work well with freeRadius and accounting data is dropped with the current code, masking this issue. The duration must be longer (I found that less than 600 seconds was iffy and anything below 120 seconds definitely doesn’t keep accurate accounting for interim, stop/start or stop/start freeradius) and that is particularly true as the system gets loaded down with more users. In order to support more users, I have found we simply have to extend the duration of the “accounting interval”. As freeRadius already has a user settable accounting interval for interim accounting. Lines 718 thru 738 but only uses that interval for the interim setting. For simplicity, I propose using it for both Stop/Start routines as well.
As the “reauthenticate every minute” setting in the CaptivePortal GUI will be redundant if the duration is longer than the accounting interval, it makes sense to also incorporate the freeRadius “accounting interval” for that as well.
It should be noted that the freeRadius GUI states that the default value for the accounting interval is 600 seconds but it is not, it is much shorter, more like a minute. This should be corrected while implementing this fix.
I have also reduced the “pause” duration at line 710 in stopstartfreeradius to support scaling to larger number of connected users. The value of 250000 microseconds or 1/4 of a second is arbitrary but working well during testing.
I took the code wrapping the interim interval and “copy/paste” wrapped the Stop/Start and Reauthenticate routines to demonstrate and test this proposed fix. It has worked well during my lab testing. No effort has been made to make this code efficient, it is included here for proof of principle and/or for testing. The fact the interim value applies to all freeRadius accounting, not just interim should be updated in the freeRadius GUI under Settings, freeRadius.
Line 684 in captiveportal.inc with modifications encapsulated inside “/*--"- commented sections follows:
684 /* do periodic reauthentication? For Radius servers, send accounting updates? /
if (!$timedout) {
//Radius servers : send accounting
if (isset($cpcfg['radacct_enable']) && $cpentry['authmethod'] === 'radius') {
if (substr($cpcfg['reauthenticateacct'], 0, 9) === "stopstart") {
/ stop and restart accounting /
if ($cpcfg['reauthenticateacct'] === "stopstartfreeradius") {
/--- Use the actual interval since the last accounting interval update
$rastart_time = 0;
$rastop_time = 60;
*/
$rastart_time = 0;
$rastop_time = $cpentry10;
} else {
/* --- Use the actual interval since the last accounting interval update to avoid cumulating time exponentially.
$rastart_time = $cpentry0;
$rastop_time = time();
*/
$rastart_time = 0;
$rastop_time = $cpentry10;
}
/*-----------------------------------------------------------------------------------------------------------*/
/*--- Override to use interim update from freeRadius GUI setting for stop/start frequency as well */
$session_time = $pruning_time - $cpentry[0];
if (!empty($cpentry[10]) && $cpentry[10] > 60) {
$interval = $cpentry[10];
} else {
$interval = 0;
}
$past_interval_min = ($session_time > $interval);
if ($interval != 0) {
$within_interval = ($session_time % $interval >= 0 && $session_time % $interval <= 59);
}
if ($interval === 0 || ($interval > 0 && $past_interval_min && $within_interval)) {/*-----------------------------------------------------------------------------------------------------------*/
captiveportal_send_server_accounting('stop',
$cpentry1, // ruleno
$cpentry4, // username
$cpentry2, // clientip
$cpentry3, // clientmac
$cpentry5, // sessionid
$rastart_time, // start time
$rastop_time, // Stop Time
10); // NAS Request
/* XXX rewrite to C wrapper pfSense_pf_anchor_zerocnt() /
captiveportal_anchor_zerocnt($cpentry2, 'auth');
if ($cpcfg['reauthenticateacct'] "stopstartfreeradius") {
/ Need to pause here or the FreeRADIUS server gets confused about packet ordering. /
/ --- 1 sec limits max # simultaneous users sleep(1); /
usleep(250000);
}
captiveportal_send_server_accounting('start',
$cpentry[1], // ruleno
$cpentry[4], // username
$cpentry[2], // clientip
$cpentry[3], // clientmac
$cpentry[5]); // sessionid
/-----------------------------------------------------------------------------------------------------------*/
}/*-----------------------------------------------------------------------------------------------------------*/
} else if ($cpcfg['reauthenticateacct'] "interimupdate") {
$session_time = $pruning_time - $cpentry0;
if (!empty($cpentry10) && $cpentry10 > 60) {
$interval = $cpentry10;
} else {
$interval = 0;
}
$past_interval_min = ($session_time > $interval);
if ($interval != 0) {
$within_interval = ($session_time % $interval >= 0 && $session_time % $interval <= 59);
}
if ($interval === 0 || ($interval > 0 && $past_interval_min && $within_interval)) {
captiveportal_send_server_accounting('update',
$cpentry1, // ruleno
$cpentry4, // username
$cpentry2, // clientip
$cpentry3, // clientmac
$cpentry5, // sessionid
$cpentry0); // start time
}
}
}
/* check this user again */
if (isset($cpcfg['reauthenticate']) && $cpentry['context'] !== 'voucher') {/*-----------------------------------------------------------------------------------------------------------*/
/*--- Override to use interim update from freeRadius GUI setting as reauthenticate frequency as well */
$session_time = $pruning_time - $cpentry[0];
if (!empty($cpentry[10]) && $cpentry[10] > 60) {
$interval = $cpentry[10];
} else {
$interval = 0;
}
$past_interval_min = ($session_time > $interval);
if ($interval != 0) {
$within_interval = ($session_time % $interval >= 0 && $session_time % $interval <= 59);
}
if ($interval === 0 || ($interval > 0 && $past_interval_min && $within_interval)) {/*-----------------------------------------------------------------------------------------------------------*/
$auth_result = captiveportal_authenticate_user(
$cpentry[4], // username
base64_decode($cpentry[6]), // password
$cpentry[3], // clientmac
$cpentry[2], // clientip
$cpentry[1], // ruleno
$cpentry['context']); // context
if ($auth_result['result'] === false) {
captiveportal_disconnect($cpentry, 17);
captiveportal_logportalauth($cpentry[4], $cpentry[3], $cpentry[2], "DISCONNECT - REAUTHENTICATION FAILED", $auth_list['reply_message']);
$unsetindexes[] = $cpentry[5];
} else if ($auth_result['result'] === true) {
if ($cpentry['authmethod'] !== $auth_result['auth_method']) {
// if the user got authenticated against another server type: we update the database
if (!empty($cpentry[5])) {
captiveportal_update_entry($cpentry['sessionid'], $auth_result['auth_method'], 'authmethod');
captiveportal_logportalauth($cpentry[4], $cpentry[3], $cpentry[2], "CHANGED AUTHENTICATION SERVER", $auth_list['reply_message']);
}
// User was logged on a RADIUS server, but is now logged in by another server type : we send an accounting Stop
if (isset($config['captiveportal'][$cpzone]['radacct_enable']) && $cpentry['authmethod'] 'radius') {
if ($cpcfg['reauthenticateacct'] = "stopstartfreeradius") {
$rastart_time = 0;
$rastop_time = 60;
} else {
$rastart_time = $cpentry[0];
$rastop_time = time();
}
captiveportal_send_server_accounting('stop',
$cpentry[1], // ruleno
$cpentry[4], // username
$cpentry[2], // clientip
$cpentry[3], // clientmac
$cpentry[5], // sessionid
$rastart_time, // start time
$rastop_time, // Stop Time
3); // Lost Service
// User was logged on a non-RADIUS Server but is now logged in by a RADIUS server : we send an accounting Start
} else if(isset($config['captiveportal'][$cpzone]['radacct_enable']) && $auth_result['auth_method'] === 'radius') {
captiveportal_send_server_accounting('start',
$cpentry[1], // ruleno
$cpentry[4], // username
$cpentry[2], // clientip
$cpentry[3], // clientmac
$cpentry[5], // sessionid
$cpentry[0]); // start_time
}
}
captiveportal_reapply_attributes($cpentry, $auth_result['attributes']);
}
/*-----------------------------------------------------------------------------------------------------------*/}/*-----------------------------------------------------------------------------------------------------------*/
}
}
Redmines #13843 & #13844 must be fully implemented before this modification can be utilized on accounts with a data quota as overflowed value logouts >4GB will occur if a data quota is set (eg: 100GB = 1.7GB overflow equivalent). In order to complete this testing, I overrode the 32 bit overflow 4 GB data quota limit as follows (line 663 in captiveportal.inc). #13844 requires checking reauthenticate option in the captive portal GUI to force a logout for now. I include the code here to permit these fixes to progress in parallel or for those that need an immediate fix for 23.01.
Line 662 in captiveportal.inc:
/* traffic quota, value retrieved from the radius attribute if the option is enabled /
if (isset($cpcfg['radiustraffic_quota'])) {
$utrafficquota = (is_numeric($cpentry[11])) ? $cpentry[11] : $trafficquota;
/-----------------------------------------------------------------------------------------------------------*/
/* new code /
$intoverflow = true; //to stop 32 bit overflow premature logout
/ new code /
/-----------------------------------------------------------------------------------------------------------*/
} else {
$utrafficquota = $trafficquota;
}if (!$timedout && $utrafficquota > 0) {
$volume = getVolume($cpentry[2]);
if (($volume['input_bytes'] + $volume['output_bytes']) > $utrafficquota) {/* edited code original $timedout = true; /
/-----------------------------------------------------------------------------------------------------------*/
if ($intoverflow != true) {
$timedout = true;
} else {
$timedout = false; //to stop 32 bit overflow premature logout
}
/*-----------------------------------------------------------------------------------------------------------*/
/* edited code */
Files
Updated by Dale Harron over 1 year ago
re: 23.01 mid Feb release: Although the $rastart_time and $rastop_time are set around line 684 for the call to line 2386 captiveportal_send_server_accounting() function, they are ignored and changed to new values depending upon the "$type" variable.
For multiple connections to a single user account, allowed time is correctly tracked by freeRadius and shared for that user account. Allowed time is not tracked for a single connection to a user account. Although the captive portal also tracks allowed time and disconnects the user at the correct consumption before calling the line 2386 function, it thus does not update the freeRadius server and the user can log back in with only the last accounting interval subtracted. It should be noted that the captive portal applies the allowed time to each connected user separately which means multiple connections to a single user account consume their share of allowed time which means the captive portal does not disconnect the users at the shared total use but correctly trigger the freeRadius allowed time limit error well before captive portal does. eg: 10 minutes allowed time, two connected users, freeRadius logs out connected users at 5 minutes whereas captive portal would log them out individually at 10 minutes if freeRadius had not initiated it already and started rejecting further login attempts (see reauthenticate every minute). Captive portal does not appear to support multiple connections to a single user sharing allowed time but freeRadius accounting does for multiple connections only.