Bug#1028301: grub: grub-probe doesn't detect that file is on cryptfs on new installation

Steve McIntyre steve at einval.com
Sat Mar 4 21:27:26 GMT 2023


Thanks Ben!

Looking at these now...

On Fri, Mar 03, 2023 at 09:28:26PM +0100, Ben Hutchings wrote:
>Control: tag -1 upstream fixed-upstream patch
>
>On Mon, 09 Jan 2023 12:12:19 +0100 Laurent Bigonville
><bigon at debian.org> wrote:
>> Package: grub-common
>> Version: 2.06-7
>> Severity: serious
>> File: /usr/sbin/grub-probe
>> 
>> Hello,
>> 
>> On a newly installed laptop, it seems that grub-probe is not able to
>> detect that files are on an encrypted FS.
>> 
>> New laptop:
>> 
>> $ sudo grub-probe -t abstraction /usr/share/images/desktop-base/desktop-grub.png
>> lvm
>> 
>> $ sudo lsblk
>> NAME                      MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINTS
>> nvme0n1                   259:0    0 476,9G  0 disk
>> ├─nvme0n1p1               259:1    0   512M  0 part  /boot/efi
>> ├─nvme0n1p2               259:2    0   488M  0 part  /boot
>> └─nvme0n1p3               259:3    0   476G  0 part
>>   └─nvme0n1p3_crypt       254:0    0 475,9G  0 crypt
>>     ├─new_laptop--vg-root    254:1    0  27,9G  0 lvm   /
>>     ├─new_laptop--vg-swap_1  254:2    0   976M  0 lvm   [SWAP]
>>     ├─new_laptop--vg-home    254:3    0    40G  0 lvm   /home
>>     └─new_laptop--vg-libvirt 254:4    0    60G  0 lvm   /var/lib/libvirt
>[...]
>
>I can reproduce this. What changed is that we now use LUKS2 instead of
>LUKS1. Although GRUB has some LUKS2 support, it doesn't probe LUKS2
>volumes automatically.
>
>I found the 3 upstream commits that are enough to make the "grub-probe
>..." line work and am attaching a debdiff with those.  I don't know
>whether this is enough to completely fix the problem.
>
>Ben.
>
>-- 
>Ben Hutchings
>Never put off till tomorrow what you can avoid all together.
>

>diff -Nru grub2-2.06/debian/changelog grub2-2.06/debian/changelog
>--- grub2-2.06/debian/changelog	2023-02-25 21:16:55.000000000 +0100
>+++ grub2-2.06/debian/changelog	2023-03-03 19:21:28.000000000 +0100
>@@ -1,3 +1,15 @@
>+grub2 (2.06-8.2) UNRELEASED; urgency=medium
>+
>+  * Non-maintainer upload.
>+  * Fix probing of LUKS2 devices (Closes: #1028301):
>+    - disk/cryptodisk: When cheatmounting, use the sector info of the cheat
>+      device
>+    - osdep/devmapper/getroot: Have devmapper recognize LUKS2
>+    - osdep/devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from DM
>+      parameters
>+
>+ -- Ben Hutchings <benh at debian.org>  Fri, 03 Mar 2023 19:21:28 +0100
>+
> grub2 (2.06-8.1) experimental; urgency=medium
> 
>   * Non-maintainer upload.
>diff -Nru grub2-2.06/debian/patches/disk-cryptodisk-when-cheatmounting-use-the-sector-info-of-the-cheat-device.patch grub2-2.06/debian/patches/disk-cryptodisk-when-cheatmounting-use-the-sector-info-of-the-cheat-device.patch
>--- grub2-2.06/debian/patches/disk-cryptodisk-when-cheatmounting-use-the-sector-info-of-the-cheat-device.patch	1970-01-01 01:00:00.000000000 +0100
>+++ grub2-2.06/debian/patches/disk-cryptodisk-when-cheatmounting-use-the-sector-info-of-the-cheat-device.patch	2023-03-03 19:21:28.000000000 +0100
>@@ -0,0 +1,72 @@
>+From: Fabian Vogt <fvogt at suse.de>
>+Date: Thu, 12 Jan 2023 17:05:07 -0600
>+Subject: disk/cryptodisk: When cheatmounting, use the sector info of the cheat
>+ device
>+Origin: https://git.savannah.gnu.org/cgit/grub.git/commit/?id=efc9c363b2aab222586b420508eb46fc13242739
>+Bug-Debian: https://bugs.debian.org/1028301
>+
>+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_device_open() complains.
>+
>+This fixes grub-probe failing with
>+"error: sector sizes of 1 bytes aren't supported yet.".
>+
>+Signed-off-by: Fabian Vogt <fvogt at suse.de>
>+Reviewed-by: Patrick Steinhardt <ps at pks.im>
>+Tested-by: Glenn Washburn <development at efficientek.com>
>+Reviewed-by: Glenn Washburn <development at efficientek.com>
>+Reviewed-by: Patrick Steinhardt <ps at pks.im>
>+Reviewed-by: Daniel Kiper <daniel.kiper at oracle.com>
>+---
>+ grub-core/disk/cryptodisk.c | 20 ++++++++++++++++++--
>+ 1 file changed, 18 insertions(+), 2 deletions(-)
>+
>+--- a/grub-core/disk/cryptodisk.c
>++++ b/grub-core/disk/cryptodisk.c
>+@@ -694,16 +694,31 @@ grub_cryptodisk_open (const char *name,
>+   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
>+ 
>+@@ -717,6 +732,7 @@ grub_cryptodisk_open (const char *name,
>+     }
>+ 
>+   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;
>diff -Nru grub2-2.06/debian/patches/osdep-devmapper-getroot-have-devmapper-recognize-luks2.patch grub2-2.06/debian/patches/osdep-devmapper-getroot-have-devmapper-recognize-luks2.patch
>--- grub2-2.06/debian/patches/osdep-devmapper-getroot-have-devmapper-recognize-luks2.patch	1970-01-01 01:00:00.000000000 +0100
>+++ grub2-2.06/debian/patches/osdep-devmapper-getroot-have-devmapper-recognize-luks2.patch	2023-03-03 19:20:49.000000000 +0100
>@@ -0,0 +1,54 @@
>+From: Josselin Poiret <dev at jpoiret.xyz>
>+Date: Thu, 12 Jan 2023 17:05:08 -0600
>+Subject: osdep/devmapper/getroot: Have devmapper recognize LUKS2
>+Origin: https://git.savannah.gnu.org/cgit/grub.git/commit/?id=9022a48dd9984fc3e90a5b42c3b5483d6061ccfb
>+Bug-Debian: https://bugs.debian.org/1028301
>+
>+Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
>+as being LUKS cryptodisks.
>+
>+Signed-off-by: Josselin Poiret <dev at jpoiret.xyz>
>+Tested-by: Glenn Washburn <development at efficientek.com>
>+Reviewed-by: Patrick Steinhardt <ps at pks.im>
>+Reviewed-by: Daniel Kiper <daniel.kiper at oracle.com>
>+---
>+ grub-core/osdep/devmapper/getroot.c | 11 +++++++----
>+ 1 file changed, 7 insertions(+), 4 deletions(-)
>+
>+--- a/grub-core/osdep/devmapper/getroot.c
>++++ b/grub-core/osdep/devmapper/getroot.c
>+@@ -143,7 +143,8 @@ grub_util_get_dm_abstraction (const char
>+       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;
>+@@ -184,7 +185,9 @@ grub_util_pull_devmapper (const char *os
>+ 	  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);
>+@@ -258,11 +261,11 @@ grub_util_get_devmapper_grub_dev (const
>+       {
>+ 	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;
>+       }
>diff -Nru grub2-2.06/debian/patches/osdep-devmapper-getroot-set-up-cheated-luks2-cryptodisk-mount-from-dm-parameters.patch grub2-2.06/debian/patches/osdep-devmapper-getroot-set-up-cheated-luks2-cryptodisk-mount-from-dm-parameters.patch
>--- grub2-2.06/debian/patches/osdep-devmapper-getroot-set-up-cheated-luks2-cryptodisk-mount-from-dm-parameters.patch	1970-01-01 01:00:00.000000000 +0100
>+++ grub2-2.06/debian/patches/osdep-devmapper-getroot-set-up-cheated-luks2-cryptodisk-mount-from-dm-parameters.patch	2023-03-03 19:21:28.000000000 +0100
>@@ -0,0 +1,154 @@
>+From: Josselin Poiret <dev at jpoiret.xyz>
>+Date: Thu, 12 Jan 2023 17:05:09 -0600
>+Subject: osdep/devmapper/getroot: Set up cheated LUKS2 cryptodisk mount from
>+ DM parameters
>+Origin: https://git.savannah.gnu.org/cgit/grub.git/commit/?id=aa5172a55cfabdd0bed3161ad44fc228b9d019f7
>+Bug-Debian: https://bugs.debian.org/1028301
>+
>+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 at jpoiret.xyz>
>+Tested-by: Glenn Washburn <development at efficientek.com>
>+Reviewed-by: Patrick Steinhardt <ps at pks.im>
>+Reviewed-by: Daniel Kiper <daniel.kiper at oracle.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 2bf4264..cc3f7da 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 (*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);
>++
>++              /*
>++               * The dm target parameters for dm-crypt are
>++               * <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
>+-- 
>+cgit v1.1
>+
>diff -Nru grub2-2.06/debian/patches/series grub2-2.06/debian/patches/series
>--- grub2-2.06/debian/patches/series	2023-02-25 21:09:36.000000000 +0100
>+++ grub2-2.06/debian/patches/series	2023-03-03 19:21:28.000000000 +0100
>@@ -115,3 +115,6 @@
> ignore_checksum_seed_incompat_feature.patch
> ignore_the_large_dir_incompat_feature.patch
> 987008-lvrename-boot-fail.patch
>+disk-cryptodisk-when-cheatmounting-use-the-sector-info-of-the-cheat-device.patch
>+osdep-devmapper-getroot-have-devmapper-recognize-luks2.patch
>+osdep-devmapper-getroot-set-up-cheated-luks2-cryptodisk-mount-from-dm-parameters.patch



-- 
Steve McIntyre, Cambridge, UK.                                steve at einval.com
"Since phone messaging became popular, the young generation has lost the
 ability to read or write anything that is longer than one hundred and sixty
 characters."  -- Ignatios Souvatzis



More information about the Pkg-grub-devel mailing list