From 0325b98c54af276064e367db603bfeb525bbb790 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 1 May 2015 10:59:03 -0400 Subject: [PATCH] Set IPset hash type to 'net' instead of 'ip' The previous hash type was 'ip' and this caused a major issue with the allowed address pairs extension since it results in CIDRs being passed to ipset. When the hash type is 'ip', a CIDR is completely enumerated into all of its addresses so 10.100.0.0/16 results in ~65k entries. This meant a single allowed_address_pairs entry could easily exhaust an entire set. This patch changes the hash type to 'net', which is designed to handle a CIDRs as a single entry. This patch also changes the names of the ipsets because creating an ipset with different parameters will cause an error and our ipset manager code isn't robust enough to handle that at this time. Related-Bug: #1439817 Related-Bug: #1444397 (based on commit a38b5df5cd3c47672705aad4c30e789ae11ec958) Change-Id: I8177699b157cd3eac46e2f481f47b5d966c49b07 --- neutron/agent/linux/ipset_manager.py | 6 +- neutron/agent/linux/iptables_firewall.py | 8 +-- neutron/tests/unit/test_iptables_firewall.py | 71 +++++++++++-------- .../tests/unit/test_security_groups_rpc.py | 10 +-- 4 files changed, 52 insertions(+), 43 deletions(-) diff --git a/neutron/agent/linux/ipset_manager.py b/neutron/agent/linux/ipset_manager.py index ddd736e8e8a..261865d3e37 100644 --- a/neutron/agent/linux/ipset_manager.py +++ b/neutron/agent/linux/ipset_manager.py @@ -25,7 +25,7 @@ class IpsetManager(object): @utils.synchronized('ipset', external=True) def create_ipset_chain(self, chain_name, ethertype): - cmd = ['ipset', 'create', '-exist', chain_name, 'hash:ip', 'family', + cmd = ['ipset', 'create', '-exist', chain_name, 'hash:net', 'family', self._get_ipset_chain_type(ethertype)] self._apply(cmd) @@ -38,8 +38,8 @@ class IpsetManager(object): def refresh_ipset_chain_by_name(self, chain_name, member_ips, ethertype): new_chain_name = chain_name + '-new' chain_type = self._get_ipset_chain_type(ethertype) - process_input = ["create %s hash:ip family %s" % (new_chain_name, - chain_type)] + process_input = ["create %s hash:net family %s" % (new_chain_name, + chain_type)] for ip in member_ips: process_input.append("add %s %s" % (new_chain_name, ip)) diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index 6afa0a46621..a4bca75defa 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -37,7 +37,7 @@ DIRECTION_IP_PREFIX = {'ingress': 'source_ip_prefix', IPSET_DIRECTION = {INGRESS_DIRECTION: 'src', EGRESS_DIRECTION: 'dst'} LINUX_DEV_LEN = 14 -IPSET_CHAIN_LEN = 20 +IPSET_CHAIN_LEN = 17 IPSET_CHANGE_BULK_THRESHOLD = 10 IPSET_ADD_BULK_THRESHOLD = 5 @@ -384,7 +384,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver): add_ips = self._get_new_sg_member_ips(sg_id, ethertype) del_ips = self._get_deleted_sg_member_ips(sg_id, ethertype) cur_member_ips = self._get_cur_sg_member_ips(sg_id, ethertype) - chain_name = ethertype + sg_id[:IPSET_CHAIN_LEN] + chain_name = 'NET' + ethertype + sg_id[:IPSET_CHAIN_LEN] if chain_name not in self.ipset_chains and cur_member_ips: self.ipset_chains[chain_name] = [] self.ipset.create_ipset_chain( @@ -414,7 +414,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver): ethertype = sg_rule.get('ethertype') # the length of ipset chain name require less than 31 # characters - ipset_chain_name = (ethertype + remote_gid[:IPSET_CHAIN_LEN]) + ipset_chain_name = ('NET' + ethertype + remote_gid[:IPSET_CHAIN_LEN]) if ipset_chain_name in self.ipset_chains: args += ['-m set', '--match-set', ipset_chain_name, @@ -539,7 +539,7 @@ class IptablesFirewallDriver(firewall.FirewallDriver): if self.enable_ipset: for ethertype in ['IPv4', 'IPv6']: removed_chain = ( - ethertype + remove_chain_id[:IPSET_CHAIN_LEN]) + 'NET' + ethertype + remove_chain_id[:IPSET_CHAIN_LEN]) if removed_chain in self.ipset_chains: self.ipset.destroy_ipset_chain_by_name(removed_chain) self.ipset_chains.pop(removed_chain, None) diff --git a/neutron/tests/unit/test_iptables_firewall.py b/neutron/tests/unit/test_iptables_firewall.py index d94da9aac83..ca8aff7ac5c 100644 --- a/neutron/tests/unit/test_iptables_firewall.py +++ b/neutron/tests/unit/test_iptables_firewall.py @@ -1256,12 +1256,12 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): self.firewall.pre_sg_members = {} port = self._fake_port() self.firewall.prepare_port_filter(port) - calls = [mock.call.create_ipset_chain('IPv4fake_sgid', 'IPv4'), + calls = [mock.call.create_ipset_chain('NETIPv4fake_sgid', 'IPv4'), mock.call.refresh_ipset_chain_by_name( - 'IPv4fake_sgid', ['10.0.0.1', '10.0.0.2'], 'IPv4'), - mock.call.create_ipset_chain('IPv6fake_sgid', 'IPv6'), + 'NETIPv4fake_sgid', ['10.0.0.1', '10.0.0.2'], 'IPv4'), + mock.call.create_ipset_chain('NETIPv6fake_sgid', 'IPv6'), mock.call.refresh_ipset_chain_by_name( - 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] + 'NETIPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls) @@ -1273,18 +1273,18 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): self.firewall.pre_sg_members = {} port = self._fake_port() self.firewall.prepare_port_filter(port) - calls = [mock.call.create_ipset_chain('IPv4fake_sgid', 'IPv4'), + calls = [mock.call.create_ipset_chain('NETIPv4fake_sgid', 'IPv4'), mock.call.refresh_ipset_chain_by_name( - 'IPv4fake_sgid', TEST_IP_RANGE[:5], 'IPv4'), - mock.call.create_ipset_chain('IPv6fake_sgid', 'IPv6'), + 'NETIPv4fake_sgid', TEST_IP_RANGE[:5], 'IPv4'), + mock.call.create_ipset_chain('NETIPv6fake_sgid', 'IPv6'), mock.call.refresh_ipset_chain_by_name( - 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] + 'NETIPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls) def test_prepare_port_filter_with_ipset_chain_exist(self): self.firewall.sg_rules = self._fake_sg_rule() - self.firewall.ipset_chains = {'IPv4fake_sgid': ['10.0.0.2']} + self.firewall.ipset_chains = {'NETIPv4fake_sgid': ['10.0.0.2']} self.firewall.sg_members = {'fake_sgid': { 'IPv4': TEST_IP_RANGE[:5], 'IPv6': ['fe80::1']}} @@ -1294,19 +1294,23 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): port = self._fake_port() self.firewall.prepare_port_filter(port) calls = [ - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.1'), - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.3'), - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.4'), - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.5'), - mock.call.create_ipset_chain('IPv6fake_sgid', 'IPv6'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.1'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.3'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.4'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.5'), + mock.call.create_ipset_chain('NETIPv6fake_sgid', 'IPv6'), mock.call.refresh_ipset_chain_by_name( - 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] + 'NETIPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls, True) def test_prepare_port_filter_with_del_member(self): self.firewall.sg_rules = self._fake_sg_rule() - self.firewall.ipset_chains = {'IPv4fake_sgid': ['10.0.0.2']} + self.firewall.ipset_chains = {'NETIPv4fake_sgid': ['10.0.0.2']} self.firewall.sg_members = {'fake_sgid': { 'IPv4': [ '10.0.0.1', '10.0.0.3', '10.0.0.4', '10.0.0.5'], @@ -1317,20 +1321,25 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): port = self._fake_port() self.firewall.prepare_port_filter(port) calls = [ - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.1'), - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.3'), - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.4'), - mock.call.add_member_to_ipset_chain('IPv4fake_sgid', '10.0.0.5'), - mock.call.del_ipset_chain_member('IPv4fake_sgid', '10.0.0.2'), - mock.call.create_ipset_chain('IPv6fake_sgid', 'IPv6'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.1'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.3'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.4'), + mock.call.add_member_to_ipset_chain('NETIPv4fake_sgid', + '10.0.0.5'), + mock.call.del_ipset_chain_member('NETIPv4fake_sgid', + '10.0.0.2'), + mock.call.create_ipset_chain('NETIPv6fake_sgid', 'IPv6'), mock.call.refresh_ipset_chain_by_name( - 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] + 'NETIPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls, True) def test_prepare_port_filter_change_beyond_9(self): self.firewall.sg_rules = self._fake_sg_rule() - self.firewall.ipset_chains = {'IPv4fake_sgid': TEST_IP_RANGE[5:]} + self.firewall.ipset_chains = {'NETIPv4fake_sgid': TEST_IP_RANGE[5:]} self.firewall.sg_members = {'fake_sgid': { 'IPv4': TEST_IP_RANGE[:5], 'IPv6': ['fe80::1']}} @@ -1340,11 +1349,11 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): port = self._fake_port() self.firewall.prepare_port_filter(port) calls = [ - mock.call.refresh_ipset_chain_by_name('IPv4fake_sgid', + mock.call.refresh_ipset_chain_by_name('NETIPv4fake_sgid', TEST_IP_RANGE[:5], 'IPv4'), - mock.call.create_ipset_chain('IPv6fake_sgid', 'IPv6'), + mock.call.create_ipset_chain('NETIPv6fake_sgid', 'IPv6'), mock.call.refresh_ipset_chain_by_name( - 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] + 'NETIPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls) @@ -1359,11 +1368,11 @@ class IptablesFirewallEnhancedIpsetTestCase(BaseIptablesFirewallTestCase): port = self._fake_port() port['security_group_source_groups'].append('fake_sgid2') self.firewall.prepare_port_filter(port) - calls = [mock.call.create_ipset_chain('IPv4fake_sgid', 'IPv4'), + calls = [mock.call.create_ipset_chain('NETIPv4fake_sgid', 'IPv4'), mock.call.refresh_ipset_chain_by_name( - 'IPv4fake_sgid', ['10.0.0.1', '10.0.0.2'], 'IPv4'), - mock.call.create_ipset_chain('IPv6fake_sgid', 'IPv6'), + 'NETIPv4fake_sgid', ['10.0.0.1', '10.0.0.2'], 'IPv4'), + mock.call.create_ipset_chain('NETIPv6fake_sgid', 'IPv6'), mock.call.refresh_ipset_chain_by_name( - 'IPv6fake_sgid', ['fe80::1'], 'IPv6')] + 'NETIPv6fake_sgid', ['fe80::1'], 'IPv6')] self.firewall.ipset.assert_has_calls(calls) diff --git a/neutron/tests/unit/test_security_groups_rpc.py b/neutron/tests/unit/test_security_groups_rpc.py index 21ad43e9122..629b5980dd1 100644 --- a/neutron/tests/unit/test_security_groups_rpc.py +++ b/neutron/tests/unit/test_security_groups_rpc.py @@ -1739,7 +1739,7 @@ IPSET_FILTER_1 = """# Generated by iptables_manager [0:0] -A %(bn)s-i_port1 -s 10.0.0.2/32 -p udp -m udp --sport 67 --dport 68 \ -j RETURN [0:0] -A %(bn)s-i_port1 -p tcp -m tcp --dport 22 -j RETURN -[0:0] -A %(bn)s-i_port1 -m set --match-set IPv4security_group1 src -j \ +[0:0] -A %(bn)s-i_port1 -m set --match-set NETIPv4security_group1 src -j \ RETURN [0:0] -A %(bn)s-i_port1 -j %(bn)s-sg-fallback [0:0] -A %(bn)s-FORWARD %(physdev_mod)s --physdev-EGRESS tap_port1 \ @@ -1898,7 +1898,7 @@ IPSET_FILTER_2 = """# Generated by iptables_manager [0:0] -A %(bn)s-i_port1 -s 10.0.0.2/32 -p udp -m udp --sport 67 --dport 68 \ -j RETURN [0:0] -A %(bn)s-i_port1 -p tcp -m tcp --dport 22 -j RETURN -[0:0] -A %(bn)s-i_port1 -m set --match-set IPv4security_group1 src -j \ +[0:0] -A %(bn)s-i_port1 -m set --match-set NETIPv4security_group1 src -j \ RETURN [0:0] -A %(bn)s-i_port1 -j %(bn)s-sg-fallback [0:0] -A %(bn)s-FORWARD %(physdev_mod)s --physdev-EGRESS tap_port1 \ @@ -1926,7 +1926,7 @@ RETURN [0:0] -A %(bn)s-i_port2 -s 10.0.0.2/32 -p udp -m udp --sport 67 --dport 68 \ -j RETURN [0:0] -A %(bn)s-i_port2 -p tcp -m tcp --dport 22 -j RETURN -[0:0] -A %(bn)s-i_port2 -m set --match-set IPv4security_group1 src -j \ +[0:0] -A %(bn)s-i_port2 -m set --match-set NETIPv4security_group1 src -j \ RETURN [0:0] -A %(bn)s-i_port2 -j %(bn)s-sg-fallback [0:0] -A %(bn)s-FORWARD %(physdev_mod)s --physdev-EGRESS tap_port2 \ @@ -1981,7 +1981,7 @@ IPSET_FILTER_2_3 = """# Generated by iptables_manager [0:0] -A %(bn)s-i_port1 -s 10.0.0.2/32 -p udp -m udp --sport 67 --dport 68 \ -j RETURN [0:0] -A %(bn)s-i_port1 -p tcp -m tcp --dport 22 -j RETURN -[0:0] -A %(bn)s-i_port1 -m set --match-set IPv4security_group1 src -j \ +[0:0] -A %(bn)s-i_port1 -m set --match-set NETIPv4security_group1 src -j \ RETURN [0:0] -A %(bn)s-i_port1 -p icmp -j RETURN [0:0] -A %(bn)s-i_port1 -j %(bn)s-sg-fallback @@ -2010,7 +2010,7 @@ RETURN [0:0] -A %(bn)s-i_port2 -s 10.0.0.2/32 -p udp -m udp --sport 67 --dport 68 \ -j RETURN [0:0] -A %(bn)s-i_port2 -p tcp -m tcp --dport 22 -j RETURN -[0:0] -A %(bn)s-i_port2 -m set --match-set IPv4security_group1 src -j \ +[0:0] -A %(bn)s-i_port2 -m set --match-set NETIPv4security_group1 src -j \ RETURN [0:0] -A %(bn)s-i_port2 -p icmp -j RETURN [0:0] -A %(bn)s-i_port2 -j %(bn)s-sg-fallback