Add source-group filtering.

Move refresh to be triggered by allocation and deallocation of IP's rather than creation/destruction of instances.

There really needs a way to use ipsets for this, but it's not widely supported yet (went into mainline linux at 2.6.39), so this implementation just uses regular iptables.

6 files changed, 92 insertions, 28 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 91a0c93b2..e909e9959 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -393,10 +393,6 @@ class API(base.Base):
         updates['hostname'] = self.hostname_factory(instance)
 
         instance = self.update(context, instance_id, **updates)
-
-        for group_id in security_groups:
-            self.trigger_security_group_members_refresh(elevated, group_id)
-
         return instance
 
     def _ask_scheduler_to_create_instance(self, context, base_options,
@@ -565,18 +561,20 @@ class API(base.Base):
                      {"method": "refresh_security_group_rules",
                       "args": {"security_group_id": security_group.id}})
 
-    def trigger_security_group_members_refresh(self, context, group_id):
+    def trigger_security_group_members_refresh(self, context, group_ids):
         """Called when a security group gains a new or loses a member.
 
         Sends an update request to each compute node for whom this is
         relevant.
         """
-        # First, we get the security group rules that reference this group as
+        # First, we get the security group rules that reference these groups as
         # the grantee..
-        security_group_rules = \
+        security_group_rules = set()
+        for group_id in group_ids:
+            security_group_rules.update(
                 self.db.security_group_rule_get_by_security_group_grantee(
                                                                      context,
-                                                                     group_id)
+                                                                     group_id))
 
         # ..then we distill the security groups to which they belong..
         security_groups = set()
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index 939fde199..64b1bd5cd 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -479,6 +479,11 @@ class SecurityGroupIngressRule(BASE, NovaBase):
     # Note: This is not the parent SecurityGroup. It's SecurityGroup we're
     # granting access for.
     group_id = Column(Integer, ForeignKey('security_groups.id'))
+    grantee_group = relationship("SecurityGroup",
+                                 foreign_keys=group_id,
+                                 primaryjoin='and_('
+        'SecurityGroupIngressRule.group_id == SecurityGroup.id,'
+        'SecurityGroupIngressRule.deleted == False)')
 
 
 class ProviderFirewallRule(BASE, NovaBase):
diff --git a/nova/network/manager.py b/nova/network/manager.py
index 402049d44..b1b3f8ba2 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -61,6 +61,7 @@ from nova import quota
 from nova import utils
 from nova import rpc
 from nova.network import api as network_api
+from nova.compute import api as compute_api
 import random
 
 
@@ -313,6 +314,7 @@ class NetworkManager(manager.SchedulerDependentManager):
             network_driver = FLAGS.network_driver
         self.driver = utils.import_object(network_driver)
         self.network_api = network_api.API()
+        self.compute_api = compute_api.API()
         super(NetworkManager, self).__init__(service_name='network',
                                                 *args, **kwargs)
 
@@ -368,6 +370,15 @@ class NetworkManager(manager.SchedulerDependentManager):
                                         self.host)
         return host
 
+    def _do_trigger_security_group_members_refresh_for_instance(self,
+                                                                instance_id):
+        admin_context = context.get_admin_context()
+        instance_ref = self.db.instance_get(admin_context, instance_id)
+        groups = instance_ref['security_groups']
+        group_ids = [group['id'] for group in groups]
+        self.compute_api.trigger_security_group_members_refresh(admin_context,
+                                                                    group_ids)
+
     def _get_networks_for_instance(self, context, instance_id, project_id):
         """Determine & return which networks an instance should connect to."""
         # TODO(tr3buchet) maybe this needs to be updated in the future if
@@ -559,6 +570,8 @@ class NetworkManager(manager.SchedulerDependentManager):
             address = self.db.fixed_ip_associate_pool(context.elevated(),
                                                       network['id'],
                                                       instance_id)
+            self._do_trigger_security_group_members_refresh_for_instance(
+                                                                   instance_id)
             get_vif = self.db.virtual_interface_get_by_instance_and_network
             vif = get_vif(context, instance_id, network['id'])
             values = {'allocated': True,
@@ -573,6 +586,11 @@ class NetworkManager(manager.SchedulerDependentManager):
         self.db.fixed_ip_update(context, address,
                                 {'allocated': False,
                                  'virtual_interface_id': None})
+        fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address)
+        instance_ref = fixed_ip_ref['instance']
+        instance_id = instance_ref['id']
+        self._do_trigger_security_group_members_refresh_for_instance(
+                                                                   instance_id)
 
     def lease_fixed_ip(self, context, address):
         """Called by dhcp-bridge when ip is leased."""
@@ -913,7 +931,8 @@ class VlanManager(RPCAllocateFixedIP, FloatingIP, NetworkManager):
             address = self.db.fixed_ip_associate_pool(context,
                                                       network['id'],
                                                       instance_id)
-
+            self._do_trigger_security_group_members_refresh_for_instance(
+                                                                   instance_id)
         vif = self.db.virtual_interface_get_by_instance_and_network(context,
                                                                  instance_id,
                                                                  network['id'])
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 2180cf4f0..8bdfd71b4 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -71,12 +71,12 @@ def _create_network_info(count=1, ipv6=None):
     return [(network, mapping) for x in xrange(0, count)]
 
 
-def _setup_networking(instance_id, ip='1.2.3.4'):
+def _setup_networking(instance_id, ip='1.2.3.4', mac='56:12:12:12:12:12'):
     ctxt = context.get_admin_context()
     network_ref = db.project_get_networks(ctxt,
                                            'fake',
                                            associate=True)[0]
-    vif = {'address': '56:12:12:12:12:12',
+    vif = {'address': mac,
            'network_id': network_ref['id'],
            'instance_id': instance_id}
     vif_ref = db.virtual_interface_create(ctxt, vif)
@@ -884,7 +884,11 @@ class IptablesFirewallTestCase(test.TestCase):
 
     def test_static_filters(self):
         instance_ref = self._create_instance_ref()
-        _setup_networking(instance_ref['id'], self.test_ip)
+        src_instance_ref = self._create_instance_ref()
+        src_ip = '10.11.12.14'
+        src_mac = '56:12:12:12:12:13'
+        _setup_networking(instance_ref['id'], self.test_ip, src_mac)
+        _setup_networking(src_instance_ref['id'], src_ip)
 
         admin_ctxt = context.get_admin_context()
         secgroup = db.security_group_create(admin_ctxt,
@@ -893,6 +897,12 @@ class IptablesFirewallTestCase(test.TestCase):
                                              'name': 'testgroup',
                                              'description': 'test group'})
 
+        src_secgroup = db.security_group_create(admin_ctxt,
+                                                {'user_id': 'fake',
+                                                 'project_id': 'fake',
+                                                 'name': 'testsourcegroup',
+                                                 'description': 'src group'})
+
         db.security_group_rule_create(admin_ctxt,
                                       {'parent_group_id': secgroup['id'],
                                        'protocol': 'icmp',
@@ -914,9 +924,19 @@ class IptablesFirewallTestCase(test.TestCase):
                                        'to_port': 81,
                                        'cidr': '192.168.10.0/24'})
 
+        db.security_group_rule_create(admin_ctxt,
+                                      {'parent_group_id': secgroup['id'],
+                                       'protocol': 'tcp',
+                                       'from_port': 80,
+                                       'to_port': 81,
+                                       'group_id': src_secgroup['id']})
+
         db.instance_add_security_group(admin_ctxt, instance_ref['id'],
                                        secgroup['id'])
+        db.instance_add_security_group(admin_ctxt, src_instance_ref['id'],
+                                       src_secgroup['id'])
         instance_ref = db.instance_get(admin_ctxt, instance_ref['id'])
+        src_instance_ref = db.instance_get(admin_ctxt, src_instance_ref['id'])
 
 #        self.fw.add_instance(instance_ref)
         def fake_iptables_execute(*cmd, **kwargs):
@@ -969,17 +989,22 @@ class IptablesFirewallTestCase(test.TestCase):
         self.assertTrue(security_group_chain,
                         "The security group chain wasn't added")
 
-        regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -j ACCEPT')
+        regex = re.compile('-A .* -j ACCEPT -p icmp -s 192.168.11.0/24')
         self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "ICMP acceptance rule wasn't added")
 
-        regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -m icmp '
-                           '--icmp-type 8 -j ACCEPT')
+        regex = re.compile('-A .* -j ACCEPT -p icmp -m icmp --icmp-type 8'
+                           ' -s 192.168.11.0/24')
         self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "ICMP Echo Request acceptance rule wasn't added")
 
-        regex = re.compile('-A .* -p tcp -s 192.168.10.0/24 -m multiport '
-                           '--dports 80:81 -j ACCEPT')
+        regex = re.compile('-A .* -j ACCEPT -p tcp -m multiport '
+                           '--dports 80:81 -s %s' % (src_ip,))
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
+                        "TCP port 80/81 acceptance rule wasn't added")
+
+        regex = re.compile('-A .* -j ACCEPT -p tcp '
+                           '-m multiport --dports 80:81 -s 192.168.10.0/24')
         self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "TCP port 80/81 acceptance rule wasn't added")
         db.instance_destroy(admin_ctxt, instance_ref['id'])
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index 547a7a1fa..c673f5d06 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -210,7 +210,11 @@ class VlanNetworkTestCase(test.TestCase):
         self.mox.StubOutWithMock(db, 'fixed_ip_update')
         self.mox.StubOutWithMock(db,
                               'virtual_interface_get_by_instance_and_network')
+        self.mox.StubOutWithMock(db, 'instance_get')
 
+        db.instance_get(mox.IgnoreArg(),
+                        mox.IgnoreArg()).AndReturn({'security_groups':
+                                                             [{'id': 0}]})
         db.fixed_ip_associate_pool(mox.IgnoreArg(),
                                    mox.IgnoreArg(),
                                    mox.IgnoreArg()).AndReturn('192.168.0.1')
diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py
index 9ce57b6c9..16e5070c6 100644
--- a/nova/virt/libvirt/firewall.py
+++ b/nova/virt/libvirt/firewall.py
@@ -664,11 +664,10 @@ class IptablesFirewallDriver(FirewallDriver):
                 LOG.debug(_('Adding security group rule: %r'), rule)
 
                 if not rule.cidr:
-                    # Eventually, a mechanism to grant access for security
-                    # groups will turn up here. It'll use ipsets.
-                    continue
+                    version = 4
+                else:
+                    version = netutils.get_ip_version(rule.cidr)
 
-                version = netutils.get_ip_version(rule.cidr)
                 if version == 4:
                     fw_rules = ipv4_rules
                 else:
@@ -678,16 +677,16 @@ class IptablesFirewallDriver(FirewallDriver):
                 if version == 6 and rule.protocol == 'icmp':
                     protocol = 'icmpv6'
 
-                args = ['-p', protocol, '-s', rule.cidr]
+                args = ['-j ACCEPT', '-p', protocol]
 
-                if rule.protocol in ['udp', 'tcp']:
+                if protocol in ['udp', 'tcp']:
                     if rule.from_port == rule.to_port:
                         args += ['--dport', '%s' % (rule.from_port,)]
                     else:
                         args += ['-m', 'multiport',
                                  '--dports', '%s:%s' % (rule.from_port,
                                                         rule.to_port)]
-                elif rule.protocol == 'icmp':
+                elif protocol == 'icmp':
                     icmp_type = rule.from_port
                     icmp_code = rule.to_port
 
@@ -706,9 +705,22 @@ class IptablesFirewallDriver(FirewallDriver):
                             args += ['-m', 'icmp6', '--icmpv6-type',
                                      icmp_type_arg]
 
-                args += ['-j ACCEPT']
-                fw_rules += [' '.join(args)]
-
+                if rule.cidr:
+                    LOG.info('Using cidr %r', rule.cidr)
+                    args += ['-s', rule.cidr]
+                    fw_rules += [' '.join(args)]
+                else:
+                    if rule['grantee_group']:
+                        for instance in rule['grantee_group']['instances']:
+                            LOG.info('instance: %r', instance)
+                            ips = db.instance_get_fixed_addresses(ctxt,
+                                                                instance['id'])
+                            LOG.info('ips: %r', ips)
+                            for ip in ips:
+                                subrule = args + ['-s %s' % ip]
+                                fw_rules += [' '.join(subrule)]
+
+                LOG.info('Using fw_rules: %r', fw_rules)
         ipv4_rules += ['-j $sg-fallback']
         ipv6_rules += ['-j $sg-fallback']
 
@@ -719,7 +731,8 @@ class IptablesFirewallDriver(FirewallDriver):
         return self.nwfilter.instance_filter_exists(instance)
 
     def refresh_security_group_members(self, security_group):
-        pass
+        self.do_refresh_security_group_rules(security_group)
+        self.iptables.apply()
 
     def refresh_security_group_rules(self, security_group, network_info=None):
         self.do_refresh_security_group_rules(security_group, network_info)