From f3d9e10b0a67adec41c4806d9c4fdfdec220bcb8 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Mon, 29 Dec 2014 17:47:25 +0000 Subject: [PATCH] fixes for captive portal voucher, locking issues etc. --- src/etc/inc/captiveportal.inc | 20 +++++++---- src/etc/inc/voucher.inc | 34 ------------------- .../OPNsense/CaptivePortal/CPClient.php | 2 +- 3 files changed, 15 insertions(+), 41 deletions(-) diff --git a/src/etc/inc/captiveportal.inc b/src/etc/inc/captiveportal.inc index 0aab6b296..58dd3cb43 100644 --- a/src/etc/inc/captiveportal.inc +++ b/src/etc/inc/captiveportal.inc @@ -119,7 +119,6 @@ function portal_allow($clientip,$clientmac,$username,$password = null, $attribut $dwfaultbw_down = isset($config['captiveportal'][$cpzone]['bwdefaultdn']) ? $config['captiveportal'][$cpzone]['bwdefaultdn'] : 0; $bw_up = isset($attributes['bw_up']) ? round(intval($attributes['bw_up'])/1000, 2) : $dwfaultbw_up; $bw_down = isset($attributes['bw_down']) ? round(intval($attributes['bw_down'])/1000, 2) : $dwfaultbw_down; - $session_terminate_time = (!empty($attributes['session_terminate_time'])) ? $attributes['session_terminate_time'] : 'NULL'; $interim_interval = (!empty($attributes['interim_interval'])) ? $attributes['interim_interval'] : 'NULL'; $session_timeout = 0 ; @@ -127,7 +126,8 @@ function portal_allow($clientip,$clientmac,$username,$password = null, $attribut $session_timeout = $attributes['session_timeout'] ; } elseif ( is_numeric($config['captiveportal'][$cpzone]["timeout"]) ){ - $session_timeout = $config['captiveportal'][$cpzone]["timeout"]; + // calculate to seconds for timeout parameters ( config in minutes ) + $session_timeout = $config['captiveportal'][$cpzone]["timeout"] * 60 ; } $idle_timeout = 0 ; @@ -135,10 +135,18 @@ function portal_allow($clientip,$clientmac,$username,$password = null, $attribut $idle_timeout = $attributes['idle_timeout'] ; } elseif ( is_numeric($config['captiveportal'][$cpzone]["idletimeout"]) ){ - $idle_timeout = $config['captiveportal'][$cpzone]["idletimeout"]; + // calculate to seconds for timeout parameters ( config in minutes ) + $idle_timeout = $config['captiveportal'][$cpzone]["idletimeout"] * 60 ; } + $session_terminate_time = 0; + if ( array_key_exists("session_timeout",$attributes ) ) { + $session_terminate_time = $attributes['session_terminate_time'] ; + } + + + if ($attributes['voucher']) { $db = new OPNsense\CaptivePortal\DB($cpzone); $clients = $db->listClients(array("username"=>$username), null, null); @@ -146,11 +154,11 @@ function portal_allow($clientip,$clientmac,$username,$password = null, $attribut // user is already connected, disconnect old session $cpc->disconnect($cpzone, $client->sessionid); - // calculate new session end time for this voucher - $session_terminate_time = $client->allow_time + $client->session_timeout - time() ; + // calculate new session end time for this voucher ( session connection time + timeout - now, correct with 1 second to trap exact cleanup hit) + $session_terminate_time = $client->allow_time + $client->session_timeout - time() - 1; } - if ($session_terminate_time <= 0) { + if ($session_terminate_time < 0) { // no time left for voucher return 0; } diff --git a/src/etc/inc/voucher.inc b/src/etc/inc/voucher.inc index f60a7ba32..3c501c4df 100644 --- a/src/etc/inc/voucher.inc +++ b/src/etc/inc/voucher.inc @@ -221,17 +221,6 @@ function voucher_expire($voucher_received) { $cpdb = new OPNsense\CaptivePortal\DB($cpzone); $cpc = new OPNsense\CaptivePortal\CPClient(); - // XMLRPC Call over to the master Voucher node - if(!empty($config['voucher'][$cpzone]['vouchersyncdbip'])) { - $syncip = $config['voucher'][$cpzone]['vouchersyncdbip']; - $syncport = $config['voucher'][$cpzone]['vouchersyncport']; - $syncpass = $config['voucher'][$cpzone]['vouchersyncpass']; - $vouchersyncusername = $config['voucher'][$cpzone]['vouchersyncusername']; - xmlrpc_sync_voucher_expire($voucher_received, $syncip, $syncport, $syncpass, $vouchersyncusername); - } - - $voucherlck = lock("voucher{$cpzone}", LOCK_EX); - // read rolls into assoc array with rollid as key and minutes as value $tickets_per_roll = array(); $minutes_per_roll = array(); @@ -316,7 +305,6 @@ function voucher_expire($voucher_received) { unset($cpdb); unset($cpc); - unlock($voucherlck); return true; } @@ -334,17 +322,6 @@ function voucher_auth($voucher_received, $test = 0) { if (!isset($config['voucher'][$cpzone]['enable'])) return 0; - // XMLRPC Call over to the master Voucher node - if(!empty($config['voucher'][$cpzone]['vouchersyncdbip'])) { - $syncip = $config['voucher'][$cpzone]['vouchersyncdbip']; - $syncport = $config['voucher'][$cpzone]['vouchersyncport']; - $syncpass = $config['voucher'][$cpzone]['vouchersyncpass']; - $vouchersyncusername = $config['voucher'][$cpzone]['vouchersyncusername']; - $remote_time_used = xmlrpc_sync_used_voucher($voucher_received, $syncip, $syncport, $syncpass, $vouchersyncusername); - } - - $voucherlck = lock("voucher{$cpzone}", LOCK_EX); - // read rolls into assoc array with rollid as key and minutes as value $tickets_per_roll = array(); $minutes_per_roll = array(); @@ -429,7 +406,6 @@ function voucher_auth($voucher_received, $test = 0) { } else { $test_result[] = sprintf(gettext("Access granted for %d Minutes in total."),$total_minutes); } - unlock($voucherlck); return $test_result; } @@ -438,20 +414,11 @@ function voucher_auth($voucher_received, $test = 0) { // Discussion: we could return the time remaining for good vouchers, but then // the user wouldn't know that he used at least one invalid voucher. if ($error) { - unlock($voucherlck); if ($total_minutes > 0) // probably not needed, but want to make sure $total_minutes = 0; // we only report -1 (expired) or 0 (no access) return $total_minutes; // well, at least one voucher had errors. Say NO ACCESS } - // If we did a XMLRPC sync earlier check the timeleft - if (!empty($config['voucher'][$cpzone]['vouchersyncdbip'])) { - if (!is_null($remote_time_used)) - $total_minutes = $remote_time_used; - else if ($remote_time_used < $total_minutes) - $total_minutes -= $remote_time_used; - } - // All given vouchers were valid and this isn't simply a test. // Write back the used DB's if (is_array($bitstring)) { @@ -482,7 +449,6 @@ function voucher_auth($voucher_received, $test = 0) { /* Triger a sync of the vouchers on config */ send_event("service sync vouchers"); - unlock($voucherlck); return $total_minutes; } diff --git a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php index 530aef62e..7e40e0d97 100644 --- a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php +++ b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php @@ -651,7 +651,7 @@ class CPClient { // if session timeout is reached, disconnect if (is_numeric($client->session_timeout) && $client->session_timeout > 0 ) { - if (((time() - $client->allow_time) / 60) > $client->session_timeout) { + if (((time() - $client->allow_time) ) > $client->session_timeout) { $this->disconnect($cpzonename, $client->sessionid); $this->logportalauth($cpzonename, $client->username, $client->mac, $client->ip, $status="SESSION TIMEOUT"); continue;