From cf25c31a3720c01498f7796defd439cd314736a9 Mon Sep 17 00:00:00 2001 From: Roland Gruber Date: Sat, 29 Dec 2018 09:51:51 +0100 Subject: [PATCH] refactoring --- lam/VERSION | 2 +- lam/lib/modules/dhcp_settings.inc | 20 +++------ lam/lib/modules/fixed_ip.inc | 37 +++++++--------- lam/lib/modules/range.inc | 53 +++++++++------------- lam/lib/security.inc | 74 ++++++++++++++++++++----------- lam/templates/login.php | 16 +++---- 6 files changed, 104 insertions(+), 98 deletions(-) diff --git a/lam/VERSION b/lam/VERSION index 4074fe20..61d3db4a 100644 --- a/lam/VERSION +++ b/lam/VERSION @@ -1 +1 @@ -6.6 +6.7.DEV diff --git a/lam/lib/modules/dhcp_settings.inc b/lam/lib/modules/dhcp_settings.inc index 6536510c..73c2e6cf 100644 --- a/lam/lib/modules/dhcp_settings.inc +++ b/lam/lib/modules/dhcp_settings.inc @@ -47,7 +47,7 @@ if (!function_exists('check_ip')) { */ function check_ip($ip, $subnet = false) { $part = explode(".", $ip); - // Wenn... Keine 4 Segmente gefunden wurde + // IP must contain 4 segments if (count($part) != 4) { return false; } @@ -55,18 +55,12 @@ if (!function_exists('check_ip')) { // check each segment for ($i = 0; $i < count($part); $i++) { // only numbers are allowed - if (!is_numeric($part[$i])) { - return false; - } - elseif ($part[$i] > 255) { - return false; - } - // non-subnet must be > 0 on last digit - elseif (!$subnet && ($i == 3) && ($part[$i] < 1)) { - return false; - } - // subnet must be >= 0 on last digit - elseif ($subnet && ($i == 3) && ($part[$i] < 0)) { + if (!is_numeric($part[$i]) + || ($part[$i] > 255) + // non-subnet must be > 0 on last digit + || (!$subnet && ($i == 3) && ($part[$i] < 1)) + // subnet must be >= 0 on last digit + || ($subnet && ($i == 3) && ($part[$i] < 0))) { return false; } } diff --git a/lam/lib/modules/fixed_ip.inc b/lam/lib/modules/fixed_ip.inc index ed7f6ac1..834f4a5b 100644 --- a/lam/lib/modules/fixed_ip.inc +++ b/lam/lib/modules/fixed_ip.inc @@ -148,15 +148,12 @@ class fixed_ip extends baseModule { } /** + * Checks if IPs are not overlaped. * - * Checked, if ips are overlapd. - * - * @param ip - * - * @return false, if overlapd, else true. - * + * @param ip IP address + * @return not overlaped **/ - public function overlapd_ip($ip) { + public function isNotOverlapedIp($ip) { if (in_array($ip, $this->overlapd)) { return false; } @@ -216,8 +213,9 @@ class fixed_ip extends baseModule { $ex = explode(".", $this->fixed_ip[$id]['ip']); $tmp = $this->fixed_ip[$id]['ip']; $this->fixed_ip[$id]['ip'] = $ex_subnet['0'].".".$ex_subnet['1'].".".$ex_subnet['2'].".".$ex['3']; - if ($tmp!=$this->fixed_ip[$id]['ip']) + if ($tmp!=$this->fixed_ip[$id]['ip']) { $ip_edit = true; + } } } } @@ -231,8 +229,8 @@ class fixed_ip extends baseModule { */ function load_attributes($attr) { if (!$this->isRootNode()) { - $attributes = array('cn', 'dhcphwaddress', 'dhcpstatements', 'dhcpcomments'); - $entries = searchLDAP($this->getAccountContainer()->dn_orig, '(objectClass=dhcpHost)', $attributes); + $searchAttributes = array('cn', 'dhcphwaddress', 'dhcpstatements', 'dhcpcomments'); + $entries = searchLDAP($this->getAccountContainer()->dn_orig, '(objectClass=dhcpHost)', $searchAttributes); for ($i = 0; $i < sizeof($entries); $i++) { $dhcphwaddress = explode(" ", $entries[$i]['dhcphwaddress'][0]); $dhcphwaddress = array_pop($dhcphwaddress); @@ -332,11 +330,8 @@ class fixed_ip extends baseModule { if (!empty($_POST['ip_'.$id])) { $_POST['ip_'.$id] = trim($_POST['ip_'.$id]); } - if (!empty($_POST['ip_'.$id]) && !(check_ip($_POST['ip_'.$id]))) { - $error = true; - $this->fixed_ip[$id]['ip'] = $_POST['ip_'.$id]; - } - elseif (!empty($_POST['ip_'.$id]) && !$this->overlapd_ip($_POST['ip_'.$id])) { + if ((!empty($_POST['ip_'.$id]) && !(check_ip($_POST['ip_'.$id]))) + || (!empty($_POST['ip_'.$id]) && !$this->isNotOverlapedIp($_POST['ip_'.$id]))) { $error = true; $this->fixed_ip[$id]['ip'] = $_POST['ip_'.$id]; } @@ -352,7 +347,9 @@ class fixed_ip extends baseModule { } // cn: - if (!empty($_POST['pc_'.$id])) $_POST['pc_'.$id] = trim($_POST['pc_'.$id]); + if (!empty($_POST['pc_'.$id])) { + $_POST['pc_'.$id] = trim($_POST['pc_'.$id]); + } if (!empty($_POST['pc_'.$id])) { // name already in use @@ -415,7 +412,7 @@ class fixed_ip extends baseModule { // auto-completion for host names $autoNames = array(); if (!empty($this->hostCache) && (sizeof($this->hostCache) < 200)) { - foreach ($this->hostCache as $index => $attrs) { + foreach ($this->hostCache as $attrs) { if (!empty($attrs['cn'][0])) { $autoNames[] = $attrs['cn'][0]; } @@ -452,10 +449,10 @@ class fixed_ip extends baseModule { $this->fixed_ip = array(); } $pcs = array(); - foreach($this->fixed_ip AS $id=>$arr) { + foreach($this->fixed_ip AS $id => $arr) { // pc name $pcError = ""; - $existsInDifferentDn = $this->hostNameExists($_POST['pc_'.$id]); + $existsInDifferentDn = !empty($_POST['pc_' . $id]) && $this->hostNameExists($_POST['pc_' . $id]); if (!$this->processed) { $pcError = ""; } @@ -507,7 +504,7 @@ class fixed_ip extends baseModule { $this->getAccountContainer()->getAccountModule('dhcp_settings')->getDHCPOption('subnet-mask'))) { $ipError = _("The IP address does not match the subnet."); } - elseif (!$this->overlapd_ip($this->fixed_ip[$id]['ip'])) { + elseif (!$this->isNotOverlapedIp($this->fixed_ip[$id]['ip'])) { $ipError = _("The IP address is already in use."); } $error = ''; diff --git a/lam/lib/modules/range.inc b/lam/lib/modules/range.inc index 0fa30c91..7d454379 100644 --- a/lam/lib/modules/range.inc +++ b/lam/lib/modules/range.inc @@ -7,7 +7,7 @@ $Id$ This code is part of LDAP Account Manager (http://www.ldap-account-manager.org/) Copyright (C) 2008 Thomas Manninger - 2008 - 2017 Roland Gruber + 2008 - 2018 Roland Gruber This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -164,16 +164,10 @@ class range extends baseModule { public function check_range($first_ip, $second_ip) { $ex_first = explode(".", $first_ip); $ex_second = explode(".", $second_ip); - if ($ex_first[0]<$ex_second[0]) - return true; - if ($ex_first[1]<$ex_second[1]) - return true; - if ($ex_first[2]<$ex_second[2]) - return true; - if ($ex_first[3]<$ex_second[3]) { - return true; - } - return false; + return ($ex_first[0] < $ex_second[0]) + || ($ex_first[1] < $ex_second[1]) + || ($ex_first[2] < $ex_second[2]) + || ($ex_first[3] < $ex_second[3]); } /** @@ -191,15 +185,15 @@ class range extends baseModule { /** * - * Checked, if Ranges are overlaped. + * Checks if ranges are not overlaped. * * @param first ip * @param second ip * - * @return false, if overlaped, else true. + * @return not overlaped * **/ - function overlaped_range($ip,$ipB) { + function isNotOverlapedRange($ip,$ipB) { $ex = explode(".", $ip); $exB = explode(".", $ipB); @@ -286,16 +280,18 @@ class range extends baseModule { $ex = explode(".", $this->ranges[$id]['range_start']); $tmp = $this->ranges[$id]['range_start']; $this->ranges[$id]['range_start'] = $ex_subnet['0'].".".$ex_subnet['1'].".".$ex_subnet['2'].".".$ex['3']; - if($tmp!=$this->ranges[$id]['range_start']) + if($tmp!=$this->ranges[$id]['range_start']) { $range_edit = true; + } } if (!empty($this->ranges[$id]['range_end']) && !range::check_subnet_range($this->ranges[$id]['range_end'],$dhcpAttrs['cn'][0], $mask)) { // Range anpassen: $ex = explode(".", $this->ranges[$id]['range_end']); $tmp = $this->ranges[$id]['range_end']; $this->ranges[$id]['range_end'] = $ex_subnet['0'].".".$ex_subnet['1'].".".$ex_subnet['2'].".".$ex['3']; - if($tmp!=$this->ranges[$id]['range_end']) + if($tmp!=$this->ranges[$id]['range_end']) { $range_edit = true; + } } } if ($range_edit) { @@ -354,7 +350,7 @@ class range extends baseModule { $this->ranges[$id]['range_end'] = $_POST['range_end_'.$id]; // Check if ip overlaped: - if(!$this->overlaped_range($_POST['range_start_'.$id],$_POST['range_end_'.$id])) { + if(!$this->isNotOverlapedRange($_POST['range_start_'.$id],$_POST['range_end_'.$id])) { $errorOccured = true; } @@ -438,16 +434,11 @@ class range extends baseModule { else { $this->poolsNew[$index]['dhcprange'][$rIndex] = $from . ' ' . $to; // check ranges - if (!check_ip($from) || !check_ip($to)) { - $errorOccured = true; - } - elseif (!$this->overlaped_range($from, $to)) { - $errorOccured = true; - } - elseif (!range::check_subnet_range($from, $subnet, $mask) || !range::check_subnet_range($to, $subnet, $mask)) { - $errorOccured = true; - } - elseif (!$this->check_range($from, $to)) { + if (!check_ip($from) || !check_ip($to) + || !$this->isNotOverlapedRange($from, $to) + || !range::check_subnet_range($from, $subnet, $mask) + || !range::check_subnet_range($to, $subnet, $mask) + || !$this->check_range($from, $to)) { $errorOccured = true; } } @@ -510,7 +501,7 @@ class range extends baseModule { $error = _("The range end needs to be greater than the range start."); } elseif ($this->processed && !range::check_subnet_range($this->ranges[$id]['range_start'], $subnet, $mask)) { $error = _("The IP does not match the subnet."); - } elseif ($this->processed && !$this->overlaped_range($this->ranges[$id]['range_start'],$this->ranges[$id]['range_end']) ) { + } elseif ($this->processed && !$this->isNotOverlapedRange($this->ranges[$id]['range_start'],$this->ranges[$id]['range_end']) ) { $error = _("The range conflicts with another range."); } } @@ -590,7 +581,7 @@ class range extends baseModule { elseif (!range::check_subnet_range($from, $subnet, $mask)) { $message = _("The IP does not match the subnet."); } - elseif (!$this->overlaped_range($from, $to)) { + elseif (!$this->isNotOverlapedRange($from, $to)) { $message = _("The range conflicts with another range."); } } @@ -781,7 +772,7 @@ class range extends baseModule { if (!empty($this->poolsNew)) { - foreach ($this->poolsNew as $index => $poolAttrs) { + foreach ($this->poolsNew as $poolAttrs) { $cn = !empty($poolAttrs['cn'][0]) ? $poolAttrs['cn'][0] : ''; $peer = ''; if (!empty($poolAttrs['dhcpstatements'])) { @@ -792,7 +783,7 @@ class range extends baseModule { } } if (!empty($poolAttrs['dhcprange'])) { - foreach ($poolAttrs['dhcprange'] as $rIndex => $range) { + foreach ($poolAttrs['dhcprange'] as $range) { $range = explode(' ', $range); $from = !empty($range[0]) ? $range[0] : ''; $to = !empty($range[1]) ? $range[1] : ''; diff --git a/lam/lib/security.inc b/lam/lib/security.inc index bea11c4d..b223e6d3 100644 --- a/lam/lib/security.inc +++ b/lam/lib/security.inc @@ -61,7 +61,9 @@ function lam_start_session() { */ function startSecureSession($redirectToLogin = true, $initSecureData = false) { // start session - if (isset($_SESSION)) unset($_SESSION); + if (isset($_SESSION)) { + unset($_SESSION); + } if (strtolower(session_module_name()) == 'files') { $sessionDir = dirname(__FILE__) . "/../sess"; session_save_path($sessionDir); @@ -125,8 +127,12 @@ function startSecureSession($redirectToLogin = true, $initSecureData = false) { * */ function checkClientIP() { - if (isset($_SESSION['cfgMain'])) $cfg = $_SESSION['cfgMain']; - else $cfg = new LAMCfgMain(); + if (isset($_SESSION['cfgMain'])) { + $cfg = $_SESSION['cfgMain']; + } + else { + $cfg = new LAMCfgMain(); + } $allowedHosts = $cfg->allowedHosts; $url = getCallingURL(); if ((strpos($url, '/selfService/selfService') !== false) || ((strpos($url, '/misc/ajax.php?') !== false) && strpos($url, 'selfservice=1') !== false)) { @@ -142,7 +148,9 @@ function checkClientIP() { for ($i = 0; $i < sizeof($allowedHosts); $i++) { $host = $allowedHosts[$i]; $ipRegex = '/^[0-9a-z\\.:\\*]+$/i'; - if (!preg_match($ipRegex, $host)) continue; + if (!preg_match($ipRegex, $host)) { + continue; + } $hostRegex = str_replace(".", "\\.", $host); $hostRegex = '/^' . str_replace("*", ".*", $hostRegex) . '$/'; $clientIP = $_SERVER['REMOTE_ADDR']; @@ -225,8 +233,12 @@ function logoffAndBackToLoginPage() { * @return boolean debug enabled */ function isDebugLoggingEnabled() { - if (isset($_SESSION['cfgMain'])) $cfg = $_SESSION['cfgMain']; - else $cfg = new LAMCfgMain(); + if (isset($_SESSION['cfgMain'])) { + $cfg = $_SESSION['cfgMain']; + } + else { + $cfg = new LAMCfgMain(); + } return $cfg->logLevel >= LOG_DEBUG; } @@ -238,13 +250,21 @@ function isDebugLoggingEnabled() { */ function logNewMessage($level, $message) { $possibleLevels = array(LOG_DEBUG => 'DEBUG', LOG_NOTICE => 'NOTICE', LOG_WARNING => 'WARNING', LOG_ERR => 'ERROR'); - if (!in_array($level, array_keys($possibleLevels))) StatusMessage('ERROR', 'Invalid log level!', $level); - if (isset($_SESSION['cfgMain'])) $cfg = $_SESSION['cfgMain']; - else $cfg = new LAMCfgMain(); + if (!in_array($level, array_keys($possibleLevels))) { + StatusMessage('ERROR', 'Invalid log level!', $level); + } + if (isset($_SESSION['cfgMain'])) { + $cfg = $_SESSION['cfgMain']; + } + else { + $cfg = new LAMCfgMain(); + } // check if logging is disabled - if ($cfg->logDestination == 'NONE') return; - // check if log level is high enough - elseif ($cfg->logLevel < $level) return; + if (($cfg->logDestination == 'NONE') + // check if log level is high enough + || ($cfg->logLevel < $level)) { + return; + } // ok to log, build log message $prefix = "LDAP Account Manager (" . session_id() . ' - ' . getClientIPForLogging() . ") - " . $possibleLevels[$level] . ": "; $message = $prefix . $message; @@ -286,11 +306,10 @@ function checkIfWriteAccessIsAllowed($scope = null) { } if ($_SESSION['config']->getAccessLevel() >= LAMConfig::ACCESS_ALL) { $typeSettings = $_SESSION['config']->get_typeSettings(); - if ($scope == null) { - return true; - } - elseif (!isset($typeSettings['readOnly_' . $scope]) || !$typeSettings['readOnly_' . $scope]) { - // check if write for this type is allowed + if (($scope == null) + // check if write for this type is allowed + || !isset($typeSettings['readOnly_' . $scope]) + || !$typeSettings['readOnly_' . $scope]) { return true; } } @@ -365,8 +384,12 @@ function checkPasswordStrength($password, $userName, $otherUserAttrs) { if ($password == null) { $password = ""; } - if (isset($_SESSION['cfgMain'])) $cfg = $_SESSION['cfgMain']; - else $cfg = new LAMCfgMain(); + if (isset($_SESSION['cfgMain'])) { + $cfg = $_SESSION['cfgMain']; + } + else { + $cfg = new LAMCfgMain(); + } // check length if (strlen($password) < $cfg->passwordMinLength) { return sprintf(_('The password is too short. You have to enter at least %s characters.'), $cfg->passwordMinLength); @@ -656,8 +679,9 @@ function lamEncrypt($data, $prefix='') { // use OpenSSL if available if (function_exists('openssl_random_pseudo_bytes')) { // OpenSSL may have been enabled in a running session - if (!isset($_COOKIE[$prefix . "IV"]) || ($_COOKIE[$prefix . "IV"] == '')) return $data; - if ($_COOKIE[$prefix . "IV"] == "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") { + if (!isset($_COOKIE[$prefix . "IV"]) + || ($_COOKIE[$prefix . "IV"] == '') + || ($_COOKIE[$prefix . "IV"] == "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")) { return $data; } // read key and iv from cookie @@ -683,8 +707,9 @@ function lamDecrypt($data, $prefix='') { // use OpenSSL if available if (function_exists('openssl_random_pseudo_bytes')) { // OpenSSL may have been enabled in a running session - if (!isset($_COOKIE[$prefix . "IV"]) || ($_COOKIE[$prefix . "IV"] == '')) return $data; - if ($_COOKIE[$prefix . "IV"] == "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx") { + if (!isset($_COOKIE[$prefix . "IV"]) + || ($_COOKIE[$prefix . "IV"] == '') + || ($_COOKIE[$prefix . "IV"] == "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")) { return $data; } // read key and iv from cookie @@ -692,8 +717,7 @@ function lamDecrypt($data, $prefix='') { $key = base64_decode($_COOKIE[$prefix . "Key"]); // decrypt string $ret = openssl_decrypt($data, lamEncryptionAlgo(), $key, 0, $iv); - $ret = base64_decode(str_replace(chr(00), "", $ret)); - return $ret; + return base64_decode(str_replace(chr(00), "", $ret)); } // otherwise do not decrypt else { diff --git a/lam/templates/login.php b/lam/templates/login.php index 6e1b0bdb..7d36fe1b 100644 --- a/lam/templates/login.php +++ b/lam/templates/login.php @@ -533,13 +533,17 @@ if(isset($_POST['checklogin'])) { else { $searchSuccess = false; $searchError = _('Unable to find the user name in LDAP.'); - if (ldap_errno($searchLDAP->server()) != 0) $searchError .= ' ' . getDefaultLDAPErrorString($searchLDAP->server()); + if (ldap_errno($searchLDAP->server()) != 0) { + $searchError .= ' ' . getDefaultLDAPErrorString($searchLDAP->server()); + } } } else { $searchSuccess = false; $searchError = _('Unable to find the user name in LDAP.'); - if (ldap_errno($searchLDAP->server()) != 0) $searchError .= ' ' . getDefaultLDAPErrorString($searchLDAP->server()); + if (ldap_errno($searchLDAP->server()) != 0) { + $searchError .= ' ' . getDefaultLDAPErrorString($searchLDAP->server()); + } } } if (!$searchSuccess) { @@ -579,12 +583,8 @@ if(isset($_POST['checklogin'])) { die(); } else { - if ($result === False) { - // connection failed - $error_message = _("Cannot connect to specified LDAP server. Please try again."); - logNewMessage(LOG_ERR, 'User ' . $username . ' (' . $clientSource . ') failed to log in (LDAP error: ' . ldap_err2str($result) . ').'); - } - elseif ($result == 81) { + if (($result === False) + || ($result == 81)) { // connection failed $error_message = _("Cannot connect to specified LDAP server. Please try again."); logNewMessage(LOG_ERR, 'User ' . $username . ' (' . $clientSource . ') failed to log in (LDAP error: ' . ldap_err2str($result) . ').');