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. --- data/templates/frr/isis.frr.tmpl | 61 ++++++++ interface-definitions/protocols-isis.xml.in | 221 +++++++++++++++++++++++++++- src/conf_mode/protocols_isis.py | 33 ++++- 3 files changed, 312 insertions(+), 3 deletions(-) diff --git a/data/templates/frr/isis.frr.tmpl b/data/templates/frr/isis.frr.tmpl index a1dae0c7c..0477f2599 100644 --- a/data/templates/frr/isis.frr.tmpl +++ b/data/templates/frr/isis.frr.tmpl @@ -31,6 +31,67 @@ router isis {{ process }} {% if spf_interval is defined and spf_interval is not none %} spf-interval {{ spf_interval }} {% endif %} +{% if traffic_engineering is defined and traffic_engineering is not none %} +{% if traffic_engineering.enable is defined %} + mpls-te on +{% endif %} +{% if traffic_engineering.address is defined %} + mpls-te router-address {{ traffic_engineering.address }} +{% endif %} +{% if traffic_engineering.inter_as is defined %} +{% if traffic_engineering.inter_as.level_1 is defined %} + mpls-te inter-as level-1 +{% endif %} +{% if traffic_engineering.inter_as.level_1_2 is defined %} + mpls-te inter-as level-1-2 +{% endif %} +{% if traffic_engineering.inter_as.level_2 is defined %} + mpls-te inter-as level-2-only +{% endif %} +{% else %} + mpls-te inter-as +{% endif %} +{% endif %} +{% if segment_routing is defined %} +{% if segment_routing.enable is defined %} + segment-routing on +{% endif %} +{% if segment_routing.maximum_label_depth is defined %} + segment-routing node-msd {{ segment_routing.maximum_label_depth }} +{% endif %} +{% if segment_routing.global_block is defined %} + segment-routing global-block {{ segment_routing.global_block.low_label_value }} {{ segment_routing.global_block.high_label_value }} +{% endif %} +{% if segment_routing.local_block is defined %} + segment-routing local-block {{ segment_routing.global_block.low_label_value }} {{ segment_routing.local_block.high_label_value }} +{% endif %} +{% if segment_routing.prefix is defined %} +{% for prefixes in segment_routing.prefix %} +{% if segment_routing.prefix[prefixes].absolute is defined %} +{% if segment_routing.prefix[prefixes].absolute.value is defined %} + segment-routing prefix {{ prefixes }} absolute {{ segment_routing.prefix[prefixes].absolute.value }} +{% if segment_routing.prefix[prefixes].absolute.explicit_null is defined %} + segment-routing prefix {{ prefixes }} absolute {{ segment_routing.prefix[prefixes].absolute.value }} explicit-null +{% endif %} +{% if segment_routing.prefix[prefixes].absolute.no_php_flag is defined %} + segment-routing prefix {{ prefixes }} absolute {{ segment_routing.prefix[prefixes].absolute.value }} no-php-flag +{% endif %} +{% endif %} +{% if segment_routing.prefix[prefixes].index is defined %} +{% if segment_routing.prefix[prefixes].index.value is defined %} + segment-routing prefix {{ prefixes }} index {{ segment_routing.prefix[prefixes].index.value }} +{% if segment_routing.prefix[prefixes].index.explicit_null is defined %} + segment-routing prefix {{ prefixes }} index {{ segment_routing.prefix[prefixes].index.value }} explicit-null +{% endif %} +{% if segment_routing.prefix[prefixes].index.no_php_flag is defined %} + segment-routing prefix {{ prefixes }} index {{ segment_routing.prefix[prefixes].index.value }} no-php-flag +{% endif %} +{% endif %} +{% endif %} +{% endif %} +{% endfor %} +{% endif %} +{% endif %} {% if spf_delay_ietf is defined and spf_delay_ietf.init_delay is defined and spf_delay_ietf.init_delay is not none %} spf-delay-ietf init-delay {{ spf_delay_ietf.init_delay }} {% endif %} diff --git a/interface-definitions/protocols-isis.xml.in b/interface-definitions/protocols-isis.xml.in index 2ceb05180..2340079a6 100644 --- a/interface-definitions/protocols-isis.xml.in +++ b/interface-definitions/protocols-isis.xml.in @@ -1,4 +1,4 @@ - + @@ -246,6 +246,225 @@ + + + Show IS-IS neighbor adjacencies + + + + + Enable MPLS traffic engineering extensions + + + + + + + MPLS traffic engineering router ID + + ipv4 + IPv4 address + + + + + + + + + + + Segment-Routing (SPRING) settings + + + + + Enable segment-routing functionality + + + + + + Global block label range + + + + + The lower bound of the global block + + u32:16-1048575 + MPLS label value + + + + + + + + + The upper bound of the global block + + u32:16-1048575 + MPLS label value + + + + + + + + + + + + Maximum MPLS labels allowed for this router + + u32:1-16 + MPLS label depth + + + + + + + + + Static IPv4/IPv6 prefix segment/label mapping + + <x.x.x.x/x> <h:h:h:h:h:h:h:h/h> + + + + + + Specify the absolute value of prefix segment/label ID + + + + + Specify the absolute value of prefix segment/label ID + + u32:16-1048575 + The absolute segment/label ID value + + + + + + + + + Request upstream neighbor to replace segment/label with explicit null label + + + + + + Do not request penultimate hop popping for segment/label + + + + + + + + Specify the index value of prefix segment/label ID + + + + + Specify the index value of prefix segment/label ID + + u32:0-65535 + The index segment/label ID value + + + + + + + + + Request upstream neighbor to replace segment/label with explicit null label + + + + + + Do not request penultimate hop popping for segment/label + + + + + + + + + Redistribute information from another routing protocol 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 cbd2d71fc85f89f322f1d5c85052034b0b57b3b9 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Tue, 5 Jan 2021 21:15:02 +0100 Subject: smoketest: tunnel: fix setUp() ordering for reference base class members With commit ce809eee ("smoketest: mirror: T3169: re-add mirror / SPAN test case") the setUp() method has been re-organized not taking into account that there will already be a reference to self.session which will only be created by the base class. --- smoketest/scripts/cli/test_interfaces_tunnel.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/smoketest/scripts/cli/test_interfaces_tunnel.py b/smoketest/scripts/cli/test_interfaces_tunnel.py index f7b7f99ca..ca68cb8ba 100755 --- a/smoketest/scripts/cli/test_interfaces_tunnel.py +++ b/smoketest/scripts/cli/test_interfaces_tunnel.py @@ -67,9 +67,6 @@ class TunnelInterfaceTest(BasicInterfaceTest.BaseTest): self.local_v4 = '192.0.2.1' self.local_v6 = '2001:db8::1' - self.session.set(['interfaces', 'dummy', source_if, 'address', self.local_v4 + '/32']) - self.session.set(['interfaces', 'dummy', source_if, 'address', self.local_v6 + '/128']) - self._options = { 'tun10': ['encapsulation ipip', 'remote-ip 192.0.2.10', 'local-ip ' + self.local_v4], 'tun20': ['encapsulation gre', 'remote-ip 192.0.2.20', 'local-ip ' + self.local_v4], @@ -78,6 +75,9 @@ class TunnelInterfaceTest(BasicInterfaceTest.BaseTest): self._interfaces = list(self._options) super().setUp() + self.session.set(['interfaces', 'dummy', source_if, 'address', self.local_v4 + '/32']) + self.session.set(['interfaces', 'dummy', source_if, 'address', self.local_v6 + '/128']) + def tearDown(self): self.session.delete(['interfaces', 'dummy', source_if]) super().tearDown() -- 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(-) 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 216b4511329f96c2a9287e0caade0a4c76a9ba69 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 6 Jan 2021 12:43:11 +0100 Subject: smoketest: bgp: refactor verify part to be reusable --- smoketest/scripts/cli/test_protocols_bgp.py | 148 +++++++++++++++++----------- 1 file changed, 88 insertions(+), 60 deletions(-) diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index 941d7828f..540ef2ec4 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -28,6 +28,8 @@ base_path = ['protocols', 'bgp', ASN] neighbor_config = { '192.0.2.1' : { + 'cap_dynamic' : '', + 'cap_ext_next': '', 'remote_as' : '100', 'adv_interv' : '400', 'passive' : '', @@ -35,6 +37,7 @@ neighbor_config = { 'shutdown' : '', 'cap_over' : '', 'ttl_security': '5', + 'local_as' : '300', }, '192.0.2.2' : { 'remote_as' : '200', @@ -49,6 +52,7 @@ neighbor_config = { 'remote_as' : '200', 'passive' : '', 'multi_hop' : '5', + 'update_src' : 'lo', }, } @@ -63,18 +67,23 @@ peer_group_config = { # 'ttl_security': '5', }, 'bar' : { +# XXX: not available in current Perl backend +# 'description' : 'foo peer bar group', 'remote_as' : '200', 'shutdown' : '', 'no_cap_nego' : '', + 'local_as' : '300', }, 'baz' : { + 'cap_dynamic' : '', + 'cap_ext_next': '', 'remote_as' : '200', 'passive' : '', 'multi_hop' : '5', + 'update_src' : 'lo', }, } - def getFRRBGPconfig(): return cmd(f'vtysh -c "show run" | sed -n "/router bgp {ASN}/,/^!/p"') @@ -87,6 +96,35 @@ class TestProtocolsBGP(unittest.TestCase): self.session.commit() del self.session + def verify_frr_config(self, peer, peer_config, frrconfig): + # recurring patterns to verify for both a simple neighbor and a peer-group + if 'cap_dynamic' in peer_config: + self.assertIn(f' neighbor {peer} capability dynamic', frrconfig) + if 'cap_ext_next' in peer_config: + self.assertIn(f' neighbor {peer} capability extended-nexthop', frrconfig) + if 'description' in peer_config: + self.assertIn(f' neighbor {peer} description {peer_config["description"]}', frrconfig) + if 'no_cap_nego' in peer_config: + self.assertIn(f' neighbor {peer} dont-capability-negotiate', frrconfig) + if 'multi_hop' in peer_config: + self.assertIn(f' neighbor {peer} ebgp-multihop {peer_config["multi_hop"]}', frrconfig) + if 'local_as' in peer_config: + self.assertIn(f' neighbor {peer} local-as {peer_config["local_as"]}', frrconfig) + if 'cap_over' in peer_config: + self.assertIn(f' neighbor {peer} override-capability', frrconfig) + if 'passive' in peer_config: + self.assertIn(f' neighbor {peer} passive', frrconfig) + if 'password' in peer_config: + self.assertIn(f' neighbor {peer} password {peer_config["password"]}', frrconfig) + if 'remote_as' in peer_config: + self.assertIn(f' neighbor {peer} remote-as {peer_config["remote_as"]}', frrconfig) + if 'shutdown' in peer_config: + self.assertIn(f' neighbor {peer} shutdown', frrconfig) + if 'ttl_security' in peer_config: + self.assertIn(f' neighbor {peer} ttl-security hops {peer_config["ttl_security"]}', frrconfig) + if 'update_src' in peer_config: + self.assertIn(f' neighbor {peer} update-source {peer_config["update_src"]}', frrconfig) + def test_bgp_01_simple(self): router_id = '127.0.0.1' local_pref = '500' @@ -113,31 +151,41 @@ class TestProtocolsBGP(unittest.TestCase): self.assertTrue(process_named_running(PROCESS_NAME)) def test_bgp_02_neighbors(self): + # Test out individual neighbor configuration items, not all of them are + # also available to a peer-group! for neighbor, config in neighbor_config.items(): - if 'remote_as' in config: - self.session.set(base_path + ['neighbor', neighbor, 'remote-as', config["remote_as"]]) - if 'description' in config: - self.session.set(base_path + ['neighbor', neighbor, 'description', config["description"]]) - if 'passive' in config: - self.session.set(base_path + ['neighbor', neighbor, 'passive']) - if 'password' in config: - self.session.set(base_path + ['neighbor', neighbor, 'password', config["password"]]) - if 'shutdown' in config: - self.session.set(base_path + ['neighbor', neighbor, 'shutdown']) if 'adv_interv' in config: self.session.set(base_path + ['neighbor', neighbor, 'advertisement-interval', config["adv_interv"]]) + if 'cap_dynamic' in config: + self.session.set(base_path + ['neighbor', neighbor, 'capability', 'dynamic']) + if 'cap_ext_next' in config: + self.session.set(base_path + ['neighbor', neighbor, 'capability', 'extended-nexthop']) + if 'description' in config: + self.session.set(base_path + ['neighbor', neighbor, 'description', config["description"]]) if 'no_cap_nego' in config: self.session.set(base_path + ['neighbor', neighbor, 'disable-capability-negotiation']) - if 'port' in config: - self.session.set(base_path + ['neighbor', neighbor, 'port', config["port"]]) if 'multi_hop' in config: self.session.set(base_path + ['neighbor', neighbor, 'ebgp-multihop', config["multi_hop"]]) + if 'local_as' in config: + self.session.set(base_path + ['neighbor', neighbor, 'local-as', config["local_as"]]) if 'cap_over' in config: self.session.set(base_path + ['neighbor', neighbor, 'override-capability']) + if 'passive' in config: + self.session.set(base_path + ['neighbor', neighbor, 'passive']) + if 'password' in config: + self.session.set(base_path + ['neighbor', neighbor, 'password', config["password"]]) + if 'port' in config: + self.session.set(base_path + ['neighbor', neighbor, 'port', config["port"]]) + if 'remote_as' in config: + self.session.set(base_path + ['neighbor', neighbor, 'remote-as', config["remote_as"]]) if 'cap_strict' in config: self.session.set(base_path + ['neighbor', neighbor, 'strict-capability-match']) + if 'shutdown' in config: + self.session.set(base_path + ['neighbor', neighbor, 'shutdown']) if 'ttl_security' in config: self.session.set(base_path + ['neighbor', neighbor, 'ttl-security', 'hops', config["ttl_security"]]) + if 'update_src' in config: + self.session.set(base_path + ['neighbor', neighbor, 'update-source', config["update_src"]]) # commit changes self.session.commit() @@ -146,49 +194,45 @@ class TestProtocolsBGP(unittest.TestCase): frrconfig = getFRRBGPconfig() self.assertIn(f'router bgp {ASN}', frrconfig) - for neighbor, config in neighbor_config.items(): - if 'remote_as' in config: - self.assertIn(f' neighbor {neighbor} remote-as {config["remote_as"]}', frrconfig) - if 'description' in config: - self.assertIn(f' neighbor {neighbor} description {config["description"]}', frrconfig) - if 'passive' in config: - self.assertIn(f' neighbor {neighbor} passive', frrconfig) - if 'password' in config: - self.assertIn(f' neighbor {neighbor} password {config["password"]}', frrconfig) - if 'shutdown' in config: - self.assertIn(f' neighbor {neighbor} shutdown', frrconfig) + for peer, peer_config in neighbor_config.items(): if 'adv_interv' in config: - self.assertIn(f' neighbor {neighbor} advertisement-interval {config["adv_interv"]}', frrconfig) - if 'no_cap_nego' in config: - self.assertIn(f' neighbor {neighbor} dont-capability-negotiate', frrconfig) + self.assertIn(f' neighbor {peer} advertisement-interval {peer_config["adv_interv"]}', frrconfig) if 'port' in config: - self.assertIn(f' neighbor {neighbor} port {config["port"]}', frrconfig) - if 'multi_hop' in config: - self.assertIn(f' neighbor {neighbor} ebgp-multihop {config["multi_hop"]}', frrconfig) - if 'cap_over' in config: - self.assertIn(f' neighbor {neighbor} override-capability', frrconfig) + self.assertIn(f' neighbor {peer} port {peer_config["port"]}', frrconfig) if 'cap_strict' in config: - self.assertIn(f' neighbor {neighbor} strict-capability-match', frrconfig) - if 'ttl_security' in config: - self.assertIn(f' neighbor {neighbor} ttl-security hops {config["ttl_security"]}', frrconfig) + self.assertIn(f' neighbor {peer} strict-capability-match', frrconfig) + + self.verify_frr_config(peer, peer_config, frrconfig) def test_bgp_03_peer_groups(self): + # Test out individual peer-group configuration items for peer_group, config in peer_group_config.items(): - self.session.set(base_path + ['peer-group', peer_group, 'remote-as', config["remote_as"]]) - if 'passive' in config: - self.session.set(base_path + ['peer-group', peer_group, 'passive']) - if 'password' in config: - self.session.set(base_path + ['peer-group', peer_group, 'password', config["password"]]) - if 'shutdown' in config: - self.session.set(base_path + ['peer-group', peer_group, 'shutdown']) + if 'cap_dynamic' in config: + self.session.set(base_path + ['peer-group', peer_group, 'capability', 'dynamic']) + if 'cap_ext_next' in config: + self.session.set(base_path + ['peer-group', peer_group, 'capability', 'extended-nexthop']) + if 'description' in config: + self.session.set(base_path + ['peer-group', peer_group, 'description', config["description"]]) if 'no_cap_nego' in config: self.session.set(base_path + ['peer-group', peer_group, 'disable-capability-negotiation']) if 'multi_hop' in config: self.session.set(base_path + ['peer-group', peer_group, 'ebgp-multihop', config["multi_hop"]]) + if 'local_as' in config: + self.session.set(base_path + ['peer-group', peer_group, 'local-as', config["local_as"]]) if 'cap_over' in config: self.session.set(base_path + ['peer-group', peer_group, 'override-capability']) + if 'passive' in config: + self.session.set(base_path + ['peer-group', peer_group, 'passive']) + if 'password' in config: + self.session.set(base_path + ['peer-group', peer_group, 'password', config["password"]]) + if 'remote_as' in config: + self.session.set(base_path + ['peer-group', peer_group, 'remote-as', config["remote_as"]]) + if 'shutdown' in config: + self.session.set(base_path + ['peer-group', peer_group, 'shutdown']) if 'ttl_security' in config: self.session.set(base_path + ['peer-group', peer_group, 'ttl-security', 'hops', config["ttl_security"]]) + if 'update_src' in config: + self.session.set(base_path + ['peer-group', peer_group, 'update-source', config["update_src"]]) # commit changes self.session.commit() @@ -197,25 +241,9 @@ class TestProtocolsBGP(unittest.TestCase): frrconfig = getFRRBGPconfig() self.assertIn(f'router bgp {ASN}', frrconfig) - for peer_group, config in peer_group_config.items(): + for peer, peer_config in peer_group_config.items(): self.assertIn(f' neighbor {peer_group} peer-group', frrconfig) - - if 'remote_as' in config: - self.assertIn(f' neighbor {peer_group} remote-as {config["remote_as"]}', frrconfig) - if 'passive' in config: - self.assertIn(f' neighbor {peer_group} passive', frrconfig) - if 'password' in config: - self.assertIn(f' neighbor {peer_group} password {config["password"]}', frrconfig) - if 'shutdown' in config: - self.assertIn(f' neighbor {peer_group} shutdown', frrconfig) - if 'no_cap_nego' in config: - self.assertIn(f' neighbor {peer_group} dont-capability-negotiate', frrconfig) - if 'multi_hop' in config: - self.assertIn(f' neighbor {peer_group} ebgp-multihop {config["multi_hop"]}', frrconfig) - if 'cap_over' in config: - self.assertIn(f' neighbor {peer_group} override-capability', frrconfig) - if 'ttl_security' in config: - self.assertIn(f' neighbor {peer_group} ttl-security hops {config["ttl_security"]}', frrconfig) + self.verify_frr_config(peer, peer_config, frrconfig) if __name__ == '__main__': unittest.main(verbosity=2) -- cgit v1.2.3 From 9f33a14ac2a2e85da16b3169465155ca6114d09e Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 6 Jan 2021 13:10:01 +0100 Subject: smoketest: bgp: add ipv4/ipv6 address-family tests --- smoketest/scripts/cli/test_protocols_bgp.py | 93 +++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index 540ef2ec4..1d93aeda4 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -245,5 +245,98 @@ class TestProtocolsBGP(unittest.TestCase): self.assertIn(f' neighbor {peer_group} peer-group', frrconfig) self.verify_frr_config(peer, peer_config, frrconfig) + + def test_bgp_04_afi_ipv4(self): + networks = { + '10.0.0.0/8' : { + 'as_set' : '', + }, + '100.64.0.0/10' : { + 'as_set' : '', + }, + '192.168.0.0/16' : { + 'summary_only' : '', + }, + } + + # We want to redistribute ... + redistributes = ['connected', 'kernel', 'ospf', 'rip', 'static'] + for redistribute in redistributes: + self.session.set(base_path + ['address-family', 'ipv4-unicast', + 'redistribute', redistribute]) + + for network, network_config in networks.items(): + self.session.set(base_path + ['address-family', 'ipv4-unicast', + 'network', network]) + if 'as_set' in network_config: + self.session.set(base_path + ['address-family', 'ipv4-unicast', + 'aggregate-address', network, 'as-set']) + if 'summary_only' in network_config: + self.session.set(base_path + ['address-family', 'ipv4-unicast', + 'aggregate-address', network, 'summary-only']) + + # commit changes + self.session.commit() + + # Verify FRR bgpd configuration + frrconfig = getFRRBGPconfig() + self.assertIn(f'router bgp {ASN}', frrconfig) + self.assertIn(f' address-family ipv4 unicast', frrconfig) + + for redistribute in redistributes: + self.assertIn(f' redistribute {redistribute}', frrconfig) + + for network, network_config in networks.items(): + self.assertIn(f' network {network}', frrconfig) + if 'as_set' in network_config: + self.assertIn(f' aggregate-address {network} as-set', frrconfig) + if 'summary_only' in network_config: + self.assertIn(f' aggregate-address {network} summary-only', frrconfig) + + + def test_bgp_05_afi_ipv6(self): + networks = { + '2001:db8:100::/48' : { + }, + '2001:db8:200::/48' : { + }, + '2001:db8:300::/48' : { + 'summary_only' : '', + }, + } + + # We want to redistribute ... + redistributes = ['connected', 'kernel', 'ospfv3', 'ripng', 'static'] + for redistribute in redistributes: + self.session.set(base_path + ['address-family', 'ipv6-unicast', + 'redistribute', redistribute]) + + for network, network_config in networks.items(): + self.session.set(base_path + ['address-family', 'ipv6-unicast', + 'network', network]) + if 'summary_only' in network_config: + self.session.set(base_path + ['address-family', 'ipv6-unicast', + 'aggregate-address', network, 'summary-only']) + + # commit changes + self.session.commit() + + # Verify FRR bgpd configuration + frrconfig = getFRRBGPconfig() + self.assertIn(f'router bgp {ASN}', frrconfig) + self.assertIn(f' address-family ipv6 unicast', frrconfig) + + for redistribute in redistributes: + # FRR calls this OSPF6 + if redistribute == 'ospfv3': + redistribute = 'ospf6' + self.assertIn(f' redistribute {redistribute}', frrconfig) + + for network, network_config in networks.items(): + self.assertIn(f' network {network}', frrconfig) + if 'as_set' in network_config: + self.assertIn(f' aggregate-address {network} summary-only', frrconfig) + + if __name__ == '__main__': unittest.main(verbosity=2) -- cgit v1.2.3 From c1e38c5cde9dd6467c8226df12262d63fc6115bb Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 6 Jan 2021 17:39:56 +0100 Subject: ifconfig.interface: use Python3 f-ormat string --- python/vyos/ifconfig/interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 35a964110..4c05ac613 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -265,7 +265,7 @@ class Interface(Control): # If we can not connect to the interface then let the caller know # as the class could not be correctly initialised else: - raise Exception('interface "{}" not found'.format(self.config['ifname'])) + raise Exception(f'interface "{ifname}" not found!') # temporary list of assigned IP addresses self._addr = [] -- cgit v1.2.3 From 470d8f1fc555dc12251152b3dc620f611be604c0 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Wed, 6 Jan 2021 18:24:35 +0100 Subject: op-mode: rename "show version quagga" to "frr" as this is the routing core --- op-mode-definitions/show-version.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-mode-definitions/show-version.xml b/op-mode-definitions/show-version.xml index caa5bcda7..2202d27b3 100644 --- a/op-mode-definitions/show-version.xml +++ b/op-mode-definitions/show-version.xml @@ -20,7 +20,7 @@ echo "Package versions:"; dpkg -l | awk '$0~/>/{exit}1' - + Show Quagga version information -- cgit v1.2.3 From 903f4a3deee33c5836d67e3ada962887dc8bf945 Mon Sep 17 00:00:00 2001 From: Brandon Stepler Date: Wed, 6 Jan 2021 23:44:00 -0500 Subject: dhcpv6-pd: verify: T3193: allow more than one VLAN interface VLAN interfaces contain periods, which make them incompatible with dict_search(). --- python/vyos/configverify.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index a425ca671..bbaf5e861 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -187,14 +187,14 @@ def verify_dhcpv6(config): # assigned IPv6 subnet from a delegated prefix for pd in dict_search('dhcpv6_options.pd', config): sla_ids = [] + interfaces = dict_search(f'dhcpv6_options.pd.{pd}.interface', config) - if not dict_search(f'dhcpv6_options.pd.{pd}.interface', config): + if not interfaces: raise ConfigError('DHCPv6-PD requires an interface where to assign ' 'the delegated prefix!') - for interface in dict_search(f'dhcpv6_options.pd.{pd}.interface', config): - sla_id = dict_search( - f'dhcpv6_options.pd.{pd}.interface.{interface}.sla_id', config) + for interface in interfaces: + sla_id = interfaces[interface].get('sla_id', None) sla_ids.append(sla_id) # Check for duplicates -- cgit v1.2.3 From 0a1c218d010df4568d4c35ee9cf68c9affe24498 Mon Sep 17 00:00:00 2001 From: Brandon Stepler Date: Wed, 6 Jan 2021 23:44:00 -0500 Subject: dhcpv6-pd: verify: T3193: allow multiple auto-assigned SLA-IDs "data/templates/dhcp-client/ipv6.tmpl" handles the auto-assigning of SLA-IDs on lines 39, 46, and 52. --- python/vyos/configverify.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index bbaf5e861..48c858ba4 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -194,8 +194,8 @@ def verify_dhcpv6(config): 'the delegated prefix!') for interface in interfaces: - sla_id = interfaces[interface].get('sla_id', None) - sla_ids.append(sla_id) + if 'sla_id' in interfaces[interface]: + sla_ids.append(interfaces[interface]['sla_id']) # Check for duplicates duplicates = [x for n, x in enumerate(sla_ids) if x in sla_ids[:n]] -- cgit v1.2.3 From 98ec1ce377ea2d21323991af3eae8ab784eb9d02 Mon Sep 17 00:00:00 2001 From: Brandon Stepler Date: Wed, 6 Jan 2021 23:44:00 -0500 Subject: dhcpv6-pd: verify: T3193: detect conflict between auto-assigned and configured SLA-IDs "data/templates/dhcp-client/ipv6.tmpl" handles the auto-assigning of SLA-IDs on lines 39, 46, and 52. --- python/vyos/configverify.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index 48c858ba4..b4447306e 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -193,9 +193,11 @@ def verify_dhcpv6(config): raise ConfigError('DHCPv6-PD requires an interface where to assign ' 'the delegated prefix!') - for interface in interfaces: + for count, interface in enumerate(interfaces): if 'sla_id' in interfaces[interface]: sla_ids.append(interfaces[interface]['sla_id']) + else: + sla_ids.append(str(count)) # Check for duplicates duplicates = [x for n, x in enumerate(sla_ids) if x in sla_ids[:n]] -- cgit v1.2.3 From a8e4317c61b1253fa02044cffc7a588d45259a5e Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 17:01:09 +0100 Subject: smoketest: interfaces: test dhcpv6 pd sla-id auto increment --- smoketest/scripts/cli/base_interfaces_test.py | 125 ++++++++++++++++++++------ 1 file changed, 96 insertions(+), 29 deletions(-) diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 8ee5395d0..ad7886414 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -1,4 +1,4 @@ -# Copyright (C) 2019-2020 VyOS maintainers and contributors +# Copyright (C) 2019-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 @@ -12,7 +12,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import re import os import unittest import json @@ -51,17 +50,6 @@ def is_mirrored_to(interface, mirror_if, qdisc): ret_val = True return ret_val - -dhcp6c_config_file = '/run/dhcp6c/dhcp6c.{}.conf' -def get_dhcp6c_config_value(interface, key): - tmp = read_file(dhcp6c_config_file.format(interface)) - tmp = re.findall(r'\n?{}\s+(.*)'.format(key), tmp) - - out = [] - for item in tmp: - out.append(item.replace(';','')) - return out - class BasicInterfaceTest: class BaseTest(unittest.TestCase): _test_ip = False @@ -378,13 +366,73 @@ class BasicInterfaceTest: self.assertEqual(dad_transmits, tmp) - def test_ipv6_dhcpv6_prefix_delegation(self): + def test_ipv6_dhcpv6_pd_auto_inc_sla_id(self): if not self._test_ipv6: return None + address = '1' + prefix_len = '56' + sla_len = str(64 - int(prefix_len)) + + delegatees = ['dum2340', 'dum2341', 'dum2342', 'dum2343', 'dum2344'] + + for interface in self._interfaces: + path = self._base_path + [interface] + for option in self._options.get(interface, []): + self.session.set(path + option.split()) + + # prefix delegation stuff + pd_base = path + ['dhcpv6-options', 'pd', '0'] + self.session.set(pd_base + ['length', prefix_len]) + + for delegatee in delegatees: + section = Section.section(delegatee) + self.session.set(['interfaces', section, delegatee]) + self.session.set(pd_base + ['interface', delegatee, 'address', address]) + # increment interface address + address = str(int(address) + 1) + + self.session.commit() + address = '1' sla_id = '0' - sla_len = '8' + for interface in self._interfaces: + dhcpc6_config = read_file(f'/run/dhcp6c/dhcp6c.{interface}.conf') + + # verify DHCPv6 prefix delegation + self.assertIn(f'prefix ::/{prefix_len} infinity;', dhcpc6_config) + + for delegatee in delegatees: + self.assertIn(f'prefix-interface {delegatee}' + r' {', dhcpc6_config) + self.assertIn(f'ifid {address};', dhcpc6_config) + self.assertIn(f'sla-id {sla_id};', dhcpc6_config) + self.assertIn(f'sla-len {sla_len};', dhcpc6_config) + + # increment sla-id + sla_id = str(int(sla_id) + 1) + # increment interface address + address = str(int(address) + 1) + + # Check for running process + self.assertTrue(process_named_running('dhcp6c')) + + for delegatee in delegatees: + # we can already cleanup the test delegatee interface here + # as until commit() is called, nothing happens + section = Section.section(delegatee) + self.session.delete(['interfaces', section, delegatee]) + + def test_ipv6_dhcpv6_pd_manual_sla_id(self): + if not self._test_ipv6: + return None + + address = '1' + prefix_len = '56' + sla_len = str(64 - int(prefix_len)) + sla_id = '1' + + delegatees = ['dum3340', 'dum3341', 'dum3342', 'dum3343', 'dum3344'] + for interface in self._interfaces: path = self._base_path + [interface] for option in self._options.get(interface, []): @@ -392,25 +440,44 @@ class BasicInterfaceTest: # prefix delegation stuff pd_base = path + ['dhcpv6-options', 'pd', '0'] - self.session.set(pd_base + ['length', '56']) - self.session.set(pd_base + ['interface', interface, 'address', address]) - self.session.set(pd_base + ['interface', interface, 'sla-id', sla_id]) + self.session.set(pd_base + ['length', prefix_len]) + + for delegatee in delegatees: + section = Section.section(delegatee) + self.session.set(['interfaces', section, delegatee]) + self.session.set(pd_base + ['interface', delegatee, 'address', address]) + self.session.set(pd_base + ['interface', delegatee, 'sla-id', address]) + + # increment interface address + address = str(int(address) + 1) + sla_id = str(int(sla_id) + 1) self.session.commit() + address = '1' + sla_id = '1' for interface in self._interfaces: + dhcpc6_config = read_file(f'/run/dhcp6c/dhcp6c.{interface}.conf') + # verify DHCPv6 prefix delegation - # will return: ['delegation', '::/56 infinity;'] - tmp = get_dhcp6c_config_value(interface, 'prefix')[1].split()[0] # mind the whitespace - self.assertEqual(tmp, '::/56') - tmp = get_dhcp6c_config_value(interface, 'prefix-interface')[0].split()[0] - self.assertEqual(tmp, interface) - tmp = get_dhcp6c_config_value(interface, 'ifid')[0] - self.assertEqual(tmp, address) - tmp = get_dhcp6c_config_value(interface, 'sla-id')[0] - self.assertEqual(tmp, sla_id) - tmp = get_dhcp6c_config_value(interface, 'sla-len')[0] - self.assertEqual(tmp, sla_len) + self.assertIn(f'prefix ::/{prefix_len} infinity;', dhcpc6_config) + + for delegatee in delegatees: + self.assertIn(f'prefix-interface {delegatee}' + r' {', dhcpc6_config) + self.assertIn(f'ifid {address};', dhcpc6_config) + self.assertIn(f'sla-id {sla_id};', dhcpc6_config) + self.assertIn(f'sla-len {sla_len};', dhcpc6_config) + + # increment sla-id + sla_id = str(int(sla_id) + 1) + # increment interface address + address = str(int(address) + 1) # Check for running process self.assertTrue(process_named_running('dhcp6c')) + + for delegatee in delegatees: + # we can already cleanup the test delegatee interface here + # as until commit() is called, nothing happens + section = Section.section(delegatee) + self.session.delete(['interfaces', section, delegatee]) -- 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(-) 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 e426b103b803b61e6593e1662c470d0f23a9a36f Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 17:12:21 +0100 Subject: xml: radius: T3192: split individual nodes to discrete includes Provide more re-usable nodes for future implementations. --- .../include/generic-disable-node.xml.in | 8 ++++ .../include/radius-server-ipv4.xml.i | 27 ++++++++++++ .../include/radius-server-key.xml.in | 7 ++++ .../include/radius-server-port.xml.in | 15 +++++++ interface-definitions/include/radius-server.xml.i | 48 ---------------------- interface-definitions/interfaces-wireless.xml.in | 2 +- interface-definitions/service_ipoe-server.xml.in | 2 +- interface-definitions/service_pppoe-server.xml.in | 2 +- interface-definitions/system-login.xml.in | 2 +- interface-definitions/vpn_l2tp.xml.in | 2 +- interface-definitions/vpn_openconnect.xml.in | 2 +- interface-definitions/vpn_pptp.xml.in | 2 +- interface-definitions/vpn_sstp.xml.in | 2 +- 13 files changed, 65 insertions(+), 56 deletions(-) create mode 100644 interface-definitions/include/generic-disable-node.xml.in create mode 100644 interface-definitions/include/radius-server-ipv4.xml.i create mode 100644 interface-definitions/include/radius-server-key.xml.in create mode 100644 interface-definitions/include/radius-server-port.xml.in delete mode 100644 interface-definitions/include/radius-server.xml.i diff --git a/interface-definitions/include/generic-disable-node.xml.in b/interface-definitions/include/generic-disable-node.xml.in new file mode 100644 index 000000000..3e41070bc --- /dev/null +++ b/interface-definitions/include/generic-disable-node.xml.in @@ -0,0 +1,8 @@ + + + + Temporary disable + + + + diff --git a/interface-definitions/include/radius-server-ipv4.xml.i b/interface-definitions/include/radius-server-ipv4.xml.i new file mode 100644 index 000000000..7c5e4eb7d --- /dev/null +++ b/interface-definitions/include/radius-server-ipv4.xml.i @@ -0,0 +1,27 @@ + + + + RADIUS based user authentication + + + #include + + + RADIUS server configuration + + ipv4 + RADIUS server IPv4 address + + + + + + + #include + #include + #include + + + + + diff --git a/interface-definitions/include/radius-server-key.xml.in b/interface-definitions/include/radius-server-key.xml.in new file mode 100644 index 000000000..1f487d3d0 --- /dev/null +++ b/interface-definitions/include/radius-server-key.xml.in @@ -0,0 +1,7 @@ + + + + Shared secret key + + + diff --git a/interface-definitions/include/radius-server-port.xml.in b/interface-definitions/include/radius-server-port.xml.in new file mode 100644 index 000000000..71b6bddb7 --- /dev/null +++ b/interface-definitions/include/radius-server-port.xml.in @@ -0,0 +1,15 @@ + + + + Authentication port + + u32:1-65535 + Numeric IP port (default: 1812) + + + + + + 1812 + + diff --git a/interface-definitions/include/radius-server.xml.i b/interface-definitions/include/radius-server.xml.i deleted file mode 100644 index c1dadd2a2..000000000 --- a/interface-definitions/include/radius-server.xml.i +++ /dev/null @@ -1,48 +0,0 @@ - - - - RADIUS based user authentication - - - #include - - - RADIUS server configuration - - ipv4 - RADIUS server IPv4 address - - - - - - - - - Temporary disable this server - - - - - - Shared secret key - - - - - Authentication port - - u32:1-65535 - Numeric IP port (default: 1812) - - - - - - 1812 - - - - - - diff --git a/interface-definitions/interfaces-wireless.xml.in b/interface-definitions/interfaces-wireless.xml.in index 86f529278..f39e5618f 100644 --- a/interface-definitions/interfaces-wireless.xml.in +++ b/interface-definitions/interfaces-wireless.xml.in @@ -722,7 +722,7 @@ Invalid WPA pass phrase, must be 8 to 63 printable characters! - #include + #include diff --git a/interface-definitions/service_ipoe-server.xml.in b/interface-definitions/service_ipoe-server.xml.in index ee09d01d6..07241fcc2 100644 --- a/interface-definitions/service_ipoe-server.xml.in +++ b/interface-definitions/service_ipoe-server.xml.in @@ -197,7 +197,7 @@ - #include + #include #include diff --git a/interface-definitions/service_pppoe-server.xml.in b/interface-definitions/service_pppoe-server.xml.in index 6d11f41a0..5c0a66527 100644 --- a/interface-definitions/service_pppoe-server.xml.in +++ b/interface-definitions/service_pppoe-server.xml.in @@ -26,7 +26,7 @@ #include #include #include - #include + #include #include diff --git a/interface-definitions/system-login.xml.in b/interface-definitions/system-login.xml.in index 812a50c8a..0bea6a22d 100644 --- a/interface-definitions/system-login.xml.in +++ b/interface-definitions/system-login.xml.in @@ -110,7 +110,7 @@ - #include + #include diff --git a/interface-definitions/vpn_l2tp.xml.in b/interface-definitions/vpn_l2tp.xml.in index 42da75a64..998a8c371 100644 --- a/interface-definitions/vpn_l2tp.xml.in +++ b/interface-definitions/vpn_l2tp.xml.in @@ -212,7 +212,7 @@ #include #include #include - #include + #include diff --git a/interface-definitions/vpn_openconnect.xml.in b/interface-definitions/vpn_openconnect.xml.in index ccf537e04..386d06509 100644 --- a/interface-definitions/vpn_openconnect.xml.in +++ b/interface-definitions/vpn_openconnect.xml.in @@ -57,7 +57,7 @@ - #include + #include diff --git a/interface-definitions/vpn_pptp.xml.in b/interface-definitions/vpn_pptp.xml.in index b17138e33..fe1bde27e 100644 --- a/interface-definitions/vpn_pptp.xml.in +++ b/interface-definitions/vpn_pptp.xml.in @@ -123,7 +123,7 @@ - #include + #include #include diff --git a/interface-definitions/vpn_sstp.xml.in b/interface-definitions/vpn_sstp.xml.in index 134858608..ebcb77db2 100644 --- a/interface-definitions/vpn_sstp.xml.in +++ b/interface-definitions/vpn_sstp.xml.in @@ -16,7 +16,7 @@ #include #include #include - #include + #include #include -- cgit v1.2.3 From 582f52764afce78b9be0d95b88f6dc8d0bff9690 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 17:25:30 +0100 Subject: xml: include: provide generic include for disable node --- interface-definitions/bcast-relay.xml.in | 14 ++----------- interface-definitions/dhcp-server.xml.in | 21 +++---------------- interface-definitions/dhcpv6-server.xml.in | 21 +++---------------- interface-definitions/firewall-options.xml.in | 7 +------ interface-definitions/igmp-proxy.xml.in | 7 +------ .../include/accel-auth-local-users.xml.i | 7 +------ .../include/generic-disable-node.xml.i | 8 ++++++++ .../include/generic-disable-node.xml.in | 8 -------- .../include/interface-eapol.xml.i | 24 +++++++++++----------- interface-definitions/include/nat-rule.xml.i | 7 +------ .../include/radius-server-ipv4.xml.i | 2 +- .../include/radius-server-key.xml.in | 14 ++++++------- interface-definitions/interfaces-openvpn.xml.in | 21 +++---------------- interface-definitions/interfaces-wireguard.xml.in | 7 +------ interface-definitions/lldp.xml.in | 7 +------ interface-definitions/nat.xml.in | 7 +------ interface-definitions/service_mdns-repeater.xml.in | 7 +------ interface-definitions/service_webproxy.xml.in | 7 +------ interface-definitions/vpn_ipsec.xml.in | 7 +------ interface-definitions/vpn_openconnect.xml.in | 7 +------ interface-definitions/vpn_pptp.xml.in | 6 +----- interface-definitions/vrrp.xml.in | 7 +------ 22 files changed, 52 insertions(+), 171 deletions(-) create mode 100644 interface-definitions/include/generic-disable-node.xml.i delete mode 100644 interface-definitions/include/generic-disable-node.xml.in diff --git a/interface-definitions/bcast-relay.xml.in b/interface-definitions/bcast-relay.xml.in index b691f79fa..1b354d885 100644 --- a/interface-definitions/bcast-relay.xml.in +++ b/interface-definitions/bcast-relay.xml.in @@ -9,12 +9,7 @@ 990 - - - Globally disable broadcast relay service - - - + #include Unique ID for each UDP port to forward @@ -27,12 +22,7 @@ - - - Disable broadcast relay service instance - - - + #include Set source IP of forwarded packets, otherwise original senders address is used diff --git a/interface-definitions/dhcp-server.xml.in b/interface-definitions/dhcp-server.xml.in index 2c1609d94..912e4eaf7 100644 --- a/interface-definitions/dhcp-server.xml.in +++ b/interface-definitions/dhcp-server.xml.in @@ -9,12 +9,7 @@ 911 - - - Disable DHCP server - - - + #include Dynamically update Domain Name System (RFC4702) @@ -63,12 +58,7 @@ Shared-network-name description - - - Option to disable DHCP configuration for shared-network - - - + #include Additional shared-network parameters for DHCP server. @@ -330,12 +320,7 @@ Invalid static mapping name. May only contain letters, numbers and .-_ - - - Option to disable static mapping - - - + #include Fixed IP address of static mapping diff --git a/interface-definitions/dhcpv6-server.xml.in b/interface-definitions/dhcpv6-server.xml.in index 37bc7e03e..fb0e79c47 100644 --- a/interface-definitions/dhcpv6-server.xml.in +++ b/interface-definitions/dhcpv6-server.xml.in @@ -9,12 +9,7 @@ 900 - - - Option to disable DHCPv6 server - - - + #include Preference of this DHCPv6 server compared with others @@ -37,12 +32,7 @@ Invalid DHCPv6 shared network name. May only contain letters, numbers and .-_ - - - Option to disable DHCPv6 configuration for shared-network - - - + #include Common options to distribute to all clients, including stateless clients @@ -324,12 +314,7 @@ Invalid static mapping name. May only contain letters, numbers and .-_ - - - Option to disable static mapping - - - + #include Client identifier (DUID) for this static mapping diff --git a/interface-definitions/firewall-options.xml.in b/interface-definitions/firewall-options.xml.in index defd44f06..8d9225a9a 100644 --- a/interface-definitions/firewall-options.xml.in +++ b/interface-definitions/firewall-options.xml.in @@ -16,12 +16,7 @@ - - - Disable this rule - - - + #include Adjust MSS for IPv4 transit packets diff --git a/interface-definitions/igmp-proxy.xml.in b/interface-definitions/igmp-proxy.xml.in index b9c52794f..d0f44eada 100644 --- a/interface-definitions/igmp-proxy.xml.in +++ b/interface-definitions/igmp-proxy.xml.in @@ -9,12 +9,7 @@ 740 - - - Option to disable IGMP proxy - - - + #include Option to disable "quickleave" diff --git a/interface-definitions/include/accel-auth-local-users.xml.i b/interface-definitions/include/accel-auth-local-users.xml.i index 0d66b8135..35c7a2a06 100644 --- a/interface-definitions/include/accel-auth-local-users.xml.i +++ b/interface-definitions/include/accel-auth-local-users.xml.i @@ -9,12 +9,7 @@ User name for authentication - - - Option to disable a PPPoE Server user - - - + #include Password for authentication diff --git a/interface-definitions/include/generic-disable-node.xml.i b/interface-definitions/include/generic-disable-node.xml.i new file mode 100644 index 000000000..520383afb --- /dev/null +++ b/interface-definitions/include/generic-disable-node.xml.i @@ -0,0 +1,8 @@ + + + + Temporary disable + + + + diff --git a/interface-definitions/include/generic-disable-node.xml.in b/interface-definitions/include/generic-disable-node.xml.in deleted file mode 100644 index 3e41070bc..000000000 --- a/interface-definitions/include/generic-disable-node.xml.in +++ /dev/null @@ -1,8 +0,0 @@ - - - - Temporary disable - - - - diff --git a/interface-definitions/include/interface-eapol.xml.i b/interface-definitions/include/interface-eapol.xml.i index 94476f0f1..8b33b4acf 100644 --- a/interface-definitions/include/interface-eapol.xml.i +++ b/interface-definitions/include/interface-eapol.xml.i @@ -1,12 +1,12 @@ - - - - Extensible Authentication Protocol over Local Area Network - - - #include - #include - #include - - - + + + + Extensible Authentication Protocol over Local Area Network + + + #include + #include + #include + + + diff --git a/interface-definitions/include/nat-rule.xml.i b/interface-definitions/include/nat-rule.xml.i index e034ef4dd..7ef90f07e 100644 --- a/interface-definitions/include/nat-rule.xml.i +++ b/interface-definitions/include/nat-rule.xml.i @@ -26,12 +26,7 @@ #include - - - Disable NAT rule - - - + #include Exclude packets matching this rule from NAT diff --git a/interface-definitions/include/radius-server-ipv4.xml.i b/interface-definitions/include/radius-server-ipv4.xml.i index 7c5e4eb7d..9c73c4c49 100644 --- a/interface-definitions/include/radius-server-ipv4.xml.i +++ b/interface-definitions/include/radius-server-ipv4.xml.i @@ -17,7 +17,7 @@ - #include + #include #include #include diff --git a/interface-definitions/include/radius-server-key.xml.in b/interface-definitions/include/radius-server-key.xml.in index 1f487d3d0..32a01b402 100644 --- a/interface-definitions/include/radius-server-key.xml.in +++ b/interface-definitions/include/radius-server-key.xml.in @@ -1,7 +1,7 @@ - - - - Shared secret key - - - + + + + Shared secret key + + + diff --git a/interface-definitions/interfaces-openvpn.xml.in b/interface-definitions/interfaces-openvpn.xml.in index 34040bf72..527f7fd54 100644 --- a/interface-definitions/interfaces-openvpn.xml.in +++ b/interface-definitions/interfaces-openvpn.xml.in @@ -418,12 +418,7 @@ - - - Option to disable client connection - - - + #include IP address of the client @@ -482,12 +477,7 @@ Pool of client IPv4 addresses - - - Disable client IP pool - - - + #include First IP address in the pool @@ -546,12 +536,7 @@ - - - Disable client IPv6 pool - - - + #include diff --git a/interface-definitions/interfaces-wireguard.xml.in b/interface-definitions/interfaces-wireguard.xml.in index 92c9f510c..acf5082d6 100644 --- a/interface-definitions/interfaces-wireguard.xml.in +++ b/interface-definitions/interfaces-wireguard.xml.in @@ -55,12 +55,7 @@ peer alias too long (limit 100 characters) - - - disables peer - - - + #include base64 encoded public key diff --git a/interface-definitions/lldp.xml.in b/interface-definitions/lldp.xml.in index 950b267ef..9fdffcea1 100644 --- a/interface-definitions/lldp.xml.in +++ b/interface-definitions/lldp.xml.in @@ -25,12 +25,7 @@ - - - Disable lldp on this interface - - - + #include LLDP-MED location data [REQUIRED] diff --git a/interface-definitions/nat.xml.in b/interface-definitions/nat.xml.in index 00aaddb17..d6bed5b27 100644 --- a/interface-definitions/nat.xml.in +++ b/interface-definitions/nat.xml.in @@ -79,12 +79,7 @@ Rule description - - - Disable NAT rule - - - + #include #include diff --git a/interface-definitions/service_mdns-repeater.xml.in b/interface-definitions/service_mdns-repeater.xml.in index e21b1b27c..33ef9a434 100644 --- a/interface-definitions/service_mdns-repeater.xml.in +++ b/interface-definitions/service_mdns-repeater.xml.in @@ -13,12 +13,7 @@ 990 - - - Disable mDNS repeater service - - - + #include Interface to repeat mDNS advertisements [REQUIRED] diff --git a/interface-definitions/service_webproxy.xml.in b/interface-definitions/service_webproxy.xml.in index 4cd8138ec..7cb0f7ece 100644 --- a/interface-definitions/service_webproxy.xml.in +++ b/interface-definitions/service_webproxy.xml.in @@ -394,12 +394,7 @@ URL filtering settings - - - Disable URL filtering - - - + #include URL filtering via squidGuard redirector diff --git a/interface-definitions/vpn_ipsec.xml.in b/interface-definitions/vpn_ipsec.xml.in index daf98a833..426d7e71c 100644 --- a/interface-definitions/vpn_ipsec.xml.in +++ b/interface-definitions/vpn_ipsec.xml.in @@ -1045,12 +1045,7 @@ - - - Option to disable vpn tunnel - - - + #include ESP group name diff --git a/interface-definitions/vpn_openconnect.xml.in b/interface-definitions/vpn_openconnect.xml.in index 386d06509..054e027fc 100644 --- a/interface-definitions/vpn_openconnect.xml.in +++ b/interface-definitions/vpn_openconnect.xml.in @@ -42,12 +42,7 @@ User name for authentication - - - Option to disable a SSL VPN Server user - - - + #include Password for authentication diff --git a/interface-definitions/vpn_pptp.xml.in b/interface-definitions/vpn_pptp.xml.in index fe1bde27e..72eda8752 100644 --- a/interface-definitions/vpn_pptp.xml.in +++ b/interface-definitions/vpn_pptp.xml.in @@ -104,11 +104,7 @@ User name for authentication - - - Option to disable a PPTP Server user - - + #include Password for authentication diff --git a/interface-definitions/vrrp.xml.in b/interface-definitions/vrrp.xml.in index c6a32930f..caa9f4a33 100644 --- a/interface-definitions/vrrp.xml.in +++ b/interface-definitions/vrrp.xml.in @@ -73,12 +73,7 @@ Group description - - - - Disable VRRP group - - + #include Health check script -- 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 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(-) 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(-) 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 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 fde684497e9be82123cffefa3e4766689e4dabc6 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 21:41:12 +0100 Subject: smoketest: interfaces: fix dhcpv6 pd testcase when using multiple interfaces Commit a8e4317c ("smoketest: interfaces: test dhcpv6 pd sla-id auto increment") added a new test, but when executed on multiple interfaces, e.g.: TEST_ETH="eth1 eth2" /usr/libexec/vyos/tests/smoke/cli/test_interfaces_ethernet.py A variable was not properly reset --- smoketest/scripts/cli/base_interfaces_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index ad7886414..5efa39bd5 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -395,13 +395,13 @@ class BasicInterfaceTest: self.session.commit() address = '1' - sla_id = '0' for interface in self._interfaces: dhcpc6_config = read_file(f'/run/dhcp6c/dhcp6c.{interface}.conf') # verify DHCPv6 prefix delegation self.assertIn(f'prefix ::/{prefix_len} infinity;', dhcpc6_config) + sla_id = '0' for delegatee in delegatees: self.assertIn(f'prefix-interface {delegatee}' + r' {', dhcpc6_config) self.assertIn(f'ifid {address};', dhcpc6_config) @@ -446,7 +446,7 @@ class BasicInterfaceTest: section = Section.section(delegatee) self.session.set(['interfaces', section, delegatee]) self.session.set(pd_base + ['interface', delegatee, 'address', address]) - self.session.set(pd_base + ['interface', delegatee, 'sla-id', address]) + self.session.set(pd_base + ['interface', delegatee, 'sla-id', sla_id]) # increment interface address address = str(int(address) + 1) -- 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(-) 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 5cb935fc1d86c9a2ae61c7406425d0eed79dc87a Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:01:29 +0100 Subject: smoketest: ethernet: check for error on non existing interface When performing a commit on an ethernet interface that does not exist, e.g. eth667, verify an exception is raised. --- smoketest/scripts/cli/test_interfaces_ethernet.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 3c4796283..971d965f5 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -123,6 +123,11 @@ class EthernetInterfaceTest(BasicInterfaceTest.BaseTest): self.assertEqual(f'{cpus:x}', f'{rps_cpus:x}') + def test_non_existing_interface(self): + self.session.set(self._base_path + ['eth667']) + # check validate() - interface does not exist + with self.assertRaises(ConfigSessionError): + self.session.commit() def test_eapol_support(self): for interface in self._interfaces: -- cgit v1.2.3 From d7dbf3966887839168ea363c89e97204573297e9 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:40:37 +0100 Subject: Debian: add python3-psutil build dependency vyos.util depends partially on python3-psutil, and some smoketests executed via "make test" include vyos.util, thus ensure the package is available. --- debian/control | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/control b/debian/control index ccdaa8492..350492180 100644 --- a/debian/control +++ b/debian/control @@ -18,6 +18,7 @@ Build-Depends: python3-lxml, python3-netifaces, python3-nose, + python3-psutil, python3-setuptools, python3-xmltodict, quilt, -- cgit v1.2.3 From 27e1779dd431bb2144256fa4cab1466967ddf104 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:41:08 +0100 Subject: smoketest: ethernet: bugfixes for dhcpc6 and unknown interfaces --- smoketest/scripts/cli/base_interfaces_test.py | 13 +++++++------ smoketest/scripts/cli/test_interfaces_ethernet.py | 9 ++++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 5efa39bd5..5056eb190 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -366,7 +366,7 @@ class BasicInterfaceTest: self.assertEqual(dad_transmits, tmp) - def test_ipv6_dhcpv6_pd_auto_inc_sla_id(self): + def test_dhcpv6pd_auto_sla_id(self): if not self._test_ipv6: return None @@ -422,14 +422,12 @@ class BasicInterfaceTest: section = Section.section(delegatee) self.session.delete(['interfaces', section, delegatee]) - def test_ipv6_dhcpv6_pd_manual_sla_id(self): + def test_dhcpv6pd_manual_sla_id(self): if not self._test_ipv6: return None - address = '1' prefix_len = '56' sla_len = str(64 - int(prefix_len)) - sla_id = '1' delegatees = ['dum3340', 'dum3341', 'dum3342', 'dum3343', 'dum3344'] @@ -439,6 +437,8 @@ class BasicInterfaceTest: self.session.set(path + option.split()) # prefix delegation stuff + address = '1' + sla_id = '1' pd_base = path + ['dhcpv6-options', 'pd', '0'] self.session.set(pd_base + ['length', prefix_len]) @@ -454,9 +454,10 @@ class BasicInterfaceTest: self.session.commit() - address = '1' - sla_id = '1' + # Verify dhcpc6 client configuration for interface in self._interfaces: + address = '1' + sla_id = '1' dhcpc6_config = read_file(f'/run/dhcp6c/dhcp6c.{interface}.conf') # verify DHCPv6 prefix delegation diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 971d965f5..0f3579c30 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -19,6 +19,7 @@ import re import unittest from base_interfaces_test import BasicInterfaceTest +from vyos.configsession import ConfigSessionError from vyos.ifconfig import Section from vyos.util import cmd from vyos.util import process_named_running @@ -124,11 +125,17 @@ class EthernetInterfaceTest(BasicInterfaceTest.BaseTest): self.assertEqual(f'{cpus:x}', f'{rps_cpus:x}') def test_non_existing_interface(self): - self.session.set(self._base_path + ['eth667']) + unknonw_interface = self._base_path + ['eth667'] + self.session.set(unknonw_interface) + # check validate() - interface does not exist with self.assertRaises(ConfigSessionError): self.session.commit() + # we need to remove this wrong interface from the configuration + # manually, else tearDown() will have problem in commit() + self.session.delete(unknonw_interface) + def test_eapol_support(self): for interface in self._interfaces: # Enable EAPoL -- cgit v1.2.3 From 551ae04df7f6ce8a6cdbdf9712b4eea5d9e0c2f3 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:41:10 +0100 Subject: smoketest: interfaces: report skipped tests --- smoketest/scripts/cli/base_interfaces_test.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 5056eb190..19be5a3a6 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -94,7 +94,7 @@ class BasicInterfaceTest: def test_span_mirror(self): if not self._mirror_interfaces: - return None + self.skipTest('testcase not enabled') # Check the two-way mirror rules of ingress and egress for mirror in self._mirror_interfaces: @@ -163,7 +163,7 @@ class BasicInterfaceTest: def test_ipv6_link_local_address(self): # Common function for IPv6 link-local address assignemnts if not self._test_ipv6: - return None + self.skipTest('testcase not enabled') for interface in self._interfaces: base = self._base_path + [interface] @@ -190,7 +190,7 @@ class BasicInterfaceTest: def test_interface_mtu(self): if not self._test_mtu: - return None + self.skipTest('testcase not enabled') for intf in self._interfaces: base = self._base_path + [intf] @@ -210,7 +210,7 @@ class BasicInterfaceTest: # Testcase if MTU can be changed to 1200 on non IPv6 # enabled interfaces if not self._test_mtu: - return None + self.skipTest('testcase not enabled') old_mtu = self._mtu self._mtu = '1200' @@ -235,7 +235,7 @@ class BasicInterfaceTest: def test_8021q_vlan_interfaces(self): if not self._test_vlan: - return None + self.skipTest('testcase not enabled') for interface in self._interfaces: base = self._base_path + [interface] @@ -262,7 +262,7 @@ class BasicInterfaceTest: def test_8021ad_qinq_vlan_interfaces(self): if not self._test_qinq: - return None + self.skipTest('testcase not enabled') for interface in self._interfaces: base = self._base_path + [interface] @@ -293,7 +293,7 @@ class BasicInterfaceTest: def test_interface_ip_options(self): if not self._test_ip: - return None + self.skipTest('testcase not enabled') for interface in self._interfaces: arp_tmo = '300' @@ -344,7 +344,7 @@ class BasicInterfaceTest: def test_interface_ipv6_options(self): if not self._test_ipv6: - return None + self.skipTest('testcase not enabled') for interface in self._interfaces: dad_transmits = '10' @@ -368,7 +368,7 @@ class BasicInterfaceTest: def test_dhcpv6pd_auto_sla_id(self): if not self._test_ipv6: - return None + self.skipTest('testcase not enabled') address = '1' prefix_len = '56' @@ -424,7 +424,7 @@ class BasicInterfaceTest: def test_dhcpv6pd_manual_sla_id(self): if not self._test_ipv6: - return None + self.skipTest('testcase not enabled') prefix_len = '56' sla_len = str(64 - int(prefix_len)) -- 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(-) 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 From 9571b5e16938b902f0c9a1bac87aec180b220560 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Thu, 7 Jan 2021 23:41:46 +0100 Subject: Debian: add build-dependency on python3-jinja2 --- debian/control | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/control b/debian/control index 350492180..f7d9fccf6 100644 --- a/debian/control +++ b/debian/control @@ -18,6 +18,7 @@ Build-Depends: python3-lxml, python3-netifaces, python3-nose, + python3-jinja2, python3-psutil, python3-setuptools, python3-xmltodict, -- cgit v1.2.3 From 7d5c1b7572bdc8a4819b04098b1b002cb855a461 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 8 Jan 2021 16:27:33 +0100 Subject: smoketest: ethernet: fix link-speed loop test --- smoketest/scripts/cli/test_interfaces_ethernet.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 6b21853c3..6a0bdf150 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -144,9 +144,8 @@ class EthernetInterfaceTest(BasicInterfaceTest.BaseTest): # 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() + self.session.set(self._base_path + [interface, 'speed', 'auto']) + self.session.commit() def test_eapol_support(self): for interface in self._interfaces: -- cgit v1.2.3 From b72e2a13e11d28a7b0345c2b496db46221e049f5 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 8 Jan 2021 16:27:54 +0100 Subject: smoketest: interfaces: dhcpv6pd final fix Previous fix somehow got lost during a rebase :( --- smoketest/scripts/cli/base_interfaces_test.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/smoketest/scripts/cli/base_interfaces_test.py b/smoketest/scripts/cli/base_interfaces_test.py index 19be5a3a6..8b04eb337 100644 --- a/smoketest/scripts/cli/base_interfaces_test.py +++ b/smoketest/scripts/cli/base_interfaces_test.py @@ -94,7 +94,7 @@ class BasicInterfaceTest: def test_span_mirror(self): if not self._mirror_interfaces: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') # Check the two-way mirror rules of ingress and egress for mirror in self._mirror_interfaces: @@ -163,7 +163,7 @@ class BasicInterfaceTest: def test_ipv6_link_local_address(self): # Common function for IPv6 link-local address assignemnts if not self._test_ipv6: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') for interface in self._interfaces: base = self._base_path + [interface] @@ -190,7 +190,7 @@ class BasicInterfaceTest: def test_interface_mtu(self): if not self._test_mtu: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') for intf in self._interfaces: base = self._base_path + [intf] @@ -210,7 +210,7 @@ class BasicInterfaceTest: # Testcase if MTU can be changed to 1200 on non IPv6 # enabled interfaces if not self._test_mtu: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') old_mtu = self._mtu self._mtu = '1200' @@ -235,7 +235,7 @@ class BasicInterfaceTest: def test_8021q_vlan_interfaces(self): if not self._test_vlan: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') for interface in self._interfaces: base = self._base_path + [interface] @@ -262,7 +262,7 @@ class BasicInterfaceTest: def test_8021ad_qinq_vlan_interfaces(self): if not self._test_qinq: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') for interface in self._interfaces: base = self._base_path + [interface] @@ -293,7 +293,7 @@ class BasicInterfaceTest: def test_interface_ip_options(self): if not self._test_ip: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') for interface in self._interfaces: arp_tmo = '300' @@ -344,7 +344,7 @@ class BasicInterfaceTest: def test_interface_ipv6_options(self): if not self._test_ipv6: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') for interface in self._interfaces: dad_transmits = '10' @@ -368,9 +368,8 @@ class BasicInterfaceTest: def test_dhcpv6pd_auto_sla_id(self): if not self._test_ipv6: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') - address = '1' prefix_len = '56' sla_len = str(64 - int(prefix_len)) @@ -381,6 +380,7 @@ class BasicInterfaceTest: for option in self._options.get(interface, []): self.session.set(path + option.split()) + address = '1' # prefix delegation stuff pd_base = path + ['dhcpv6-options', 'pd', '0'] self.session.set(pd_base + ['length', prefix_len]) @@ -394,13 +394,13 @@ class BasicInterfaceTest: self.session.commit() - address = '1' for interface in self._interfaces: dhcpc6_config = read_file(f'/run/dhcp6c/dhcp6c.{interface}.conf') # verify DHCPv6 prefix delegation self.assertIn(f'prefix ::/{prefix_len} infinity;', dhcpc6_config) + address = '1' sla_id = '0' for delegatee in delegatees: self.assertIn(f'prefix-interface {delegatee}' + r' {', dhcpc6_config) @@ -424,7 +424,7 @@ class BasicInterfaceTest: def test_dhcpv6pd_manual_sla_id(self): if not self._test_ipv6: - self.skipTest('testcase not enabled') + self.skipTest('not enabled') prefix_len = '56' sla_len = str(64 - int(prefix_len)) -- cgit v1.2.3 From b01fd3e053f20876f19dff6f0d2e58ed0e7394e3 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 8 Jan 2021 16:28:56 +0100 Subject: smoketest: bridge: bond: enable ip subsystem tests --- smoketest/scripts/cli/test_interfaces_bonding.py | 1 + smoketest/scripts/cli/test_interfaces_bridge.py | 1 + 2 files changed, 2 insertions(+) diff --git a/smoketest/scripts/cli/test_interfaces_bonding.py b/smoketest/scripts/cli/test_interfaces_bonding.py index a35682b7c..d73ff09e9 100755 --- a/smoketest/scripts/cli/test_interfaces_bonding.py +++ b/smoketest/scripts/cli/test_interfaces_bonding.py @@ -26,6 +26,7 @@ from vyos.util import read_file class BondingInterfaceTest(BasicInterfaceTest.BaseTest): def setUp(self): + self._test_ip = True self._test_mtu = True self._test_vlan = True self._test_qinq = True diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py index 7444701c1..d47d236d0 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -28,6 +28,7 @@ from vyos.util import read_file class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): def setUp(self): + self._test_ip = True self._test_ipv6 = True self._test_vlan = True self._test_qinq = True -- cgit v1.2.3 From 23f55c4bcbe5475ed98d57cf54b645ef0c2cc1a8 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 8 Jan 2021 18:15:06 +0100 Subject: smoketest: dummy: fix indent --- smoketest/scripts/cli/test_interfaces_dummy.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/smoketest/scripts/cli/test_interfaces_dummy.py b/smoketest/scripts/cli/test_interfaces_dummy.py index c482a6f0b..60465a1d5 100755 --- a/smoketest/scripts/cli/test_interfaces_dummy.py +++ b/smoketest/scripts/cli/test_interfaces_dummy.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 @@ -20,9 +20,9 @@ from base_interfaces_test import BasicInterfaceTest class DummyInterfaceTest(BasicInterfaceTest.BaseTest): def setUp(self): - self._base_path = ['interfaces', 'dummy'] - self._interfaces = ['dum0', 'dum1', 'dum2'] - super().setUp() + self._base_path = ['interfaces', 'dummy'] + self._interfaces = ['dum435', 'dum8677', 'dum0931', 'dum089'] + super().setUp() if __name__ == '__main__': unittest.main(verbosity=2) -- cgit v1.2.3