pfSense

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 = $cpentry¹⁰;
} else {
/* --- Use the actual interval since the last accounting interval update to avoid cumulating time exponentially.
$rastart_time = $cpentry⁰;
$rastop_time = time();
*/
$rastart_time = 0;
$rastop_time = $cpentry¹⁰;
}

/*-----------------------------------------------------------------------------------------------------------*/
/*--- 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',
$cpentry¹, // ruleno
$cpentry⁴, // username
$cpentry², // clientip
$cpentry³, // clientmac
$cpentry⁵, // sessionid
$rastart_time, // start time
$rastop_time, // Stop Time
10); // NAS Request
/* XXX rewrite to C wrapper pfSense_pf_anchor_zerocnt() /
captiveportal_anchor_zerocnt($cpentry², '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 - $cpentry⁰;
if (!empty($cpentry¹⁰) && $cpentry¹⁰ > 60) {
$interval = $cpentry¹⁰;
} 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',
$cpentry¹, // ruleno
$cpentry⁴, // username
$cpentry², // clientip
$cpentry³, // clientmac
$cpentry⁵, // sessionid
$cpentry⁰); // 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 */