From dff712f2a99508529cd0eace11d2998cd8f4f53e Mon Sep 17 00:00:00 2001 From: Abhinandan Prateek Date: Thu, 21 Jul 2011 13:40:06 +0530 Subject: [PATCH] bug 10731: sending source cidrs to the required router element, on domR allowing these source cidrs thru iptables status 10731: resolved fixed --- api/src/com/cloud/agent/api/to/LoadBalancerTO.java | 13 ++++++++++++- .../api/commands/CreatePortForwardingRuleCmd.java | 1 - .../com/cloud/network/rules/PortForwardingRule.java | 4 ---- .../hypervisor/xen/resource/CitrixResourceBase.java | 3 ++- core/src/com/cloud/network/HAProxyConfigurator.java | 8 +++++++- patches/systemvm/debian/config/root/loadbalancer.sh | 12 +++++++----- .../src/com/cloud/network/FirewallRulesCidrsVO.java | 4 ++++ server/src/com/cloud/network/LoadBalancerVO.java | 2 +- .../com/cloud/network/dao/LoadBalancerDaoImpl.java | 6 +----- .../network/lb/LoadBalancingRulesManagerImpl.java | 13 ++++++++----- .../router/VirtualNetworkApplianceManagerImpl.java | 9 +++++++-- .../network/rules/dao/PortForwardingRulesDao.java | 3 --- .../rules/dao/PortForwardingRulesDaoImpl.java | 6 +----- 13 files changed, 50 insertions(+), 34 deletions(-) diff --git a/api/src/com/cloud/agent/api/to/LoadBalancerTO.java b/api/src/com/cloud/agent/api/to/LoadBalancerTO.java index eff9b4a530b..4925b9f6377 100644 --- a/api/src/com/cloud/agent/api/to/LoadBalancerTO.java +++ b/api/src/com/cloud/agent/api/to/LoadBalancerTO.java @@ -20,19 +20,21 @@ package com.cloud.agent.api.to; import java.util.List; import com.cloud.network.lb.LoadBalancingRule.LbDestination; +import com.cloud.utils.StringUtils; public class LoadBalancerTO { String srcIp; int srcPort; String protocol; + List sourceCidrs; String algorithm; boolean revoked; boolean alreadyAdded; DestinationTO[] destinations; - public LoadBalancerTO (String srcIp, int srcPort, String protocol, String algorithm, boolean revoked, boolean alreadyAdded, List destinations) { + public LoadBalancerTO (String srcIp, int srcPort, String protocol, List sourceCidrs, String algorithm, boolean revoked, boolean alreadyAdded, List destinations) { this.srcIp = srcIp; this.srcPort = srcPort; this.protocol = protocol; @@ -40,6 +42,7 @@ public class LoadBalancerTO { this.revoked = revoked; this.alreadyAdded = alreadyAdded; this.destinations = new DestinationTO[destinations.size()]; + this.sourceCidrs = sourceCidrs; int i = 0; for (LbDestination destination : destinations) { this.destinations[i++] = new DestinationTO(destination.getIpAddress(), destination.getDestinationPortStart(), destination.isRevoked(), false); @@ -56,6 +59,14 @@ public class LoadBalancerTO { public int getSrcPort() { return srcPort; } + + public List getSourceCidrs(){ + return sourceCidrs; + } + + public String getStringSourceCidrs(){ + return StringUtils.join(sourceCidrs, "-"); + } public String getAlgorithm() { return algorithm; diff --git a/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java b/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java index cb0c8b489e0..89eeb442f28 100644 --- a/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java @@ -108,7 +108,6 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P return s_name; } - @Override public void setSourceCidrList(List cidrs){ cidrlist = cidrs; } diff --git a/api/src/com/cloud/network/rules/PortForwardingRule.java b/api/src/com/cloud/network/rules/PortForwardingRule.java index 74176c9bceb..25930653deb 100644 --- a/api/src/com/cloud/network/rules/PortForwardingRule.java +++ b/api/src/com/cloud/network/rules/PortForwardingRule.java @@ -49,8 +49,4 @@ public interface PortForwardingRule extends FirewallRule { * @return source cidr to forward */ List getSourceCidrList(); - /** - * @return source cidr to forward - */ - void setSourceCidrList(List cidrs); } diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index 02e51add020..80a61242ad5 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -154,6 +154,7 @@ 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.IpAddressTO; +import com.cloud.agent.api.to.LoadBalancerTO; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.PortForwardingRuleTO; import com.cloud.agent.api.to.StaticNatRuleTO; @@ -1283,7 +1284,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe if (routerIp == null) { return new Answer(cmd); } - + LoadBalancerConfigurator cfgtr = new HAProxyConfigurator(); String[] config = cfgtr.generateConfiguration(cmd); String[][] rules = cfgtr.generateFwRules(cmd); diff --git a/core/src/com/cloud/network/HAProxyConfigurator.java b/core/src/com/cloud/network/HAProxyConfigurator.java index cd491a8a6b6..ae8569376e1 100644 --- a/core/src/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/com/cloud/network/HAProxyConfigurator.java @@ -227,6 +227,12 @@ public class HAProxyConfigurator implements LoadBalancerConfigurator { StringBuilder sb = new StringBuilder(); sb.append(lbTO.getSrcIp()).append(":"); sb.append(lbTO.getSrcPort()).append(":"); + if (lbTO.getSourceCidrs() != null && lbTO.getSourceCidrs().size() > 0){ + sb.append(lbTO.getStringSourceCidrs()).append(":"); + } + else { + sb.append("0/0:"); + } String lbRuleEntry = sb.toString(); if (!lbTO.isRevoked()) { toAdd.add(lbRuleEntry); @@ -236,7 +242,7 @@ public class HAProxyConfigurator implements LoadBalancerConfigurator { } toRemove.removeAll(toAdd); result[ADD] = toAdd.toArray(new String[toAdd.size()]); - result[REMOVE] = toRemove.toArray(new String[toRemove.size()]); + result[REMOVE] = toRemove.toArray(new String[toRemove.size()]); return result; } diff --git a/patches/systemvm/debian/config/root/loadbalancer.sh b/patches/systemvm/debian/config/root/loadbalancer.sh index b01cb38f497..f4405b3a813 100755 --- a/patches/systemvm/debian/config/root/loadbalancer.sh +++ b/patches/systemvm/debian/config/root/loadbalancer.sh @@ -64,11 +64,12 @@ fw_entry() { for i in $a do local pubIp=$(echo $i | cut -d: -f1) - local dport=$(echo $i | cut -d: -f2) + local dport=$(echo $i | cut -d: -f2) + local cidrs=$(echo $i | cut -d: -f3 | sed 's/-/,/') for vif in $VIF_LIST; do - iptables -D INPUT -i $vif -p tcp -d $pubIp --dport $dport -j ACCEPT 2> /dev/null - iptables -A INPUT -i $vif -p tcp -d $pubIp --dport $dport -j ACCEPT + iptables -D INPUT -i $vif -s $cidrs -p tcp -d $pubIp --dport $dport -j ACCEPT 2> /dev/null + iptables -A INPUT -i $vif -s $cidrs -p tcp -d $pubIp --dport $dport -j ACCEPT if [ $? -gt 0 ] then @@ -80,10 +81,11 @@ fw_entry() { for i in $r do local pubIp=$(echo $i | cut -d: -f1) - local dport=$(echo $i | cut -d: -f2) + local dport=$(echo $i | cut -d: -f2) + local cidrs=$(echo $i | cut -d: -f3 | sed 's/-/,/') for vif in $VIF_LIST; do - iptables -D INPUT -i $vif -p tcp -d $pubIp --dport $dport -j ACCEPT + iptables -D INPUT -i $vif -s $cidrs -p tcp -d $pubIp --dport $dport -j ACCEPT done done diff --git a/server/src/com/cloud/network/FirewallRulesCidrsVO.java b/server/src/com/cloud/network/FirewallRulesCidrsVO.java index fde8d879b1b..1bc86b182a3 100644 --- a/server/src/com/cloud/network/FirewallRulesCidrsVO.java +++ b/server/src/com/cloud/network/FirewallRulesCidrsVO.java @@ -53,6 +53,10 @@ public class FirewallRulesCidrsVO { public long getFirewallRuleId() { return firewallRuleId; } + + public void setFirewallRuleId(long firewallRuleId){ + this.firewallRuleId = firewallRuleId; + } public String getCidr() { return sourceCidrList; diff --git a/server/src/com/cloud/network/LoadBalancerVO.java b/server/src/com/cloud/network/LoadBalancerVO.java index ae79aee7067..193c8e1beca 100644 --- a/server/src/com/cloud/network/LoadBalancerVO.java +++ b/server/src/com/cloud/network/LoadBalancerVO.java @@ -18,6 +18,7 @@ package com.cloud.network; +import java.util.ArrayList; import java.util.List; import javax.persistence.Column; @@ -81,7 +82,6 @@ public class LoadBalancerVO extends FirewallRuleVO implements LoadBalancer { public void setSourceCidrList(List sourceCidrs) { this.sourceCidrs=sourceCidrs; } - @Override public List getSourceCidrList() { return sourceCidrs; diff --git a/server/src/com/cloud/network/dao/LoadBalancerDaoImpl.java b/server/src/com/cloud/network/dao/LoadBalancerDaoImpl.java index 4bd8fd4abb5..38ef6faeeaf 100644 --- a/server/src/com/cloud/network/dao/LoadBalancerDaoImpl.java +++ b/server/src/com/cloud/network/dao/LoadBalancerDaoImpl.java @@ -142,12 +142,9 @@ public class LoadBalancerDaoImpl extends GenericDaoBase im txn.start(); LoadBalancerVO dbfirewallRule = super.persist(loadBalancerRule); - saveSourceCidrs(loadBalancerRule); - loadSourceCidrs(dbfirewallRule); txn.commit(); - return dbfirewallRule; } @@ -161,10 +158,9 @@ public class LoadBalancerDaoImpl extends GenericDaoBase im if (!persisted) { return persisted; } - saveSourceCidrs(loadBalancerRule); + txn.commit(); - return persisted; } diff --git a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 5d55c390ea2..73b3fb265bb 100755 --- a/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -52,6 +52,7 @@ import com.cloud.network.LoadBalancerVO; import com.cloud.network.Network; import com.cloud.network.Network.Service; import com.cloud.network.NetworkManager; +import com.cloud.network.dao.FirewallRulesCidrsDao; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.LoadBalancerDao; @@ -61,6 +62,7 @@ import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.FirewallRule.Purpose; import com.cloud.network.rules.FirewallRuleVO; import com.cloud.network.rules.LoadBalancer; +import com.cloud.network.rules.PortForwardingRuleVO; import com.cloud.network.rules.RulesManager; import com.cloud.user.Account; import com.cloud.user.AccountManager; @@ -118,6 +120,8 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, NicDao _nicDao; @Inject UsageEventDao _usageEventDao; + @Inject + FirewallRulesCidrsDao _firewallCidrsDao; @Override @DB @@ -222,7 +226,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, _lb2VmMapDao.persist(map); s_logger.debug("Set load balancer rule for revoke: rule id " + loadBalancerId + ", vmId " + instanceId); } - + if (!applyLoadBalancerConfig(loadBalancerId)) { s_logger.warn("Failed to remove load balancer rule id " + loadBalancerId + " for vms " + instanceIds); throw new CloudRuntimeException("Failed to remove load balancer rule id " + loadBalancerId + " for vms " + instanceIds); @@ -431,7 +435,6 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, @Override public boolean applyLoadBalancersForNetwork(long networkId) throws ResourceUnavailableException { List lbs = _lbDao.listByNetworkId(networkId); - if (lbs != null) { return applyLoadBalancerRules(lbs); } else { @@ -462,10 +465,10 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, txn.start(); if (lb.getState() == FirewallRule.State.Revoke) { _lbDao.remove(lb.getId()); - s_logger.debug("LB " + lb.getId() + " is successfully removed"); + s_logger.warn("LB " + lb.getId() + " is successfully removed"); } else if (lb.getState() == FirewallRule.State.Add) { lb.setState(FirewallRule.State.Active); - s_logger.debug("LB rule " + lb.getId() + " state is set to Active"); + s_logger.warn("LB rule " + lb.getId() + " state is set to Active"); _lbDao.persist(lb); } @@ -484,7 +487,7 @@ public class LoadBalancingRulesManagerImpl implements LoadBalancingRulesManager, if (_lb2VmMapDao.listByLoadBalancerId(lb.getId()).isEmpty()) { lb.setState(FirewallRule.State.Add); - _lbDao.persist(lb); + _lbDao.persist(lb); s_logger.debug("LB rule " + lb.getId() + " state is set to Add as there are no more active LB-VM mappings"); } diff --git a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index a1ab455d5e0..ce3d3ea4aec 100755 --- a/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -114,6 +114,7 @@ import com.cloud.network.VirtualNetworkApplianceService; import com.cloud.network.VpnUser; import com.cloud.network.VpnUserVO; import com.cloud.network.addr.PublicIp; +import com.cloud.network.dao.FirewallRulesCidrsDao; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.LoadBalancerDao; @@ -290,6 +291,8 @@ public class VirtualNetworkApplianceManagerImpl implements VirtualNetworkApplian NicDao _nicDao; @Inject VolumeDao _volumeDao = null; + @Inject + FirewallRulesCidrsDao _firewallCidrsDao; int _routerRamSize; int _routerCpuMHz; @@ -1843,11 +1846,12 @@ public class VirtualNetworkApplianceManagerImpl implements VirtualNetworkApplian boolean revoked = (rule.getState().equals(FirewallRule.State.Revoke)); String protocol = rule.getProtocol(); String algorithm = rule.getAlgorithm(); + List sourceCidrs = rule.getSourceCidrList(); String srcIp = _networkMgr.getIp(rule.getSourceIpAddressId()).getAddress().addr(); int srcPort = rule.getSourcePortStart(); List destinations = rule.getDestinations(); - LoadBalancerTO lb = new LoadBalancerTO(srcIp, srcPort, protocol, algorithm, revoked, false, destinations); + LoadBalancerTO lb = new LoadBalancerTO(srcIp, srcPort, protocol, sourceCidrs, algorithm, revoked, false, destinations); lbs[i++] = lb; } @@ -2007,10 +2011,11 @@ public class VirtualNetworkApplianceManagerImpl implements VirtualNetworkApplian List lbRules = new ArrayList(); for (LoadBalancerVO lb : lbs) { List dstList = _lbMgr.getExistingDestinations(lb.getId()); + // load the cidrs, + lb.setSourceCidrList(_firewallCidrsDao.getSourceCidrs(lb.getId())); LoadBalancingRule loadBalancing = new LoadBalancingRule(lb, dstList); lbRules.add(loadBalancing); } - result = result && applyLBRules(router, lbRules); } else if (rules.get(0).getPurpose() == Purpose.PortForwarding) { result = result && applyPortForwardingRules(router, (List) rules); diff --git a/server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java b/server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java index f29ec55bc71..60abeeec36a 100644 --- a/server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java +++ b/server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java @@ -43,7 +43,4 @@ public interface PortForwardingRulesDao extends GenericDao listByAccount(long accountId); - void loadSourceCidrs(PortForwardingRuleVO portForwardingRule); - - void saveSourceCidrs(PortForwardingRuleVO portForwardingRule); } diff --git a/server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java b/server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java index 6e368334f65..1846aaa3010 100644 --- a/server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java +++ b/server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java @@ -177,12 +177,9 @@ public class PortForwardingRulesDaoImpl extends GenericDaoBase