From 3bd36b5624d0b1d6ce014e0eddd8e8ffca510afb Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Tue, 17 Aug 2021 12:30:42 +0200 Subject: [PATCH] Interfaces - uniform test if interface is already assigned somewhere using new is_interface_assigned() funciton in utils.inc, implement check in openvpn client/server while here. closes https://github.com/opnsense/core/issues/5163 --- src/etc/inc/util.inc | 25 +++++++++++++++++++ src/www/interfaces_bridge.php | 11 +-------- src/www/interfaces_gif.php | 11 +-------- src/www/interfaces_gre.php | 2 +- src/www/interfaces_lagg.php | 2 +- src/www/interfaces_vlan.php | 20 ++------------- src/www/interfaces_wireless.php | 15 +---------- src/www/vpn_openvpn_client.php | 44 +++++++++++++++++++++++++++------ src/www/vpn_openvpn_server.php | 31 ++++++++++++++++++++--- 9 files changed, 97 insertions(+), 64 deletions(-) diff --git a/src/etc/inc/util.inc b/src/etc/inc/util.inc index 93c9953b9..b559722a5 100644 --- a/src/etc/inc/util.inc +++ b/src/etc/inc/util.inc @@ -1514,3 +1514,28 @@ function get_dyndns_ip($int, $ipver = 4) return $ip_address; } + + +/** + * check if interface is assigned + * @param $interface technical interface name + * @return string interface name (lan, wan, optX) + */ +function is_interface_assigned($interface) +{ + global $config; + + foreach (legacy_config_get_interfaces() as $if => $intf) { + if (isset($intf['if']) && $intf['if'] == $interface) { + return true; + } + } + if (isset($config['vlans']['vlan'])) { + foreach ($config['vlans']['vlan'] as $vlan) { + if($vlan['if'] == $interface) { + return true; + } + } + } + return false; +} diff --git a/src/www/interfaces_bridge.php b/src/www/interfaces_bridge.php index 15a7e0f6a..0565c33ea 100644 --- a/src/www/interfaces_bridge.php +++ b/src/www/interfaces_bridge.php @@ -32,15 +32,6 @@ require_once("interfaces.inc"); $a_bridges = &config_read_array('bridges', 'bridged') ; -function bridge_inuse($bridge_if) { - foreach (legacy_config_get_interfaces() as $if => $intf) { - if ($intf['if'] == $bridge_if) { - return true; - } - } - return false; -} - if ($_SERVER['REQUEST_METHOD'] === 'POST') { $input_errors = array(); if (!empty($a_bridges[$_POST['id']])) { @@ -48,7 +39,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } if (!empty($_POST['action']) && $_POST['action'] == "del" && isset($id)) { - if (bridge_inuse($a_bridges[$id]['bridgeif'])) { + if (is_interface_assigned($a_bridges[$id]['bridgeif'])) { $input_errors[] = gettext("This bridge cannot be deleted because it is assigned as an interface."); } else { if (!does_interface_exist($a_bridges[$id]['bridgeif'])) { diff --git a/src/www/interfaces_gif.php b/src/www/interfaces_gif.php index 81a76aa6a..62f199f00 100644 --- a/src/www/interfaces_gif.php +++ b/src/www/interfaces_gif.php @@ -30,15 +30,6 @@ require_once("guiconfig.inc"); require_once("interfaces.inc"); -function gif_inuse($gif_intf) { - foreach (legacy_config_get_interfaces() as $if => $intf) { - if ($intf['if'] == $gif_intf) { - return true; - } - } - return false; -} - $a_gifs = &config_read_array('gifs', 'gif') ; if ($_SERVER['REQUEST_METHOD'] === 'POST') { @@ -48,7 +39,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } if (!empty($_POST['action']) && $_POST['action'] == "del" && isset($id)) { - if (gif_inuse($a_gifs[$id]['gifif'])) { + if (is_interface_assigned($a_gifs[$id]['gifif'])) { $input_errors[] = gettext("This gif TUNNEL cannot be deleted because it is still being used as an interface."); } else { mwexec("/sbin/ifconfig " . escapeshellarg($a_gifs[$id]['gifif']) . " destroy"); diff --git a/src/www/interfaces_gre.php b/src/www/interfaces_gre.php index 368559ed6..200286082 100644 --- a/src/www/interfaces_gre.php +++ b/src/www/interfaces_gre.php @@ -49,7 +49,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } if (!empty($_POST['action']) && $_POST['action'] == "del" && isset($id)) { - if (gre_inuse($a_gres[$id]['greif'])) { + if (is_interface_assigned($a_gres[$id]['greif'])) { $input_errors[] = gettext("This GRE tunnel cannot be deleted because it is still being used as an interface."); } else { mwexec("/sbin/ifconfig " . escapeshellarg($a_gres[$id]['greif']) . " destroy"); diff --git a/src/www/interfaces_lagg.php b/src/www/interfaces_lagg.php index a48672027..d7c130ba8 100644 --- a/src/www/interfaces_lagg.php +++ b/src/www/interfaces_lagg.php @@ -59,7 +59,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } if (!empty($_POST['action']) && $_POST['action'] == "del" && isset($id)) { - if (lagg_inuse($a_laggs[$id]['laggif'])) { + if (is_interface_assigned($a_laggs[$id]['laggif'])) { $input_errors[] = gettext("This LAGG interface cannot be deleted because it is still being used."); } else { mwexecf('/sbin/ifconfig %s destroy', $a_laggs[$id]['laggif']); diff --git a/src/www/interfaces_vlan.php b/src/www/interfaces_vlan.php index 32da3998d..991b20f99 100644 --- a/src/www/interfaces_vlan.php +++ b/src/www/interfaces_vlan.php @@ -30,19 +30,6 @@ require_once("guiconfig.inc"); require_once("interfaces.inc"); -function vlan_inuse($vlan_intf) -{ - global $config; - - foreach ($config['interfaces'] as $if => $intf) { - if ($intf['if'] == $vlan_intf) { - return $if; - } - } - - return false; -} - $a_vlans = &config_read_array('vlans', 'vlan'); if ($_SERVER['REQUEST_METHOD'] === 'POST') { @@ -52,11 +39,8 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } if (!empty($_POST['action']) && $_POST['action'] == "del" && isset($id)) { - if (($ifid = vlan_inuse($a_vlans[$id]['vlanif'])) !== false) { - $ifdescr = empty($config['interfaces'][$ifid]['descr']) ? $ifid : $config['interfaces'][$ifid]['descr']; - $input_errors[] = sprintf( - gettext("This VLAN cannot be deleted because it is still being used as an interface (%s).") - , $ifdescr); + if (is_interface_assigned($a_vlans[$id]['vlanif'])) { + $input_errors[] = gettext("This VLAN cannot be deleted because it is assigned as an interface."); } else { if (does_interface_exist($a_vlans[$id]['vlanif'])) { legacy_interface_destroy($a_vlans[$id]['vlanif']); diff --git a/src/www/interfaces_wireless.php b/src/www/interfaces_wireless.php index c210dc217..da14247d1 100644 --- a/src/www/interfaces_wireless.php +++ b/src/www/interfaces_wireless.php @@ -29,25 +29,12 @@ require_once("guiconfig.inc"); -function clone_inuse($cloneif) -{ - global $config; - - foreach (array_keys(legacy_config_get_interfaces(['virtual' => false])) as $if) { - if ($config['interfaces'][$if]['if'] == $cloneif) { - return true; - } - } - - return false; -} - $a_clones = &config_read_array('wireless', 'clone'); if ($_SERVER['REQUEST_METHOD'] === 'POST') { $input_errors = array(); if (!empty($_POST['action']) && $_POST['action'] == "del" && !empty($a_clones[$_POST['id']])) { - if (clone_inuse($a_clones[$_POST['id']]['cloneif'])) { + if (is_interface_assigned($a_clones[$_POST['id']]['cloneif'])) { /* check if still in use */ $input_errors[] = gettext("This wireless clone cannot be deleted because it is assigned as an interface."); } else { diff --git a/src/www/vpn_openvpn_client.php b/src/www/vpn_openvpn_client.php index c659e35c1..df710f59a 100644 --- a/src/www/vpn_openvpn_client.php +++ b/src/www/vpn_openvpn_client.php @@ -125,17 +125,29 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { if ($act == "del") { // remove client + $response = ["status" => "failed", "message" => gettext("not found")]; if (isset($id)) { - openvpn_delete('client', $a_client[$id]); - unset($a_client[$id]); - write_config(); + $vpn_id = !empty($a_client[$id]) ? $a_client[$id]['vpnid'] : null; + if ($vpn_id !== null && is_interface_assigned("ovpnc{$vpn_id}")) { + $response = [ + "status" => "failed", + "message" => gettext("This tunnel cannot be deleted because it is still being used as an interface.") + ]; + } elseif ($vpn_id !== null) { + openvpn_delete('client', $a_client[$id]); + unset($a_client[$id]); + write_config(); + $response = ["status" => "ok"]; + } } - header(url_safe('Location: /vpn_openvpn_client.php')); + echo json_encode($response); exit; } elseif ($act == "del_x") { if (!empty($pconfig['rule']) && is_array($pconfig['rule'])) { foreach ($pconfig['rule'] as $rulei) { - if (isset($a_client[$rulei])) { + $vpn_id = !empty($a_client[$rulei]) ? $a_client[$rulei]['vpnid'] : null; + // XXX: silently ignore entries that can't be removed, no clean option to pass messages in form result + if ($vpn_id !== null && !is_interface_assigned("ovpnc{$vpn_id}")) { openvpn_delete('client', $a_client[$rulei]); unset($a_client[$rulei]); } @@ -387,8 +399,26 @@ $( document ).ready(function() { label: "", action: function(dialogRef) { $.post(window.location, {act: 'del', id:id}, function(data) { - location.reload(); - }); + if (data.status == 'failed' && data.message !== undefined) { + dialogRef.close(); + BootstrapDialog.show({ + type:BootstrapDialog.TYPE_DANGER, + title: "", + message: data.message, + buttons: [ + { + label: "", + action: function(dialogRef) { + dialogRef.close(); + } + } + ] + }); + return; + } else { + location.reload(); + } + }, 'json'); dialogRef.close(); } }] diff --git a/src/www/vpn_openvpn_server.php b/src/www/vpn_openvpn_server.php index fb17041b9..c6d0fc414 100644 --- a/src/www/vpn_openvpn_server.php +++ b/src/www/vpn_openvpn_server.php @@ -135,12 +135,19 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { if ($act == "del") { // action delete - if (isset($a_server[$id])) { + $vpn_id = !empty($a_server[$id]) ? $a_server[$id]['vpnid'] : null; + if ($vpn_id !== null && is_interface_assigned("ovpns{$vpn_id}")) { + $response = [ + "status" => "failed", + "message" => gettext("This tunnel cannot be deleted because it is still being used as an interface.") + ]; + } elseif ($vpn_id !== null) { openvpn_delete('server', $a_server[$id]); unset($a_server[$id]); write_config(); + $response = ["status" => "ok"]; } - header(url_safe('Location: /vpn_openvpn_server.php')); + echo json_encode($response); exit; } elseif ($act == "toggle") { if (isset($id)) { @@ -461,8 +468,26 @@ $( document ).ready(function() { label: "", action: function(dialogRef) { $.post(window.location, {act: 'del', id:id}, function(data) { + if (data.status == 'failed' && data.message !== undefined) { + dialogRef.close(); + BootstrapDialog.show({ + type:BootstrapDialog.TYPE_DANGER, + title: "", + message: data.message, + buttons: [ + { + label: "", + action: function(dialogRef) { + dialogRef.close(); + } + } + ] + }); + return; + } else { location.reload(); - }); + } + }, 'json'); dialogRef.close(); } }]