openstack
/
neutron
Code Issues Proposed changes

Merge "Allow metadata proxy to log with nobody user/group"

This commit is contained in:
Jenkins 2015-04-02 11:39:27 +00:00 committed by Gerrit Code Review
parent ef53ce40df fbc2278414
commit 3f45031d68
9 changed files with 168 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
etc/neutron.conf
View File

@ -257,6 +257,14 @@ lock_path = $state_path/lock
# (if empty: agent effective group)
# metadata_proxy_group =
# 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_watch_log =
# Location of Metadata Proxy UNIX domain socket
# metadata_proxy_socket = $state_path/metadata_proxy
# =========== end of items for metadata proxy configuration ==============

4
neutron/agent/common/config.py
View File

@ -73,7 +73,7 @@ PROCESS_MONITOR_OPTS = [
]
def get_log_args(conf, log_file_name):
def get_log_args(conf, log_file_name, **kwargs):
cmd_args = []
if conf.debug:
cmd_args.append('--debug')
@ -91,6 +91,8 @@ def get_log_args(conf, log_file_name):
log_dir = os.path.dirname(conf.log_file)
if log_dir:
cmd_args.append('--log-dir=%s' % log_dir)
if kwargs.get('metadata_proxy_watch_log') is False:
cmd_args.append('--metadata_proxy_watch_log=false')
else:
if conf.use_syslog:
cmd_args.append('--use-syslog')

26
neutron/agent/linux/daemon.py
View File

@ -15,6 +15,8 @@
import atexit
import fcntl
import grp
import logging as std_logging
from logging import handlers
import os
import pwd
import signal
@ -56,6 +58,25 @@ def setgid(group_id_or_name):
raise exceptions.FailToDropPrivilegesExit(msg)
def unwatch_log():
"""Replace WatchedFileHandler handlers by FileHandler ones.
Neutron logging uses WatchedFileHandler handlers but they do not
support privileges drop, this method replaces them by FileHandler
handlers supporting privileges drop.
"""
log_root = logging.getLogger(None).logger
to_replace = [h for h in log_root.handlers
if isinstance(h, handlers.WatchedFileHandler)]
for handler in to_replace:
new_handler = std_logging.FileHandler(handler.baseFilename,
mode=handler.mode,
encoding=handler.encoding,
delay=handler.delay)
log_root.removeHandler(handler)
log_root.addHandler(new_handler)
def drop_privileges(user=None, group=None):
"""Drop privileges to user/group privileges."""
if user is None and group is None:
@ -136,7 +157,7 @@ class Daemon(object):
"""
def __init__(self, pidfile, stdin='/dev/null', stdout='/dev/null',
stderr='/dev/null', procname='python', uuid=None,
user=None, group=None):
user=None, group=None, watch_log=True):
self.stdin = stdin
self.stdout = stdout
self.stderr = stderr
@ -144,6 +165,7 @@ class Daemon(object):
self.pidfile = Pidfile(pidfile, procname, uuid)
self.user = user
self.group = group
self.watch_log = watch_log
def _fork(self):
try:
@ -206,4 +228,6 @@ class Daemon(object):
start() will call this method after the process has daemonized.
"""
if not self.watch_log:
unwatch_log()
drop_privileges(self.user, self.group)

10
neutron/agent/linux/utils.py
View File

@ -17,6 +17,7 @@ import fcntl
import glob
import httplib
import os
import pwd
import shlex
import socket
import struct
@ -334,6 +335,15 @@ def ensure_directory_exists_without_file(path):
ensure_dir(dirname)
def is_effective_user(user_id_or_name):
"""Returns True if user_id_or_name is effective user (id/name)."""
euid = os.geteuid()
if str(user_id_or_name) == str(euid):
return True
effective_user_name = pwd.getpwuid(euid).pw_name
return user_id_or_name == effective_user_name
class UnixDomainHTTPConnection(httplib.HTTPConnection):
"""Connection class for HTTP over UNIX domain socket."""
def __init__(self, host, port=None, strict=None, timeout=None,

35
neutron/agent/metadata/driver.py
View File

@ -21,6 +21,7 @@ from oslo_log import log as logging
from neutron.agent.common import config
from neutron.agent.l3 import namespaces
from neutron.agent.linux import external_process
from neutron.agent.linux import utils
from neutron.common import exceptions
from neutron.services import advanced_service
@ -47,7 +48,18 @@ class MetadataDriver(advanced_service.AdvancedService):
default='',
help=_("Group (gid or name) running metadata proxy after "
"its initialization (if empty: agent effective "
"group)"))
"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):
@ -112,10 +124,17 @@ class MetadataDriver(advanced_service.AdvancedService):
'port': port})]
@classmethod
def _get_metadata_proxy_user_group(cls, conf):
user = conf.metadata_proxy_user or os.geteuid()
group = conf.metadata_proxy_group or os.getegid()
return user, group
def _get_metadata_proxy_user_group_watchlog(cls, conf):
user = conf.metadata_proxy_user or str(os.geteuid())
group = conf.metadata_proxy_group or str(os.getegid())
watch_log = conf.metadata_proxy_watch_log
if watch_log is None:
# NOTE(cbrandily): Commonly, log watching can be enabled only
# when metadata proxy user is agent effective user (id/name).
watch_log = utils.is_effective_user(user)
return user, group, watch_log
@classmethod
def _get_metadata_proxy_callback(cls, port, conf, network_id=None,
@ -131,7 +150,8 @@ class MetadataDriver(advanced_service.AdvancedService):
def callback(pid_file):
metadata_proxy_socket = conf.metadata_proxy_socket
user, group = cls._get_metadata_proxy_user_group(conf)
user, group, watch_log = (
cls._get_metadata_proxy_user_group_watchlog(conf))
proxy_cmd = ['neutron-ns-metadata-proxy',
'--pid_file=%s' % pid_file,
'--metadata_proxy_socket=%s' % metadata_proxy_socket,
@ -141,7 +161,8 @@ class MetadataDriver(advanced_service.AdvancedService):
'--metadata_proxy_user=%s' % user,
'--metadata_proxy_group=%s' % group]
proxy_cmd.extend(config.get_log_args(
conf, 'neutron-ns-metadata-proxy-%s.log' % uuid))
conf, 'neutron-ns-metadata-proxy-%s.log' % uuid,
metadata_proxy_watch_log=watch_log))
return proxy_cmd
return callback

12
neutron/agent/metadata/namespace_proxy.py
View File

@ -110,10 +110,10 @@ class NetworkMetadataProxyHandler(object):
class ProxyDaemon(daemon.Daemon):
def __init__(self, pidfile, port, network_id=None, router_id=None,
user=None, group=None):
user=None, group=None, watch_log=True):
uuid = network_id or router_id
super(ProxyDaemon, self).__init__(pidfile, uuid=uuid, user=user,
group=group)
group=group, watch_log=watch_log)
self.network_id = network_id
self.router_id = router_id
self.port = port
@ -160,6 +160,11 @@ def main():
default=None,
help=_("Group (gid or name) running metadata proxy after "
"its initialization")),
cfg.BoolOpt('metadata_proxy_watch_log',
default=True,
help=_("Watch file log. Log watch should be disabled when "
"metadata_proxy_user/group has no read/write "
"permissions on metadata proxy log file.")),
]
cfg.CONF.register_cli_opts(opts)
@ -173,7 +178,8 @@ def main():
network_id=cfg.CONF.network_id,
router_id=cfg.CONF.router_id,
user=cfg.CONF.metadata_proxy_user,
group=cfg.CONF.metadata_proxy_group)
group=cfg.CONF.metadata_proxy_group,
watch_log=cfg.CONF.metadata_proxy_watch_log)
if cfg.CONF.daemonize:
proxy.start()

39
neutron/tests/unit/agent/linux/test_utils.py
View File

@ -210,7 +210,18 @@ class TestPathUtilities(base.BaseTestCase):
self.assertFalse(utils.cmd_matches_expected('foo', 'bar'))
class FakeUser(object):
def __init__(self, name):
self.pw_name = name
class TestBaseOSUtils(base.BaseTestCase):
EUID = 123
EUNAME = 'user'
EGID = 456
EGNAME = 'group'
@mock.patch.object(os.path, 'isdir', return_value=False)
@mock.patch.object(os, 'makedirs')
def test_ensure_dir_not_exist(self, makedirs, isdir):
@ -225,6 +236,34 @@ class TestBaseOSUtils(base.BaseTestCase):
isdir.assert_called_once_with('/the')
self.assertFalse(makedirs.called)
@mock.patch('os.geteuid', return_value=EUID)
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
def test_is_effective_user_id(self, getpwuid, geteuid):
self.assertTrue(utils.is_effective_user(self.EUID))
geteuid.assert_called_once_with()
self.assertFalse(getpwuid.called)
@mock.patch('os.geteuid', return_value=EUID)
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
def test_is_effective_user_str_id(self, getpwuid, geteuid):
self.assertTrue(utils.is_effective_user(str(self.EUID)))
geteuid.assert_called_once_with()
self.assertFalse(getpwuid.called)
@mock.patch('os.geteuid', return_value=EUID)
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
def test_is_effective_user_name(self, getpwuid, geteuid):
self.assertTrue(utils.is_effective_user(self.EUNAME))
geteuid.assert_called_once_with()
getpwuid.assert_called_once_with(self.EUID)
@mock.patch('os.geteuid', return_value=EUID)
@mock.patch('pwd.getpwuid', return_value=FakeUser(EUNAME))
def test_is_not_effective_user(self, getpwuid, geteuid):
self.assertFalse(utils.is_effective_user('wrong'))
geteuid.assert_called_once_with()
getpwuid.assert_called_once_with(self.EUID)
class TestUnixDomainHttpConnection(base.BaseTestCase):
def test_connect(self):

65
neutron/tests/unit/agent/metadata/test_driver.py
View File

