From 69419778276a722c3f376046d1e0cc7fc4684e99 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Thu, 30 Nov 2017 16:57:51 -0500 Subject: [PATCH] Add iptables metadata marking rule on router init Move the iptables metadata marking rule earlier in router init, that way any stray metadata requests that arrive before the filter metadata redirect rule is installed will just be dropped. We do this irregardless of whether we will be running the metadata proxy. Partial-bug: #1735724 Change-Id: I8982523dbb94a7c5b8a4db88a196fabc4dd2873f --- neutron/agent/l3/router_info.py | 18 ++++++++++- neutron/agent/metadata/driver.py | 12 -------- neutron/tests/unit/agent/l3/test_agent.py | 30 +++++++++++++++++++ .../agent/l3/test_l3_agent_extension_api.py | 6 +++- .../tests/unit/agent/l3/test_router_info.py | 2 ++ .../tests/unit/agent/metadata/test_driver.py | 9 ------ 6 files changed, 54 insertions(+), 23 deletions(-) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 334722ca38a..372045b7105 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -52,6 +52,7 @@ class RouterInfo(object): use_ipv6=False): self.agent = agent self.router_id = router_id + self.agent_conf = agent_conf self.ex_gw_port = None self._snat_enabled = None self.fip_map = {} @@ -73,8 +74,8 @@ class RouterInfo(object): use_ipv6=use_ipv6, namespace=self.ns_name) self.initialize_address_scope_iptables() + self.initialize_metadata_iptables() self.routes = [] - self.agent_conf = agent_conf self.driver = interface_driver self.process_monitor = None # radvd is a neutron.agent.linux.ra.DaemonMonitor @@ -992,6 +993,21 @@ class RouterInfo(object): iptables_manager.ipv6['mangle'].add_rule( 'PREROUTING', copy_address_scope_for_existing) + def initialize_metadata_iptables(self): + # Always mark incoming metadata requests, that way any stray + # requests that arrive before the filter metadata redirect + # rule is installed will be dropped. + mark_metadata_for_internal_interfaces = ( + '-d 169.254.169.254/32 ' + '-i %(interface_name)s ' + '-p tcp -m tcp --dport 80 ' + '-j MARK --set-xmark %(value)s/%(mask)s' % + {'interface_name': INTERNAL_DEV_PREFIX + '+', + 'value': self.agent_conf.metadata_access_mark, + 'mask': n_const.ROUTER_MARK_MASK}) + self.iptables_manager.ipv4['mangle'].add_rule( + 'PREROUTING', mark_metadata_for_internal_interfaces) + def _get_port_devicename_scopemark(self, ports, name_generator): devicename_scopemark = {lib_constants.IP_VERSION_4: dict(), lib_constants.IP_VERSION_6: dict()} diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py index d29ecd1f38d..ef93b3b85ae 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -176,16 +176,6 @@ class MetadataDriver(object): ('INPUT', '-p tcp -m tcp --dport %s ' '-j DROP' % port)] - @classmethod - def metadata_mangle_rules(cls, mark): - return [('PREROUTING', '-d 169.254.169.254/32 ' - '-i %(interface_name)s ' - '-p tcp -m tcp --dport 80 ' - '-j MARK --set-xmark %(value)s/%(mask)s' % - {'interface_name': namespaces.INTERNAL_DEV_PREFIX + '+', - 'value': mark, - 'mask': constants.ROUTER_MARK_MASK})] - @classmethod def metadata_nat_rules(cls, port): return [('PREROUTING', '-d 169.254.169.254/32 ' @@ -294,8 +284,6 @@ def after_router_added(resource, event, l3_agent, **kwargs): for c, r in proxy.metadata_filter_rules(proxy.metadata_port, proxy.metadata_access_mark): router.iptables_manager.ipv4['filter'].add_rule(c, r) - for c, r in proxy.metadata_mangle_rules(proxy.metadata_access_mark): - router.iptables_manager.ipv4['mangle'].add_rule(c, r) for c, r in proxy.metadata_nat_rules(proxy.metadata_port): router.iptables_manager.ipv4['nat'].add_rule(c, r) for c, r in proxy.metadata_checksum_rules(proxy.metadata_port): diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 540395f546d..9431e8b8374 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -3506,6 +3506,15 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): [mock.call.add_chain('floatingip'), mock.call.add_chain('float-snat'), mock.call.add_rule('PREROUTING', '-j $floatingip'), + mock.call.add_rule( + 'PREROUTING', + '-d 169.254.169.254/32 -i %(interface_name)s ' + '-p tcp -m tcp --dport 80 ' + '-j MARK --set-xmark %(value)s/%(mask)s' % + {'interface_name': + namespaces.INTERNAL_DEV_PREFIX + '+', + 'value': self.conf.metadata_access_mark, + 'mask': n_const.ROUTER_MARK_MASK}), mock.call.add_rule( 'float-snat', '-m connmark --mark 0x0/0xffff0000 ' @@ -3541,6 +3550,27 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): self._verify_address_scopes_iptables_rule( ri.snat_iptables_manager) + def _verify_metadata_iptables_rule(self, mock_iptables_manager): + v4_mangle_calls = ([mock.call.add_rule( + 'PREROUTING', + '-d 169.254.169.254/32 -i %(interface_name)s ' + '-p tcp -m tcp --dport 80 ' + '-j MARK --set-xmark %(value)s/%(mask)s' % + {'interface_name': + namespaces.INTERNAL_DEV_PREFIX + '+', + 'value': self.conf.metadata_access_mark, + 'mask': n_const.ROUTER_MARK_MASK})]) + mock_iptables_manager.ipv4['mangle'].assert_has_calls(v4_mangle_calls, + any_order=True) + + def test_initialize_metadata_iptables_rules(self): + id = _uuid() + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + with mock.patch('neutron.agent.linux.iptables_manager.' + 'IptablesManager'): + ri = l3router.RouterInfo(agent, id, {}, **self.ri_kwargs) + self._verify_metadata_iptables_rule(ri.iptables_manager) + @mock.patch.object(l3router.RouterInfo, 'delete') @mock.patch.object(ha_router.HaRouter, 'destroy_state_change_monitor') def test_delete_ha_router_initialize_fails(self, mock_dscm, mock_delete): diff --git a/neutron/tests/unit/agent/l3/test_l3_agent_extension_api.py b/neutron/tests/unit/agent/l3/test_l3_agent_extension_api.py index 78075c0697d..4b943d936c2 100644 --- a/neutron/tests/unit/agent/l3/test_l3_agent_extension_api.py +++ b/neutron/tests/unit/agent/l3/test_l3_agent_extension_api.py @@ -21,6 +21,8 @@ from oslo_utils import uuidutils from neutron.agent.l3 import l3_agent_extension_api as l3_agent_api from neutron.agent.l3 import router_info from neutron.agent.linux import ip_lib +from neutron.conf.agent import common as config +from neutron.conf.agent.l3 import config as l3_config from neutron.tests import base @@ -29,9 +31,11 @@ class TestL3AgentExtensionApi(base.BaseTestCase): def _prepare_router_data(self, ports=None): self.router_id = uuidutils.generate_uuid() self.project_id = uuidutils.generate_uuid() + self.conf = config.setup_conf() + l3_config.register_l3_agent_config_opts(l3_config.OPTS, self.conf) ri_kwargs = {'router': {'id': self.router_id, 'project_id': self.project_id}, - 'agent_conf': mock.ANY, + 'agent_conf': self.conf, 'interface_driver': mock.ANY, 'use_ipv6': mock.ANY} ri = router_info.RouterInfo(mock.Mock(), self.router_id, **ri_kwargs) diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py index f13773fab7a..364c114251d 100644 --- a/neutron/tests/unit/agent/l3/test_router_info.py +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -18,6 +18,7 @@ from neutron.agent.l3 import router_info from neutron.agent.linux import ip_lib from neutron.common import exceptions as n_exc from neutron.conf.agent import common as config +from neutron.conf.agent.l3 import config as l3_config from neutron.tests import base @@ -29,6 +30,7 @@ class TestRouterInfo(base.BaseTestCase): super(TestRouterInfo, self).setUp() conf = config.setup_conf() + l3_config.register_l3_agent_config_opts(l3_config.OPTS, conf) self.ip_cls_p = mock.patch('neutron.agent.linux.ip_lib.IPWrapper') ip_cls = self.ip_cls_p.start() diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py index 1f883a63827..87c9df8007b 100644 --- a/neutron/tests/unit/agent/metadata/test_driver.py +++ b/neutron/tests/unit/agent/metadata/test_driver.py @@ -52,15 +52,6 @@ class TestMetadataDriverRules(base.BaseTestCase): rules, metadata_driver.MetadataDriver.metadata_filter_rules(9697, '0x1')) - def test_metadata_mangle_rules(self): - rule = ('PREROUTING', '-d 169.254.169.254/32 -i qr-+ ' - '-p tcp -m tcp --dport 80 ' - '-j MARK --set-xmark 0x1/%s' % - constants.ROUTER_MARK_MASK) - self.assertEqual( - [rule], - metadata_driver.MetadataDriver.metadata_mangle_rules('0x1')) - def test_metadata_checksum_rules(self): rules = ('POSTROUTING', '-o qr-+ -p tcp -m tcp --sport 9697 ' '-j CHECKSUM --checksum-fill')