This is an automated email from the ASF dual-hosted git repository. DaanHoogland pushed a commit to branch ghi13298-logging in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 0107dc50133a96ee742dbc87a73e9d959bd04e55 Author: Daan Hoogland <[email protected]> AuthorDate: Wed Jul 22 12:38:14 2026 +0200 Fix sensitive data exposure in Baremetal PING PXE resource logs (#13298) SSHCmdHelper.sshExecuteCmdOneShot only redacted logged commands by splitting on the literal keystore filename "cloud.jks", which never appears in baremetal PXE commands. As a result, CIFS storage passwords and raw VM user-data/SSH keys built by BaremetalPingPxeResource were logged in plaintext at debug level. Add maskedCmd-accepting overloads to SSHCmdHelper so callers can supply an already-redacted command for logging, and use them in BaremetalPingPxeResource for the CIFS password and VM user-data code paths, including the failure messages returned in the Answer objects. Co-Authored-By: Claude Sonnet 5 <[email protected]> --- .../networkservice/BaremetalPingPxeResource.java | 30 ++++++++++--- .../java/com/cloud/utils/ssh/SSHCmdHelper.java | 50 ++++++++++++++++++++-- .../java/com/cloud/utils/ssh/SSHCmdHelperTest.java | 47 ++++++++++++++++++++ 3 files changed, 117 insertions(+), 10 deletions(-) diff --git a/plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java b/plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java index a54cd4a1a11..c87329be2b8 100644 --- a/plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java +++ b/plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java @@ -46,6 +46,7 @@ import com.cloud.utils.ssh.SSHCmdHelper; public class BaremetalPingPxeResource extends BaremetalPxeResourceBase { private static final String Name = "BaremetalPingPxeResource"; + private static final String SENSITIVE_VALUE_MASK = "*****"; String _storageServer; String _pingDir; String _share; @@ -157,8 +158,11 @@ public class BaremetalPingPxeResource extends BaremetalPxeResourceBase { String script = String.format("python /usr/bin/prepare_tftp_bootfile.py restore %1$s %2$s %3$s %4$s %5$s %6$s %7$s %8$s %9$s %10$s %11$s", _tftpDir, cmd.getMac(), _storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, _cifsPassword, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay()); - if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script)) { - return new PreparePxeServerAnswer(cmd, "prepare PING at " + _ip + " failed, command:" + script); + String maskedScript = + String.format("python /usr/bin/prepare_tftp_bootfile.py restore %1$s %2$s %3$s %4$s %5$s %6$s %7$s %8$s %9$s %10$s %11$s", _tftpDir, cmd.getMac(), + _storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, SENSITIVE_VALUE_MASK, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay()); + if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script, maskedScript)) { + return new PreparePxeServerAnswer(cmd, "prepare PING at " + _ip + " failed, command:" + maskedScript); } logger.debug("Prepare Ping PXE server successfully"); @@ -185,8 +189,11 @@ public class BaremetalPingPxeResource extends BaremetalPxeResourceBase { String script = String.format("python /usr/bin/prepare_tftp_bootfile.py backup %1$s %2$s %3$s %4$s %5$s %6$s %7$s %8$s %9$s %10$s %11$s", _tftpDir, cmd.getMac(), _storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, _cifsPassword, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay()); - if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script)) { - return new Answer(cmd, false, "prepare for creating template failed, command:" + script); + String maskedScript = + String.format("python /usr/bin/prepare_tftp_bootfile.py backup %1$s %2$s %3$s %4$s %5$s %6$s %7$s %8$s %9$s %10$s %11$s", _tftpDir, cmd.getMac(), + _storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, SENSITIVE_VALUE_MASK, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay()); + if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script, maskedScript)) { + return new Answer(cmd, false, "prepare for creating template failed, command:" + maskedScript); } logger.debug("Prepare for creating template successfully"); @@ -219,6 +226,7 @@ public class BaremetalPingPxeResource extends BaremetalPxeResourceBase { try { List<String[]> vmData = cmd.getVmData(); StringBuilder sb = new StringBuilder(); + StringBuilder maskedSb = new StringBuilder(); for (String[] data : vmData) { String folder = data[0]; String file = data[1]; @@ -231,8 +239,17 @@ public class BaremetalPingPxeResource extends BaremetalPxeResourceBase { sb.append(","); sb.append(contents); sb.append(";"); + maskedSb.append(cmd.getVmIpAddress()); + maskedSb.append(","); + maskedSb.append(folder); + maskedSb.append(","); + maskedSb.append(file); + maskedSb.append(","); + maskedSb.append(SENSITIVE_VALUE_MASK); + maskedSb.append(";"); } String arg = StringUtils.stripEnd(sb.toString(), ";"); + String maskedArg = StringUtils.stripEnd(maskedSb.toString(), ";"); sshConnection.connect(null, 60000, 60000); if (!sshConnection.authenticateWithPassword(_username, _password)) { @@ -241,8 +258,9 @@ public class BaremetalPingPxeResource extends BaremetalPxeResourceBase { } String script = String.format("python /usr/bin/baremetal_user_data.py '%s'", arg); - if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script)) { - return new Answer(cmd, false, "Failed to add user data, command:" + script); + String maskedScript = String.format("python /usr/bin/baremetal_user_data.py '%s'", maskedArg); + if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script, maskedScript)) { + return new Answer(cmd, false, "Failed to add user data, command:" + maskedScript); } return new Answer(cmd, true, "Success"); diff --git a/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java b/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java index 944f63391a9..dae250da04b 100644 --- a/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java +++ b/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java @@ -128,9 +128,13 @@ public class SSHCmdHelper { } public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, String cmd, int nTimes) { + return sshExecuteCmd(sshConnection, cmd, null, nTimes); + } + + public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd, int nTimes) { for (int i = 0; i < nTimes; i++) { try { - final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd); + final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd, maskedCmd); if (result.isSuccess()) { return true; } @@ -142,9 +146,13 @@ public class SSHCmdHelper { } public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd, int nTimes) { + return sshExecuteCmdWithResult(sshConnection, cmd, null, nTimes); + } + + public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd, int nTimes) { for (int i = 0; i < nTimes; i++) { try { - final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd); + final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd, maskedCmd); if (result.isSuccess()) { return result; } @@ -159,12 +167,32 @@ public class SSHCmdHelper { return sshExecuteCmd(sshConnection, cmd, 3); } + /** + * Same as {@link #sshExecuteCmd(com.trilead.ssh2.Connection, String)}, but takes a + * separate, already-redacted version of {@code cmd} to use for logging. Callers that build + * commands containing secrets (passwords, user-data, keys, etc.) must supply a + * {@code maskedCmd} with those values replaced, since generic log sanitization cannot + * reliably detect arbitrary positional/free-form secrets. + */ + public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd) { + return sshExecuteCmd(sshConnection, cmd, maskedCmd, 3); + } + public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd) { return sshExecuteCmdWithResult(sshConnection, cmd, 3); } + public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd) { + return sshExecuteCmdWithResult(sshConnection, cmd, maskedCmd, 3); + } + public static SSHCmdResult sshExecuteCmdOneShot(com.trilead.ssh2.Connection sshConnection, String cmd) throws SshException { - LOGGER.debug("Executing cmd: " + cmd.split(KeyStoreUtils.KS_FILENAME)[0]); + return sshExecuteCmdOneShot(sshConnection, cmd, null); + } + + public static SSHCmdResult sshExecuteCmdOneShot(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd) throws SshException { + String cmdForLogging = getCmdForLogging(cmd, maskedCmd); + LOGGER.debug("Executing cmd: " + cmdForLogging); Session sshSession = null; try { sshSession = sshConnection.openSession(); @@ -227,7 +255,7 @@ public class SSHCmdHelper { final SSHCmdResult result = new SSHCmdResult(-1, sbStdoutResult.toString(), sbStdErrResult.toString()); if (!StringUtils.isAllEmpty(result.getStdOut(), result.getStdErr())) { - LOGGER.debug("SSH command: " + cmd.split(KeyStoreUtils.KS_FILENAME)[0] + "\nSSH command output:" + result.getStdOut().split("-----BEGIN")[0] + "\n" + result.getStdErr()); + LOGGER.debug("SSH command: " + cmdForLogging + "\nSSH command output:" + result.getStdOut().split("-----BEGIN")[0] + "\n" + result.getStdErr()); } // exit status delivery might get delayed @@ -248,4 +276,18 @@ public class SSHCmdHelper { sshSession.close(); } } + + /** + * Returns the version of {@code cmd} that should be logged. When the caller provides an + * already-redacted {@code maskedCmd}, that is used as-is. Otherwise, falls back to the + * legacy heuristic of stripping everything from the first occurrence of the keystore + * filename onwards, which only hides secrets that happen to follow it (e.g. keystore + * setup commands built by {@code LibvirtServerDiscoverer}). + */ + protected static String getCmdForLogging(String cmd, String maskedCmd) { + if (maskedCmd != null) { + return maskedCmd; + } + return cmd.split(KeyStoreUtils.KS_FILENAME)[0]; + } } diff --git a/utils/src/test/java/com/cloud/utils/ssh/SSHCmdHelperTest.java b/utils/src/test/java/com/cloud/utils/ssh/SSHCmdHelperTest.java new file mode 100644 index 00000000000..0bdc1723e2a --- /dev/null +++ b/utils/src/test/java/com/cloud/utils/ssh/SSHCmdHelperTest.java @@ -0,0 +1,47 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.utils.ssh; + +import org.junit.Assert; +import org.junit.Test; + +public class SSHCmdHelperTest { + + @Test + public void getCmdForLoggingReturnsMaskedCmdWhenProvided() { + String cmd = "python /usr/bin/prepare_tftp_bootfile.py restore tftp mac server share dir template user SuperSecretPassword ip mask gw"; + String maskedCmd = "python /usr/bin/prepare_tftp_bootfile.py restore tftp mac server share dir template user ***** ip mask gw"; + + String result = SSHCmdHelper.getCmdForLogging(cmd, maskedCmd); + + Assert.assertEquals(maskedCmd, result); + Assert.assertFalse(result.contains("SuperSecretPassword")); + } + + @Test + public void getCmdForLoggingFallsBackToKeystoreSplitWhenNoMaskProvided() { + String cmd = "setup.sh /etc/cloudstack/agent/agent.properties cloud.jks SuperSecretPassword 825"; + + String result = SSHCmdHelper.getCmdForLogging(cmd, null); + + Assert.assertFalse(result.contains("SuperSecretPassword")); + Assert.assertEquals("setup.sh /etc/cloudstack/agent/agent.properties ", result); + } +}
