From af5e9fcbf8806a966d0a955608f9f422491f238f Mon Sep 17 00:00:00 2001 From: Monviech <79600909+Monviech@users.noreply.github.com> Date: Tue, 18 Mar 2025 19:47:24 +0100 Subject: [PATCH] Firewall: Automation filter ui revamp (#8377) This commit adds backwards compatible changes to the automation api and associated user interface. Although this is likely not the final state, it adds quite some improvements in making this a valid replacement for the current firewall user interface. --- plist | 3 + .../Firewall/Api/FilterController.php | 326 +++++++- .../OPNsense/Firewall/FilterController.php | 42 +- .../Firewall/forms/dialogFilterRule.xml | 378 ++++++++- .../app/library/OPNsense/Firewall/Plugin.php | 34 +- .../Firewall/FieldTypes/FilterRuleField.php | 13 + .../FieldTypes/FilterSequenceField.php | 55 ++ .../Firewall/FieldTypes/GroupField.php | 5 + .../app/models/OPNsense/Firewall/Filter.xml | 8 +- .../views/OPNsense/Firewall/filter_rule.volt | 746 ++++++++++++++++++ .../scripts/filter/list_non_mvc_rules.php | 89 +++ .../conf/actions.d/actions_filter.conf | 9 +- 12 files changed, 1626 insertions(+), 82 deletions(-) create mode 100644 src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterSequenceField.php create mode 100644 src/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt create mode 100755 src/opnsense/scripts/filter/list_non_mvc_rules.php diff --git a/plist b/plist index 6eddb7bcc..8d7a77b99 100644 --- a/plist +++ b/plist @@ -752,6 +752,7 @@ /usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/AliasField.php /usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/AliasNameField.php /usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterRuleField.php +/usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterSequenceField.php /usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/GroupField.php /usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/GroupNameField.php /usr/local/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/InterfaceField.php @@ -946,6 +947,7 @@ /usr/local/opnsense/mvc/app/views/OPNsense/Firewall/alias_util.volt /usr/local/opnsense/mvc/app/views/OPNsense/Firewall/category.volt /usr/local/opnsense/mvc/app/views/OPNsense/Firewall/filter.volt +/usr/local/opnsense/mvc/app/views/OPNsense/Firewall/filter_rule.volt /usr/local/opnsense/mvc/app/views/OPNsense/Firewall/group.volt /usr/local/opnsense/mvc/app/views/OPNsense/IDS/index.volt /usr/local/opnsense/mvc/app/views/OPNsense/IDS/policy.volt @@ -1118,6 +1120,7 @@ /usr/local/opnsense/scripts/filter/lib/alias/pf.py /usr/local/opnsense/scripts/filter/lib/alias/uri.py /usr/local/opnsense/scripts/filter/lib/states.py +/usr/local/opnsense/scripts/filter/list_non_mvc_rules.php /usr/local/opnsense/scripts/filter/list_osfp.py /usr/local/opnsense/scripts/filter/list_pfsync.py /usr/local/opnsense/scripts/filter/list_rule_ids.py diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php index 159bb4303..1ca1db6ee 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php @@ -1,7 +1,7 @@ [], 'interface' => []]; + foreach ((new Category())->categories->category->iterateItems() as $key => $category) { + $result['category'][$key] = (string)$category->name; + } + foreach ((Config::getInstance()->object())->interfaces->children() as $key => $ifdetail) { + $descr = !empty($ifdetail->descr) ? $ifdetail->descr : strtoupper($key); + $result['interface'][$key] = $descr; + } + $result['action'] = [ + 'pass' => 'Pass', + 'block' => 'Block', + 'reject' => 'Reject' + ]; + + $result['ipprotocol'] = [ + 'inet' => gettext('IPv4'), + 'inet6' => gettext('IPv6'), + 'inet46' => gettext('IPv4+IPv6') + ]; + + return $result; + } + + /** + * Retrieves and merges firewall rules from model and internal sources, then paginates them. + * + * @return array The final paginated, merged result. + */ public function searchRuleAction() { - $category = $this->request->get('category'); - $filter_funct = function ($record) use ($category) { - return empty($category) || array_intersect(explode(',', $record->categories), $category); + $categories = $this->request->get('category'); + if (!empty($this->request->get('interface'))) { + $interface = $this->request->get('interface'); + $interfaces = [$interface]; + /* add groups which contain the selected interface */ + foreach ((new Group())->ifgroupentry->iterateItems() as $groupItem) { + if (in_array($interface, explode(',', (string)$groupItem->members))) { + $interfaces[] = (string)$groupItem->ifname; + } + } + } else { + $interfaces = null; + } + $show_all = !empty($this->request->get('show_all')); + + /* filter logic for mvc rules */ + $filter_funct_mvc = function ($record) use ($categories, $interfaces, $show_all) { + $is_cat = empty($categories) || array_intersect(explode(',', $record->categories), $categories); + if (empty($interfaces)) { + $is_if = count(explode(',', $record->interface)) > 1 || $record->interface->isEmpty(); + } else { + $is_if = array_intersect(explode(',', $record->interface), $interfaces) || $record->interface->isEmpty(); + } + return $is_cat && $is_if; }; - return $this->searchBase("rules.rule", ['enabled', 'sequence', 'action', 'description'], "sequence", $filter_funct); + + /* filter logic for legacy and internal rules */ + $fieldmap = $this->getFieldMap(); + if ($show_all) { + /* only query stats when fill info is requested */ + $rule_stats = json_decode((new Backend())->configdRun("filter rule stats") ?? '', true) ?? []; + } else { + $rule_stats = []; + } + + $catcolors = []; + foreach ((new Category())->categories->category->iterateItems() as $category) { + $color = trim((string)$category->color); + // Assign default color if empty + $catcolors[trim((string)$category->name)] = empty($color) ? "#C03E14" : "#{$color}"; + } + + $filter_funct_rs = function (&$record) use ( + $categories, + $interfaces, + $show_all, + $fieldmap, + $rule_stats, + $catcolors + ) { + /* always merge stats when found */ + if (!empty($record['uuid']) && !empty($rule_stats[$record['uuid']])) { + foreach ($rule_stats[$record['uuid']] as $key => $value) { + $record[$key] = $value; + } + } + /* frontend can format aliases with an alias icon */ + foreach (['source_net', 'source_port', 'destination_net', 'destination_port'] as $field) { + if (!empty($record[$field])) { + $record["is_alias_{$field}"] = array_map(function ($value) { + return Util::isAlias($value); + }, array_map('trim', explode(',', $record[$field]))); + } + } + + /* frontend can format categories with colors */ + if (!empty($record['categories'])) { + $catnames = array_map('trim', explode(',', $record['categories'])); + $record['category_colors'] = array_map(fn($name) => $catcolors[$name], $catnames);; + } else { + $record['category_colors'] = []; + } + + + if (empty($record['legacy'])) { + /* mvc already filtered */ + return true; + } + $is_cat = empty($categories) || array_intersect(explode(',', $record['category'] ?? ''), $categories); + + if (empty($interfaces)) { + $is_if = count(explode(',', $record['interface'])) > 1 || empty($record['interface']); + } else { + $is_if = array_intersect(explode(',', $record['interface'] ?? ''), $interfaces) ; + $is_if = $is_if || empty($record['interface']); + } + if ($is_cat && $is_if) { + /* translate/convert legacy fields before returning, similar to mvc handling */ + foreach ($fieldmap as $topic => $data) { + if (!empty($record[$topic])) { + $tmp = []; + foreach (explode(',', $record[$topic]) as $item) { + $tmp[] = $data[$item] ?? $item; + } + $record[$topic] = implode(',', $tmp); + } + } + return true; + } else { + return false; + } + }; + + /** + * XXX: fetch mvc results first, we need to collect all to ensure proper pagination + * as pagination is passed using the request, we need to reset it temporary here as we don't know + * which page we need (yet) and don't want to duplicate large portions of code. + **/ + $ORG_REQ = $_REQUEST; + unset($_REQUEST['rowCount']); + unset($_REQUEST['current']); + $filterset = $this->searchBase("rules.rule", null, "sort_order", $filter_funct_mvc)['rows']; + + /* only fetch internal and legacy rules when 'show_all' is set */ + if ($show_all) { + $otherrules = json_decode((new Backend())->configdRun("filter list non_mvc_rules") ?? '', true) ?? []; + } else { + $otherrules = []; + } + + $_REQUEST = $ORG_REQ; /* XXX: fix me ?*/ + $result = $this->searchRecordsetBase(array_merge($otherrules, $filterset), null, "sort_order", $filter_funct_rs); + + return $result; } public function setRuleAction($uuid) @@ -52,7 +209,16 @@ class FilterController extends FilterBaseController public function getRuleAction($uuid = null) { - return $this->getBase("rule", "rules.rule", $uuid); + $result = $this->getBase("rule", "rules.rule", $uuid); + if ($this->request->get('fetchmode') === 'copy' && !empty($result['rule'])) { + /* copy mode, generate new sequence at the end */ + $max = 0; + foreach ($this->getModel()->rules->rule->iterateItems() as $rule) { + $max = (int)((string)$rule->sequence) > $max ? (int)((string)$rule->sequence) : $max; + } + $result['rule']['sequence'] = $max + 100; + } + return $result; } public function delRuleAction($uuid) @@ -64,4 +230,152 @@ class FilterController extends FilterBaseController { return $this->toggleBase("rules.rule", $uuid, $enabled); } + + /** + * Moves the selected rule so that it appears immediately before the target rule. + * + * Uses integer gap numbering to update the sequence for only the moved rule. + * Rules will be renumbered within the selected range to prevent movements causing overlaps, + * but try to keep the changes as minimal as possible. + * + * Floating, Group, and Interface rules cannot be moved before another. + * + * @param string $selected_uuid The UUID of the rule to be moved. + * @param string $target_uuid The UUID of the target rule (the rule before which the selected rule is to be placed). + * @return array Returns ["status" => "ok"] on success, throws a userexception otherwise. + */ + public function moveRuleBeforeAction($selected_uuid, $target_uuid) + { + if (!$this->request->isPost()) { + return ["status" => "error", "message" => gettext("Invalid request method")]; + } + Config::getInstance()->lock(); + $mdl = $this->getModel(); + $prev_record = null; + $selected_id = null; + $selected_node = null; + $target_node = null; + foreach ($mdl->rules->rule->sortedBy(['prio_group', 'sequence']) as $record) { + $uuid = $record->getAttribute('uuid'); + if ($prev_record != null && (string)$prev_record->prio_group == (string)$record->prio_group) { + $prev_sequence = $prev_record->sequence->asFloat(); + /* distance will be averaged, which is why the minimum should be at least 2 (half is 1) */ + $distance = max($record->sequence->asFloat() - $prev_record->sequence->asFloat(), 2); + } elseif ($selected_node !== null && $target_node !== null){ + /* group processed */ + break; + } else { + /* first record, no previous one */ + $prev_sequence = 1; + $distance = 2; + } + + if ($uuid == $target_uuid) { + /** + * found our target, which will be the sources new place, + * reserve the full distance to facilitate for a swap. + **/ + $selected_id = (int)$record->sequence->asFloat(); + $record->sequence = (string)($prev_sequence + $distance); + $target_node = $record; + } elseif ($uuid == $selected_uuid) { + $selected_node = $record; + } elseif ($selected_id !== null && $prev_sequence >= $record->sequence->asFloat()) { + $record->sequence = (string)($prev_sequence + $distance/2); + } elseif ($target_node !== null && $selected_node !== null) { + /* both nodes found and the next one is in sequence, stop moving data */ + break; + } + /* validate overflow */ + if ($record->sequence->asFloat() > 999999) { + throw new UserException( + gettext("Cannot renumber rules without exceeding the maximum sequence limit"), + gettext("Filter") + ); + } + $prev_record = $record; + } + + if ($selected_node !== null) { + $selected_node->sequence = (string)$selected_id; + } + + /* validate what we plan to commit */ + if ($selected_node === null || $target_node === null) { + /* out of scope */ + throw new UserException( + gettext("Either source or destination is not a rule managed with this component"), + gettext("Filter") + ); + } elseif ((string)$selected_node->prio_group != (string)$target_node->prio_group) { + /* types don't match */ + $typeNames = [ + '2' => gettext("Floating"), + '3' => gettext("Group"), + '4' => gettext("Interface") + ]; + $selectedType = $typeNames[substr($selected_node->prio_group, 0, 1)] ?? gettext("Unknown"); + $targetType = $typeNames[substr($target_node->prio_group, 0, 1)] ?? gettext("Unknown"); + throw new UserException( + sprintf( + gettext("Cannot move '%s Rule' before '%s Rule'."), + $selectedType, + $targetType + ), + gettext("Filter") + ); + } + $mdl->serializeToConfig(false, true); /* we're only changing sequences, forcefully save */ + Config::getInstance()->save(); + + return ["status" => "ok"]; + } + + /** + * return interface options + */ + public function getInterfaceListAction() + { + $result = [ + 'floating' => [ + 'label' => gettext('Floating'), + 'icon' => 'fa fa-layer-group text-primary', + 'items' => [ + [ + 'value' => '', + 'label' => gettext('Any') + ] + ] + ], + 'groups' => [ + 'label' => gettext('Groups'), + 'icon' => 'fa fa-sitemap text-warning', + 'items' => [] + ], + 'interfaces' => [ + 'label' => gettext('Interfaces'), + 'icon' => 'fa fa-ethernet text-info', + 'items' => [] + ] + ]; + + foreach ((new Group())->ifgroupentry->iterateItems() as $groupItem) { + $groupName = (string)$groupItem->ifname; + $result['groups']['items'][$groupName] = ['value' => $groupName, 'label' => $groupName]; + } + foreach (Config::getInstance()->object()->interfaces->children() as $key => $intf) { + if (!isset($result['groups']['items'][$key])) { + $result['interfaces']['items'][$key] = [ + 'value' => $key, + 'label' => empty($intf->descr) ? strtoupper($key) : (string)$intf->descr + ]; + } + } + + foreach (array_keys($result) as $key) { + usort($result[$key]['items'], fn($a, $b) => strcasecmp($a['label'], $b['label'])); + } + + return $result; + } } diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/FilterController.php b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/FilterController.php index e558fe813..00afa48a0 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/FilterController.php +++ b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/FilterController.php @@ -1,7 +1,7 @@ view->pick('OPNsense/Firewall/filter'); - $this->view->SavePointBtns = true; - $this->view->ruleController = "filter"; - $this->view->gridFields = [ - [ - 'id' => 'enabled', 'formatter' => 'rowtoggle' ,'width' => '6em', 'heading' => gettext('Enabled') - ], - [ - 'id' => 'sequence','width' => '9em', 'heading' => gettext('Sequence') - ], - [ - 'id' => 'description', 'heading' => gettext('Description') - ] - ]; + $this->view->pick('OPNsense/Firewall/filter_rule'); $this->view->formDialogFilterRule = $this->getForm("dialogFilterRule"); + $this->view->formGridFilterRule = $this->getFormGrid('dialogFilterRule'); + $this->view->advancedFieldIds = $this->getAdvancedIds($this->view->formDialogFilterRule); } + + /** + * Get an array of field IDs that have the advanced flag set to "true". + * + * @param array $form An array of field definitions + * @return string list of fieldnames, comma separated for easy template usage + */ + protected function getAdvancedIds($form) + { + $advancedFieldIds = []; + + foreach ($form as $field) { + if (!empty($field['advanced']) && $field['advanced'] == "true") { + if (!empty($field['id'])) { + $tmp = explode('.', $field['id']); + $advancedFieldIds[] = $tmp[count($tmp)-1]; + } + } + } + + return implode(',', $advancedFieldIds); + } + } 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 d8d36ed8d..09b74cca6 100644 --- a/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml +++ b/src/opnsense/mvc/app/controllers/OPNsense/Firewall/forms/dialogFilterRule.xml @@ -1,13 +1,46 @@ +
diff --git a/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php b/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php index 1d73d5dcf..509faba12 100644 --- a/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php +++ b/src/opnsense/mvc/app/library/OPNsense/Firewall/Plugin.php @@ -37,21 +37,21 @@ use OPNsense\Core\Config; class Plugin { private $gateways = null; - private $anchors = array(); - private $filterRules = array(); - private $natRules = array(); - private $interfaceMapping = array(); - private $gatewayMapping = array(); - private $systemDefaults = array(); - private $tables = array(); - private $ifconfigDetails = array(); + private $anchors = []; + private $filterRules = []; + private $natRules = []; + private $interfaceMapping = []; + private $gatewayMapping = []; + private $systemDefaults = []; + private $tables = []; + private $ifconfigDetails = []; /** * init firewall plugin component */ public function __construct() { - $this->systemDefaults = array("filter" => array(), "forward" => array(), "nat" => array()); + $this->systemDefaults = array("filter" => [], "forward" => [], "nat" => []); if (!empty(Config::getInstance()->object()->system->disablereplyto)) { $this->systemDefaults['filter']['disablereplyto'] = true; } @@ -82,7 +82,7 @@ class Plugin if (!empty($intf['ipaddrv6']) && ($intf['ipaddrv6'] == '6rd' || $intf['ipaddrv6'] == '6to4')) { $realif = "{$key}_stf"; // create new interface - $this->interfaceMapping[$realif] = array(); + $this->interfaceMapping[$realif] = []; $this->interfaceMapping[$realif]['ifconfig']['ipv6'] = $intf['ifconfig']['ipv6']; $this->interfaceMapping[$realif]['gatewayv6'] = $intf['gatewayv6']; $this->interfaceMapping[$realif]['is_IPv6_override'] = true; @@ -134,7 +134,7 @@ class Plugin { if (is_array($groups)) { foreach ($groups as $key => $gwgr) { - $routeto = array(); + $routeto = []; $proto = 'inet'; foreach ($gwgr as $gw) { if (Util::isIpAddress($gw['gwip']) && !empty($gw['int'])) { @@ -277,7 +277,7 @@ class Plugin $conf['#priority'] = $prio; $rule = new FilterRule($this->interfaceMapping, $this->gatewayMapping, $conf); if (empty($this->filterRules[$prio])) { - $this->filterRules[$prio] = array(); + $this->filterRules[$prio] = []; } $this->filterRules[$prio][] = $rule; } @@ -294,7 +294,7 @@ class Plugin } $rule = new ForwardRule($this->interfaceMapping, $conf); if (empty($this->natRules[$prio])) { - $this->natRules[$prio] = array(); + $this->natRules[$prio] = []; } $this->natRules[$prio][] = $rule; } @@ -311,7 +311,7 @@ class Plugin } $rule = new DNatRule($this->interfaceMapping, $conf); if (empty($this->natRules[$prio])) { - $this->natRules[$prio] = array(); + $this->natRules[$prio] = []; } $this->natRules[$prio][] = $rule; } @@ -325,7 +325,7 @@ class Plugin { $rule = new SNatRule($this->interfaceMapping, $conf); if (empty($this->natRules[$prio])) { - $this->natRules[$prio] = array(); + $this->natRules[$prio] = []; } $this->natRules[$prio][] = $rule; } @@ -339,7 +339,7 @@ class Plugin { $rule = new NptRule($this->interfaceMapping, $conf); if (empty($this->natRules[$prio])) { - $this->natRules[$prio] = array(); + $this->natRules[$prio] = []; } $this->natRules[$prio][] = $rule; } @@ -370,7 +370,7 @@ class Plugin ksort($this->filterRules); /* sort rules by priority */ foreach ($this->filterRules as $prio => $ruleset) { foreach ($ruleset as $rule) { - yield $rule; + yield $prio => $rule; } } } diff --git a/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterRuleField.php b/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterRuleField.php index 0311699c4..e4cbbba83 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterRuleField.php +++ b/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterRuleField.php @@ -153,4 +153,17 @@ class FilterRuleField extends ArrayField $container_node->setParentModel($parentmodel); return $container_node; } + + protected function actionPostLoadingEvent() + { + foreach ($this->internalChildnodes as $node) { + /** + * Evaluation order consists of a priority group and a sequence within the set, + * prefixed with 0 as these precede legacy rules + **/ + $node->sort_order = sprintf("%d.0%06d", $node->getPriority(), (string)$node->sequence); + $node->prio_group = (string)$node->getPriority(); + } + return parent::actionPostLoadingEvent(); + } } diff --git a/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterSequenceField.php b/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterSequenceField.php new file mode 100644 index 000000000..45154095d --- /dev/null +++ b/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/FilterSequenceField.php @@ -0,0 +1,55 @@ +minimum_value; + if (isset($this->internalParentNode->internalParentNode)) { + foreach ($this->internalParentNode->internalParentNode->iterateItems() as $node) { + $currentNumber = (int)((string)$node->{$this->internalXMLTagName}); + // Update maxNumber if this value is greater + if ($currentNumber >= $maxNumber) { + $maxNumber = $currentNumber; + } + } + } + $this->internalValue = (string)($maxNumber + 100); + } +} diff --git a/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/GroupField.php b/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/GroupField.php index 01393826c..852b24059 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/GroupField.php +++ b/src/opnsense/mvc/app/models/OPNsense/Firewall/FieldTypes/GroupField.php @@ -52,6 +52,11 @@ class GroupField extends ArrayField 'sequence' => 10, 'ifname' => 'enc0', 'descr' => gettext('IPsec') + ], + 'wireguard' => [ + 'sequence' => 10, + 'ifname' => 'wireguard', + 'descr' => gettext('Wireguard') ] ]; } diff --git a/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.xml b/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.xml index b03b2a326..2d4a252ae 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.xml +++ b/src/opnsense/mvc/app/models/OPNsense/Firewall/Filter.xml @@ -28,13 +28,15 @@