From 8773a20c5a32c348ebc5ce958bd73d9ff79a07f3 Mon Sep 17 00:00:00 2001 From: Cheeze_It Date: Sun, 3 Jan 2021 15:52:10 -0700 Subject: ISIS: T3156: Adding segment routing for ISIS In this commit we add the segment routing portion for ISIS. There's also an additional check that is added so that the global block label ranges are properly configured. Also added traffic engineering configurations as well. --- src/conf_mode/protocols_isis.py | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) (limited to 'src/conf_mode') diff --git a/src/conf_mode/protocols_isis.py b/src/conf_mode/protocols_isis.py index 97ab79583..b7afad473 100755 --- a/src/conf_mode/protocols_isis.py +++ b/src/conf_mode/protocols_isis.py @@ -22,6 +22,7 @@ from vyos.config import Config from vyos.configdict import node_changed from vyos import ConfigError from vyos.util import call +from vyos.util import dict_search from vyos.template import render from vyos.template import render_to_string from vyos import frr @@ -48,7 +49,7 @@ def verify(isis): # If more then one isis process is defined (Frr only supports one) # http://docs.frrouting.org/en/latest/isisd.html#isis-router if len(isis) > 1: - raise ConfigError('Only one isis process can be definded') + raise ConfigError('Only one isis process can be defined') # If network entity title (net) not defined if 'net' not in isis_config: @@ -63,7 +64,7 @@ def verify(isis): if {'md5', 'plaintext_password'} <= set(isis_config['encryption']): raise ConfigError('Can not use both md5 and plaintext-password for ISIS area-password!') - # If one param from deley set, but not set others + # If one param from delay set, but not set others if 'spf_delay_ietf' in isis_config: required_timers = ['holddown', 'init_delay', 'long_delay', 'short_delay', 'time_to_learn'] exist_timers = [] @@ -85,6 +86,34 @@ def verify(isis): if proc_level and proc_level != 'level_1_2' and proc_level != redistribute_level: raise ConfigError('\"protocols isis {0} redistribute ipv4 {2} {3}\" cannot be used with \"protocols isis {0} level {1}\"'.format(process, proc_level, proto, redistribute_level)) + # Segment routing checks + if dict_search('segment_routing', isis_config): + if dict_search('segment_routing.global_block', isis_config): + high_label_value = dict_search('segment_routing.global_block.high_label_value', isis_config) + low_label_value = dict_search('segment_routing.global_block.low_label_value', isis_config) + # If segment routing global block high value is blank, throw error + if low_label_value and not high_label_value: + raise ConfigError('Segment routing global block high value must not be left blank') + # If segment routing global block low value is blank, throw error + if high_label_value and not low_label_value: + raise ConfigError('Segment routing global block low value must not be left blank') + # If segment routing global block low value is higher than the high value, throw error + if int(low_label_value) > int(high_label_value): + raise ConfigError('Segment routing global block low value must be lower than high value') + + if dict_search('segment_routing.local_block', isis_config): + high_label_value = dict_search('segment_routing.local_block.high_label_value', isis_config) + low_label_value = dict_search('segment_routing.local_block.low_label_value', isis_config) + # If segment routing local block high value is blank, throw error + if low_label_value and not high_label_value: + raise ConfigError('Segment routing local block high value must not be left blank') + # If segment routing local block low value is blank, throw error + if high_label_value and not low_label_value: + raise ConfigError('Segment routing local block low value must not be left blank') + # If segment routing local block low value is higher than the high value, throw error + if int(low_label_value) > int(high_label_value): + raise ConfigError('Segment routing local block low value must be lower than high value') + return None def generate(isis): -- cgit v1.2.3 From f78201c25611cf6b8bc1ef7ff9ff0b7e4c992519 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 6 Jan 2021 12:09:37 +0100 Subject: bgp: T2174: verify() proper existance of remote-as --- src/conf_mode/protocols_bgp.py | 43 +++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) (limited to 'src/conf_mode') diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py index a3f32fd2d..d0dfb55ec 100755 --- a/src/conf_mode/protocols_bgp.py +++ b/src/conf_mode/protocols_bgp.py @@ -50,32 +50,37 @@ def verify(bgp): # Check if declared more than one ASN if len(bgp) > 1: - raise ConfigError('Only one BGP AS can be defined!') + raise ConfigError('Only one BGP AS number can be defined!') for asn, asn_config in bgp.items(): + import pprint + pprint.pprint(asn_config) + # Common verification for both peer-group and neighbor statements - for neigh in ['neighbor', 'peer_group']: + for neighbor in ['neighbor', 'peer_group']: # bail out early if there is no neighbor or peer-group statement # this also saves one indention level - if neigh not in asn_config: + if neighbor not in asn_config: + print(f'no {neighbor} found in config') continue - #for neighbor, config in asn_config[neigh].items(): - ''' - # These checks need to be modified. Because peer-group can be declared without 'remote-as'. - # When 'remote-as' configured for specific neighbor in peer-group. For example - # - - set protocols nbgp 65001 neighbor 100.64.0.2 peer-group 'FOO' - set protocols nbgp 65001 neighbor 100.64.0.2 remote-as '65002' - set protocols nbgp 65001 peer-group FOO - - ''' - #if 'remote_as' not in config and 'peer_group' not in config: - # raise ConfigError(f'BGP remote-as must be specified for "{neighbor}"!') - - #if 'remote_as' in config and 'peer_group' in config: - # raise ConfigError(f'BGP peer-group member "{neighbor}" cannot override remote-as of peer-group!') + for peer, peer_config in asn_config[neighbor].items(): + # Only regular "neighbor" statement can have a peer-group set + # Check if the configure peer-group exists + if 'peer_group' in peer_config: + peer_group = peer_config['peer_group'] + if peer_group not in asn_config['peer_group']: + raise ConfigError(f'Specified peer-group "{peer_group}" for '\ + f'neighbor "{neighbor}" does not exist!') + + # Some checks can/must only be done on a neighbor and nor a peer-group + if neighbor == 'neighbor': + # remote-as must be either set explicitly for the neighbor + # or for the entire peer-group + if 'remote_as' not in peer_config: + peer_group = peer_config['peer_group'] + if 'remote_as' not in asn_config['peer_group'][peer_group]: + raise ConfigError('Remote AS must be set for neighbor or peer-group!') return None -- cgit v1.2.3 From 1f82b771f685bbb493958cdeabf05549266f2aa8 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 17:01:55 +0100 Subject: bgp: T2174: verify() existence of route-map and prefix-list --- src/conf_mode/protocols_bgp.py | 46 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) (limited to 'src/conf_mode') diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py index d0dfb55ec..678be5066 100755 --- a/src/conf_mode/protocols_bgp.py +++ b/src/conf_mode/protocols_bgp.py @@ -17,10 +17,11 @@ from sys import exit from vyos.config import Config -from vyos.util import call -from vyos.util import dict_search +from vyos.configdict import dict_merge from vyos.template import render from vyos.template import render_to_string +from vyos.util import call +from vyos.util import dict_search from vyos import ConfigError from vyos import frr from vyos import airbag @@ -42,6 +43,16 @@ def get_config(): if not conf.exists(base + ['route-map']): call('vtysh -c \"conf t\" -c \"no ip protocol bgp\" ') + # We also need some additional information from the config, + # prefix-lists and route-maps for instance. + base = ['policy'] + tmp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True) + # As we only support one ASN (later checked in begin of verify()) we add the + # new information only to the first AS number + asn = next(iter(bgp)) + # Merge policy dict into bgp dict + bgp[asn] = dict_merge(tmp, bgp[asn]) + return bgp def verify(bgp): @@ -78,10 +89,37 @@ def verify(bgp): # remote-as must be either set explicitly for the neighbor # or for the entire peer-group if 'remote_as' not in peer_config: - peer_group = peer_config['peer_group'] - if 'remote_as' not in asn_config['peer_group'][peer_group]: + if 'peer_group' not in peer_config or 'remote_as' not in asn_config['peer_group'][peer_config['peer_group']]: raise ConfigError('Remote AS must be set for neighbor or peer-group!') + for afi in ['ipv4_unicast', 'ipv6_unicast']: + # Bail out early if address family is not configured + if 'address_family' not in peer_config or afi not in peer_config['address_family']: + continue + + afi_config = peer_config['address_family'][afi] + # Validate if configured Prefix list exists + if 'prefix_list' in afi_config: + for tmp in ['import', 'export']: + if tmp in afi_config['prefix_list']: + if afi == 'ipv4_unicast': + prefix_list = afi_config['prefix_list'][tmp] + if 'prefix_list' not in asn_config or prefix_list not in asn_config['prefix_list']: + raise ConfigError(f'prefix-list "{prefix_list}" used for "{tmp}" does not exist!') + if afi == 'ipv6_unicast': + prefix_list = afi_config['prefix_list6'][tmp] + if 'prefix_list6' not in asn_config or prefix_list not in asn_config['prefix_list6']: + raise ConfigError(f'prefix-list "{prefix_list}" used for "{tmp}" does not exist!') + + + if 'route_map' in afi_config: + for tmp in ['import', 'export']: + if tmp in afi_config['route_map']: + route_map = afi_config['route_map'][tmp] + if 'route_map' not in asn_config or route_map not in asn_config['route_map']: + raise ConfigError(f'route-map "{route_map}" used for "{tmp}" does not exist!') + + return None def generate(bgp): -- cgit v1.2.3 From b9feaf0d6be3adf179df6f35fcd8416d128750f6 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 18:33:23 +0100 Subject: login: radius: T3192: support IPv6 server(s) and source-address --- data/templates/login/pam_radius_auth.conf.tmpl | 33 +++++++++++ .../system-login/pam_radius_auth.conf.tmpl | 16 ------ .../include/radius-server-ipv4-ipv6.xml.i | 32 +++++++++++ .../include/source-address-ipv4-ipv6.xml.i | 1 + interface-definitions/system-login.xml.in | 2 +- smoketest/scripts/cli/test_system_login.py | 66 +++++++++++++++++++++- src/conf_mode/system-login.py | 47 ++++++++------- 7 files changed, 157 insertions(+), 40 deletions(-) create mode 100644 data/templates/login/pam_radius_auth.conf.tmpl delete mode 100644 data/templates/system-login/pam_radius_auth.conf.tmpl create mode 100644 interface-definitions/include/radius-server-ipv4-ipv6.xml.i (limited to 'src/conf_mode') diff --git a/data/templates/login/pam_radius_auth.conf.tmpl b/data/templates/login/pam_radius_auth.conf.tmpl new file mode 100644 index 000000000..56a5e10ee --- /dev/null +++ b/data/templates/login/pam_radius_auth.conf.tmpl @@ -0,0 +1,33 @@ +# Automatically generated by system-login.py +# RADIUS configuration file + +{# RADIUS IPv6 source address must be specified in [] notation #} +{% set source_address = namespace() %} +{% if radius_source_address is defined and radius_source_address is not none %} +{% for address in radius_source_address %} +{% if address | is_ipv4 %} +{% set source_address.ipv4 = address %} +{% elif address | is_ipv6 %} +{% set source_address.ipv6 = "[" + address + "]" %} +{% endif %} +{% endfor %} +{% endif %} +{% if radius_server is defined and radius_server is not none %} +# server[:port] shared_secret timeout source_ip +{% for server in radius_server | sort(attribute='priority') if not server.disabled %} +{# RADIUS IPv6 servers must be specified in [] notation #} +{% if server.address | is_ipv4 %} +{{ server.address }}:{{ server.port }} {{ "%-25s" | format(server.key) }} {{ "%-10s" | format(server.timeout) }} {{ source_address.ipv4 if source_address.ipv4 is defined }} +{% else %} +[{{ server.address }}]:{{ server.port }} {{ "%-25s" | format(server.key) }} {{ "%-10s" | format(server.timeout) }} {{ source_address.ipv6 if source_address.ipv6 is defined }} +{% endif %} +{% endfor %} + +priv-lvl 15 +mapped_priv_user radius_priv_user + +{% if radius_vrf %} +vrf-name {{ radius_vrf }} +{% endif %} +{% endif %} + diff --git a/data/templates/system-login/pam_radius_auth.conf.tmpl b/data/templates/system-login/pam_radius_auth.conf.tmpl deleted file mode 100644 index ec2d6df95..000000000 --- a/data/templates/system-login/pam_radius_auth.conf.tmpl +++ /dev/null @@ -1,16 +0,0 @@ -# Automatically generated by system-login.py -# RADIUS configuration file -{% if radius_server %} -# server[:port] shared_secret timeout source_ip -{% for s in radius_server|sort(attribute='priority') if not s.disabled %} -{% set addr_port = s.address + ":" + s.port %} -{{ "%-22s" | format(addr_port) }} {{ "%-25s" | format(s.key) }} {{ "%-10s" | format(s.timeout) }} {{ radius_source_address if radius_source_address }} -{% endfor %} - -priv-lvl 15 -mapped_priv_user radius_priv_user - -{% if radius_vrf %} -vrf-name {{ radius_vrf }} -{% endif %} -{% endif %} diff --git a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i new file mode 100644 index 000000000..e947c09e2 --- /dev/null +++ b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i @@ -0,0 +1,32 @@ + + + + RADIUS based user authentication + + + #include + + + RADIUS server configuration + + ipv4 + RADIUS server IPv4 address + + + ipv6 + RADIUS server IPv6 address + + + + + + + + #include + #include + #include + + + + + diff --git a/interface-definitions/include/source-address-ipv4-ipv6.xml.i b/interface-definitions/include/source-address-ipv4-ipv6.xml.i index 004e04f7b..4da4698c2 100644 --- a/interface-definitions/include/source-address-ipv4-ipv6.xml.i +++ b/interface-definitions/include/source-address-ipv4-ipv6.xml.i @@ -17,6 +17,7 @@ + diff --git a/interface-definitions/system-login.xml.in b/interface-definitions/system-login.xml.in index 0bea6a22d..6c573bf96 100644 --- a/interface-definitions/system-login.xml.in +++ b/interface-definitions/system-login.xml.in @@ -110,7 +110,7 @@ - #include + #include diff --git a/smoketest/scripts/cli/test_system_login.py b/smoketest/scripts/cli/test_system_login.py index 6188cf38b..bb6f57fc2 100755 --- a/smoketest/scripts/cli/test_system_login.py +++ b/smoketest/scripts/cli/test_system_login.py @@ -24,8 +24,10 @@ from platform import release as kernel_version from subprocess import Popen, PIPE from vyos.configsession import ConfigSession +from vyos.configsession import ConfigSessionError from vyos.util import cmd from vyos.util import read_file +from vyos.template import inc_ip base_path = ['system', 'login'] users = ['vyos1', 'vyos2'] @@ -42,7 +44,7 @@ class TestSystemLogin(unittest.TestCase): self.session.commit() del self.session - def test_local_user(self): + def test_system_login_user(self): # Check if user can be created and we can SSH to localhost self.session.set(['service', 'ssh', 'port', '22']) @@ -82,7 +84,7 @@ class TestSystemLogin(unittest.TestCase): for option in options: self.assertIn(f'{option}=y', kernel_config) - def test_radius_config(self): + def test_system_login_radius_ipv4(self): # Verify generated RADIUS configuration files radius_key = 'VyOSsecretVyOS' @@ -95,6 +97,12 @@ class TestSystemLogin(unittest.TestCase): self.session.set(base_path + ['radius', 'server', radius_server, 'port', radius_port]) self.session.set(base_path + ['radius', 'server', radius_server, 'timeout', radius_timeout]) self.session.set(base_path + ['radius', 'source-address', radius_source]) + self.session.set(base_path + ['radius', 'source-address', inc_ip(radius_source, 1)]) + + # check validate() - Only one IPv4 source-address supported + with self.assertRaises(ConfigSessionError): + self.session.commit() + self.session.delete(base_path + ['radius', 'source-address', inc_ip(radius_source, 1)]) self.session.commit() @@ -130,5 +138,59 @@ class TestSystemLogin(unittest.TestCase): tmp = re.findall(r'group:\s+mapname\s+files', nsswitch_conf) self.assertTrue(tmp) + def test_system_login_radius_ipv6(self): + # Verify generated RADIUS configuration files + + radius_key = 'VyOS-VyOS' + radius_server = '2001:db8::1' + radius_source = '::1' + radius_port = '4000' + radius_timeout = '4' + + self.session.set(base_path + ['radius', 'server', radius_server, 'key', radius_key]) + self.session.set(base_path + ['radius', 'server', radius_server, 'port', radius_port]) + self.session.set(base_path + ['radius', 'server', radius_server, 'timeout', radius_timeout]) + self.session.set(base_path + ['radius', 'source-address', radius_source]) + self.session.set(base_path + ['radius', 'source-address', inc_ip(radius_source, 1)]) + + # check validate() - Only one IPv4 source-address supported + with self.assertRaises(ConfigSessionError): + self.session.commit() + self.session.delete(base_path + ['radius', 'source-address', inc_ip(radius_source, 1)]) + + self.session.commit() + + # this file must be read with higher permissions + pam_radius_auth_conf = cmd('sudo cat /etc/pam_radius_auth.conf') + tmp = re.findall(r'\n?\[{}\]:{}\s+{}\s+{}\s+\[{}\]'.format(radius_server, + radius_port, radius_key, radius_timeout, + radius_source), pam_radius_auth_conf) + self.assertTrue(tmp) + + # required, static options + self.assertIn('priv-lvl 15', pam_radius_auth_conf) + self.assertIn('mapped_priv_user radius_priv_user', pam_radius_auth_conf) + + # PAM + pam_common_account = read_file('/etc/pam.d/common-account') + self.assertIn('pam_radius_auth.so', pam_common_account) + + pam_common_auth = read_file('/etc/pam.d/common-auth') + self.assertIn('pam_radius_auth.so', pam_common_auth) + + pam_common_session = read_file('/etc/pam.d/common-session') + self.assertIn('pam_radius_auth.so', pam_common_session) + + pam_common_session_noninteractive = read_file('/etc/pam.d/common-session-noninteractive') + self.assertIn('pam_radius_auth.so', pam_common_session_noninteractive) + + # NSS + nsswitch_conf = read_file('/etc/nsswitch.conf') + tmp = re.findall(r'passwd:\s+mapuid\s+files\s+mapname', nsswitch_conf) + self.assertTrue(tmp) + + tmp = re.findall(r'group:\s+mapname\s+files', nsswitch_conf) + self.assertTrue(tmp) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/system-login.py b/src/conf_mode/system-login.py index 39bad717d..92f717df8 100755 --- a/src/conf_mode/system-login.py +++ b/src/conf_mode/system-login.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2020 VyOS maintainers and contributors +# Copyright (C) 2020-2021 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 @@ -25,6 +25,7 @@ from sys import exit from vyos.config import Config from vyos.template import render +from vyos.template import is_ipv4 from vyos.util import cmd, call, DEVNULL, chmod_600, chmod_755 from vyos import ConfigError @@ -38,7 +39,7 @@ default_config_data = { 'add_users': [], 'del_users': [], 'radius_server': [], - 'radius_source_address': '', + 'radius_source_address': [], 'radius_vrf': '' } @@ -134,7 +135,7 @@ def get_config(config=None): conf.set_level(base_level + ['radius']) if conf.exists(['source-address']): - login['radius_source_address'] = conf.return_value(['source-address']) + login['radius_source_address'] = conf.return_values(['source-address']) # retrieve VRF instance if conf.exists(['vrf']): @@ -214,6 +215,17 @@ def verify(login): if fail: raise ConfigError('At least one RADIUS server must be active.') + ipv4_count = 0 + ipv6_count = 0 + for address in login['radius_source_address']: + if is_ipv4(address): ipv4_count += 1 + else: ipv6_count += 1 + + if ipv4_count > 1: + raise ConfigError('Only one IPv4 source-address can be set!') + if ipv6_count > 1: + raise ConfigError('Only one IPv6 source-address can be set!') + vrf_name = login['radius_vrf'] if vrf_name and vrf_name not in interfaces(): raise ConfigError(f'VRF "{vrf_name}" does not exist') @@ -255,13 +267,8 @@ def generate(login): pass if len(login['radius_server']) > 0: - render(radius_config_file, 'system-login/pam_radius_auth.conf.tmpl', - login) - - uid = getpwnam('root').pw_uid - gid = getpwnam('root').pw_gid - os.chown(radius_config_file, uid, gid) - chmod_600(radius_config_file) + render(radius_config_file, 'login/pam_radius_auth.conf.tmpl', login, + permission=0o600, user='root', group='root') else: if os.path.isfile(radius_config_file): os.unlink(radius_config_file) @@ -284,16 +291,15 @@ def apply(login): # we need to use '' quotes when passing formatted data to the shell # else it will not work as some data parts are lost in translation if user['password_encrypted']: - command += " -p '{}'".format(user['password_encrypted']) + command += " -p '{password_encrypted}'".format(**user) if user['full_name']: - command += " -c '{}'".format(user['full_name']) + command += " -c '{full_name}'".format(**user) if user['home_dir']: - command += " -d '{}'".format(user['home_dir']) + command += " -d '{home_dir}'".format(**user) - command += " -G frrvty,vyattacfg,sudo,adm,dip,disk" - command += " {}".format(user['name']) + command += " -G frrvty,vyattacfg,sudo,adm,dip,disk {name}".format(**user) try: cmd(command) @@ -321,10 +327,9 @@ def apply(login): for id in user['public_keys']: line = '' if id['options']: - line = '{} '.format(id['options']) + line = '{options} '.format(**id) - line += '{} {} {}\n'.format(id['type'], - id['key'], id['name']) + line += '{type} {key} {name}\n'.format(**id) f.write(line) os.chown(ssh_key_file, uid, gid) @@ -339,8 +344,8 @@ def apply(login): try: # Logout user if he is logged in if user in list(set([tmp[0] for tmp in users()])): - print('{} is logged in, forcing logout'.format(user)) - call('pkill -HUP -u {}'.format(user)) + print(f'{user} is logged in, forcing logout') + call(f'pkill -HUP -u {user}') # Remove user account but leave home directory to be safe call(f'userdel -r {user}', stderr=DEVNULL) @@ -356,7 +361,7 @@ def apply(login): env = os.environ.copy() env['DEBIAN_FRONTEND'] = 'noninteractive' # Enable RADIUS in PAM - cmd("pam-auth-update --package --enable radius", env=env) + cmd('pam-auth-update --package --enable radius', env=env) # Make NSS system aware of RADIUS, too command = "sed -i -e \'/\smapname/b\' \ -- cgit v1.2.3 From 65ee3a66077c7708f366d9492033634024887545 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 21:30:00 +0100 Subject: ssh: T2635: change sshd_config path to /run/sshd --- data/templates/ssh/sshd_config.tmpl | 1 + smoketest/scripts/cli/test_service_ssh.py | 2 +- src/conf_mode/ssh.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src/conf_mode') diff --git a/data/templates/ssh/sshd_config.tmpl b/data/templates/ssh/sshd_config.tmpl index 52d537aca..1dc700d38 100644 --- a/data/templates/ssh/sshd_config.tmpl +++ b/data/templates/ssh/sshd_config.tmpl @@ -27,6 +27,7 @@ Banner /etc/issue.net Subsystem sftp /usr/lib/openssh/sftp-server UsePAM yes PermitRootLogin no +PidFile /run/sshd/sshd.pid # # User configurable section diff --git a/smoketest/scripts/cli/test_service_ssh.py b/smoketest/scripts/cli/test_service_ssh.py index 0bb907c3a..e9e4639fc 100755 --- a/smoketest/scripts/cli/test_service_ssh.py +++ b/smoketest/scripts/cli/test_service_ssh.py @@ -25,7 +25,7 @@ from vyos.util import process_named_running from vyos.util import read_file PROCESS_NAME = 'sshd' -SSHD_CONF = '/run/ssh/sshd_config' +SSHD_CONF = '/run/sshd/sshd_config' base_path = ['service', 'ssh'] vrf = 'ssh-test' diff --git a/src/conf_mode/ssh.py b/src/conf_mode/ssh.py index 8f99053d2..07c057fd7 100755 --- a/src/conf_mode/ssh.py +++ b/src/conf_mode/ssh.py @@ -28,7 +28,7 @@ from vyos import ConfigError from vyos import airbag airbag.enable() -config_file = r'/run/ssh/sshd_config' +config_file = r'/run/sshd/sshd_config' systemd_override = r'/etc/systemd/system/ssh.service.d/override.conf' def get_config(config=None): -- cgit v1.2.3 From dcdc4f3ea27f1a26f8baa6b72b51c7911f21e6ba Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:22:58 +0100 Subject: ssh: T2635: harden Jinja2 template and daemon startup --- data/templates/ssh/sshd_config.tmpl | 30 +++++++++++++++--------------- smoketest/scripts/cli/test_service_ssh.py | 12 +++++------- src/conf_mode/ssh.py | 5 ++--- 3 files changed, 22 insertions(+), 25 deletions(-) (limited to 'src/conf_mode') diff --git a/data/templates/ssh/sshd_config.tmpl b/data/templates/ssh/sshd_config.tmpl index 1dc700d38..7d7257cae 100644 --- a/data/templates/ssh/sshd_config.tmpl +++ b/data/templates/ssh/sshd_config.tmpl @@ -48,59 +48,59 @@ LogLevel {{ loglevel | upper }} # Specifies whether password authentication is allowed PasswordAuthentication {{ "no" if disable_password_authentication is defined else "yes" }} -{% if listen_address %} +{% if listen_address is defined and listen_address is not none %} # Specifies the local addresses sshd should listen on {% for address in listen_address %} ListenAddress {{ address }} {% endfor %} {% endif %} -{% if ciphers %} +{% if ciphers is defined and ciphers is not none %} # Specifies the ciphers allowed for protocol version 2 -{% set value = ciphers if ciphers is string else ciphers | join(',') %} +{% set value = ciphers if ciphers is string else ciphers | join(',') %} Ciphers {{ value }} {% endif %} -{% if mac %} +{% if mac is defined and mac is not none %} # Specifies the available MAC (message authentication code) algorithms -{% set value = mac if mac is string else mac | join(',') %} +{% set value = mac if mac is string else mac | join(',') %} MACs {{ value }} {% endif %} -{% if key_exchange %} +{% if key_exchange is defined and key_exchange is not none %} # Specifies the available Key Exchange algorithms -{% set value = key_exchange if key_exchange is string else key_exchange | join(',') %} +{% set value = key_exchange if key_exchange is string else key_exchange | join(',') %} KexAlgorithms {{ value }} {% endif %} -{% if access_control is defined %} -{% if access_control.allow is defined %} +{% if access_control is defined and access_control is not none %} +{% if access_control.allow is defined and access_control.allow is not none %} {% if access_control.allow.user is defined %} # If specified, login is allowed only for user names that match -{% set value = access_control.allow.user if access_control.allow.user is string else access_control.allow.user | join(' ') %} +{% set value = access_control.allow.user if access_control.allow.user is string else access_control.allow.user | join(' ') %} AllowUsers {{ value }} {% endif %} {% if access_control.allow.group is defined %} # If specified, login is allowed only for users whose primary group or supplementary group list matches -{% set value = access_control.allow.group if access_control.allow.group is string else access_control.allow.group | join(' ') %} +{% set value = access_control.allow.group if access_control.allow.group is string else access_control.allow.group | join(' ') %} AllowGroups {{ value }} {% endif %} {% endif %} -{% if access_control.deny is defined %} +{% if access_control.deny is defined and access_control.deny is not none %} {% if access_control.deny.user is defined %} # Login is disallowed for user names that match -{% set value = access_control.deny.user if access_control.deny.user is string else access_control.deny.user | join(' ') %} +{% set value = access_control.deny.user if access_control.deny.user is string else access_control.deny.user | join(' ') %} DenyUsers {{ value }} {% endif %} {% if access_control.deny.group is defined %} # Login is disallowed for users whose primary group or supplementary group list matches -{% set value = access_control.deny.group if access_control.deny.group is string else access_control.deny.group | join(' ') %} +{% set value = access_control.deny.group if access_control.deny.group is string else access_control.deny.group | join(' ') %} DenyGroups {{ value }} {% endif %} {% endif %} {% endif %} -{% if client_keepalive_interval %} +{% if client_keepalive_interval is defined and client_keepalive_interval is not none %} # Sets a timeout interval in seconds after which if no data has been received from the client, # sshd(8) will send a message through the encrypted channel to request a response from the client ClientAliveInterval {{ client_keepalive_interval }} diff --git a/smoketest/scripts/cli/test_service_ssh.py b/smoketest/scripts/cli/test_service_ssh.py index e9e4639fc..eede042de 100755 --- a/smoketest/scripts/cli/test_service_ssh.py +++ b/smoketest/scripts/cli/test_service_ssh.py @@ -44,11 +44,6 @@ class TestServiceSSH(unittest.TestCase): def tearDown(self): # delete testing SSH config self.session.delete(base_path) - # restore "plain" SSH access - self.session.set(base_path) - # delete VRF - self.session.delete(['vrf', 'name', vrf]) - self.session.commit() del self.session @@ -109,7 +104,7 @@ class TestServiceSSH(unittest.TestCase): def test_ssh_multiple_listen_addresses(self): # Check if SSH service can be configured and runs with multiple # listen ports and listen-addresses - ports = ['22', '2222'] + ports = ['22', '2222', '2223', '2224'] for port in ports: self.session.set(base_path + ['port', port]) @@ -143,7 +138,7 @@ class TestServiceSSH(unittest.TestCase): with self.assertRaises(ConfigSessionError): self.session.commit() - self.session.set(['vrf', 'name', vrf, 'table', '1001']) + self.session.set(['vrf', 'name', vrf, 'table', '1338']) # commit changes self.session.commit() @@ -159,5 +154,8 @@ class TestServiceSSH(unittest.TestCase): tmp = cmd(f'ip vrf pids {vrf}') self.assertIn(PROCESS_NAME, tmp) + # delete VRF + self.session.delete(['vrf', 'name', vrf]) + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/src/conf_mode/ssh.py b/src/conf_mode/ssh.py index 07c057fd7..28e606663 100755 --- a/src/conf_mode/ssh.py +++ b/src/conf_mode/ssh.py @@ -68,6 +68,8 @@ def generate(ssh): render(config_file, 'ssh/sshd_config.tmpl', ssh) render(systemd_override, 'ssh/override.conf.tmpl', ssh) + # Reload systemd manager configuration + call('systemctl daemon-reload') return None @@ -76,9 +78,6 @@ def apply(ssh): # SSH access is removed in the commit call('systemctl stop ssh.service') - # Reload systemd manager configuration - call('systemctl daemon-reload') - if ssh: call('systemctl restart ssh.service') -- cgit v1.2.3 From e8a1c291b1d4b90709a68038e16522b4cee82904 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 21:28:04 +0100 Subject: login: radius: T3192: migrate to get_config_dict() --- data/templates/login/authorized_keys.tmpl | 9 + data/templates/login/pam_radius_auth.conf.tmpl | 29 +- .../include/radius-server-ipv4-ipv6.xml.i | 2 +- interface-definitions/system-login.xml.in | 13 +- python/vyos/util.py | 2 +- src/conf_mode/system-login.py | 473 ++++++++------------- 6 files changed, 211 insertions(+), 317 deletions(-) create mode 100644 data/templates/login/authorized_keys.tmpl (limited to 'src/conf_mode') diff --git a/data/templates/login/authorized_keys.tmpl b/data/templates/login/authorized_keys.tmpl new file mode 100644 index 000000000..639a80e1d --- /dev/null +++ b/data/templates/login/authorized_keys.tmpl @@ -0,0 +1,9 @@ +### Automatically generated by system-login.py ### + +{% if authentication is defined and authentication.public_keys is defined and authentication.public_keys is not none %} +{% for key, key_options in authentication.public_keys.items() %} +{# The whitespace after options is wisely chosen #} +{{ key_options.options + ' ' if key_options.options is defined }}{{ key_options.type }} {{ key_options.key }} {{ key }} +{% endfor %} +{% endif %} + diff --git a/data/templates/login/pam_radius_auth.conf.tmpl b/data/templates/login/pam_radius_auth.conf.tmpl index 56a5e10ee..fad8e7dcb 100644 --- a/data/templates/login/pam_radius_auth.conf.tmpl +++ b/data/templates/login/pam_radius_auth.conf.tmpl @@ -1,10 +1,11 @@ # Automatically generated by system-login.py # RADIUS configuration file +{% if radius is defined and radius is not none %} {# RADIUS IPv6 source address must be specified in [] notation #} {% set source_address = namespace() %} -{% if radius_source_address is defined and radius_source_address is not none %} -{% for address in radius_source_address %} +{% if radius.source_address is defined and radius.source_address is not none %} +{% for address in radius.source_address %} {% if address | is_ipv4 %} {% set source_address.ipv4 = address %} {% elif address | is_ipv6 %} @@ -12,22 +13,24 @@ {% endif %} {% endfor %} {% endif %} -{% if radius_server is defined and radius_server is not none %} +{% if radius.server is defined and radius.server is not none %} # server[:port] shared_secret timeout source_ip -{% for server in radius_server | sort(attribute='priority') if not server.disabled %} +{# .items() returns a tuple of two elements: key and value. 1 relates to the 2nd element i.e. the value and .priority relates to the key from the internal dict #} +{% for server, options in radius.server.items() | sort(attribute='1.priority') if not options.disabled %} {# RADIUS IPv6 servers must be specified in [] notation #} -{% if server.address | is_ipv4 %} -{{ server.address }}:{{ server.port }} {{ "%-25s" | format(server.key) }} {{ "%-10s" | format(server.timeout) }} {{ source_address.ipv4 if source_address.ipv4 is defined }} -{% else %} -[{{ server.address }}]:{{ server.port }} {{ "%-25s" | format(server.key) }} {{ "%-10s" | format(server.timeout) }} {{ source_address.ipv6 if source_address.ipv6 is defined }} -{% endif %} -{% endfor %} +{% if server | is_ipv4 %} +{{ server }}:{{ options.port }} {{ "%-25s" | format(options.key) }} {{ "%-10s" | format(options.timeout) }} {{ source_address.ipv4 if source_address.ipv4 is defined }} +{% else %} +[{{ server }}]:{{ options.port }} {{ "%-25s" | format(options.key) }} {{ "%-10s" | format(options.timeout) }} {{ source_address.ipv6 if source_address.ipv6 is defined }} +{% endif %} +{% endfor %} +{% endif %} priv-lvl 15 mapped_priv_user radius_priv_user -{% if radius_vrf %} -vrf-name {{ radius_vrf }} -{% endif %} +{% if radius.vrf is defined and radius.vrf is not none %} +vrf-name {{ radius.vrf }} +{% endif %} {% endif %} diff --git a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i index e947c09e2..e4919d86a 100644 --- a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i +++ b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i @@ -4,7 +4,6 @@ RADIUS based user authentication - #include RADIUS server configuration @@ -27,6 +26,7 @@ #include + #include diff --git a/interface-definitions/system-login.xml.in b/interface-definitions/system-login.xml.in index 6c573bf96..34e14d8e7 100644 --- a/interface-definitions/system-login.xml.in +++ b/interface-definitions/system-login.xml.in @@ -34,6 +34,7 @@ Invalid encrypted password for $VAR(../../@). + ! @@ -44,7 +45,7 @@ Remote access public keys - >identifier< + txt Key identifier used by ssh-keygen (usually of form user@host) @@ -61,7 +62,7 @@ - + Public key type ssh-dss ssh-rsa ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521 ssh-ed25519 @@ -86,7 +87,7 @@ - (ssh-dss|ssh-rsa|ecdsa-sha2-nistp256|ecdsa-sha2-nistp384|ecdsa-sha2-nistp521|ssh-ed25519) + ^(ssh-dss|ssh-rsa|ecdsa-sha2-nistp256|ecdsa-sha2-nistp384|ecdsa-sha2-nistp521|ssh-ed25519)$ @@ -119,7 +120,7 @@ Session timeout - 1-30 + u32:1-30 Session timeout in seconds (default: 2) @@ -127,18 +128,20 @@ Timeout must be between 1 and 30 seconds + 2 Server priority - 1-255 + u32:1-255 Server priority (default: 255) + 255 diff --git a/python/vyos/util.py b/python/vyos/util.py index 494c8155e..699f05892 100644 --- a/python/vyos/util.py +++ b/python/vyos/util.py @@ -311,7 +311,7 @@ def chmod_755(path): def makedir(path, user=None, group=None): if os.path.exists(path): return - os.mkdir(path) + os.makedirs(path, mode=0o755) chown(path, user, group) diff --git a/src/conf_mode/system-login.py b/src/conf_mode/system-login.py index 92f717df8..82accd404 100755 --- a/src/conf_mode/system-login.py +++ b/src/conf_mode/system-login.py @@ -16,34 +16,30 @@ import os -from crypt import crypt, METHOD_SHA512 -from netifaces import interfaces +from crypt import crypt +from crypt import METHOD_SHA512 from psutil import users -from pwd import getpwall, getpwnam +from pwd import getpwall +from pwd import getpwnam from spwd import getspnam from sys import exit from vyos.config import Config +from vyos.configdict import dict_merge +from vyos.configverify import verify_vrf from vyos.template import render from vyos.template import is_ipv4 -from vyos.util import cmd, call, DEVNULL, chmod_600, chmod_755 +from vyos.util import cmd +from vyos.util import call +from vyos.util import DEVNULL +from vyos.util import dict_search +from vyos.xml import defaults from vyos import ConfigError - from vyos import airbag airbag.enable() radius_config_file = "/etc/pam_radius_auth.conf" -default_config_data = { - 'deleted': False, - 'add_users': [], - 'del_users': [], - 'radius_server': [], - 'radius_source_address': [], - 'radius_vrf': '' -} - - def get_local_users(): """Return list of dynamically allocated users (see Debian Policy Manual)""" local_users = [] @@ -58,215 +54,130 @@ def get_local_users(): def get_config(config=None): - login = default_config_data if config: conf = config else: conf = Config() - base_level = ['system', 'login'] - - # We do not need to check if the nodes exist or not and bail out early - # ... this would interrupt the following logic on determine which users - # should be deleted and which users should stay. - # - # All fine so far! - - # Read in all local users and store to list - for username in conf.list_nodes(base_level + ['user']): - user = { - 'name': username, - 'password_plaintext': '', - 'password_encrypted': '!', - 'public_keys': [], - 'full_name': '', - 'home_dir': '/home/' + username, - } - conf.set_level(base_level + ['user', username]) - - # Plaintext password - if conf.exists(['authentication', 'plaintext-password']): - user['password_plaintext'] = conf.return_value( - ['authentication', 'plaintext-password']) - - # Encrypted password - if conf.exists(['authentication', 'encrypted-password']): - user['password_encrypted'] = conf.return_value( - ['authentication', 'encrypted-password']) - - # User real name - if conf.exists(['full-name']): - user['full_name'] = conf.return_value(['full-name']) - - # User home-directory - if conf.exists(['home-directory']): - user['home_dir'] = conf.return_value(['home-directory']) - - # Read in public keys - for id in conf.list_nodes(['authentication', 'public-keys']): - key = { - 'name': id, - 'key': '', - 'options': '', - 'type': '' - } - conf.set_level(base_level + ['user', username, 'authentication', - 'public-keys', id]) - - # Public Key portion - if conf.exists(['key']): - key['key'] = conf.return_value(['key']) - - # Options for individual public key - if conf.exists(['options']): - key['options'] = conf.return_value(['options']) - - # Type of public key - if conf.exists(['type']): - key['type'] = conf.return_value(['type']) - - # Append individual public key to list of user keys - user['public_keys'].append(key) - - login['add_users'].append(user) - - # - # RADIUS configuration - # - conf.set_level(base_level + ['radius']) - - if conf.exists(['source-address']): - login['radius_source_address'] = conf.return_values(['source-address']) - - # retrieve VRF instance - if conf.exists(['vrf']): - login['radius_vrf'] = conf.return_value(['vrf']) - - # Read in all RADIUS servers and store to list - for server in conf.list_nodes(['server']): - server_cfg = { - 'address': server, - 'disabled': False, - 'key': '', - 'port': '1812', - 'timeout': '2', - 'priority': 255 - } - conf.set_level(base_level + ['radius', 'server', server]) - - # Check if RADIUS server was temporary disabled - if conf.exists(['disable']): - server_cfg['disabled'] = True - - # RADIUS shared secret - if conf.exists(['key']): - server_cfg['key'] = conf.return_value(['key']) - - # RADIUS authentication port - if conf.exists(['port']): - server_cfg['port'] = conf.return_value(['port']) - - # RADIUS session timeout - if conf.exists(['timeout']): - server_cfg['timeout'] = conf.return_value(['timeout']) - - # Check if RADIUS server has priority - if conf.exists(['priority']): - server_cfg['priority'] = int(conf.return_value(['priority'])) - - # Append individual RADIUS server configuration to global server list - login['radius_server'].append(server_cfg) + base = ['system', 'login'] + login = conf.get_config_dict(base, key_mangling=('-', '_'), + get_first_key=True) # users no longer existing in the running configuration need to be deleted local_users = get_local_users() - cli_users = [tmp['name'] for tmp in login['add_users']] - # create a list of all users, cli and users - all_users = list(set(local_users+cli_users)) + cli_users = [] + if 'user' in login: + cli_users = list(login['user']) + + # XXX: T2665: we can not safely rely on the defaults() when there are + # tagNodes in place, it is better to blend in the defaults manually. + default_values = defaults(base + ['user']) + for user in login['user']: + login['user'][user] = dict_merge(default_values, login['user'][user]) + + # XXX: T2665: we can not safely rely on the defaults() when there are + # tagNodes in place, it is better to blend in the defaults manually. + default_values = defaults(base + ['radius', 'server']) + for server in dict_search('radius.server', login) or []: + login['radius']['server'][server] = dict_merge(default_values, + login['radius']['server'][server]) + + # XXX: for a yet unknown reason when we only have one source-address + # get_config_dict() will show a string over a string + if 'radius' in login and 'source_address' in login['radius']: + print(type(login['radius']['source_address'])) + if isinstance(login['radius']['source_address'], str): + login['radius']['source_address'] = [login['radius']['source_address']] - # Remove any normal users that dos not exist in the current configuration. - # This can happen if user is added but configuration was not saved and - # system is rebooted. - login['del_users'] = [tmp for tmp in all_users if tmp not in cli_users] + # create a list of all users, cli and users + all_users = list(set(local_users + cli_users)) + # We will remove any normal users that dos not exist in the current + # configuration. This can happen if user is added but configuration was not + # saved and system is rebooted. + rm_users = [tmp for tmp in all_users if tmp not in cli_users] + if rm_users: login.update({'rm_users' : rm_users}) return login - def verify(login): - cur_user = os.environ['SUDO_USER'] - if cur_user in login['del_users']: - raise ConfigError( - 'Attempting to delete current user: {}'.format(cur_user)) - - for user in login['add_users']: - for key in user['public_keys']: - if not key['type']: - raise ConfigError( - 'SSH public key type missing for "{name}"!'.format(**key)) - - if not key['key']: - raise ConfigError( - 'SSH public key for id "{name}" missing!'.format(**key)) + if 'rm_users' in login: + cur_user = os.environ['SUDO_USER'] + if cur_user in login['rm_users']: + raise ConfigError(f'Attempting to delete current user: {cur_user}') + + if 'user' in login: + for user, user_config in login['user'].items(): + for pubkey, pubkey_options in (dict_search('authentication.public_keys', user_config) or {}).items(): + if 'type' not in pubkey_options: + raise ConfigError(f'Missing type for public-key "{pubkey}"!') + if 'key' not in pubkey_options: + raise ConfigError(f'Missing key for public-key "{pubkey}"!') # At lease one RADIUS server must not be disabled - if len(login['radius_server']) > 0: + if 'radius' in login: + if 'server' not in login['radius']: + raise ConfigError('No RADIUS server defined!') + fail = True - for server in login['radius_server']: - if not server['disabled']: + for server, server_config in dict_search('radius.server', login).items(): + if 'key' not in server_config: + raise ConfigError(f'RADIUS server "{server}" requires key!') + + if 'disabled' not in server_config: fail = False + continue if fail: - raise ConfigError('At least one RADIUS server must be active.') + raise ConfigError('All RADIUS servers are disabled') - ipv4_count = 0 - ipv6_count = 0 - for address in login['radius_source_address']: - if is_ipv4(address): ipv4_count += 1 - else: ipv6_count += 1 + verify_vrf(login['radius']) - if ipv4_count > 1: - raise ConfigError('Only one IPv4 source-address can be set!') - if ipv6_count > 1: - raise ConfigError('Only one IPv6 source-address can be set!') + if 'source_address' in login['radius']: + ipv4_count = 0 + ipv6_count = 0 + for address in login['radius']['source_address']: + if is_ipv4(address): ipv4_count += 1 + else: ipv6_count += 1 - vrf_name = login['radius_vrf'] - if vrf_name and vrf_name not in interfaces(): - raise ConfigError(f'VRF "{vrf_name}" does not exist') + if ipv4_count > 1: + raise ConfigError('Only one IPv4 source-address can be set!') + if ipv6_count > 1: + raise ConfigError('Only one IPv6 source-address can be set!') return None def generate(login): # calculate users encrypted password - for user in login['add_users']: - if user['password_plaintext']: - user['password_encrypted'] = crypt( - user['password_plaintext'], METHOD_SHA512) - user['password_plaintext'] = '' - - # remove old plaintext password and set new encrypted password - env = os.environ.copy() - env['vyos_libexec_dir'] = '/usr/libexec/vyos' - - call("/opt/vyatta/sbin/my_delete system login user '{name}' " - "authentication plaintext-password" - .format(**user), env=env) - - call("/opt/vyatta/sbin/my_set system login user '{name}' " - "authentication encrypted-password '{password_encrypted}'" - .format(**user), env=env) - - else: - try: - if getspnam(user['name']).sp_pwdp == user['password_encrypted']: - # If the current encrypted bassword matches the encrypted password - # from the config - do not update it. This will remove the encrypted - # value from the system logs. - # - # The encrypted password will be set only once during the first boot - # after an image upgrade. - user['password_encrypted'] = '' - except: - pass - - if len(login['radius_server']) > 0: + if 'user' in login: + for user, user_config in login['user'].items(): + tmp = dict_search('authentication.plaintext_password', user_config) + if tmp: + encrypted_password = crypt(tmp, METHOD_SHA512) + login['user'][user]['authentication']['encrypted_password'] = encrypted_password + del login['user'][user]['authentication']['plaintext_password'] + + # remove old plaintext password and set new encrypted password + env = os.environ.copy() + env['vyos_libexec_dir'] = '/usr/libexec/vyos' + + call(f"/opt/vyatta/sbin/my_delete system login user '{user}' " + "authentication plaintext-password", env=env) + + call(f"/opt/vyatta/sbin/my_set system login user '{user}' " + "authentication encrypted-password '{encrypted_password}'", env=env) + else: + try: + if getspnam(user).sp_pwdp == dict_search('authentication.encrypted_password', user_config): + # If the current encrypted bassword matches the encrypted password + # from the config - do not update it. This will remove the encrypted + # value from the system logs. + # + # The encrypted password will be set only once during the first boot + # after an image upgrade. + del login['user'][user]['authentication']['encrypted_password'] + except: + pass + + if 'radius' in login: render(radius_config_file, 'login/pam_radius_auth.conf.tmpl', login, permission=0o600, user='root', group='root') else: @@ -277,93 +188,72 @@ def generate(login): def apply(login): - for user in login['add_users']: - # make new user using vyatta shell and make home directory (-m), - # default group of 100 (users) - command = "useradd -m -N" - # check if user already exists: - if user['name'] in get_local_users(): - # update existing account - command = "usermod" - - # all accounts use /bin/vbash - command += " -s /bin/vbash" - # we need to use '' quotes when passing formatted data to the shell - # else it will not work as some data parts are lost in translation - if user['password_encrypted']: - command += " -p '{password_encrypted}'".format(**user) - - if user['full_name']: - command += " -c '{full_name}'".format(**user) - - if user['home_dir']: - command += " -d '{home_dir}'".format(**user) - - command += " -G frrvty,vyattacfg,sudo,adm,dip,disk {name}".format(**user) - - try: - cmd(command) - - uid = getpwnam(user['name']).pw_uid - gid = getpwnam(user['name']).pw_gid - - # we should not rely on the value stored in user['home_dir'], as a - # crazy user will choose username root or any other system user - # which will fail. Should we deny using root at all? - home_dir = getpwnam(user['name']).pw_dir - - # install ssh keys - ssh_key_dir = home_dir + '/.ssh' - if not os.path.isdir(ssh_key_dir): - os.mkdir(ssh_key_dir) - os.chown(ssh_key_dir, uid, gid) - chmod_755(ssh_key_dir) - - ssh_key_file = ssh_key_dir + '/authorized_keys' - with open(ssh_key_file, 'w') as f: - f.write("# Automatically generated by VyOS\n") - f.write("# Do not edit, all changes will be lost\n") - - for id in user['public_keys']: - line = '' - if id['options']: - line = '{options} '.format(**id) - - line += '{type} {key} {name}\n'.format(**id) - f.write(line) - - os.chown(ssh_key_file, uid, gid) - chmod_600(ssh_key_file) - - except Exception as e: - print(e) - raise ConfigError('Adding user "{name}" raised exception' - .format(**user)) - - for user in login['del_users']: - try: - # Logout user if he is logged in - if user in list(set([tmp[0] for tmp in users()])): - print(f'{user} is logged in, forcing logout') - call(f'pkill -HUP -u {user}') - - # Remove user account but leave home directory to be safe - call(f'userdel -r {user}', stderr=DEVNULL) - - except Exception as e: - raise ConfigError(f'Deleting user "{user}" raised exception: {e}') + if 'user' in login: + for user, user_config in login['user'].items(): + # make new user using vyatta shell and make home directory (-m), + # default group of 100 (users) + command = 'useradd -m -N' + # check if user already exists: + if user in get_local_users(): + # update existing account + command = 'usermod' + + # all accounts use /bin/vbash + command += ' -s /bin/vbash' + # we need to use '' quotes when passing formatted data to the shell + # else it will not work as some data parts are lost in translation + tmp = dict_search('authentication.encrypted_password', user_config) + if tmp: command += f" -p '{tmp}'" + + tmp = dict_search('full_name', user_config) + if tmp: command += f" -c '{tmp}'" + + tmp = dict_search('home_directory', user_config) + if tmp: command += f" -d '{tmp}'" + else: command += f" -d '/home/{user}'" + + command += f' -G frrvty,vyattacfg,sudo,adm,dip,disk {user}' + + try: + cmd(command) + + # we should not rely on the value stored in + # user_config['home_directory'], as a crazy user will choose + # username root or any other system user which will fail. + # + # XXX: Should we deny using root at all? + home_dir = getpwnam(user).pw_dir + render(f'{home_dir}/.ssh/authorized_keys', 'login/authorized_keys.tmpl', + user_config, permission=0o600, user=user, group='users') + + except Exception as e: + raise ConfigError(f'Adding user "{user}" raised exception: "{e}"') + + if 'rm_users' in login: + for user in login['rm_users']: + try: + # Logout user if he is still logged in + if user in list(set([tmp[0] for tmp in users()])): + print(f'{user} is logged in, forcing logout!') + call(f'pkill -HUP -u {user}') + + # Remove user account but leave home directory to be safe + call(f'userdel -r {user}', stderr=DEVNULL) + + except Exception as e: + raise ConfigError(f'Deleting user "{user}" raised exception: {e}') # # RADIUS configuration # - if len(login['radius_server']) > 0: - try: - env = os.environ.copy() - env['DEBIAN_FRONTEND'] = 'noninteractive' + env = os.environ.copy() + env['DEBIAN_FRONTEND'] = 'noninteractive' + try: + if 'radius' in login: # Enable RADIUS in PAM cmd('pam-auth-update --package --enable radius', env=env) - - # Make NSS system aware of RADIUS, too + # Make NSS system aware of RADIUS + # This fancy snipped was copied from old Vyatta code command = "sed -i -e \'/\smapname/b\' \ -e \'/^passwd:/s/\s\s*/&mapuid /\' \ -e \'/^passwd:.*#/s/#.*/mapname &/\' \ @@ -371,31 +261,20 @@ def apply(login): -e \'/^group:.*#/s/#.*/ mapname &/\' \ -e \'/^group:[^#]*$/s/: */&mapname /\' \ /etc/nsswitch.conf" - - cmd(command) - - except Exception as e: - raise ConfigError('RADIUS configuration failed: {}'.format(e)) - - else: - try: - env = os.environ.copy() - env['DEBIAN_FRONTEND'] = 'noninteractive' - + else: # Disable RADIUS in PAM - cmd("pam-auth-update --package --remove radius", env=env) - + cmd('pam-auth-update --package --remove radius', env=env) + # Drop RADIUS from NSS NSS system + # This fancy snipped was copied from old Vyatta code command = "sed -i -e \'/^passwd:.*mapuid[ \t]/s/mapuid[ \t]//\' \ -e \'/^passwd:.*[ \t]mapname/s/[ \t]mapname//\' \ -e \'/^group:.*[ \t]mapname/s/[ \t]mapname//\' \ -e \'s/[ \t]*$//\' \ /etc/nsswitch.conf" - cmd(command) - - except Exception as e: - raise ConfigError( - 'Removing RADIUS configuration failed.\n{}'.format(e)) + cmd(command) + except Exception as e: + raise ConfigError(f'RADIUS configuration failed: {e}') return None -- cgit v1.2.3 From aa7b5972d48ad05824bfffdf1c5df5a6c2b1e37b Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 22:59:55 +0100 Subject: vyos.configverify: provide generic helper to check for interface existence --- python/vyos/configverify.py | 7 +++---- src/conf_mode/interfaces-ethernet.py | 8 ++++---- src/conf_mode/interfaces-tunnel.py | 8 ++++++-- 3 files changed, 13 insertions(+), 10 deletions(-) (limited to 'src/conf_mode') diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index b4447306e..bcaec55be 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -136,15 +136,14 @@ def verify_bridge_delete(config): 'Interface "{ifname}" cannot be deleted as it is a ' 'member of bridge "{is_bridge_member}"!'.format(**config)) -def verify_interface_exists(config): +def verify_interface_exists(ifname): """ Common helper function used by interface implementations to perform recurring validation if an interface actually exists. """ from netifaces import interfaces - if not config['ifname'] in interfaces(): - raise ConfigError('Interface "{ifname}" does not exist!' - .format(**config)) + if ifname not in interfaces(): + raise ConfigError(f'Interface "{ifname}" does not exist!') def verify_source_interface(config): """ diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index bc102826f..0a904c592 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -23,13 +23,13 @@ from vyos.config import Config from vyos.configdict import get_interface_dict from vyos.configverify import verify_address from vyos.configverify import verify_dhcpv6 +from vyos.configverify import verify_eapol from vyos.configverify import verify_interface_exists +from vyos.configverify import verify_mirror from vyos.configverify import verify_mtu from vyos.configverify import verify_mtu_ipv6 from vyos.configverify import verify_vlan_config from vyos.configverify import verify_vrf -from vyos.configverify import verify_eapol -from vyos.configverify import verify_mirror from vyos.ifconfig import EthernetIf from vyos.template import render from vyos.util import call @@ -59,7 +59,8 @@ def verify(ethernet): if 'deleted' in ethernet: return None - verify_interface_exists(ethernet) + ifname = ethernet['ifname'] + verify_interface_exists(ifname) if ethernet.get('speed', None) == 'auto': if ethernet.get('duplex', None) != 'auto': @@ -77,7 +78,6 @@ def verify(ethernet): verify_eapol(ethernet) verify_mirror(ethernet) - ifname = ethernet['ifname'] # verify offloading capabilities if 'offload' in ethernet and 'rps' in ethernet['offload']: if not os.path.exists(f'/sys/class/net/{ifname}/queues/rx-0/rps_cpus'): diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index 1a7e9a96d..ffeb57784 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2018-2020 VyOS maintainers and contributors +# Copyright (C) 2018-2021 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 @@ -24,10 +24,11 @@ from vyos.configdict import dict_merge from vyos.configdict import get_interface_dict from vyos.configdict import node_changed from vyos.configdict import leaf_node_changed -from vyos.configverify import verify_vrf from vyos.configverify import verify_address from vyos.configverify import verify_bridge_delete +from vyos.configverify import verify_interface_exists from vyos.configverify import verify_mtu_ipv6 +from vyos.configverify import verify_vrf from vyos.ifconfig import Interface from vyos.ifconfig import GREIf from vyos.ifconfig import GRETapIf @@ -122,6 +123,9 @@ def verify(tunnel): if 'local_ip' in tunnel and is_ipv6(tunnel['local_ip']): raise ConfigError('Can not use local IPv6 address is for mGRE tunnels') + if 'source_interface' in tunnel: + verify_interface_exists(tunnel['source_interface']) + def generate(tunnel): return None -- cgit v1.2.3 From 7a95aa238ba4d6743be645ffa3baef0e69251926 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:41:33 +0100 Subject: smoketest: ethernet: verify() speed/duplex must both be auto or discrete --- smoketest/scripts/cli/test_interfaces_ethernet.py | 12 ++++++++++++ src/conf_mode/interfaces-ethernet.py | 11 ++++------- 2 files changed, 16 insertions(+), 7 deletions(-) (limited to 'src/conf_mode') diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 0f3579c30..6b21853c3 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -136,6 +136,18 @@ class EthernetInterfaceTest(BasicInterfaceTest.BaseTest): # manually, else tearDown() will have problem in commit() self.session.delete(unknonw_interface) + def test_speed_duplex_verify(self): + for interface in self._interfaces: + self.session.set(self._base_path + [interface, 'speed', '1000']) + + # check validate() - if either speed or duplex is not auto, the + # other one must be manually configured, too + with self.assertRaises(ConfigSessionError): + self.session.commit() + self.session.set(self._base_path + [interface, 'duplex', 'full']) + + self.session.commit() + def test_eapol_support(self): for interface in self._interfaces: # Enable EAPoL diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py index 0a904c592..e7f0cd6a5 100755 --- a/src/conf_mode/interfaces-ethernet.py +++ b/src/conf_mode/interfaces-ethernet.py @@ -62,13 +62,10 @@ def verify(ethernet): ifname = ethernet['ifname'] verify_interface_exists(ifname) - if ethernet.get('speed', None) == 'auto': - if ethernet.get('duplex', None) != 'auto': - raise ConfigError('If speed is hardcoded, duplex must be hardcoded, too') - - if ethernet.get('duplex', None) == 'auto': - if ethernet.get('speed', None) != 'auto': - raise ConfigError('If duplex is hardcoded, speed must be hardcoded, too') + # No need to check speed and duplex keys as both have default values. + if ((ethernet['speed'] == 'auto' and ethernet['duplex'] != 'auto') or + (ethernet['speed'] != 'auto' and ethernet['duplex'] == 'auto')): + raise ConfigError('Speed/Duplex missmatch. Must be both auto or manually configured') verify_mtu(ethernet) verify_mtu_ipv6(ethernet) -- cgit v1.2.3