Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id C867D10107 for ; Tue, 10 Dec 2013 09:13:47 +0000 (UTC) Received: (qmail 84032 invoked by uid 500); 10 Dec 2013 09:13:46 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 84012 invoked by uid 500); 10 Dec 2013 09:13:46 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 84003 invoked by uid 99); 10 Dec 2013 09:13:45 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 10 Dec 2013 09:13:45 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 86EC39038DB; Tue, 10 Dec 2013 09:13:45 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jayapal@apache.org To: commits@cloudstack.apache.org Message-Id: <1da23aad02a34f5faf4ec84a3e9125a9@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: updated refs/heads/4.3 to 3caef2b Date: Tue, 10 Dec 2013 09:13:45 +0000 (UTC) Updated Branches: refs/heads/4.3 ff9786177 -> 3caef2b1d 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. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/3caef2b1 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/3caef2b1 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/3caef2b1 Branch: refs/heads/4.3 Commit: 3caef2b1d50bc44c89811bf61b97cbb2d824d1e6 Parents: ff97861 Author: Jayapal Authored: Tue Dec 10 13:02:14 2013 +0530 Committer: Jayapal Committed: Tue Dec 10 14:43:13 2013 +0530 ---------------------------------------------------------------------- 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 ++++++++++++++------ .../src/com/cloud/network/NetworkModelImpl.java | 13 +++++ .../network/element/VirtualRouterElement.java | 7 +++ .../network/firewall/FirewallManagerImpl.java | 10 ++-- .../cloud/network/MockFirewallManagerImpl.java | 2 +- .../com/cloud/network/MockNetworkModelImpl.java | 5 ++ .../com/cloud/vpc/MockNetworkModelImpl.java | 5 ++ 11 files changed, 102 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/api/src/com/cloud/network/NetworkModel.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/NetworkModel.java b/api/src/com/cloud/network/NetworkModel.java index f067787..8ae1cb7 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 http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/engine/components-api/src/com/cloud/network/rules/FirewallManager.java ---------------------------------------------------------------------- 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 57ea88f..3717bb8 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; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java ---------------------------------------------------------------------- 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 5577db8..c647f8e 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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/plugins/network-elements/juniper-srx/src/com/cloud/network/element/JuniperSRXExternalFirewallElement.java ---------------------------------------------------------------------- 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 8521037..0a863e8 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); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java ---------------------------------------------------------------------- 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 4658759..e7425a3 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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/server/src/com/cloud/network/NetworkModelImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkModelImpl.java b/server/src/com/cloud/network/NetworkModelImpl.java index 4a298cb..2533ce8 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; + } + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/server/src/com/cloud/network/element/VirtualRouterElement.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/element/VirtualRouterElement.java b/server/src/com/cloud/network/element/VirtualRouterElement.java index 28bfb6f..73966f3 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 { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/server/src/com/cloud/network/firewall/FirewallManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index 6ccf500..0a98062 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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/server/test/com/cloud/network/MockFirewallManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/network/MockFirewallManagerImpl.java b/server/test/com/cloud/network/MockFirewallManagerImpl.java index 1f3e1b1..f117486 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. } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/server/test/com/cloud/network/MockNetworkModelImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/network/MockNetworkModelImpl.java b/server/test/com/cloud/network/MockNetworkModelImpl.java index 8a9da83..f9dd9fe 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. + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3caef2b1/server/test/com/cloud/vpc/MockNetworkModelImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/vpc/MockNetworkModelImpl.java b/server/test/com/cloud/vpc/MockNetworkModelImpl.java index 992a81d..19ed9d5 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. + } }