<fix>[crypto]: keep TPM key ref until VM removed from DB

zhong.zhou · zhong.zhou · commit 70f437ff0747 · 2026-04-04T00:09:29.000+08:00
Delay/trash deletion no longer drops EncryptedResourceKeyRef;
cleanup runs in vmJustBeforeDeleteFromDb (expunge/direct delete).
Centralize ref delete in VmTpmManager; wire RemoveTpm/add-tpm
rollback and drop unused destroy extension from VmTpmExtensions.

Resolves: ZSV-11721

Change-Id: I68676e706f6a786773627074636f6e6a617a6e4f
diff --git a/compute/src/main/java/org/zstack/compute/vm/devices/DummyEncryptedResourceKeyManager.java b/compute/src/main/java/org/zstack/compute/vm/devices/DummyEncryptedResourceKeyManager.java
@@ -23,4 +23,9 @@ public void getOrCreateKey(GetOrCreateResourceKeyContext ctx,
     public void rollbackCreatedKey(ResourceKeyResult result, Completion completion) {
         completion.success();
     }
+
+    @Override
+    public void deleteRef(String resourceType, String resourceUuid) {
+        // do-nothing
+    }
 }
diff --git a/compute/src/main/java/org/zstack/compute/vm/devices/VmTpmExtensions.java b/compute/src/main/java/org/zstack/compute/vm/devices/VmTpmExtensions.java
@@ -9,7 +9,9 @@
 import org.zstack.header.tpm.entity.TpmVO;
 import org.zstack.header.tpm.entity.TpmVO_;
 import org.zstack.header.vm.CreateVmInstanceMsg;
+import org.zstack.header.vm.VmInstanceInventory;
 import org.zstack.header.vm.VmInstanceCreateExtensionPoint;
+import org.zstack.header.vm.VmJustBeforeDeleteFromDbExtensionPoint;
 import org.zstack.header.vm.VmInstanceSpec;
 import org.zstack.header.vm.VmInstanceVO;
 import org.zstack.header.vm.VmMachineType;
@@ -23,7 +25,7 @@
 import static org.zstack.compute.vm.VmGlobalConfig.ENABLE_UEFI_SECURE_BOOT;
 
 public class VmTpmExtensions implements VmInstanceCreateExtensionPoint,
-        BuildVmSpecExtensionPoint {
+        BuildVmSpecExtensionPoint, VmJustBeforeDeleteFromDbExtensionPoint {
     private static final CLogger logger = Utils.getLogger(VmTpmExtensions.class);
 
     @Autowired
@@ -70,11 +72,7 @@ public void afterRollbackPersistVmInstanceVO(VmInstanceVO vo, CreateVmInstanceMs
         new SQLBatch() {
             @Override
             protected void scripts() {
-                try {
-                    resourceKeyBackend.detachKeyProviderFromTpm(tpmUuid);
-                } finally {
-                    vmTpmManager.deleteTpmVO(tpmUuid);
-                }
+                vmTpmManager.deleteTpmAndResourceKeyRef(tpmUuid);
             }
         }.execute();
     }
@@ -129,4 +127,15 @@ public void afterBuildVmSpec(VmInstanceSpec spec) {
                     "auto-set machineType to q35 for VM[uuid:%s] because NvRam/TPM registration is needed", vmUuid));
         }
     }
+
+    /**
+     * Remove the TPM resource-key ref when the VM row is about to be removed from the database
+     * (e.g. direct delete, expunge). A VM has at most one vTPM; ref follows that device. Delay
+     * (recycle bin) does not call this hook, so refs remain for recovery and rekey. RemoveTpm
+     * already drops ref via {@link VmTpmManager#deleteResourceKeyRefForTpm} before TpmVO is removed.
+     */
+    @Override
+    public void vmJustBeforeDeleteFromDb(VmInstanceInventory inv) {
+        vmTpmManager.deleteResourceKeyRefForVmTpmIfPresent(inv.getUuid());
+    }
 }
diff --git a/compute/src/main/java/org/zstack/compute/vm/devices/VmTpmManager.java b/compute/src/main/java/org/zstack/compute/vm/devices/VmTpmManager.java
@@ -5,6 +5,7 @@
 import org.zstack.core.db.DatabaseFacade;
 import org.zstack.core.db.Q;
 import org.zstack.header.image.ImageBootMode;
+import org.zstack.header.keyprovider.EncryptedResourceKeyManager;
 import org.zstack.header.tpm.entity.TpmVO;
 import org.zstack.header.tpm.entity.TpmVO_;
 import org.zstack.resourceconfig.ResourceConfig;
@@ -23,6 +24,8 @@ public class VmTpmManager {
     private DatabaseFacade databaseFacade;
     @Autowired
     private ResourceConfigFacade resourceConfigFacade;
+    @Autowired
+    private EncryptedResourceKeyManager resourceKeyManager;
 
     public TpmVO persistTpmVO(String tpmUuid, String vmUuid) {
         if (tpmUuid == null) {
@@ -42,6 +45,36 @@ public void deleteTpmVO(String tpmUuid) {
         databaseFacade.removeByPrimaryKey(tpmUuid, TpmVO.class);
     }
 
+    /**
+     * Drop encrypted-resource key ref for this TPM device (logical key binding). Call when the vTPM is
+     * removed or before the {@link TpmVO} row disappears (e.g. VM expunged).
+     */
+    public void deleteResourceKeyRefForTpm(String tpmUuid) {
+        resourceKeyManager.deleteRef(TpmVO.class.getSimpleName(), tpmUuid);
+    }
+
+    /**
+     * Same as {@link #deleteResourceKeyRefForTpm} for the vTPM attached to this VM, if any.
+     * A VM has at most one {@link TpmVO}.
+     */
+    public void deleteResourceKeyRefForVmTpmIfPresent(String vmInstanceUuid) {
+        String tpmUuid = Q.New(TpmVO.class)
+                .eq(TpmVO_.vmInstanceUuid, vmInstanceUuid)
+                .select(TpmVO_.uuid)
+                .findValue();
+        if (tpmUuid != null) {
+            deleteResourceKeyRefForTpm(tpmUuid);
+        }
+    }
+
+    /**
+     * Remove TPM resource-key ref then delete {@link TpmVO} (e.g. create-rollback or add-tpm rollback).
+     */
+    public void deleteTpmAndResourceKeyRef(String tpmUuid) {
+        deleteResourceKeyRefForTpm(tpmUuid);
+        deleteTpmVO(tpmUuid);
+    }
+
     /**
      * @param bootMode boot mode, null is Legacy
      */
diff --git a/conf/springConfigXml/VmInstanceManager.xml b/conf/springConfigXml/VmInstanceManager.xml
@@ -286,6 +286,7 @@
         <zstack:plugin>
             <zstack:extension interface="org.zstack.header.vm.VmInstanceCreateExtensionPoint" />
             <zstack:extension interface="org.zstack.compute.vm.BuildVmSpecExtensionPoint" />
+            <zstack:extension interface="org.zstack.header.vm.VmJustBeforeDeleteFromDbExtensionPoint" />
         </zstack:plugin>
     </bean>
 
diff --git a/header/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.java b/header/src/main/java/org/zstack/header/keyprovider/EncryptedResourceKeyManager.java
@@ -34,10 +34,17 @@ void getOrCreateKey(GetOrCreateResourceKeyContext ctx,
      */
     void rollbackCreatedKey(ResourceKeyResult result, Completion completion);
 
+    /**
+     * Delete the resource-to-key-provider relationship for the specified resource.
+     * This only removes the persisted ref record and does not delete the remote secret.
+     */
+    void deleteRef(String resourceType, String resourceUuid);
+
     class GetOrCreateResourceKeyContext {
         private String resourceUuid;
         private String resourceType;
         private String keyProviderUuid;
+        private String keyProviderName;
         private String purpose;
 
         public String getResourceUuid() {
@@ -64,6 +71,14 @@ public void setKeyProviderUuid(String keyProviderUuid) {
             this.keyProviderUuid = keyProviderUuid;
         }
 
+        public String getKeyProviderName() {
+            return keyProviderName;
+        }
+
+        public void setKeyProviderName(String keyProviderName) {
+            this.keyProviderName = keyProviderName;
+        }
+
         public String getPurpose() {
             return purpose;
         }
diff --git a/plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java b/plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java
@@ -173,7 +173,7 @@ public void fail(ErrorCode errorCode) {
             @Override
             public boolean skip(Map data) {
                 boolean shouldSkip = VmGlobalConfig.ALLOWED_TPM_VM_WITHOUT_KMS.value(Boolean.class) &&
-                        (StringUtils.isBlank(context.providerUuid) || StringUtils.isBlank(context.providerName));
+                        (StringUtils.isBlank(context.providerUuid) && StringUtils.isBlank(context.providerName));
                 if (shouldSkip) {
                     logger.info("skip create-dek: allowed.tpm.vm.without.kms is enabled and no KMS provider bound");
                 }
@@ -182,7 +182,7 @@ public boolean skip(Map data) {
 
             @Override
             public void run(FlowTrigger trigger, Map data) {
-                if (StringUtils.isBlank(context.providerUuid) || StringUtils.isBlank(context.providerName)) {
+                if (StringUtils.isBlank(context.providerUuid) && StringUtils.isBlank(context.providerName)) {
                     trigger.fail(operr("missing TPM resource key binding for tpm[uuid:%s], attachKeyProviderToTpm must run before create-dek", context.tpmUuid));
                     return;
                 }
@@ -191,13 +191,15 @@ public void run(FlowTrigger trigger, Map data) {
                 keyCtx.setResourceUuid(context.tpmUuid);
                 keyCtx.setResourceType(TpmVO.class.getSimpleName());
                 keyCtx.setKeyProviderUuid(context.providerUuid);
+                keyCtx.setKeyProviderName(context.providerName);
                 keyCtx.setPurpose("vtpm");
 
                 resourceKeyManager.getOrCreateKey(keyCtx, new ReturnValueCompletion<ResourceKeyResult>(trigger) {
                     @Override
                     public void success(ResourceKeyResult result) {
                         tpmSpec.setResourceKeyCreatedNew(result.isCreatedNewKey());
                         tpmSpec.setResourceKeyProviderUuid(result.getKeyProviderUuid());
+                        context.providerUuid = result.getKeyProviderUuid();
                         context.providerName = result.getKeyProviderName();
                         context.dekBase64 = result.getDekBase64();
                         trigger.next();
diff --git a/plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.java b/plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmManager.java
@@ -244,12 +244,10 @@ private void addTpmToVm(AddTpmToVmContext context, Completion completion) {
                     }
                 })
                 .rollback(trigger -> {
-                    try {
-                        if (context.keyProviderAttached && context.createdTpmUuid != null) {
-                            tpmKeyBackend.detachKeyProviderFromTpm(context.createdTpmUuid);
-                        }
-                    } finally {
-                        if (context.tpmCreated && context.createdTpmUuid != null) {
+                    if (context.tpmCreated && context.createdTpmUuid != null) {
+                        if (context.keyProviderAttached) {
+                            vmTpmManager.deleteTpmAndResourceKeyRef(context.createdTpmUuid);
+                        } else {
                             vmTpmManager.deleteTpmVO(context.createdTpmUuid);
                         }
                     }
@@ -386,7 +384,7 @@ public void done(ErrorCodeList errorCodeList) {
                 .build())
             .then(Flow.of("detach-resource-key")
                 .handle(trigger -> {
-                    tpmKeyBackend.detachKeyProviderFromTpm(context.tpmUuid);
+                    vmTpmManager.deleteResourceKeyRefForTpm(context.tpmUuid);
                     trigger.next();
                 })
                 .build())

Original file line number	Diff line number	Diff line change
`@@ -23,4 +23,9 @@ public void getOrCreateKey(GetOrCreateResourceKeyContext ctx,`
`23`	`23`	`public void rollbackCreatedKey(ResourceKeyResult result, Completion completion) {`
`24`	`24`	`completion.success();`
`25`	`25`	`}`
	`26`	`+`
	`27`	`+ @Override`
	`28`	`+ public void deleteRef(String resourceType, String resourceUuid) {`
	`29`	`+ // do-nothing`
	`30`	`+ }`
`26`	`31`	`}`