Skip to content

Commit 0107dc5

Browse files
Daan Hooglandclaude
andcommitted
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>
1 parent e8df87e commit 0107dc5

3 files changed

Lines changed: 117 additions & 10 deletions

File tree

‎plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646

4747
public class BaremetalPingPxeResource extends BaremetalPxeResourceBase {
4848
private static final String Name = "BaremetalPingPxeResource";
49+
private static final String SENSITIVE_VALUE_MASK = "*****";
4950
String _storageServer;
5051
String _pingDir;
5152
String _share;
@@ -157,8 +158,11 @@ protected PreparePxeServerAnswer execute(PreparePxeServerCommand cmd) {
157158
String script =
158159
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(),
159160
_storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, _cifsPassword, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay());
160-
if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script)) {
161-
return new PreparePxeServerAnswer(cmd, "prepare PING at " + _ip + " failed, command:" + script);
161+
String maskedScript =
162+
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(),
163+
_storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, SENSITIVE_VALUE_MASK, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay());
164+
if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script, maskedScript)) {
165+
return new PreparePxeServerAnswer(cmd, "prepare PING at " + _ip + " failed, command:" + maskedScript);
162166
}
163167
logger.debug("Prepare Ping PXE server successfully");
164168

@@ -185,8 +189,11 @@ protected Answer execute(PrepareCreateTemplateCommand cmd) {
185189
String script =
186190
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(),
187191
_storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, _cifsPassword, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay());
188-
if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script)) {
189-
return new Answer(cmd, false, "prepare for creating template failed, command:" + script);
192+
String maskedScript =
193+
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(),
194+
_storageServer, _share, _dir, cmd.getTemplate(), _cifsUserName, SENSITIVE_VALUE_MASK, cmd.getIp(), cmd.getNetMask(), cmd.getGateWay());
195+
if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script, maskedScript)) {
196+
return new Answer(cmd, false, "prepare for creating template failed, command:" + maskedScript);
190197
}
191198
logger.debug("Prepare for creating template successfully");
192199

@@ -219,6 +226,7 @@ private Answer execute(VmDataCommand cmd) {
219226
try {
220227
List<String[]> vmData = cmd.getVmData();
221228
StringBuilder sb = new StringBuilder();
229+
StringBuilder maskedSb = new StringBuilder();
222230
for (String[] data : vmData) {
223231
String folder = data[0];
224232
String file = data[1];
@@ -231,8 +239,17 @@ private Answer execute(VmDataCommand cmd) {
231239
sb.append(",");
232240
sb.append(contents);
233241
sb.append(";");
242+
maskedSb.append(cmd.getVmIpAddress());
243+
maskedSb.append(",");
244+
maskedSb.append(folder);
245+
maskedSb.append(",");
246+
maskedSb.append(file);
247+
maskedSb.append(",");
248+
maskedSb.append(SENSITIVE_VALUE_MASK);
249+
maskedSb.append(";");
234250
}
235251
String arg = StringUtils.stripEnd(sb.toString(), ";");
252+
String maskedArg = StringUtils.stripEnd(maskedSb.toString(), ";");
236253

237254
sshConnection.connect(null, 60000, 60000);
238255
if (!sshConnection.authenticateWithPassword(_username, _password)) {
@@ -241,8 +258,9 @@ private Answer execute(VmDataCommand cmd) {
241258
}
242259

243260
String script = String.format("python /usr/bin/baremetal_user_data.py '%s'", arg);
244-
if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script)) {
245-
return new Answer(cmd, false, "Failed to add user data, command:" + script);
261+
String maskedScript = String.format("python /usr/bin/baremetal_user_data.py '%s'", maskedArg);
262+
if (!SSHCmdHelper.sshExecuteCmd(sshConnection, script, maskedScript)) {
263+
return new Answer(cmd, false, "Failed to add user data, command:" + maskedScript);
246264
}
247265

248266
return new Answer(cmd, true, "Success");

‎utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,13 @@ public static void releaseSshConnection(com.trilead.ssh2.Connection sshConnectio
128128
}
129129

130130
public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, String cmd, int nTimes) {
131+
return sshExecuteCmd(sshConnection, cmd, null, nTimes);
132+
}
133+
134+
public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd, int nTimes) {
131135
for (int i = 0; i < nTimes; i++) {
132136
try {
133-
final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd);
137+
final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd, maskedCmd);
134138
if (result.isSuccess()) {
135139
return true;
136140
}
@@ -142,9 +146,13 @@ public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, S
142146
}
143147

144148
public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd, int nTimes) {
149+
return sshExecuteCmdWithResult(sshConnection, cmd, null, nTimes);
150+
}
151+
152+
public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd, int nTimes) {
145153
for (int i = 0; i < nTimes; i++) {
146154
try {
147-
final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd);
155+
final SSHCmdResult result = sshExecuteCmdOneShot(sshConnection, cmd, maskedCmd);
148156
if (result.isSuccess()) {
149157
return result;
150158
}
@@ -159,12 +167,32 @@ public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, S
159167
return sshExecuteCmd(sshConnection, cmd, 3);
160168
}
161169

170+
/**
171+
* Same as {@link #sshExecuteCmd(com.trilead.ssh2.Connection, String)}, but takes a
172+
* separate, already-redacted version of {@code cmd} to use for logging. Callers that build
173+
* commands containing secrets (passwords, user-data, keys, etc.) must supply a
174+
* {@code maskedCmd} with those values replaced, since generic log sanitization cannot
175+
* reliably detect arbitrary positional/free-form secrets.
176+
*/
177+
public static boolean sshExecuteCmd(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd) {
178+
return sshExecuteCmd(sshConnection, cmd, maskedCmd, 3);
179+
}
180+
162181
public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd) {
163182
return sshExecuteCmdWithResult(sshConnection, cmd, 3);
164183
}
165184

