From b5588fafcdce8c441cf005d14925c454f3be1539 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 12 Jan 2026 14:28:15 +0530 Subject: [PATCH] Mask vncPasswd being logged in agent.log --- .../wrapper/LibvirtMigrateCommandWrapper.java | 21 ++++++++++----- .../wrapper/LibvirtStartCommandWrapper.java | 5 ++-- .../LibvirtMigrateCommandWrapperTest.java | 26 ++++++++++++++++++- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 32f2a4b122ca..753757e01893 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -158,7 +158,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. final String target = command.getDestinationIp(); xmlDesc = dm.getXMLDesc(xmlFlag); if (logger.isDebugEnabled()) { - logger.debug(String.format("VM [%s] with XML configuration [%s] will be migrated to host [%s].", vmName, xmlDesc, target)); + logger.debug("VM {} with XML configuration {} will be migrated to host {}.", vmName, maskSensitiveInfoInXML(xmlDesc), target); } // Limit the VNC password in case the length is greater than 8 characters @@ -173,7 +173,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. logger.debug(String.format("Editing mount path of ISO from %s to %s", oldIsoVolumePath, newIsoVolumePath)); xmlDesc = replaceDiskSourceFile(xmlDesc, newIsoVolumePath, vmName); if (logger.isDebugEnabled()) { - logger.debug(String.format("Replaced disk mount point [%s] with [%s] in Instance [%s] XML configuration. New XML configuration is [%s].", oldIsoVolumePath, newIsoVolumePath, vmName, xmlDesc)); + logger.debug("Replaced disk mount point {} with {} in Instance {} XML configuration. New XML configuration is {}.", oldIsoVolumePath, newIsoVolumePath, vmName, maskSensitiveInfoInXML(xmlDesc)); } } @@ -204,11 +204,11 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. if (migrateStorage) { if (logger.isDebugEnabled()) { - logger.debug(String.format("Changing VM [%s] volumes during migration to host: [%s].", vmName, target)); + logger.debug("Changing VM {} volumes during migration to host: {}.", vmName, target); } xmlDesc = replaceStorage(xmlDesc, mapMigrateStorage, migrateStorageManaged); if (logger.isDebugEnabled()) { - logger.debug(String.format("Changed VM [%s] XML configuration of used storage. New XML configuration is [%s].", vmName, xmlDesc)); + logger.debug("Changed VM {} XML configuration of used storage. New XML configuration is {}.", vmName, maskSensitiveInfoInXML(xmlDesc)); } migrateDiskLabels = getMigrateStorageDeviceLabels(disks, mapMigrateStorage); } @@ -216,11 +216,11 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. Map dpdkPortsMapping = command.getDpdkInterfaceMapping(); if (MapUtils.isNotEmpty(dpdkPortsMapping)) { if (logger.isTraceEnabled()) { - logger.trace(String.format("Changing VM [%s] DPDK interfaces during migration to host: [%s].", vmName, target)); + logger.trace("Changing VM {} DPDK interfaces during migration to host: {}.", vmName, target); } xmlDesc = replaceDpdkInterfaces(xmlDesc, dpdkPortsMapping); if (logger.isDebugEnabled()) { - logger.debug(String.format("Changed VM [%s] XML configuration of DPDK interfaces. New XML configuration is [%s].", vmName, xmlDesc)); + logger.debug("Changed VM {} XML configuration of DPDK interfaces. New XML configuration is {}.", vmName, maskSensitiveInfoInXML(xmlDesc)); } } @@ -233,7 +233,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. } //run migration in thread so we can monitor it - logger.info(String.format("Starting live migration of instance [%s] to destination host [%s] having the final XML configuration: [%s].", vmName, dconn.getURI(), xmlDesc)); + logger.info("Starting live migration of instance {} to destination host {} having the final XML configuration: {}.", vmName, dconn.getURI(), maskSensitiveInfoInXML(xmlDesc)); final ExecutorService executor = Executors.newFixedThreadPool(1); boolean migrateNonSharedInc = command.isMigrateNonSharedInc() && !migrateStorageManaged; @@ -910,4 +910,11 @@ private boolean findSourceNode(Document doc, Node diskNode, String vmName, Strin } return false; } + + public static String maskSensitiveInfoInXML(String xmlDesc) { + if (xmlDesc == null) return null; + // Mask VNC password in XML for logging + return xmlDesc.replaceAll("(graphics\\s+[^>]*type=['\"]vnc['\"][^>]*passwd=['\"])([^'\"]*)(['\"])", + "$1*****$3"); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java index a174c9a6f14f..6e9787157555 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java @@ -80,8 +80,9 @@ public Answer execute(final StartCommand command, final LibvirtComputingResource } libvirtComputingResource.createVifs(vmSpec, vm); - - logger.debug("starting " + vmName + ": " + vm.toString()); + if (logger.isDebugEnabled()) { + logger.debug("Starting {} : {}", vmName, LibvirtMigrateCommandWrapper.maskSensitiveInfoInXML(vm.toString())); + } String vmInitialSpecification = vm.toString(); String vmFinalSpecification = performXmlTransformHook(vmInitialSpecification, libvirtComputingResource); libvirtComputingResource.startVM(conn, vmName, vmFinalSpecification); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java index 3c5e54e2ba89..05d89cc3d972 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java @@ -589,7 +589,7 @@ private Map createMapMigrateStorage(String sourceText, @Test public void testReplaceIpForVNCInDescFile() { final String targetIp = "192.168.22.21"; - final String result = libvirtMigrateCmdWrapper.replaceIpForVNCInDescFileAndNormalizePassword(fullfile, targetIp, null, ""); + final String result = libvirtMigrateCmdWrapper.replaceIpForVNCInDescFileAndNormalizePassword(fullfile, targetIp, "vncSecretPwd", ""); assertEquals("transformation does not live up to expectation:\n" + result, targetfile, result); } @@ -1019,4 +1019,28 @@ public void replaceCdromIsoPathTest() throws ParserConfigurationException, IOExc Assert.assertTrue(finalXml.contains(newIsoVolumePath)); } + + @Test + public void testMaskVncPwdDomain() { + // Test case 1: Single quotes + String xml1 = ""; + String expected1 = ""; + assertEquals(expected1, LibvirtMigrateCommandWrapper.maskSensitiveInfoInXML(xml1)); + + // Test case 2: Double quotes + String xml2 = ""; + String expected2 = ""; + assertEquals(expected2, LibvirtMigrateCommandWrapper.maskSensitiveInfoInXML(xml2)); + + // Test case 3: Non-VNC graphics (should remain unchanged) + String xml3 = ""; + assertEquals(xml3, LibvirtMigrateCommandWrapper.maskSensitiveInfoInXML(xml3)); + + // Test case 4: Multiple VNC entries in one string + String xml4 = "\n" + + ""; + String expected4 = "\n" + + ""; + assertEquals(expected4, LibvirtMigrateCommandWrapper.maskSensitiveInfoInXML(xml4)); + } }