Merge pull request #2208 from sarthurdev/T5550

interface: T5550: Interface source-validation priority over global value 

6 files changed, 94 insertions, 54 deletions

diff --git a/data/templates/firewall/nftables.j2 b/data/templates/firewall/nftables.j2
index 0fbddfaa9..a82a5537b 100644
--- a/data/templates/firewall/nftables.j2
+++ b/data/templates/firewall/nftables.j2
@@ -5,29 +5,36 @@
 flush chain raw FW_CONNTRACK
 flush chain ip6 raw FW_CONNTRACK
 
+flush chain raw vyos_global_rpfilter
+flush chain ip6 raw vyos_global_rpfilter
+
 table raw {
     chain FW_CONNTRACK {
         {{ ipv4_conntrack_action }}
     }
+
+    chain vyos_global_rpfilter {
+{% if global_options.source_validation is vyos_defined('loose') %}
+        fib saddr oif 0 counter drop
+{% elif global_options.source_validation is vyos_defined('strict') %}
+        fib saddr . iif oif 0 counter drop
+{% endif %}
+        return
+    }
 }
 
 table ip6 raw {
     chain FW_CONNTRACK {
         {{ ipv6_conntrack_action }}
     }
-}
 
-{% if first_install is not vyos_defined %}
-delete table inet vyos_global_rpfilter
-{% endif %}
-table inet vyos_global_rpfilter {
-    chain PREROUTING {
-        type filter hook prerouting priority -300; policy accept;
-{% if global_options.source_validation is vyos_defined('loose') %}
+    chain vyos_global_rpfilter {
+{% if global_options.ipv6_source_validation is vyos_defined('loose') %}
         fib saddr oif 0 counter drop
-{% elif global_options.source_validation is vyos_defined('strict') %}
+{% elif global_options.ipv6_source_validation is vyos_defined('strict') %}
         fib saddr . iif oif 0 counter drop
 {% endif %}
+        return
     }
 }
 
diff --git a/data/vyos-firewall-init.conf b/data/vyos-firewall-init.conf
index 41e7627f5..b0026fdf3 100644
--- a/data/vyos-firewall-init.conf
+++ b/data/vyos-firewall-init.conf
@@ -19,6 +19,15 @@ table raw {
         type filter hook forward priority -300; policy accept;
     }
 
+    chain vyos_global_rpfilter {
+        return
+    }
+
+    chain vyos_rpfilter {
+        type filter hook prerouting priority -300; policy accept;
+        counter jump vyos_global_rpfilter
+    }
+
     chain PREROUTING {
         type filter hook prerouting priority -300; policy accept;
         counter jump VYOS_CT_IGNORE
@@ -82,8 +91,13 @@ table ip6 raw {
         type filter hook forward priority -300; policy accept;
     }
 
+    chain vyos_global_rpfilter {
+        return
+    }
+
     chain vyos_rpfilter {
         type filter hook prerouting priority -300; policy accept;
+        counter jump vyos_global_rpfilter
     }
 
     chain PREROUTING {
diff --git a/interface-definitions/include/firewall/global-options.xml.i b/interface-definitions/include/firewall/global-options.xml.i
index a63874cb0..e655cd6ac 100644
--- a/interface-definitions/include/firewall/global-options.xml.i
+++ b/interface-definitions/include/firewall/global-options.xml.i
@@ -145,21 +145,21 @@
     </leafNode>
     <leafNode name="source-validation">
       <properties>
-        <help>Policy for source validation by reversed path, as specified in RFC3704</help>
+        <help>Policy for IPv4 source validation by reversed path, as specified in RFC3704</help>
         <completionHelp>
           <list>strict loose disable</list>
         </completionHelp>
         <valueHelp>
           <format>strict</format>
-          <description>Enable Strict Reverse Path Forwarding as defined in RFC3704</description>
+          <description>Enable IPv4 Strict Reverse Path Forwarding as defined in RFC3704</description>
         </valueHelp>
         <valueHelp>
           <format>loose</format>
-          <description>Enable Loose Reverse Path Forwarding as defined in RFC3704</description>
+          <description>Enable IPv4 Loose Reverse Path Forwarding as defined in RFC3704</description>
         </valueHelp>
         <valueHelp>
           <format>disable</format>
-          <description>No source validation</description>
+          <description>No IPv4 source validation</description>
         </valueHelp>
         <constraint>
           <regex>(strict|loose|disable)</regex>
@@ -227,6 +227,30 @@
       </properties>
       <defaultValue>disable</defaultValue>
     </leafNode>
+    <leafNode name="ipv6-source-validation">
+      <properties>
+        <help>Policy for IPv6 source validation by reversed path, as specified in RFC3704</help>
+        <completionHelp>
+          <list>strict loose disable</list>
+        </completionHelp>
+        <valueHelp>
+          <format>strict</format>
+          <description>Enable IPv6 Strict Reverse Path Forwarding as defined in RFC3704</description>
+        </valueHelp>
+        <valueHelp>
+          <format>loose</format>
+          <description>Enable IPv6 Loose Reverse Path Forwarding as defined in RFC3704</description>
+        </valueHelp>
+        <valueHelp>
+          <format>disable</format>
+          <description>No IPv6 source validation</description>
+        </valueHelp>
+        <constraint>
+          <regex>(strict|loose|disable)</regex>
+        </constraint>
+      </properties>
+      <defaultValue>disable</defaultValue>
+    </leafNode>
     <leafNode name="ipv6-src-route">
       <properties>
         <help>Policy for handling IPv6 packets with routing extension header</help>
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 53256abb7..050095364 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -174,10 +174,6 @@ class Interface(Control):
             'validate': assert_boolean,
             'location': '/proc/sys/net/ipv4/conf/{ifname}/bc_forwarding',
         },
-        'rp_filter': {
-            'validate': lambda flt: assert_range(flt,0,3),
-            'location': '/proc/sys/net/ipv4/conf/{ifname}/rp_filter',
-        },
         'ipv6_accept_ra': {
             'validate': lambda ara: assert_range(ara,0,3),
             'location': '/proc/sys/net/ipv6/conf/{ifname}/accept_ra',
@@ -251,9 +247,6 @@ class Interface(Control):
         'ipv4_directed_broadcast': {
             'location': '/proc/sys/net/ipv4/conf/{ifname}/bc_forwarding',
         },
-        'rp_filter': {
-            'location': '/proc/sys/net/ipv4/conf/{ifname}/rp_filter',
-        },
         'ipv6_accept_ra': {
             'location': '/proc/sys/net/ipv6/conf/{ifname}/accept_ra',
         },
@@ -763,44 +756,36 @@ class Interface(Control):
             return None
         return self.set_interface('ipv4_directed_broadcast', forwarding)
 
-    def set_ipv4_source_validation(self, value):
+    def _cleanup_ipv4_source_validation_rules(self, ifname):
+        results = self._cmd(f'nft -a list chain ip raw vyos_rpfilter').split("\n")
+        for line in results:
+            if f'iifname "{ifname}"' in line:
+                handle_search = re.search('handle (\d+)', line)
+                if handle_search:
+                    self._cmd(f'nft delete rule ip raw vyos_rpfilter handle {handle_search[1]}')
+
+    def set_ipv4_source_validation(self, mode):
         """
-        Help prevent attacks used by Spoofing IP Addresses. Reverse path
-        filtering is a Kernel feature that, when enabled, is designed to ensure
-        packets that are not routable to be dropped. The easiest example of this
-        would be and IP Address of the range 10.0.0.0/8, a private IP Address,
-        being received on the Internet facing interface of the router.
+        Set IPv4 reverse path validation
 
-        As per RFC3074.
+        Example:
+        >>> from vyos.ifconfig import Interface
+        >>> Interface('eth0').set_ipv4_source_validation('strict')
         """
         # Don't allow for netns yet
         if 'netns' in self.config:
             return None
 
-        if value == 'strict':
-            value = 1
-        elif value == 'loose':
-            value = 2
-        else:
-            value = 0
-
-        all_rp_filter = int(read_file('/proc/sys/net/ipv4/conf/all/rp_filter'))
-        if all_rp_filter > value:
-            global_setting = 'disable'
-            if   all_rp_filter == 1: global_setting = 'strict'
-            elif all_rp_filter == 2: global_setting = 'loose'
-
-            from vyos.base import Warning
-            Warning(f'Global source-validation is set to "{global_setting}", this '\
-                    f'overrides per interface setting on "{self.ifname}"!')
-
-        tmp = self.get_interface('rp_filter')
-        if int(tmp) == value:
-            return None
-        return self.set_interface('rp_filter', value)
+        self._cleanup_ipv4_source_validation_rules(self.ifname)
+        nft_prefix = f'nft insert rule ip raw vyos_rpfilter iifname "{self.ifname}"'
+        if mode in ['strict', 'loose']:
+            self._cmd(f"{nft_prefix} counter return")
+        if mode == 'strict':
+            self._cmd(f"{nft_prefix} fib saddr . iif oif 0 counter drop")
+        elif mode == 'loose':
+            self._cmd(f"{nft_prefix} fib saddr oif 0 counter drop")
 
     def _cleanup_ipv6_source_validation_rules(self, ifname):
-        commands = []
         results = self._cmd(f'nft -a list chain ip6 raw vyos_rpfilter').split("\n")
         for line in results:
             if f'iifname "{ifname}"' in line:
@@ -821,7 +806,9 @@ class Interface(Control):
             return None
 
         self._cleanup_ipv6_source_validation_rules(self.ifname)
-        nft_prefix = f'nft add rule ip6 raw vyos_rpfilter iifname "{self.ifname}"'
+        nft_prefix = f'nft insert rule ip6 raw vyos_rpfilter iifname "{self.ifname}"'
+        if mode in ['strict', 'loose']:
+            self._cmd(f"{nft_prefix} counter return")
         if mode == 'strict':
             self._cmd(f"{nft_prefix} fib saddr . iif oif 0 counter drop")
         elif mode == 'loose':
diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py
index 820024dc9..51ccbc9e6 100644
--- a/smoketest/scripts/cli/base_interfaces_test.py
+++ b/smoketest/scripts/cli/base_interfaces_test.py
@@ -834,8 +834,12 @@ class BasicInterfaceTest:
                     self.assertEqual('1', tmp)
 
                 if cli_defined(self._base_path + ['ip'], 'source-validation'):
-                    tmp = read_file(f'{proc_base}/rp_filter')
-                    self.assertEqual('2', tmp)
+                    base_options = f'iifname "{interface}"'
+                    out = cmd('sudo nft list chain ip raw vyos_rpfilter')
+                    for line in out.splitlines():
+                        if line.startswith(base_options):
+                            self.assertIn('fib saddr oif 0', line)
+                            self.assertIn('drop', line)
 
         def test_interface_ipv6_options(self):
             if not self._test_ipv6:
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index ee6ccb710..6f9093f4d 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -529,23 +529,27 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
     def test_source_validation(self):
         # Strict
         self.cli_set(['firewall', 'global-options', 'source-validation', 'strict'])
+        self.cli_set(['firewall', 'global-options', 'ipv6-source-validation', 'strict'])
         self.cli_commit()
 
         nftables_strict_search = [
             ['fib saddr . iif oif 0', 'drop']
         ]
 
-        self.verify_nftables(nftables_strict_search, 'inet vyos_global_rpfilter')
+        self.verify_nftables_chain(nftables_strict_search, 'ip raw', 'vyos_global_rpfilter')
+        self.verify_nftables_chain(nftables_strict_search, 'ip6 raw', 'vyos_global_rpfilter')
 
         # Loose
         self.cli_set(['firewall', 'global-options', 'source-validation', 'loose'])
+        self.cli_set(['firewall', 'global-options', 'ipv6-source-validation', 'loose'])
         self.cli_commit()
 
         nftables_loose_search = [
             ['fib saddr oif 0', 'drop']
         ]
 
-        self.verify_nftables(nftables_loose_search, 'inet vyos_global_rpfilter')
+        self.verify_nftables_chain(nftables_loose_search, 'ip raw', 'vyos_global_rpfilter')
+        self.verify_nftables_chain(nftables_loose_search, 'ip6 raw', 'vyos_global_rpfilter')
 
     def test_sysfs(self):
         for name, conf in sysfs_config.items():