185+
public static SSHCmdResult sshExecuteCmdWithResult(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd) {
186+
return sshExecuteCmdWithResult(sshConnection, cmd, maskedCmd, 3);
187+
}
188+
166189
public static SSHCmdResult sshExecuteCmdOneShot(com.trilead.ssh2.Connection sshConnection, String cmd) throws SshException {
167-
LOGGER.debug("Executing cmd: " + cmd.split(KeyStoreUtils.KS_FILENAME)[0]);
190+
return sshExecuteCmdOneShot(sshConnection, cmd, null);
191+
}
192+
193+
public static SSHCmdResult sshExecuteCmdOneShot(com.trilead.ssh2.Connection sshConnection, String cmd, String maskedCmd) throws SshException {
194+
String cmdForLogging = getCmdForLogging(cmd, maskedCmd);
195+
LOGGER.debug("Executing cmd: " + cmdForLogging);
168196
Session sshSession = null;
169197
try {
170198
sshSession = sshConnection.openSession();
@@ -227,7 +255,7 @@ public static SSHCmdResult sshExecuteCmdOneShot(com.trilead.ssh2.Connection sshC
227255

228256
final SSHCmdResult result = new SSHCmdResult(-1, sbStdoutResult.toString(), sbStdErrResult.toString());
229257
if (!StringUtils.isAllEmpty(result.getStdOut(), result.getStdErr())) {
230-
LOGGER.debug("SSH command: " + cmd.split(KeyStoreUtils.KS_FILENAME)[0] + "\nSSH command output:" + result.getStdOut().split("-----BEGIN")[0] + "\n" + result.getStdErr());
258+
LOGGER.debug("SSH command: " + cmdForLogging + "\nSSH command output:" + result.getStdOut().split("-----BEGIN")[0] + "\n" + result.getStdErr());
231259
}
232260

233261
// exit status delivery might get delayed
@@ -248,4 +276,18 @@ public static SSHCmdResult sshExecuteCmdOneShot(com.trilead.ssh2.Connection sshC
248276
sshSession.close();
249277
}
250278
}
279+
280+
/**
281+
* Returns the version of {@code cmd} that should be logged. When the caller provides an
282+
* already-redacted {@code maskedCmd}, that is used as-is. Otherwise, falls back to the
283+
* legacy heuristic of stripping everything from the first occurrence of the keystore
284+
* filename onwards, which only hides secrets that happen to follow it (e.g. keystore
285+
* setup commands built by {@code LibvirtServerDiscoverer}).
286+
*/
287+
protected static String getCmdForLogging(String cmd, String maskedCmd) {
288+
if (maskedCmd != null) {
289+
return maskedCmd;
290+
}
291+
return cmd.split(KeyStoreUtils.KS_FILENAME)[0];
292+
}
251293
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
//
2+
// Licensed to the Apache Software Foundation (ASF) under one
3+
// or more contributor license agreements. See the NOTICE file
4+
// distributed with this work for additional information
5+
// regarding copyright ownership. The ASF licenses this file
6+
// to you under the Apache License, Version 2.0 (the
7+
// "License"); you may not use this file except in compliance
8+
// with the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing,
13+
// software distributed under the License is distributed on an
14+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
// KIND, either express or implied. See the License for the
16+
// specific language governing permissions and limitations
17+
// under the License.
18+
//
19+
20+
package com.cloud.utils.ssh;
21+
22+
import org.junit.Assert;
23+
import org.junit.Test;
24+
25+
public class SSHCmdHelperTest {
26+
27+
@Test
28+
public void getCmdForLoggingReturnsMaskedCmdWhenProvided() {
29+
String cmd = "python /usr/bin/prepare_tftp_bootfile.py restore tftp mac server share dir template user SuperSecretPassword ip mask gw";
30+
String maskedCmd = "python /usr/bin/prepare_tftp_bootfile.py restore tftp mac server share dir template user ***** ip mask gw";
31+
32+
String result = SSHCmdHelper.getCmdForLogging(cmd, maskedCmd);
33+
34+
Assert.assertEquals(maskedCmd, result);
35+
Assert.assertFalse(result.contains("SuperSecretPassword"));
36+
}
37+
38+
@Test
39+
public void getCmdForLoggingFallsBackToKeystoreSplitWhenNoMaskProvided() {
40+
String cmd = "setup.sh /etc/cloudstack/agent/agent.properties cloud.jks SuperSecretPassword 825";
41+
42+
String result = SSHCmdHelper.getCmdForLogging(cmd, null);
43+
44+
Assert.assertFalse(result.contains("SuperSecretPassword"));
45+
Assert.assertEquals("setup.sh /etc/cloudstack/agent/agent.properties ", result);
46+
}
47+
}

0 commit comments

Comments
 (0)