From 3e1a585f9714ffa990bb5751c7a4c7025d7c02fa Mon Sep 17 00:00:00 2001 From: aapostoliuk Date: Wed, 7 Feb 2024 12:26:49 +0200 Subject: T5960: Rewritten authentication node in PPTP to a single view Rewritten authentication node in accel-ppp services to a single view. In particular - PPTP authentication. (cherry picked from commit 018110200c9a82815dd5d0510f0732d7159c0d59) --- data/templates/accel-ppp/pptp.config.j2 | 10 +- .../include/version/pptp-version.xml.i | 2 +- interface-definitions/service_ipoe-server.xml.in | 150 +++++++++---------- interface-definitions/vpn_l2tp.xml.in | 34 ++--- interface-definitions/vpn_pptp.xml.in | 96 +++---------- python/vyos/accel_ppp_util.py | 4 + smoketest/scripts/cli/test_vpn_l2tp.py | 2 +- smoketest/scripts/cli/test_vpn_pptp.py | 160 --------------------- src/conf_mode/service_ipoe-server.py | 12 +- src/conf_mode/service_pppoe-server.py | 8 +- src/conf_mode/vpn_l2tp.py | 7 - src/migration-scripts/pptp/4-to-5 | 66 +++++++++ 12 files changed, 185 insertions(+), 366 deletions(-) create mode 100755 src/migration-scripts/pptp/4-to-5 diff --git a/data/templates/accel-ppp/pptp.config.j2 b/data/templates/accel-ppp/pptp.config.j2 index 7fe4b17bf..290e6235d 100644 --- a/data/templates/accel-ppp/pptp.config.j2 +++ b/data/templates/accel-ppp/pptp.config.j2 @@ -9,15 +9,7 @@ ippool {# Common IPv6 definitions #} {% include 'accel-ppp/config_modules_ipv6.j2' %} {# Common authentication protocols (pap, chap ...) #} -{% if authentication.require is vyos_defined %} -{% if authentication.require == 'chap' %} -auth_chap_md5 -{% elif authentication.require == 'mschap' %} -auth_mschap_v1 -{% else %} -auth_{{ authentication.require.replace('-', '_') }} -{% endif %} -{% endif %} +{% include 'accel-ppp/config_modules_auth_protocols.j2' %} [core] thread-count={{ thread_count }} diff --git a/interface-definitions/include/version/pptp-version.xml.i b/interface-definitions/include/version/pptp-version.xml.i index 3e1482ecc..a877d77ff 100644 --- a/interface-definitions/include/version/pptp-version.xml.i +++ b/interface-definitions/include/version/pptp-version.xml.i @@ -1,3 +1,3 @@ - + diff --git a/interface-definitions/service_ipoe-server.xml.in b/interface-definitions/service_ipoe-server.xml.in index eeec2aeef..23d6e54d1 100644 --- a/interface-definitions/service_ipoe-server.xml.in +++ b/interface-definitions/service_ipoe-server.xml.in @@ -8,6 +8,81 @@ 900 + + + Client authentication methods + + + #include + + + Network interface for client MAC addresses + + + + + + + + Media Access Control (MAC) address + + macaddr + Hardware (MAC) address + + + + + + + + + Upload/Download speed limits + + + + + Upload bandwidth limit in kbits/sec + + + + + + + + Download bandwidth limit in kbits/sec + + + + + + + + + + VLAN monitor for automatic creation of VLAN interfaces + + u32:1-4094 + Client VLAN id + + + + + VLAN IDs need to be in range 1-4094 + + + + + + + #include + #include + + + #include + + + + Interface to listen dhcp or unclassified packets @@ -107,81 +182,6 @@ #include #include #include - - - Client authentication methods - - - #include - - - Network interface for client MAC addresses - - - - - - - - Media Access Control (MAC) address - - macaddr - Hardware (MAC) address - - - - - - - - - Upload/Download speed limits - - - - - Upload bandwidth limit in kbits/sec - - - - - - - - Download bandwidth limit in kbits/sec - - - - - - - - - - VLAN monitor for automatic creation of VLAN interfaces - - u32:1-4094 - Client VLAN id - - - - - VLAN IDs need to be in range 1-4094 - - - - - - - - - #include - - - #include - #include - - #include #include diff --git a/interface-definitions/vpn_l2tp.xml.in b/interface-definitions/vpn_l2tp.xml.in index 942690bca..6148e3269 100644 --- a/interface-definitions/vpn_l2tp.xml.in +++ b/interface-definitions/vpn_l2tp.xml.in @@ -13,6 +13,23 @@ Remote access L2TP VPN + + + Authentication for remote access L2TP VPN + + + #include + #include + #include + #include + #include + + + #include + + + + #include #include @@ -117,23 +134,6 @@ #include #include #include - - - Authentication for remote access L2TP VPN - - - #include - #include - #include - #include - #include - - - #include - - - - #include #include #include diff --git a/interface-definitions/vpn_pptp.xml.in b/interface-definitions/vpn_pptp.xml.in index d23086c02..2e2a3bec4 100644 --- a/interface-definitions/vpn_pptp.xml.in +++ b/interface-definitions/vpn_pptp.xml.in @@ -13,6 +13,23 @@ Remote access PPTP VPN + + + Authentication for remote access PPTP VPN + + + #include + #include + #include + #include + #include + + + #include + + + + #include #include @@ -30,85 +47,6 @@ #include #include #include - - - Authentication for remote access PPTP VPN - - - - - Authentication protocol for remote access peer PPTP VPN - - pap chap mschap mschap-v2 - - - pap - Require the peer to authenticate itself using PAP [Password Authentication Protocol]. - - - chap - Require the peer to authenticate itself using CHAP [Challenge Handshake Authentication Protocol]. - - - mschap - Require the peer to authenticate itself using CHAP [Challenge Handshake Authentication Protocol]. - - - mschap-v2 - Require the peer to authenticate itself using MS-CHAPv2 [Microsoft Challenge Handshake Authentication Protocol, Version 2]. - - - (pap|chap|mschap|mschap-v2) - - - mschap-v2 - - #include - - - Local user authentication for remote access PPTP VPN - - - - - User name for authentication - - - #include - - - Password for authentication - - - - - Static client IP address - - * - - - - - - - - #include - - - #include - #include - - - - 30 - - - 30 - - - - - #include #include #include diff --git a/python/vyos/accel_ppp_util.py b/python/vyos/accel_ppp_util.py index d60402e48..bd0c46a19 100644 --- a/python/vyos/accel_ppp_util.py +++ b/python/vyos/accel_ppp_util.py @@ -144,6 +144,10 @@ def verify_accel_ppp_base_service(config, local_users=True): if "key" not in radius_config: raise ConfigError(f'Missing RADIUS secret key for server "{server}"') + if dict_search('authentication.radius.dynamic_author.server', config): + if not dict_search('authentication.radius.dynamic_author.key', config): + raise ConfigError('DAE/CoA server key required!') + if "name_server_ipv4" in config: if len(config["name_server_ipv4"]) > 2: raise ConfigError( diff --git a/smoketest/scripts/cli/test_vpn_l2tp.py b/smoketest/scripts/cli/test_vpn_l2tp.py index e253f0e49..c3b5b500d 100755 --- a/smoketest/scripts/cli/test_vpn_l2tp.py +++ b/smoketest/scripts/cli/test_vpn_l2tp.py @@ -39,7 +39,7 @@ class TestVPNL2TPServer(BasicAccelPPPTest.TestCase): pass def test_l2tp_server_authentication_protocols(self): - # Test configuration of local authentication for PPPoE server + # Test configuration of local authentication protocols self.basic_config() # explicitly test mschap-v2 - no special reason diff --git a/smoketest/scripts/cli/test_vpn_pptp.py b/smoketest/scripts/cli/test_vpn_pptp.py index 40dcb7f80..ac46d210d 100755 --- a/smoketest/scripts/cli/test_vpn_pptp.py +++ b/smoketest/scripts/cli/test_vpn_pptp.py @@ -40,165 +40,5 @@ class TestVPNPPTPServer(BasicAccelPPPTest.TestCase): def basic_protocol_specific_config(self): pass - def test_accel_local_authentication(self): - # Test configuration of local authentication - self.basic_config() - - # upload / download limit - user = "test" - password = "test2" - static_ip = "100.100.100.101" - upload = "5000" - download = "10000" - - self.set( - [ - "authentication", - "local-users", - "username", - user, - "password", - password, - ] - ) - self.set( - [ - "authentication", - "local-users", - "username", - user, - "static-ip", - static_ip, - ] - ) - - # commit changes - self.cli_commit() - - # Validate configuration values - conf = ConfigParser(allow_no_value=True, delimiters="=", strict=False) - conf.read(self._config_file) - - # check proper path to chap-secrets file - self.assertEqual(conf["chap-secrets"]["chap-secrets"], self._chap_secrets) - - # basic verification - self.verify(conf) - - # check local users - tmp = cmd(f"sudo cat {self._chap_secrets}") - regex = f"{user}\s+\*\s+{password}\s+{static_ip}\s" - tmp = re.findall(regex, tmp) - self.assertTrue(tmp) - - # Check local-users default value(s) - self.delete(["authentication", "local-users", "username", user, "static-ip"]) - # commit changes - self.cli_commit() - - # check local users - tmp = cmd(f"sudo cat {self._chap_secrets}") - regex = f"{user}\s+\*\s+{password}\s+\*\s" - tmp = re.findall(regex, tmp) - self.assertTrue(tmp) - - def test_accel_radius_authentication(self): - # Test configuration of RADIUS authentication for PPPoE server - self.basic_config() - - radius_server = "192.0.2.22" - radius_key = "secretVyOS" - radius_port = "2000" - radius_port_acc = "3000" - - self.set(["authentication", "mode", "radius"]) - self.set( - ["authentication", "radius", "server", radius_server, "key", radius_key] - ) - self.set( - [ - "authentication", - "radius", - "server", - radius_server, - "port", - radius_port, - ] - ) - self.set( - [ - "authentication", - "radius", - "server", - radius_server, - "acct-port", - radius_port_acc, - ] - ) - - nas_id = "VyOS-PPPoE" - nas_ip = "7.7.7.7" - self.set(["authentication", "radius", "nas-identifier", nas_id]) - self.set(["authentication", "radius", "nas-ip-address", nas_ip]) - - source_address = "1.2.3.4" - self.set(["authentication", "radius", "source-address", source_address]) - - # commit changes - self.cli_commit() - - # Validate configuration values - conf = ConfigParser(allow_no_value=True, delimiters="=", strict=False) - conf.read(self._config_file) - - # basic verification - self.verify(conf) - - # check auth - self.assertTrue(conf["radius"].getboolean("verbose")) - self.assertEqual(conf["radius"]["acct-timeout"], "30") - self.assertEqual(conf["radius"]["timeout"], "30") - self.assertEqual(conf["radius"]["max-try"], "3") - - self.assertEqual(conf["radius"]["nas-identifier"], nas_id) - self.assertEqual(conf["radius"]["nas-ip-address"], nas_ip) - self.assertEqual(conf["radius"]["bind"], source_address) - - server = conf["radius"]["server"].split(",") - self.assertEqual(radius_server, server[0]) - self.assertEqual(radius_key, server[1]) - self.assertEqual(f"auth-port={radius_port}", server[2]) - self.assertEqual(f"acct-port={radius_port_acc}", server[3]) - self.assertEqual(f"req-limit=0", server[4]) - self.assertEqual(f"fail-time=0", server[5]) - - # - # Disable Radius Accounting - # - self.delete(["authentication", "radius", "server", radius_server, "acct-port"]) - self.set( - [ - "authentication", - "radius", - "server", - radius_server, - "disable-accounting", - ] - ) - - # commit changes - self.cli_commit() - - conf.read(self._config_file) - - server = conf["radius"]["server"].split(",") - self.assertEqual(radius_server, server[0]) - self.assertEqual(radius_key, server[1]) - self.assertEqual(f"auth-port={radius_port}", server[2]) - self.assertEqual(f"acct-port=0", server[3]) - self.assertEqual(f"req-limit=0", server[4]) - self.assertEqual(f"fail-time=0", server[5]) - - if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/service_ipoe-server.py b/src/conf_mode/service_ipoe-server.py index 6df6f3dc7..5f72b983c 100755 --- a/src/conf_mode/service_ipoe-server.py +++ b/src/conf_mode/service_ipoe-server.py @@ -26,6 +26,7 @@ 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.accel_ppp_util import verify_accel_ppp_base_service from vyos import ConfigError from vyos import airbag airbag.enable() @@ -68,18 +69,9 @@ 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) verify_accel_ppp_ip_pool(ipoe) - if dict_search('authentication.mode', ipoe) == 'radius': - if not dict_search('authentication.radius.server', ipoe): - raise ConfigError('RADIUS authentication requires at least one server') - - for server in dict_search('authentication.radius.server', ipoe): - radius_config = ipoe['authentication']['radius']['server'][server] - if 'key' not in radius_config: - raise ConfigError(f'Missing RADIUS secret key for server "{server}"') - - return None diff --git a/src/conf_mode/service_pppoe-server.py b/src/conf_mode/service_pppoe-server.py index 31299a15c..c2dfbdb44 100755 --- a/src/conf_mode/service_pppoe-server.py +++ b/src/conf_mode/service_pppoe-server.py @@ -68,6 +68,7 @@ def verify(pppoe): return None verify_accel_ppp_base_service(pppoe) + verify_accel_ppp_ip_pool(pppoe) if 'wins_server' in pppoe and len(pppoe['wins_server']) > 2: raise ConfigError('Not more then two WINS name-servers can be configured') @@ -79,13 +80,6 @@ def verify(pppoe): for interface in pppoe['interface']: verify_interface_exists(interface) - 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): - raise ConfigError('DA/CoE server key required!') - - return None diff --git a/src/conf_mode/vpn_l2tp.py b/src/conf_mode/vpn_l2tp.py index 4ca717814..266381754 100755 --- a/src/conf_mode/vpn_l2tp.py +++ b/src/conf_mode/vpn_l2tp.py @@ -27,7 +27,6 @@ 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.base import Warning from vyos import ConfigError from vyos import airbag @@ -64,14 +63,8 @@ def verify(l2tp): return None verify_accel_ppp_base_service(l2tp) - - if dict_search('authentication.radius.dynamic_author.server', l2tp): - if not dict_search('authentication.radius.dynamic_author.key', l2tp): - raise ConfigError('DA/CoE server key required!') - verify_accel_ppp_ip_pool(l2tp) - if 'wins_server' in l2tp and len(l2tp['wins_server']) > 2: raise ConfigError( 'Not more then two WINS name-servers can be configured') diff --git a/src/migration-scripts/pptp/4-to-5 b/src/migration-scripts/pptp/4-to-5 new file mode 100755 index 000000000..d4b3f9a14 --- /dev/null +++ b/src/migration-scripts/pptp/4-to-5 @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2024 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 +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# - Move 'require' from 'protocols' in 'authentication' node +# - Migrate to new default values in radius timeout and acct-timeout + +import os + +from sys import argv +from sys import exit +from vyos.configtree import ConfigTree + + +if len(argv) < 2: + print("Must specify file name!") + exit(1) + +file_name = argv[1] + +with open(file_name, 'r') as f: + config_file = f.read() + +config = ConfigTree(config_file) +base = ['vpn', 'pptp', 'remote-access'] + +if not config.exists(base): + exit(0) + +#migrate require to protocols +require_path = base + ['authentication', 'require'] +if config.exists(require_path): + protocols = list(config.return_values(require_path)) + for protocol in protocols: + config.set(base + ['authentication', 'protocols'], value=protocol, + replace=False) + config.delete(require_path) +else: + config.set(base + ['authentication', 'protocols'], value='mschap-v2') + +radius_path = base + ['authentication', 'radius'] +if config.exists(radius_path): + if not config.exists(radius_path + ['timeout']): + config.set(radius_path + ['timeout'], value=3) + if not config.exists(radius_path + ['acct-timeout']): + config.set(radius_path + ['acct-timeout'], value=3) + + +try: + with open(file_name, 'w') as f: + f.write(config.to_string()) +except OSError as e: + print("Failed to save the modified config: {}".format(e)) + exit(1) -- cgit v1.2.3