From 3caef2b1d50bc44c89811bf61b97cbb2d824d1e6 Mon Sep 17 00:00:00 2001 From: Jayapal Date: Tue, 10 Dec 2013 13:02:14 +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 | 24 +++++++-- .../JuniperSRXExternalFirewallElement.java | 8 +++ .../network/resource/JuniperSrxResource.java | 53 +++++++++++++------ .../com/cloud/network/NetworkModelImpl.java | 13 +++++ .../network/element/VirtualRouterElement.java | 7 +++ .../network/firewall/FirewallManagerImpl.java | 10 ++-- .../network/MockFirewallManagerImpl.java | 2 +- .../cloud/network/MockNetworkModelImpl.java | 5 ++ .../com/cloud/vpc/MockNetworkModelImpl.java | 5 ++ 11 files changed, 102 insertions(+), 29 deletions(-) diff --git a/api/src/com/cloud/network/NetworkModel.java b/api/src/com/cloud/network/NetworkModel.java index f067787b9d6..8ae1cb70958 100644 --- a/api/src/com/cloud/network/NetworkModel.java +++ b/api/src/com/cloud/network/NetworkModel.java @@ -271,4 +271,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 57ea88f0784..3717bb87861 100644 --- a/engine/components-api/src/com/cloud/network/rules/FirewallManager.java +++ b/engine/components-api/src/com/cloud/network/rules/FirewallManager.java @@ -85,5 +85,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 5577db848b7..c647f8e67cd 100755 --- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -1093,21 +1093,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() && _networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && + 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(), true); + _firewallMgr.applyDefaultEgressFirewallRule(network.getId(), offering.getEgressDefaultPolicy(), 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; } - } // apply port forwarding rules if (!_rulesMgr.applyPortForwardingRulesForNetwork(networkId, false, caller)) { @@ -2668,6 +2667,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 85210379af7..0a863e8d2f2 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 @@ -222,6 +222,14 @@ PortForwardingServiceProvider, IpDeployer, JuniperSRXFirewallElementService, Sta 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 46587595e94..e7425a3c1b9 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 @@ -842,6 +842,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); @@ -849,23 +858,23 @@ public class JuniperSrxResource implements ServerResource { removeEgressSecurityPolicyAndApplications(SecurityPolicyType.SECURITYPOLICY_EGRESS, guestVlan, extractCidrs(activeRulesForGuestNw), defaultEgressPolicy); 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); + } } } @@ -2803,12 +2812,24 @@ 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); } else { xml = replaceXmlValue(xml, "from-zone", fromZone); diff --git a/server/src/com/cloud/network/NetworkModelImpl.java b/server/src/com/cloud/network/NetworkModelImpl.java index 4a298cbe258..2533ce89931 100755 --- a/server/src/com/cloud/network/NetworkModelImpl.java +++ b/server/src/com/cloud/network/NetworkModelImpl.java @@ -2187,4 +2187,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 28bfb6fee5a..73966f36bbd 100755 --- a/server/src/com/cloud/network/element/VirtualRouterElement.java +++ b/server/src/com/cloud/network/element/VirtualRouterElement.java @@ -238,6 +238,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 6ccf5003e5c..0a98062b847 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -617,7 +617,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); } @@ -651,12 +650,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); @@ -665,6 +660,9 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, sourceCidr.add(NetUtils.ALL_CIDRS); 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 1f3e1b1db2d..f117486a579 100644 --- a/server/test/com/cloud/network/MockFirewallManagerImpl.java +++ b/server/test/com/cloud/network/MockFirewallManagerImpl.java @@ -162,7 +162,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 8a9da832304..f9dd9feb75d 100644 --- a/server/test/com/cloud/network/MockNetworkModelImpl.java +++ b/server/test/com/cloud/network/MockNetworkModelImpl.java @@ -874,4 +874,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 992a81da126..19ed9d5167e 100644 --- a/server/test/com/cloud/vpc/MockNetworkModelImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkModelImpl.java @@ -885,4 +885,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. + } }