From f4f6b83b38ebd0d46941d0d91af010bc7bd443bc Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Mon, 29 Dec 2014 08:55:33 +0000 Subject: [PATCH] captive portal fixes --- src/etc/inc/captiveportal.inc | 55 ++++++++++--------- src/etc/inc/voucher.inc | 32 +++++------ .../app/models/OPNsense/CaptivePortal/ARP.php | 2 +- .../OPNsense/CaptivePortal/CPClient.php | 40 ++++++++++---- .../app/models/OPNsense/CaptivePortal/DB.php | 2 +- .../models/OPNsense/CaptivePortal/Rules.php | 7 ++- .../mvc/app/models/OPNsense/Core/Shell.php | 2 +- src/www/status_captiveportal.php | 2 +- 8 files changed, 83 insertions(+), 59 deletions(-) diff --git a/src/etc/inc/captiveportal.inc b/src/etc/inc/captiveportal.inc index 620774a4a..38774ef3e 100644 --- a/src/etc/inc/captiveportal.inc +++ b/src/etc/inc/captiveportal.inc @@ -35,10 +35,10 @@ via returned RADIUS attributes. This page has been modified to delete any added rules which may have been created by other per-user code (index.php, etc). These changes are (c) 2004 Keycom PLC. - + pfSense_BUILDER_BINARIES: /sbin/ipfw /sbin/route pfSense_BUILDER_BINARIES: /usr/local/sbin/lighttpd /usr/local/bin/minicron /sbin/pfctl - pfSense_BUILDER_BINARIES: /bin/hostname /bin/cp + pfSense_BUILDER_BINARIES: /bin/hostname /bin/cp pfSense_MODULE: captiveportal */ @@ -104,8 +104,9 @@ function portal_allow($clientip,$clientmac,$username,$password = null, $attribut global $cpzone ,$type,$g; // Ensure we create an array if we are missing attributes - if (!is_array($attributes)) - $attributes = array(); + if (!is_array($attributes)) { + $attributes = array(); + } // handle $dwfaultbw_up = isset($config['captiveportal'][$cpzone]['bwdefaultup']) ? $config['captiveportal'][$cpzone]['bwdefaultup'] : 0; @@ -244,9 +245,9 @@ function get_default_captive_portal_html() { global $config, $g, $cpzone; $htmltext = << - -
+ + +
@@ -327,7 +328,7 @@ EOD;
- + EOD; @@ -377,7 +378,7 @@ function captiveportal_configure_zone($cpcfg) { global $config, $g, $cpzone, $cpzoneid; $captiveportallck = lock("captiveportal{$cpzone}", LOCK_EX); - + if (isset($cpcfg['enable'])) { if ($g['booting']) { @@ -571,7 +572,7 @@ EOD; } unlock($captiveportallck); - + return 0; } @@ -588,7 +589,7 @@ function captiveportal_init_webgui() { function captiveportal_init_webgui_zonename($zone) { global $config, $cpzone; - + if (isset($config['captiveportal'][$zone])) { $cpzone = $zone; captiveportal_init_webgui_zone($config['captiveportal'][$zone]); @@ -638,7 +639,7 @@ function captiveportal_init_webgui_zone($cpcfg) { } -/* +/* * Remove clients that have been around for longer than the specified amount of time * db file structure: * timestamp,ipfw_rule_no,clientip,clientmac,username,sessionid,password,session_timeout,idle_timeout,session_terminate_time,interim_interval @@ -660,7 +661,7 @@ function captiveportal_prune_old() { }else{ // cleanup sessions if radius accounting is enable // TODO: this code needs a rewrite, probably the easiest thing todo is update the zone administration and run - // the normal cleanup (portal_cleanup_sessions) to detach both processes + // the normal cleanup (portal_cleanup_sessions) to detach both processes // $vcpcfg = $config['voucher'][$cpzone]; @@ -931,7 +932,7 @@ function captiveportal_init_radius_servers() { fwrite($fd,"\n" . $radiusip3 . "," . $radiusport3 . "," . $radiusacctport . "," . $radiuskey3 . ",second"); if (isset($radiusip4)) fwrite($fd,"\n" . $radiusip4 . "," . $radiusport4 . "," . $radiusacctport . "," . $radiuskey4 . ",second"); - + fclose($fd); unlock($cprdsrvlck); @@ -945,7 +946,7 @@ function captiveportal_get_radius_servers() { $cprdsrvlck = lock("captiveportalradius{$cpzone}"); if (file_exists("{$g['vardb_path']}/captiveportal_radius_{$cpzone}.db")) { $radiusservers = array(); - $cpradiusdb = file("{$g['vardb_path']}/captiveportal_radius_{$cpzone}.db", + $cpradiusdb = file("{$g['vardb_path']}/captiveportal_radius_{$cpzone}.db", FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); if ($cpradiusdb) { foreach($cpradiusdb as $cpradiusentry) { @@ -1042,7 +1043,7 @@ function radius($username,$password,$clientip,$clientmac,$type, $radiusctx = nul function captiveportal_write_elements() { global $g, $config, $cpzone; - + $cpcfg = $config['captiveportal'][$cpzone]; if (!is_dir($g['captiveportal_element_path'])) @@ -1060,7 +1061,7 @@ function captiveportal_write_elements() { } conf_mount_ro(); } - + return 0; } @@ -1182,8 +1183,8 @@ function captiveportal_get_next_ipfw_ruleno($rulenos_start = 2, $rulenos_range_m $rules[$ridx] = $cpzone; break; } else { - /* - * This allows our traffic shaping pipes to be the in pipe the same as ruleno + /* + * This allows our traffic shaping pipes to be the in pipe the same as ruleno * and the out pipe ruleno + 1. */ $ridx += 2; @@ -1310,7 +1311,7 @@ function getNasIP() else $nasIp = get_interface_ip($config['captiveportal'][$cpzone]['radiussrcip_attribute']); } - + if(!is_ipaddr($nasIp)) $nasIp = "0.0.0.0"; @@ -1344,9 +1345,9 @@ function portal_ip_from_client_ip($cliip) { } // doesn't match up to any particular interface - // so let's set the portal IP to what PHP says - // the server IP issuing the request is. - // allows same behavior as 1.2.x where IP isn't + // so let's set the portal IP to what PHP says + // the server IP issuing the request is. + // allows same behavior as 1.2.x where IP isn't // in the subnet of any CP interface (static routes, etc.) // rather than forcing to DNS hostname resolution $ip = $_SERVER['SERVER_ADDR']; @@ -1364,7 +1365,7 @@ function portal_hostname_from_client_ip($cliip) { if (isset($cpcfg['httpslogin'])) { $listenporthttps = $cpcfg['listenporthttps'] ? $cpcfg['listenporthttps'] : ($cpcfg['zoneid'] + 8001); $ourhostname = $cpcfg['httpsname']; - + if ($listenporthttps != 443) $ourhostname .= ":" . $listenporthttps; } else { @@ -1374,11 +1375,11 @@ function portal_hostname_from_client_ip($cliip) { $ourhostname = "{$config['system']['hostname']}.{$config['system']['domain']}"; else $ourhostname = (is_ipaddrv6($ifip)) ? "[{$ifip}]" : "{$ifip}"; - + if ($listenporthttp != 80) $ourhostname .= ":" . $listenporthttp; } - + return $ourhostname; } @@ -1416,7 +1417,7 @@ function portal_reply_page($redirurl, $type = null, $message = null, $clientmac $htmltext = str_replace("\$CLIENT_MAC\$", htmlspecialchars($clientmac), $htmltext); $htmltext = str_replace("\$CLIENT_IP\$", htmlspecialchars($clientip), $htmltext); - // Special handling case for captive portal master page so that it can be ran + // Special handling case for captive portal master page so that it can be ran // through the PHP interpreter using the include method above. We convert the // $VARIABLE$ case to #VARIABLE# in /usr/local/etc/inc/captiveportal.inc before writing out. $htmltext = str_replace("#PORTAL_ZONE#", htmlspecialchars($cpzone), $htmltext); diff --git a/src/etc/inc/voucher.inc b/src/etc/inc/voucher.inc index f60a7ba32..af629694d 100644 --- a/src/etc/inc/voucher.inc +++ b/src/etc/inc/voucher.inc @@ -4,17 +4,17 @@ Copyright (C) 2010 Scott Ullrich Copyright (C) 2007 Marcel Wiget All rights reserved. - + Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: - + 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. - + 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. - + THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE @@ -243,7 +243,7 @@ function voucher_expire($voucher_received) { } // split into an array. Useful for multiple vouchers given - $a_vouchers_received = preg_split("/[\t\n\r ]+/s", $voucher_received); + $a_vouchers_received = preg_split("/[\t\n\r ]+/s", $voucher_received); $active_dirty = false; // go through all received vouchers, check their valid and extract @@ -257,9 +257,9 @@ function voucher_expire($voucher_received) { $_gb = exec("/usr/local/bin/voucher -c {$g['varetc_path']}/voucher_{$cpzone}.cfg -k {$g['varetc_path']}/voucher_{$cpzone}.public -- $v", $output); list($status, $roll, $nr) = explode(" ", $output[0]); if ($status == "OK") { - // check if we have this ticket on a registered roll for this ticket + // check if we have this ticket on a registered roll for this ticket if ($tickets_per_roll[$roll] && ($nr <= $tickets_per_roll[$roll])) { - // voucher is from a registered roll. + // voucher is from a registered roll. if (!isset($active_vouchers[$roll])) $active_vouchers[$roll] = voucher_read_active_db($roll); // valid voucher. Store roll# and ticket# @@ -321,7 +321,7 @@ function voucher_expire($voucher_received) { return true; } -/* +/* * Authenticate a voucher and return the remaining time credit in minutes * if $test is set, don't mark the voucher as used nor add it to the list * of active vouchers @@ -356,7 +356,7 @@ function voucher_auth($voucher_received, $test = 0) { } // split into an array. Useful for multiple vouchers given - $a_vouchers_received = preg_split("/[\t\n\r ]+/s", $voucher_received); + $a_vouchers_received = preg_split("/[\t\n\r ]+/s", $voucher_received); $error = 0; $test_result = array(); // used to display for voucher test option in GUI $total_minutes = 0; @@ -378,9 +378,9 @@ function voucher_auth($voucher_received, $test = 0) { $first_voucher = $voucher; $first_voucher_roll = $roll; } - // check if we have this ticket on a registered roll for this ticket + // check if we have this ticket on a registered roll for this ticket if ($tickets_per_roll[$roll] && ($nr <= $tickets_per_roll[$roll])) { - // voucher is from a registered roll. + // voucher is from a registered roll. if (!isset($active_vouchers[$roll])) $active_vouchers[$roll] = voucher_read_active_db($roll); // valid voucher. Store roll# and ticket# @@ -448,7 +448,7 @@ function voucher_auth($voucher_received, $test = 0) { if (!empty($config['voucher'][$cpzone]['vouchersyncdbip'])) { if (!is_null($remote_time_used)) $total_minutes = $remote_time_used; - else if ($remote_time_used < $total_minutes) + else if ($remote_time_used < $total_minutes) $total_minutes -= $remote_time_used; } @@ -572,7 +572,7 @@ function voucher_configure_zone($sync = false) { return 0; } -/* write bitstring of used vouchers to ramdisk. +/* write bitstring of used vouchers to ramdisk. * Bitstring must already be base64_encoded! */ function voucher_write_used_db($roll, $vdb) { @@ -587,7 +587,7 @@ function voucher_write_used_db($roll, $vdb) { } /* return assoc array of active vouchers with activation timestamp - * voucher is index. + * voucher is index. */ function voucher_read_active_db($roll) { global $g, $cpzone; @@ -642,7 +642,7 @@ function voucher_used_count($roll) { $max = strlen($bitstring) * 8; $used = 0; for ($i = 1; $i <= $max; $i++) { - // check if ticket already used or not. + // check if ticket already used or not. $pos = $i >> 3; // divide by 8 -> octet $mask = 1 << ($i % 8); // mask to test bit in octet if (ord($bitstring[$pos]) & $mask) @@ -701,7 +701,7 @@ function voucher_save_db_to_config() { function voucher_save_db_to_config_zone() { global $config, $g, $cpzone; - + if (!isset($config['voucher'][$cpzone]['enable'])) return; // no vouchers or don't want to save DB's diff --git a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/ARP.php b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/ARP.php index a44891702..6b3a97721 100644 --- a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/ARP.php +++ b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/ARP.php @@ -109,4 +109,4 @@ class ARP } -} +} \ No newline at end of file diff --git a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php index 84a474cc8..66a8d05df 100644 --- a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php +++ b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/CPClient.php @@ -58,7 +58,7 @@ class CPClient { /** * link to shell object - * @var \Core\Shell + * @var Core\Shell */ private $shell = null; @@ -71,10 +71,10 @@ class CPClient { * @param $ip * @param string $message */ - private function logportalauth($user,$mac,$ip,$status,$message=""){ + private function logportalauth($cpzonename, $user, $mac, $ip, $status, $message=""){ $message = trim($message); - $message = "{$status}: {$user}, {$mac}, {$ip}, {$message}"; + $message = "Zone : {$cpzonename} {$status}: {$user}, {$mac}, {$ip}, {$message}"; $logger = new \Phalcon\Logger\Adapter\Syslog("logportalauth", array( 'option' => LOG_PID, @@ -83,6 +83,7 @@ class CPClient { $logger->info($message); } + /** * Request new pipeno * @return int @@ -134,13 +135,30 @@ class CPClient { } } + /** + * load accounting rules into ruleset, used for reinitialisation of the ruleset. + * triggers add_accounting() for all active clients in all zones + */ + private function loadAccounting() + { + foreach ($this->config->object()->captiveportal->children() as $cpzonename => $zone) + { + $db = new DB($cpzonename); + foreach ($db->listClients(array()) as $client) + { + $this->add_accounting($zone->zoneid, $client->ip) ; + } + unset($db); + } + } + /** * * @param $zoneid * @param $ip */ public function add_accounting($zoneid,$ip){ - // TODO: check speed, this might need some improvement + // TODO: check processing speed, this might need some improvement // check if our ip is already in the list and collect first free rule number to place it there if necessary $shell_output=array(); $this->shell->exec("/sbin/ipfw show",false,false,$shell_output); @@ -227,7 +245,7 @@ class CPClient { */ function __construct() { // Request handle to configuration - $this->config = \Core\Config::getInstance(); + $this->config = Core\Config::getInstance(); // generate new ruleset $this->rules = new Rules(); // keep a link to the shell object @@ -246,6 +264,9 @@ class CPClient { // update tables $this->update(); + + // after reinit all accounting rules are vanished, reapply them for active sessions + $this->loadAccounting(); } /** @@ -552,7 +573,7 @@ class CPClient { $this->reset_bandwidth($pipeno_in,$bw_down); // log - $this->logportalauth($username,$clientmac,$clientip,$status="LOGIN"); + $this->logportalauth($cpzonename, $username, $clientmac, $clientip, $status="LOGIN"); // cleanup unset($db); @@ -576,7 +597,6 @@ class CPClient { } } - /** * flush zone (null flushes all zones) * @param null $zone @@ -633,7 +653,7 @@ class CPClient { if (is_numeric($client->session_timeout) && $client->session_timeout > 0 ) { if (((time() - $client->allow_time) / 60) > $client->session_timeout) { $this->disconnect($cpzonename, $client->sessionid); - $this->logportalauth($client->username,$client->mac,$client->ip,$status="SESSION TIMEOUT"); + $this->logportalauth($cpzonename, $client->username, $client->mac, $client->ip, $status="SESSION TIMEOUT"); continue; } } @@ -642,7 +662,7 @@ class CPClient { if (is_numeric($client->idle_timeout) && $client->idle_timeout > 0 && $idle_time > 0) { if ($idle_time > $client->idle_timeout) { $this->disconnect($cpzonename, $client->sessionid); - $this->logportalauth($client->username,$client->mac,$client->ip,$status="IDLE TIMEOUT"); + $this->logportalauth($cpzonename, $client->username, $client->mac, $client->ip, $status="IDLE TIMEOUT"); continue; } } @@ -650,7 +670,7 @@ class CPClient { // disconnect on session terminate time if ( is_numeric($client->session_terminate_time) && $client->session_terminate_time > 0 && $client->session_terminate_time < time()) { $this->disconnect($cpzonename, $client->sessionid); - $this->logportalauth($client->username,$client->mac,$client->ip,$status="TERMINATE TIME REACHED"); + $this->logportalauth($cpzonename, $client->username, $client->mac, $client->ip, $status="TERMINATE TIME REACHED"); continue; } } diff --git a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/DB.php b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/DB.php index 3fd649371..df663e119 100644 --- a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/DB.php +++ b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/DB.php @@ -384,4 +384,4 @@ class DB { -} +} \ No newline at end of file diff --git a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/Rules.php b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/Rules.php index d762d4514..f5cddb10d 100644 --- a/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/Rules.php +++ b/src/opnsense/mvc/app/models/OPNsense/CaptivePortal/Rules.php @@ -33,6 +33,7 @@ namespace OPNsense\CaptivePortal; +use OPNsense\Core; /** * Class Rules @@ -59,7 +60,7 @@ class Rules { function __construct() { // Request handle to configuration - $this->config = \Core\Config::getInstance(); + $this->config = Core\Config::getInstance(); } @@ -130,6 +131,8 @@ class Rules { $this->rules[] = "#========================================================================================================="; foreach( $this->config->object()->interfaces->children() as $interface => $content ){ if ( $interface != "wan" && $content->ipaddr != "dhcp" ){ + // only keep state of dns traffic to prevent dns resolver failures + $this->rules[] = "add ".$rulenum++." allow udp from any to ".$content->ipaddr." dst-port 53 keep-state in"; $this->rules[] = "add ".$rulenum++." allow ip from any to { 255.255.255.255 or ".$content->ipaddr." } in"; $this->rules[] = "add ".$rulenum++." allow ip from { 255.255.255.255 or ".$content->ipaddr." } to any out"; $this->rules[] = "add ".$rulenum++." allow icmp from { 255.255.255.255 or ".$content->ipaddr." } to any out icmptypes 0"; @@ -318,4 +321,4 @@ class Rules { file_put_contents($filename,$ruleset_txt); } -} +} \ No newline at end of file diff --git a/src/opnsense/mvc/app/models/OPNsense/Core/Shell.php b/src/opnsense/mvc/app/models/OPNsense/Core/Shell.php index 58714f15b..f840e200f 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Core/Shell.php +++ b/src/opnsense/mvc/app/models/OPNsense/Core/Shell.php @@ -123,4 +123,4 @@ class Shell } -} +} \ No newline at end of file diff --git a/src/www/status_captiveportal.php b/src/www/status_captiveportal.php index 7e101b33a..77b679490 100644 --- a/src/www/status_captiveportal.php +++ b/src/www/status_captiveportal.php @@ -200,7 +200,7 @@ $mac_man = load_mac_manufacturer_table(); - ')">"> + ')">">