This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new d77998c2d44 utils,ui: obfuscate sensitive log info, use POST for
configureOutOfBandManagement (#9126)
d77998c2d44 is described below
commit d77998c2d44ac14f82cf52d54a3ba04996d63169
Author: Abhishek Kumar <[email protected]>
AuthorDate: Mon Jun 3 15:27:44 2024 +0530
utils,ui: obfuscate sensitive log info, use POST for
configureOutOfBandManagement (#9126)
Signed-off-by: Abhishek Kumar <[email protected]>
---
ui/src/config/section/infra/hosts.js | 1 +
.../cloudstack/utils/process/ProcessRunner.java | 51 ++++++++++++------
.../utils/process/ProcessRunnerTest.java | 63 ++++++++++++++++++++++
3 files changed, 100 insertions(+), 15 deletions(-)
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<Ternary<String, String, String>> commandLogReplacements
= new ArrayList<>();
+
+ String removeCommandSensitiveInfoForLogging(String command) {
+ String commandLog = command.trim();
+
+ for (Ternary<String, String, String> 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<Integer> processFuture = executor.submit(new
Callable<Integer>() {
@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));
+ }
+}
