diff --git a/ui/src/config/section/infra/hosts.js b/ui/src/config/section/infra/hosts.js index 329b77fe2d7..81ca0c917b2 100644 --- a/ui/src/config/section/infra/hosts.js +++ b/ui/src/config/section/infra/hosts.js @@ -150,6 +150,7 @@ export default { message: 'label.outofbandmanagement.configure', docHelp: 'adminguide/hosts.html#out-of-band-management', dataView: true, + post: true, args: ['hostid', 'address', 'port', 'username', 'password', 'driver'], mapping: { hostid: { diff --git a/utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java b/utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java index b8b2555f4c9..6ca02b91263 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java @@ -19,13 +19,9 @@ package org.apache.cloudstack.utils.process; -import com.google.common.base.Preconditions; -import com.google.common.io.CharStreams; -import org.apache.log4j.Logger; -import org.joda.time.Duration; - import java.io.IOException; import java.io.InputStreamReader; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; @@ -33,7 +29,16 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.commons.lang3.StringUtils; +import org.apache.log4j.Logger; +import org.joda.time.Duration; + +import com.cloud.utils.Ternary; +import com.google.common.base.Preconditions; +import com.google.common.io.CharStreams; public final class ProcessRunner { public static final Logger LOG = Logger.getLogger(ProcessRunner.class); @@ -41,9 +46,26 @@ public final class ProcessRunner { // Default maximum timeout of 5 minutes for any command public static final Duration DEFAULT_MAX_TIMEOUT = new Duration(5 * 60 * 1000); private final ExecutorService executor; + private final List> commandLogReplacements = new ArrayList<>(); + + String removeCommandSensitiveInfoForLogging(String command) { + String commandLog = command.trim(); + + for (Ternary replacement : commandLogReplacements) { + if (commandLog.contains(replacement.first())) { + Pattern pattern = Pattern.compile(replacement.second()); + Matcher matcher = pattern.matcher(commandLog); + if (matcher.find()) { + commandLog = matcher.replaceAll(replacement.third()); + } + } + } + return commandLog; + } public ProcessRunner(ExecutorService executor) { this.executor = executor; + commandLogReplacements.add(new Ternary<>("ipmitool", "-P\\s+\\S+", "-P *****")); } /** @@ -72,14 +94,13 @@ public final class ProcessRunner { int retVal = -2; String stdOutput = null; String stdError = null; - - String oneLineCommand = StringUtils.join(commands, " "); + String commandLog = removeCommandSensitiveInfoForLogging(StringUtils.join(commands, " ")); try { - LOG.debug(String.format("Preparing command [%s] to execute.", oneLineCommand)); + LOG.debug(String.format("Preparing command [%s] to execute.", commandLog)); final Process process = new ProcessBuilder().command(commands).start(); - LOG.debug(String.format("Submitting command [%s].", oneLineCommand)); + LOG.debug(String.format("Submitting command [%s].", commandLog)); final Future processFuture = executor.submit(new Callable() { @Override public Integer call() throws Exception { @@ -87,14 +108,14 @@ public final class ProcessRunner { } }); try { - LOG.debug(String.format("Waiting for a response from command [%s]. Defined timeout: [%s].", oneLineCommand, timeOut.getStandardSeconds())); + LOG.debug(String.format("Waiting for a response from command [%s]. Defined timeout: [%s].", commandLog, timeOut.getStandardSeconds())); retVal = processFuture.get(timeOut.getStandardSeconds(), TimeUnit.SECONDS); } catch (ExecutionException e) { - LOG.warn(String.format("Failed to complete the requested command [%s] due to execution error.", oneLineCommand), e); + LOG.warn(String.format("Failed to complete the requested command [%s] due to execution error.", commands), e); retVal = -2; stdError = e.getMessage(); } catch (TimeoutException e) { - LOG.warn(String.format("Failed to complete the requested command [%s] within timeout. Defined timeout: [%s].", oneLineCommand, timeOut.getStandardSeconds()), e); + LOG.warn(String.format("Failed to complete the requested command [%s] within timeout. Defined timeout: [%s].", commandLog, timeOut.getStandardSeconds()), e); retVal = -1; stdError = "Operation timed out, aborted."; } finally { @@ -105,10 +126,10 @@ public final class ProcessRunner { process.destroy(); } - LOG.debug(String.format("Process standard output for command [%s]: [%s].", oneLineCommand, stdOutput)); - LOG.debug(String.format("Process standard error output command [%s]: [%s].", oneLineCommand, stdError)); + LOG.debug(String.format("Process standard output for command [%s]: [%s].", commandLog, stdOutput)); + LOG.debug(String.format("Process standard error output command [%s]: [%s].", commandLog, stdError)); } catch (IOException | InterruptedException e) { - LOG.error(String.format("Exception caught error running command [%s].", oneLineCommand), e); + LOG.error(String.format("Exception caught error running command [%s].", commandLog), e); stdError = e.getMessage(); } return new ProcessResult(stdOutput, stdError, retVal); diff --git a/utils/src/test/java/org/apache/cloudstack/utils/process/ProcessRunnerTest.java b/utils/src/test/java/org/apache/cloudstack/utils/process/ProcessRunnerTest.java new file mode 100644 index 00000000000..6fc34ded259 --- /dev/null +++ b/utils/src/test/java/org/apache/cloudstack/utils/process/ProcessRunnerTest.java @@ -0,0 +1,63 @@ +// 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.utils.process; + +import java.util.concurrent.ExecutorService; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class ProcessRunnerTest { + + @InjectMocks + ProcessRunner processRunner = new ProcessRunner(Mockito.mock(ExecutorService.class)); + + private int countSubstringOccurrences(String mainString, String subString) { + int count = 0; + int index = 0; + while ((index = mainString.indexOf(subString, index)) != -1) { + count++; + index += subString.length(); + } + return count; + } + + @Test + public void testRemoveCommandSensitiveInfoForLoggingIpmi() { + String password = "R@ndomP@ss"; + String command = String.format("/usr/bin/ipmitool -H host -p 1234 -U Admin " + + "-P %s chassis power status", password); + String log = processRunner.removeCommandSensitiveInfoForLogging(command); + Assert.assertFalse(log.contains(password)); + + String command1 = String.format("%s -D %s", command, password); + log = processRunner.removeCommandSensitiveInfoForLogging(command1); + Assert.assertTrue(log.contains(password)); + Assert.assertEquals(1, countSubstringOccurrences(log, password)); + + String command2 = command.replace("ipmitool", "impit00l"); + log = processRunner.removeCommandSensitiveInfoForLogging(command2); + Assert.assertTrue(log.contains(password)); + Assert.assertEquals(1, countSubstringOccurrences(log, password)); + } +}