@ -61,6 +61,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
EUID = 123
EGID = 456
EUNAME = 'neutron'
def setUp(self):
super(TestMetadataDriverProcess, self).setUp()
@ -79,11 +80,13 @@ class TestMetadataDriverProcess(base.BaseTestCase):
cfg.CONF.register_opts(metadata_driver.MetadataDriver.OPTS)
def _test_spawn_metadata_proxy(self, expected_user, expected_group,
user='', group=''):
user='', group='', watch_log=True):
router_id = _uuid()
router_ns = 'qrouter-%s' % router_id
metadata_port = 8080
ip_class_path = 'neutron.agent.linux.ip_lib.IPWrapper'
is_effective_user = 'neutron.agent.linux.utils.is_effective_user'
fake_is_effective_user = lambda x: x in [self.EUNAME, str(self.EUID)]
cfg.CONF.set_override('metadata_proxy_user', user)
cfg.CONF.set_override('metadata_proxy_group', group)
@ -94,42 +97,58 @@ class TestMetadataDriverProcess(base.BaseTestCase):
with contextlib.nested(
mock.patch('os.geteuid', return_value=self.EUID),
mock.patch('os.getegid', return_value=self.EGID),
mock.patch(ip_class_path)) as (geteuid, getegid, ip_mock):
mock.patch(is_effective_user,
side_effect=fake_is_effective_user),
mock.patch(ip_class_path)) as (
geteuid, getegid, is_effective_user, ip_mock):
agent.metadata_driver.spawn_monitored_metadata_proxy(
agent.process_monitor,
router_ns,
metadata_port,
agent.conf,
router_id=router_id)
netns_execute_args = [
'neutron-ns-metadata-proxy',
mock.ANY,
mock.ANY,
'--router_id=%s' % router_id,
mock.ANY,
'--metadata_port=%s' % metadata_port,
'--metadata_proxy_user=%s' % expected_user,
'--metadata_proxy_group=%s' % expected_group,
'--debug',
'--verbose',
'--log-file=neutron-ns-metadata-proxy-%s.log' %
router_id]
if not watch_log:
netns_execute_args.append(
'--metadata_proxy_watch_log=false')
ip_mock.assert_has_calls([
mock.call(namespace=router_ns),
mock.call().netns.execute([
'neutron-ns-metadata-proxy',
mock.ANY,
mock.ANY,
'--router_id=%s' % router_id,
mock.ANY,
'--metadata_port=%s' % metadata_port,
'--metadata_proxy_user=%s' % expected_user,
'--metadata_proxy_group=%s' % expected_group,
'--debug',
'--verbose',
'--log-file=neutron-ns-metadata-proxy-%s.log' %
router_id
], addl_env=None)
mock.call().netns.execute(netns_execute_args, addl_env=None)
])
def test_spawn_metadata_proxy_with_user(self):
self._test_spawn_metadata_proxy('user', self.EGID, user='user')
def test_spawn_metadata_proxy_with_agent_user(self):
self._test_spawn_metadata_proxy(
self.EUNAME, str(self.EGID), user=self.EUNAME)
def test_spawn_metadata_proxy_with_uid(self):
self._test_spawn_metadata_proxy('321', self.EGID, user='321')
def test_spawn_metadata_proxy_with_nonagent_user(self):
self._test_spawn_metadata_proxy(
'notneutron', str(self.EGID), user='notneutron', watch_log=False)
def test_spawn_metadata_proxy_with_agent_uid(self):
self._test_spawn_metadata_proxy(
str(self.EUID), str(self.EGID), user=str(self.EUID))
def test_spawn_metadata_proxy_with_nonagent_uid(self):
self._test_spawn_metadata_proxy(
'321', str(self.EGID), user='321', watch_log=False)
def test_spawn_metadata_proxy_with_group(self):
self._test_spawn_metadata_proxy(self.EUID, 'group', group='group')
self._test_spawn_metadata_proxy(str(self.EUID), 'group', group='group')
def test_spawn_metadata_proxy_with_gid(self):
self._test_spawn_metadata_proxy(self.EUID, '654', group='654')
self._test_spawn_metadata_proxy(str(self.EUID), '654', group='654')
def test_spawn_metadata_proxy(self):
self._test_spawn_metadata_proxy(self.EUID, self.EGID)
self._test_spawn_metadata_proxy(str(self.EUID), str(self.EGID))

6
neutron/tests/unit/test_metadata_namespace_proxy.py
View File

@ -292,7 +292,8 @@ class TestProxyDaemon(base.BaseTestCase):
router_id='router_id',
network_id=None,
user=mock.ANY,
group=mock.ANY),
group=mock.ANY,
watch_log=mock.ANY),
mock.call().start()]
)
@ -315,6 +316,7 @@ class TestProxyDaemon(base.BaseTestCase):
router_id='router_id',
network_id=None,
user=mock.ANY,
group=mock.ANY),
group=mock.ANY,
watch_log=mock.ANY),
mock.call().run()]
)