Merge pull request #1666 from murali-reddy/egress_rules

CLOUDSTACK-9480,  CLOUDSTACK-9495 fix egress rule incorrect behaviorWhen 'default egress policy' is set to 'allow' in the network offering, any egress rule that is added will 'deny' the traffic overriding the default behaviour.

Conversely, when 'default egress policy' is set to 'deny' in the network offering, any egress rule that is added will 'allow' the traffic overriding the default behaviour.

While this works for 'tcp', 'udp' as expected, for 'icmp' protocol its always set to ALLOW. This patch keeps all protocols behaviour consistent.

Results of running test/integration/component/test_egress_fw_rules.py.  With out the patch test_02_egress_fr2 test was failing. This patch fixes the test_02_egress_fr2  scenario.
-----------------------------------------------------------------------------------------------------
Test By-default the communication from guest n/w to public n/w is NOT allowed. ... === TestName: test_01_1_egress_fr1 | Status : SUCCESS ===
ok
Test By-default the communication from guest n/w to public n/w is allowed. ... === TestName: test_01_egress_fr1 | Status : SUCCESS ===
ok
Test Allow Communication using Egress rule with CIDR + Port Range + Protocol. ... === TestName: test_02_1_egress_fr2 | Status : SUCCESS ===
ok
Test Allow Communication using Egress rule with CIDR + Port Range + Protocol. ... === TestName: test_02_egress_fr2 | Status : SUCCESS ===
ok
Test Communication blocked with network that is other than specified ... === TestName: test_03_1_egress_fr3 | Status : SUCCESS ===
ok
Test Communication blocked with network that is other than specified ... === TestName: test_03_egress_fr3 | Status : SUCCESS ===
ok
Test Create Egress rule and check the Firewall_Rules DB table ... === TestName: test_04_1_egress_fr4 | Status : SUCCESS ===
ok
Test Create Egress rule and check the Firewall_Rules DB table ... === TestName: test_04_egress_fr4 | Status : SUCCESS ===
ok
Test Create Egress rule and check the IP tables ... SKIP: Skip
Test Create Egress rule and check the IP tables ... SKIP: Skip
Test Create Egress rule without CIDR ... === TestName: test_06_1_egress_fr6 | Status : SUCCESS ===
ok
Test Create Egress rule without CIDR ... === TestName: test_06_egress_fr6 | Status : SUCCESS ===
ok
Test Create Egress rule without End Port ... === TestName: test_07_1_egress_fr7 | Status : EXCEPTION ===
ERROR
Test Create Egress rule without End Port ... === TestName: test_07_egress_fr7 | Status : SUCCESS ===
ok
Test Port Forwarding and Egress Conflict ... SKIP: Skip
Test Port Forwarding and Egress Conflict ... SKIP: Skip
Test Delete Egress rule ... === TestName: test_09_1_egress_fr9 | Status : SUCCESS ===
ok
Test Delete Egress rule ... === TestName: test_09_egress_fr9 | Status : SUCCESS ===
ok
Test Invalid CIDR and Invalid Port ranges ... === TestName: test_10_1_egress_fr10 | Status : SUCCESS ===
ok
Test Invalid CIDR and Invalid Port ranges ... === TestName: test_10_egress_fr10 | Status : SUCCESS ===
ok
Test Regression on Firewall + PF + LB + SNAT ... === TestName: test_11_1_egress_fr11 | Status : SUCCESS ===
ok
Test Regression on Firewall + PF + LB + SNAT ... === TestName: test_11_egress_fr11 | Status : SUCCESS ===
ok
Test Reboot Router ... === TestName: test_12_1_egress_fr12 | Status : SUCCESS ===
ok
Test Reboot Router ... === TestName: test_12_egress_fr12 | Status : EXCEPTION ===
ERROR
Test Redundant Router : Master failover ... === TestName: test_13_1_egress_fr13 | Status : SUCCESS ===
ok
Test Redundant Router : Master failover ... === TestName: test_13_egress_fr13 | Status : SUCCESS ===
ok
-----------------------------------------------------------------------------------------------------

* pr/1666:
  fix egress rule incorrect behavior

Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>
This commit is contained in:
Rajani Karuturi 2016-09-21 14:54:31 +05:30
commit cc043e9f8f
2 changed files with 90 additions and 35 deletions

View File

@ -46,9 +46,9 @@ from cs.CsStaticRoutes import CsStaticRoutes
class CsPassword(CsDataBag):
TOKEN_FILE="/tmp/passwdsrvrtoken"
def process(self):
for item in self.dbag:
if item == "id":
@ -99,7 +99,7 @@ class CsAcl(CsDataBag):
self.rule['allowed'] = True
self.rule['action'] = "ACCEPT"
if self.rule['type'] == 'all' and not obj['source_cidr_list']:
self.rule['cidr'] = ['0.0.0.0/0']
else:
@ -145,41 +145,40 @@ class CsAcl(CsDataBag):
logging.debug("Current ACL IP direction is ==> %s", self.direction)
if self.direction == 'egress':
self.fw.append(["filter", "", " -A FW_OUTBOUND -j FW_EGRESS_RULES"])
if rule['protocol'] == "icmp":
self.fw.append(["filter", "front",
" -A FW_EGRESS_RULES" +
" -s %s " % cidr +
" -p %s " % rule['protocol'] +
" -m %s " % rule['protocol'] +
" --icmp-type %s -j %s" % (icmp_type, self.rule['action'])])
else:
fwr = " -I FW_EGRESS_RULES"
#In case we have a default rule (accept all or drop all), we have to evaluate the action again.
if rule['type'] == 'all' and not rule['source_cidr_list']:
fwr = " -A FW_EGRESS_RULES"
# For default egress ALLOW or DENY, the logic is inverted.
# Having default_egress_policy == True, means that the default rule should have ACCEPT,
# otherwise DROP. The rule should be appended, not inserted.
if self.rule['default_egress_policy']:
self.rule['action'] = "ACCEPT"
else:
self.rule['action'] = "DROP"
fwr = " -I FW_EGRESS_RULES"
# In case we have a default rule (accept all or drop all), we have to evaluate the action again.
if rule['type'] == 'all' and not rule['source_cidr_list']:
fwr = " -A FW_EGRESS_RULES"
# For default egress ALLOW or DENY, the logic is inverted.
# Having default_egress_policy == True, means that the default rule should have ACCEPT,
# otherwise DROP. The rule should be appended, not inserted.
if self.rule['default_egress_policy']:
self.rule['action'] = "ACCEPT"
else:
# For other rules added, if default_egress_policy == True, following rules should be DROP,
# otherwise ACCEPT
if self.rule['default_egress_policy']:
self.rule['action'] = "DROP"
else:
self.rule['action'] = "ACCEPT"
self.rule['action'] = "DROP"
else:
# For other rules added, if default_egress_policy == True, following rules should be DROP,
# otherwise ACCEPT
if self.rule['default_egress_policy']:
self.rule['action'] = "DROP"
else:
self.rule['action'] = "ACCEPT"
if rule['protocol'] != "all":
fwr += " -s %s " % cidr + \
" -p %s " % rule['protocol'] + \
" -m %s " % rule['protocol'] + \
" --dport %s" % rnge
self.fw.append(["filter", "", "%s -j %s" % (fwr, rule['action'])])
if rule['protocol'] == "icmp":
fwr += " -s %s " % cidr + \
" -p %s " % rule['protocol'] + \
" -m %s " % rule['protocol'] + \
" --icmp-type %s" % icmp_type
elif rule['protocol'] != "all":
fwr += " -s %s " % cidr + \
" -p %s " % rule['protocol'] + \
" -m %s " % rule['protocol'] + \
" --dport %s" % rnge
elif rule['protocol'] == "all":
fwr += " -s %s " % cidr
self.fw.append(["filter", "", "%s -j %s" % (fwr, rule['action'])])
logging.debug("EGRESS rule configured for protocol ==> %s, action ==> %s", rule['protocol'], rule['action'])
class AclDevice():

View File

@ -348,6 +348,26 @@ class TestEgressFWRules(cloudstackTestCase):
except Exception as e:
self.fail("Warning! Cleanup failed: %s" % e)
def create_another_vm(self):
self.debug("Deploying instance in the account: %s and network: %s" % (self.account.name, self.network.id))
project = None
self.virtual_machine1 = VirtualMachine.create(self.apiclient,
self.services["virtual_machine"],
accountid=self.account.name,
domainid=self.domain.id,
serviceofferingid=self.service_offering.id,
mode=self.zone.networktype,
networkids=[str(self.network.id)],
projectid=project.id if project else None)
self.debug("Deployed instance %s in account: %s" % (self.virtual_machine.id,self.account.name))
# Checking if VM is running or not, in case it is deployed in error state, test case fails
self.vm_list = list_virtual_machines(self.apiclient, id=self.virtual_machine.id)
self.assertEqual(validateList(self.vm_list)[0], PASS, "vm list validation failed, vm list is %s" % self.vm_list)
self.assertEqual(str(self.vm_list[0].state).lower(),'running',"VM state should be running, it is %s" % self.vm_list[0].state)
@attr(tags=["advanced"], required_hardware="true")
def test_01_egress_fr1(self):
"""Test By-default the communication from guest n/w to public n/w is allowed.
@ -385,6 +405,25 @@ class TestEgressFWRules(cloudstackTestCase):
"['100']",
negative_test=False)
@attr(tags=["advanced"], required_hardware="true")
def test_01_2_egress_fr1(self):
"""Test egress rule with /32 CIDR of a VM, and check other VM in the
network does not have public access
"""
# Validate the following:
# 1. deploy VM using network offering with egress policy false.
# 2. deploy another VM into the network created in step #1
# 3. create egress rule with /32 CIDR of the second VM
# 4. login to first VM.
# 5. ping public network.
# 6. public network should not be reachable from the first VM.
self.create_vm(egress_policy=False)
self.create_another_vm()
self.createEgressRule(protocol='all', cidr=self.virtual_machine1.ipaddress+"/32")
self.exec_script_on_user_vm('ping -c 1 www.google.com',
"| grep -oP \'\d+(?=% packet loss)\'",
"['100']",
negative_test=False)
@attr(tags=["advanced"], required_hardware="true")
def test_02_egress_fr2(self):
@ -420,6 +459,23 @@ class TestEgressFWRules(cloudstackTestCase):
"['0']",
negative_test=False)
@attr(tags=["advanced"], required_hardware="true")
def test_02_2_egress_fr2(self):
"""Test Allow Communication using Egress rule with /32 CIDR + Port Range + Protocol.
"""
# Validate the following:
# 1. deploy VM using network offering with egress policy false.
# 3. create egress rule with specific /32 CIDR + port range.
# 4. login to VM.
# 5. ping public network.
# 6. public network should be reachable from the VM.
self.create_vm(egress_policy=False)
self.createEgressRule(cidr=self.virtual_machine.ipaddress+"/32")
self.exec_script_on_user_vm('ping -c 1 www.google.com',
"| grep -oP \'\d+(?=% packet loss)\'",
"['0']",
negative_test=False)
@attr(tags=["advanced"], required_hardware="true")
def test_03_egress_fr3(self):
"""Test Communication blocked with network that is other than specified