This is an automated email from the ASF dual-hosted git repository.
bstoyanov pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 35e6d7c5ba8 fix that log sensitive infomation in cmd of script (#12024)
35e6d7c5ba8 is described below
commit 35e6d7c5ba8f921c7611ab6c0c84d7b3db557456
Author: Edward-x <[email protected]>
AuthorDate: Wed Jan 28 19:16:59 2026 +0800
fix that log sensitive infomation in cmd of script (#12024)
* fix that log sensitive infomation in cmd of script
* Remove unnecessary line break in Script.java
* Update utils/src/main/java/com/cloud/utils/script/Script.java
Co-authored-by: Copilot <[email protected]>
* Refactor logging in Script class to simplify handling of sensitive
arguments
* Improve command logging in Script class to include full command line when
debugging
* Remove unused _passwordCommand flag from Script class to simplify code
* Update utils/src/main/java/com/cloud/utils/script/Script.java
Co-authored-by: Copilot <[email protected]>
* Remove unused import for KeyStoreUtils
* Update utils/src/main/java/com/cloud/utils/script/Script.java
---------
Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: dahn <[email protected]>
Co-authored-by: dahn <[email protected]>
---
.../LibvirtUpdateHostPasswordCommandWrapper.java | 3 +-
.../CitrixUpdateHostPasswordCommandWrapper.java | 2 +-
.../main/java/com/cloud/utils/script/Script.java | 99 +++++++++++++---------
.../java/com/cloud/utils/script/ScriptTest.java | 30 +++++++
4 files changed, 93 insertions(+), 41 deletions(-)
diff --git
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUpdateHostPasswordCommandWrapper.java
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUpdateHostPasswordCommandWrapper.java
index b8fe0ded716..80c723b5a6e 100644
---
a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUpdateHostPasswordCommandWrapper.java
+++
b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUpdateHostPasswordCommandWrapper.java
@@ -37,7 +37,8 @@ public final class LibvirtUpdateHostPasswordCommandWrapper
extends CommandWrappe
final String newPassword = command.getNewPassword();
final Script script =
libvirtUtilitiesHelper.buildScript(libvirtComputingResource.getUpdateHostPasswdPath());
- script.add(username, newPassword);
+ script.add(username);
+ script.addSensitive(newPassword);
final String result = script.execute();
if (result != null) {
diff --git
a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUpdateHostPasswordCommandWrapper.java
b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUpdateHostPasswordCommandWrapper.java
index af868d8c1c7..e3ee0ca13ca 100644
---
a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUpdateHostPasswordCommandWrapper.java
+++
b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUpdateHostPasswordCommandWrapper.java
@@ -47,7 +47,7 @@ public final class CitrixUpdateHostPasswordCommandWrapper
extends CommandWrapper
try {
logger.debug("Executing password update command on host: {} for
user: {}", hostIp, username);
final String hostPassword = citrixResourceBase.getPwdFromQueue();
- result = xenServerUtilitiesHelper.executeSshWrapper(hostIp, 22,
username, null, hostPassword, cmdLine.toString());
+ result = xenServerUtilitiesHelper.executeSshWrapper(hostIp, 22,
username, null, hostPassword, cmdLine);
} catch (final Exception e) {
return new Answer(command, false, e.getMessage());
}
diff --git a/utils/src/main/java/com/cloud/utils/script/Script.java
b/utils/src/main/java/com/cloud/utils/script/Script.java
index ffda782edda..09c58dce9a8 100644
--- a/utils/src/main/java/com/cloud/utils/script/Script.java
+++ b/utils/src/main/java/com/cloud/utils/script/Script.java
@@ -30,8 +30,10 @@ import java.net.URISyntaxException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
@@ -43,7 +45,6 @@ import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
-import org.apache.cloudstack.utils.security.KeyStoreUtils;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.io.IOUtils;
import org.apache.logging.log4j.LogManager;
@@ -66,8 +67,8 @@ public class Script implements Callable<String> {
private static final int DEFAULT_TIMEOUT = 3600 * 1000; /* 1 hour */
private volatile boolean _isTimeOut = false;
- private boolean _passwordCommand = false;
private boolean avoidLoggingCommand = false;
+ private final Set<Integer> sensitiveArgIndices = new HashSet<>();
private static final ScheduledExecutorService s_executors =
Executors.newScheduledThreadPool(10, new NamedThreadFactory("Script"));
@@ -145,6 +146,11 @@ public class Script implements Callable<String> {
_command.add(param);
}
+ public void addSensitive(String param) {
+ _command.add(param);
+ sensitiveArgIndices.add(_command.size() - 1);
+ }
+
public Script set(String name, String value) {
_command.add(name);
_command.add(value);
@@ -163,7 +169,7 @@ public class Script implements Callable<String> {
if (sanitizeViCmdParameter(cmd, builder) ||
sanitizeRbdFileFormatCmdParameter(cmd, builder)) {
continue;
}
- if (obscureParam) {
+ if (obscureParam || sensitiveArgIndices.contains(i)) {
builder.append("******").append(" ");
obscureParam = false;
} else {
@@ -172,7 +178,6 @@ public class Script implements Callable<String> {
if ("-y".equals(cmd) || "-z".equals(cmd)) {
obscureParam = true;
- _passwordCommand = true;
}
}
return builder.toString();
@@ -240,8 +245,8 @@ public class Script implements Callable<String> {
public String execute(OutputInterpreter interpreter) {
String[] command = _command.toArray(new String[_command.size()]);
String commandLine = buildCommandLine(command);
- if (_logger.isDebugEnabled() && !avoidLoggingCommand) {
- _logger.debug(String.format("Executing command [%s].",
commandLine.split(KeyStoreUtils.KS_FILENAME)[0]));
+ if (_logger.isDebugEnabled() ) {
+ _logger.debug(String.format("Executing command [%s].",
commandLine));
}
try {
@@ -263,48 +268,62 @@ public class Script implements Callable<String> {
_thread = Thread.currentThread();
ScheduledFuture<String> future = null;
if (_timeout > 0) {
- _logger.trace(String.format("Scheduling the execution of
command [%s] with a timeout of [%s] milliseconds.", commandLine, _timeout));
+ _logger.trace(String.format(
+ "Scheduling the execution of command [%s] with a
timeout of [%s] milliseconds.",
+ commandLine, _timeout));
future = s_executors.schedule(this, _timeout,
TimeUnit.MILLISECONDS);
}
long processPid = _process.pid();
Task task = null;
if (interpreter != null && interpreter.drain()) {
- _logger.trace(String.format("Executing interpreting task of
process [%s] for command [%s].", processPid, commandLine));
+ _logger.trace(String.format("Executing interpreting task of
process [%s] for command [%s].",
+ processPid, commandLine));
task = new Task(interpreter, ir);
s_executors.execute(task);
}
while (true) {
- _logger.trace(String.format("Attempting process [%s] execution
for command [%s] with timeout [%s].", processPid, commandLine, _timeout));
+ _logger.trace(String.format("Attempting process [%s] execution
for command [%s] with timeout [%s].",
+ processPid, commandLine, _timeout));
try {
if (_process.waitFor(_timeout, TimeUnit.MILLISECONDS)) {
- _logger.trace(String.format("Process [%s] execution
for command [%s] completed within timeout period [%s].", processPid,
commandLine,
+ _logger.trace(String.format(
+ "Process [%s] execution for command [%s]
completed within timeout period [%s].",
+ processPid, commandLine,
_timeout));
if (_process.exitValue() == 0) {
- _logger.debug(String.format("Successfully executed
process [%s] for command [%s].", processPid, commandLine));
+ _logger.debug(String.format("Successfully executed
process [%s] for command [%s].",
+ processPid, commandLine));
if (interpreter != null) {
if (interpreter.drain()) {
- _logger.trace(String.format("Returning
task result of process [%s] for command [%s].", processPid, commandLine));
+ _logger.trace(
+ String.format("Returning task
result of process [%s] for command [%s].",
+ processPid, commandLine));
return task.getResult();
}
- _logger.trace(String.format("Returning
interpretation of process [%s] for command [%s].", processPid, commandLine));
+ _logger.trace(
+ String.format("Returning
interpretation of process [%s] for command [%s].",
+ processPid, commandLine));
return interpreter.interpret(ir);
} else {
// null return exitValue apparently
- _logger.trace(String.format("Process [%s] for
command [%s] exited with value [%s].", processPid, commandLine,
+ _logger.trace(String.format("Process [%s] for
command [%s] exited with value [%s].",
+ processPid, commandLine,
_process.exitValue()));
return String.valueOf(_process.exitValue());
}
} else {
- _logger.warn(String.format("Execution of process
[%s] for command [%s] failed.", processPid, commandLine));
+ _logger.warn(String.format("Execution of process
[%s] for command [%s] failed.",
+ processPid, commandLine));
break;
}
}
} catch (InterruptedException e) {
if (!_isTimeOut) {
- _logger.debug(String.format("Exception [%s] occurred;
however, it was not a timeout. Therefore, proceeding with the execution of
process [%s] for command "
- + "[%s].", e.getMessage(), processPid,
commandLine), e);
+ _logger.debug(String.format(
+ "Exception [%s] occurred; however, it was not
a timeout. Therefore, proceeding with the execution of process [%s] for command
[%s].",
+ e.getMessage(), processPid, commandLine), e);
continue;
}
} finally {
@@ -317,18 +336,17 @@ public class Script implements Callable<String> {
TimedOutLogger log = new TimedOutLogger(_process);
Task timedoutTask = new Task(log, ir);
- _logger.trace(String.format("Running timed out task of process
[%s] for command [%s].", processPid, commandLine));
+ _logger.trace(String.format("Running timed out task of process
[%s] for command [%s].", processPid,
+ commandLine));
timedoutTask.run();
- if (!_passwordCommand) {
- _logger.warn(String.format("Process [%s] for command [%s]
timed out. Output is [%s].", processPid, commandLine,
timedoutTask.getResult()));
- } else {
- _logger.warn(String.format("Process [%s] for command [%s]
timed out.", processPid, commandLine));
- }
+ _logger.warn(String.format("Process [%s] for command [%s]
timed out. Output is [%s].", processPid,
+ commandLine, timedoutTask.getResult()));
return ERR_TIMEOUT;
}
- _logger.debug(String.format("Exit value of process [%s] for
command [%s] is [%s].", processPid, commandLine, _process.exitValue()));
+ _logger.debug(String.format("Exit value of process [%s] for
command [%s] is [%s].", processPid,
+ commandLine, _process.exitValue()));
BufferedReader reader = new BufferedReader(new
InputStreamReader(_process.getInputStream()), 128);
@@ -339,19 +357,24 @@ public class Script implements Callable<String> {
error = String.valueOf(_process.exitValue());
}
- _logger.warn(String.format("Process [%s] for command [%s]
encountered the error: [%s].", processPid, commandLine, error));
+ _logger.warn(String.format("Process [%s] for command [%s]
encountered the error: [%s].", processPid,
+ commandLine, error));
return error;
} catch (SecurityException ex) {
- _logger.warn(String.format("Exception [%s] occurred. This may be
due to an attempt of executing command [%s] as non root.", ex.getMessage(),
commandLine),
+ _logger.warn(String.format(
+ "Exception [%s] occurred. This may be due to an attempt of
executing command [%s] as non root.",
+ ex.getMessage(), commandLine),
ex);
return stackTraceAsString(ex);
} catch (Exception ex) {
- _logger.warn(String.format("Exception [%s] occurred when
attempting to run command [%s].", ex.getMessage(), commandLine), ex);
+ _logger.warn(String.format("Exception [%s] occurred when
attempting to run command [%s].",
+ ex.getMessage(), commandLine), ex);
return stackTraceAsString(ex);
} finally {
if (_process != null) {
- _logger.trace(String.format("Destroying process [%s] for
command [%s].", _process.pid(), commandLine));
+ _logger.trace(
+ String.format("Destroying process [%s] for command
[%s].", _process.pid(), commandLine));
IOUtils.closeQuietly(_process.getErrorStream());
IOUtils.closeQuietly(_process.getOutputStream());
IOUtils.closeQuietly(_process.getInputStream());
@@ -362,9 +385,10 @@ public class Script implements Callable<String> {
public String executeIgnoreExitValue(OutputInterpreter interpreter, int
exitValue) {
String[] command = _command.toArray(new String[_command.size()]);
+ String commandLine = buildCommandLine(command);
if (_logger.isDebugEnabled()) {
- _logger.debug(String.format("Executing: %s",
buildCommandLine(command).split(KeyStoreUtils.KS_FILENAME)[0]));
+ _logger.debug(String.format("Executing: %s", commandLine));
}
try {
@@ -375,7 +399,7 @@ public class Script implements Callable<String> {
_process = pb.start();
if (_process == null) {
- _logger.warn(String.format("Unable to execute: %s",
buildCommandLine(command)));
+ _logger.warn(String.format("Unable to execute: %s",
commandLine));
return String.format("Unable to execute the command: %s",
command[0]);
}
@@ -439,11 +463,8 @@ public class Script implements Callable<String> {
Task timedoutTask = new Task(log, ir);
timedoutTask.run();
- if (!_passwordCommand) {
- _logger.warn(String.format("Timed out: %s. Output is:
%s", buildCommandLine(command), timedoutTask.getResult()));
- } else {
- _logger.warn(String.format("Timed out: %s",
buildCommandLine(command)));
- }
+ _logger.warn(String.format("Timed out: %s. Output is: %s",
commandLine,
+ timedoutTask.getResult()));
return ERR_TIMEOUT;
}
@@ -467,7 +488,7 @@ public class Script implements Callable<String> {
_logger.warn("Security Exception....not running as root?", ex);
return stackTraceAsString(ex);
} catch (Exception ex) {
- _logger.warn(String.format("Exception: %s",
buildCommandLine(command)), ex);
+ _logger.warn(String.format("Exception: %s", commandLine), ex);
return stackTraceAsString(ex);
} finally {
if (_process != null) {
@@ -516,9 +537,9 @@ public class Script implements Callable<String> {
} catch (Exception ex) {
result = stackTraceAsString(ex);
} finally {
- done = true;
- notifyAll();
- IOUtils.closeQuietly(reader);
+ done = true;
+ notifyAll();
+ IOUtils.closeQuietly(reader);
}
}
}
diff --git a/utils/src/test/java/com/cloud/utils/script/ScriptTest.java
b/utils/src/test/java/com/cloud/utils/script/ScriptTest.java
index cc6047959da..a52f3840bea 100644
--- a/utils/src/test/java/com/cloud/utils/script/ScriptTest.java
+++ b/utils/src/test/java/com/cloud/utils/script/ScriptTest.java
@@ -78,4 +78,34 @@ public class ScriptTest {
String result = Script.getExecutableAbsolutePath("ls");
Assert.assertTrue(List.of("/usr/bin/ls", "/bin/ls").contains(result));
}
+
+ @Test
+ public void testBuildCommandLineWithSensitiveData() {
+ Script script = new Script("test.sh");
+ script.add("normal-arg");
+ script.addSensitive("sensitive-arg");
+ String commandLine = script.toString();
+ Assert.assertEquals("test.sh normal-arg ****** ", commandLine);
+ }
+
+ @Test
+ public void testBuildCommandLineWithMultipleSensitiveData() {
+ Script script = new Script("test.sh");
+ script.add("normal-arg");
+ script.addSensitive("sensitive-arg1");
+ script.add("another-normal-arg");
+ script.addSensitive("sensitive-arg2");
+ String commandLine = script.toString();
+ Assert.assertEquals("test.sh normal-arg ****** another-normal-arg
****** ", commandLine);
+ }
+
+ @Test
+ public void testBuildCommandLineWithLegacyPasswordOption() {
+ Script script = new Script("test.sh");
+ script.add("-y");
+ script.add("legacy-password");
+ String commandLine = script.toString();
+ Assert.assertEquals("test.sh -y ****** ", commandLine);
+ }
+
}
