diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index 8a64e14310e..b85517a5909 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -156,7 +156,6 @@ import com.cloud.agent.api.storage.CreatePrivateTemplateAnswer; import com.cloud.agent.api.storage.DestroyCommand; import com.cloud.agent.api.storage.PrimaryStorageDownloadAnswer; import com.cloud.agent.api.storage.PrimaryStorageDownloadCommand; -import com.cloud.agent.api.to.FirewallRuleTO; import com.cloud.agent.api.to.IpAddressTO; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.PortForwardingRuleTO; @@ -6479,7 +6478,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe callResult = callHostPlugin(conn, "vmops", "setFirewallRule", "args", args); if (callResult == null || callResult.isEmpty()) { - //FIXME - in the future we have to process each rule separately; now we temporarely set every rule to be false if single rule fails + //FIXME - in the future we have to process each rule separately; now we temporarily set every rule to be false if single rule fails for (int i=0; i < results.length; i++) { results[i] = "Failed"; } diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index f02a29c73ca..b6cea06e7f7 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -115,12 +115,17 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); } - validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol); + validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall); + //icmp code and icmp type can't be passed in for any other protocol rather than icmp if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != null || icmpType != null)) { throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only"); } + if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) { + throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP"); + } + Long networkId = ipAddress.getAssociatedWithNetworkId(); Long accountId = ipAddress.getAccountId(); Long domainId = ipAddress.getDomainId(); @@ -254,11 +259,8 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma 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()); } - } - } - - + } if (s_logger.isDebugEnabled()) { s_logger.debug("No network rule conflicts detected for " + newRule + " against " + (rules.size() - 1) + " existing rules"); @@ -267,7 +269,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma @Override - public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer portStart, Integer portEnd, String proto) { + public void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer portStart, Integer portEnd, String proto, Purpose purpose) { // Validate ip address _accountMgr.checkAccess(caller, ipAddress); @@ -297,6 +299,8 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma String supportedProtocols = firewallCapabilities.get(Capability.SupportedProtocols).toLowerCase(); if (!supportedProtocols.contains(proto.toLowerCase())) { throw new InvalidParameterValueException("Protocol " + proto + " is not supported in zone " + network.getDataCenterId()); + } else if (proto.equalsIgnoreCase(NetUtils.ICMP_PROTO) && purpose != Purpose.Firewall) { + throw new InvalidParameterValueException("Protocol " + proto + " is currently supported only for rules with purpose " + Purpose.Firewall); } } diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 41bce78d820..aeeaf3ef8a8 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -396,7 +396,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, throw new InvalidParameterValueException("Unable to create load balancer rule, invalid IP address id" + ipId); } - _firewallMgr.validateFirewallRule(caller.getCaller(), ipAddr, srcPortStart, srcPortEnd, lb.getProtocol()); + _firewallMgr.validateFirewallRule(caller.getCaller(), ipAddr, srcPortStart, srcPortEnd, lb.getProtocol(), Purpose.LoadBalancing); networkId = ipAddr.getAssociatedWithNetworkId(); diff --git a/server/src/com/cloud/network/rules/FirewallManager.java b/server/src/com/cloud/network/rules/FirewallManager.java index c7faf66b6ac..6367c482c29 100644 --- a/server/src/com/cloud/network/rules/FirewallManager.java +++ b/server/src/com/cloud/network/rules/FirewallManager.java @@ -7,6 +7,7 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IPAddressVO; import com.cloud.network.IpAddress; import com.cloud.network.firewall.FirewallService; +import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.user.Account; public interface FirewallManager extends FirewallService{ @@ -29,7 +30,7 @@ public interface FirewallManager extends FirewallService{ */ void detectRulesConflict(FirewallRule newRule, IpAddress ipAddress) throws NetworkRuleConflictException; - void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer portStart, Integer portEnd, String proto); + void validateFirewallRule(Account caller, IPAddressVO ipAddress, Integer portStart, Integer portEnd, String proto, Purpose purpose); boolean applyRules(List rules, boolean continueOnError) throws ResourceUnavailableException; diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 2a044ec656b..972d7816e51 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -165,7 +165,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); } - _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol()); + _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding); Long networkId = ipAddress.getAssociatedWithNetworkId(); Long accountId = ipAddress.getAccountId(); @@ -245,7 +245,7 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); } - _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol()); + _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat); Long networkId = ipAddress.getAssociatedWithNetworkId(); Long accountId = ipAddress.getAccountId();