Avoid logging secrets in Engine debug logs

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateOvaCommand.java

@@ -36,6 +36,7 @@
 import org.ovirt.engine.core.common.errors.EngineError;
 import org.ovirt.engine.core.common.errors.EngineException;
 import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.compat.Version;
@@ -189,8 +190,8 @@ private AnsibleCommandParameters createImageMeasureParameters(String path, Guid
     private AnsibleCommandParameters createPackOvaParameters(String ovf,
             Collection<DiskImage> disks,
             Map<Guid, String> diskIdToPath,
-            String tpmData,
-            String nvramData,
+            SecretValue<String> tpmData,
+            SecretValue<String> nvramData,
             Version compatibilityVersion) {
         String encodedOvf = encode(ovf);
         AnsibleCommandParameters params = new AnsibleCommandParameters();
@@ -229,24 +230,24 @@ private void packOva() {
         setSucceeded(true);
     }
 
-    private String getTpmData() {
+    private SecretValue<String> getTpmData() {
         var tpmDataAndHash = vmDao.getTpmData(getParameters().getEntityId());
         return tpmDataAndHash != null ? tpmDataAndHash.getFirst() : null;
     }
 
-    private String getNvramData() {
+    private SecretValue<String> getNvramData() {
         var nvramDataAndHash = vmDao.getNvramData(getParameters().getEntityId());
         return nvramDataAndHash != null ? nvramDataAndHash.getFirst() : null;
     }
 
-    private Map<VmExternalDataKind, String> getVmExternalData() {
-        Map<VmExternalDataKind, String> externalData = new HashMap<>();
-        String tpmData = getTpmData();
-        if (!StringUtils.isEmpty(tpmData)) {
+    private Map<VmExternalDataKind, SecretValue<String>> getVmExternalData() {
+        Map<VmExternalDataKind, SecretValue<String>> externalData = new HashMap<>();
+        SecretValue<String> tpmData = getTpmData();
+        if (tpmData != null && !StringUtils.isEmpty(tpmData.getValue())) {
             externalData.put(VmExternalDataKind.TPM, tpmData);
         }
-        String nvramData = getNvramData();
-        if (!StringUtils.isEmpty(nvramData)) {
+        SecretValue<String> nvramData = getNvramData();
+        if (nvramData != null && !StringUtils.isEmpty(nvramData.getValue())) {
             externalData.put(VmExternalDataKind.NVRAM, nvramData);
         }
         return externalData;
@@ -256,12 +257,12 @@ private void updateDiskVmElementFromDb(DiskImage diskImage) {
         diskHandler.updateDiskVmElementFromDb(diskImage, getParameters().getEntityId());
     }
 
-    private long calcOvaSize(Collection<DiskImage> disks, String tpmData, String nvramData, String ovf) {
+    private long calcOvaSize(Collection<DiskImage> disks, SecretValue<String> tpmData, SecretValue<String> nvramData,
+            String ovf) {
         // 1 block for the OVF, 1 block per-disk and 2 null-blocks at the end
-        return TAR_BLOCK_SIZE * (1 + disks.size() + 2)
-                + blockAlignedSize(ovf.length())
-                + (tpmData != null ? blockAlignedSize(tpmData.length()) : 0)
-                + (nvramData != null ? blockAlignedSize(nvramData.length()) : 0)
+        return TAR_BLOCK_SIZE * (1 + disks.size() + 2) + blockAlignedSize(ovf.length())
+                + (SecretValue.isNull(tpmData) ? 0 : blockAlignedSize(tpmData.getValue().length()))
+                + (SecretValue.isNull(nvramData) ? 0 : blockAlignedSize(nvramData.getValue().length()))
                 + disks.stream().mapToLong(DiskImage::getActualSizeInBytes).sum();
     }
 

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HasNvramDataQuery.java

@@ -5,6 +5,7 @@
 import org.ovirt.engine.core.bll.context.EngineContext;
 import org.ovirt.engine.core.common.queries.IdQueryParameters;
 import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.dao.VmDao;
 
 public class HasNvramDataQuery<P extends IdQueryParameters> extends QueriesCommandBase<P> {
@@ -17,7 +18,7 @@ public HasNvramDataQuery(P parameters, EngineContext engineContext) {
 
     @Override
     protected void executeQueryCommand() {
-        Pair<String, String> result = vmDao.getNvramData(getParameters().getId());
-        getQueryReturnValue().setReturnValue(result.getFirst() != null);
+        Pair<SecretValue<String>, String> result = vmDao.getNvramData(getParameters().getId());
+        getQueryReturnValue().setReturnValue(!SecretValue.isNull(result.getFirst()));
     }
 }

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HasTpmDataQuery.java

@@ -5,6 +5,7 @@
 import org.ovirt.engine.core.bll.context.EngineContext;
 import org.ovirt.engine.core.common.queries.IdQueryParameters;
 import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.dao.VmDao;
 
 public class HasTpmDataQuery<P extends IdQueryParameters> extends QueriesCommandBase<P> {
@@ -17,7 +18,7 @@ public HasTpmDataQuery(P parameters, EngineContext engineContext) {
 
     @Override
     protected void executeQueryCommand() {
-        Pair<String, String> result = vmDao.getTpmData(getParameters().getId());
-        getQueryReturnValue().setReturnValue(result.getFirst() != null);
+        Pair<SecretValue<String>, String> result = vmDao.getTpmData(getParameters().getId());
+        getQueryReturnValue().setReturnValue(!SecretValue.isNull(result.getFirst()));
     }
 }

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SaveVmExternalDataCommand.java

@@ -14,6 +14,7 @@
 import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
 import org.ovirt.engine.core.common.errors.EngineError;
 import org.ovirt.engine.core.common.errors.EngineException;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
 import org.ovirt.engine.core.compat.Guid;
@@ -114,7 +115,7 @@ protected void executeCommand() {
                 }
             } else {
                 externalDataStatus.setFinished(dataKind);
-                String data = ((VmExternalDataReturn) returnValue.getReturnValue()).data;
+                SecretValue<String> data = ((VmExternalDataReturn) returnValue.getReturnValue()).data;
                 String hash = ((VmExternalDataReturn) returnValue.getReturnValue()).hash;
                 if (data != null) {
                     switch (dataKind) {

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ExtractOvaCommand.java

@@ -27,6 +27,7 @@
 import org.ovirt.engine.core.common.businessentities.storage.VolumeFormat;
 import org.ovirt.engine.core.common.errors.EngineError;
 import org.ovirt.engine.core.common.errors.EngineException;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.common.utils.ansible.AnsibleCommandConfig;
 import org.ovirt.engine.core.common.utils.ansible.AnsibleConstants;
 import org.ovirt.engine.core.common.utils.ansible.AnsibleExecutor;
@@ -233,16 +234,15 @@ private void teardownImage(DiskImage image) {
     }
 
     private void storeExternalData(String stdout) {
-        Map<String, String> externalData = Arrays.stream(stdout.trim().split(";"))
-                .filter(s -> !StringUtils.isEmpty(s))
-                .map(s -> s.split("=", 2))
-                .collect(Collectors.toMap(part -> part[0], part -> part[1]));
-        String tpmData = externalData.get("tpm");
-        if (!StringUtils.isEmpty(tpmData)) {
+        Map<String, SecretValue<String>> externalData = Arrays.stream(stdout.trim().split(";"))
+                .filter(s -> !StringUtils.isEmpty(s)).map(s -> s.split("=", 2))
+                .collect(Collectors.toMap(part -> part[0], part -> new SecretValue<String>(part[1])));
+        SecretValue<String> tpmData = externalData.get("tpm");
+        if (!StringUtils.isEmpty(tpmData.getValue())) {
             vmDao.updateTpmData(getVmId(), tpmData, null);
         }
-        String nvramData = externalData.get("nvram");
-        if (!StringUtils.isEmpty(nvramData)) {
+        SecretValue<String> nvramData = externalData.get("nvram");
+        if (!StringUtils.isEmpty(nvramData.getValue())) {
             vmDao.updateNvramData(getVmId(), nvramData, null);
         }
     }

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java

@@ -54,6 +54,7 @@
 import org.ovirt.engine.core.common.businessentities.storage.FullEntityOvfData;
 import org.ovirt.engine.core.common.businessentities.storage.ImageStatus;
 import org.ovirt.engine.core.common.businessentities.storage.ImageStorageDomainMap;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils;
 import org.ovirt.engine.core.common.utils.VmDeviceType;
 import org.ovirt.engine.core.compat.Guid;
@@ -344,16 +345,16 @@ private void populateDisksWithVmData(List<? extends Disk> disks, Guid vmId) {
         }
     }
 
-    private void addVmExternalData(Map<VmExternalDataKind, String> vmExternalData, VM vm) {
+    private void addVmExternalData(Map<VmExternalDataKind, SecretValue<String>> vmExternalData, VM vm) {
         if (VmDeviceCommonUtils.isVmDeviceExists(vm.getStaticData().getManagedDeviceMap(), VmDeviceType.TPM)) {
-            String tpmData = vmDao.getTpmData(vm.getId()).getFirst();
-            if (tpmData != null && !tpmData.equals("")) {
+            SecretValue<String> tpmData = vmDao.getTpmData(vm.getId()).getFirst();
+            if (!SecretValue.isNull(tpmData) && !tpmData.getValue().equals("")) {
                 vmExternalData.put(VmExternalDataKind.TPM, tpmData);
             }
         }
         if (vm.getBiosType() == BiosType.Q35_SECURE_BOOT) {
-            String nvramData = vmDao.getNvramData(vm.getId()).getFirst();
-            if (nvramData != null && !nvramData.equals("")) {
+            SecretValue<String> nvramData = vmDao.getNvramData(vm.getId()).getFirst();
+            if (!SecretValue.isNull(nvramData) && !nvramData.getValue().equals("")) {
                 vmExternalData.put(VmExternalDataKind.NVRAM, nvramData);
             }
         }

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java

@@ -56,6 +56,7 @@
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.common.osinfo.OsRepository;
 import org.ovirt.engine.core.common.utils.CompatibilityVersionUtils;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils;
 import org.ovirt.engine.core.common.utils.VmDeviceType;
 import org.ovirt.engine.core.common.utils.VmDeviceUpdate;
@@ -2291,19 +2292,19 @@ public void copyVmExternalData(Guid sourceVmId, Guid targetVmId) {
     }
 
     public void updateVmExternalData(VM vm) {
-        Map<VmExternalDataKind, String> vmExternalData = vm.getVmExternalData();
+        Map<VmExternalDataKind, SecretValue<String>> vmExternalData = vm.getVmExternalData();
         if (vmExternalData == null) {
             return;
         }
         Guid vmId = vm.getId();
-        String tpmData = vmExternalData.get(VmExternalDataKind.TPM);
-        if (tpmData == null) {
+        SecretValue<String> tpmData = vmExternalData.get(VmExternalDataKind.TPM);
+        if (SecretValue.isNull(tpmData)) {
             vmDao.deleteTpmData(vmId);
         } else {
             vmDao.updateTpmData(vmId, tpmData, "");
         }
-        String nvramData = vmExternalData.get(VmExternalDataKind.NVRAM);
-        if (nvramData == null) {
+        SecretValue<String> nvramData = vmExternalData.get(VmExternalDataKind.NVRAM);
+        if (SecretValue.isNull(nvramData)) {
             vmDao.deleteNvramData(vmId);
         } else {
             vmDao.updateNvramData(vmId, nvramData, "");

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java

@@ -16,6 +16,7 @@
 import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
 import org.ovirt.engine.core.common.businessentities.storage.DiskStorageType;
 import org.ovirt.engine.core.common.locks.LockInfo;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.compat.StringHelper;
 import org.ovirt.engine.core.compat.Version;
@@ -66,7 +67,7 @@ public class VM implements Queryable, BusinessEntityWithStatus<Guid, VMStatus>,
     private BiosType clusterBiosType;
     @TransientField
     private boolean differentTimeZone;
-    private Map<VmExternalDataKind, String> vmExternalData;
+    private Map<VmExternalDataKind, SecretValue<String>> vmExternalData;
 
     @TransientField
     private boolean vnicsOutOfSync;
@@ -1985,11 +1986,11 @@ public void setLargeIconId(Guid largeIconId) {
         vmStatic.setLargeIconId(largeIconId);
     }
 
-    public Map<VmExternalDataKind, String> getVmExternalData() {
+    public Map<VmExternalDataKind, SecretValue<String>> getVmExternalData() {
         return vmExternalData;
     }
 
-    public void setVmExternalData(Map<VmExternalDataKind, String> vmExternalData) {
+    public void setVmExternalData(Map<VmExternalDataKind, SecretValue<String>> vmExternalData) {
         this.vmExternalData = vmExternalData;
     }
 

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java

@@ -12,6 +12,7 @@
 import org.ovirt.engine.core.common.businessentities.storage.DiskImage;
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.common.validation.annotation.ValidI18NName;
 import org.ovirt.engine.core.common.validation.group.CreateEntity;
 import org.ovirt.engine.core.common.validation.group.ImportClonedEntity;
@@ -68,7 +69,7 @@ public class VmTemplate extends VmBase implements BusinessEntityWithStatus<Guid,
     @TransientField
     private BiosType clusterBiosType;
 
-    private Map<VmExternalDataKind, String> vmExternalData;
+    private Map<VmExternalDataKind, SecretValue<String>> vmExternalData;
 
     public VmTemplate() {
         setNiceLevel(0);
@@ -425,11 +426,11 @@ public void setClusterBiosType(BiosType clusterBiosType) {
         this.clusterBiosType = clusterBiosType;
     }
 
-    public Map<VmExternalDataKind, String> getVmExternalData() {
+    public Map<VmExternalDataKind, SecretValue<String>> getVmExternalData() {
         return vmExternalData;
     }
 
-    public void setVmExternalData(Map<VmExternalDataKind, String> vmExternalData) {
+    public void setVmExternalData(Map<VmExternalDataKind, SecretValue<String>> vmExternalData) {
         this.vmExternalData = vmExternalData;
     }
 

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/storage/FullEntityOvfData.java

@@ -18,6 +18,7 @@
 import org.ovirt.engine.core.common.businessentities.aaa.DbUser;
 import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
 import org.ovirt.engine.core.common.scheduling.AffinityGroup;
+import org.ovirt.engine.core.common.utils.SecretValue;
 
 public class FullEntityOvfData implements Serializable {
     private VM vm;
@@ -31,7 +32,7 @@ public class FullEntityOvfData implements Serializable {
     private Set<DbUser> dbUsers = new HashSet<>();
     private Map<String, Set<String>> userToRoles = new HashMap<>();
     private List<Label> affinityLabelsNames = new ArrayList<>();
-    private Map<VmExternalDataKind, String> vmExternalData = new EnumMap<>(VmExternalDataKind.class);
+    private Map<VmExternalDataKind, SecretValue<String>> vmExternalData = new EnumMap<>(VmExternalDataKind.class);
 
     public FullEntityOvfData() {
     }
@@ -156,7 +157,7 @@ public void setAffinityLabels(List<Label> affinityLabelsNames) {
         this.affinityLabelsNames = affinityLabelsNames;
     }
 
-    public Map<VmExternalDataKind, String> getVmExternalData() {
+    public Map<VmExternalDataKind, SecretValue<String>> getVmExternalData() {
         return vmExternalData;
     }
 

backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SecretValue.java

@@ -0,0 +1,45 @@
+package org.ovirt.engine.core.common.utils;
+
+import java.util.Objects;
+
+public class SecretValue<T> {
+    private T value;
+
+    public T getValue() {
+        return value;
+    }
+
+    public SecretValue(T value) {
+        this.value = value;
+    }
+
+    private SecretValue() {
+    }
+
+    public String toString() {
+        return "***";
+    }
+
+    /**
+     * @return Whether {@code secret} or its value is {@code null}.
+     */
+    public static boolean isNull(SecretValue<?> secret) {
+        return secret == null || secret.getValue() == null;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(value);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (!(obj instanceof SecretValue)) {
+            return false;
+        }
+        return Objects.equals(value, ((SecretValue<?>)obj).getValue());
+    }
+}

backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDao.java

@@ -8,6 +8,7 @@
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VmDevice;
 import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.common.utils.SecretValue;
 import org.ovirt.engine.core.compat.Guid;
 
 /**
@@ -397,7 +398,7 @@ public interface VmDao extends Dao {
      *
      * @return Pair of data and its hash; any of the pair values can be null if not available
      */
-    Pair<String, String> getTpmData(Guid vmId);
+    Pair<SecretValue<String>, String> getTpmData(Guid vmId);
 
     /**
      * Stores the specified TPM data for the given VM.
@@ -406,7 +407,7 @@ public interface VmDao extends Dao {
      * @param tpmData the data
      * @param tpmDataHash hash of the data as obtained from the VDS
      */
-    void updateTpmData(Guid vmId, String tpmData, String tpmDataHash);
+    void updateTpmData(Guid vmId, SecretValue<String> tpmData, String tpmDataHash);
 
     /**
      * Deletes the TPM data for the given VM.
@@ -430,7 +431,7 @@ public interface VmDao extends Dao {
      *
      * @return Pair of data and its hash; any of the pair values can be null if not available
      */
-    Pair<String, String> getNvramData(Guid vmId);
+    Pair<SecretValue<String>, String> getNvramData(Guid vmId);
 
     /**
      * Stores the specified secure boot NVRAM data for the given VM.
@@ -439,7 +440,7 @@ public interface VmDao extends Dao {
      * @param nvramData the data
      * @param nvramDataHash hash of the data as obtained from the VDS
      */
-    void updateNvramData(Guid vmId, String nvramData, String nvramDataHash);
+    void updateNvramData(Guid vmId, SecretValue<String> nvramData, String nvramDataHash);
 
     /**
      * Deletes the NVRAM data for the given VM.