Re: [PATCH v2 1/4] cryptodisk: Add infrastructure to pass data from cryptomount to cryptodisk modules

[Patrick Steinhardt]

On Mon, Sep 27, 2021 at 06:14:00PM -0500, Glenn Washburn wrote:
> As an example, passing a password as a cryptomount argument is implemented.
> However, the backends are not implemented, so testing this will return a not
> implemented error.
> 
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/cryptodisk.c | 31 +++++++++++++++++++++----------
>  grub-core/disk/geli.c       |  6 +++++-
>  grub-core/disk/luks.c       |  7 ++++++-
>  grub-core/disk/luks2.c      |  7 ++++++-
>  include/grub/cryptodisk.h   |  9 ++++++++-
>  5 files changed, 46 insertions(+), 14 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 90f82b2d3..ca034859e 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =
>      /* TRANSLATORS: It's still restricted to cryptodisks only.  */
>      {"all", 'a', 0, N_("Mount all."), 0, 0},
>      {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
> +    {"password", 'p', 0, N_("Password to open volumes."), 0, 
> ARG_TYPE_STRING},
>      {0, 0, 0, 0, 0, 0}
>    };
>  
> @@ -996,7 +997,9 @@ cryptodisk_close (grub_cryptodisk_t dev)
>  }
>  
>  static grub_err_t
> -grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
> +grub_cryptodisk_scan_device_real (const char *name,
> +                               grub_disk_t source,
> +                               grub_cryptomount_args_t cargs)
>  {
>    grub_err_t err;
>    grub_cryptodisk_t dev;
> @@ -1015,7 +1018,7 @@ grub_cryptodisk_scan_device_real (const char *name, 
> grub_disk_t source)
>      if (!dev)
>        continue;
>      
> -    err = cr->recover_key (source, dev);
> +    err = cr->recover_key (source, dev, cargs);
>      if (err)
>      {
>        cryptodisk_close (dev);
> @@ -1080,10 +1083,11 @@ grub_cryptodisk_cheat_mount (const char *sourcedev, 
> const char *cheat)
>  
>  static int
>  grub_cryptodisk_scan_device (const char *name,
> -                          void *data __attribute__ ((unused)))
> +                          void *data)
>  {
>    grub_err_t err;
>    grub_disk_t source;
> +  grub_cryptomount_args_t cargs = data;
>  
>    /* Try to open disk.  */
>    source = grub_disk_open (name);
> @@ -1093,7 +1097,7 @@ grub_cryptodisk_scan_device (const char *name,
>        return 0;
>      }
>  
> -  err = grub_cryptodisk_scan_device_real (name, source);
> +  err = grub_cryptodisk_scan_device_real (name, source, cargs);
>  
>    grub_disk_close (source);
>    
> @@ -1106,12 +1110,19 @@ static grub_err_t
>  grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>  {
>    struct grub_arg_list *state = ctxt->state;
> +  struct grub_cryptomount_args cargs = {0};
>  
>    if (argc < 1 && !state[1].set && !state[2].set)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
>  
> +  if (state[3].set) /* password */
> +    {
> +      cargs.key_data = (grub_uint8_t *) state[3].arg;
> +      cargs.key_len = grub_strlen(state[3].arg);

Nit: there's a missing space after the function name here. Other than
that, the patch looks good to me.

Patrick

signature.asc
Description: PGP signature