Skip to content

<fix>[kvm]: backport TLS reconnect PID fix #4465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MatheMatrix wants to merge 8 commits into from
Closed

<fix>[kvm]: backport TLS reconnect PID fix #4465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
56927a6
<feature>[kvm]: add libvirt TLS config
huhu0316 Mar 11, 2026
57b77b7
<fix>[kvm]: update TLS certs via kvmagent on host reconnect
huhu0316 Apr 7, 2026
a2d2b80
<fix>[kvm]: use SSH cert check + ansible deploy instead of kvmagent H…
huhu0316 Apr 13, 2026
71b0984
<fix>[kvm]: unify TLS cert IP collection to avoid reconnect-triggered…
huhu0316 Apr 21, 2026
93340f6
<fix>[kvm]: decouple TLS cert detection from libvirtd restart toggle
huhu0316 Apr 27, 2026
b63451a
<fix>[kvm]: exclude br_conn_all_ns IP from TLS cert SAN
huhu0316 Apr 29, 2026
a258599
<fix>[kvm]: fix tls gray version for 5.4.10
huhu0316 Jul 6, 2026
59bf199
<fix>[kvm]: clean tls migrate test line endings
huhu0316 Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3741,6 +3741,26 @@ public static class MigrateVmCmd extends AgentCommand implements HasThreadContex
private boolean reload;
@GrayVersion(value = "5.0.0")
private long bandwidth;
@GrayVersion(value = "5.4.0")
private boolean useTls;
@GrayVersion(value = "5.4.0")
private String srcHostManagementIp;

public String getSrcHostManagementIp() {
return srcHostManagementIp;
}

public void setSrcHostManagementIp(String srcHostManagementIp) {
this.srcHostManagementIp = srcHostManagementIp;
}

public boolean isUseTls() {
return useTls;
}

public void setUseTls(boolean useTls) {
this.useTls = useTls;
}

public Integer getDownTime() {
return downTime;
Expand Down
1 change: 1 addition & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public interface KVMConstant {
String KVM_DELETE_CONSOLE_FIREWALL_PATH = "/vm/console/deletefirewall";
String KVM_UPDATE_HOST_OS_PATH = "/host/updateos";
String KVM_HOST_UPDATE_DEPENDENCY_PATH = "/host/updatedependency";

String HOST_SHUTDOWN = "/host/shutdown";
String HOST_REBOOT = "/host/reboot";
String HOST_UPDATE_SPICE_CHANNEL_CONFIG_PATH = "/host/updateSpiceChannelConfig";
Expand Down
4 changes: 4 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMGlobalConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ public class KVMGlobalConfig {
@BindResourceConfig({HostVO.class, ClusterVO.class})
public static GlobalConfig RECONNECT_HOST_RESTART_LIBVIRTD_SERVICE = new GlobalConfig(CATEGORY, "reconnect.host.restart.libvirtd.service");

@GlobalConfigValidation(validValues = {"true", "false"})
@GlobalConfigDef(defaultValue = "true", type = Boolean.class, description = "enable TLS encryption for libvirt remote connections (migration)")
public static GlobalConfig LIBVIRT_TLS_ENABLED = new GlobalConfig(CATEGORY, "libvirt.tls.enabled");

@GlobalConfigValidation
public static GlobalConfig KVMAGENT_PHYSICAL_MEMORY_USAGE_ALARM_THRESHOLD = new GlobalConfig(CATEGORY, "kvmagent.physicalmemory.usage.alarm.threshold");

Expand Down
106 changes: 106 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.zstack.core.db.SQLBatch;
import org.zstack.core.db.SimpleQuery;
import org.zstack.core.db.SimpleQuery.Op;
import org.zstack.core.jsonlabel.JsonLabel;
import org.zstack.core.thread.*;
import org.zstack.core.timeout.ApiTimeoutManager;
import org.zstack.core.timeout.TimeHelper;
Expand Down Expand Up @@ -126,6 +127,20 @@ public class KVMHost extends HostBase implements Host {
protected static OperationChecker allowedOperations = new OperationChecker(true);
protected static OperationChecker skipOperations = new OperationChecker(true);

public static Set<String> parseSanIps(String sanOutput) {
Set<String> sanIps = new HashSet<>();
if (sanOutput == null || sanOutput.isEmpty()) {
return sanIps;
}
for (String line : sanOutput.split(",|\n")) {
String trimmed = line.trim();
if (trimmed.startsWith("IP Address:")) {
sanIps.add(trimmed.substring("IP Address:".length()).trim());
}
}
return sanIps;
}

@Autowired
@Qualifier("KVMHostFactory")
protected KVMHostFactory factory;
Expand Down Expand Up @@ -3165,6 +3180,7 @@ public void run(final FlowTrigger trigger, Map data) {
cmd.setDestHostIp(dstHostMigrateIp);
cmd.setSrcHostIp(srcHostMigrateIp);
cmd.setDestHostManagementIp(dstHostMnIp);
cmd.setSrcHostManagementIp(srcHostMnIp);
cmd.setMigrateFromDestination(migrateFromDestination);
cmd.setStorageMigrationPolicy(storageMigrationPolicy == null ? null : storageMigrationPolicy.toString());
cmd.setVmUuid(vmUuid);
Expand All @@ -3176,6 +3192,8 @@ public void run(final FlowTrigger trigger, Map data) {
cmd.setDownTime(s.downTime);
cmd.setBandwidth(s.bandwidth);
cmd.setNics(nicTos);
cmd.setUseTls(KVMGlobalConfig.LIBVIRT_TLS_ENABLED.value(Boolean.class)
&& rcf.getResourceConfigValue(KVMGlobalConfig.RECONNECT_HOST_RESTART_LIBVIRTD_SERVICE, self.getUuid(), Boolean.class));

if (s.diskMigrationMap != null) {
Map<String, VolumeTO> diskMigrationMap = new HashMap<>();
Expand Down Expand Up @@ -5663,6 +5681,72 @@ public void run(FlowTrigger trigger, Map data) {
}
});

flow(new NoRollbackFlow() {
String __name__ = "check-tls-certs-if-needed";

@Override
public boolean skip(Map data) {
// ZSTAC-84446: run detection whenever TLS is enabled so check
// and first-deploy share the same IP source.
return CoreGlobalProperty.UNIT_TEST_ON
|| !KVMGlobalConfig.LIBVIRT_TLS_ENABLED.value(Boolean.class);
}

@Override
public void run(FlowTrigger trigger, Map data) {
// ZSTAC-84446: detection is best-effort. SSH failures must NOT
// break reconnect; on error we skip and let the deploy step
// fall back to mgmtIp + EXTRA_IPS.
try {
String managementIp = getSelf().getManagementIp();

SshShell sshShell = new SshShell();
sshShell.setHostname(managementIp);
sshShell.setUsername(getSelf().getUsername());
sshShell.setPassword(getSelf().getPassword());
sshShell.setPort(getSelf().getPort());

// Same logic as zstack-utility host_plugin.fact() so MN's
// expectation matches what the host itself reports.
String certIpList = KVMHostUtils.collectHostIps(
sshShell, self.getUuid(), managementIp);
List<String> allIps = new ArrayList<>(Arrays.asList(certIpList.split(",")));
// Save detected IPs so apply-ansible-playbook can union with
// EXTRA_IPS without running a second SSH.
data.put("TLS_DETECTED_IPS", certIpList);

SshResult sanResult = sshShell.runCommand(
"openssl x509 -in /etc/pki/libvirt/servercert.pem -noout -ext subjectAltName 2>/dev/null");

boolean needDeploy = false;
if (sanResult.isSshFailure() || sanResult.getReturnCode() != 0
|| sanResult.getStdout() == null || sanResult.getStdout().trim().isEmpty()) {
logger.info(String.format("TLS cert not found or unreadable on host[uuid:%s], need deploy", self.getUuid()));
needDeploy = true;
} else {
Set<String> sanIps = parseSanIps(sanResult.getStdout());
for (String ip : allIps) {
if (!sanIps.contains(ip)) {
logger.info(String.format("TLS cert SAN missing IP %s on host[uuid:%s], need deploy", ip, self.getUuid()));
needDeploy = true;
break;
}
}
}

if (needDeploy) {
data.put("NEED_DEPLOY_TLS_CERT", true);
}
} catch (Exception e) {
logger.warn(String.format(
"TLS cert detection failed on host[uuid:%s], continue connect flow: %s",
self.getUuid(), e.getMessage()), e);
}

trigger.next();
}
});

flow(new NoRollbackFlow() {
String __name__ = "apply-ansible-playbook";

Expand Down Expand Up @@ -5797,6 +5881,27 @@ public void run(final FlowTrigger trigger, Map data) {
deployArguments.setSkipPackages(info.getSkipPackages());
deployArguments.setUpdatePackages(String.valueOf(CoreGlobalProperty.UPDATE_PKG_WHEN_CONNECT));

String managementIp = getSelf().getManagementIp();
String detectedIps = (String) data.get("TLS_DETECTED_IPS");
String tlsCertIps = KVMHostUtils.unionTlsCertIps(
self.getUuid(), managementIp, detectedIps);
deployArguments.setTlsCertIps(tlsCertIps);

// ZSTAC-84446: force ansible re-run only when policy allows;
// see KVMHostUtils#shouldForceTlsRedeploy.
Boolean needDeployTlsCert = (Boolean) data.get("NEED_DEPLOY_TLS_CERT");
boolean allowRestart = rcf.getResourceConfigValue(
KVMGlobalConfig.RECONNECT_HOST_RESTART_LIBVIRTD_SERVICE,
self.getUuid(), Boolean.class);
if (KVMHostUtils.shouldForceTlsRedeploy(
Boolean.TRUE.equals(needDeployTlsCert), allowRestart, info.isNewAdded())) {
runner.setForceRun(true);
deployArguments.setRestartLibvirtd("true");
} else if (Boolean.TRUE.equals(needDeployTlsCert)) {
logger.info(String.format("TLS cert needs deploy on host[uuid:%s], skip " +
"force-run to keep kvmagent PID stable", self.getUuid()));
}

if (deployArguments.isForceRun()) {
runner.setForceRun(true);
}
Expand Down Expand Up @@ -5989,6 +6094,7 @@ public void fail(ErrorCode errorCode) {

flow(createCollectHostFactsFlow(info));


if (info.isNewAdded()) {
flow(new NoRollbackFlow() {
String __name__ = "check-qemu-libvirt-version";
Expand Down
10 changes: 10 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostDeployArguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public class KVMHostDeployArguments extends SyncTimeRequestedDeployArguments {
private String restartLibvirtd;
@SerializedName("extra_packages")
private String extraPackages;
@SerializedName("tls_cert_ips")
private String tlsCertIps;

private transient boolean forceRun = false;

Expand Down Expand Up @@ -134,6 +136,14 @@ public void setExtraPackages(String extraPackages) {
this.extraPackages = extraPackages;
}

public String getTlsCertIps() {
return tlsCertIps;
}

public void setTlsCertIps(String tlsCertIps) {
this.tlsCertIps = tlsCertIps;
}

public String getEnableSpiceTls() {
return enableSpiceTls;
}
Expand Down
57 changes: 57 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package org.zstack.kvm;

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.web.util.UriComponentsBuilder;
import org.zstack.compute.host.HostGlobalConfig;
import org.zstack.compute.vm.CrashStrategy;
import org.zstack.compute.vm.VmGlobalConfig;
import org.zstack.compute.vm.VmNicManager;
import org.zstack.core.jsonlabel.JsonLabel;
import org.zstack.core.jsonlabel.JsonLabelInventory;
import org.zstack.core.CoreGlobalProperty;
import org.zstack.core.ansible.AnsibleFacade;
import org.zstack.core.cloudbus.CloudBus;
Expand Down Expand Up @@ -75,6 +78,7 @@
import org.zstack.resourceconfig.ResourceConfigFacade;
import org.zstack.utils.CollectionUtils;
import org.zstack.utils.IpRangeSet;
import org.zstack.utils.ShellUtils;
import org.zstack.utils.SizeUtils;
import org.zstack.utils.Utils;
import org.zstack.utils.data.SizeUnit;
Expand All @@ -85,6 +89,7 @@
import org.zstack.utils.logging.CLogger;

import javax.persistence.Tuple;
import java.io.File;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
Expand Down Expand Up @@ -121,6 +126,10 @@ public class KVMHostFactory extends AbstractService implements HypervisorFactory
HypervisorMessageFactory {
private static final CLogger logger = Utils.getLogger(KVMHostFactory.class);

private static final String LIBVIRT_TLS_CA_KEY = "libvirtTLSCA";
private static final String LIBVIRT_TLS_PRIVATE_KEY = "libvirtTLSPrivateKey";
private static final String CA_DIR = "/var/lib/zstack/pki/CA";

public static final HypervisorType hypervisorType = new HypervisorType(KVMConstant.KVM_HYPERVISOR_TYPE);
public static final VolumeFormat QCOW2_FORMAT = new VolumeFormat(VolumeConstant.VOLUME_FORMAT_QCOW2, hypervisorType);
public static final VolumeFormat RAW_FORMAT = new VolumeFormat(VolumeConstant.VOLUME_FORMAT_RAW, hypervisorType);
Expand Down Expand Up @@ -457,8 +466,56 @@ private void processKvmagentPhysicalMemUsageAbnormal(HostProcessPhysicalMemoryUs
bus.send(restartKvmAgentMsg);
}

private void initLibvirtTlsCA() {
if (CoreGlobalProperty.UNIT_TEST_ON) {
return;
}

try {
ShellUtils.run(String.format("mkdir -p %s", CA_DIR));
ShellUtils.run("chown -R zstack:zstack /var/lib/zstack/pki");

File caFile = new File(CA_DIR + "/cacert.pem");
File keyFile = new File(CA_DIR + "/cakey.pem");

// Local CA missing - generate with openssl
// NOTE: ShellUtils.run() prepends sudo only to the first command in &&-chains,
// so each command must be a separate call.
if (!caFile.exists() || !keyFile.exists()) {
ShellUtils.run(String.format(
"openssl genrsa -out %s/cakey.pem 4096", CA_DIR));
ShellUtils.run(String.format(
"openssl req -new -x509 -days 3650 -key %s/cakey.pem " +
"-out %s/cacert.pem -subj '/O=ZStack/CN=ZStack Libvirt CA'",
CA_DIR, CA_DIR));
ShellUtils.run(String.format("chown zstack:zstack %s/cakey.pem %s/cacert.pem",
CA_DIR, CA_DIR));
ShellUtils.run(String.format("chmod 600 %s/cakey.pem", CA_DIR));
ShellUtils.run(String.format("chmod 644 %s/cacert.pem", CA_DIR));
}

String ca = FileUtils.readFileToString(caFile).trim();
String key = FileUtils.readFileToString(keyFile).trim();

// createIfAbsent: DB has no record -> write; DB has record -> return DB value
JsonLabelInventory caInv = new JsonLabel().createIfAbsent(LIBVIRT_TLS_CA_KEY, ca);
JsonLabelInventory keyInv = new JsonLabel().createIfAbsent(LIBVIRT_TLS_PRIVATE_KEY, key);

// Use DB as source of truth - overwrite local files (HA: MN2 uses MN1's CA from DB)
FileUtils.writeStringToFile(caFile, caInv.getLabelValue());
FileUtils.writeStringToFile(keyFile, keyInv.getLabelValue());
Comment on lines +497 to +506

@coderabbitai coderabbitai Bot Jul 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant factory code and nearby context
sed -n '470,540p' plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java

# Find JsonLabel usage and implementation
rg -n "class JsonLabel|createIfAbsent\(|LIBVIRT_TLS_PRIVATE_KEY|LIBVIRT_TLS_CA_KEY|JsonLabel" -S .

# Inspect the JsonLabel class and related storage paths if present
files=$(git ls-files | rg 'JsonLabel|jsonlabel|Label')
printf '%s\n' "$files"

Repository: MatheMatrix/zstack

Length of output: 20622

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect JsonLabel storage semantics
sed -n '1,180p' core/src/main/java/org/zstack/core/jsonlabel/JsonLabel.java

# Inspect JsonLabelVO persistence fields
sed -n '1,140p' core/src/main/java/org/zstack/core/jsonlabel/JsonLabelVO.java

# Inspect the schema definition for JsonLabelVO
sed -n '120,170p' conf/db/upgrade/V1.7__schema.sql

# Inspect any later schema changes affecting labelValue
sed -n '1,40p' conf/db/upgrade/V3.9.0__schema.sql

Repository: MatheMatrix/zstack

Length of output: 9348

CA 私钥不要明文落库
cakey.pem 通过 JsonLabel.createIfAbsent() 直接写入 JsonLabelVO.labelValue，该字段只是普通 text/MEDIUMTEXT，没有加密或脱敏；数据库访问面一旦暴露，CA 私钥就会泄露。若需要跨管理节点共享，改为加密存储或受控密钥分发。

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java` around lines 497
- 506, The KVMHostFactory TLS sync logic is persisting the CA private key in
plain text through JsonLabel.createIfAbsent() and JsonLabelVO.labelValue; update
this flow to avoid storing the raw key directly. In the block that reads
cakey.pem and writes via LIBVIRT_TLS_PRIVATE_KEY, switch to encrypted storage or
a controlled secret-distribution mechanism, and only write/decrypt the key when
restoring local files. Ensure the handling around createIfAbsent(), caInv, and
keyInv no longer exposes the private key in the database.

ShellUtils.run(String.format("chmod 600 %s/cakey.pem", CA_DIR));
ShellUtils.run(String.format("chmod 644 %s/cacert.pem", CA_DIR));

logger.info("Libvirt TLS CA initialized and persisted to database");
} catch (Exception e) {
logger.warn("Failed to initialize libvirt TLS CA", e);
}
}

@Override
public boolean start() {
initLibvirtTlsCA();
deployAnsibleModule();
populateExtensions();
configKVMDeviceType();
Expand Down
Loading