accel-ppp: T5688: Standardized pool configuration in accel-ppp

Standardized pool configuration for all accel-ppp services.
1. Only named pools are used now.
2. Allows all services to use range in x.x.x.x/mask
   and x.x.x.x-x.x.x.y format
3. next-pool can be used in all services
2. Allows to use in ipoe gw-ip-address without pool configuration
   which allows to use Fraimed-IP-Address attribute by radius.
3. Default pool name should be explicidly configured
   with default-pool.
4. In ipoe netmask and range subnet can be different.

5 files changed, 74 insertions, 145 deletions

diff --git a/src/conf_mode/service_ipoe-server.py b/src/conf_mode/service_ipoe-server.py
index b70e32373..36f00dec5 100755
--- a/src/conf_mode/service_ipoe-server.py
+++ b/src/conf_mode/service_ipoe-server.py
@@ -15,17 +15,17 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
-import jmespath
 
 from sys import exit
 
 from vyos.config import Config
 from vyos.configdict import get_accel_dict
-from vyos.configverify import verify_accel_ppp_base_service
 from vyos.configverify import verify_interface_exists
 from vyos.template import render
 from vyos.utils.process import call
 from vyos.utils.dict import dict_search
+from vyos.accel_ppp_util import get_pools_in_order
+from vyos.accel_ppp_util import verify_accel_ppp_ip_pool
 from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
@@ -35,87 +35,6 @@ ipoe_conf = '/run/accel-pppd/ipoe.conf'
 ipoe_chap_secrets = '/run/accel-pppd/ipoe.chap-secrets'
 
 
-def get_pools_in_order(data: dict) -> list:
-    """Return a list of dictionaries representing pool data in the order
-    in which they should be allocated. Pool must be defined before we can
-    use it with 'next-pool' option.
-
-    Args:
-        data: A dictionary of pool data, where the keys are pool names and the
-        values are dictionaries containing the 'subnet' key and the optional
-        'next_pool' key.
-
-    Returns:
-        list: A list of dictionaries
-
-    Raises:
-        ValueError: If a 'next_pool' key references a pool name that
-                    has not been defined.
-        ValueError: If a circular reference is found in the 'next_pool' keys.
-
-    Example:
-        config_data = {
-        ... 'first-pool': {
-        ... 'next_pool': 'second-pool',
-        ... 'subnet': '192.0.2.0/25'
-        ... },
-        ... 'second-pool': {
-        ... 'next_pool': 'third-pool',
-        ... 'subnet': '203.0.113.0/25'
-        ... },
-        ... 'third-pool': {
-        ... 'subnet': '198.51.100.0/24'
-        ... },
-        ... 'foo': {
-        ... 'subnet': '100.64.0.0/24',
-        ... 'next_pool': 'second-pool'
-        ... }
-        ... }
-
-        % get_pools_in_order(config_data)
-        [{'third-pool': {'subnet': '198.51.100.0/24'}},
-        {'second-pool': {'next_pool': 'third-pool', 'subnet': '203.0.113.0/25'}},
-        {'first-pool': {'next_pool': 'second-pool', 'subnet': '192.0.2.0/25'}},
-        {'foo': {'next_pool': 'second-pool', 'subnet': '100.64.0.0/24'}}]
-    """
-    pools = []
-    unresolved_pools = {}
-
-    for pool, pool_config in data.items():
-        if 'next_pool' not in pool_config:
-            pools.insert(0, {pool: pool_config})
-        else:
-            unresolved_pools[pool] = pool_config
-
-    while unresolved_pools:
-        resolved_pools = []
-
-        for pool, pool_config in unresolved_pools.items():
-            next_pool_name = pool_config['next_pool']
-
-            if any(p for p in pools if next_pool_name in p):
-                index = next(
-                    (i for i, p in enumerate(pools) if next_pool_name in p),
-                    None)
-                pools.insert(index + 1, {pool: pool_config})
-                resolved_pools.append(pool)
-            elif next_pool_name in unresolved_pools:
-                # next pool not yet resolved
-                pass
-            else:
-                raise ValueError(
-                    f"Pool '{next_pool_name}' not defined in configuration data"
-                )
-
-        if not resolved_pools:
-            raise ValueError("Circular reference in configuration data")
-
-        for pool in resolved_pools:
-            unresolved_pools.pop(pool)
-
-    return pools
-
-
 def get_config(config=None):
     if config:
         conf = config
@@ -128,18 +47,11 @@ def get_config(config=None):
     # retrieve common dictionary keys
     ipoe = get_accel_dict(conf, base, ipoe_chap_secrets)
 
-    if jmespath.search('client_ip_pool.name', ipoe):
-        dict_named_pools = jmespath.search('client_ip_pool.name', ipoe)
+    if dict_search('client_ip_pool', ipoe):
         # Multiple named pools require ordered values T5099
-        ipoe['ordered_named_pools'] = get_pools_in_order(dict_named_pools)
-        # T5099 'next-pool' option
-        if jmespath.search('client_ip_pool.name.*.next_pool', ipoe):
-            for pool, pool_config in ipoe['client_ip_pool']['name'].items():
-                if 'next_pool' in pool_config:
-                    ipoe['first_named_pool'] = pool
-                    ipoe['first_named_pool_subnet'] = pool_config
-                    break
+        ipoe['ordered_named_pools'] = get_pools_in_order(dict_search('client_ip_pool', ipoe))
 
+    ipoe['server_type'] = 'ipoe'
     return ipoe
 
 
@@ -156,9 +68,7 @@ def verify(ipoe):
             raise ConfigError('Option "client-subnet" incompatible with "vlan"!'
                               'Use "ipoe client-ip-pool" instead.')
 
-    #verify_accel_ppp_base_service(ipoe, local_users=False)
-    # IPoE server does not have 'gateway' option in the CLI
-    # we cannot use configverify.py verify_accel_ppp_base_service for ipoe-server
+    verify_accel_ppp_ip_pool(ipoe)
 
     if dict_search('authentication.mode', ipoe) == 'radius':
         if not dict_search('authentication.radius.server', ipoe):
diff --git a/src/conf_mode/service_pppoe-server.py b/src/conf_mode/service_pppoe-server.py
index 87660c127..7c624f034 100755
--- a/src/conf_mode/service_pppoe-server.py
+++ b/src/conf_mode/service_pppoe-server.py
@@ -21,13 +21,16 @@ from sys import exit
 from vyos.config import Config
 from vyos.configdict import get_accel_dict
 from vyos.configdict import is_node_changed
-from vyos.configverify import verify_accel_ppp_base_service
 from vyos.configverify import verify_interface_exists
 from vyos.template import render
 from vyos.utils.process import call
 from vyos.utils.dict import dict_search
+from vyos.accel_ppp_util import verify_accel_ppp_base_service
+from vyos.accel_ppp_util import verify_accel_ppp_ip_pool
+from vyos.accel_ppp_util import get_pools_in_order
 from vyos import ConfigError
 from vyos import airbag
+
 airbag.enable()
 
 pppoe_conf = r'/run/accel-pppd/pppoe.conf'
@@ -45,6 +48,10 @@ def get_config(config=None):
     # retrieve common dictionary keys
     pppoe = get_accel_dict(conf, base, pppoe_chap_secrets)
 
+    if dict_search('client_ip_pool', pppoe):
+        # Multiple named pools require ordered values T5099
+        pppoe['ordered_named_pools'] = get_pools_in_order(dict_search('client_ip_pool', pppoe))
+
     # reload-or-restart does not implemented in accel-ppp
     # use this workaround until it will be implemented
     # https://phabricator.accel-ppp.org/T3
@@ -53,7 +60,7 @@ def get_config(config=None):
                   is_node_changed(conf, base + ['interface'])]
     if any(conditions):
         pppoe.update({'restart_required': {}})
-
+    pppoe['server_type'] = 'pppoe'
     return pppoe
 
 def verify(pppoe):
@@ -72,12 +79,7 @@ def verify(pppoe):
     for interface in pppoe['interface']:
         verify_interface_exists(interface)
 
-    # local ippool and gateway settings config checks
-    if not (dict_search('client_ip_pool.subnet', pppoe) or
-           (dict_search('client_ip_pool.name', pppoe) or
-           (dict_search('client_ip_pool.start', pppoe) and
-            dict_search('client_ip_pool.stop', pppoe)))):
-        print('Warning: No PPPoE client pool defined')
+    verify_accel_ppp_ip_pool(pppoe)
 
     if dict_search('authentication.radius.dynamic_author.server', pppoe):
         if not dict_search('authentication.radius.dynamic_author.key', pppoe):
diff --git a/src/conf_mode/vpn_l2tp.py b/src/conf_mode/vpn_l2tp.py
index 6232ce64a..9a022d93c 100755
--- a/src/conf_mode/vpn_l2tp.py
+++ b/src/conf_mode/vpn_l2tp.py
@@ -21,15 +21,16 @@ from copy import deepcopy
 from stat import S_IRUSR, S_IWUSR, S_IRGRP
 from sys import exit
 
-from ipaddress import ip_network
-
 from vyos.config import Config
 from vyos.template import is_ipv4
 from vyos.template import render
 from vyos.utils.process import call
 from vyos.utils.system import get_half_cpus
+from vyos.utils.dict import dict_search
 from vyos.utils.network import check_port_availability
 from vyos.utils.network import is_listen_port_bind_service
+from vyos.accel_ppp_util import verify_accel_ppp_ip_pool
+from vyos.accel_ppp_util import get_pools_in_order
 from vyos import ConfigError
 
 from vyos import airbag
@@ -43,7 +44,7 @@ default_config_data = {
     'auth_ppp_mppe': 'prefer',
     'auth_proto': ['auth_mschap_v2'],
     'chap_secrets_file': l2tp_chap_secrets, # used in Jinja2 template
-    'client_ip_pool': None,
+    'client_ip_pool': {},
     'client_ip_subnets': [],
     'client_ipv6_pool': [],
     'client_ipv6_pool_configured': False,
@@ -246,13 +247,14 @@ def get_config(config=None):
 
     conf.set_level(base_path)
     if conf.exists(['client-ip-pool']):
-        if conf.exists(['client-ip-pool', 'start']) and conf.exists(['client-ip-pool', 'stop']):
-            start = conf.return_value(['client-ip-pool', 'start'])
-            stop  = conf.return_value(['client-ip-pool', 'stop'])
-            l2tp['client_ip_pool'] = start + '-' + re.search('[0-9]+$', stop).group(0)
+        for pool_name in conf.list_nodes(['client-ip-pool']):
+            l2tp['client_ip_pool'][pool_name] = {}
+            l2tp['client_ip_pool'][pool_name]['range'] = conf.return_value(['client-ip-pool', pool_name, 'range'])
+            l2tp['client_ip_pool'][pool_name]['next_pool'] = conf.return_value(['client-ip-pool', pool_name, 'next-pool'])
 
-    if conf.exists(['client-ip-pool', 'subnet']):
-        l2tp['client_ip_subnets'] = conf.return_values(['client-ip-pool', 'subnet'])
+    if dict_search('client_ip_pool', l2tp):
+        # Multiple named pools require ordered values T5099
+        l2tp['ordered_named_pools'] = get_pools_in_order(dict_search('client_ip_pool', l2tp))
 
     if conf.exists(['client-ipv6-pool', 'prefix']):
         l2tp['client_ipv6_pool_configured'] = True
@@ -281,23 +283,15 @@ def get_config(config=None):
 
             l2tp['client_ipv6_delegate_prefix'].append(tmp)
 
+    if conf.exists(['default-pool']):
+        l2tp['default_pool'] = conf.return_value(['default-pool'])
+
     if conf.exists(['mtu']):
         l2tp['mtu'] = conf.return_value(['mtu'])
 
     # gateway address
     if conf.exists(['gateway-address']):
         l2tp['gateway_address'] = conf.return_value(['gateway-address'])
-    else:
-        # calculate gw-ip-address
-        if conf.exists(['client-ip-pool', 'start']):
-            # use start ip as gw-ip-address
-            l2tp['gateway_address'] = conf.return_value(['client-ip-pool', 'start'])
-
-        elif conf.exists(['client-ip-pool', 'subnet']):
-            # use first ip address from first defined pool
-            subnet = conf.return_values(['client-ip-pool', 'subnet'])[0]
-            subnet = ip_network(subnet)
-            l2tp['gateway_address'] = str(list(subnet.hosts())[0])
 
     # LNS secret
     if conf.exists(['lns', 'shared-secret']):
@@ -330,9 +324,13 @@ def get_config(config=None):
     if conf.exists(['ppp-options', 'ipv6-peer-intf-id']):
         l2tp['ppp_ipv6_peer_intf_id'] = conf.return_value(['ppp-options', 'ipv6-peer-intf-id'])
 
+    l2tp['server_type'] = 'l2tp'
     return l2tp
 
 
+
+
+
 def verify(l2tp):
     if not l2tp:
         return None
@@ -366,10 +364,11 @@ def verify(l2tp):
                 not is_listen_port_bind_service(int(port), 'accel-pppd'):
                 raise ConfigError(f'"{proto}" port "{port}" is used by another service')
 
-    # check for the existence of a client ip pool
-    if not (l2tp['client_ip_pool'] or l2tp['client_ip_subnets']):
-        raise ConfigError(
-            "set vpn l2tp remote-access client-ip-pool requires subnet or start/stop IP pool")
+    if l2tp['auth_mode'] == 'local' or l2tp['auth_mode'] == 'noauth':
+        if not l2tp['client_ip_pool']:
+            raise ConfigError(
+                "L2TP local auth mode requires local client-ip-pool to be configured!")
+    verify_accel_ppp_ip_pool(l2tp)
 
     # check ipv6
     if l2tp['client_ipv6_delegate_prefix'] and not l2tp['client_ipv6_pool']:
diff --git a/src/conf_mode/vpn_pptp.py b/src/conf_mode/vpn_pptp.py
index d542f57fe..6243c3ed3 100755
--- a/src/conf_mode/vpn_pptp.py
+++ b/src/conf_mode/vpn_pptp.py
@@ -21,10 +21,14 @@ from copy import deepcopy
 from stat import S_IRUSR, S_IWUSR, S_IRGRP
 from sys import exit
 
+
 from vyos.config import Config
 from vyos.template import render
 from vyos.utils.system import get_half_cpus
 from vyos.utils.process import call
+from vyos.utils.dict import dict_search
+from vyos.accel_ppp_util import verify_accel_ppp_ip_pool
+from vyos.accel_ppp_util import get_pools_in_order
 from vyos import ConfigError
 
 from vyos import airbag
@@ -54,7 +58,7 @@ default_pptp = {
     'outside_addr': '',
     'dnsv4': [],
     'wins': [],
-    'client_ip_pool': '',
+    'client_ip_pool': {},
     'mtu': '1436',
     'auth_proto' : ['auth_mschap_v2'],
     'ppp_mppe' : 'prefer',
@@ -205,22 +209,24 @@ def get_config(config=None):
 
     conf.set_level(base_path)
     if conf.exists(['client-ip-pool']):
-        if conf.exists(['client-ip-pool', 'start']) and conf.exists(['client-ip-pool', 'stop']):
-            start = conf.return_value(['client-ip-pool', 'start'])
-            stop  = conf.return_value(['client-ip-pool', 'stop'])
-            pptp['client_ip_pool'] = start + '-' + re.search('[0-9]+$', stop).group(0)
+        for pool_name in conf.list_nodes(['client-ip-pool']):
+            pptp['client_ip_pool'][pool_name] = {}
+            pptp['client_ip_pool'][pool_name]['range'] = conf.return_value(['client-ip-pool', pool_name, 'range'])
+            pptp['client_ip_pool'][pool_name]['next_pool'] = conf.return_value(['client-ip-pool', pool_name, 'next-pool'])
+
+    if dict_search('client_ip_pool', pptp):
+        # Multiple named pools require ordered values T5099
+        pptp['ordered_named_pools'] = get_pools_in_order(dict_search('client_ip_pool', pptp))
+
+    if conf.exists(['default-pool']):
+        pptp['default_pool'] = conf.return_value(['default-pool'])
 
     if conf.exists(['mtu']):
         pptp['mtu'] = conf.return_value(['mtu'])
 
     # gateway address
     if conf.exists(['gateway-address']):
-        pptp['gw_ip'] = conf.return_value(['gateway-address'])
-    else:
-        # calculate gw-ip-address
-        if conf.exists(['client-ip-pool', 'start']):
-            # use start ip as gw-ip-address
-            pptp['gateway_address'] = conf.return_value(['client-ip-pool', 'start'])
+        pptp['gateway_address'] = conf.return_value(['gateway-address'])
 
     if conf.exists(['authentication', 'require']):
         # clear default list content, now populate with actual CLI values
@@ -238,6 +244,7 @@ def get_config(config=None):
     if conf.exists(['authentication', 'mppe']):
         pptp['ppp_mppe'] = conf.return_value(['authentication', 'mppe'])
 
+    pptp['server_type'] = 'pptp'
     return pptp
 
 
@@ -248,21 +255,25 @@ def verify(pptp):
     if pptp['auth_mode'] == 'local':
         if not pptp['local_users']:
             raise ConfigError('PPTP local auth mode requires local users to be configured!')
-
         for user in pptp['local_users']:
             username = user['name']
             if not user['password']:
                 raise ConfigError(f'Password required for local user "{username}"')
-
     elif pptp['auth_mode'] == 'radius':
         if len(pptp['radius_server']) == 0:
             raise ConfigError('RADIUS authentication requires at least one server')
-
         for radius in pptp['radius_server']:
             if not radius['key']:
                 server = radius['server']
                 raise ConfigError(f'Missing RADIUS secret key for server "{ server }"')
 
+    if pptp['auth_mode'] == 'local' or pptp['auth_mode'] == 'noauth':
+        if not pptp['client_ip_pool']:
+            raise ConfigError(
+                "PPTP local auth mode requires local client-ip-pool to be configured!")
+
+    verify_accel_ppp_ip_pool(pptp)
+
     if len(pptp['dnsv4']) > 2:
         raise ConfigError('Not more then two IPv4 DNS name-servers can be configured')
 
diff --git a/src/conf_mode/vpn_sstp.py b/src/conf_mode/vpn_sstp.py
index e98d8385b..ac053cc76 100755
--- a/src/conf_mode/vpn_sstp.py
+++ b/src/conf_mode/vpn_sstp.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 #
-# Copyright (C) 2018-2022 VyOS maintainers and contributors
+# Copyright (C) 2018-2023 VyOS maintainers and contributors
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 2 or later as
@@ -21,13 +21,15 @@ from sys import exit
 from vyos.config import Config
 from vyos.configdict import get_accel_dict
 from vyos.configdict import dict_merge
-from vyos.configverify import verify_accel_ppp_base_service
 from vyos.pki import wrap_certificate
 from vyos.pki import wrap_private_key
 from vyos.template import render
 from vyos.utils.process import call
 from vyos.utils.network import check_port_availability
 from vyos.utils.dict import dict_search
+from vyos.accel_ppp_util import verify_accel_ppp_base_service
+from vyos.accel_ppp_util import verify_accel_ppp_ip_pool
+from vyos.accel_ppp_util import get_pools_in_order
 from vyos.utils.network import is_listen_port_bind_service
 from vyos.utils.file import write_file
 from vyos import ConfigError
@@ -53,13 +55,17 @@ def get_config(config=None):
 
     # retrieve common dictionary keys
     sstp = get_accel_dict(conf, base, sstp_chap_secrets)
+    if dict_search('client_ip_pool', sstp):
+        # Multiple named pools require ordered values T5099
+        sstp['ordered_named_pools'] = get_pools_in_order(dict_search('client_ip_pool', sstp))
     if sstp:
         sstp['pki'] = conf.get_config_dict(['pki'], key_mangling=('-', '_'),
                                            get_first_key=True,
                                            no_tag_node_value_mangle=True)
-
+    sstp['server_type'] = 'sstp'
     return sstp
 
+
 def verify(sstp):
     if not sstp:
         return None
@@ -75,6 +81,7 @@ def verify(sstp):
     if 'client_ip_pool' not in sstp and 'client_ipv6_pool' not in sstp:
         raise ConfigError('Client IP subnet required')
 
+    verify_accel_ppp_ip_pool(sstp)
     #
     # SSL certificate checks
     #