This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new 8074c3e45e7 VPC egress ACL: end ACL_OUTBOUND with RETURN so its rules
stay in order (#14355)
8074c3e45e7 is described below
commit 8074c3e45e7005b5881dd02bb92634d80bb75a8b
Author: Brad House - Nexthop <[email protected]>
AuthorDate: Thu Oct 22 08:43:39 2026 -0400
VPC egress ACL: end ACL_OUTBOUND with RETURN so its rules stay in order
(#14355)
Signed-off-by: Brad House <[email protected]>
---
systemvm/debian/opt/cloud/bin/cs/CsAddress.py | 10 ++++
systemvm/test/TestCsAddress.py | 77 ++++++++++++++++++++++++++-
2 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py
b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py
index e15eb7a2e3d..c95aec0be3e 100755
--- a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py
+++ b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py
@@ -586,6 +586,10 @@ class CsIP:
["mangle", "front", "-A ACL_OUTBOUND_%s -d 225.0.0.50/32
-j ACCEPT" % self.dev])
self.fw.append(
["mangle", "front", "-A ACL_OUTBOUND_%s -d 224.0.0.18/32
-j ACCEPT" % self.dev])
+ # ACL rules are inserted ahead of the chain's last rule, so
like ACL_INBOUND's DROP
+ # this has to stay last; RETURN is what falling off the end of
the chain does anyway
+ self.fw.append(
+ ["mangle", "", "-A ACL_OUTBOUND_%s -j RETURN" % self.dev])
else:
self.fw.append(["filter", "", "-A FORWARD -d %s -o %s -j
ACCEPT" % (guestNetworkCidr, self.dev)])
@@ -639,6 +643,11 @@ class CsIP:
self.fw.append(["filter", "front",
"-A FORWARD -d %s -o %s -m state --state
RELATED,ESTABLISHED -j ACCEPT" %
(static_route['network'], self.dev)])
+ if self.get_type() in ["guest"] and not
self.config.has_public_network():
+ # Without a public network a tier's ACL chains get no
last rule of their own, and are
+ # only jumped to from here; close them too, so the ACL
rules stay in order
+ self.fw.append(["filter", "", "-A ACL_INBOUND_%s -j
RETURN" % self.dev])
+ self.fw.append(["mangle", "", "-A ACL_OUTBOUND_%s -j
RETURN" % self.dev])
if self.is_private_gateway():
self.fw.append(["filter", "front", "-A FORWARD -d %s -o %s -j
ACL_INBOUND_%s" %
@@ -646,6 +655,7 @@ class CsIP:
self.fw.append(["filter", "front", "-A FORWARD -d %s -o %s -m
state --state RELATED,ESTABLISHED -j ACCEPT" %
(self.address['network'], self.dev)])
self.fw.append(["filter", "", "-A ACL_INBOUND_%s -j DROP" %
self.dev])
+ self.fw.append(["mangle", "", "-A ACL_OUTBOUND_%s -j RETURN" %
self.dev])
self.fw.append(["mangle", "",
"-A PREROUTING -m state --state NEW -i %s -s %s !
-d %s/32 -j ACL_OUTBOUND_%s" %
(self.dev, self.address['network'],
self.address['gateway'], self.dev)])
diff --git a/systemvm/test/TestCsAddress.py b/systemvm/test/TestCsAddress.py
index 0ad9ae861b9..695cc6d7ba4 100644
--- a/systemvm/test/TestCsAddress.py
+++ b/systemvm/test/TestCsAddress.py
@@ -16,10 +16,42 @@
# under the License.
import unittest
-from cs.CsAddress import CsAddress
+from unittest import mock
+from cs.CsAddress import CsAddress, CsIP
import merge
+class FakeConfig:
+
+ def __init__(self):
+ self.fw = []
+ self.nft_ipv4_fw = []
+ self.nft_ipv4_acl = []
+
+ def get_fw(self):
+ return self.fw
+
+ def get_nft_ipv4_fw(self):
+ return self.nft_ipv4_fw
+
+ def get_nft_ipv4_acl(self):
+ return self.nft_ipv4_acl
+
+ def cmdline(self):
+ cl = mock.Mock()
+ cl.get_vpccidr.return_value = "10.0.0.0/16"
+ return cl
+
+ def is_vpc(self):
+ return True
+
+ def is_routed(self):
+ return False
+
+ def has_public_network(self):
+ return True
+
+
class TestCsAddress(unittest.TestCase):
def setUp(self):
@@ -38,6 +70,49 @@ class TestCsAddress(unittest.TestCase):
def test_get_guest_netmask(self):
self.assertTrue(self.csaddress.get_guest_netmask() == "255.255.255.0")
+ def acl_rules(self, address, chain="ACL_OUTBOUND_eth3",
public_network=True, static_routes=None):
+ config = FakeConfig()
+ config.has_public_network = lambda: public_network
+ with mock.patch.object(CsIP, "list"):
+ ip = CsIP("eth3", config)
+ ip.setAddress(address)
+ routes = mock.Mock()
+ routes.get_bag.return_value = static_routes or {}
+ with mock.patch("cs.CsAddress.CsStaticRoutes", return_value=routes):
+ ip.fw_vpcrouter()
+ return [fw for fw in config.fw if "-A %s " % chain in fw[2]]
+
+ def acl_outbound_rules(self, address):
+ return self.acl_rules(address)
+
+ def test_acl_outbound_ends_with_return_on_guest_tier(self):
+ # ACL rules are inserted ahead of the last rule of the chain, so that
rule must be a
+ # terminal one, or it would end up behind the ACL rules
+ rules = self.acl_outbound_rules({"nw_type": "guest", "network":
"10.0.1.0/24", "gateway": "10.0.1.1"})
+ self.assertEqual(rules[-1], ["mangle", "", "-A ACL_OUTBOUND_eth3 -j
RETURN"])
+ self.assertTrue(all(fw[1] == "front" for fw in rules[:-1]))
+
+ def
test_acl_chains_end_with_return_on_static_route_tier_without_public_network(self):
+ # such a tier's ACL chains are only jumped to for the static route,
and get no last rule otherwise
+ address = {"nw_type": "guest", "network": "10.0.1.0/24", "gateway":
"10.0.1.1", "public_ip": "10.0.1.1"}
+ routes = {"id": "staticroutes", "192.168.50.0/24": {"network":
"192.168.50.0/24", "ip_address": "10.0.1.1", "revoke": False}}
+ for chain, table in (("ACL_INBOUND_eth3", "filter"),
("ACL_OUTBOUND_eth3", "mangle")):
+ rules = self.acl_rules(address, chain, public_network=False,
static_routes=routes)
+ self.assertEqual(rules, [[table, "", "-A %s -j RETURN" % chain]])
+
+ def
test_acl_chains_get_one_last_rule_on_static_route_tier_with_public_network(self):
+ address = {"nw_type": "guest", "network": "10.0.1.0/24", "gateway":
"10.0.1.1", "public_ip": "10.0.1.1"}
+ routes = {"id": "staticroutes", "192.168.50.0/24": {"network":
"192.168.50.0/24", "ip_address": "10.0.1.1", "revoke": False}}
+ inbound = self.acl_rules(address, "ACL_INBOUND_eth3",
static_routes=routes)
+ self.assertEqual([fw[2] for fw in inbound if fw[1] == ""], ["-A
ACL_INBOUND_eth3 -j DROP"])
+ outbound = self.acl_rules(address, "ACL_OUTBOUND_eth3",
static_routes=routes)
+ self.assertEqual([fw[2] for fw in outbound if fw[1] == ""], ["-A
ACL_OUTBOUND_eth3 -j RETURN"])
+
+ def test_acl_outbound_ends_with_return_on_private_gateway(self):
+ rules = self.acl_outbound_rules({"nw_type": "public",
"is_private_gateway": True, "network": "172.16.0.0/24",
+ "gateway": "172.16.0.1", "public_ip":
"172.16.0.10", "source_nat": False})
+ self.assertEqual(rules, [["mangle", "", "-A ACL_OUTBOUND_eth3 -j
RETURN"]])
+
if __name__ == '__main__':
unittest.main()
