From 80bea7a38670620934faafd5f583fe6164b9f9b3 Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Tue, 17 Mar 2015 15:20:07 +0000 Subject: [PATCH] Allow metadata proxy running with nobody user/group Currently metadata proxy cannot run with nobody user/group as metadata proxy requires to connect to metadata_proxy_socket when queried. This change allows to run metadata proxy with nobody user/group by allowing to choose the metadata_proxy_socket mode with the new option metadata_proxy_socket_mode (4 choices) in order to adapt socket permissions to metadata proxy user/group. This change refactors also where options are defined to enable metadata_proxy_user/group options in the metadata agent. In practice: * if metadata_proxy_user is agent effective user or root, then: * metadata proxy is allowed to use rootwrap (unsecure) * set metadata_proxy_socket_mode = user (0o644) * else if metadata_proxy_group is agent effective group, then: * metadata proxy is not allowed to use rootwrap (secure) * set metadata_proxy_socket_mode = group (0o664) * set metadata_proxy_log_watch = false * else: * metadata proxy has lowest permissions (securest) but metadata proxy socket can be opened by everyone * set metadata_proxy_socket_mode = all (0o666) * set metadata_proxy_log_watch = false An alternative is to set metadata_proxy_socket_mode = deduce, in such case metadata agent uses previous rules to choose the correct mode. DocImpact Closes-Bug: #1427228 Change-Id: I235a0cc4f0cbd55ae4ec1570daf2ebbb6a72441d --- etc/metadata_agent.ini | 9 +++ neutron/agent/dhcp_agent.py | 5 +- neutron/agent/l3_agent.py | 5 +- neutron/agent/linux/utils.py | 14 ++++- neutron/agent/metadata/agent.py | 28 +++++++++- neutron/agent/metadata/config.py | 56 ++++++++++++++++++- neutron/agent/metadata/driver.py | 29 ---------- neutron/agent/metadata_agent.py | 1 + .../tests/functional/agent/linux/helpers.py | 29 ++++++++++ .../tests/functional/agent/test_l3_agent.py | 45 ++++++++++++++- neutron/tests/unit/agent/linux/test_utils.py | 33 +++++++++++ .../tests/unit/agent/metadata/test_driver.py | 4 +- neutron/tests/unit/test_metadata_agent.py | 4 +- 13 files changed, 221 insertions(+), 41 deletions(-) diff --git a/etc/metadata_agent.ini b/etc/metadata_agent.ini index 06588a514b8..4a0331ee125 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 afb71da6e65..8b7bbae31ad 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 ef912646039..12c152d0536 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 ca9bfba722a..548b4cddb92 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 062acbaa004..e2cad9c9ace 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 2ac2642d6c8..be8d6b79219 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 411a6b7c466..437e5efe0b2 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 8c06a37f976..b6e27914bd1 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 1dfd591c82f..1e51a9b81c0 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\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 0767b068d3c..6bfb11a7408 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 709a65e0f0c..a1e1b85f2e9 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 864c1e9a94f..950ad910821 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 4d14d38b73d..e5bbe9b3791 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()] )