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/5c12250d
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/5c12250d
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/5c12250d
Branch: refs/heads/ui-restyle
Commit: 5c12250deaf1276d74c8ac959c43a507666b05fc
Parents: ccfe557
Author: Jayapal <jaya...@apache.org>
Authored: Tue Dec 10 14:18:56 2013 +0530
Committer: Jayapal <jaya...@apache.org>
Committed: Tue Dec 10 14:19:03 2013 +0530
----------------------------------------------------------------------
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 +++++++++++++-------
.../src/com/cloud/network/NetworkModelImpl.java | 13 +++++
.../network/element/VirtualRouterElement.java | 7 +++
.../network/firewall/FirewallManagerImpl.java | 8 +--
.../cloud/network/MockFirewallManagerImpl.java | 2 +-
.../com/cloud/network/MockNetworkModelImpl.java | 5 ++
.../com/cloud/vpc/MockNetworkModelImpl.java | 5 ++
11 files changed, 106 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 9bf5d27..fcb6a2e 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
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 6085b4b..3ba9861 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;
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 b5c1539..4ad0cd7 100755
---
a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
+++
b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@ -1111,23 +1111,21 @@ 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() &&
+ 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)) {
s_logger.warn("Failed to reapply port forwarding rule(s) as a part
of network id=" + networkId + " restart");
@@ -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);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 2ff90fb..82f701b 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);
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 338e095..b828ab6 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<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);
@@ -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);
- }
- 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);
+ /* 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<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);
+ }
}
-
}
commitConfiguration();
} else {
@@ -2811,13 +2819,23 @@ 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);
}
- xml = replaceXmlValue(xml, "action", action);
} else {
xml = replaceXmlValue(xml, "from-zone", fromZone);
xml = replaceXmlValue(xml, "to-zone", toZone);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 7568da0..71fbbf5 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;
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 145c6eb..7833d24 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 {
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 848ef1f..448abe3 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<FirewallRuleVO> 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<FirewallRuleVO> rules = new ArrayList<FirewallRuleVO>();
rules.add(ruleVO);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 5d74c85..d3d11f1 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.
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 78f3d51..6ae6ae8 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.
+ }
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/5c12250d/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 05093c7..e0583d8 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.
+ }
}
