From 31282787dfb7de70ae8e5ba4e1c9aa6db43a34d1 Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Fri, 23 Feb 2018 19:01:47 +0100 Subject: [PATCH] Interfaces / reconfigure * remove race conditions in interface_bring_down() so when an old configuration is provided we will actually use the contents of that configuration. * Next make sure we only save the first occurrence of a changed interface until an apply is hit, to prevent lossing the running configuration. * Do some additional cleanups for removing virtual ip's when an interface is requested to go down. For https://github.com/opnsense/core/issues/2221 --- src/etc/inc/interfaces.inc | 111 +++++++++++++++---------------------- src/www/interfaces.php | 30 ++++++---- 2 files changed, 63 insertions(+), 78 deletions(-) diff --git a/src/etc/inc/interfaces.inc b/src/etc/inc/interfaces.inc index bd313c9ff..f07f22679 100644 --- a/src/etc/inc/interfaces.inc +++ b/src/etc/inc/interfaces.inc @@ -1112,47 +1112,51 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) { global $config; - if (!isset($config['interfaces'][$interface])) { + if (!isset($config['interfaces'][$interface]) || ($ifacecfg !== false && !is_array($ifacecfg))) { + // exit on invalid input return; } - $realif = get_real_interface($interface); - $realifv6 = get_real_interface($interface, "inet6"); + if ($ifacecfg === false) { + $realif = get_real_interface($interface); + $realifv6 = get_real_interface($interface, "inet6"); + $ifcfg = $config['interfaces'][$interface]; + $ppps = isset($config['ppps']['ppp']) ? $config['ppps']['ppp'] : array(); + } else { + $ifcfg = $ifacecfg['ifcfg']; + $ppps = $ifacecfg['ppps']; + /* When $ifacecfg is passed, it should contain the original interfaces */ + $realif = $ifacecfg['ifcfg']['realif']; + $realifv6 = $ifacecfg['ifcfg']['realifv6']; + } + $ipaddrv6 = empty($ifcfg['ipaddrv6']) ? null : $ifcfg['ipaddrv6']; if (!in_array($interface, array('openvpn', 'ppp', 'ipsec', 'enc0'))) { /** * NOTE: The $realifv6 is needed when WANv4 is type PPP and v6 is DHCP and the option v6 from v4 is used. * XXX: we kept this in place due to dropping the $realv6iface parameter from get_real_interface(), */ - $cfg = &config_read_array('interfaces', $interface); - $ipaddrv6 = empty($cfg['ipaddrv6']) ? null : $cfg['ipaddrv6']; if (!in_array($ipaddrv6, array('6rd', '6to4', 'pppoe', 'ppp', 'l2tp', 'pptp'))) { - $ipaddr = empty($cfg['ipaddr']) ? null : $cfg['ipaddr']; + $ipaddr = empty($ifcfg['ipaddr']) ? null : $ifcfg['ipaddr']; if (in_array($ipaddr, array('pppoe', 'ppp', 'l2tp', 'pptp'))) { - $parents = get_parent_interface($interface); - $realifv6 = !empty($parents[0]) ? $parents[0] : $cfg['if']; + foreach ($ppps as $ppp) { + if ($ifcfg['if'] == $ppp['if']) { + $ports = explode(',', $ppp['ports']); + foreach ($ports as $pid => $parent_if) { + $realifv6 = $parent_if; + break; + } + } + } } } } - if ($ifacecfg === false) { - $ifcfg = $config['interfaces'][$interface]; - $ppps = array(); - if (isset($config['ppps']['ppp'])) { - $ppps = $config['ppps']['ppp']; - } - } elseif (!is_array($ifacecfg)) { - log_error('Wrong parameters used during interface_bring_down()'); - $ifcfg = $config['interfaces'][$interface]; - $ppps = array(); - if (isset($config['ppps']['ppp'])) { - $ppps = $config['ppps']['ppp']; - } - } else { - $ifcfg = $ifacecfg['ifcfg']; - $ppps = $ifacecfg['ppps']; - if (isset($ifacecfg['ifcfg']['realif'])) { - $realif = $ifacecfg['ifcfg']['realif']; - /* XXX: Any better way? */ - $realifv6 = $realif; + + // force VIPs to go down + if (isset($config['virtualip']['vip'])) { + foreach ($config['virtualip']['vip'] as $vip) { + if ($vip['interface'] == $interface) { + interface_vip_bring_down($vip); + } } } @@ -1161,10 +1165,10 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) case "pppoe": case "pptp": case "l2tp": - if (is_array($ppps) && count($ppps)) { - foreach ($ppps as $pppid => $ppp) { - if ($realif == $ppp['if']) { - if (isset($ppp['ondemand']) && !$destroy) { + if (!empty($ppps)) { + foreach ($ppps as $ppp) { + if ($ifcfg['if'] == $ppp['if']) { + if (isset($ppp['ondemand']) && $ifacecfg === false) { configd_run("interface reconfigure {$interface}"); break; } @@ -1176,6 +1180,7 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) break; } } + return; } break; case "dhcp": @@ -1183,7 +1188,6 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) @unlink("/var/etc/dhclient_{$interface}.conf"); if (does_interface_exist("$realif")) { mwexec("/sbin/ifconfig " . escapeshellarg($realif) . " delete", true); - interface_ipalias_cleanup($interface); if ($destroy) { legacy_interface_flags($realif, 'down'); } @@ -1193,7 +1197,6 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) default: if (does_interface_exist("$realif")) { mwexec("/sbin/ifconfig " . escapeshellarg($realif) . " delete", true); - interface_ipalias_cleanup($interface); if ($destroy) { legacy_interface_flags($realif, 'down'); } @@ -1203,11 +1206,6 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) } $track6 = array(); - if (isset($ifcfg['ipaddrv6'])) { - $ipaddrv6 = $ifcfg['ipaddrv6']; - } else { - $ipaddrv6 = null; - } switch ($ipaddrv6) { case "slaac": case "dhcp6": @@ -1218,7 +1216,6 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) if (is_ipaddrv6($ip6) && $ip6 != "::") { mwexec("/sbin/ifconfig " . escapeshellarg($realifv6) . " inet6 {$ip6} delete", true); } - interface_ipalias_cleanup($interface, "inet6"); if ($destroy) { legacy_interface_flags($realif, 'down'); } @@ -1234,8 +1231,7 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) if (is_ipaddrv6($ip6)) { mwexec("/sbin/ifconfig " . escapeshellarg($realif) . " inet6 {$ip6} delete", true); } - interface_ipalias_cleanup($interface, "inet6"); - if ($destroy) { + if ($ifacecfg !== false) { legacy_interface_flags($realif, 'down'); } } @@ -1250,7 +1246,6 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) if (!empty($ifcfg['ipaddrv6']) && is_ipaddrv6($ifcfg['ipaddrv6'])) { mwexec("/sbin/ifconfig " . escapeshellarg($realif) . " inet6 {$ifcfg['ipaddrv6']} delete", true); } - interface_ipalias_cleanup($interface, "inet6"); if ($destroy) { legacy_interface_flags($realif, 'down'); } @@ -1260,19 +1255,17 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) break; } - if (!empty($track6) && is_array($track6)) { + if (!empty($track6)) { /* Bring down radvd and dhcp6 on these interfaces */ services_dhcpd_configure('inet6', $track6); } - $old_router = ''; if (file_exists("/tmp/{$realif}_router")) { $old_router = trim(file_get_contents("/tmp/{$realif}_router")); - } - - if (!empty($old_router)) { - log_error("Clearing states to old gateway {$old_router}."); - mwexec("/sbin/pfctl -i " . escapeshellarg($realif) . " -Fs"); + if (!empty($old_router)) { + log_error("Clearing states to old gateway {$old_router}."); + mwexec("/sbin/pfctl -i " . escapeshellarg($realif) . " -Fs"); + } } @unlink("/var/db/{$interface}_ip"); @@ -1290,7 +1283,7 @@ function interface_bring_down($interface = "wan", $ifacecfg = false) /* hostapd and wpa_supplicant do not need to be running when the interface is down. * They will also use 100% CPU if running after the wireless clone gets deleted. */ - if (isset($ifcfg['wireless']) && is_array($ifcfg['wireless'])) { + if (!empty($ifcfg['wireless'])) { kill_hostapd($realif); mwexec(kill_wpasupplicant($realif)); } @@ -1762,22 +1755,6 @@ function interface_proxyarp_configure($interface = '') } } -function interface_ipalias_cleanup($interface, $inet = 'inet4') -{ - global $config; - - if (isset($config['virtualip']['vip'])) { - foreach ($config['virtualip']['vip'] as $vip) { - if ($vip['mode'] == 'ipalias' && $vip['interface'] == $interface) { - if ($inet == 'inet6' && is_ipaddrv6($vip['subnet'])) { - interface_vip_bring_down($vip); - } elseif ($inet == 'inet4' && is_ipaddrv4($vip['subnet'])) { - interface_vip_bring_down($vip); - } - } - } - } -} function interfaces_vips_configure($interface = '', $verbose = false) { diff --git a/src/www/interfaces.php b/src/www/interfaces.php index 6a3c5c47b..756c07fbf 100644 --- a/src/www/interfaces.php +++ b/src/www/interfaces.php @@ -511,11 +511,9 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { if (file_exists('/tmp/.interfaces.apply')) { $toapplylist = unserialize(file_get_contents('/tmp/.interfaces.apply')); foreach ($toapplylist as $ifapply => $ifcfgo) { + interface_bring_down($ifapply, $ifcfgo); if (isset($config['interfaces'][$ifapply]['enable'])) { - interface_bring_down($ifapply, $ifcfgo); interface_configure($ifapply, true); - } else { - interface_bring_down($ifapply, $ifcfgo); } } } @@ -557,10 +555,14 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } else { $toapplylist = array(); } - $toapplylist[$if]['ifcfg'] = $a_interfaces[$if]; - $toapplylist[$if]['ppps'] = $a_ppps; - /* we need to be able remove IP aliases for IPv6 */ - file_put_contents('/tmp/.interfaces.apply', serialize($toapplylist)); + if (empty($toapplylist[$if])) { + // only flush if the running config is not in our list yet + $toapplylist[$if]['ifcfg'] = $a_interfaces[$if]; + $toapplylist[$if]['ifcfg']['realif'] = get_real_interface($if); + $toapplylist[$if]['ifcfg']['realifv6'] = get_real_interface($if, "inet6"); + $toapplylist[$if]['ppps'] = $a_ppps; + file_put_contents('/tmp/.interfaces.apply', serialize($toapplylist)); + } header(url_safe('Location: /interfaces.php?if=%s', array($if))); exit; } else { @@ -925,6 +927,9 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { // save form data if (count($input_errors) == 0) { $old_config = $a_interfaces[$if]; + // retrieve our interface names before anything changes + $old_config['realif'] = get_real_interface($if); + $old_config['realifv6'] = get_real_interface($if, "inet6"); $new_config = array(); $new_ppp_config = array(); @@ -1253,6 +1258,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { $new_config['if'] = $a_ppps[$pppid]['ports']; unset($a_ppps[$pppid]); } + // save interface details $a_interfaces[$if] = $new_config; @@ -1276,11 +1282,13 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') { } else { $toapplylist = array(); } - $old_config['realif'] = get_real_interface($if); - $toapplylist[$if]['ifcfg'] = $old_config; - $toapplylist[$if]['ppps'] = $old_ppps; - file_put_contents('/tmp/.interfaces.apply', serialize($toapplylist)); + if (empty($toapplylist[$if])) { + // only flush if the running config is not in our list yet + $toapplylist[$if]['ifcfg'] = $old_config; + $toapplylist[$if]['ppps'] = $old_ppps; + file_put_contents('/tmp/.interfaces.apply', serialize($toapplylist)); + } mark_subsystem_dirty('interfaces');