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

[Michael Chang <mchang@suse.com>]

On Tue, Jun 14, 2022 at 03:47:30PM +0200, The development of GNU GRUB wrote:
> 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 <grub/emu/misc.h>
>  #include <grub/emu/hostdisk.h>
>  
> +#include <grub/cryptodisk.h>
> +
>  static int
>  grub_util_open_dm (const char *os_dev, struct dm_tree **tree,
>  		   struct dm_tree_node **node)
> @@ -186,7 +188,6 @@ grub_util_pull_devmapper (const char *os_dev)
>        && lastsubdev)
>      {
>        char *grdev = grub_util_get_grub_dev (lastsubdev);
> -      dm_tree_free (tree);
>        if (grdev)
>  	{
>  	  grub_err_t err;
> @@ -194,7 +195,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;

Just a heads up. This failed to build on 32-bit architectures (i586,
armv7l) with following error:

[  141s] ../grub-core/osdep/devmapper/getroot.c: In function 'grub_util_pull_devmapper':
[  141s] ../grub-core/osdep/devmapper/getroot.c:234:46: error: passing argument 3 of 'dm_get_next_target' from incompatible pointer type [-Werror=incompatible-pointer-types]
[  141s]   234 |               dm_get_next_target (dmt, NULL, &start, &length,
[  141s]       |                                              ^~~~~~
[  141s]       |                                              |
[  141s]       |                                              grub_addr_t * {aka unsigned int *}
[  141s] In file included from ../grub-core/osdep/devmapper/getroot.c:44:
[  141s] /usr/include/libdevmapper.h:288:48: note: expected 'uint64_t *' {aka 'long long unsigned int *'} but argument is of type 'grub_addr_t *' {aka 'unsigned int *'}
[  141s]   288 |                          void *next, uint64_t *start, uint64_t *length,
[  141s]       |                                      ~~~~~~~~~~^~~~~
[  141s] ../grub-core/osdep/devmapper/getroot.c:234:54: error: passing argument 4 of 'dm_get_next_target' from incompatible pointer type [-Werror=incompatible-pointer-types]
[  141s]   234 |               dm_get_next_target (dmt, NULL, &start, &length,
[  141s]       |                                                      ^~~~~~~
[  141s]       |                                                      |
[  141s]       |                                                      grub_addr_t * {aka unsigned int *}
[  141s] /usr/include/libdevmapper.h:288:65: note: expected 'uint64_t *' {aka 'long long unsigned int *'} but argument is of type 'grub_addr_t *' {aka 'unsigned int *'}
[  141s]   288 |                          void *next, uint64_t *start, uint64_t *length,
[  141s]       |                                                       ~~~~~~~~~~^~~~~~

Apparently changing to use 'grub_uint64_t' for both start and length
fixed the problem for me.

Thanks,
Michael

> +              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, &params);
> +              if (strncmp (target_type, "crypt", sizeof ("crypt")) != 0)
> +                grub_util_error (_("dm target of type `%s' is not `crypt'"),
> +                                 target_type);
> +
> +              /* dm target parameters for dm-crypt is
> +               * <cipher> <key> <iv_offset> <device path> <offset> [<#opt_params> <opt_param1> ...]
> +               */
> +              c = params;
> +              remaining = grub_strlen (c);
> +
> +              /* first, get the cipher name from the cipher */
> +              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 mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel