Merge branch 'pptp-rewrite' of github.com:c-po/vyos-1x into current

* 'pptp-rewrite' of github.com:c-po/vyos-1x:
  accel-ppp: fix wrong reference in verify() on missing attributes
  accel-ppp: T2314: bugfix wrong placement of endif in Jinja2 template
  vpn: pptp: T2351: add support for common radius-additions XML
  vpn: pptp: T2351: migrate to common radius CLI
  vpn: pptp: T2351: migrate to common name-server, wins-server nodes
  accel-ppp: provide common wins-server include definition
  vpn: pptp: T2351: use first IP from client pool as gateway address
  vpn: pptp: T2351: align configuration to other accel implementations
  vpn: pptp: T2351: migrate from SysVinit to systemd
  vyos.util: migrate all cpu_count() occurances to common get_half_cpus()

5 files changed, 237 insertions, 223 deletions

diff --git a/src/conf_mode/service_ipoe-server.py b/src/conf_mode/service_ipoe-server.py
index 23a2091bd..17fa2c3f0 100755
--- a/src/conf_mode/service_ipoe-server.py
+++ b/src/conf_mode/service_ipoe-server.py
@@ -238,7 +238,7 @@ def verify(ipoe):
 
     for interface in ipoe['interfaces']:
         if not interface['range']:
-            raise ConfigError(f'No IPoE client subnet defined on interface "{{ interface }}"')
+            raise ConfigError(f'No IPoE client subnet defined on interface "{ interface }"')
 
     if len(ipoe['dnsv4']) > 2:
         raise ConfigError('Not more then two IPv4 DNS name-servers can be configured')
@@ -253,7 +253,7 @@ def verify(ipoe):
         for radius in ipoe['radius_server']:
             if not radius['key']:
                 server = radius['server']
-                raise ConfigError(f'Missing RADIUS secret key for server "{{ server }}"')
+                raise ConfigError(f'Missing RADIUS secret key for server "{ server }"')
 
     if ipoe['client_ipv6_delegate_prefix'] and not ipoe['client_ipv6_pool']:
         raise ConfigError('IPoE IPv6 deletate-prefix requires IPv6 prefix to be configured!')
diff --git a/src/conf_mode/service_pppoe-server.py b/src/conf_mode/service_pppoe-server.py
index 94eb675f7..95cb066d8 100755
--- a/src/conf_mode/service_pppoe-server.py
+++ b/src/conf_mode/service_pppoe-server.py
@@ -23,7 +23,7 @@ from sys import exit
 
 from vyos.config import Config
 from vyos.template import render
-from vyos.util import call
+from vyos.util import call, get_half_cpus()
 from vyos.validate import is_ipv4
 from vyos import ConfigError
 
@@ -78,7 +78,7 @@ default_config_data = {
     'radius_dynamic_author': '',
     'sesscrtl': 'replace',
     'snmp': False,
-    'thread_cnt': '1'
+    'thread_cnt': get_half_cpus()
 }
 
 def get_config():
@@ -90,10 +90,6 @@ def get_config():
     conf.set_level(base_path)
     pppoe = deepcopy(default_config_data)
 
-    cpu = os.cpu_count()
-    if cpu > 1:
-        pppoe['thread_cnt'] = int(cpu/2)
-
     # general options
     if conf.exists(['access-concentrator']):
         pppoe['concentrator'] = conf.return_value(['access-concentrator'])
@@ -393,7 +389,7 @@ def verify(pppoe):
         for radius in pppoe['radius_server']:
             if not radius['key']:
                 server = radius['server']
-                raise ConfigError(f'Missing RADIUS secret key for server "{{ server }}"')
+                raise ConfigError(f'Missing RADIUS secret key for server "{ server }"')
 
     if len(pppoe['wins']) > 2:
         raise ConfigError('Not more then two IPv4 WINS name-servers can be configured')
diff --git a/src/conf_mode/vpn_l2tp.py b/src/conf_mode/vpn_l2tp.py
index a640e2a94..a4ef99d45 100755
--- a/src/conf_mode/vpn_l2tp.py
+++ b/src/conf_mode/vpn_l2tp.py
@@ -25,7 +25,7 @@ from time import sleep
 from ipaddress import ip_network
 
 from vyos.config import Config
-from vyos.util import call
+from vyos.util import call, get_half_cpus
 from vyos.validate import is_ipv4
 from vyos import ConfigError
 from vyos.template import render
@@ -65,7 +65,7 @@ default_config_data = {
     'radius_dynamic_author': '',
     'wins': [],
     'ip6_column': [],
-    'thread_cnt': 1
+    'thread_cnt': get_half_cpus()
 }
 
 def get_config():
@@ -77,10 +77,6 @@ def get_config():
     conf.set_level(base_path)
     l2tp = deepcopy(default_config_data)
 
-    cpu = os.cpu_count()
-    if cpu > 1:
-        l2tp['thread_cnt'] = int(cpu/2)
-
     ### general options ###
     if conf.exists(['name-server']):
         for name_server in conf.return_values(['name-server']):
@@ -313,7 +309,7 @@ def verify(l2tp):
 
         for radius in l2tp['radius_server']:
             if not radius['key']:
-                raise ConfigError(f"Missing RADIUS secret for server {{ radius['key'] }}")
+                raise ConfigError(f"Missing RADIUS secret for server { radius['key'] }")
 
     # check for the existence of a client ip pool
     if not (l2tp['client_ip_pool'] or l2tp['client_ip_subnets']):
diff --git a/src/conf_mode/vpn_pptp.py b/src/conf_mode/vpn_pptp.py
index 15b80f984..046fc8f9c 100755
--- a/src/conf_mode/vpn_pptp.py
+++ b/src/conf_mode/vpn_pptp.py
@@ -17,234 +17,260 @@
 import os
 import re
 
-from socket import socket, AF_INET, SOCK_STREAM
+from copy import deepcopy
+from stat import S_IRUSR, S_IWUSR, S_IRGRP
 from sys import exit
-from time import sleep
 
 from vyos.config import Config
-from vyos import ConfigError
-from vyos.util import run
 from vyos.template import render
+from vyos.util import call, get_half_cpus
+from vyos import ConfigError
 
+pptp_conf = '/run/accel-pppd/pptp.conf'
+pptp_chap_secrets = '/run/accel-pppd/pptp.chap-secrets'
+
+default_pptp = {
+    'auth_mode' : 'local',
+    'local_users' : [],
+    'radius_server' : [],
+    'radius_acct_tmo' : '30',
+    'radius_max_try' : '3',
+    'radius_timeout' : '30',
+    'radius_nas_id' : '',
+    'radius_nas_ip' : '',
+    'radius_source_address' : '',
+    'radius_shaper_attr' : '',
+    'radius_shaper_vendor': '',
+    'radius_dynamic_author' : '',
+    'chap_secrets_file': pptp_chap_secrets, # used in Jinja2 template
+    'outside_addr': '',
+    'dnsv4': [],
+    'wins': [],
+    'client_ip_pool': '',
+    'mtu': '1436',
+    'auth_proto' : ['auth_mschap_v2'],
+    'ppp_mppe' : 'prefer',
+    'thread_cnt': get_half_cpus()
+}
 
-pidfile = r'/var/run/accel_pptp.pid'
-pptp_cnf_dir = r'/etc/accel-ppp/pptp'
-chap_secrets = pptp_cnf_dir + '/chap-secrets'
-pptp_conf = pptp_cnf_dir + '/pptp.config'
+def get_config():
+    conf = Config()
+    base_path = ['vpn', 'pptp', 'remote-access']
+    if not conf.exists(base_path):
+        return None
 
-# config path creation
-if not os.path.exists(pptp_cnf_dir):
-    os.makedirs(pptp_cnf_dir)
+    pptp = deepcopy(default_pptp)
+    conf.set_level(base_path)
 
-def _chk_con():
-    cnt = 0
-    s = socket(AF_INET, SOCK_STREAM)
-    while True:
-        try:
-            s.connect(("127.0.0.1", 2003))
-            break
-        except ConnectionRefusedError:
-            sleep(0.5)
-            cnt += 1
-            if cnt == 100:
-                raise("failed to start pptp server")
-                break
+    if conf.exists(['name-server']):
+        pptp['dnsv4'] = conf.return_values(['name-server'])
 
+    if conf.exists(['wins-server']):
+        pptp['wins'] = conf.return_values(['wins-server'])
 
-def _accel_cmd(command):
-    return run('/usr/bin/accel-cmd -p 2003 {command}')
+    if conf.exists(['outside-address']):
+        pptp['outside_addr'] = conf.return_value(['outside-address'])
 
-###
-# inline helper functions end
-###
+    if conf.exists(['authentication', 'mode']):
+        pptp['auth_mode'] = conf.return_value(['authentication', 'mode'])
 
+    #
+    # local auth
+    if conf.exists(['authentication', 'local-users']):
+        for username in conf.list_nodes(['authentication', 'local-users', 'username']):
+            user = {
+                'name': username,
+                'password' : '',
+                'state' : 'enabled',
+                'ip' : '*',
+            }
 
-def get_config():
-    c = Config()
-    if not c.exists(['vpn', 'pptp', 'remote-access']):
-        return None
+            conf.set_level(base_path + ['authentication', 'local-users', 'username', username])
+
+            if conf.exists(['password']):
+                user['password'] = conf.return_value(['password'])
+
+            if conf.exists(['disable']):
+                user['state'] = 'disable'
+
+            if conf.exists(['static-ip']):
+                user['ip'] = conf.return_value(['static-ip'])
+
+            if not conf.exists(['disable']):
+                pptp['local_users'].append(user)
+
+    #
+    # RADIUS auth and settings
+    conf.set_level(base_path + ['authentication', 'radius'])
+    if conf.exists(['server']):
+        for server in conf.list_nodes(['server']):
+            radius = {
+                'server' : server,
+                'key' : '',
+                'fail_time' : 0,
+                'port' : '1812'
+            }
+
+            conf.set_level(base_path + ['authentication', 'radius', 'server', server])
+
+            if conf.exists(['fail-time']):
+                radius['fail-time'] = conf.return_value(['fail-time'])
+
+            if conf.exists(['port']):
+                radius['port'] = conf.return_value(['port'])
+
+            if conf.exists(['key']):
+                radius['key'] = conf.return_value(['key'])
+
+            if not conf.exists(['disable']):
+                pptp['radius_server'].append(radius)
+
+        #
+        # advanced radius-setting
+        conf.set_level(base_path + ['authentication', 'radius'])
+
+        if conf.exists(['acct-timeout']):
+            pptp['radius_acct_tmo'] = conf.return_value(['acct-timeout'])
+
+        if conf.exists(['max-try']):
+            pptp['radius_max_try'] = conf.return_value(['max-try'])
+
+        if conf.exists(['timeout']):
+            pptp['radius_timeout'] = conf.return_value(['timeout'])
+
+        if conf.exists(['nas-identifier']):
+            pptp['radius_nas_id'] = conf.return_value(['nas-identifier'])
+
+        if conf.exists(['nas-ip-address']):
+            pptp['radius_nas_ip'] = conf.return_value(['nas-ip-address'])
+
+        if conf.exists(['source-address']):
+            pptp['radius_source_address'] = conf.return_value(['source-address'])
+
+        # Dynamic Authorization Extensions (DOA)/Change Of Authentication (COA)
+        if conf.exists(['dae-server']):
+            dae = {
+                'port' : '',
+                'server' : '',
+                'key' : ''
+            }
 
-    c.set_level(['vpn', 'pptp', 'remote-access'])
-    config_data = {
-        'authentication': {
-            'mode': 'local',
-            'local-users': {
-            },
-            'radiussrv': {},
-            'auth_proto': 'auth_mschap_v2',
-            'mppe': 'require'
-        },
-        'outside_addr': '',
-        'dns': [],
-        'wins': [],
-        'client_ip_pool': '',
-        'mtu': '1436',
-    }
-
-    ### general options ###
-
-    if c.exists(['dns-servers', 'server-1']):
-        config_data['dns'].append(c.return_value(['dns-servers', 'server-1']))
-    if c.exists(['dns-servers', 'server-2']):
-        config_data['dns'].append(c.return_value(['dns-servers', 'server-2']))
-    if c.exists(['wins-servers', 'server-1']):
-        config_data['wins'].append(
-            c.return_value(['wins-servers', 'server-1']))
-    if c.exists(['wins-servers', 'server-2']):
-        config_data['wins'].append(
-            c.return_value(['wins-servers', 'server-2']))
-    if c.exists(['outside-address']):
-        config_data['outside_addr'] = c.return_value(['outside-address'])
-
-    # auth local
-    if c.exists(['authentication', 'mode', 'local']):
-        if c.exists(['authentication', 'local-users', 'username']):
-            for usr in c.list_nodes(['authentication', 'local-users', 'username']):
-                config_data['authentication']['local-users'].update(
-                    {
-                        usr: {
-                            'passwd': '',
-                            'state': 'enabled',
-                            'ip': ''
-                        }
-                    }
-                )
-
-                if c.exists(['authentication', 'local-users', 'username', usr, 'password']):
-                    config_data['authentication']['local-users'][usr]['passwd'] = c.return_value(
-                        ['authentication', 'local-users', 'username', usr, 'password'])
-                if c.exists(['authentication', 'local-users', 'username', usr, 'disable']):
-                    config_data['authentication']['local-users'][usr]['state'] = 'disable'
-                if c.exists(['authentication', 'local-users', 'username', usr, 'static-ip']):
-                    config_data['authentication']['local-users'][usr]['ip'] = c.return_value(
-                        ['authentication', 'local-users', 'username', usr, 'static-ip'])
-
-    # authentication mode radius servers and settings
-
-    if c.exists(['authentication', 'mode', 'radius']):
-        config_data['authentication']['mode'] = 'radius'
-        rsrvs = c.list_nodes(['authentication', 'radius', 'server'])
-        for rsrv in rsrvs:
-            if not c.return_value(['authentication', 'radius', 'server', rsrv, 'fail-time']):
-                ftime = '0'
-            else:
-                ftime = c.return_value(
-                    ['authentication', 'radius', 'server', rsrv, 'fail-time'])
-            if not c.return_value(['authentication', 'radius-server', rsrv, 'req-limit']):
-                reql = '0'
-            else:
-                reql = c.return_value(
-                    ['authentication', 'radius', 'server', rsrv, 'req-limit'])
-
-            config_data['authentication']['radiussrv'].update(
-                {
-                    rsrv: {
-                        'secret': c.return_value(['authentication', 'radius', 'server', rsrv, 'key']),
-                        'fail-time': ftime,
-                        'req-limit': reql
-                    }
-                }
-            )
-
-    if c.exists(['client-ip-pool']):
-        if c.exists(['client-ip-pool', 'start']):
-            config_data['client_ip_pool'] = c.return_value(
-                ['client-ip-pool', 'start'])
-        if c.exists(['client-ip-pool', 'stop']):
-            config_data['client_ip_pool'] += '-' + \
-                re.search(
-                    '[0-9]+$', c.return_value(['client-ip-pool', 'stop'])).group(0)
-    if c.exists(['mtu']):
-        config_data['mtu'] = c.return_value(['mtu'])
+            if conf.exists(['dynamic-author', 'ip-address']):
+                dae['server'] = conf.return_value(['dynamic-author', 'ip-address'])
+
+            if conf.exists(['dynamic-author', 'port']):
+                dae['port'] = conf.return_value(['dynamic-author', 'port'])
+
+            if conf.exists(['dynamic-author', 'key']):
+                dae['key'] = conf.return_value(['dynamic-author', 'key'])
+
+            pptp['radius_dynamic_author'] = dae
+
+        if conf.exists(['rate-limit', 'enable']):
+            pptp['radius_shaper_attr'] = 'Filter-Id'
+            c_attr = ['rate-limit', 'enable', 'attribute']
+            if conf.exists(c_attr):
+                pptp['radius_shaper_attr'] = conf.return_value(c_attr)
+
+            c_vendor = ['rate-limit', 'enable', 'vendor']
+            if conf.exists(c_vendor):
+                pptp['radius_shaper_vendor'] = conf.return_value(c_vendor)
+
+    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)
+
+    if conf.exists(['mtu']):
+        pptp['mtu'] = conf.return_value(['mtu'])
 
     # gateway address
-    if c.exists(['gateway-address']):
-        config_data['gw_ip'] = c.return_value(['gateway-address'])
+    if conf.exists(['gateway-address']):
+        pptp['gw_ip'] = conf.return_value(['gateway-address'])
     else:
-        config_data['gw_ip'] = re.sub(
-            '[0-9]+$', '1', config_data['client_ip_pool'])
+        # 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'])
 
-    if c.exists(['authentication', 'require']):
-        if c.return_value(['authentication', 'require']) == 'pap':
-            config_data['authentication']['auth_proto'] = 'auth_pap'
-        if c.return_value(['authentication', 'require']) == 'chap':
-            config_data['authentication']['auth_proto'] = 'auth_chap_md5'
-        if c.return_value(['authentication', 'require']) == 'mschap':
-            config_data['authentication']['auth_proto'] = 'auth_mschap_v1'
-        if c.return_value(['authentication', 'require']) == 'mschap-v2':
-            config_data['authentication']['auth_proto'] = 'auth_mschap_v2'
+    if conf.exists(['authentication', 'require']):
+        # clear default list content, now populate with actual CLI values
+        pptp['auth_proto'] = []
+        auth_mods = {
+            'pap': 'auth_pap',
+            'chap': 'auth_chap_md5',
+            'mschap': 'auth_mschap_v1',
+            'mschap-v2': 'auth_mschap_v2'
+        }
 
-        if c.exists(['authentication', 'mppe']):
-            config_data['authentication']['mppe'] = c.return_value(
-                ['authentication', 'mppe'])
+        for proto in conf.return_values(['authentication', 'require']):
+            pptp['auth_proto'].append(auth_mods[proto])
 
-    return config_data
+    if conf.exists(['authentication', 'mppe']):
+        pptp['ppp_mppe'] = conf.return_value(['authentication', 'mppe'])
 
+    return pptp
 
-def verify(c):
-    if c == None:
-        return None
 
-    if c['authentication']['mode'] == 'local':
-        if not c['authentication']['local-users']:
-            raise ConfigError(
-                'pptp-server authentication local-users required')
-        for usr in c['authentication']['local-users']:
-            if not c['authentication']['local-users'][usr]['passwd']:
-                raise ConfigError('user ' + usr + ' requires a password')
-
-    if c['authentication']['mode'] == 'radius':
-        if len(c['authentication']['radiussrv']) == 0:
-            raise ConfigError('radius server required')
-        for rsrv in c['authentication']['radiussrv']:
-            if c['authentication']['radiussrv'][rsrv]['secret'] == None:
-                raise ConfigError('radius server ' + rsrv +
-                                  ' needs a secret configured')
-
-
-def generate(c):
-    if c == None:
+def verify(pptp):
+    if not pptp:
         return None
 
-    # accel-cmd reload doesn't work so any change results in a restart of the daemon
-    try:
-        if os.cpu_count() == 1:
-            c['thread_cnt'] = 1
-        else:
-            c['thread_cnt'] = int(os.cpu_count()/2)
-    except KeyError:
-        if os.cpu_count() == 1:
-            c['thread_cnt'] = 1
-        else:
-            c['thread_cnt'] = int(os.cpu_count()/2)
-
-    render(pptp_conf, 'pptp/pptp.config.tmpl', c, trim_blocks=True)
-
-    if c['authentication']['local-users']:
-        old_umask = os.umask(0o077)
-        render(chap_secrets, 'pptp/chap-secrets.tmpl', c, trim_blocks=True)
-        os.umask(old_umask)
-    # return c ??
-    return c
-
-
-def apply(c):
-    if c == None:
-        if os.path.exists(pidfile):
-            _accel_cmd('shutdown hard')
-            if os.path.exists(pidfile):
-                os.remove(pidfile)
+    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 len(pptp['dnsv4']) > 2:
+        raise ConfigError('Not more then two IPv4 DNS name-servers can be configured')
+
+    if len(pptp['wins']) > 2:
+        raise ConfigError('Not more then two IPv4 WINS name-servers can be configured')
+
+
+def generate(pptp):
+    if not pptp:
         return None
 
-    if not os.path.exists(pidfile):
-        ret = run(f'/usr/sbin/accel-pppd -c {pptp_conf} -p {pidfile} -d')
-        _chk_con()
-        if ret != 0 and os.path.exists(pidfile):
-            os.remove(pidfile)
-            raise ConfigError('accel-pppd failed to start')
+    dirname = os.path.dirname(pptp_conf)
+    if not os.path.exists(dirname):
+        os.mkdir(dirname)
+
+    render(pptp_conf, 'accel-ppp/pptp.config.tmpl', pptp, trim_blocks=True)
+
+    if pptp['local_users']:
+        render(pptp_chap_secrets, 'accel-ppp/chap-secrets.tmpl', pptp, trim_blocks=True)
+        os.chmod(pptp_chap_secrets, S_IRUSR | S_IWUSR | S_IRGRP)
     else:
-        # if gw ip changes, only restart doesn't work
-        _accel_cmd('restart')
+        if os.path.exists(pptp_chap_secrets):
+             os.unlink(pptp_chap_secrets)
+
+
+def apply(pptp):
+    if not pptp:
+        call('systemctl stop accel-ppp@pptp.service')
+        for file in [pptp_conf, pptp_chap_secrets]:
+            if os.path.exists(file):
+                os.unlink(file)
+
+        return None
+
+    call('systemctl restart accel-ppp@pptp.service')
 
 if __name__ == '__main__':
     try:
diff --git a/src/conf_mode/vpn_sstp.py b/src/conf_mode/vpn_sstp.py
index 9ec352290..e6ce94709 100755
--- a/src/conf_mode/vpn_sstp.py
+++ b/src/conf_mode/vpn_sstp.py
@@ -23,7 +23,7 @@ from stat import S_IRUSR, S_IWUSR, S_IRGRP
 
 from vyos.config import Config
 from vyos import ConfigError
-from vyos.util import call, run
+from vyos.util import call, run, get_half_cpus
 from vyos.template import render
 
 
@@ -56,7 +56,7 @@ default_config_data = {
     'ppp_echo_failure' : '',
     'ppp_echo_interval' : '',
     'ppp_echo_timeout' : '',
-    'thread_cnt' : 1
+    'thread_cnt' : get_half_cpus()
 }
 
 def get_config():
@@ -68,10 +68,6 @@ def get_config():
 
     conf.set_level(base_path)
 
-    cpu = os.cpu_count()
-    if cpu > 1:
-        sstp['thread_cnt'] = int(cpu/2)
-
     if conf.exists(['authentication', 'mode']):
         sstp['auth_mode'] = conf.return_value(['authentication', 'mode'])
 
@@ -301,7 +297,7 @@ def verify(sstp):
         for radius in sstp['radius_server']:
             if not radius['key']:
                 server = radius['server']
-                raise ConfigError(f'Missing RADIUS secret key for server "{{ server }}"')
+                raise ConfigError(f'Missing RADIUS secret key for server "{ server }"')
 
 def generate(sstp):
     if not sstp: