Merge pull request #3938 from talmakion/feature/T6430-local-pbr

pbr: T6430: Local IP rules targeting VRFs by name as well as route table IDs

7 files changed, 244 insertions, 40 deletions

diff --git a/interface-definitions/include/firewall/set-packet-modifications-table-and-vrf.xml.i b/interface-definitions/include/firewall/set-packet-modifications-table-and-vrf.xml.i
index c7875b31d..5eb1984a5 100644
--- a/interface-definitions/include/firewall/set-packet-modifications-table-and-vrf.xml.i
+++ b/interface-definitions/include/firewall/set-packet-modifications-table-and-vrf.xml.i
@@ -25,24 +25,7 @@
         </completionHelp>
       </properties>
     </leafNode>
-    <leafNode name="vrf">
-      <properties>
-        <help>VRF to forward packet with</help>
-        <valueHelp>
-          <format>txt</format>
-          <description>VRF instance name</description>
-        </valueHelp>
-        <valueHelp>
-          <format>default</format>
-          <description>Forward into default global VRF</description>
-        </valueHelp>
-        <completionHelp>
-          <list>default</list>
-          <path>vrf name</path>
-        </completionHelp>
-        #include <include/constraint/vrf.xml.i>
-      </properties>
-    </leafNode>
+    #include <include/firewall/vrf.xml.i>
   </children>
 </node>
 <!-- include end -->
diff --git a/interface-definitions/include/firewall/vrf.xml.i b/interface-definitions/include/firewall/vrf.xml.i
new file mode 100644
index 000000000..af8ce3ab4
--- /dev/null
+++ b/interface-definitions/include/firewall/vrf.xml.i
@@ -0,0 +1,20 @@
+<!-- include start from firewall/vrf.xml.i -->
+<leafNode name="vrf">
+  <properties>
+    <help>VRF to forward packet with</help>
+    <valueHelp>
+      <format>txt</format>
+      <description>VRF instance name</description>
+    </valueHelp>
+    <valueHelp>
+      <format>default</format>
+      <description>Forward into default global VRF</description>
+    </valueHelp>
+    <completionHelp>
+      <list>default</list>
+      <path>vrf name</path>
+    </completionHelp>
+    #include <include/constraint/vrf.xml.i>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/policy_local-route.xml.in b/interface-definitions/policy_local-route.xml.in
index 7a019154a..9f6588db8 100644
--- a/interface-definitions/policy_local-route.xml.in
+++ b/interface-definitions/policy_local-route.xml.in
@@ -39,6 +39,7 @@
                       </completionHelp>
                     </properties>
                   </leafNode>
+                  #include <include/firewall/vrf.xml.i>
                 </children>
               </node>
               <leafNode name="fwmark">
@@ -113,6 +114,7 @@
                       </completionHelp>
                     </properties>
                   </leafNode>
+                  #include <include/firewall/vrf.xml.i>
                 </children>
               </node>
               <leafNode name="fwmark">
diff --git a/smoketest/scripts/cli/base_vyostest_shim.py b/smoketest/scripts/cli/base_vyostest_shim.py
index 940306ac3..a383e596c 100644
--- a/smoketest/scripts/cli/base_vyostest_shim.py
+++ b/smoketest/scripts/cli/base_vyostest_shim.py
@@ -147,6 +147,18 @@ class VyOSUnitTestSHIM:
                         break
                 self.assertTrue(not matched if inverse else matched, msg=search)
 
+        # Verify ip rule output
+        def verify_rules(self, rules_search, inverse=False, addr_family='inet'):
+            rule_output = cmd(f'ip -family {addr_family} rule show')
+
+            for search in rules_search:
+                matched = False
+                for line in rule_output.split("\n"):
+                    if all(item in line for item in search):
+                        matched = True
+                        break
+                self.assertTrue(not matched if inverse else matched, msg=search)
+
 # standard construction; typing suggestion: https://stackoverflow.com/a/70292317
 def ignore_warning(warning: Type[Warning]):
     import warnings
diff --git a/smoketest/scripts/cli/test_policy_local-route.py b/smoketest/scripts/cli/test_policy_local-route.py
new file mode 100644
index 000000000..8d6ba40dc
--- /dev/null
+++ b/smoketest/scripts/cli/test_policy_local-route.py
@@ -0,0 +1,171 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2024 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import unittest
+
+from base_vyostest_shim import VyOSUnitTestSHIM
+
+interface = 'eth0'
+mark = '100'
+table_id = '101'
+extra_table_id = '102'
+vrf_name = 'LPBRVRF'
+vrf_rt_id = '202'
+
+class TestPolicyLocalRoute(VyOSUnitTestSHIM.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        super(TestPolicyLocalRoute, cls).setUpClass()
+        # Clear out current configuration to allow running this test on a live system
+        cls.cli_delete(cls, ['policy', 'local-route'])
+        cls.cli_delete(cls, ['policy', 'local-route6'])
+
+        cls.cli_set(cls, ['vrf', 'name', vrf_name, 'table', vrf_rt_id])
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.cli_delete(cls, ['vrf', 'name', vrf_name])
+
+        super(TestPolicyLocalRoute, cls).tearDownClass()
+
+    def tearDown(self):
+        self.cli_delete(['policy', 'local-route'])
+        self.cli_delete(['policy', 'local-route6'])
+        self.cli_commit()
+
+        ip_rule_search = [
+            [f'lookup {table_id}']
+        ]
+
+        self.verify_rules(ip_rule_search, inverse=True)
+        self.verify_rules(ip_rule_search, inverse=True, addr_family='inet6')
+
+    def test_local_pbr_matching_criteria(self):
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'inbound-interface', interface])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'protocol', 'udp'])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'fwmark', mark])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'destination', 'address', '198.51.100.0/24'])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'destination', 'port', '111'])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'source', 'address', '198.51.100.1'])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'source', 'port', '443'])
+        self.cli_set(['policy', 'local-route', 'rule', '4', 'set', 'table', table_id])
+
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'inbound-interface', interface])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'protocol', 'tcp'])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'fwmark', mark])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'destination', 'address', '2001:db8::/64'])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'destination', 'port', '123'])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'source', 'address', '2001:db8::1'])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'source', 'port', '80'])
+        self.cli_set(['policy', 'local-route6', 'rule', '6', 'set', 'table', table_id])
+
+        self.cli_commit()
+
+        rule_lookup = f'lookup {table_id}'
+        rule_fwmark = 'fwmark ' + hex(int(mark))
+        rule_interface = f'iif {interface}'
+
+        ip4_rule_search = [
+            ['from 198.51.100.1', 'to 198.51.100.0/24', rule_fwmark, rule_interface, 'ipproto udp', 'sport 443', 'dport 111', rule_lookup]
+        ]
+
+        self.verify_rules(ip4_rule_search)
+
+        ip6_rule_search = [
+            ['from 2001:db8::1', 'to 2001:db8::/64', rule_fwmark, rule_interface, 'ipproto tcp', 'sport 80', 'dport 123', rule_lookup]
+        ]
+
+        self.verify_rules(ip6_rule_search, addr_family='inet6')
+
+    def test_local_pbr_rule_removal(self):
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'destination', 'address', '198.51.100.1'])
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'set', 'table', table_id])
+
+        self.cli_set(['policy', 'local-route', 'rule', '2', 'destination', 'address', '198.51.100.2'])
+        self.cli_set(['policy', 'local-route', 'rule', '2', 'set', 'table', table_id])
+
+        self.cli_set(['policy', 'local-route', 'rule', '3', 'destination', 'address', '198.51.100.3'])
+        self.cli_set(['policy', 'local-route', 'rule', '3', 'set', 'table', table_id])
+
+        self.cli_commit()
+
+        rule_lookup = f'lookup {table_id}'
+
+        ip_rule_search = [
+            ['to 198.51.100.1', rule_lookup],
+            ['to 198.51.100.2', rule_lookup],
+            ['to 198.51.100.3', rule_lookup],
+        ]
+
+        self.verify_rules(ip_rule_search)
+
+        self.cli_delete(['policy', 'local-route', 'rule', '2'])
+        self.cli_commit()
+
+        ip_rule_missing = [
+            ['to 198.51.100.2', rule_lookup],
+        ]
+
+        self.verify_rules(ip_rule_missing, inverse=True)
+
+    def test_local_pbr_rule_changes(self):
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'destination', 'address', '198.51.100.0/24'])
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'set', 'table', table_id])
+
+        self.cli_commit()
+
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'set', 'table', extra_table_id])
+        self.cli_commit()
+
+        ip_rule_search_extra = [
+            ['to 198.51.100.0/24', f'lookup {extra_table_id}']
+        ]
+
+        self.verify_rules(ip_rule_search_extra)
+
+        ip_rule_search_orig = [
+            ['to 198.51.100.0/24', f'lookup {table_id}']
+        ]
+
+        self.verify_rules(ip_rule_search_orig, inverse=True)
+
+        self.cli_delete(['policy', 'local-route', 'rule', '1', 'set', 'table'])
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'set', 'vrf', vrf_name])
+
+        self.cli_commit()
+
+        ip_rule_search_vrf = [
+            ['to 198.51.100.0/24', f'lookup {vrf_name}']
+        ]
+
+        self.verify_rules(ip_rule_search_extra, inverse=True)
+        self.verify_rules(ip_rule_search_vrf)
+
+    def test_local_pbr_target_vrf(self):
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'destination', 'address', '198.51.100.0/24'])
+        self.cli_set(['policy', 'local-route', 'rule', '1', 'set', 'vrf', vrf_name])
+
+        self.cli_commit()
+
+        ip_rule_search = [
+            ['to 198.51.100.0/24', f'lookup {vrf_name}']
+        ]
+
+        self.verify_rules(ip_rule_search)
+
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)
diff --git a/smoketest/scripts/cli/test_policy_route.py b/smoketest/scripts/cli/test_policy_route.py
index 797ab9770..672865eb0 100755
--- a/smoketest/scripts/cli/test_policy_route.py
+++ b/smoketest/scripts/cli/test_policy_route.py
@@ -18,8 +18,6 @@ import unittest
 
 from base_vyostest_shim import VyOSUnitTestSHIM
 
-from vyos.utils.process import cmd
-
 mark = '100'
 conn_mark = '555'
 conn_mark_set = '111'
@@ -41,7 +39,7 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
 
         cls.cli_set(cls, ['interfaces', 'ethernet', interface, 'address', interface_ip])
         cls.cli_set(cls, ['protocols', 'static', 'table', table_id, 'route', '0.0.0.0/0', 'interface', interface])
-        
+
         cls.cli_set(cls, ['vrf', 'name', vrf, 'table', vrf_table_id])
 
     @classmethod
@@ -73,17 +71,6 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
 
         self.verify_rules(ip_rule_search, inverse=True)
 
-    def verify_rules(self, rules_search, inverse=False):
-        rule_output = cmd('ip rule show')
-
-        for search in rules_search:
-            matched = False
-            for line in rule_output.split("\n"):
-                if all(item in line for item in search):
-                    matched = True
-                    break
-            self.assertTrue(not matched if inverse else matched, msg=search)
-
     def test_pbr_group(self):
         self.cli_set(['firewall', 'group', 'network-group', 'smoketest_network', 'network', '172.16.99.0/24'])
         self.cli_set(['firewall', 'group', 'network-group', 'smoketest_network1', 'network', '172.16.101.0/24'])
diff --git a/src/conf_mode/policy_local-route.py b/src/conf_mode/policy_local-route.py
index 331fd972d..9be2bc227 100755
--- a/src/conf_mode/policy_local-route.py
+++ b/src/conf_mode/policy_local-route.py
@@ -54,6 +54,7 @@ def get_config(config=None):
                 dst = leaf_node_changed(conf, base_rule + [rule, 'destination', 'address'])
                 dst_port = leaf_node_changed(conf, base_rule + [rule, 'destination', 'port'])
                 table = leaf_node_changed(conf, base_rule + [rule, 'set', 'table'])
+                vrf = leaf_node_changed(conf, base_rule + [rule, 'set', 'vrf'])
                 proto = leaf_node_changed(conf, base_rule + [rule, 'protocol'])
                 rule_def = {}
                 if src:
@@ -70,6 +71,8 @@ def get_config(config=None):
                     rule_def = dict_merge({'destination': {'port': dst_port}}, rule_def)
                 if table:
                     rule_def = dict_merge({'table' : table}, rule_def)
+                if vrf:
+                    rule_def = dict_merge({'vrf' : vrf}, rule_def)
                 if proto:
                     rule_def = dict_merge({'protocol' : proto}, rule_def)
                 dict = dict_merge({dict_id : {rule : rule_def}}, dict)
@@ -90,6 +93,7 @@ def get_config(config=None):
                 dst = leaf_node_changed(conf, base_rule + [rule, 'destination', 'address'])
                 dst_port = leaf_node_changed(conf, base_rule + [rule, 'destination', 'port'])
                 table = leaf_node_changed(conf, base_rule + [rule, 'set', 'table'])
+                vrf = leaf_node_changed(conf, base_rule + [rule, 'set', 'vrf'])
                 proto = leaf_node_changed(conf, base_rule + [rule, 'protocol'])
                 # keep track of changes in configuration
                 # otherwise we might remove an existing node although nothing else has changed
@@ -179,6 +183,15 @@ def get_config(config=None):
                     if len(table) > 0:
                         rule_def = dict_merge({'table' : table}, rule_def)
 
+                # vrf
+                if vrf is None:
+                    if 'set' in rule_config and 'vrf' in rule_config['set']:
+                        rule_def = dict_merge({'vrf': [rule_config['set']['vrf']]}, rule_def)
+                else:
+                    changed = True
+                    if len(vrf) > 0:
+                        rule_def = dict_merge({'vrf' : vrf}, rule_def)
+
                 # protocol
                 if proto is None:
                     if 'protocol' in rule_config:
@@ -218,8 +231,15 @@ def verify(pbr):
                 ):
                     raise ConfigError('Source or destination address or fwmark or inbound-interface or protocol is required!')
 
-                if 'set' not in pbr_route['rule'][rule] or 'table' not in pbr_route['rule'][rule]['set']:
-                    raise ConfigError('Table set is required!')
+                if 'set' not in pbr_route['rule'][rule]:
+                    raise ConfigError('Either set table or set vrf is required!')
+
+                set_tgts = pbr_route['rule'][rule]['set']
+                if 'table' not in set_tgts and 'vrf' not in set_tgts:
+                    raise ConfigError('Either set table or set vrf is required!')
+
+                if 'table' in set_tgts and 'vrf' in set_tgts:
+                    raise ConfigError('set table and set vrf cannot both be set!')
 
                 if 'inbound_interface' in pbr_route['rule'][rule]:
                     interface = pbr_route['rule'][rule]['inbound_interface']
@@ -250,11 +270,14 @@ def apply(pbr):
                 fwmark = rule_config.get('fwmark', [''])
                 inbound_interface = rule_config.get('inbound_interface', [''])
                 protocol = rule_config.get('protocol', [''])
-                table = rule_config.get('table', [''])
+                # VRF 'default' is actually table 'main' for RIB rules
+                vrf = [ 'main' if x == 'default' else x for x in rule_config.get('vrf', ['']) ]
+                # See generate section below for table/vrf overlap explanation 
+                table_or_vrf = rule_config.get('table', vrf)
 
-                for src, dst, src_port, dst_port, fwmk, iif, proto, table in product(
+                for src, dst, src_port, dst_port, fwmk, iif, proto, table_or_vrf in product(
                         source, destination, source_port, destination_port,
-                        fwmark, inbound_interface, protocol, table):
+                        fwmark, inbound_interface, protocol, table_or_vrf):
                     f_src = '' if src == '' else f' from {src} '
                     f_src_port = '' if src_port == '' else f' sport {src_port} '
                     f_dst = '' if dst == '' else f' to {dst} '
@@ -262,7 +285,7 @@ def apply(pbr):
                     f_fwmk = '' if fwmk == '' else f' fwmark {fwmk} '
                     f_iif = '' if iif == '' else f' iif {iif} '
                     f_proto = '' if proto == '' else f' ipproto {proto} '
-                    f_table = '' if table == '' else f' lookup {table} '
+                    f_table = '' if table_or_vrf == '' else f' lookup {table_or_vrf} '
 
                     call(f'ip{v6} rule del prio {rule} {f_src}{f_dst}{f_proto}{f_src_port}{f_dst_port}{f_fwmk}{f_iif}{f_table}')
 
@@ -276,7 +299,13 @@ def apply(pbr):
 
         if 'rule' in pbr_route:
             for rule, rule_config in pbr_route['rule'].items():
-                table = rule_config['set'].get('table', '')
+                # VRFs get configred as route table alias names for iproute2 and only 
+                # one 'set' can get past validation. Either can be fed to lookup. 
+                vrf = rule_config['set'].get('vrf', '')
+                if vrf == 'default':
+                    table_or_vrf = 'main'
+                else:
+                    table_or_vrf = rule_config['set'].get('table', vrf)
                 source = rule_config.get('source', {}).get('address', ['all'])
                 source_port = rule_config.get('source', {}).get('port', '')
                 destination = rule_config.get('destination', {}).get('address', ['all'])
@@ -295,7 +324,7 @@ def apply(pbr):
                         f_iif = f' iif {inbound_interface} ' if inbound_interface else ''
                         f_proto = f' ipproto {protocol} ' if protocol else ''
 
-                        call(f'ip{v6} rule add prio {rule}{f_src}{f_dst}{f_proto}{f_src_port}{f_dst_port}{f_fwmk}{f_iif} lookup {table}')
+                        call(f'ip{v6} rule add prio {rule}{f_src}{f_dst}{f_proto}{f_src_port}{f_dst_port}{f_fwmk}{f_iif} lookup {table_or_vrf}')
 
     return None