[PATCH 1/2] devmapper/getroot: Have devmapper recognize LUKS2
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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