Merge "NSX|v limit access to metadata service to specific protocols"

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"
 }

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 =

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")),

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',

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):
+    def _get_ports_list_from_string(self, port_str):
-        if not port_range:
+        """Receives a string representation of the service ports,
-            return [None, None]
+        and return a list of integers
-        min_port, sep, max_port = port_range.partition(":")
+        Supported formats:
-        if not max_port:
+        Empty string - no ports
-            max_port = min_port
+        "number" - a single port
-        return [int(min_port), int(max_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':

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()