diff --git a/api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java b/api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java index 346f4e4c2cb..0594a5c5108 100644 --- a/api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreateFirewallRuleCmd.java @@ -279,5 +279,10 @@ public class CreateFirewallRuleCmd extends BaseAsyncCreateCmd implements Firewal } return null; } + + @Override + public Long getRelated() { + return null; + } } diff --git a/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java b/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java index e2798eafea9..963e55d4d3c 100644 --- a/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreateIpForwardingRuleCmd.java @@ -124,6 +124,11 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta } finally { if (!result || rule == null) { _rulesService.revokeStaticNatRule(getEntityId(), true); + + if (getOpenFirewall()) { + _rulesService.revokeRelatedFirewallRule(getEntityId(), true); + } + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Error in creating ip forwarding rule on the domr"); } } @@ -276,4 +281,9 @@ public class CreateIpForwardingRuleCmd extends BaseAsyncCreateCmd implements Sta return null; } + @Override + public Long getRelated() { + return null; + } + } diff --git a/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java b/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java index f0d390ad2f4..66094639cce 100644 --- a/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java @@ -35,11 +35,11 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.network.IpAddress; +import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.PortForwardingRule; import com.cloud.user.Account; import com.cloud.user.UserContext; import com.cloud.utils.net.Ip; -import com.cloud.utils.net.NetUtils; @Implementation(description = "Creates a port forwarding rule", responseObject = FirewallRuleResponse.class) public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements PortForwardingRule { @@ -146,6 +146,11 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P } finally { if (!success || rule == null) { _rulesService.revokePortForwardingRule(getEntityId(), true); + + if (getOpenFirewall()) { + _rulesService.revokeRelatedFirewallRule(getEntityId(), true); + } + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to apply port forwarding rule"); } } @@ -286,5 +291,10 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P public Integer getIcmpType() { return null; } + + @Override + public Long getRelated() { + return null; + } } diff --git a/api/src/com/cloud/api/commands/CreateRemoteAccessVpnCmd.java b/api/src/com/cloud/api/commands/CreateRemoteAccessVpnCmd.java index 4f62dee3349..07a1a644151 100644 --- a/api/src/com/cloud/api/commands/CreateRemoteAccessVpnCmd.java +++ b/api/src/com/cloud/api/commands/CreateRemoteAccessVpnCmd.java @@ -27,7 +27,6 @@ import com.cloud.api.BaseCmd; import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; -import com.cloud.api.BaseCmd.CommandType; import com.cloud.api.response.RemoteAccessVpnResponse; import com.cloud.event.EventTypes; import com.cloud.exception.InvalidParameterValueException; diff --git a/api/src/com/cloud/api/commands/DeleteIpForwardingRuleCmd.java b/api/src/com/cloud/api/commands/DeleteIpForwardingRuleCmd.java index 9427dc283f4..26ff77ac693 100644 --- a/api/src/com/cloud/api/commands/DeleteIpForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/DeleteIpForwardingRuleCmd.java @@ -70,6 +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); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); diff --git a/api/src/com/cloud/api/commands/DeleteLoadBalancerRuleCmd.java b/api/src/com/cloud/api/commands/DeleteLoadBalancerRuleCmd.java index 0e870a0d2d2..373a53d0266 100644 --- a/api/src/com/cloud/api/commands/DeleteLoadBalancerRuleCmd.java +++ b/api/src/com/cloud/api/commands/DeleteLoadBalancerRuleCmd.java @@ -84,6 +84,8 @@ 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); + if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); this.setResponseObject(response); diff --git a/api/src/com/cloud/api/commands/DeletePortForwardingRuleCmd.java b/api/src/com/cloud/api/commands/DeletePortForwardingRuleCmd.java index 45a67f6b5ee..240569b43cb 100644 --- a/api/src/com/cloud/api/commands/DeletePortForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/DeletePortForwardingRuleCmd.java @@ -88,9 +88,10 @@ public class DeletePortForwardingRuleCmd extends BaseAsyncCmd { } @Override - public void execute() throws ResourceUnavailableException { + public void execute(){ UserContext.current().setEventDetails("Rule Id: "+id); boolean result = _rulesService.revokePortForwardingRule(id, true); + result = result && _rulesService.revokeRelatedFirewallRule(id, true); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); diff --git a/api/src/com/cloud/network/firewall/FirewallService.java b/api/src/com/cloud/network/firewall/FirewallService.java index 9fd665144d7..468cc4cf3bd 100644 --- a/api/src/com/cloud/network/firewall/FirewallService.java +++ b/api/src/com/cloud/network/firewall/FirewallService.java @@ -22,4 +22,5 @@ public interface FirewallService { boolean applyFirewallRules(long ipId, Account caller) throws ResourceUnavailableException; FirewallRule getFirewallRule(long ruleId); + } diff --git a/api/src/com/cloud/network/lb/LoadBalancingRule.java b/api/src/com/cloud/network/lb/LoadBalancingRule.java index 2103badfb2d..2eaa4382caf 100644 --- a/api/src/com/cloud/network/lb/LoadBalancingRule.java +++ b/api/src/com/cloud/network/lb/LoadBalancingRule.java @@ -169,4 +169,9 @@ public class LoadBalancingRule implements FirewallRule, LoadBalancer{ public List getSourceCidrList() { return null; } + + @Override + public Long getRelated() { + return null; + } } diff --git a/api/src/com/cloud/network/rules/FirewallRule.java b/api/src/com/cloud/network/rules/FirewallRule.java index aa08fd1cfa9..1ce1b8332af 100644 --- a/api/src/com/cloud/network/rules/FirewallRule.java +++ b/api/src/com/cloud/network/rules/FirewallRule.java @@ -75,5 +75,7 @@ public interface FirewallRule extends ControlledEntity { Integer getIcmpType(); List getSourceCidrList(); + + Long getRelated(); } diff --git a/api/src/com/cloud/network/rules/RulesService.java b/api/src/com/cloud/network/rules/RulesService.java index 61e912ae5e0..57a093e6a84 100644 --- a/api/src/com/cloud/network/rules/RulesService.java +++ b/api/src/com/cloud/network/rules/RulesService.java @@ -70,5 +70,7 @@ public interface RulesService { StaticNatRule buildStaticNatRule(FirewallRule rule); List getSourceCidrs(long ruleId); + + boolean revokeRelatedFirewallRule(long ruleId, boolean apply); } diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java index d51ca001c1e..e911d443ca6 100755 --- a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java @@ -47,8 +47,8 @@ import com.cloud.agent.api.proxy.CheckConsoleProxyLoadCommand; import com.cloud.agent.api.proxy.ConsoleProxyLoadAnswer; import com.cloud.agent.api.proxy.WatchConsoleProxyLoadCommand; import com.cloud.agent.api.routing.DhcpEntryCommand; -import com.cloud.agent.api.routing.IpAssocCommand; import com.cloud.agent.api.routing.IpAssocAnswer; +import com.cloud.agent.api.routing.IpAssocCommand; import com.cloud.agent.api.routing.LoadBalancerConfigCommand; import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.agent.api.routing.SavePasswordCommand; @@ -203,7 +203,11 @@ public class VirtualRoutingResource implements Manager { //1:1 NAT needs instanceip;publicip;domrip;op command.add(" -l ", rule.getSrcIp()); command.add(" -r ", rule.getDstIp()); - command.add(" -P ", rule.getProtocol().toLowerCase()); + + if (rule.getProtocol() != null) { + command.add(" -P ", rule.getProtocol().toLowerCase()); + } + command.add(" -d ", rule.getStringSrcPortRange()); command.add(" -G ") ; diff --git a/server/src/com/cloud/network/LoadBalancerVO.java b/server/src/com/cloud/network/LoadBalancerVO.java index 1f211699f71..c8c517a2c0d 100644 --- a/server/src/com/cloud/network/LoadBalancerVO.java +++ b/server/src/com/cloud/network/LoadBalancerVO.java @@ -55,7 +55,7 @@ public class LoadBalancerVO extends FirewallRuleVO implements LoadBalancer { } public LoadBalancerVO(String xId, String name, String description, long srcIpId, int srcPort, int dstPort, String algorithm, long networkId, long accountId, long domainId) { - super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, null, null, null); + super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, null, null, null, null); this.name = name; this.description = description; this.algorithm = algorithm; diff --git a/server/src/com/cloud/network/dao/FirewallRulesDao.java b/server/src/com/cloud/network/dao/FirewallRulesDao.java index a80fe8be2b4..cb930de262b 100644 --- a/server/src/com/cloud/network/dao/FirewallRulesDao.java +++ b/server/src/com/cloud/network/dao/FirewallRulesDao.java @@ -46,5 +46,6 @@ public interface FirewallRulesDao extends GenericDao { List listStaticNatByVmId(long vmId); List listByIpPurposeAndProtocolAndNotRevoked(long ipAddressId, Integer startPort, Integer endPort, String protocol, FirewallRule.Purpose purpose); - + + FirewallRuleVO findByRelatedId(long ruleId); } diff --git a/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java b/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java index ac56fa18487..ef21bd4ad8e 100644 --- a/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java +++ b/server/src/com/cloud/network/dao/FirewallRulesDaoImpl.java @@ -61,6 +61,7 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i AllFieldsSearch.and("domain", AllFieldsSearch.entity().getDomainId(), Op.EQ); AllFieldsSearch.and("id", AllFieldsSearch.entity().getId(), Op.EQ); AllFieldsSearch.and("networkId", AllFieldsSearch.entity().getNetworkId(), Op.EQ); + AllFieldsSearch.and("related", AllFieldsSearch.entity().getRelated(), Op.EQ); AllFieldsSearch.done(); NotRevokedSearch = createSearchBuilder(); @@ -218,6 +219,14 @@ public class FirewallRulesDaoImpl extends GenericDaoBase i return listBy(sc); + } + + @Override + public FirewallRuleVO findByRelatedId(long ruleId) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("related", ruleId); + sc.setParameters("purpose", Purpose.Firewall); + + return findOneBy(sc); } - } diff --git a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java index d55e8d5b6aa..eff3e2210c9 100644 --- a/server/src/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/com/cloud/network/firewall/FirewallManagerImpl.java @@ -107,13 +107,13 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma public FirewallRule createFirewallRule(FirewallRule rule) throws NetworkRuleConflictException { Account caller = UserContext.current().getCaller(); - return createFirewallRule(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart() ,rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), rule.getIcmpCode(), rule.getIcmpType()); + return createFirewallRule(rule.getSourceIpAddressId(), caller, rule.getXid(), rule.getSourcePortStart() ,rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), rule.getIcmpCode(), rule.getIcmpType(), null); } @DB @Override @ActionEvent(eventType = EventTypes.EVENT_FIREWALL_OPEN, eventDescription = "creating firewll rule", create = true) - public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart,Integer portEnd, String protocol, List sourceCidrList, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException{ + public FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart,Integer portEnd, String protocol, List sourceCidrList, Integer icmpCode, Integer icmpType, Long relatedRuleId) throws NetworkRuleConflictException{ IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); // Validate ip address @@ -139,7 +139,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma Transaction txn = Transaction.currentTxn(); txn.start(); - FirewallRuleVO newRule = new FirewallRuleVO (xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountId, domainId, Purpose.Firewall, sourceCidrList, icmpCode, icmpType); + FirewallRuleVO newRule = new FirewallRuleVO (xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountId, domainId, Purpose.Firewall, sourceCidrList, icmpCode, icmpType, relatedRuleId); newRule = _firewallDao.persist(newRule); detectRulesConflict(newRule, ipAddress); @@ -478,7 +478,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma } @Override - public FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer startPort, Integer endPort, String protocol, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException{ + public FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer startPort, Integer endPort, String protocol, Integer icmpCode, Integer icmpType, Long relatedRuleId) throws NetworkRuleConflictException{ //If firwallRule for this port range already exists, return it List rules = _firewallDao.listByIpPurposeAndProtocolAndNotRevoked(ipAddrId, startPort, endPort, protocol, Purpose.Firewall); @@ -488,7 +488,7 @@ public class FirewallManagerImpl implements FirewallService, FirewallManager, Ma List oneCidr = new ArrayList(); oneCidr.add(NetUtils.ALL_CIDRS); - return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, icmpCode, icmpType); + return createFirewallRule(ipAddrId, caller, null, startPort, endPort, protocol, oneCidr, icmpCode, icmpType, relatedRuleId); } @Override diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index aeeaf3ef8a8..0d84e78f5ca 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -416,14 +416,16 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, Transaction txn = Transaction.currentTxn(); txn.start(); - if (openFirewall) { - _firewallMgr.createRuleForAllCidrs(ipId, caller.getCaller(), lb.getSourcePortStart(), lb.getSourcePortEnd(), lb.getProtocol(), null, null); - } LoadBalancerVO newRule = new LoadBalancerVO(lb.getXid(), lb.getName(), lb.getDescription(), lb.getSourceIpAddressId(), lb.getSourcePortEnd(), lb.getDefaultPortStart(), lb.getAlgorithm(), network.getId(), ipAddr.getAccountId(), ipAddr.getDomainId()); newRule = _lbDao.persist(newRule); + + if (openFirewall) { + _firewallMgr.createRuleForAllCidrs(ipId, caller.getCaller(), lb.getSourcePortStart(), lb.getSourcePortEnd(), lb.getProtocol(), null, null, newRule.getId()); + } + boolean success = true; try { diff --git a/server/src/com/cloud/network/rules/FirewallManager.java b/server/src/com/cloud/network/rules/FirewallManager.java index 6367c482c29..a3827eac978 100644 --- a/server/src/com/cloud/network/rules/FirewallManager.java +++ b/server/src/com/cloud/network/rules/FirewallManager.java @@ -49,10 +49,10 @@ public interface FirewallManager extends FirewallService{ */ boolean revokeFirewallRule(long ruleId, boolean apply, Account caller, long userId); - FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart, Integer portEnd, String protocol, List sourceCidrList, Integer icmpCode, Integer icmpType) + FirewallRule createFirewallRule(long ipAddrId, Account caller, String xId, Integer portStart, Integer portEnd, String protocol, List sourceCidrList, Integer icmpCode, Integer icmpType, Long relatedRuleId) throws NetworkRuleConflictException; - FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer startPort, Integer endPort, String protocol, Integer icmpCode, Integer icmpType) throws NetworkRuleConflictException; + FirewallRule createRuleForAllCidrs(long ipAddrId, Account caller, Integer startPort, Integer endPort, String protocol, Integer icmpCode, Integer icmpType, Long relatedRuleId) throws NetworkRuleConflictException; boolean revokeAllFirewallRulesForNetwork(long networkId, long userId, Account caller) throws ResourceUnavailableException; diff --git a/server/src/com/cloud/network/rules/FirewallRuleVO.java b/server/src/com/cloud/network/rules/FirewallRuleVO.java index cf73e52d75a..9a2f5ae923a 100644 --- a/server/src/com/cloud/network/rules/FirewallRuleVO.java +++ b/server/src/com/cloud/network/rules/FirewallRuleVO.java @@ -90,6 +90,9 @@ public class FirewallRuleVO implements FirewallRule { @Column(name="icmp_type") Integer icmpType; + @Column(name="related") + Long related; + // This is a delayed load value. If the value is null, // then this field has not been loaded yet. // Call firewallrules dao to load it. @@ -172,7 +175,7 @@ public class FirewallRuleVO implements FirewallRule { protected FirewallRuleVO() { } - public FirewallRuleVO(String xId, long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType) { + public FirewallRuleVO(String xId, long ipAddressId, Integer portStart, Integer portEnd, String protocol, long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType, Long related) { this.xId = xId; if (xId == null) { this.xId = UUID.randomUUID().toString(); @@ -189,10 +192,16 @@ public class FirewallRuleVO implements FirewallRule { this.icmpCode = icmpCode; this.icmpType = icmpType; this.sourceCidrs = sourceCidrs; + + if (related != null) { + assert (purpose == Purpose.Firewall) : "related field can be set for rule of purpose " + Purpose.Firewall + " only"; + } + + this.related = related; } - public FirewallRuleVO(String xId, long ipAddressId, int port, String protocol, long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType) { - this(xId, ipAddressId, port, port, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType); + public FirewallRuleVO(String xId, long ipAddressId, int port, String protocol, long networkId, long accountId, long domainId, Purpose purpose, List sourceCidrs, Integer icmpCode, Integer icmpType, Long related) { + this(xId, ipAddressId, port, port, protocol, networkId, accountId, domainId, purpose, sourceCidrs, icmpCode, icmpType, related); } @Override @@ -209,5 +218,10 @@ public class FirewallRuleVO implements FirewallRule { public Integer getIcmpType() { return icmpType; } + + @Override + public Long getRelated() { + return related; + } } diff --git a/server/src/com/cloud/network/rules/PortForwardingRuleVO.java b/server/src/com/cloud/network/rules/PortForwardingRuleVO.java index 69d8230a307..d6a26dac192 100644 --- a/server/src/com/cloud/network/rules/PortForwardingRuleVO.java +++ b/server/src/com/cloud/network/rules/PortForwardingRuleVO.java @@ -53,7 +53,7 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi } public PortForwardingRuleVO(String xId, long srcIpId, int srcPortStart, int srcPortEnd, Ip dstIp, int dstPortStart, int dstPortEnd, String protocol, long networkId, long accountId, long domainId, long instanceId) { - super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding, null, null, null); + super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId, accountId, domainId, Purpose.PortForwarding, null, null, null, null); this.destinationIpAddress = dstIp; this.virtualMachineId = instanceId; this.destinationPortStart = dstPortStart; @@ -82,6 +82,11 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi @Override public long getVirtualMachineId() { return virtualMachineId; + } + + @Override + public Long getRelated() { + return null; } } diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index f8ec8f32cf2..a1eaa9f632b 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -201,14 +201,14 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { Transaction txn = Transaction.currentTxn(); txn.start(); - //create firewallRule for 0.0.0.0/0 cidr - if (openFirewall) { - _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null); - } - PortForwardingRuleVO newRule = new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIp, rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); newRule = _forwardingDao.persist(newRule); + + //create firewallRule for 0.0.0.0/0 cidr + if (openFirewall) { + _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null, newRule.getId()); + } try { _firewallMgr.detectRulesConflict(newRule, ipAddress); @@ -256,14 +256,15 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { Transaction txn = Transaction.currentTxn(); txn.start(); + FirewallRuleVO newRule = new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), + networkId, accountId, domainId, rule.getPurpose(), null, null, null, null); + newRule = _firewallDao.persist(newRule); + //create firewallRule for 0.0.0.0/0 cidr if (openFirewall) { - _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null); + _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null, newRule.getId()); } - FirewallRuleVO newRule = new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), - networkId, accountId, domainId, rule.getPurpose(), null, null, null); - newRule = _firewallDao.persist(newRule); try { _firewallMgr.detectRulesConflict(newRule, ipAddress); @@ -948,12 +949,12 @@ public class RulesManagerImpl implements RulesManager, RulesService, Manager { txn.start(); for (int i = 0; i < ports.length; i++) { - if (openFirewall) { - _firewallMgr.createRuleForAllCidrs(ip.getId(), caller, ports[i], ports[i], protocol, null, null); - } - - rules[i] = new FirewallRuleVO(null, ip.getId(), ports[i], protocol, ip.getAssociatedWithNetworkId(), ip.getAllocatedToAccountId(), ip.getAllocatedInDomainId(), purpose, null, null, null); + rules[i] = new FirewallRuleVO(null, ip.getId(), ports[i], protocol, ip.getAssociatedWithNetworkId(), ip.getAllocatedToAccountId(), ip.getAllocatedInDomainId(), purpose, null, null, null, null); rules[i] = _firewallDao.persist(rules[i]); + + if (openFirewall) { + _firewallMgr.createRuleForAllCidrs(ip.getId(), caller, ports[i], ports[i], protocol, null, null, rules[i].getId()); + } } txn.commit(); @@ -1099,4 +1100,18 @@ 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); + + } + } diff --git a/server/src/com/cloud/network/rules/StaticNatRuleImpl.java b/server/src/com/cloud/network/rules/StaticNatRuleImpl.java index 58e8cda4384..a133112a4ba 100644 --- a/server/src/com/cloud/network/rules/StaticNatRuleImpl.java +++ b/server/src/com/cloud/network/rules/StaticNatRuleImpl.java @@ -123,4 +123,9 @@ public class StaticNatRuleImpl implements StaticNatRule{ return null; } + @Override + public Long getRelated() { + return null; + } + } diff --git a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index f4c3bd5332b..4e3c4f15cad 100755 --- a/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -17,6 +17,7 @@ */ package com.cloud.network.vpn; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -53,6 +54,7 @@ import com.cloud.network.router.VirtualNetworkApplianceManager; import com.cloud.network.rules.FirewallManager; import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.FirewallRule.Purpose; +import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.rules.RulesManager; import com.cloud.user.Account; import com.cloud.user.AccountManager; @@ -228,24 +230,51 @@ public class RemoteAccessVpnManagerImpl implements RemoteAccessVpnService, Manag } } finally { if (success) { + //Cleanup corresponding ports + List vpnFwRules = _rulesDao.listByIpAndPurpose(ipId, Purpose.Vpn); Transaction txn = Transaction.currentTxn(); - try { - txn.start(); - _remoteAccessVpnDao.remove(ipId); - - //Cleanup corresponding ports - List ports = _rulesDao.listByIpAndPurpose(ipId, Purpose.Vpn); - if (ports != null) { - for (FirewallRule port : ports) { - _rulesDao.remove(port.getId()); - s_logger.debug("Successfully removed firewall rule with ip id=" + port.getSourceIpAddressId() + " and port " + port.getSourcePortStart() + " as a part of vpn cleanup"); - } - } - txn.commit(); - } catch (Exception ex) { - txn.rollback(); - s_logger.warn("Unable to release the three vpn ports from the firewall rules", ex); + + boolean applyFirewall = false; + List fwRules = new ArrayList(); + //if related firewall rule is created for the first vpn port, it would be created for the 2 other ports as well, so need to cleanup the backend + if (_rulesDao.findByRelatedId(vpnFwRules.get(0).getId()) != null) { + applyFirewall = true; } + + if (applyFirewall) { + txn.start(); + + for (FirewallRule vpnFwRule : vpnFwRules) { + //don't apply on the backend yet; send all 3 rules in a banch + _rulesMgr.revokeRelatedFirewallRule(vpnFwRule.getId(), false); + fwRules.add(_rulesDao.findByRelatedId(vpnFwRule.getId())); + } + + txn.commit(); + + //now apply vpn rules on the backend + s_logger.debug("Applying " + fwRules.size() + " firewall rules as a part of disable remote access vpn"); + success = _firewallMgr.applyFirewallRules(fwRules, false, caller); + } + + if (success) { + + try { + txn.start(); + _remoteAccessVpnDao.remove(ipId); + if (vpnFwRules != null) { + for (FirewallRule vpnFwRule : vpnFwRules) { + _rulesDao.remove(vpnFwRule.getId()); + s_logger.debug("Successfully removed firewall rule with ip id=" + vpnFwRule.getSourceIpAddressId() + " and port " + vpnFwRule.getSourcePortStart() + " as a part of vpn cleanup"); + } + } + txn.commit(); + } catch (Exception ex) { + txn.rollback(); + s_logger.warn("Unable to release the three vpn ports from the firewall rules", ex); + } + } + } } } diff --git a/server/src/com/cloud/upgrade/dao/DbUpgradeUtils.java b/server/src/com/cloud/upgrade/dao/DbUpgradeUtils.java index 574bc79fba9..e61174a3ca0 100644 --- a/server/src/com/cloud/upgrade/dao/DbUpgradeUtils.java +++ b/server/src/com/cloud/upgrade/dao/DbUpgradeUtils.java @@ -14,8 +14,8 @@ public class DbUpgradeUtils { public static void dropKeysIfExist(Connection conn, String tableName, List keys, boolean isForeignKey) { for (String key : keys) { + PreparedStatement pstmt = null; try { - PreparedStatement pstmt = null; if (isForeignKey) { pstmt = conn.prepareStatement("ALTER TABLE " + tableName + " DROP FOREIGN KEY " + key); } else { @@ -23,10 +23,17 @@ public class DbUpgradeUtils { } pstmt.executeUpdate(); s_logger.debug("Key " + key + " is dropped successfully from the table " + tableName); - pstmt.close(); } catch (SQLException e) { // do nothing here + continue; + } finally { + try { + if (pstmt != null) { + pstmt.close(); + } + } catch (SQLException e) { + } } } } @@ -39,7 +46,6 @@ public class DbUpgradeUtils { try { pstmt = conn.prepareStatement("SELECT " + column + " FROM " + tableName); pstmt.executeQuery(); - } catch (SQLException e) { // if there is an exception, it means that field doesn't exist, so do nothing here s_logger.trace("Field " + column + " doesn't exist in " + tableName); @@ -49,11 +55,17 @@ public class DbUpgradeUtils { pstmt = conn.prepareStatement("ALTER TABLE " + tableName + " DROP COLUMN " + column); pstmt.executeUpdate(); s_logger.debug("Column " + column + " is dropped successfully from the table " + tableName); - pstmt.close(); } } catch (SQLException e) { s_logger.warn("Unable to drop columns using query " + pstmt + " due to exception", e); throw new CloudRuntimeException("Unable to drop columns due to ", e); + } finally { + try { + if (pstmt != null) { + pstmt.close(); + } + } catch (SQLException e) { + } } } } diff --git a/server/src/com/cloud/upgrade/dao/Upgrade229to2210.java b/server/src/com/cloud/upgrade/dao/Upgrade229to2210.java index d457c833861..0e8b74187cd 100644 --- a/server/src/com/cloud/upgrade/dao/Upgrade229to2210.java +++ b/server/src/com/cloud/upgrade/dao/Upgrade229to2210.java @@ -20,7 +20,9 @@ package com.cloud.upgrade.dao; import java.io.File; import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import java.util.UUID; import org.apache.log4j.Logger; @@ -57,15 +59,7 @@ public class Upgrade229to2210 implements DbUpgrade { @Override public void performDataMigration(Connection conn) { - PreparedStatement pstmt; - try { - pstmt = conn.prepareStatement("INSERT IGNORE INTO `cloud`.`configuration` (category, instance, name, value, description) VALUES ('Network', 'DEFAULT', 'firewall.rule.ui.enabled', 'true', 'enable/disable UI that separates firewall rules from NAT/LB rules')"); - pstmt.execute(); - - pstmt.close(); - } catch (SQLException e) { - throw new CloudRuntimeException("Unable to perform data migration", e); - } + updateFirewallRules(conn); } @Override @@ -73,4 +67,88 @@ public class Upgrade229to2210 implements DbUpgrade { return null; } + + private void updateFirewallRules(Connection conn) { + PreparedStatement pstmt = null; + ResultSet rs = null; + long currentRuleId = 0; + try { + // Host and Primary storage capacity types + pstmt = conn.prepareStatement("select id, ip_address_id, start_port, end_port, protocol, account_id, domain_id, network_id from firewall_rules"); + rs = pstmt.executeQuery(); + while (rs.next()) { + long id = rs.getLong(1); + long ipId = rs.getLong(2); + int startPort = rs.getInt(3); + int endPort = rs.getInt(4); + String protocol = rs.getString(5); + long accountId = rs.getLong(6); + long domainId = rs.getLong(7); + long networkId = rs.getLong(8); + currentRuleId = id; + Long firewallRuleId = null; + + pstmt = conn.prepareStatement("INSERT INTO firewall_rules (ip_address_id, start_port, end_port, protocol, account_id, domain_id, network_id, purpose, state, xid, created, related) VALUES (?, ?, ?, ?, ?, ?, ?, 'Firewall', 'Active', ?, now(), ?)"); + + pstmt.setLong(1, ipId); + pstmt.setInt(2, startPort); + pstmt.setInt(3, endPort); + pstmt.setString(4, protocol); + pstmt.setLong(5, accountId); + pstmt.setLong(6, domainId); + pstmt.setLong(7, networkId); + pstmt.setString(8, UUID.randomUUID().toString()); + pstmt.setLong(9, id); + + s_logger.debug("Updating firewall rule with the statement " + pstmt); + pstmt.executeUpdate(); + + //get new FirewallRule update + pstmt = conn.prepareStatement("SELECT id from firewall_rules where purpose='Firewall' and start_port=? and end_port=? and protocol=?"); + pstmt.setInt(1, startPort); + pstmt.setInt(2, endPort); + pstmt.setString(3, protocol); + + ResultSet rs1 = pstmt.executeQuery(); + + if (rs1.next()) { + firewallRuleId = rs1.getLong(1); + } else { + throw new CloudRuntimeException("Unable to find just inserted firewall rule for ptocol " + protocol + ", start_port " + startPort + " and end_port " + endPort); + } + + pstmt = conn.prepareStatement("select id from firewall_rules_cidrs where firewall_rule_id=?"); + pstmt.setLong(1, id); + + ResultSet rs2 = pstmt.executeQuery(); + + if (rs2.next()) { + pstmt = conn.prepareStatement("update firewall_rules_cidrs set firewall_rule_id=? where firewall_rule_id=?"); + pstmt.setLong(1, firewallRuleId); + pstmt.setLong(2, id); + s_logger.debug("Updating existing cidrs for the rule id=" + id + " with the new Firewall rule id=" + firewallRuleId + " with statement" + pstmt); + pstmt.executeUpdate(); + } else { + pstmt = conn.prepareStatement("insert into firewall_rules_cidrs (firewall_rule_id,source_cidr) values (?, '0.0.0.0/0')"); + pstmt.setLong(1, firewallRuleId); + s_logger.debug("Inserting rule for cidr 0.0.0.0/0 for the new Firewall rule id=" + firewallRuleId + " with statement " + pstmt); + pstmt.executeUpdate(); + } + } + } catch (SQLException e) { + throw new CloudRuntimeException("Unable to update firewall rule id=" + currentRuleId, e); + } finally { + try { + if (rs != null) { + rs.close(); + } + + if (pstmt != null) { + pstmt.close(); + } + } catch (SQLException e) { + } + } + } + } diff --git a/setup/db/create-schema.sql b/setup/db/create-schema.sql index 7e863df623c..aa4a125b246 100755 --- a/setup/db/create-schema.sql +++ b/setup/db/create-schema.sql @@ -588,11 +588,13 @@ CREATE TABLE `cloud`.`firewall_rules` ( `created` datetime COMMENT 'Date created', `icmp_code` int(10) COMMENT 'The ICMP code (if protocol=ICMP). A value of -1 means all codes for the given ICMP type.', `icmp_type` int(10) COMMENT 'The ICMP type (if protocol=ICMP). A value of -1 means all types.', + `related` bigint unsigned COMMENT 'related to what other firewall rule', PRIMARY KEY (`id`), CONSTRAINT `fk_firewall_rules__ip_address_id` FOREIGN KEY(`ip_address_id`) REFERENCES `user_ip_address`(`id`), CONSTRAINT `fk_firewall_rules__network_id` FOREIGN KEY(`network_id`) REFERENCES `networks`(`id`) ON DELETE CASCADE, CONSTRAINT `fk_firewall_rules__account_id` FOREIGN KEY(`account_id`) REFERENCES `account`(`id`) ON DELETE CASCADE, CONSTRAINT `fk_firewall_rules__domain_id` FOREIGN KEY(`domain_id`) REFERENCES `domain`(`id`) ON DELETE CASCADE, + CONSTRAINT `fk_firewall_rules__related` FOREIGN KEY(`related`) REFERENCES `firewall_rules`(`id`) ON DELETE CASCADE, INDEX `i_firewall_rules__purpose`(`purpose`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/setup/db/db/schema-229to2210.sql b/setup/db/db/schema-229to2210.sql index dffa1a3fbfc..be9ebf80609 100644 --- a/setup/db/db/schema-229to2210.sql +++ b/setup/db/db/schema-229to2210.sql @@ -18,3 +18,15 @@ ALTER TABLE `cloud`.`host` MODIFY `storage_ip_address` char(40); INSERT IGNORE INTO configuration VALUES ('Network', 'DEFAULT', 'management-server', 'network.redundantrouter', 'false', 'enable/disable redundant virtual router'); INSERT IGNORE INTO configuration VALUES ('Storage', 'DEFAULT', 'management-server', 'storage.pool.max.waitseconds', '3600', 'Timeout (in seconds) to synchronize storage pool operations.'); INSERT IGNORE INTO configuration VALUES ('Storage', 'DEFAULT', 'management-server', 'storage.template.cleanup.enabled', 'true', 'Enable/disable template cleanup activity, only take effect when overall storage cleanup is enabled'); + + +ALTER TABLE `cloud`.`firewall_rules` ADD COLUMN `icmp_code` int(10) COMMENT 'The ICMP code (if protocol=ICMP). A value of -1 means all codes for the given ICMP type.'; +ALTER TABLE `cloud`.`firewall_rules` ADD COLUMN `icmp_type` int(10) COMMENT 'The ICMP type (if protocol=ICMP). A value of -1 means all types.'; +ALTER TABLE `cloud`.`firewall_rules` ADD COLUMN `related` bigint unsigned COMMENT 'related to what other firewall rule'; +ALTER TABLE `cloud`.`firewall_rules` ADD CONSTRAINT `fk_firewall_rules__related` FOREIGN KEY(`related`) REFERENCES `firewall_rules`(`id`) ON DELETE CASCADE; + +ALTER TABLE `cloud`.`firewall_rules` MODIFY `start_port` int(10) COMMENT 'starting port of a port range'; +ALTER TABLE `cloud`.`firewall_rules` MODIFY `end_port` int(10) COMMENT 'end port of a port range'; + +INSERT IGNORE INTO `cloud`.`configuration` (category, instance, name, value, description) VALUES ('Network', 'DEFAULT', 'firewall.rule.ui.enabled', 'true', 'enable/disable UI that separates firewall rules from NAT/LB rules'); +