From a5fed616a50d2e6f09e53da03b98cc388cb3a12c Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Sun, 1 Feb 2026 13:21:26 +0100 Subject: [PATCH] Firewall: Schedule - add missing schedules support in "Firewall: Rules [new]" and refactor existing usage to avoid duplication of logic. closes https://github.com/opnsense/core/issues/9690 This commit moves the schedule logic out of filter_core_rules_user() where it didn't belong in the first place. Since we need legacy code to determine schedule behavior, we cannot move it to the plugin classes easily, instead sweep all registered rules after registration so we can process "sched" for all of them in the same way. We can next add a simple action into the model to ask if there actually is a schedule, which pf_cron() needs to schedule the rule updates. Finally add an icon and link into the mvc page to refer to the schedule itself. --- src/etc/inc/filter.inc | 31 +++++++++++++++++-- src/etc/inc/filter.lib.inc | 25 +-------------- src/etc/inc/plugins.inc.d/pf.inc | 21 +++++-------- .../Firewall/forms/dialogFilterRule.xml | 1 + .../app/library/OPNsense/Firewall/Rule.php | 10 ++++++ .../app/models/OPNsense/Firewall/Filter.php | 18 +++++++++++ .../views/OPNsense/Firewall/filter_rule.volt | 11 +++++++ 7 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/etc/inc/filter.inc b/src/etc/inc/filter.inc index 54a0b8f4b8..dd4602971d 100644 --- a/src/etc/inc/filter.inc +++ b/src/etc/inc/filter.inc @@ -187,8 +187,8 @@ function filter_configure_sync($verbose = false, $load_aliases = true) filter_core_bootstrap($fw); $cnfint = iterator_to_array($fw->getInterfaceMapping()); plugins_firewall($fw); - // register user rules, returns kill states for schedules - $sched_kill_states = filter_core_rules_user($fw); + // register legacy user rules + filter_core_rules_user($fw); // manual outbound nat rules if ( @@ -256,6 +256,33 @@ function filter_configure_sync($verbose = false, $load_aliases = true) foreach ((new OPNsense\Firewall\DNat())->rule->sortedBy(['sequence']) as $key => $rule) { $fw->registerForwardRule(600, $rule->getNodeContent()); } + /** + * XXX: Apply schedules on all installed rules so we can use both legacy and MVC ones. + * Eventually this needs to be replaced, but as this requires legacy code, we cannot do that easily now + * without increasing impact. + */ + $sched_kill_states = []; + foreach ($fw->iterateFilterRules() as $rule) { + $rrule = $rule->getRawRule(); + if (!empty($rrule['sched'])) { + $descr = $rrule['descr'] ?? ''; + $descr .= " ({$rrule['sched']})"; + $rule->updateDescription($descr); + foreach ($config['schedules']['schedule'] as $sched) { + if ($sched['name'] == $rrule['sched']) { + if (!filter_get_time_based_rule_status($sched)) { + if (!isset($config['system']['schedule_states'])) { + $sched_kill_states[] = $rrule['label']; + } + /* disable rule, suffix label to mark end of schedule */ + $rule->disable(); + $rule->updateDescription("[FIN] " . $descr); + } + break; + } + } + } + } openlog("firewall", LOG_DAEMON, LOG_LOCAL4); diff --git a/src/etc/inc/filter.lib.inc b/src/etc/inc/filter.lib.inc index 0c30924eda..19deaad004 100644 --- a/src/etc/inc/filter.lib.inc +++ b/src/etc/inc/filter.lib.inc @@ -667,14 +667,12 @@ function filter_core_rules_system($fw, $defaults) } /** - * register user rules, returns kill states for schedules + * register user rules */ function filter_core_rules_user($fw) { global $config; - $sched_kill_states = []; - foreach ((new \OPNsense\Firewall\Group())->ifgroupentry->iterateItems() as $node) { $ifgroups[(string)$node->ifname] = !empty((string)$node->sequence) ? (string)$node->sequence : 0; } @@ -696,9 +694,6 @@ function filter_core_rules_user($fw) if (empty($rule['descr'])) { $rule['descr'] = ''; } - if (!empty($rule['sched'])) { - $rule['descr'] .= " ({$rule['sched']})"; - } if (isset($rule['floating'])) { $prio = 200000; @@ -707,25 +702,7 @@ function filter_core_rules_user($fw) } else { $prio = 400000; } - /* is a time based rule schedule attached? */ - if (!empty($rule['sched']) && !empty($config['schedules'])) { - foreach ($config['schedules']['schedule'] as $sched) { - if ($sched['name'] == $rule['sched']) { - if (!filter_get_time_based_rule_status($sched)) { - if (!isset($config['system']['schedule_states'])) { - $sched_kill_states[] = $rule['label']; - } - /* disable rule, suffix label to mark end of schedule */ - $rule['disabled'] = true; - $rule['descr'] = "[FIN] " . $rule['descr']; - } - break; - } - } - } $fw->registerFilterRule($prio, $rule); } } - - return $sched_kill_states; } diff --git a/src/etc/inc/plugins.inc.d/pf.inc b/src/etc/inc/plugins.inc.d/pf.inc index 58fe6a1341..1095eb8096 100644 --- a/src/etc/inc/plugins.inc.d/pf.inc +++ b/src/etc/inc/plugins.inc.d/pf.inc @@ -58,36 +58,31 @@ function pf_cron() { global $config; - $jobs = array(); + $jobs = []; - if (isset($config['filter']['rule'])) { - foreach ($config['filter']['rule'] as $rule) { - if (empty($rule['disabled']) && !empty($rule['sched'])) { - $jobs[]['autocron'] = array('/usr/bin/logger "reload filter for configured schedules" ; /usr/local/etc/rc.filter_configure', '1,16,31,46'); - break; - } - } + if ((new OPNsense\Firewall\Filter(true))->hasSchedule()) { + $jobs[]['autocron'] = ['/usr/bin/logger "reload filter for configured schedules" ; /usr/local/etc/rc.filter_configure', '1,16,31,46']; } /* bogons fetch always set in default config.xml */ switch ($config['system']['bogons']['interval']) { case 'daily': - $jobs[]['autocron'] = array('/usr/local/sbin/configctl -d filter schedule bogons', '1', '3', '*', '*', '*'); + $jobs[]['autocron'] = ['/usr/local/sbin/configctl -d filter schedule bogons', '1', '3', '*', '*', '*']; break; case 'weekly': - $jobs[]['autocron'] = array('/usr/local/sbin/configctl -d filter schedule bogons', '1', '3', '*', '*', '0'); + $jobs[]['autocron'] = ['/usr/local/sbin/configctl -d filter schedule bogons', '1', '3', '*', '*', '0']; break; case 'monthly': default: - $jobs[]['autocron'] = array('/usr/local/sbin/configctl -d filter schedule bogons', '1', '3', '1', '*', '*'); + $jobs[]['autocron'] = ['/usr/local/sbin/configctl -d filter schedule bogons', '1', '3', '1', '*', '*']; break; } - $jobs[]['autocron'] = array( + $jobs[]['autocron'] = [ '/usr/local/bin/flock -n -E 0 -o /tmp/filter_update_tables.lock ' . '/usr/local/opnsense/scripts/filter/update_tables.py --quick', '*' - ); + ]; return $jobs; } diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml index 895d52fba5..641e1ec01c 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml +++ b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml @@ -286,6 +286,7 @@ true false + sched diff --git a/src/opnsense/mvc/app/library/OPNsense/Firewall/Rule.php b/src/opnsense/mvc/app/library/OPNsense/Firewall/Rule.php index 7d5757a42c..696e8d380b 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Firewall/Rule.php +++ b/src/opnsense/mvc/app/library/OPNsense/Firewall/Rule.php @@ -476,6 +476,16 @@ abstract class Rule return 'internal2'; // late } + public function updateDescription($descr) + { + $this->rule['descr'] = $descr; + } + + public function disable() + { + $this->rule['disabled'] = 1; + } + /** * return raw rule */ diff --git a/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.php b/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.php index 58a56a314e..947e9c5546 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.php +++ b/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.php @@ -366,4 +366,22 @@ class Filter extends BaseModel } return false; } + + public function hasSchedule() + { + foreach ($this->rules->rule->iterateItems() as $rule) { + if (!$rule->sched->isEmpty() && !$rule->enabled->isEmpty()) { + return true; + } + } + $cfg = (\OPNsense\Core\Config::getInstance())->object(); + if (isset($cfg->filter->rule)) { + foreach ($cfg->filter->children() as $tag => $rule) { + if ($tag === 'rule' && !empty($rule->sched) && empty((string)$rule->disabled)) { + return true; + } + } + } + return false; + } } diff --git a/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt b/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt index f296ea83fd..90d986fe84 100644 --- a/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt +++ b/src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt @@ -561,6 +561,17 @@ `; }, + sched: function(column, row) { + if (row[column.id] === '') { + return ""; + } + return ` + ${row[column.id]}   + + + + `; + }, }, }, commands: {