Skip to content

Commit 8dde0b1

Browse files
fixed Password Exposure in IPMI Tool Command Execution
1 parent e90e436 commit 8dde0b1

File tree

2 files changed

+23
-5
lines changed

2 files changed

+23
-5
lines changed

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,13 @@ String removeCommandSensitiveInfoForLogging(String command) {
6767
public ProcessRunner(ExecutorService executor) {
6868
this.executor = executor;
6969
commandLogReplacements.add(new Ternary<>("ipmitool", "-P\\s+\\S+", "-P *****"));
70+
commandLogReplacements.add(new Ternary<>("ipmitool", "(?i)(password)(\\s+)\\S+(\\s+)\\S+", "$1****$2****"));
7071
}
7172

7273
/**
7374
* Executes a process with provided list of commands with a max default timeout
7475
* of 5 minutes
76+
*
7577
* @param commands list of string commands
7678
* @return returns process result
7779
*/
@@ -80,10 +82,12 @@ public ProcessResult executeCommands(final List<String> commands) {
8082
}
8183

8284
/**
83-
* Executes a process with provided list of commands with a given timeout that is less
85+
* Executes a process with provided list of commands with a given timeout that
86+
* is less
8487
* than or equal to DEFAULT_MAX_TIMEOUT
88+
*
8589
* @param commands list of string commands
86-
* @param timeOut timeout duration
90+
* @param timeOut timeout duration
8791
* @return returns process result
8892
*/
8993
public ProcessResult executeCommands(final List<String> commands, final Duration timeOut) {
@@ -109,14 +113,16 @@ public Integer call() throws Exception {
109113
}
110114
});
111115
try {
112-
logger.debug("Waiting for a response from command [{}]. Defined timeout: [{}].", commandLog, timeOut.getStandardSeconds());
116+
logger.debug("Waiting for a response from command [{}]. Defined timeout: [{}].", commandLog,
117+
timeOut.getStandardSeconds());
113118
retVal = processFuture.get(timeOut.getStandardSeconds(), TimeUnit.SECONDS);
114119
} catch (ExecutionException e) {
115-
logger.warn("Failed to complete the requested command [{}] due to execution error.", commands, e);
120+
logger.warn("Failed to complete the requested command [{}] due to execution error.", commandLog, e);
116121
retVal = -2;
117122
stdError = e.getMessage();
118123
} catch (TimeoutException e) {
119-
logger.warn("Failed to complete the requested command [{}] within timeout. Defined timeout: [{}].", commandLog, timeOut.getStandardSeconds(), e);
124+
logger.warn("Failed to complete the requested command [{}] within timeout. Defined timeout: [{}].",
125+
commandLog, timeOut.getStandardSeconds(), e);
120126
retVal = -1;
121127
stdError = "Operation timed out, aborted.";
122128
} finally {

utils/src/test/java/org/apache/cloudstack/utils/process/ProcessRunnerTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,16 @@ public void testRemoveCommandSensitiveInfoForLoggingIpmi() {
6060
Assert.assertTrue(log.contains(password));
6161
Assert.assertEquals(1, countSubstringOccurrences(log, password));
6262
}
63+
64+
@Test
65+
public void testRemoveCommandSensitiveInfoForLoggingIpmiPasswordCommand() {
66+
String userId = "3";
67+
String newPassword = "Sup3rSecr3t!";
68+
String command = String.format("/usr/bin/ipmitool user set password %s %s", userId, newPassword);
69+
String log = processRunner.removeCommandSensitiveInfoForLogging(command);
70+
71+
Assert.assertFalse(log.contains(userId));
72+
Assert.assertFalse(log.contains(newPassword));
73+
Assert.assertTrue(log.contains("password **** ****"));
74+
}
6375
}

0 commit comments

Comments
 (0)