firewall: fix ignored rule handling

circle/firewall/fw.py

@@ -33,6 +33,7 @@ class BuildFirewall:
     def build_ipt_nat(self):
     def build_ipt_nat(self):
         # portforward
         # portforward
         for rule in Rule.objects.filter(
         for rule in Rule.objects.filter(
+                action__in=['accept', 'drop'],
                 nat=True, direction='in').select_related('host'):
                 nat=True, direction='in').select_related('host'):
             self.add_rules(PREROUTING=IptRule(
             self.add_rules(PREROUTING=IptRule(
                 priority=1000,
                 priority=1000,
@@ -55,7 +56,8 @@ class BuildFirewall:
     def ipt_filter_firewall(self):
     def ipt_filter_firewall(self):
         """Build firewall's own rules."""
         """Build firewall's own rules."""
 
 
-        for rule in Rule.objects.exclude(firewall=None).select_related(
+        rules = Rule.objects.filter(action__in=['accept', 'drop'])
+        for rule in rules.exclude(firewall=None).select_related(
                 'foreign_network').prefetch_related('foreign_network__vlans'):
                 'foreign_network').prefetch_related('foreign_network__vlans'):
             self.add_rules(**rule.get_ipt_rules())
             self.add_rules(**rule.get_ipt_rules())
 
 
@@ -63,12 +65,13 @@ class BuildFirewall:
         """Build hosts' rules."""
         """Build hosts' rules."""
 
 
         # host rules
         # host rules
-        for rule in Rule.objects.exclude(host=None).select_related(
-                'foreign_network', 'host',
-                'host__vlan').prefetch_related('foreign_network__vlans'):
+        rules = Rule.objects.filter(action__in=['accept', 'drop'])
+        for rule in rules.exclude(host=None).select_related(
+                'foreign_network', 'host', 'host__vlan').prefetch_related(
+                'foreign_network__vlans'):
             self.add_rules(**rule.get_ipt_rules(rule.host))
             self.add_rules(**rule.get_ipt_rules(rule.host))
         # group rules
         # group rules
-        for rule in Rule.objects.exclude(hostgroup=None).select_related(
+        for rule in rules.exclude(hostgroup=None).select_related(
                 'hostgroup', 'foreign_network').prefetch_related(
                 'hostgroup', 'foreign_network').prefetch_related(
                 'hostgroup__host_set__vlan', 'foreign_network__vlans'):
                 'hostgroup__host_set__vlan', 'foreign_network__vlans'):
             for host in rule.hostgroup.host_set.all():
             for host in rule.hostgroup.host_set.all():
@@ -77,7 +80,8 @@ class BuildFirewall:
     def ipt_filter_vlan_rules(self):
     def ipt_filter_vlan_rules(self):
         """Enable communication between VLANs."""
         """Enable communication between VLANs."""
 
 
-        for rule in Rule.objects.exclude(vlan=None).select_related(
+        rules = Rule.objects.filter(action__in=['accept', 'drop'])
+        for rule in rules.exclude(vlan=None).select_related(
                 'vlan', 'foreign_network').prefetch_related(
                 'vlan', 'foreign_network').prefetch_related(
                 'foreign_network__vlans'):
                 'foreign_network__vlans'):
             self.add_rules(**rule.get_ipt_rules())
             self.add_rules(**rule.get_ipt_rules())

circle/firewall/iptables.py

@@ -16,7 +16,7 @@ class IptRule(object):
 
 
     def __init__(self, priority=1000, action=None, src=None, dst=None,
     def __init__(self, priority=1000, action=None, src=None, dst=None,
                  proto=None, sport=None, dport=None, extra=None,
                  proto=None, sport=None, dport=None, extra=None,
-                 ipv4_only=False, ignored=False):
+                 ipv4_only=False):
         if proto not in ['tcp', 'udp', 'icmp', None]:
         if proto not in ['tcp', 'udp', 'icmp', None]:
             raise InvalidRuleExcepion()
             raise InvalidRuleExcepion()
         if proto not in ['tcp', 'udp'] and (sport is not None or
         if proto not in ['tcp', 'udp'] and (sport is not None or
@@ -44,7 +44,6 @@ class IptRule(object):
         self.extra = extra
         self.extra = extra
         self.ipv4_only = (ipv4_only or
         self.ipv4_only = (ipv4_only or
                           extra is not None and bool(ipv4_re.search(extra)))
                           extra is not None and bool(ipv4_re.search(extra)))
-        self.ignored = ignored
 
 
     def __hash__(self):
     def __hash__(self):
         return hash(frozenset(self.__dict__.items()))
         return hash(frozenset(self.__dict__.items()))
@@ -72,8 +71,6 @@ class IptRule(object):
         params = [opts[param] % getattr(self, param)
         params = [opts[param] % getattr(self, param)
                   for param in opts
                   for param in opts
                   if getattr(self, param) is not None]
                   if getattr(self, param) is not None]
-        if self.ignored:
-            params.insert(0, '# ')
         return ' '.join(params)
         return ' '.join(params)
 
 
 
 

circle/firewall/models.py

@@ -210,8 +210,7 @@ class Rule(models.Model):
         for foreign_vlan in self.foreign_network.vlans.all():
         for foreign_vlan in self.foreign_network.vlans.all():
             r = IptRule(priority=self.weight, action=action,
             r = IptRule(priority=self.weight, action=action,
                         proto=self.proto, extra=self.extra,
                         proto=self.proto, extra=self.extra,
-                        src=src, dst=dst, dport=dport, sport=sport,
-                        ignored=(self.action == 'ignore'))
+                        src=src, dst=dst, dport=dport, sport=sport)
             # host, hostgroup or vlan rule
             # host, hostgroup or vlan rule
             if host or self.vlan_id:
             if host or self.vlan_id:
                 local_vlan = host.vlan.name if host else self.vlan.name
                 local_vlan = host.vlan.name if host else self.vlan.name

circle/firewall/tests/test_firewall.py

@@ -140,9 +140,6 @@ class IptablesTestCase(TestCase):
                   IptRule(priority=2, action='ACCEPT',
                   IptRule(priority=2, action='ACCEPT',
                           dst=('127.0.0.2', None),
                           dst=('127.0.0.2', None),
                           proto='icmp'),
                           proto='icmp'),
-                  IptRule(priority=10, action='ACCEPT',
-                          dst=('127.0.0.10', None),
-                          proto='icmp', ignored=True),
                   IptRule(priority=6, action='ACCEPT',
                   IptRule(priority=6, action='ACCEPT',
                           dst=('127.0.0.6', None),
                           dst=('127.0.0.6', None),
                           proto='tcp', dport='1337')]
                           proto='tcp', dport='1337')]
@@ -157,9 +154,6 @@ class IptablesTestCase(TestCase):
         self.assertEqual(self.r[5].compile(),
         self.assertEqual(self.r[5].compile(),
                          '-d 127.0.0.5 -p tcp --dport 443 -g ACCEPT')
                          '-d 127.0.0.5 -p tcp --dport 443 -g ACCEPT')
 
 
-    def test_ignored_rule_compile_ok(self):
-        assert self.r[7].compile().startswith('# ')
-
     def test_rule_compile_fail(self):
     def test_rule_compile_fail(self):
         self.assertRaises(InvalidRuleExcepion,
         self.assertRaises(InvalidRuleExcepion,
                           IptRule, **{'proto': 'test'})
                           IptRule, **{'proto': 'test'})