From: Kevin Benton <blak111@gmail.com>
Date: Mon, 5 Oct 2015 14:06:54 +0000 (-0700)
Subject: Remove excessive fallback iptables ACCEPT rules
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ea8de8a9dd808b1379060e0ce578b8129f45c3ae;p=openstack-build%2Fneutron-build.git

Remove excessive fallback iptables ACCEPT rules

The previous code was generating a fallback ACCEPT rule for every
port when there should only be one at the very end. The reason that
this wasn't causing a bug is because we have a duplicate rule remover
that was silently throwing away the extras and it happened to get them
in the right order.

Closes-Bug: #1502906
Change-Id: I83cf574f93b512be1ccefdc8da63e1783d279233
---

diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py
index b0ac0f793..7e88db66d 100644
--- a/neutron/agent/linux/iptables_firewall.py
+++ b/neutron/agent/linux/iptables_firewall.py
@@ -209,8 +209,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
         for port in ports.values():
             self._setup_chain(port, firewall.INGRESS_DIRECTION)
             self._setup_chain(port, firewall.EGRESS_DIRECTION)
-            self.iptables.ipv4['filter'].add_rule(SG_CHAIN, '-j ACCEPT')
-            self.iptables.ipv6['filter'].add_rule(SG_CHAIN, '-j ACCEPT')
+        self.iptables.ipv4['filter'].add_rule(SG_CHAIN, '-j ACCEPT')
+        self.iptables.ipv6['filter'].add_rule(SG_CHAIN, '-j ACCEPT')
 
         for port in unfiltered_ports.values():
             self._add_accept_rule_port_sec(port, firewall.INGRESS_DIRECTION)
diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py
index 24c1b5d8b..fc4f81660 100644
--- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py
+++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py
@@ -1696,6 +1696,18 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase):
         self.assertFalse(self.firewall.sg_members)
         self.assertFalse(self.firewall.sg_rules)
 
+    def test_single_fallback_accept_rule(self):
+        p1, p2 = self._fake_port(), self._fake_port()
+        self.firewall._setup_chains_apply(dict(p1=p1, p2=p2), {})
+        v4_adds = self.firewall.iptables.ipv4['filter'].add_rule.mock_calls
+        v6_adds = self.firewall.iptables.ipv6['filter'].add_rule.mock_calls
+        sg_chain_v4_accept = [call for call in v4_adds
+                              if call == mock.call('sg-chain', '-j ACCEPT')]
+        sg_chain_v6_accept = [call for call in v6_adds
+                              if call == mock.call('sg-chain', '-j ACCEPT')]
+        self.assertEqual(1, len(sg_chain_v4_accept))
+        self.assertEqual(1, len(sg_chain_v6_accept))
+
     def test_prepare_port_filter_with_deleted_member(self):
         self.firewall.sg_rules = self._fake_sg_rules()
         self.firewall.pre_sg_rules = self._fake_sg_rules()
diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py
index 7bf968388..668071c8e 100644
--- a/neutron/tests/unit/agent/test_securitygroups_rpc.py
+++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py
@@ -2340,6 +2340,7 @@ IPTABLES_FILTER_EMPTY = """# Generated by iptables_manager
 [0:0] -A OUTPUT -j %(bn)s-OUTPUT
 [0:0] -A FORWARD -j %(bn)s-FORWARD
 [0:0] -A %(bn)s-sg-fallback -j DROP
+[0:0] -A %(bn)s-sg-chain -j ACCEPT
 COMMIT
 # Completed by iptables_manager
 """ % IPTABLES_ARG
@@ -2488,6 +2489,7 @@ IPTABLES_FILTER_V6_EMPTY = """# Generated by iptables_manager
 [0:0] -A OUTPUT -j %(bn)s-OUTPUT
 [0:0] -A FORWARD -j %(bn)s-FORWARD
 [0:0] -A %(bn)s-sg-fallback -j DROP
+[0:0] -A %(bn)s-sg-chain -j ACCEPT
 COMMIT
 # Completed by iptables_manager
 """ % IPTABLES_ARG