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

2021-12-09 Thread Josselin Poiret via Grub-devel
Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.
---
 grub-core/osdep/devmapper/getroot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index 9ba5c9865..ad1daf9c8 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -138,7 +138,7 @@ 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-LUKS", sizeof ("CRYPT-LUKS") - 1) == 0)
 {
   grub_free (uuid);
   return GRUB_DEV_ABSTRACTION_LUKS;
@@ -179,7 +179,7 @@ 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-LUKS", sizeof ("CRYPT-LUKS") - 1) == 0
   && lastsubdev)
 {
   char *grdev = grub_util_get_grub_dev (lastsubdev);
@@ -253,11 +253,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.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-12-09 Thread Josselin Poiret via Grub-devel
This lets a LUKS2 cryptodisk have all the cipher, hash, and sizes
filled out, otherwise they wouldn't be initialized if cheat mounted.
---
 grub-core/osdep/devmapper/getroot.c | 51 -
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index ad1daf9c8..574d07a20 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -51,6 +51,8 @@
 #include 
 #include 
 
+#include 
+
 static int
 grub_util_open_dm (const char *os_dev, struct dm_tree **tree,
   struct dm_tree_node **node)
@@ -183,7 +185,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;
@@ -191,7 +192,55 @@ 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 and size from dm parameter, since it is not
+ possible to determine the correct ones without unlocking, as
+ there might be multiple segments. */
+  grub_disk_t source = grub_disk_open (grdev);
+  grub_cryptodisk_t cryptodisk = 
grub_cryptodisk_get_by_source_disk (source);
+  grub_disk_close (source);
+  grub_addr_t start, length;
+  char *target_type = NULL;
+  char *params;
+  const char *name = dm_tree_node_get_name (node);
+  char *cipher, *cipher_mode;
+  struct dm_task *dmt;
+  grub_util_info ("creating dm task for `%s'", name);
+  if (!(dmt = dm_task_create (DM_DEVICE_TABLE)))
+grub_util_error (_("can't create dm task DM_DEVICE_TABLE"));
+  if (!dm_task_set_name (dmt, name))
+grub_util_error (_("can't set dm task name to `%s'"), name);
+  if (!dm_task_run (dmt))
+grub_util_error (_("can't run dm task for `%s'"), name);
+  dm_get_next_target (dmt, NULL, &start, &length,
+  &target_type, ¶ms);
+  if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
+grub_util_error (_("dm target is not `crypt'"));
+
+  cryptodisk->total_sectors = length;
+  cryptodisk->log_sector_size = GRUB_LUKS1_LOG_SECTOR_SIZE;
+
+  char *seek_head, *c;
+  c = params;
+  if (!(seek_head = grub_memchr (c, '-', grub_strlen (c
+grub_util_error (_("can't get cipher from dm task"));
+  cipher = grub_strndup (c, seek_head - c);
+
+  c = seek_head + 1;
+  if (!(seek_head = grub_memchr (c, ' ', grub_strlen (c
+grub_util_error (_("can't get cipher mode from dm task"));
+  cipher_mode = grub_strndup (c, seek_head - c);
+
+  grub_cryptodisk_setcipher (cryptodisk, cipher, cipher_mode);
+
+  /* This is the only hash usable by PBKDF2, so set it as such */
+  cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256");
+
+  dm_task_destroy (dmt);
+}
}
+  dm_tree_free (tree);
   grub_free (grdev);
 }
   else
-- 
2.34.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 0/2] Have LUKS2 cryptomounts be useable with grub-probe

2021-12-09 Thread Josselin Poiret via Grub-devel
Hello,

These two draft patches make devmapper set up LUKS2 cryptomount
properties when pulling, as well as report LUKS2 cryptomounts as
having GRUB_DEV_ABSTRACTION_LUKS.  This makes grub-probe and
grub-install behave properly wrt. LUKS2 drives: `grub-probe -t
abstraction /` reports all the needed modules for the GRUB image, and
grub-install leads to a working GRUB without manually adding modules.

One small part that I am unsure about, although I have tested it and
it does seem to work properly: if I understand correctly, all dm
devices have a 512 sector size, however LUKS2 lets one choose up to
4096 for the encryption sector size.  Which of these two should be
used as cryptodisk->sector_size?  I put 512 here since we're reading
through a cheated mount, but I'm not so sure.

Best,
Josselin Poiret

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

 grub-core/osdep/devmapper/getroot.c | 59 ++---
 1 file changed, 54 insertions(+), 5 deletions(-)

-- 
2.34.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v2 1/2] devmapper/getroot: Have devmapper recognize LUKS2

2021-12-11 Thread Josselin Poiret via Grub-devel
Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.
---
 grub-core/osdep/devmapper/getroot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index 9ba5c9865..ad1daf9c8 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -138,7 +138,7 @@ 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-LUKS", sizeof ("CRYPT-LUKS") - 1) == 0)
 {
   grub_free (uuid);
   return GRUB_DEV_ABSTRACTION_LUKS;
@@ -179,7 +179,7 @@ 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-LUKS", sizeof ("CRYPT-LUKS") - 1) == 0
   && lastsubdev)
 {
   char *grdev = grub_util_get_grub_dev (lastsubdev);
@@ -253,11 +253,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.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2021-12-11 Thread Josselin Poiret via Grub-devel
This lets a LUKS2 cryptodisk have all the cipher, hash, and sizes
filled out, otherwise they wouldn't be initialized if cheat mounted.
---
 grub-core/osdep/devmapper/getroot.c | 99 -
 1 file changed, 98 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index ad1daf9c8..4b3458639 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -51,6 +51,8 @@
 #include 
 #include 
 
+#include 
+
 static int
 grub_util_open_dm (const char *os_dev, struct dm_tree **tree,
   struct dm_tree_node **node)
@@ -183,7 +185,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;
@@ -191,7 +192,103 @@ 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 and size from dm parameter, since it is not
+   * possible to determine the correct ones without unlocking, as
+   * there might be multiple segments.
+   */
+  grub_disk_t source = grub_disk_open (grdev);
+  grub_cryptodisk_t cryptodisk = 
grub_cryptodisk_get_by_source_disk (source);
+  grub_disk_close (source);
+  grub_addr_t start, length;
+  char *target_type = NULL;
+  char *params;
+  const char *name = dm_tree_node_get_name (node);
+  char *cipher, *cipher_mode;
+  struct dm_task *dmt;
+  grub_util_info ("populating parameters of cryptomount `%s' from 
DM device `%s'",
+  uuid, name);
+  if (!(dmt = dm_task_create (DM_DEVICE_TABLE)))
+grub_util_error (_("can't create dm task DM_DEVICE_TABLE"));
+  if (!dm_task_set_name (dmt, name))
+grub_util_error (_("can't set dm task name to `%s'"), name);
+  if (!dm_task_run (dmt))
+grub_util_error (_("can't run dm task for `%s'"), name);
+  dm_get_next_target (dmt, NULL, &start, &length,
+  &target_type, ¶ms);
+  if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
+grub_util_error (_("dm target is not `crypt'"));
+
+  cryptodisk->total_sectors = length;
+  cryptodisk->log_sector_size = 9; /* default dm sector size */
+
+  /* dm target parameters for dm-crypt is
+   *  
[<#opt_params> ]
+   */
+  char *seek_head, *c;
+  c = params;
+
+  /* first, get the cipher name from the cipher */
+  if (!(seek_head = grub_memchr (c, '-', grub_strlen (c
+grub_util_error (_("can't get cipher from dm-crypt parameters 
`%s'"),
+ params);
+  cipher = grub_strndup (c, seek_head - c);
+  c = seek_head + 1;
+
+  /* now, the cipher mode */
+  if (!(seek_head = grub_memchr (c, ' ', grub_strlen (c
+grub_util_error (_("can't get cipher mode from dm-crypt 
parameters `%s'"),
+ params);
+  cipher_mode = grub_strndup (c, seek_head - c);
+
+  grub_cryptodisk_setcipher (cryptodisk, cipher, cipher_mode);
+
+  /* This is the only hash usable by PBKDF2, so set it as such */
+  cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256");
+
+  /* drop key, iv_offset, device path and offset */
+  for (int dropped_tokens = 0; dropped_tokens < 4; 
dropped_tokens++)
+{
+  seek_head++;
+  seek_head = grub_memchr (seek_head, ' ', grub_strlen 
(seek_head));
+}
+
+  /* if we have optional parameters, skip #opt_params */
+  if (seek_head && (seek_head = grub_memchr (seek_head, ' ', 
grub_strlen (seek_head
+{
+  seek_head++;
+  for (;seek_head;seek_head = grub_memchr (seek_head, ' ', 
grub_strlen (seek_head)))
+{
+  seek_head++;
+  if (strncmp (seek_head, "sector_size:", sizeof 
("sector_size:") - 1) == 0)
+{
+  char *sector_size_str;
+  unsigned long long sector_size;
+  c = seek_head + sizeof ("sector_size:") - 1;
+  seek_head = grub_memchr (c, ' ', grub_strlen (c));
+  if (seek_head)
+ 

[PATCH v2 0/2] Have LUKS2 cryptomounts be useable with grub-probe

2021-12-11 Thread Josselin Poiret via Grub-devel
Glenn Washburn  writes:
> Its not clear to me, did you test a LUKS2 device with sector size 4096
> with this change? I believe DM does use 512-byte sectors internally,
> but it can create block devices that report and use other sector sizes.
> You can verfiy this by creating a 4096 sector size LUKS2 devices, open
> it with cryptsetup, and then run "blockdev --getbsz /dev/mapper/".

You're right, blockdev does indeed report 4096.  Here is an updated
patch that parses the optional sector_size argument from the DM
parameters, I have checked that it does indeed set the right
log_sector_size.  I think it worked without it because you can
technically just read with a lower sector size, but better be safe
than sorry!

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

 grub-core/osdep/devmapper/getroot.c | 107 ++--
 1 file changed, 102 insertions(+), 5 deletions(-)

-- 
2.34.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 3/4] luks2: set up dummy sector size during scan

2021-12-22 Thread Josselin Poiret via Grub-devel
Hello everyone,

Fabian Vogt  writes:
> It looks like we have a third patch (series) for this feature meanwhile:
> [PATCH 0/2] Have LUKS2 cryptomounts be useable with grub-probe
>
> I CC'd the author, let's try to coordinate.
>
> Thanks,
> Fabian

Let me just say that I had not found this patch series while searching
beforehand.  Let me just recap what my patches do differently (in
relation to patches 3 and 4 of this series):

Because cheat-mounting cryptodisks only happens (from my understanding)
when pulling devmapper devices, we can simply ask the kernel for the dm
and dm-crypt parameters that it's opened with, and populate our
cheat-mounted device from that.  This completely circumvents the multiple
segments issue, as this will always yield the parameters corresponding
to the user-specified mountpoint of `grub-probe` or `grub-install`.

I also opted not to add a GRUB_DEV_ABSTRACTION_LUKS2 abstraction, so as
to reuse all existing code that supports LUKS1, although that can be
confusing.  We could simply rename GRUB_DEV_ABSTRACTION_LUKS1 to
GRUB_DEV_ABSTRACTION_CRYTODISK, as LUKS1 and LUKS2 only differ in how
they're unlocked, not in underlying algorithms.

What do you think?

-- 
Josselin Poiret

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 3/4] luks2: set up dummy sector size during scan

2022-02-07 Thread Josselin Poiret via Grub-devel
Hi Fabian,

Fabian Vogt  writes:

> Did you have a look at my approach? That effectively does the same, but using 
> a
> single ioctl instead of anything complex with DM directly.

I agree that it's sufficient for sector_size, but we still need the
cryptodisk algorithm so that grub-install will know which crypto modules
to include.  libdm is actually a helper library around dm-specific
ioctls, since they are apparently annoying to use, so in the end we're
still using the same interface :)

As for the 4 patches, since the actual sector_size is available to us, I
think using 512 in all cases or adding a specific 0 sector_size is
something we should avoid.

Best,
-- 
Josselin Poiret

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 3/3] grub-core/kern/disk.c: handle LUKS2 devices

2022-05-09 Thread Josselin Poiret via Grub-devel
Hello everyone,

Glenn Washburn  writes:

> I don't really like this, but it gets the job done and is a work-around
> for a peculiarity of the LUKS2 backend. The cheat mount code for
> cryptodisk does only calls scan() and not recover_key(). For LUKS1 scan
> will return a grub_cryptodisk_t with log_sector_size set, but LUKS2
> will not. This is because for LUKS1 the log_sector_size is constant
> (LUKS1 also sets most of the other properties of the cryptodisk device,
> like crypto algorithm, because they are in the binary header). However,
> for LUKS2 the sector size (along with other properties) is in the json
> header, which isn't getting parsed in scan(). 
>
> For single segment LUKS2 containers, scan() could get the sector size
> from the json segment object. The LUKS2 spec says that normal LUKS2
> devices are single segment[1], so this should work in the the cases the
> care about (currently). scan() would not be able to fill in the other
> properties, like crypto algorithm, because that depends on the keyslot
> used, which needs key recovery to be determined. To avoid parsing the
> json data twice, once in scan() and once in recover_key(), which should
> be avoided, the parsed json object could be put in the grub_cryptodisk_t
> in scan(), and used and freed in recover_key(). We'd probably also want
> to add a way for grub_cryptodisk_t objects to get cleaned up by the
> backend using them, so that the json object could be freed even if
> recover_key() is never called.
>
> I think the above is the real fix, a moderate amount more work, and not
> something I'd expect Pierre-Louis to take up. So if we're not going to
> do this to get this functionality to work, we'll need a hack to get it
> working. However, I'd prefer a different one.
>
> I've not tested this, but it seems to me that we can set the
> log_sector_size field to GRUB_DISK_SECTOR_BITS _if_ it equals zero in
> grub_cryptodisk_cheat_insert(). This limits the hack to only GRUB
> host/user-space code.

Regarding these last lines, it's also possible to directly ask dm for
the actual sector size when cheatmounting, as well as the crypto
algorithm, bypassing the whole issue of parsing the json and finding the
right slot.  This is roughly what's done in patch 2 of [1], maybe this
workaround would be more to your liking?

I've distributed this patch to several people that were having issues on
GNU Guix and they've been happily using LUKS2 with GRUB with it.

[1] https://lists.gnu.org/archive/html/grub-devel/2021-12/msg00079.html
(20211211122945.6326-1-...@jpoiret.xyz)

Best,
-- 
Josselin Poiret

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 1/2] devmapper/getroot: Have devmapper recognize LUKS2

2022-05-20 Thread Josselin Poiret via Grub-devel
Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.
---
 grub-core/osdep/devmapper/getroot.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index 9ba5c9865..ad1daf9c8 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -138,7 +138,7 @@ 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-LUKS", sizeof ("CRYPT-LUKS") - 1) == 0)
 {
   grub_free (uuid);
   return GRUB_DEV_ABSTRACTION_LUKS;
@@ -179,7 +179,7 @@ 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-LUKS", sizeof ("CRYPT-LUKS") - 1) == 0
   && lastsubdev)
 {
   char *grdev = grub_util_get_grub_dev (lastsubdev);
@@ -253,11 +253,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.36.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3 0/2] Have LUKS2 cryptomounts be useable with grub-probe

2022-05-20 Thread Josselin Poiret via Grub-devel
Hello Glenn,

Thanks for your review!  I've adressed all of your comments with these
revised patches (only the second one has changed), with the caveats below:

Glenn Washburn  writes:
>
>> +grub_util_error (_("can't set dm task name to `%s'"), name);
>> +  if (!dm_task_run (dmt))
>> +grub_util_error (_("can't run dm task for `%s'"), name);
>> +  dm_get_next_target (dmt, NULL, &start, &length,
>> +  &target_type, ¶ms);
>
> It looks like this might return NULL on error? Or does it never error
> here?

dm_get_next_target only unpacks the result of a dm_task, it does no
error handling at all.  We only need the first (and only) target here,
hence the NULL parameter.

> [...]
>
>> +  /* This is the only hash usable by PBKDF2, so set it as such 
>> */
>> +  cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256");
>
> So does this mean that when/if argon2 support is added, that this will
> need to be detected as well? If so, is there a way to get the hash via
> devmapper? It doesn't show up with the table command, so I'm guessing
> not. On the otherhand, I don't think cryptodisk->hash is ever used in
> the user-space utilities because the cryptodisk is never attempted to
> be unlocked, so there's no password to hash. Perhaps this should be
> NULL with a comment that its not used (if I'm right about that).

Looking at the LUKS2 spec (see Key Derivation Object, section 3.2.5 of
[1]), it doesn't seem that argon2 has a way to specify different hash
functions, so I don't think we would need to detect that.  We need to
set cryptodisk->hash though so that the right abstractions are picked
up and included by grub-install in the binary (this was my motivation
for this whole thing in the first place)!

> [...]
>
>> +  else
>> +sector_size_str = c;
>> +  sector_size = grub_strtoull (sector_size_str, 
>> NULL, 10);
>
> This should check for in grub_strtoull errors as is done in commit
> ac8a37dda (net/http: Allow use of non-standard TCP/IP ports).

I didn't spot anything out of the ordinary in that commit, but note
that just after grub_strtoull, we check that sector_size is either
512, 1024, 2048 or 4096, and error if it isn't (unlikely).

In any case, I've tested this in a VM with a LUKS2 device that has a
4096 sector_size, and `-v` does indeed report the right sector_size as
well as the number of (encryption) sectors!  Installation of GNU Guix
went as expected on it, I even stacked LVM with BTRFS on top and
everything worked properly.

[1] https://gitlab.com/cryptsetup/LUKS2-docs/blob/master/luks2_doc_wip.pdf

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

 grub-core/osdep/devmapper/getroot.c | 138 +++-
 1 file changed, 133 insertions(+), 5 deletions(-)

-- 
2.36.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2022-05-20 Thread Josselin Poiret via Grub-devel
This lets a LUKS2 cryptodisk have all the cipher, hash, and sizes
filled out, otherwise they wouldn't be initialized if cheat mounted.
---
 grub-core/osdep/devmapper/getroot.c | 130 +++-
 1 file changed, 129 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index ad1daf9c8..b8d66029a 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -43,6 +43,8 @@
 #include 
 #include 
 
+#define DM_LOG_SECTOR_SIZE 9
+
 #include 
 #include 
 
@@ -51,6 +53,8 @@
 #include 
 #include 
 
+#include 
+
 static int
 grub_util_open_dm (const char *os_dev, struct dm_tree **tree,
   struct dm_tree_node **node)
@@ -183,7 +187,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;
@@ -191,7 +194,132 @@ 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 and size from dm parameters, since it is not
+   * possible to determine the correct ones without unlocking, as
+   * there might be multiple segments.
+   */
+  grub_disk_t source;
+  grub_cryptodisk_t cryptodisk;
+  grub_addr_t start, length;
+  char *target_type;
+  char *params;
+  const char *name;
+  char *cipher, *cipher_mode;
+  struct dm_task *dmt;
+
+  source = grub_disk_open (grdev);
+  cryptodisk = grub_cryptodisk_get_by_source_disk (source);
+  grub_disk_close (source);
+
+  name = dm_tree_node_get_name (node);
+
+  grub_util_info ("populating parameters of cryptomount `%s' from 
DM device `%s'",
+  uuid, name);
+
+  dmt = dm_task_create (DM_DEVICE_TABLE);
+  if (dmt == 0)
+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, ¶ms);
+  if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
+grub_util_error (_("dm target of type `%s' is not `crypt'"),
+ target_type);
+
+  cryptodisk->total_sectors = length;
+  cryptodisk->log_sector_size = DM_LOG_SECTOR_SIZE;
+
+  /* dm target parameters for dm-crypt is
+   *  
[<#opt_params>  ...]
+   */
+  char *seek_head, *c;
+  unsigned int remaining;
+  c = params;
+  remaining = grub_strlen (c);
+
+  /* first, get the cipher name from the cipher */
+  if (!(seek_head = grub_memchr (c, '-', remaining)))
+grub_util_error (_("can't get cipher from dm-crypt parameters 
`%s'"),
+ params);
+  cipher = grub_strndup (c, seek_head - c);
+  remaining -= seek_head - c;
+  c = seek_head + 1;
+
+  /* now, the cipher mode */
+  if (!(seek_head = grub_memchr (c, ' ', remaining)))
+grub_util_error (_("can't get cipher mode from dm-crypt 
parameters `%s'"),
+ params);
+  cipher_mode = grub_strndup (c, seek_head - c);
+  remaining -= seek_head - c;
+  c = seek_head + 1;
+
+  grub_cryptodisk_setcipher (cryptodisk, cipher, cipher_mode);
+
+  grub_free (cipher);
+  grub_free (cipher_mode);
+
+  /* This is the only hash usable by PBKDF2, so set it as such */
+  cryptodisk->hash = grub_crypto_lookup_md_by_name ("sha256");
+
+  /* drop key, iv_offset, device path and offset */
+  for (char dropped_tokens = 0; dropped_tokens < 4; 
dropped_tokens++)
+{
+  seek_head = grub_memchr (c, ' ', remaining);
+  remaining -= seek_head - c;
+  c = seek_head + 1;
+}
+
+  /* if we have optional parameters, skip #opt_params */
+  if (seek_head && (seek_head = grub_m

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

2022-05-21 Thread Josselin Poiret via Grub-devel
Hello Glenn, 

Glenn Washburn  writes:

> After reviewing this, I stumbled upon Fabian's patch. I wrote a
> response on the thread of that patch with the suggestion that his patch
> be used to get the total number of sectors and sector size and this
> patch be used to get the crypto information. This would obviate the
> problematic parsing part of this patch. What do you think about
> removing all the parsing code after the setting of cryptodisk->hash?

I just read that mail, and it does seem like the proper way (I don't
need much convincing to remove most of that ugly parsing code!).  I'll
send a cleaned up patch later with all the logic errors adressed, and
without the further sector size code.

Best,
-- 
Josselin Poiret

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v4 1/2] devmapper/getroot: Have devmapper recognize LUKS2

2022-06-14 Thread Josselin Poiret via Grub-devel
Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.
---
 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 9ba5c9865..2bf4264cf 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.36.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2022-06-14 Thread Josselin Poiret via Grub-devel
This lets a LUKS2 cryptodisk have its cipher and hash filled out,
otherwise they wouldn't be initialized if cheat mounted.
---
 grub-core/osdep/devmapper/getroot.c | 91 -
 1 file changed, 90 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index 2bf4264cf..ac90761ea 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -51,6 +51,8 @@
 #include 
 #include 
 
+#include 
+
 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,95 @@ 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_addr_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);
+  cryptodisk = grub_cryptodisk_get_by_source_disk (source);
+  grub_disk_close (source);
+
+  name = dm_tree_node_get_name (node);
+
+  grub_util_info ("populating parameters of cryptomount `%s' from 
DM device `%s'",
+  uuid, name);
+
+  dmt = dm_task_create (DM_DEVICE_TABLE);
+  if (dmt == 0)
+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, ¶ms);
+  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
+   *  
[<#opt_params>  ...]
+   */
+  c = params;
+  remaining = grub_strlen (c);
+
+  /* first, get the cipher name from the cipher */
+  if (!(seek_head = grub_memchr (c, '-', remaining)))
+grub_util_error (_("can't get cipher from dm-crypt parameters 
`%s'"),
+ params);
+  cipher = grub_strndup (c, seek_head - c);
+  remaining -= seek_head - c + 1;
+  c = seek_head + 1;
+
+  /* now, the cipher mode */
+  if (!(seek_head = grub_memchr (c, ' ', remaining)))
+grub_util_error (_("can't get cipher mode from dm-crypt 
parameters `%s'"),
+ params);
+  cipher_mode = grub_strndup (c, 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 == 0)
+{
+  grub_util_error (_("can't lookup hash sha256 by name"));
+}
+
+  dm_task_destroy (dmt);
+}
}
+  dm_tree_free (tree);
   grub_free (grdev);
 }
   else

[PATCH v4 0/2] Have LUKS2 cryptomounts be useable with grub-probe

2022-06-14 Thread Josselin Poiret via Grub-devel
Hello Glenn,

I took the time to rebase all the patches on the latest master, then
remove the problematic parsing, keeping only the cipher part, as well
as add some additional error checking for grub_cryptodisk_setcipher
and grub_crypto_lookup_md_by_name.  I also took note of another mail
saying that we shouldn't pretend to support LUKS3 if it ever comes
out, so I've modified the first patch to only recognize LUKS1 and
LUKS2.  With Fabian's patch [1] applied, this made grub-install
install fine on a LUKS2 drive with a 4096 sector size.

I hope all the logic errors were corrected (removing the complicated
part should have helped).

[1] 8075647.t7z3s40...@linux-e202.suse.de
(https://lists.gnu.org/archive/html/grub-devel/2022-06/msg00097.html)

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

 grub-core/osdep/devmapper/getroot.c | 102 ++--
 1 file changed, 97 insertions(+), 5 deletions(-)

-- 
2.36.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v5 0/2] Have LUKS2 cryptomounts be useable with grub-probe

2022-06-15 Thread Josselin Poiret via Grub-devel
Hello again,

Following Michael's mail, here's hopefully the latest patch series!
This fixes building on 32-bit by using grub_uint64_t indiscriminately,
and removes the curly braces for the the two ifs at the end.

Sorry for all the noise.

Best,
-- 
Josselin Poiret

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v5 1/2] devmapper/getroot: Have devmapper recognize LUKS2

2022-06-15 Thread Josselin Poiret via Grub-devel
Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.
---
 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 9ba5c9865..2bf4264cf 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.36.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2022-06-15 Thread Josselin Poiret via Grub-devel
This lets a LUKS2 cryptodisk have its cipher and hash filled out,
otherwise they wouldn't be initialized if cheat mounted.
---
 grub-core/osdep/devmapper/getroot.c | 87 -
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/grub-core/osdep/devmapper/getroot.c 
b/grub-core/osdep/devmapper/getroot.c
index 2bf4264cf..80c7e54da 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -51,6 +51,8 @@
 #include 
 #include 
 
+#include 
+
 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,91 @@ 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);
+  cryptodisk = grub_cryptodisk_get_by_source_disk (source);
+  grub_disk_close (source);
+
+  name = dm_tree_node_get_name (node);
+
+  grub_util_info ("populating parameters of cryptomount `%s' from 
DM device `%s'",
+  uuid, name);
+
+  dmt = dm_task_create (DM_DEVICE_TABLE);
+  if (dmt == 0)
+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, ¶ms);
+  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
+   *  
[<#opt_params>  ...]
+   */
+  c = params;
+  remaining = grub_strlen (c);
+
+  /* first, get the cipher name from the cipher */
+  if (!(seek_head = grub_memchr (c, '-', remaining)))
+grub_util_error (_("can't get cipher from dm-crypt parameters 
`%s'"),
+ params);
+  cipher = grub_strndup (c, seek_head - c);
+  remaining -= seek_head - c + 1;
+  c = seek_head + 1;
+
+  /* now, the cipher mode */
+  if (!(seek_head = grub_memchr (c, ' ', remaining)))
+grub_util_error (_("can't get cipher mode from dm-crypt 
parameters `%s'"),
+ params);
+  cipher_mode = grub_strndup (c, 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 == 0)
+  grub_util_error (_("can't lookup hash sha256 by name"));
+
+  dm_task_destroy (dmt);
+}
}
+  dm_tree_free (tree);
   grub_free (grdev);
 }
   else
-- 
2.36.1


___
Grub-devel m

[PATCH v6 1/2] devmapper/getroot: Have devmapper recognize LUKS2

2022-07-08 Thread Josselin Poiret via Grub-devel
Changes UUID comparisons so that LUKS1 and LUKS2 are both recognized
as being LUKS cryptodisks.

Signed-off-by: Josselin Poiret 
---
 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 9ba5c9865..2bf4264cf 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.36.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v6 0/2] Have LUKS2 cryptomounts be useable with grub-probe

2022-07-08 Thread Josselin Poiret via Grub-devel
Hello Daniel,

Thanks for the review.  The following updated patches should contain all the 
changes you asked for.

>Please add your Signed-off-by here. Same applies to first patch too.
Done.

> Please format comments, here and below, properly [1].
Sorry about that, I missed the empty first line. Fixed.

> This function may fail. Please check the returned value and fail if needed.
> [...]
> Ditto.
Fixed (I've used grub_util_error for all the unrecoverable errors, I
hope that's ok).

>> +  name = dm_tree_node_get_name (node);
> I think same applies here...
Right, it can be "", so added a check and comment about the error mode.

> s/0/NULL/ Please use NULL instead of 0 for pointers. Same below...
Done.

> grub_strndup() may fail. Please check if cipher != NULL.
Right, sorry.  Fixed.

> seek_head = grub_memchr (c, ' ', remaining);
> if (seek_head == NULL)
Done in both places.

> Again, grub_strndup() may fail. In general please do not ignore errors
> from functions which may fail.
Done as above.

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

 grub-core/osdep/devmapper/getroot.c | 118 ++--
 1 file changed, 113 insertions(+), 5 deletions(-)

Range-diff against v5:
1:  7af629fca = 1:  f4cbda414 devmapper/getroot: Have devmapper recognize LUKS2
2:  5f9f26118 ! 2:  25ce8bbcc devmapper/getroot: Set up cheated LUKS2 
cryptodisk mount from DM parameters
@@ grub-core/osdep/devmapper/getroot.c: grub_util_pull_devmapper (const 
char *os_de
 lastsubdev, grub_errmsg);
 +  if (strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) 
== 0)
 +{
-+  /* set LUKS2 cipher from dm parameters, since it is not
++  /*
++   * 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-core/osdep/devmapper/getroot.c: grub_util_pull_devmapper (const 
char *os_de
 +  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 == 0)
++  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
++  /*
++   * 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, ¶ms);
 +  if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
-+grub_util_error (_("dm target of type `%s' is not 
`crypt'"),
-+ target_type);
++grub_util_error (_("dm target of type `%s' is not 
`crypt'"), target_type);
 +
-+  /* dm target parameters for dm-crypt is
++  /*
++   * dm target parameters for dm-crypt is
 +   *  
[<#opt_params>  ...]
 +   */
 +  c = params;
 +  remaining = grub_strlen (c);
 +
 +  /* first, get the cipher name from the cipher */
-+  if (!(seek_head = grub_memchr (c, '-', remaining)))
++  seek_head = grub_memchr (c, '-', remaining);
++  

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

2022-07-08 Thread Josselin Poiret via Grub-devel
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 
---
 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 2bf4264cf..090ac7167 100644
--- a/grub-core/osdep/devmapper/getroot.c
+++ b/grub-core/osdep/devmapper/getroot.c
@@ -51,6 +51,8 @@
 #include 
 #include 
 
+#include 
+
 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, ¶ms);
+  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
+   *  
[<#opt_params>  ...]
+   */
+  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_setc

Support in grub userland tools for other PBKDF2 hashes

2023-05-04 Thread Josselin Poiret via Grub-devel
Hi everyone,

It's been brought to my attention that in my commit [1], I mistakenly
indicated that SHA256 was the only hash supported by the PBKDF2 kdf.  I
may have misread the default value for the list of possible values in
the upstream spec, since more hashes are supported.  One possible
problem though is that it would not be possible to simply dynamically
ask dm-crypt for the hash function that was used when unlocking, since
that isn't kept around, from what I remember.

I don't have the bandwidth to work on this currently, but I can see two
solutions: either indiscriminately add all abstractions for all possible
hash functions of PBKDF2, or parse the LUKS2 headers of the partition to
find out which hash function is used.  In the meantime, if you use a
hash function that isn't SHA256, like SHA512, you'll need to add
--modules="gcry_sha512" to your grub-install invocation.

[1] aa5172a55cfabdd0bed3161ad44fc228b9d019f7

Best,
-- 
Josselin Poiret


signature.asc
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel