Skip to content

Commit fa4fc8a

Browse files
committed
kvm: clone encrypted roots from a dense base image, not from the sparse template
An encrypted root is a librbd CoW clone of a plaintext template carrying a LUKS2 header. librbd serves the ranges the clone's own objects do not hold from that parent, as plaintext, which is what makes the inherited filesystem readable. That stops the moment an object exists in the clone: one 4 KiB guest write copies up the whole 4 MiB object, and the ranges inside it that the parent never materialised are from then on read through the crypto layer - where zeros decrypted with AES-XTS are not zeros. Measured on a clone of the stock Ubuntu 26.04 template (1684 MiB of its 4 GiB unallocated), Ceph 19.2.3 and 20.2.0: of 10 sectors that read as zeros before, 10 still read as zeros after a write elsewhere in the same object while the object lived in the parent, and 0 of 10 once the write had copied the object up. The damage is therefore latent and spreads with ordinary writes - a mostly-empty separate /boot shows it first, and ext4lazyinit walks the whole disk. Clone from a per-template base image holding a dense copy of the template instead: a parent with no holes has nothing left to materialise, so every range of the clone stays readable and the clone stays thin. The base is built once per template per pool, under a staging name and renamed into place at the very end, so a half-written base can never be cloned from and two hosts racing to build one both end up on a complete image. It is removed with the template. The template's own image is no longer touched - it keeps its sparseness and stays the parent of every plaintext clone, and no protected snapshot is left on it. QemuImg grows setWriteZeroRanges() for the -S 0 this needs, since qemu-img skips the source's zero ranges by default.
1 parent 3c71d1e commit fa4fc8a

5 files changed

Lines changed: 235 additions & 29 deletions

File tree

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java‎

Lines changed: 122 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
101101
usable (decrypted) size still matches the requested volume size. */
102102
private static final long LUKS2_HEADER_RESERVE_BYTES = 16L << 20; // 16 MiB
103103

104+
/** Suffix of the per-template base image encrypted roots are cloned from. */
105+
private static final String LUKS_CLONE_BASE_SUFFIX = "-luks";
106+
104107
private static final Set<StoragePoolType> QEMU_IMG_MANAGED_POOL_TYPES = Set.of(StoragePoolType.NetworkFilesystem, StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint);
105108

