Merge pull request #5364 from l0crian1/normalize-proto-values

firewall: T8247: Normalize protocol values to protocol names

2 files changed, 58 insertions, 6 deletions

diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index d8f29d4d5..f006c30d8 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -1687,5 +1687,34 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         ]
         self.verify_nftables(nftables_search, 'ip vyos_filter')
 
+    def test_protocol_normalization(self):
+        # Protocol cannot be ipv6-icmp on IPv4
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'protocol', '58'])
+
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+
+        self.cli_discard()
+
+        # Protocol must be TCP if flags are set
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'protocol', '6'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '1', 'tcp', 'flags', 'syn'])
+
+        # Protocol must be GRE if GRE specific fields are set
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'protocol', '47'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'gre', 'flags', 'key', 'unset'])
+
+        self.cli_commit()
+
+        nftables_search = [
+            ['tcp flags & syn == syn'],
+            ['gre flags & 4 == 0'],
+        ]
+        self.verify_nftables(nftables_search, 'ip vyos_filter')
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index eacded5ef..32356f6d7 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -106,6 +106,24 @@ def geoip_updated(conf):
     return any(any(dict_search_recursive(diff.get(section, {}), 'geoip'))
                for section in ('add', 'delete'))
 
+def generate_inet_proto_keymap(firewall):
+    """
+    Build a keymap of IP protocol numbers to protocol names from
+    `nft describe inet_proto`.
+
+    Returns:
+        dict[int, str]: Mapping of protocol number -> protocol name.
+    """
+    keymap = {}
+
+    for line in cmdl(['nft', 'describe', 'inet_proto']).splitlines():
+        match = re.match(r'^\s*([A-Za-z0-9_.-]+)\s+(\d+)\s*$', line)
+        if match:
+            name, number = match.groups()
+            keymap[number] = name
+
+    firewall['inet_proto_map'] = keymap
+
 def get_config(config=None):
     if config:
         conf = config
@@ -261,6 +279,17 @@ def verify_rule(firewall, family, hook, priority, rule_id, rule_conf):
     elif 'offload_target' in rule_conf:
         Warning(f'{rule_num}offload-target is specified but action is not set to "offload"')
 
+    if 'protocol' in rule_conf:
+        if rule_conf['protocol'].isdigit():
+            if not dict_search('inet_proto_map', firewall):
+                generate_inet_proto_keymap(firewall)
+            rule_conf['protocol'] = dict_search(rule_conf['protocol'], firewall['inet_proto_map'], rule_conf['protocol'])
+
+        if rule_conf['protocol'] == 'icmp' and family == 'ipv6':
+            raise ConfigError(f'{rule_num}Cannot match IPv4 ICMP protocol on IPv6, use ipv6-icmp')
+        if rule_conf['protocol'] == 'ipv6-icmp' and family == 'ipv4':
+            raise ConfigError(f'{rule_num}Cannot match IPv6 ICMP protocol on IPv4, use icmp')
+
     if rule_conf['action'] != 'synproxy' and 'synproxy' in rule_conf:
         raise ConfigError(f'{rule_num}"synproxy" option allowed only for action synproxy')
     if rule_conf['action'] == 'synproxy':
@@ -364,12 +393,6 @@ def verify_rule(firewall, family, hook, priority, rule_id, rule_conf):
             if duplicates:
                 raise ConfigError(f'{rule_num}Cannot match a tcp flag as set and not set')
 
-    if 'protocol' in rule_conf:
-        if rule_conf['protocol'] == 'icmp' and family == 'ipv6':
-            raise ConfigError(f'{rule_num}Cannot match IPv4 ICMP protocol on IPv6, use ipv6-icmp')
-        if rule_conf['protocol'] == 'ipv6-icmp' and family == 'ipv4':
-            raise ConfigError(f'{rule_num}Cannot match IPv6 ICMP protocol on IPv4, use icmp')
-
     for side in ['destination', 'source']:
         if side in rule_conf:
             side_conf = rule_conf[side]