From 868a74e058b3fa09c7315c2cd21ede589def750a Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Fri, 4 Apr 2025 10:58:35 +0200
Subject: [PATCH] interfaces: cleanup spurious functions regarding VIP access

We do this for different reasons, but mainly for code maintenance
and simplicity.  It also fixes a few aquire/display bugs with
overlapping linl-local VIPs across interfaces, but not all.
---
 src/etc/inc/interfaces.inc                    | 98 +++++++------------
 src/etc/inc/util.inc                          | 27 -----
 .../app/models/OPNsense/IPsec/Menu/Menu.xml   |  2 +-
 src/www/vpn_ipsec_phase1.php                  | 18 ++--
 src/www/vpn_openvpn_client.php                | 17 ++--
 src/www/vpn_openvpn_server.php                | 30 ++++--
 6 files changed, 79 insertions(+), 113 deletions(-)

diff --git a/src/etc/inc/interfaces.inc b/src/etc/inc/interfaces.inc
index 893f2a755..4671bdd5f 100644
--- a/src/etc/inc/interfaces.inc
+++ b/src/etc/inc/interfaces.inc
@@ -3679,39 +3679,34 @@ function link_interface_to_bridge($interface, $attach_device = null, $ifconfig_d
 
 function link_interface_to_gre($interface, $update = false, $family = null)
 {
-    global $config;
-
-    $aliaslist = get_configured_ip_aliases_list();
     $result = [];
 
-    if (isset($config['gres']['gre'])) {
-        foreach ($config['gres']['gre'] as $gre) {
-            $parent = explode('_vip', $gre['if'])[0];
-            if (is_ipaddr($parent)) {
-                foreach ($aliaslist as $ip => $int) {
-                    if ($ip == $parent) {
-                        $parent = $int;
-                        break;
-                    }
+    foreach (config_read_array('gres', 'gre') as $gre) {
+        $parent = explode('_vip', $gre['if'])[0];
+        if (is_ipaddr($parent)) {
+            foreach (config_read_array('virtualip', 'vip') as $vip) {
+                if ($vip['mode'] == 'ipalias' && $vip['subnet'] == $parent) {
+                    $parent = $vip['interface'];
+                    break;
                 }
             }
-
-            if ($parent != $interface) {
-                continue;
-            } elseif ($family == 4 && !is_ipaddrv4($gre['remote-addr'])) {
-                continue;
-            } elseif ($family == 6 && !is_ipaddrv6($gre['remote-addr'])) {
-                continue;
-            }
-
-            if ($update && empty(_interfaces_gre_configure($gre))) {
-                /* only return the ones that did configure correctly */
-                continue;
-            }
-
-            /* callers are only concerned with the resulting device names */
-            $result[] = $gre['greif'];
         }
+
+        if ($parent != $interface) {
+            continue;
+        } elseif ($family == 4 && !is_ipaddrv4($gre['remote-addr'])) {
+            continue;
+        } elseif ($family == 6 && !is_ipaddrv6($gre['remote-addr'])) {
+            continue;
+        }
+
+        if ($update && empty(_interfaces_gre_configure($gre))) {
+            /* only return the ones that did configure correctly */
+            continue;
+        }
+
+        /* callers are only concerned with the resulting device names */
+        $result[] = $gre['greif'];
     }
 
     return $result;
@@ -3719,28 +3714,24 @@ function link_interface_to_gre($interface, $update = false, $family = null)
 
 function link_interface_to_gif($interface, $update = false, $family = null)
 {
-    global $config;
-
     $result = [];
 
-    if (isset($config['gifs']['gif'])) {
-        foreach ($config['gifs']['gif'] as $gif) {
-            if (explode('_vip', $gif['if'])[0] != $interface) {
-                continue;
-            } elseif ($family == 4 && !is_ipaddrv4($gif['remote-addr'])) {
-                continue;
-            } elseif ($family == 6 && !is_ipaddrv6($gif['remote-addr'])) {
-                continue;
-            }
-
-            if ($update && empty(_interfaces_gif_configure($gif))) {
-                /* only return the ones that did configure correctly */
-                continue;
-            }
-
-            /* callers are only concerned with the resulting device names */
-            $result[] = $gif['gifif'];
+    foreach (config_read_array('gifs', 'gif') as $gif) {
+        if (explode('_vip', $gif['if'])[0] != $interface) {
+            continue;
+        } elseif ($family == 4 && !is_ipaddrv4($gif['remote-addr'])) {
+            continue;
+        } elseif ($family == 6 && !is_ipaddrv6($gif['remote-addr'])) {
+            continue;
         }
+
+        if ($update && empty(_interfaces_gif_configure($gif))) {
+            /* only return the ones that did configure correctly */
+            continue;
+        }
+
+        /* callers are only concerned with the resulting device names */
+        $result[] = $gif['gifif'];
     }
 
     return $result;
@@ -3838,19 +3829,6 @@ function get_interface_mac($interface, $ifconfig_details = null)
     return $intf_details['macaddr'];
 }
 
-function get_vip_descr($ipaddress)
-{
-    global $config;
-
-    foreach ($config['virtualip']['vip'] as $vip) {
-        if ($vip['subnet'] == $ipaddress) {
-            return ($vip['descr'] ?? '');
-        }
-    }
-
-    return '';
-}
-
 function interfaces_staticarp_configure($if, $ifconfig_details = null)
 {
     global $config;
diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc
index d095c864a..b3b446a4e 100644
--- a/src/etc/inc/util.inc
+++ b/src/etc/inc/util.inc
@@ -32,7 +32,6 @@
 
 require_once 'IPv6.inc';
 
-/* XXX only two callers left, better remove this unreliable function */
 function killbyname($procname, $sig = 'TERM', $waitforit = true)
 {
     /* pgrep -n only kills the newest matching process */
@@ -802,32 +801,6 @@ function is_inrange($test, $start, $end)
     return is_ipaddrv6($test) ? is_inrange_v6($test, $start, $end) : is_inrange_v4($test, $start, $end);
 }
 
-function get_configured_carp_interface_list()
-{
-    $carp_list = [];
-
-    foreach (config_read_array('virtualip', 'vip') as $vip) {
-        if ($vip['mode'] == 'carp') {
-            $carp_list["{$vip['interface']}_vip{$vip['vhid']}"] = $vip['subnet'];
-        }
-    }
-
-    return $carp_list;
-}
-
-function get_configured_ip_aliases_list()
-{
-    $alias_list = [];
-
-    foreach (config_read_array('virtualip', 'vip') as $vip) {
-        if ($vip['mode'] == 'ipalias') {
-            $alias_list[$vip['subnet']] = $vip['interface'];
-        }
-    }
-
-    return $alias_list;
-}
-
 function get_configured_interface_with_descr()
 {
     $iflist = [];
diff --git a/src/opnsense/mvc/app/models/OPNsense/IPsec/Menu/Menu.xml b/src/opnsense/mvc/app/models/OPNsense/IPsec/Menu/Menu.xml
index 54f7415b0..7d959fd98 100644
--- a/src/opnsense/mvc/app/models/OPNsense/IPsec/Menu/Menu.xml
+++ b/src/opnsense/mvc/app/models/OPNsense/IPsec/Menu/Menu.xml
@@ -6,7 +6,7 @@
                 <Phase1 url="/vpn_ipsec_phase1.php*" visibility="hidden"/>
                 <Phase2 url="/vpn_ipsec_phase2.php*" visibility="hidden"/>
             </Tunnels>
-            <Mobile order="20" VisibleName="Mobile Clients (legacy)" url="/vpn_ipsec_mobile.php">
+            <Mobile order="20" VisibleName="Mobile Clients [legacy]" url="/vpn_ipsec_mobile.php">
                 <Act url="/vpn_ipsec_mobile.php*" visibility="hidden"/>
             </Mobile>
             <Keys order="30" VisibleName="Pre-Shared Keys" url="/ui/ipsec/pre_shared_keys"/>
diff --git a/src/www/vpn_ipsec_phase1.php b/src/www/vpn_ipsec_phase1.php
index ed063d7dc..c938089a4 100644
--- a/src/www/vpn_ipsec_phase1.php
+++ b/src/www/vpn_ipsec_phase1.php
@@ -721,14 +721,16 @@ include("head.inc");
                       <select name="interface">
 <?php
                       $interfaces = get_configured_interface_with_descr();
-                      $carplist = get_configured_carp_interface_list();
-                      foreach ($carplist as $cif => $carpip) {
-                          $interfaces[$cif] = $carpip." (".get_vip_descr($carpip).")";
-                      }
-
-                      $aliaslist = get_configured_ip_aliases_list();
-                      foreach ($aliaslist as $aliasip => $aliasif) {
-                          $interfaces[$aliasip] = $aliasip." (".get_vip_descr($aliasip).")";
+                      foreach (config_read_array('virtualip', 'vip') as $vip) {
+                          $label = $vip['subnet'] . (empty($vip['descr']) ? '' : " (${vip['descr']})");
+                          if ($vip['mode'] == 'carp') {
+                              $value = "{$vip['interface']}_vip{$vip['vhid']}";
+                          } elseif ($vip['mode'] == 'ipalias') {
+                              $value = $vip['subnet'];
+                          } else {
+                              continue;
+                          }
+                          $interfaces[$value] = $label;
                       }
                       foreach ($interfaces as $iface => $ifacename) :
 ?>
diff --git a/src/www/vpn_openvpn_client.php b/src/www/vpn_openvpn_client.php
index 85c313bfe..1f07f2469 100644
--- a/src/www/vpn_openvpn_client.php
+++ b/src/www/vpn_openvpn_client.php
@@ -639,13 +639,16 @@ $( document ).ready(function() {
               <select name="interface" class="form-control">
 <?php
               $interfaces = get_configured_interface_with_descr();
-              $carplist = get_configured_carp_interface_list();
-              foreach ($carplist as $cif => $carpip) {
-                  $interfaces[$cif.'|'.$carpip] = $carpip." (".get_vip_descr($carpip).")";
-              }
-              $aliaslist = get_configured_ip_aliases_list();
-              foreach ($aliaslist as $aliasip => $aliasif) {
-                  $interfaces[$aliasif.'|'.$aliasip] = $aliasip." (".get_vip_descr($aliasip).")";
+              foreach (config_read_array('virtualip', 'vip') as $vip) {
+                  $label = $vip['subnet'] . (empty($vip['descr']) ? '' : " (${vip['descr']})");
+                  if ($vip['mode'] == 'carp') {
+                      $value = "{$vip['interface']}_vip{$vip['vhid']}";
+                  } elseif ($vip['mode'] == 'ipalias') {
+                      $value = $vip['interface'];
+                  } else {
+                      continue;
+                  }
+                  $interfaces["{$value}|{$vip['subnet']}"] = $label;
               }
               $interfaces['lo0'] = "Localhost";
               $interfaces['any'] = "any";
diff --git a/src/www/vpn_openvpn_server.php b/src/www/vpn_openvpn_server.php
index 2cea2ca95..0719c4906 100644
--- a/src/www/vpn_openvpn_server.php
+++ b/src/www/vpn_openvpn_server.php
@@ -805,11 +805,16 @@ $( document ).ready(function() {
                         <select name="interface" class="selectpicker" data-size="5" data-live-search="true">
 <?php
                         $interfaces = get_configured_interface_with_descr();
-                        foreach (get_configured_carp_interface_list() as $cif => $carpip) {
-                            $interfaces[$cif.'|'.$carpip] = $carpip." (".get_vip_descr($carpip).")";
-                        }
-                        foreach (get_configured_ip_aliases_list() as $aliasip => $aliasif) {
-                            $interfaces[$aliasif.'|'.$aliasip] = $aliasip." (".get_vip_descr($aliasip).")";
+                        foreach (config_read_array('virtualip', 'vip') as $vip) {
+                            $label = $vip['subnet'] . (empty($vip['descr']) ? '' : " (${vip['descr']})");
+                            if ($vip['mode'] == 'carp') {
+                                $value = "{$vip['interface']}_vip{$vip['vhid']}";
+                            } elseif ($vip['mode'] == 'ipalias') {
+                                $value = $vip['interface'];
+                            } else {
+                                continue;
+                            }
+                            $interfaces["{$value}|{$vip['subnet']}"] = $label;
                         }
                         $interfaces['lo0'] = "Localhost";
                         $interfaces['any'] = "any";
@@ -1140,11 +1145,16 @@ $( document ).ready(function() {
 <?php
                         $serverbridge_interface['none'] = "none";
                         $serverbridge_interface = array_merge($serverbridge_interface, get_configured_interface_with_descr());
-                        foreach (get_configured_carp_interface_list() as $cif => $carpip) {
-                            $serverbridge_interface[$cif.'|'.$carpip] = $carpip." (".get_vip_descr($carpip).")";
-                        }
-                        foreach (get_configured_ip_aliases_list() as $aliasip => $aliasif) {
-                            $serverbridge_interface[$aliasif.'|'.$aliasip] = $aliasip." (".get_vip_descr($aliasip).")";
+                        foreach (config_read_array('virtualip', 'vip') as $vip) {
+                            $label = $vip['subnet'] . (empty($vip['descr']) ? '' : " (${vip['descr']})");
+                            if ($vip['mode'] == 'carp') {
+                                $value = "{$vip['interface']}_vip{$vip['vhid']}";
+                            } elseif ($vip['mode'] == 'ipalias') {
+                                $value = $vip['interface'];
+                            } else {
+                                continue;
+                            }
+                            $serverbridge_interface["{$value}|{$vip['subnet']}"] = $label;
                         }
                         foreach ($serverbridge_interface as $iface => $ifacename) :
                           $selected = "";