openstack
/
neutron
Code Issues Proposed changes

Allow metadata proxy to log with nobody user/group 

Currently metadata proxy cannot run with nobody user/group as
metadata proxy (as other services) uses WatchedFileHandler handler to
log to file which does not support permissions drop (the process must
be able to r/w after permissions drop to "watch" the file).

This change allows to enable/disable log watch in metadata proxies with
the new option metadata_proxy_log_watch. It should be disabled when
metadata_proxy_user/group is not allowed to read/write metadata proxy
log files. Option default value is deduced from metadata_proxy_user:

* True if metadata_proxy_user is agent effective user id/name,
* False otherwise.

When log watch is disabled and logrotate is enabled on metadata proxy
logging files, 'copytruncate' logrotate option must be used otherwise
metadata proxy logs will be lost after the first log rotation.

DocImpact
Change-Id: I40a7bd82a2c60d9198312fdb52e3010c60db3511
Partial-Bug: #1427228
This commit is contained in:
Cedric Brandily 2015-03-03 22:26:52 +00:00
parent 1a089e6059
commit fbc2278414
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

@ -239,6 +239,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

@ -209,7 +209,18 @@ class TestPathUtilities(base.BaseTestCase):
['/usr/bin/ping6', '8.8.8.8']))
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):
@ -224,6 +235,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()]
)