You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ja...@apache.org on 2013/12/10 10:13:45 UTC

git commit: updated refs/heads/4.3 to 3caef2b

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 <ja...@apache.org>
Authored: Tue Dec 10 13:02:14 2013 +0530
Committer: Jayapal <ja...@apache.org>
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<FirewallRuleVO> 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<String> cidrs = new ArrayList<String>();
+                cidrs.add(guestCidr);
+
+                List<Object[]> applications = new ArrayList<Object[]>();
+                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<FirewallRuleTO> 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<Object[]> applications = new ArrayList<Object[]>();
-                    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<String> cidrs = new ArrayList<String>();
-                    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 = "<destination-address>any</destination-address>";
                 xml = replaceXmlValue(xml, "dst-address", dstAddrs);
-                if (defaultEgressAction == true) {
-                    //configure block rules and default allow the traffic
-                    action = "<deny></deny>";
+
+
+                if (type.equals(SecurityPolicyType.SECURITYPOLICY_EGRESS_DEFAULT)) {
+                    if (defaultEgressAction == false) {
+                        //for default policy is false add default deny rules
+                        action = "<deny></deny>";
+                    } else {
+                        action = "<permit></permit>";
+                    }
                 } else {
-                    action = "<permit></permit>";
+                    if (defaultEgressAction == true) {
+                        //configure egress rules to deny the traffic when default egress is allow
+                        action = "<deny></deny>";
+                    } else {
+                        action = "<permit></permit>";
+                    }
                 }
+
                 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<FirewallRuleVO> 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<FirewallRuleVO> rules = new ArrayList<FirewallRuleVO>();
         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.
+    }
 }