From 4e92f00d1b860c42b87873be51e2471c8f58af49 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Sun, 3 Apr 2016 16:04:40 +0300 Subject: [PATCH] NSX|v limit access to metadata service to specific protocols The firewall rule created on the differents edges to allow access to the metadata service, should be restricted to the specific supported protocols (tcp 80, 443, 8775), and not open to all protocols The list of allowed ports can be extended using the nsx.ini parameter 'metadata_service_allowed_ports' Change-Id: If2f0f30937eb3b7489a36feff1635de4822710bb --- devstack/lib/vmware_nsx_v | 1 + etc/nsx.ini | 4 + vmware_nsx/common/config.py | 4 + vmware_nsx/plugins/nsx_v/md_proxy.py | 15 ++- .../nsx_v/vshield/edge_firewall_driver.py | 38 ++++-- vmware_nsx/tests/unit/nsx_v/test_plugin.py | 121 ++++++++++++++++++ 6 files changed, 171 insertions(+), 12 deletions(-) diff --git a/devstack/lib/vmware_nsx_v b/devstack/lib/vmware_nsx_v index 8b0ac852bd..5a0245ec9c 100644 --- a/devstack/lib/vmware_nsx_v +++ b/devstack/lib/vmware_nsx_v @@ -107,6 +107,7 @@ function neutron_plugin_configure_service { _nsxv_ini_set metadata_insecure "$NSXV_METADATA_INSECURE" _nsxv_ini_set metadata_nova_client_cert "$NSXV_METADATA_NOVA_CERT" _nsxv_ini_set metadata_nova_client_priv_key "$NSXV_METADATA_NOVA_PRIV_KEY" + _nsxv_ini_set metadata_service_allowed_ports "$NSXV_METADATA_SERVICE_ALLOWED_PORTS" _nsxv_ini_set edge_ha "$NSXV_EDGE_HA" _nsxv_ini_set exclusive_router_appliance_size "$NSXV_EXCLUSIVE_ROUTER_APPLIANCE_SIZE" } diff --git a/etc/nsx.ini b/etc/nsx.ini index 04e05e1715..9a24c6ea63 100644 --- a/etc/nsx.ini +++ b/etc/nsx.ini @@ -147,6 +147,10 @@ # not verified. If False, the default CA truststore is used for verification. # metadata_insecure = +# (Optional) Comma separated list of tcp ports, to be allowed access to the +# metadata proxy, in addition to the default 80,443,8775 tcp ports +# metadata_service_allowed_ports = + # (Optional) Client certificate to use when metadata connection is to be # verified. If not provided, a self signed certificate will be used. # metadata_nova_client_cert = diff --git a/vmware_nsx/common/config.py b/vmware_nsx/common/config.py index ce26819629..34d5a3ec34 100644 --- a/vmware_nsx/common/config.py +++ b/vmware_nsx/common/config.py @@ -388,6 +388,10 @@ nsxv_opts = [ default=True, help=_("If True, the server instance will attempt to " "initialize the metadata infrastructure")), + cfg.ListOpt('metadata_service_allowed_ports', + help=_('List of tcp ports, to be allowed access to the ' + 'metadata proxy, in addition to the default ' + '80,443,8775 tcp ports')), cfg.BoolOpt('edge_ha', default=False, help=_("Enable HA for NSX Edges")), diff --git a/vmware_nsx/plugins/nsx_v/md_proxy.py b/vmware_nsx/plugins/nsx_v/md_proxy.py index 6747fc89a6..9059869037 100644 --- a/vmware_nsx/plugins/nsx_v/md_proxy.py +++ b/vmware_nsx/plugins/nsx_v/md_proxy.py @@ -60,13 +60,26 @@ DEFAULT_EDGE_FIREWALL_RULE = { def get_router_fw_rules(): + # build the allowed destination ports list + int_ports = [METADATA_TCP_PORT, + METADATA_HTTPS_PORT, + METADATA_HTTPS_VIP_PORT] + str_ports = [str(p) for p in int_ports] + # the list of ports can be extended by configuration + if cfg.CONF.nsxv.metadata_service_allowed_ports: + str_ports = str_ports + cfg.CONF.nsxv.metadata_service_allowed_ports + separator = ',' + dest_ports = separator.join(str_ports) + fw_rules = [ DEFAULT_EDGE_FIREWALL_RULE, { 'name': 'MDServiceIP', 'enabled': True, 'action': 'allow', - 'destination_ip_address': [METADATA_IP_ADDR] + 'destination_ip_address': [METADATA_IP_ADDR], + 'protocol': 'tcp', + 'destination_port': dest_ports }, { 'name': 'MDInterEdgeNet', diff --git a/vmware_nsx/plugins/nsx_v/vshield/edge_firewall_driver.py b/vmware_nsx/plugins/nsx_v/vshield/edge_firewall_driver.py index fb2274950a..c19ed520a9 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/edge_firewall_driver.py +++ b/vmware_nsx/plugins/nsx_v/vshield/edge_firewall_driver.py @@ -64,13 +64,31 @@ class EdgeFirewallDriver(db_base_plugin_v2.NeutronDbPluginV2): else: return '%d:%d' % (min_port, max_port) - def _get_min_max_ports_from_range(self, port_range): - if not port_range: - return [None, None] - min_port, sep, max_port = port_range.partition(":") - if not max_port: - max_port = min_port - return [int(min_port), int(max_port)] + def _get_ports_list_from_string(self, port_str): + """Receives a string representation of the service ports, + and return a list of integers + Supported formats: + Empty string - no ports + "number" - a single port + "num1:num2" - a range + "num1,num2,num3" - a list + """ + if not port_str: + return [] + if ':' in port_str: + min_port, sep, max_port = port_str.partition(":") + return list(range(int(min_port.strip()), + int(max_port.strip()) + 1)) + if ',' in port_str: + # remove duplications (using set) and empty/non numeric entries + ports_set = set() + for orig_port in port_str.split(','): + port = orig_port.strip() + if port and port.isdigit(): + ports_set.add(int(port)) + return sorted(list(ports_set)) + else: + return [int(port_str.strip())] def _convert_firewall_rule(self, context, rule, index=None): vcns_rule = { @@ -100,13 +118,11 @@ class EdgeFirewallDriver(db_base_plugin_v2.NeutronDbPluginV2): vcns_rule['application'] = rule['application'] service = {} if rule.get('source_port'): - min_port, max_port = self._get_min_max_ports_from_range( + service['sourcePort'] = self._get_ports_list_from_string( rule['source_port']) - service['sourcePort'] = [i for i in range(min_port, max_port + 1)] if rule.get('destination_port'): - min_port, max_port = self._get_min_max_ports_from_range( + service['port'] = self._get_ports_list_from_string( rule['destination_port']) - service['port'] = [i for i in range(min_port, max_port + 1)] if rule.get('protocol'): service['protocol'] = rule['protocol'] if rule['protocol'] == 'icmp': diff --git a/vmware_nsx/tests/unit/nsx_v/test_plugin.py b/vmware_nsx/tests/unit/nsx_v/test_plugin.py index 802324d9d4..6a56b76267 100644 --- a/vmware_nsx/tests/unit/nsx_v/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_v/test_plugin.py @@ -58,7 +58,9 @@ from vmware_nsx.extensions import securitygrouplogging from vmware_nsx.extensions import vnicindex as ext_vnic_idx from vmware_nsx.plugins.nsx_v.drivers import ( shared_router_driver as router_driver) +from vmware_nsx.plugins.nsx_v import md_proxy from vmware_nsx.plugins.nsx_v.vshield.common import constants as vcns_const +from vmware_nsx.plugins.nsx_v.vshield import edge_firewall_driver from vmware_nsx.plugins.nsx_v.vshield import edge_utils from vmware_nsx.services.qos.nsx_v import utils as qos_utils from vmware_nsx.tests import unit as vmware @@ -2643,6 +2645,125 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase, s2['subnet']['id'], None) + @mock.patch.object(edge_utils, "update_firewall") + def test_router_interfaces_with_update_firewall_metadata(self, mock): + cfg.CONF.set_override('dhcp_force_metadata', True, group='nsxv') + self.plugin_instance.metadata_proxy_handler = mock.Mock() + s1_cidr = '10.0.0.0/24' + s2_cidr = '11.0.0.0/24' + with self.router() as r,\ + self.subnet(cidr=s1_cidr) as s1,\ + self.subnet(cidr=s2_cidr) as s2: + self._router_interface_action('add', + r['router']['id'], + s1['subnet']['id'], + None) + self._router_interface_action('add', + r['router']['id'], + s2['subnet']['id'], + None) + # build the list of expected fw rules + expected_cidrs = [s1_cidr, s2_cidr] + fw_rule = {'action': 'allow', + 'enabled': True, + 'source_ip_address': expected_cidrs, + 'destination_ip_address': expected_cidrs} + vse_rule = {'action': 'allow', + 'enabled': True, + 'name': 'VSERule', + 'source_vnic_groups': ['vse']} + dest_intern = [md_proxy.INTERNAL_SUBNET] + md_inter = {'action': 'deny', + 'destination_ip_address': dest_intern, + 'enabled': True, + 'name': 'MDInterEdgeNet'} + dest_srvip = [md_proxy.METADATA_IP_ADDR] + md_srvip = {'action': 'allow', + 'destination_ip_address': dest_srvip, + 'destination_port': '80,443,8775', + 'enabled': True, + 'name': 'MDServiceIP', + 'protocol': 'tcp'} + expected_fw = [fw_rule, + vse_rule, + md_inter, + md_srvip] + fw_rules = mock.call_args[0][3]['firewall_rule_list'] + self.assertEqual(self._recursive_sort_list(expected_fw), + self._recursive_sort_list(fw_rules)) + + # Also test the md_srvip conversion: + drv = edge_firewall_driver.EdgeFirewallDriver() + rule = drv._convert_firewall_rule( + context.get_admin_context(), md_srvip) + exp_service = {'service': [{'port': [80, 443, 8775], + 'protocol': 'tcp'}]} + exp_rule = {'action': 'accept', + 'application': exp_service, + 'destination': {'ipAddress': dest_srvip}, + 'enabled': True, + 'name': 'MDServiceIP'} + self.assertEqual(exp_rule, rule) + + self._router_interface_action('remove', + r['router']['id'], + s1['subnet']['id'], + None) + self._router_interface_action('remove', + r['router']['id'], + s2['subnet']['id'], + None) + + @mock.patch.object(edge_utils, "update_firewall") + def test_router_interfaces_with_update_firewall_metadata_conf(self, mock): + """Test the metadata proxy firewall rule with additional configured ports + """ + cfg.CONF.set_override('dhcp_force_metadata', True, group='nsxv') + cfg.CONF.set_override('metadata_service_allowed_ports', + ['55', ' 66 ', '55', 'xx'], group='nsxv') + self.plugin_instance.metadata_proxy_handler = mock.Mock() + s1_cidr = '10.0.0.0/24' + with self.router() as r,\ + self.subnet(cidr=s1_cidr) as s1: + self._router_interface_action('add', + r['router']['id'], + s1['subnet']['id'], + None) + # build the expected fw rule + # at this stage the string of ports is not sorted/unique/validated + dest_srvip = [md_proxy.METADATA_IP_ADDR] + rule_name = 'MDServiceIP' + md_srvip = {'action': 'allow', + 'destination_ip_address': dest_srvip, + 'destination_port': '80,443,8775,55, 66 ,55,xx', + 'enabled': True, + 'name': rule_name, + 'protocol': 'tcp'} + # compare it to the rule with the same name + fw_rules = mock.call_args[0][3]['firewall_rule_list'] + rule_found = False + for fw_rule in fw_rules: + if (attributes.is_attr_set(fw_rule.get("name")) and + fw_rule['name'] == rule_name): + self.assertEqual(md_srvip, fw_rule) + rule_found = True + break + self.assertTrue(rule_found) + + # Also test the rule conversion + # Ports should be sorted & unique, and ignore non numeric values + drv = edge_firewall_driver.EdgeFirewallDriver() + rule = drv._convert_firewall_rule( + context.get_admin_context(), md_srvip) + exp_service = {'service': [{'port': [55, 66, 80, 443, 8775], + 'protocol': 'tcp'}]} + exp_rule = {'action': 'accept', + 'application': exp_service, + 'destination': {'ipAddress': dest_srvip}, + 'enabled': True, + 'name': 'MDServiceIP'} + self.assertEqual(exp_rule, rule) + @mock.patch.object(edge_utils, "update_firewall") def test_router_interfaces_different_tenants_update_firewall(self, mock): tenant_id = _uuid()