From 5c12250deaf1276d74c8ac959c43a507666b05fc Mon Sep 17 00:00:00 2001 From: Jayapal Date: Tue, 10 Dec 2013 14:18:56 +0530 Subject: [PATCH] CLOUDSTACK-5278 Fixed cleaning up egress default rules on VR and SRX 1. Egress default policy rules is send to the firewall provider. It is up to the provider to configure the rules. 2. The default policy rules are send for both allow and deny default policy. 3. On network shutdown rules for delete are send. 4. For VR and SRX, by default deny the traffic. So no default rule to deny traffic is required. --- api/src/com/cloud/network/NetworkModel.java | 2 + .../cloud/network/rules/FirewallManager.java | 2 +- .../orchestration/NetworkOrchestrator.java | 39 +++++++++----- .../JuniperSRXExternalFirewallElement.java | 8 +++ .../network/resource/JuniperSrxResource.java | 54 ++++++++++++------- .../com/cloud/network/NetworkModelImpl.java | 13 +++++ .../network/element/VirtualRouterElement.java | 7 +++ .../network/firewall/FirewallManagerImpl.java | 8 +-- .../network/MockFirewallManagerImpl.java | 2 +- .../cloud/network/MockNetworkModelImpl.java | 5 ++ .../com/cloud/vpc/MockNetworkModelImpl.java | 5 ++ 11 files changed, 106 insertions(+), 39 deletions(-) diff --git a/api/src/com/cloud/network/NetworkModel.java b/api/src/com/cloud/network/NetworkModel.java index 9bf5d27eea8..fcb6a2e58f4 100644 --- a/api/src/com/cloud/network/NetworkModel.java +++ b/api/src/com/cloud/network/NetworkModel.java @@ -269,4 +269,6 @@ public interface NetworkModel { boolean getExecuteInSeqNtwkElmtCmd(); boolean isNetworkReadyForGc(long networkId); + + boolean getNetworkEgressDefaultPolicy(Long networkId); } \ No newline at end of file diff --git a/engine/components-api/src/com/cloud/network/rules/FirewallManager.java b/engine/components-api/src/com/cloud/network/rules/FirewallManager.java index 6085b4b4341..3ba9861b1b2 100644 --- a/engine/components-api/src/com/cloud/network/rules/FirewallManager.java +++ b/engine/components-api/src/com/cloud/network/rules/FirewallManager.java @@ -86,5 +86,5 @@ public interface FirewallManager extends FirewallService { */ void removeRule(FirewallRule rule); - boolean applyDefaultEgressFirewallRule(Long networkId, boolean defaultPolicy) throws ResourceUnavailableException; + boolean applyDefaultEgressFirewallRule(Long networkId, boolean defaultPolicy, boolean add) throws ResourceUnavailableException; } diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index b5c1539ba9d..4ad0cd7af8f 100755 --- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -1111,22 +1111,20 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } List firewallEgressRulesToApply = _firewallDao.listByNetworkPurposeTrafficType(networkId, Purpose.Firewall, FirewallRule.TrafficType.Egress); - if (firewallEgressRulesToApply.size() == 0) { - NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); - //there are no egress rules then apply the default egress rule - DataCenter zone = _dcDao.findById(network.getDataCenterId()); - if (offering.getEgressDefaultPolicy() && + NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); + //there are no egress rules then apply the default egress rule + DataCenter zone = _dcDao.findById(network.getDataCenterId()); + if ( _networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && _networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && (network.getGuestType() == Network.GuestType.Isolated || (network.getGuestType() == Network.GuestType.Shared && zone.getNetworkType() == NetworkType.Advanced))) { - // add default egress rule to accept the traffic - _firewallMgr.applyDefaultEgressFirewallRule(network.getId(), true); - } - } else { - if (!_firewallMgr.applyFirewallRules(firewallEgressRulesToApply, false, caller)) { - s_logger.warn("Failed to reapply firewall Egress rule(s) as a part of network id=" + networkId + " restart"); - success = false; - } + // add default egress rule to accept the traffic + _firewallMgr.applyDefaultEgressFirewallRule(network.getId(), offering.getEgressDefaultPolicy(), true); } + if (!_firewallMgr.applyFirewallRules(firewallEgressRulesToApply, false, caller)) { + s_logger.warn("Failed to reapply firewall Egress rule(s) as a part of network id=" + networkId + " restart"); + success = false; + } + // apply port forwarding rules if (!_rulesMgr.applyPortForwardingRulesForNetwork(networkId, false, caller)) { @@ -2695,6 +2693,21 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra s_logger.debug("Releasing " + firewallEgressRules.size() + " firewall egress rules for network id=" + networkId + " as a part of shutdownNetworkRules"); } + try { + // delete default egress rule + DataCenter zone = _dcDao.findById(network.getDataCenterId()); + if ( _networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && + (network.getGuestType() == Network.GuestType.Isolated || (network.getGuestType() == Network.GuestType.Shared && zone.getNetworkType() == NetworkType.Advanced))) { + // add default egress rule to accept the traffic + _firewallMgr.applyDefaultEgressFirewallRule(network.getId(), _networkModel.getNetworkEgressDefaultPolicy(networkId), false); + } + + } catch (ResourceUnavailableException ex) { + s_logger.warn("Failed to cleanup firewall default egress rule as a part of shutdownNetworkRules due to ", ex); + success = false; + } + + for (FirewallRuleVO firewallRule : firewallEgressRules) { s_logger.trace("Marking firewall egress rule " + firewallRule + " with Revoke state"); firewallRule.setState(FirewallRule.State.Revoke); diff --git a/plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java b/plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java index 2ff90fb6d7b..82f701b4a06 100644 --- a/plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java +++ b/plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java @@ -218,6 +218,14 @@ public class JuniperSRXExternalFirewallElement extends ExternalFirewallDeviceMan return false; } + if (rules != null && rules.size() == 1 ) { + // for SRX no need to add default egress rule to DENY traffic + if (rules.get(0).getTrafficType() == FirewallRule.TrafficType.Egress && rules.get(0).getType() == FirewallRule.FirewallRuleType.System && + ! _networkManager.getNetworkEgressDefaultPolicy(config.getId())) + return true; + } + + return applyFirewallRules(config, rules); } diff --git a/plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java b/plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java index 338e0953249..b828ab66db6 100644 --- a/plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java +++ b/plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java @@ -835,6 +835,15 @@ public class JuniperSrxResource implements ServerResource { FirewallRule.FirewallRuleType type = rules[0].getType(); //getting String guestCidr = rules[0].getGuestCidr(); + List cidrs = new ArrayList(); + cidrs.add(guestCidr); + + List applications = new ArrayList(); + Object[] application = new Object[3]; + application[0] = Protocol.all; + application[1] = NetUtils.PORT_RANGE_MIN; + application[2] = NetUtils.PORT_RANGE_MAX; + applications.add(application); for (String guestVlan : guestVlans) { List activeRulesForGuestNw = activeRules.get(guestVlan); @@ -844,25 +853,24 @@ public class JuniperSrxResource implements ServerResource { if (activeRulesForGuestNw.size() > 0 && type == FirewallRule.FirewallRuleType.User) { addEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS, guestVlan, extractApplications(activeRulesForGuestNw), extractCidrs(activeRulesForGuestNw), defaultEgressPolicy); + + /* Adding default policy rules are required because the order of rules is important. + * Depending on the rules order the traffic accept/drop is performed + */ + removeEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT, guestVlan, cidrs, defaultEgressPolicy); + addEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT, guestVlan, applications, cidrs, defaultEgressPolicy); } - List applications = new ArrayList(); - Object[] application = new Object[3]; - application[0] = Protocol.all; - application[1] = NetUtils.PORT_RANGE_MIN; - application[2] = NetUtils.PORT_RANGE_MAX; - applications.add(application); - - List cidrs = new ArrayList(); - cidrs.add(guestCidr); //remove required with out comparing default policy because in upgrade network offering we may required to delete // the previously added rule removeEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT, guestVlan, cidrs, false); - if (defaultEgressPolicy == true) { - //add default egress security policy - addEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT, guestVlan, applications, cidrs, false); + if (defaultEgressPolicy == true && type == FirewallRule.FirewallRuleType.System) { + removeEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT, guestVlan, cidrs, defaultEgressPolicy); + if (activeRulesForGuestNw.size() > 0) { + //add default egress security policy + addEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT, guestVlan, applications, cidrs, defaultEgressPolicy); + } } - } commitConfiguration(); } else { @@ -2811,13 +2819,23 @@ public class JuniperSrxResource implements ServerResource { xml = replaceXmlValue(xml, "src-address", srcAddrs); dstAddrs = "any"; xml = replaceXmlValue(xml, "dst-address", dstAddrs); - if (defaultEgressAction == true) { - //configure block rules and default allow the traffic - action = ""; + if (type.equals(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT)) { + if (defaultEgressAction == false) { + //for default policy is false add default deny rules + action = ""; + } else { + action = ""; + } } else { - action = ""; + if (defaultEgressAction == true) { + //configure egress rules to deny the traffic when default egress is allow + action = ""; + } else { + action = ""; + } + + xml = replaceXmlValue(xml, "action", action); } - xml = replaceXmlValue(xml, "action", action); } else { xml = replaceXmlValue(xml, "from-zone", fromZone); xml = replaceXmlValue(xml, "to-zone", toZone); diff --git a/server/src/com/cloud/network/NetworkModelImpl.java b/server/src/com/cloud/network/NetworkModelImpl.java index 7568da07702..71fbbf5c149 100755 --- a/server/src/com/cloud/network/NetworkModelImpl.java +++ b/server/src/com/cloud/network/NetworkModelImpl.java @@ -2183,4 +2183,17 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel { return true; } + + @Override + public boolean getNetworkEgressDefaultPolicy (Long networkId) { + NetworkVO network = _networksDao.findById(networkId); + + if (network != null) { + NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); + return offering.getEgressDefaultPolicy(); + } else { + InvalidParameterValueException ex = new InvalidParameterValueException("network with network id does not exist"); + throw ex; + } + } } diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java index 145c6ebb7ea..7833d241d6d 100755 --- a/server/src/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VirtualRouterElement.java @@ -246,6 +246,13 @@ public class VirtualRouterElement extends AdapterBase implements VirtualRouterEl return true; } + if (rules != null && rules.size() == 1 ) { + // for VR no need to add default egress rule to DENY traffic + if (rules.get(0).getTrafficType() == FirewallRule.TrafficType.Egress && rules.get(0).getType() == FirewallRule.FirewallRuleType.System && + ! _networkMgr.getNetworkEgressDefaultPolicy(config.getId())) + return true; + } + if (!_routerMgr.applyFirewallRules(config, rules, routers)) { throw new CloudRuntimeException("Failed to apply firewall rules in network " + config.getId()); } else { diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index 848ef1f70e3..448abe317b6 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -612,7 +612,6 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, @Override public boolean applyEgressFirewallRules(FirewallRule rule, Account caller) throws ResourceUnavailableException { List rules = _firewallDao.listByNetworkPurposeTrafficType(rule.getNetworkId(), Purpose.Firewall, FirewallRule.TrafficType.Egress); - applyDefaultEgressFirewallRule(rule.getNetworkId(), true); return applyFirewallRules(rules, false, caller); } @@ -646,12 +645,8 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, } @Override - public boolean applyDefaultEgressFirewallRule(Long networkId, boolean defaultPolicy) throws ResourceUnavailableException { + public boolean applyDefaultEgressFirewallRule(Long networkId, boolean defaultPolicy, boolean add) throws ResourceUnavailableException { - if (defaultPolicy == false) { - //If default policy is false no need apply rules on backend because firewall provider blocks by default - return true; - } s_logger.debug("applying default firewall egress rules "); NetworkVO network = _networkDao.findById(networkId); @@ -661,6 +656,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, FirewallRuleVO ruleVO = new FirewallRuleVO(null, null, null, null, "all", networkId, network.getAccountId(), network.getDomainId(), Purpose.Firewall, sourceCidr, null, null, null, FirewallRule.TrafficType.Egress, FirewallRuleType.System); + ruleVO.setState(add ? State.Add : State.Revoke); List rules = new ArrayList(); rules.add(ruleVO); diff --git a/server/test/com/cloud/network/MockFirewallManagerImpl.java b/server/test/com/cloud/network/MockFirewallManagerImpl.java index 5d74c851144..d3d11f1ab4f 100644 --- a/server/test/com/cloud/network/MockFirewallManagerImpl.java +++ b/server/test/com/cloud/network/MockFirewallManagerImpl.java @@ -149,7 +149,7 @@ public class MockFirewallManagerImpl extends ManagerBase implements FirewallMana } @Override - public boolean applyDefaultEgressFirewallRule(Long networkId, boolean defaultPolicy) throws ResourceUnavailableException { + public boolean applyDefaultEgressFirewallRule(Long networkId, boolean defaultPolicy, boolean add) throws ResourceUnavailableException { return false; //To change body of implemented methods use File | Settings | File Templates. } diff --git a/server/test/com/cloud/network/MockNetworkModelImpl.java b/server/test/com/cloud/network/MockNetworkModelImpl.java index 78f3d51370b..6ae6ae8beef 100644 --- a/server/test/com/cloud/network/MockNetworkModelImpl.java +++ b/server/test/com/cloud/network/MockNetworkModelImpl.java @@ -864,4 +864,9 @@ public class MockNetworkModelImpl extends ManagerBase implements NetworkModel { public boolean isNetworkReadyForGc(long networkId) { return true; } + + @Override + public boolean getNetworkEgressDefaultPolicy(Long networkId) { + return false; //To change body of implemented methods use File | Settings | File Templates. + } } diff --git a/server/test/com/cloud/vpc/MockNetworkModelImpl.java b/server/test/com/cloud/vpc/MockNetworkModelImpl.java index 05093c7a14a..e0583d8d317 100644 --- a/server/test/com/cloud/vpc/MockNetworkModelImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkModelImpl.java @@ -878,4 +878,9 @@ public class MockNetworkModelImpl extends ManagerBase implements NetworkModel { public boolean isNetworkReadyForGc(long networkId) { return true; } + + @Override + public boolean getNetworkEgressDefaultPolicy(Long networkId) { + return false; //To change body of implemented methods use File | Settings | File Templates. + } }