From 7f0b486dbb83a161da5ec32898461c5b9e803cfd Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Mon, 23 Jul 2018 20:31:42 +0200
Subject: [PATCH] Firewall/alias, replace legacy alias usage and move
 transitional code into model for https://github.com/opnsense/core/issues/1858

This code keeps the legacy aliases functional until switched (migrated) to the new ones, improves isAlias() performance which was in 18.7r2 and removes some code duplication.
---
 src/etc/inc/config.inc                        | 19 ++------
 src/etc/inc/filter.inc                        | 38 ++++++++--------
 src/etc/inc/filter.lib.inc                    | 22 +++++----
 src/etc/inc/legacy_bindings.inc               | 45 +++++--------------
 src/etc/inc/util.inc                          | 16 +++----
 .../app/library/OPNsense/Firewall/Util.php    | 33 ++++++++------
 .../app/models/OPNsense/Firewall/Alias.php    | 28 ++++++++++++
 src/www/interfaces.php                        |  8 ----
 src/www/wizard.php                            | 14 +++---
 9 files changed, 102 insertions(+), 121 deletions(-)

diff --git a/src/etc/inc/config.inc b/src/etc/inc/config.inc
index 93e15ac44..b5fb44519 100644
--- a/src/etc/inc/config.inc
+++ b/src/etc/inc/config.inc
@@ -51,23 +51,12 @@ require_once('plugins.inc');
 function alias_make_table($config)
 {
     global $aliastable;
-
     $aliastable = array();
-    $aliasMdl = new \OPNsense\Firewall\Alias();
-    // MVC defined aliases
-    foreach ($aliasMdl->aliases->alias->__items as $alias) {
-        if (strncmp((string)$alias->type, 'url', 3) !== 0) {
-            $aliastable[(string)$alias->name] = implode(" ", explode("\n", $alias->content));
+    foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $alias) {
+        if (strncmp($alias['type'], 'url', 3) !== 0) {
+            $aliastable[$alias['name']] = implode(" ", explode("\n", $alias['content']));
         } else {
-            $aliastable[(string)$alias->name] = "";
-        }
-    }
-    // legacy defined, keep during transition.
-    if (empty($aliastable) && isset($config['aliases']['alias'])) {
-        foreach ($config['aliases']['alias'] as $alias) {
-            if ($alias['name']) {
-                $aliastable[$alias['name']] = isset($alias['address']) ? $alias['address'] : null;
-            }
+            $aliastable[$alias['name']] = "";
         }
     }
 }
diff --git a/src/etc/inc/filter.inc b/src/etc/inc/filter.inc
index f6f44d7f6..498a201a8 100644
--- a/src/etc/inc/filter.inc
+++ b/src/etc/inc/filter.inc
@@ -617,26 +617,24 @@ function filter_generate_aliases()
         $aliases .= "table <bogonsv6> persist file \"/usr/local/etc/bogonsv6\"\n";
     }
 
-    if (isset($config['aliases']['alias'])) {
-        $aliases .= "\n# User Aliases\n";
-        foreach ($config['aliases']['alias'] as $aliased) {
-            switch ($aliased['type']) {
-                case "urltable_ports":
-                case "url_ports":
-                    # a bit of a hack, but prevents the ruleset from not being able to load if these types are in
-                    # the configuration.
-                    $aliases .= "{$aliased['name']} = \"{ 0 <> 65535 }\"\n";
-                    file_notice('filter_load', sprintf(gettext('URL port aliases types not supported [%s]'), $aliased['name']), 'Filter Reload', '');
-                    break;
-                case "port":
-                    $tmp_ports = implode(" ", filter_core_get_port_alias($aliased['name']));
-                    $aliases .= "{$aliased['name']} = \"{ {$tmp_ports} }\"\n";
-                    break;
-                default:
-                    $aliases .= "table <{$aliased['name']}> persist\n";
-                    $aliases .= "{$aliased['name']} = \"<{$aliased['name']}>\"\n";
-                    break;
-            }
+    $aliases .= "\n# User Aliases\n";
+    foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $aliased) {
+        switch ($aliased['type']) {
+            case "urltable_ports":
+            case "url_ports":
+                # a bit of a hack, but prevents the ruleset from not being able to load if these types are in
+                # the configuration.
+                $aliases .= "{$aliased['name']} = \"{ 0 <> 65535 }\"\n";
+                file_notice('filter_load', sprintf(gettext('URL port aliases types not supported [%s]'), $aliased['name']), 'Filter Reload', '');
+                break;
+            case "port":
+                $tmp_ports = implode(" ", filter_core_get_port_alias($aliased['name']));
+                $aliases .= "{$aliased['name']} = \"{ {$tmp_ports} }\"\n";
+                break;
+            default:
+                $aliases .= "table <{$aliased['name']}> persist\n";
+                $aliases .= "{$aliased['name']} = \"<{$aliased['name']}>\"\n";
+                break;
         }
     }
 
diff --git a/src/etc/inc/filter.lib.inc b/src/etc/inc/filter.lib.inc
index 4636e7f54..0e995e369 100644
--- a/src/etc/inc/filter.lib.inc
+++ b/src/etc/inc/filter.lib.inc
@@ -121,21 +121,19 @@ function filter_core_get_port_alias($aliasname, $aliases=array())
 
     $response = array();
     $aliases[] = $aliasname;
-    if (isset($config['aliases']['alias'])) {
-        foreach ($config['aliases']['alias'] as $aliased) {
-            if ($aliasname == $aliased['name'] && $aliased['type'] == 'port') {
-                foreach (explode(" ", $aliased['address']) as $address) {
-                    if (is_alias($address)) {
-                        if (!in_array($address, $aliases)) {
-                            foreach (filter_core_get_port_alias($address, $aliases) as $port) {
-                                if (!in_array($port, $response)) {
-                                    $response[] = $port;
-                                }
+    foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $aliased) {
+        if ($aliasname == $aliased['name'] && $aliased['type'] == 'port') {
+            foreach (explode("\n", $aliased['content']) as $address) {
+                if (is_alias($address)) {
+                    if (!in_array($address, $aliases)) {
+                        foreach (filter_core_get_port_alias($address, $aliases) as $port) {
+                            if (!in_array($port, $response)) {
+                                $response[] = $port;
                             }
                         }
-                    } elseif ((is_port($address) || is_portrange($address)) && !in_array($address, $response)) {
-                        $response[] = $address ;
                     }
+                } elseif ((is_port($address) || is_portrange($address)) && !in_array($address, $response)) {
+                    $response[] = $address ;
                 }
             }
         }
diff --git a/src/etc/inc/legacy_bindings.inc b/src/etc/inc/legacy_bindings.inc
index dc3a8ad4e..075332f0a 100644
--- a/src/etc/inc/legacy_bindings.inc
+++ b/src/etc/inc/legacy_bindings.inc
@@ -74,29 +74,11 @@ function legacy_list_aliases($type)
     global $config;
     $result = array();
 
-    $aliasMdl = new \OPNsense\Firewall\Alias();
-    // MVC defined aliases
-    foreach ($aliasMdl->aliases->alias->__items as $alias) {
-        if ($type == "port" && preg_match("/port/i", (string)$alias->type)) {
-            $result[] = array("name" => (string)$alias->name);
-        } elseif ($type != "port" && !preg_match("/port/i", (string)$alias->type)) {
-            $result[] = array("name" => (string)$alias->name);
-        }
-    }
-    // legacy defined, keep during transition.
-    if (isset($config['aliases']['alias']) && empty($result)) {
-        foreach ($config['aliases']['alias'] as $alias) {
-            if (!empty($alias['address']) || !empty($alias['url']) || !empty($alias['aliasurl']) || $alias['type'] == 'external') {
-                if ($type == "port") {
-                    if (preg_match("/port/i", $alias['type'])) {
-                        $result[] = $alias;
-                    }
-                } else {
-                    if (!preg_match("/port/i", $alias['type'])){
-                        $result[] = $alias;
-                    }
-                }
-            }
+    foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $alias) {
+        if ($type == "port" && preg_match("/port/i", $alias['type'])) {
+            $result[] = array("name" => $alias['name']);
+        } elseif ($type != "port" && !preg_match("/port/i", $alias['type'])) {
+            $result[] = array("name" => $alias['name']);
         }
     }
     return $result;
@@ -162,19 +144,14 @@ function get_alias_description($name)
 
     $aliasMdl = new \OPNsense\Firewall\Alias();
     // MVC defined aliases
-    foreach ($aliasMdl->aliases->alias->__items as $alias) {
-        if ((string)$alias->name == $name) {
-            return (string)$alias->description;
-        }
-    }
-    // legacy defined, keep during transition.
-    if (isset($config['aliases']['alias'])) {
-        foreach ($config['aliases']['alias'] as $alias) {
-            if ($alias['name'] == $name) {
-                return (isset($alias['descr'])) ? $alias['descr'] : "";
+    foreach ((new \OPNsense\Firewall\Alias())->aliasIterator()  as $alias) {
+        if ($alias['name'] == $name) {
+            if (isset($alias['descr'])) {
+                return $alias['descr'];
+            } elseif ($alias['description']) {
+                return $alias['description'];
             }
         }
     }
-
     return null;
 }
diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc
index 79877683b..7c1d66b52 100644
--- a/src/etc/inc/util.inc
+++ b/src/etc/inc/util.inc
@@ -519,11 +519,9 @@ function is_ipaddroralias($ipaddr)
     global $config;
 
     if (is_alias($ipaddr)) {
-        if (isset($config['aliases']['alias'])) {
-            foreach ($config['aliases']['alias'] as $alias) {
-                if ($alias['name'] == $ipaddr && !preg_match("/port/i", $alias['type'])) {
-                    return true;
-                }
+        foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $alias) {
+            if ($alias['name'] == $ipaddr && !preg_match("/port/i", $alias['type'])) {
+                return true;
             }
         }
         return false;
@@ -643,11 +641,9 @@ function is_portoralias($port)
     global $config;
 
     if (is_alias($port)) {
-        if (isset($config['aliases']['alias'])) {
-            foreach ($config['aliases']['alias'] as $alias) {
-                if ($alias['name'] == $port && preg_match("/port/i", $alias['type'])) {
-                    return true;
-                }
+        foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $alias) {
+            if ($alias['name'] == $port && preg_match("/port/i", $alias['type'])) {
+                return true;
             }
         }
         return false;
diff --git a/src/opnsense/mvc/app/library/OPNsense/Firewall/Util.php b/src/opnsense/mvc/app/library/OPNsense/Firewall/Util.php
index 51a4aa001..a6dd34404 100644
--- a/src/opnsense/mvc/app/library/OPNsense/Firewall/Util.php
+++ b/src/opnsense/mvc/app/library/OPNsense/Firewall/Util.php
@@ -37,6 +37,11 @@ use \OPNsense\Firewall\Alias;
  */
 class Util
 {
+    /**
+     * @var null|Alias reference to alias object
+     */
+    private static $aliasObject = null;
+
     /**
      * is provided address an ip address.
      * @param string $network address
@@ -72,26 +77,26 @@ class Util
     /**
      * check if name exists in alias config section
      * @param string $name name
+     * @param boolean $valid check if the alias can safely be used
      * @return boolean
      * @throws \OPNsense\Base\ModelException
      */
-    public static function isAlias($name)
+    public static function isAlias($name, $valid=false)
     {
+        if (self::$aliasObject == null) {
+            // Cache the alias object to avoid object creation overhead.
+            self::$aliasObject = new Alias();
+        }
         if (!empty($name)) {
-            $aliasMdl = new Alias();
-            // MVC defined aliases
-            foreach ($aliasMdl->aliases->alias->__items as $alias) {
-                if ((string)$alias->name == $name) {
-                    return true;
-                }
-            }
-            // legacy style aliases
-            $cnf = Config::getInstance()->object();
-            if (!empty($cnf->aliases)) {
-                foreach ($cnf->aliases->children() as $node) {
-                    if ($node->name == $name) {
-                        return true;
+            foreach (self::$aliasObject->aliasIterator() as $alias) {
+                if ($alias['name'] == $name) {
+                    if ($valid) {
+                        // check validity for port type aliases
+                        if (preg_match("/port/i", $alias['type']) && trim($alias['content']) == "") {
+                            return false;
+                        }
                     }
+                    return true;
                 }
             }
         }
diff --git a/src/opnsense/mvc/app/models/OPNsense/Firewall/Alias.php b/src/opnsense/mvc/app/models/OPNsense/Firewall/Alias.php
index bfece476e..86d91d673 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Firewall/Alias.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Firewall/Alias.php
@@ -150,4 +150,32 @@ class Alias extends BaseModel
             }
         }
     }
+
+    /**
+     * return aliases as array
+     * @return Generator with aliases
+     */
+    public function aliasIterator()
+    {
+        $use_legacy = true;
+        foreach ($this->aliases->alias->__items as $alias) {
+            $record = array();
+            foreach (array_keys($alias->__items) as $key) {
+                $record[$key] = (string)$alias->$key;
+            }
+            yield $record;
+            $use_legacy = false;
+        }
+        // MVC not used (yet) return legacy type aliases
+        if ($use_legacy) {
+            $cfgObj = Config::getInstance()->object();
+            if (!empty($cfgObj->aliases->alias)) {
+                foreach ($cfgObj->aliases->children() as $alias) {
+                    $alias = (array)$alias;
+                    $alias['content'] = !empty($alias['address']) ? str_replace(" ", "\n", $alias['address']) : null;
+                    yield $alias;
+                }
+            }
+        }
+    }
 }
diff --git a/src/www/interfaces.php b/src/www/interfaces.php
index 94712f677..bc8ee5a2e 100644
--- a/src/www/interfaces.php
+++ b/src/www/interfaces.php
@@ -580,14 +580,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
 
         $old_ppps = $a_ppps;
 
-        // perform validations
-        if (isset($config['aliases']['alias'])) {
-          foreach($config['aliases']['alias'] as $alias) {
-            if ($alias['name'] == $pconfig['descr']) {
-              $input_errors[] = sprintf(gettext("Sorry, an alias with the name %s already exists."),$pconfig['descr']);
-            }
-          }
-        }
         /* description unique? */
         foreach ($ifdescrs as $ifent => $ifdescr) {
             if ($if != $ifent && $ifdescr == $pconfig['descr']) {
diff --git a/src/www/wizard.php b/src/www/wizard.php
index 2e8b29806..0026d5a3d 100644
--- a/src/www/wizard.php
+++ b/src/www/wizard.php
@@ -941,14 +941,12 @@ function showchange() {
 	$aliases = "";
 	$addrisfirst = 0;
 	$aliasesaddr = "";
-	if (isset($config['aliases']['alias'])) {
-		foreach ($config['aliases']['alias'] as $alias_name) {
-				if ($isfirst == 1) {
-					$aliases .= ",";
-				}
-				$aliases .= "'" . $alias_name['name'] . "'";
-				$isfirst = 1;
-		}
+	foreach ((new \OPNsense\Firewall\Alias())->aliasIterator() as $alias_name) {
+			if ($isfirst == 1) {
+				$aliases .= ",";
+			}
+			$aliases .= "'" . $alias_name['name'] . "'";
+			$isfirst = 1;
 	}
 ?>