From 26332dcbd8c25fb581c27ced1eb175bcd209c1fa Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Wed, 9 Jan 2013 15:08:02 -0800 Subject: [PATCH] _validate_security_groups_on_port was not validating external_ids The function _validate_security_groups_on_port was not validating a ports security group id if the id was an external id. The unit tests now use set_override() rather than setting cfg values directly. Lastly, quantum.conf now has the proxy_mode option exposed. Fixes bug 1095864 Change-Id: I0ec7f9ed36f1a46156c47a115be936bb41ef75d9 --- etc/quantum.conf | 4 + quantum/db/securitygroups_db.py | 28 +++-- quantum/extensions/securitygroup.py | 13 ++ .../plugins/linuxbridge/lb_quantum_plugin.py | 6 +- .../unit/test_extension_security_group.py | 112 ++++++++++++++++-- 5 files changed, 140 insertions(+), 23 deletions(-) diff --git a/etc/quantum.conf b/etc/quantum.conf index aa39442c4..a9242d594 100644 --- a/etc/quantum.conf +++ b/etc/quantum.conf @@ -197,3 +197,7 @@ notification_topics = notifications # by the default service type. # Each service definition should be in the following format: # :[:driver] + +[SECURITYGROUP] +# If set to true this allows quantum to receive proxied security group calls from nova +# proxy_mode = False diff --git a/quantum/db/securitygroups_db.py b/quantum/db/securitygroups_db.py index 5bd890bbe..50b35bba6 100644 --- a/quantum/db/securitygroups_db.py +++ b/quantum/db/securitygroups_db.py @@ -15,7 +15,6 @@ # under the License. # # @author: Aaron Rosen, Nicira, Inc -# import sqlalchemy as sa from sqlalchemy import orm @@ -446,22 +445,33 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): else: return default_group[0]['id'] - def _validate_security_groups_on_port(self, context, port): + def _get_security_groups_on_port(self, context, port): + """Check that all security groups on port belong to tenant. + + :returns: all security groups IDs on port belonging to tenant. + """ p = port['port'] if not attr.is_attr_set(p.get(ext_sg.SECURITYGROUPS)): return if p.get('device_owner') and p['device_owner'].startswith('network:'): return - valid_groups = self.get_security_groups(context, fields={'id': None}) - valid_groups_set = set([x['id'] for x in valid_groups]) - req_sg_set = set(p[ext_sg.SECURITYGROUPS]) - invalid_sg_set = req_sg_set - valid_groups_set - if invalid_sg_set: - msg = ' '.join(str(x) for x in invalid_sg_set) - raise ext_sg.SecurityGroupNotFound(id=msg) + valid_groups = self.get_security_groups( + context, fields=['external_id', 'id']) + valid_group_map = dict((g['id'], g['id']) for g in valid_groups) + valid_group_map.update((g['external_id'], g['id']) + for g in valid_groups if g.get('external_id')) + try: + return set([valid_group_map[sg_id] + for sg_id in p.get(ext_sg.SECURITYGROUPS, [])]) + except KeyError as e: + raise ext_sg.SecurityGroupNotFound(id=str(e)) def _ensure_default_security_group_on_port(self, context, port): + # return if proxy_mode is enabled since nova will handle adding + # the port to the default security group. + if cfg.CONF.SECURITYGROUP.proxy_mode: + return # we don't apply security groups for dhcp, router if (port['port'].get('device_owner') and port['port']['device_owner'].startswith('network:')): diff --git a/quantum/extensions/securitygroup.py b/quantum/extensions/securitygroup.py index f9c46e0b9..fffe6de73 100644 --- a/quantum/extensions/securitygroup.py +++ b/quantum/extensions/securitygroup.py @@ -22,6 +22,7 @@ from quantum.api.v2 import base from quantum.common import exceptions as qexception from quantum import manager from quantum.openstack.common import cfg +from quantum.openstack.common import uuidutils from quantum import quota @@ -126,6 +127,17 @@ def convert_validate_port_value(port): raise SecurityGroupInvalidPortValue(port=port) +def convert_to_uuid_or_int_list(value_list): + if value_list is None: + return + try: + return [sg_id if uuidutils.is_uuid_like(sg_id) else int(sg_id) + for sg_id in value_list] + except (ValueError, TypeError): + msg = _("'%s' is not an integer or uuid") % sg_id + raise qexception.InvalidInput(error_message=msg) + + def _validate_name_not_default(data, valid_values=None): if not cfg.CONF.SECURITYGROUP.proxy_mode and data == "default": raise SecurityGroupDefaultAlreadyExists() @@ -208,6 +220,7 @@ EXTENDED_ATTRIBUTES_2_0 = { 'ports': {SECURITYGROUPS: {'allow_post': True, 'allow_put': True, 'is_visible': True, + 'convert_to': convert_to_uuid_or_int_list, 'default': attr.ATTR_NOT_SPECIFIED}}} security_group_quota_opts = [ cfg.IntOpt('quota_security_group', diff --git a/quantum/plugins/linuxbridge/lb_quantum_plugin.py b/quantum/plugins/linuxbridge/lb_quantum_plugin.py index beb775dad..b1d4db82b 100644 --- a/quantum/plugins/linuxbridge/lb_quantum_plugin.py +++ b/quantum/plugins/linuxbridge/lb_quantum_plugin.py @@ -456,8 +456,7 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2, session = context.session with session.begin(subtransactions=True): self._ensure_default_security_group_on_port(context, port) - self._validate_security_groups_on_port(context, port) - sgids = port['port'].get(ext_sg.SECURITYGROUPS) + sgids = self._get_security_groups_on_port(context, port) port = super(LinuxBridgePluginV2, self).create_port(context, port) self._process_port_create_security_group( @@ -471,13 +470,14 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2, return self._extend_port_dict_binding(context, port) def update_port(self, context, id, port): - self._validate_security_groups_on_port(context, port) original_port = self.get_port(context, id) session = context.session port_updated = False with session.begin(subtransactions=True): # delete the port binding and read it with the new rules if ext_sg.SECURITYGROUPS in port['port']: + port['port'][ext_sg.SECURITYGROUPS] = ( + self._get_security_groups_on_port(context, port)) self._delete_port_security_group_bindings(context, id) self._process_port_create_security_group( context, diff --git a/quantum/tests/unit/test_extension_security_group.py b/quantum/tests/unit/test_extension_security_group.py index e82961352..8d3547c08 100644 --- a/quantum/tests/unit/test_extension_security_group.py +++ b/quantum/tests/unit/test_extension_security_group.py @@ -168,10 +168,9 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, default_sg = self._ensure_default_security_group(context, tenant_id) if not port['port'].get(ext_sg.SECURITYGROUPS): port['port'][ext_sg.SECURITYGROUPS] = [default_sg] - self._validate_security_groups_on_port(context, port) session = context.session with session.begin(subtransactions=True): - sgids = port['port'].get(ext_sg.SECURITYGROUPS) + sgids = self._get_security_groups_on_port(context, port) port = super(SecurityGroupTestPlugin, self).create_port(context, port) self._process_port_create_security_group(context, port['id'], @@ -183,7 +182,8 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, session = context.session with session.begin(subtransactions=True): if ext_sg.SECURITYGROUPS in port['port']: - self._validate_security_groups_on_port(context, port) + port['port'][ext_sg.SECURITYGROUPS] = ( + self._get_security_groups_on_port(context, port)) # delete the port binding and read it with the new rules self._delete_port_security_group_bindings(context, id) self._process_port_create_security_group( @@ -218,7 +218,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(security_group['security_group'][k], v) def test_create_security_group_external_id(self): - cfg.CONF.SECURITYGROUP.proxy_mode = True + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') name = 'webservers' description = 'my webservers' external_id = 10 @@ -235,7 +235,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(len(groups['security_groups']), 1) def test_create_security_group_proxy_mode_not_admin(self): - cfg.CONF.SECURITYGROUP.proxy_mode = True + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') res = self._create_security_group('json', 'webservers', 'webservers', '1', tenant_id='bad_tenant', @@ -244,7 +244,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(res.status_int, 403) def test_create_security_group_no_external_id_proxy_mode(self): - cfg.CONF.SECURITYGROUP.proxy_mode = True + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') res = self._create_security_group('json', 'webservers', 'webservers') self.deserialize('json', res) @@ -264,7 +264,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(res.status_int, 409) def test_create_security_group_duplicate_external_id(self): - cfg.CONF.SECURITYGROUP.proxy_mode = True + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') name = 'webservers' description = 'my webservers' external_id = 1 @@ -415,7 +415,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(res.status_int, 404) def test_create_security_group_rule_exteral_id_proxy_mode(self): - cfg.CONF.SECURITYGROUP.proxy_mode = True + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') with self.security_group(external_id=1) as sg: rule = {'security_group_rule': {'security_group_id': sg['security_group']['id'], @@ -448,7 +448,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(res.status_int, 409) def test_create_security_group_rule_not_admin(self): - cfg.CONF.SECURITYGROUP.proxy_mode = True + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') with self.security_group(external_id='1') as sg: rule = {'security_group_rule': {'security_group_id': sg['security_group']['id'], @@ -608,7 +608,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): port['port'][ext_sg.SECURITYGROUPS]), 2) self._delete('ports', port['port']['id']) - def test_update_port_remove_security_group(self): + def test_update_port_remove_security_group_empty_list(self): with self.network() as n: with self.subnet(n): with self.security_group() as sg: @@ -628,6 +628,26 @@ class TestSecurityGroups(SecurityGroupDBTestCase): []) self._delete('ports', port['port']['id']) + def test_update_port_remove_security_group_none(self): + with self.network() as n: + with self.subnet(n): + with self.security_group() as sg: + res = self._create_port('json', n['network']['id'], + security_groups=( + [sg['security_group']['id']])) + port = self.deserialize('json', res) + + data = {'port': {'fixed_ips': port['port']['fixed_ips'], + 'name': port['port']['name'], + 'security_groups': None}} + + req = self.new_update_request('ports', data, + port['port']['id']) + res = self.deserialize('json', req.get_response(self.api)) + self.assertEqual(res['port'].get(ext_sg.SECURITYGROUPS), + []) + self._delete('ports', port['port']['id']) + def test_create_port_with_bad_security_group(self): with self.network() as n: with self.subnet(n): @@ -635,7 +655,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): security_groups=['bad_id']) self.deserialize('json', res) - self.assertEqual(res.status_int, 404) + self.assertEqual(res.status_int, 400) def test_create_delete_security_group_port_in_use(self): with self.network() as n: @@ -820,3 +840,73 @@ class TestSecurityGroups(SecurityGroupDBTestCase): res = self._create_security_group_rule('json', rule) self.deserialize('json', res) self.assertEqual(res.status_int, 400) + + def test_validate_port_external_id_quantum_id(self): + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') + with self.network() as n: + with self.subnet(n): + sg1 = (self.deserialize('json', + self._create_security_group('json', 'foo', 'bar', '1'))) + sg2 = (self.deserialize('json', + self._create_security_group('json', 'foo', 'bar', '2'))) + res = self._create_port( + 'json', n['network']['id'], + security_groups=[sg1['security_group']['id']]) + + port = self.deserialize('json', res) + # This request updates the port sending the quantum security + # group id in and a nova security group id. + data = {'port': {'fixed_ips': port['port']['fixed_ips'], + 'name': port['port']['name'], + ext_sg.SECURITYGROUPS: + [sg1['security_group']['external_id'], + sg2['security_group']['id']]}} + req = self.new_update_request('ports', data, + port['port']['id']) + res = self.deserialize('json', req.get_response(self.api)) + self.assertEquals(len(res['port'][ext_sg.SECURITYGROUPS]), 2) + for sg_id in res['port'][ext_sg.SECURITYGROUPS]: + # only security group id's should be + # returned and not external_ids + self.assertEquals(len(sg_id), 36) + self._delete('ports', port['port']['id']) + + def test_validate_port_external_id_string_or_int(self): + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') + with self.network() as n: + with self.subnet(n): + string_id = '1' + int_id = 2 + self.deserialize( + 'json', self._create_security_group('json', 'foo', 'bar', + string_id)) + self.deserialize( + 'json', self._create_security_group('json', 'foo', 'bar', + int_id)) + res = self._create_port( + 'json', n['network']['id'], + security_groups=[string_id, int_id]) + + port = self.deserialize('json', res) + self._delete('ports', port['port']['id']) + + def test_create_port_with_non_uuid_or_int(self): + with self.network() as n: + with self.subnet(n): + res = self._create_port('json', n['network']['id'], + security_groups=['not_valid']) + + self.deserialize('json', res) + self.assertEqual(res.status_int, 400) + + def test_validate_port_external_id_fail(self): + cfg.CONF.set_override('proxy_mode', True, 'SECURITYGROUP') + with self.network() as n: + with self.subnet(n): + bad_id = 1 + res = self._create_port( + 'json', n['network']['id'], + security_groups=[bad_id]) + + self.deserialize('json', res) + self.assertEqual(res.status_int, 404) -- 2.45.2