Skip to content

Commit f948e96

Browse files
remibergsmaremibergsma
committed
Merge pull request apache#1023 from ekholabs/fix/egress_state-CLOUDSTACK-8925
CLOUDSTACK-8925 - Default allow for Egress rules is not being configured properly in VR iptables rulesThis PR fixes the router default policy for egress. When the default is DENY, the router still allows outgoing connections. The test component/test_routers_network_ops.py was improved to cover that case as well. The results were: Test redundant router internals ... === TestName: test_01_isolate_network_FW_PF_default_routes_egress_true | Status : SUCCESS === ok Test redundant router internals ... === TestName: test_02_isolate_network_FW_PF_default_routes_egress_false | Status : SUCCESS === ok Test redundant router internals ... === TestName: test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | Status : SUCCESS === ok Test redundant router internals ... === TestName: test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | Status : SUCCESS === ok ---------------------------------------------------------------------- Ran 4 tests in 3636.656s OK /tmp//MarvinLogs/test_routers_network_ops_QDL429/results.txt (END) * pr/1023: CLOUDSTACK-8925 - Implement the default egress DENY/ALLOW properly CLOUDSTACK-8925 - Improve the default egress tests in order to cover newly entered rules CLOUDSTACK-8925 - Add egress dataset to test_data.py CLOUDSTACK-8925 - Drop the traffic when default egress is set to false Signed-off-by: Remi Bergsma <github@remi.nl>
2 parents ea70a18 + 79dabfd commit f948e96

3 files changed

Lines changed: 532 additions & 92 deletions

File tree

‎systemvm/patches/debian/config/opt/cloud/bin/configure.py‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,15 @@ def __init__(self, obj, fw):
9595
if 'src_port_range' in obj:
9696
self.rule['first_port'] = obj['src_port_range'][0]
9797
self.rule['last_port'] = obj['src_port_range'][1]
98-
self.rule['allowed'] = True
9998

99+
self.rule['allowed'] = True
100+
self.rule['action'] = "ACCEPT"
101+
100102
if self.rule['type'] == 'all' and not obj['source_cidr_list']:
101103
self.rule['cidr'] = ['0.0.0.0/0']
102104
else:
103105
self.rule['cidr'] = obj['source_cidr_list']
104106

105-
self.rule['action'] = "ACCEPT"
106107
logging.debug("AclIP created for rule ==> %s", self.rule)
107108

108109
def create(self):
@@ -151,7 +152,25 @@ def add_rule(self, cidr):
151152
" -m %s " % rule['protocol'] +
152153
" --icmp-type %s -j %s" % (icmp_type, self.rule['action'])])
153154
else:
154-
fwr = " -A FW_EGRESS_RULES"
155+
fwr = " -I FW_EGRESS_RULES"
156+
#In case we have a default rule (accept all or drop all), we have to evaluate the action again.
157+
if rule['type'] == 'all' and not rule['source_cidr_list']:
158+
fwr = " -A FW_EGRESS_RULES"
159+
# For default egress ALLOW or DENY, the logic is inverted.
160+
# Having default_egress_policy == True, means that the default rule should have ACCEPT,
161+
# otherwise DROP. The rule should be appended, not inserted.
162+
if self.rule['default_egress_policy']:
163+
self.rule['action'] = "ACCEPT"
164+
else:
165+
self.rule['action'] = "DROP"
166+
else:
167+
# For other rules added, if default_egress_policy == True, following rules should be DROP,
168+
# otherwise ACCEPT
169+
if self.rule['default_egress_policy']:
170+
self.rule['action'] = "DROP"
171+
else:
172+
self.rule['action'] = "ACCEPT"
173+
155174
if rule['protocol'] != "all":
156175
fwr += " -s %s " % cidr + \
157176
" -p %s " % rule['protocol'] + \
@@ -226,7 +245,7 @@ def init_vpc(self, direction, acl, rule, config):
226245
self.protocol = rule['protocol']
227246
self.action = "DROP"
228247
self.dport = ""
229-
if 'allowed' in rule.keys() and rule['allowed'] and rule['allowed']:
248+
if 'allowed' in rule.keys() and rule['allowed']:
230249
self.action = "ACCEPT"
231250
if 'first_port' in rule.keys():
232251
self.dport = "-m %s --dport %s" % (self.protocol, rule['first_port'])

0 commit comments

Comments
 (0)