filter: Gateway Monitoring/Kill states, make sure our factory default…

…s match input and only trigger a state reset using the existing filter_configure_sync() parameter.

o remove <kill_states/> from our default config, since it was evaluated as empty (feature enabled), we might as well remove the option to reach the same effect.
o system_advanced_firewall.php isset() vs !empty(), we use !empty() in our support code, make sure the ui page does the same
o remove hook from filter_configure_sync(), so state resets only happen on request.
o monitor/10-dpinger request conditional state reset

ref https://forum.opnsense.org/index.php?topic=18068.msg82231#msg82231

src/etc/config.xml.sample

@@ -248,7 +248,6 @@
     <bogons>
       <interval>monthly</interval>
     </bogons>
-    <kill_states/>
     <backupcount>60</backupcount>
     <crypto_hardware>aesni</crypto_hardware>
     <pf_share_forward>1</pf_share_forward>

src/etc/inc/filter.inc

@@ -125,26 +125,27 @@ function filter_configure()
     }
 }
 
-function filter_delete_states_for_down_gateways()
+function filter_should_trigger_kill_states()
 {
-    $a_gateways = return_gateways_status();
-    $ifdetails = legacy_interfaces_details();
-    $any_gateway_down = false;
-    foreach ((new \OPNsense\Routing\Gateways($ifdetails))->gatewaysIndexedByName(false, true) as $gateway) {
-        if (empty($gateway['monitor']) || empty($a_gateways[$gateway['name']])) {
-            continue;
-        } elseif (!is_ipaddr($gateway['monitor']) || strstr($gateway['monitor'], '127.0.0.')) {
-            continue;
-        }
-        if (stristr($a_gateways[$gateway['name']]['status'], 'down')) {
-            $any_gateway_down = true;
-            break;
+    global $config;
+    if (empty($config['system']['kill_states'])) {
+        $a_gateways = return_gateways_status();
+        $ifdetails = legacy_interfaces_details();
+        $any_gateway_down = false;
+        foreach ((new \OPNsense\Routing\Gateways($ifdetails))->gatewaysIndexedByName(false, true) as $gateway) {
+            if (empty($gateway['monitor']) || empty($a_gateways[$gateway['name']])) {
+                continue;
+            } elseif (!is_ipaddr($gateway['monitor']) || strstr($gateway['monitor'], '127.0.0.')) {
+                continue;
+            }
+            if (stristr($a_gateways[$gateway['name']]['status'], 'down')) {
+                $any_gateway_down = true;
+                break;
+            }
         }
+        return $any_gateway_down;
     }
-
-    if ($any_gateway_down == true) {
-        mwexec("/sbin/pfctl -Fs");
-    }
+    return false;
 }
 
 /**
@@ -563,10 +564,6 @@ function filter_configure_sync($verbose = false, $flush_states = false, $load_al
         flush();
     }
 
-    if (empty($config['system']['kill_states'])) {
-        filter_delete_states_for_down_gateways();
-    }
-
     if ($verbose) {
         echo '.';
         flush();

src/etc/rc.filter_configure

@@ -33,8 +33,20 @@ require_once("filter.inc");
 require_once("system.inc");
 require_once("interfaces.inc");
 
-if (count($argv) >= 1 && $argv[1] == 'skip_alias' ) {
+$event_arg = count($argv) >= 1 ? $argv[1] : "";
+
+if ($event_arg == 'skip_alias' ) {
     filter_configure_sync(true, false, false);
 } else {
-    filter_configure_sync(true);
+    if ($event_arg == 'gateway' && filter_should_trigger_kill_states()) {
+        /**
+         * XXX: When "Kill states" on gateway monitoring is used, we should reset states after this event.
+         *      Originally filter_configure_sync() reset states after each event. Although the feature isn't great
+         *      in terms of what it does (not really fine grained, just kill all), for historic reasons we probably should
+         *      leave it in.
+         */
+        filter_configure_sync(true, true);
+    } else {
+        filter_configure_sync(true);
+    }
 }

src/etc/rc.syshook.d/monitor/10-dpinger

@@ -35,6 +35,6 @@ fi
 /usr/bin/logger -t dpinger "GATEWAY ALARM: ${GATEWAY} (Addr: ${2} Alarm: ${3} RTT: ${4}ms RTTd: ${5}ms Loss: ${6}%)"
 
 echo -n "Reloading filter: "
-configctl filter reload
+configctl filter reload gateway
 
 exit 0

src/www/system_advanced_firewall.php

@@ -51,7 +51,7 @@
     $pconfig['disablereplyto'] = isset($config['system']['disablereplyto']);
     $pconfig['bogonsinterval'] = !empty($config['system']['bogons']['interval']) ? $config['system']['bogons']['interval'] : null;
     $pconfig['schedule_states'] = isset($config['system']['schedule_states']);
-    $pconfig['kill_states'] = isset($config['system']['kill_states']);
+    $pconfig['kill_states'] = !empty($config['system']['kill_states']);
     $pconfig['skip_rules_gw_down'] = isset($config['system']['skip_rules_gw_down']);
     $pconfig['lb_use_sticky'] = isset($config['system']['lb_use_sticky']);
     $pconfig['pf_share_forward'] = isset($config['system']['pf_share_forward']);