Skip to content

Commit e23c7ef

Browse files
author
Daan Hoogland
committed
Merge release branch 4.20 to 4.22
* 4.20: fixed Password Exposure in IPMI Tool Command Execution (#12028) server: fix volume offering not updated after offering change (#12003) fix API Request Parameters Logged Credential Masking in ApiServer (#12020)
2 parents e33f475 + 028dd86 commit e23c7ef

File tree

5 files changed

+46
-9
lines changed

5 files changed

+46
-9
lines changed

server/src/main/java/com/cloud/api/ApiServer.java

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.HashSet;
4040
import java.util.Iterator;
4141
import java.util.List;
42+
import java.util.Arrays;
4243
import java.util.Map;
4344
import java.util.Set;
4445
import java.util.TimeZone;
@@ -251,6 +252,12 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
251252
@Inject
252253
private MessageBus messageBus;
253254

255+
private static final Set<String> sensitiveFields = new HashSet<>(Arrays.asList(
256+
"password", "secretkey", "apikey", "token",
257+
"sessionkey", "accesskey", "signature",
258+
"authorization", "credential", "secret"
259+
));
260+
254261
private static final ConfigKey<Integer> IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED
255262
, Integer.class
256263
, "integration.api.port"
@@ -624,10 +631,23 @@ public String handleRequest(final Map params, final String responseType, final S
624631
logger.error("invalid request, no command sent");
625632
if (logger.isTraceEnabled()) {
626633
logger.trace("dumping request parameters");
627-
for (final Object key : params.keySet()) {
628-
final String keyStr = (String)key;
629-
final String[] value = (String[])params.get(key);
630-
logger.trace(" key: " + keyStr + ", value: " + ((value == null) ? "'null'" : value[0]));
634+
635+
for (final Object key : params.keySet()) {
636+
final String keyStr = (String) key;
637+
final String[] value = (String[]) params.get(key);
638+
639+
String lowerKeyStr = keyStr.toLowerCase();
640+
boolean isSensitive = sensitiveFields.stream()
641+
.anyMatch(lowerKeyStr::contains);
642+
643+
String logValue;
644+
if (isSensitive) {
645+
logValue = "******"; // mask sensitive values
646+
} else {
647+
logValue = (value == null) ? "'null'" : value[0];
648+
}
649+
650+
logger.trace(" key: " + keyStr + ", value: " + logValue);
631651
}
632652
}
633653
throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "Invalid request, no command sent");

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1578,6 +1578,7 @@ private VolumeVO orchestrateResizeVolume(long volumeId, long currentSize, long n
15781578
}
15791579
}
15801580

1581+
volume = _volsDao.findById(volumeId);
15811582
if (newDiskOfferingId != null) {
15821583
volume.setDiskOfferingId(newDiskOfferingId);
15831584
_volumeMgr.saveVolumeDetails(newDiskOfferingId, volume.getId());
@@ -1592,7 +1593,6 @@ private VolumeVO orchestrateResizeVolume(long volumeId, long currentSize, long n
15921593
}
15931594

15941595
// Update size if volume has same size as before, else it is already updated
1595-
volume = _volsDao.findById(volumeId);
15961596
if (currentSize == volume.getSize() && currentSize != newSize) {
15971597
volume.setSize(newSize);
15981598
} else if (volume.getSize() != newSize) {

ui/public/locales/en.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3121,7 +3121,7 @@
31213121
"message.change.offering.confirm": "Please confirm that you wish to change the service offering of this virtual Instance.",
31223122
"message.change.offering.for.volume": "Successfully changed offering for the volume",
31233123
"message.change.offering.for.volume.failed": "Change offering for the volume failed",
3124-
"message.change.offering.processing": "Changing offering for the volume...",
3124+
"message.change.offering.for.volume.processing": "Changing offering for the volume...",
31253125
"message.change.password": "Please change your password.",
31263126
"message.change.scope.failed": "Scope change failed",
31273127
"message.change.scope.processing": "Scope change in progress",

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

Lines changed: 8 additions & 3 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+", "password **** ****"));
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
*/
@@ -82,6 +84,7 @@ public ProcessResult executeCommands(final List<String> commands) {
8284
/**
8385
* Executes a process with provided list of commands with a given timeout that is less
8486
* than or equal to DEFAULT_MAX_TIMEOUT
87+
*
8588
* @param commands list of string commands
8689
* @param timeOut timeout duration
8790
* @return returns process result
@@ -109,14 +112,16 @@ public Integer call() throws Exception {
109112
}
110113
});
111114
try {
112-
logger.debug("Waiting for a response from command [{}]. Defined timeout: [{}].", commandLog, timeOut.getStandardSeconds());
115+
logger.debug("Waiting for a response from command [{}]. Defined timeout: [{}].", commandLog,
116+
timeOut.getStandardSeconds());
113117
retVal = processFuture.get(timeOut.getStandardSeconds(), TimeUnit.SECONDS);
114118
} catch (ExecutionException e) {
115-
logger.warn("Failed to complete the requested command [{}] due to execution error.", commands, e);
119+
logger.warn("Failed to complete the requested command [{}] due to execution error.", commandLog, e);
116120
retVal = -2;
117121
stdError = e.getMessage();
118122
} catch (TimeoutException e) {
119-
logger.warn("Failed to complete the requested command [{}] within timeout. Defined timeout: [{}].", commandLog, timeOut.getStandardSeconds(), e);
123+
logger.warn("Failed to complete the requested command [{}] within timeout. Defined timeout: [{}].",
124+
commandLog, timeOut.getStandardSeconds(), e);
120125
retVal = -1;
121126
stdError = "Operation timed out, aborted.";
122127
} 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)