Merge pull request #2597 from GurliGebis/T5775-sagitta-backport

T5775: firewall: re-add state-policy to firewall. (manual backport #2539)

8 files changed, 141 insertions, 21 deletions

diff --git a/data/templates/firewall/nftables-zone.j2 b/data/templates/firewall/nftables-zone.j2
index ee468c6c1..506ad815e 100644
--- a/data/templates/firewall/nftables-zone.j2
+++ b/data/templates/firewall/nftables-zone.j2
@@ -1,8 +1,11 @@
-{% macro zone_chains(zone, ipv6=False) %}
+{% macro zone_chains(zone, ipv6=False, state_policy=False) %}
 {% set fw_name = 'ipv6_name' if ipv6 else 'name' %}
 {% set suffix = '6' if ipv6 else '' %}
     chain VYOS_ZONE_FORWARD {
         type filter hook forward priority 1; policy accept;
+{% if state_policy %}
+        jump VYOS_STATE_POLICY{{ suffix }}
+{% endif %}
 {% for zone_name, zone_conf in zone.items() %}
 {%     if 'local_zone' not in zone_conf %}
         oifname { {{ zone_conf.interface | join(',') }} } counter jump VZONE_{{ zone_name }}
@@ -11,6 +14,9 @@
     }
     chain VYOS_ZONE_LOCAL {
         type filter hook input priority 1; policy accept;
+{% if state_policy %}
+        jump VYOS_STATE_POLICY{{ suffix }}
+{% endif %}
 {% for zone_name, zone_conf in zone.items() %}
 {%     if 'local_zone' in zone_conf %}
         counter jump VZONE_{{ zone_name }}_IN
@@ -19,6 +25,9 @@
     }
     chain VYOS_ZONE_OUTPUT {
         type filter hook output priority 1; policy accept;
+{% if state_policy %}
+        jump VYOS_STATE_POLICY{{ suffix }}
+{% endif %}
 {% for zone_name, zone_conf in zone.items() %}
 {%     if 'local_zone' in zone_conf %}
         counter jump VZONE_{{ zone_name }}_OUT
diff --git a/data/templates/firewall/nftables.j2 b/data/templates/firewall/nftables.j2
index 91c4d0788..3f7906628 100644
--- a/data/templates/firewall/nftables.j2
+++ b/data/templates/firewall/nftables.j2
@@ -50,6 +50,9 @@ table ip vyos_filter {
 {%         for prior, conf in ipv4.forward.items() %}
     chain VYOS_FORWARD_{{ prior }} {
         type filter hook forward priority {{ prior }}; policy accept;
+{%             if global_options.state_policy is vyos_defined %}
+        jump VYOS_STATE_POLICY
+{%             endif %}
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
         {{ rule_conf | nft_rule('FWD', prior, rule_id) }}
@@ -67,6 +70,9 @@ table ip vyos_filter {
 {%         for prior, conf in ipv4.input.items() %}
     chain VYOS_INPUT_{{ prior }} {
         type filter hook input priority {{ prior }}; policy accept;
+{%             if global_options.state_policy is vyos_defined %}
+        jump VYOS_STATE_POLICY
+{%             endif %}
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
         {{ rule_conf | nft_rule('INP',prior, rule_id) }}
@@ -84,6 +90,9 @@ table ip vyos_filter {
 {%         for prior, conf in ipv4.output.items() %}
     chain VYOS_OUTPUT_{{ prior }} {
         type filter hook output priority {{ prior }}; policy accept;
+{%             if global_options.state_policy is vyos_defined %}
+        jump VYOS_STATE_POLICY
+{%             endif %}
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
         {{ rule_conf | nft_rule('OUT', prior, rule_id) }}
@@ -158,7 +167,21 @@ table ip vyos_filter {
 {{ group_tmpl.groups(group, False, True) }}
 
 {% if zone is vyos_defined %}
-{{ zone_tmpl.zone_chains(zone, False) }}
+{{ zone_tmpl.zone_chains(zone, False, global_options.state_policy is vyos_defined) }}
+{% endif %}
+{% if global_options.state_policy is vyos_defined %}
+    chain VYOS_STATE_POLICY {
+{%     if global_options.state_policy.established is vyos_defined %}
+        {{ global_options.state_policy.established | nft_state_policy('established') }}
+{%     endif %}
+{%     if global_options.state_policy.invalid is vyos_defined %}
+        {{ global_options.state_policy.invalid | nft_state_policy('invalid') }}
+{%     endif %}
+{%     if global_options.state_policy.related is vyos_defined %}
+        {{ global_options.state_policy.related | nft_state_policy('related') }}
+{%     endif %}
+        return
+    }
 {% endif %}
 }
 
@@ -178,6 +201,9 @@ table ip6 vyos_filter {
 {%         for prior, conf in ipv6.forward.items() %}
     chain VYOS_IPV6_FORWARD_{{ prior }} {
         type filter hook forward priority {{ prior }}; policy accept;
+{%             if global_options.state_policy is vyos_defined %}
+        jump VYOS_STATE_POLICY6
+{%             endif %}
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
         {{ rule_conf | nft_rule('FWD', prior, rule_id ,'ip6') }}
@@ -195,6 +221,9 @@ table ip6 vyos_filter {
 {%         for prior, conf in ipv6.input.items() %}
     chain VYOS_IPV6_INPUT_{{ prior }} {
         type filter hook input priority {{ prior }}; policy accept;
+{%             if global_options.state_policy is vyos_defined %}
+        jump VYOS_STATE_POLICY6
+{%             endif %}
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
         {{ rule_conf | nft_rule('INP', prior, rule_id ,'ip6') }}
@@ -212,6 +241,9 @@ table ip6 vyos_filter {
 {%         for prior, conf in ipv6.output.items() %}
     chain VYOS_IPV6_OUTPUT_{{ prior }} {
         type filter hook output priority {{ prior }}; policy accept;
+{%             if global_options.state_policy is vyos_defined %}
+        jump VYOS_STATE_POLICY6
+{%             endif %}
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
         {{ rule_conf | nft_rule('OUT', prior, rule_id ,'ip6') }}
@@ -271,7 +303,21 @@ table ip6 vyos_filter {
 {{ group_tmpl.groups(group, True, True) }}
 
 {% if zone is vyos_defined %}
-{{ zone_tmpl.zone_chains(zone, True) }}
+{{ zone_tmpl.zone_chains(zone, True, global_options.state_policy is vyos_defined) }}
+{% endif %}
+{% if global_options.state_policy is vyos_defined %}
+    chain VYOS_STATE_POLICY6 {
+{%     if global_options.state_policy.established is vyos_defined %}
+        {{ global_options.state_policy.established | nft_state_policy('established') }}
+{%     endif %}
+{%     if global_options.state_policy.invalid is vyos_defined %}
+        {{ global_options.state_policy.invalid | nft_state_policy('invalid') }}
+{%     endif %}
+{%     if global_options.state_policy.related is vyos_defined %}
+        {{ global_options.state_policy.related | nft_state_policy('related') }}
+{%     endif %}
+        return
+    }
 {% endif %}
 
 }
diff --git a/interface-definitions/firewall.xml.in b/interface-definitions/firewall.xml.in
index 0bb14a1b3..70afdc995 100644
--- a/interface-definitions/firewall.xml.in
+++ b/interface-definitions/firewall.xml.in
@@ -393,7 +393,7 @@
             <properties>
               <help>Zone from which to filter traffic</help>
               <completionHelp>
-                <path>zone-policy zone</path>
+                <path>firewall zone</path>
               </completionHelp>
             </properties>
             <children>
diff --git a/interface-definitions/include/firewall/global-options.xml.i b/interface-definitions/include/firewall/global-options.xml.i
index a63874cb0..3026b54ab 100644
--- a/interface-definitions/include/firewall/global-options.xml.i
+++ b/interface-definitions/include/firewall/global-options.xml.i
@@ -167,6 +167,43 @@
       </properties>
       <defaultValue>disable</defaultValue>
     </leafNode>
+    <node name="state-policy">
+      <properties>
+        <help>Global firewall state-policy</help>
+      </properties>
+      <children>
+        <node name="established">
+          <properties>
+            <help>Global firewall policy for packets part of an established connection</help>
+          </properties>
+          <children>
+            #include <include/firewall/action-accept-drop-reject.xml.i>
+            #include <include/firewall/log.xml.i>
+            #include <include/firewall/rule-log-level.xml.i>
+          </children>
+        </node>
+        <node name="invalid">
+          <properties>
+            <help>Global firewall policy for packets part of an invalid connection</help>
+          </properties>
+          <children>
+            #include <include/firewall/action-accept-drop-reject.xml.i>
+            #include <include/firewall/log.xml.i>
+            #include <include/firewall/rule-log-level.xml.i>
+          </children>
+        </node>
+        <node name="related">
+          <properties>
+            <help>Global firewall policy for packets part of a related connection</help>
+          </properties>
+          <children>
+            #include <include/firewall/action-accept-drop-reject.xml.i>
+            #include <include/firewall/log.xml.i>
+            #include <include/firewall/rule-log-level.xml.i>
+          </children>
+        </node>
+      </children>
+    </node>
     <leafNode name="syn-cookies">
       <properties>
         <help>Policy for using TCP SYN cookies with IPv4</help>
diff --git a/python/vyos/template.py b/python/vyos/template.py
index 320abe256..216f967a7 100644
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -602,7 +602,7 @@ def nft_default_rule(fw_conf, fw_name, ipv6=False):
 def nft_state_policy(conf, state):
     out = [f'ct state {state}']
 
-    if 'log' in conf and 'enable' in conf['log']:
+    if 'log' in conf:
         log_state = state[:3].upper()
         log_action = (conf['action'] if 'action' in conf else 'accept')[:1].upper()
         out.append(f'log prefix "[STATE-POLICY-{log_state}-{log_action}]"')
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index 85b2fbbc7..980b50556 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -382,6 +382,10 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         name = 'v6-smoketest'
         interface = 'eth0'
 
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'established', 'action', 'accept'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'related', 'action', 'accept'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'invalid', 'action', 'drop'])
+
         self.cli_set(['firewall', 'ipv6', 'name', name, 'default-action', 'drop'])
         self.cli_set(['firewall', 'ipv6', 'name', name, 'enable-default-log'])
 
@@ -426,7 +430,12 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
             ['log prefix "[ipv6-OUT-filter-default-D]"','OUT-filter default-action drop', 'drop'],
             [f'chain NAME6_{name}'],
             ['saddr 2002::1', 'daddr 2002::1:1', 'log prefix "[ipv6-NAM-v6-smoketest-1-A]" log level crit', 'accept'],
-            [f'"{name} default-action drop"', f'log prefix "[ipv6-{name}-default-D]"', 'drop']
+            [f'"{name} default-action drop"', f'log prefix "[ipv6-{name}-default-D]"', 'drop'],
+            ['jump VYOS_STATE_POLICY6'],
+            ['chain VYOS_STATE_POLICY6'],
+            ['ct state established', 'accept'],
+            ['ct state invalid', 'drop'],
+            ['ct state related', 'accept']
         ]
 
         self.verify_nftables(nftables_search, 'ip6 vyos_filter')
@@ -508,6 +517,10 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         name = 'smoketest-state'
         interface = 'eth0'
 
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'established', 'action', 'accept'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'related', 'action', 'accept'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'invalid', 'action', 'drop'])
+
         self.cli_set(['firewall', 'ipv4', 'name', name, 'default-action', 'drop'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'action', 'accept'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'state', 'established'])
@@ -534,7 +547,12 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
             ['ct state new', 'ct status dnat', 'accept'],
             ['ct state { established, new }', 'ct status snat', 'accept'],
             ['ct state related', 'ct helper { "ftp", "pptp" }', 'accept'],
-            ['drop', f'comment "{name} default-action drop"']
+            ['drop', f'comment "{name} default-action drop"'],
+            ['jump VYOS_STATE_POLICY'],
+            ['chain VYOS_STATE_POLICY'],
+            ['ct state established', 'accept'],
+            ['ct state invalid', 'drop'],
+            ['ct state related', 'accept']
         ]
 
         self.verify_nftables(nftables_search, 'ip vyos_filter')
@@ -623,6 +641,10 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'zone', 'smoketest-eth0', 'from', 'smoketest-local', 'firewall', 'name', 'smoketest'])
         self.cli_set(['firewall', 'zone', 'smoketest-local', 'local-zone'])
         self.cli_set(['firewall', 'zone', 'smoketest-local', 'from', 'smoketest-eth0', 'firewall', 'name', 'smoketest'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'established', 'action', 'accept'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'established', 'log'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'related', 'action', 'accept'])
+        self.cli_set(['firewall', 'global-options', 'state-policy', 'invalid', 'action', 'drop'])
 
         self.cli_commit()
 
@@ -640,7 +662,12 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
             ['jump VZONE_smoketest-local_IN'],
             ['jump VZONE_smoketest-local_OUT'],
             ['iifname "eth0"', 'jump NAME_smoketest'],
-            ['oifname "eth0"', 'jump NAME_smoketest']
+            ['oifname "eth0"', 'jump NAME_smoketest'],
+            ['jump VYOS_STATE_POLICY'],
+            ['chain VYOS_STATE_POLICY'],
+            ['ct state established', 'log prefix "[STATE-POLICY-EST-A]"', 'accept'],
+            ['ct state invalid', 'drop'],
+            ['ct state related', 'accept']
         ]
 
         nftables_output = cmd('sudo nft list table ip vyos_filter')
diff --git a/src/migration-scripts/firewall/10-to-11 b/src/migration-scripts/firewall/10-to-11
index b739fb139..e14ea0e51 100755
--- a/src/migration-scripts/firewall/10-to-11
+++ b/src/migration-scripts/firewall/10-to-11
@@ -63,19 +63,11 @@ if not config.exists(base):
 
 ### Migration of state policies
 if config.exists(base + ['state-policy']):
-    for family in ['ipv4', 'ipv6']:
-        for hook in ['forward', 'input', 'output']:
-            for priority in ['filter']:
-                # Add default-action== accept for compatibility reasons:
-                config.set(base + [family, hook, priority, 'default-action'], value='accept')
-                position = 1
-                for state in config.list_nodes(base + ['state-policy']):
-                    action = config.return_value(base + ['state-policy', state, 'action'])
-                    config.set(base + [family, hook, priority, 'rule'])
-                    config.set_tag(base + [family, hook, priority, 'rule'])
-                    config.set(base + [family, hook, priority, 'rule', position, 'state', state], value='enable')
-                    config.set(base + [family, hook, priority, 'rule', position, 'action'], value=action)
-                    position = position + 1
+    for state in config.list_nodes(base + ['state-policy']):
+        action = config.return_value(base + ['state-policy', state, 'action'])
+        config.set(base + ['global-options', 'state-policy', state, 'action'], value=action)
+        if config.exists(base + ['state-policy', state, 'log']):
+            config.set(base + ['global-options', 'state-policy', state, 'log'], value='enable')
     config.delete(base + ['state-policy'])
 
 ## migration of global options:
diff --git a/src/migration-scripts/firewall/12-to-13 b/src/migration-scripts/firewall/12-to-13
index 4eaae779b..8396dd9d1 100755
--- a/src/migration-scripts/firewall/12-to-13
+++ b/src/migration-scripts/firewall/12-to-13
@@ -49,6 +49,15 @@ if not config.exists(base):
     # Nothing to do
     exit(0)
 
+# State Policy logs:
+if config.exists(base + ['global-options', 'state-policy']):
+    for state in config.list_nodes(base + ['global-options', 'state-policy']):
+        if config.exists(base + ['global-options', 'state-policy', state, 'log']):
+            log_value = config.return_value(base + ['global-options', 'state-policy', state, 'log'])
+            config.delete(base + ['global-options', 'state-policy', state, 'log'])
+            if log_value == 'enable':
+                config.set(base + ['global-options', 'state-policy', state, 'log'])
+
 for family in ['ipv4', 'ipv6', 'bridge']:
     if config.exists(base + [family]):
         for hook in ['forward', 'input', 'output', 'name']: