openstack/neutron
Code Issues Proposed changes

Do not run neutron-ns-metadata-proxy as root on dhcp agent

Browse Source 
Currently neutron-ns-metadata-proxy runs with root permissions when
namespaces are enabled on the dhcp agent because root permissions are
required to "enter" in the namespace. But neutron-ns-metadata-proxy
permissions should be reduced as much as possible because it is
reachable from vms.

This change allows to change neutron-ns-metadata-proxy permissions
after its startup through the 2 new options metadata_proxy_user and
metadata_proxy_group which allow to define user/group running metadata
proxy after its initialization. Their default values are
neutron-dhcp-agent effective user and group.

This change delegates metadata proxy management to metadata driver
methods in order to reuse the work already done on l3 agent side.

Permissions drop is done after metadata proxy daemon writes its
pid in its pidfile (it could be disallowed after permissions drop) and
after metadata proxy daemon binds its privileged server port (80).

Using nobody as metadata_proxy_user/group (more secure) is currently
not supported because:

* nobody has not the permission to connect the metadata socket,
* nobody has not the permission to log to file because neutron uses
  WatchedFileHandler (which requires read/write permissions after
  permissions drop).
This limitation will be addressed in a daughter change.

DocImpact
Closes-Bug: #1187107
Change-Id: I53e97254d560e608101010f67bd2dcdec81fb6a2
This commit is contained in:
Cedric Brandily 2015-01-07 23:12:20 +00:00
parent 1948efa261
commit ac6cf68517
12 changed files with 101 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
etc/dhcp_agent.ini
View File

@ -75,6 +75,14 @@
# Use broadcast in DHCP replies
# dhcp_broadcast_reply = False
# User (uid or name) running metadata proxy after its initialization
# (if empty: dhcp agent effective user)
# metadata_proxy_user =
# Group (gid or name) running metadata proxy after its initialization
# (if empty: dhcp agent effective group)
# metadata_proxy_group =
# Location of Metadata Proxy UNIX domain socket
# metadata_proxy_socket = $state_path/metadata_proxy

29
neutron/agent/dhcp/agent.py
View File

@ -22,9 +22,9 @@ from oslo_config import cfg
import oslo_messaging
from oslo_utils import importutils
from neutron.agent.common import config
from neutron.agent.linux import dhcp
from neutron.agent.linux import external_process
from neutron.agent.metadata import driver as metadata_driver
from neutron.agent import rpc as agent_rpc
from neutron.common import constants
from neutron.common import exceptions
@ -336,7 +336,7 @@ class DhcpAgent(manager.Manager):
# The proxy might work for either a single network
# or all the networks connected via a router
# to the one passed as a parameter
neutron_lookup_param = '--network_id=%s' % network.id
kwargs = {'network_id': network.id}
# When the metadata network is enabled, the proxy might
# be started for the router attached to the network
if self.conf.enable_metadata_network:
@ -353,28 +353,15 @@ class DhcpAgent(manager.Manager):
{'port_num': len(router_ports),
'port_id': router_ports[0].id,
'router_id': router_ports[0].device_id})
neutron_lookup_param = ('--router_id=%s' %
router_ports[0].device_id)
kwargs = {'router_id': router_ports[0].device_id}
def callback(pid_file):
metadata_proxy_socket = cfg.CONF.metadata_proxy_socket
proxy_cmd = ['neutron-ns-metadata-proxy',
'--pid_file=%s' % pid_file,
'--metadata_proxy_socket=%s' % metadata_proxy_socket,
neutron_lookup_param,
'--state_path=%s' % self.conf.state_path,
'--metadata_port=%d' % dhcp.METADATA_PORT]
proxy_cmd.extend(config.get_log_args(
cfg.CONF, 'neutron-ns-metadata-proxy-%s.log' % network.id))
return proxy_cmd
self._process_monitor.enable(uuid=network.id,
cmd_callback=callback,
namespace=network.namespace)
metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy(
self._process_monitor, network.namespace, dhcp.METADATA_PORT,
self.conf, **kwargs)
def disable_isolated_metadata_proxy(self, network):
self._process_monitor.disable(uuid=network.id,
namespace=network.namespace)
metadata_driver.MetadataDriver.destroy_monitored_metadata_proxy(
self._process_monitor, network.id, network.namespace)
class DhcpPluginApi(object):

6
neutron/agent/dhcp/config.py
View File

@ -29,11 +29,7 @@ DHCP_AGENT_OPTS = [
"dedicated network. Requires "
"enable_isolated_metadata = True")),
cfg.IntOpt('num_sync_threads', default=4,
help=_('Number of threads to use during sync process.')),
cfg.StrOpt('metadata_proxy_socket',
default='$state_path/metadata_proxy',
help=_('Location of Metadata Proxy UNIX domain '
'socket')),
help=_('Number of threads to use during sync process.'))
]
DHCP_OPTS = [

2
neutron/agent/dhcp_agent.py
View File

@ -21,6 +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.common import config as common_config
from neutron.common import topics
from neutron.openstack.common import service
@ -34,6 +35,7 @@ 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(interface.OPTS)

3
neutron/agent/l3/ha_router.py
View File

@ -110,7 +110,8 @@ class HaRouter(router.RouterInfo):
def _add_keepalived_notifiers(self):
callback = (
metadata_driver.MetadataDriver._get_metadata_proxy_callback(
self.router_id, self.agent_conf))
self.agent_conf.metadata_port, self.agent_conf,
router_id=self.router_id))
# TODO(mangelajo): use the process monitor in keepalived when
# keepalived stops killing/starting metadata
# proxy on its own

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

@ -19,6 +19,7 @@ from oslo_config import cfg
from neutron.agent.common import config
from neutron.agent.linux import external_process
from neutron.common import exceptions
from neutron.openstack.common import log as logging
from neutron.services import advanced_service
@ -38,12 +39,12 @@ class MetadataDriver(advanced_service.AdvancedService):
cfg.StrOpt('metadata_proxy_user',
default='',
help=_("User (uid or name) running metadata proxy after "
"its initialization (if empty: L3 agent effective "
"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: L3 agent effective "
"its initialization (if empty: agent effective "
"group)"))
]
@ -63,8 +64,12 @@ class MetadataDriver(advanced_service.AdvancedService):
router.iptables_manager.apply()
if not router.is_ha:
self._spawn_monitored_metadata_proxy(router.router_id,
router.ns_name)
self.spawn_monitored_metadata_proxy(
self.l3_agent.process_monitor,
router.ns_name,
self.metadata_port,
self.l3_agent.conf,
router_id=router.router_id)
def before_router_removed(self, router):
for c, r in self.metadata_filter_rules(self.metadata_port,
@ -76,7 +81,8 @@ class MetadataDriver(advanced_service.AdvancedService):
router.iptables_manager.ipv4['nat'].remove_rule(c, r)
router.iptables_manager.apply()
self._destroy_monitored_metadata_proxy(router.router['id'],
self.destroy_monitored_metadata_proxy(self.l3_agent.process_monitor,
router.router['id'],
router.ns_name)
@classmethod
@ -106,7 +112,16 @@ class MetadataDriver(advanced_service.AdvancedService):
return user, group
@classmethod
def _get_metadata_proxy_callback(cls, router_id, conf):
def _get_metadata_proxy_callback(cls, port, conf, network_id=None,
router_id=None):
uuid = network_id or router_id
if uuid is None:
raise exceptions.NetworkIdOrRouterIdRequiredError()
if network_id:
lookup_param = '--network_id=%s' % network_id
else:
lookup_param = '--router_id=%s' % router_id
def callback(pid_file):
metadata_proxy_socket = conf.metadata_proxy_socket
@ -114,25 +129,27 @@ class MetadataDriver(advanced_service.AdvancedService):
proxy_cmd = ['neutron-ns-metadata-proxy',
'--pid_file=%s' % pid_file,
'--metadata_proxy_socket=%s' % metadata_proxy_socket,
'--router_id=%s' % router_id,
lookup_param,
'--state_path=%s' % conf.state_path,
'--metadata_port=%s' % conf.metadata_port,
'--metadata_port=%s' % port,
'--metadata_proxy_user=%s' % user,
'--metadata_proxy_group=%s' % group]
proxy_cmd.extend(config.get_log_args(
conf, 'neutron-ns-metadata-proxy-%s.log' %
router_id))
conf, 'neutron-ns-metadata-proxy-%s.log' % uuid))
return proxy_cmd
return callback
def _spawn_monitored_metadata_proxy(self, router_id, ns_name):
callback = self._get_metadata_proxy_callback(
router_id, self.l3_agent.conf)
self.l3_agent.process_monitor.enable(router_id, callback, ns_name)
@classmethod
def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,
network_id=None, router_id=None):
callback = cls._get_metadata_proxy_callback(
port, conf, network_id=network_id, router_id=router_id)
monitor.enable(network_id or router_id, callback, ns_name)
def _destroy_monitored_metadata_proxy(self, router_id, ns_name):
self.l3_agent.process_monitor.disable(router_id, ns_name)
@classmethod
def destroy_monitored_metadata_proxy(cls, monitor, uuid, ns_name):
monitor.disable(uuid, ns_name)
# TODO(mangelajo): remove the unmonitored _get_*_process_manager,
# _spawn_* and _destroy* when keepalived stops
@ -145,12 +162,13 @@ class MetadataDriver(advanced_service.AdvancedService):
ns_name)
@classmethod
def _spawn_metadata_proxy(cls, router_id, ns_name, conf):
callback = cls._get_metadata_proxy_callback(router_id, conf)
def spawn_metadata_proxy(cls, router_id, ns_name, port, conf):
callback = cls._get_metadata_proxy_callback(port, conf,
router_id=router_id)
pm = cls._get_metadata_proxy_process_manager(router_id, ns_name, conf)
pm.enable(callback)
@classmethod
def _destroy_metadata_proxy(cls, router_id, ns_name, conf):
def destroy_metadata_proxy(cls, router_id, ns_name, conf):
pm = cls._get_metadata_proxy_process_manager(router_id, ns_name, conf)
pm.disable()

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

@ -22,6 +22,7 @@ import webob
from neutron.agent.linux import daemon
from neutron.common import config
from neutron.common import exceptions
from neutron.common import utils
from neutron.i18n import _LE
from neutron.openstack.common import log as logging
@ -56,8 +57,7 @@ class NetworkMetadataProxyHandler(object):
self.router_id = router_id
if network_id is None and router_id is None:
msg = _('network_id and router_id are None. One must be provided.')
raise ValueError(msg)
raise exceptions.NetworkIdOrRouterIdRequiredError()
@webob.dec.wsgify(RequestClass=webob.Request)
def __call__(self, req):
@ -135,12 +135,15 @@ class ProxyDaemon(daemon.Daemon):
self.port = port
def run(self):
super(ProxyDaemon, self).run()
handler = NetworkMetadataProxyHandler(
self.network_id,
self.router_id)
proxy = wsgi.Server('neutron-network-metadata-proxy')
proxy.start(handler, self.port)
# Drop privileges after port bind
super(ProxyDaemon, self).run()
proxy.wait()

4
neutron/common/exceptions.py
View File

@ -369,6 +369,10 @@ class IpTablesApplyException(NeutronException):
super(IpTablesApplyException, self).__init__()
class NetworkIdOrRouterIdRequiredError(NeutronException):
message = _('network_id and router_id are None. One must be provided.')
# Shared *aas exceptions, pending them being refactored out of Neutron
# proper.

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

@ -68,7 +68,6 @@ class TestMetadataDriver(base.BaseTestCase):
metadata_port = 8080
ip_class_path = 'neutron.agent.linux.ip_lib.IPWrapper'
cfg.CONF.set_override('metadata_port', metadata_port)
cfg.CONF.set_override('metadata_proxy_user', user)
cfg.CONF.set_override('metadata_proxy_group', group)
cfg.CONF.set_override('log_file', 'test.log')
@ -79,7 +78,8 @@ class TestMetadataDriver(base.BaseTestCase):
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):
driver._spawn_metadata_proxy(router_id, router_ns, cfg.CONF)
driver.spawn_metadata_proxy(router_id, router_ns, metadata_port,
cfg.CONF)
ip_mock.assert_has_calls([
mock.call(namespace=router_ns),
mock.call().netns.execute([

34
neutron/tests/unit/test_dhcp_agent.py
View File

@ -751,34 +751,28 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
])
def test_disable_isolated_metadata_proxy(self):
method_path = ('neutron.agent.metadata.driver.MetadataDriver'
'.destroy_monitored_metadata_proxy')
with mock.patch(method_path) as destroy:
self.dhcp.disable_isolated_metadata_proxy(fake_network)
self.external_process.assert_has_calls([
self._process_manager_constructor_call(),
mock.call().disable()
])
destroy.assert_called_once_with(self.dhcp._process_monitor,
fake_network.id,
fake_network.namespace)
def _test_metadata_network(self, network):
cfg.CONF.set_override('enable_metadata_network', True)
cfg.CONF.set_override('debug', True)
cfg.CONF.set_override('verbose', False)
cfg.CONF.set_override('log_file', 'test.log')
method_path = ('neutron.agent.metadata.driver.MetadataDriver'
'.spawn_monitored_metadata_proxy')
with mock.patch(method_path) as spawn:
self.dhcp.enable_isolated_metadata_proxy(network)
self.external_process.assert_has_calls([
self._process_manager_constructor_call()])
callback = self.external_process.call_args[1]['default_cmd_callback']
result_cmd = callback('pidfile')
self.assertEqual(
result_cmd,
['neutron-ns-metadata-proxy',
'--pid_file=pidfile',
'--metadata_proxy_socket=%s' % cfg.CONF.metadata_proxy_socket,
'--router_id=forzanapoli',
'--state_path=%s' % cfg.CONF.state_path,
'--metadata_port=%d' % dhcp.METADATA_PORT,
'--debug',
'--log-file=neutron-ns-metadata-proxy-%s.log' % network.id])
spawn.assert_called_once_with(self.dhcp._process_monitor,
network.namespace,
dhcp.METADATA_PORT,
cfg.CONF,
router_id='forzanapoli')
def test_enable_isolated_metadata_proxy_with_metadata_network(self):
self._test_metadata_network(fake_meta_network)

16
neutron/tests/unit/test_l3_agent.py
View File

@ -1608,18 +1608,24 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
'distributed': False}
driver = metadata_driver.MetadataDriver
with mock.patch.object(
driver, '_destroy_monitored_metadata_proxy') as destroy_proxy:
driver, 'destroy_monitored_metadata_proxy') as destroy_proxy:
with mock.patch.object(
driver, '_spawn_monitored_metadata_proxy') as spawn_proxy:
driver, 'spawn_monitored_metadata_proxy') as spawn_proxy:
agent._process_added_router(router)
if enableflag:
spawn_proxy.assert_called_with(router_id,
mock.ANY)
spawn_proxy.assert_called_with(
mock.ANY,
mock.ANY,
self.conf.metadata_port,
mock.ANY,
router_id=router_id
)
else:
self.assertFalse(spawn_proxy.call_count)
agent._router_removed(router_id)
if enableflag:
destroy_proxy.assert_called_with(router_id,
destroy_proxy.assert_called_with(mock.ANY,
router_id,
mock.ANY)
else:
self.assertFalse(destroy_proxy.call_count)

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

@ -19,6 +19,7 @@ import testtools
import webob
from neutron.agent.metadata import namespace_proxy as ns_proxy
from neutron.common import exceptions
from neutron.common import utils
from neutron.tests import base
@ -75,7 +76,8 @@ class TestNetworkMetadataProxyHandler(base.BaseTestCase):
req.body)
def test_no_argument_passed_to_init(self):
with testtools.ExpectedException(ValueError):
with testtools.ExpectedException(
exceptions.NetworkIdOrRouterIdRequiredError):
ns_proxy.NetworkMetadataProxyHandler()
def test_call_internal_server_error(self):