From 239942bbe1ca12415d27c0cdab4008b2cbe23b39 Mon Sep 17 00:00:00 2001 From: abhishek Date: Wed, 10 Nov 2010 18:51:56 -0800 Subject: [PATCH] refactored all the commands to cater to keshav's needs, and added the new apis. the initial set of testing is complete, will now focus on corner cases --- client/tomcatconf/commands.properties.in | 1 + .../api/routing/SetFirewallRuleCommand.java | 22 +++---- .../xen/resource/CitrixResourceBase.java | 62 +++++++++++-------- .../systemvm/debian/config/root/firewall.sh | 37 ++++++++++- scripts/network/domr/call_firewall.sh | 12 ++-- scripts/network/domr/firewall.sh | 44 +------------ .../com/cloud/network/NetworkManagerImpl.java | 49 +++++++++++---- 7 files changed, 128 insertions(+), 99 deletions(-) diff --git a/client/tomcatconf/commands.properties.in b/client/tomcatconf/commands.properties.in index 8a106fffc42..5bfcfad7079 100755 --- a/client/tomcatconf/commands.properties.in +++ b/client/tomcatconf/commands.properties.in @@ -109,6 +109,7 @@ updatePortForwardingRule=com.cloud.api.commands.UpdatePortForwardingRuleCmd;15 #### NAT commands createIpForwardingRule=com.cloud.api.commands.CreateIpForwardingRuleCmd;15 +deleteIpForwardingRule=com.cloud.api.commands.DeleteIpForwardingRuleCmd;15 #### load balancer commands createLoadBalancerRule=com.cloud.api.commands.CreateLoadBalancerRuleCmd;15 diff --git a/core/src/com/cloud/agent/api/routing/SetFirewallRuleCommand.java b/core/src/com/cloud/agent/api/routing/SetFirewallRuleCommand.java index 89d44032934..92ebcdd8195 100755 --- a/core/src/com/cloud/agent/api/routing/SetFirewallRuleCommand.java +++ b/core/src/com/cloud/agent/api/routing/SetFirewallRuleCommand.java @@ -25,8 +25,8 @@ public class SetFirewallRuleCommand extends RoutingCommand { String routerIpAddress; String oldPrivateIP = null; String oldPrivatePort = null; - String guestIp = null; - String portRange = null; + boolean nat = false; + boolean create = false; protected SetFirewallRuleCommand() { } @@ -39,12 +39,12 @@ public class SetFirewallRuleCommand extends RoutingCommand { this.oldPrivatePort = oldPrivatePort; } - public SetFirewallRuleCommand(String routerName, String routerIpAddress, String guestIp, FirewallRuleVO rule, String portRange) { + public SetFirewallRuleCommand(String routerName, String routerIpAddress, boolean nat, FirewallRuleVO rule, boolean create) { this.routerName = routerName; this.routerIpAddress = routerIpAddress; - this.guestIp = guestIp; + this.nat = nat; this.rule = rule; - this.portRange = portRange; + this.create = create; } @Override @@ -100,12 +100,12 @@ public class SetFirewallRuleCommand extends RoutingCommand { return this.oldPrivatePort; } - public String getPortRange(){ - return this.portRange; - } - - public String getGuestIp(){ - return this.guestIp; + public boolean isNat(){ + return this.nat; } + + public boolean isCreate() { + return create; + } } diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index 72e3761c766..89f97e00f46 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -1115,32 +1115,44 @@ public abstract class CitrixResourceBase implements StoragePoolResource, ServerR protected Answer execute(final SetFirewallRuleCommand cmd) { String args; - if (cmd.isEnable()) { - args = "-A"; - } else { - args = "-D"; + if(cmd.isNat()){ + //1:1 NAT needs instanceip;publicip;domrip;op + if(cmd.isCreate()) + args = "-A"; + else + args = "-D"; + + args += " -l " + cmd.getPublicIpAddress(); + args += " -i " + cmd.getRouterIpAddress(); + args += " -r " + cmd.getPrivateIpAddress(); + args += " -G " + cmd.getProtocol(); + }else{ + if (cmd.isEnable()) { + args = "-A"; + } else { + args = "-D"; + } + + args += " -P " + cmd.getProtocol().toLowerCase(); + args += " -l " + cmd.getPublicIpAddress(); + args += " -p " + cmd.getPublicPort(); + args += " -n " + cmd.getRouterName(); + args += " -i " + cmd.getRouterIpAddress(); + args += " -r " + cmd.getPrivateIpAddress(); + args += " -d " + cmd.getPrivatePort(); + args += " -N " + cmd.getVlanNetmask(); + + String oldPrivateIP = cmd.getOldPrivateIP(); + String oldPrivatePort = cmd.getOldPrivatePort(); + + if (oldPrivateIP != null) { + args += " -w " + oldPrivateIP; + } + + if (oldPrivatePort != null) { + args += " -x " + oldPrivatePort; + } } - - args += " -P " + cmd.getProtocol().toLowerCase(); - args += " -l " + cmd.getPublicIpAddress(); - args += " -p " + cmd.getPublicPort(); - args += " -n " + cmd.getRouterName(); - args += " -i " + cmd.getRouterIpAddress(); - args += " -r " + cmd.getPrivateIpAddress(); - args += " -d " + cmd.getPrivatePort(); - args += " -N " + cmd.getVlanNetmask(); - - String oldPrivateIP = cmd.getOldPrivateIP(); - String oldPrivatePort = cmd.getOldPrivatePort(); - - if (oldPrivateIP != null) { - args += " -w " + oldPrivateIP; - } - - if (oldPrivatePort != null) { - args += " -x " + oldPrivatePort; - } - String result = callHostPlugin("vmops", "setFirewallRule", "args", args); if (result == null || result.isEmpty()) { diff --git a/patches/systemvm/debian/config/root/firewall.sh b/patches/systemvm/debian/config/root/firewall.sh index 89cd0d4a95e..35f29d8d41d 100755 --- a/patches/systemvm/debian/config/root/firewall.sh +++ b/patches/systemvm/debian/config/root/firewall.sh @@ -9,7 +9,7 @@ usage() { printf "Usage: %s: (-A|-D) -i -r -P protocol (-p port_range | -t icmp_type_code) -l -d [-f -u -y -z ] \n" $(basename $0) >&2 } -set -x +# set -x get_dom0_ip () { eval "$1=$(ifconfig eth0 | awk '/inet addr/ {split ($2,A,":"); print A[2]}')" @@ -71,6 +71,27 @@ icmp_entry() { return $? } +#Add 1:1 NAT entry +add_one_to_one_nat_entry() { + local guestIp=$1 + local publicIp=$2 + local dIp=$3 + local op=$4 + iptables -t nat $op PREROUTING -i eth2 -d $publicIp -j DNAT --to-destination $guestIp + iptables -t nat $op POSTROUTING -o eth2 -s $guestIp -j SNAT --to-source $publicIp + if [ "$op" == "-A" ] + then + iptables -P FORWARD DROP + else + iptables -P FORWARD ACCEPT + fi + iptables $op FORWARD -m state --state RELATED,ESTABLISHED -j ACCEPT + iptables $op FORWARD -i eth2 -o eth0 -d $guestIp -m state --state NEW -j ACCEPT + iptables $op FORWARD -i eth0 -o eth2 -s $guestIp -m state --state NEW -j ACCEPT + + return $? +} + get_vif_list() { local vif_list="" for i in /sys/class/net/eth*; do @@ -107,11 +128,12 @@ wflag= xflag= nflag= Nflag= +Gflag= op="" oldPrivateIP="" oldPrivatePort="" -while getopts 'ADr:i:P:p:t:l:d:w:x:n:N:' OPTION +while getopts 'ADr:i:P:p:t:l:d:w:x:n:N:G:' OPTION do case $OPTION in A) Aflag=1 @@ -153,12 +175,23 @@ do N) Nflag=1 netmask="$OPTARG" ;; + G) Gflag=1 + nat="$OPTARG" + ;; ?) usage exit 2 ;; esac done +#1:1 NAT +if [ "$Gflag" == "1" ] + then + add_one_to_one_nat_entry $instanceIp $publicIp $domRIp $op + fi + exit $? +fi + reverseOp=$(reverse_op $op) VIF_LIST=$(get_vif_list) diff --git a/scripts/network/domr/call_firewall.sh b/scripts/network/domr/call_firewall.sh index 287efa21f5d..8c84a121829 100755 --- a/scripts/network/domr/call_firewall.sh +++ b/scripts/network/domr/call_firewall.sh @@ -33,11 +33,12 @@ wflag= xflag= nflag= Nflag= +Gflag= op="" oldPrivateIP="" oldPrivatePort="" -while getopts 'ADr:i:P:p:t:l:d:w:x:n:N:' OPTION +while getopts 'ADr:i:P:p:t:l:d:w:x:n:N:G:' OPTION do case $OPTION in A) Aflag=1 @@ -79,6 +80,9 @@ do N) Nflag=1 netmask="$OPTARG" ;; + G) Gflag=1 + nat="OPTARG" + ;; ?) usage exit 2 ;; @@ -101,12 +105,6 @@ then exit 2 fi -if [ "$rflag$iflag$Pflag$pflag$tflag$lflag" != "11111" ] -then - usage - exit 2 -fi - #Require -d with -p if [ "$pflag$dflag" != 11 -a "$pflag$dflag" != "" ] then diff --git a/scripts/network/domr/firewall.sh b/scripts/network/domr/firewall.sh index c4d5d3052f9..91cf622c051 100755 --- a/scripts/network/domr/firewall.sh +++ b/scripts/network/domr/firewall.sh @@ -87,31 +87,6 @@ icmp_entry() { } - -#Add 1:1 NAT entry -add_one_to_one_nat_entry() { - local guestIp=$1 - local publicIp=$2 - local dIp=$3 - local portRange=$4 - local op=$5 - ssh -p 3922 -o StrictHostKeyChecking=no -i $cert root@$dIp "\ - iptables -t nat $op PREROUTING -i eth2 -d $publicIp -j DNAT --to-destination $guestIp - iptables -t nat $op POSTROUTING -o eth2 -s $guestIp -j SNAT --to-source $publicIp":"$portRange - if [ "$op" == "-A" ] - then - iptables -P FORWARD DROP - else - iptables -P FORWARD ACCEPT - fi - iptables $op FORWARD -m state --state RELATED,ESTABLISHED -j ACCEPT - iptables $op FORWARD -i eth2 -o eth0 -d $guestIp -m state --state NEW -j ACCEPT - iptables $op FORWARD -i eth0 -o eth2 -s $guestIp -m state --state NEW -j ACCEPT - " - - return $? -} - reverse_op() { local op=$1 @@ -135,13 +110,11 @@ wflag= xflag= nflag= Nflag= -Gflag= -Rflag= op="" oldPrivateIP="" oldPrivatePort="" -while getopts 'ADr:i:P:p:t:l:d:w:x:n:N:G:R:' OPTION +while getopts 'ADr:i:P:p:t:l:d:w:x:n:N:' OPTION do case $OPTION in A) Aflag=1 @@ -183,11 +156,6 @@ do N) Nflag=1 netmask="$OPTARG" ;; - G) Gflag=1 - guestIp="$OPTARG" - ;; - R) Rflag=1 - portRange="$OPTARG" ?) usage exit 2 ;; @@ -224,14 +192,6 @@ then exit 2 fi -#1:1 NAT -if [ "$Gflag$Rflag" == "11" ] - then - add_one_to_one_nat_entry $guestIp $publicIp $domRIp $portRange $op - fi - exit $? -fi - reverseOp=$(reverse_op $op) case $protocol in @@ -271,4 +231,4 @@ case $protocol in ;; esac -exit 0 +exit 0 \ No newline at end of file diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 03cd2310d4b..fc40614e1cc 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -108,7 +108,6 @@ import com.cloud.network.Network.TrafficType; import com.cloud.network.configuration.NetworkGuru; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; -import com.cloud.network.dao.IprulePortrangeMapDao; import com.cloud.network.dao.LoadBalancerDao; import com.cloud.network.dao.LoadBalancerVMMapDao; import com.cloud.network.dao.NetworkConfigurationDao; @@ -220,7 +219,6 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag @Inject RemoteAccessVpnDao _remoteAccessVpnDao = null; @Inject VpnUserDao _vpnUsersDao = null; @Inject DomainRouterManager _routerMgr; - @Inject IprulePortrangeMapDao _iprulePortrangeMapDao = null; @Inject(adapter=NetworkGuru.class) Adapters _networkGurus; @@ -898,7 +896,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag if (rule.isForwarding()) { fwdRules.add(rule); - final SetFirewallRuleCommand cmd = new SetFirewallRuleCommand(routerName, routerIp, null, rule, null); + final SetFirewallRuleCommand cmd = new SetFirewallRuleCommand(routerName, routerIp, false, rule, false); cmds.addCommand(cmd); } else { lbRules.add(rule); @@ -977,7 +975,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag rule.setVlanNetmask(vlanNetmask); if (rule.isForwarding()) { fwdRules.add(rule); - final SetFirewallRuleCommand cmd = new SetFirewallRuleCommand(router.getInstanceName(), router.getPrivateIpAddress(), null, rule, null); + final SetFirewallRuleCommand cmd = new SetFirewallRuleCommand(router.getInstanceName(), router.getPrivateIpAddress(), false, rule, false); cmds.addCommand(cmd); } } @@ -2948,6 +2946,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag UserVmVO userVM = null; FirewallRuleVO newFwRule = null; boolean locked = false; + boolean success = false; try { // validate IP Address exists IPAddressVO ipAddress = _ipAddressDao.findById(cmd.getIpAddress()); @@ -2999,7 +2998,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag if(existingNatRules.size() > 0){ throw new NetworkRuleConflictException("The specified rule for public ip:"+cmd.getIpAddress()+" vm id:"+cmd.getVirtualMachineId()+" already exists"); } - + newFwRule = new FirewallRuleVO(); newFwRule.setEnabled(true); newFwRule.setForwarding(true); @@ -3009,6 +3008,15 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag newFwRule.setPublicIpAddress(ipAddress.getAddress()); newFwRule.setPrivateIpAddress(userVM.getGuestIpAddress()); newFwRule.setGroupId(null); + + //get the domain router object + final DomainRouterVO router = _routerMgr.getRouter(ipAddress.getAccountId(), ipAddress.getDataCenterId()); + success = createOrDeleteIpForwardingRuleOnDomr(newFwRule,router,userVM.getGuestIpAddress(),true); //true +> create + + if(!success){ + throw new PermissionDeniedException("Cannot create ip forwarding rule on domr"); + } + _rulesDao.persist(newFwRule); // Save and create the event @@ -3025,6 +3033,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } catch (Exception e) { s_logger.warn("Unable to create new firewall rule for 1:1 NAT"); txn.rollback(); + throw new ServerApiException(BaseCmd.IP_ALLOCATION_ERROR,"Unable to create new firewall rule for 1:1 NAT:"+e.getMessage()); }finally{ if(locked) _vmDao.releaseFromLockTable(userVM.getId()); @@ -3085,14 +3094,10 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag locked = true; txn.start(); - //if there is an open port range associated with the rule, don't allow deletion - List portRecordsForRule = _iprulePortrangeMapDao.listPortRecordsForRule(ruleId); - - if (portRecordsForRule != null && portRecordsForRule.size() > 0) { - throw new InvalidParameterValueException("Cannot delete rule as port mappings for this rule exist; please delete those mappings first"); - } - success = _firewallRulesDao.remove(ruleId); + final DomainRouterVO router = _routerMgr.getRouter(ipAddress.getAccountId(), ipAddress.getDataCenterId()); + success = createOrDeleteIpForwardingRuleOnDomr(rule, router, rule.getPrivateIpAddress(), false); + _firewallRulesDao.remove(ruleId); String description; String type = EventTypes.EVENT_NET_RULE_DELETE; @@ -3121,4 +3126,24 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } return success; } + + private boolean createOrDeleteIpForwardingRuleOnDomr(FirewallRuleVO fwRule, DomainRouterVO router, String guestIp, boolean create){ + + Commands cmds = new Commands(OnError.Continue); + final SetFirewallRuleCommand cmd = new SetFirewallRuleCommand(router.getInstanceName(), router.getPrivateIpAddress(), true, fwRule, create); + cmds.addCommand(cmd); + try { + _agentMgr.send(router.getHostId(), cmds); + } catch (final AgentUnavailableException e) { + s_logger.warn("agent unavailable", e); + } catch (final OperationTimedoutException e) { + s_logger.warn("Timed Out", e); + } + Answer[] answers = cmds.getAnswers(); + if (answers == null || answers[0].getResult() == false ){ + return false; + }else{ + return true; + } + } }