Switch isolated metadata proxy to bind to 169.254.169.254

Currently the metadata proxy binds to default 0.0.0.0, which does not
add any advantage (metadata requests are not sent to random IP
addresses), and may allow access to cloud information from
third parties.

This changes the generated configuration to bind to METADATA_DEFAULT_IP
address instead.

This is not enabled in other metadata proxy configuration (in the L3
agent), as this would require net.ipv4.ip_nonlocal_bind everywhere
(currently only enabled for DVR) or transparent mode in haproxy (which
requires net.ipv4.ip_nonlocal_bind anyway)

Changed set_ip_nonlocal_bind_for_namespace() to support setting the
value in both the given and root namespace correctly, since it was
only used from inside the neutron codebase according to codesearch.

Change-Id: I388391cf697dade1a163d15ab568b33134f7b2d9
Co-Authored-By: Andrey Arapov <andrey.arapov@nixaid.com>
Closes-Bug: #1745618
This commit is contained in:
Bernard Cafarelli 2018-09-06 10:48:13 +02:00
parent 8914f8247f
commit 6124f60297
10 changed files with 45 additions and 28 deletions

View File

@ -657,7 +657,7 @@ class DhcpAgent(manager.Manager):
metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy(
self._process_monitor, network.namespace, dhcp.METADATA_PORT,
self.conf, **kwargs)
self.conf, bind_address=dhcp.METADATA_DEFAULT_IP, **kwargs)
def disable_isolated_metadata_proxy(self, network):
if (self.conf.enable_metadata_network and

View File

@ -208,16 +208,8 @@ class FipNamespace(namespaces.Namespace):
LOG.debug("DVR: add fip namespace: %s", self.name)
# parent class will ensure the namespace exists and turn-on forwarding
super(FipNamespace, self).create()
# Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was
# changed to be a per-namespace attribute. To be backwards
# compatible we need to try both if at first we fail.
failed = ip_lib.set_ip_nonlocal_bind(
value=1, namespace=self.name, log_fail_as_error=False)
if failed:
LOG.debug('DVR: fip namespace (%s) does not support setting '
'net.ipv4.ip_nonlocal_bind, trying in root namespace',
self.name)
ip_lib.set_ip_nonlocal_bind(value=1)
ip_lib.set_ip_nonlocal_bind_for_namespace(self.name, 1,
root_namespace=True)
# no connection tracking needed in fip namespace
self._iptables_manager.ipv4['raw'].add_rule('PREROUTING',

View File

@ -32,7 +32,7 @@ class SnatNamespace(namespaces.Namespace):
super(SnatNamespace, self).create()
# This might be an HA router namespaces and it should not have
# ip_nonlocal_bind enabled
ip_lib.set_ip_nonlocal_bind_for_namespace(self.name)
ip_lib.set_ip_nonlocal_bind_for_namespace(self.name, 0)
# Set nf_conntrack_tcp_loose to 0 to ensure mid-stream
# TCP conversations aren't taken over by SNAT
cmd = ['net.netfilter.nf_conntrack_tcp_loose=0']

View File

@ -57,7 +57,7 @@ class HaRouterNamespace(namespaces.RouterNamespace):
def create(self):
super(HaRouterNamespace, self).create(ipv6_forwarding=False)
# HA router namespaces should not have ip_nonlocal_bind enabled
ip_lib.set_ip_nonlocal_bind_for_namespace(self.name)
ip_lib.set_ip_nonlocal_bind_for_namespace(self.name, 0)
class HaRouter(router.RouterInfo):

View File

@ -1486,6 +1486,8 @@ class DeviceManager(object):
# and added back statically in the call to init_l3() below.
if network.namespace:
ip_lib.IPWrapper().ensure_namespace(network.namespace)
ip_lib.set_ip_nonlocal_bind_for_namespace(network.namespace, 1,
root_namespace=True)
if ipv6_utils.is_enabled_and_bind_by_default():
self.driver.configure_ipv6_ra(network.namespace, 'default',
n_const.ACCEPT_RA_DISABLED)

View File

@ -1175,18 +1175,25 @@ def set_ip_nonlocal_bind(value, namespace=None, log_fail_as_error=True):
log_fail_as_error=log_fail_as_error)
def set_ip_nonlocal_bind_for_namespace(namespace):
def set_ip_nonlocal_bind_for_namespace(namespace, value, root_namespace=False):
"""Set ip_nonlocal_bind but don't raise exception on failure."""
failed = set_ip_nonlocal_bind(value=0, namespace=namespace,
failed = set_ip_nonlocal_bind(value, namespace=namespace,
log_fail_as_error=False)
if failed and root_namespace:
# Somewhere in the 3.19 kernel timeframe ip_nonlocal_bind was
# changed to be a per-namespace attribute. To be backwards
# compatible we need to try both if at first we fail.
LOG.debug('Namespace (%s) does not support setting %s, '
'trying in root namespace', namespace, IP_NONLOCAL_BIND)
return set_ip_nonlocal_bind(value)
if failed:
LOG.warning(
"%s will not be set to 0 in the root namespace in order to "
"%s will not be set to %d in the root namespace in order to "
"not break DVR, which requires this value be set to 1. This "
"may introduce a race between moving a floating IP to a "
"different network node, and the peer side getting a "
"populated ARP cache for a given floating IP address.",
IP_NONLOCAL_BIND)
IP_NONLOCAL_BIND, value)
def get_ipv6_forwarding(device, namespace=None):

View File

@ -61,7 +61,7 @@ defaults
timeout http-keep-alive 30s
listen listener
bind 0.0.0.0:%(port)s
bind %(host)s:%(port)s
server metadata %(unix_socket_path)s
http-request add-header X-Neutron-%(res_type)s-ID %(res_id)s
"""
@ -72,13 +72,14 @@ class InvalidUserOrGroupException(Exception):
class HaproxyConfigurator(object):
def __init__(self, network_id, router_id, unix_socket_path, port, user,
group, state_path, pid_file):
def __init__(self, network_id, router_id, unix_socket_path, host, port,
user, group, state_path, pid_file):
self.network_id = network_id
self.router_id = router_id
if network_id is None and router_id is None:
raise exceptions.NetworkIdOrRouterIdRequiredError()
self.host = host
self.port = port
self.user = user
self.group = group
@ -116,6 +117,7 @@ class HaproxyConfigurator(object):
_("Invalid group/gid: '%s'") % self.group)
cfg_info = {
'host': self.host,
'port': self.port,
'unix_socket_path': self.unix_socket_path,
'user': username,
@ -209,8 +211,8 @@ class MetadataDriver(object):
return user, group
@classmethod
def _get_metadata_proxy_callback(cls, port, conf, network_id=None,
router_id=None):
def _get_metadata_proxy_callback(cls, bind_address, port, conf,
network_id=None, router_id=None):
def callback(pid_file):
metadata_proxy_socket = conf.metadata_proxy_socket
user, group = (
@ -218,6 +220,7 @@ class MetadataDriver(object):
haproxy = HaproxyConfigurator(network_id,
router_id,
metadata_proxy_socket,
bind_address,
port,
user,
group,
@ -232,10 +235,12 @@ class MetadataDriver(object):
@classmethod
def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,
network_id=None, router_id=None):
bind_address="0.0.0.0", network_id=None,
router_id=None):
uuid = network_id or router_id
callback = cls._get_metadata_proxy_callback(
port, conf, network_id=network_id, router_id=router_id)
bind_address, port, conf, network_id=network_id,
router_id=router_id)
pm = cls._get_metadata_proxy_process_manager(uuid, conf,
ns_name=ns_name,
callback=callback)
@ -269,7 +274,7 @@ def after_router_added(resource, event, l3_agent, **kwargs):
router = kwargs['router']
proxy = l3_agent.metadata_driver
for c, r in proxy.metadata_filter_rules(proxy.metadata_port,
proxy.metadata_access_mark):
proxy.metadata_access_mark):
router.iptables_manager.ipv4['filter'].add_rule(c, r)
for c, r in proxy.metadata_nat_rules(proxy.metadata_port):
router.iptables_manager.ipv4['nat'].add_rule(c, r)

View File

@ -220,6 +220,9 @@ fake_down_network = dhcp.NetModel(
class TestDhcpAgent(base.BaseTestCase):
METADATA_DEFAULT_IP = dhcp.METADATA_DEFAULT_IP
def setUp(self):
super(TestDhcpAgent, self).setUp()
entry.register_options(cfg.CONF)
@ -494,6 +497,7 @@ class TestDhcpAgent(base.BaseTestCase):
dhcp.configure_dhcp_for_network(fake_network)
md_cls.spawn_monitored_metadata_proxy.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, mock.ANY,
bind_address=self.METADATA_DEFAULT_IP,
network_id=fake_network.id)
dhcp.disable_dhcp_helper(fake_network.id)
md_cls.destroy_monitored_metadata_proxy.assert_called_once_with(
@ -889,10 +893,12 @@ class TestDhcpAgentEventHandler(base.BaseTestCase):
'.spawn_monitored_metadata_proxy')
with mock.patch(method_path) as spawn:
self.dhcp.enable_isolated_metadata_proxy(network)
metadata_ip = dhcp.METADATA_DEFAULT_IP
spawn.assert_called_once_with(self.dhcp._process_monitor,
network.namespace,
dhcp.METADATA_PORT,
cfg.CONF,
bind_address=metadata_ip,
router_id='forzanapoli')
def test_enable_isolated_metadata_proxy_with_metadata_network(self):

View File

@ -1713,7 +1713,7 @@ class TestSetIpNonlocalBindForHaNamespace(base.BaseTestCase):
def test_setting_failure(self):
"""Make sure message is formatted correctly."""
with mock.patch.object(ip_lib, 'set_ip_nonlocal_bind', return_value=1):
ip_lib.set_ip_nonlocal_bind_for_namespace('foo')
ip_lib.set_ip_nonlocal_bind_for_namespace('foo', value=1)
class TestSysctl(base.BaseTestCase):

View File

@ -64,6 +64,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
EUNAME = 'neutron'
EGNAME = 'neutron'
METADATA_DEFAULT_IP = '169.254.169.254'
METADATA_PORT = 8080
METADATA_SOCKET = '/socket/path'
PIDFILE = 'pidfile'
@ -146,6 +147,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
router_ns,
self.METADATA_PORT,
agent.conf,
bind_address=self.METADATA_DEFAULT_IP,
router_id=router_id)
netns_execute_args = [
@ -157,6 +159,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
cfg_contents = metadata_driver._HAPROXY_CONFIG_TEMPLATE % {
'user': self.EUNAME,
'group': self.EGNAME,
'host': self.METADATA_DEFAULT_IP,
'port': self.METADATA_PORT,
'unix_socket_path': self.METADATA_SOCKET,
'res_type': 'Router',
@ -178,7 +181,8 @@ class TestMetadataDriverProcess(base.BaseTestCase):
def test_create_config_file_wrong_user(self):
with mock.patch('pwd.getpwnam', side_effect=KeyError):
config = metadata_driver.HaproxyConfigurator(_uuid(), mock.ANY,
config = metadata_driver.HaproxyConfigurator(_uuid(),
mock.ANY, mock.ANY,
mock.ANY, mock.ANY,
self.EUNAME,
self.EGNAME,
@ -190,7 +194,8 @@ class TestMetadataDriverProcess(base.BaseTestCase):
with mock.patch('grp.getgrnam', side_effect=KeyError),\
mock.patch('pwd.getpwnam',
return_value=test_utils.FakeUser(self.EUNAME)):
config = metadata_driver.HaproxyConfigurator(_uuid(), mock.ANY,
config = metadata_driver.HaproxyConfigurator(_uuid(),
mock.ANY, mock.ANY,
mock.ANY, mock.ANY,
self.EUNAME,
self.EGNAME,