diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index c78ac05102f..5a3c8c2c717 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -39,6 +39,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Arrays; import java.util.Map; import java.util.Set; import java.util.TimeZone; @@ -251,6 +252,12 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer @Inject private MessageBus messageBus; + private static final Set sensitiveFields = new HashSet<>(Arrays.asList( + "password", "secretkey", "apikey", "token", + "sessionkey", "accesskey", "signature", + "authorization", "credential", "secret" + )); + private static final ConfigKey IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED , Integer.class , "integration.api.port" @@ -624,10 +631,23 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer logger.error("invalid request, no command sent"); if (logger.isTraceEnabled()) { logger.trace("dumping request parameters"); - for (final Object key : params.keySet()) { - final String keyStr = (String)key; - final String[] value = (String[])params.get(key); - logger.trace(" key: " + keyStr + ", value: " + ((value == null) ? "'null'" : value[0])); + + for (final Object key : params.keySet()) { + final String keyStr = (String) key; + final String[] value = (String[]) params.get(key); + + String lowerKeyStr = keyStr.toLowerCase(); + boolean isSensitive = sensitiveFields.stream() + .anyMatch(lowerKeyStr::contains); + + String logValue; + if (isSensitive) { + logValue = "******"; // mask sensitive values + } else { + logValue = (value == null) ? "'null'" : value[0]; + } + + logger.trace(" key: " + keyStr + ", value: " + logValue); } } throw new ServerApiException(ApiErrorCode.UNSUPPORTED_ACTION_ERROR, "Invalid request, no command sent"); diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index a2b87c84b4c..43c2ffcbad4 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1578,6 +1578,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } + volume = _volsDao.findById(volumeId); if (newDiskOfferingId != null) { volume.setDiskOfferingId(newDiskOfferingId); _volumeMgr.saveVolumeDetails(newDiskOfferingId, volume.getId()); @@ -1592,7 +1593,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } // Update size if volume has same size as before, else it is already updated - volume = _volsDao.findById(volumeId); if (currentSize == volume.getSize() && currentSize != newSize) { volume.setSize(newSize); } else if (volume.getSize() != newSize) { diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 4f450e940fc..ecc24ecadea 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -3121,7 +3121,7 @@ "message.change.offering.confirm": "Please confirm that you wish to change the service offering of this virtual Instance.", "message.change.offering.for.volume": "Successfully changed offering for the volume", "message.change.offering.for.volume.failed": "Change offering for the volume failed", -"message.change.offering.processing": "Changing offering for the volume...", +"message.change.offering.for.volume.processing": "Changing offering for the volume...", "message.change.password": "Please change your password.", "message.change.scope.failed": "Scope change failed", "message.change.scope.processing": "Scope change in progress", 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 430fa56aa68..e2d3be05772 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 @@ -67,11 +67,13 @@ public final class ProcessRunner { public ProcessRunner(ExecutorService executor) { this.executor = executor; commandLogReplacements.add(new Ternary<>("ipmitool", "-P\\s+\\S+", "-P *****")); + commandLogReplacements.add(new Ternary<>("ipmitool", "(?i)password\\s+\\S+\\s+\\S+", "password **** ****")); } /** * Executes a process with provided list of commands with a max default timeout * of 5 minutes + * * @param commands list of string commands * @return returns process result */ @@ -82,6 +84,7 @@ public final class ProcessRunner { /** * Executes a process with provided list of commands with a given timeout that is less * than or equal to DEFAULT_MAX_TIMEOUT + * * @param commands list of string commands * @param timeOut timeout duration * @return returns process result @@ -109,14 +112,16 @@ public final class ProcessRunner { } }); try { - logger.debug("Waiting for a response from command [{}]. Defined timeout: [{}].", commandLog, timeOut.getStandardSeconds()); + logger.debug("Waiting for a response from command [{}]. Defined timeout: [{}].", commandLog, + timeOut.getStandardSeconds()); retVal = processFuture.get(timeOut.getStandardSeconds(), TimeUnit.SECONDS); } catch (ExecutionException e) { - logger.warn("Failed to complete the requested command [{}] due to execution error.", commands, e); + logger.warn("Failed to complete the requested command [{}] due to execution error.", commandLog, e); retVal = -2; stdError = e.getMessage(); } catch (TimeoutException e) { - logger.warn("Failed to complete the requested command [{}] within timeout. Defined timeout: [{}].", commandLog, timeOut.getStandardSeconds(), e); + logger.warn("Failed to complete the requested command [{}] within timeout. Defined timeout: [{}].", + commandLog, timeOut.getStandardSeconds(), e); retVal = -1; stdError = "Operation timed out, aborted."; } finally { 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 index 6fc34ded259..0e594f2b0c9 100644 --- a/utils/src/test/java/org/apache/cloudstack/utils/process/ProcessRunnerTest.java +++ b/utils/src/test/java/org/apache/cloudstack/utils/process/ProcessRunnerTest.java @@ -60,4 +60,16 @@ public class ProcessRunnerTest { Assert.assertTrue(log.contains(password)); Assert.assertEquals(1, countSubstringOccurrences(log, password)); } + + @Test + public void testRemoveCommandSensitiveInfoForLoggingIpmiPasswordCommand() { + String userId = "3"; + String newPassword = "Sup3rSecr3t!"; + String command = String.format("/usr/bin/ipmitool user set password %s %s", userId, newPassword); + String log = processRunner.removeCommandSensitiveInfoForLogging(command); + + Assert.assertFalse(log.contains(userId)); + Assert.assertFalse(log.contains(newPassword)); + Assert.assertTrue(log.contains("password **** ****")); + } }