firewall: T4694: Adding rt ipsec exists/missing match to firewall con…

…figs (#3616)

* Change ipsec match-ipsec/none to match-ipsec-in and match-none-in for
   fw rules
 * Add ipsec match-ipsec-out and match-none-out
 * Change all the points where the match-ipsec.xml.i include was used
   before, making sure the new includes (match-ipsec-in/out.xml.i) are
   used appropriately. There were a handful of spots where match-ipsec.xml.i
   had snuck back in for output hooked chains already
   (the common-rule-* includes)
 * Add the -out generators to rendered templates
 * Heavy modification to firewall config validators:
   * I needed to check for ipsec-in matches no matter how deeply nested
     under an output-hook chain(via jump-target) - this always generates
     an error.
   * Ended up retrofitting the jump-targets validator from root chains
     and for named custom chains. It checks for recursive loops and improper
     IPsec matches.
 * Added "test_ipsec_metadata_match" and "test_cyclic_jump_validation"
   smoketests

interface-definitions/include/firewall/common-rule-inet.xml.i

@@ -7,7 +7,6 @@
 #include <include/generic-disable-node.xml.i>
 #include <include/firewall/dscp.xml.i>
 #include <include/firewall/fragment.xml.i>
-#include <include/firewall/match-ipsec.xml.i>
 #include <include/firewall/limit.xml.i>
 #include <include/firewall/log.xml.i>
 #include <include/firewall/log-options.xml.i>

interface-definitions/include/firewall/common-rule-ipv4-raw.xml.i

@@ -9,7 +9,6 @@
 #include <include/firewall/limit.xml.i>
 #include <include/firewall/log.xml.i>
 #include <include/firewall/log-options.xml.i>
-#include <include/firewall/match-ipsec.xml.i>
 #include <include/firewall/protocol.xml.i>
 #include <include/firewall/nft-queue.xml.i>
 #include <include/firewall/recent.xml.i>

interface-definitions/include/firewall/common-rule-ipv6-raw.xml.i

@@ -9,7 +9,6 @@
 #include <include/firewall/limit.xml.i>
 #include <include/firewall/log.xml.i>
 #include <include/firewall/log-options.xml.i>
-#include <include/firewall/match-ipsec.xml.i>
 #include <include/firewall/protocol.xml.i>
 #include <include/firewall/nft-queue.xml.i>
 #include <include/firewall/recent.xml.i>

interface-definitions/include/firewall/ipv4-hook-input.xml.i

@@ -27,7 +27,7 @@
           <children>
             #include <include/firewall/common-rule-ipv4.xml.i>
             #include <include/firewall/inbound-interface.xml.i>
-            #include <include/firewall/match-ipsec.xml.i>
+            #include <include/firewall/match-ipsec-in.xml.i>
           </children>
         </tagNode>
       </children>

interface-definitions/include/firewall/ipv4-hook-output.xml.i

@@ -26,6 +26,7 @@
           </properties>
           <children>
             #include <include/firewall/common-rule-ipv4.xml.i>
+            #include <include/firewall/match-ipsec-out.xml.i>
             #include <include/firewall/outbound-interface.xml.i>
           </children>
         </tagNode>
@@ -53,6 +54,7 @@
           </properties>
           <children>
             #include <include/firewall/common-rule-ipv4-raw.xml.i>
+            #include <include/firewall/match-ipsec-out.xml.i>
             #include <include/firewall/outbound-interface.xml.i>
           </children>
         </tagNode>

interface-definitions/include/firewall/ipv4-hook-prerouting.xml.i

@@ -33,6 +33,7 @@
           </properties>
           <children>
             #include <include/firewall/common-rule-ipv4-raw.xml.i>
+            #include <include/firewall/match-ipsec-in.xml.i>
             #include <include/firewall/inbound-interface.xml.i>
             <leafNode name="jump-target">
               <properties>

interface-definitions/include/firewall/ipv6-hook-input.xml.i

@@ -27,7 +27,7 @@
           <children>
             #include <include/firewall/common-rule-ipv6.xml.i>
             #include <include/firewall/inbound-interface.xml.i>
-            #include <include/firewall/match-ipsec.xml.i>
+            #include <include/firewall/match-ipsec-in.xml.i>
           </children>
         </tagNode>
       </children>

interface-definitions/include/firewall/ipv6-hook-output.xml.i

@@ -26,6 +26,7 @@
           </properties>
           <children>
             #include <include/firewall/common-rule-ipv6.xml.i>
+            #include <include/firewall/match-ipsec-out.xml.i>
             #include <include/firewall/outbound-interface.xml.i>
           </children>
         </tagNode>
@@ -53,6 +54,7 @@
           </properties>
           <children>
             #include <include/firewall/common-rule-ipv6-raw.xml.i>
+            #include <include/firewall/match-ipsec-out.xml.i>
             #include <include/firewall/outbound-interface.xml.i>
           </children>
         </tagNode>

interface-definitions/include/firewall/ipv6-hook-prerouting.xml.i

@@ -33,6 +33,7 @@
           </properties>
           <children>
             #include <include/firewall/common-rule-ipv6-raw.xml.i>
+            #include <include/firewall/match-ipsec-in.xml.i>
             #include <include/firewall/inbound-interface.xml.i>
             <leafNode name="jump-target">
               <properties>

interface-definitions/include/firewall/match-ipsec-in.xml.i

@@ -0,0 +1,21 @@
+<!-- include start from firewall/match-ipsec-in.xml.i -->
+<node name="ipsec">
+  <properties>
+    <help>Inbound IPsec packets</help>
+  </properties>
+  <children>
+    <leafNode name="match-ipsec-in">
+      <properties>
+        <help>Inbound traffic that was IPsec encapsulated</help>
+        <valueless/>
+      </properties>
+    </leafNode>
+    <leafNode name="match-none-in">
+      <properties>
+        <help>Inbound traffic that was not IPsec encapsulated</help>
+        <valueless/>
+      </properties>
+    </leafNode>
+  </children>
+</node>
+<!-- include end -->

interface-definitions/include/firewall/match-ipsec-out.xml.i

@@ -0,0 +1,21 @@
+<!-- include start from firewall/match-ipsec-out.xml.i -->
+<node name="ipsec">
+  <properties>
+    <help>Outbound IPsec packets</help>
+  </properties>
+  <children>
+    <leafNode name="match-ipsec-out">
+      <properties>
+        <help>Outbound traffic to be IPsec encapsulated</help>
+        <valueless/>
+      </properties>
+    </leafNode>
+    <leafNode name="match-none-out">
+      <properties>
+        <help>Outbound traffic that will not be IPsec encapsulated</help>
+        <valueless/>
+      </properties>
+    </leafNode>
+  </children>
+</node>
+<!-- include end -->

interface-definitions/include/firewall/match-ipsec.xml.i

@@ -1,21 +1,33 @@
 <!-- include start from firewall/match-ipsec.xml.i -->
 <node name="ipsec">
   <properties>
-    <help>Inbound IPsec packets</help>
+    <help>IPsec encapsulated packets</help>
   </properties>
   <children>
-    <leafNode name="match-ipsec">
+    <leafNode name="match-ipsec-in">
       <properties>
-        <help>Inbound IPsec packets</help>
+        <help>Inbound traffic that was IPsec encapsulated</help>
         <valueless/>
       </properties>
     </leafNode>
-    <leafNode name="match-none">
+    <leafNode name="match-none-in">
       <properties>
-        <help>Inbound non-IPsec packets</help>
+        <help>Inbound traffic that was not IPsec encapsulated</help>
         <valueless/>
       </properties>
     </leafNode>
+    <leafNode name="match-ipsec-out">
+      <properties>
+        <help>Outbound traffic to be IPsec encapsulated</help>
+        <valueless/>
+      </properties>
+    </leafNode>
+    <leafNode name="match-none-out">
+      <properties>
+        <help>Outbound traffic that will not be IPsec encapsulated</help>
+        <valueless/>
+      </properties>
+    </leafNode>    
   </children>
 </node>
 <!-- include end -->

interface-definitions/include/version/firewall-version.xml.i

@@ -1,3 +1,3 @@
 <!-- include start from include/version/firewall-version.xml.i -->
-<syntaxVersion component='firewall' version='16'></syntaxVersion>
+<syntaxVersion component='firewall' version='17'></syntaxVersion>
 <!-- include end -->

python/vyos/firewall.py

@@ -366,10 +366,14 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
         output.append(f'ip{def_suffix} dscp != {{{negated_dscp_str}}}')
 
     if 'ipsec' in rule_conf:
-        if 'match_ipsec' in rule_conf['ipsec']:
+        if 'match_ipsec_in' in rule_conf['ipsec']:
             output.append('meta ipsec == 1')
-        if 'match_none' in rule_conf['ipsec']:
+        if 'match_none_in' in rule_conf['ipsec']:
             output.append('meta ipsec == 0')
+        if 'match_ipsec_out' in rule_conf['ipsec']:
+            output.append('rt ipsec exists')
+        if 'match_none_out' in rule_conf['ipsec']:
+            output.append('rt ipsec missing')
 
     if 'fragment' in rule_conf:
         # Checking for fragmentation after priority -400 is not possible,

smoketest/scripts/cli/test_firewall.py

@@ -995,5 +995,81 @@ def test_zone_flow_offload(self):
         self.verify_nftables_chain([['accept']], 'ip vyos_conntrack', 'FW_CONNTRACK')
         self.verify_nftables_chain([['accept']], 'ip6 vyos_conntrack', 'FW_CONNTRACK')
 
+    def test_ipsec_metadata_match(self):
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-in4', 'rule', '1', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-in4', 'rule', '1', 'ipsec', 'match-ipsec-in'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-in4', 'rule', '2', 'action', 'drop'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-in4', 'rule', '2', 'ipsec', 'match-none-in'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-out4', 'rule', '1', 'action', 'continue'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-out4', 'rule', '1', 'ipsec', 'match-ipsec-out'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-out4', 'rule', '2', 'action', 'reject'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-out4', 'rule', '2', 'ipsec', 'match-none-out'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-in6', 'rule', '1', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-in6', 'rule', '1', 'ipsec', 'match-ipsec-in'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-in6', 'rule', '2', 'action', 'drop'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-in6', 'rule', '2', 'ipsec', 'match-none-in'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-out6', 'rule', '1', 'action', 'continue'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-out6', 'rule', '1', 'ipsec', 'match-ipsec-out'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-out6', 'rule', '2', 'action', 'reject'])
+        self.cli_set(['firewall', 'ipv6', 'name', 'smoketest-ipsec-out6', 'rule', '2', 'ipsec', 'match-none-out'])
+
+        self.cli_commit()
+
+        nftables_search = [
+            ['meta ipsec exists', 'accept comment'],
+            ['meta ipsec missing', 'drop comment'],
+            ['rt ipsec exists', 'continue comment'],
+            ['rt ipsec missing', 'reject comment'],
+        ]
+
+        self.verify_nftables(nftables_search, 'ip vyos_filter')
+        self.verify_nftables(nftables_search, 'ip6 vyos_filter')
+
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'jump-target', 'smoketest-ipsec-in4'])
+        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'jump-target', 'smoketest-ipsec-in4'])
+        self.cli_set(['firewall', 'ipv4', 'prerouting', 'raw', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'prerouting', 'raw', 'rule', '1', 'jump-target', 'smoketest-ipsec-in4'])
+
+        self.cli_set(['firewall', 'ipv4', 'output', 'filter', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'output', 'filter', 'rule', '1', 'jump-target', 'smoketest-ipsec-out4'])
+        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'jump-target', 'smoketest-ipsec-out4'])
+
+        # All valid directional usage of ipsec matches
+        self.cli_commit()
+
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-in-indirect', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-ipsec-in-indirect', 'rule', '1', 'jump-target', 'smoketest-ipsec-in4'])
+
+        self.cli_set(['firewall', 'ipv4', 'output', 'filter', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'output', 'filter', 'rule', '1', 'jump-target', 'smoketest-ipsec-in-indirect'])
+
+        # nft does not support ANY usage of 'meta ipsec' under an output hook, it will fail to load cfg
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+
+    def test_cyclic_jump_validation(self):
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-1', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-1', 'rule', '1', 'jump-target', 'smoketest-cycle-2'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-2', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-2', 'rule', '1', 'jump-target', 'smoketest-cycle-3'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-3', 'rule', '1', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-3', 'rule', '1', 'log'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'jump-target', 'smoketest-cycle-1'])
+
+        # Multi-level jumps are unwise but allowed
+        self.cli_commit()
+
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-3', 'rule', '1', 'action', 'jump'])
+        self.cli_set(['firewall', 'ipv4', 'name', 'smoketest-cycle-3', 'rule', '1', 'jump-target', 'smoketest-cycle-1'])
+
+        # nft will fail to load cyclic jumps in any form, whether the rule is reachable or not. 
+        # It should be caught by conf validation. 
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)

