Merge "Allow metadata proxy running with nobody user/group"

13 files changed, 221 insertions, 41 deletions

diff --git a/etc/metadata_agent.ini b/etc/metadata_agent.ini
index 06588a514b..4a0331ee12 100644
--- a/etc/metadata_agent.ini
+++ b/etc/metadata_agent.ini
@@ -45,6 +45,15 @@ admin_password = %SERVICE_PASSWORD%
 # Location of Metadata Proxy UNIX domain socket
 # metadata_proxy_socket = $state_path/metadata_proxy
 
+# Metadata Proxy UNIX domain socket mode, 3 values allowed:
+# 'deduce': deduce mode from metadata_proxy_user/group values,
+# 'user': set metadata proxy socket mode to 0o644, to use when
+# metadata_proxy_user is agent effective user or root,
+# 'group': set metadata proxy socket mode to 0o664, to use when
+# metadata_proxy_group is agent effective group,
+# 'all': set metadata proxy socket mode to 0o666, to use otherwise.
+# metadata_proxy_socket_mode = deduce
+
 # Number of separate worker processes for metadata server. Defaults to
 # half the number of CPU cores
 # metadata_workers =
diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py
index afb71da6e6..8b7bbae31a 100644
--- a/neutron/agent/dhcp_agent.py
+++ b/neutron/agent/dhcp_agent.py
@@ -21,7 +21,7 @@ from oslo_config import cfg
 from neutron.agent.common import config
 from neutron.agent.dhcp import config as dhcp_config
 from neutron.agent.linux import interface
-from neutron.agent.metadata import driver as metadata_driver
+from neutron.agent.metadata import config as metadata_config
 from neutron.common import config as common_config
 from neutron.common import topics
 from neutron.openstack.common import service
@@ -35,7 +35,8 @@ def register_options():
     cfg.CONF.register_opts(dhcp_config.DHCP_AGENT_OPTS)
     cfg.CONF.register_opts(dhcp_config.DHCP_OPTS)
     cfg.CONF.register_opts(dhcp_config.DNSMASQ_OPTS)
-    cfg.CONF.register_opts(metadata_driver.MetadataDriver.OPTS)
+    cfg.CONF.register_opts(metadata_config.DRIVER_OPTS)
+    cfg.CONF.register_opts(metadata_config.SHARED_OPTS)
     cfg.CONF.register_opts(interface.OPTS)
 
 
diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index ef91264603..12c152d053 100644
--- a/neutron/agent/l3_agent.py
+++ b/neutron/agent/l3_agent.py
@@ -23,7 +23,7 @@ from neutron.agent.l3 import config as l3_config
 from neutron.agent.l3 import ha
 from neutron.agent.linux import external_process
 from neutron.agent.linux import interface
-from neutron.agent.metadata import driver as metadata_driver
+from neutron.agent.metadata import config as metadata_config
 from neutron.common import config as common_config
 from neutron.common import topics
 from neutron.openstack.common import service
@@ -32,7 +32,8 @@ from neutron import service as neutron_service
 
 def register_opts(conf):
     conf.register_opts(l3_config.OPTS)
-    conf.register_opts(metadata_driver.MetadataDriver.OPTS)
+    conf.register_opts(metadata_config.DRIVER_OPTS)
+    conf.register_opts(metadata_config.SHARED_OPTS)
     conf.register_opts(ha.OPTS)
     config.register_interface_driver_opts_helper(conf)
     config.register_use_namespaces_opts_helper(conf)
diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py
index ca9bfba722..548b4cddb9 100644
--- a/neutron/agent/linux/utils.py
+++ b/neutron/agent/linux/utils.py
@@ -15,6 +15,7 @@
 
 import fcntl
 import glob
+import grp
 import httplib
 import os
 import pwd
@@ -344,6 +345,15 @@ def is_effective_user(user_id_or_name):
     return user_id_or_name == effective_user_name
 
 
