From 1ce8e672b57e03e144998cb0a56ad1622c667a1d Mon Sep 17 00:00:00 2001 From: Nicolas Fort Date: Wed, 18 Jan 2023 15:09:32 +0000 Subject: T4857: Cleaning pr --- data/templates/snmp/etc.snmpd.conf.j2 | 31 ++++++++++++---- data/templates/snmp/override.conf.j2 | 3 +- interface-definitions/snmp.xml.in | 25 +++++++++---- smoketest/scripts/cli/test_service_snmp.py | 22 ++++++++++++ src/conf_mode/snmp.py | 4 +++ src/migration-scripts/snmp/2-to-3 | 57 ++++++++++++++++++++++++++++++ 6 files changed, 128 insertions(+), 14 deletions(-) create mode 100755 src/migration-scripts/snmp/2-to-3 diff --git a/data/templates/snmp/etc.snmpd.conf.j2 b/data/templates/snmp/etc.snmpd.conf.j2 index 47bf6878f..155ee2822 100644 --- a/data/templates/snmp/etc.snmpd.conf.j2 +++ b/data/templates/snmp/etc.snmpd.conf.j2 @@ -59,28 +59,47 @@ agentaddress unix:/run/snmpd.socket{{ ',' ~ options | join(',') if options is vy {% if comm_config.client is vyos_defined %} {% for client in comm_config.client %} {% if client | is_ipv4 %} -{{ comm_config.authorization }}community {{ comm }} {{ client }} +{{ comm_config.authorization }}community {{ comm }} {{ client }} -V RESTRICTED {% elif client | is_ipv6 %} -{{ comm_config.authorization }}community6 {{ comm }} {{ client }} +{{ comm_config.authorization }}community6 {{ comm }} {{ client }} -V RESTRICTED {% endif %} {% endfor %} {% endif %} {% if comm_config.network is vyos_defined %} {% for network in comm_config.network %} {% if network | is_ipv4 %} -{{ comm_config.authorization }}community {{ comm }} {{ network }} +{{ comm_config.authorization }}community {{ comm }} {{ network }} -V RESTRICTED {% elif network | is_ipv6 %} -{{ comm_config.authorization }}community6 {{ comm }} {{ network }} +{{ comm_config.authorization }}community6 {{ comm }} {{ network }} -V RESTRICTED {% endif %} {% endfor %} {% endif %} {% if comm_config.client is not vyos_defined and comm_config.network is not vyos_defined %} -{{ comm_config.authorization }}community {{ comm }} -{{ comm_config.authorization }}community6 {{ comm }} +{{ comm_config.authorization }}community {{ comm }} -V RESTRICTED +{{ comm_config.authorization }}community6 {{ comm }} -V RESTRICTED {% endif %} {% endfor %} {% endif %} +# Default RESTRICTED view +view RESTRICTED included .1 80 +{% if 'ip-route-table' not in oid_enable %} +# ipRouteTable oid: excluded +view RESTRICTED excluded .1.3.6.1.2.1.4.21 +{% endif %} +{% if 'ip-net-to-media-table' not in oid_enable %} +# ipNetToMediaTable oid: excluded +view RESTRICTED excluded .1.3.6.1.2.1.4.22 +{% endif %} +{% if 'ip-net-to-physical-phys-address' not in oid_enable %} +# ipNetToPhysicalPhysAddress oid: excluded +view RESTRICTED excluded .1.3.6.1.2.1.4.35 +{% endif %} +{% if 'ip-forward' not in oid_enable %} +# ipForward oid: excluded +view RESTRICTED excluded .1.3.6.1.2.1.4.24 +{% endif %} + {% if contact is vyos_defined %} # system contact information SysContact {{ contact }} diff --git a/data/templates/snmp/override.conf.j2 b/data/templates/snmp/override.conf.j2 index 5d787de86..443ee64db 100644 --- a/data/templates/snmp/override.conf.j2 +++ b/data/templates/snmp/override.conf.j2 @@ -1,5 +1,4 @@ {% set vrf_command = 'ip vrf exec ' ~ vrf ~ ' ' if vrf is vyos_defined else '' %} -{% set oid_route_table = ' ' if oid_enable is vyos_defined('route-table') else '-I -ipCidrRouteTable,inetCidrRouteTable' %} [Unit] StartLimitIntervalSec=0 After=vyos-router.service @@ -8,7 +7,7 @@ After=vyos-router.service Environment= Environment="MIBDIRS=/usr/share/snmp/mibs:/usr/share/snmp/mibs/iana:/usr/share/snmp/mibs/ietf:/usr/share/vyos/mibs" ExecStart= -ExecStart={{ vrf_command }}/usr/sbin/snmpd -LS0-5d -Lf /dev/null -u Debian-snmp -g Debian-snmp {{ oid_route_table }} -f -p /run/snmpd.pid +ExecStart={{ vrf_command }}/usr/sbin/snmpd -LS0-5d -Lf /dev/null -u Debian-snmp -g Debian-snmp -f -p /run/snmpd.pid Restart=always RestartSec=10 diff --git a/interface-definitions/snmp.xml.in b/interface-definitions/snmp.xml.in index 7ec60b2e7..bb6e91167 100644 --- a/interface-definitions/snmp.xml.in +++ b/interface-definitions/snmp.xml.in @@ -123,18 +123,31 @@ - Enable specific OIDs + Enable specific OIDs that by default are disable - route-table + ip-forward ip-route-table ip-net-to-media-table ip-net-to-physical-phys-address - route-table - Enable routing table OIDs (ipCidrRouteTable inetCidrRouteTable) + ip-forward + Enable oid .1.3.6.1.2.1.4.24 + + + ip-route-table + Enable oid .1.3.6.1.2.1.4.21 + + + ip-net-to-media-table + Enable oid .1.3.6.1.2.1.4.22 + + + ip-net-to-physical-phys-address + Enable oid .1.3.6.1.2.1.4.35 - (route-table) + (ip-forward|ip-route-table|ip-net-to-media-table|ip-net-to-physical-phys-address) - OID must be 'route-table' + OID must be one of the liste options + #include diff --git a/smoketest/scripts/cli/test_service_snmp.py b/smoketest/scripts/cli/test_service_snmp.py index e80c689cc..b18b9e7a1 100755 --- a/smoketest/scripts/cli/test_service_snmp.py +++ b/smoketest/scripts/cli/test_service_snmp.py @@ -123,6 +123,28 @@ class TestSNMPService(VyOSUnitTestSHIM.TestCase): self.assertTrue(process_named_running(PROCESS_NAME)) self.cli_delete(['interfaces', 'dummy', dummy_if]) + ## Check communities and default view RESTRICTED + for auth in ['ro', 'rw']: + community = 'VyOS' + auth + for addr in clients: + if is_ipv4(addr): + entry = auth + 'community ' + community + ' ' + addr + ' -V' + else: + entry = auth + 'community6 ' + community + ' ' + addr + ' -V' + config = get_config_value(entry) + expected = 'RESTRICTED' + self.assertIn(expected, config) + for addr in networks: + if is_ipv4(addr): + entry = auth + 'community ' + community + ' ' + addr + ' -V' + else: + entry = auth + 'community6 ' + community + ' ' + addr + ' -V' + config = get_config_value(entry) + expected = 'RESTRICTED' + self.assertIn(expected, config) + # And finally check global entry for RESTRICTED view + config = get_config_value('view RESTRICTED included .1') + self.assertIn('80', config) def test_snmpv3_sha(self): # Check if SNMPv3 can be configured with SHA authentication diff --git a/src/conf_mode/snmp.py b/src/conf_mode/snmp.py index 5cd24db32..addfb9e26 100755 --- a/src/conf_mode/snmp.py +++ b/src/conf_mode/snmp.py @@ -166,6 +166,10 @@ def verify(snmp): if 'community' not in trap_config: raise ConfigError(f'Trap target "{trap}" requires a community to be set!') + if 'oid_enable' in snmp: + Warning(f'Custom oids are enabled and may lead to system instability and high resource consumption') + + verify_vrf(snmp) # bail out early if SNMP v3 is not configured diff --git a/src/migration-scripts/snmp/2-to-3 b/src/migration-scripts/snmp/2-to-3 new file mode 100755 index 000000000..5f8d9c88d --- /dev/null +++ b/src/migration-scripts/snmp/2-to-3 @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2022 VyOS maintainers and contributors +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# T4857: Implement FRR SNMP recomendations +# cli changes from: +# set service snmp oid-enable route-table +# To +# set service snmp oid-enable ip-forward + +import re + +from sys import argv +from sys import exit + +from vyos.configtree import ConfigTree +from vyos.ifconfig import Section + +if (len(argv) < 1): + print("Must specify file name!") + exit(1) + +file_name = argv[1] + +with open(file_name, 'r') as f: + config_file = f.read() + +base = ['service snmp'] +config = ConfigTree(config_file) + +if not config.exists(base): + # Nothing to do + exit(0) + +if config.exists(base + ['oid-enable']): + config.delete(base + ['oid-enable']) + config.set(base + ['oid-enable'], 'ip-forward') + + +try: + with open(file_name, 'w') as f: + f.write(config.to_string()) +except OSError as e: + print("Failed to save the modified config: {}".format(e)) + exit(1) -- cgit v1.2.3 From 67fa5f55ac79838768b4b0a5f6d2c4b3b4c8e762 Mon Sep 17 00:00:00 2001 From: Nicolas Fort Date: Wed, 18 Jan 2023 16:17:17 +0000 Subject: T4857: change description in cli, and change word oid to uppercase OIDs in warning message --- interface-definitions/include/version/snmp-version.xml.i | 2 +- interface-definitions/snmp.xml.in | 8 ++++---- src/conf_mode/snmp.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/interface-definitions/include/version/snmp-version.xml.i b/interface-definitions/include/version/snmp-version.xml.i index 0416288f0..fa58672a5 100644 --- a/interface-definitions/include/version/snmp-version.xml.i +++ b/interface-definitions/include/version/snmp-version.xml.i @@ -1,3 +1,3 @@ - + diff --git a/interface-definitions/snmp.xml.in b/interface-definitions/snmp.xml.in index bb6e91167..10dd828a5 100644 --- a/interface-definitions/snmp.xml.in +++ b/interface-definitions/snmp.xml.in @@ -129,19 +129,19 @@ ip-forward - Enable oid .1.3.6.1.2.1.4.24 + Enable ipForward: .1.3.6.1.2.1.4.24 ip-route-table - Enable oid .1.3.6.1.2.1.4.21 + Enable ipRouteTable: .1.3.6.1.2.1.4.21 ip-net-to-media-table - Enable oid .1.3.6.1.2.1.4.22 + Enable ipNetToMediaTable: .1.3.6.1.2.1.4.22 ip-net-to-physical-phys-address - Enable oid .1.3.6.1.2.1.4.35 + Enable ipNetToPhysicalPhysAddress: .1.3.6.1.2.1.4.35 (ip-forward|ip-route-table|ip-net-to-media-table|ip-net-to-physical-phys-address) diff --git a/src/conf_mode/snmp.py b/src/conf_mode/snmp.py index addfb9e26..9651b358e 100755 --- a/src/conf_mode/snmp.py +++ b/src/conf_mode/snmp.py @@ -167,7 +167,7 @@ def verify(snmp): raise ConfigError(f'Trap target "{trap}" requires a community to be set!') if 'oid_enable' in snmp: - Warning(f'Custom oids are enabled and may lead to system instability and high resource consumption') + Warning(f'Custom OIDs are enabled and may lead to system instability and high resource consumption') verify_vrf(snmp) -- cgit v1.2.3