From 7928b0d755714418fe1837748eb73a44adf4c1fd Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez <ralonsoh@redhat.com> Date: Fri, 22 Jan 2021 16:55:00 +0000 Subject: [PATCH] Remove rootwrap execution (2) Replace rootwrap execution with privsep context execution. This series of patches will progressively replace any rootwrap call. Change-Id: Id3db4fbba44dd5644563481b6767ad0acbdcfb3e Story: #2007686 Task: #41558 --- neutron/debug/debug_agent.py | 2 ++ neutron/tests/common/net_helpers.py | 15 +++++++++------ neutron/tests/functional/agent/l3/framework.py | 10 ++++++---- .../functional/agent/l3/test_legacy_router.py | 2 +- .../functional/agent/linux/test_keepalived.py | 3 ++- neutron/tests/functional/agent/test_dhcp_agent.py | 3 ++- neutron/tests/functional/agent/test_ovs_flows.py | 5 +++-- .../functional/cmd/test_linuxbridge_cleanup.py | 8 +++++++- .../tests/functional/cmd/test_netns_cleanup.py | 4 ++-- .../privileged/agent/linux/test_ip_lib.py | 3 +++ 10 files changed, 37 insertions(+), 18 deletions(-) diff --git a/neutron/debug/debug_agent.py b/neutron/debug/debug_agent.py index 3e377f1daca..5fbc6d2f2f6 100644 --- a/neutron/debug/debug_agent.py +++ b/neutron/debug/debug_agent.py @@ -120,6 +120,8 @@ class NeutronDebugAgent(object): if not command: return "sudo ip netns exec %s" % self._get_namespace(port) namespace = ip.ensure_namespace(namespace) + # NOTE(ralonsoh): this is going to be called from inside the + # "neutron-debug" shell command; privsep is not configured. return namespace.netns.execute(shlex.split(command)) def ensure_probe(self, network_id): diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 4aca5ad4efb..26b17addcb1 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -111,8 +111,9 @@ def assert_ping(src_namespace, dst_ip, timeout=1, count=3): ipversion = netaddr.IPAddress(dst_ip).version ping_command = 'ping' if ipversion == 4 else 'ping6' ns_ip_wrapper = ip_lib.IPWrapper(src_namespace) - ns_ip_wrapper.netns.execute([ping_command, '-W', timeout, '-c', count, - dst_ip]) + ns_ip_wrapper.netns.execute( + [ping_command, '-W', timeout, '-c', count, dst_ip], + privsep_exec=True) def assert_async_ping(src_namespace, dst_ip, timeout=1, count=1, interval=1): @@ -124,8 +125,9 @@ def assert_async_ping(src_namespace, dst_ip, timeout=1, count=1, interval=1): # cannot be used and it needs to be done using the following workaround. for _index in range(count): start_time = time.time() - ns_ip_wrapper.netns.execute([ping_command, '-W', timeout, '-c', '1', - dst_ip]) + ns_ip_wrapper.netns.execute( + [ping_command, '-W', timeout, '-c', '1', dst_ip], + privsep_exec=True) end_time = time.time() diff = end_time - start_time if 0 < diff < interval: @@ -167,7 +169,7 @@ def assert_arping(src_namespace, dst_ip, source=None, timeout=1, count=1): if source: arping_cmd.extend(['-s', source]) arping_cmd.append(dst_ip) - ns_ip_wrapper.netns.execute(arping_cmd) + ns_ip_wrapper.netns.execute(arping_cmd, privsep_exec=True) def assert_no_arping(src_namespace, dst_ip, source=None, timeout=1, count=1): @@ -223,7 +225,8 @@ def get_free_namespace_port(protocol, namespace=None, start=1024, end=None): raise ValueError("Unsupported protocol %s" % protocol) ip_wrapper = ip_lib.IPWrapper(namespace=namespace) - output = ip_wrapper.netns.execute(['ss', param], run_as_root=True) + output = ip_wrapper.netns.execute(['ss', param], run_as_root=True, + privsep_exec=True) used_ports = _get_source_ports_from_ss_output(output) return get_unused_port(used_ports, start, end) diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index edacfb5157b..10d490e8c52 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -199,8 +199,9 @@ class L3AgentTestFramework(base.BaseSudoTestCase): self.addCleanup(netcat.stop_processes) def assert_num_of_conntrack_rules(n): - out = router_ns.netns.execute(["conntrack", "-L", - "--orig-src", client_address]) + out = router_ns.netns.execute( + ["conntrack", "-L", "--orig-src", client_address], + privsep_exec=True) self.assertEqual( n, len([line for line in out.strip().split('\n') if line])) @@ -274,8 +275,9 @@ class L3AgentTestFramework(base.BaseSudoTestCase): ip_wrapper = ip_lib.IPWrapper(namespace=ns_name) def _ipv6_accept_ra_state(): - ra_state = ip_wrapper.netns.execute(['sysctl', '-b', - 'net.ipv6.conf.%s.accept_ra' % device_name]) + ra_state = ip_wrapper.netns.execute( + ['sysctl', '-b', 'net.ipv6.conf.%s.accept_ra' % device_name], + privsep_exec=True) return ( enabled == (int(ra_state) != constants.ACCEPT_RA_DISABLED)) diff --git a/neutron/tests/functional/agent/l3/test_legacy_router.py b/neutron/tests/functional/agent/l3/test_legacy_router.py index e27bbeaee03..00f34102c2f 100644 --- a/neutron/tests/functional/agent/l3/test_legacy_router.py +++ b/neutron/tests/functional/agent/l3/test_legacy_router.py @@ -320,7 +320,7 @@ class L3AgentTestCase(framework.L3AgentTestFramework): # Verify that the ping replys with fip ns_ip_wrapper = ip_lib.IPWrapper(src_machine.namespace) result = ns_ip_wrapper.netns.execute( - ['ping', '-W', 5, '-c', 1, dst_fip]) + ['ping', '-W', 5, '-c', 1, dst_fip], privsep_exec=True) self._assert_ping_reply_from_expected_address(result, dst_fip) def _setup_address_scope(self, internal_address_scope1, diff --git a/neutron/tests/functional/agent/linux/test_keepalived.py b/neutron/tests/functional/agent/linux/test_keepalived.py index 1451b021710..4145a896989 100644 --- a/neutron/tests/functional/agent/linux/test_keepalived.py +++ b/neutron/tests/functional/agent/linux/test_keepalived.py @@ -92,7 +92,8 @@ class KeepalivedManagerTestCase(base.BaseSudoTestCase, # Exit the process, and see that when it comes back # It's indeed a different process - self.ip_wrapper.netns.execute(['kill', exit_code, pid]) + self.ip_wrapper.netns.execute(['kill', exit_code, pid], + privsep_exec=True) common_utils.wait_until_true( lambda: process.active and pid != process.pid, timeout=5, diff --git a/neutron/tests/functional/agent/test_dhcp_agent.py b/neutron/tests/functional/agent/test_dhcp_agent.py index 2635a33257b..0eef55d62a3 100644 --- a/neutron/tests/functional/agent/test_dhcp_agent.py +++ b/neutron/tests/functional/agent/test_dhcp_agent.py @@ -178,7 +178,8 @@ class DHCPAgentOVSTestFramework(base.BaseSudoTestCase): def assert_accept_ra_disabled(self, namespace): actual = ip_lib.IPWrapper(namespace=namespace).netns.execute( - ['sysctl', '-b', 'net.ipv6.conf.default.accept_ra']) + ['sysctl', '-b', 'net.ipv6.conf.default.accept_ra'], + privsep_exec=True) self.assertEqual('0', actual) def assert_dhcp_device(self, namespace, dhcp_iface_name, dhcp_enabled): diff --git a/neutron/tests/functional/agent/test_ovs_flows.py b/neutron/tests/functional/agent/test_ovs_flows.py index c1fb16c7ead..d779dbbb244 100644 --- a/neutron/tests/functional/agent/test_ovs_flows.py +++ b/neutron/tests/functional/agent/test_ovs_flows.py @@ -175,8 +175,9 @@ class ARPSpoofTestCase(OVSAgentTestBase): self.dst_p.addr.add('%s/24' % self.dst_addr) ns_ip_wrapper = ip_lib.IPWrapper(self.src_namespace) try: - ns_ip_wrapper.netns.execute(['arping', '-I', self.src_p.name, - '-c1', self.dst_addr]) + ns_ip_wrapper.netns.execute( + ['arping', '-I', self.src_p.name, '-c1', self.dst_addr], + privsep_exec=True) tools.fail("arping should have failed. The arp request should " "have been blocked.") except RuntimeError: diff --git a/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py b/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py index c9ebeddd74f..63b97268d19 100644 --- a/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py +++ b/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import os from unittest import mock import fixtures @@ -40,10 +41,15 @@ class LinuxbridgeCleanupTest(base.BaseSudoTestCase): # NOTE(slaweq): use of oslo.privsep inside neutron-linuxbridge-cleanup # script requires rootwrap helper to be configured in this script's # config + privsep_helper = os.path.join( + os.getenv('VIRTUAL_ENV'), 'bin', 'privsep-helper') config.update({ 'AGENT': { 'root_helper': tests_base.get_rootwrap_cmd(), 'root_helper_daemon': tests_base.get_rootwrap_daemon_cmd() + }, + 'privsep': { + 'helper_command': ' '.join(['sudo', '-E', privsep_helper]), } }) @@ -57,7 +63,7 @@ class LinuxbridgeCleanupTest(base.BaseSudoTestCase): cmd = 'neutron-linuxbridge-cleanup', '--config-file', conf.filename ip_wrapper = ip_lib.IPWrapper(br_fixture.namespace) - ip_wrapper.netns.execute(cmd) + ip_wrapper.netns.execute(cmd, privsep_exec=True) self.assertEqual(bridge_exists, ip_lib.device_exists( br_fixture.bridge.name, br_fixture.namespace)) diff --git a/neutron/tests/functional/cmd/test_netns_cleanup.py b/neutron/tests/functional/cmd/test_netns_cleanup.py index a7019ecd66d..af3d453a8e4 100644 --- a/neutron/tests/functional/cmd/test_netns_cleanup.py +++ b/neutron/tests/functional/cmd/test_netns_cleanup.py @@ -144,8 +144,8 @@ class NetnsCleanupTest(base.BaseSudoTestCase): # Otherwise, it won't find the necessary packages such as # oslo_config ip.netns.execute(command, - addl_env={'PATH': - os.environ.get('PATH')}) + addl_env={'PATH': os.environ.get('PATH')}, + privsep_exec=True) return proc_count @staticmethod diff --git a/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py b/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py index d1bd44f21a9..1f5b16af0eb 100644 --- a/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py @@ -638,6 +638,9 @@ class ListNamespacePids(functional_base.BaseSudoTestCase): @staticmethod def _run_sleep(namespace, timeout): ip_wrapper = ip_lib.IPWrapper(namespace=namespace) + # NOTE(ralonsoh): this is a "long" (more than one second) lived + # process. It should not be executed in a privsep context to avoid + # a possible privsep thread starvation. ip_wrapper.netns.execute(['sleep', timeout], check_exit_code=False) def _check_pids(self, num_pids, namespace=None):