From: jdegraeve Date: Thu, 20 Apr 2006 12:43:48 +0000 (+0000) Subject: further bugfixes in CP X-Git-Url: https://git.gsnw.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a0d850268c453f99baad64fbaeba3bf63b839184;p=m0n0chwall.git further bugfixes in CP git-svn-id: https://svn.m0n0.ch/wall/trunk@133 e36fee2c-cc09-0410-a7cc-ebac5c6737de --- diff --git a/CHANGELOG b/CHANGELOG index 85fefa8..e2ebc96 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,12 @@ $Id$ *** Note: Please add new entries to the top of this file. *** ------------------------------------------------------------------------------- +- changes in Captive portal (jdegraeve): + - Fixes a bug in the way we handle authentication mechanism. (Potentially allowing double logins and faulty locking) + +1.22 +---- + - updated base system to FreeBSD 4.11-RELEASE-p16 (mkasper) - updated Dnsmasq to 2.27 diff --git a/captiveportal/index.php b/captiveportal/index.php index ea8fe01..e331d96 100755 --- a/captiveportal/index.php +++ b/captiveportal/index.php @@ -199,7 +199,7 @@ function portal_reply_page($redirurl, $type = null, $message = null) { function portal_mac_fixed($clientmac) { global $g ; - + /* open captive portal mac db */ if (file_exists("{$g['vardb_path']}/captiveportal_mac.db")) { $fd = @fopen("{$g['vardb_path']}/captiveportal_mac.db","r") ; @@ -216,7 +216,7 @@ function portal_mac_fixed($clientmac) { fclose($fd) ; } return FALSE ; -} +} function portal_mac_radius($clientmac,$clientip) { global $config ; @@ -231,7 +231,7 @@ function portal_mac_radius($clientmac,$clientip) { return FALSE; } -function portal_allow($clientip,$clientmac,$clientuser,$password = null, $attributes = null, $ruleno = null) { +function portal_allow($clientip,$clientmac,$username,$password = null, $attributes = null, $ruleno = null) { global $redirurl, $g, $config; @@ -255,68 +255,71 @@ function portal_allow($clientip,$clientmac,$clientuser,$password = null, $attrib /* read in client database */ $cpdb = captiveportal_read_db(); - if ((isset($config['captiveportal']['noconcurrentlogins'])) && ($clientuser != 'unauthenticated')) { - /* Ensure that only one username is used by one client at a time - * by Paul Taylor - */ - if (isset($cpdb)) { - /* find duplicate entry */ - for ($i = 0; $i < count($cpdb); $i++) { - if ($cpdb[$i][4] == $user) { - /* This user was already logged in */ - $cpdb = disconnect_client($cpdb[$i][5],"CONCURRENT LOGIN - TERMINATING OLD SESSION",13,$cpdb); - } + $radiusservers = captiveportal_get_radius_servers(); + + /* Find an existing session on a different ip with the same username */ + + if ((isset($config['captiveportal']['noconcurrentlogins'])) && ($username != 'unauthenticated')) { + /* find duplicate entry */ + for ($i = 0; $i < count($cpdb); $i++) { + if (($cpdb[$i][2] != $clientip) && ($cpdb[$i][4] == $username)) { + /* This user was already logged in so we disconnect the old one */ + captiveportal_disconnect($cpdb[$i],$radiusservers, 13); + captiveportal_logportalauth($cpdb[$i][4],$cpdb[$i][3],$cpdb[$i][2],"CONCURRENT LOGIN - TERMINATING OLD SESSION"); + unset($cpdb[$i]); + break; } } } + /* Find an existing session on the same ip and reuse it */ + for ($i = 0; $i < count($cpdb); $i++) { + if($cpdb[$i][2] == $clientip) { + captiveportal_logportalauth($cpdb[$i][4],$cpdb[$i][3],$cpdb[$i][2],"CONCURRENT LOGIN - REUSING OLD SESSION"); + $sessionid = $cpdb[$i][5]; + break; + } + } - /* generate unique session ID */ - $tod = gettimeofday(); - $sessionid = substr(md5(mt_rand() . $tod['sec'] . $tod['usec'] . $clientip . $clientmac), 0, 16); + if (!isset($sessionid)) { - /* add ipfw rules for layer 3 */ - exec("/sbin/ipfw add $ruleno set 2 skipto 50000 ip from $clientip to any in"); - exec("/sbin/ipfw add $ruleno set 2 skipto 50000 ip from any to $clientip out"); + /* generate unique session ID */ + $tod = gettimeofday(); + $sessionid = substr(md5(mt_rand() . $tod['sec'] . $tod['usec'] . $clientip . $clientmac), 0, 16); - /* add ipfw rules for layer 2 */ - if (!isset($config['captiveportal']['nomacfilter'])) { - $l2ruleno = $ruleno + 10000; - exec("/sbin/ipfw add $l2ruleno set 3 deny all from $clientip to any not MAC any $clientmac layer2 in"); - exec("/sbin/ipfw add $l2ruleno set 3 deny all from any to $clientip not MAC $clientmac any layer2 out"); - } + /* add ipfw rules for layer 3 */ + exec("/sbin/ipfw add $ruleno set 2 skipto 50000 ip from $clientip to any in"); + exec("/sbin/ipfw add $ruleno set 2 skipto 50000 ip from any to $clientip out"); - $radiusservers = captiveportal_get_radius_servers(); + /* add ipfw rules for layer 2 */ + if (!isset($config['captiveportal']['nomacfilter'])) { + $l2ruleno = $ruleno + 10000; + exec("/sbin/ipfw add $l2ruleno set 3 deny all from $clientip to any not MAC any $clientmac layer2 in"); + exec("/sbin/ipfw add $l2ruleno set 3 deny all from any to $clientip not MAC $clientmac any layer2 out"); + } - /* WHY DO WE DO THIS? WE ARE ALREADY KICKING OUT USERS WITH kick_concurrent_logins - find an existing entry and delete it - for ($i = 0; $i < count($cpdb); $i++) { - if(!strcasecmp($cpdb[$i][2],$clientip)) { - if(isset($config['captiveportal']['radacct_enable']) && isset($radiusservers[0])) { - RADIUS_ACCOUNTING_STOP($cpdb[$i][1], // ruleno - $cpdb[$i][4], // username - $cpdb[$i][5], // sessionid - $cpdb[$i][0], // start time - $radiusservers[0]['ipaddr'], - $radiusservers[0]['acctport'], - $radiusservers[0]['key'], - $cpdb[$i][2], // clientip - $cpdb[$i][3], // clientmac - 13); // Port Preempted - } - mwexec("/sbin/ipfw delete " . $cpdb[$i][1] . " " . ($cpdb[$i][1]+10000)); - unset($cpdb[$i]); - break; + /* encode password in Base64 just in case it contains commas */ + $bpassword = base64_encode($password); + $cpdb[] = array(time(), $ruleno, $clientip, $clientmac, $username, $sessionid, $bpassword, + $attributes['session_timeout'], + $attributes['idle_timeout'], + $attributes['session_terminate_time']); + + if (isset($config['captiveportal']['radacct_enable']) && isset($radiusservers[0])) { + $acct_val = RADIUS_ACCOUNTING_START($ruleno, + $username, + $sessionid, + $radiusservers[0]['ipaddr'], + $radiusservers[0]['acctport'], + $radiusservers[0]['key'], + $clientip, + $clientmac); + if ($acct_val == 1) + captiveportal_logportalauth($username,$clientmac,$clientip,$type,"RADIUS ACCOUNTING FAILED"); } - } - */ - /* encode password in Base64 just in case it contains commas */ - $bpassword = base64_encode($password); - $cpdb[] = array(time(), $ruleno, $clientip, $clientmac, $clientuser, $sessionid, $bpassword, - $attributes['session_timeout'], - $attributes['idle_timeout'], - $attributes['session_terminate_time']); + + } /* rewrite information to database */ captiveportal_write_db($cpdb); @@ -381,52 +384,30 @@ EOD; /* remove a single client by session ID by Dinesh Nair */ -function disconnect_client($sessionid, $logoutReason = "LOGOUT", $term_cause = 1, $cpdb = null) { +function disconnect_client($sessionid, $logoutReason = "LOGOUT", $term_cause = 1) { global $g, $config; - /* Retrieve the user database if it isn't passed through */ - if (is_null($cpdb)) { - captiveportal_lock(); - /* read database */ - $cpdb = captiveportal_read_db(); - $cp_unlock = true; - } + captiveportal_lock(); + /* read database */ + $cpdb = captiveportal_read_db(); $radiusservers = captiveportal_get_radius_servers(); /* find entry */ for ($i = 0; $i < count($cpdb); $i++) { if ($cpdb[$i][5] == $sessionid) { - /* this client needs to be deleted - remove ipfw rules */ - if(isset($config['captiveportal']['radacct_enable']) && isset($radiusservers[0])) { - RADIUS_ACCOUNTING_STOP($cpdb[$i][1], // ruleno - $cpdb[$i][4], // username - $cpdb[$i][5], // sessionid - $cpdb[$i][0], // start time - $radiusservers[0]['ipaddr'], - $radiusservers[0]['acctport'], - $radiusservers[0]['key'], - $cpdb[$i][2], // clientip - $cpdb[$i][3], // clientmac - $term_cause); - } - mwexec("/sbin/ipfw delete " . $cpdb[$i][1] . " " . ($cpdb[$i][1]+10000)); + captiveportal_disconnect($cpdb[$i],$radiusservers, $term_cause); captiveportal_logportalauth($cpdb[$i][4],$cpdb[$i][3],$cpdb[$i][2],$logoutReason); unset($cpdb[$i]); break; } } - if ($cp_unlock) { - /* rewrite information to database */ - captiveportal_write_db($cpdb); + /* write database */ + captiveportal_write_db($cpdb); - captiveportal_unlock(); - } - else { - return $cpdb; - } + captiveportal_unlock(); } diff --git a/phpconf/inc/captiveportal.inc b/phpconf/inc/captiveportal.inc index bb83a23..7c62db9 100644 --- a/phpconf/inc/captiveportal.inc +++ b/phpconf/inc/captiveportal.inc @@ -341,7 +341,7 @@ add 1305 set 1 pass tcp from $cpip 8001 to any out EOD; } - + $cprules .= <<= $timeout) { @@ -413,7 +413,7 @@ function captiveportal_prune_old() { $term_cause = 5; // Session-Timeout } } - + /* check if the radius idle_timeout attribute has been set and if its set change the idletimeout to this value */ $idletimeout = (is_numeric($cpdb[$i][8])) ? $cpdb[$i][8] : $idletimeout; /* if an idle timeout is specified, get last activity timestamp from ipfw */ @@ -433,17 +433,17 @@ function captiveportal_prune_old() { $term_cause = 5; // Session-Timeout } } - + if ($timedout) { captiveportal_disconnect($cpdb[$i], $radiusservers,$term_cause,$stop_time); captiveportal_logportalauth($cpdb[$i][4], $cpdb[$i][3], $cpdb[$i][2], "TIMEOUT"); unset($cpdb[$i]); } - + /* do periodic RADIUS reauthentication? */ if (!$timedout && isset($config['captiveportal']['reauthenticate']) && ($radiusservers !== false)) { - + if (isset($config['captiveportal']['radacct_enable'])) { if ($config['captiveportal']['reauthenticateacct'] == "stopstart") { /* stop and restart accounting */ @@ -480,7 +480,7 @@ function captiveportal_prune_old() { true); // Interim Updates } } - + /* check this user against RADIUS again */ $auth_list = RADIUS_AUTHENTICATION($cpdb[$i][4], // username base64_decode($cpdb[$i][6]), // password @@ -488,7 +488,7 @@ function captiveportal_prune_old() { $cpdb[$i][2], // clientip $cpdb[$i][3], // clientmac $cpdb[$i][1]); // ruleno - + if ($auth_list['auth_val'] == 3) { captiveportal_disconnect($cpdb[$i], $radiusservers, 17); captiveportal_logportalauth($cpdb[$i][4], $cpdb[$i][3], $cpdb[$i][2], "RADIUS_DISCONNECT", $auth_list['reply_message']); @@ -496,20 +496,20 @@ function captiveportal_prune_old() { } } } - + /* write database */ captiveportal_write_db($cpdb); - + captiveportal_unlock(); } /* remove a single client according to the DB entry */ function captiveportal_disconnect($dbent, $radiusservers,$term_cause = 1,$stop_time = null) { - + global $g, $config; $stop_time = (empty($stop_time)) ? time() : $stop_time; - + /* this client needs to be deleted - remove ipfw rules */ if (isset($config['captiveportal']['radacct_enable']) && isset($radiusservers[0])) { RADIUS_ACCOUNTING_STOP($dbent[1], // ruleno @@ -525,9 +525,9 @@ function captiveportal_disconnect($dbent, $radiusservers,$term_cause = 1,$stop_t false, $stop_time); } - + mwexec("/sbin/ipfw delete " . $dbent[1] . " " . ($dbent[1]+10000)); - + //KEYCOM: we need to delete +40500 and +45500 as well... //these are the rule numbers we use to control traffic shaping for each logged in user via captive portal //we only need to remove our rules if peruserbw is turned on. @@ -539,16 +539,16 @@ function captiveportal_disconnect($dbent, $radiusservers,$term_cause = 1,$stop_t /* remove a single client by ipfw rule number */ function captiveportal_disconnect_client($id,$term_cause = 1) { - + global $g, $config; - + captiveportal_lock(); - + /* read database */ $cpdb = captiveportal_read_db(); $radiusservers = captiveportal_get_radius_servers(); - - /* find entry */ + + /* find entry */ for ($i = 0; $i < count($cpdb); $i++) { if ($cpdb[$i][1] == $id) { captiveportal_disconnect($cpdb[$i], $radiusservers, $term_cause); @@ -557,25 +557,25 @@ function captiveportal_disconnect_client($id,$term_cause = 1) { break; } } - + /* write database */ captiveportal_write_db($cpdb); - + captiveportal_unlock(); } /* send RADIUS acct stop for all current clients */ function captiveportal_radius_stop_all() { global $g, $config; - + if (!isset($config['captiveportal']['radacct_enable'])) return; captiveportal_lock(); $cpdb = captiveportal_read_db(); - + $radiusservers = captiveportal_get_radius_servers(); - + if (isset($radiusservers[0])) { for ($i = 0; $i < count($cpdb); $i++) { RADIUS_ACCOUNTING_STOP($cpdb[$i][1], // ruleno @@ -595,31 +595,31 @@ function captiveportal_radius_stop_all() { function captiveportal_passthrumac_configure() { global $config, $g; - + captiveportal_lock(); - + /* clear out passthru macs, if necessary */ unlink_if_exists("{$g['vardb_path']}/captiveportal_mac.db"); - + if (is_array($config['captiveportal']['passthrumac'])) { - + $fd = @fopen("{$g['vardb_path']}/captiveportal_mac.db", "w"); if (!$fd) { printf("Error: cannot open passthru mac DB file in captiveportal_passthrumac_configure().\n"); captiveportal_unlock(); - return 1; + return 1; } - + foreach ($config['captiveportal']['passthrumac'] as $macent) { /* record passthru mac so it can be recognized and let thru */ fwrite($fd, $macent['mac'] . "\n"); } - + fclose($fd); } - + captiveportal_unlock(); - + return 0; } @@ -790,7 +790,6 @@ function radius($username,$password,$clientip,$clientmac,$type) { } $radiusservers = captiveportal_get_radius_servers(); - $radacct_enable = isset($config['captiveportal']['radacct_enable']); $auth_list = RADIUS_AUTHENTICATION($username, $password, @@ -807,19 +806,6 @@ function radius($username,$password,$clientip,$clientmac,$type) { $password, $auth_list, $ruleno); - - if ($radacct_enable) { - $auth_list['acct_val'] = RADIUS_ACCOUNTING_START($ruleno, - $username, - $sessionid, - $radiusservers[0]['ipaddr'], - $radiusservers[0]['acctport'], - $radiusservers[0]['key'], - $clientip, - $clientmac); - if ($auth_list['acct_val'] == 1) - captiveportal_logportalauth($username,$clientmac,$clientip,$type,"RADIUS ACCOUNTING FAILED"); - } } else { captiveportal_unlock();