etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

bug 11185: support multiple CIDR on overlapping port ranges for firewall rules 

status 11185: resolved fixed
This commit is contained in:
alena 2011-08-25 12:00:02 -07:00
parent 4374bb6774
commit 066ebc1368
14 changed files with 84 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java
View File

@ -126,7 +126,7 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta
_rulesService.revokeStaticNatRule(getEntityId(), true);
if (getOpenFirewall()) {
_rulesService.revokeRelatedFirewallRule(getEntityId(), true);
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
}
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Error in creating ip forwarding rule on the domr");

2
api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java
View File

@ -148,7 +148,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
_rulesService.revokePortForwardingRule(getEntityId(), true);
if (getOpenFirewall()) {
_rulesService.revokeRelatedFirewallRule(getEntityId(), true);
_firewallService.revokeRelatedFirewallRule(getEntityId(), true);
}
throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to apply port forwarding rule");

2
api/src/com/cloud/api/commands/DeleteIpForwardingRuleCmd.java
View File

@ -70,7 +70,7 @@ public class DeleteIpForwardingRuleCmd extends BaseAsyncCmd {
public void execute(){
UserContext.current().setEventDetails("Rule Id: "+id);
boolean result = _rulesService.revokeStaticNatRule(id, true);
result = result && _rulesService.revokeRelatedFirewallRule(id, true);
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());

2
api/src/com/cloud/api/commands/DeleteLoadBalancerRuleCmd.java
View File

@ -84,7 +84,7 @@ public class DeleteLoadBalancerRuleCmd extends BaseAsyncCmd {
public void execute(){
UserContext.current().setEventDetails("Load balancer Id: "+getId());
boolean result = _lbService.deleteLoadBalancerRule(id, true);
result = result && _rulesService.revokeRelatedFirewallRule(id, true);
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());

2
api/src/com/cloud/api/commands/DeletePortForwardingRuleCmd.java
View File

@ -91,7 +91,7 @@ public class DeletePortForwardingRuleCmd extends BaseAsyncCmd {
public void execute(){
UserContext.current().setEventDetails("Rule Id: "+id);
boolean result = _rulesService.revokePortForwardingRule(id, true);
result = result && _rulesService.revokeRelatedFirewallRule(id, true);
result = result && _firewallService.revokeRelatedFirewallRule(id, true);
if (result) {
SuccessResponse response = new SuccessResponse(getCommandName());

2
api/src/com/cloud/network/firewall/FirewallService.java
View File

@ -23,4 +23,6 @@ public interface FirewallService {
FirewallRule getFirewallRule(long ruleId);
boolean revokeRelatedFirewallRule(long ruleId, boolean apply);
}

2
api/src/com/cloud/network/rules/RulesService.java
View File

@ -70,7 +70,5 @@ public interface RulesService {
StaticNatRule buildStaticNatRule(FirewallRule rule);
List<String> getSourceCidrs(long ruleId);
boolean revokeRelatedFirewallRule(long ruleId, boolean apply);
}

2
server/src/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java
View File

@ -45,7 +45,7 @@ public class FirewallRulesCidrsDaoImpl extends GenericDaoBase<FirewallRulesCidrs
@Override @DB
public List<String> getSourceCidrs(long firewallRuleId) {
SearchCriteria sc = CidrsSearch.create();
SearchCriteria<FirewallRulesCidrsVO> sc = CidrsSearch.create();
sc.setParameters("firewallRuleId", firewallRuleId);
List<FirewallRulesCidrsVO> results = search(sc, null);

12
server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java
View File

@ -183,19 +183,18 @@ public class FirewallRulesDaoImpl extends GenericDaoBase<FirewallRuleVO, Long> i
txn.start();
FirewallRuleVO dbfirewallRule = super.persist(firewallRule);
saveSourceCidrs(firewallRule);
saveSourceCidrs(firewallRule, firewallRule.getSourceCidrList());
txn.commit();
return dbfirewallRule;
}
public void saveSourceCidrs(FirewallRuleVO firewallRule) {
List<String> cidrlist = firewallRule.getSourceCidrList();
if (cidrlist == null) {
public void saveSourceCidrs(FirewallRuleVO firewallRule, List<String> cidrList) {
if (cidrList == null) {
return;
}
_firewallRulesCidrsDao.persist(firewallRule.getId(), cidrlist);
_firewallRulesCidrsDao.persist(firewallRule.getId(), cidrList);
}
@ -228,5 +227,6 @@ public class FirewallRulesDaoImpl extends GenericDaoBase<FirewallRuleVO, Long> i
sc.setParameters("purpose", Purpose.Firewall);
return findOneBy(sc);
}
}
}

76
server/src/com/cloud/network/firewall/FirewallManagerImpl.java
View File

@ -19,6 +19,8 @@
package com.cloud.network.firewall;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@ -248,12 +250,30 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
if (rule.getId() == newRule.getId()) {
continue; // Skips my own rule.
}
boolean oneOfRulesIsFirewall = ((rule.getPurpose() == Purpose.Firewall || newRule.getPurpose() == Purpose.Firewall) && ((newRule.getPurpose() != rule.getPurpose()) || (!newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()))));
//if both rules are firewall and their cidrs are different, we can skip port ranges verification
boolean bothRulesFirewall = (rule.getPurpose() == newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
boolean duplicatedCidrs = false;
if (bothRulesFirewall) {
//Verify that the rules have different cidrs
List<String> ruleCidrList = rule.getSourceCidrList();
List<String> newRuleCidrList = newRule.getSourceCidrList();
if (ruleCidrList == null || newRuleCidrList == null) {
continue;
}
Collection<String> similar = new HashSet<String>(ruleCidrList);
similar.retainAll(newRuleCidrList);
if (similar.size() > 0) {
duplicatedCidrs = true;
}
}
boolean allowFirewall = ((rule.getPurpose() == Purpose.Firewall || newRule.getPurpose() == Purpose.Firewall) && ((newRule.getPurpose() != rule.getPurpose()) || (!newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()))));
if (!allowFirewall) {
if (!oneOfRulesIsFirewall) {
if (rule.getPurpose() == Purpose.StaticNat && newRule.getPurpose() != Purpose.StaticNat) {
throw new NetworkRuleConflictException("There is 1 to 1 Nat rule specified for the ip address id=" + newRule.getSourceIpAddressId());
} else if (rule.getPurpose() != Purpose.StaticNat && newRule.getPurpose() == Purpose.StaticNat) {
@ -264,28 +284,30 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
if (rule.getNetworkId() != newRule.getNetworkId() && rule.getState() != State.Revoke) {
throw new NetworkRuleConflictException("New rule is for a different network than what's specified in rule " + rule.getXid());
}
boolean notNullPorts = (newRule.getSourcePortStart() != null && newRule.getSourcePortEnd() != null && rule.getSourcePortStart() != null && rule.getSourcePortEnd() != null);
if (!allowFirewall && notNullPorts && ((rule.getSourcePortStart() <= newRule.getSourcePortStart() && rule.getSourcePortEnd() >= newRule.getSourcePortStart())
|| (rule.getSourcePortStart() <= newRule.getSourcePortEnd() && rule.getSourcePortEnd() >= newRule.getSourcePortEnd())
|| (newRule.getSourcePortStart() <= rule.getSourcePortStart() && newRule.getSourcePortEnd() >= rule.getSourcePortStart())
|| (newRule.getSourcePortStart() <= rule.getSourcePortEnd() && newRule.getSourcePortEnd() >= rule.getSourcePortEnd()))) {
// we allow port forwarding rules with the same parameters but different protocols
boolean allowPf = (rule.getPurpose() == Purpose.PortForwarding && newRule.getPurpose() == Purpose.PortForwarding && !newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()));
boolean allowStaticNat = (rule.getPurpose() == Purpose.StaticNat && newRule.getPurpose() == Purpose.StaticNat && !newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()));
if (!(allowPf || allowStaticNat || allowFirewall)) {
throw new NetworkRuleConflictException("The range specified, " + newRule.getSourcePortStart() + "-" + newRule.getSourcePortEnd() + ", conflicts with rule " + rule.getId()
+ " which has " + rule.getSourcePortStart() + "-" + rule.getSourcePortEnd());
}
}
if (newRule.getProtocol().equalsIgnoreCase(NetUtils.ICMP_PROTO) && newRule.getProtocol().equalsIgnoreCase(rule.getProtocol())) {
if (newRule.getIcmpCode().longValue() == rule.getIcmpCode().longValue() && newRule.getIcmpType().longValue() == rule.getIcmpType().longValue() && newRule.getProtocol().equalsIgnoreCase(rule.getProtocol())) {
throw new InvalidParameterValueException("New rule conflicts with existing rule id=" + rule.getId());
}
}
boolean notNullPorts = (newRule.getSourcePortStart() != null && newRule.getSourcePortEnd() != null && rule.getSourcePortStart() != null && rule.getSourcePortEnd() != null) ;
if (!notNullPorts) {
continue;
} else if (!oneOfRulesIsFirewall && !(bothRulesFirewall && !duplicatedCidrs) && ((rule.getSourcePortStart().intValue() <= newRule.getSourcePortStart().intValue() && rule.getSourcePortEnd().intValue() >= newRule.getSourcePortStart().intValue())
|| (rule.getSourcePortStart().intValue() <= newRule.getSourcePortEnd().intValue() && rule.getSourcePortEnd().intValue() >= newRule.getSourcePortEnd().intValue())
|| (newRule.getSourcePortStart().intValue() <= rule.getSourcePortStart().intValue() && newRule.getSourcePortEnd().intValue() >= rule.getSourcePortStart().intValue())
|| (newRule.getSourcePortStart().intValue() <= rule.getSourcePortEnd().intValue() && newRule.getSourcePortEnd().intValue() >= rule.getSourcePortEnd().intValue()))) {
// we allow port forwarding rules with the same parameters but different protocols
boolean allowPf = (rule.getPurpose() == Purpose.PortForwarding && newRule.getPurpose() == Purpose.PortForwarding && !newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()));
boolean allowStaticNat = (rule.getPurpose() == Purpose.StaticNat && newRule.getPurpose() == Purpose.StaticNat && !newRule.getProtocol().equalsIgnoreCase(rule.getProtocol()));
if (!(allowPf || allowStaticNat || oneOfRulesIsFirewall)) {
throw new NetworkRuleConflictException("The range specified, " + newRule.getSourcePortStart() + "-" + newRule.getSourcePortEnd() + ", conflicts with rule " + rule.getId()
+ " which has " + rule.getSourcePortStart() + "-" + rule.getSourcePortEnd());
}
}
}
if (s_logger.isDebugEnabled()) {
@ -538,4 +560,18 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma
return rules.size() == 0;
}
@Override
public boolean revokeRelatedFirewallRule(long ruleId, boolean apply) {
FirewallRule fwRule = _firewallDao.findByRelatedId(ruleId);
if (fwRule == null) {
s_logger.trace("No related firewall rule exists for rule id=" + ruleId + " so returning true here");
return true;
}
s_logger.debug("Revoking Firewall rule id=" + fwRule.getId() + " as a part of rule delete id=" + ruleId + " with apply=" + apply);
return revokeFirewallRule(fwRule.getId(), apply);
}
}

8
server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
View File

@ -454,17 +454,13 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager,
throw new CloudRuntimeException("Unable to add rule for ip address id=" + newRule.getSourceIpAddressId(), e);
} finally {
if (!success) {
txn.start();
_firewallDao.remove(_firewallDao.findByRelatedId(newRule.getId()).getId());
//no need to apply the rule as it wasn't programmed on the backend yet
_firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false);
_lbDao.remove(newRule.getId());
txn.commit();
_lbDao.remove(newRule.getId());
}
}
}
@Override

7
server/src/com/cloud/network/rules/FirewallRuleVO.java
View File

@ -35,6 +35,8 @@ import javax.persistence.InheritanceType;
import javax.persistence.Table;
import javax.persistence.Transient;
import com.cloud.network.dao.FirewallRulesCidrsDaoImpl;
import com.cloud.utils.component.ComponentLocator;
import com.cloud.utils.db.GenericDao;
import com.cloud.utils.net.NetUtils;
@ -43,6 +45,8 @@ import com.cloud.utils.net.NetUtils;
@Inheritance(strategy=InheritanceType.JOINED)
@DiscriminatorColumn(name="purpose", discriminatorType=DiscriminatorType.STRING, length=32)
public class FirewallRuleVO implements FirewallRule {
protected final FirewallRulesCidrsDaoImpl _firewallRulesCidrsDao = ComponentLocator.inject(FirewallRulesCidrsDaoImpl.class);
@Id
@GeneratedValue(strategy=GenerationType.IDENTITY)
@Column(name="id")
@ -106,6 +110,9 @@ public class FirewallRuleVO implements FirewallRule {
@Override
public List<String> getSourceCidrList() {
if (sourceCidrs == null && purpose == Purpose.Firewall) {
return _firewallRulesCidrsDao.getSourceCidrs(id);
}
return sourceCidrs;
}

22
server/src/com/cloud/network/rules/RulesManagerImpl.java
View File

@ -223,8 +223,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
} catch (Exception e) {
txn.start();
_firewallDao.remove(_firewallDao.findByRelatedId(newRule.getId()).getId());
//no need to apply the rule as it wasn't programmed on the backend yet
_firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false);
_forwardingDao.remove(newRule.getId());
txn.commit();
@ -289,7 +289,8 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
} catch (Exception e) {
txn.start();
_firewallDao.remove(_firewallDao.findByRelatedId(newRule.getId()).getId());
//no need to apply the rule as it wasn't programmed on the backend yet
_firewallMgr.revokeRelatedFirewallRule(newRule.getId(), false);
_forwardingDao.remove(newRule.getId());
txn.commit();
@ -1118,19 +1119,4 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager {
return true;
}
@Override
public boolean revokeRelatedFirewallRule(long ruleId, boolean apply) {
FirewallRule fwRule = _firewallDao.findByRelatedId(ruleId);
if (fwRule == null) {
s_logger.trace("No related firewall rule exists for rule id=" + ruleId + " so returning true here");
return true;
}
s_logger.debug("Revoking Firewall rule id=" + fwRule.getId() + " as a part of rule delete id=" + ruleId + " with apply=" + apply);
return _firewallMgr.revokeFirewallRule(fwRule.getId(), apply);
}
}

2
server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java
View File

@ -246,7 +246,7 @@ public class RemoteAccessVpnManagerImpl implements RemoteAccessVpnService, Manag
for (FirewallRule vpnFwRule : vpnFwRules) {
//don't apply on the backend yet; send all 3 rules in a banch
_rulesMgr.revokeRelatedFirewallRule(vpnFwRule.getId(), false);
_firewallMgr.revokeRelatedFirewallRule(vpnFwRule.getId(), false);
fwRules.add(_rulesDao.findByRelatedId(vpnFwRule.getId()));
}