diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 9670b6e7c13a..76c040af002f 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -39,3 +39,13 @@ example.ver.1 > example.ver.2: which can now be attached to Instances. This is to prevent the Secondary Storage to grow to enormous sizes as Linux Distributions keep growing in size while a stripped down Linux should fit on a 2.88MB floppy. + + +4.22.0.0 > 4.23.0.0: + * KVM/Ceph: RBD volumes can now be encrypted at rest using native librbd + LUKS2 (), for both data disks + and root disks. Encryption is transparent to the guest and reuses the + existing CloudStack volume-encryption passphrase handling, so no + additional key store is required. Note: attaching an encrypted RBD volume + to a running Instance requires libvirt >= 10.1.0; booting an Instance from + an encrypted RBD root disk works on older libvirt. diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java index 275fd41edc34..d9b9107ba20b 100644 --- a/agent/src/main/java/com/cloud/agent/Agent.java +++ b/agent/src/main/java/com/cloud/agent/Agent.java @@ -969,6 +969,7 @@ private Answer migrateAgentToOtherMS(final MigrateAgentConnectionCommand cmd) { } ScheduledExecutorService migrateAgentConnectionService = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("MigrateAgentConnection-Job")); migrateAgentConnectionService.schedule(() -> { + waitForCommandsToFinish(MIGRATE_AGENT_CONNECTION_MAX_WAIT_SECS); migrateAgentConnection(cmd.getAvoidMsList()); }, 3, TimeUnit.SECONDS); migrateAgentConnectionService.shutdown(); @@ -980,6 +981,37 @@ private Answer migrateAgentToOtherMS(final MigrateAgentConnectionCommand cmd) { return new MigrateAgentConnectionAnswer(true); } + private static final int MIGRATE_AGENT_CONNECTION_MAX_WAIT_SECS = 300; + + /** + * Answers travel back over the current link, so a command still running or queued when the agent reconnects to + * another management server would lose its answer and fail there. Wait until nothing has run or been queued for two + * consecutive checks; the management server holds new commands for this host while it is Rebalancing. + */ + private void waitForCommandsToFinish(final int maxWaitSecs) { + final long deadline = System.currentTimeMillis() + maxWaitSecs * 1000L; + int idleChecks = 0; + while (idleChecks < 2) { + final int queued = requestHandler instanceof ThreadPoolExecutor ? ((ThreadPoolExecutor)requestHandler).getQueue().size() : 0; + idleChecks = (commandsInProgress.get() == 0 && queued == 0) ? idleChecks + 1 : 0; + if (idleChecks >= 2) { + break; + } + if (System.currentTimeMillis() >= deadline) { + logger.warn("Moving the agent connection with {} commands still in progress and {} queued after {} s", + commandsInProgress.get(), queued, maxWaitSecs); + return; + } + try { + Thread.sleep(1000); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } + logger.info("No commands in progress, moving the agent connection"); + } + private void migrateAgentConnection(List avoidMsList) { final String[] msHosts = shell.getHosts(); if (msHosts == null || msHosts.length < 1) { diff --git a/api/src/main/java/com/cloud/host/Host.java b/api/src/main/java/com/cloud/host/Host.java index c110e4ca94e1..cca2edd70c6b 100644 --- a/api/src/main/java/com/cloud/host/Host.java +++ b/api/src/main/java/com/cloud/host/Host.java @@ -56,6 +56,7 @@ public static String[] toStrings(Host.Type... types) { String HOST_UEFI_ENABLE = "host.uefi.enable"; String HOST_VOLUME_ENCRYPTION = "host.volume.encryption"; + String HOST_RBD_VOLUME_ENCRYPTION = "host.volume.encryption.rbd"; String HOST_INSTANCE_CONVERSION = "host.instance.conversion"; String HOST_VDDK_SUPPORT = "host.vddk.support"; String HOST_VDDK_LIB_DIR = "vddk.lib.dir"; diff --git a/api/src/main/java/com/cloud/storage/Storage.java b/api/src/main/java/com/cloud/storage/Storage.java index 3511b4e88cb9..275f6d25268d 100644 --- a/api/src/main/java/com/cloud/storage/Storage.java +++ b/api/src/main/java/com/cloud/storage/Storage.java @@ -172,7 +172,7 @@ public static enum StoragePoolType { LVM(false, false, EncryptionSupport.Unsupported), // XenServer local LVM SR CLVM(true, false, EncryptionSupport.Unsupported), CLVM_NG(true, false, EncryptionSupport.Hypervisor), - RBD(true, true, EncryptionSupport.Unsupported), // http://libvirt.org/storage.html#StorageBackendRBD + RBD(true, true, EncryptionSupport.Hypervisor), // http://libvirt.org/storage.html#StorageBackendRBD ; encrypted natively by librbd (LUKS2, engine='librbd') SharedMountPoint(true, true, EncryptionSupport.Hypervisor), VMFS(true, true, EncryptionSupport.Unsupported), // VMware VMFS storage PreSetup(true, true, EncryptionSupport.Unsupported), // for XenServer, Storage Pool is set up by customers. diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java index bd72f248364e..3d2058c523a3 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java @@ -327,6 +327,10 @@ public Integer getSourcePortEnd() { } public String getProtocol() { + // a UDP load balancer needs a UDP firewall rule and must not clash with UDP port forwarding on the same port + if (NetUtils.UDP_PROTO.equals(getLbProtocol())) { + return NetUtils.UDP_PROTO; + } return NetUtils.TCP_PROTO; } diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/LoadBalancerConfigItem.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/LoadBalancerConfigItem.java index 6dae886b4137..71e68deba611 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/LoadBalancerConfigItem.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/LoadBalancerConfigItem.java @@ -58,6 +58,7 @@ public List generateConfig(final NetworkElementCommand cmd) { final LoadBalancerRule loadBalancerRule = new LoadBalancerRule(configuration, tmpCfgFilePath, tmpCfgFileName, addRules, removeRules, statRules, routerIp); final LoadBalancerRule.SslCertEntry[] sslCerts = cfgtr.generateSslCertEntries(command); loadBalancerRule.setSslCerts(sslCerts); + loadBalancerRule.setUdpRules(cfgtr.generateUdpEntries(command)); final List rules = new LinkedList(); rules.add(loadBalancerRule); diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/LoadBalancerRule.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/LoadBalancerRule.java index 361c4765cc52..1e6623f94719 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/LoadBalancerRule.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/LoadBalancerRule.java @@ -26,6 +26,7 @@ public class LoadBalancerRule { private String tmpCfgFilePath; private String tmpCfgFileName; private SslCertEntry[] sslCerts; + private UdpLbEntry[] udpRules; private String[] addRules; private String[] removeRules; @@ -80,6 +81,50 @@ public String getPassword() { } } + /** + * A UDP load balancer rule. The router serves it with IPVS (NAT mode), not haproxy. + */ + public static class UdpLbEntry { + private String sourceIp; + private int sourcePort; + // IPVS scheduler: rr, lc or sh + private String scheduler; + // IPVS persistence timeout in seconds, 0 = none + private int persistence; + // "ip:port" of each backend + private String[] destinations; + // allowed source CIDRs, empty = everyone + private String[] cidrs; + + public UdpLbEntry(String sourceIp, int sourcePort, String scheduler, int persistence, String[] destinations, String[] cidrs) { + this.sourceIp = sourceIp; + this.sourcePort = sourcePort; + this.scheduler = scheduler; + this.persistence = persistence; + this.destinations = destinations; + this.cidrs = cidrs; + } + + public String getSourceIp() { + return sourceIp; + } + public int getSourcePort() { + return sourcePort; + } + public String getScheduler() { + return scheduler; + } + public int getPersistence() { + return persistence; + } + public String[] getDestinations() { + return destinations; + } + public String[] getCidrs() { + return cidrs; + } + } + public LoadBalancerRule() { // Empty constructor for (de)serialization } @@ -157,4 +202,12 @@ public SslCertEntry[] getSslCerts() { public void setSslCerts(final SslCertEntry[] sslCerts) { this.sslCerts = sslCerts; } + + public UdpLbEntry[] getUdpRules() { + return udpRules; + } + + public void setUdpRules(final UdpLbEntry[] udpRules) { + this.udpRules = udpRules; + } } diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java index 7d544c2e49c0..d1b9b8ce046d 100644 --- a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java @@ -37,6 +37,7 @@ import com.cloud.agent.api.to.LoadBalancerTO.StickinessPolicyTO; import com.cloud.agent.api.to.PortForwardingRuleTO; import com.cloud.agent.resource.virtualnetwork.model.LoadBalancerRule.SslCertEntry; +import com.cloud.agent.resource.virtualnetwork.model.LoadBalancerRule.UdpLbEntry; import com.cloud.network.lb.LoadBalancingRule.LbSslCert; import com.cloud.network.rules.LbStickinessMethod.StickinessMethodType; import com.cloud.utils.Pair; @@ -683,7 +684,7 @@ public String[] generateConfiguration(final LoadBalancerConfigCommand lbCmd) { result.add(blankLine); boolean has_listener = false; for (final LoadBalancerTO lbTO : lbCmd.getLoadBalancers()) { - if (lbTO.isRevoked()) { + if (lbTO.isRevoked() || isUdp(lbTO)) { continue; } final List poolRules = getRulesForPool(lbTO, lbCmd); @@ -708,6 +709,9 @@ public String[][] generateFwRules(final LoadBalancerConfigCommand lbCmd) { final Set toStats = new HashSet(); for (final LoadBalancerTO lbTO : lbCmd.getLoadBalancers()) { + if (isUdp(lbTO)) { + continue; + } final StringBuilder sb = new StringBuilder(); sb.append(lbTO.getSrcIp()).append(":"); @@ -764,4 +768,73 @@ private void addSslCertEntry(Set sslCertEntries, LoadBalancerTO lb final SslCertEntry sslCertEntry = new SslCertEntry(name, cert.getCert(), cert.getKey(), cert.getChain(), cert.getPassword()); sslCertEntries.add(sslCertEntry); } + + private static boolean isUdp(final LoadBalancerTO lbTO) { + return NetUtils.UDP_PROTO.equalsIgnoreCase(lbTO.getLbProtocol()); + } + + @Override + public UdpLbEntry[] generateUdpEntries(final LoadBalancerConfigCommand lbCmd) { + final List result = new ArrayList<>(); + for (final LoadBalancerTO lbTO : lbCmd.getLoadBalancers()) { + if (lbTO.isRevoked() || !isUdp(lbTO)) { + continue; + } + final List destinations = new ArrayList<>(); + for (final DestinationTO dest : lbTO.getDestinations()) { + if (!dest.isRevoked()) { + destinations.add(dest.getDestIp() + ":" + dest.getDestPort()); + } + } + final String[] cidrs = StringUtils.isBlank(lbTO.getCidrList()) ? new String[0] : lbTO.getCidrList().trim().split("\\s+"); + result.add(new UdpLbEntry(lbTO.getSrcIp(), lbTO.getSrcPort(), getIpvsScheduler(lbTO.getAlgorithm()), getIpvsPersistence(lbTO), + destinations.toArray(new String[0]), cidrs)); + } + return result.toArray(new UdpLbEntry[0]); + } + + protected static String getIpvsScheduler(final String algorithm) { + if ("leastconn".equals(algorithm)) { + return "lc"; + } + if ("source".equals(algorithm)) { + return "sh"; + } + return "rr"; + } + + /** + * Source based stickiness becomes IPVS persistence; its "expire" value (default 3h, as for haproxy) is the timeout. + */ + protected static int getIpvsPersistence(final LoadBalancerTO lbTO) { + if (lbTO.getStickinessPolicies() == null) { + return 0; + } + for (final StickinessPolicyTO policy : lbTO.getStickinessPolicies()) { + if (policy == null || !StickinessMethodType.SourceBased.getName().equalsIgnoreCase(policy.getMethodName())) { + continue; + } + String expire = "3h"; + for (final Pair param : policy.getParams()) { + if ("expire".equalsIgnoreCase(param.first()) && StringUtils.isNotBlank(param.second())) { + expire = param.second().trim(); + } + } + return durationToSeconds(expire); + } + return 0; + } + + private static int durationToSeconds(final String duration) { + final char unit = duration.charAt(duration.length() - 1); + final int multiplier; + switch (unit) { + case 's': multiplier = 1; break; + case 'm': multiplier = 60; break; + case 'h': multiplier = 3600; break; + case 'd': multiplier = 86400; break; + default: return Integer.parseInt(duration); + } + return Integer.parseInt(duration.substring(0, duration.length() - 1)) * multiplier; + } } diff --git a/core/src/main/java/com/cloud/network/LoadBalancerConfigurator.java b/core/src/main/java/com/cloud/network/LoadBalancerConfigurator.java index 8814f60b0714..839f94dcd486 100644 --- a/core/src/main/java/com/cloud/network/LoadBalancerConfigurator.java +++ b/core/src/main/java/com/cloud/network/LoadBalancerConfigurator.java @@ -24,6 +24,7 @@ import com.cloud.agent.api.routing.LoadBalancerConfigCommand; import com.cloud.agent.api.to.PortForwardingRuleTO; import com.cloud.agent.resource.virtualnetwork.model.LoadBalancerRule.SslCertEntry; +import com.cloud.agent.resource.virtualnetwork.model.LoadBalancerRule.UdpLbEntry; public interface LoadBalancerConfigurator { public final static int ADD = 0; @@ -37,4 +38,6 @@ public interface LoadBalancerConfigurator { public String[][] generateFwRules(LoadBalancerConfigCommand lbCmd); public SslCertEntry[] generateSslCertEntries(LoadBalancerConfigCommand lbCmd); + + public UdpLbEntry[] generateUdpEntries(LoadBalancerConfigCommand lbCmd); } diff --git a/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java b/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java index 972c2eaf7bb4..50bf16360d7d 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java @@ -24,7 +24,9 @@ import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import java.util.Arrays; import java.util.List; +import java.util.Map; public class RestoreBackupCommand extends Command { private String vmName; @@ -40,11 +42,38 @@ public class RestoreBackupCommand extends Command { private Boolean vmExists; private VirtualMachine.State vmState; private Integer mountTimeout; + /** + * Passphrase of every encrypted volume being restored, keyed by volume path. The backup itself is + * LUKS-encrypted (see TakeBackupCommand), so the agent needs these both to read the backup and to + * write the data back through the volume's own encryption. + */ + @LogLevel(LogLevel.Log4jLevel.Off) + private Map volumePassphrases; protected RestoreBackupCommand() { super(); } + public Map getVolumePassphrases() { + return volumePassphrases; + } + + public void setVolumePassphrases(Map volumePassphrases) { + this.volumePassphrases = volumePassphrases; + } + + /** Wipes the passphrases once the command has been handled. */ + public void clearPassphrases() { + if (volumePassphrases == null) { + return; + } + for (byte[] passphrase : volumePassphrases.values()) { + if (passphrase != null) { + Arrays.fill(passphrase, (byte) 0); + } + } + } + public String getVmName() { return vmName; } diff --git a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java index 34f8d7b8bcdd..ae05f28d7595 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/TakeBackupCommand.java @@ -23,7 +23,9 @@ import com.cloud.agent.api.LogLevel; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import java.util.Arrays; import java.util.List; +import java.util.Map; public class TakeBackupCommand extends Command { private String vmName; @@ -35,6 +37,15 @@ public class TakeBackupCommand extends Command { private Boolean quiesce; @LogLevel(LogLevel.Log4jLevel.Off) private String mountOptions; + /** + * Passphrase of every encrypted volume of the VM, keyed by volume path (the uuid the backup + * files are named after). Volumes that are not encrypted are absent. The agent uses these to + * make the backup itself LUKS-encrypted, so an encrypted volume is never written out in the + * clear. Keyed rather than positional because the running-VM path enumerates disks from + * libvirt instead of from {@link #volumePaths}. + */ + @LogLevel(LogLevel.Log4jLevel.Off) + private Map volumePassphrases; // Incremental backup fields (NAS provider; null/empty for legacy full-only callers). private String mode; // "full" or "incremental"; null => legacy behaviour (script default) @@ -53,6 +64,26 @@ public TakeBackupCommand(String vmName, String backupPath) { this.backupPath = backupPath; } + public Map getVolumePassphrases() { + return volumePassphrases; + } + + public void setVolumePassphrases(Map volumePassphrases) { + this.volumePassphrases = volumePassphrases; + } + + /** Wipes the passphrases once the command has been handled. */ + public void clearPassphrases() { + if (volumePassphrases == null) { + return; + } + for (byte[] passphrase : volumePassphrases.values()) { + if (passphrase != null) { + Arrays.fill(passphrase, (byte) 0); + } + } + } + public String getVmName() { return vmName; } diff --git a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java index 073f976719b5..0709a967c7da 100644 --- a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java +++ b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java @@ -32,6 +32,9 @@ import com.cloud.agent.api.to.LoadBalancerTO; import com.cloud.network.lb.LoadBalancingRule.LbDestination; import com.cloud.network.lb.LoadBalancingRule.LbSslCert; +import com.cloud.network.lb.LoadBalancingRule.LbStickinessPolicy; +import com.cloud.agent.resource.virtualnetwork.model.LoadBalancerRule.UdpLbEntry; +import com.cloud.utils.Pair; import java.util.List; import java.util.ArrayList; @@ -158,6 +161,65 @@ public void generateConfigurationTestWithSslCert() { Assert.assertTrue(result.contains("bind 10.2.0.1:443 ssl crt /etc/cloudstack/ssl/10_2_0_1-443.pem")); } + private LoadBalancerConfigCommand udpAndTcpCommand(List stickiness) { + final List dests = new ArrayList<>(); + dests.add(new LbDestination(5353, 5353, "10.1.10.2", false)); + dests.add(new LbDestination(5353, 5353, "10.1.10.3", true)); + LoadBalancerTO udp = new LoadBalancerTO("1", "10.2.0.1", 53, "tcp", "source", false, false, false, dests, stickiness); + udp.setLbProtocol("udp"); + udp.setCidrList("1.1.1.1 2.2.2.0/24"); + LoadBalancerTO tcp = new LoadBalancerTO("2", "10.2.0.1", 22, "tcp", "roundrobin", false, false, false, null); + LoadBalancerTO[] lba = new LoadBalancerTO[] {udp, tcp}; + return new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false, 0L); + } + + @Test + public void generateConfigurationTestUdpRuleNotInHaproxy() { + HAProxyConfigurator hpg = new HAProxyConfigurator(); + LoadBalancerConfigCommand cmd = udpAndTcpCommand(null); + String result = genConfig(hpg, cmd); + Assert.assertTrue(result.contains("listen 10_2_0_1-22")); + Assert.assertFalse(result.contains("10_2_0_1-53")); + + String[][] fwRules = hpg.generateFwRules(cmd); + Assert.assertArrayEquals(new String[] {"10.2.0.1:22:"}, fwRules[LoadBalancerConfigurator.ADD]); + } + + @Test + public void generateUdpEntriesTest() { + HAProxyConfigurator hpg = new HAProxyConfigurator(); + UdpLbEntry[] entries = hpg.generateUdpEntries(udpAndTcpCommand(null)); + Assert.assertEquals(1, entries.length); + Assert.assertEquals("10.2.0.1", entries[0].getSourceIp()); + Assert.assertEquals(53, entries[0].getSourcePort()); + Assert.assertEquals("sh", entries[0].getScheduler()); + Assert.assertEquals(0, entries[0].getPersistence()); + Assert.assertArrayEquals("revoked destinations are left out", new String[] {"10.1.10.2:5353"}, entries[0].getDestinations()); + Assert.assertArrayEquals(new String[] {"1.1.1.1", "2.2.2.0/24"}, entries[0].getCidrs()); + } + + @Test + public void generateUdpEntriesTestSourceStickinessBecomesPersistence() { + List> params = new ArrayList<>(); + params.add(new Pair<>("expire", "30m")); + List stickiness = new ArrayList<>(); + stickiness.add(new LbStickinessPolicy("SourceBased", params)); + UdpLbEntry[] entries = new HAProxyConfigurator().generateUdpEntries(udpAndTcpCommand(stickiness)); + Assert.assertEquals(1800, entries[0].getPersistence()); + + stickiness.clear(); + stickiness.add(new LbStickinessPolicy("SourceBased", new ArrayList<>())); + entries = new HAProxyConfigurator().generateUdpEntries(udpAndTcpCommand(stickiness)); + Assert.assertEquals("default expire is 3h", 10800, entries[0].getPersistence()); + } + + @Test + public void getIpvsSchedulerTest() { + Assert.assertEquals("rr", HAProxyConfigurator.getIpvsScheduler("roundrobin")); + Assert.assertEquals("lc", HAProxyConfigurator.getIpvsScheduler("leastconn")); + Assert.assertEquals("sh", HAProxyConfigurator.getIpvsScheduler("source")); + } + private String genConfig(HAProxyConfigurator hpg, LoadBalancerConfigCommand cmd) { String[] sa = hpg.generateConfiguration(cmd); StringBuilder sb = new StringBuilder(); diff --git a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java index 4d63fae33560..d118588a760c 100644 --- a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java +++ b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java @@ -54,6 +54,10 @@ public interface AgentManager { "This timeout overrides the wait global config. This holds a comma separated key value pairs containing timeout (in seconds) for specific commands. " + "For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300", false); + ConfigKey AgentHandoffWait = new ConfigKey<>("Advanced", Integer.class, "agent.handoff.wait", "180", + "Seconds a command to a host waits while the host's agent is moved to another management server (host in Rebalancing), " + + "instead of failing with the agent unavailable. After that the command is sent as before.", true); + ConfigKey KVMHostDiscoverySshPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class, "kvm.host.discovery.ssh.port", String.valueOf(Host.DEFAULT_SSH_PORT), "SSH port used for KVM host discovery and any other operations on host (using SSH)." + " Please note that this is applicable when port is not defined through host url while adding the KVM host.", true, ConfigKey.Scope.Cluster); @@ -155,6 +159,11 @@ enum TapAgentsAction { boolean isAgentAttached(long hostId); + /** + * @return true when no command sent from this management server to the host is queued or waiting for its answer + */ + boolean isAgentIdle(long hostId); + void disconnectWithoutInvestigation(long hostId, Status.Event event); void disconnectWithInvestigation(long hostId, Status.Event event); diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 1215829d92f8..c30d4e8bfa31 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -75,6 +75,7 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.CheckHealthCommand; import com.cloud.agent.api.Command; +import com.cloud.agent.api.MigrateAgentConnectionCommand; import com.cloud.agent.api.PingAnswer; import com.cloud.agent.api.PingCommand; import com.cloud.agent.api.PingRoutingCommand; @@ -654,6 +655,8 @@ public Answer[] send(final Long hostId, final Commands commands, int timeout) th final Command[] cmds = checkForCommandsAndTag(commands); + waitWhileAgentHandoff(hostId, cmds); + //check what agent is returned. final AgentAttache agent = getAttache(hostId); if (agent == null || agent.isClosed()) { @@ -703,8 +706,57 @@ protected AgentAttache getAttache(final Long hostId) throws AgentUnavailableExce return agent; } + /** + * A planned agent move (management server maintenance) puts the host into Rebalancing while its agent finishes the + * commands it is running and reconnects to another management server. Hold new commands for that host until the + * move is over, so they go to the new owner instead of failing with the agent unavailable. The command that moves + * the agent is never held. + */ + protected void waitWhileAgentHandoff(final Long hostId, final Command[] cmds) { + if (hostId == null || cmds == null) { + return; + } + for (final Command cmd : cmds) { + if (cmd instanceof MigrateAgentConnectionCommand) { + return; + } + } + HostVO host = _hostDao.findById(hostId); + if (host == null || host.getStatus() != Status.Rebalancing) { + return; + } + final long start = System.currentTimeMillis(); + final long deadline = start + AgentHandoffWait.value() * 1000L; + logger.info("Holding {} for {} until its agent has moved to another management server", cmds[0].getClass().getSimpleName(), host); + while (System.currentTimeMillis() < deadline) { + try { + Thread.sleep(500); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + host = _hostDao.findById(hostId); + // Connecting is still part of the move: until the host is Up on its new owner, getAttache() here would not + // switch to a forwarding attache and the command would fail with "not in the right state: Connecting". + if (host == null || (host.getStatus() != Status.Rebalancing && host.getStatus() != Status.Connecting)) { + logger.info("Released {} for {} after {} ms, host is {}", cmds[0].getClass().getSimpleName(), host, + System.currentTimeMillis() - start, host == null ? null : host.getStatus()); + return; + } + } + logger.warn("{} is still Rebalancing after {} s ({}), sending {} anyway", host, AgentHandoffWait.value(), + AgentHandoffWait.key(), cmds[0].getClass().getSimpleName()); + } + + @Override + public boolean isAgentIdle(final long hostId) { + final AgentAttache attache = findAttache(hostId); + return attache == null || (attache.getQueueSize() == 0 && attache.getNonRecurringListenersSize() == 0); + } + @Override public long send(final Long hostId, final Commands commands, final Listener listener) throws AgentUnavailableException { + waitWhileAgentHandoff(hostId, commands.toCommands()); final AgentAttache agent = getAttache(hostId); if (agent.isClosed()) { throw new AgentUnavailableException(String.format( @@ -1713,7 +1765,8 @@ protected void processRequest(final Link link, final Request request) { logger.debug("Not processing {} for agent id={}; can't find the host in the DB", PingRoutingCommand.class.getSimpleName(), cmdHostId); } } - if (host != null && host.getStatus() != Status.Up && gatewayAccessible) { + // A Rebalancing host is moving to another management server; asking for its startup here would pull it back. + if (host != null && host.getStatus() != Status.Up && host.getStatus() != Status.Rebalancing && gatewayAccessible) { requestStartupCommand = true; } final List avoidMsList = _mshostDao.listNonUpStateMsIPs(); @@ -2134,7 +2187,7 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] { CheckTxnBeforeSending, Workers, Port, Wait, AlertWait, DirectAgentLoadSize, DirectAgentPoolSize, DirectAgentThreadCap, EnableKVMAutoEnableDisable, ReadyCommandWait, GranularWaitTimeForCommands, RemoteAgentSslHandshakeTimeout, RemoteAgentMaxConcurrentNewConnections, - RemoteAgentNewConnectionsMonitorInterval, KVMHostDiscoverySshPort }; + RemoteAgentNewConnectionsMonitorInterval, KVMHostDiscoverySshPort, AgentHandoffWait }; } protected class SetHostParamsListener implements Listener { diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java index 38a198b73040..7d223f65faa7 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/ClusteredAgentManagerImpl.java @@ -493,10 +493,47 @@ public void closePeer(final String peerName) { } } + /** + * A peer that restarted quickly gets no left/joined event (ClusterManagerImpl only updates its runid), so we keep + * a channel the peer process has already closed. The first write to it still succeeds locally and its data is lost; + * only the next write fails with "Broken pipe". Peer channels are only ever written to, so a non-blocking read that + * returns -1 means the peer has closed this connection. Inbound bytes (e.g. TLS post-handshake messages) are + * discarded; nothing unwraps on this channel. + */ + protected boolean isPeerChannelClosed(final SocketChannel ch) { + if (ch == null || !ch.isOpen() || !ch.isConnected()) { + return true; + } + if (ch.isBlocking()) { + return false; + } + try { + final ByteBuffer discard = ByteBuffer.allocate(4096); + int read; + while ((read = ch.read(discard)) > 0) { + discard.clear(); + } + return read < 0; + } catch (final IOException e) { + return true; + } + } + public SocketChannel connectToPeer(final String peerName, final SocketChannel prevCh) { synchronized (_peers) { - final SocketChannel ch = _peers.get(peerName); + SocketChannel ch = _peers.get(peerName); SSLEngine sslEngine; + if (ch != null && ch != prevCh && isPeerChannelClosed(ch)) { + logger.info("Peer management server {} has closed our connection (restarted?), opening a new one", peerName); + try { + ch.close(); + } catch (final IOException e) { + logger.debug("[ignored] failed to close stale peer channel: {}", e.getLocalizedMessage()); + } + _peers.remove(peerName); + _sslEngines.remove(peerName); + ch = null; + } if (prevCh != null) { try { prevCh.close(); diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/ClusteredAgentManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/ClusteredAgentManagerImplTest.java index 5e4678f62225..a220ef69be4a 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/ClusteredAgentManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/ClusteredAgentManagerImplTest.java @@ -23,6 +23,7 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.resource.ResourceManagerImpl; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -30,12 +31,16 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.net.InetSocketAddress; +import java.nio.channels.ServerSocketChannel; +import java.nio.channels.SocketChannel; import java.util.ArrayList; import java.util.List; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -147,4 +152,31 @@ public void scanDirectAgentToLoadHostWithNonForwardAttacheAndDisconnectedTest() verify(clusteredAgentManagerImpl).investigate(agentAttache); verify(clusteredAgentManagerImpl).loadDirectlyConnectedHost(hostVO, false); } + + @Test + public void isPeerChannelClosedDetectsPeerThatClosedTheConnection() throws Exception { + ClusteredAgentManagerImpl clusteredAgentManagerImpl = mock(ClusteredAgentManagerImpl.class); + doCallRealMethod().when(clusteredAgentManagerImpl).isPeerChannelClosed(any()); + try (ServerSocketChannel server = ServerSocketChannel.open()) { + server.bind(new InetSocketAddress("127.0.0.1", 0)); + SocketChannel client = SocketChannel.open(server.getLocalAddress()); + client.configureBlocking(false); + SocketChannel accepted = server.accept(); + + Assert.assertFalse("open connection must not be reported closed", clusteredAgentManagerImpl.isPeerChannelClosed(client)); + + accepted.close(); + boolean closed = false; + for (int i = 0; i < 50 && !closed; i++) { + closed = clusteredAgentManagerImpl.isPeerChannelClosed(client); + if (!closed) { + Thread.sleep(20); + } + } + Assert.assertTrue("connection closed by the peer must be detected", closed); + client.close(); + Assert.assertTrue(clusteredAgentManagerImpl.isPeerChannelClosed(client)); + Assert.assertTrue(clusteredAgentManagerImpl.isPeerChannelClosed(null)); + } + } } diff --git a/engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/DirectAgentManagerSimpleImpl.java b/engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/DirectAgentManagerSimpleImpl.java index 1d072985a667..4938df176950 100644 --- a/engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/DirectAgentManagerSimpleImpl.java +++ b/engine/storage/integration-test/src/test/java/org/apache/cloudstack/storage/test/DirectAgentManagerSimpleImpl.java @@ -272,6 +272,11 @@ public boolean isAgentAttached(long hostId) { return false; } + @Override + public boolean isAgentIdle(long hostId) { + return true; + } + @Override public boolean handleDirectConnectAgent(Host host, StartupCommand[] cmds, ServerResource resource, boolean forRebalance, boolean newHost) throws ConnectionException { return false; diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java index d8385e9aecd1..df137a55c704 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java @@ -113,9 +113,20 @@ public AsyncJobDaoImpl() { pendingNonPseudoAsyncJobsSearch = createSearchBuilder(Long.class); pendingNonPseudoAsyncJobsSearch.select(null, SearchCriteria.Func.COUNT, pendingNonPseudoAsyncJobsSearch.entity().getId()); - pendingNonPseudoAsyncJobsSearch.and("instanceTypeNEQ", pendingNonPseudoAsyncJobsSearch.entity().getInstanceType(), SearchCriteria.Op.NEQ); + // VmWork jobs carry a NULL instance_type; "NEQ 'Thread'" alone would skip them (NULL != x is not true in SQL), + // and the maintenance drain would then finish while VM work still runs on this management server. + pendingNonPseudoAsyncJobsSearch.and().op("instanceTypeNULL", pendingNonPseudoAsyncJobsSearch.entity().getInstanceType(), SearchCriteria.Op.NULL); + pendingNonPseudoAsyncJobsSearch.or("instanceTypeNEQ", pendingNonPseudoAsyncJobsSearch.entity().getInstanceType(), SearchCriteria.Op.NEQ); + pendingNonPseudoAsyncJobsSearch.cp(); pendingNonPseudoAsyncJobsSearch.and("jobStatusEQ", pendingNonPseudoAsyncJobsSearch.entity().getStatus(), SearchCriteria.Op.EQ); - pendingNonPseudoAsyncJobsSearch.and("executingMsidIN", pendingNonPseudoAsyncJobsSearch.entity().getExecutingMsid(), SearchCriteria.Op.IN); + // Same ownership rule as the jobs a restarting management server fails (getResetJobs): executing here, or + // queued here and not picked up yet. + pendingNonPseudoAsyncJobsSearch.and().op("executingMsidIN", pendingNonPseudoAsyncJobsSearch.entity().getExecutingMsid(), SearchCriteria.Op.IN); + pendingNonPseudoAsyncJobsSearch.or().op("executingMsidNULL", pendingNonPseudoAsyncJobsSearch.entity().getExecutingMsid(), SearchCriteria.Op.NULL); + pendingNonPseudoAsyncJobsSearch.and("initMsidIN", pendingNonPseudoAsyncJobsSearch.entity().getInitMsid(), SearchCriteria.Op.IN); + pendingNonPseudoAsyncJobsSearch.cp(); + pendingNonPseudoAsyncJobsSearch.cp(); + pendingNonPseudoAsyncJobsSearch.done(); } @Override @@ -283,6 +294,7 @@ public long countPendingNonPseudoJobs(Long... msIds) { sc.setParameters("jobStatusEQ", JobInfo.Status.IN_PROGRESS); if (msIds != null) { sc.setParameters("executingMsidIN", (Object[])msIds); + sc.setParameters("initMsidIN", (Object[])msIds); } List results = customSearch(sc, null); return results.get(0); diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index 8e6d33c4e668..e7d8bd7f7ca2 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -36,6 +36,8 @@ import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VolumeDao; +import org.apache.cloudstack.secret.PassphraseVO; +import org.apache.cloudstack.secret.dao.PassphraseDao; import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.db.GlobalLock; @@ -71,6 +73,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -130,6 +133,9 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co @Inject private VolumeDao volumeDao; + @Inject + private PassphraseDao passphraseDao; + @Inject private StoragePoolHostDao storagePoolHostDao; @@ -591,6 +597,11 @@ public Pair takeBackup(final VirtualMachine vm, Boolean quiesce command.setVolumePaths(volumePoolsAndPaths.second()); } + // Sent for every VM state: an encrypted volume must not be written to the backup target in + // the clear, and the running-VM path enumerates its disks from libvirt rather than from the + // paths above. + command.setVolumePassphrases(getVolumePassphrases(volumeDao.findByInstance(vm.getId()))); + BackupAnswer answer; try { answer = (BackupAnswer) agentManager.send(host.getId(), command); @@ -689,9 +700,14 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup, boolean qui } private Pair restoreVMBackup(VirtualMachine vm, Backup backup) { - List backedVolumes = backup.getBackedUpVolumes(); - List backedVolumesUUIDs = backedVolumes.stream() + // Sort once and derive every per-volume list from that same ordering. The UUID list and the + // backup file list are consumed index-by-index on the agent side, so they have to agree; + // sorting only one of them leaves the two out of step whenever the stored order of the + // backed up volumes differs from their device id order. + List backedVolumes = backup.getBackedUpVolumes().stream() .sorted(Comparator.comparingLong(Backup.VolumeInfo::getDeviceId)) + .collect(Collectors.toList()); + List backedVolumesUUIDs = backedVolumes.stream() .map(Backup.VolumeInfo::getUuid) .collect(Collectors.toList()); @@ -714,6 +730,9 @@ private Pair restoreVMBackup(VirtualMachine vm, Backup backup) restoreCommand.setRestoreVolumePools(volumePoolsAndPaths.first()); restoreCommand.setRestoreVolumePaths(volumePoolsAndPaths.second()); restoreCommand.setBackupFiles(getBackupFiles(backedVolumes)); + // The backup of an encrypted volume is itself LUKS-encrypted, so the agent needs the + // passphrase to read it back and to rewrite the volume through its own encryption. + restoreCommand.setVolumePassphrases(getVolumePassphrases(restoreVolumes)); restoreCommand.setVmExists(vm.getRemoved() == null); restoreCommand.setVmState(vm.getState()); restoreCommand.setMountTimeout(NASBackupRestoreMountTimeout.value()); @@ -742,6 +761,28 @@ private List getBackupFiles(List backedVolumes) { return backupFiles; } + /** + * Passphrases of the encrypted volumes among {@code volumes}, keyed by volume path (the uuid the + * backup files are named after). Unencrypted volumes are omitted, so an empty map means there is + * nothing to encrypt and the backup behaves exactly as before. + */ + private Map getVolumePassphrases(List volumes) { + Map passphrases = new HashMap<>(); + for (VolumeVO volume : volumes) { + if (volume.getPassphraseId() == null) { + continue; + } + PassphraseVO passphrase = passphraseDao.findById(volume.getPassphraseId()); + if (passphrase == null || passphrase.getPassphrase() == null || passphrase.getPassphrase().length == 0) { + throw new CloudRuntimeException(String.format( + "Volume %s is encrypted but its passphrase could not be read; refusing to back it up unencrypted", + volume.getUuid())); + } + passphrases.put(volume.getPath(), passphrase.getPassphrase()); + } + return passphrases; + } + private Pair, List> getVolumePoolsAndPaths(List volumes) { List volumePools = new ArrayList<>(); List volumePaths = new ArrayList<>(); diff --git a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java index f1d5613ab7fc..e1d49eb108bf 100644 --- a/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java +++ b/plugins/backup/nas/src/test/java/org/apache/cloudstack/backup/NASBackupProviderTest.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Map; import java.util.Optional; import com.cloud.vm.snapshot.dao.VMSnapshotDao; @@ -66,6 +67,8 @@ import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.backup.dao.BackupDetailsDao; +import org.apache.cloudstack.secret.PassphraseVO; +import org.apache.cloudstack.secret.dao.PassphraseDao; import org.apache.cloudstack.backup.dao.BackupRepositoryDao; import org.apache.cloudstack.backup.dao.BackupOfferingDao; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; @@ -126,6 +129,9 @@ public class NASBackupProviderTest { @Mock private ResourceLimitService resourceLimitMgr; + @Mock + private PassphraseDao passphraseDao; + @Test public void testDeleteBackup() throws OperationTimedoutException, AgentUnavailableException { Long hostId = 1L; @@ -238,9 +244,12 @@ public void takeBackupSuccessfully() throws AgentUnavailableException, Operation VolumeVO volume1 = mock(VolumeVO.class); Mockito.when(volume1.getState()).thenReturn(Volume.State.Ready); Mockito.when(volume1.getSize()).thenReturn(100L); + // Unencrypted volumes: nothing for the backup to encrypt. + Mockito.when(volume1.getPassphraseId()).thenReturn(null); VolumeVO volume2 = mock(VolumeVO.class); Mockito.when(volume2.getState()).thenReturn(Volume.State.Ready); Mockito.when(volume2.getSize()).thenReturn(200L); + Mockito.when(volume2.getPassphraseId()).thenReturn(null); Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(volume1, volume2)); BackupAnswer answer = mock(BackupAnswer.class); @@ -1034,4 +1043,62 @@ public void sweepContinuesPastFailedTombstoneDelete() Mockito.verify(backupDao, Mockito.never()).remove(51L); Mockito.verify(backupDao).remove(50L); } + + /** + * An encrypted volume must not reach the backup target in the clear: its passphrase has to travel + * on the command so the agent can make QEMU write the backup already encrypted. + */ + @Test + public void takeBackupSendsPassphraseOfEncryptedVolume() throws AgentUnavailableException, OperationTimedoutException { + Long vmId = 1L; + Long hostId = 2L; + Long backupOfferingId = 3L; + Long passphraseId = 77L; + byte[] secret = "volume-passphrase".getBytes(java.nio.charset.StandardCharsets.UTF_8); + + VMInstanceVO vm = mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getHostId()).thenReturn(hostId); + Mockito.when(vm.getInstanceName()).thenReturn("test-vm"); + Mockito.when(vm.getBackupOfferingId()).thenReturn(backupOfferingId); + Mockito.when(vm.getState()).thenReturn(VMInstanceVO.State.Running); + + BackupRepository backupRepository = mock(BackupRepository.class); + Mockito.when(backupRepository.getType()).thenReturn("nfs"); + Mockito.when(backupRepository.getAddress()).thenReturn("10.0.0.1:/export"); + Mockito.when(backupRepositoryDao.findByBackupOfferingId(backupOfferingId)).thenReturn(backupRepository); + + HostVO host = mock(HostVO.class); + Mockito.when(host.getId()).thenReturn(hostId); + Mockito.when(host.getStatus()).thenReturn(Status.Up); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(hostDao.findById(hostId)).thenReturn(host); + + VolumeVO encryptedVolume = mock(VolumeVO.class); + Mockito.when(encryptedVolume.getState()).thenReturn(Volume.State.Ready); + Mockito.when(encryptedVolume.getSize()).thenReturn(100L); + Mockito.when(encryptedVolume.getPassphraseId()).thenReturn(passphraseId); + Mockito.when(encryptedVolume.getPath()).thenReturn("volume-path-uuid"); + Mockito.when(volumeDao.findByInstance(vmId)).thenReturn(List.of(encryptedVolume)); + + PassphraseVO passphrase = mock(PassphraseVO.class); + Mockito.when(passphrase.getPassphrase()).thenReturn(secret); + Mockito.when(passphraseDao.findById(passphraseId)).thenReturn(passphrase); + + BackupAnswer answer = mock(BackupAnswer.class); + Mockito.when(answer.getResult()).thenReturn(true); + Mockito.when(answer.getSize()).thenReturn(100L); + Mockito.when(agentManager.send(anyLong(), Mockito.any(TakeBackupCommand.class))).thenReturn(answer); + Mockito.when(backupDao.persist(Mockito.any(BackupVO.class))).thenAnswer(invocation -> invocation.getArgument(0)); + Mockito.when(backupDao.update(Mockito.anyLong(), Mockito.any(BackupVO.class))).thenReturn(true); + + nasBackupProvider.takeBackup(vm, false, false); + + ArgumentCaptor captor = ArgumentCaptor.forClass(TakeBackupCommand.class); + Mockito.verify(agentManager).send(anyLong(), captor.capture()); + Map sent = captor.getValue().getVolumePassphrases(); + Assert.assertNotNull("the encrypted volume's passphrase must be sent to the agent", sent); + Assert.assertArrayEquals(secret, sent.get("volume-path-uuid")); + } + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 4281036d9456..176cc6891883 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -19,6 +19,7 @@ import static com.cloud.host.Host.HOST_CDROM_MAX_COUNT; import static com.cloud.host.Host.HOST_INSTANCE_CONVERSION; import static com.cloud.host.Host.HOST_OVFTOOL_VERSION; +import static com.cloud.host.Host.HOST_RBD_VOLUME_ENCRYPTION; import static com.cloud.host.Host.HOST_VDDK_LIB_DIR; import static com.cloud.host.Host.HOST_VDDK_SUPPORT; import static com.cloud.host.Host.HOST_VDDK_VERSION; @@ -91,6 +92,7 @@ import org.apache.cloudstack.storage.volume.VolumeOnStorageTO; import org.apache.cloudstack.utils.bytescale.ByteScaleUtils; import org.apache.cloudstack.utils.cryptsetup.CryptSetup; +import org.apache.cloudstack.utils.rbd.RbdEncryption; import org.apache.cloudstack.utils.hypervisor.HypervisorUtils; import org.apache.cloudstack.utils.linux.CPUStat; import org.apache.cloudstack.utils.linux.KVMHostInfo; @@ -3787,7 +3789,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { DiskDef.DiskBus diskBusTypeData = getDataDiskModelFromVMDetail(vmSpec); if (diskBusTypeData == null) { - diskBusTypeData = (diskBusType == DiskDef.DiskBus.SCSI) ? diskBusType : DiskDef.DiskBus.VIRTIO; + diskBusTypeData = (diskBusType == DiskDef.DiskBus.SCSI || diskBusType == DiskDef.DiskBus.VIRTIOBLK) ? diskBusType : DiskDef.DiskBus.VIRTIO; } final DiskDef disk = new DiskDef(); @@ -3882,7 +3884,9 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { if (volumeObjectTO.requiresEncryption() && pool.getType().encryptionSupportMode() == Storage.EncryptionSupport.Hypervisor ) { String secretUuid = createLibvirtVolumeSecret(conn, volumeObjectTO.getPath(), volumeObjectTO.getPassphrase()); - DiskDef.LibvirtDiskEncryptDetails encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(volumeObjectTO.getEncryptFormat())); + // RBD volumes are encrypted natively by librbd, so request the librbd encryption engine. + String encryptEngine = (pool.getType() == StoragePoolType.RBD) ? "librbd" : null; + DiskDef.LibvirtDiskEncryptDetails encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(volumeObjectTO.getEncryptFormat()), encryptEngine); disk.setLibvirtDiskEncryptDetails(encryptDetails); } } @@ -4410,6 +4414,7 @@ public StartupCommand[] initialize() { cmd.setGatewayIpAddress(localGateway); cmd.setIqn(getIqn()); cmd.getHostDetails().put(HOST_VOLUME_ENCRYPTION, String.valueOf(hostSupportsVolumeEncryption())); + cmd.getHostDetails().put(HOST_RBD_VOLUME_ENCRYPTION, String.valueOf(hostSupportsRbdVolumeEncryption())); cmd.setHostTags(getHostTags()); boolean instanceConversionSupported = hostSupportsInstanceConversion(); cmd.getHostDetails().put(HOST_INSTANCE_CONVERSION, String.valueOf(instanceConversionSupported)); @@ -6195,7 +6200,10 @@ public boolean isHostSecured() { } /** - * Test host for volume encryption support + * Test host for qemu-native LUKS volume encryption (qemu-img LUKS support + cryptsetup), + * reported as {@code host.volume.encryption}. RBD/librbd encryption support is a separate + * capability, reported as {@code host.volume.encryption.rbd} + * (see {@link #hostSupportsRbdVolumeEncryption()}). * @return boolean */ public boolean hostSupportsVolumeEncryption() { @@ -6220,6 +6228,13 @@ public boolean hostSupportsVolumeEncryption() { return true; } + /** + * Test host for librbd native LUKS encryption support (rbd CLI with the encryption subcommand). + */ + public boolean hostSupportsRbdVolumeEncryption() { + return new RbdEncryption().isSupported(); + } + public boolean isSecureMode(String bootMode) { if (StringUtils.isNotBlank(bootMode) && "secure".equalsIgnoreCase(bootMode)) { return true; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java index 74529d9d5fa2..3f221d749bbf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java @@ -788,14 +788,21 @@ public static class DiskDef { public static class LibvirtDiskEncryptDetails { String passphraseUuid; QemuObject.EncryptFormat encryptFormat; + String engine; // optional libvirt encryption engine (e.g. "librbd"); null => libvirt/qemu default public LibvirtDiskEncryptDetails(String passphraseUuid, QemuObject.EncryptFormat encryptFormat) { + this(passphraseUuid, encryptFormat, null); + } + + public LibvirtDiskEncryptDetails(String passphraseUuid, QemuObject.EncryptFormat encryptFormat, String engine) { this.passphraseUuid = passphraseUuid; this.encryptFormat = encryptFormat; + this.engine = engine; } public String getPassphraseUuid() { return this.passphraseUuid; } public QemuObject.EncryptFormat getEncryptFormat() { return this.encryptFormat; } + public String getEngine() { return this.engine; } } public static class DiskGeometry { @@ -864,6 +871,17 @@ public enum DiskBus { _bus = bus; } + /** + * The name libvirt knows this bus by. It is the enum's own value everywhere except + * VIRTIOBLK: 'virtio-blk' is a CloudStack disk controller name, not one of libvirt's + * target buses (ide, scsi, virtio, xen, usb, sata, sd, fdc, uml), and a virtio-blk + * disk is a virtio one. Rendering the enum value instead makes libvirt reject the + * domain XML, so the VM cannot start. + */ + public String libvirtBus() { + return this == VIRTIOBLK ? VIRTIO._bus : _bus; + } + public static DiskBus fromValue(String bus) { for (DiskBus b : DiskBus.values()) { if (b.toString().equalsIgnoreCase(bus)) { @@ -1024,7 +1042,7 @@ private String getDevLabel(int devId, DiskBus bus, boolean forIso) { if (bus == DiskBus.SCSI) { return "sd" + getDevLabelSuffix(devId); - } else if (bus == DiskBus.VIRTIO) { + } else if (bus == DiskBus.VIRTIO || bus == DiskBus.VIRTIOBLK) { return "vd" + getDevLabelSuffix(devId); } else if (bus == DiskBus.SATA){ return "sd" + getDevLabelSuffix(devId); @@ -1379,7 +1397,7 @@ public String toString() { diskBuilder.append(String.format("io='%s' ", ioDriver)); } - if (isIothreadsEnabled && _bus == DiskBus.VIRTIO) { + if (isIothreadsEnabled && (_bus == DiskBus.VIRTIO || _bus == DiskBus.VIRTIOBLK)) { diskBuilder.append(String.format("iothread='%s' ", NUMBER_OF_IOTHREADS)); } diskBuilder.append("/>\n"); @@ -1422,7 +1440,7 @@ public String toString() { } diskBuilder.append("\n"); @@ -1446,7 +1464,11 @@ public String toString() { } if (encryptDetails != null) { - diskBuilder.append("\n"); + diskBuilder.append("\n"); diskBuilder.append("\n"); diskBuilder.append("\n"); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java index a43b584dd6d6..20e3891478b4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtResizeVolumeCommandWrapper.java @@ -33,6 +33,7 @@ import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuObject; +import org.apache.cloudstack.utils.rbd.RbdEncryption; import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.DomainInfo; @@ -93,6 +94,13 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR final String path = vol.getPath(); String type = notifyOnlyType; + // Encrypted RBD volumes are encrypted natively by librbd; they must be resized via + // `rbd resize --encryption-passphrase-file` so librbd grows the encrypted payload and keeps + // the LUKS header consistent. The libvirt/qemu-img resize paths below are for qemu-native + // encryption and would not handle the librbd LUKS2 layout. + final boolean rbdEncrypted = pool.getType() == StoragePoolType.RBD + && command.getPassphrase() != null && command.getPassphrase().length > 0; + if (spool.getType().equals(StoragePoolType.PowerFlex) && vol.getFormat().equals(PhysicalDiskFormat.QCOW2)) { // PowerFlex QCOW2 sizing needs to consider overhead. newSize = ScaleIOStorageAdaptor.getUsableBytesFromRawBytes(newSize); @@ -115,7 +123,7 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR /* libvirt doesn't support resizing (C)LVM devices, and corrupts QCOW2 in some scenarios, so we have to do these via qemu-img */ if (pool.getType() != StoragePoolType.CLVM && pool.getType() != StoragePoolType.CLVM_NG && pool.getType() != StoragePoolType.Linstor && pool.getType() != StoragePoolType.PowerFlex - && vol.getFormat() != PhysicalDiskFormat.QCOW2) { + && vol.getFormat() != PhysicalDiskFormat.QCOW2 && !rbdEncrypted) { logger.debug("Volume " + path + " can be resized by libvirt. Asking libvirt to resize the volume."); try { final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); @@ -139,11 +147,15 @@ public Answer execute(final ResizeVolumeCommand command, final LibvirtComputingR boolean vmIsRunning = isVmRunning(vmInstanceName, libvirtComputingResource); - /* when VM is offline, we use qemu-img directly to resize encrypted volumes. - If VM is online, the existing resize script will call virsh blockresize which works - with both encrypted and non-encrypted volumes. + /* when VM is offline, we use qemu-img (or rbd, for librbd-encrypted RBD) directly to resize + encrypted volumes. If VM is online, the existing resize script calls virsh blockresize, + which for an librbd-encrypted RBD disk lets qemu/librbd grow the encrypted payload and + notify the guest in one step (no passphrase needed, qemu already has the secret loaded). */ - if (!vmIsRunning && command.getPassphrase() != null && command.getPassphrase().length > 0 ) { + if (rbdEncrypted && !vmIsRunning) { + logger.debug("Invoking rbd to resize an offline, encrypted (librbd) RBD volume"); + resizeRbdEncryptedVolume(pool, vol, newSize, shrinkOk, command.getPassphrase()); + } else if (!vmIsRunning && command.getPassphrase() != null && command.getPassphrase().length > 0 ) { logger.debug("Invoking qemu-img to resize an offline, encrypted volume"); QemuObject.EncryptFormat encryptFormat = QemuObject.EncryptFormat.enumValue(command.getEncryptFormat()); resizeEncryptedQcowFile(vol, encryptFormat,newSize, command.getPassphrase(), libvirtComputingResource); @@ -213,6 +225,16 @@ private void resizeEncryptedQcowFile(final KVMPhysicalDisk vol, final QemuObject } } + private void resizeRbdEncryptedVolume(final KVMStoragePool pool, final KVMPhysicalDisk vol, long newSize, + boolean shrinkOk, byte[] passphrase) throws CloudRuntimeException { + try { + new RbdEncryption().resize(pool.getSourceHost(), pool.getSourcePort(), pool.getAuthUserName(), + pool.getAuthSecret(), pool.getSourceDir(), vol.getName(), newSize, shrinkOk, passphrase); + } finally { + Arrays.fill(passphrase, (byte) 0); + } + } + private Answer handleMultipathSCSIResize(ResizeVolumeCommand command, KVMStoragePool pool) { ((MultipathSCSIPool)pool).resize(command.getPath(), command.getInstanceName(), command.getNewSize()); return new ResizeVolumeAnswer(command, true, ""); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 3839d7f6bdbd..bee325ed9d50 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -25,16 +25,22 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.EnumMap; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.cloudstack.utils.qemu.QemuObject; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; import org.libvirt.LibvirtException; @@ -55,6 +61,13 @@ @ResourceWrapper(handles = RestoreBackupCommand.class) public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper { private static final String BACKUP_TEMP_FILE_PREFIX = "csbackup"; + /** qemu secret id used while reading an encrypted backup and writing it back to the volume. */ + private static final String RESTORE_SECRET_ID = "restoresec0"; + /** + * Passphrases of the volumes being restored, keyed by volume path. Held for the duration of a + * single execute() so the per-volume restore helpers can reach them. + */ + private Map currentVolumePassphrases; private static final String FILE_PATH_PLACEHOLDER = "%s/%s"; // Detects whether a qcow2 file references a parent in its backing-file metadata. // Returns 0 (true) when a backing file is present, 1 when not. Uses --output=json @@ -92,6 +105,7 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser int timeout = command.getWait() > 0 ? command.getWait() * 1000 : serverResource.getCmdsTimeout(); KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr(); List backupFiles = command.getBackupFiles(); + currentVolumePassphrases = command.getVolumePassphrases(); String newVolumeId = null; try { @@ -131,11 +145,38 @@ private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, Li String backupPath, List backupFiles, String mountDirectory, int timeout) { String diskType = "root"; try { - for (int idx = 0; idx < restoreVolumePaths.size(); idx++) { - PrimaryDataStoreTO restoreVolumePool = restoreVolumePools.get(idx); - String restoreVolumePath = restoreVolumePaths.get(idx); - String backupFile = backupFiles.get(idx); + // Match each backed up volume to the instance's volume with the SAME UUID. Both lists + // arrive ordered by device id, but device ids are not stable across restores, so relying + // on the position within the list can write a backup into a different volume than the one + // it was taken from. + Map targetIndexByVolumeUuid = new HashMap<>(); + for (int i = 0; i < restoreVolumePaths.size(); i++) { + targetIndexByVolumeUuid.put(getVolumeUuidFromPath(restoreVolumePaths.get(i), restoreVolumePools.get(i)), i); + } + + // Creating an instance from a backup gives it brand new volumes, so none of the uuids + // recorded in the backup can match. Only then fall back to the device id ordering both + // lists already carry; when some of them do match, a missing one really is a volume that + // was detached, and writing its backup into another volume would be wrong. + boolean restoringIntoNewVolumes = backedVolumesUUIDs.stream().noneMatch(targetIndexByVolumeUuid::containsKey); + if (restoringIntoNewVolumes && backedVolumesUUIDs.size() != restoreVolumePaths.size()) { + throw new CloudRuntimeException(String.format("Unable to restore backup: it holds %d volumes but the instance has %d.", + backedVolumesUUIDs.size(), restoreVolumePaths.size())); + } + if (restoringIntoNewVolumes) { + logger.debug("None of the backed up volumes belong to this instance; restoring into its volumes in device id order."); + } + + for (int idx = 0; idx < backedVolumesUUIDs.size(); idx++) { String backupVolumeUuid = backedVolumesUUIDs.get(idx); + Integer targetIdx = restoringIntoNewVolumes ? Integer.valueOf(idx) : targetIndexByVolumeUuid.get(backupVolumeUuid); + if (targetIdx == null) { + throw new CloudRuntimeException(String.format("Unable to restore backup: volume [%s] recorded in the backup" + + " is not attached to the instance any more.", backupVolumeUuid)); + } + PrimaryDataStoreTO restoreVolumePool = restoreVolumePools.get(targetIdx); + String restoreVolumePath = restoreVolumePaths.get(targetIdx); + String backupFile = backupFiles.get(idx); String fullPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); diskType = "datadisk"; @@ -262,10 +303,41 @@ private String getBackupPath(String mountDirectory, String backupPath, String ba } private boolean checkBackupFileImage(String backupPath) { + // The backup of an encrypted volume is LUKS-encrypted, so qemu-img needs its passphrase to + // open it at all; without one the check fails and a perfectly good backup looks corrupt. + byte[] passphrase = anyPassphrase(); + if (passphrase != null && passphrase.length > 0) { + try (KeyFile keyFile = new KeyFile(passphrase)) { + String command = String.format( + "qemu-img check --object secret,id=%s,file=%s --image-opts driver=qcow2,file.filename=%s,encrypt.key-secret=%s", + RESTORE_SECRET_ID, keyFile.toString(), backupPath, RESTORE_SECRET_ID); + return Script.runSimpleBashScriptForExitValue(command) == 0; + } catch (IOException ex) { + logger.error("Failed to create the key file needed to check the encrypted backup {}", backupPath, ex); + return false; + } + } int exitValue = Script.runSimpleBashScriptForExitValue(String.format("qemu-img check %s", backupPath)); return exitValue == 0; } + /** + * Any passphrase carried by this restore. The pre-restore image check runs per backup file rather + * than per volume, and every volume of a VM that has encrypted volumes is backed up with its own + * passphrase, so the first one is enough to tell whether a key is needed at all. + */ + private byte[] anyPassphrase() { + if (currentVolumePassphrases == null) { + return null; + } + for (byte[] passphrase : currentVolumePassphrases.values()) { + if (passphrase != null && passphrase.length > 0) { + return passphrase; + } + } + return null; + } + private boolean checkBackupPathExists(String backupPath) { int exitValue = Script.runSimpleBashScriptForExitValue(String.format("ls %s", backupPath)); return exitValue == 0; @@ -275,6 +347,23 @@ private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, Pr return replaceVolumeWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, false, null); } + /** + * Passphrase of the volume being restored, or null when it is not encrypted. The map is keyed by + * the bare volume path as the management server knows it, while the paths handed around here may + * carry a pool prefix (for example "cloudstack/<uuid>" for RBD), so fall back to the last + * path segment. + */ + private byte[] passphraseFor(String volumePath) { + if (currentVolumePassphrases == null || volumePath == null) { + return null; + } + byte[] passphrase = currentVolumePassphrases.get(volumePath); + if (passphrase == null && volumePath.contains("/")) { + passphrase = currentVolumePassphrases.get(volumePath.substring(volumePath.lastIndexOf('/') + 1)); + } + return passphrase; + } + private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { if (List.of(Storage.StoragePoolType.RBD, Storage.StoragePoolType.Linstor).contains(volumePool.getPoolType())) { return replaceBlockDeviceWithBackup(storagePoolMgr, volumePool, volumePath, backupPath, timeout, createTargetVolume, size); @@ -302,6 +391,52 @@ private boolean hasBackingChain(String qcow2Path) { String.format(QEMU_IMG_HAS_BACKING_COMMAND, qcow2Path)) == 0; } + /** + * Restores a LUKS-encrypted backup onto an encrypted volume. The backup is opened with its + * passphrase and written through the volume's own encryption, so the restored volume carries the + * same LUKS header the rest of CloudStack expects; writing the decrypted bytes straight onto the + * volume would leave an image the hypervisor can no longer open. + */ + private void restoreEncryptedBlockDevice(QemuImg qemu, PrimaryDataStoreTO volumePool, KVMStoragePool volumeStoragePool, + String volumePath, String backupPath, byte[] passphrase) throws QemuImgException { + try (KeyFile keyFile = new KeyFile(passphrase)) { + EnumMap secretParams = new EnumMap<>(QemuObject.ObjectParameter.class); + secretParams.put(QemuObject.ObjectParameter.ID, RESTORE_SECRET_ID); + secretParams.put(QemuObject.ObjectParameter.FILE, keyFile.toString()); + QemuObject secret = new QemuObject(QemuObject.ObjectType.SECRET, secretParams); + + Map srcParams = new HashMap<>(); + srcParams.put("driver", "qcow2"); + srcParams.put("file.filename", backupPath); + srcParams.put("encrypt.key-secret", RESTORE_SECRET_ID); + QemuImageOptions srcOpts = new QemuImageOptions(srcParams); + // Without this the options collapse to a bare filename and the backup's own + // encrypt.key-secret is dropped, so qemu cannot open it. + srcOpts.setImageOptsFlag(true); + + if (!Storage.StoragePoolType.RBD.equals(volumePool.getPoolType())) { + throw new CloudRuntimeException(String.format( + "Restoring an encrypted backup onto a %s volume is not supported yet", volumePool.getPoolType())); + } + + Map destParams = new HashMap<>(); + destParams.put("driver", "rbd"); + destParams.put("pool", volumeStoragePool.getSourceDir()); + destParams.put("image", volumePath.contains("/") ? volumePath.substring(volumePath.lastIndexOf('/') + 1) : volumePath); + destParams.put("encrypt.format", QemuObject.EncryptFormat.LUKS2.toString()); + destParams.put("encrypt.key-secret", RESTORE_SECRET_ID); + if (volumeStoragePool.getAuthUserName() != null) { + destParams.put("user", volumeStoragePool.getAuthUserName()); + } + QemuImageOptions destOpts = new QemuImageOptions(destParams); + + QemuImgFile srcFile = new QemuImgFile(backupPath, QemuImg.PhysicalDiskFormat.QCOW2); + qemu.convertIntoExistingTarget(srcFile, null, List.of(secret), srcOpts, destOpts, false); + } catch (IOException ex) { + throw new CloudRuntimeException(String.format("Failed to create the key file needed to restore %s", volumePath), ex); + } + } + private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String backupPath, int timeout, boolean createTargetVolume, Long size) { KVMStoragePool volumeStoragePool = storagePoolMgr.getStoragePool(volumePool.getPoolType(), volumePool.getUuid()); QemuImg qemu; @@ -348,7 +483,14 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg } destVolumeFile = new QemuImgFile(destVolume, QemuImg.PhysicalDiskFormat.RAW); logger.debug("Starting convert backup {} to volume {}", backupPath, volumePath); - qemu.convert(srcBackupFile, destVolumeFile); + byte[] passphrase = passphraseFor(volumePath); + if (passphrase != null && passphrase.length > 0) { + // The backup is LUKS-encrypted and the volume is encrypted too, so the data has to be + // read through the backup's encryption and written back through the volume's own. + restoreEncryptedBlockDevice(qemu, volumePool, volumeStoragePool, volumePath, backupPath, passphrase); + } else { + qemu.convert(srcBackupFile, destVolumeFile); + } logger.debug("Successfully converted backup {} to volume {}", backupPath, volumePath); } catch (QemuImgException | LibvirtException e) { String srcFilename = srcBackupFile != null ? srcBackupFile.getFileName() : null; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java index 106fe31a0f18..44d6c78db832 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtTakeBackupCommandWrapper.java @@ -22,6 +22,7 @@ import com.amazonaws.util.CollectionUtils; import com.cloud.agent.api.Answer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtConnection; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; @@ -33,10 +34,16 @@ import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.TakeBackupCommand; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.libvirt.Connect; +import org.libvirt.LibvirtException; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; @ResourceWrapper(handles = TakeBackupCommand.class) @@ -52,6 +59,14 @@ public class LibvirtTakeBackupCommandWrapper extends CommandWrapper result = runBackupScript(libvirtComputingResource, command, vmName, backupRepoType, backupRepoAddress, - mountOptions, backupPath, diskPaths, command.getMode(), - command.getBitmapNew(), command.getBitmapParent(), command.getParentPaths(), timeout); + // Encrypted volumes: hand QEMU a secret so it writes the backup already LUKS-encrypted, and a + // key file for the qemu-img paths, which cannot consume libvirt secrets. + final Map passphrases = command.getVolumePassphrases(); + final Map secretUuids = new HashMap<>(); + final Map keyFilePaths = new HashMap<>(); + final List keyFiles = new ArrayList<>(); + Connect conn = null; + + Pair result; + try { + if (passphrases != null && !passphrases.isEmpty()) { + try { + conn = LibvirtConnection.getConnectionByVmName(vmName); + for (Map.Entry entry : passphrases.entrySet()) { + String volumePath = entry.getKey(); + secretUuids.put(volumePath, libvirtComputingResource.createLibvirtVolumeSecret( + conn, BACKUP_SECRET_CONSUMER_PREFIX + volumePath, entry.getValue())); + KeyFile keyFile = new KeyFile(entry.getValue()); + keyFiles.add(keyFile); + keyFilePaths.put(volumePath, keyFile.toString()); + } + } catch (LibvirtException | IOException ex) { + // Failing here would otherwise mean silently backing an encrypted volume up in the clear. + logger.error("Failed to prepare encryption for the backup of VM {}", vmName, ex); + return new BackupAnswer(command, false, + String.format("Failed to prepare encryption for the backup of VM %s: %s", vmName, ex.getMessage())); + } + } + + result = runBackupScript(libvirtComputingResource, command, vmName, backupRepoType, backupRepoAddress, + mountOptions, backupPath, diskPaths, command.getMode(), + command.getBitmapNew(), command.getBitmapParent(), command.getParentPaths(), + secretUuids, keyFilePaths, timeout); + } finally { + for (KeyFile keyFile : keyFiles) { + // Cleanup must never mask the outcome of the backup itself. + try { + keyFile.close(); + } catch (IOException ex) { + logger.warn("Failed to remove a temporary key file used for a backup. The error was: {}", ex.getMessage(), ex); + } + } + removeBackupSecrets(libvirtComputingResource, conn, secretUuids.values()); + command.clearPassphrases(); + } if (result.first() != 0) { logger.debug("Failed to take VM backup: " + result.second()); @@ -141,7 +198,8 @@ private String stripMarkerLines(String stdout) { private Pair runBackupScript(LibvirtComputingResource libvirtComputingResource, TakeBackupCommand command, String vmName, String backupRepoType, String backupRepoAddress, String mountOptions, String backupPath, List diskPaths, String mode, - String bitmapNew, String bitmapParent, List parentPaths, int timeout) { + String bitmapNew, String bitmapParent, List parentPaths, + Map secretUuids, Map keyFilePaths, int timeout) { List argv = new ArrayList<>(Arrays.asList( libvirtComputingResource.getNasBackupPath(), "-o", "backup", @@ -169,6 +227,11 @@ private Pair runBackupScript(LibvirtComputingResource libvirtCo argv.add("--parent-paths"); argv.add(String.join(",", parentPaths)); } + // Per-volume qemu secret uuids and key file paths for encrypted volumes; empty when none are. + argv.add("-S"); + argv.add(joinVolumeMap(secretUuids)); + argv.add("-K"); + argv.add(joinVolumeMap(keyFilePaths)); List commands = new ArrayList<>(); commands.add(argv.toArray(new String[0])); @@ -212,19 +275,77 @@ private String validateBackupArgs(TakeBackupCommand command) { * Sum the per-disk size lines emitted by nasbackup.sh. Single-volume mode emits one * line containing just the byte count; multi-volume mode emits one line per disk * whose first whitespace-separated token is the byte count. + * + * Lines that do not start with a byte count are ignored rather than parsed. The + * script is not the only thing that can write to this stream: mount helpers and + * storage clients emit warnings of their own, and a single one of them used to + * abort the whole backup with a NumberFormatException. That failure is unusually + * expensive - the backup record stays in BackingUp with no timeout and no + * transition to Failed, and blocks every later restore of that instance - and it + * only ever hit the multi-volume branch, which is taken for a STOPPED instance, + * so it went unnoticed while running instances backed up normally. */ private long parseBackupSize(String stdout, List diskPaths) { long backupSize = 0L; if (CollectionUtils.isNullOrEmpty(diskPaths)) { - List outputLines = Arrays.asList(stdout.split("\n")); - if (!outputLines.isEmpty()) { - backupSize = Long.parseLong(outputLines.get(outputLines.size() - 1).trim()); + String[] outputLines = stdout.split("\n"); + for (int i = outputLines.length - 1; i >= 0; i--) { + Long size = parseSizeLine(outputLines[i]); + if (size != null) { + backupSize = size; + break; + } } } else { for (String line : stdout.split("\n")) { - backupSize = backupSize + Long.parseLong(line.split(" ")[0].trim()); + Long size = parseSizeLine(line); + if (size != null) { + backupSize = backupSize + size; + } } } return backupSize; } + + /** + * The byte count a size line starts with, or null when the line is not one. + */ + private Long parseSizeLine(String line) { + String token = line.trim().split("\\s+")[0]; + if (token.isEmpty()) { + return null; + } + try { + return Long.parseLong(token); + } catch (NumberFormatException e) { + logger.debug("Ignoring non-numeric line in backup script output: {}", line.trim()); + return null; + } + } + + /** Renders a per-volume map as the "<volumePath>:<value>,..." form the backup script parses. */ + protected String joinVolumeMap(Map values) { + if (values == null || values.isEmpty()) { + return ""; + } + List entries = new ArrayList<>(); + for (Map.Entry entry : values.entrySet()) { + entries.add(entry.getKey() + ":" + entry.getValue()); + } + return String.join(",", entries); + } + + /** Removes the ephemeral libvirt secrets registered for this backup; never throws. */ + private void removeBackupSecrets(LibvirtComputingResource libvirtComputingResource, Connect conn, java.util.Collection secretUuids) { + if (conn == null) { + return; + } + for (String secretUuid : secretUuids) { + try { + libvirtComputingResource.removeLibvirtVolumeSecret(conn, secretUuid); + } catch (LibvirtException ex) { + logger.warn("Failed to remove the libvirt secret [{}] used for a backup. The error was: {}", secretUuid, ex.getMessage(), ex); + } + } + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 11acb9546b53..0ad3127d6a08 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -182,6 +182,12 @@ public class KVMStorageProcessor implements StorageProcessor { private static final String CEPH_AUTH_KEY = "key"; private static final String CEPH_CLIENT_MOUNT_TIMEOUT = "client_mount_timeout"; private static final String CEPH_DEFAULT_MOUNT_TIMEOUT = "30"; + + // libvirt < 10.1.0 has an object apply-order bug (fixed in 10.1.0) that breaks hot-plug of an encrypted + // blockdev: on attach the disk is opened before its LUKS secret object is defined, so the attach fails with + // "No secret with id '...-format-encryption-secret0'". Booting a VM from an encrypted disk is unaffected (the + // QEMU command line resolves all -object before -blockdev). See qemuBlockStorageSourceAttachApply() in libvirt. + private static final long MIN_LIBVIRT_VERSION_FOR_RBD_ENCRYPTED_HOTPLUG = 10001000L; // libvirt 10.1.0 /** * Time interval before rechecking virsh commands */ @@ -1788,13 +1794,36 @@ protected DiskDef.DiskBus getAttachDiskBusType(int deviceId, List disks } if (disk.getBusType() == DiskDef.DiskBus.SCSI) { return DiskDef.DiskBus.SCSI; - } else if (disk.getBusType() == DiskDef.DiskBus.VIRTIOBLK) { - return DiskDef.DiskBus.VIRTIOBLK; } } + if (deviceId != 0 && DiskDef.DiskBus.VIRTIOBLK == DiskDef.DiskBus.fromValue( + MapUtils.getString(controllerInfo, VmDetailConstants.ROOT_DISK_CONTROLLER))) { + // A data disk with no controller of its own follows the root, as it already does when the + // VM starts. The scan above cannot see this case: a virtio-blk disk is rendered with + // libvirt's 'virtio' bus, so it is indistinguishable from a plain virtio disk in the domain + // XML, and the disk would silently lose discard until the next stop/start. The scan still + // wins when the running VM is on SCSI, so a detail changed after the VM started cannot + // put a virtio disk next to its SCSI ones. + logger.debug("Attaching disk (deviceId={}) with the root controller virtio-blk", deviceId); + return DiskDef.DiskBus.VIRTIOBLK; + } return DiskDef.DiskBus.VIRTIO; } + /** + * libvirt < 10.1.0 cannot hot-plug an encrypted rbd blockdev (the LUKS secret is applied after the disk is + * opened), so refuse the attach with a clear message rather than letting libvirt fail with an opaque + * "No secret with id ..." error. Only the RBD hot-plug path is affected; booting a VM from an encrypted RBD + * disk works on older libvirt, so this does not gate the boot/root path. + */ + protected void ensureLibvirtSupportsEncryptedRbdHotplug(StoragePoolType poolType) { + if (poolType == StoragePoolType.RBD + && resource.getHypervisorLibvirtVersion() < MIN_LIBVIRT_VERSION_FOR_RBD_ENCRYPTED_HOTPLUG) { + throw new CloudRuntimeException("Libvirt version 10.1.0 required to attach an encrypted RBD volume to a running VM, but version " + + resource.getHypervisorLibvirtVersion() + " detected. Booting a VM from an encrypted RBD disk is not affected."); + } + } + @Override public Answer attachVolume(final AttachCommand cmd) { final DiskTO disk = cmd.getDisk(); @@ -1807,8 +1836,13 @@ public Answer attachVolume(final AttachCommand cmd) { final Connect conn = LibvirtConnection.getConnectionByVmName(vmName); DiskDef.LibvirtDiskEncryptDetails encryptDetails = null; if (vol.requiresEncryption()) { + // Encrypted RBD is decrypted by librbd inside qemu; hot-plugging it needs a libvirt new enough to + // emit the LUKS secret before the rbd blockdev. Booting from an encrypted RBD disk is unaffected. + ensureLibvirtSupportsEncryptedRbdHotplug(primaryStore.getPoolType()); String secretUuid = resource.createLibvirtVolumeSecret(conn, vol.getPath(), vol.getPassphrase()); - encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(vol.getEncryptFormat())); + // RBD volumes are encrypted natively by librbd, so request the librbd encryption engine. + String encryptEngine = (primaryStore.getPoolType() == StoragePoolType.RBD) ? "librbd" : null; + encryptDetails = new DiskDef.LibvirtDiskEncryptDetails(secretUuid, QemuObject.EncryptFormat.enumValue(vol.getEncryptFormat()), encryptEngine); vol.clearPassphrase(); } @@ -2568,33 +2602,36 @@ private CreateObjectAnswer takeClvmVolumeSnapshotOfStoppedVm(KVMPhysicalDisk dis * barriers properly (>2.6.32) this won't be any different then pulling the power * cord out of a running machine. */ - private Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { + protected Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { Long snapshotSize = null; + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage image = null; try { - Rados r = radosConnect(primaryPool); + r = radosConnect(primaryPool); - final IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir()); - final Rbd rbd = new Rbd(io); - final RbdImage image = rbd.open(disk.getName()); + io = r.ioCtxCreate(primaryPool.getSourceDir()); + rbd = new Rbd(io); + image = rbd.open(disk.getName()); logger.debug("Attempting to create RBD snapshot {}@{}", disk.getName(), snapshotName); image.snapCreate(snapshotName); - image.snapCreate(snapshotName); long rbdSnapshotSize = getRbdSnapshotSize(primaryPool.getSourceDir(), disk.getName(), snapshotName, primaryPool.getSourceHost(), primaryPool.getAuthUserName(), primaryPool.getAuthSecret()); if (rbdSnapshotSize > 0) { snapshotSize = rbdSnapshotSize; } - - rbd.close(image); - r.ioCtxDestroy(io); } catch (final Exception e) { logger.error("A RBD snapshot operation on [{}] failed. The error was: {}", disk.getName(), e.getMessage(), e); + } finally { + closeRbdImage(rbd, image, disk.getName()); + destroyRadosIoCtx(r, io, disk.getName()); } return snapshotSize; } - private long getRbdSnapshotSize(String poolPath, String diskName, String snapshotName, String rbdMonitor, String authUser, String authSecret) { + protected long getRbdSnapshotSize(String poolPath, String diskName, String snapshotName, String rbdMonitor, String authUser, String authSecret) { logger.debug("Get RBD snapshot size for {}/{}@{}", poolPath, diskName, snapshotName); //cmd: rbd du /@ --format json --mon-host --id --key 2>/dev/null String snapshotDetailsInJson = Script.runSimpleBashScript(String.format("rbd du %s/%s@%s --format json --mon-host %s --id %s --key %s 2>/dev/null", poolPath, diskName, snapshotName, rbdMonitor, authUser, authSecret)); @@ -2881,7 +2918,7 @@ protected boolean isAvailablePoolSizeDividedByDiskSizeLesserThanMinRate(long ava return ((availablePoolSize * 1d) / (diskSize * 1d)) < MIN_RATE_BETWEEN_AVAILABLE_POOL_AND_DISK_SIZE_TO_TAKE_DISK_SNAPSHOT; } - private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { + protected Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { Rados r = new Rados(primaryPool.getAuthUserName()); r.confSet(CEPH_MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); r.confSet(CEPH_AUTH_KEY, primaryPool.getAuthSecret()); @@ -2891,6 +2928,50 @@ private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosExcepti return r; } + /** + * Closes an RBD image if it was opened; never throws. An image left open keeps this client's RBD + * exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and keeps + * the image busy so it cannot be removed. + */ + protected void closeRbdImage(Rbd rbd, RbdImage image, String imageName) { + if (image == null) { + return; + } + try { + rbd.close(image); + } catch (final Exception e) { + logger.warn("Failed to close RBD image [{}]. The error was: {}", imageName, e.getMessage(), e); + } + } + + /** Destroys a RADOS IO context if it was created; never throws. */ + protected void destroyRadosIoCtx(Rados r, IoCTX io, String contextDescription) { + if (io == null) { + return; + } + try { + r.ioCtxDestroy(io); + } catch (final Exception e) { + logger.warn("Failed to destroy the RADOS IO context used for [{}]. The error was: {}", contextDescription, e.getMessage(), e); + } + } + + /** + * Unprotects an RBD snapshot if it was protected; never throws. A snapshot left protected cannot + * be deleted, and neither can its volume. + */ + protected void unprotectRbdSnapshot(RbdImage image, String snapshotName, boolean snapProtected) { + if (!snapProtected) { + return; + } + try { + image.snapUnprotect(snapshotName); + } catch (final Exception e) { + logger.error("Failed to unprotect RBD snapshot [{}]; it and its volume cannot be deleted until this is resolved manually. The error was: {}", + snapshotName, e.getMessage(), e); + } + } + @Override public Answer deleteVolume(final DeleteCommand cmd) { final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData(); @@ -3046,17 +3127,24 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S disk.setSize(size > volume.getVirtualSize() ? size : volume.getVirtualSize()); disk.setVirtualSize(size > volume.getVirtualSize() ? size : disk.getSize()); + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage srcImage = null; + RbdImage diskImage = null; + boolean snapProtected = false; + try { - Rados r = new Rados(srcPool.getAuthUserName()); + r = new Rados(srcPool.getAuthUserName()); r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); r.confSet("key", srcPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); - IoCTX io = r.ioCtxCreate(srcPool.getSourceDir()); - Rbd rbd = new Rbd(io); - RbdImage srcImage = rbd.open(volume.getName()); + io = r.ioCtxCreate(srcPool.getSourceDir()); + rbd = new Rbd(io); + srcImage = rbd.open(volume.getName()); List snaps = srcImage.snapList(); boolean snapFound = false; @@ -3072,23 +3160,26 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S return null; } srcImage.snapProtect(snapshotName); + snapProtected = true; logger.debug(String.format("Try to clone snapshot %s on RBD", snapshotName)); rbd.clone(volume.getName(), snapshotName, io, disk.getName(), LibvirtStorageAdaptor.RBD_FEATURES, 0); - RbdImage diskImage = rbd.open(disk.getName()); + diskImage = rbd.open(disk.getName()); if (disk.getVirtualSize() > volume.getVirtualSize()) { diskImage.resize(disk.getVirtualSize()); } diskImage.flatten(); - rbd.close(diskImage); - - srcImage.snapUnprotect(snapshotName); - rbd.close(srcImage); - r.ioCtxDestroy(io); } catch (RadosException | RbdException e) { logger.error(String.format("Failed due to %s", e.getMessage()), e); disk = null; + } finally { + // Every handle has to be released on all paths, including the "snapshot not found" return and + // any failure of clone/resize/flatten. + closeRbdImage(rbd, diskImage, newUuid); + unprotectRbdSnapshot(srcImage, snapshotName, snapProtected); + closeRbdImage(rbd, srcImage, volume.getName()); + destroyRadosIoCtx(r, io, snapshotName); } return disk; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 4bfac31b68f9..5f5cfdfe7f70 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -34,7 +34,9 @@ import com.cloud.agent.properties.AgentProperties; import com.cloud.agent.properties.AgentPropertiesFileHandler; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.utils.cryptsetup.CryptSetup; import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.rbd.RbdEncryption; import org.apache.cloudstack.utils.qemu.QemuImageOptions; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; @@ -95,6 +97,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { private static final int RBD_FEATURE_DEEP_FLATTEN = 32; public static final int RBD_FEATURES = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN; private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */ + /* Space reserved at the front of an encrypted RBD image for the LUKS2 header/keyslots so the + usable (decrypted) size still matches the requested volume size. */ + private static final long LUKS2_HEADER_RESERVE_BYTES = 16L << 20; // 16 MiB private static final Set QEMU_IMG_MANAGED_POOL_TYPES = Set.of(StoragePoolType.NetworkFilesystem, StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint); @@ -988,8 +993,18 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Map details = pool.getDetails(); String dataPool = (details == null) ? null : details.get(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL); - return (dataPool == null) ? createPhysicalDiskByLibVirt(name, pool, PhysicalDiskFormat.RAW, provisioningType, size) : - createPhysicalDiskByQemuImg(name, pool, PhysicalDiskFormat.RAW, provisioningType, size, passphrase); + // Create the raw RBD image first. For encrypted volumes we apply a native librbd LUKS header + // afterwards via `rbd encryption format` (engine='librbd'). We deliberately do NOT hand the + // passphrase to qemu-img, which would instead produce a qemu-native LUKS container. + KVMPhysicalDisk disk = (dataPool == null) ? + createPhysicalDiskByLibVirt(name, pool, PhysicalDiskFormat.RAW, provisioningType, size) : + createPhysicalDiskByQemuImg(name, pool, PhysicalDiskFormat.RAW, provisioningType, size, null); + + if (passphrase != null && passphrase.length > 0) { + formatRbdImageEncryption(pool, name, passphrase); + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + } + return disk; } else if (QEMU_IMG_MANAGED_POOL_TYPES.contains(poolType)) { switch (format) { case QCOW2: @@ -1244,7 +1259,7 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, KVMPhysicalDisk disk = null; if (destPool.getType() == StoragePoolType.RBD) { - disk = createDiskFromTemplateOnRBD(template, name, format, provisioningType, size, destPool, timeout); + disk = createDiskFromTemplateOnRBD(template, name, format, provisioningType, size, destPool, timeout, passphrase); } else { try (KeyFile keyFile = new KeyFile(passphrase)){ String newUuid = name; @@ -1324,7 +1339,7 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, } private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, - String name, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, KVMStoragePool destPool, int timeout){ + String name, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, KVMStoragePool destPool, int timeout, byte[] passphrase){ /* With RBD you can't run qemu-img convert with an existing RBD image as destination @@ -1351,6 +1366,16 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, } + if (passphrase != null && passphrase.length > 0) { + boolean sameClusterRbd = srcPool.getType() == StoragePoolType.RBD + && srcPool.getSourceHost().equals(destPool.getSourceHost()) + && srcPool.getSourceDir().equals(destPool.getSourceDir()); + if (sameClusterRbd) { + return createEncryptedRootCoWClone(template, destPool, newUuid, disk, passphrase); + } + return createEncryptedRootFullCopy(srcPool, template, destPool, newUuid, disk, passphrase); + } + QemuImgFile srcFile; QemuImgFile destFile = new QemuImgFile(KVMPhysicalDisk.RBDStringBuilder(destPool, disk.getPath())); destFile.setFormat(format); @@ -1493,9 +1518,139 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, disk = null; } } + + // Encrypted volumes are handled by the early return above (create empty -> luks2 format -> + // import template through encryption); the clone/convert path here is for plaintext volumes. + return disk; + } + + /** + * Option A (thin CoW encrypted root), used when the template already lives on the same RBD cluster + * as the destination pool. Per the Ceph "Image Encryption" clone recipe: grow the template base to + * reserve LUKS2-header space, snapshot+protect that grown state, clone from it, apply a LUKS2 header, + * then resize the clone to the requested size. The inherited (plaintext) template data stays readable + * through the clone's encryption, and the clone is a thin CoW image (only the header is written). + * + * @return the encrypted CoW clone, or {@code null} if the Ceph operations failed + */ + private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KVMStoragePool destPool, + String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { + // not "-luks": templates prepared before carry a snapshot of that name that still has holes + String luksReservedSnapshotName = rbdTemplateSnapName + "-luks2"; + Rados radosConnection = null; + IoCTX ioContext = null; + Rbd rbdClient = null; + RbdImage templateImage = null; + try { + radosConnection = new Rados(destPool.getAuthUserName()); + radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + radosConnection.confSet("key", destPool.getAuthSecret()); + radosConnection.confSet("client_mount_timeout", "30"); + radosConnection.connect(); + ioContext = radosConnection.ioCtxCreate(destPool.getSourceDir()); + rbdClient = new Rbd(ioContext); + templateImage = rbdClient.open(template.getName()); + boolean luksSnapshotExists = false; + for (RbdSnapInfo snapshotInfo : templateImage.snapList()) { + if (luksReservedSnapshotName.equals(snapshotInfo.name)) { + luksSnapshotExists = true; + break; + } + } + if (!luksSnapshotExists) { + templateImage.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES); + // fill the template's holes first: an encrypted clone reads a hole in its parent as + // garbage once a guest write copies up the object around it + new RbdEncryption().copyTemplateDense(destPool.getSourceHost(), destPool.getSourcePort(), + destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), + template.getName(), rbdTemplateSnapName); + try { + templateImage.snapCreate(luksReservedSnapshotName); + templateImage.snapProtect(luksReservedSnapshotName); + } catch (RbdException e) { + // fine if another host prepared the same template at the same time + if (!templateImage.snapIsProtected(luksReservedSnapshotName)) { + throw e; + } + } + logger.debug("Prepared LUKS-reserved template snapshot {}@{}", template.getName(), luksReservedSnapshotName); + } + rbdClient.clone(template.getName(), luksReservedSnapshotName, ioContext, newUuid, RBD_FEATURES, rbdOrder); + } catch (RadosException | RbdException e) { + logger.error("Failed to create encrypted CoW clone {}: {}", newUuid, e.getMessage()); + return null; + } finally { + if (rbdClient != null && templateImage != null) { + try { + rbdClient.close(templateImage); + } catch (RbdException ignored) { + // best-effort close of the template handle + } + } + if (radosConnection != null && ioContext != null) { + radosConnection.ioCtxDestroy(ioContext); + } + } + formatRbdImageEncryption(destPool, newUuid, passphrase); + if (disk.getVirtualSize() > template.getVirtualSize()) { + // grow the clone to the requested root size (encryption-aware) + new RbdEncryption().resize(destPool.getSourceHost(), destPool.getSourcePort(), + destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(), + newUuid, disk.getVirtualSize(), false, passphrase); + } + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); return disk; } + /** + * Option B (full-copy encrypted root), used when the template is not on the same RBD cluster (e.g. first + * use from secondary storage). Create an empty image, apply a LUKS2 header, then import the template + * THROUGH the encryption layer (qemu-img convert -n). Correct but not thin (no CoW). + * + * @return the encrypted image, or {@code null} if the Ceph operations failed + */ + private KVMPhysicalDisk createEncryptedRootFullCopy(KVMStoragePool srcPool, KVMPhysicalDisk template, + KVMStoragePool destPool, String newUuid, KVMPhysicalDisk disk, byte[] passphrase) { + long imageSizeWithLuksHeader = disk.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES; + Rados radosConnection = null; + IoCTX ioContext = null; + try { + radosConnection = new Rados(destPool.getAuthUserName()); + radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); + radosConnection.confSet("key", destPool.getAuthSecret()); + radosConnection.confSet("client_mount_timeout", "30"); + radosConnection.connect(); + ioContext = radosConnection.ioCtxCreate(destPool.getSourceDir()); + Rbd rbdClient = new Rbd(ioContext); + rbdClient.create(newUuid, imageSizeWithLuksHeader, RBD_FEATURES, rbdOrder); + } catch (RadosException | RbdException e) { + logger.error("Failed to create encrypted RBD image {}: {}", newUuid, e.getMessage()); + return null; + } finally { + if (radosConnection != null && ioContext != null) { + radosConnection.ioCtxDestroy(ioContext); + } + } + formatRbdImageEncryption(destPool, newUuid, passphrase); + boolean sourceIsRbdPool = srcPool.getType() == StoragePoolType.RBD; + new RbdEncryption().importTemplate( + sourceIsRbdPool ? srcPool.getSourceDir() : null, sourceIsRbdPool ? template.getName() : null, + sourceIsRbdPool ? null : template.getPath(), sourceIsRbdPool ? null : template.getFormat().toString(), + destPool.getSourceHost(), destPool.getSourcePort(), destPool.getAuthUserName(), destPool.getAuthSecret(), + destPool.getSourceDir(), newUuid, passphrase, CryptSetup.LuksType.LUKS2); + disk.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS2); + return disk; + } + + /** + * Apply native librbd LUKS encryption to an existing RBD image via the rbd CLI. + * Isolated here so the CLI dependency can later be swapped for a native (JNA) librbd binding. + */ + private void formatRbdImageEncryption(KVMStoragePool pool, String image, byte[] passphrase) { + new RbdEncryption().format(pool.getSourceHost(), pool.getSourcePort(), pool.getAuthUserName(), + pool.getAuthSecret(), pool.getSourceDir(), image, passphrase, CryptSetup.LuksType.LUKS2); + } + @Override public KVMPhysicalDisk createTemplateFromDisk(KVMPhysicalDisk disk, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool) { return null; diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java index 4a577ef3400c..10e39cb5ffcb 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImageOptions.java @@ -86,8 +86,17 @@ public String[] toCommandFlag() { return new String[] { params.get(FILENAME_PARAM_KEY) }; } } + return toCommandFlag(QemuImg.IMAGE_OPTS_FLAG); + } + + /** + * Converts QemuImageOptions into the command strings under the given qemu-img flag, + * e.g. {@link QemuImg#TARGET_IMAGE_OPTS_FLAG} for a convert destination. + * @return array of strings representing the flag and its options value + */ + public String[] toCommandFlag(String flagName) { Map sorted = new TreeMap<>(params); String paramString = Joiner.on(",").withKeyValueSeparator("=").join(sorted); - return new String[] {"--image-opts", paramString}; + return new String[] {flagName, paramString}; } } diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java index cae6832999eb..0b5010adf365 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java @@ -52,6 +52,8 @@ public class QemuImg { public static final String ENCRYPT_FORMAT = "encrypt.format"; public static final String ENCRYPT_KEY_SECRET = "encrypt.key-secret"; public static final String TARGET_ZERO_FLAG = "--target-is-zero"; + public static final String IMAGE_OPTS_FLAG = "--image-opts"; + public static final String TARGET_IMAGE_OPTS_FLAG = "--target-image-opts"; public static final String PREALLOCATION = "preallocation"; public static final long QEMU_2_10 = 2010000; public static final long QEMU_5_1 = 5001000; @@ -64,6 +66,7 @@ public class QemuImg { private String cloudQemuImgPath = "cloud-qemu-img"; private long timeout; private boolean skipZero = false; + private boolean writeZeroRanges = false; private boolean skipTargetVolumeCreation = false; private boolean noCache = false; private long version; @@ -402,6 +405,21 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, convert(srcFile, destFile, null, options, qemuObjects, srcImageOpts, snapshotName, forceSourceFormat, false, false, false, null, null); } + /** + * Converts an image into an existing destination that is described by explicit image options + * ({@code --target-image-opts}) instead of a plain filename - for example an RBD image written + * through librbd encryption ({@code driver=rbd,...,encrypt.format=...}). The destination is + * never created ({@code -n} is implied) and must already exist with the wanted size and format. + * + * @param destImageOpts + * image options describing the existing destination; passed as --target-image-opts. + */ + public void convertIntoExistingTarget(final QemuImgFile srcFile, final Map options, + final List qemuObjects, final QemuImageOptions srcImageOpts, final QemuImageOptions destImageOpts, + final boolean forceSourceFormat) throws QemuImgException { + convert(srcFile, null, null, options, qemuObjects, srcImageOpts, destImageOpts, null, forceSourceFormat, false, false, false, null, null); + } + protected Map getResizeOptionsFromConvertOptions(final Map options) { if (MapUtils.isEmpty(options)) { return null; @@ -450,6 +468,25 @@ protected Map getResizeOptionsFromConvertOptions(final Map options, final List qemuObjects, final QemuImageOptions srcImageOpts, final String snapshotName, final boolean forceSourceFormat, boolean keepBitmaps, boolean outOfOrderWrites, boolean compress, Integer coroutines, Integer rateLimit) throws QemuImgException { + convert(srcFile, destFile, backingFile, options, qemuObjects, srcImageOpts, null, snapshotName, forceSourceFormat, keepBitmaps, outOfOrderWrites, compress, coroutines, + rateLimit); + } + + /** + * Converts an image from source to destination, optionally into an existing destination described by explicit image options + * ({@code --target-image-opts}) instead of a plain filename; see {@link #convertIntoExistingTarget}. All other parameters + * behave as documented above. + * + * @param destImageOpts + * If not null, the destination is described by these image options and {@code destFile} is unused. + */ + public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuImgFile backingFile, final Map options, final List qemuObjects, + final QemuImageOptions srcImageOpts, final QemuImageOptions destImageOpts, final String snapshotName, final boolean forceSourceFormat, boolean keepBitmaps, + boolean outOfOrderWrites, boolean compress, Integer coroutines, Integer rateLimit) throws QemuImgException { + if (destImageOpts != null && this.version < QEMU_2_10) { + throw new QemuImgException(String.format("qemu >= 2.10 is required to convert into a destination described by %s", TARGET_IMAGE_OPTS_FLAG)); + } + Script script = new Script(_qemuImgPath, timeout); if (StringUtils.isNotBlank(snapshotName)) { String qemuPath = Script.runSimpleBashScript(getQemuImgPathScript); @@ -458,7 +495,10 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("convert"); - if (skipZero && Files.exists(Paths.get(destFile.getFileName()))) { + if (destImageOpts != null) { + // a destination described by image options always exists already; qemu-img requires -n with --target-image-opts + script.add("-n"); + } else if (skipZero && Files.exists(Paths.get(destFile.getFileName()))) { script.add("-n"); script.add(TARGET_ZERO_FLAG); script.add("-W"); @@ -469,8 +509,15 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("-n"); } - script.add("-O"); - script.add(destFile.getFormat().toString()); + if (writeZeroRanges) { + script.add("-S"); + script.add("0"); + } + + if (destImageOpts == null) { + script.add("-O"); + script.add(destFile.getFormat().toString()); + } addBackingFileToConvertCommand(script, backingFile); addScriptOptionsFromMap(options, script); @@ -524,14 +571,19 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI script.add("--bitmaps"); } - script.add(destFile.getFileName()); + if (destImageOpts != null) { + script.add(destImageOpts.toCommandFlag(TARGET_IMAGE_OPTS_FLAG)); + } else { + script.add(destFile.getFileName()); + } final String result = script.execute(); if (result != null) { throw new QemuImgException(result); } - if (srcFile.getSize() < destFile.getSize()) { + // an image-options destination already exists with its final size; 'qemu-img resize' cannot address it by filename + if (destImageOpts == null && srcFile.getSize() < destFile.getSize()) { this.resize(destFile, destFile.getSize(), getResizeOptionsFromConvertOptions(options)); } } @@ -965,6 +1017,14 @@ public void setSkipZero(boolean skipZero) { this.skipZero = skipZero; } + /** + * Make convert write the source's zero ranges to the destination ({@code -S 0}) instead of + * leaving them unallocated. + */ + public void setWriteZeroRanges(boolean writeZeroRanges) { + this.writeZeroRanges = writeZeroRanges; + } + public void setSkipTargetVolumeCreation(boolean skipTargetVolumeCreation) { this.skipTargetVolumeCreation = skipTargetVolumeCreation; } diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java index efeee04cb90f..511e91074969 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuObject.java @@ -54,6 +54,7 @@ public enum ObjectParameter { */ public enum EncryptFormat { LUKS("luks"), + LUKS2("luks2"), AES("aes"); private final String format; diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java new file mode 100644 index 000000000000..833fb82807be --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java @@ -0,0 +1,330 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.utils.rbd; + +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.cloudstack.utils.cryptsetup.CryptSetup; +import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgException; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.cloudstack.utils.qemu.QemuObject; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.libvirt.LibvirtException; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumMap; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Thin wrapper around the {@code rbd} CLI to apply native librbd LUKS encryption to an + * RBD image via {@code rbd encryption format}. This is only used at volume create time; + * runtime decryption is handled by libvirt/qemu through {@code }. + * + * The CLI dependency is intentionally isolated in this class so it can later be replaced + * by a native librbd (JNA) binding without touching callers. rados-java (0.x) does not + * expose the rbd_encryption_format API, hence the CLI for now. + * + * The command builders ({@code build*Script}) are separated from execution so the generated + * argv can be unit-tested without a live Ceph cluster (see {@code RbdEncryptionTest}). + */ +public class RbdEncryption { + protected Logger logger = LogManager.getLogger(getClass()); + + protected String commandPath = "rbd"; + + /** qemu secret id used to hand the LUKS passphrase to qemu-img during template import */ + protected static final String LUKS_SECRET_ID = "luks0"; + + public RbdEncryption() {} + + public RbdEncryption(String commandPath) { + this.commandPath = commandPath; + } + + private static String monSpec(String monHost, int monPort) { + return monPort > 0 ? monHost + ":" + monPort : monHost; + } + + /** + * Apply a LUKS header to an existing RBD image so librbd can transparently encrypt it. + *

+ * cephx authentication is supplied via {@code --id} plus a temporary keyfile so the secret + * never appears on the command line. The LUKS passphrase is supplied via a temporary file + * ({@link KeyFile}). Both temp files are deleted when this method returns. + * + * @param monHost ceph monitor host + * @param monPort ceph monitor port (0 to omit) + * @param authUser cephx user (e.g. "cloudstack"); null to skip --id + * @param authSecret cephx secret/key (as used for ceph "key" config); null to skip --keyfile + * @param cephPool ceph pool name + * @param image rbd image name + * @param passphrase LUKS passphrase + * @param luksType LUKS1/LUKS2 (librbd engine supports both; LUKS2 recommended) + */ + public void format(String monHost, int monPort, String authUser, String authSecret, + String cephPool, String image, byte[] passphrase, CryptSetup.LuksType luksType) { + final String imageSpec = cephPool + "/" + image; + if (passphrase == null || passphrase.length == 0) { + throw new CloudRuntimeException("Cannot LUKS-format RBD image " + imageSpec + ": empty passphrase"); + } + try (KeyFile passFile = new KeyFile(passphrase); + KeyFile cephKeyFile = new KeyFile(authSecret == null ? null : authSecret.getBytes(StandardCharsets.UTF_8))) { + final Script script = buildFormatScript(imageSpec, luksType, passFile.toString(), + monSpec(monHost, monPort), authUser, cephKeyFile.isSet() ? cephKeyFile.toString() : null); + final String result = script.execute(); + if (result != null) { + throw new CloudRuntimeException(String.format("Failed to apply librbd %s encryption to %s: %s", luksType, imageSpec, result)); + } + logger.debug("Applied {} encryption to RBD image {}", luksType, imageSpec); + } catch (IOException ex) { + throw new CloudRuntimeException(String.format("Failed to apply librbd %s encryption to %s", luksType, imageSpec), ex); + } + } + + protected Script buildFormatScript(String imageSpec, CryptSetup.LuksType luksType, String passFilePath, + String monSpec, String authUser, String cephKeyFilePath) { + final Script script = new Script(commandPath); + script.add("encryption"); + script.add("format"); + script.add(imageSpec); + script.add(luksType.toString()); + script.add(passFilePath); + script.add("--mon-host"); + script.add(monSpec); + if (authUser != null) { + script.add("--id"); + script.add(authUser); + } + if (cephKeyFilePath != null) { + script.add("--keyfile"); + script.add(cephKeyFilePath); + } + return script; + } + + /** + * Resize an encrypted RBD image. librbd needs the passphrase so it can resize the encrypted + * payload (not just the raw image) and keep the LUKS header consistent. {@code newSizeBytes} is + * the usable (decrypted) size requested; rbd {@code --size} is expressed in MiB. + * + * @param allowShrink pass --allow-shrink when shrinking is permitted + */ + public void resize(String monHost, int monPort, String authUser, String authSecret, + String cephPool, String image, long newSizeBytes, boolean allowShrink, byte[] passphrase) { + final String imageSpec = cephPool + "/" + image; + if (passphrase == null || passphrase.length == 0) { + throw new CloudRuntimeException("Cannot resize encrypted RBD image " + imageSpec + ": empty passphrase"); + } + // rbd --size is in MiB; round up so a non-MiB-aligned request never shrinks the volume below what was asked for. + final long sizeMiB = (newSizeBytes + (1024L * 1024L) - 1) / (1024L * 1024L); + try (KeyFile passFile = new KeyFile(passphrase); + KeyFile cephKeyFile = new KeyFile(authSecret == null ? null : authSecret.getBytes(StandardCharsets.UTF_8))) { + final Script script = buildResizeScript(imageSpec, sizeMiB, passFile.toString(), allowShrink, + monSpec(monHost, monPort), authUser, cephKeyFile.isSet() ? cephKeyFile.toString() : null); + final String result = script.execute(); + if (result != null) { + throw new CloudRuntimeException(String.format("Failed to resize encrypted RBD image %s to %d MiB: %s", imageSpec, sizeMiB, result)); + } + logger.debug("Resized encrypted RBD image {} to {} MiB", imageSpec, sizeMiB); + } catch (IOException ex) { + throw new CloudRuntimeException(String.format("Failed to resize encrypted RBD image %s", imageSpec), ex); + } + } + + protected Script buildResizeScript(String imageSpec, long sizeMiB, String passFilePath, boolean allowShrink, + String monSpec, String authUser, String cephKeyFilePath) { + final Script script = new Script(commandPath); + script.add("resize"); + script.add("--size"); + script.add(String.valueOf(sizeMiB)); + script.add(imageSpec); + script.add("--encryption-passphrase-file"); + script.add(passFilePath); + if (allowShrink) { + script.add("--allow-shrink"); + } + script.add("--mon-host"); + script.add(monSpec); + if (authUser != null) { + script.add("--id"); + script.add(authUser); + } + if (cephKeyFilePath != null) { + script.add("--keyfile"); + script.add(cephKeyFilePath); + } + return script; + } + + /** + * Import a template into an already-created, already-LUKS-formatted RBD image by writing it + * THROUGH the librbd encryption layer with {@code qemu-img convert -n} (so the data lands + * encrypted). This is how encrypted root disks are populated: we never clone-then-format a + * plaintext template (that leaves the inherited OS data unreadable) — instead we format an + * empty image and convert the template into it. + * + * Exactly one source must be given: an RBD image ({@code srcRbdPool}+{@code srcRbdImage}) or a + * local file ({@code srcFilePath}[+{@code srcFileFormat}]). cephx auth is provided to qemu-img + * via a temporary ceph.conf + keyring (deleted on return). The conversion itself goes through + * {@link QemuImg#convertIntoExistingTarget}. + */ + public void importTemplate(String srcRbdPool, String srcRbdImage, + String srcFilePath, String srcFileFormat, + String monHost, int monPort, String authUser, String authSecret, + String cephPool, String destImage, byte[] passphrase, CryptSetup.LuksType luksType) { + final String imageSpec = cephPool + "/" + destImage; + if (passphrase == null || passphrase.length == 0) { + throw new CloudRuntimeException("Cannot import template into encrypted RBD image " + imageSpec + ": empty passphrase"); + } + Path conf = null; + Path keyring = null; + try (KeyFile passFile = new KeyFile(passphrase)) { + // These temp files hold the cephx secret; create them 0600 up front (matching KeyFile) rather than relying on the umask. + final FileAttribute ownerOnly = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")); + keyring = Files.createTempFile("cs-ceph-", ".keyring", ownerOnly); + Files.writeString(keyring, "[client." + authUser + "]\n\tkey = " + authSecret + "\n"); + conf = Files.createTempFile("cs-ceph-", ".conf", ownerOnly); + Files.writeString(conf, "[global]\nmon_host = " + monSpec(monHost, monPort) + "\nkeyring = " + keyring + "\n"); + + QemuImgFile srcQemuFile; + QemuImageOptions srcImageOpts; + boolean forceSourceFormat = false; + if (srcRbdImage != null) { + srcQemuFile = new QemuImgFile(srcRbdPool + "/" + srcRbdImage, QemuImg.PhysicalDiskFormat.RAW); + srcImageOpts = new QemuImageOptions(rbdImageOptions(srcRbdPool, srcRbdImage, conf.toString(), authUser)); + srcImageOpts.setImageOptsFlag(true); + } else { + QemuImg.PhysicalDiskFormat srcFormat = srcFileFormat != null ? QemuImg.PhysicalDiskFormat.valueOf(srcFileFormat.toUpperCase()) : null; + srcQemuFile = srcFormat != null ? new QemuImgFile(srcFilePath, srcFormat) : new QemuImgFile(srcFilePath); + srcImageOpts = new QemuImageOptions(srcFilePath); + if (srcFormat != null) { + // emit the source as --image-opts driver=,file.filename= (the -f equivalent) + srcImageOpts.setImageOptsFlag(true); + forceSourceFormat = true; + } + } + + Map destParams = rbdImageOptions(cephPool, destImage, conf.toString(), authUser); + destParams.put("encrypt.format", luksType.toString()); + destParams.put("encrypt.key-secret", LUKS_SECRET_ID); + QemuImageOptions destImageOpts = new QemuImageOptions(destParams); + + EnumMap secretParams = new EnumMap<>(QemuObject.ObjectParameter.class); + secretParams.put(QemuObject.ObjectParameter.ID, LUKS_SECRET_ID); + secretParams.put(QemuObject.ObjectParameter.FILE, passFile.toString()); + QemuObject luksSecret = new QemuObject(QemuObject.ObjectType.SECRET, secretParams); + + QemuImg qemu = createQemuImg(); + qemu.convertIntoExistingTarget(srcQemuFile, null, List.of(luksSecret), srcImageOpts, destImageOpts, forceSourceFormat); + logger.debug("Imported template into encrypted RBD image {}", imageSpec); + } catch (IOException | QemuImgException | LibvirtException ex) { + throw new CloudRuntimeException(String.format("Failed to import template into encrypted RBD image %s", imageSpec), ex); + } finally { + deleteQuietly(conf); + deleteQuietly(keyring); + } + } + + /** + * Seam for unit tests; {@link QemuImg} probes the qemu version through libvirt on construction. + */ + /** + * Rewrite an RBD image from one of its snapshots, writing out the zero ranges too ({@code -S 0}), + * so the image has no holes. Used on a template before taking the snapshot encrypted roots are + * cloned from: a hole in that parent reads as garbage through the clone's encryption once a + * guest write copies up the object around it. + */ + public void copyTemplateDense(String monHost, int monPort, String authUser, String authSecret, + String cephPool, String image, String srcSnapshot) { + Path conf = null; + Path keyring = null; + try { + final FileAttribute ownerOnly = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------")); + keyring = Files.createTempFile("cs-ceph-", ".keyring", ownerOnly); + Files.writeString(keyring, "[client." + authUser + "]\n\tkey = " + authSecret + "\n"); + conf = Files.createTempFile("cs-ceph-", ".conf", ownerOnly); + Files.writeString(conf, "[global]\nmon_host = " + monSpec(monHost, monPort) + "\nkeyring = " + keyring + "\n"); + + QemuImgFile srcQemuFile = new QemuImgFile(cephPool + "/" + image + "@" + srcSnapshot, QemuImg.PhysicalDiskFormat.RAW); + Map srcParams = rbdImageOptions(cephPool, image, conf.toString(), authUser); + srcParams.put("snapshot", srcSnapshot); + QemuImageOptions srcImageOpts = new QemuImageOptions(srcParams); + srcImageOpts.setImageOptsFlag(true); + QemuImageOptions destImageOpts = new QemuImageOptions(rbdImageOptions(cephPool, image, conf.toString(), authUser)); + + QemuImg qemu = createQemuImg(); + qemu.setWriteZeroRanges(true); + qemu.convertIntoExistingTarget(srcQemuFile, null, null, srcImageOpts, destImageOpts, false); + logger.debug("Rewrote RBD image {}/{} from @{} without holes", cephPool, image, srcSnapshot); + } catch (IOException | QemuImgException | LibvirtException ex) { + throw new CloudRuntimeException(String.format("Failed to rewrite RBD image %s/%s from @%s", cephPool, image, srcSnapshot), ex); + } finally { + deleteQuietly(conf); + deleteQuietly(keyring); + } + } + + protected QemuImg createQemuImg() throws QemuImgException, LibvirtException { + return new QemuImg(0); + } + + /** qemu image options addressing an RBD image (as used with --image-opts / --target-image-opts). */ + private static Map rbdImageOptions(String cephPool, String image, String confPath, String authUser) { + Map opts = new HashMap<>(); + opts.put("driver", "rbd"); + opts.put("pool", cephPool); + opts.put("image", image); + opts.put("conf", confPath); + if (authUser != null) { + opts.put("user", authUser); + } + return opts; + } + + private static void deleteQuietly(Path p) { + if (p == null) { + return; + } + try { + Files.deleteIfExists(p); + } catch (IOException ignored) { + // best-effort cleanup of the temporary ceph auth files + } + } + + /** + * Best-effort probe that the local rbd CLI supports the encryption subcommand. + */ + public boolean isSupported() { + final Script script = new Script(commandPath); + script.add("help"); + script.add("encryption"); + script.add("format"); + return script.execute() == null; + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 56ad267eac7e..9b52874fddb1 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -19,6 +19,9 @@ package com.cloud.hypervisor.kvm.resource; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.io.File; import java.util.Arrays; import java.util.List; @@ -597,4 +600,71 @@ public void testTpmModel() { "\n" + "\n", tpmDef.toString()); } + + @Test + public void testDiskDefVirtioBlkUsesVirtioBusAndVdLabel() { + DiskDef disk = new DiskDef(); + disk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", 1, DiskDef.DiskBus.VIRTIOBLK, DiskDef.DiskFmtType.QCOW2); + + String xmlDef = disk.toString(); + // 'virtio-blk' is a CloudStack controller name; libvirt only accepts the bus 'virtio' and + // rejects the whole domain XML otherwise, which is what stopped these VMs from starting. + assertTrue(xmlDef, xmlDef.contains("")); + assertFalse(xmlDef, xmlDef.contains("virtio-blk")); + assertEquals(DiskDef.DiskBus.VIRTIOBLK, disk.getBusType()); + } + + @Test + public void testDiskDefVirtioBlkLabelsMatchVirtio() { + for (int devId = 0; devId < 4; devId++) { + DiskDef virtio = new DiskDef(); + virtio.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", devId, DiskDef.DiskBus.VIRTIO, DiskDef.DiskFmtType.QCOW2); + DiskDef virtioBlk = new DiskDef(); + virtioBlk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", devId, DiskDef.DiskBus.VIRTIOBLK, DiskDef.DiskFmtType.QCOW2); + assertEquals(virtio.getDiskLabel(), virtioBlk.getDiskLabel()); + } + } + + @Test + public void testDiskDefVirtioBlkWithDiscardUnmap() { + DiskDef disk = new DiskDef(); + disk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", 0, DiskDef.DiskBus.VIRTIOBLK, DiskDef.DiskFmtType.QCOW2); + disk.setQemuDriver(true); + disk.setDiscard(DiskDef.DiscardType.UNMAP); + + String xmlDef = disk.toString(); + // the point of the virtio-blk controller: a virtio disk that passes the guest's discard through + assertTrue(xmlDef, xmlDef.contains("discard='unmap'")); + assertTrue(xmlDef, xmlDef.contains("bus='virtio'")); + assertFalse(xmlDef, xmlDef.contains("virtio-blk")); + } + + @Test + public void testDiskDefVirtioIsUnchanged() { + DiskDef disk = new DiskDef(); + disk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", 0, DiskDef.DiskBus.VIRTIO, DiskDef.DiskFmtType.QCOW2); + + String xmlDef = disk.toString(); + assertTrue(xmlDef, xmlDef.contains("")); + assertFalse(xmlDef, xmlDef.contains("discard=")); + } + + @Test + public void testDiskDefScsiIsUnchanged() { + DiskDef disk = new DiskDef(); + disk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", 0, DiskDef.DiskBus.SCSI, DiskDef.DiskFmtType.QCOW2); + + String xmlDef = disk.toString(); + assertTrue(xmlDef, xmlDef.contains("")); + } + + @Test + public void testDiskDefVirtioBlkGetsAnIothreadLikeVirtio() { + DiskDef disk = new DiskDef(); + disk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", 0, DiskDef.DiskBus.VIRTIOBLK, DiskDef.DiskFmtType.QCOW2); + disk.isIothreadsEnabled(true); + + String xmlDef = disk.toString(); + assertTrue(xmlDef, xmlDef.contains("iothread=")); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java index fc2341632984..5a9094ee6d68 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java @@ -581,4 +581,188 @@ public void testExecuteWithMultipleVolumes() throws Exception { } } } + + /** + * An in-place restore renumbers the device ids of the data disks, so the instance's volumes and + * the backed up volumes do not necessarily arrive in the same order. The backup of a volume must + * still be written into that same volume, matched by UUID rather than by position in the list. + */ + @Test + public void testRestoreOfExistingVmMapsBackupsToVolumesByUuid() throws Exception { + when(command.getVmName()).thenReturn("test-vm"); + when(command.getBackupPath()).thenReturn("backup/path"); + when(command.getBackupRepoAddress()).thenReturn("192.168.1.100:/backup"); + when(command.getBackupRepoType()).thenReturn("nfs"); + when(command.getMountOptions()).thenReturn("rw"); + when(command.isVmExists()).thenReturn(true); + when(command.getDiskType()).thenReturn("root"); + PrimaryDataStoreTO pool = Mockito.mock(PrimaryDataStoreTO.class); + lenient().when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(command.getRestoreVolumePools()).thenReturn(Arrays.asList(pool, pool, pool)); + // the instance's volumes, ordered by their CURRENT device ids: the data disks are swapped + // relative to the backup, which is the state left behind by a previous restore + when(command.getRestoreVolumePaths()).thenReturn(Arrays.asList( + "/var/lib/libvirt/images/root-vol", + "/var/lib/libvirt/images/data-vol-b", + "/var/lib/libvirt/images/data-vol-a" + )); + // the backed up volumes, ordered by the device ids recorded in the backup + when(command.getBackupVolumesUUIDs()).thenReturn(Arrays.asList("root-vol", "data-vol-a", "data-vol-b")); + when(command.getBackupFiles()).thenReturn(Arrays.asList("root-vol", "data-vol-a", "data-vol-b")); + when(command.getMountTimeout()).thenReturn(30); + + try (MockedStatic filesMock = mockStatic(Files.class)) { + Path tempPath = Mockito.mock(Path.class); + when(tempPath.toString()).thenReturn("/tmp/csbackup.abc123"); + filesMock.when(() -> Files.createTempDirectory(anyString())).thenReturn(tempPath); + + try (MockedStatic