Merge release branch 4.20 to 4.22

* 4.20:
  fixed Password Exposure in IPMI Tool Command Execution (apache#12028)
  server: fix volume offering not updated after offering change (apache#12003)
  fix API Request Parameters Logged Credential Masking in ApiServer (apache#12020)

Author: Daan Hoogland

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

@@ -39,6 +39,7 @@
 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<String> sensitiveFields = new HashSet<>(Arrays.asList(
+        "password", "secretkey", "apikey", "token",
+        "sessionkey", "accesskey", "signature",
+        "authorization", "credential", "secret"
+    ));
+
     private static final ConfigKey<Integer> IntegrationAPIPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED
             , Integer.class
             , "integration.api.port"
@@ -624,10 +631,23 @@ public String handleRequest(final Map params, final String responseType, final S
                 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");

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

@@ -1578,6 +1578,7 @@ private VolumeVO orchestrateResizeVolume(long volumeId, long currentSize, long n
                 }
             }
 
+            volume = _volsDao.findById(volumeId);
             if (newDiskOfferingId != null) {
                 volume.setDiskOfferingId(newDiskOfferingId);
                 _volumeMgr.saveVolumeDetails(newDiskOfferingId, volume.getId());
@@ -1592,7 +1593,6 @@ private VolumeVO orchestrateResizeVolume(long volumeId, long currentSize, long n
             }
 
             // 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) {

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",

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

@@ -67,11 +67,13 @@ String removeCommandSensitiveInfoForLogging(String command) {
     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 ProcessResult executeCommands(final List<String> commands) {
     /**
      * 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 Integer call() throws Exception {
                 }
             });
             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 {

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

@@ -60,4 +60,16 @@ public void testRemoveCommandSensitiveInfoForLoggingIpmi() {
         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 **** ****"));
+    }
 }