Merge pull request #1134 from sarthurdev/firewall

firewall: zone-policy: T2199: T4130: Fixes for firewall, state-policy and zone-policy

3 files changed, 27 insertions, 24 deletions

diff --git a/src/conf_mode/firewall-interface.py b/src/conf_mode/firewall-interface.py
index 516fa6c48..b0df9dff4 100755
--- a/src/conf_mode/firewall-interface.py
+++ b/src/conf_mode/firewall-interface.py
@@ -32,13 +32,13 @@ from vyos import airbag
 airbag.enable()
 
 NFT_CHAINS = {
-    'in': 'VYOS_FW_IN',
-    'out': 'VYOS_FW_OUT',
+    'in': 'VYOS_FW_FORWARD',
+    'out': 'VYOS_FW_FORWARD',
     'local': 'VYOS_FW_LOCAL'
 }
 NFT6_CHAINS = {
-    'in': 'VYOS_FW6_IN',
-    'out': 'VYOS_FW6_OUT',
+    'in': 'VYOS_FW6_FORWARD',
+    'out': 'VYOS_FW6_FORWARD',
     'local': 'VYOS_FW6_LOCAL'
 }
 
@@ -91,11 +91,11 @@ def verify(if_firewall):
 def generate(if_firewall):
     return None
 
-def cleanup_rule(table, chain, ifname, new_name=None):
+def cleanup_rule(table, chain, prefix, ifname, new_name=None):
     results = cmd(f'nft -a list chain {table} {chain}').split("\n")
     retval = None
     for line in results:
-        if f'ifname "{ifname}"' in line:
+        if f'{prefix}ifname "{ifname}"' in line:
             if new_name and f'jump {new_name}' in line:
                 # new_name is used to clear rules for any previously referenced chains
                 # returns true when rule exists and doesn't need to be created
@@ -108,6 +108,7 @@ def cleanup_rule(table, chain, ifname, new_name=None):
     return retval
 
 def state_policy_handle(table, chain):
+    # Find any state-policy rule to ensure interface rules are only inserted afterwards
     results = cmd(f'nft -a list chain {table} {chain}').split("\n")
     for line in results:
         if 'jump VYOS_STATE_POLICY' in line:
@@ -126,11 +127,12 @@ def apply(if_firewall):
 
         name = dict_search_args(if_firewall, direction, 'name')
         if name:
-            rule_exists = cleanup_rule('ip filter', chain, ifname, name)
-            rule_action = 'insert'
-            rule_prefix = ''
+            rule_exists = cleanup_rule('ip filter', chain, if_prefix, ifname, name)
 
             if not rule_exists:
+                rule_action = 'insert'
+                rule_prefix = ''
+
                 handle = state_policy_handle('ip filter', chain)
                 if handle:
                     rule_action = 'add'
@@ -138,15 +140,16 @@ def apply(if_firewall):
 
                 run(f'nft {rule_action} rule ip filter {chain} {rule_prefix} {if_prefix}ifname {ifname} counter jump {name}')
         else:
-            cleanup_rule('ip filter', chain, ifname)
+            cleanup_rule('ip filter', chain, if_prefix, ifname)
 
         ipv6_name = dict_search_args(if_firewall, direction, 'ipv6_name')
         if ipv6_name:
-            rule_exists = cleanup_rule('ip6 filter', ipv6_chain, ifname, ipv6_name)
-            rule_action = 'insert'
-            rule_prefix = ''
+            rule_exists = cleanup_rule('ip6 filter', ipv6_chain, if_prefix, ifname, ipv6_name)
 
             if not rule_exists:
+                rule_action = 'insert'
+                rule_prefix = ''
+
                 handle = state_policy_handle('ip filter', chain)
                 if handle:
                     rule_action = 'add'
@@ -154,7 +157,7 @@ def apply(if_firewall):
 
                 run(f'nft {rule_action} rule ip6 filter {ipv6_chain} {rule_prefix} {if_prefix}ifname {ifname} counter jump {ipv6_name}')
         else:
