Remove rootwrap execution (1)

Replace rootwrap execution with privsep context execution.
This series of patches will progressively replace any
rootwrap call.

This patch replaces some "IpNetnsCommand" command execution
methods.

Change-Id: Ic5fdf221a2a2cd0951539b0e040d2a941feee287
Story: #2007686
Task: #41558

neutron/agent/l3/dvr_fip_ns.py

@@ -192,7 +192,7 @@ class FipNamespace(namespaces.Namespace):
         self.agent_gateway_port = ex_gw_port
 
         cmd = ['sysctl', '-w', 'net.ipv4.conf.%s.proxy_arp=1' % interface_name]
-        ip_wrapper.netns.execute(cmd, check_exit_code=False)
+        ip_wrapper.netns.execute(cmd, check_exit_code=False, privsep_exec=True)
 
     def create(self):
         LOG.debug("DVR: add fip namespace: %s", self.name)

neutron/agent/l3/dvr_local_router.py

@@ -804,7 +804,8 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
                'via', route['nexthop'], 'table', tbl_index]
         ip_wrapper = ip_lib.IPWrapper(namespace=fip_ns_name)
         if ip_wrapper.netns.exists(fip_ns_name):
-            ip_wrapper.netns.execute(cmd, check_exit_code=False)
+            ip_wrapper.netns.execute(cmd, check_exit_code=False,
+                                     privsep_exec=True)
         else:
             LOG.debug("The FIP namespace %(ns)s does not exist for "
                       "router %(id)s",

neutron/agent/l3/namespaces.py

@@ -94,18 +94,18 @@ class Namespace(object):
         # these sysctl values.
         ip_wrapper = self.ip_wrapper_root.ensure_namespace(self.name)
         cmd = ['sysctl', '-w', 'net.ipv4.ip_forward=1']
-        ip_wrapper.netns.execute(cmd)
+        ip_wrapper.netns.execute(cmd, privsep_exec=True)
         # 1. Reply only if the target IP address is local address configured
         #    on the incoming interface; and
         # 2. Always use the best local address
         cmd = ['sysctl', '-w', 'net.ipv4.conf.all.arp_ignore=1']
-        ip_wrapper.netns.execute(cmd)
+        ip_wrapper.netns.execute(cmd, privsep_exec=True)
         cmd = ['sysctl', '-w', 'net.ipv4.conf.all.arp_announce=2']
-        ip_wrapper.netns.execute(cmd)
+        ip_wrapper.netns.execute(cmd, privsep_exec=True)
         if self.use_ipv6:
             cmd = ['sysctl', '-w',
                    'net.ipv6.conf.all.forwarding=%d' % int(ipv6_forwarding)]
-            ip_wrapper.netns.execute(cmd)
+            ip_wrapper.netns.execute(cmd, privsep_exec=True)
 
     def delete(self):
         try:

neutron/agent/linux/external_process.py

@@ -98,7 +98,7 @@ class ProcessManager(MonitoredProcess):
         else:
             self.disable('HUP')
 
-    def disable(self, sig='9', get_stop_command=None):
+    def disable(self, sig='9', get_stop_command=None, privsep_exec=False):
         pid = self.pid
 
         if self.active:
@@ -106,10 +106,12 @@ class ProcessManager(MonitoredProcess):
                 cmd = get_stop_command(self.get_pid_file_name())
                 ip_wrapper = ip_lib.IPWrapper(namespace=self.namespace)
                 ip_wrapper.netns.execute(cmd, addl_env=self.cmd_addl_env,
-                                         run_as_root=self.run_as_root)
+                                         run_as_root=self.run_as_root,
+                                         privsep_exec=privsep_exec)
             else:
                 cmd = self.get_kill_cmd(sig, pid)
-                utils.execute(cmd, run_as_root=self.run_as_root)
+                utils.execute(cmd, run_as_root=self.run_as_root,
+                              privsep_exec=privsep_exec)
                 # In the case of shutting down, remove the pid file
                 if sig == '9':
                     utils.delete_if_exists(self.get_pid_file_name(),

neutron/agent/linux/ip_lib.py

@@ -354,7 +354,7 @@ class IPDevice(SubProcessBase):
         try:
             ip_wrapper.netns.execute(["conntrack", "-D", "-d", ip_str],
                                      check_exit_code=True,
-                                     extra_ok_codes=[1])
+                                     extra_ok_codes=[1], privsep_exec=True)
 
         except RuntimeError:
             LOG.exception("Failed deleting ingress connection state of"
@@ -364,7 +364,7 @@ class IPDevice(SubProcessBase):
         try:
             ip_wrapper.netns.execute(["conntrack", "-D", "-q", ip_str],
                                      check_exit_code=True,
-                                     extra_ok_codes=[1])
+                                     extra_ok_codes=[1], privsep_exec=True)
         except RuntimeError:
             LOG.exception("Failed deleting egress connection state of"
                           " floatingip %s", ip_str)
