From 500c82f1817f96a04eb6f9185a174e1dc22df5fb Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Wed, 22 Sep 2021 15:37:55 +0200 Subject: [PATCH] Firewall - refactor getInterfaceGateway() to support extracting a dynamic property instead of the fixed address, refactor route-to behaviour to match reply-to and outbound nat. remove getInterfaceGateways() from firewall plugin as being unused now. closes https://github.com/opnsense/core/issues/5230 --- src/etc/inc/filter.lib.inc | 48 +++++++++---------- src/etc/inc/plugins.inc.d/ipsec.inc | 13 +---- .../app/library/OPNsense/Firewall/Plugin.php | 19 -------- .../app/library/OPNsense/Routing/Gateways.php | 4 +- 4 files changed, 25 insertions(+), 59 deletions(-) diff --git a/src/etc/inc/filter.lib.inc b/src/etc/inc/filter.lib.inc index 91cb90fa9..d9a3aa68b 100644 --- a/src/etc/inc/filter.lib.inc +++ b/src/etc/inc/filter.lib.inc @@ -594,33 +594,29 @@ function filter_core_rules_system($fw, $defaults) if (empty($config['system']['pf_disable_force_gw'])) { foreach ($fw->getInterfaceMapping() as $ifdescr => $ifcfg) { if (!isset($ifcfg['internal_dynamic']) && $ifcfg['if'] != 'lo0') { - $intf_has_v4 = false; - $intf_has_v6 = false; + $protos_found = []; + $address_check = ['inet' => 'is_subnetv4', 'inet6' => 'is_subnetv6']; foreach (array_keys(interfaces_addresses($ifcfg['if'], true, $fw->getIfconfigDetails())) as $addr) { - $intf_has_v4 = $intf_has_v4 || is_subnetv4($addr); - $intf_has_v6 = $intf_has_v6 || is_subnetv6($addr); - } - foreach ($fw->getInterfaceGateways($ifcfg['if']) as $gwname) { - $gwcfg = $fw->getGateway($gwname); - if (empty($gwcfg['gateway'])) { - // XXX: only force traffic on explict gateway [address] selection, - // exclude gateway_interface types. - continue; - } - // only try to add gateway rules for traffic leaving this interface - // when the correct protocol is assigned to the interface - if (($gwcfg['proto'] == 'inet' && $intf_has_v4) || ($gwcfg['proto'] == 'inet6' && $intf_has_v6)) { - $fw->registerFilterRule( - 100000, - array('from' => "({$ifcfg['if']})", 'direction' => 'out', 'gateway' => $gwname, - 'destination' => array('network' => $ifdescr, "not" => true), - 'statetype' => 'keep', - 'allowopts' => true, - 'quick' => false, - '#ref' => 'system_advanced_firewall.php#pf_disable_force_gw', - 'descr' => "let out anything from firewall host itself (force gw)"), - $defaults['pass'] - ); + foreach (['inet', 'inet6'] as $inet) { + if (!in_array($inet, $protos_found) && $address_check[$inet]($addr)) { + $gwname = $fw->getGateways()->getInterfaceGateway($ifdescr, $inet, true, 'name'); + if (!empty($gwname)) { + // only try to add gateway rules for traffic leaving this interface + // when the correct protocol is assigned to the interface + $fw->registerFilterRule( + 100000, + array('from' => "({$ifcfg['if']})", 'direction' => 'out', 'gateway' => $gwname, + 'destination' => array('network' => $ifdescr, "not" => true), + 'statetype' => 'keep', + 'allowopts' => true, + 'quick' => false, + '#ref' => 'system_advanced_firewall.php#pf_disable_force_gw', + 'descr' => "let out anything from firewall host itself (force gw)"), + $defaults['pass'] + ); + } + $protos_found[] = $inet; + } } } } diff --git a/src/etc/inc/plugins.inc.d/ipsec.inc b/src/etc/inc/plugins.inc.d/ipsec.inc index 2fe1dbb6c..eda4c999a 100644 --- a/src/etc/inc/plugins.inc.d/ipsec.inc +++ b/src/etc/inc/plugins.inc.d/ipsec.inc @@ -268,18 +268,7 @@ function ipsec_firewall(\OPNsense\Firewall\Plugin $fw) ); // find gateway - $gwname = null; - foreach ($fw->getInterfaceMapping() as $intfnm => $intf) { - if ($intfnm == $interface) { - foreach ($fw->getInterfaceGateways($intf['if']) as $gwnm) { - $gw = $fw->getGateway($gwnm); - if ($gw['proto'] == $ph1ent['protocol']) { - $gwname = $gwnm; - break; - } - } - } - } + $gwname = $fw->getGateways()->getInterfaceGateway($interface, $ph1ent['protocol'], true, 'name'); // register rules $fw->registerFilterRule( 500000, diff --git a/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php b/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php index 51ecec4d5..a7d491259 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php +++ b/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php @@ -160,25 +160,6 @@ class Plugin } } - /** - * fetch gateway (names) for provided interface, would return both ipv4/ipv6 - * @param string $intf interface (e.g. em0, igb0,...) - */ - public function getInterfaceGateways($intf) - { - $result = array(); - $protos_found = array(); - foreach ($this->gatewayMapping as $key => $gw) { - if ($gw['type'] == 'gateway' && $gw['interface'] == $intf) { - if (!in_array($gw['proto'], $protos_found)) { - $result[] = $key; - $protos_found[] = $gw['proto']; - } - } - } - return $result; - } - /** * Fetch gateway * @param string $gw gateway name diff --git a/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php b/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php index fe678e525..64a2dc481 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php +++ b/src/opnsense/mvc/app/library/OPNsense/Routing/Gateways.php @@ -403,7 +403,7 @@ class Gateways * @param boolean $only_configured only return configured in interface or dynamic gateways * @return string|null gateway address */ - public function getInterfaceGateway($interface, $ipproto = "inet", $only_configured = false) + public function getInterfaceGateway($interface, $ipproto = "inet", $only_configured = false, $property='gateway') { foreach ($this->getGateways() as $gateway) { if (!empty($gateway['disabled']) || $gateway['ipprotocol'] != $ipproto) { @@ -424,7 +424,7 @@ class Gateways // are not returned as valid gateway address (automatic outbound nat rules). // An alternative setup option would be practical here, less fuzzy. if (!$only_configured || $intf_gateway == $gateway['name'] || !empty($gateway['dynamic'])) { - return $gateway['gateway']; + return isset($gateway[$property]) ? $gateway[$property] : null ; } } }