src/conf_mode/firewall.py

@@ -128,7 +128,49 @@ def get_config(config=None):
 
     return firewall
 
-def verify_rule(firewall, rule_conf, ipv6):
+def verify_jump_target(firewall, root_chain, jump_target, ipv6, recursive=False):
+    targets_seen = []
+    targets_pending = [jump_target]
+
+    while targets_pending:
+        target = targets_pending.pop()
+
+        if not ipv6:
+            if target not in dict_search_args(firewall, 'ipv4', 'name'):
+                raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
+            target_rules = dict_search_args(firewall, 'ipv4', 'name', target, 'rule')
+        else:
+            if target not in dict_search_args(firewall, 'ipv6', 'name'):
+                raise ConfigError(f'Invalid jump-target. Firewall ipv6 name {target} does not exist on the system')
+            target_rules = dict_search_args(firewall, 'ipv6', 'name', target, 'rule')
+
+        no_ipsec_in = root_chain in ('output', )
+
+        if target_rules:
+            for target_rule_conf in target_rules.values():
+                # Output hook types will not tolerate 'meta ipsec exists' matches even in jump targets:
+                if no_ipsec_in and (dict_search_args(target_rule_conf, 'ipsec', 'match_ipsec_in') is not None \
+                                    or dict_search_args(target_rule_conf, 'ipsec', 'match_none_in') is not None):
+                    if not ipv6:
+                        raise ConfigError(f'Invalid jump-target for {root_chain}. Firewall name {target} rules contain incompatible ipsec inbound matches')
+                    else:
+                        raise ConfigError(f'Invalid jump-target for {root_chain}. Firewall ipv6 name {target} rules contain incompatible ipsec inbound matches')
+                # Make sure we're not looping back on ourselves somewhere:
+                if recursive and 'jump_target' in target_rule_conf:
+                    child_target = target_rule_conf['jump_target']
+                    if child_target in targets_seen:
+                        if not ipv6:
+                            raise ConfigError(f'Loop detected in jump-targets, firewall name {target} refers to previously traversed name {child_target}')
+                        else:
+                            raise ConfigError(f'Loop detected in jump-targets, firewall ipv6 name {target} refers to previously traversed ipv6 name {child_target}')
+                    targets_pending.append(child_target)
+                    if len(targets_seen) == 7:
+                        path_txt = ' -> '.join(targets_seen)
+                        Warning(f'Deep nesting of jump targets has reached 8 levels deep, following the path {path_txt} -> {child_target}!')
+
+        targets_seen.append(target)
+
+def verify_rule(firewall, chain_name, rule_conf, ipv6):
     if 'action' not in rule_conf:
         raise ConfigError('Rule action must be defined')
 
