From 6a8a1f81aea35933996494674157454e7abfb1ee 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 (cherry picked from commit 69419778276a722c3f376046d1e0cc7fc4684e99) --- 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 ab3b505f8bc..1b8d9a10bf1 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 @@ -1000,6 +1001,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 5422c9f527d..79f6d2f138e 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -184,16 +184,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 ' @@ -302,8 +292,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 8d618c5dc56..e2545ab4244 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -3487,6 +3487,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 ' @@ -3522,6 +3531,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 f4618afbe62..4c4a7ac0580 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 21649733640..037c75ec75f 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')