From 393d1562a03e0446e0e004b95dd7a5c8710ad341 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Thu, 1 Aug 2019 11:14:17 +0200 Subject: [locations] Refine location matching checks --- modules-available/locations/inc/location.inc.php | 30 ++++++++++++---------- .../locations/inc/locationutil.inc.php | 19 +++++++++----- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/modules-available/locations/inc/location.inc.php b/modules-available/locations/inc/location.inc.php index e35670cc..ac866cbc 100644 --- a/modules-available/locations/inc/location.inc.php +++ b/modules-available/locations/inc/location.inc.php @@ -329,7 +329,7 @@ class Location if ($uuid !== false) { // Machine ip maps to a location, and we have a client supplied uuid (which might not be known if the client boots for the first time) $uuidLoc = self::getFromMachineUuid($uuid); - if (self::isUuidLocationValid($uuidLoc, $ipLoc)) { + if (self::isFixedLocationValid($uuidLoc, $ipLoc)) { $locationId = $uuidLoc; } } @@ -337,20 +337,22 @@ class Location return $locationId; } - public static function isUuidLocationValid($uuidLoc, $ipLoc) + public static function isFixedLocationValid($uuidLoc, $ipLoc) { - if ($ipLoc !== false && $uuidLoc !== false && $uuidLoc !== $ipLoc) { - // Validate that the location the IP maps to is in the chain we get using the - // location determined by the uuid - $uuidLocations = self::getLocationRootChain($uuidLoc); - $ipLocations = self::getLocationRootChain($ipLoc); - if (count($ipLocations) + 2 >= count($uuidLocations) && in_array($ipLoc, $uuidLocations)) { - // UUID is max one level deeper than IP loc, accept - return true; - } - // UUID and IP disagree too much, play safe and ignore the UUID - } - return false; + $uuidLoc = (int)$uuidLoc; + $ipLoc = (int)$ipLoc; + if ($uuidLoc === $ipLoc || $uuidLoc === 0) + return true; + if ($ipLoc === 0) + return false; // roomplanner assignment, but no subnet - deny + // Validate that the location the IP maps to is in the chain we get using the + // location determined by roomplanner + $uuidLocations = self::getLocationRootChain($uuidLoc); + if (!empty(self::$assocLocationCache[$uuidLoc]['children'])) + return false; // roomplanner location isn't actually leaf node, this is not valid + $idx = array_search($ipLoc, $uuidLocations); + // If roomplanner loc is max two levels deeper than IP loc, accept + return $idx !== false && $idx <= 2; } /** diff --git a/modules-available/locations/inc/locationutil.inc.php b/modules-available/locations/inc/locationutil.inc.php index 6b18d864..708cc8a2 100644 --- a/modules-available/locations/inc/locationutil.inc.php +++ b/modules-available/locations/inc/locationutil.inc.php @@ -125,9 +125,7 @@ class LocationUtil $return = []; $locs = false; while ($row = $res->fetch(PDO::FETCH_ASSOC)) { - if ($row['subnetlocationid'] === $row['fixedlocationid']) - continue; - if (Location::isUuidLocationValid((int)$row['fixedlocationid'], (int)$row['subnetlocationid'])) + if (Location::isFixedLocationValid($row['fixedlocationid'], $row['subnetlocationid'])) continue; $lid = (int)$row['fixedlocationid']; if (!isset($return[$lid])) { @@ -145,14 +143,21 @@ class LocationUtil $return[$lid]['count']++; } else { $slid = (int)$row['subnetlocationid']; - $return[$lid]['clients'][] = [ + $data = [ 'machineuuid' => $row['machineuuid'], 'hostname' => $row['hostname'], 'clientip' => $row['clientip'], - 'iplocationid' => $slid, - 'iplocationname' => $locs[$slid]['locationname'], - 'canmove' => !$checkPerms || $ipLocs === true || in_array($slid, $ipLocs), // Can machine be moved to subnet's locationid? ]; + if ($slid !== 0) { + $data += [ + 'iplocationid' => $slid, + 'iplocationname' => $locs[$slid]['locationname'], + 'ipisleaf' => empty($locs[$slid]['children']), + 'canmove' => empty($locs[$slid]['children']) + && (!$checkPerms || $ipLocs === true || in_array($slid, $ipLocs)), // Can machine be moved to subnet's locationid? + ]; + } + $return[$lid]['clients'][] = $data; } } if (empty($return)) -- cgit v1.2.3-55-g7522