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 2032a42f1d..0739913168 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()