-            cleanup_rule('ip6 filter', ipv6_chain, ifname)
+            cleanup_rule('ip6 filter', ipv6_chain, if_prefix, ifname)
 
     return None
 
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index 8e037c679..6016d94fa 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -53,14 +53,12 @@ preserve_chains = [
     'INPUT',
     'FORWARD',
     'OUTPUT',
-    'VYOS_FW_IN',
-    'VYOS_FW_OUT',
+    'VYOS_FW_FORWARD',
     'VYOS_FW_LOCAL',
     'VYOS_FW_OUTPUT',
     'VYOS_POST_FW',
     'VYOS_FRAG_MARK',
-    'VYOS_FW6_IN',
-    'VYOS_FW6_OUT',
+    'VYOS_FW6_FORWARD',
     'VYOS_FW6_LOCAL',
     'VYOS_FW6_OUTPUT',
     'VYOS_POST_FW6',
@@ -228,7 +226,7 @@ def cleanup_commands(firewall):
                         commands.append(f'delete chain {table} {chain}')
             elif 'rule' in item:
                 rule = item['rule']
-                if rule['chain'] in ['VYOS_FW_IN', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL', 'VYOS_FW6_IN', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']:
+                if rule['chain'] in ['VYOS_FW_FORWARD', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL', 'VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']:
                     if 'expr' in rule and any([True for expr in rule['expr'] if dict_search_args(expr, 'jump', 'target') == state_chain]):
                         if 'state_policy' not in firewall:
                             chain = rule['chain']
@@ -303,7 +301,7 @@ def post_apply_trap(firewall):
 
 def state_policy_rule_exists():
     # Determine if state policy rules already exist in nft
-    search_str = cmd(f'nft list chain ip filter VYOS_FW_IN')
+    search_str = cmd(f'nft list chain ip filter VYOS_FW_FORWARD')
     return 'VYOS_STATE_POLICY' in search_str
 
 def apply(firewall):
@@ -317,10 +315,10 @@ def apply(firewall):
         raise ConfigError('Failed to apply firewall')
 
     if 'state_policy' in firewall and not state_policy_rule_exists():
-        for chain in ['VYOS_FW_IN', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL']:
+        for chain in ['VYOS_FW_FORWARD', 'VYOS_FW_OUTPUT', 'VYOS_FW_LOCAL']:
             cmd(f'nft insert rule ip filter {chain} jump VYOS_STATE_POLICY')
 
-        for chain in ['VYOS_FW6_IN', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']:
+        for chain in ['VYOS_FW6_FORWARD', 'VYOS_FW6_OUTPUT', 'VYOS_FW6_LOCAL']:
             cmd(f'nft insert rule ip6 filter {chain} jump VYOS_STATE_POLICY6')
 
     apply_sysfs(firewall)
diff --git a/src/conf_mode/zone_policy.py b/src/conf_mode/zone_policy.py
index 2535ea33b..d605e9639 100755
--- a/src/conf_mode/zone_policy.py
+++ b/src/conf_mode/zone_policy.py
@@ -152,7 +152,9 @@ def cleanup_commands():
                     continue
                 for expr in item['rule']['expr']:
                     target = dict_search_args(expr, 'jump', 'target')
-                    if target and target.startswith("VZONE"):
+                    if not target:
+                        continue
+                    if target.startswith("VZONE") or target.startswith("VYOS_STATE_POLICY"):
                         commands.append(f'delete rule {table} {chain} handle {handle}')
         for item in obj['nftables']:
             if 'chain' in item:
@@ -180,7 +182,7 @@ def generate(zone_policy):
 
 def apply(zone_policy):
     install_result = run(f'nft -f {nftables_conf}')
-    if install_result == 1:
+    if install_result != 0:
         raise ConfigError('Failed to apply zone-policy')
 
     return None