vrf: T7544: Ensure correct quoting for VRF ifnames in nftables

* For VRF create/delete:
  * Simple dquoting, as before, was parsed away by the shell
  * Just escaping the double quotes could cause issues with the shell mangling
    VRF names (however unlikely)
  * Wrapping original quotes in shell-escaped single quotes is a quick & easy
    way to guard against both improper shell parsing and string names being
    taken as nft keywords.

* Firewall configuration:
  * Firewall "interface name" rules support VRF ifnames and used them unquoted,
    fixed for nft_rule template tags (parse_rule)
  * Went through and quoted all iif/oifname usage by zones and interface
    groups. VRF ifnames weren't available for all cases, but there is
    no harm in completeness.
  * For this, also created a simple quoted_join template filter to replace
    any use of |join(',')

* PBR calls nft but doesn't mind the "vni" name - table IDs used instead

I may have missed some niche nft use-cases that would be exposed to this problem.

6 files changed, 29 insertions, 25 deletions

diff --git a/data/templates/firewall/nftables-defines.j2 b/data/templates/firewall/nftables-defines.j2
index a1d1fa4f6..c4b6b7eba 100644
--- a/data/templates/firewall/nftables-defines.j2
+++ b/data/templates/firewall/nftables-defines.j2
@@ -111,7 +111,7 @@
         flags interval
         auto-merge
 {%             if group_conf.interface is vyos_defined or includes %}
-        elements = { {{ group_conf.interface | nft_nested_group(includes, group.interface_group, 'interface') | join(",") }} }
+        elements = { {{ group_conf.interface | nft_nested_group(includes, group.interface_group, 'interface') | quoted_join(",") }} }
 {%             endif %}
     }
 {%         endfor %}
diff --git a/data/templates/firewall/nftables-zone.j2 b/data/templates/firewall/nftables-zone.j2
index 645a38706..66f7e0b1c 100644
--- a/data/templates/firewall/nftables-zone.j2
+++ b/data/templates/firewall/nftables-zone.j2
@@ -9,11 +9,11 @@
 {% for zone_name, zone_conf in zone.items() %}
 {%     if 'local_zone' not in zone_conf %}
 {%         if 'interface' in zone_conf.member %}
-        oifname { {{ zone_conf.member.interface | join(',') }} } counter jump VZONE_{{ zone_name }}
+        oifname { {{ zone_conf.member.interface | quoted_join(',') }} } counter jump VZONE_{{ zone_name }}
 {%         endif %}
 {%         if 'vrf' in zone_conf.member %}
 {%             for vrf_name in zone_conf.member.vrf %}
-        oifname { {{ zone_conf['vrf_interfaces'][vrf_name] }} } counter jump VZONE_{{ zone_name }}
+        oifname { "{{ zone_conf['vrf_interfaces'][vrf_name] }}" } counter jump VZONE_{{ zone_name }}
 {%             endfor %}
 {%         endif %}
 {%     endif %}
@@ -49,12 +49,12 @@
 {%             for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall[fw_name] is vyos_defined %}
 
 {%                 if 'interface' in zone[from_zone].member %}
-        iifname { {{ zone[from_zone].member.interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
-        iifname { {{ zone[from_zone].member.interface | join(",") }} } counter return
+        iifname { {{ zone[from_zone].member.interface | quoted_join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
+        iifname { {{ zone[from_zone].member.interface | quoted_join(",") }} } counter return
 {%                 endif %}
 {%                 if 'vrf' in zone[from_zone].member %}
-        iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
-        iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter return
+        iifname { {{ zone[from_zone].member.vrf | quoted_join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
+        iifname { {{ zone[from_zone].member.vrf | quoted_join(",") }} } counter return
 {%                 endif %}
 {%             endfor %}
 {%         endif %}
@@ -65,13 +65,13 @@
 {%         if zone_conf.from_local is vyos_defined %}
 {%             for from_zone, from_conf in zone_conf.from_local.items() if from_conf.firewall[fw_name] is vyos_defined %}
 {%                 if 'interface' in zone[from_zone].member %}
-        oifname { {{ zone[from_zone].member.interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
-        oifname { {{ zone[from_zone].member.interface | join(",") }} } counter return
+        oifname { {{ zone[from_zone].member.interface | quoted_join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
+        oifname { {{ zone[from_zone].member.interface | quoted_join(",") }} } counter return
 {%                 endif %}
 {%                 if 'vrf' in zone[from_zone].member %}
 {%                     for vrf_name in zone[from_zone].member.vrf %}
-        oifname { {{ zone[from_zone]['vrf_interfaces'][vrf_name] }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
-        oifname { {{ zone[from_zone]['vrf_interfaces'][vrf_name] }} } counter return
+        oifname { "{{ zone[from_zone]['vrf_interfaces'][vrf_name] }}" } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
+        oifname { "{{ zone[from_zone]['vrf_interfaces'][vrf_name] }}" } counter return
 {%                     endfor %}
 {%                 endif %}
 {%             endfor %}
@@ -81,29 +81,29 @@
 {%     else %}
     chain VZONE_{{ zone_name }} {
 {%         if 'interface' in zone_conf.member %}
-        iifname { {{ zone_conf.member.interface | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }}
+        iifname { {{ zone_conf.member.interface | quoted_join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }}
 {%         endif %}
 {%         if 'vrf' in zone_conf.member %}
-        iifname { {{ zone_conf.member.vrf | join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }}
+        iifname { {{ zone_conf.member.vrf | quoted_join(",") }} } counter {{ zone_conf | nft_intra_zone_action(ipv6) }}
 {%         endif %}
 {%         if zone_conf.intra_zone_filtering is vyos_defined %}
 {%             if 'interface' in zone_conf.member %}
-        iifname { {{ zone_conf.member.interface | join(",") }} } counter return
+        iifname { {{ zone_conf.member.interface | quoted_join(",") }} } counter return
 {%             endif %}
 {%             if 'vrf' in zone_conf.member %}
-        iifname { {{ zone_conf.member.vrf | join(",") }} } counter return
+        iifname { {{ zone_conf.member.vrf | quoted_join(",") }} } counter return
 {%             endif %}
 {%         endif %}
 {%         if zone_conf.from is vyos_defined %}
 {%             for from_zone, from_conf in zone_conf.from.items() if from_conf.firewall[fw_name] is vyos_defined %}
 {%                 if zone[from_zone].local_zone is not defined %}
 {%                     if 'interface' in zone[from_zone].member %}
-        iifname { {{ zone[from_zone].member.interface | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
-        iifname { {{ zone[from_zone].member.interface | join(",") }} } counter return
+        iifname { {{ zone[from_zone].member.interface | quoted_join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
+        iifname { {{ zone[from_zone].member.interface | quoted_join(",") }} } counter return
 {%                     endif %}
 {%                     if 'vrf' in zone[from_zone].member %}
-        iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
-        iifname { {{ zone[from_zone].member.vrf | join(",") }} } counter return
+        iifname { {{ zone[from_zone].member.vrf | quoted_join(",") }} } counter jump NAME{{ suffix }}_{{ from_conf.firewall[fw_name] }}
+        iifname { {{ zone[from_zone].member.vrf | quoted_join(",") }} } counter return
 {%                     endif %}
 {%                 endif %}
 {%             endfor %}
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index 64022db84..0643107a9 100755
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -361,7 +361,7 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
             if iiface[0] == '!':
                 operator = '!='
                 iiface = iiface[1:]
-            output.append(f'iifname {operator} {{{iiface}}}')
+            output.append(f'iifname {operator} {{"{iiface}"}}')
         elif 'group' in rule_conf['inbound_interface']:
             iiface = rule_conf['inbound_interface']['group']
             if iiface[0] == '!':
@@ -376,7 +376,7 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
             if oiface[0] == '!':
                 operator = '!='
                 oiface = oiface[1:]
-            output.append(f'oifname {operator} {{{oiface}}}')
+            output.append(f'oifname {operator} {{"{oiface}"}}')
         elif 'group' in rule_conf['outbound_interface']:
             oiface = rule_conf['outbound_interface']['group']
             if oiface[0] == '!':
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 91b3a0c28..33c6830bc 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -423,11 +423,11 @@ class Interface(Control):
             self._cmd(f'nft {nft_command}')
 
     def _del_interface_from_ct_iface_map(self):
-        nft_command = f'delete element inet vrf_zones ct_iface_map {{ "{self.ifname}" }}'
+        nft_command = f'delete element inet vrf_zones ct_iface_map {{ \'"{self.ifname}"\' }}'
         self._nft_check_and_run(nft_command)
 
     def _add_interface_to_ct_iface_map(self, vrf_table_id: int):
-        nft_command = f'add element inet vrf_zones ct_iface_map {{ "{self.ifname}" : {vrf_table_id} }}'
+        nft_command = f'add element inet vrf_zones ct_iface_map {{ \'"{self.ifname}"\' : {vrf_table_id} }}'
         self._nft_check_and_run(nft_command)
 
     def get_ifindex(self):
diff --git a/python/vyos/template.py b/python/vyos/template.py
index bf2f13183..c6e35e9c7 100755
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -582,6 +582,10 @@ def snmp_auth_oid(type):
     }
     return OIDs[type]
 
+@register_filter('quoted_join')
+def quoted_join(input_list, join_str, quote='"'):
+    return str(join_str).join(f'{quote}{elem}{quote}' for elem in input_list)
+
 @register_filter('nft_action')
 def nft_action(vyos_action):
     if vyos_action == 'accept':
diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py
index 6e9d4147a..00a202df4 100755
--- a/src/conf_mode/vrf.py
+++ b/src/conf_mode/vrf.py
@@ -240,7 +240,7 @@ def apply(vrf):
                 vrf_iface.set_dhcpv6(False)
 
             # Remove nftables conntrack zone map item
-            nft_del_element = f'delete element inet vrf_zones ct_iface_map {{ "{tmp}" }}'
+            nft_del_element = f'delete element inet vrf_zones ct_iface_map {{ \'"{tmp}"\' }}'
             # Check if deleting is possible first to avoid raising errors
             _, err = popen(f'nft --check {nft_del_element}')
             if not err:
@@ -320,7 +320,7 @@ def apply(vrf):
             state = 'down' if 'disable' in config else 'up'
             vrf_if.set_admin_state(state)
             # Add nftables conntrack zone map item
-            nft_add_element = f'add element inet vrf_zones ct_iface_map {{ "{name}" : {table} }}'
+            nft_add_element = f'add element inet vrf_zones ct_iface_map {{ \'"{name}"\' : {table} }}'
             cmd(f'nft {nft_add_element}')
 
         # Only call into nftables as long as there is nothing setup to avoid wasting