curtin-dev team mailing list archive

[Dan Bungert <mp+486154@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:lp-2107381-ctd into curtin:master.

Commit message:
zfs: choose our luks header size
    
Choosing a luks header size allows us to protect against surprises later
if the detected luks header size changes.
    
LP: #2107381


Requested reviews:
  curtin developers (curtin-dev)
Related bugs:
  Bug #2107381 in curtin: "zfs + encryption fails with plucky iso dated 20250415  - Requested offset is beyond real size of device /dev/zvol/rpool/keystore"
  https://bugs.launchpad.net/curtin/+bug/2107381

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/486154

DO NOT SQUASH

unpatched curtin using a 20MiB keystore with no offset value on plucky+ fails the test_zfs_luks_keystore integration test, so we have test coverage here in that sense.
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:lp-2107381-ctd into curtin:master.

diff --git a/curtin/block/zfs.py b/curtin/block/zfs.py
index 29ace32..bb78138 100644
--- a/curtin/block/zfs.py
+++ b/curtin/block/zfs.py
@@ -31,6 +31,21 @@ ZFS_DEFAULT_PROPERTIES = {
 ZFS_UNSUPPORTED_ARCHES = ['i386']
 ZFS_UNSUPPORTED_RELEASES = ['precise', 'trusty']
 
+# The keystore consists of the LUKS header, which is a size we can configure,
+# and the usable volume size of the keystore.  While the file we store here is
+# rather small we leave a little room. In LP: #2107381 we learned that the
+# cryptsetup detected offset can vary, so choosing a LUKS header size avoids
+# surprises later where luksFormat fails due to insufficient volume size.
+# The mechanism behind that: cryptsetup LUKS2_hdr_get_storage_params() decides
+# on several values, including offset to the actual usable device space.
+# offset may be supplied with the cryptset --offset argument, or it will be
+# chosen in a way based on the BLKIOOPT / BLKALIGNOFF ioctls in cryptsetup
+# device_topology_alignment(), which is a bit overkill for the keystore, so
+# just choose a size and keep it small.
+LUKS_HEADER_SIZE = 16 << 20
+USABLE_VOLUME_SIZE = 4 << 20
+KEYSTORE_VOLUME_SIZE = LUKS_HEADER_SIZE + USABLE_VOLUME_SIZE
+
 
 class ZPoolEncryption:
     def __init__(self, vdevs, poolname, style, keyfile):
@@ -86,11 +101,9 @@ class ZPoolEncryption:
 
         # Create the dataset for the keystore.  This is a bit special as it
         # won't be ZFS despite being on the zpool.
-        # We previously hardcoded the size to 20M but raised it to 36M for
-        # plucky, see LP: #2107381.
-        keystore_size = util.human2bytes("36M")
         zfs_create(
-            self.poolname, "keystore", {"encryption": "off"}, keystore_size,
+            self.poolname, "keystore", {"encryption": "off"},
+            str(KEYSTORE_VOLUME_SIZE),
         )
         keystore_volume = f"/dev/zvol/{self.poolname}/keystore"
         udevadm_settle(exists=keystore_volume)
@@ -99,8 +112,16 @@ class ZPoolEncryption:
             for vdev in self.vdevs:
                 es.enter_context(util.FlockEx(vdev))
 
-            # cryptsetup format and open this keystore
-            cmd = ["cryptsetup", "luksFormat", keystore_volume, self.keyfile]
+            # cryptsetup format and open this keystore. pick a fixed offset
+            # size, in sectors, to work with the fixed volume size.
+            cmd = [
+                "cryptsetup",
+                "luksFormat",
+                "--offset",
+                str(LUKS_HEADER_SIZE // 512),
+                keystore_volume,
+                self.keyfile
+            ]
 
             # strace has shown that udevd does indeed probe this keystore
             with util.FlockEx(keystore_volume):
diff --git a/tools/vmtest-system-setup b/tools/vmtest-system-setup
index 7d416b6..b0aab8a 100755
--- a/tools/vmtest-system-setup
+++ b/tools/vmtest-system-setup
@@ -46,6 +46,7 @@ DEPS=(
   tgt
   tox
   wget
+  zfsutils-linux
 )
 
 apt_get() {