@@ -139,12 +181,10 @@ def verify_rule(firewall, rule_conf, ipv6):
         if 'jump' not in rule_conf['action']:
             raise ConfigError('jump-target defined, but action jump needed and it is not defined')
         target = rule_conf['jump_target']
-        if not ipv6:
-            if target not in dict_search_args(firewall, 'ipv4', 'name'):
-                raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
+        if chain_name != 'name': # This is a bit clumsy, but consolidates a chunk of code. 
+            verify_jump_target(firewall, chain_name, target, ipv6, recursive=True)
         else:
-            if target not in dict_search_args(firewall, 'ipv6', 'name'):
-                raise ConfigError(f'Invalid jump-target. Firewall ipv6 name {target} does not exist on the system')
+            verify_jump_target(firewall, chain_name, target, ipv6, recursive=False)
 
     if rule_conf['action'] == 'offload':
         if 'offload_target' not in rule_conf:
@@ -185,8 +225,10 @@ def verify_rule(firewall, rule_conf, ipv6):
                 raise ConfigError('Limit rate integer cannot be less than 1')
 
     if 'ipsec' in rule_conf:
-        if {'match_ipsec', 'match_non_ipsec'} <= set(rule_conf['ipsec']):
-            raise ConfigError('Cannot specify both "match-ipsec" and "match-non-ipsec"')
+        if {'match_ipsec_in', 'match_none_in'} <= set(rule_conf['ipsec']):
+            raise ConfigError('Cannot specify both "match-ipsec" and "match-none"')
+        if {'match_ipsec_out', 'match_none_out'} <= set(rule_conf['ipsec']):
+            raise ConfigError('Cannot specify both "match-ipsec" and "match-none"')
 
     if 'recent' in rule_conf:
         if not {'count', 'time'} <= set(rule_conf['recent']):