@@ -376,7 +376,7 @@ class IPDevice(SubProcessBase):
                '--dport', dport]
         try:
             ip_wrapper.netns.execute(cmd, check_exit_code=True,
-                                     extra_ok_codes=[1])
+                                     extra_ok_codes=[1], privsep_exec=True)
 
         except RuntimeError:
             LOG.exception("Failed deleting ingress connection state of "
@@ -699,7 +699,8 @@ class IpNetnsCommand(IpCommandBase):
         create_network_namespace(name)
         wrapper = IPWrapper(namespace=name)
         wrapper.netns.execute(['sysctl', '-w',
-                               'net.ipv4.conf.all.promote_secondaries=1'])
+                               'net.ipv4.conf.all.promote_secondaries=1'],
+                              privsep_exec=True)
         return wrapper
 
     def delete(self, name):
@@ -707,7 +708,7 @@ class IpNetnsCommand(IpCommandBase):
 
     def execute(self, cmds, addl_env=None, check_exit_code=True,
                 log_fail_as_error=True, extra_ok_codes=None,
-                run_as_root=False):
+                run_as_root=False, privsep_exec=False):
         ns_params = []
         if self._parent.namespace:
             run_as_root = True
@@ -721,7 +722,8 @@ class IpNetnsCommand(IpCommandBase):
         return utils.execute(cmd, check_exit_code=check_exit_code,
                              extra_ok_codes=extra_ok_codes,
                              log_fail_as_error=log_fail_as_error,
-                             run_as_root=run_as_root)
+                             run_as_root=run_as_root,
+                             privsep_exec=privsep_exec)
 
     def exists(self, name):
         return network_namespace_exists(name)
@@ -1014,7 +1016,8 @@ def _arping(ns_name, iface_name, address, count, log_exception):
                           '-w', 2, address]
             try:
                 ip_wrapper.netns.execute(arping_cmd,
-                                         extra_ok_codes=extra_ok_codes)
+                                         extra_ok_codes=extra_ok_codes,
+                                         privsep_exec=True)
             except Exception as exc:
                 # Since this is spawned in a thread and executed 2 seconds
                 # apart, something may have been deleted while we were
