Merge pull request #623 from c-po/nat

nat: T3092: migrate to get_config_dict()

3 files changed, 136 insertions, 240 deletions

diff --git a/data/templates/firewall/nftables-nat.tmpl b/data/templates/firewall/nftables-nat.tmpl
index eabad56f2..8769c2384 100644
--- a/data/templates/firewall/nftables-nat.tmpl
+++ b/data/templates/firewall/nftables-nat.tmpl
@@ -1,161 +1,143 @@
 #!/usr/sbin/nft -f
 
-# Start with clean NAT table
-flush table nat
-
-{% if helper_functions == 'remove' %}
-{# NAT if going to be disabled - remove rules and targets from nftables #}
-
-{%   set base_command = "delete rule ip raw" %}
-{{ base_command }} PREROUTING handle {{ pre_ct_ignore }}
-{{ base_command }} OUTPUT     handle {{ out_ct_ignore }}
-{{ base_command }} PREROUTING handle {{ pre_ct_conntrack }}
-{{ base_command }} OUTPUT     handle {{ out_ct_conntrack }}
-
-delete chain ip raw NAT_CONNTRACK
-
-{% elif helper_functions == 'add' %}
-{# NAT if enabled - add targets to nftables #}
-add chain ip raw NAT_CONNTRACK
-add rule ip raw NAT_CONNTRACK counter accept
-
-{%   set base_command = "add rule ip raw" %}
-
-{{ base_command }} PREROUTING position {{ pre_ct_ignore }}    counter jump VYATTA_CT_HELPER
-{{ base_command }} OUTPUT     position {{ out_ct_ignore }}    counter jump VYATTA_CT_HELPER
-{{ base_command }} PREROUTING position {{ pre_ct_conntrack }} counter jump NAT_CONNTRACK
-{{ base_command }} OUTPUT     position {{ out_ct_conntrack }} counter jump NAT_CONNTRACK
-{% endif %}
-
-{% macro nat_rule(rule, chain) %}
+{% macro nat_rule(rule, config, chain) %}
 {%   set comment  = "" %}
 {%   set base_log = "" %}
-
-{%   set src_addr  = "ip saddr " + rule.source_address if rule.source_address %}
-{%   set dst_addr  = "ip daddr " + rule.dest_address if rule.dest_address %}
-
+{%   set src_addr  = "ip saddr " + config.source.address if config.source is defined and config.source.address is defined and config.source.address is not none %}
+{%   set dst_addr  = "ip daddr " + config.destination.address if config.destination is defined and config.destination.address is defined and config.destination.address is not none %}
 {#   negated port groups need special treatment, move != in front of { } group #}
-{%   if rule.source_port.startswith('!=') %}
-{%     set src_port  = "sport != { " + rule.source_port.replace('!=','') +" }" if rule.source_port %}
+{%   if config.source is defined and config.source.port is defined and config.source.port is not none and config.source.port.startswith('!=') %}
+{%     set src_port  = "sport != { " + config.source.port.replace('!=','') +" }" %}
 {%   else %}
-{%     set src_port  = "sport { " + rule.source_port +" }" if rule.source_port %}
+{%     set src_port  = "sport { " + config.source.port +" }" if config.source is defined and config.source.port is defined and config.source.port is not none %}
 {%   endif %}
-
 {#   negated port groups need special treatment, move != in front of { } group #}
-{%   if rule.dest_port.startswith('!=') %}
-{%     set dst_port  = "dport != { " + rule.dest_port.replace('!=','') +" }" if rule.dest_port %}
+{%   if config.destination is defined and config.destination.port is defined and config.destination.port is not none and config.destination.port.startswith('!=') %}
+{%     set dst_port  = "dport != { " + config.destination.port.replace('!=','') +" }" %}
 {%   else %}
-{%     set dst_port  = "dport { " + rule.dest_port +" }" if rule.dest_port %}
+{%     set dst_port  = "dport { " + config.destination.port +" }" if config.destination is defined and config.destination.port is defined and config.destination.port is not none %}
 {%   endif %}
-
 {%   if chain == "PREROUTING" %}
-{%     set comment   = "DST-NAT-" + rule.number %}
-{%     set base_log  = "[NAT-DST-" + rule.number %}
-{%     set interface = " iifname \"" + rule.interface_in + "\"" if rule.interface_in is defined and rule.interface_in != 'any' else '' %}
-{%     set trns_addr = "dnat to " + rule.translation_address %}
-
+{%     set comment   = "DST-NAT-" + rule %}
+{%     set base_log  = "[NAT-DST-" + rule %}
+{%     set interface = " iifname \"" + config.inbound_interface + "\"" if config.inbound_interface is defined and config.inbound_interface != 'any' else '' %}
+{%     set trns_addr = "dnat to " + config.translation.address if config.translation is defined and config.translation.address is defined and config.translation.address is not none %}
 {%   elif chain == "POSTROUTING" %}
-{%     set comment   = "SRC-NAT-" + rule.number %}
-{%     set base_log  = "[NAT-SRC-" + rule.number %}
-{%     set interface = " oifname \"" + rule.interface_out + "\"" if rule.interface_out is defined and rule.interface_out != 'any' else '' %}
-{%     if rule.translation_address == 'masquerade' %}
-{%       set trns_addr = rule.translation_address %}
-{%       if rule.translation_port %}
+{%     set comment   = "SRC-NAT-" + rule %}
+{%     set base_log  = "[NAT-SRC-" + rule %}
+{%     set interface = " oifname \"" + config.outbound_interface + "\"" if config.outbound_interface is defined and config.outbound_interface != 'any' else '' %}
+{%     if config.translation is defined and config.translation.address is defined and config.translation.address == 'masquerade' %}
+{%       set trns_addr = config.translation.address %}
+{%       if config.translation.port is defined and config.translation.port is not none %}
 {%         set trns_addr = trns_addr + " to " %}
 {%       endif %}
 {%     else %}
-{%       set trns_addr = "snat to " + rule.translation_address %}
+{%       set trns_addr = "snat to " + config.translation.address if config.translation is defined and config.translation.address is defined and config.translation.address is not none %}
 {%     endif %}
 {%   endif %}
-{%   set trns_port = ":" + rule.translation_port if rule.translation_port %}
-
-{%   if rule.protocol == "tcp_udp" %}
+{%   set trns_port = ":" + config.translation.port if config.translation is defined and config.translation.port is defined and config.translation.port is not none %}
+{#   protocol has a default value thus it is always present #}
+{%   if config.protocol == "tcp_udp" %}
 {%     set protocol  = "tcp" %}
 {%     set comment   = comment + " tcp_udp" %}
 {%   else %}
-{%     set protocol  = rule.protocol %}
+{%     set protocol  = config.protocol %}
 {%   endif %}
-
-{%   if rule.log %}
-{%     if rule.exclude %}
+{%   if config.log is defined %}
+{%     if config.exclude is defined %}
 {%       set log = base_log + "-EXCL]" %}
-{%     elif rule.translation_address == 'masquerade' %}
+{%     elif config.translation is defined and config.translation.address is defined and config.translation.address == 'masquerade' %}
 {%       set log = base_log + "-MASQ]" %}
 {%     else %}
 {%       set log = base_log + "]" %}
 {%     endif %}
 {%   endif %}
-
-{%   if rule.exclude %}
+{%   if config.exclude is defined %}
 {#     rule has been marked as "exclude" thus we simply return here #}
 {%     set trns_addr = "return" %}
 {%     set trns_port = "" %}
 {%   endif %}
-
 {%   set output = "add rule ip nat " + chain + interface %}
-
 {%   if protocol != "all" %}
 {%     set output = output + " ip protocol " + protocol %}
 {%   endif %}
-
 {%   if src_addr %}
 {%     set output = output + " " + src_addr %}
 {%   endif %}
 {%   if src_port %}
 {%     set output = output + " " + protocol + " " + src_port %}
 {%   endif %}
-
 {%   if dst_addr %}
 {%     set output = output + " " + dst_addr %}
 {%   endif %}
 {%   if dst_port %}
 {%     set output = output + " " + protocol + " " + dst_port %}
 {%   endif %}
-
 {#   Count packets #}
 {%     set output = output + " counter" %}
-
 {#   Special handling of log option, we must repeat the entire rule before the #}
 {#   NAT translation options are added, this is essential                      #}
 {%   if log %}
 {%     set log_output = output + " log prefix \"" + log + "\" comment \"" + comment + "\"" %}
 {%   endif %}
-
 {%   if trns_addr %}
 {%     set output = output + " " + trns_addr %}
 {%   endif %}
-
 {%   if trns_port %}
 {#     Do not add a whitespace here, translation port must be directly added after IP address #}
 {#     e.g. 192.0.2.10:3389                                                                   #}
 {%     set output = output + trns_port %}
 {%   endif %}
-
 {%   if comment %}
 {%     set output = output + " comment \"" + comment + "\"" %}
 {%   endif %}
-
 {{ log_output if log_output }}
 {{ output }}
-
 {#   Special handling if protocol is tcp_udp, we must repeat the entire rule with udp as protocol #}
-{%   if rule.protocol == "tcp_udp" %}
+{%   if config.protocol == "tcp_udp" %}
 {#     Beware of trailing whitespace, without it the comment tcp_udp will be changed to udp_udp   #}
 {{ log_output | replace("tcp ", "udp ") if log_output }}
 {{ output | replace("tcp ", "udp ") }}
 {%   endif %}
 {% endmacro %}
 
+# Start with clean NAT table
+flush table nat
+{% if helper_functions == 'remove' %}
+{# NAT if going to be disabled - remove rules and targets from nftables #}
+{%   set base_command = "delete rule ip raw" %}
+{{ base_command }} PREROUTING handle {{ pre_ct_ignore }}
+{{ base_command }} OUTPUT     handle {{ out_ct_ignore }}
+{{ base_command }} PREROUTING handle {{ pre_ct_conntrack }}
+{{ base_command }} OUTPUT     handle {{ out_ct_conntrack }}
+
+delete chain ip raw NAT_CONNTRACK
+
+{% elif helper_functions == 'add' %}
+{# NAT if enabled - add targets to nftables #}
+add chain ip raw NAT_CONNTRACK
+add rule ip raw NAT_CONNTRACK counter accept
+{%   set base_command = "add rule ip raw" %}
+{{ base_command }} PREROUTING position {{ pre_ct_ignore }}    counter jump VYATTA_CT_HELPER
+{{ base_command }} OUTPUT     position {{ out_ct_ignore }}    counter jump VYATTA_CT_HELPER
+{{ base_command }} PREROUTING position {{ pre_ct_conntrack }} counter jump NAT_CONNTRACK
+{{ base_command }} OUTPUT     position {{ out_ct_conntrack }} counter jump NAT_CONNTRACK
+{% endif %}
+
 #
 # Destination NAT rules build up here
 #
-{% for rule in destination if not rule.disabled %}
-{{ nat_rule(rule, 'PREROUTING') }}
-{% endfor %}
+{% if destination is defined and destination.rule is defined and destination.rule is not none %}
+{%   for rule, config in destination.rule.items() if config.disable is not defined %}
+{{ nat_rule(rule, config, 'PREROUTING') }}
+{%   endfor %}
+{% endif %}
 
 #
 # Source NAT rules build up here
 #
-{% for rule in source if not rule.disabled %}
-{{ nat_rule(rule, 'POSTROUTING') }}
-{% endfor %}
+{% if source is defined and source.rule is defined and source.rule is not none %}
+{%   for rule, config in source.rule.items() if config.disable is not defined %}
+{{ nat_rule(rule, config, 'POSTROUTING') }}
+{%   endfor %}
+{% endif %}
diff --git a/interface-definitions/include/nat-rule.xml.i b/interface-definitions/include/nat-rule.xml.i
index f91834ae0..e70cfd796 100644
--- a/interface-definitions/include/nat-rule.xml.i
+++ b/interface-definitions/include/nat-rule.xml.i
@@ -290,6 +290,7 @@
           <validator name="ip-protocol"/>
         </constraint>
       </properties>
+      <defaultValue>all</defaultValue>
     </leafNode>
     <node name="source">
       <properties>
diff --git a/src/conf_mode/nat.py b/src/conf_mode/nat.py
index b467f3d74..f5c023b81 100755
--- a/src/conf_mode/nat.py
+++ b/src/conf_mode/nat.py
@@ -18,18 +18,19 @@ import jmespath
 import json
 import os
 
-from copy import deepcopy
 from distutils.version import LooseVersion
 from platform import release as kernel_version
 from sys import exit
 from netifaces import interfaces
 
 from vyos.config import Config
+from vyos.configdict import dict_merge
 from vyos.template import render
-from vyos.util import call
 from vyos.util import cmd
 from vyos.util import check_kmod
+from vyos.util import dict_search
 from vyos.validate import is_addr_assigned
+from vyos.xml import defaults
 from vyos import ConfigError
 
 from vyos import airbag
@@ -40,17 +41,6 @@ if LooseVersion(kernel_version()) > LooseVersion('5.1'):
 else:
     k_mod = ['nft_nat', 'nft_chain_nat_ipv4']
 
-default_config_data = {
-    'deleted': False,
-    'destination': [],
-    'helper_functions': None,
-    'pre_ct_helper': '',
-    'pre_ct_conntrack': '',
-    'out_ct_helper': '',
-    'out_ct_conntrack': '',
-    'source': []
-}
-
 iptables_nat_config = '/tmp/vyos-nat-rules.nft'
 
 def get_handler(json, chain, target):
@@ -66,114 +56,43 @@ def get_handler(json, chain, target):
     return None
 
 
-def verify_rule(rule, err_msg):
+def verify_rule(config, err_msg):
     """ Common verify steps used for both source and destination NAT """
-    if rule['translation_port'] or rule['dest_port'] or rule['source_port']:
-        if rule['protocol'] not in ['tcp', 'udp', 'tcp_udp']:
-            proto = rule['protocol']
-            raise ConfigError(f'{err_msg} ports can only be specified when protocol is "tcp", "udp" or "tcp_udp" (currently "{proto}")')
 
-        if '/' in rule['translation_address']:
+    if (dict_search('translation.port', config) != None or
+        dict_search('destination.port', config) != None or
+        dict_search('source.port', config)):
+
+        if config['protocol'] not in ['tcp', 'udp', 'tcp_udp']:
+            raise ConfigError(f'{err_msg}\n' \
+                              'ports can only be specified when protocol is '\
+                              'either tcp, udp or tcp_udp!')
+
+        if '/' in (dict_search('translation.address', config) or []):
             raise ConfigError(f'{err_msg}\n' \
                              'Cannot use ports with an IPv4net type translation address as it\n' \
                              'statically maps a whole network of addresses onto another\n' \
                              'network of addresses')
 
-
-def parse_configuration(conf, source_dest):
-    """ Common wrapper to read in both NAT source and destination CLI """
-    ruleset = []
-    base_level = ['nat', source_dest]
-    conf.set_level(base_level)
-    for number in conf.list_nodes(['rule']):
-        rule = {
-            'description': '',
-            'dest_address': '',
-            'dest_port': '',
-            'disabled': False,
-            'exclude': False,
-            'interface_in': '',
-            'interface_out': '',
-            'log': False,
-            'protocol': 'all',
-            'number': number,
-            'source_address': '',
-            'source_prefix': '',
-            'source_port': '',
-            'translation_address': '',
-            'translation_prefix': '',
-            'translation_port': ''
-        }
-        conf.set_level(base_level + ['rule', number])
-
-        if conf.exists(['description']):
-            rule['description'] = conf.return_value(['description'])
-
-        if conf.exists(['destination', 'address']):
-            tmp = conf.return_value(['destination', 'address'])
-            if tmp.startswith('!'):
-                tmp = tmp.replace('!', '!=')
-            rule['dest_address'] = tmp
-
-        if conf.exists(['destination', 'port']):
-            tmp = conf.return_value(['destination', 'port'])
-            if tmp.startswith('!'):
-                tmp = tmp.replace('!', '!=')
-            rule['dest_port'] = tmp
-
-        if conf.exists(['disable']):
-            rule['disabled'] = True
-
-        if conf.exists(['exclude']):
-            rule['exclude'] = True
-
-        if conf.exists(['inbound-interface']):
-            rule['interface_in'] = conf.return_value(['inbound-interface'])
-
-        if conf.exists(['outbound-interface']):
-            rule['interface_out'] = conf.return_value(['outbound-interface'])
-
-        if conf.exists(['log']):
-            rule['log'] = True
-
-        if conf.exists(['protocol']):
-            rule['protocol'] = conf.return_value(['protocol'])
-
-        if conf.exists(['source', 'address']):
-            tmp = conf.return_value(['source', 'address'])
-            if tmp.startswith('!'):
-                tmp = tmp.replace('!', '!=')
-            rule['source_address'] = tmp
-
-        if conf.exists(['source', 'prefix']):
-            rule['source_prefix'] = conf.return_value(['source', 'prefix'])
-
-        if conf.exists(['source', 'port']):
-            tmp = conf.return_value(['source', 'port'])
-            if tmp.startswith('!'):
-                tmp = tmp.replace('!', '!=')
-            rule['source_port'] = tmp
-
-        if conf.exists(['translation', 'address']):
-            rule['translation_address'] = conf.return_value(['translation', 'address'])
-
-        if conf.exists(['translation', 'prefix']):
-            rule['translation_prefix'] = conf.return_value(['translation', 'prefix'])
-
-        if conf.exists(['translation', 'port']):
-            rule['translation_port'] = conf.return_value(['translation', 'port'])
-
-        ruleset.append(rule)
-
-    return ruleset
-
 def get_config(config=None):
-    nat = deepcopy(default_config_data)
     if config:
         conf = config
     else:
         conf = Config()
 
+    base = ['nat']
+    nat = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
+
+    # T2665: we must add the tagNode defaults individually until this is
+    # moved to the base class
+    for direction in ['source', 'destination']:
+        if direction in nat:
+            default_values = defaults(base + [direction, 'rule'])
+            for rule in nat[direction]['rule']:
+                nat[direction]['rule'][rule] = dict_merge(default_values,
+                    nat[direction]['rule'][rule])
+
+
     # read in current nftable (once) for further processing
     tmp = cmd('nft -j list table raw')
     nftable_json = json.loads(tmp)
@@ -182,7 +101,7 @@ def get_config(config=None):
     pattern = 'nftables[?rule].rule[?expr[].jump].{chain: chain, handle: handle, target: expr[].jump.target | [0]}'
     condensed_json = jmespath.search(pattern, nftable_json)
 
-    if not conf.exists(['nat']):
+    if not conf.exists(base):
         nat['helper_functions'] = 'remove'
 
         # Retrieve current table handler positions
@@ -190,9 +109,7 @@ def get_config(config=None):
         nat['pre_ct_conntrack'] = get_handler(condensed_json, 'PREROUTING', 'NAT_CONNTRACK')
         nat['out_ct_ignore'] = get_handler(condensed_json, 'OUTPUT', 'VYATTA_CT_HELPER')
         nat['out_ct_conntrack'] = get_handler(condensed_json, 'OUTPUT', 'NAT_CONNTRACK')
-
-        nat['deleted'] = True
-
+        nat['deleted'] = ''
         return nat
 
     # check if NAT connection tracking helpers need to be set up - this has to
@@ -206,19 +123,10 @@ def get_config(config=None):
         nat['out_ct_ignore'] = get_handler(condensed_json, 'OUTPUT', 'VYATTA_CT_IGNORE')
         nat['out_ct_conntrack'] = get_handler(condensed_json, 'OUTPUT', 'VYATTA_CT_OUTPUT_HOOK')
 
-    # set config level for parsing in NAT configuration
-    conf.set_level(['nat'])
-
-    # use a common wrapper function to read in the source / destination
-    # tree from the config - thus we do not need to replicate almost the
-    # same code :-)
-    for tgt in ['source', 'destination', 'nptv6']:
-        nat[tgt] = parse_configuration(conf, tgt)
-
     return nat
 
 def verify(nat):
-    if nat['deleted']:
+    if not nat or 'deleted' in nat:
         # no need to verify the CLI as NAT is going to be deactivated
         return None
 
@@ -226,44 +134,49 @@ def verify(nat):
         if not (nat['pre_ct_ignore'] or nat['pre_ct_conntrack'] or nat['out_ct_ignore'] or nat['out_ct_conntrack']):
             raise Exception('could not determine nftable ruleset handlers')
 
-    for rule in nat['source']:
-        interface = rule['interface_out']
-        err_msg = f'Source NAT configuration error in rule "{rule["number"]}":'
-
-        if interface and interface not in 'any' and interface not in interfaces():
-            print(f'Warning: rule "{rule["number"]}" interface "{interface}" does not exist on this system')
-
-        if not rule['interface_out']:
-            raise ConfigError(f'{err_msg} outbound-interface not specified')
-
-        if rule['translation_address']:
-            addr = rule['translation_address']
-            if addr != 'masquerade':
-                for ip in addr.split('-'):
-                    if not is_addr_assigned(ip):
-                        print(f'Warning: IP address {ip} does not exist on the system!')
-
-        elif not rule['exclude']:
-            raise ConfigError(f'{err_msg} translation address not specified')
-
-        # common rule verification
-        verify_rule(rule, err_msg)
-
-    for rule in nat['destination']:
-        interface = rule['interface_in']
-        err_msg = f'Destination NAT configuration error in rule "{rule["number"]}":'
-
-        if interface and interface not in 'any' and interface not in interfaces():
-            print(f'Warning: rule "{rule["number"]}" interface "{interface}" does not exist on this system')
-
-        if not rule['interface_in']:
-            raise ConfigError(f'{err_msg} inbound-interface not specified')
-
-        if not rule['translation_address'] and not rule['exclude']:
-            raise ConfigError(f'{err_msg} translation address not specified')
-
-        # common rule verification
-        verify_rule(rule, err_msg)
+    if dict_search('source.rule', nat):
+        for rule, config in dict_search('source.rule', nat).items():
+            err_msg = f'Source NAT configuration error in rule {rule}:'
+            if 'outbound_interface' not in config:
+                raise ConfigError(f'{err_msg}\n' \
+                                  'outbound-interface not specified')
+            else:
+                if config['outbound_interface'] not in 'any' and config['outbound_interface'] not in interfaces():
+                    print(f'WARNING: rule "{rule}" interface "{config["outbound_interface"]}" does not exist on this system')
+
+
+            addr = dict_search('translation.address', config)
+            if addr != None:
+                if addr != 'masquerade':
+                    for ip in addr.split('-'):
+                        if not is_addr_assigned(ip):
+                            print(f'WARNING: IP address {ip} does not exist on the system!')
+            elif 'exclude' not in config:
+                raise ConfigError(f'{err_msg}\n' \
+                                  'translation address not specified')
+
+            # common rule verification
+            verify_rule(config, err_msg)
+
+
+    if dict_search('destination.rule', nat):
+        for rule, config in dict_search('destination.rule', nat).items():
+            err_msg = f'Destination NAT configuration error in rule {rule}:'
+
+            if 'inbound_interface' not in config:
+                raise ConfigError(f'{err_msg}\n' \
+                                  'inbound-interface not specified')
+            else:
+                if config['inbound_interface'] not in 'any' and config['inbound_interface'] not in interfaces():
+                    print(f'WARNING: rule "{rule}" interface "{config["inbound_interface"]}" does not exist on this system')
+
+
+            if dict_search('translation.address', config) == None and 'exclude' not in config:
+                raise ConfigError(f'{err_msg}\n' \
+                                  'translation address not specified')
+
+            # common rule verification
+            verify_rule(config, err_msg)
 
     return None