core: Use SecretsWrapper to protect TPM and secure boot data

oVirt · Sep 8, 2022 · 2bfb590 · 2bfb590
1 parent 88da56d
commit 2bfb590
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 17 deletions.
diff --git a/...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SaveVmExternalDataCommand.java b/...anager/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.SecretsWrapper;
 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;
+                SecretsWrapper<String> data = ((VmExternalDataReturn) returnValue.getReturnValue()).data;
                 String hash = ((VmExternalDataReturn) returnValue.getReturnValue()).hash;
                 if (data != null) {
                     switch (dataKind) {

diff --git a/...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ExtractOvaCommand.java b/...r/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.SecretsWrapper;
 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;
@@ -239,11 +240,11 @@ private void storeExternalData(String stdout) {
                 .collect(Collectors.toMap(part -> part[0], part -> part[1]));
         String tpmData = externalData.get("tpm");
         if (!StringUtils.isEmpty(tpmData)) {
-            vmDao.updateTpmData(getVmId(), tpmData, null);
+            vmDao.updateTpmData(getVmId(), new SecretsWrapper<String>(tpmData), null);
         }
         String nvramData = externalData.get("nvram");
         if (!StringUtils.isEmpty(nvramData)) {
-            vmDao.updateNvramData(getVmId(), nvramData, null);
+            vmDao.updateNvramData(getVmId(), new SecretsWrapper<String>(nvramData), null);
         }
     }
 

diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java b/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.SecretsWrapper;
 import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils;
 import org.ovirt.engine.core.common.utils.VmDeviceType;
 import org.ovirt.engine.core.common.utils.VmDeviceUpdate;
@@ -2300,13 +2301,13 @@ public void updateVmExternalData(VM vm) {
         if (tpmData == null) {
             vmDao.deleteTpmData(vmId);
         } else {
-            vmDao.updateTpmData(vmId, tpmData, "");
+            vmDao.updateTpmData(vmId, new SecretsWrapper<String>(tpmData), "");
         }
         String nvramData = vmExternalData.get(VmExternalDataKind.NVRAM);
         if (nvramData == null) {
             vmDao.deleteNvramData(vmId);
         } else {
-            vmDao.updateNvramData(vmId, nvramData, "");
+            vmDao.updateNvramData(vmId, new SecretsWrapper<String>(nvramData), "");
         }
     }
 }
diff --git a/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SecretsWrapper.java b/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/SecretsWrapper.java
@@ -11,6 +11,9 @@ public SecretsWrapper(T value) {
         this.value = value;
     }
 
+    private SecretsWrapper() {
+    }
+
     public String toString() {
         return "***";
     }

diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDao.java b/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.SecretsWrapper;
 import org.ovirt.engine.core.compat.Guid;
 
 /**
@@ -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, SecretsWrapper<String> tpmData, String tpmDataHash);
 
     /**
      * Deletes the TPM 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, SecretsWrapper<String> nvramData, String nvramDataHash);
 
     /**
      * Deletes the NVRAM data for the given VM.

diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDaoImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDaoImpl.java
@@ -18,6 +18,7 @@
 import org.ovirt.engine.core.common.businessentities.VmDevice;
 import org.ovirt.engine.core.common.di.interceptor.InvocationLogger;
 import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.common.utils.SecretsWrapper;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.utils.SerializationFactory;
 import org.springframework.jdbc.core.RowMapper;
@@ -427,11 +428,11 @@ public Pair<String, String> getTpmData(Guid vmId) {
     }
 
     @Override
-    public void updateTpmData(Guid vmId, String tpmData, String tpmDataHash) {
+    public void updateTpmData(Guid vmId, SecretsWrapper<String> tpmData, String tpmDataHash) {
         getCallsHandler().executeModification("UpdateTpmData",
                 getCustomMapSqlParameterSource()
                         .addValue("vm_id", vmId)
-                        .addValue("tpm_data", tpmData)
+                        .addValue("tpm_data", tpmData.getValue())
                         .addValue("tpm_hash", tpmDataHash));
     }
 
@@ -456,11 +457,11 @@ public Pair<String, String> getNvramData(Guid vmId) {
     }
 
     @Override
-    public void updateNvramData(Guid vmId, String nvramData, String nvramDataHash) {
+    public void updateNvramData(Guid vmId, SecretsWrapper<String> nvramData, String nvramDataHash) {
         getCallsHandler().executeModification("UpdateNvramData",
                 getCustomMapSqlParameterSource()
                         .addValue("vm_id", vmId)
-                        .addValue("nvram_data", nvramData)
+                        .addValue("nvram_data", nvramData.getValue())
                         .addValue("nvram_hash", nvramDataHash));
     }
 

diff --git a/...er/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmExternalDataMonitoring.java b/...er/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmExternalDataMonitoring.java
@@ -12,6 +12,7 @@
 
 import org.ovirt.engine.core.common.action.VmExternalDataKind;
 import org.ovirt.engine.core.common.qualifiers.VmDeleted;
+import org.ovirt.engine.core.common.utils.SecretsWrapper;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
 import org.ovirt.engine.core.compat.Guid;
@@ -82,7 +83,7 @@ public void updateVm(Guid vmId, Guid vdsId, String tpmDataHash, String nvramData
     }
 
     private void saveExternalData(Guid vmId, Guid vdsId, ExternalDataHashes externalDataHashes,
-            VmExternalDataKind dataKind, String newDataHash, BiConsumer<String, String> storeFunction) {
+            VmExternalDataKind dataKind, String newDataHash, BiConsumer<SecretsWrapper<String>, String> storeFunction) {
         if (newDataHash != null) {
             synchronized (externalDataHashes) {
                 if (newDataHash.equals(externalDataHashes.getDataHash(dataKind))) {
@@ -100,8 +101,8 @@ private void saveExternalData(Guid vmId, Guid vdsId, ExternalDataHashes external
             }
             if (retVal.getSucceeded()) {
                 VmExternalDataReturn externalDataReturn = (VmExternalDataReturn) retVal.getReturnValue();
-                String data = externalDataReturn.data;
-                if (data != null && !data.equals("")) {
+                SecretsWrapper<String> data = externalDataReturn.data;
+                if (data.getValue() != null && !data.getValue().equals("")) {
                     synchronized (externalDataHashes) {
                         try {
                             storeFunction.accept(data, externalDataReturn.hash);

diff --git a/...sbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmExternalDataReturn.java b/...sbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmExternalDataReturn.java
@@ -2,6 +2,8 @@
 
 import java.util.Map;
 
+import org.ovirt.engine.core.common.utils.SecretsWrapper;
+
 @SuppressWarnings("unchecked")
 public class VmExternalDataReturn {
 
@@ -11,17 +13,18 @@ public class VmExternalDataReturn {
     private static final String HASH = "hash";
 
     public final Status status;
-    public final String data;
+    public final SecretsWrapper<String> data;
     public final String hash;
 
     public VmExternalDataReturn(Map<String, Object> innerMap) {
         status = new Status((Map<String, Object>) innerMap.get(STATUS));
         final Map<String, String> result = (Map<String, String>) innerMap.get(INFO);
         if (status.code == 0) {
-            data = result.getOrDefault(DATA, null);
+            data = new SecretsWrapper<>(result.getOrDefault(DATA, null));
             hash = result.get(HASH);
         } else {
-            data = hash = "";
+            data = new SecretsWrapper<>("");
+            hash = "";
         }
     }
 }