Blueprint xenapi-provider-firewall and Bug #915403.

  1. Provides dom0 IPtables driver to implement the Provider firewall rules.
  2. Existing libvirt code has been refactored to reduce the amount of duplicated code to a minimum
  3. The three provider apis in ec2/admin.py file are now fixed the following way:
    a.    remove_external_address_block returned 'OK' on removing blocks which didn't exist. This is now fixed.
    b.    block_external_addresses raised exception earlier on duplicate network blocks. Now the exception is logged and failed status message is returned.
    c.  all the three provider apis now logs for invalid and improper inputs and return uniform (a dictionary ) and proper status messages for all cases.
  4. appropriate unit tests added to cover the same

Change-Id: I27d83186f850423a6268947aed0c9a349d8f8d65

6 files changed, 111 insertions, 47 deletions

diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 7a9347542..fa40160f6 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -380,7 +380,7 @@ class ComputeDriver(object):
             * another host 'H1' runs an instance 'i-1'
             * instance 'i-1' is a member of security group 'b'
 
-            When 'i-1' launches or terminates we will recieve the message
+            When 'i-1' launches or terminates we will receive the message
             to update members of group 'b', at which time we will make
             any changes needed to the rules for instance 'i-0' to allow
             or deny traffic coming from 'i-1', depending on if it is being
@@ -399,7 +399,7 @@ class ComputeDriver(object):
         # TODO(Vek): Need to pass context in for access to auth_token
         raise NotImplementedError()
 
-    def refresh_provider_fw_rules(self, security_group_id):
+    def refresh_provider_fw_rules(self):
         """This triggers a firewall update based on database changes.
 
         When this is called, rules have either been added or removed from the
diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py
index 3426120ac..ed60051d9 100644
--- a/nova/virt/firewall.py
+++ b/nova/virt/firewall.py
@@ -34,7 +34,7 @@ flags.DEFINE_bool('allow_same_net_traffic',
 class FirewallDriver(object):
     """ Firewall Driver base class.
 
-        Defines methos that any driver providing security groups
+        Defines methods that any driver providing security groups
         and provider fireall functionality should implement.
     """
     def prepare_instance_filter(self, instance, network_info):
@@ -129,12 +129,18 @@ class IptablesFirewallDriver(FirewallDriver):
         self.instances[instance['id']] = instance
         self.network_infos[instance['id']] = network_info
         self.add_filters_for_instance(instance)
+        LOG.debug(_('Filters added to the instance: %r'), instance)
+        self.refresh_provider_fw_rules()
+        LOG.debug(_('Provider Firewall Rules refreshed'))
         self.iptables.apply()
 
     def _create_filter(self, ips, chain_name):
         return ['-d %s -j $%s' % (ip, chain_name) for ip in ips]
 
     def _filters_for_instance(self, chain_name, network_info):
+        """Creates a rule corresponding to each ip that defines a
+             jump to the corresponding instance - chain for all the traffic
+             destined to that ip"""
         ips_v4 = [ip['ip'] for (_n, mapping) in network_info
                  for ip in mapping['ips']]
         ipv4_rules = self._create_filter(ips_v4, chain_name)
@@ -190,6 +196,10 @@ class IptablesFirewallDriver(FirewallDriver):
         ipv4_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
         ipv6_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
 
+        # Pass through provider-wide drops
+        ipv4_rules += ['-j $provider']
+        ipv6_rules += ['-j $provider']
+
     def _do_dhcp_rules(self, ipv4_rules, network_info):
         dhcp_servers = [info['dhcp_server'] for (_n, info) in network_info]
 
@@ -345,3 +355,38 @@ class IptablesFirewallDriver(FirewallDriver):
         for instance in self.instances.values():
             self.remove_filters_for_instance(instance)
             self.add_filters_for_instance(instance)
+
+    def refresh_provider_fw_rules(self):
+        """See class:FirewallDriver: docs."""
+        self._do_refresh_provider_fw_rules()
+        self.iptables.apply()
+
+    @utils.synchronized('iptables', external=True)
+    def _do_refresh_provider_fw_rules(self):
+        """Internal, synchronized version of refresh_provider_fw_rules."""
+        self._purge_provider_fw_rules()
+        self._build_provider_fw_rules()
+
+    def _purge_provider_fw_rules(self):
+        """Remove all rules from the provider chains."""
+        self.iptables.ipv4['filter'].empty_chain('provider')
+        if FLAGS.use_ipv6:
+            self.iptables.ipv6['filter'].empty_chain('provider')
+
+    def _build_provider_fw_rules(self):
+        """Create all rules for the provider IP DROPs."""
+        self.iptables.ipv4['filter'].add_chain('provider')
+        if FLAGS.use_ipv6:
+            self.iptables.ipv6['filter'].add_chain('provider')
+        ipv4_rules, ipv6_rules = self._provider_rules()
+        for rule in ipv4_rules:
+            self.iptables.ipv4['filter'].add_rule('provider', rule)
+
+        if FLAGS.use_ipv6:
+            for rule in ipv6_rules:
+                self.iptables.ipv6['filter'].add_rule('provider', rule)
+
+    @staticmethod
+    def _provider_rules():
+        """Generate a list of rules from provider for IP4 & IP6."""
+        raise NotImplementedError()
diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py
index 9b9e3540e..3e299b932 100644
--- a/nova/virt/libvirt/firewall.py
+++ b/nova/virt/libvirt/firewall.py
@@ -259,7 +259,7 @@ class NWFilterFirewall(base_firewall.FirewallDriver):
                 LOG.debug(_('The nwfilter(%(instance_filter_name)s) '
                             'for %(instance_name)s is not found.') % locals())
 
-        instance_secgroup_filter_name =\
+        instance_secgroup_filter_name = \
             '%s-secgroup' % (self._instance_filter_name(instance))
 
         try:
@@ -486,53 +486,10 @@ class IptablesFirewallDriver(base_firewall.IptablesFirewallDriver):
             LOG.info(_('Attempted to unfilter instance %s which is not '
                      'filtered'), instance['id'])
 
-    def _do_basic_rules(self, ipv4_rules, ipv6_rules, network_info):
-        # Always drop invalid packets
-        ipv4_rules += ['-m state --state ' 'INVALID -j DROP']
-        ipv6_rules += ['-m state --state ' 'INVALID -j DROP']
-
-        # Allow established connections
-        ipv4_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
-        ipv6_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
-
-        # Pass through provider-wide drops
-        ipv4_rules += ['-j $provider']
-        ipv6_rules += ['-j $provider']
-
     def instance_filter_exists(self, instance, network_info):
         """Check nova-instance-instance-xxx exists"""
         return self.nwfilter.instance_filter_exists(instance, network_info)
 
-    def refresh_provider_fw_rules(self):
-        """See class:FirewallDriver: docs."""
-        self._do_refresh_provider_fw_rules()
-        self.iptables.apply()
-
-    @utils.synchronized('iptables', external=True)
-    def _do_refresh_provider_fw_rules(self):
-        """Internal, synchronized version of refresh_provider_fw_rules."""
-        self._purge_provider_fw_rules()
-        self._build_provider_fw_rules()
-
-    def _purge_provider_fw_rules(self):
-        """Remove all rules from the provider chains."""
-        self.iptables.ipv4['filter'].empty_chain('provider')
-        if FLAGS.use_ipv6:
-            self.iptables.ipv6['filter'].empty_chain('provider')
-
-    def _build_provider_fw_rules(self):
-        """Create all rules for the provider IP DROPs."""
-        self.iptables.ipv4['filter'].add_chain('provider')
-        if FLAGS.use_ipv6:
-            self.iptables.ipv6['filter'].add_chain('provider')
-        ipv4_rules, ipv6_rules = self._provider_rules()
-        for rule in ipv4_rules:
-            self.iptables.ipv4['filter'].add_rule('provider', rule)
-
-        if FLAGS.use_ipv6:
-            for rule in ipv6_rules:
-                self.iptables.ipv6['filter'].add_rule('provider', rule)
-
     @staticmethod
     def _provider_rules():
         """Generate a list of rules from provider for IP4 & IP6."""
diff --git a/nova/virt/xenapi/firewall.py b/nova/virt/xenapi/firewall.py
index 8f6f17de3..41d67de5c 100644
--- a/nova/virt/xenapi/firewall.py
+++ b/nova/virt/xenapi/firewall.py
@@ -19,8 +19,12 @@
 
 import json
 
+
+from nova import context
 from nova import flags
 from nova import log as logging
+from nova.db import api as db
+from nova.virt import netutils
 from nova.virt.firewall import IptablesFirewallDriver
 
 
@@ -65,3 +69,55 @@ class Dom0IptablesFirewallDriver(IptablesFirewallDriver):
             #  No multiport needed for XS!
             return ['--dport', '%s:%s' % (rule.from_port,
                                            rule.to_port)]
+
+    @staticmethod
+    def _provider_rules():
+        """Generate a list of rules from provider for IP4 & IP6.
+        Note: We could not use the common code from virt.firewall because
+        XS doesn't accept the '-m multiport' option"""
+
+        ctxt = context.get_admin_context()
+        ipv4_rules = []
+        ipv6_rules = []
+        rules = db.provider_fw_rule_get_all(ctxt)
+        for rule in rules:
+            LOG.debug(_('Adding provider rule: %s'), rule['cidr'])
+            version = netutils.get_ip_version(rule['cidr'])
+            if version == 4:
+                fw_rules = ipv4_rules
+            else:
+                fw_rules = ipv6_rules
+
+            protocol = rule['protocol']
+            if version == 6 and protocol == 'icmp':
+                protocol = 'icmpv6'
+
+            args = ['-p', protocol, '-s', rule['cidr']]
+
+            if protocol in ['udp', 'tcp']:
+                if rule['from_port'] == rule['to_port']:
+                    args += ['--dport', '%s' % (rule['from_port'],)]
+                else:
+                    args += ['--dport', '%s:%s' % (rule['from_port'],
+                                                    rule['to_port'])]
+            elif protocol == 'icmp':
+                icmp_type = rule['from_port']
+                icmp_code = rule['to_port']
+
+                if icmp_type == -1:
+                    icmp_type_arg = None
+                else:
+                    icmp_type_arg = '%s' % icmp_type
+                    if not icmp_code == -1:
+                        icmp_type_arg += '/%s' % icmp_code
+
+                if icmp_type_arg:
+                    if version == 4:
+                        args += ['-m', 'icmp', '--icmp-type',
+                                 icmp_type_arg]
+                    elif version == 6:
+                        args += ['-m', 'icmp6', '--icmpv6-type',
+                                 icmp_type_arg]
+            args += ['-j DROP']
+            fw_rules += [' '.join(args)]
+        return ipv4_rules, ipv6_rules
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 1309879b5..f681aceef 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -1744,6 +1744,9 @@ class VMOps(object):
         """ recreates security group rules for every instance """
         self.firewall_driver.refresh_security_group_members(security_group_id)
 
+    def refresh_provider_fw_rules(self):
+        self.firewall_driver.refresh_provider_fw_rules()
+
     def unfilter_instance(self, instance_ref, network_info):
         """Removes filters for each VIF of the specified instance."""
         self.firewall_driver.unfilter_instance(instance_ref,
diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py
index 1bd0b52a5..42f035217 100644
--- a/nova/virt/xenapi_conn.py
+++ b/nova/virt/xenapi_conn.py
@@ -443,6 +443,9 @@ class XenAPIConnection(driver.ComputeDriver):
         """
         return self._vmops.refresh_security_group_members(security_group_id)
 
+    def refresh_provider_fw_rules(self):
+        return self._vmops.refresh_provider_fw_rules()
+
     def update_host_status(self):
         """Update the status info of the host, and return those values
             to the calling program."""