Re: [PATCH 04/14] cryptodisk: Support key protectors

[Gary Lin]

On Wed, Feb 22, 2023 at 03:00:44PM +0800, Gary Lin via Grub-devel wrote:
> From: Hernan Gatta <hegatta@linux.microsoft.com>
> 
> Add a new parameter to cryptomount to support the key protectors framework: 
> -k.
> The parameter is used to automatically retrieve a key from specified key
> protectors. The parameter may be repeated to specify any number of key
> protectors. These are tried in order until one provides a usable key for any
> given disk.
> 
> Signed-off-by: <Hernan Gatta hegatta@linux.microsoft.com>
The email format is broken here and I didn't notice it...
Will fix the format in V2.

> Signed-off-by: Gary Lin <glin@suse.com>
> ---
>  Makefile.util.def           |   1 +
>  grub-core/disk/cryptodisk.c | 175 +++++++++++++++++++++++++++++-------
>  include/grub/cryptodisk.h   |  14 +++
>  3 files changed, 157 insertions(+), 33 deletions(-)
> 
> diff --git a/Makefile.util.def b/Makefile.util.def
> index beaef1168..1da5a826c 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -35,6 +35,7 @@ library = {
>    common = grub-core/kern/list.c;
>    common = grub-core/kern/misc.c;
>    common = grub-core/kern/partition.c;
> +  common = grub-core/kern/protectors.c;
>    common = grub-core/lib/crypto.c;
>    common = grub-core/lib/json/json.c;
>    common = grub-core/disk/luks.c;
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 34b67a705..c990d5985 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -26,6 +26,7 @@
>  #include <grub/file.h>
>  #include <grub/procfs.h>
>  #include <grub/partition.h>
> +#include <grub/protector.h>
>  
>  #ifdef GRUB_UTIL
>  #include <grub/emu/hostdisk.h>
> @@ -44,7 +45,8 @@ enum
>      OPTION_KEYFILE,
>      OPTION_KEYFILE_OFFSET,
>      OPTION_KEYFILE_SIZE,
> -    OPTION_HEADER
> +    OPTION_HEADER,
> +    OPTION_PROTECTOR
>    };
>  
>  static const struct grub_arg_option options[] =
> @@ -58,6 +60,8 @@ static const struct grub_arg_option options[] =
>      {"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0, 
> ARG_TYPE_INT},
>      {"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0, 
> ARG_TYPE_INT},
>      {"header", 'H', 0, N_("Read header from file"), 0, ARG_TYPE_STRING},
> +    {"protector", 'k', GRUB_ARG_OPTION_REPEATABLE,
> +     N_("Unlock volume(s) using key protector(s)."), 0, ARG_TYPE_STRING},
>      {0, 0, 0, 0, 0, 0}
>    };
>  
> @@ -1060,7 +1064,8 @@ grub_cryptodisk_scan_device_real (const char *name,
>  {
>    grub_err_t ret = GRUB_ERR_NONE;
>    grub_cryptodisk_t dev;
> -  grub_cryptodisk_dev_t cr;
> +  grub_cryptodisk_dev_t cr, crd = NULL;
> +  int i;
>    struct cryptodisk_read_hook_ctx read_hook_data = {0};
>    int askpass = 0;
>    char *part = NULL;
> @@ -1113,41 +1118,112 @@ grub_cryptodisk_scan_device_real (const char *name,
>        goto error_no_close;
>      if (!dev)
>        continue;
> +    crd = cr;
> +  }
>  
> -    if (!cargs->key_len)
> -      {
> -     /* Get the passphrase from the user, if no key data. */
> -     askpass = 1;
> -     part = grub_partition_get_name (source->partition);
> -     grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> -                  source->partition != NULL ? "," : "",
> -                  part != NULL ? part : N_("UNKNOWN"),
> -                  dev->uuid);
> -     grub_free (part);
> -
> -     cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> -     if (cargs->key_data == NULL)
> -       goto error_no_close;
> -
> -     if (!grub_password_get ((char *) cargs->key_data, 
> GRUB_CRYPTODISK_MAX_PASSPHRASE))
> -       {
> -         grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
> -         goto error;
> -       }
> -     cargs->key_len = grub_strlen ((char *) cargs->key_data);
> -      }
> +  if (!dev)
> +    {
> +      grub_error (GRUB_ERR_BAD_MODULE,
> +                  "no cryptodisk module can handle this device");
> +      goto error_no_close;
> +    }
>  
> -    ret = cr->recover_key (source, dev, cargs);
> -    if (ret != GRUB_ERR_NONE)
> -      goto error;
> +  if (cargs->protectors)
> +    {
> +      for (i = 0; cargs->protectors[i]; i++)
> +        {
> +          if (cargs->key_cache[i].invalid)
> +            continue;
> +
> +          if (!cargs->key_cache[i].key)
> +            {
> +              ret = grub_key_protector_recover_key (cargs->protectors[i],
> +                                                    &cargs->key_cache[i].key,
> +                                                    
> &cargs->key_cache[i].key_len);
> +              if (ret)
> +                {
> +                  if (grub_errno)
> +                    {
> +                      grub_print_error ();
> +                      grub_errno = GRUB_ERR_NONE;
> +                    }
> +
> +                  grub_dprintf ("cryptodisk",
> +                                "failed to recover a key from key protector "
> +                                "%s, will not try it again for any other "
> +                                "disks, if any, during this invocation of "
> +                                "cryptomount\n",
> +                                cargs->protectors[i]);
> +
> +                  cargs->key_cache[i].invalid = 1;
> +                  continue;
> +                }
> +            }
> +
> +          cargs->key_data = cargs->key_cache[i].key;
> +          cargs->key_len = cargs->key_cache[i].key_len;
> +
> +          ret = crd->recover_key (source, dev, cargs);
> +          if (ret)
> +            {
> +              part = grub_partition_get_name (source->partition);
> +              grub_dprintf ("cryptodisk",
> +                            "recovered a key from key protector %s but it "
> +                            "failed to unlock %s%s%s (%s)\n",
> +                             cargs->protectors[i], source->name,
> +                             source->partition != NULL ? "," : "",
> +                             part != NULL ? part : N_("UNKNOWN"), dev->uuid);
> +               grub_free (part);
> +               continue;
> +            }
> +         else
> +           {
> +             ret = grub_cryptodisk_insert (dev, name, source);
> +             if (ret != GRUB_ERR_NONE)
> +               goto error;
> +             goto cleanup;
> +           }
> +        }
>  
> -    ret = grub_cryptodisk_insert (dev, name, source);
> -    if (ret != GRUB_ERR_NONE)
> +      part = grub_partition_get_name (source->partition);
> +      grub_error (GRUB_ERR_ACCESS_DENIED,
> +                  N_("no key protector provided a usable key for %s%s%s 
> (%s)"),
> +                  source->name, source->partition != NULL ? "," : "",
> +                  part != NULL ? part : N_("UNKNOWN"), dev->uuid);
> +      grub_free (part);
>        goto error;
> +    }
> +
> +  if (!cargs->key_len)
> +    {
> +      /* Get the passphrase from the user, if no key data. */
> +      askpass = 1;
> +      part = grub_partition_get_name (source->partition);
> +      grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> +                    source->partition != NULL ? "," : "",
> +                    part != NULL ? part : N_("UNKNOWN"), dev->uuid);
> +      grub_free (part);
> +
> +      cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> +      if (cargs->key_data == NULL)
> +        goto error;
> +
> +      if (!grub_password_get ((char *) cargs->key_data, 
> GRUB_CRYPTODISK_MAX_PASSPHRASE))
> +        {
> +          grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
> +          goto error;
> +        }
> +      cargs->key_len = grub_strlen ((char *) cargs->key_data);
> +    }
> +
> +  ret = crd->recover_key (source, dev, cargs);
> +  if (ret != GRUB_ERR_NONE)
> +    goto error;
> +
> +  ret = grub_cryptodisk_insert (dev, name, source);
> +  if (ret != GRUB_ERR_NONE)
> +    goto error;
>  
> -    goto cleanup;
> -  }
> -  grub_error (GRUB_ERR_BAD_MODULE, "no cryptodisk module can handle this 
> device");
>    goto cleanup;
>  
>   error:
> @@ -1258,6 +1334,20 @@ grub_cryptodisk_scan_device (const char *name,
>    return ret;
>  }
>  
> +static void
> +grub_cryptodisk_clear_key_cache (struct grub_cryptomount_args *cargs)
> +{
> +  int i;
> +
> +  if (!cargs->key_cache)
> +    return;
> +
> +  for (i = 0; cargs->protectors[i]; i++)
> +    grub_free (cargs->key_cache[i].key);
> +
> +  grub_free (cargs->key_cache);
> +}
> +
>  static grub_err_t
>  grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>  {
> @@ -1270,6 +1360,10 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>    if (grub_cryptodisk_list == NULL)
>      return grub_error (GRUB_ERR_BAD_MODULE, "no cryptodisk modules loaded");
>  
> +  if (state[OPTION_PASSWORD].set && state[OPTION_PROTECTOR].set) /* password 
> and key protector */
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       "a password and a key protector cannot both be set");
> +
>    if (state[OPTION_PASSWORD].set) /* password */
>      {
>        cargs.key_data = (grub_uint8_t *) state[OPTION_PASSWORD].arg;
> @@ -1362,6 +1456,15 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>       return grub_errno;
>      }
>  
> +  if (state[OPTION_PROTECTOR].set) /* key protector(s) */
> +    {
> +      cargs.key_cache = grub_zalloc (state[OPTION_PROTECTOR].set * sizeof 
> (*cargs.key_cache));
> +      if (!cargs.key_cache)
> +        return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> +                           "no memory for key protector key cache");
> +      cargs.protectors = state[OPTION_PROTECTOR].args;
> +    }
> +
>    if (state[OPTION_UUID].set) /* uuid */
>      {
>        int found_uuid;
> @@ -1370,6 +1473,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>        dev = grub_cryptodisk_get_by_uuid (args[0]);
>        if (dev)
>       {
> +          grub_cryptodisk_clear_key_cache (&cargs);
>         grub_dprintf ("cryptodisk",
>                       "already mounted as crypto%lu\n", dev->id);
>         return GRUB_ERR_NONE;
> @@ -1378,6 +1482,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>        cargs.check_boot = state[OPTION_BOOT].set;
>        cargs.search_uuid = args[0];
>        found_uuid = grub_device_iterate (&grub_cryptodisk_scan_device, 
> &cargs);
> +      grub_cryptodisk_clear_key_cache (&cargs);
>  
>        if (found_uuid)
>       return GRUB_ERR_NONE;
> @@ -1397,6 +1502,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>      {
>        cargs.check_boot = state[OPTION_BOOT].set;
>        grub_device_iterate (&grub_cryptodisk_scan_device, &cargs);
> +      grub_cryptodisk_clear_key_cache (&cargs);
>        return GRUB_ERR_NONE;
>      }
>    else
> @@ -1420,6 +1526,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>        disk = grub_disk_open (diskname);
>        if (!disk)
>       {
> +          grub_cryptodisk_clear_key_cache (&cargs);
>         if (disklast)
>           *disklast = ')';
>         return grub_errno;
> @@ -1430,12 +1537,14 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>       {
>         grub_dprintf ("cryptodisk", "already mounted as crypto%lu\n", 
> dev->id);
>         grub_disk_close (disk);
> +          grub_cryptodisk_clear_key_cache (&cargs);
>         if (disklast)
>           *disklast = ')';
>         return GRUB_ERR_NONE;
>       }
>  
>        dev = grub_cryptodisk_scan_device_real (diskname, disk, &cargs);
> +      grub_cryptodisk_clear_key_cache (&cargs);
>  
>        grub_disk_close (disk);
>        if (disklast)
> @@ -1576,7 +1685,7 @@ GRUB_MOD_INIT (cryptodisk)
>    cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
>                             N_("[ [-p password] | [-k keyfile"
>                                " [-O keyoffset] [-S keysize] ] ] [-H file]"
> -                              " <SOURCE|-u UUID|-a|-b>"),
> +                              " [-k protector [-k protector ...]] <SOURCE|-u 
> UUID|-a|-b>"),
>                             N_("Mount a crypto device."), options);
>    grub_procfs_register ("luks_script", &luks_script);
>  }
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index d94df68b6..ce18df883 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -70,6 +70,16 @@ typedef gcry_err_code_t
>  (*grub_cryptodisk_rekey_func_t) (struct grub_cryptodisk *dev,
>                                grub_uint64_t zoneno);
>  
> +struct grub_cryptomount_cached_key
> +{
> +  grub_uint8_t *key;
> +  grub_size_t key_len;
> +
> +  /* The key protector associated with this cache entry failed, so avoid it
> +   * even if the cached entry (an instance of this structure) is empty. */
> +  int invalid;
> +};
> +
>  struct grub_cryptomount_args
>  {
>    /* scan: Flag to indicate that only bootable volumes should be decrypted */
> @@ -81,6 +91,10 @@ struct grub_cryptomount_args
>    /* recover_key: Length of key_data */
>    grub_size_t key_len;
>    grub_file_t hdr_file;
> +  /* recover_key: Names of the key protectors to use (NULL-terminated) */
> +  char **protectors;
> +  /* recover_key: Key cache to avoid invoking the same key protector twice */
> +  struct grub_cryptomount_cached_key *key_cache;
>  };
>  typedef struct grub_cryptomount_args *grub_cryptomount_args_t;
>  
> -- 
> 2.35.3
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel