[PATCH 0/4] LUKS1/2 testing in fs-tester and LUKS2 support in grub-probe

[PATCH 0/4] LUKS1/2 testing in fs-tester and LUKS2 support in grub-probe

[Glenn Washburn]

This series adds the testing of LUKS 1 and 2 encrypted container to the
user-space fs tester. While this does replicate some of the testing done
by grub_cmd_cryptomount, and is less flexible, it is run natively (that is
without need for virtualization). It also tests the added LUKS2 support in
grub-probe, unlike grub_cmd_cryptomount which does not test any user-space
grub components.

All of these patches have previously been sent to the list. Only patch 4 has
been slightly modified to fix some issues. Since utility of any one is greatly
enhanced, and in the case of patch 4 required, by the others, I think it makes
sense to evaluate them together.

Fabian has noted a memory leak with error handling when using LUKS2 with a
null_cipher[1]. Since this isn't something that I expect to happen in the wild
(why would you use an unencrypted LUKS container?) and even if it does occur
grub-probe is exiting, so the leak would be very short lived, I don't think
this is something that we need to worry about. It wouldn't be that hard to fix
though, just annoying and add a little more complexity to the source.

The corresponding patches can be found in the order of this series here:
 1: [PATCH v3] disk/cryptodisk: When cheatmounting, use the sector info of the cheat device
    https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00106.html
 2: [PATCH v6 1/2] devmapper/getroot: Have devmapper recognize LUKS2
    https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00036.html
 3: [PATCH v6 2/2] devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM parameters
    https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00038.html
 4: [PATCH v4] grub-fs-tester: Add luks1 and luks2 support
    https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00127.html

Glenn

[1] https://lists.gnu.org/archive/html/grub-devel/2022-11/msg00032.html

Fabian Vogt (1):
  disk/cryptodisk: When cheatmounting, use the sector info of the cheat
    device

Josselin Poiret (2):
  devmapper/getroot: Have devmapper recognize LUKS2
  devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM
    parameters

Pierre-Louis Bonicoli (1):
  grub-fs-tester: Add luks1 and luks2 support

 .gitignore                          |   2 +
 Makefile.util.def                   |  12 +++
 grub-core/disk/cryptodisk.c         |  20 ++++-
 grub-core/osdep/devmapper/getroot.c | 118 ++++++++++++++++++++++++++--
 tests/luks1_test.in                 |  23 ++++++
 tests/luks2_test.in                 |  23 ++++++
 tests/util/grub-fs-tester.in        |  57 +++++++++++++-
 7 files changed, 246 insertions(+), 9 deletions(-)
 create mode 100644 tests/luks1_test.in
 create mode 100644 tests/luks2_test.in

-- 
2.34.1

[PATCH 1/4] disk/cryptodisk: When cheatmounting, use the sector info of the cheat device

[Glenn Washburn]

From: Fabian Vogt <fvogt@suse.de>

When using grub-probe with cryptodisk, the mapped block device from the host
is used directly instead of decrypting the source device in GRUB code.
In that case, the sector size and count of the host device needs to be used.
This is especially important when using luks2, which does not assign
total_sectors and log_sector_size when scanning, but only later when the
segments in the JSON area are evaluated. With an unset log_sector_size,
grub_open_device complains.

This fixes grub-probe failing with
"error: sector sizes of 1 bytes aren't supported yet."

Signed-off-by: Fabian Vogt <fvogt@suse.de>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Tested-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/cryptodisk.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 14308822aa..ed1769eabb 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -718,16 +718,31 @@ grub_cryptodisk_open (const char *name, grub_disk_t disk)
   if (!dev)
     return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "No such device");
 
-  disk->log_sector_size = dev->log_sector_size;
-
 #ifdef GRUB_UTIL
   if (dev->cheat)
     {
+      grub_uint64_t cheat_dev_size;
+      unsigned int cheat_log_sector_size;
+
       if (!GRUB_UTIL_FD_IS_VALID (dev->cheat_fd))
 	dev->cheat_fd = grub_util_fd_open (dev->cheat, GRUB_UTIL_FD_O_RDONLY);
       if (!GRUB_UTIL_FD_IS_VALID (dev->cheat_fd))
 	return grub_error (GRUB_ERR_IO, N_("cannot open `%s': %s"),
 			   dev->cheat, grub_util_fd_strerror ());
+
+      /* Use the sector size and count of the cheat device */
+      cheat_dev_size = grub_util_get_fd_size (dev->cheat_fd, dev->cheat, &cheat_log_sector_size);
+      if (cheat_dev_size == -1)
+        {
+          const char *errmsg = grub_util_fd_strerror ();
+          grub_util_fd_close (dev->cheat_fd);
+          dev->cheat_fd = GRUB_UTIL_FD_INVALID;
+          return grub_error (GRUB_ERR_IO, N_("failed to query size of device `%s': %s"),
+                             dev->cheat, errmsg);
+        }
+
+      dev->log_sector_size = cheat_log_sector_size;
+      dev->total_sectors = cheat_dev_size >> cheat_log_sector_size;
     }
 #endif
 
@@ -741,6 +756,7 @@ grub_cryptodisk_open (const char *name, grub_disk_t disk)
     }
 
   disk->data = dev;
+  disk->log_sector_size = dev->log_sector_size;
   disk->total_sectors = dev->total_sectors;
   disk->max_agglomerate = GRUB_DISK_MAX_MAX_AGGLOMERATE;
   disk->id = dev->id;
-- 
2.34.1

[PATCH 2/4] devmapper/getroot: Have devmapper recognize LUKS2

[Glenn Washburn]

From: Josselin Poiret <dev@jpoiret.xyz>

Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.

Signed-off-by: Josselin Poiret <dev@jpoiret.xyz>
Tested-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/osdep/devmapper/getroot.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/grub-core/osdep/devmapper/getroot.c b/grub-core/osdep/devmapper/getroot.c
index 9ba5c98655..2bf4264cf0 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -138,7 +138,8 @@ grub_util_get_dm_abstraction (const char *os_dev)
       grub_free (uuid);
       return GRUB_DEV_ABSTRACTION_LVM;
     }
-  if (strncmp (uuid, "CRYPT-LUKS1-", 12) == 0)
+  if (strncmp (uuid, "CRYPT-LUKS1-", sizeof ("CRYPT-LUKS1-") - 1) == 0
+      || strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) == 0)
     {
       grub_free (uuid);
       return GRUB_DEV_ABSTRACTION_LUKS;
@@ -179,7 +180,9 @@ grub_util_pull_devmapper (const char *os_dev)
 	  grub_util_pull_device (subdev);
 	}
     }
-  if (uuid && strncmp (uuid, "CRYPT-LUKS1-", sizeof ("CRYPT-LUKS1-") - 1) == 0
+  if (uuid
+      && (strncmp (uuid, "CRYPT-LUKS1-", sizeof ("CRYPT-LUKS1-") - 1) == 0
+          || strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) == 0)
       && lastsubdev)
     {
       char *grdev = grub_util_get_grub_dev (lastsubdev);
@@ -253,11 +256,11 @@ grub_util_get_devmapper_grub_dev (const char *os_dev)
       {
 	char *dash;
 
-	dash = grub_strchr (uuid + sizeof ("CRYPT-LUKS1-") - 1, '-');
+	dash = grub_strchr (uuid + sizeof ("CRYPT-LUKS*-") - 1, '-');
 	if (dash)
 	  *dash = 0;
 	grub_dev = grub_xasprintf ("cryptouuid/%s",
-				   uuid + sizeof ("CRYPT-LUKS1-") - 1);
+				   uuid + sizeof ("CRYPT-LUKS*-") - 1);
 	grub_free (uuid);
 	return grub_dev;
       }
-- 
2.34.1

[PATCH 3/4] devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM parameters

[Glenn Washburn]

From: Josselin Poiret <dev@jpoiret.xyz>

This lets a LUKS2 cryptodisk have its cipher and hash filled out,
otherwise they wouldn't be initialized if cheat mounted.

Signed-off-by: Josselin Poiret <dev@jpoiret.xyz>
Tested-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/osdep/devmapper/getroot.c | 107 +++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/devmapper/getroot.c b/grub-core/osdep/devmapper/getroot.c
index 2bf4264cf0..090ac71670 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -51,6 +51,8 @@
 #include <grub/emu/misc.h>
 #include <grub/emu/hostdisk.h>
 
+#include <grub/cryptodisk.h>
+
 static int
 grub_util_open_dm (const char *os_dev, struct dm_tree **tree,
 		   struct dm_tree_node **node)
@@ -186,7 +188,6 @@ grub_util_pull_devmapper (const char *os_dev)
       && lastsubdev)
     {
       char *grdev = grub_util_get_grub_dev (lastsubdev);
-      dm_tree_free (tree);
       if (grdev)
 	{
 	  grub_err_t err;
@@ -194,7 +195,111 @@ grub_util_pull_devmapper (const char *os_dev)
 	  if (err)
 	    grub_util_error (_("can't mount encrypted volume `%s': %s"),
 			     lastsubdev, grub_errmsg);
+          if (strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) == 0)
+            {
+              /*
+               * set LUKS2 cipher from dm parameters, since it is not
+               * possible to determine the correct one without
+               * unlocking, as there might be multiple segments.
+               */
+              grub_disk_t source;
+              grub_cryptodisk_t cryptodisk;
+              grub_uint64_t start, length;
+              char *target_type;
+              char *params;
+              const char *name;
+              char *cipher, *cipher_mode;
+              struct dm_task *dmt;
+              char *seek_head, *c;
+              unsigned int remaining;
+
+              source = grub_disk_open (grdev);
+              if (! source)
+                grub_util_error (_("cannot open grub disk `%s'"), grdev);
+              cryptodisk = grub_cryptodisk_get_by_source_disk (source);
+              if (! cryptodisk)
+                grub_util_error (_("cannot get cryptodisk from source disk `%s'"), grdev);
+              grub_disk_close (source);
+
+              /*
+               * the following function always returns a non-NULL pointer,
+               * but the string may be empty if the relevant info is not present
+               */
+              name = dm_tree_node_get_name (node);
+              if (grub_strlen (name) == 0)
+                grub_util_error (_("cannot get dm node name for grub dev `%s'"), grdev);
+
+              grub_util_info ("populating parameters of cryptomount `%s' from DM device `%s'",
+                              uuid, name);
+
+              dmt = dm_task_create (DM_DEVICE_TABLE);
+              if (dmt == NULL)
+                grub_util_error (_("can't create dm task DM_DEVICE_TABLE"));
+              if (dm_task_set_name (dmt, name) == 0)
+                grub_util_error (_("can't set dm task name to `%s'"), name);
+              if (dm_task_run (dmt) == 0)
+                grub_util_error (_("can't run dm task for `%s'"), name);
+              /*
+               * dm_get_next_target doesn't have any error modes, everything has
+               * been handled by dm_task_run.
+               */
+              dm_get_next_target (dmt, NULL, &start, &length,
+                                  &target_type, &params);
+              if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
+                grub_util_error (_("dm target of type `%s' is not `crypt'"), target_type);
+
+              /*
+               * dm target parameters for dm-crypt is
+               * <cipher> <key> <iv_offset> <device path> <offset> [<#opt_params> <opt_param1> ...]
+               */
+              c = params;
+              remaining = grub_strlen (c);
+
+              /* first, get the cipher name from the cipher */
+              seek_head = grub_memchr (c, '-', remaining);
+              if (seek_head == NULL)
+                grub_util_error (_("can't get cipher from dm-crypt parameters `%s'"),
+                                 params);
+              cipher = grub_strndup (c, seek_head - c);
+              if (cipher == NULL)
+                grub_util_error (_("could not strndup cipher of length `%lu'"), seek_head - c);
+              remaining -= seek_head - c + 1;
+              c = seek_head + 1;
+
+              /* now, the cipher mode */
+              seek_head = grub_memchr (c, ' ', remaining);
+              if (seek_head == NULL)
+                grub_util_error (_("can't get cipher mode from dm-crypt parameters `%s'"),
+                                 params);
+              cipher_mode = grub_strndup (c, seek_head - c);
+              if (cipher_mode == NULL)
+                grub_util_error (_("could not strndup cipher_mode of length `%lu'"), seek_head - c);
+
+              remaining -= seek_head - c + 1;
+              c = seek_head + 1;
+
+              err = grub_cryptodisk_setcipher (cryptodisk, cipher, cipher_mode);
+              if (err)
+                  grub_util_error (_("can't set cipher of cryptodisk `%s' to `%s' with mode `%s'"),
+                                   uuid, cipher, cipher_mode);
+
+              grub_free (cipher);
+              grub_free (cipher_mode);
+
+              /*
+               * This is the only hash usable by PBKDF2, and we don't
+               * have Argon2 support yet, so set it by default,
+               * otherwise grub-probe would miss the required
+               * abstraction
+               */
+              cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256");
+              if (cryptodisk->hash == NULL)
+                  grub_util_error (_("can't lookup hash sha256 by name"));
+
+              dm_task_destroy (dmt);
+            }
 	}
+      dm_tree_free (tree);
       grub_free (grdev);
     }
   else
-- 
2.34.1

[PATCH 4/4] grub-fs-tester: Add luks1 and luks2 support

[Glenn Washburn]

From: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr>

The logical sector size used by LUKS1 is 512 bytes and LUKS2 uses 512 to
4069 bytes. The deafualt password used is "pass", but can be overridden
by setting the PASS environment variable. The device mapper name is set
to the name of the temp directory so that its easy to corrolate device
mapper name with a particular test run. Also since this name is unique
per test run, multiple simultaneous test runs are allowed.

Note that cryptsetup is passing the --disable-locks parameter to allow
cryptsetup run successfully when /run/lock/cryptsetup is not accessible.
Since the device mapper name is unique per test run, there is no need to
worry about locking the device to serialize access.

Signed-off-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr>
Tested-by: Glenn Washburn <development@efficientek.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 .gitignore                   |  2 ++
 Makefile.util.def            | 12 ++++++++
 tests/luks1_test.in          | 23 +++++++++++++++
 tests/luks2_test.in          | 23 +++++++++++++++
 tests/util/grub-fs-tester.in | 57 ++++++++++++++++++++++++++++++++++--
 5 files changed, 115 insertions(+), 2 deletions(-)
 create mode 100644 tests/luks1_test.in
 create mode 100644 tests/luks2_test.in

diff --git a/.gitignore b/.gitignore
index f6a1bd0517..4064d3d1ec 100644
--- a/.gitignore
+++ b/.gitignore
@@ -230,6 +230,8 @@ widthspec.bin
 /lib/libgcrypt-grub
 /libgrub_a_init.c
 /lzocompress_test
+/luks1_test
+/luks2_test
 /m4/
 /minixfs_test
 /missing
diff --git a/Makefile.util.def b/Makefile.util.def
index d919c562c4..3f1162b765 100644
--- a/Makefile.util.def
+++ b/Makefile.util.def
@@ -1213,6 +1213,18 @@ script = {
   common = tests/syslinux_test.in;
 };
 
+script = {
+  testcase = native;
+  name = luks1_test;
+  common = tests/luks1_test.in;
+};
+
+script = {
+  testcase = native;
+  name = luks2_test;
+  common = tests/luks2_test.in;
+};
+
 program = {
   testcase = native;
   name = example_unit_test;
diff --git a/tests/luks1_test.in b/tests/luks1_test.in
new file mode 100644
index 0000000000..cd28fd7149
--- /dev/null
+++ b/tests/luks1_test.in
@@ -0,0 +1,23 @@
+#!@BUILD_SHEBANG@
+
+set -e
+
+if [ "x$EUID" = "x" ] ; then
+  EUID=`id -u`
+fi
+
+if [ "$EUID" != 0 ] ; then
+   exit 99
+fi
+
+if ! which mkfs.ext2 >/dev/null 2>&1; then
+   echo "mkfs.ext2 not installed; cannot test luks."
+   exit 99
+fi
+
+if ! which cryptsetup >/dev/null 2>&1; then
+   echo "cryptsetup not installed; cannot test luks."
+   exit 99
+fi
+
+"@builddir@/grub-fs-tester" luks1
diff --git a/tests/luks2_test.in b/tests/luks2_test.in
new file mode 100644
index 0000000000..6a26ba6266
--- /dev/null
+++ b/tests/luks2_test.in
@@ -0,0 +1,23 @@
+#!@BUILD_SHEBANG@
+
+set -e
+
+if [ "x$EUID" = "x" ] ; then
+  EUID=`id -u`
+fi
+
+if [ "$EUID" != 0 ] ; then
+   exit 99
+fi
+
+if ! which mkfs.ext2 >/dev/null 2>&1; then
+   echo "mkfs.ext2 not installed; cannot test luks2."
+   exit 99
+fi
+
+if ! which cryptsetup >/dev/null 2>&1; then
+   echo "cryptsetup not installed; cannot test luks2."
+   exit 99
+fi
+
+"@builddir@/grub-fs-tester" luks2
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 7a48c4399c..3fbec2c255 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -9,6 +9,7 @@ unset SOURCE_DATE_EPOCH
 fs="$1"
 
 GRUBFSTEST="@builddir@/grub-fstest"
+GRUBPROBE="@builddir@/grub-probe"
 
 tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date '+%Y%m%d%H%M%S%N').${fs}.XXX"` ||
 { echo "Failed to make temporary directory"; exit 99; }
@@ -16,6 +17,8 @@ tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date '+%Y%m%d%H%M%S%N').${fs}.XXX
 # xorriso -as mkisofs options to ignore locale when processing file names and
 # FSLABEL. This is especially needed for the conversion to Joliet UCS-2.
 XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
+DMNAME="${tempdir##*/}"
+PASS="${PASS:-pass}"
 
 MOUNTS=
 LODEVICES=
@@ -31,6 +34,10 @@ cleanup() {
 	umount "$i" || :
     done
 
+    if [ -e /dev/mapper/"$DMNAME" ]; then
+	cryptsetup close --disable-locks "$DMNAME"
+    fi
+
     for lodev in $LODEVICES; do
 	local i=600
 	while losetup -l -O NAME | grep -q "^$lodev\$"; do
@@ -71,7 +78,12 @@ run_grubfstest () {
 	need_images="$need_images $FSIMAGEP${i}.img";
     done
 
-    run_it -c $NEED_IMAGES_N $need_images  "$@"
+    case x"$fs" in
+	xluks*)
+	    echo -n "$PASS" | run_it -C -c $NEED_IMAGES_N $need_images  "$@";;
+	*)
+	    run_it -c $NEED_IMAGES_N $need_images  "$@";;
+    esac
 }
 
 # OS LIMITATION: GNU/Linux has no AFS support, so we use a premade image and a reference tar file. I.a. no multiblocksize test
@@ -79,6 +91,8 @@ run_grubfstest () {
 MINLOGSECSIZE=9
 MAXLOGSECSIZE=9
 case x"$fs" in
+    xluks2)
+	MAXLOGSECSIZE=12;;
     xntfs*)
 	MINLOGSECSIZE=8
 	MAXLOGSECSIZE=12;;
@@ -366,7 +380,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    #FSLABEL="g;/_é莭莽😁кит u"
 		    ;;
 		# FS LIMITATION: reiserfs, extN and jfs label is at most 16 UTF-8 characters
-		x"reiserfs_old" | x"reiserfs" | x"ext"* | x"lvm"* | x"mdraid"* | x"jfs" | x"jfs_caseins")
+		x"reiserfs_old" | x"reiserfs" | x"ext"* | x"lvm"* | x"luks"* | x"mdraid"* | x"jfs" | x"jfs_caseins")
 		    FSLABEL="g;/éт 莭😁";;
 		# FS LIMITATION: No underscore, space, semicolon, slash or international characters in UFS* in label. Limited to 32 UTF-8 characters
 		x"ufs1" | x"ufs1_sun" | x"ufs2")
@@ -835,6 +849,12 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    MOUNTDEVICE="/dev/mapper/grub_test-testvol"
 		    MOUNTFS=ext2
 		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
+		x"luks"*)
+		    echo -n "$PASS" | cryptsetup luksFormat --type "$fs" --sector-size $SECSIZE --pbkdf pbkdf2 --force-password --disable-locks $LODEVICE
+		    echo -n "$PASS" | cryptsetup open --disable-locks $LODEVICE "$DMNAME"
+		    MOUNTDEVICE="/dev/mapper/${DMNAME}"
+		    MOUNTFS=ext2
+		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
 		xf2fs)
 		    "mkfs.f2fs" -l "$FSLABEL" -q "${MOUNTDEVICE}" ;;
 		xnilfs2)
@@ -947,6 +967,22 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    GRUBDEVICE="mduuid/`mdadm --detail --export $MOUNTDEVICE | grep MD_UUID=|sed 's,MD_UUID=,,g;s,:,,g'`";;
 		xlvm*)
 		    GRUBDEVICE="lvm/grub_test-testvol";;
+		xluks*)
+		    if test x"$fs" = xluks2 && ! (cryptsetup luksDump --debug-json --disable-locks $LODEVICE | grep -q "\"sector_size\":$SECSIZE"); then
+			    echo "Unexpected sector size for $LODEVICE (expected: $SECSIZE)"
+			    exit 1
+		    fi
+
+		    UUID=$(cryptsetup luksUUID --disable-locks $LODEVICE | tr -d '-')
+		    PROBE_UUID=$("$GRUBPROBE" --device $MOUNTDEVICE --target=cryptodisk_uuid | tr -d '-')
+		    if [ x"$UUID" != x"$PROBE_UUID" ]; then
+			echo "UUID FAIL"
+			echo "$UUID"
+			echo "$PROBE_UUID"
+			exit 1
+		    fi
+		    GRUBDEVICE="cryptouuid/${UUID}"
+		    ;;
 	    esac
 	    GRUBDIR="($GRUBDEVICE)"
 	    case x"$fs" in
@@ -1105,6 +1141,15 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    sleep 1
 		    vgchange -a n grub_test
 		    ;;
+		xluks*)
+		    for try in $(range 0 20 1); do
+			if umount "$MNTPOINTRW" ; then
+			    break;
+			fi
+		    done
+		    UMOUNT_TIME=$(date -u "+%Y-%m-%d %H:%M:%S")
+		    cryptsetup close --disable-locks "$DMNAME"
+		    ;;
 		xmdraid*)
 		    sleep 1
 		    for try in $(range 0 20 1); do
@@ -1155,6 +1200,11 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
 		    MOUNTS="$MOUNTS $MNTPOINTRO"
 		    ;;
+		xluks*)
+		    echo -n "$PASS" | cryptsetup open --disable-locks $LODEVICE "$DMNAME"
+		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
+		    MOUNTS="$MOUNTS $MNTPOINTRO"
+		    ;;
 		xmdraid*)
 		    mdadm --assemble /dev/md/"${fs}_$NDEVICES" $LODEVICES
 		    sleep 1
@@ -1603,6 +1653,9 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
 		    vgchange -a n grub_test
 		    sleep 1
 		    ;;
+		xluks*)
+		    cryptsetup close --disable-locks "$DMNAME"
+		    ;;
 	    esac
 	    case x"$fs" in
 		x"tarfs" | x"cpio_"* | x"iso9660" | xrockridge | xjoliet | xrockridge_joliet | x"ziso9660" | x"romfs" | x"squash4_"* | x"iso9660_1999" | xrockridge_1999 | xjoliet_1999 | xrockridge_joliet_1999) ;;
-- 
2.34.1

Re: [PATCH 3/4] devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM parameters

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 6858 bytes --]

On Thu, Jan 12, 2023 at 05:05:09PM -0600, Glenn Washburn wrote:
> From: Josselin Poiret <dev@jpoiret.xyz>
> 
> This lets a LUKS2 cryptodisk have its cipher and hash filled out,
> otherwise they wouldn't be initialized if cheat mounted.
> 
> Signed-off-by: Josselin Poiret <dev@jpoiret.xyz>
> Tested-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/osdep/devmapper/getroot.c | 107 +++++++++++++++++++++++++++-
>  1 file changed, 106 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/osdep/devmapper/getroot.c b/grub-core/osdep/devmapper/getroot.c
> index 2bf4264cf0..090ac71670 100644
> --- a/grub-core/osdep/devmapper/getroot.c
> +++ b/grub-core/osdep/devmapper/getroot.c
> @@ -51,6 +51,8 @@
>  #include <grub/emu/misc.h>
>  #include <grub/emu/hostdisk.h>
>  
> +#include <grub/cryptodisk.h>
> +
>  static int
>  grub_util_open_dm (const char *os_dev, struct dm_tree **tree,
>  		   struct dm_tree_node **node)
> @@ -186,7 +188,6 @@ grub_util_pull_devmapper (const char *os_dev)
>        && lastsubdev)
>      {
>        char *grdev = grub_util_get_grub_dev (lastsubdev);
> -      dm_tree_free (tree);
>        if (grdev)
>  	{
>  	  grub_err_t err;
> @@ -194,7 +195,111 @@ grub_util_pull_devmapper (const char *os_dev)
>  	  if (err)
>  	    grub_util_error (_("can't mount encrypted volume `%s': %s"),
>  			     lastsubdev, grub_errmsg);
> +          if (strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) == 0)
> +            {
> +              /*
> +               * set LUKS2 cipher from dm parameters, since it is not
> +               * possible to determine the correct one without
> +               * unlocking, as there might be multiple segments.
> +               */
> +              grub_disk_t source;
> +              grub_cryptodisk_t cryptodisk;
> +              grub_uint64_t start, length;
> +              char *target_type;
> +              char *params;
> +              const char *name;
> +              char *cipher, *cipher_mode;
> +              struct dm_task *dmt;
> +              char *seek_head, *c;
> +              unsigned int remaining;
> +
> +              source = grub_disk_open (grdev);
> +              if (! source)
> +                grub_util_error (_("cannot open grub disk `%s'"), grdev);
> +              cryptodisk = grub_cryptodisk_get_by_source_disk (source);
> +              if (! cryptodisk)
> +                grub_util_error (_("cannot get cryptodisk from source disk `%s'"), grdev);
> +              grub_disk_close (source);
> +
> +              /*
> +               * the following function always returns a non-NULL pointer,
> +               * but the string may be empty if the relevant info is not present
> +               */
> +              name = dm_tree_node_get_name (node);
> +              if (grub_strlen (name) == 0)
> +                grub_util_error (_("cannot get dm node name for grub dev `%s'"), grdev);
> +
> +              grub_util_info ("populating parameters of cryptomount `%s' from DM device `%s'",
> +                              uuid, name);
> +
> +              dmt = dm_task_create (DM_DEVICE_TABLE);
> +              if (dmt == NULL)
> +                grub_util_error (_("can't create dm task DM_DEVICE_TABLE"));
> +              if (dm_task_set_name (dmt, name) == 0)
> +                grub_util_error (_("can't set dm task name to `%s'"), name);
> +              if (dm_task_run (dmt) == 0)
> +                grub_util_error (_("can't run dm task for `%s'"), name);
> +              /*
> +               * dm_get_next_target doesn't have any error modes, everything has
> +               * been handled by dm_task_run.
> +               */
> +              dm_get_next_target (dmt, NULL, &start, &length,
> +                                  &target_type, &params);
> +              if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
> +                grub_util_error (_("dm target of type `%s' is not `crypt'"), target_type);
> +
> +              /*
> +               * dm target parameters for dm-crypt is
> +               * <cipher> <key> <iv_offset> <device path> <offset> [<#opt_params> <opt_param1> ...]
> +               */
> +              c = params;
> +              remaining = grub_strlen (c);
> +
> +              /* first, get the cipher name from the cipher */
> +              seek_head = grub_memchr (c, '-', remaining);
> +              if (seek_head == NULL)
> +                grub_util_error (_("can't get cipher from dm-crypt parameters `%s'"),
> +                                 params);
> +              cipher = grub_strndup (c, seek_head - c);
> +              if (cipher == NULL)
> +                grub_util_error (_("could not strndup cipher of length `%lu'"), seek_head - c);
> +              remaining -= seek_head - c + 1;
> +              c = seek_head + 1;
> +
> +              /* now, the cipher mode */
> +              seek_head = grub_memchr (c, ' ', remaining);
> +              if (seek_head == NULL)
> +                grub_util_error (_("can't get cipher mode from dm-crypt parameters `%s'"),
> +                                 params);
> +              cipher_mode = grub_strndup (c, seek_head - c);
> +              if (cipher_mode == NULL)
> +                grub_util_error (_("could not strndup cipher_mode of length `%lu'"), seek_head - c);
> +
> +              remaining -= seek_head - c + 1;
> +              c = seek_head + 1;
> +
> +              err = grub_cryptodisk_setcipher (cryptodisk, cipher, cipher_mode);
> +              if (err)
> +                  grub_util_error (_("can't set cipher of cryptodisk `%s' to `%s' with mode `%s'"),
> +                                   uuid, cipher, cipher_mode);
> +
> +              grub_free (cipher);
> +              grub_free (cipher_mode);
> +
> +              /*
> +               * This is the only hash usable by PBKDF2, and we don't
> +               * have Argon2 support yet, so set it by default,
> +               * otherwise grub-probe would miss the required
> +               * abstraction
> +               */
> +              cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256");
> +              if (cryptodisk->hash == NULL)
> +                  grub_util_error (_("can't lookup hash sha256 by name"));

I had to re-check whether this is actually true, but at least the LUKS2
spec says it's always SHA256. The format allows for a different hashing
function, but that does not yet seem to be used according to the spec.

We can iterate at a later point as required.

Patrick

> +              dm_task_destroy (dmt);
> +            }
>  	}
> +      dm_tree_free (tree);
>        grub_free (grdev);
>      }
>    else
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] LUKS1/2 testing in fs-tester and LUKS2 support in grub-probe

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

On Thu, Jan 12, 2023 at 05:05:06PM -0600, Glenn Washburn wrote:
> This series adds the testing of LUKS 1 and 2 encrypted container to the
> user-space fs tester. While this does replicate some of the testing done
> by grub_cmd_cryptomount, and is less flexible, it is run natively (that is
> without need for virtualization). It also tests the added LUKS2 support in
> grub-probe, unlike grub_cmd_cryptomount which does not test any user-space
> grub components.
> 
> All of these patches have previously been sent to the list. Only patch 4 has
> been slightly modified to fix some issues. Since utility of any one is greatly
> enhanced, and in the case of patch 4 required, by the others, I think it makes
> sense to evaluate them together.

Yeah, makes sense to have them all in one place.

> Fabian has noted a memory leak with error handling when using LUKS2 with a
> null_cipher[1]. Since this isn't something that I expect to happen in the wild
> (why would you use an unencrypted LUKS container?) and even if it does occur
> grub-probe is exiting, so the leak would be very short lived, I don't think
> this is something that we need to worry about. It wouldn't be that hard to fix
> though, just annoying and add a little more complexity to the source.
> 
> The corresponding patches can be found in the order of this series here:
>  1: [PATCH v3] disk/cryptodisk: When cheatmounting, use the sector info of the cheat device
>     https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00106.html
>  2: [PATCH v6 1/2] devmapper/getroot: Have devmapper recognize LUKS2
>     https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00036.html
>  3: [PATCH v6 2/2] devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM parameters
>     https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00038.html
>  4: [PATCH v4] grub-fs-tester: Add luks1 and luks2 support
>     https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00127.html

For all patches:

    Reviewed-by: Patrick Steinhardt <ps@pks.im>

Thanks a bunch!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM parameters

[Vladimir 'phcoder' Serbinenko]

> +              name = dm_tree_node_get_name (node);
> +              if (grub_strlen (name) == 0)
> +                grub_util_error (_("cannot get dm node name for grub dev `%s'"), grdev);

if (name[0] == '\0')
  ...

Re: [PATCH 0/4] LUKS1/2 testing in fs-tester and LUKS2 support in grub-probe

[Glenn Washburn]

On Fri, 13 Jan 2023 09:31:43 +0100
Patrick Steinhardt <ps@pks.im> wrote:

> On Thu, Jan 12, 2023 at 05:05:06PM -0600, Glenn Washburn wrote:
> > This series adds the testing of LUKS 1 and 2 encrypted container to
> > the user-space fs tester. While this does replicate some of the
> > testing done by grub_cmd_cryptomount, and is less flexible, it is
> > run natively (that is without need for virtualization). It also
> > tests the added LUKS2 support in grub-probe, unlike
> > grub_cmd_cryptomount which does not test any user-space grub
> > components.
> > 
> > All of these patches have previously been sent to the list. Only
> > patch 4 has been slightly modified to fix some issues. Since
> > utility of any one is greatly enhanced, and in the case of patch 4
> > required, by the others, I think it makes sense to evaluate them
> > together.
> 
> Yeah, makes sense to have them all in one place.
> 
> > Fabian has noted a memory leak with error handling when using LUKS2
> > with a null_cipher[1]. Since this isn't something that I expect to
> > happen in the wild (why would you use an unencrypted LUKS
> > container?) and even if it does occur grub-probe is exiting, so the
> > leak would be very short lived, I don't think this is something
> > that we need to worry about. It wouldn't be that hard to fix
> > though, just annoying and add a little more complexity to the
> > source.
> > 
> > The corresponding patches can be found in the order of this series
> > here: 1: [PATCH v3] disk/cryptodisk: When cheatmounting, use the
> > sector info of the cheat device
> > https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00106.html
> > 2: [PATCH v6 1/2] devmapper/getroot: Have devmapper recognize LUKS2
> > https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00036.html
> > 3: [PATCH v6 2/2] devmapper/getroot: Set up cheated LUKS2
> > cryptodisk mount from DM parameters
> > https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00038.html
> > 4: [PATCH v4] grub-fs-tester: Add luks1 and luks2 support
> > https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00127.html
> 
> For all patches:
> 
>     Reviewed-by: Patrick Steinhardt <ps@pks.im>
> 
> Thanks a bunch!

And thanks Patrick for taking a look at these again and the other
cryptomount test patches.

Glenn

Re: [PATCH 0/4] LUKS1/2 testing in fs-tester and LUKS2 support in grub-probe

[Daniel Kiper]

On Fri, Jan 13, 2023 at 09:31:43AM +0100, Patrick Steinhardt wrote:
> On Thu, Jan 12, 2023 at 05:05:06PM -0600, Glenn Washburn wrote:
> > This series adds the testing of LUKS 1 and 2 encrypted container to the
> > user-space fs tester. While this does replicate some of the testing done
> > by grub_cmd_cryptomount, and is less flexible, it is run natively (that is
> > without need for virtualization). It also tests the added LUKS2 support in
> > grub-probe, unlike grub_cmd_cryptomount which does not test any user-space
> > grub components.
> >
> > All of these patches have previously been sent to the list. Only patch 4 has
> > been slightly modified to fix some issues. Since utility of any one is greatly
> > enhanced, and in the case of patch 4 required, by the others, I think it makes
> > sense to evaluate them together.
>
> Yeah, makes sense to have them all in one place.
>
> > Fabian has noted a memory leak with error handling when using LUKS2 with a
> > null_cipher[1]. Since this isn't something that I expect to happen in the wild
> > (why would you use an unencrypted LUKS container?) and even if it does occur
> > grub-probe is exiting, so the leak would be very short lived, I don't think
> > this is something that we need to worry about. It wouldn't be that hard to fix
> > though, just annoying and add a little more complexity to the source.
> >
> > The corresponding patches can be found in the order of this series here:
> >  1: [PATCH v3] disk/cryptodisk: When cheatmounting, use the sector info of the cheat device
> >     https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00106.html
> >  2: [PATCH v6 1/2] devmapper/getroot: Have devmapper recognize LUKS2
> >     https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00036.html
> >  3: [PATCH v6 2/2] devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM parameters
> >     https://lists.gnu.org/archive/html/grub-devel/2022-07/msg00038.html
> >  4: [PATCH v4] grub-fs-tester: Add luks1 and luks2 support
> >     https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00127.html
>
> For all patches:
>
>     Reviewed-by: Patrick Steinhardt <ps@pks.im>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel