Skip to content

Commit d77998c

Browse files
authored
utils,ui: obfuscate sensitive log info, use POST for configureOutOfBandManagement (#9126)
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent 87b55af commit d77998c

3 files changed

Lines changed: 100 additions & 15 deletions

File tree

ui/src/config/section/infra/hosts.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ export default {
150150
message: 'label.outofbandmanagement.configure',
151151
docHelp: 'adminguide/hosts.html#out-of-band-management',
152152
dataView: true,
153+
post: true,
153154
args: ['hostid', 'address', 'port', 'username', 'password', 'driver'],
154155
mapping: {
155156
hostid: {

utils/src/main/java/org/apache/cloudstack/utils/process/ProcessRunner.java

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,31 +19,53 @@
1919

2020
package org.apache.cloudstack.utils.process;
2121

22-
import com.google.common.base.Preconditions;
23-
import com.google.common.io.CharStreams;
24-
import org.apache.log4j.Logger;
25-
import org.joda.time.Duration;
26-
2722
import java.io.IOException;
2823
import java.io.InputStreamReader;
24+
import java.util.ArrayList;
2925
import java.util.List;
3026
import java.util.concurrent.Callable;
3127
import java.util.concurrent.ExecutionException;
3228
import java.util.concurrent.ExecutorService;
3329
import java.util.concurrent.Future;
3430
import java.util.concurrent.TimeUnit;
3531
import java.util.concurrent.TimeoutException;
32+
import java.util.regex.Matcher;
33+
import java.util.regex.Pattern;
34+
3635
import org.apache.commons.lang3.StringUtils;
36+
import org.apache.log4j.Logger;
37+
import org.joda.time.Duration;
38+
39+
import com.cloud.utils.Ternary;
40+
import com.google.common.base.Preconditions;
41+
import com.google.common.io.CharStreams;
3742

3843
public final class ProcessRunner {
3944
public static final Logger LOG = Logger.getLogger(ProcessRunner.class);
4045

4146
// Default maximum timeout of 5 minutes for any command
4247
public static final Duration DEFAULT_MAX_TIMEOUT = new Duration(5 * 60 * 1000);
4348
private final ExecutorService executor;
49+
private final List<Ternary<String, String, String>> commandLogReplacements = new ArrayList<>();
50+
51+
String removeCommandSensitiveInfoForLogging(String command) {
52+
String commandLog = command.trim();
53+
54+
for (Ternary<String, String, String> replacement : commandLogReplacements) {
55+
if (commandLog.contains(replacement.first())) {
56+
Pattern pattern = Pattern.compile(replacement.second());
57+
Matcher matcher = pattern.matcher(commandLog);
58+
if (matcher.find()) {
59+
commandLog = matcher.replaceAll(replacement.third());
60+
}
61+
}
62+
}
63+
return commandLog;
64+
}
4465

4566
public ProcessRunner(ExecutorService executor) {
4667
this.executor = executor;
68+
commandLogReplacements.add(new Ternary<>("ipmitool", "-P\\s+\\S+", "-P *****"));
4769
}
4870

4971
/**
@@ -72,29 +94,28 @@ public ProcessResult executeCommands(final List<String> commands, final Duration
7294
int retVal = -2;
7395
String stdOutput = null;
7496
String stdError = null;
75-
76-
String oneLineCommand = StringUtils.join(commands, " ");
97+
String commandLog = removeCommandSensitiveInfoForLogging(StringUtils.join(commands, " "));
7798

7899
try {
79-
LOG.debug(String.format("Preparing command [%s] to execute.", oneLineCommand));
100+
LOG.debug(String.format("Preparing command [%s] to execute.", commandLog));
80101
final Process process = new ProcessBuilder().command(commands).start();
81102

82-
LOG.debug(String.format("Submitting command [%s].", oneLineCommand));
103+
LOG.debug(String.format("Submitting command [%s].", commandLog));
83104
final Future<Integer> processFuture = executor.submit(new Callable<Integer>() {
84105
@Override
85106
public Integer call() throws Exception {
86107
return process.waitFor();
87108
}
88109
});
89110
try {
90-
LOG.debug(String.format("Waiting for a response from command [%s]. Defined timeout: [%s].", oneLineCommand, timeOut.getStandardSeconds()));
111+
LOG.debug(String.format("Waiting for a response from command [%s]. Defined timeout: [%s].", commandLog, timeOut.getStandardSeconds()));
91112
retVal = processFuture.get(timeOut.getStandardSeconds(), TimeUnit.SECONDS);
92113
} catch (ExecutionException e) {
93-
LOG.warn(String.format("Failed to complete the requested command [%s] due to execution error.", oneLineCommand), e);
114+
LOG.warn(String.format("Failed to complete the requested command [%s] due to execution error.", commands), e);
94115
retVal = -2;
95116
stdError = e.getMessage();
96117
} catch (TimeoutException e) {
97-
LOG.warn(String.format("Failed to complete the requested command [%s] within timeout. Defined timeout: [%s].", oneLineCommand, timeOut.getStandardSeconds()), e);
118+
LOG.warn(String.format("Failed to complete the requested command [%s] within timeout. Defined timeout: [%s].", commandLog, timeOut.getStandardSeconds()), e);
98119
retVal = -1;
99120
stdError = "Operation timed out, aborted.";
100121
} finally {
@@ -105,10 +126,10 @@ public Integer call() throws Exception {
105126
process.destroy();
106127
}
107128

108-
LOG.debug(String.format("Process standard output for command [%s]: [%s].", oneLineCommand, stdOutput));
109-
LOG.debug(String.format("Process standard error output command [%s]: [%s].", oneLineCommand, stdError));
129+
LOG.debug(String.format("Process standard output for command [%s]: [%s].", commandLog, stdOutput));
130+
LOG.debug(String.format("Process standard error output command [%s]: [%s].", commandLog, stdError));
110131
} catch (IOException | InterruptedException e) {
111-
LOG.error(String.format("Exception caught error running command [%s].", oneLineCommand), e);
132+
LOG.error(String.format("Exception caught error running command [%s].", commandLog), e);
112133
stdError = e.getMessage();
113134
}
114135
return new ProcessResult(stdOutput, stdError, retVal);
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.cloudstack.utils.process;
19+
20+
import java.util.concurrent.ExecutorService;
21+
22+
import org.junit.Assert;
23+
import org.junit.Test;
24+
import org.junit.runner.RunWith;
25+
import org.mockito.InjectMocks;
26+
import org.mockito.Mockito;
27+
import org.mockito.junit.MockitoJUnitRunner;
28+
29+
@RunWith(MockitoJUnitRunner.class)
30+
public class ProcessRunnerTest {
31+
32+
@InjectMocks
33+
ProcessRunner processRunner = new ProcessRunner(Mockito.mock(ExecutorService.class));
34+
35+
private int countSubstringOccurrences(String mainString, String subString) {
36+
int count = 0;
37+
int index = 0;
38+
while ((index = mainString.indexOf(subString, index)) != -1) {
39+
count++;
40+
index += subString.length();
41+
}
42+
return count;
43+
}
44+
45+
@Test
46+
public void testRemoveCommandSensitiveInfoForLoggingIpmi() {
47+
String password = "R@ndomP@ss";
48+
String command = String.format("/usr/bin/ipmitool -H host -p 1234 -U Admin " +
49+
"-P %s chassis power status", password);
50+
String log = processRunner.removeCommandSensitiveInfoForLogging(command);
51+
Assert.assertFalse(log.contains(password));
52+
53+
String command1 = String.format("%s -D %s", command, password);
54+
log = processRunner.removeCommandSensitiveInfoForLogging(command1);
55+
Assert.assertTrue(log.contains(password));
56+
Assert.assertEquals(1, countSubstringOccurrences(log, password));
57+
58+
String command2 = command.replace("ipmitool", "impit00l");
59+
log = processRunner.removeCommandSensitiveInfoForLogging(command2);
60+
Assert.assertTrue(log.contains(password));
61+
Assert.assertEquals(1, countSubstringOccurrences(log, password));
62+
}
63+
}

0 commit comments

Comments
 (0)