Adopt privsep and read routing table with pyroute2

Make use of oslo.privsep to support namespaces. This includes all
relevant code necessary for oslo.privsep to work.

Change ip_lib's get_routing_table method to use pyroute2, rather than
parsing the output of 'ip route'.

Change-Id: I89bfa3dbf1776da973cfca389b2841019a520f75
Partial-Bug: 1492714
Co-Authored-By: Angus Lees <gus@inodes.org>

etc/neutron/rootwrap.d/privsep.filters

@@ -0,0 +1,36 @@
+# Command filters to allow privsep daemon to be started via rootwrap.
+#
+# This file should be owned by (and only-writeable by) the root user
+
+[Filters]
+
+# By installing the following, the local admin is asserting that:
+#
+# 1. The python module load path used by privsep-helper
+#    command as root (as started by sudo/rootwrap) is trusted.
+# 2. Any oslo.config files matching the --config-file
+#    arguments below are trusted.
+# 3. Users allowed to run sudo/rootwrap with this configuration(*) are
+#    also allowed to invoke python "entrypoint" functions from
+#    --privsep_context with the additional (possibly root) privileges
+#    configured for that context.
+#
+# (*) ie: the user is allowed by /etc/sudoers to run rootwrap as root
+#
+# In particular, the oslo.config and python module path must not
+# be writeable by the unprivileged user.
+
+# oslo.privsep default neutron context
+privsep: PathFilter, privsep-helper, root,
+ --config-file, /etc,
+ --privsep_context, neutron.privileged.default,
+ --privsep_sock_path, /
+
+# Same as above with a second `--config-file` arg, since many neutron
+# components are installed like that (eg: by devstack).  Adjust to
+# suit local requirements.
+privsep: PathFilter, privsep-helper, root,
+ --config-file, /etc,
+ --config-file, /etc,
+ --privsep_context, neutron.privileged.default,
+ --privsep_sock_path, /

neutron/agent/common/config.py

@@ -14,8 +14,10 @@
 #    under the License.
 
 import os
+import shlex
 
 from oslo_config import cfg
+from oslo_privsep import priv_context
 
 from neutron._i18n import _
 from neutron.common import config
@@ -169,3 +171,7 @@ def setup_conf():
 
 # add a logging setup method here for convenience
 setup_logging = config.setup_logging
+
+
+def setup_privsep():
+    priv_context.init(root_helper=shlex.split(get_root_helper(cfg.CONF)))

neutron/agent/linux/ip_lib.py

@@ -29,6 +29,7 @@ from neutron._i18n import _, _LE, _LW
 from neutron.agent.common import utils
 from neutron.common import exceptions as n_exc
 from neutron.common import utils as common_utils
+from neutron.privileged.agent.linux import ip_lib as privileged
 
 LOG = logging.getLogger(__name__)
 
@@ -935,39 +936,7 @@ def get_device_mac(device_name, namespace=None):
     return IPDevice(device_name, namespace=namespace).link.address
 
 
-_IP_ROUTE_PARSE_KEYS = {
+NetworkNamespaceNotFound = privileged.NetworkNamespaceNotFound
-    'via': 'nexthop',
-    'dev': 'device',
-    'scope': 'scope'
-}
-
-
-def _parse_ip_route_line(line):
-    """Parse a line output from ip route.
-    Example for output from 'ip route':
-    default via 192.168.3.120 dev wlp3s0  proto static  metric 1024
-    10.0.0.0/8 dev tun0  proto static  scope link  metric 1024
-    10.0.1.0/8 dev tun1  proto static  scope link  metric 1024 linkdown
-    The first column is the destination, followed by key/value pairs and flags.
-    @param line A line output from ip route
-    @return: a dictionary representing a route.
-    """
-    line = line.split()
-    result = {
-        'destination': line[0],
-        'nexthop': None,
-        'device': None,
-        'scope': None
-    }
-    idx = 1
-    while idx < len(line):
-        field = _IP_ROUTE_PARSE_KEYS.get(line[idx])
-        if not field:
-            idx = idx + 1
-        else:
-            result[field] = line[idx + 1]
-            idx = idx + 2
-    return result
 
 
 def get_routing_table(ip_version, namespace=None):
@@ -981,14 +950,8 @@ def get_routing_table(ip_version, namespace=None):
                                'device': device_name,
                                'scope': scope}
     """
-
+    # oslo.privsep turns lists to tuples in its IPC code. Change it back
-    ip_wrapper = IPWrapper(namespace=namespace)
+    return list(privileged.get_routing_table(ip_version, namespace))
-    table = ip_wrapper.netns.execute(
-        ['ip', '-%s' % ip_version, 'route'],
-        check_exit_code=True)
-
-    return [_parse_ip_route_line(line)
-            for line in table.split('\n') if line.strip()]
 
 
 def ensure_device_is_ready(device_name, namespace=None):

neutron/common/eventlet_utils.py

@@ -16,6 +16,7 @@
 import os
 
 import eventlet
+from oslo_utils import importutils
 
 
 def monkey_patch():
@@ -30,3 +31,5 @@ def monkey_patch():
         eventlet.monkey_patch(os=False, thread=False)
     else:
         eventlet.monkey_patch()
+        p_c_e = importutils.import_module('pyroute2.config.eventlet')
+        p_c_e.eventlet_config()

neutron/privileged/__init__.py

@@ -0,0 +1,26 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from oslo_privsep import capabilities as caps
+from oslo_privsep import priv_context
+
+# It is expected that most (if not all) neutron operations can be
+# executed with these privileges.
+default = priv_context.PrivContext(
+    __name__,
+    cfg_section='privsep',
+    pypath=__name__ + '.default',
+    # TODO(gus): CAP_SYS_ADMIN is required (only?) for manipulating
+    # network namespaces.  SYS_ADMIN is a lot of scary powers, so
+    # consider breaking this out into a separate minimal context.
+    capabilities=[caps.CAP_SYS_ADMIN, caps.CAP_NET_ADMIN],
+)

neutron/privileged/agent/linux/ip_lib.py

@@ -0,0 +1,68 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import errno
+import socket
+
+import pyroute2
+from pyroute2.netlink import rtnl
+
+from neutron._i18n import _
+from neutron import privileged
+
+
+_IP_VERSION_FAMILY_MAP = {4: socket.AF_INET, 6: socket.AF_INET6}
+
+
+def _get_scope_name(scope):
+    """Return the name of the scope (given as a number), or the scope number
+    if the name is unknown.
+    """
+    return rtnl.rt_scope.get(scope, scope)
+
+
+class NetworkNamespaceNotFound(RuntimeError):
+    message = _("Network namespace %(netns_name)s could not be found.")
+
+    def __init__(self, netns_name):
+        super(NetworkNamespaceNotFound, self).__init__(
+            self.message % {'netns_name': netns_name})
+
+
+@privileged.default.entrypoint
+def get_routing_table(ip_version, namespace=None):
+    """Return a list of dictionaries, each representing a route.
+
+    :param ip_version: IP version of routes to return, for example 4
+    :param namespace: The name of the namespace from which to get the routes
+    :return: a list of dictionaries, each representing a route.
+    The dictionary format is: {'destination': cidr,
+                               'nexthop': ip,
+                               'device': device_name,
+                               'scope': scope}
+    """
+    family = _IP_VERSION_FAMILY_MAP[ip_version]
+    try:
+        netns = pyroute2.NetNS(namespace, flags=0) if namespace else None
+    except OSError as e:
+        if e.errno == errno.ENOENT:
+            raise NetworkNamespaceNotFound(netns_name=namespace)
+        raise
+    with pyroute2.IPDB(nl=netns) as ipdb:
+        ipdb_routes = ipdb.routes
+        ipdb_interfaces = ipdb.interfaces
+        routes = [{'destination': route['dst'],
+                   'nexthop': route.get('gateway'),
+                   'device': ipdb_interfaces[route['oif']]['ifname'],
+                   'scope': _get_scope_name(route['scope'])}
+                  for route in ipdb_routes if route['family'] == family]
+    return routes

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

@@ -20,6 +20,7 @@ from neutron_lib import constants
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import importutils
+import testtools
 
 from neutron.agent.common import config
 from neutron.agent.linux import interface
@@ -184,16 +185,21 @@ class IpLibTestCase(IpLibTestFramework):
         device.link.delete()
 
     def test_get_routing_table(self):
-        attr = self.generate_device_details()
+        attr = self.generate_device_details(
+            ip_cidrs=["%s/24" % TEST_IP, "fd00::1/64"]
+        )
         device = self.manage_device(attr)
         device_ip = attr.ip_cidrs[0].split('/')[0]
         destination = '8.8.8.0/24'
         device.route.add_route(destination, device_ip)
 
+        destination6 = 'fd01::/64'
+        device.route.add_route(destination6, "fd00::2")
+
         expected_routes = [{'nexthop': device_ip,
                             'device': attr.name,
                             'destination': destination,
-                            'scope': None},
+                            'scope': 'universe'},
                            {'nexthop': None,
                             'device': attr.name,
                             'destination': str(
@@ -201,7 +207,25 @@ class IpLibTestCase(IpLibTestFramework):
                             'scope': 'link'}]
 
         routes = ip_lib.get_routing_table(4, namespace=attr.namespace)
-        self.assertEqual(expected_routes, routes)
+        self.assertItemsEqual(expected_routes, routes)
+        self.assertIsInstance(routes, list)
+
+        expected_routes6 = [{'nexthop': "fd00::2",
+                             'device': attr.name,
+                             'destination': destination6,
+                             'scope': 'universe'},
+                            {'nexthop': None,
+                             'device': attr.name,
+                             'destination': str(
+                                 netaddr.IPNetwork(attr.ip_cidrs[1]).cidr),
+                             'scope': 'universe'}]
+        routes6 = ip_lib.get_routing_table(6, namespace=attr.namespace)
+        self.assertItemsEqual(expected_routes6, routes6)
+        self.assertIsInstance(routes6, list)
+
+    def test_get_routing_table_no_namespace(self):
+        with testtools.ExpectedException(ip_lib.NetworkNamespaceNotFound):
+            ip_lib.get_routing_table(4, namespace="nonexistent-netns")
 
     def _check_for_device_name(self, ip, name, should_exist):
         exist = any(d for d in ip.get_devices() if d.name == name)

neutron/tests/functional/base.py

@@ -69,6 +69,7 @@ class BaseSudoTestCase(BaseLoggingTestCase):
         self.config(group='AGENT',
                     root_helper_daemon=os.environ.get(
                         'OS_ROOTWRAP_DAEMON_CMD'))
+        config.setup_privsep()
 
     @common_base.no_skip_on_missing_deps
     def check_command(self, cmd, error_text, skip_msg, run_as_root=False):

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

@@ -13,14 +13,18 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import errno
+
 import mock
 import netaddr
 from neutron_lib import exceptions
+import pyroute2
 import testtools
 
 from neutron.agent.common import utils  # noqa
 from neutron.agent.linux import ip_lib
 from neutron.common import exceptions as n_exc
+from neutron import privileged
 from neutron.tests import base
 
 NETNS_SAMPLE = [
@@ -1307,54 +1311,268 @@ class TestDeviceExists(base.BaseTestCase):
 
 
 class TestGetRoutingTable(base.BaseTestCase):
-    @mock.patch.object(ip_lib, 'IPWrapper')
+    ip_db_interfaces = {
-    def _test_get_routing_table(self, version, ip_route_output, expected,
+        1: {
-                                mock_ipwrapper):
+            'family': 0,
-        instance = mock_ipwrapper.return_value
+            'txqlen': 0,
-        mock_netns = instance.netns
+            'ipdb_scope': 'system',
-        mock_execute = mock_netns.execute
+            'index': 1,
-        mock_execute.return_value = ip_route_output
+            'operstate': 'DOWN',
+            'num_tx_queues': 1,
+            'group': 0,
+            'carrier_changes': 0,
+            'ipaddr': [],
+            'neighbours': [],
+            'ifname': 'lo',
+            'promiscuity': 0,
+            'linkmode': 0,
+            'broadcast': '00:00:00:00:00:00',
+            'address': '00:00:00:00:00:00',
+            'vlans': [],
+            'ipdb_priority': 0,
+            'qdisc': 'noop',
+            'mtu': 65536,
+            'num_rx_queues': 1,
+            'carrier': 1,
+            'flags': 8,
+            'ifi_type': 772,
+            'ports': []
+        },
+        2: {
+            'family': 0,
+            'txqlen': 500,
+            'ipdb_scope': 'system',
+            'index': 2,
+            'operstate': 'DOWN',
+            'num_tx_queues': 1,
+            'group': 0,
+            'carrier_changes': 1,
+            'ipaddr': ['1111:1111:1111:1111::3/64', '10.0.0.3/24'],
+            'neighbours': [],
+            'ifname': 'tap-1',
+            'promiscuity': 0,
+            'linkmode': 0,
+            'broadcast': 'ff:ff:ff:ff:ff:ff',
+            'address': 'b6:d5:f6:a8:2e:62',
+            'vlans': [],
+            'ipdb_priority': 0,
+            'kind': 'tun',
+            'qdisc': 'fq_codel',
+            'mtu': 1500,
+            'num_rx_queues': 1,
+            'carrier': 0,
+            'flags': 4099,
+            'ifi_type': 1,
+            'ports': []
+        },
+        'tap-1': {
+            'family': 0,
+            'txqlen': 500,
+            'ipdb_scope': 'system',
+            'index': 2,
+            'operstate': 'DOWN',
+            'num_tx_queues': 1,
+            'group': 0,
+            'carrier_changes': 1,
+            'ipaddr': ['1111:1111:1111:1111::3/64', '10.0.0.3/24'],
+            'neighbours': [],
+            'ifname': 'tap-1',
+            'promiscuity': 0,
+            'linkmode': 0,
+            'broadcast': 'ff:ff:ff:ff:ff:ff',
+            'address': 'b6:d5:f6:a8:2e:62',
+            'vlans': [],
+            'ipdb_priority': 0,
+            'kind': 'tun',
+            'qdisc': 'fq_codel',
+            'mtu': 1500,
+            'num_rx_queues': 1,
+            'carrier': 0,
+            'flags': 4099,
+            'ifi_type': 1,
+            'ports': []
+        },
+        'lo': {
+            'family': 0,
+            'txqlen': 0,
+            'ipdb_scope': 'system',
+            'index': 1,
+            'operstate': 'DOWN',
+            'num_tx_queues': 1,
+            'group': 0,
+            'carrier_changes': 0,
+            'ipaddr': [],
+            'neighbours': [],
+            'ifname': 'lo',
+            'promiscuity': 0,
+            'linkmode': 0,
+            'broadcast': '00:00:00:00:00:00',
+            'address': '00:00:00:00:00:00',
+            'vlans': [],
+            'ipdb_priority': 0,
+            'qdisc': 'noop',
+            'mtu': 65536,
+            'num_rx_queues': 1,
+            'carrier': 1,
+            'flags': 8,
+            'ifi_type': 772,
+            'ports': []
+        }
+    }
+
+    ip_db_routes = [
+        {
+            'oif': 2,
+            'dst_len': 24,
+            'family': 2,
+            'proto': 3,
+            'tos': 0,
+            'dst': '10.0.1.0/24',
+            'flags': 16,
+            'ipdb_priority': 0,
+            'metrics': {},
+            'scope': 0,
+            'encap': {},
+            'src_len': 0,
+            'table': 254,
+            'multipath': [],
+            'type': 1,
+            'gateway': '10.0.0.1',
+            'ipdb_scope': 'system'
+        }, {
+            'oif': 2,
+            'type': 1,
+            'dst_len': 24,
+            'family': 2,
+            'proto': 2,
+            'tos': 0,
+            'dst': '10.0.0.0/24',
+            'ipdb_priority': 0,
+            'metrics': {},
+            'flags': 16,
+            'encap': {},
+            'src_len': 0,
+            'table': 254,
+            'multipath': [],
+            'prefsrc': '10.0.0.3',
+            'scope': 253,
+            'ipdb_scope': 'system'
+        }, {
+            'oif': 2,
+            'dst_len': 0,
+            'family': 2,
+            'proto': 3,
+            'tos': 0,
+            'dst': 'default',
+            'flags': 16,
+            'ipdb_priority': 0,
+            'metrics': {},
+            'scope': 0,
+            'encap': {},
+            'src_len': 0,
+            'table': 254,
+            'multipath': [],
+            'type': 1,
+            'gateway': '10.0.0.2',
+            'ipdb_scope': 'system'
+        }, {
+            'metrics': {},
+            'oif': 2,
+            'dst_len': 64,
+            'family': 10,
+            'proto': 2,
+            'tos': 0,
+            'dst': '1111:1111:1111:1111::/64',
+            'pref': '00',
+            'ipdb_priority': 0,
+            'priority': 256,
+            'flags': 0,
+            'encap': {},
+            'src_len': 0,
+            'table': 254,
+            'multipath': [],
+            'type': 1,
+            'scope': 0,
+            'ipdb_scope': 'system'
+        }, {
+            'metrics': {},
+            'oif': 2,
+            'dst_len': 64,
+            'family': 10,
+            'proto': 3,
+            'tos': 0,
+            'dst': '1111:1111:1111:1112::/64',
+            'pref': '00',
+            'flags': 0,
+            'ipdb_priority': 0,
+            'priority': 1024,
+            'scope': 0,
+            'encap': {},
+            'src_len': 0,
+            'table': 254,
+            'multipath': [],
+            'type': 1,
+            'gateway': '1111:1111:1111:1111::1',
+            'ipdb_scope': 'system'
+        }
+    ]
+
+    def setUp(self):
+        super(TestGetRoutingTable, self).setUp()
+        self.addCleanup(privileged.default.set_client_mode, True)
+        privileged.default.set_client_mode(False)
+
+    @mock.patch.object(pyroute2, 'IPDB')
+    @mock.patch.object(pyroute2, 'NetNS')
+    def test_get_routing_table_nonexistent_namespace(self,
+                                                     mock_netns, mock_ip_db):
+        mock_netns.side_effect = OSError(errno.ENOENT, None)
+        with testtools.ExpectedException(ip_lib.NetworkNamespaceNotFound):
+            ip_lib.get_routing_table(4, 'ns')
+
+    @mock.patch.object(pyroute2, 'IPDB')
+    @mock.patch.object(pyroute2, 'NetNS')
+    def test_get_routing_table_other_error(self, mock_netns, mock_ip_db):
+        expected_exception = OSError(errno.EACCES, None)
+        mock_netns.side_effect = expected_exception
+        with testtools.ExpectedException(expected_exception.__class__):
+            ip_lib.get_routing_table(4, 'ns')
+
+    @mock.patch.object(pyroute2, 'IPDB')
+    @mock.patch.object(pyroute2, 'NetNS')
+    def _test_get_routing_table(self, version, ip_db_routes, expected,
+                                mock_netns, mock_ip_db):
+        mock_ip_db_instance = mock_ip_db.return_value
+        mock_ip_db_enter = mock_ip_db_instance.__enter__.return_value
+        mock_ip_db_enter.interfaces = self.ip_db_interfaces
+        mock_ip_db_enter.routes = ip_db_routes
         self.assertEqual(expected, ip_lib.get_routing_table(version))
 
     def test_get_routing_table_4(self):
-        ip_route_output = ("""
+        expected = [{'destination': '10.0.1.0/24',
-default via 192.168.3.120 dev wlp3s0  proto static  metric 1024
+                     'nexthop': '10.0.0.1',
-10.0.0.0/8 dev tun0  proto static  scope link  metric 1024
+                     'device': 'tap-1',
-10.0.1.0/8 dev tun1  proto static  scope link  metric 1024 linkdown
+                     'scope': 'universe'},
-""")
+                    {'destination': '10.0.0.0/24',
-        expected = [{'destination': 'default',
-                     'nexthop': '192.168.3.120',
-                     'device': 'wlp3s0',
-                     'scope': None},
-                    {'destination': '10.0.0.0/8',
                      'nexthop': None,
-                     'device': 'tun0',
+                     'device': 'tap-1',
                      'scope': 'link'},
-                    {'destination': '10.0.1.0/8',
+                    {'destination': 'default',
-                     'nexthop': None,
+                     'nexthop': '10.0.0.2',
-                     'device': 'tun1',
+                     'device': 'tap-1',
-                     'scope': 'link'}]
+                     'scope': 'universe'}]
-        self._test_get_routing_table(4, ip_route_output, expected)
+        self._test_get_routing_table(4, self.ip_db_routes, expected)
 
     def test_get_routing_table_6(self):
-        ip_route_output = ("""
+        expected = [{'destination': '1111:1111:1111:1111::/64',
-2001:db8:0:f101::/64 dev tap-1  proto kernel  metric 256  pref medium
-2001:db8:0:f102::/64 dev tap-2  proto kernel  metric 256  pref medium linkdown
-default via 2001:db8:0:f101::4 dev tap-1  metric 1024  pref medium
-""")
-        expected = [{'destination': '2001:db8:0:f101::/64',
                      'nexthop': None,
                      'device': 'tap-1',
-                     'scope': None},
+                     'scope': 'universe'},
-                    {'destination': '2001:db8:0:f102::/64',
+                    {'destination': '1111:1111:1111:1112::/64',
-                     'nexthop': None,
+                     'nexthop': '1111:1111:1111:1111::1',
-                     'device': 'tap-2',
-                     'scope': None},
-                    {'destination': 'default',
-                     'nexthop': '2001:db8:0:f101::4',
                      'device': 'tap-1',
-                     'scope': None}]
+                     'scope': 'universe'}]
-        self._test_get_routing_table(6, ip_route_output, expected)
+        self._test_get_routing_table(6, self.ip_db_routes, expected)
 
 
 class TestIpNeighCommand(TestIPCmdBase):

releasenotes/notes/use-pyroute2-in-ip-lib-558bfea8f14d1fea.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Initial support for oslo.privsep has been added. A usage example,
+    including unit tests, exists with ip_lib.get_routing_table.

test-requirements.txt

@@ -16,6 +16,8 @@ testscenarios>=0.4 # Apache-2.0/BSD
 WebTest>=2.0 # MIT
 oslotest>=1.10.0 # Apache-2.0
 os-testr>=0.8.0 # Apache-2.0
+oslo.privsep>=1.9.0  # Apache-2.0
+pyroute2>=0.4.3  # Apache-2.0 (+ dual licensed GPL2)
 ddt>=1.0.1 # MIT
 pylint==1.4.5 # GPLv2
 reno>=1.8.0 # Apache-2.0