diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 59ac4f44938..9df8906070f 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -531,8 +531,8 @@ public class SecondaryStorageManagerImpl extends ManagerBase implements Secondar /** * Get the default network for the secondary storage VM, based on the zone it is in. Delegates to - * either {@link #getDefaultNetworkForZone(DataCenter)} or {@link #getDefaultNetworkForAdvancedSGZone(DataCenter)}, - * depending on the zone network type and whether or not security groups are enabled in the zone. + * either {@link #getDefaultNetworkForAdvancedZone(DataCenter)} or {@link #getDefaultNetworkForBasicZone(DataCenter)}, + * depending on the zone network type and whether security groups are enabled in the zone. * @param dc - The zone (DataCenter) of the secondary storage VM. * @return The default network for use with the secondary storage VM. */ diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/IpTablesHelper.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/IpTablesHelper.java new file mode 100644 index 00000000000..b9f0d2fdc77 --- /dev/null +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/IpTablesHelper.java @@ -0,0 +1,66 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package org.apache.cloudstack.storage.resource; + +import com.cloud.utils.script.Script; +import org.apache.log4j.Logger; + +public class IpTablesHelper { + public static final Logger LOGGER = Logger.getLogger(IpTablesHelper.class); + + public static final String OUTPUT_CHAIN = "OUTPUT"; + public static final String INPUT_CHAIN = "INPUT"; + public static final String INSERT = " -I "; + public static final String APPEND = " -A "; + + public static boolean needsAdding(String chain, String rule) { + Script command = new Script("/bin/bash", LOGGER); + command.add("-c"); + command.add("iptables -C " + chain + " " + rule); + + String commandOutput = command.execute(); + boolean needsAdding = (commandOutput != null && commandOutput.contains("iptables: Bad rule (does a matching rule exist in that chain?).")); + LOGGER.debug(String.format("Rule [%s], %s need adding to [%s] : %s", + rule, + needsAdding ? "does indeed" : "doesn't", + chain, + commandOutput + )); + return needsAdding; + } + + public static String addConditionally(String chain, boolean insert, String rule, String errMsg) { + LOGGER.info(String.format("Adding rule [%s] to [%s] if required.", rule, chain)); + if (needsAdding(chain, rule)) { + Script command = new Script("/bin/bash", LOGGER); + command.add("-c"); + command.add("iptables" + (insert ? INSERT : APPEND) + chain + " " + rule); + String result = command.execute(); + LOGGER.debug(String.format("Executed [%s] with result [%s]", command, result)); + if (result != null) { + LOGGER.warn(String.format("%s , err = %s", errMsg, result)); + return errMsg + result; + } + } else { + LOGGER.warn("Rule already defined in SVM: " + rule); + } + return null; + } +} diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index d8f7395c885..8d2ebcd0458 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -2287,15 +2287,14 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S if (!_inSystemVM) { return null; } - Script command = new Script("/bin/bash", s_logger); String intf = "eth1"; - command.add("-c"); - command.add("iptables -I OUTPUT -o " + intf + " -d " + destCidr + " -p tcp -m state --state NEW -m tcp -j ACCEPT"); + String rule = String.format("-o %s -d %s -p tcp -m state --state NEW -m tcp -j ACCEPT", intf, destCidr); + String errMsg = String.format("Error in allowing outgoing to %s", destCidr); - String result = command.execute(); + s_logger.info(String.format("Adding rule if required: " + rule)); + String result = IpTablesHelper.addConditionally(IpTablesHelper.OUTPUT_CHAIN, true, rule, errMsg); if (result != null) { - s_logger.warn("Error in allowing outgoing to " + destCidr + ", err=" + result); - return "Error in allowing outgoing to " + destCidr + ", err=" + result; + return result; } addRouteToInternalIpOrCidr(_localgw, _eth1ip, _eth1mask, destCidr); @@ -2832,13 +2831,8 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S if (result != null) { s_logger.warn("Error in starting sshd service err=" + result); } - command = new Script("/bin/bash", s_logger); - command.add("-c"); - command.add("iptables -I INPUT -i eth1 -p tcp -m state --state NEW -m tcp --dport 3922 -j ACCEPT"); - result = command.execute(); - if (result != null) { - s_logger.warn("Error in opening up ssh port err=" + result); - } + String rule = "-i eth1 -p tcp -m state --state NEW -m tcp --dport 3922 -j ACCEPT"; + IpTablesHelper.addConditionally(IpTablesHelper.INPUT_CHAIN, true, rule, "Error in opening up ssh port"); } private void addRouteToInternalIpOrCidr(String localgw, String eth1ip, String eth1mask, String destIpOrCidr) { diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java index e14977682b6..4d68aff76a2 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java @@ -60,6 +60,7 @@ import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType; import org.apache.cloudstack.storage.command.DownloadProgressCommand; import org.apache.cloudstack.storage.command.DownloadProgressCommand.RequestType; import org.apache.cloudstack.storage.NfsMountManagerImpl.PathParser; +import org.apache.cloudstack.storage.resource.IpTablesHelper; import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource; import org.apache.cloudstack.storage.resource.SecondaryStorageResource; import org.apache.log4j.Logger; @@ -1092,17 +1093,14 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager } private void blockOutgoingOnPrivate() { - Script command = new Script("/bin/bash", LOGGER); - String intf = "eth1"; - command.add("-c"); - command.add("iptables -A OUTPUT -o " + intf + " -p tcp -m state --state NEW -m tcp --dport " + "80" + " -j REJECT;" + "iptables -A OUTPUT -o " + intf + - " -p tcp -m state --state NEW -m tcp --dport " + "443" + " -j REJECT;"); - - String result = command.execute(); - if (result != null) { - LOGGER.warn("Error in blocking outgoing to port 80/443 err=" + result); - return; - } + IpTablesHelper.addConditionally(IpTablesHelper.OUTPUT_CHAIN + , false + , "-o " + TemplateConstants.TMPLT_COPY_INTF_PRIVATE + " -p tcp -m state --state NEW -m tcp --dport 80 -j REJECT;" + , "Error in blocking outgoing to port 80"); + IpTablesHelper.addConditionally(IpTablesHelper.OUTPUT_CHAIN + , false + , "-o " + TemplateConstants.TMPLT_COPY_INTF_PRIVATE + " -p tcp -m state --state NEW -m tcp --dport 443 -j REJECT;" + , "Error in blocking outgoing to port 443"); } @Override @@ -1128,17 +1126,19 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager if (result != null) { LOGGER.warn("Error in stopping httpd service err=" + result); } - String port = Integer.toString(TemplateConstants.DEFAULT_TMPLT_COPY_PORT); - String intf = TemplateConstants.DEFAULT_TMPLT_COPY_INTF; - command = new Script("/bin/bash", LOGGER); - command.add("-c"); - command.add("iptables -I INPUT -i " + intf + " -p tcp -m state --state NEW -m tcp --dport " + port + " -j ACCEPT;" + "iptables -I INPUT -i " + intf + - " -p tcp -m state --state NEW -m tcp --dport " + "443" + " -j ACCEPT;"); - - result = command.execute(); + result = IpTablesHelper.addConditionally(IpTablesHelper.INPUT_CHAIN + , true + , "-i " + TemplateConstants.DEFAULT_TMPLT_COPY_INTF + " -p tcp -m state --state NEW -m tcp --dport " + TemplateConstants.DEFAULT_TMPLT_COPY_PORT + " -j ACCEPT" + , "Error in opening up apache2 port " + TemplateConstants.TMPLT_COPY_INTF_PRIVATE); + if (result != null) { + return; + } + result = IpTablesHelper.addConditionally(IpTablesHelper.INPUT_CHAIN + , true + , "-i " + TemplateConstants.DEFAULT_TMPLT_COPY_INTF + " -p tcp -m state --state NEW -m tcp --dport 443 -j ACCEPT;" + , "Error in opening up apache2 port 443"); if (result != null) { - LOGGER.warn("Error in opening up apache2 port err=" + result); return; } diff --git a/systemvm/debian/opt/cloud/bin/cs/CsHelper.py b/systemvm/debian/opt/cloud/bin/cs/CsHelper.py index 2a83c215403..45f1953dc99 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsHelper.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsHelper.py @@ -221,7 +221,7 @@ def save_iptables(command, iptables_file): def execute2(command, wait=True): """ Execute command """ - logging.info("Executing: %s" % command) + logging.info("Executing2: %s" % command) p = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) if wait: p.wait() diff --git a/utils/src/main/java/com/cloud/utils/script/Script.java b/utils/src/main/java/com/cloud/utils/script/Script.java index fb57f256690..31585534cbd 100644 --- a/utils/src/main/java/com/cloud/utils/script/Script.java +++ b/utils/src/main/java/com/cloud/utils/script/Script.java @@ -243,12 +243,19 @@ public class Script implements Callable { //process completed successfully if (_process.exitValue() == 0) { _logger.debug("Execution is successful."); + String result; + String method; if (interpreter != null) { - return interpreter.drain() ? task.getResult() : interpreter.interpret(ir); + _logger.debug("interpreting the result..."); + method = "result interpretation of execution: "; + result= interpreter.drain() ? task.getResult() : interpreter.interpret(ir); } else { // null return exitValue apparently - return String.valueOf(_process.exitValue()); + method = "return code of execution: "; + result = String.valueOf(_process.exitValue()); } + _logger.debug(method + result); + return result; } else { //process failed break; }