+def is_effective_group(group_id_or_name):
+    """Returns True if group_id_or_name is effective group (id/name)."""
+    egid = os.getegid()
+    if str(group_id_or_name) == str(egid):
+        return True
+    effective_group_name = grp.getgrgid(egid).gr_name
+    return group_id_or_name == effective_group_name
+
+
 class UnixDomainHTTPConnection(httplib.HTTPConnection):
     """Connection class for HTTP over UNIX domain socket."""
     def __init__(self, host, port=None, strict=None, timeout=None,
@@ -375,10 +385,12 @@ class UnixDomainWSGIServer(wsgi.Server):
         self._server = None
         super(UnixDomainWSGIServer, self).__init__(name)
 
-    def start(self, application, file_socket, workers, backlog):
+    def start(self, application, file_socket, workers, backlog, mode=None):
         self._socket = eventlet.listen(file_socket,
                                        family=socket.AF_UNIX,
                                        backlog=backlog)
+        if mode is not None:
+            os.chmod(file_socket, mode)
 
         self._launch(application, workers=workers)
 
diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py
index 062acbaa00..e2cad9c9ac 100644
--- a/neutron/agent/metadata/agent.py
+++ b/neutron/agent/metadata/agent.py
@@ -24,6 +24,7 @@ import six.moves.urllib.parse as urlparse
 import webob
 
 from neutron.agent.linux import utils as agent_utils
+from neutron.agent.metadata import config
 from neutron.agent import rpc as agent_rpc
 from neutron.common import constants as n_const
 from neutron.common import rpc as n_rpc
@@ -36,6 +37,12 @@ from neutron.openstack.common import loopingcall
 
 LOG = logging.getLogger(__name__)
 
+MODE_MAP = {
+    config.USER_MODE: 0o644,
+    config.GROUP_MODE: 0o664,
+    config.ALL_MODE: 0o666,
+}
+
 
 class MetadataPluginAPI(object):
     """Agent-side RPC for metadata agent-to-plugin interaction.
@@ -305,10 +312,29 @@ class UnixDomainMetadataProxy(object):
             return
         self.agent_state.pop('start_flag', None)
 
+    def _get_socket_mode(self):
+        mode = self.conf.metadata_proxy_socket_mode
+        if mode == config.DEDUCE_MODE:
+            user = self.conf.metadata_proxy_user
+            if (not user or user == '0' or user == 'root'
+                    or agent_utils.is_effective_user(user)):
+                # user is agent effective user or root => USER_MODE
+                mode = config.USER_MODE
+            else:
+                group = self.conf.metadata_proxy_group
+                if not group or agent_utils.is_effective_group(group):
+                    # group is agent effective group => GROUP_MODE
+                    mode = config.GROUP_MODE
+                else:
+                    # otherwise => ALL_MODE
+                    mode = config.ALL_MODE
+        return MODE_MAP[mode]
+
     def run(self):
         server = agent_utils.UnixDomainWSGIServer('neutron-metadata-agent')
         server.start(MetadataProxyHandler(self.conf),
                      self.conf.metadata_proxy_socket,
                      workers=self.conf.metadata_workers,
-                     backlog=self.conf.metadata_backlog)
+                     backlog=self.conf.metadata_backlog,
+                     mode=self._get_socket_mode())
         server.wait()
diff --git a/neutron/agent/metadata/config.py b/neutron/agent/metadata/config.py
index 2ac2642d6c..be8d6b7921 100644
--- a/neutron/agent/metadata/config.py
+++ b/neutron/agent/metadata/config.py
@@ -17,6 +17,38 @@ from oslo_config import cfg
 from neutron.common import utils
 
 
+SHARED_OPTS = [
+    cfg.StrOpt('metadata_proxy_socket',
+               default='$state_path/metadata_proxy',
+               help=_('Location for Metadata Proxy UNIX domain socket.')),
+    cfg.StrOpt('metadata_proxy_user',
+               default='',
+               help=_("User (uid or name) running metadata proxy after "
+                      "its initialization (if empty: agent effective "
+                      "user).")),
+    cfg.StrOpt('metadata_proxy_group',
+               default='',
+               help=_("Group (gid or name) running metadata proxy after "
+                      "its initialization (if empty: agent effective "
+                      "group)."))
+]
+
+
+DRIVER_OPTS = [
+    cfg.BoolOpt('metadata_proxy_watch_log',
+                default=None,
+                help=_("Enable/Disable log watch by metadata proxy. It "
+                       "should be disabled when metadata_proxy_user/group "
+                       "is not allowed to read/write its log file and "
+                       "copytruncate logrotate option must be used if "
+                       "logrotate is enabled on metadata proxy log "
+                       "files. Option default value is deduced from "
+                       "metadata_proxy_user: watch log is enabled if "
+                       "metadata_proxy_user is agent effective user "
+                       "id/name.")),
+]
+
+
 METADATA_PROXY_HANDLER_OPTS = [
      cfg.StrOpt('admin_user',
                 help=_("Admin user")),
@@ -66,11 +98,29 @@ METADATA_PROXY_HANDLER_OPTS = [
                 help=_("Private key of client certificate."))
 ]
 
+DEDUCE_MODE = 'deduce'
+USER_MODE = 'user'
+GROUP_MODE = 'group'
+ALL_MODE = 'all'
+SOCKET_MODES = (DEDUCE_MODE, USER_MODE, GROUP_MODE, ALL_MODE)
+
 
 UNIX_DOMAIN_METADATA_PROXY_OPTS = [
-    cfg.StrOpt('metadata_proxy_socket',
-               default='$state_path/metadata_proxy',
-               help=_('Location for Metadata Proxy UNIX domain socket')),
+    cfg.StrOpt('metadata_proxy_socket_mode',
+               default=DEDUCE_MODE,
+               choices=SOCKET_MODES,
+               help=_("Metadata Proxy UNIX domain socket mode, 3 values "
+                      "allowed: "
+                      "'deduce': deduce mode from metadata_proxy_user/group "
+                      "values, "
+                      "'user': set metadata proxy socket mode to 0o644, to "
+                      "use when metadata_proxy_user is agent effective user "
+                      "or root, "
+                      "'group': set metadata proxy socket mode to 0o664, to "
+                      "use when metadata_proxy_group is agent effective "
+                      "group or root, "
+                      "'all': set metadata proxy socket mode to 0o666, to use "
+                      "otherwise.")),
     cfg.IntOpt('metadata_workers',
                default=utils.cpu_count() // 2,
                help=_('Number of separate worker processes for metadata '
diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py
index 411a6b7c46..437e5efe0b 100644
--- a/neutron/agent/metadata/driver.py
+++ b/neutron/agent/metadata/driver.py
@@ -15,7 +15,6 @@
 
 import os
 
-from oslo_config import cfg
 from oslo_log import log as logging
 
 from neutron.agent.common import config
@@ -36,34 +35,6 @@ METADATA_SERVICE_NAME = 'metadata-proxy'
 
 class MetadataDriver(object):
 
-    OPTS = [
-        cfg.StrOpt('metadata_proxy_socket',
-                   default='$state_path/metadata_proxy',
-                   help=_('Location of Metadata Proxy UNIX domain '
-                          'socket')),
-        cfg.StrOpt('metadata_proxy_user',
-                   default='',
-                   help=_("User (uid or name) running metadata proxy after "
-                          "its initialization (if empty: agent effective "
-                          "user)")),
-        cfg.StrOpt('metadata_proxy_group',
-                   default='',
-                   help=_("Group (gid or name) running metadata proxy after "
-                          "its initialization (if empty: agent effective "
-                          "group)")),
-        cfg.BoolOpt('metadata_proxy_watch_log',
-                    default=None,
-                    help=_("Enable/Disable log watch by metadata proxy. It "
-                           "should be disabled when metadata_proxy_user/group "
-                           "is not allowed to read/write its log file and "
-                           "copytruncate logrotate option must be used if "
-                           "logrotate is enabled on metadata proxy log "
-                           "files. Option default value is deduced from "
-                           "metadata_proxy_user: watch log is enabled if "
-                           "metadata_proxy_user is agent effective user "
-                           "id/name.")),
-    ]
-
     def __init__(self, l3_agent):
         self.metadata_port = l3_agent.conf.metadata_port
         self.metadata_access_mark = l3_agent.conf.metadata_access_mark
diff --git a/neutron/agent/metadata_agent.py b/neutron/agent/metadata_agent.py
index 8c06a37f97..b6e27914bd 100644
--- a/neutron/agent/metadata_agent.py
+++ b/neutron/agent/metadata_agent.py
@@ -28,6 +28,7 @@ LOG = logging.getLogger(__name__)
 
 
 def main():
+    cfg.CONF.register_opts(metadata_conf.SHARED_OPTS)
     cfg.CONF.register_opts(metadata_conf.UNIX_DOMAIN_METADATA_PROXY_OPTS)
     cfg.CONF.register_opts(metadata_conf.METADATA_PROXY_HANDLER_OPTS)
     cache.register_oslo_configs(cfg.CONF)
diff --git a/neutron/tests/functional/agent/linux/helpers.py b/neutron/tests/functional/agent/linux/helpers.py
index 1dfd591c82..1e51a9b81c 100644
--- a/neutron/tests/functional/agent/linux/helpers.py
+++ b/neutron/tests/functional/agent/linux/helpers.py
@@ -20,6 +20,8 @@ import select
 import shlex
 import subprocess
 
+import fixtures
+
 from neutron.agent.common import config
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import utils
@@ -33,6 +35,33 @@ SS_SOURCE_PORT_PATTERN = re.compile(
     r'^.*\s+\d+\s+.*:(?P<port>\d+)\s+[0-9:].*')
 
 
+class RecursivePermDirFixture(fixtures.Fixture):
+    """Ensure at least perms permissions on directory and ancestors."""
+
+    def __init__(self, directory, perms):
+        super(RecursivePermDirFixture, self).__init__()
+        self.directory = directory
+        self.least_perms = perms
+
+    def setUp(self):
+        super(RecursivePermDirFixture, self).setUp()
+        previous_directory = None
+        current_directory = self.directory
+        while previous_directory != current_directory:
+            perms = os.stat(current_directory).st_mode
+            if perms & self.least_perms != self.least_perms:
+                os.chmod(current_directory, perms | self.least_perms)
+                self.addCleanup(self.safe_chmod, current_directory, perms)
+            previous_directory = current_directory
+            current_directory = os.path.dirname(current_directory)
+
+    def safe_chmod(self, path, mode):
+        try:
+            os.chmod(path, mode)
+        except OSError:
+            pass
+
+
 def get_free_namespace_port(tcp=True, namespace=None):
     """Return an unused port from given namespace
 
diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py
index 0767b068d3..6bfb11a740 100755
--- a/neutron/tests/functional/agent/test_l3_agent.py
+++ b/neutron/tests/functional/agent/test_l3_agent.py
@@ -15,6 +15,7 @@
 
 import copy
 import functools
+import os.path
 
 import mock
 import netaddr
@@ -776,12 +777,21 @@ class MetadataFakeProxyHandler(object):
 
 class MetadataL3AgentTestCase(L3AgentTestFramework):
 
+    SOCKET_MODE = 0o644
+
     def _create_metadata_fake_server(self, status):
         server = utils.UnixDomainWSGIServer('metadata-fake-server')
         self.addCleanup(server.stop)
+
+        # NOTE(cbrandily): TempDir fixture creates a folder with 0o700
+        # permissions but metadata_proxy_socket folder must be readable by all
+        # users
+        self.useFixture(
+            helpers.RecursivePermDirFixture(
+                os.path.dirname(self.agent.conf.metadata_proxy_socket), 0o555))
         server.start(MetadataFakeProxyHandler(status),
                      self.agent.conf.metadata_proxy_socket,
-                     workers=0, backlog=4096)
+                     workers=0, backlog=4096, mode=self.SOCKET_MODE)
 
     def test_access_to_metadata_proxy(self):
         """Test access to the l3-agent metadata proxy.
@@ -830,6 +840,39 @@ class MetadataL3AgentTestCase(L3AgentTestFramework):
         self.assertIn(str(webob.exc.HTTPOk.code), firstline.split())
 
 
+class UnprivilegedUserMetadataL3AgentTestCase(MetadataL3AgentTestCase):
+    """Test metadata proxy with least privileged user.
+
+    The least privileged user has uid=65534 and is commonly named 'nobody' but
+    not always, that's why we use its uid.
+    """
+
+    SOCKET_MODE = 0o664
+
+    def setUp(self):
+        super(UnprivilegedUserMetadataL3AgentTestCase, self).setUp()
+        self.agent.conf.set_override('metadata_proxy_user', '65534')
+        self.agent.conf.set_override('metadata_proxy_watch_log', False)
+
+
+class UnprivilegedUserGroupMetadataL3AgentTestCase(MetadataL3AgentTestCase):
+    """Test metadata proxy with least privileged user/group.
+
+    The least privileged user has uid=65534 and is commonly named 'nobody' but
+    not always, that's why we use its uid.
+    Its group has gid=65534 and is commonly named 'nobody' or 'nogroup', that's
+    why we use its gid.
+    """
+
+    SOCKET_MODE = 0o666
+
+    def setUp(self):
+        super(UnprivilegedUserGroupMetadataL3AgentTestCase, self).setUp()
+        self.agent.conf.set_override('metadata_proxy_user', '65534')
+        self.agent.conf.set_override('metadata_proxy_group', '65534')
+        self.agent.conf.set_override('metadata_proxy_watch_log', False)
+
+
 class TestDvrRouter(L3AgentTestFramework):
     def test_dvr_router_lifecycle_without_ha_without_snat_with_fips(self):
         self._dvr_router_lifecycle(enable_ha=False, enable_snat=False)
diff --git a/neutron/tests/unit/agent/linux/test_utils.py b/neutron/tests/unit/agent/linux/test_utils.py
index 709a65e0f0..a1e1b85f2e 100644
--- a/neutron/tests/unit/agent/linux/test_utils.py
+++ b/neutron/tests/unit/agent/linux/test_utils.py
@@ -215,6 +215,11 @@ class FakeUser(object):
         self.pw_name = name
 
 
+class FakeGroup(object):
+    def __init__(self, name):
+        self.gr_name = name
+
+
 class TestBaseOSUtils(base.BaseTestCase):
 
     EUID = 123
@@ -264,6 +269,34 @@ class TestBaseOSUtils(base.BaseTestCase):
         geteuid.assert_called_once_with()
         getpwuid.assert_called_once_with(self.EUID)
 
+    @mock.patch('os.getegid', return_value=EGID)
+    @mock.patch('grp.getgrgid', return_value=FakeGroup(EGNAME))
+    def test_is_effective_group_id(self, getgrgid, getegid):
+        self.assertTrue(utils.is_effective_group(self.EGID))
+        getegid.assert_called_once_with()
+        self.assertFalse(getgrgid.called)
+
+    @mock.patch('os.getegid', return_value=EGID)
+    @mock.patch('grp.getgrgid', return_value=FakeGroup(EGNAME))
+    def test_is_effective_group_str_id(self, getgrgid, getegid):
+        self.assertTrue(utils.is_effective_group(str(self.EGID)))
+        getegid.assert_called_once_with()
+        self.assertFalse(getgrgid.called)
+
+    @mock.patch('os.getegid', return_value=EGID)
+    @mock.patch('grp.getgrgid', return_value=FakeGroup(EGNAME))
+    def test_is_effective_group_name(self, getgrgid, getegid):
+        self.assertTrue(utils.is_effective_group(self.EGNAME))
+        getegid.assert_called_once_with()
+        getgrgid.assert_called_once_with(self.EGID)
+
+    @mock.patch('os.getegid', return_value=EGID)
+    @mock.patch('grp.getgrgid', return_value=FakeGroup(EGNAME))
+    def test_is_not_effective_group(self, getgrgid, getegid):
+        self.assertFalse(utils.is_effective_group('wrong'))
+        getegid.assert_called_once_with()
+        getgrgid.assert_called_once_with(self.EGID)
+
 
 class TestUnixDomainHttpConnection(base.BaseTestCase):
     def test_connect(self):
diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py
index 864c1e9a94..950ad91082 100644
--- a/neutron/tests/unit/agent/metadata/test_driver.py
+++ b/neutron/tests/unit/agent/metadata/test_driver.py
@@ -23,6 +23,7 @@ from neutron.agent.common import config as agent_config
 from neutron.agent.l3 import agent as l3_agent
 from neutron.agent.l3 import config as l3_config
 from neutron.agent.l3 import ha as l3_ha_agent
+from neutron.agent.metadata import config
 from neutron.agent.metadata import driver as metadata_driver
 from neutron.openstack.common import uuidutils
 from neutron.tests import base
@@ -77,7 +78,8 @@ class TestMetadataDriverProcess(base.BaseTestCase):
 
         cfg.CONF.register_opts(l3_config.OPTS)
         cfg.CONF.register_opts(l3_ha_agent.OPTS)
-        cfg.CONF.register_opts(metadata_driver.MetadataDriver.OPTS)
+        cfg.CONF.register_opts(config.SHARED_OPTS)
+        cfg.CONF.register_opts(config.DRIVER_OPTS)
 
     def _test_spawn_metadata_proxy(self, expected_user, expected_group,
                                    user='', group='', watch_log=True):
diff --git a/neutron/tests/unit/test_metadata_agent.py b/neutron/tests/unit/test_metadata_agent.py
index 4d14d38b73..e5bbe9b379 100644
--- a/neutron/tests/unit/test_metadata_agent.py
+++ b/neutron/tests/unit/test_metadata_agent.py
@@ -20,6 +20,7 @@ import webob
 
 from neutron.agent.linux import utils as agent_utils
 from neutron.agent.metadata import agent
+from neutron.agent.metadata import config
 from neutron.agent import metadata_agent
 from neutron.common import constants
 from neutron.common import utils
@@ -521,6 +522,7 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
         self.cfg.CONF.metadata_proxy_socket = '/the/path'
         self.cfg.CONF.metadata_workers = 0
         self.cfg.CONF.metadata_backlog = 128
+        self.cfg.CONF.metadata_proxy_socket_mode = config.USER_MODE
 
     @mock.patch.object(agent_utils, 'ensure_dir')
     def test_init_doesnot_exists(self, ensure_dir):
@@ -569,7 +571,7 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
             mock.call('neutron-metadata-agent'),
             mock.call().start(handler.return_value,
                               '/the/path', workers=0,
-                              backlog=128),
+                              backlog=128, mode=0o644),
             mock.call().wait()]
         )