Skip to content

Commit 0cb22fd

Browse files
author
zhijian.liu
committed
<fix>[crypto]: fix conflict and review
Resolves: ZSV-11630
2 parents 74011f4 + 61d98c2 commit 0cb22fd

4 files changed

Lines changed: 219 additions & 77 deletions

File tree

‎plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5327,14 +5327,16 @@ private void handle(SecretHostGetMsg msg) {
53275327
}
53285328

53295329
String url = buildUrl(KVMConstant.KVM_GET_SECRET_PATH);
5330-
Map<String, String> headers = new HashMap<>();
5331-
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
53325330
KVMAgentCommands.SecretHostGetCmd cmd = new KVMAgentCommands.SecretHostGetCmd();
53335331
cmd.setVmUuid(msg.getVmUuid());
53345332
cmd.setPurpose(msg.getPurpose());
53355333
cmd.setKeyVersion(msg.getKeyVersion());
53365334
cmd.setUsageInstance(KVMConstant.HOST_SECRET_USAGE_INSTANCE_VTPM);
5337-
restf.asyncJsonPost(url, cmd, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) {
5335+
Map<String, String> headers = new HashMap<>();
5336+
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
5337+
Http<KVMAgentCommands.SecretHostGetResponse> http = new Http<>(url, cmd, KVMAgentCommands.SecretHostGetResponse.class);
5338+
http.runBeforeAsyncJsonPostExts(headers);
5339+
restf.asyncJsonPost(url, http.commandStr, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostGetResponse>(msg, reply) {
53385340
@Override
53395341
public void fail(ErrorCode err) {
53405342
reply.setError(err != null ? err : operr("get secret on agent failed"));
@@ -5343,8 +5345,13 @@ public void fail(ErrorCode err) {
53435345

53445346
@Override
53455347
public void success(KVMAgentCommands.SecretHostGetResponse rsp) {
5346-
if (rsp != null && rsp.isSuccess()) {
5348+
if (rsp != null && rsp.isSuccess() && StringUtils.isNotBlank(rsp.getSecretUuid())) {
53475349
reply.setSecretUuid(rsp.getSecretUuid());
5350+
} else if (rsp != null && rsp.isSuccess()) {
5351+
ErrorCode err = new ErrorCode();
5352+
err.setCode(SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND);
5353+
err.setDetails("get secret succeeded but secretUuid is empty");
5354+
reply.setError(err);
53485355
} else {
53495356
reply.setError(buildSecretAgentError(rsp, "get secret failed"));
53505357
}
@@ -5440,16 +5447,18 @@ private void handle(SecretHostDefineMsg msg) {
54405447
}
54415448
String envelopeDekBase64 = java.util.Base64.getEncoder().encodeToString(envelope);
54425449
String url = buildUrl(KVMConstant.KVM_ENSURE_SECRET_PATH);
5443-
Map<String, String> headers = new HashMap<>();
5444-
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
54455450
KVMAgentCommands.SecretHostDefineCmd cmd = new KVMAgentCommands.SecretHostDefineCmd();
54465451
cmd.setEncryptedDek(envelopeDekBase64);
54475452
cmd.setVmUuid(msg.getVmUuid());
54485453
cmd.setPurpose(msg.getPurpose());
54495454
cmd.setKeyVersion(msg.getKeyVersion());
54505455
cmd.setDescription(msg.getDescription() != null ? msg.getDescription() : "");
54515456
cmd.setUsageInstance(KVMConstant.HOST_SECRET_USAGE_INSTANCE_VTPM);
5452-
restf.asyncJsonPost(url, cmd, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) {
5457+
Map<String, String> headers = new HashMap<>();
5458+
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
5459+
Http<KVMAgentCommands.SecretHostDefineResponse> http = new Http<>(url, cmd, KVMAgentCommands.SecretHostDefineResponse.class);
5460+
http.runBeforeAsyncJsonPostExts(headers);
5461+
restf.asyncJsonPost(url, http.commandStr, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDefineResponse>(msg, reply) {
54535462
@Override
54545463
public void fail(ErrorCode err) {
54555464
reply.setError(err != null ? err : operr("ensure secret on agent failed"));
@@ -5483,40 +5492,35 @@ private void handle(SecretHostDeleteMsg msg) {
54835492
return;
54845493
}
54855494
if (msg.getKeyVersion() == null) {
5486-
logger.debug(String.format(
5487-
"skip delete host secret on host[uuid:%s]: keyVersion is null (vm[uuid:%s], purpose:%s)",
5488-
getSelf().getUuid(), msg.getVmUuid(), msg.getPurpose()));
5495+
reply.setError(operr("keyVersion is required for delete secret"));
54895496
bus.reply(msg, reply);
54905497
return;
54915498
}
54925499

54935500
String url = buildUrl(KVMConstant.KVM_DELETE_SECRET_PATH);
5494-
Map<String, String> headers = new HashMap<>();
5495-
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
54965501
KVMAgentCommands.SecretHostDeleteCmd cmd = new KVMAgentCommands.SecretHostDeleteCmd();
54975502
cmd.setVmUuid(msg.getVmUuid());
54985503
cmd.setPurpose(msg.getPurpose());
54995504
cmd.setKeyVersion(msg.getKeyVersion());
55005505
cmd.setUsageInstance(KVMConstant.HOST_SECRET_USAGE_INSTANCE_VTPM);
5501-
5502-
restf.asyncJsonPost(url, cmd, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDeleteResponse>(msg, reply) {
5506+
Map<String, String> headers = new HashMap<>();
5507+
headers.put(Constants.AGENT_HTTP_HEADER_RESOURCE_UUID, getSelf().getUuid());
5508+
Http<KVMAgentCommands.SecretHostDeleteResponse> http = new Http<>(url, cmd, KVMAgentCommands.SecretHostDeleteResponse.class);
5509+
http.runBeforeAsyncJsonPostExts(headers);
5510+
restf.asyncJsonPost(url, http.commandStr, headers, new JsonAsyncRESTCallback<KVMAgentCommands.SecretHostDeleteResponse>(msg, reply) {
55035511
@Override
55045512
public void fail(ErrorCode err) {
5505-
logger.warn(String.format(
5506-
"best-effort delete secret on agent failed on host[uuid:%s] for vm[uuid:%s]: %s",
5507-
getSelf().getUuid(), msg.getVmUuid(),
5508-
err != null ? err.getDetails() : "unknown error"));
5513+
reply.setError(err != null ? err : operr("delete secret on agent failed"));
55095514
bus.reply(msg, reply);
55105515
}
55115516

55125517
@Override
55135518
public void success(KVMAgentCommands.SecretHostDeleteResponse rsp) {
55145519
if (rsp == null || !rsp.isSuccess()) {
55155520
ErrorCode agentErr = buildSecretAgentError(rsp, "delete secret failed");
5516-
logger.warn(String.format(
5517-
"best-effort delete secret on agent failed on host[uuid:%s] for vm[uuid:%s]: %s",
5518-
getSelf().getUuid(), msg.getVmUuid(),
5519-
agentErr.getDetails()));
5521+
if (!SecretHostGetReply.ERROR_CODE_SECRET_NOT_FOUND.equals(agentErr.getCode())) {
5522+
reply.setError(agentErr);
5523+
}
55205524
}
55215525
bus.reply(msg, reply);
55225526
}

‎plugin/kvm/src/main/java/org/zstack/kvm/tpm/KvmTpmExtensions.java‎

Lines changed: 114 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,23 @@
3131
import org.zstack.header.secret.SecretHostDefineMsg;
3232
import org.zstack.header.storage.snapshot.group.VolumeSnapshotGroupVO;
3333
import org.zstack.header.storage.snapshot.group.VolumeSnapshotGroupVO_;
34-
import org.zstack.header.secret.SecretHostDefineReply;
3534
import org.zstack.header.secret.SecretHostGetMsg;
3635
import org.zstack.header.secret.SecretHostGetReply;
3736
import org.zstack.header.tpm.entity.TpmSpec;
3837
import org.zstack.header.tpm.entity.TpmVO;
3938
import org.zstack.header.tpm.entity.TpmVO_;
4039
import org.zstack.header.vm.HaStartVmInstanceMsg;
41-
import org.zstack.header.vm.PreVmInstantiateResourceExtensionPoint;
4240
import org.zstack.header.vm.VmAfterExpungeExtensionPoint;
43-
import org.zstack.header.vm.VmInstanceInventory;
4441
import org.zstack.header.vm.VmInstanceMigrateExtensionPoint;
42+
import org.zstack.header.secret.SecretHostDefineReply;
43+
import org.zstack.header.tpm.message.RemoveTpmMsg;
44+
import org.zstack.header.vm.PreVmInstantiateResourceExtensionPoint;
45+
import org.zstack.header.vm.VmInstanceInventory;
4546
import org.zstack.header.vm.VmInstanceSpec;
4647
import org.zstack.header.vm.VmInstanceState;
4748
import org.zstack.header.vm.VmInstantiateResourceException;
4849
import org.zstack.header.vm.VmStateChangedExtensionPoint;
50+
import org.zstack.header.vm.VmJustBeforeDeleteFromDbExtensionPoint;
4951
import org.zstack.header.vm.additions.VmHostBackupFileVO;
5052
import org.zstack.header.vm.additions.VmHostBackupFileVO_;
5153
import org.zstack.header.vm.VmInstanceVO;
@@ -64,16 +66,22 @@
6466

6567
import java.sql.Timestamp;
6668
import java.time.Instant;
69+
import java.util.HashSet;
70+
import java.util.List;
71+
import java.util.Set;
72+
import java.util.concurrent.ConcurrentHashMap;
6773
import java.util.Map;
6874

75+
import static org.zstack.header.tpm.TpmConstants.SERVICE_ID;
6976
import static org.zstack.kvm.KVMConstant.*;
7077
import static org.zstack.core.Platform.operr;
7178

7279
public class KvmTpmExtensions implements KVMStartVmExtensionPoint,
7380
PreVmInstantiateResourceExtensionPoint,
7481
VmInstanceMigrateExtensionPoint,
7582
VmAfterExpungeExtensionPoint,
76-
VmStateChangedExtensionPoint {
83+
VmStateChangedExtensionPoint,
84+
VmJustBeforeDeleteFromDbExtensionPoint {
7785
private static final CLogger logger = Utils.getLogger(KvmTpmExtensions.class);
7886

7987
@Autowired
@@ -88,6 +96,7 @@ public class KvmTpmExtensions implements KVMStartVmExtensionPoint,
8896
private CloudBus bus;
8997

9098
private final Object hostFileLock = new Object();
99+
private final Map<String, String> volumeMigratingSourceHostCache = new ConcurrentHashMap<>();
91100

92101
@Override
93102
public void beforeStartVmOnKvm(KVMHostInventory host, VmInstanceSpec spec, KVMAgentCommands.StartVmCmd cmd) {
@@ -477,6 +486,50 @@ public void fail(ErrorCode errorCode) {
477486
});
478487
}
479488

489+
@Override
490+
public void vmJustBeforeDeleteFromDb(VmInstanceInventory inv) {
491+
String tpmUuid = Q.New(TpmVO.class)
492+
.eq(TpmVO_.vmInstanceUuid, inv.getUuid())
493+
.select(TpmVO_.uuid)
494+
.findValue();
495+
if (tpmUuid == null) {
496+
return;
497+
}
498+
499+
// Delete host secrets while TPM row and key version are still resolvable.
500+
// RemoveTpm may skip or fail (e.g. VM not in Stopped), and callback order may change.
501+
Integer keyVersion = resourceKeyBackend.findKeyVersionByTpm(tpmUuid);
502+
Set<String> hostUuids = new HashSet<>();
503+
List<VmHostFileVO> tpmStateFiles = Q.New(VmHostFileVO.class)
504+
.eq(VmHostFileVO_.vmInstanceUuid, inv.getUuid())
505+
.eq(VmHostFileVO_.type, VmHostFileType.TpmState)
506+
.list();
507+
for (VmHostFileVO f : tpmStateFiles) {
508+
if (StringUtils.isNotBlank(f.getHostUuid())) {
509+
hostUuids.add(f.getHostUuid());
510+
}
511+
}
512+
if (StringUtils.isNotBlank(inv.getHostUuid())) {
513+
hostUuids.add(inv.getHostUuid());
514+
}
515+
if (StringUtils.isNotBlank(inv.getLastHostUuid())) {
516+
hostUuids.add(inv.getLastHostUuid());
517+
}
518+
for (String hostUuid : hostUuids) {
519+
deleteHostSecretBestEffort(hostUuid, inv.getUuid(), keyVersion, "vm-just-before-delete-from-db");
520+
}
521+
522+
RemoveTpmMsg removeMsg = new RemoveTpmMsg();
523+
removeMsg.setVmInstanceUuid(inv.getUuid());
524+
removeMsg.setTpmUuid(tpmUuid);
525+
bus.makeTargetServiceIdByResourceUuid(removeMsg, SERVICE_ID, removeMsg.getTpmUuid());
526+
MessageReply reply = bus.call(removeMsg);
527+
if (!reply.isSuccess()) {
528+
logger.warn(String.format("failed to remove TPM[uuid:%s] of VM[uuid:%s], error: %s",
529+
tpmUuid, inv.getUuid(), reply.getError()));
530+
}
531+
}
532+
480533
private void clearRollbackInfo(VmInstanceSpec spec) {
481534
if (spec.getDevicesSpec() == null || spec.getDevicesSpec().getTpm() == null) {
482535
return;
@@ -530,22 +583,68 @@ public void afterMigrateVm(VmInstanceInventory inv, String srcHostUuid, NoErrorC
530583

531584
@Override
532585
public void vmStateChanged(VmInstanceInventory vm, VmInstanceState oldState, VmInstanceState newState) {
586+
String vmUuid = vm == null ? null : vm.getUuid();
587+
if (StringUtils.isBlank(vmUuid)) {
588+
logger.info(String.format("vmStateChanged skip: vmUuid is blank, oldState=%s, newState=%s", oldState, newState));
589+
return;
590+
}
591+
592+
// Record source host when storage migration starts. In some end-state events (e.g. VolumeMigrating->Stopped),
593+
// inventory host fields may not carry both src/dst host values reliably.
594+
if (newState == VmInstanceState.VolumeMigrating) {
595+
String srcHostUuid = vm.getLastHostUuid();
596+
if (StringUtils.isBlank(srcHostUuid)) {
597+
srcHostUuid = Q.New(VmInstanceVO.class)
598+
.select(VmInstanceVO_.lastHostUuid)
599+
.eq(VmInstanceVO_.uuid, vmUuid)
600+
.findValue();
601+
}
602+
if (StringUtils.isNotBlank(srcHostUuid)) {
603+
volumeMigratingSourceHostCache.put(vmUuid, srcHostUuid);
604+
logger.info(String.format(
605+
"vmStateChanged cache volume-migrating src host: vm[uuid:%s], oldState=%s, newState=%s, srcHostUuid=%s",
606+
vmUuid, oldState, newState, srcHostUuid));
607+
} else {
608+
logger.info(String.format(
609+
"vmStateChanged skip cache: source host is blank, vm[uuid:%s], oldState=%s, newState=%s",
610+
vmUuid, oldState, newState));
611+
}
612+
return;
613+
}
614+
533615
if (oldState != VmInstanceState.VolumeMigrating) {
534616
return;
535617
}
536618

537-
String vmUuid = vm == null ? null : vm.getUuid();
538-
String srcHostUuid = vm == null ? null : vm.getLastHostUuid();
539-
String destHostUuid = vm == null ? null : vm.getHostUuid();
540-
if (StringUtils.isBlank(vmUuid) || StringUtils.isBlank(srcHostUuid) || srcHostUuid.equals(destHostUuid)) {
619+
String srcHostUuid = volumeMigratingSourceHostCache.remove(vmUuid);
620+
if (StringUtils.isBlank(srcHostUuid)) {
621+
logger.info(String.format(
622+
"vmStateChanged skip delete: no cached source host, vm[uuid:%s], oldState=%s, newState=%s",
623+
vmUuid, oldState, newState));
541624
return;
542625
}
543626

544-
if (!isVmCurrentlyOnExpectedHost(vmUuid, destHostUuid)) {
627+
String destHostUuid = Q.New(VmInstanceVO.class)
628+
.select(VmInstanceVO_.hostUuid)
629+
.eq(VmInstanceVO_.uuid, vmUuid)
630+
.findValue();
631+
if (StringUtils.isBlank(destHostUuid)) {
632+
destHostUuid = Q.New(VmInstanceVO.class)
633+
.select(VmInstanceVO_.lastHostUuid)
634+
.eq(VmInstanceVO_.uuid, vmUuid)
635+
.findValue();
636+
}
637+
if (StringUtils.isBlank(destHostUuid) || srcHostUuid.equals(destHostUuid)) {
638+
logger.info(String.format(
639+
"vmStateChanged skip delete: invalid host mapping, vm[uuid:%s], oldState=%s, newState=%s, srcHostUuid=%s, destHostUuid=%s",
640+
vmUuid, oldState, newState, srcHostUuid, destHostUuid));
545641
return;
546642
}
547643

548644
Integer keyVersion = findTpmKeyVersionByVmUuid(vmUuid);
645+
logger.info(String.format(
646+
"vmStateChanged trigger delete: vm[uuid:%s], srcHostUuid=%s, destHostUuid=%s, keyVersion=%s, reason=volume-migrated-host-change",
647+
vmUuid, srcHostUuid, destHostUuid, keyVersion));
549648
deleteHostSecretBestEffort(srcHostUuid, vmUuid, keyVersion, "volume-migrated-host-change");
550649
}
551650

@@ -604,9 +703,15 @@ private static boolean isVtpmSecretNotFoundOnHost(ErrorCode errorCode) {
604703

605704
private void deleteHostSecretBestEffort(String hostUuid, String vmUuid, Integer keyVersion, String reason) {
606705
if (StringUtils.isBlank(hostUuid) || StringUtils.isBlank(vmUuid) || keyVersion == null) {
706+
logger.info(String.format(
707+
"skip delete host secret: reason=%s, hostUuid=%s, vmUuid=%s, keyVersion=%s",
708+
reason, hostUuid, vmUuid, keyVersion));
607709
return;
608710
}
609711

712+
logger.info(String.format(
713+
"send delete host secret: reason=%s, hostUuid=%s, vmUuid=%s, keyVersion=%s",
714+
reason, hostUuid, vmUuid, keyVersion));
610715
SecretHostDeleteMsg dmsg = new SecretHostDeleteMsg();
611716
dmsg.setHostUuid(hostUuid);
612717
dmsg.setVmUuid(vmUuid);

0 commit comments

Comments
 (0)