diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index bf48be66694c..a1dd8666a630 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -71,6 +71,15 @@ def removeUndesiredCidrs(cidrs, version): return None +def nftAddrSet(cidrs): + """ nft rejects a bare comma-separated address list; several addresses must be a set """ + cidrList = [c for c in cidrs.split(",") if c] + if len(cidrList) > 1: + # spaced so that no shell brace-expands it on the way to nft + return "{ %s }" % ", ".join(cidrList) + return ",".join(cidrList) + + def appendStringIfNotEmpty(s1, s2): if s2: if not isinstance(s2, str): @@ -440,9 +449,9 @@ def __process_routing_ip4(self, direction, rule_list): continue addr = "" if cidr: - addr = "ip daddr " + cidr + addr = "ip daddr " + nftAddrSet(cidr) if direction == "ingress": - addr = "ip saddr " + cidr + addr = "ip saddr " + nftAddrSet(cidr) proto = "" protocol = rule['type'] @@ -518,9 +527,9 @@ def __process_ip6(self, direction, rule_list): continue addr = "" if cidr: - addr = "ip6 daddr " + cidr + addr = "ip6 daddr " + nftAddrSet(cidr) if direction == "ingress": - addr = "ip6 saddr " + cidr + addr = "ip6 saddr " + nftAddrSet(cidr) proto = "" protocol = rule['type'] @@ -583,16 +592,21 @@ def process(self, direction, rule_list, base, is_routed): count = base for i in rule_list: - ruleData = copy.copy(i) - cidr = ruleData['cidr'] + cidr = i['cidr'] if cidr is not None and cidr != "": cidr = removeUndesiredCidrs(cidr, 6) if cidr is None or cidr == "": continue - ruleData['cidr'] = cidr - r = self.AclRule(direction, self, ruleData, self.config, count) - r.create() - count += 1 + # iptables expands "-s a,b" into one rule per address, but CsNetfilters + # counts each entry as a single rule when placing the next ACL rule ahead + # of the DROP. Emit one entry per CIDR so the two stay in step. + cidrs = [c for c in cidr.split(",") if c] if cidr else [cidr] + for c in cidrs: + ruleData = copy.copy(i) + ruleData['cidr'] = c + r = self.AclRule(direction, self, ruleData, self.config, count) + r.create() + count += 1 # Prepare IPv6 ACL rules self.__process_ip6(direction, rule_list) diff --git a/systemvm/test/TestCsAcl.py b/systemvm/test/TestCsAcl.py new file mode 100644 index 000000000000..81cdd890251f --- /dev/null +++ b/systemvm/test/TestCsAcl.py @@ -0,0 +1,127 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import unittest +from configure import CsAcl + + +class FakeConfig: + + def __init__(self, routed=False): + self.routed = routed + self.fw = [] + self.ipv6_acl = [] + self.nft_ipv4_acl = [] + + def get_fw(self): + return self.fw + + def get_ipv6_acl(self): + return self.ipv6_acl + + def get_nft_ipv4_acl(self): + return self.nft_ipv4_acl + + def is_vpc(self): + return True + + def is_routed(self): + return self.routed + + def get_ingress_chain(self, device, ip): + return "ACL_INBOUND_%s" % device + + def get_egress_chain(self, device, ip): + return "ACL_OUTBOUND_%s" % device + + def get_egress_table(self): + return "mangle" + + +class TestCsAcl(unittest.TestCase): + + def acl_device(self, config, ingress, egress=None): + obj = {"device": "eth3", "nic_ip": "10.1.1.1", "nic_netmask": "24", "nic_ip6_cidr": "fd00:1::/64", + "ingress_rules": ingress, "egress_rules": egress or []} + return CsAcl.AclDevice(obj, config) + + def test_multiple_cidrs_emit_one_iptables_rule_each(self): + config = FakeConfig() + acl = self.acl_device(config, [ + {"type": "tcp", "cidr": "1.2.3.4/32,2.3.4.5/32", "first_port": 22, "last_port": 22, "allowed": True}, + {"type": "all", "cidr": "0.0.0.0/0", "allowed": False}]) + acl.process("ingress", acl.ingress, acl.FIXED_RULES_INGRESS, False) + + # iptables expands "-s a,b" into several rules, which CsNetfilters would count as one + # and so insert the following rules between them, i.e. behind a later deny + rules = [fw[2] for fw in config.fw] + self.assertEqual(rules, [ + "-A ACL_INBOUND_eth3 -p tcp -s 1.2.3.4/32 -m tcp --dport 22 -j ACCEPT", + "-A ACL_INBOUND_eth3 -p tcp -s 2.3.4.5/32 -m tcp --dport 22 -j ACCEPT", + "-A ACL_INBOUND_eth3 -p all -s 0.0.0.0/0 -j DROP"]) + self.assertEqual([fw[1] for fw in config.fw], [3, 4, 5]) + + def test_multiple_egress_cidrs_emit_one_iptables_rule_each(self): + config = FakeConfig() + acl = self.acl_device(config, [], [ + {"type": "udp", "cidr": "8.8.8.8/32,8.8.4.4/32", "first_port": 53, "last_port": 53, "allowed": True}, + {"type": "all", "cidr": "0.0.0.0/0", "allowed": False}]) + acl.process("egress", acl.egress, acl.FIXED_RULES_EGRESS, False) + + self.assertEqual(config.fw, [ + ["mangle", 3, "-A ACL_OUTBOUND_eth3 -p udp -d 8.8.8.8/32 -m udp --dport 53 -j ACCEPT"], + ["mangle", 4, "-A ACL_OUTBOUND_eth3 -p udp -d 8.8.4.4/32 -m udp --dport 53 -j ACCEPT"], + ["mangle", 5, "-A ACL_OUTBOUND_eth3 -p all -d 0.0.0.0/0 -j DROP"]]) + + def test_empty_cidr_elements_are_skipped(self): + config = FakeConfig() + acl = self.acl_device(config, [ + {"type": "tcp", "cidr": "1.2.3.4/32,,2.3.4.5/32,", "first_port": 22, "last_port": 22, "allowed": True}]) + acl.process("ingress", acl.ingress, acl.FIXED_RULES_INGRESS, False) + + self.assertEqual([fw[2] for fw in config.fw], [ + "-A ACL_INBOUND_eth3 -p tcp -s 1.2.3.4/32 -m tcp --dport 22 -j ACCEPT", + "-A ACL_INBOUND_eth3 -p tcp -s 2.3.4.5/32 -m tcp --dport 22 -j ACCEPT"]) + rules = [r['rule'] for r in config.ipv6_acl if r.get('chain') == "eth3_ingress_policy" and 'rule' in r] + self.assertNotIn("{", " ".join(rules)) + + def test_multiple_ipv6_cidrs_use_nft_set(self): + config = FakeConfig() + acl = self.acl_device(config, [ + {"type": "tcp", "cidr": "2001:db8:1::/64,2001:db8:2::/64,1.2.3.4/32", "first_port": 22, "last_port": 22, + "allowed": True}]) + acl.process("ingress", acl.ingress, acl.FIXED_RULES_INGRESS, False) + + # the IPv4 CIDR goes to iptables on its own, the IPv6 ones to nft as a set + self.assertEqual([fw[2] for fw in config.fw], ["-A ACL_INBOUND_eth3 -p tcp -s 1.2.3.4/32 -m tcp --dport 22 -j ACCEPT"]) + rules = [r['rule'] for r in config.ipv6_acl if r.get('chain') == "eth3_ingress_policy" and 'rule' in r] + self.assertIn("ip6 saddr { 2001:db8:1::/64, 2001:db8:2::/64 } tcp dport 22 accept", rules) + + def test_multiple_cidrs_use_nft_set_when_routed(self): + config = FakeConfig(routed=True) + acl = self.acl_device(config, [ + {"type": "tcp", "cidr": "1.2.3.4/32,2.3.4.5/32", "first_port": 22, "last_port": 22, "allowed": True}, + {"type": "tcp", "cidr": "3.4.5.6/32", "first_port": 443, "last_port": 443, "allowed": True}]) + acl.process("ingress", acl.ingress, acl.FIXED_RULES_INGRESS, True) + + rules = [r['rule'] for r in config.nft_ipv4_acl if r.get('chain') == "eth3_ingress_policy" and 'rule' in r] + self.assertIn("ip saddr { 1.2.3.4/32, 2.3.4.5/32 } tcp dport 22 accept", rules) + self.assertIn("ip saddr 3.4.5.6/32 tcp dport 443 accept", rules) + + +if __name__ == '__main__': + unittest.main()