diff options
Diffstat (limited to 'python')
49 files changed, 411 insertions, 441 deletions
diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py index 1a3165a8f..ff784251a 100644 --- a/python/vyos/configdict.py +++ b/python/vyos/configdict.py @@ -21,7 +21,7 @@ import json from vyos.config import Config from vyos.utils.dict import dict_search -from vyos.utils.process import cmd +from vyos.utils.process import cmdl def retrieve_config(path_hash, base_path, config): """ @@ -660,7 +660,7 @@ def get_vlan_ids(interface): """ vlan_ids = set() - bridge_status = cmd('bridge -j vlan show', shell=True) + bridge_status = cmdl(['bridge', '-j', 'vlan', 'show']) vlan_filter_status = json.loads(bridge_status) if vlan_filter_status is not None: @@ -677,7 +677,7 @@ def get_vlan_ids(interface): def get_vlans_ids_and_range(interface): vlan_ids = set() - vlan_filter_status = json.loads(cmd(f'bridge -j -d vlan show dev {interface}')) + vlan_filter_status = json.loads(cmdl(['bridge', '-j', '-d', 'vlan', 'show', 'dev', interface])) if vlan_filter_status is not None: for interface_status in vlan_filter_status: diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py index bb17c152c..c1c038a7a 100644 --- a/python/vyos/configverify.py +++ b/python/vyos/configverify.py @@ -429,7 +429,7 @@ def verify_diffie_hellman_length(file, min_keysize): then or equal to min_keysize """ import os import re - from vyos.utils.process import cmd + from vyos.utils.process import cmdl try: keysize = str(min_keysize) @@ -437,7 +437,7 @@ def verify_diffie_hellman_length(file, min_keysize): return False if os.path.exists(file): - out = cmd(f'openssl dhparam -inform PEM -in {file} -text') + out = cmdl(['openssl', 'dhparam', '-inform', 'PEM', '-in', file, '-text']) prog = re.compile('\d+\s+bit') if prog.search(out): bits = prog.search(out)[0].split()[0] diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py index e33ee5b12..dc1502b7c 100755 --- a/python/vyos/firewall.py +++ b/python/vyos/firewall.py @@ -21,7 +21,7 @@ from socket import getaddrinfo from vyos.template import is_ipv4 from vyos.utils.dict import dict_search_args from vyos.utils.dict import dict_search_recursive -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.utils.network import get_vrf_tableid from vyos.defaults import rt_global_table from vyos.defaults import rt_global_vrf @@ -84,7 +84,7 @@ def fqdn_resolve(fqdn, ipv6=False): def find_nftables_rule(table, chain, rule_matches=[]): # Find rule in table/chain that matches all criteria and return the handle - results = cmd(f'sudo nft --handle list chain {table} {chain}').split("\n") + results = cmdl(['nft', '--handle', 'list', 'chain', table, chain], sudo=True).split("\n") for line in results: if all(rule_match in line for rule_match in rule_matches): handle_search = re.search('handle (\d+)', line) @@ -93,7 +93,7 @@ def find_nftables_rule(table, chain, rule_matches=[]): return None def remove_nftables_rule(table, chain, handle): - cmd(f'sudo nft delete rule {table} {chain} handle {handle}') + cmdl(['nft', 'delete', 'rule', table, chain, 'handle', str(handle)], sudo=True) # Functions below used by template generation diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py index a905c9a00..266da3d3d 100644 --- a/python/vyos/frrender.py +++ b/python/vyos/frrender.py @@ -36,7 +36,7 @@ from vyos.utils.dict import dict_search from vyos.utils.dict import dict_set_nested from vyos.utils.file import read_file from vyos.utils.file import write_file -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.utils.process import rc_cmd from vyos.template import get_dhcp_router from vyos.template import render_to_string @@ -880,4 +880,4 @@ class FRRender: raise ConfigError(emsg) # T3217: Save FRR configuration to /run/frr/config/frr.conf - return cmd('/usr/bin/vtysh -n --writeconfig') + return cmdl(['/usr/bin/vtysh', '-n', '--writeconfig']) diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py index d54eb2470..f04d52546 100644 --- a/python/vyos/ifconfig/bridge.py +++ b/python/vyos/ifconfig/bridge.py @@ -378,8 +378,7 @@ class BridgeIf(Interface): mac = entry.get('mac') entry_vlan = entry.get('vlan') if entry_vlan and mac and str(entry_vlan) == vlan: - cmd = f'bridge fdb del {mac} dev {self.ifname} vlan {vlan}' - self._cmd(cmd) + self._cmdl(['bridge', 'fdb', 'del', mac, 'dev', self.ifname, 'vlan', str(vlan)]) for vlan in config.get('vif', {}): self._cmdl(['bridge', 'vlan', 'add', 'dev', self.ifname, 'vid', str(vlan), 'self']) diff --git a/python/vyos/ifconfig/control.py b/python/vyos/ifconfig/control.py index 1f817b370..bc74ffc1d 100644 --- a/python/vyos/ifconfig/control.py +++ b/python/vyos/ifconfig/control.py @@ -14,13 +14,14 @@ # License along with this library. If not, see <http://www.gnu.org/licenses/>. import os +import shlex from inspect import signature from inspect import _empty from vyos.ifconfig.section import Section from vyos.utils.process import popen -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.utils.file import read_file from vyos.utils.file import write_file from vyos import debug @@ -50,23 +51,7 @@ class Control(Section): def _popen(self, command): return popen(command, self.debug) - def _cmd(self, command, env=None): - import re - if 'netns' in self.config: - # This command must be executed from default netns 'ip link set dev X netns X' - # exclude set netns cmd from netns to avoid: - # failed to run command: ip netns exec ns01 ip link set dev veth20 netns ns01 - pattern = r'ip link set dev (\S+) netns (\S+)' - matches = re.search(pattern, command) - if matches and matches.group(2) == self.config['netns']: - # Command already includes netns and matches desired namespace: - command = command - else: - command = f'ip netns exec {self.config["netns"]} {command}' - return cmd(command, self.debug, env=env) - def _cmdl(self, command, env=None): - from vyos.utils.process import cmdl if not isinstance(command, list): raise TypeError(f'_cmdl() requires a list, got {type(command).__name__}') netns = self.config.get('netns') @@ -84,7 +69,7 @@ class Control(Section): Using the defined names, set data write to sysfs. """ cmd = self._command_get[name]['shellcmd'].format(**config) - return self._command_get[name].get('format', lambda _: _)(self._cmd(cmd)) + return self._command_get[name].get('format', lambda _: _)(self._cmdl(shlex.split(cmd))) def _values(self, name, validate, value): """ @@ -135,7 +120,7 @@ class Control(Section): config = {**config, **{'value': value}} cmd = self._command_set[name]['shellcmd'].format(**config) - return self._command_set[name].get('format', lambda _: _)(self._cmd(cmd)) + return self._command_set[name].get('format', lambda _: _)(self._cmdl(shlex.split(cmd))) _sysfs_get = {} _sysfs_set = {} diff --git a/python/vyos/ifconfig/ethernet.py b/python/vyos/ifconfig/ethernet.py index 3c4c86f4b..b7b7b12f7 100644 --- a/python/vyos/ifconfig/ethernet.py +++ b/python/vyos/ifconfig/ethernet.py @@ -236,13 +236,13 @@ class EthernetIf(Interface): # bail out early as nothing is to change return - cmd = f'ethtool --change {ifname}' + cmd = ['ethtool', '--change', ifname] try: if speed == 'auto' or duplex == 'auto': - cmd += ' autoneg on' + cmd += ['autoneg', 'on'] else: - cmd += f' speed {speed} duplex {duplex} autoneg off' - return self._cmd(cmd) + cmd += ['speed', str(speed), 'duplex', str(duplex), 'autoneg', 'off'] + return self._cmdl(cmd) except PermissionError: # Some NICs do not tell that they don't support settings speed/duplex, # but they do not actually support it either. @@ -547,7 +547,7 @@ class EthernetIf(Interface): if code != 0: print(f'{ifname} does not support switchdev mode') elif not enable and enabled: - self._cmd(f'/sbin/devlink dev eswitch set pci/{addr} mode legacy') + self._cmdl(['/sbin/devlink', 'dev', 'eswitch', 'set', f'pci/{addr}', 'mode', 'legacy']) def update(self, config): """General helper function which works on a dictionary retrieved by diff --git a/python/vyos/ifconfig/geneve.py b/python/vyos/ifconfig/geneve.py index 7c5b7c0fb..16f760a99 100644 --- a/python/vyos/ifconfig/geneve.py +++ b/python/vyos/ifconfig/geneve.py @@ -48,17 +48,19 @@ class GeneveIf(Interface): 'parameters.ipv6.flowlabel' : 'flowlabel', } - cmd = 'ip link add name {ifname} type geneve id {vni} remote {remote} dstport {port}' + cmd = ['ip', 'link', 'add', 'name', self.ifname, 'type', 'geneve', + 'id', str(self.config['vni']), 'remote', self.config['remote'], + 'dstport', str(self.config['port'])] for vyos_key, iproute2_key in mapping.items(): # dict_search will return an empty dict "{}" for valueless nodes like # "parameters.nolearning" - thus we need to test the nodes existence # by using isinstance() tmp = dict_search(vyos_key, self.config) if isinstance(tmp, dict): - cmd += f' {iproute2_key}' + cmd += [iproute2_key] elif tmp != None: - cmd += f' {iproute2_key} {tmp}' + cmd += [iproute2_key, str(tmp)] - self._cmd(cmd.format(**self.config)) + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 4846ddfa7..e62812d31 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -50,6 +50,7 @@ from vyos.utils.network import is_netns_interface from vyos.utils.process import is_systemd_service_active from vyos.utils.process import stop_systemd_unit from vyos.utils.process import run +from vyos.utils.process import cmdl from vyos.utils.file import read_file from vyos.utils.file import write_file from vyos.utils.network import is_intf_addr_assigned @@ -421,25 +422,28 @@ class Interface(Control): # NOTE (Improvement): # after interface removal no other commands should be allowed # to be called and instead should raise an Exception: - cmd = 'ip link del dev {ifname}'.format(**self.config) + cmd = ['ip', 'link', 'del', 'dev', self.ifname] # for delete we can't get data from self.config{'netns'} netns = get_interface_namespace(self.ifname) - if netns: cmd = f'ip netns exec {netns} {cmd}' - return self._cmd(cmd) + if netns: cmd = ['ip', 'netns', 'exec', netns] + cmd + return self._cmdl(cmd) def _nft_check_and_run(self, nft_command): + # nft_command: list[str] # Check if deleting is possible first to avoid raising errors - _, err = self._popen(f'nft --check {nft_command} 2>/dev/null') + _, err = self._popen(f'nft --check {" ".join(nft_command)} 2>/dev/null') if not err: # Remove map element - self._cmd(f'nft {nft_command}') + self._cmdl(['nft'] + nft_command) def _del_interface_from_ct_iface_map(self): - nft_command = f'delete element inet vrf_zones ct_iface_map {{ \'"{self.ifname}"\' }}' + nft_command = ['delete', 'element', 'inet', 'vrf_zones', 'ct_iface_map', + '{', f'"{self.ifname}"', '}'] self._nft_check_and_run(nft_command) def _add_interface_to_ct_iface_map(self, vrf_table_id: int): - nft_command = f'add element inet vrf_zones ct_iface_map {{ \'"{self.ifname}"\' : {vrf_table_id} }}' + nft_command = ['add', 'element', 'inet', 'vrf_zones', 'ct_iface_map', + '{', f'"{self.ifname}"', ':', str(vrf_table_id), '}'] self._nft_check_and_run(nft_command) def get_ifindex(self): @@ -529,7 +533,11 @@ class Interface(Control): from hashlib import sha256 # Get processor ID number - cpu_id = self._cmd('sudo dmidecode -t 4 | grep ID | head -n1 | sed "s/.*ID://;s/ //g"') + cpu_id = '' + for line in self._cmdl(['sudo', 'dmidecode', '-t', '4']).splitlines(): + if 'ID' in line: + cpu_id = re.sub(r'.*ID:', '', line).replace(' ', '') + break # XXX: T3894 - it seems not all systems have eth0 - get a list of all # available Ethernet interfaces on the system (without VLAN subinterfaces) @@ -589,7 +597,7 @@ class Interface(Control): # Check if interface exists in network namespace if is_netns_interface(self.ifname, netns): - self._cmd(f'ip netns exec {netns} ip link del dev {self.ifname}') + cmdl(['ip', 'link', 'del', 'dev', self.ifname], self.debug, netns=netns) return True return False @@ -697,13 +705,13 @@ class Interface(Control): return self.set_interface('ipv6_cache_tmo', tmo) def _cleanup_mss_rules(self, table, ifname): - commands = [] - results = self._cmd(f'nft -a list chain {table} VYOS_TCP_MSS').split("\n") + # table: list[str], e.g. ['raw'] or ['ip6', 'raw'] + results = self._cmdl(['nft', '-a', 'list', 'chain'] + table + ['VYOS_TCP_MSS']).split("\n") for line in results: if f'oifname "{ifname}"' in line: handle_search = re.search('handle (\d+)', line) if handle_search: - self._cmd(f'nft delete rule {table} VYOS_TCP_MSS handle {handle_search[1]}') + self._cmdl(['nft', 'delete', 'rule'] + table + ['VYOS_TCP_MSS', 'handle', handle_search[1]]) def set_tcp_ipv4_mss(self, mss): """ @@ -720,14 +728,15 @@ class Interface(Control): if 'netns' in self.config: return None - self._cleanup_mss_rules('raw', self.ifname) - nft_prefix = 'nft add rule raw VYOS_TCP_MSS' - base_cmd = f'oifname "{self.ifname}" tcp flags & (syn|rst) == syn' + self._cleanup_mss_rules(['raw'], self.ifname) + nft_prefix = ['nft', 'add', 'rule', 'raw', 'VYOS_TCP_MSS'] + base_cmd = ['oifname', f'"{self.ifname}"', 'tcp', 'flags', '&', '(syn|rst)', '==', 'syn'] if mss == 'clamp-mss-to-pmtu': - self._cmd(f"{nft_prefix} '{base_cmd} tcp option maxseg size set rt mtu'") + self._cmdl(nft_prefix + base_cmd + ['tcp', 'option', 'maxseg', 'size', 'set', 'rt', 'mtu']) elif int(mss) > 0: low_mss = str(int(mss) + 1) - self._cmd(f"{nft_prefix} '{base_cmd} tcp option maxseg size {low_mss}-65535 tcp option maxseg size set {mss}'") + self._cmdl(nft_prefix + base_cmd + ['tcp', 'option', 'maxseg', 'size', + f'{low_mss}-65535', 'tcp', 'option', 'maxseg', 'size', 'set', str(mss)]) def set_tcp_ipv6_mss(self, mss): """ @@ -744,14 +753,15 @@ class Interface(Control): if 'netns' in self.config: return None - self._cleanup_mss_rules('ip6 raw', self.ifname) - nft_prefix = 'nft add rule ip6 raw VYOS_TCP_MSS' - base_cmd = f'oifname "{self.ifname}" tcp flags & (syn|rst) == syn' + self._cleanup_mss_rules(['ip6', 'raw'], self.ifname) + nft_prefix = ['nft', 'add', 'rule', 'ip6', 'raw', 'VYOS_TCP_MSS'] + base_cmd = ['oifname', f'"{self.ifname}"', 'tcp', 'flags', '&', '(syn|rst)', '==', 'syn'] if mss == 'clamp-mss-to-pmtu': - self._cmd(f"{nft_prefix} '{base_cmd} tcp option maxseg size set rt mtu'") + self._cmdl(nft_prefix + base_cmd + ['tcp', 'option', 'maxseg', 'size', 'set', 'rt', 'mtu']) elif int(mss) > 0: low_mss = str(int(mss) + 1) - self._cmd(f"{nft_prefix} '{base_cmd} tcp option maxseg size {low_mss}-65535 tcp option maxseg size set {mss}'") + self._cmdl(nft_prefix + base_cmd + ['tcp', 'option', 'maxseg', 'size', + f'{low_mss}-65535', 'tcp', 'option', 'maxseg', 'size', 'set', str(mss)]) def set_arp_filter(self, arp_filter): """ @@ -849,12 +859,12 @@ class Interface(Control): return self.set_interface('ipv4_directed_broadcast', forwarding) def _cleanup_ipv4_source_validation_rules(self, ifname): - results = self._cmd(f'nft -a list chain ip raw vyos_rpfilter').split("\n") + results = self._cmdl(['nft', '-a', 'list', 'chain', 'ip', 'raw', 'vyos_rpfilter']).split("\n") for line in results: if f'iifname "{ifname}"' in line: handle_search = re.search('handle (\d+)', line) if handle_search: - self._cmd(f'nft delete rule ip raw vyos_rpfilter handle {handle_search[1]}') + self._cmdl(['nft', 'delete', 'rule', 'ip', 'raw', 'vyos_rpfilter', 'handle', handle_search[1]]) def set_ipv4_source_validation(self, mode): """ @@ -869,21 +879,22 @@ class Interface(Control): return None self._cleanup_ipv4_source_validation_rules(self.ifname) - nft_prefix = f'nft insert rule ip raw vyos_rpfilter iifname "{self.ifname}"' + nft_prefix = ['nft', 'insert', 'rule', 'ip', 'raw', 'vyos_rpfilter', + 'iifname', f'"{self.ifname}"'] if mode in ['strict', 'loose']: - self._cmd(f"{nft_prefix} counter return") + self._cmdl(nft_prefix + ['counter', 'return']) if mode == 'strict': - self._cmd(f"{nft_prefix} fib saddr . iif oif 0 counter drop") + self._cmdl(nft_prefix + ['fib', 'saddr', '.', 'iif', 'oif', '0', 'counter', 'drop']) elif mode == 'loose': - self._cmd(f"{nft_prefix} fib saddr oif 0 counter drop") + self._cmdl(nft_prefix + ['fib', 'saddr', 'oif', '0', 'counter', 'drop']) def _cleanup_ipv6_source_validation_rules(self, ifname): - results = self._cmd(f'nft -a list chain ip6 raw vyos_rpfilter').split("\n") + results = self._cmdl(['nft', '-a', 'list', 'chain', 'ip6', 'raw', 'vyos_rpfilter']).split("\n") for line in results: if f'iifname "{ifname}"' in line: handle_search = re.search('handle (\d+)', line) if handle_search: - self._cmd(f'nft delete rule ip6 raw vyos_rpfilter handle {handle_search[1]}') + self._cmdl(['nft', 'delete', 'rule', 'ip6', 'raw', 'vyos_rpfilter', 'handle', handle_search[1]]) def set_ipv6_source_validation(self, mode): """ @@ -898,13 +909,14 @@ class Interface(Control): return None self._cleanup_ipv6_source_validation_rules(self.ifname) - nft_prefix = f'nft insert rule ip6 raw vyos_rpfilter iifname "{self.ifname}"' + nft_prefix = ['nft', 'insert', 'rule', 'ip6', 'raw', 'vyos_rpfilter', + 'iifname', f'"{self.ifname}"'] if mode in ['strict', 'loose']: - self._cmd(f"{nft_prefix} counter return") + self._cmdl(nft_prefix + ['counter', 'return']) if mode == 'strict': - self._cmd(f"{nft_prefix} fib saddr . iif oif 0 counter drop") + self._cmdl(nft_prefix + ['fib', 'saddr', '.', 'iif', 'oif', '0', 'counter', 'drop']) elif mode == 'loose': - self._cmd(f"{nft_prefix} fib saddr oif 0 counter drop") + self._cmdl(nft_prefix + ['fib', 'saddr', 'oif', '0', 'counter', 'drop']) def set_ipv6_accept_ra(self, accept_ra): """ @@ -968,15 +980,13 @@ class Interface(Control): """ Set the interface identifier for IPv6 autoconf. """ - cmd = f'ip token set {identifier} dev {self.ifname}' - self._cmd(cmd) + self._cmdl(['ip', 'token', 'set', identifier, 'dev', self.ifname]) def del_ipv6_interface_identifier(self): """ Delete the interface identifier for IPv6 autoconf. """ - cmd = f'ip token delete dev {self.ifname}' - self._cmd(cmd) + self._cmdl(['ip', 'token', 'delete', 'dev', self.ifname]) def set_ipv6_forwarding(self, forwarding): """ @@ -1381,10 +1391,8 @@ class Interface(Control): return netns = get_interface_namespace(self.ifname) - netns_cmd = f'ip netns exec {netns}' if netns else '' - cmd = f'{netns_cmd} ip addr flush dev {self.ifname}' # flush all addresses - self._cmd(cmd) + cmdl(['ip', 'addr', 'flush', 'dev', self.ifname], self.debug, netns=netns) def flush_ipv6_slaac_addrs(self) -> list: """ @@ -1395,7 +1403,6 @@ class Interface(Control): Will return a list of flushed IPv6 addresses. """ netns = get_interface_namespace(self.ifname) - netns_cmd = f'ip netns exec {netns}' if netns else '' tmp = get_interface_address(self.ifname) if not tmp or 'addr_info' not in tmp: return @@ -1414,8 +1421,7 @@ class Interface(Control): # Flush IPv6 addresses installed by router advertisement ra_addr = f"{addr_info['local']}/{addr_info['prefixlen']}" flushed.append(ra_addr) - cmd = f'{netns_cmd} ip -6 addr del dev {self.ifname} {ra_addr}' - self._cmd(cmd) + cmdl(['ip', '-6', 'addr', 'del', 'dev', self.ifname, ra_addr], self.debug, netns=netns) return flushed def flush_ipv6_slaac_routes(self, ra_addrs: list=[]) -> None: @@ -1431,9 +1437,8 @@ class Interface(Control): connected.append(str(IPv6Interface(addr).network)) netns = get_interface_namespace(self.ifname) - netns_cmd = f'ip netns exec {netns}' if netns else '' - tmp = self._cmd(f'{netns_cmd} ip -j -6 route show dev {self.ifname}') + tmp = cmdl(['ip', '-j', '-6', 'route', 'show', 'dev', self.ifname], self.debug, netns=netns) tmp = json.loads(tmp) # Parse interface routes. Example data: # {'dst': 'default', 'gateway': 'fe80::250:56ff:feb3:cdba', @@ -1443,11 +1448,11 @@ class Interface(Control): # If it's a default route received from RA, delete it if (dict_search('dst', route) == 'default' and dict_search('protocol', route) == 'ra'): - self._cmd(f'{netns_cmd} ip -6 route del default via {route["gateway"]} dev {self.ifname}') + cmdl(['ip', '-6', 'route', 'del', 'default', 'via', route['gateway'], 'dev', self.ifname], self.debug, netns=netns) # Remove connected prefixes received from RA if dict_search('dst', route) in connected: # If it's a connected prefix, delete it - self._cmd(f'{netns_cmd} ip -6 route del {route["dst"]} dev {self.ifname}') + cmdl(['ip', '-6', 'route', 'del', route['dst'], 'dev', self.ifname], self.debug, netns=netns) return None @@ -1499,16 +1504,13 @@ class Interface(Control): # Remove redundant VLANs from the system for vlan in list_diff(cur_vlan_ids, add_vlan): - cmd = f'bridge vlan del dev {self.ifname} vid {vlan} master' - self._cmd(cmd) + self._cmdl(['bridge', 'vlan', 'del', 'dev', self.ifname, 'vid', str(vlan), 'master']) for vlan in allowed_vlan_ids: - cmd = f'bridge vlan add dev {self.ifname} vid {vlan} master' - self._cmd(cmd) + self._cmdl(['bridge', 'vlan', 'add', 'dev', self.ifname, 'vid', str(vlan), 'master']) # Setting native VLAN to system if native_vlan_id: - cmd = f'bridge vlan add dev {self.ifname} vid {native_vlan_id} pvid untagged master' - self._cmd(cmd) + self._cmdl(['bridge', 'vlan', 'add', 'dev', self.ifname, 'vid', str(native_vlan_id), 'pvid', 'untagged', 'master']) def set_dhcp(self, enable: bool, vrf_changed: bool=False): """ @@ -1544,7 +1546,7 @@ class Interface(Control): render(dhclient_config_file, 'dhcp-client/ipv4.j2', self.config) # Reload systemd unit definitions as some options are dynamically generated - self._cmd('systemctl daemon-reload') + self._cmdl(['systemctl', 'daemon-reload']) netns = self.config['netns'] if 'netns' in self.config else None # When the DHCP client is restarted a brief outage will occur, as @@ -1554,7 +1556,7 @@ class Interface(Control): if (vrf_changed or ('dhcp_options_changed' in self.config) or (not is_systemd_service_active(systemd_service, netns=netns))): - return self._cmd(f'systemctl restart {systemd_service}') + return self._cmdl(['systemctl', 'restart', systemd_service]) else: netns = self.config['netns'] if 'netns' in self.config else None stop_systemd_unit(systemd_service, netns=netns) @@ -1604,7 +1606,7 @@ class Interface(Control): render(script_file, 'dhcp-client/dhcp6c-script.j2', config, permission=0o755) # Reload systemd unit definitions as some options are dynamically generated - self._cmd('systemctl daemon-reload') + self._cmdl(['systemctl', 'daemon-reload']) netns = self.config['netns'] if 'netns' in self.config else None # We must ignore any return codes. This is required to enable @@ -1753,7 +1755,7 @@ class Interface(Control): eapol_action='reload-or-restart' # start/stop WPA supplicant service - self._cmd(f'systemctl {eapol_action} wpa_supplicant-wired@{self.ifname}') + self._cmdl(['systemctl', eapol_action, f'wpa_supplicant-wired@{self.ifname}']) if 'eapol' not in self.config: # delete configuration on interface removal @@ -2128,15 +2130,18 @@ class VLANIf(Interface): if 'vlan_id' not in self.config: self.config['vlan_id'] = self.ifname.split('.')[-1] - cmd = 'ip link add link {source_interface} name {ifname} type vlan id {vlan_id}' + cmd = ['ip', 'link', 'add', 'link', self.config['source_interface'], + 'name', self.ifname, 'type', 'vlan', 'id', str(self.config['vlan_id'])] if 'protocol' in self.config: - cmd += ' protocol {protocol}' + cmd += ['protocol', self.config['protocol']] if 'ingress_qos' in self.config: - cmd += ' ingress-qos-map {ingress_qos}' + # ingress_qos is a space-separated list of "from:to" mappings - + # each one is its own argument to iproute2 + cmd += ['ingress-qos-map'] + self.config['ingress_qos'].split() if 'egress_qos' in self.config: - cmd += ' egress-qos-map {egress_qos}' + cmd += ['egress-qos-map'] + self.config['egress_qos'].split() - self._cmd(cmd.format(**self.config)) + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') diff --git a/python/vyos/ifconfig/l2tpv3.py b/python/vyos/ifconfig/l2tpv3.py index 141a77e7c..18a043e36 100644 --- a/python/vyos/ifconfig/l2tpv3.py +++ b/python/vyos/ifconfig/l2tpv3.py @@ -68,11 +68,11 @@ class L2TPv3If(Interface): wait_for_add_l2tpv3(cmd=c) # setup session - cmd = 'ip l2tp add session name {ifname}' - cmd += ' tunnel_id {tunnel_id}' - cmd += ' session_id {session_id}' - cmd += ' peer_session_id {peer_session_id}' - self._cmd(cmd.format(**self.config)) + cmd = ['ip', 'l2tp', 'add', 'session', 'name', self.ifname, + 'tunnel_id', str(self.config['tunnel_id']), + 'session_id', str(self.config['session_id']), + 'peer_session_id', str(self.config['peer_session_id'])] + self._cmdl(cmd) # No need for interface shut down. There exist no function to permanently enable tunnel. # But you can disable interface permanently with shutdown/disable command. @@ -101,12 +101,12 @@ class L2TPv3If(Interface): self._del_interface_from_ct_iface_map() if {'tunnel_id', 'session_id'} <= set(self.config): - cmd = 'ip l2tp del session tunnel_id {tunnel_id}' - cmd += ' session_id {session_id}' - self._cmd(cmd.format(**self.config)) + self._cmdl(['ip', 'l2tp', 'del', 'session', + 'tunnel_id', str(self.config['tunnel_id']), + 'session_id', str(self.config['session_id'])]) if 'tunnel_id' in self.config: - cmd = 'ip l2tp del tunnel tunnel_id {tunnel_id}' - self._cmd(cmd.format(**self.config)) + self._cmdl(['ip', 'l2tp', 'del', 'tunnel', + 'tunnel_id', str(self.config['tunnel_id'])]) # No need to call the baseclass as the interface is now already gone diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py index 4d76a1d46..fedaf63b5 100644 --- a/python/vyos/ifconfig/macsec.py +++ b/python/vyos/ifconfig/macsec.py @@ -42,32 +42,33 @@ class MACsecIf(Interface): """ # create tunnel interface - cmd = 'ip link add link {source_interface} {ifname} type macsec'.format(**self.config) - cmd += f' cipher {self.config["security"]["cipher"]}' + cmd = ['ip', 'link', 'add', 'link', self.config['source_interface'], + self.ifname, 'type', 'macsec', + 'cipher', self.config['security']['cipher']] if 'encrypt' in self.config["security"]: - cmd += ' encrypt on' + cmd += ['encrypt', 'on'] - self._cmd(cmd) + self._cmdl(cmd) # Check if using static keys if 'static' in self.config["security"]: # Set static TX key - cmd = 'ip macsec add {ifname} tx sa 0 pn 1 on key 00'.format(**self.config) - cmd += f' {self.config["security"]["static"]["key"]}' - self._cmd(cmd) + self._cmdl(['ip', 'macsec', 'add', self.ifname, 'tx', 'sa', '0', + 'pn', '1', 'on', 'key', '00', + self.config['security']['static']['key']]) for peer, peer_config in self.config["security"]["static"]["peer"].items(): if 'disable' in peer_config: continue # Create the address - cmd = 'ip macsec add {ifname} rx port 1 address'.format(**self.config) - cmd += f' {peer_config["mac"]}' - self._cmd(cmd) + cmd = ['ip', 'macsec', 'add', self.ifname, 'rx', 'port', '1', + 'address', peer_config['mac']] + self._cmdl(cmd) # Add the encryption key to the address - cmd += f' sa 0 pn 1 on key 01 {peer_config["key"]}' - self._cmd(cmd) + cmd += ['sa', '0', 'pn', '1', 'on', 'key', '01', peer_config['key']] + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') diff --git a/python/vyos/ifconfig/macvlan.py b/python/vyos/ifconfig/macvlan.py index 45fd1d351..0626b3d21 100644 --- a/python/vyos/ifconfig/macvlan.py +++ b/python/vyos/ifconfig/macvlan.py @@ -38,8 +38,9 @@ class MACVLANIf(Interface): down by default. """ # please do not change the order when assembling the command - cmd = 'ip link add {ifname} link {source_interface} type macvlan mode {mode}' - self._cmd(cmd.format(**self.config)) + cmd = ['ip', 'link', 'add', self.ifname, 'link', self.config['source_interface'], + 'type', 'macvlan', 'mode', self.config['mode']] + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') @@ -107,8 +108,7 @@ class MACVLANIf(Interface): return super().remove(skip_delete=skip_delete) def set_mode(self, mode): - cmd = f'ip link set dev {self.ifname} type macvlan mode {mode}' - return self._cmd(cmd) + return self._cmdl(['ip', 'link', 'set', 'dev', self.ifname, 'type', 'macvlan', 'mode', mode]) def get_source_interface(self): interface_config = get_interface_config(self.ifname) diff --git a/python/vyos/ifconfig/pppoe.py b/python/vyos/ifconfig/pppoe.py index 0f0640159..327c41719 100644 --- a/python/vyos/ifconfig/pppoe.py +++ b/python/vyos/ifconfig/pppoe.py @@ -45,11 +45,9 @@ class PPPoEIf(Interface): def _remove_routes(self, vrf=None): # Always delete default routes when interface is removed - vrf_cmd = '' - if vrf: - vrf_cmd = f'-c "vrf {vrf}"' - self._cmd(f'vtysh -c "conf t" {vrf_cmd} -c "no ip route 0.0.0.0/0 {self.ifname} tag 210"') - self._cmd(f'vtysh -c "conf t" {vrf_cmd} -c "no ipv6 route ::/0 {self.ifname} tag 210"') + vrf_cmd = ['-c', f'vrf {vrf}'] if vrf else [] + self._cmdl(['vtysh', '-c', 'conf t'] + vrf_cmd + ['-c', f'no ip route 0.0.0.0/0 {self.ifname} tag 210']) + self._cmdl(['vtysh', '-c', 'conf t'] + vrf_cmd + ['-c', f'no ipv6 route ::/0 {self.ifname} tag 210']) def remove(self): """ @@ -125,10 +123,10 @@ class PPPoEIf(Interface): super().update(config) # generate proper configuration string when VRFs are in use - vrf = '' + vrf = [] if 'vrf' in config: tmp = config['vrf'] - vrf = f'-c "vrf {tmp}"' + vrf = ['-c', f'vrf {tmp}'] # learn default router in Router Advertisement. tmp = '0' if 'no_default_route' in config else '1' @@ -137,10 +135,10 @@ class PPPoEIf(Interface): if 'no_default_route' not in config: # Set default route(s) pointing to PPPoE interface distance = config['default_route_distance'] - self._cmd(f'vtysh -c "conf t" {vrf} -c "ip route 0.0.0.0/0 {self.ifname} tag 210 {distance}"') + self._cmdl(['vtysh', '-c', 'conf t'] + vrf + ['-c', f'ip route 0.0.0.0/0 {self.ifname} tag 210 {distance}']) if 'ipv6' in config: - self._cmd(f'vtysh -c "conf t" {vrf} -c "ipv6 route ::/0 {self.ifname} tag 210 {distance}"') + self._cmdl(['vtysh', '-c', 'conf t'] + vrf + ['-c', f'ipv6 route ::/0 {self.ifname} tag 210 {distance}']) # kick RS when IPv6 is up. if dict_search('ipv6.address.autoconf', config) is not None: - self._cmd(f'rdisc6 --single --retry 3 {self.ifname}') + self._cmdl(['rdisc6', '--single', '--retry', '3', self.ifname]) diff --git a/python/vyos/ifconfig/tunnel.py b/python/vyos/ifconfig/tunnel.py index befaed8fd..958d497a3 100644 --- a/python/vyos/ifconfig/tunnel.py +++ b/python/vyos/ifconfig/tunnel.py @@ -108,12 +108,12 @@ class TunnelIf(Interface): else: mapping = { **self.mapping, **self.mapping_ipv4 } - cmd = 'ip tunnel add {ifname} mode {encapsulation}' + cmd = ['ip', 'tunnel', 'add', self.ifname, 'mode', self.config['encapsulation']] if self.config['encapsulation'] in ['gretap', 'ip6gretap', 'erspan', 'ip6erspan']: - cmd = 'ip link add name {ifname} type {encapsulation}' + cmd = ['ip', 'link', 'add', 'name', self.ifname, 'type', self.config['encapsulation']] # ERSPAN requires the serialisation of packets if self.config['encapsulation'] in ['erspan', 'ip6erspan']: - cmd += ' seq' + cmd += ['seq'] for vyos_key, iproute2_key in mapping.items(): # dict_search will return an empty dict "{}" for valueless nodes like @@ -121,11 +121,11 @@ class TunnelIf(Interface): # by using isinstance() tmp = dict_search(vyos_key, self.config) if isinstance(tmp, dict): - cmd += f' {iproute2_key}' + cmd += [iproute2_key] elif tmp != None: - cmd += f' {iproute2_key} {tmp}' + cmd += [iproute2_key, str(tmp)] - self._cmd(cmd.format(**self.config)) + self._cmdl(cmd) self.set_admin_state('down') @@ -139,18 +139,18 @@ class TunnelIf(Interface): else: mapping = { **self.mapping, **self.mapping_ipv4 } - cmd = 'ip tunnel change {ifname} mode {encapsulation}' + cmd = ['ip', 'tunnel', 'change', self.ifname, 'mode', self.config['encapsulation']] for vyos_key, iproute2_key in mapping.items(): # dict_search will return an empty dict "{}" for valueless nodes like # "parameters.nolearning" - thus we need to test the nodes existence # by using isinstance() tmp = dict_search(vyos_key, self.config) if isinstance(tmp, dict): - cmd += f' {iproute2_key}' + cmd += [iproute2_key] elif tmp != None: - cmd += f' {iproute2_key} {tmp}' + cmd += [iproute2_key, str(tmp)] - self._cmd(cmd.format(**self.config)) + self._cmdl(cmd) def get_mac(self): """ Get a synthetic MAC address. """ diff --git a/python/vyos/ifconfig/veth.py b/python/vyos/ifconfig/veth.py index 9868ea526..14bc9a22a 100644 --- a/python/vyos/ifconfig/veth.py +++ b/python/vyos/ifconfig/veth.py @@ -46,9 +46,9 @@ class VethIf(Interface): return # create virtual-ethernet interface - cmd = f'ip link add {self.ifname} type veth' - cmd += f' peer name {self.config["peer_name"]}' - self._cmd(cmd) + cmd = ['ip', 'link', 'add', self.ifname, 'type', 'veth'] + cmd += ['peer', 'name', self.config['peer_name']] + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') diff --git a/python/vyos/ifconfig/vti.py b/python/vyos/ifconfig/vti.py index 01d724593..699601aad 100644 --- a/python/vyos/ifconfig/vti.py +++ b/python/vyos/ifconfig/vti.py @@ -44,18 +44,18 @@ class VTIIf(Interface): # not have a lookup key configuration - thus we shift the key by one # to also support a vti0 interface if_id = str(int(if_id) +1) - cmd = f'ip link add {self.ifname} type xfrm if_id {if_id}' + cmd = ['ip', 'link', 'add', self.ifname, 'type', 'xfrm', 'if_id', if_id] for vyos_key, iproute2_key in mapping.items(): # dict_search will return an empty dict "{}" for valueless nodes like # "parameters.nolearning" - thus we need to test the nodes existence # by using isinstance() tmp = dict_search(vyos_key, self.config) if isinstance(tmp, dict): - cmd += f' {iproute2_key}' + cmd += [iproute2_key] elif tmp != None: - cmd += f' {iproute2_key} {tmp}' + cmd += [iproute2_key, str(tmp)] - self._cmd(cmd.format(**self.config)) + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_interface('admin_state', 'down') diff --git a/python/vyos/ifconfig/vtun.py b/python/vyos/ifconfig/vtun.py index e6963ce5d..fbe7127d5 100644 --- a/python/vyos/ifconfig/vtun.py +++ b/python/vyos/ifconfig/vtun.py @@ -33,8 +33,9 @@ class VTunIf(Interface): server can be reached, thus we might need to create this interface in advance for the service to be operational. """ try: - cmd = 'openvpn --mktun --dev-type {device_type} --dev {ifname}'.format(**self.config) - return self._cmd(cmd) + cmd = ['openvpn', '--mktun', '--dev-type', self.config['device_type'], + '--dev', self.ifname] + return self._cmdl(cmd) except PermissionError: # interface created by OpenVPN daemon in the meantime ... pass diff --git a/python/vyos/ifconfig/vxlan.py b/python/vyos/ifconfig/vxlan.py index 996a5c8f8..31559d206 100644 --- a/python/vyos/ifconfig/vxlan.py +++ b/python/vyos/ifconfig/vxlan.py @@ -92,27 +92,28 @@ class VXLANIf(Interface): remote_list = self.config['remote'][1:] self.config['remote'] = self.config['remote'][0] - cmd = 'ip link add {ifname} type vxlan dstport {port}' + cmd = ['ip', 'link', 'add', self.ifname, 'type', 'vxlan', + 'dstport', str(self.config['port'])] for vyos_key, iproute2_key in mapping.items(): # dict_search will return an empty dict "{}" for valueless nodes like # "parameters.nolearning" - thus we need to test the nodes existence # by using isinstance() tmp = dict_search(vyos_key, self.config) if isinstance(tmp, dict): - cmd += f' {iproute2_key}' + cmd += [iproute2_key] elif tmp != None: - cmd += f' {iproute2_key} {tmp}' + cmd += [iproute2_key, str(tmp)] - self._cmd(cmd.format(**self.config)) + self._cmdl(cmd) # interface is always A/D down. It needs to be enabled explicitly self.set_admin_state('down') # VXLAN tunnel is always recreated on any change - see interfaces_vxlan.py if remote_list: for remote in remote_list: - cmd = f'bridge fdb append to 00:00:00:00:00:00 dst {remote} ' \ - 'port {port} dev {ifname}' - self._cmd(cmd.format(**self.config)) + cmd = ['bridge', 'fdb', 'append', 'to', '00:00:00:00:00:00', + 'dst', remote, 'port', str(self.config['port']), 'dev', self.ifname] + self._cmdl(cmd) def set_neigh_suppress(self, state): """ diff --git a/python/vyos/ifconfig/wireguard.py b/python/vyos/ifconfig/wireguard.py index 94de0bb6b..4609aff25 100644 --- a/python/vyos/ifconfig/wireguard.py +++ b/python/vyos/ifconfig/wireguard.py @@ -214,7 +214,7 @@ class WireGuardOperational(Operational): print(f'WireGuard interface "{self.ifname}" peer "{peer_name}" disabled!') continue - cmd = f'wg set {self.ifname} peer {peer_public_key} endpoint {new_endpoint}' + cmd = ['wg', 'set', self.ifname, 'peer', peer_public_key, 'endpoint', new_endpoint] try: if (peer_public_key in current_peers and 'endpoint' in current_peers[peer_public_key] @@ -226,11 +226,11 @@ class WireGuardOperational(Operational): message = f'Resetting {self.ifname} peer {peer_public_key} endpoint to {new_endpoint} ... ' print(message, end='') - self._cmd(cmd, env={'WG_ENDPOINT_RESOLUTION_RETRIES': + self._cmdl(cmd, env={'WG_ENDPOINT_RESOLUTION_RETRIES': tmp['max_dns_retry']}) print('done') except: - print(f'Error\nPlease try to run command manually:\n{cmd}\n') + print(f'Error\nPlease try to run command manually:\n{" ".join(cmd)}\n') @Interface.register @@ -272,41 +272,42 @@ class WireGuardIf(Interface): on any interface.""" # Wireguard base command is identical for every peer - base_cmd = f'wg set {self.ifname}' + base_cmd = ['wg', 'set', self.ifname] - interface_cmd = base_cmd + interface_cmd = list(base_cmd) if 'port' in config: - interface_cmd += ' listen-port {port}' + interface_cmd += ['listen-port', str(config['port'])] if 'fwmark' in config: - interface_cmd += ' fwmark {fwmark}' + interface_cmd += ['fwmark', str(config['fwmark'])] with NamedTemporaryFile('w') as tmp_file: tmp_file.write(config['private_key']) tmp_file.flush() - interface_cmd += f' private-key {tmp_file.name}' - interface_cmd = interface_cmd.format(**config) + interface_cmd += ['private-key', tmp_file.name] # T6490: execute command to ensure interface configured - self._cmd(interface_cmd) + self._cmdl(interface_cmd) current_peer_public_keys = get_wireguard_peers(self.ifname) if 'rebuild_required' in config: # Remove all existing peers that no longer exist in config current_public_keys = self.get_peer_public_keys(config) - cmd_remove_peers = [f' peer {public_key} remove' - for public_key in current_peer_public_keys - if public_key not in current_public_keys] + cmd_remove_peers = [] + for public_key in current_peer_public_keys: + if public_key not in current_public_keys: + cmd_remove_peers += ['peer', public_key, 'remove'] if cmd_remove_peers: - self._cmd(base_cmd + ''.join(cmd_remove_peers)) + self._cmdl(base_cmd + cmd_remove_peers) if 'peer' in config: # Group removal of disabled peers in one command current_disabled_peers = self.get_peer_public_keys(config, disabled=True) - cmd_disabled_peers = [f' peer {public_key} remove' - for public_key in current_disabled_peers] + cmd_disabled_peers = [] + for public_key in current_disabled_peers: + cmd_disabled_peers += ['peer', public_key, 'remove'] if cmd_disabled_peers: - self._cmd(base_cmd + ''.join(cmd_disabled_peers)) + self._cmdl(base_cmd + cmd_disabled_peers) peer_cmds = [] peer_domain_cmds = [] @@ -319,58 +320,56 @@ class WireGuardIf(Interface): if 'disable' in peer_config: continue - # start of with a fresh 'wg' command - peer_cmd = ' peer {public_key}' - - cmd = peer_cmd + # start off with a fresh 'wg' peer command + cmd = ['peer', peer_config['public_key']] if 'preshared_key' in peer_config: with NamedTemporaryFile(mode='w', delete=False) as tmp_file: tmp_file.write(peer_config['preshared_key']) tmp_file.flush() - cmd += f' preshared-key {tmp_file.name}' + cmd += ['preshared-key', tmp_file.name] peer_psk_files.append(tmp_file.name) else: # If no PSK is given remove it by using /dev/null - passing keys via # the shell (usually bash) is considered insecure, thus we use a file - cmd += f' preshared-key /dev/null' + cmd += ['preshared-key', '/dev/null'] # Persistent keepalive is optional if 'persistent_keepalive' in peer_config: - cmd += ' persistent-keepalive {persistent_keepalive}' + cmd += ['persistent-keepalive', str(peer_config['persistent_keepalive'])] # Multiple allowed-ip ranges can be defined - ensure we are always # dealing with a list if isinstance(peer_config['allowed_ips'], str): peer_config['allowed_ips'] = [peer_config['allowed_ips']] - cmd += ' allowed-ips ' + ','.join(peer_config['allowed_ips']) + cmd += ['allowed-ips', ','.join(peer_config['allowed_ips'])] - peer_cmds.append(cmd.format(**peer_config)) + peer_cmds += cmd - cmd = peer_cmd + cmd = ['peer', peer_config['public_key']] # Ensure peer is created even if dns not working if {'address', 'port'} <= set(peer_config): if is_ipv6(peer_config['address']): - cmd += ' endpoint [{address}]:{port}' + cmd += ['endpoint', f"[{peer_config['address']}]:{peer_config['port']}"] elif is_ipv4(peer_config['address']): - cmd += ' endpoint {address}:{port}' + cmd += ['endpoint', f"{peer_config['address']}:{peer_config['port']}"] else: # don't set endpoint if address uses domain name continue elif {'host_name', 'port'} <= set(peer_config): - cmd += ' endpoint {host_name}:{port}' + cmd += ['endpoint', f"{peer_config['host_name']}:{peer_config['port']}"] else: continue - peer_domain_cmds.append(cmd.format(**peer_config)) + peer_domain_cmds += cmd try: if peer_cmds: - self._cmd(base_cmd + ''.join(peer_cmds)) + self._cmdl(base_cmd + peer_cmds) if peer_domain_cmds: - self._cmd(base_cmd + ''.join(peer_domain_cmds), env={ + self._cmdl(base_cmd + peer_domain_cmds, env={ 'WG_ENDPOINT_RESOLUTION_RETRIES': config['max_dns_retry']}) except Exception as e: Warning(f'Failed to apply Wireguard peers on {self.ifname}: {e}') diff --git a/python/vyos/ifconfig/wireless.py b/python/vyos/ifconfig/wireless.py index cafd7d0b4..9131f8f4c 100644 --- a/python/vyos/ifconfig/wireless.py +++ b/python/vyos/ifconfig/wireless.py @@ -30,16 +30,15 @@ class WiFiIf(Interface): } def _create(self): # all interfaces will be added in monitor mode - cmd = 'iw phy {physical_device} interface add {ifname} type monitor' - self._cmd(cmd.format(**self.config)) + cmd = ['iw', 'phy', self.config['physical_device'], 'interface', 'add', + self.ifname, 'type', 'monitor'] + self._cmdl(cmd) # wireless interface is administratively down by default self.set_admin_state('down') def _delete(self): - cmd = 'iw dev {ifname} del' \ - .format(**self.config) - self._cmd(cmd) + self._cmdl(['iw', 'dev', self.ifname, 'del']) def update(self, config): """ General helper function which works on a dictionary retrieved by diff --git a/python/vyos/ipt_netflow.py b/python/vyos/ipt_netflow.py index 14d2a458d..8fe7aec48 100644 --- a/python/vyos/ipt_netflow.py +++ b/python/vyos/ipt_netflow.py @@ -19,7 +19,7 @@ from vyos.utils.kernel import check_kmod from vyos.utils.kernel import unload_kmod -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos import ConfigError module_name = 'ipt_NETFLOW' @@ -35,8 +35,8 @@ def _iptables_get_rules(command, chain, table): rules = [] # run iptables, save output and split it by lines - iptables_command = f'{command} -vn -t {table} -L {chain}' - tmp = cmd(iptables_command, message='Failed to get flows list') + iptables_command = [command, '-vn', '-t', table, '-L', chain] + tmp = cmdl(iptables_command, message='Failed to get flows list') lines = tmp.splitlines() # Sample output to parse: @@ -114,7 +114,7 @@ def _iptables_config(command, configured_ifaces, direction): rulenums_delete.sort(reverse=True) for rulenum in rulenums_delete: iptables_commands.append( - f'{command} -t {iptables_table} -D {iptables_chain} {rulenum}' + [command, '-t', iptables_table, '-D', iptables_chain, str(rulenum)] ) # do not create new rules for already configured interfaces @@ -127,12 +127,13 @@ def _iptables_config(command, configured_ifaces, direction): iface = iface_extended['iface'] iface_option = "o" if direction == "egress" else "i" # iptables -t raw -A PREROUTING -j NETFLOW -i eth0 - rule_definition = f'{command} -t {iptables_table} -A {iptables_chain} -j NETFLOW -{iface_option} {iface}' + rule_definition = [command, '-t', iptables_table, '-A', iptables_chain, + '-j', 'NETFLOW', f'-{iface_option}', iface] iptables_commands.append(rule_definition) # change iptables for command in iptables_commands: - cmd(command, raising=ConfigError) + cmdl(command, raising=ConfigError) def _iptables_config_v4_and_v6(configured_ifaces, direction): diff --git a/python/vyos/qos/base.py b/python/vyos/qos/base.py index f77a6e020..65991704d 100644 --- a/python/vyos/qos/base.py +++ b/python/vyos/qos/base.py @@ -18,7 +18,7 @@ import jmespath from vyos.base import Warning from vyos.ifconfig import Interface -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.utils.dict import dict_search from vyos.utils.file import read_file @@ -70,10 +70,12 @@ class QoSBase: self._debug = True self._interface = interface - def _cmd(self, command): + def _cmdl(self, command): + if not isinstance(command, list): + raise TypeError(f'_cmdl() requires a list, got {type(command).__name__}') if self._debug: print(f'DEBUG/QoS: {command}') - return cmd(command) + return cmdl(command) def get_direction(self) -> list: return self._direction @@ -119,50 +121,52 @@ class QoSBase: https://github.com/vyos/vyatta-cfg-qos/blob/equuleus/lib/Vyatta/Qos/ShaperClass.pm#L223-L229 """ queue_type = dict_search('queue_type', config) - default_tc = f'tc qdisc replace dev {self._interface} parent {self._parent}:{cls_id:x}' + default_tc = ['tc', 'qdisc', 'replace', 'dev', self._interface, + 'parent', f'{self._parent}:{cls_id:x}'] if queue_type == 'priority': handle = 0x4000 + cls_id - default_tc += f' handle {handle:x}: prio' - self._cmd(default_tc) + default_tc += ['handle', f'{handle:x}:', 'prio'] + self._cmdl(default_tc) queue_limit = dict_search('queue_limit', config) for ii in range(1, 4): - tmp = f'tc qdisc replace dev {self._interface} parent {handle:x}:{ii:x} pfifo' - if queue_limit: tmp += f' limit {queue_limit}' - self._cmd(tmp) + tmp = ['tc', 'qdisc', 'replace', 'dev', self._interface, + 'parent', f'{handle:x}:{ii:x}', 'pfifo'] + if queue_limit: tmp += ['limit', str(queue_limit)] + self._cmdl(tmp) elif queue_type == 'fair-queue': - default_tc += f' sfq' + default_tc += ['sfq'] tmp = dict_search('queue_limit', config) - if tmp: default_tc += f' limit {tmp}' + if tmp: default_tc += ['limit', str(tmp)] - self._cmd(default_tc) + self._cmdl(default_tc) elif queue_type == 'fq-codel': - default_tc += f' fq_codel' + default_tc += ['fq_codel'] tmp = dict_search('codel_quantum', config) - if tmp: default_tc += f' quantum {tmp}' + if tmp: default_tc += ['quantum', str(tmp)] tmp = dict_search('flows', config) - if tmp: default_tc += f' flows {tmp}' + if tmp: default_tc += ['flows', str(tmp)] tmp = dict_search('interval', config) - if tmp: default_tc += f' interval {tmp}ms' + if tmp: default_tc += ['interval', f'{tmp}ms'] tmp = dict_search('queue_limit', config) - if tmp: default_tc += f' limit {tmp}' + if tmp: default_tc += ['limit', str(tmp)] tmp = dict_search('target', config) - if tmp: default_tc += f' target {tmp}ms' + if tmp: default_tc += ['target', f'{tmp}ms'] - default_tc += f' noecn' + default_tc += ['noecn'] - self._cmd(default_tc) + self._cmdl(default_tc) elif queue_type == 'random-detect': - default_tc += f' red' + default_tc += ['red'] qparams = self._calc_random_detect_queue_params( avg_pkt=dict_search('average_packet', config) or 1024, @@ -172,19 +176,19 @@ class QoSBase: mark_probability=dict_search('mark_probability', config) or 10 ) - default_tc += f' limit {qparams["limit"]} avpkt {qparams["avg_pkt"]}' - default_tc += f' max {qparams["max_val"]} min {qparams["min_val"]}' - default_tc += f' burst {qparams["burst"]} probability {qparams["probability"]}' + default_tc += ['limit', str(qparams['limit']), 'avpkt', str(qparams['avg_pkt'])] + default_tc += ['max', str(qparams['max_val']), 'min', str(qparams['min_val'])] + default_tc += ['burst', str(qparams['burst']), 'probability', str(qparams['probability'])] - self._cmd(default_tc) + self._cmdl(default_tc) elif queue_type == 'drop-tail': - default_tc += f' pfifo' + default_tc += ['pfifo'] tmp = dict_search('queue_limit', config) - if tmp: default_tc += f' limit {tmp}' + if tmp: default_tc += ['limit', str(tmp)] - self._cmd(default_tc) + self._cmdl(default_tc) def _rate_convert(self, rate) -> int: rates = { @@ -238,20 +242,21 @@ class QoSBase: self._build_base_qdisc(cls_config, int(cls)) # every match criteria has it's tc instance - filter_cmd_base = f'tc filter add dev {self._interface} parent {self._parent:x}:' + filter_cmd_base = ['tc', 'filter', 'add', 'dev', self._interface, + 'parent', f'{self._parent:x}:'] if priority: - filter_cmd_base += f' prio {cls}' + filter_cmd_base += ['prio', str(cls)] elif 'priority' in cls_config: prio = cls_config['priority'] - filter_cmd_base += f' prio {prio}' + filter_cmd_base += ['prio', str(prio)] if 'match' in cls_config: has_filter = False has_action_policy = any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in cls_config) max_index = len(cls_config['match']) for index, (match, match_config) in enumerate(cls_config['match'].items(), start=1): - filter_cmd = filter_cmd_base + filter_cmd = list(filter_cmd_base) if not has_filter: for key in ['mark', 'vif', 'ip', 'ipv6', 'interface', 'ether']: if key in match_config: @@ -259,22 +264,22 @@ class QoSBase: break tmp = dict_search(f'ether.protocol', match_config) or 'all' - filter_cmd += f' protocol {tmp}' + filter_cmd += ['protocol', str(tmp)] - if self.qostype in ['shaper', 'shaper_hfsc'] and 'prio ' not in filter_cmd: - filter_cmd += f' prio {index}' + if self.qostype in ['shaper', 'shaper_hfsc'] and 'prio' not in filter_cmd: + filter_cmd += ['prio', str(index)] if 'mark' in match_config: mark = match_config['mark'] - filter_cmd += f' handle {mark} fw' + filter_cmd += ['handle', str(mark), 'fw'] if 'vif' in match_config: vif = match_config['vif'] - filter_cmd += f' basic match "meta(vlan mask 0xfff eq {vif})"' + filter_cmd += ['basic', 'match', f'meta(vlan mask 0xfff eq {vif})'] elif 'interface' in match_config: iif_name = match_config['interface'] iif = Interface(iif_name).get_ifindex() - filter_cmd += f' basic match "meta(rt_iif eq {iif})"' + filter_cmd += ['basic', 'match', f'meta(rt_iif eq {iif})'] for af in ['ip', 'ipv6', 'ether']: tc_af = af @@ -282,42 +287,42 @@ class QoSBase: tc_af = 'ip6' if af in match_config: - filter_cmd += ' u32' + filter_cmd += ['u32'] if af == 'ether': src = dict_search(f'{af}.source', match_config) - if src: filter_cmd += f' match {tc_af} src {src}' + if src: filter_cmd += ['match', tc_af, 'src', str(src)] dst = dict_search(f'{af}.destination', match_config) - if dst: filter_cmd += f' match {tc_af} dst {dst}' + if dst: filter_cmd += ['match', tc_af, 'dst', str(dst)] if not src and not dst: - filter_cmd += f' match u32 0 0' + filter_cmd += ['match', 'u32', '0', '0'] else: tmp = dict_search(f'{af}.source.address', match_config) - if tmp: filter_cmd += f' match {tc_af} src {tmp}' + if tmp: filter_cmd += ['match', tc_af, 'src', str(tmp)] tmp = dict_search(f'{af}.source.port', match_config) - if tmp: filter_cmd += f' match {tc_af} sport {tmp} 0xffff' + if tmp: filter_cmd += ['match', tc_af, 'sport', str(tmp), '0xffff'] tmp = dict_search(f'{af}.destination.address', match_config) - if tmp: filter_cmd += f' match {tc_af} dst {tmp}' + if tmp: filter_cmd += ['match', tc_af, 'dst', str(tmp)] tmp = dict_search(f'{af}.destination.port', match_config) - if tmp: filter_cmd += f' match {tc_af} dport {tmp} 0xffff' + if tmp: filter_cmd += ['match', tc_af, 'dport', str(tmp), '0xffff'] ### tmp = dict_search(f'{af}.protocol', match_config) if tmp: tmp = get_protocol_by_name(tmp) - filter_cmd += f' match {tc_af} protocol {tmp} 0xff' + filter_cmd += ['match', tc_af, 'protocol', str(tmp), '0xff'] tmp = dict_search(f'{af}.dscp', match_config) if tmp: tmp = self._get_dsfield(tmp) if af == 'ip': - filter_cmd += f' match {tc_af} dsfield {tmp} 0xff' + filter_cmd += ['match', tc_af, 'dsfield', str(tmp), '0xff'] elif af == 'ipv6': - filter_cmd += f' match u16 {tmp} 0x0ff0 at 0' + filter_cmd += ['match', 'u16', str(tmp), '0x0ff0', 'at', '0'] # Will match against total length of an IPv4 packet and # payload length of an IPv6 packet. @@ -331,9 +336,9 @@ class QoSBase: tmp = hex(0xffff & ~int(tmp)) if af == 'ip': - filter_cmd += f' match u16 0x0000 {tmp} at 2' + filter_cmd += ['match', 'u16', '0x0000', str(tmp), 'at', '2'] elif af == 'ipv6': - filter_cmd += f' match u16 0x0000 {tmp} at 4' + filter_cmd += ['match', 'u16', '0x0000', str(tmp), 'at', '4'] # We match against specific TCP flags - we assume the IPv4 # header length is 20 bytes and assume the IPv6 packet is @@ -352,15 +357,15 @@ class QoSBase: mask = hex(mask) if af == 'ip': - filter_cmd += f' match u8 {mask} {mask} at 33' + filter_cmd += ['match', 'u8', str(mask), str(mask), 'at', '33'] elif af == 'ipv6': - filter_cmd += f' match u8 {mask} {mask} at 53' + filter_cmd += ['match', 'u8', str(mask), str(mask), 'at', '53'] if index != max_index or not has_action_policy: # avoid duplicate last match rule cls = int(cls) - filter_cmd += f' flowid {self._parent:x}:{cls:x}' - self._cmd(filter_cmd) + filter_cmd += ['flowid', f'{self._parent:x}:{cls:x}'] + self._cmdl(filter_cmd) vlan_expression = "match.*.vif" match_vlan = jmespath.search(vlan_expression, cls_config) @@ -368,30 +373,30 @@ class QoSBase: if has_action_policy and has_filter: # For "vif" "basic match" is used instead of "action police" T5961 if not match_vlan: - filter_cmd += f' action police' + filter_cmd += ['action', 'police'] if 'exceed' in cls_config: action = cls_config['exceed'] - filter_cmd += f' conform-exceed {action}' + filter_cmd += ['conform-exceed', str(action)] if 'not_exceed' in cls_config: action = cls_config['not_exceed'] - filter_cmd += f'/{action}' + filter_cmd[-1] += f'/{action}' if 'bandwidth' in cls_config: rate = self._rate_convert(cls_config['bandwidth']) - filter_cmd += f' rate {rate}' + filter_cmd += ['rate', str(rate)] if 'burst' in cls_config: burst = cls_config['burst'] - filter_cmd += f' burst {burst}' + filter_cmd += ['burst', str(burst)] if 'mtu' in cls_config: mtu = cls_config['mtu'] - filter_cmd += f' mtu {mtu}' + filter_cmd += ['mtu', str(mtu)] cls = int(cls) - filter_cmd += f' flowid {self._parent:x}:{cls:x}' - self._cmd(filter_cmd) + filter_cmd += ['flowid', f'{self._parent:x}:{cls:x}'] + self._cmdl(filter_cmd) # The police block allows limiting of the byte or packet rate of # traffic matched by the filter it is attached to. @@ -427,36 +432,37 @@ class QoSBase: if self.qostype == 'limiter': if 'default' in config: - filter_cmd = f'tc filter replace dev {self._interface} parent {self._parent:x}: ' - filter_cmd += 'prio 255 protocol all basic' + filter_cmd = ['tc', 'filter', 'replace', 'dev', self._interface, + 'parent', f'{self._parent:x}:', + 'prio', '255', 'protocol', 'all', 'basic'] # The police block allows limiting of the byte or packet rate of # traffic matched by the filter it is attached to. # https://man7.org/linux/man-pages/man8/tc-police.8.html if any(tmp in ['exceed', 'bandwidth', 'burst'] for tmp in config['default']): - filter_cmd += f' action police' + filter_cmd += ['action', 'police'] if 'exceed' in config['default']: action = config['default']['exceed'] - filter_cmd += f' conform-exceed {action}' + filter_cmd += ['conform-exceed', str(action)] if 'not_exceed' in config['default']: action = config['default']['not_exceed'] - filter_cmd += f'/{action}' + filter_cmd[-1] += f'/{action}' if 'bandwidth' in config['default']: rate = self._rate_convert(config['default']['bandwidth']) - filter_cmd += f' rate {rate}' + filter_cmd += ['rate', str(rate)] if 'burst' in config['default']: burst = config['default']['burst'] - filter_cmd += f' burst {burst}' + filter_cmd += ['burst', str(burst)] if 'mtu' in config['default']: mtu = config['default']['mtu'] - filter_cmd += f' mtu {mtu}' + filter_cmd += ['mtu', str(mtu)] if 'class' in config: - filter_cmd += f' flowid {self._parent:x}:{default_cls_id:x}' + filter_cmd += ['flowid', f'{self._parent:x}:{default_cls_id:x}'] - self._cmd(filter_cmd) + self._cmdl(filter_cmd) diff --git a/python/vyos/qos/cake.py b/python/vyos/qos/cake.py index 05a737649..34e100a33 100644 --- a/python/vyos/qos/cake.py +++ b/python/vyos/qos/cake.py @@ -65,7 +65,7 @@ class CAKE(QoSBase): tmp += ' nat' if 'flow_isolation_nat' in config else ' nonat' tmp += ' no-split-gso' if 'no_split_gso' in config else ' split-gso' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) diff --git a/python/vyos/qos/droptail.py b/python/vyos/qos/droptail.py index 223ab1e64..6e4ab3117 100644 --- a/python/vyos/qos/droptail.py +++ b/python/vyos/qos/droptail.py @@ -22,7 +22,7 @@ class DropTail(QoSBase): if 'queue_limit' in config: limit = config["queue_limit"] tmp += f' limit {limit}' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) diff --git a/python/vyos/qos/fairqueue.py b/python/vyos/qos/fairqueue.py index 8f4fe2d47..94627186a 100644 --- a/python/vyos/qos/fairqueue.py +++ b/python/vyos/qos/fairqueue.py @@ -25,7 +25,7 @@ class FairQueue(QoSBase): if 'queue_limit' in config: tmp += f' limit {config["queue_limit"]}' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) diff --git a/python/vyos/qos/fqcodel.py b/python/vyos/qos/fqcodel.py index d574226ef..ae5fa4ccc 100644 --- a/python/vyos/qos/fqcodel.py +++ b/python/vyos/qos/fqcodel.py @@ -34,7 +34,7 @@ class FQCodel(QoSBase): tmp += f' target {target}' tmp += f' noecn' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) diff --git a/python/vyos/qos/limiter.py b/python/vyos/qos/limiter.py index dce376d3e..36189e71b 100644 --- a/python/vyos/qos/limiter.py +++ b/python/vyos/qos/limiter.py @@ -21,7 +21,7 @@ class Limiter(QoSBase): def update(self, config, direction): tmp = f'tc qdisc add dev {self._interface} handle {self._parent:x}: {direction}' - self._cmd(tmp) + self._cmdl(tmp.split()) # base class must be called last super().update(config, direction) diff --git a/python/vyos/qos/netem.py b/python/vyos/qos/netem.py index 8fdd75387..54ac0743a 100644 --- a/python/vyos/qos/netem.py +++ b/python/vyos/qos/netem.py @@ -47,7 +47,7 @@ class NetEm(QoSBase): duplicate = config["duplicate"] tmp += f' duplicate {duplicate}%' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) diff --git a/python/vyos/qos/priority.py b/python/vyos/qos/priority.py index 5f373f696..2b323512a 100644 --- a/python/vyos/qos/priority.py +++ b/python/vyos/qos/priority.py @@ -29,13 +29,13 @@ class Priority(QoSBase): f'{class_id_max} {class_id_max} {class_id_max} {class_id_max} ' \ f'{class_id_max} {class_id_max} {class_id_max} {class_id_max} ' \ f'{class_id_max} {class_id_max} {class_id_max} {class_id_max} ' - self._cmd(tmp) + self._cmdl(tmp.split()) if 'class' in config: for cls in config['class']: cls = int(cls) tmp = f'tc qdisc add dev {self._interface} parent {self._parent:x}:{cls:x} pfifo' - self._cmd(tmp) + self._cmdl(tmp.split()) # base class must be called last super().update(config, direction, priority=True) diff --git a/python/vyos/qos/randomdetect.py b/python/vyos/qos/randomdetect.py index 63445bb62..56e15e404 100644 --- a/python/vyos/qos/randomdetect.py +++ b/python/vyos/qos/randomdetect.py @@ -24,7 +24,7 @@ class RandomDetect(QoSBase): # # Generalized Random Early Detection handle = self._parent tmp = f'tc qdisc add dev {self._interface} root handle {self._parent}:0 gred setup DPs 8 default 0 grio' - self._cmd(tmp) + self._cmdl(tmp.split()) bandwidth = self._rate_convert(config['bandwidth']) # set VQ (virtual queue) parameters @@ -40,7 +40,7 @@ class RandomDetect(QoSBase): ) tmp = f'tc qdisc change dev {self._interface} handle {handle}:0 gred limit {qparams["limit"]} min {qparams["min_val"]} max {qparams["max_val"]} avpkt {qparams["avg_pkt"]} ' tmp += f'burst {qparams["burst"]} bandwidth {bandwidth} probability {qparams["probability"]} DP {precedence} prio {8 - precedence:x}' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) diff --git a/python/vyos/qos/ratelimiter.py b/python/vyos/qos/ratelimiter.py index b0d7b3072..0a6011430 100644 --- a/python/vyos/qos/ratelimiter.py +++ b/python/vyos/qos/ratelimiter.py @@ -34,4 +34,4 @@ class RateLimiter(QoSBase): latency = config['latency'] tmp += f' latency {latency}ms' - self._cmd(tmp) + self._cmdl(tmp.split()) diff --git a/python/vyos/qos/roundrobin.py b/python/vyos/qos/roundrobin.py index d07dc0f52..b75b19fb2 100644 --- a/python/vyos/qos/roundrobin.py +++ b/python/vyos/qos/roundrobin.py @@ -22,16 +22,16 @@ class RoundRobin(QoSBase): # https://man7.org/linux/man-pages/man8/tc-drr.8.html def update(self, config, direction): tmp = f'tc qdisc add dev {self._interface} root handle 1: drr' - self._cmd(tmp) + self._cmdl(tmp.split()) if 'class' in config: for cls in config['class']: cls = int(cls) tmp = f'tc class replace dev {self._interface} parent 1:1 classid 1:{cls:x} drr' - self._cmd(tmp) + self._cmdl(tmp.split()) tmp = f'tc qdisc replace dev {self._interface} parent 1:{cls:x} pfifo' - self._cmd(tmp) + self._cmdl(tmp.split()) if 'default' in config: class_id_max = self._get_class_max_id(config) @@ -39,7 +39,7 @@ class RoundRobin(QoSBase): # class ID via CLI is in range 1-4095, thus 1000 hex = 4096 tmp = f'tc class replace dev {self._interface} parent 1:1 classid 1:{default_cls_id:x} drr' - self._cmd(tmp) + self._cmdl(tmp.split()) # You need to add at least one filter to classify packets # otherwise, all packets will be dropped. @@ -49,7 +49,7 @@ class RoundRobin(QoSBase): 'u32 match u32 0 0 ' f'flowid {self._parent}:{default_cls_id}' ) - self._cmd(filter_cmd) + self._cmdl(filter_cmd.split()) # call base class super().update(config, direction, priority=True) diff --git a/python/vyos/qos/trafficshaper.py b/python/vyos/qos/trafficshaper.py index 3840e7d0e..24e2ca06e 100644 --- a/python/vyos/qos/trafficshaper.py +++ b/python/vyos/qos/trafficshaper.py @@ -61,10 +61,10 @@ class TrafficShaper(QoSBase): default_minor_id = int(class_id_max) +1 tmp = f'tc qdisc replace dev {self._interface} root handle {self._parent:x}: htb r2q {r2q} default {default_minor_id:x}' # default is in hex - self._cmd(tmp) + self._cmdl(tmp.split()) tmp = f'tc class replace dev {self._interface} parent {self._parent:x}: classid {self._parent:x}:1 htb rate {speed}' - self._cmd(tmp) + self._cmdl(tmp.split()) if 'class' in config: for cls, cls_config in config['class'].items(): @@ -94,10 +94,10 @@ class TrafficShaper(QoSBase): if 'ceiling' in cls_config: f_ceil = self._rate_convert(cls_config['ceiling']) tmp += f' ceil {f_ceil}' - self._cmd(tmp) + self._cmdl(tmp.split()) tmp = f'tc qdisc replace dev {self._interface} parent {self._parent:x}:{cls:x} sfq' - self._cmd(tmp) + self._cmdl(tmp.split()) if 'default' in config: if config['default']['bandwidth'].endswith('%'): @@ -118,10 +118,10 @@ class TrafficShaper(QoSBase): else: f_ceil = self._rate_convert(config['default']['ceiling']) tmp += f' ceil {f_ceil}' - self._cmd(tmp) + self._cmdl(tmp.split()) tmp = f'tc qdisc replace dev {self._interface} parent {self._parent:x}:{default_minor_id:x} sfq' - self._cmd(tmp) + self._cmdl(tmp.split()) # call base class super().update(config, direction) @@ -163,10 +163,10 @@ class TrafficShaperHFSC(QoSBase): f' m2 {self._rate_convert(param["m2"])}' ) - self._cmd(tmp) + self._cmdl(tmp.split()) tmp = f'tc qdisc replace dev {self._interface} parent {self._parent:x}:{cls:x} sfq perturb 10' - self._cmd(tmp) + self._cmdl(tmp.split()) def update(self, config, direction): class_id_max = self._get_class_max_id(config) @@ -175,13 +175,13 @@ class TrafficShaperHFSC(QoSBase): speed = self._rate_convert(config['bandwidth']) tmp = f'tc qdisc replace dev {self._interface} root handle {self._parent:x}: hfsc default {default_cls_id:x}' # default is in hex - self._cmd(tmp) + self._cmdl(tmp.split()) tmp = f'tc class replace dev {self._interface} parent {self._parent:x}: classid {self._parent:x}:1 hfsc sc rate {speed} ul rate {speed}' - self._cmd(tmp) + self._cmdl(tmp.split()) # tmp = f'tc qdisc add dev {self._interface} parent {self._parent:x}:1 handle f1: sfq perturb 10' - # self._cmd(tmp) + # self._cmdl(tmp.split()) if 'class' in config: for cls, cls_config in config['class'].items(): diff --git a/python/vyos/raid.py b/python/vyos/raid.py index 4ae63a100..a693cdd7d 100644 --- a/python/vyos/raid.py +++ b/python/vyos/raid.py @@ -14,7 +14,7 @@ # along with this library. If not, see <http://www.gnu.org/licenses/>. from vyos.utils.disk import device_from_id -from vyos.utils.process import cmd +from vyos.utils.process import cmdl def raid_sets(): """ @@ -54,9 +54,9 @@ def add_raid_member(raid_set_name: str, member: str, by_id: bool = False): raise ValueError(f"Partition {member} does not exist") if member in raid_set_members(raid_set_name): raise ValueError(f"Partition {member} is already a member of RAID set {raid_set_name}") - cmd(f'mdadm --add /dev/{raid_set_name} /dev/{member}') - disk = cmd(f'lsblk -ndo PKNAME /dev/{member}') - cmd(f'grub-install /dev/{disk}') + cmdl(['mdadm', '--add', f'/dev/{raid_set_name}', f'/dev/{member}']) + disk = cmdl(['lsblk', '-ndo', 'PKNAME', f'/dev/{member}']) + cmdl(['grub-install', f'/dev/{disk}']) def delete_raid_member(raid_set_name: str, member: str, by_id: bool = False): """ @@ -68,4 +68,4 @@ def delete_raid_member(raid_set_name: str, member: str, by_id: bool = False): raise ValueError(f"RAID set {raid_set_name} does not exist") if member not in raid_set_members(raid_set_name): raise ValueError(f"Partition {member} is not a member of RAID set {raid_set_name}") - cmd(f'mdadm --remove /dev/{raid_set_name} /dev/{member}') + cmdl(['mdadm', '--remove', f'/dev/{raid_set_name}', f'/dev/{member}']) diff --git a/python/vyos/remote.py b/python/vyos/remote.py index 724b46e98..a3cbc1abb 100644 --- a/python/vyos/remote.py +++ b/python/vyos/remote.py @@ -53,7 +53,7 @@ from vyos.utils.io import ask_yes_no from vyos.utils.io import is_interactive from vyos.utils.io import print_error from vyos.utils.misc import begin -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.utils.process import rc_cmd from vyos.version import get_version from vyos.base import Warning @@ -477,21 +477,23 @@ class TftpC: source_port=0, timeout=10, vrf=None): - source_option = f'--interface {source_host} --local-port {source_port}' if source_host else '' - progress_flag = '--progress-bar' if progressbar else '--silent' - self.command = f'curl {source_option} {progress_flag} --connect-timeout {timeout}' + self.command = ['curl'] + if source_host: + self.command += ['--interface', str(source_host), '--local-port', str(source_port)] + self.command += ['--progress-bar'] if progressbar else ['--silent'] + self.command += ['--connect-timeout', str(timeout)] self.urlstring = urllib.parse.urlunsplit(url) self.vrf = vrf def download(self, location: str): with open(location, 'wb') as f: - f.write(cmd(f'{self.command} "{self.urlstring}"', - vrf=self.vrf).encode()) + f.write(cmdl(self.command + [self.urlstring], + vrf=self.vrf).encode()) def upload(self, location: str): with open(location, 'rb') as f: - cmd(f'{self.command} --upload-file - "{self.urlstring}"', - input=f.read(), vrf=self.vrf) + cmdl(self.command + ['--upload-file', '-', self.urlstring], + input=f.read(), vrf=self.vrf) class GitC: def __init__(self, diff --git a/python/vyos/system/disk.py b/python/vyos/system/disk.py index 7ae9a15bb..359afa2aa 100644 --- a/python/vyos/system/disk.py +++ b/python/vyos/system/disk.py @@ -20,7 +20,7 @@ from time import sleep from psutil import disk_partitions -from vyos.utils.process import run, cmd +from vyos.utils.process import run, cmdl @dataclass @@ -103,7 +103,7 @@ def partition_list(drive_path: str) -> list[str]: Returns: list[str]: a list of partition paths """ - lsblk: str = cmd(f'lsblk -Jp {drive_path}') + lsblk: str = cmdl(['lsblk', '-Jp', drive_path]) drive_info: dict = json_loads(lsblk) device: list = drive_info.get('blockdevices') children: list[str] = device[0].get('children', []) if device else [] @@ -120,7 +120,7 @@ def partition_parent(partition_path: str) -> str: Returns: str: path to a parent device """ - parent: str = cmd(f'lsblk -ndpo pkname {partition_path}') + parent: str = cmdl(['lsblk', '-ndpo', 'pkname', partition_path]) return parent @@ -234,7 +234,7 @@ def disks_size() -> dict[str, int]: dict[str, int]: a dictionary with name: size mapping """ disks_size: dict[str, int] = {} - lsblk: str = cmd('lsblk -Jbp') + lsblk: str = cmdl(['lsblk', '-Jbp']) blk_list = json_loads(lsblk) for device in blk_list.get('blockdevices'): if device['type'] == 'disk': diff --git a/python/vyos/system/grub.py b/python/vyos/system/grub.py index 9435d18d2..fea12d803 100644 --- a/python/vyos/system/grub.py +++ b/python/vyos/system/grub.py @@ -26,7 +26,7 @@ from uuid import UUID from vyos.flavor import get_image_serial_console from vyos.system import disk from vyos.template import render -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.utils.process import rc_cmd # Define variables @@ -69,25 +69,27 @@ def install(drive_path: str, boot_dir: str, efi_dir: str, id: str = 'VyOS', chro efi_dir (str): a path to '/boot/efi' directory """ - if chroot: - chroot_cmd = f"chroot {chroot}" - else: - chroot_cmd = "" + chroot_prefix = ['chroot', chroot] if chroot else [] efi_installation_arch = "x86_64" if platform.machine() == "aarch64": efi_installation_arch = "arm64" elif platform.machine() == "x86_64": - cmd( - f'{chroot_cmd} grub-install --no-floppy --target=i386-pc \ - --boot-directory={boot_dir} {drive_path} --force' + cmdl( + chroot_prefix + [ + 'grub-install', '--no-floppy', '--target=i386-pc', + f'--boot-directory={boot_dir}', drive_path, '--force', + ] ) - cmd( - f'{chroot_cmd} grub-install --no-floppy --recheck --target={efi_installation_arch}-efi \ - --force-extra-removable --boot-directory={boot_dir} \ - --efi-directory={efi_dir} --bootloader-id="{id}" \ - --uefi-secure-boot' + cmdl( + chroot_prefix + [ + 'grub-install', '--no-floppy', '--recheck', + f'--target={efi_installation_arch}-efi', + '--force-extra-removable', f'--boot-directory={boot_dir}', + f'--efi-directory={efi_dir}', f'--bootloader-id={id}', + '--uefi-secure-boot', + ] ) @@ -190,7 +192,7 @@ def read_env(env_file: str = '') -> dict[str, str]: root_dir: str = disk.find_persistence() env_file = f'{root_dir}/{GRUB_DIR_MAIN}/grubenv' - env_content: str = cmd(f'grub-editenv {env_file} list').splitlines() + env_content: str = cmdl(['grub-editenv', env_file, 'list']).splitlines() regex_filter = re_compile(r'^(?P<variable_name>.*)=(?P<variable_value>.*)$') env_dict: dict[str, str] = {} for env_item in env_content: diff --git a/python/vyos/system/raid.py b/python/vyos/system/raid.py index c03764ad1..93d8e21fe 100644 --- a/python/vyos/system/raid.py +++ b/python/vyos/system/raid.py @@ -19,7 +19,7 @@ from pathlib import Path from shutil import copy from dataclasses import dataclass -from vyos.utils.process import cmd, run +from vyos.utils.process import cmdl, run from vyos.system import disk @@ -43,18 +43,17 @@ def raid_create(raid_members: list[str], raid_level (str, optional): an array level. Defaults to 'raid1'. """ raid_devices_num: int = len(raid_members) - raid_members_str: str = ' '.join(raid_members) for part in raid_members: drive: str = disk.partition_parent(part) # set partition type GUID for raid member; cf. # https://en.wikipedia.org/wiki/GUID_Partition_Table#Partition_type_GUIDs - command: str = f'sgdisk --typecode=3:A19D880F-05FC-4D3B-A006-743F0F84911E {drive}' - cmd(command) - command: str = f'mdadm --create /dev/{raid_name} -R --metadata=1.0 \ - --raid-devices={raid_devices_num} --level={raid_level} \ - {raid_members_str}' + command: list[str] = ['sgdisk', '--typecode=3:A19D880F-05FC-4D3B-A006-743F0F84911E', drive] + cmdl(command) + command: list[str] = ['mdadm', '--create', f'/dev/{raid_name}', '-R', '--metadata=1.0', + f'--raid-devices={raid_devices_num}', f'--level={raid_level}', + *raid_members] - cmd(command) + cmdl(command) raid = RaidDetails( name = f'/dev/{raid_name}', @@ -67,8 +66,8 @@ def raid_create(raid_members: list[str], def clear(): """Deactivate all RAID arrays""" - command: str = 'mdadm --examine --scan' - raid_config = cmd(command) + command: list[str] = ['mdadm', '--examine', '--scan'] + raid_config = cmdl(command) if not raid_config: return command: str = 'mdadm --run /dev/md?*' @@ -85,8 +84,8 @@ def update_initramfs() -> None: copy('/usr/share/initramfs-tools/scripts/local-block/mdadm', mdadm_script) p = Path(mdadm_script) p.write_text(p.read_text().replace('$((COUNT + 1))', '20')) - command: str = 'update-initramfs -u' - cmd(command) + command: list[str] = ['update-initramfs', '-u'] + cmdl(command) def update_default(target_dir: str) -> None: """Update /etc/default/mdadm to start MD monitoring daemon at boot @@ -101,8 +100,8 @@ def update_default(target_dir: str) -> None: def get_uuid(device: str) -> str: """Get UUID of a device""" - command: str = f'tune2fs -l {device}' - l = cmd(command).splitlines() + command: list[str] = ['tune2fs', '-l', device] + l = cmdl(command).splitlines() uuid = next((x for x in l if x.startswith('Filesystem UUID')), '') return uuid.split(':')[1].strip() if uuid else '' diff --git a/python/vyos/utils/assertion.py b/python/vyos/utils/assertion.py index 35baa556b..f4e90df62 100644 --- a/python/vyos/utils/assertion.py +++ b/python/vyos/utils/assertion.py @@ -39,8 +39,8 @@ def assert_mtu(mtu, ifname): assert_number(mtu) import json - from vyos.utils.process import cmd - out = cmd(f'ip -j -d link show dev {ifname}') + from vyos.utils.process import cmdl + out = cmdl(['ip', '-j', '-d', 'link', 'show', 'dev', ifname]) # [{"ifindex":2,"ifname":"eth0","flags":["BROADCAST","MULTICAST","UP","LOWER_UP"],"mtu":1500,"qdisc":"pfifo_fast","operstate":"UP","linkmode":"DEFAULT","group":"default","txqlen":1000,"link_type":"ether","address":"08:00:27:d9:5b:04","broadcast":"ff:ff:ff:ff:ff:ff","promiscuity":0,"min_mtu":46,"max_mtu":16110,"inet6_addr_gen_mode":"none","num_tx_queues":1,"num_rx_queues":1,"gso_max_size":65536,"gso_max_segs":65535}] parsed = json.loads(out)[0] min_mtu = int(parsed.get('min_mtu', '0')) diff --git a/python/vyos/utils/auth.py b/python/vyos/utils/auth.py index 7123bd0a5..e85dc9984 100644 --- a/python/vyos/utils/auth.py +++ b/python/vyos/utils/auth.py @@ -24,7 +24,7 @@ from enum import StrEnum from typing import List from typing import Optional -from vyos.utils.process import cmd +from vyos.utils.process import cmdl # Minimum UID used when adding system users MIN_USER_UID: int = 1000 @@ -118,8 +118,7 @@ def evaluate_strength(passwd: str) -> dict[str, str]: def make_password_hash(password): """ Makes a password hash for /etc/shadow using mkpasswd """ - mkpassword = 'mkpasswd --method=yescrypt --stdin' - return cmd(mkpassword, input=password, timeout=5) + return cmdl(['mkpasswd', '--method=yescrypt', '--stdin'], input=password, timeout=5) def split_ssh_public_key(key_string, defaultname=""): """ Splits an SSH public key into its components """ diff --git a/python/vyos/utils/cpu.py b/python/vyos/utils/cpu.py index 0f47123a4..a4c0f2d54 100644 --- a/python/vyos/utils/cpu.py +++ b/python/vyos/utils/cpu.py @@ -110,9 +110,9 @@ def get_available_cpus(): """ import json - from vyos.utils.process import cmd + from vyos.utils.process import cmdl - out = json.loads(cmd('lscpu --extended -b --json')) + out = json.loads(cmdl(['lscpu', '--extended', '-b', '--json'])) return out['cpus'] diff --git a/python/vyos/utils/disk.py b/python/vyos/utils/disk.py index b822badde..03da4d346 100644 --- a/python/vyos/utils/disk.py +++ b/python/vyos/utils/disk.py @@ -25,7 +25,7 @@ def device_from_id(id): def get_storage_stats(directory, human_units=True): """ Return basic storage stats for given directory """ from re import sub as re_sub - from vyos.utils.process import cmd + from vyos.utils.process import cmdl from vyos.utils.convert import human_to_bytes # XXX: using `df -h` and converting human units to bytes @@ -40,7 +40,7 @@ def get_storage_stats(directory, human_units=True): # Filesystem Size Used Avail Use% # /dev/sda1 16G 7.6G 7.3G 51% - out = cmd(f"df -h --output=source,size,used,avail,pcent {directory}") + out = cmdl(['df', '-h', '--output=source,size,used,avail,pcent', str(directory)]) lines = out.splitlines() lists = [l.split() for l in lines] res = {lists[0][i]: lists[1][i] for i in range(len(lists[0]))} diff --git a/python/vyos/utils/file.py b/python/vyos/utils/file.py index f230977a7..08a3fa82d 100644 --- a/python/vyos/utils/file.py +++ b/python/vyos/utils/file.py @@ -39,8 +39,8 @@ def read_file(fname, defaultonfailure=None, sudo=False): try: # Some files can only be read by root - emulate sudo cat call if sudo: - from vyos.utils.process import cmd - data = cmd(['sudo', 'cat', fname]) + from vyos.utils.process import cmdl + data = cmdl(['cat', fname], sudo=True) else: # If not sudo, just read the file with open(fname, 'r') as f: diff --git a/python/vyos/utils/misc.py b/python/vyos/utils/misc.py index 2fe25da91..d52f0015a 100644 --- a/python/vyos/utils/misc.py +++ b/python/vyos/utils/misc.py @@ -32,7 +32,8 @@ def begin0(*args): def install_into_config(conf, config_paths, override_prompt=True): # Allows op-mode scripts to install values if called from an active config session - # config_paths: dict of config paths + # config_paths: list of config paths, each path a list of node/value components, + # e.g. ['pki', 'ca', name, 'crl', crl_pem] # override_prompt: if True, user will be prompted before existing nodes are overwritten if not config_paths: return None @@ -42,12 +43,12 @@ def install_into_config(conf, config_paths, override_prompt=True): from vyos.config import Config from vyos.defaults import base_dir from vyos.utils.io import ask_yes_no - from vyos.utils.process import cmd + from vyos.utils.process import cmdl if not Config().in_session(): print('You are not in configure mode, commands to install manually from configure mode:') for path in config_paths: - print(f'set {path}') + print('set ' + ' '.join(str(p) for p in path)) return None count = 0 @@ -58,15 +59,18 @@ def install_into_config(conf, config_paths, override_prompt=True): env['vyos_validators_dir'] = f'{base_dir}/validators' for path in config_paths: - if override_prompt and conf.exists(path) and not conf.is_multi(path): - if not ask_yes_no(f'Config node "{path}" already exists. Do you want to overwrite it?'): + # exists()/is_multi() are called with a string: the live-session + # config source backend requires it (it does " ".join(level) + " " + path) + path_str = ' '.join(str(p) for p in path) + if override_prompt and conf.exists(path_str) and not conf.is_multi(path_str): + if not ask_yes_no(f'Config node "{path_str}" already exists. Do you want to overwrite it?'): continue try: - cmd(f'/opt/vyatta/sbin/my_set {path}', env=env) + cmdl(['/opt/vyatta/sbin/my_set'] + [str(p) for p in path], env=env) count += 1 except: - failed.append(path) + failed.append(path_str) if failed: print(f'Failed to install {len(failed)} value(s). Commands to manually install:') diff --git a/python/vyos/utils/network.py b/python/vyos/utils/network.py index cde374d03..5b75de1cc 100644 --- a/python/vyos/utils/network.py +++ b/python/vyos/utils/network.py @@ -18,7 +18,7 @@ import hashlib from json import loads from socket import AF_INET from socket import AF_INET6 -from vyos.utils.process import cmd +from vyos.utils.process import cmdl def _are_same_ip(one, two): from socket import inet_pton @@ -72,11 +72,11 @@ def get_host_identity() -> str: uuid_file = '/sys/class/dmi/id/product_uuid' if os.path.exists(uuid_file): - uuid = cmd(f"sudo cat {uuid_file}").strip().replace("-", "").lower() + uuid = cmdl(['cat', uuid_file], sudo=True).strip().replace("-", "").lower() else: uuid = None - host = cmd("hostname").strip().lower() + host = cmdl(['hostname']).strip().lower() if uuid is not None: return f"{uuid}:{host}" @@ -109,7 +109,7 @@ def gen_mac(name: str, addr: str, ident: str) -> str: return ":".join(f"{x:02x}" for x in b) def get_netns_all() -> list: - tmp = loads(cmd('ip --json netns ls')) + tmp = loads(cmdl(['ip', '--json', 'netns', 'ls'])) return [ netns['name'] for netns in tmp ] def get_vrf_members(vrf: str) -> list: @@ -122,7 +122,7 @@ def get_vrf_members(vrf: str) -> list: try: if not interface_exists(vrf): raise ValueError(f'VRF "{vrf}" does not exist!') - output = cmd(f'ip --json --brief link show vrf {vrf}') + output = cmdl(['ip', '--json', '--brief', 'link', 'show', 'vrf', vrf]) answer = loads(output) for data in answer: if 'ifname' in data: @@ -199,7 +199,7 @@ def get_interface_config(interface): """ if not interface_exists(interface): return None - tmp = loads(cmd(f'ip --detail --json link show dev {interface}'))[0] + tmp = loads(cmdl(['ip', '--detail', '--json', 'link', 'show', 'dev', interface]))[0] return tmp def get_interface_address(interface): @@ -208,7 +208,7 @@ def get_interface_address(interface): """ if not interface_exists(interface): return None - tmp = loads(cmd(f'ip --detail --json addr show dev {interface}'))[0] + tmp = loads(cmdl(['ip', '--detail', '--json', 'addr', 'show', 'dev', interface]))[0] return tmp def get_interface_namespace(interface: str): @@ -216,13 +216,13 @@ def get_interface_namespace(interface: str): Returns which netns the interface belongs to """ # Bail out early if netns does not exist - tmp = cmd(f'ip --json netns ls') + tmp = cmdl(['ip', '--json', 'netns', 'ls']) if not tmp: return None for ns in loads(tmp): netns = f'{ns["name"]}' # Search interface in each netns - data = loads(cmd(f'ip netns exec {netns} ip --json link show')) + data = loads(cmdl(['ip', 'netns', 'exec', netns, 'ip', '--json', 'link', 'show'])) for tmp in data: if interface == tmp["ifname"]: return netns @@ -274,7 +274,7 @@ def is_wwan_connected(interface): modem = interface.lstrip('wwan') try: - tmp = cmd(f'mmcli --modem {modem} --output-json') + tmp = cmdl(['mmcli', '--modem', modem, '--output-json']) except OSError: return False @@ -287,12 +287,12 @@ def get_bridge_fdb(interface): """ Returns the forwarding database entries for a given interface """ if not interface_exists(interface): return None - tmp = loads(cmd(f'bridge -j fdb show dev {interface}')) + tmp = loads(cmdl(['bridge', '-j', 'fdb', 'show', 'dev', interface])) return tmp def get_all_vrfs(): """ Return a dictionary of all system wide known VRF instances """ - tmp = loads(cmd('ip --json vrf list')) + tmp = loads(cmdl(['ip', '--json', 'vrf', 'list'])) # Result is of type [{"name":"red","table":1000},{"name":"blue","table":2000}] # so we will re-arrange it to a more nicer representation: # {'red': {'table': 1000}, 'blue': {'table': 2000}} @@ -525,7 +525,7 @@ def is_wireguard_key_pair(private_key: str, public_key:str) -> bool: :return: If public/private keys are keypair returns True else False :rtype: bool """ - gen_public_key = cmd('wg pubkey', input=private_key) + gen_public_key = cmdl(['wg', 'pubkey'], input=private_key) if gen_public_key == public_key: return True else: @@ -541,7 +541,7 @@ def get_wireguard_peers(ifname: str) -> list: """ if not interface_exists(ifname): return [] - peers = cmd(f'wg show {ifname} peers') + peers = cmdl(['wg', 'show', ifname, 'peers']) return peers.splitlines() def is_subnet_connected(subnet, primary=False): @@ -628,7 +628,7 @@ def get_vxlan_vlan_tunnels(interface: str) -> list: # } ] # os_configured_vlan_ids = [] - tmp = loads(cmd(f'bridge --json vlan tunnelshow dev {interface}')) + tmp = loads(cmdl(['bridge', '--json', 'vlan', 'tunnelshow', 'dev', interface])) if tmp: for tunnel in tmp[0].get('tunnels', {}): vlanStart = tunnel['vlan'] @@ -658,7 +658,7 @@ def get_vxlan_vni_filter(interface: str) -> list: # # Example output: ['10010', '10020', '10021', '10022'] os_configured_vnis = [] - tmp = loads(cmd(f'bridge --json vni show dev {interface}')) + tmp = loads(cmdl(['bridge', '--json', 'vni', 'show', 'dev', interface])) if tmp: for tunnel in tmp[0].get('vnis', {}): vniStart = tunnel['vni'] @@ -721,7 +721,7 @@ def get_nft_vrf_zone_mapping() -> dict: from jmespath import search output = [] - tmp = loads(cmd('sudo nft -j list table inet vrf_zones')) + tmp = loads(cmdl(['nft', '-j', 'list', 'table', 'inet', 'vrf_zones'], sudo=True)) # {'nftables': [{'metainfo': {'json_schema_version': 1, # 'release_name': 'Old Doc Yak #3', # 'version': '1.0.9'}}, diff --git a/python/vyos/utils/process.py b/python/vyos/utils/process.py index 7b6f41a99..9d629f2c6 100644 --- a/python/vyos/utils/process.py +++ b/python/vyos/utils/process.py @@ -175,41 +175,6 @@ def run(command, flag='', shell=None, input=None, timeout=None, env=None, return code -def cmd(command, flag='', shell=None, input=None, timeout=None, env=None, - stdout=PIPE, stderr=PIPE, raising=None, message='', - expect=[0], vrf=None, netns=None): - """ - A wrapper around popen, which returns the stdout and - will raise the error code of a command - - raising: specify which call should be used when raising - the class should only require a string as parameter - (default is OSError) with the error code - expect: a list of error codes to consider as normal - """ - decoded, code = popen( - command, flag, - stdout=stdout, stderr=stderr, - input=input, timeout=timeout, - env=env, shell=shell, - decode='utf-8', - vrf=vrf, - netns=netns, - ) - if code not in expect: - wrapper = get_wrapper(vrf, netns) - command = f'{wrapper} {command}' - feedback = message + '\n' if message else '' - feedback += f'failed to run command: {command}\n' - feedback += f'returned: {decoded}\n' - feedback += f'exit code: {code}' - if raising is None: - # error code can be recovered with .errno - raise OSError(code, feedback) - else: - raise raising(feedback) - return decoded - def cmdl(command: list[str], flag: str = '', input: str | bytes | None = None, timeout: float | None = None, env: dict[str, str] | None = None, stdout: int = PIPE, stderr: int = PIPE, @@ -217,7 +182,8 @@ def cmdl(command: list[str], flag: str = '', input: str | bytes | None = None, expect: list[int] | None = None, vrf: str | None = None, netns: str | None = None, sudo: bool = False) -> str: """ - A list-argument variant of cmd() for safer subprocess execution. + A wrapper around popen() for safer subprocess execution, which returns + the stdout and will raise the error code of a command. command must be a list of strings; no shell interpolation is performed, which eliminates a class of command-injection risks present when building @@ -339,8 +305,8 @@ def is_systemd_service_active(service: str, vrf=None, netns=None) -> bool: """ Test is a specified systemd service is activated. Returns True if service is active, false otherwise. Copied from: https://unix.stackexchange.com/a/435317 """ - tmp = cmd(f'systemctl show --value -p ActiveState {service}', - vrf=vrf, netns=netns) + tmp = cmdl(['systemctl', 'show', '--value', '-p', 'ActiveState', service], + vrf=vrf, netns=netns) return bool((tmp == 'active')) def stop_systemd_unit(service: str, retries: int=3, delay_s: float=0.250, @@ -382,14 +348,14 @@ def is_systemd_service_running(service): """ Test is a specified systemd service is actually running. Returns True if service is running, false otherwise. Copied from: https://unix.stackexchange.com/a/435317 """ - tmp = cmd(f'systemctl show --value -p SubState {service}') + tmp = cmdl(['systemctl', 'show', '--value', '-p', 'SubState', service]) return bool((tmp == 'running')) def ip_cmd(args, json=True): """ A helper for easily calling iproute2 commands """ if json: from json import loads - res = cmd(f"ip --json {args}").strip() + res = cmdl(['ip', '--json'] + args.split()).strip() if res: return loads(res) else: @@ -397,7 +363,7 @@ def ip_cmd(args, json=True): # return an empty string return None else: - res = cmd(f"ip {args}") + res = cmdl(['ip'] + args.split()) return res diff --git a/python/vyos/utils/serial.py b/python/vyos/utils/serial.py index fcb7a21c9..9036e4cb0 100644 --- a/python/vyos/utils/serial.py +++ b/python/vyos/utils/serial.py @@ -20,7 +20,7 @@ from typing import List from vyos.base import Warning from vyos.utils.io import ask_yes_no -from vyos.utils.process import cmd +from vyos.utils.process import cmdl GLOB_GETTY_UNITS = 'serial-getty@*.service' RE_GETTY_DEVICES = re.compile(r'.+@(.+).service$') @@ -31,7 +31,7 @@ UTMP_PATH = '/run/utmp' def get_serial_units(include_devices=[]): # Since we cannot depend on the current config for decommissioned ports, # we just grab everything that systemd knows about. - tmp = cmd(f'systemctl list-units {GLOB_GETTY_UNITS} --all --output json --no-pager') + tmp = cmdl(['systemctl', 'list-units', GLOB_GETTY_UNITS, '--all', '--output', 'json', '--no-pager']) getty_units = json.loads(tmp) for sdunit in getty_units: m = RE_GETTY_DEVICES.search(sdunit['unit']) @@ -62,7 +62,7 @@ def get_authenticated_ports(units): # # We can safely skip blank or LOGIN sessions with valid device names. # - for line in cmd(f'utmpdump {UTMP_PATH}').splitlines(): + for line in cmdl(['utmpdump', UTMP_PATH]).splitlines(): row = line.split('] [') user_name = row[3].strip() user_term = row[4].strip() @@ -85,7 +85,7 @@ def restart_login_consoles(prompt_user=False, quiet=True, devices: List[str]=[]) # quiet intentionally does not suppress a vyos.base.Warning() for malformed # device names in _get_serial_units(). # - cmd('systemctl daemon-reload') + cmdl(['systemctl', 'daemon-reload']) units = get_serial_units(devices) connected = get_authenticated_ports(units) @@ -112,10 +112,10 @@ def restart_login_consoles(prompt_user=False, quiet=True, devices: List[str]=[]) unit_name = unit['unit'] unit_device = unit['device'] if os.path.exists(os.path.join(SD_UNIT_PATH, unit_name)): - cmd(f'systemctl restart {unit_name}') + cmdl(['systemctl', 'restart', unit_name]) else: # Deleted stubs don't need to be restarted, just shut them down. - cmd(f'systemctl stop {unit_name}') + cmdl(['systemctl', 'stop', unit_name]) return True diff --git a/python/vyos/utils/system.py b/python/vyos/utils/system.py index fd5f49645..fd32a486a 100644 --- a/python/vyos/utils/system.py +++ b/python/vyos/utils/system.py @@ -147,9 +147,9 @@ def get_load_averages(): return res def get_secure_boot_state() -> bool: - from vyos.utils.process import cmd + from vyos.utils.process import cmdl from vyos.utils.boot import is_uefi_system if not is_uefi_system(): return False - tmp = cmd('mokutil --sb-state', expect=[0, 255]) + tmp = cmdl(['mokutil', '--sb-state'], expect=[0, 255]) return bool('enabled' in tmp) diff --git a/python/vyos/vpp/config_resource_checks/memory.py b/python/vyos/vpp/config_resource_checks/memory.py index c8073f64b..855527ff1 100644 --- a/python/vyos/vpp/config_resource_checks/memory.py +++ b/python/vyos/vpp/config_resource_checks/memory.py @@ -20,7 +20,7 @@ import os import re import psutil -from vyos.utils.process import cmd +from vyos.utils.process import cmdl from vyos.vpp.utils import human_memory_to_bytes from vyos.vpp.config_resource_checks.resource_defaults import default_resource_map @@ -74,7 +74,8 @@ def get_vpp_used_memory() -> int: Returns memory currently used by VPP in bytes (RSS value) """ try: - out = cmd('ps -o rss= -p $(pidof vpp)') + pid = cmdl(['pidof', 'vpp']).strip() + out = cmdl(['ps', '-o', 'rss=', '-p', pid]) except OSError: out = 0 return int(out) << 10 @@ -84,7 +85,7 @@ def get_numa_count(): """ Run `numactl --hardware` and parse the 'available:' line. """ - out = cmd('numactl --hardware') + out = cmdl(['numactl', '--hardware']) # e.g. "available: 2 nodes (0-1)" m = re.search(r'available:\s*(\d+)\s+nodes', out) return int(m.group(1)) if m else 0 |