diff --git a/neutron_fwaas/db/firewall/v2/firewall_db_v2.py b/neutron_fwaas/db/firewall/v2/firewall_db_v2.py index d9447d3bf..5d40a40bf 100644 --- a/neutron_fwaas/db/firewall/v2/firewall_db_v2.py +++ b/neutron_fwaas/db/firewall/v2/firewall_db_v2.py @@ -1108,16 +1108,30 @@ class Firewall_db_mixin_v2(fw_ext.Firewallv2PluginBase, base_db.CommonDbMixin): query = self._model_query(context, DefaultFirewallGroup) def_fwg_id = query.filter_by( project_id=project_id).one().firewall_group_id - return self._get_firewall_group(context, def_fwg_id) + return self.get_firewall_group(context, def_fwg_id) - def set_port_for_default_firewall_group(self, context, port, project_id): + def set_port_for_default_firewall_group(self, context, port_id, + project_id): + """Set a port into default firewall group + + :param context: Context object + :param port_id: Port ID + :param project_id: ProjectID + """ with context.session.begin(subtransactions=True): def_fwg_db = self._get_default_fwg(context, project_id) - new_ports = [p.port_id for p in def_fwg_db['ports']] + [port] - fwg_ports = {'ports': new_ports} - self._set_ports_for_firewall_group(context, def_fwg_db, fwg_ports) - return self._make_firewall_group_dict_with_rules( - context, def_fwg_db['id']) + try: + self._set_ports_for_firewall_group( + context, def_fwg_db, {'ports': [port_id]}) + except f_exc.FirewallGroupPortInUse: + LOG.warning("Port %s has been already associated with default " + "firewall group %s and skip association", port_id, + def_fwg_db['id']) + else: + # Update default fwg status to PENDING_UPDATE to wait updating + # from agent + self.update_firewall_group_status( + context, def_fwg_db['id'], nl_constants.PENDING_UPDATE) def _is_default(fwg_db): diff --git a/neutron_fwaas/services/firewall/agents/l2/fwaas_v2.py b/neutron_fwaas/services/firewall/agents/l2/fwaas_v2.py index 62fc80219..e424a4e65 100644 --- a/neutron_fwaas/services/firewall/agents/l2/fwaas_v2.py +++ b/neutron_fwaas/services/firewall/agents/l2/fwaas_v2.py @@ -1,4 +1,4 @@ -# Copyright 2017 FUJITSU LIMITED +# Copyright 2017-2018 FUJITSU LIMITED # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -24,6 +24,7 @@ from neutron.plugins.ml2.drivers.openvswitch.agent import vlanmanager from neutron_lib.agent import l2_extension from neutron_lib import constants as nl_const from neutron_lib.exceptions import firewall_v2 as f_exc +from neutron_lib.utils import net as nl_net from neutron_fwaas._i18n import _ from neutron_fwaas.common import fwaas_constants as consts @@ -288,6 +289,9 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): @lockutils.synchronized('fwg') def create_firewall_group(self, context, firewall_group, host): """Handles create firewall group event""" + + # TODO(chandanc): Fix agent RPC endpoint to remove host arg + host = cfg.CONF.host with self.driver.defer_apply(): try: self._create_firewall_group(context, firewall_group, host) @@ -300,6 +304,9 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): @lockutils.synchronized('fwg') def delete_firewall_group(self, context, firewall_group, host): """Handles delete firewall group event""" + + # TODO(chandanc): Fix agent RPC endpoint to remove host arg + host = cfg.CONF.host with self.driver.defer_apply(): try: self._delete_firewall_group(context, firewall_group, host) @@ -312,6 +319,9 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): @lockutils.synchronized('fwg') def update_firewall_group(self, context, firewall_group, host): """Handles update firewall group event""" + + # TODO(chandanc): Fix agent RPC endpoint to remove host arg + host = cfg.CONF.host with self.driver.defer_apply(): try: self._delete_firewall_group( @@ -328,6 +338,12 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): def handle_port(self, context, port): """Handle port update event""" + # Check if port is trusted and called at once. + if nl_net.is_port_trusted(port) and not self.fwg_map.get_port(port): + self._add_rule_for_trusted_port(port) + self.fwg_map.set_port(port) + return + if not self._is_port_layer2(port): return @@ -348,6 +364,13 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): self._send_fwg_status( context, fwg_id=fwg['id'], status=status, host=self.conf.host) + def _add_rule_for_trusted_port(self, port): + self._add_local_vlan_to_ports([port]) + self.driver.process_trusted_ports([port]) + + def _delete_rule_for_trusted_port(self, port): + self.driver.remove_trusted_ports([port['port_id']]) + def delete_port(self, context, port): """This is being called when a port is deleted by the agent. """ @@ -358,6 +381,12 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): return port = self.fwg_map.get_port(port) + + if port and nl_net.is_port_trusted(port): + self._delete_rule_for_trusted_port(port) + self.fwg_map.remove_port(port) + return + if not self._is_port_layer2(port): return @@ -381,11 +410,13 @@ class FWaaSV2AgentExtension(l2_extension.L2AgentExtension): class PortFirewallGroupMap(object): - """Store relations between Port and FirewallGroup + """Store relations between Port and Firewall Group and trusted port This map is used in deleting firewall_group because the firewall_group has been deleted at that time. Therefore, it is impossible to refer 'ports'. This map enables to refer 'ports' for specified firewall_group. + Furthermore, it is necessary to check 'device_owner' for trusted port, this + Map also stores trusted port data. """ def __init__(self): self.known_fwgs = {} @@ -412,6 +443,11 @@ class PortFirewallGroupMap(object): if fwg_id: return self.get_fwg(fwg_id) + def set_port(self, port): + """Add a new port into port_detail""" + port_id = self.port_id(port) + self.port_detail[port_id] = port + def set_port_fwg(self, port, fwg): """Add a new port into fwg['ports']""" port_id = self.port_id(port) @@ -436,6 +472,11 @@ class PortFirewallGroupMap(object): if port_id in self.port_fwg: fwg_id = self.port_fwg.get(port_id) if not fwg_id: + # This case is trusted port. Try to delete port_detail dict + try: + del self.port_detail[port_id] + except KeyError: + pass return new_fwg = self.known_fwgs[fwg_id] new_fwg['ports'] = [p for p in new_fwg['ports'] if p != port_id] diff --git a/neutron_fwaas/services/firewall/drivers/linux/l2/noop/noop_driver.py b/neutron_fwaas/services/firewall/drivers/linux/l2/noop/noop_driver.py index 631ba5060..ff40c0e2c 100644 --- a/neutron_fwaas/services/firewall/drivers/linux/l2/noop/noop_driver.py +++ b/neutron_fwaas/services/firewall/drivers/linux/l2/noop/noop_driver.py @@ -30,3 +30,11 @@ class NoopFirewallL2Driver(driver_base.FirewallL2DriverBase): @log_helpers.log_method_call def delete_firewall_group(self, ports, firewall_group): pass + + @log_helpers.log_method_call + def process_trusted_ports(self, ports): + pass + + @log_helpers.log_method_call + def remove_trusted_ports(self, port_ids): + pass diff --git a/neutron_fwaas/services/firewall/drivers/linux/l2/openvswitch_firewall/firewall.py b/neutron_fwaas/services/firewall/drivers/linux/l2/openvswitch_firewall/firewall.py index f1cd3f973..2876abb87 100644 --- a/neutron_fwaas/services/firewall/drivers/linux/l2/openvswitch_firewall/firewall.py +++ b/neutron_fwaas/services/firewall/drivers/linux/l2/openvswitch_firewall/firewall.py @@ -445,6 +445,21 @@ class OVSFirewallDriver(driver_base.FirewallL2DriverBase): self.fwg_port_map.delete_fwg(fwg_id) + def process_trusted_ports(self, ports): + """Pass packets from these ports directly to ingress pipeline.""" + if self.sg_with_ovs: + return + + for port in ports: + self._initialize_egress_no_port_security(port) + + def remove_trusted_ports(self, port_ids): + if self.sg_with_ovs: + return + + for port_id in port_ids: + self._remove_egress_no_port_security(port_id) + # NOTE(ivasilevskaya) That's a copy-paste from neutron ovsfw driver def filter_defer_apply_on(self): self._deferred = True @@ -1001,8 +1016,8 @@ class OVSFirewallDriver(driver_base.FirewallL2DriverBase): self._delete_flows(reg_port=port.ofport) def create_firewall_group(self, ports_for_fwg, firewall_group): - ingress_rules = firewall_group['egress_rule_list'] - egress_rules = firewall_group['ingress_rule_list'] + egress_rules = firewall_group['egress_rule_list'] + ingress_rules = firewall_group['ingress_rule_list'] fwg_id = firewall_group['id'] self.update_firewall_group_rules(fwg_id, ingress_rules, egress_rules) diff --git a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py index 03d31ee04..6f19e2e5d 100644 --- a/neutron_fwaas/services/firewall/fwaas_plugin_v2.py +++ b/neutron_fwaas/services/firewall/fwaas_plugin_v2.py @@ -34,14 +34,6 @@ from neutron_fwaas.db.firewall.v2 import firewall_db_v2 LOG = logging.getLogger(__name__) -FW_V2_OPTS = [ - cfg.BoolOpt( - 'auto_associate_default_firewall_group', - default=False, - help=("Apply default fwg to all new VM ports within a project")), -] -cfg.CONF.register_opts(FW_V2_OPTS, 'fwaas') - def add_provider_configuration(type_manager, service_type): type_manager.add_provider_configuration( @@ -185,10 +177,6 @@ class FirewallPluginV2( cfg.CONF.host ) - self.apply_default_fwg = ( - cfg.CONF.fwaas.auto_associate_default_firewall_group - ) - @property def _core_plugin(self): return directory.get_plugin() @@ -300,22 +288,30 @@ class FirewallPluginV2( """Returns an ID of project for specified port_id. """ return self._core_plugin.get_port(context, port_id)['project_id'] - @registry.receives(resources.PORT, [events.AFTER_CREATE]) - def handle_create_port_event(self, resource, event, trigger, **kwargs): + @registry.receives(resources.PORT, [events.AFTER_UPDATE]) + def handle_update_port(self, resource, event, trigger, **kwargs): - if not self.apply_default_fwg: + updated_port = kwargs['port'] + if not updated_port['device_owner'].startswith( + nl_constants.DEVICE_OWNER_COMPUTE_PREFIX): + return + + if (kwargs.get('original_port')[pb_def.VIF_TYPE] != + pb_def.VIF_TYPE_UNBOUND): + # Checking newly vm port binding allows us to avoid call to DB + # when a port update_event like restart, setting name, etc... + # Moreover, that will help us in case of tenant admin wants to + # only attach security group to vm port. + return + + if updated_port[pb_def.VIF_TYPE] in [pb_def.VIF_TYPE_UNBOUND, + pb_def.VIF_TYPE_BINDING_FAILED]: return context = kwargs['context'] - port_id = kwargs['port']['id'] - project_id = kwargs['port']['project_id'] - fwg_with_rules = self.set_port_for_default_firewall_group( - context, port_id, project_id) - fwg_with_rules['add-port-ids'] = port_id - fwg_with_rules['del-ports-ids'] = [] - fwg_with_rules['port_details'] = self._get_fwg_port_details( - context, port_id) - - self.agent_rpc.update_firewall_group(context, fwg_with_rules) + port_id = updated_port['id'] + project_id = updated_port['project_id'] + LOG.debug("Try to associate port %s at %s", port_id, project_id) + self.set_port_for_default_firewall_group(context, port_id, project_id) def create_firewall_group(self, context, firewall_group): LOG.debug("create_firewall_group() called") @@ -351,7 +347,7 @@ class FirewallPluginV2( self._make_firewall_group_dict_with_rules(context, fwg['id'])) fwg_with_rules['add-port-ids'] = fwg_ports - fwg_with_rules['del-ports-ids'] = [] + fwg_with_rules['del-port-ids'] = [] fwg_with_rules['port_details'] = self._get_fwg_port_details( context, fwg_ports) diff --git a/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py b/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py index 9e0a93779..34038f044 100644 --- a/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py +++ b/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py @@ -1976,3 +1976,49 @@ class TestFirewallDBPluginV2(FirewallPluginV2DbTestCase): with self.firewall_group(name='fireWall1') as fw: res = self._show('firewall_groups', fw['firewall_group']['id']) self.assertEqual('fireWall1', res['firewall_group']['name']) + + def test_set_port_in_use_for_firewall_group(self): + fwg_db = {'id': 'fake_id'} + new_ports = {'ports': ['fake_port1', 'fake_port2']} + m_context = context.get_admin_context() + with mock.patch.object(m_context.session, 'add', + side_effect=[None, f_exc.FirewallGroupPortInUse( + port_ids=['fake_port2'])]): + self.assertRaises(f_exc.FirewallGroupPortInUse, + self.plugin._set_ports_for_firewall_group, + m_context, + fwg_db, + new_ports) + + def test_set_port_for_default_firewall_group(self): + ctx = self._get_nonadmin_context() + self._build_default_fwg(ctx=ctx) + with self.port(project_id=ctx.tenant_id) as port1, \ + self.port(project_id=ctx.tenant_id) as port2: + port1_id = port1['port']['id'] + port2_id = port2['port']['id'] + port_ids = [port1_id, port2_id] + project_id = ctx.tenant_id + + self.plugin.set_port_for_default_firewall_group( + ctx, port1_id, project_id) + self.plugin.set_port_for_default_firewall_group( + ctx, port2_id, project_id) + def_fwg_db = self.plugin._get_default_fwg(ctx, project_id) + self.assertEqual('PENDING_UPDATE', def_fwg_db['status']) + self.assertEqual(sorted(port_ids), sorted(def_fwg_db['ports'])) + + def test_set_port_for_default_firewall_group_raised_port_in_use(self): + ctx = self._get_nonadmin_context() + self._build_default_fwg(ctx=ctx) + self.plugin.update_firewall_group_status = mock.Mock() + with self.port(project_id=ctx.tenant_id) as port1: + port1_id = port1['port']['id'] + port_ids = [port1_id] + self.plugin._set_ports_for_firewall_group = mock.Mock( + side_effect=f_exc.FirewallGroupPortInUse(port_ids=port_ids)) + project_id = ctx.tenant_id + + self.plugin.set_port_for_default_firewall_group( + ctx, port1_id, project_id) + self.plugin.update_firewall_group_status.assert_not_called() diff --git a/neutron_fwaas/tests/unit/services/firewall/agents/l2/test_fwaas_v2.py b/neutron_fwaas/tests/unit/services/firewall/agents/l2/test_fwaas_v2.py index 5ba11df19..8db9f8179 100644 --- a/neutron_fwaas/tests/unit/services/firewall/agents/l2/test_fwaas_v2.py +++ b/neutron_fwaas/tests/unit/services/firewall/agents/l2/test_fwaas_v2.py @@ -80,6 +80,7 @@ class TestHandlePort(TestFWaasV2AgentExtensionBase): self.l2._apply_fwg_rules = mock.Mock(return_value=True) self.l2._send_fwg_status = mock.Mock() self.ctx = context.get_admin_context() + self.l2._add_rule_for_trusted_port = mock.Mock() def test_normal(self): self.l2.fwg_map.get_port_fwg.return_value = None @@ -127,6 +128,24 @@ class TestHandlePort(TestFWaasV2AgentExtensionBase): self.l2.fwg_map.set_port_fwg.assert_not_called() self.l2._send_fwg_status.assert_not_called() + def test_trusted_port(self): + self.l2.fwg_map.get_port.return_value = None + self.port['device_owner'] = 'network:foo' + self.l2.handle_port(self.ctx, self.port) + + self.l2._add_rule_for_trusted_port.assert_called_once_with(self.port) + self.l2.fwg_map.set_port.assert_called_once_with(self.port) + self.rpc.get_firewall_group_for_port.assert_not_called() + + def test_trusted_port_registered_map(self): + self.port['device_owner'] = 'network:dhcp' + self.l2.fwg_map.get_port.return_value = self.port + self.l2.handle_port(self.ctx, self.port) + + self.l2._add_rule_for_trusted_port.assert_not_called() + self.l2.fwg_map.set_port.assert_not_called() + self.rpc.get_firewall_group_for_port.assert_not_called() + class TestDeletePort(TestFWaasV2AgentExtensionBase): @@ -135,6 +154,7 @@ class TestDeletePort(TestFWaasV2AgentExtensionBase): self.l2._compute_status = mock.Mock(return_value=nl_consts.ACTIVE) self.l2._apply_fwg_rules = mock.Mock(return_value=True) self.l2._send_fwg_status = mock.Mock() + self.l2._delete_rule_for_trusted_port = mock.Mock() self.l2.fwg_map.get_port_fwg = mock.Mock(return_value=self.fwg) self.l2.fwg_map.set_fwg = mock.Mock() @@ -185,6 +205,15 @@ class TestDeletePort(TestFWaasV2AgentExtensionBase): self.l2.fwg_map.get_port_fwg.assert_called_once_with(self.port) self.l2._apply_fwg_rules.assert_not_called() + def test_trusted_port_with_map(self): + self.port['device_owner'] = 'network:dhcp' + self.l2.fwg_map.get_port.return_value = self.port + self.l2.delete_port(self.ctx, self.port_minimal) + + self.l2._delete_rule_for_trusted_port.assert_called_once_with( + self.port) + self.l2.fwg_map.remove_port.assert_called_once_with(self.port) + class TestCreateFirewallGroup(TestFWaasV2AgentExtensionBase): @@ -713,13 +742,21 @@ class TestPortFirewallGroupMap(base.BaseTestCase): self.assertIsNone(self.map.get_port(port2)) self.assertEqual([], self.map.get_fwg(self.fwg_id)['ports']) + def test_remove_non_exist_port(self): + port1 = self.port + port2 = self.fake.create('port') + self.map.set_port_fwg(port1, self.fwg) + + self.map.remove_port(port2) + self.assertIsNone(self.map.get_port(port2)) + def test_illegal_remove_port_no_relation_with_fwg(self): port1 = self.port port1_id = port1['port_id'] self.map.set_port_fwg(port1, self.fwg) self.map.port_fwg[port1_id] = None self.map.remove_port(port1) - self.assertEqual(port1, self.map.get_port(port1)) + self.assertIsNone(self.map.get_port(port1)) def test_remove_fwg(self): self.map.set_fwg(self.fwg) diff --git a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py index a55159ef7..a00e31434 100644 --- a/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py +++ b/neutron_fwaas/tests/unit/services/firewall/test_fwaas_plugin_v2.py @@ -775,33 +775,66 @@ class TestFirewallPluginBasev2(TestFirewallRouterPortBase, None) -class TestEnabledAutomaticAssociation(TestFirewallPluginBasev2): +class TestAutomaticAssociation(TestFirewallPluginBasev2): def setUp(self): - # set auto association fwg - cfg.CONF.set_override( - 'auto_associate_default_firewall_group', True, 'fwaas') - super(TestEnabledAutomaticAssociation, self).setUp() + # TODO(yushiro): Replace constant value for this test class + # Set auto association fwg + super(TestAutomaticAssociation, self).setUp() self.agent_rpc = self.plugin.agent_rpc - - def test_enabled_auto_association_fwg(self): - fwg_with_rule = {'id': 'fake_id', - 'name': 'default'} - self.plugin.set_port_for_default_firewall_group = \ - mock.Mock(return_value=fwg_with_rule) + self.plugin.set_port_for_default_firewall_group = mock.Mock() + self.plugin._get_fwg_port_details = mock.Mock() + + def test_vm_port(self): self.plugin._get_fwg_port_details = mock.Mock() - self.agent_rpc.update_firewall_group = mock.Mock() - m_context = mock.ANY kwargs = { - "context": m_context, + "context": mock.ANY, "port": {"id": "fake_port", - "project_id": "fake_project"} + "device_owner": "compute:nova", + "binding:vif_type": "ovs", + "project_id": "fake_project"}, + "original_port": {"binding:vif_type": "unbound"} } - self.plugin.handle_create_port_event( - "PORT", "after_create", "test_plugin", **kwargs) + self.plugin.handle_update_port( + "PORT", "after_update", "test_plugin", **kwargs) self.plugin.set_port_for_default_firewall_group.\ - assert_called_once_with(m_context, + assert_called_once_with(mock.ANY, kwargs['port']['id'], kwargs['port']['project_id']) - self.agent_rpc.update_firewall_group.assert_called_once_with( - m_context, fwg_with_rule) + + def test_vm_port_not_newly_created(self): + # Just updated for VM port(name or description...etc.) + kwargs = { + "context": mock.ANY, + "port": { + "id": "fake_port", + "device_owner": "compute:nova", + "binding:vif_type": "ovs", + "project_id": "fake_project" + }, + "original_port": { + "device_owner": "compute:nova", + "binding:vif_type": "ovs", + "project_id": "fake_project" + } + } + self.plugin.handle_update_port( + "PORT", "after_update", "test_plugin", **kwargs) + self.plugin.set_port_for_default_firewall_group.assert_not_called() + + def test_not_vm_port(self): + for device_owner in ["network:router_interface", + "network:router_gateway", + "network:dhcp"]: + kwargs = { + "context": mock.ANY, + "port": {"id": "fake_port", + "device_owner": device_owner, + "project_id": "fake_project"}, + "original_port": {"device_owner": device_owner, + "binding:vif_type": "unbound", + "project_id": "fake_project"} + } + self.plugin.handle_update_port( + "PORT", "after_update", "test_plugin", **kwargs) + self.plugin.set_port_for_default_firewall_group.assert_not_called() diff --git a/releasenotes/notes/bug-1746404-493a66faac333403.yaml b/releasenotes/notes/bug-1746404-493a66faac333403.yaml new file mode 100644 index 000000000..959d0eb88 --- /dev/null +++ b/releasenotes/notes/bug-1746404-493a66faac333403.yaml @@ -0,0 +1,10 @@ +--- +prelude: > + Taking security for VM instance into consideration, we've removed an option + to disable automatic association with default firewall group feature. + Therefore, `auto_associate_default_firewall_group` has been removed. +fixes: + - | + There is no validation to check if an updated port is for VM or not so far. + After this fix, default firewall group association is called only for + VM ports which are newly created.