mirror of
https://github.com/apache/cloudstack
synced 2026-08-02 05:26:35 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
e8df87e89b
commit
0107dc5013
@ -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");
|
||||
|
||||
@ -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];
|
||||
}
|
||||
}
|
||||
|
||||
@ -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);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user