106109
public LibvirtStorageAdaptor(StorageLayer storage) {
@@ -1200,6 +1203,7 @@ public boolean deletePhysicalDisk(String uuid, KVMStoragePool pool, Storage.Imag
12001203
}
12011204
logger.info("Successfully unprotected and removed any remaining snapshots (" + snaps.size() + ") of "
12021205
+ pool.getSourceDir() + "/" + uuid + " Continuing to remove the RBD image");
1206+
removeLuksCloneBaseIfPresent(rbd, uuid);
12031207
} catch (RbdException e) {
12041208
logger.error("Failed to remove snapshot with exception: " + e.toString() +
12051209
", RBD error: " + ErrorCode.getErrorMessage(e.getReturnValue()));
@@ -1526,54 +1530,35 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template,
15261530

15271531
/**
15281532
* Option A (thin CoW encrypted root), used when the template already lives on the same RBD cluster
1529-
* as the destination pool. Per the Ceph "Image Encryption" clone recipe: grow the template base to
1530-
* reserve LUKS2-header space, snapshot+protect that grown state, clone from it, apply a LUKS2 header,
1531-
* then resize the clone to the requested size. The inherited (plaintext) template data stays readable
1532-
* through the clone's encryption, and the clone is a thin CoW image (only the header is written).
1533+
* as the destination pool: clone a per-template base image that carries LUKS2-header room, apply a
1534+
* LUKS2 header to the clone, then grow it to the requested size. The base is a dense copy of the
1535+
* template, not the template itself, because a hole in the parent of an encrypted clone reads as
1536+
* garbage once a write copies up the object around it. It is built once per template per pool.
15331537
*
15341538
* @return the encrypted CoW clone, or {@code null} if the Ceph operations failed
15351539
*/
15361540
private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KVMStoragePool destPool,
15371541
String newUuid, KVMPhysicalDisk disk, byte[] passphrase) {
1538-
String luksReservedSnapshotName = rbdTemplateSnapName + "-luks";
1542+
final String cloneBaseImage = template.getName() + LUKS_CLONE_BASE_SUFFIX;
15391543
Rados radosConnection = null;
15401544
IoCTX ioContext = null;
1541-
Rbd rbdClient = null;
1542-
RbdImage templateImage = null;
15431545
try {
15441546
radosConnection = new Rados(destPool.getAuthUserName());
15451547
radosConnection.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort());
15461548
radosConnection.confSet("key", destPool.getAuthSecret());
15471549
radosConnection.confSet("client_mount_timeout", "30");
15481550
radosConnection.connect();
15491551
ioContext = radosConnection.ioCtxCreate(destPool.getSourceDir());
1550-
rbdClient = new Rbd(ioContext);
1551-
templateImage = rbdClient.open(template.getName());
1552-
boolean luksSnapshotExists = false;
1553-
for (RbdSnapInfo snapshotInfo : templateImage.snapList()) {
1554-
if (luksReservedSnapshotName.equals(snapshotInfo.name)) {
1555-
luksSnapshotExists = true;
1556-
break;
1557-
}
1558-
}
1559-
if (!luksSnapshotExists) {
1560-
templateImage.resize(template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES);
1561-
templateImage.snapCreate(luksReservedSnapshotName);
1562-
templateImage.snapProtect(luksReservedSnapshotName);
1563-
logger.debug("Prepared LUKS-reserved template snapshot {}@{}", template.getName(), luksReservedSnapshotName);
1552+
Rbd rbdClient = new Rbd(ioContext);
1553+
if (!isLuksCloneBaseReady(rbdClient, cloneBaseImage)) {
1554+
buildLuksCloneBase(rbdClient, template, destPool, cloneBaseImage);
15641555
}
1565-
rbdClient.clone(template.getName(), luksReservedSnapshotName, ioContext, newUuid, RBD_FEATURES, rbdOrder);
1556+
rbdClient.clone(cloneBaseImage, rbdTemplateSnapName, ioContext, newUuid, RBD_FEATURES, rbdOrder);
1557+
logger.debug("Cloned {}/{}@{} to {}", destPool.getSourceDir(), cloneBaseImage, rbdTemplateSnapName, newUuid);
15661558
} catch (RadosException | RbdException e) {
15671559
logger.error("Failed to create encrypted CoW clone {}: {}", newUuid, e.getMessage());
15681560
return null;
15691561
} finally {
1570-
if (rbdClient != null && templateImage != null) {
1571-
try {
1572-
rbdClient.close(templateImage);
1573-
} catch (RbdException ignored) {
1574-
// best-effort close of the template handle
1575-
}
1576-
}
15771562
if (radosConnection != null && ioContext != null) {
15781563
radosConnection.ioCtxDestroy(ioContext);
15791564
}
@@ -1589,6 +1574,114 @@ private KVMPhysicalDisk createEncryptedRootCoWClone(KVMPhysicalDisk template, KV
15891574
return disk;
15901575
}
15911576

1577+
/** Does {@code cloneBaseImage} exist with its protected snapshot? It gets its name only once complete. */
1578+
private boolean isLuksCloneBaseReady(Rbd rbdClient, String cloneBaseImage) {
1579+
RbdImage image = null;
1580+
try {
1581+
image = rbdClient.open(cloneBaseImage);
1582+
for (RbdSnapInfo snapshotInfo : image.snapList()) {
1583+
if (rbdTemplateSnapName.equals(snapshotInfo.name)) {
1584+
return image.snapIsProtected(snapshotInfo.name);
1585+
}
1586+
}
1587+
return false;
1588+
} catch (RbdException e) {
1589+
// not there yet, or not readable: it gets built
1590+
return false;
1591+
} finally {
1592+
if (image != null) {
1593+
try {
1594+
rbdClient.close(image);
1595+
} catch (RbdException ignored) {
1596+
// best-effort close of the base image handle
1597+
}
1598+
}
1599+
}
1600+
}
1601+
1602+
/**
1603+
* Build the base image: template size plus LUKS2-header room, a dense copy of the template, and a
1604+
* protected snapshot. It is built under a staging name and renamed at the end, so a half-written
1605+
* base is never cloned; a host that loses the rename to another one discards its copy.
1606+
*/
1607+
private void buildLuksCloneBase(Rbd rbdClient, KVMPhysicalDisk template, KVMStoragePool destPool,
1608+
String cloneBaseImage) throws RbdException {
1609+
final String stagingImage = cloneBaseImage + "." + UUID.randomUUID();
1610+
logger.info("Building dense LUKS clone base {}/{} from template {} as {}", destPool.getSourceDir(),
1611+
cloneBaseImage, template.getName(), stagingImage);
1612+
rbdClient.create(stagingImage, template.getVirtualSize() + LUKS2_HEADER_RESERVE_BYTES, RBD_FEATURES, rbdOrder);
1613+
boolean renamed = false;
1614+
try {
1615+
new RbdEncryption().copyTemplateDense(destPool.getSourceHost(), destPool.getSourcePort(),
1616+
destPool.getAuthUserName(), destPool.getAuthSecret(), destPool.getSourceDir(),
1617+
template.getName(), stagingImage);
1618+
RbdImage stagedImage = rbdClient.open(stagingImage);
1619+
try {
1620+
stagedImage.snapCreate(rbdTemplateSnapName);
1621+
stagedImage.snapProtect(rbdTemplateSnapName);
1622+
} finally {
1623+
rbdClient.close(stagedImage);
1624+
}
1625+
try {
1626+
rbdClient.rename(stagingImage, cloneBaseImage);
1627+
renamed = true;
1628+
logger.info("Prepared dense LUKS clone base {}/{}", destPool.getSourceDir(), cloneBaseImage);
1629+
} catch (RbdException e) {
1630+
logger.info("Another host prepared LUKS clone base {}/{} first; discarding {}",
1631+
destPool.getSourceDir(), cloneBaseImage, stagingImage);
1632+
}
1633+
} finally {
1634+
if (!renamed) {
1635+
removeRbdImageQuietly(rbdClient, stagingImage);
1636+
}
1637+
}
1638+
if (!renamed && !isLuksCloneBaseReady(rbdClient, cloneBaseImage)) {
1639+
throw new RbdException("LUKS clone base " + destPool.getSourceDir() + "/" + cloneBaseImage
1640+
+ " could not be prepared and no other host has prepared it");
1641+
}
1642+
}
1643+
1644+
/** Remove a template's LUKS clone base image along with the template, if it has one. */
1645+
private void removeLuksCloneBaseIfPresent(Rbd rbdClient, String volumeName) {
1646+
final String cloneBaseImage = volumeName + LUKS_CLONE_BASE_SUFFIX;
1647+
RbdImage image;
1648+
try {
1649+
image = rbdClient.open(cloneBaseImage);
1650+
} catch (RbdException e) {
1651+
// everything that is not a template of an encrypted root has no base image
1652+
return;
1653+
}
1654+
try {
1655+
rbdClient.close(image);
1656+
} catch (RbdException ignored) {
1657+
// best-effort close of the probe handle
1658+
}
1659+
logger.info("Removing LUKS clone base image {} along with {}", cloneBaseImage, volumeName);
1660+
removeRbdImageQuietly(rbdClient, cloneBaseImage);
1661+
}
1662+
1663+
/** Remove an RBD image and its snapshots, logging instead of failing. */
1664+
private void removeRbdImageQuietly(Rbd rbdClient, String imageName) {
1665+
try {
1666+
RbdImage image = rbdClient.open(imageName);
1667+
try {
1668+
for (RbdSnapInfo snapshotInfo : image.snapList()) {
1669+
if (image.snapIsProtected(snapshotInfo.name)) {
1670+
image.snapUnprotect(snapshotInfo.name);
1671+
}
1672+
image.snapRemove(snapshotInfo.name);
1673+
}
1674+
} finally {
1675+
rbdClient.close(image);
1676+
}
1677+
rbdClient.remove(imageName);
1678+
logger.debug("Removed RBD image {}", imageName);
1679+
} catch (RbdException e) {
1680+
// e.g. a base image still has clones of live encrypted roots
1681+
logger.warn("Could not remove RBD image {}: {}", imageName, e.getMessage());
1682+
}
1683+
}
1684+
15921685
/**
15931686
* Option B (full-copy encrypted root), used when the template is not on the same RBD cluster (e.g. first
15941687
* use from secondary storage). Create an empty image, apply a LUKS2 header, then import the template

‎plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public class QemuImg {
6666
private String cloudQemuImgPath = "cloud-qemu-img";
6767
private long timeout;
6868
private boolean skipZero = false;
69+
private boolean writeZeroRanges = false;
6970
private boolean skipTargetVolumeCreation = false;
7071
private boolean noCache = false;
7172
private long version;
@@ -508,6 +509,11 @@ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, QemuI
508509
script.add("-n");
509510
}
510511

512+
if (writeZeroRanges) {
513+
script.add("-S");
514+
script.add("0");
515+
}
516+
511517
if (destImageOpts == null) {
512518
script.add("-O");
513519
script.add(destFile.getFormat().toString());
@@ -1011,6 +1017,14 @@ public void setSkipZero(boolean skipZero) {
10111017
this.skipZero = skipZero;
10121018
}
10131019

1020+
/**
1021+
* Make convert write the source's zero ranges to the destination ({@code -S 0}) instead of
1022+
* leaving them unallocated.
1023+
*/
1024+
public void setWriteZeroRanges(boolean writeZeroRanges) {
1025+
this.writeZeroRanges = writeZeroRanges;
1026+
}
1027+
10141028
public void setSkipTargetVolumeCreation(boolean skipTargetVolumeCreation) {
10151029
this.skipTargetVolumeCreation = skipTargetVolumeCreation;
10161030
}

‎plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/rbd/RbdEncryption.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,40 @@ public void importTemplate(String srcRbdPool, String srcRbdImage,
253253
/**
254254
* Seam for unit tests; {@link QemuImg} probes the qemu version through libvirt on construction.
255255
*/
256+
/**
257+
* Copy a plaintext RBD image onto an existing image in the same pool, writing out its zero ranges
258+
* too ({@code -S 0}), so the copy has no holes. Used for the base image encrypted roots are cloned
259+
* from: a hole in that parent reads as garbage through the clone's encryption once a guest write
260+
* copies up the object around it.
261+
*/
262+
public void copyTemplateDense(String monHost, int monPort, String authUser, String authSecret,
263+
String cephPool, String srcImage, String destImage) {
264+
Path conf = null;
265+
Path keyring = null;
266+
try {
267+
final FileAttribute<?> ownerOnly = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"));
268+
keyring = Files.createTempFile("cs-ceph-", ".keyring", ownerOnly);
269+
Files.writeString(keyring, "[client." + authUser + "]\n\tkey = " + authSecret + "\n");
270+
conf = Files.createTempFile("cs-ceph-", ".conf", ownerOnly);
271+
Files.writeString(conf, "[global]\nmon_host = " + monSpec(monHost, monPort) + "\nkeyring = " + keyring + "\n");
272+
273+
QemuImgFile srcQemuFile = new QemuImgFile(cephPool + "/" + srcImage, QemuImg.PhysicalDiskFormat.RAW);
274+
QemuImageOptions srcImageOpts = new QemuImageOptions(rbdImageOptions(cephPool, srcImage, conf.toString(), authUser));
275+
srcImageOpts.setImageOptsFlag(true);
276+
QemuImageOptions destImageOpts = new QemuImageOptions(rbdImageOptions(cephPool, destImage, conf.toString(), authUser));
277+
278+
QemuImg qemu = createQemuImg();
279+
qemu.setWriteZeroRanges(true);
280+
qemu.convertIntoExistingTarget(srcQemuFile, null, null, srcImageOpts, destImageOpts, false);
281+
logger.debug("Copied RBD image {}/{} into {} without holes", cephPool, srcImage, destImage);
282+
} catch (IOException | QemuImgException | LibvirtException ex) {
283+
throw new CloudRuntimeException(String.format("Failed to copy RBD image %s/%s into %s", cephPool, srcImage, destImage), ex);
284+
} finally {
285+
deleteQuietly(conf);
286+
deleteQuietly(keyring);
287+
}
288+
}
289+
256290
protected QemuImg createQemuImg() throws QemuImgException, LibvirtException {
257291
return new QemuImg(0);
258292
}

‎plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,41 @@ public void testCreateWithBackingFile() throws QemuImgException, LibvirtExceptio
323323
}
324324
}
325325

326+
@Test
327+
public void testConvertWriteZeroRangesAllocatesTheWholeDestination() throws QemuImgException, LibvirtException {
328+
// qemu-img skips the source's zero ranges by default, so the destination keeps its holes.
329+
// That is wrong for an image whose unallocated ranges are not read as zeros - an RBD image
330+
// with a librbd LUKS header, cloned, is the case setWriteZeroRanges exists for.
331+
String srcPath = "/tmp/" + UUID.randomUUID() + ".raw";
332+
String sparsePath = "/tmp/" + UUID.randomUUID() + ".raw";
333+
String densePath = "/tmp/" + UUID.randomUUID() + ".raw";
334+
long size = 16L * 1024 * 1024;
335+
336+
QemuImgFile srcFile = new QemuImgFile(srcPath, size, PhysicalDiskFormat.RAW);
337+
QemuImg qemu = new QemuImg(0);
338+
qemu.create(srcFile);
339+
// data in the first MiB only: the remaining 15 MiB of the source are zeros
340+
Script.runSimpleBashScript(String.format("dd if=/dev/urandom of=%s bs=1M count=1 conv=notrunc 2>/dev/null", srcPath));
341+
342+
qemu.convert(srcFile, new QemuImgFile(sparsePath, PhysicalDiskFormat.RAW));
343+
qemu.setWriteZeroRanges(true);
344+
qemu.convert(srcFile, new QemuImgFile(densePath, PhysicalDiskFormat.RAW));
345+
346+
long apparent = Long.parseLong(Script.runSimpleBashScript(String.format("stat -c %%s %s", densePath)));
347+
long sparseAllocated = Long.parseLong(Script.runSimpleBashScript(String.format("du --block-size=1 %s | cut -f1", sparsePath)));
348+
long denseAllocated = Long.parseLong(Script.runSimpleBashScript(String.format("du --block-size=1 %s | cut -f1", densePath)));
349+
350+
assertEquals(size, apparent);
351+
assertTrue("the default convert should have left the zero ranges unallocated, allocated " + sparseAllocated,
352+
sparseAllocated < size);
353+
assertTrue("-S 0 should have written every zero range, allocated " + denseAllocated,
354+
denseAllocated >= size);
355+
356+
assertTrue(new File(srcPath).delete());
357+
assertTrue(new File(sparsePath).delete());
358+
assertTrue(new File(densePath).delete());
359+
}
360+
326361
@Test
327362
public void testConvertBasic() throws QemuImgException, LibvirtException {
328363
long srcSize = 20480;

‎plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/rbd/RbdEncryptionTest.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,36 @@ public void importTemplateFromFileSourceForcesSourceFormat() throws Exception {
143143
Assert.assertTrue(src, src.contains("file.filename=/tmp/tmpl.qcow2"));
144144
}
145145

146+
@Test
147+
public void copyTemplateDenseWritesZeroRangesAndLeavesTheCopyPlaintext() throws Exception {
148+
RbdEncryption spy = Mockito.spy(new RbdEncryption());
149+
QemuImg qemuImg = Mockito.mock(QemuImg.class);
150+
Mockito.doReturn(qemuImg).when(spy).createQemuImg();
151+
152+
spy.copyTemplateDense("1.2.3.4", 6789, "cloudstack", "secret", "cloudstack", "tmpl", "tmpl-luks");
153+
154+
// The whole point of this copy: without -S 0 it inherits the template's holes, and a hole in
155+
// the parent of an encrypted clone is read through the crypto layer - as garbage - as soon as
156+
// anything writes into the same 4 MiB object.
157+
Mockito.verify(qemuImg).setWriteZeroRanges(true);
158+
159+
ArgumentCaptor<QemuImageOptions> srcOpts = ArgumentCaptor.forClass(QemuImageOptions.class);
160+
ArgumentCaptor<QemuImageOptions> destOpts = ArgumentCaptor.forClass(QemuImageOptions.class);
161+
Mockito.verify(qemuImg).convertIntoExistingTarget(Mockito.any(QemuImgFile.class), Mockito.isNull(),
162+
Mockito.isNull(), srcOpts.capture(), destOpts.capture(), Mockito.eq(false));
163+
164+
String src = String.join(" ", srcOpts.getValue().toCommandFlag());
165+
Assert.assertTrue(src, src.startsWith("--image-opts "));
166+
Assert.assertTrue(src, src.contains("pool=cloudstack"));
167+
Assert.assertTrue(src, src.contains("image=tmpl"));
168+
169+
String dest = String.join(" ", destOpts.getValue().toCommandFlag(QemuImg.TARGET_IMAGE_OPTS_FLAG));
170+
Assert.assertTrue(dest, dest.startsWith(QemuImg.TARGET_IMAGE_OPTS_FLAG + " "));
171+
Assert.assertTrue(dest, dest.contains("image=tmpl-luks"));
172+
// the base image is a plaintext copy; the LUKS header belongs to the clone, not to the base
173+
Assert.assertFalse(dest, dest.contains("encrypt."));
174+
}
175+
146176
@Test
147177
public void formatRejectsEmptyPassphrase() {
148178
Assert.assertThrows(CloudRuntimeException.class, () -> rbdEncryption.format(

0 commit comments

Comments
 (0)