@@ -349,13 +391,11 @@ def verify(firewall):
                             raise ConfigError('default-jump-target defined, but default-action jump needed and it is not defined')
                         if name_conf['default_jump_target'] == name_id:
                             raise ConfigError(f'Loop detected on default-jump-target.')
-                        ## Now need to check that default-jump-target exists (other firewall chain/name)
-                        if target not in dict_search_args(firewall['ipv4'], 'name'):
-                            raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
+                        verify_jump_target(firewall, name, target, False, recursive=True)
 
                     if 'rule' in name_conf:
                         for rule_id, rule_conf in name_conf['rule'].items():
-                            verify_rule(firewall, rule_conf, False)
+                            verify_rule(firewall, name, rule_conf, False)
 
     if 'ipv6' in firewall:
         for name in ['name','forward','input','output', 'prerouting']:
@@ -369,13 +409,11 @@ def verify(firewall):
                             raise ConfigError('default-jump-target defined, but default-action jump needed and it is not defined')
                         if name_conf['default_jump_target'] == name_id:
                             raise ConfigError(f'Loop detected on default-jump-target.')
-                        ## Now need to check that default-jump-target exists (other firewall chain/name)
-                        if target not in dict_search_args(firewall['ipv6'], 'name'):
-                            raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
+                        verify_jump_target(firewall, name, target, True, recursive=True)
 
                     if 'rule' in name_conf:
                         for rule_id, rule_conf in name_conf['rule'].items():
-                            verify_rule(firewall, rule_conf, True)
+                            verify_rule(firewall, name, rule_conf, True)
 
     #### ZONESSSS
     local_zone = False