Re: [PATCH 1/3] cryptodisk: make the password getter and additional argument to recover_key

[Glenn Washburn]

On Thu, 12 Nov 2020 17:22:04 -0800
James Bottomley <jejb@linux.ibm.com> wrote:

> For AMD SEV environments, the grub boot password has to be retrieved
> from a given memory location rather than prompted for.  This means
> that the standard password getter needs to be replaced with one that
> gets the passphrase from the SEV area and uses that instead.  Adding
> the password getter as a passed in argument to recover_key() makes
> this possible.
> 
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  grub-core/disk/cryptodisk.c |  2 +-
>  grub-core/disk/geli.c       |  5 +++--
>  grub-core/disk/luks.c       | 12 +++++++-----
>  grub-core/disk/luks2.c      | 12 +++++++-----
>  include/grub/cryptodisk.h   |  6 +++++-
>  5 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index a3d672f68..682f5a55d 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -997,7 +997,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, grub_password_get);
>      if (err)
>      {
>        cryptodisk_close (dev);
> diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
> index e9d23299a..5514c16a3 100644
> --- a/grub-core/disk/geli.c
> +++ b/grub-core/disk/geli.c
> @@ -398,7 +398,8 @@ configure_ciphers (grub_disk_t disk, const char
> *check_uuid, }
>  
>  static grub_err_t
> -recover_key (grub_disk_t source, grub_cryptodisk_t dev)
> +recover_key (grub_disk_t source, grub_cryptodisk_t dev,
> +          grub_passwd_cb *password_get)
>  {
>    grub_size_t keysize;
>    grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN];
> @@ -442,7 +443,7 @@ recover_key (grub_disk_t source,
> grub_cryptodisk_t dev) source->partition ? "," : "", tmp ? : "",
>               dev->uuid);
>    grub_free (tmp);

In luks.c and luks2.c below, grub_printf_ is made conditional, but not
here. It probably should be.

> -  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +  if (!password_get (passphrase, MAX_PASSPHRASE))
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> supplied"); 
>    /* Calculate the PBKDF2 of the user supplied passphrase.  */
> diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> index 59702067a..165f4a6bd 100644
> --- a/grub-core/disk/luks.c
> +++ b/grub-core/disk/luks.c
> @@ -152,7 +152,8 @@ configure_ciphers (grub_disk_t disk, const char
> *check_uuid, 
>  static grub_err_t
>  luks_recover_key (grub_disk_t source,
> -               grub_cryptodisk_t dev)
> +               grub_cryptodisk_t dev,
> +               grub_passwd_cb *password_get)
>  {
>    struct grub_luks_phdr header;
>    grub_size_t keysize;
> @@ -187,11 +188,12 @@ luks_recover_key (grub_disk_t source,
>    tmp = NULL;
>    if (source->partition)
>      tmp = grub_partition_get_name (source->partition);
> -  grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> source->name,
> -            source->partition ? "," : "", tmp ? : "",
> -            dev->uuid);
> +  if (password_get == grub_password_get)
> +       grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> source->name,
> +                     source->partition ? "," : "", tmp ? : "",
> +                     dev->uuid);
>    grub_free (tmp);
> -  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +  if (!password_get (passphrase, MAX_PASSPHRASE))
>      {
>        grub_free (split_key);
>        return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> supplied"); diff --git a/grub-core/disk/luks2.c
> b/grub-core/disk/luks2.c index 31d7166fc..984182aa9 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -531,7 +531,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
>  
>  static grub_err_t
>  luks2_recover_key (grub_disk_t disk,
> -                grub_cryptodisk_t crypt)
> +                grub_cryptodisk_t crypt,
> +                grub_passwd_cb *password_get)
>  {
>    grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
>    char passphrase[MAX_PASSPHRASE], cipher[32];
> @@ -573,10 +574,11 @@ luks2_recover_key (grub_disk_t disk,
>    /* Get the passphrase from the user. */
>    if (disk->partition)
>      part = grub_partition_get_name (disk->partition);
> -  grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
> -             disk->partition ? "," : "", part ? : "",
> -             crypt->uuid);
> -  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +  if (password_get == grub_password_get)
> +    grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> disk->name,
> +               disk->partition ? "," : "", part ? : "",
> +               crypt->uuid);
> +  if (!password_get (passphrase, MAX_PASSPHRASE))
>      {
>        ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> supplied"); goto err;
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index e1b21e785..45dae5483 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -101,6 +101,9 @@ struct grub_cryptodisk
>  };
>  typedef struct grub_cryptodisk *grub_cryptodisk_t;
>  
> +/* must match prototype for grub_password_get */
> +typedef int (grub_passwd_cb)(char buf[], unsigned buf_size);
> +
>  struct grub_cryptodisk_dev
>  {
>    struct grub_cryptodisk_dev *next;
> @@ -108,7 +111,8 @@ struct grub_cryptodisk_dev
>  
>    grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
> *check_uuid, int boot_only);
> -  grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> dev);
> +  grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
> +                          grub_passwd_cb *get_password);
>  };
>  typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
>