@@ -1101,7 +1104,8 @@ def sysctl(cmd, namespace=None, log_fail_as_error=True):
     ip_wrapper = IPWrapper(namespace=namespace)
     try:
         ip_wrapper.netns.execute(cmd, run_as_root=True,
-                                 log_fail_as_error=log_fail_as_error)
+                                 log_fail_as_error=log_fail_as_error,
+                                 privsep_exec=True)
     except RuntimeError as rte:
         LOG.warning(
             "Setting %(cmd)s in namespace %(ns)s failed: %(err)s.",
@@ -1127,7 +1131,8 @@ def get_ip_nonlocal_bind(namespace=None):
     """Get kernel option value of ip_nonlocal_bind in given namespace."""
     cmd = ['sysctl', '-bn', IP_NONLOCAL_BIND]
     ip_wrapper = IPWrapper(namespace)
-    return int(ip_wrapper.netns.execute(cmd, run_as_root=True))
+    return int(ip_wrapper.netns.execute(cmd, run_as_root=True,
+                                        privsep_exec=True))
 
 
 def set_ip_nonlocal_bind(value, namespace=None, log_fail_as_error=True):
@@ -1162,7 +1167,8 @@ def get_ipv6_forwarding(device, namespace=None):
     """Get kernel value of IPv6 forwarding for device in given namespace."""
     cmd = ['sysctl', '-b', "net.ipv6.conf.%s.forwarding" % device]
     ip_wrapper = IPWrapper(namespace)
-    return int(ip_wrapper.netns.execute(cmd, run_as_root=True))
+    return int(ip_wrapper.netns.execute(cmd, run_as_root=True,
+                                        privsep_exec=True))
 
 
 def _parse_ip_rule(rule, ip_version):

neutron/agent/linux/l3_tc_lib.py

@@ -33,7 +33,8 @@ class FloatingIPTcCommandBase(ip_lib.IPDevice):
     def _execute_tc_cmd(self, cmd, **kwargs):
         cmd = ['tc'] + cmd
         ip_wrapper = ip_lib.IPWrapper(self.namespace)
-        return ip_wrapper.netns.execute(cmd, run_as_root=True, **kwargs)
+        return ip_wrapper.netns.execute(cmd, run_as_root=True,
+                                        privsep_exec=True, **kwargs)
 
     def _get_qdisc_id_for_filter(self, direction):
         qdiscs = tc_lib.list_tc_qdiscs(self.name, namespace=self.namespace)

neutron/agent/linux/utils.py

@@ -35,7 +35,6 @@ from oslo_utils import excutils
 from oslo_utils import fileutils
 import psutil
 
-from neutron.common import utils
 from neutron.conf.agent import common as config
 from neutron.privileged.agent.linux import utils as priv_utils
 from neutron import wsgi
@@ -79,16 +78,26 @@ def create_process(cmd, run_as_root=False, addl_env=None):
     """
     cmd = list(map(str, addl_env_args(addl_env) + cmd))
     if run_as_root:
+        # NOTE(ralonsoh): to be removed once the migration to privsep
+        # execution is done.
         cmd = shlex.split(config.get_root_helper(cfg.CONF)) + cmd
     LOG.debug("Running command: %s", cmd)
-    obj = utils.subprocess_popen(cmd, shell=False,
+    obj = subprocess.Popen(cmd, shell=False, stdin=subprocess.PIPE,
-                                 stdin=subprocess.PIPE,
+                           stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-                                 stdout=subprocess.PIPE,
-                                 stderr=subprocess.PIPE)
 
     return obj, cmd
 
 
+def _execute_process(cmd, _process_input, addl_env, run_as_root):
+    obj, cmd = create_process(cmd, run_as_root=run_as_root, addl_env=addl_env)
+    _stdout, _stderr = obj.communicate(_process_input)
+    returncode = obj.returncode
+    obj.stdin.close()
+    _stdout = helpers.safe_decode_utf8(_stdout)
+    _stderr = helpers.safe_decode_utf8(_stderr)
+    return _stdout, _stderr, returncode
+
+
 def execute_rootwrap_daemon(cmd, process_input, addl_env):
     cmd = list(map(str, addl_env_args(addl_env) + cmd))
     # NOTE(twilson) oslo_rootwrap.daemon will raise on filter match
@@ -99,31 +108,34 @@ def execute_rootwrap_daemon(cmd, process_input, addl_env):
     LOG.debug("Running command (rootwrap daemon): %s", cmd)
     client = RootwrapDaemonHelper.get_client()
     try:
-        return client.execute(cmd, process_input)
+        returncode, _stdout, _stderr = client.execute(cmd, process_input)
     except Exception:
         with excutils.save_and_reraise_exception():
             LOG.error("Rootwrap error running command: %s", cmd)
 
+    _stdout = helpers.safe_decode_utf8(_stdout)
+    _stderr = helpers.safe_decode_utf8(_stderr)
+    return _stdout, _stderr, returncode
+
 
 def execute(cmd, process_input=None, addl_env=None,
             check_exit_code=True, return_stderr=False, log_fail_as_error=True,
-            extra_ok_codes=None, run_as_root=False):
+            extra_ok_codes=None, run_as_root=False, privsep_exec=False):
     try:
         if process_input is not None:
             _process_input = encodeutils.to_utf8(process_input)
         else:
             _process_input = None
-        if run_as_root and cfg.CONF.AGENT.root_helper_daemon:
+
-            returncode, _stdout, _stderr = (
+        if run_as_root and privsep_exec:
-                execute_rootwrap_daemon(cmd, process_input, addl_env))
+            _stdout, _stderr, returncode = priv_utils.execute_process(
+                cmd, _process_input, addl_env)
+        elif run_as_root and cfg.CONF.AGENT.root_helper_daemon:
+            _stdout, _stderr, returncode = execute_rootwrap_daemon(
+                cmd, process_input, addl_env)
         else:
-            obj, cmd = create_process(cmd, run_as_root=run_as_root,
+            _stdout, _stderr, returncode = _execute_process(
-                                      addl_env=addl_env)
+                cmd, _process_input, addl_env, run_as_root)
-            _stdout, _stderr = obj.communicate(_process_input)
-            returncode = obj.returncode
-            obj.stdin.close()
-        _stdout = helpers.safe_decode_utf8(_stdout)
-        _stderr = helpers.safe_decode_utf8(_stderr)
 
         extra_ok_codes = extra_ok_codes or []
         if returncode and returncode not in extra_ok_codes:
@@ -202,10 +214,11 @@ def find_fork_top_parent(pid):
             return pid
 
 
-def kill_process(pid, signal, run_as_root=False):
+def kill_process(pid, signal, run_as_root=False, privsep_exec=False):
     """Kill the process with the given pid using the given signal."""
     try:
-        execute(['kill', '-%d' % signal, pid], run_as_root=run_as_root)
+        execute(['kill', '-%d' % signal, pid], run_as_root=run_as_root,
+                privsep_exec=privsep_exec)
     except exceptions.ProcessExecutionError:
         if process_is_running(pid):
             raise

neutron/agent/windows/utils.py

@@ -100,7 +100,7 @@ def kill_process(pid, signal, run_as_root=False):
 
 def execute(cmd, process_input=None, addl_env=None,
             check_exit_code=True, return_stderr=False, log_fail_as_error=True,
-            extra_ok_codes=None, run_as_root=False):
+            extra_ok_codes=None, run_as_root=False, privsep_exec=False):
 
     if process_input is not None:
         _process_input = encodeutils.to_utf8(process_input)

neutron/privileged/agent/linux/utils.py

@@ -15,6 +15,8 @@
 import os
 import re
 
+from eventlet.green import subprocess
+from neutron_lib.utils import helpers
 from oslo_concurrency import processutils
 from oslo_utils import fileutils
 
@@ -47,3 +49,36 @@ def _find_listen_pids_namespace(namespace):
 @privileged.default.entrypoint
 def delete_if_exists(path, remove=os.unlink):
     fileutils.delete_if_exists(path, remove=remove)
+
+
+@privileged.default.entrypoint
+def execute_process(cmd, _process_input, addl_env):
+    obj, cmd = _create_process(cmd, addl_env=addl_env)
+    _stdout, _stderr = obj.communicate(_process_input)
+    returncode = obj.returncode
+    obj.stdin.close()
+    _stdout = helpers.safe_decode_utf8(_stdout)
+    _stderr = helpers.safe_decode_utf8(_stderr)
+    return _stdout, _stderr, returncode
+
+
+def _addl_env_args(addl_env):
+    """Build arguments for adding additional environment vars with env"""
+
+    # NOTE (twilson) If using rootwrap, an EnvFilter should be set up for the
+    # command instead of a CommandFilter.
+    if addl_env is None:
+        return []
+    return ['env'] + ['%s=%s' % pair for pair in addl_env.items()]
+
+
+def _create_process(cmd, addl_env=None):
+    """Create a process object for the given command.
+
+    The return value will be a tuple of the process object and the
+    list of command arguments used to create it.
+    """
+    cmd = list(map(str, _addl_env_args(addl_env) + list(cmd)))
+    obj = subprocess.Popen(cmd, shell=False, stdin=subprocess.PIPE,
+                           stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    return obj, cmd

neutron/tests/common/net_helpers.py

@@ -620,7 +620,8 @@ class NamespaceFixture(fixtures.Fixture):
                 if self.ip_wrapper.netns.exists(self.name):
                     for pid in ip_lib.list_namespace_pids(self.name):
                         utils.kill_process(pid, signal.SIGKILL,
-                                           run_as_root=True)
+                                           run_as_root=True,
+                                           privsep_exec=True)
                     self.ip_wrapper.netns.delete(self.name)
             except helpers.TestTimerTimeout:
                 LOG.warning('Namespace %s was not deleted due to a timeout.',

neutron/tests/functional/agent/linux/test_tc_lib.py

@@ -134,8 +134,8 @@ class TcPolicyClassTestCase(functional_base.BaseSudoTestCase):
 
         bytes = tc_classes[0]['stats']['bytes']
         packets = tc_classes[0]['stats']['packets']
-        net_helpers.assert_ping(self.ns[1], netaddr.IPNetwork(self.ip[0]).ip,
+        net_helpers.assert_ping(
-                                count=1)
+            self.ns[1], str(netaddr.IPNetwork(self.ip[0]).ip), count=1)
         tc_classes = tc_lib.list_tc_policy_class(self.device[0],
                                                  namespace=self.ns[0])
         self.assertGreater(tc_classes[0]['stats']['bytes'], bytes)
@@ -268,7 +268,7 @@ class TcFiltersTestCase(functional_base.BaseSudoTestCase):
                                       'device': self.device[0]})
 
         net_helpers.assert_ping(
-            self.ns[1], netaddr.IPNetwork(self.ip_vxlan[0]).ip, count=1)
+            self.ns[1], str(netaddr.IPNetwork(self.ip_vxlan[0]).ip), count=1)
         tc_classes = tc_lib.list_tc_policy_class(self.device[0],
                                                  namespace=self.ns[0])
         for tc_class in tc_classes:

neutron/tests/unit/agent/l3/test_agent.py

@@ -2595,13 +2595,17 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             'qrouter-bar', self.conf, agent.driver, agent.use_ipv6)
         ns.create()
 
-        calls = [mock.call(['sysctl', '-w', 'net.ipv4.ip_forward=1']),
+        calls = [mock.call(['sysctl', '-w', 'net.ipv4.ip_forward=1'],
-                 mock.call(['sysctl', '-w', 'net.ipv4.conf.all.arp_ignore=1']),
+                           privsep_exec=True),
+                 mock.call(['sysctl', '-w', 'net.ipv4.conf.all.arp_ignore=1'],
+                           privsep_exec=True),
                  mock.call(
-                     ['sysctl', '-w', 'net.ipv4.conf.all.arp_announce=2'])]
+                     ['sysctl', '-w', 'net.ipv4.conf.all.arp_announce=2'],
+                     privsep_exec=True)]
         if agent.use_ipv6:
             calls.append(mock.call(
-                ['sysctl', '-w', 'net.ipv6.conf.all.forwarding=1']))
+                ['sysctl', '-w', 'net.ipv6.conf.all.forwarding=1'],
+                privsep_exec=True))
 
         self.mock_ip.netns.execute.assert_has_calls(calls)
 

neutron/tests/unit/agent/l3/test_dvr_fip_ns.py

@@ -217,13 +217,14 @@ class TestDvrFipNs(base.BaseTestCase):
         bind_cmd = ['sysctl', '-w', 'net.ipv4.ip_nonlocal_bind=1']
         expected = [mock.call(netns_cmd + bind_cmd, check_exit_code=True,
                               extra_ok_codes=None, log_fail_as_error=False,
-                              run_as_root=True)]
+                              run_as_root=True, privsep_exec=True)]
 
         if old_kernel:
             expected.append(mock.call(bind_cmd, check_exit_code=True,
                                       extra_ok_codes=None,
                                       log_fail_as_error=True,
-                                      run_as_root=True))
+                                      run_as_root=True,
+                                      privsep_exec=True))
 
         execute.assert_has_calls(expected)
 

neutron/tests/unit/agent/l3/test_dvr_snat_ns.py

@@ -50,7 +50,8 @@ class TestDvrSnatNs(base.BaseTestCase):
         loose_cmd = ['sysctl', '-w', 'net.netfilter.nf_conntrack_tcp_loose=0']
         expected = [mock.call(netns_cmd + loose_cmd,
                               check_exit_code=True, extra_ok_codes=None,
-                              log_fail_as_error=True, run_as_root=True)]
+                              log_fail_as_error=True, run_as_root=True,
+                              privsep_exec=True)]
 
         create.assert_called_once_with(self.snat_ns.name)
         execute.assert_has_calls(expected)

neutron/tests/unit/agent/linux/openvswitch_firewall/test_iptables.py

@@ -110,8 +110,10 @@ class TestHybridIptablesHelper(base.BaseTestCase):
     def test_overloaded_remove_conntrack(self):
         with mock.patch.object(iptables_firewall.IptablesFirewallDriver,
                 '_remove_conntrack_entries_from_port_deleted') as rcefpd, \
-             mock.patch("neutron.agent.linux.ip_conntrack.IpConntrackManager"
+                mock.patch("neutron.agent.linux.ip_conntrack."
-                        "._populate_initial_zone_map"):
+                           "IpConntrackManager._populate_initial_zone_map"), \
+                mock.patch.object(iptables_firewall.IptablesFirewallDriver,
+                                  '_check_netfilter_for_bridges'):
             firewall = iptables.get_iptables_driver_instance()
             firewall._remove_conntrack_entries_from_port_deleted(None)
             rcefpd.assert_not_called()

neutron/tests/unit/agent/linux/test_external_process.py

@@ -138,7 +138,8 @@ class TestProcessManager(base.BaseTestCase):
                                                      check_exit_code=True,
                                                      extra_ok_codes=None,
                                                      run_as_root=False,
-                                                     log_fail_as_error=True)
+                                                     log_fail_as_error=True,
+                                                     privsep_exec=False)
 
     def test_enable_with_namespace(self):
         callback = mock.Mock()
@@ -228,7 +229,8 @@ class TestProcessManager(base.BaseTestCase):
                     manager.disable()
                     utils.assert_has_calls([
                         mock.call.execute(['kill', '-9', 4],
-                                          run_as_root=False)])
+                                          run_as_root=False,
+                                          privsep_exec=False)])
 
     def test_disable_namespace(self):
         with mock.patch.object(ep.ProcessManager, 'pid') as pid:
@@ -242,7 +244,8 @@ class TestProcessManager(base.BaseTestCase):
                     manager.disable()
                     utils.assert_has_calls([
                         mock.call.execute(['kill', '-9', 4],
-                                          run_as_root=True)])
+                                          run_as_root=True,
+                                          privsep_exec=False)])
 
     def test_disable_not_active(self):
         with mock.patch.object(ep.ProcessManager, 'pid') as pid:
@@ -284,7 +287,8 @@ class TestProcessManager(base.BaseTestCase):
                                           return_value=kill_script_exists):
                     manager.disable()
                     utils.execute.assert_called_with(
-                        expected_cmd, run_as_root=bool(namespace))
+                        expected_cmd, run_as_root=bool(namespace),
+                        privsep_exec=False)
 
     def test_disable_custom_kill_script_no_namespace(self):
         self._test_disable_custom_kill_script(

neutron/tests/unit/agent/linux/test_ip_lib.py

@@ -858,7 +858,7 @@ class TestIpNetnsCommand(TestIPCmdBase):
                 ['ip', 'netns', 'exec', 'ns',
                  'sysctl', '-w', 'net.ipv4.conf.all.promote_secondaries=1'],
                 run_as_root=True, check_exit_code=True, extra_ok_codes=None,
-                log_fail_as_error=True)
+                log_fail_as_error=True, privsep_exec=True)
 
     @mock.patch.object(priv_lib, 'remove_netns')
     def test_delete_namespace(self, remove):
@@ -899,7 +899,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
                                             run_as_root=True,
                                             check_exit_code=True,
                                             extra_ok_codes=None,
-                                            log_fail_as_error=True)
+                                            log_fail_as_error=True,
+                                            privsep_exec=False)
 
     def test_execute_env_var_prepend(self):
         self.parent.namespace = 'ns'
@@ -911,7 +912,7 @@ class TestIpNetnsCommand(TestIPCmdBase):
                 ['%s=%s' % (k, v) for k, v in env.items()] +
                 ['ip', 'link', 'list'],
                 run_as_root=True, check_exit_code=True, extra_ok_codes=None,
-                log_fail_as_error=True)
+                log_fail_as_error=True, privsep_exec=False)
 
     def test_execute_nosudo_with_no_namespace(self):
         with mock.patch('neutron.agent.common.utils.execute') as execute:
@@ -921,7 +922,8 @@ class TestIpNetnsCommand(TestIPCmdBase):
                                             check_exit_code=True,
                                             extra_ok_codes=None,
                                             run_as_root=False,
-                                            log_fail_as_error=True)
+                                            log_fail_as_error=True,
+                                            privsep_exec=False)
 
 
 class TestDeviceExists(base.BaseTestCase):
@@ -1378,12 +1380,18 @@ class TestArpPing(TestIPCmdBase):
         self.assertTrue(spawn_n.called)
         mIPWrapper.assert_has_calls([
             mock.call(namespace=mock.sentinel.ns_name),
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1]),
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1],
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1]),
+                                      privsep_exec=True),
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2]),
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1],
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2]),
+                                      privsep_exec=True),
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2]),
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2],
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2])])
+                                      privsep_exec=True),
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2],
+                                      privsep_exec=True),
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2],
+                                      privsep_exec=True),
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1, 2],
+                                      privsep_exec=True)])
 
         ip_wrapper = mIPWrapper(namespace=mock.sentinel.ns_name)
 
@@ -1395,7 +1403,8 @@ class TestArpPing(TestIPCmdBase):
                           '-w', mock.ANY,
                           address]
             ip_wrapper.netns.execute.assert_any_call(arping_cmd,
-                                                     extra_ok_codes=mock.ANY)
+                                                     extra_ok_codes=mock.ANY,
+                                                     privsep_exec=True)
 
     @mock.patch.object(ip_lib, 'IPWrapper')
     @mock.patch('eventlet.spawn_n')
@@ -1415,7 +1424,8 @@ class TestArpPing(TestIPCmdBase):
         # should return early with a single call when ENODEV
         mIPWrapper.assert_has_calls([
             mock.call(namespace=mock.sentinel.ns_name),
-            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1])
+            mock.call().netns.execute(mock.ANY, extra_ok_codes=[1],
+                                      privsep_exec=True)
         ] * 1)
 
     @mock.patch('eventlet.spawn_n')
@@ -1458,7 +1468,7 @@ class TestSysctl(base.BaseTestCase):
             dev.disable_ipv6()
             self.execute.assert_called_once_with(
                 ['sysctl', '-w', 'net.ipv6.conf.tap0.disable_ipv6=1'],
-                log_fail_as_error=True, run_as_root=True)
+                log_fail_as_error=True, run_as_root=True, privsep_exec=True)
 
     def test_disable_ipv6_when_ipv6_globally_disabled(self):
         dev = ip_lib.IPDevice('tap0', 'ns1')
@@ -1483,7 +1493,8 @@ class TestConntrack(base.BaseTestCase):
                       '--dport', dport]
         device.delete_socket_conntrack_state(ip_str, dport, protocol)
         self.execute.assert_called_once_with(expect_cmd, check_exit_code=True,
-                                             extra_ok_codes=[1])
+                                             extra_ok_codes=[1],
+                                             privsep_exec=True)
 
 
 class ParseIpRuleTestCase(base.BaseTestCase):

neutron/tests/unit/agent/linux/test_keepalived.py

@@ -47,6 +47,8 @@ class KeepalivedBaseTestCase(base.BaseTestCase):
     def setUp(self):
         super(KeepalivedBaseTestCase, self).setUp()
         l3_config.register_l3_agent_config_opts(l3_config.OPTS, cfg.CONF)
+        self._mock_no_track_supported = mock.patch.object(
+            keepalived, '_is_keepalived_use_no_track_supported')
 
 
 class KeepalivedGetFreeRangeTestCase(KeepalivedBaseTestCase):
@@ -193,6 +195,7 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
             self.assertEqual(self.expected, config.get_config_str())
 
     def test_config_generation_no_track_not_supported(self):
+        self._mock_no_track_supported.start().return_value = False
         config = self._get_config()
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',

neutron/tests/unit/agent/linux/test_l3_tc_lib.py

@@ -171,7 +171,8 @@ class TestFloatingIPTcCommandBase(base.BaseTestCase):
             run_as_root=True,
             check_exit_code=True,
             log_fail_as_error=True,
-            extra_ok_codes=None
+            extra_ok_codes=None,
+            privsep_exec=True
         )
 
     def _test__get_filterid_for_ip(self, filters):
@@ -236,7 +237,8 @@ class TestFloatingIPTcCommandBase(base.BaseTestCase):
             run_as_root=True,
             check_exit_code=True,
             log_fail_as_error=True,
-            extra_ok_codes=None
+            extra_ok_codes=None,
+            privsep_exec=True
         )
 
     def _test__get_qdisc_filters(self, filters):
@@ -277,7 +279,8 @@ class TestFloatingIPTcCommandBase(base.BaseTestCase):
             run_as_root=True,
             check_exit_code=True,
             log_fail_as_error=True,
-            extra_ok_codes=None
+            extra_ok_codes=None,
+            privsep_exec=True
         )
 
     def test__get_or_create_qdisc(self):
@@ -382,7 +385,8 @@ class TestFloatingIPTcCommand(base.BaseTestCase):
                         run_as_root=True,
                         check_exit_code=True,
                         log_fail_as_error=True,
-                        extra_ok_codes=None
+                        extra_ok_codes=None,
+                        privsep_exec=True
                     )
 
     def test_set_ip_rate_limit_no_qdisc_without_chain(self):
@@ -409,7 +413,8 @@ class TestFloatingIPTcCommand(base.BaseTestCase):
                     run_as_root=True,
                     check_exit_code=True,
                     log_fail_as_error=True,
-                    extra_ok_codes=None
+                    extra_ok_codes=None,
+                    privsep_exec=True
                 )
 
     def test_get_filter_id_for_ip(self):

neutron/tests/unit/agent/linux/test_utils.py

@@ -25,6 +25,7 @@ from oslo_config import cfg
 import oslo_i18n
 
 from neutron.agent.linux import utils
+from neutron.privileged.agent.linux import utils as priv_utils
 from neutron.tests import base
 
 
@@ -46,7 +47,15 @@ class AgentUtilsExecuteTest(base.BaseTestCase):
         result = utils.execute(["ls", self.test_file])
         self.assertEqual(result, expected)
 
-    def test_with_helper(self):
+    def test_with_root_privileges_privsep(self):
+        with mock.patch.object(priv_utils, 'execute_process') as \
+                mock_execute_process:
+            mock_execute_process.return_value = ['', '', 0]
+            utils.execute(['ls', self.test_file], run_as_root=True,
+                          privsep_exec=True)
+        mock_execute_process.assert_called_once()
+
+    def test_with_root_privileges_rootwrap(self):
         expected = "ls %s\n" % self.test_file
         self.mock_popen.return_value = [expected, ""]
         self.config(group='AGENT', root_helper='echo')
@@ -54,7 +63,7 @@ class AgentUtilsExecuteTest(base.BaseTestCase):
         self.assertEqual(result, expected)
 
     @mock.patch.object(utils.RootwrapDaemonHelper, 'get_client')
-    def test_with_helper_exception(self, get_client):
+    def test_with_root_privileges_rootwrap_exception(self, get_client):
         client_inst = mock.Mock()
         client_inst.execute.side_effect = RuntimeError
         get_client.return_value = client_inst
@@ -229,10 +238,11 @@ class TestKillProcess(base.BaseTestCase):
                                side_effect=exc) as mock_execute:
             with mock.patch.object(utils, 'process_is_running',
                                    return_value=not pid_killed):
-                utils.kill_process(pid, kill_signal, run_as_root=True)
+                utils.kill_process(pid, kill_signal, run_as_root=True,
+                                   privsep_exec=False)
 
         mock_execute.assert_called_with(['kill', '-%d' % kill_signal, pid],
-                                        run_as_root=True)
+                                        run_as_root=True, privsep_exec=False)
 
     def test_kill_process_returns_none_for_valid_pid(self):
         self._test_kill_process('1')

neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_arp_protect.py

@@ -66,34 +66,40 @@ class TestLinuxBridgeARPSpoofing(base.BaseTestCase):
         expected = [
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-L'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.ANY,
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-N',
                        'neutronMAC-%s' % vif, '-P', 'DROP'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-A',
                        'PREROUTING', '-i', vif, '-j', mac_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-A',
                        mac_chain, '-i', vif,
                        '--among-src', '%s' % ','.join(sorted(mac_addresses)),
                        '-j', 'RETURN'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.ANY,
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-N',
                        spoof_chain, '-P', 'DROP'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-F',
                        spoof_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
         ]
         for addr in sorted(ip_addresses):
             expected.extend([
@@ -101,7 +107,8 @@ class TestLinuxBridgeARPSpoofing(base.BaseTestCase):
                            spoof_chain, '-p', 'ARP',
                            '--arp-ip-src', addr, '-j', 'ACCEPT'],
                           check_exit_code=True, extra_ok_codes=None,
-                          log_fail_as_error=True, run_as_root=True),
+                          log_fail_as_error=True, run_as_root=True,
+                          privsep_exec=False),
             ])
         expected.extend([
             mock.ANY,
@@ -109,7 +116,8 @@ class TestLinuxBridgeARPSpoofing(base.BaseTestCase):
                        'PREROUTING', '-i', vif, '-j',
                        spoof_chain, '-p', 'ARP'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
         ])
 
         arp_protect.setup_arp_spoofing_protection(vif, port)
@@ -129,56 +137,68 @@ class TestLinuxBridgeARPSpoofing(base.BaseTestCase):
         expected = [
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-L'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-D',
                        'PREROUTING', '-i', VIF, '-j', spoof_chain,
                        '-p', 'ARP'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-F',
                        spoof_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-X',
                        spoof_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-F',
                        mac_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'nat', '--concurrent', '-X',
                        mac_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'filter', '--concurrent', '-L'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.call(['ebtables', '-t', 'filter', '--concurrent', '-D',
                        'FORWARD', '-i', VIF, '-j', spoof_chain,
                        '-p', 'ARP'],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'filter', '--concurrent', '-F',
                        spoof_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'filter', '--concurrent', '-X',
                        spoof_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.ANY,
             mock.call(['ebtables', '-t', 'filter', '--concurrent', '-F',
                        mac_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
             mock.call(['ebtables', '-t', 'filter', '--concurrent', '-X',
                        mac_chain],
                       check_exit_code=True, extra_ok_codes=None,
-                      log_fail_as_error=True, run_as_root=True),
+                      log_fail_as_error=True, run_as_root=True,
+                      privsep_exec=False),
         ]
 
         arp_protect.delete_arp_spoofing_protection([VIF])

releasenotes/notes/replace-rootwrap-with-privsep-5b85f1ba83df9554.yaml

@@ -0,0 +1,17 @@
+---
+other:
+  - |
+    As defined in `Migrate from oslo.rootwrap to oslo.privsep
+    <https://opendev.org/openstack/governance/src/branch/master/goals/selected/wallaby/migrate-to-privsep.rst>`_,
+    all OpenStack proyects should migrate from oslo.rootwrap to oslo.privsep
+    because "oslo.privsep offers a superior security model, faster and more
+    secure".
+    This migration will end with the deprecation and removal of oslo.rootwrap
+    from Neutron. To ensure the quality of the Neutron code, this migration
+    will be done sequentially in several patches, checking none of them breaks
+    the current functionality.
+    In order to easily migrate to execute all external commands inside a
+    privsep context, a new input variable "privsep_exec", that defaults to
+    "False", is added to ``neutron.agent.linux.utils.execute``. That will
+    divert the code to a privsep decorated executor.
+    Once the migration finishes, this new input parameter will be removed.