Fix sensitive data exposure in Baremetal PING PXE resource logs (#13298) - #13668
Fix sensitive data exposure in Baremetal PING PXE resource logs (#13298)#13668DaanHoogland wants to merge 2 commits into
Conversation
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>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13668 +/- ##
=========================================
Coverage 16.26% 16.26%
- Complexity 13435 13436 +1
=========================================
Files 5667 5667
Lines 500731 500769 +38
Branches 60803 60803
=========================================
+ Hits 81430 81453 +23
- Misses 410197 410210 +13
- Partials 9104 9106 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Addresses a sensitive logging exposure in the baremetal PING PXE plugin by allowing callers to provide a pre-redacted command string for logging, preventing secrets (CIFS password, VM user-data/SSH keys) from being logged in plaintext.
Changes:
- Added
maskedCmd-accepting overloads inSSHCmdHelperand centralized command selection viagetCmdForLogging. - Updated
BaremetalPingPxeResourceto pass masked variants of sensitive SSH commands and to return masked commands in failureAnswermessages. - Added unit tests covering
getCmdForLoggingbehavior with and without a provided mask.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| utils/src/test/java/com/cloud/utils/ssh/SSHCmdHelperTest.java | Adds unit coverage for masked vs fallback command logging. |
| utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java | Introduces masked-command overloads and shared logging sanitization helper. |
| plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java | Uses masked commands for CIFS password and VM user-data SSH execution and error messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
plugins/hypervisors/baremetal/src/main/java/com/cloud/baremetal/networkservice/BaremetalPingPxeResource.java:165
- The command format string is duplicated to build
scriptandmaskedScript(and similarly in the template-creation path). This is easy to accidentally diverge if arguments/order change. A more maintainable approach is to extract a small helper that builds the command given a password parameter (real vs masked), or build an argument list once and format it twice with only the password value swapped.
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());
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);


Description
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.
This PR...
Fixes: #13298
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?