[PATCH 0/3] Add ability to use SEV provisioned secrets for disk decryption

[PATCH 0/3] Add ability to use SEV provisioned secrets for disk decryption

[James Bottomley]

To achieve encrypted disk images in the AMD SEV encrypted virtual
machine, we need to add the ability for grub to retrieve the disk
passphrase from the SEV launch secret.  To do this, we've modified
OVMF to set aside an area for the injected secret and pass up a
configuration table for it:

https://edk2.groups.io/g/devel/topic/78198617#67339

The patches in this series modify grub to look for the disk passphrase
in the secret configuration table and use it to decrypt any disks in
the system if they are found.  This is so an encrypted image with a
properly injected password will boot without any user intervention.

The three patches firstly modify the cryptodisk consumers to allow
arbitrary password getters instead of the current console based one.
The next patch adds a '-s' option to cryptodisk to allow it to use a
saved password and the final one adds a sevsecret command to check for
the secrets configuration table and provision the disk passphrase from
it if an entry is found.  With all this in place, the sequence to boot
an encrypted volume without user intervention is:

sevsecret
cryptomount -s
source (crypto0)/boot/grub.cfg

Assuming there's a standard Linux root partition.

James

---

James Bottomley (3):
  cryptodisk: make the password getter and additional argument to
    recover_key
  cryptodisk: add OS provided secret support
  efi: Add API for retrieving the AMD SEV injected secret for cryptodisk

 grub-core/Makefile.core.def    |   8 +++
 grub-core/disk/cryptodisk.c    |  60 +++++++++++++++--
 grub-core/disk/efi/sevsecret.c | 118 +++++++++++++++++++++++++++++++++
 grub-core/disk/geli.c          |   5 +-
 grub-core/disk/luks.c          |  12 ++--
 grub-core/disk/luks2.c         |  12 ++--
 include/grub/cryptodisk.h      |   8 ++-
 include/grub/efi/api.h         |  15 +++++
 8 files changed, 221 insertions(+), 17 deletions(-)
 create mode 100644 grub-core/disk/efi/sevsecret.c

-- 
2.26.2

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

[James Bottomley]

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);
-  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;
 
-- 
2.26.2

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;
>  

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

[James Bottomley]

On Fri, 2020-11-13 at 00:02 -0600, Glenn Washburn wrote:
[...]
> 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.

Yes, I can add that.

James

[PATCH 2/3] cryptodisk: add OS provided secret support

[James Bottomley]

Make use of the new OS provided secrets API so that if the new '-s'
option is passed in we try to extract the secret from the API rather
than prompting for it.

The primary consumer of this is AMD SEV, which has been programmed to
provide an injectable secret to the encrypted virtual machine.  OVMF
provides the secret area and passes it into the EFI Configuration
Tables.  The grub EFI layer pulls the secret out and primes the
secrets API with it.  The upshot of all of this is that a SEV
protected VM can do an encrypted boot with a protected boot secret.

Signed-off-by: James Bottomley <jejb@linux.ibm.com>
---
 grub-core/disk/cryptodisk.c | 60 ++++++++++++++++++++++++++++++++++---
 include/grub/cryptodisk.h   |  2 ++
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 682f5a55d..02104aad4 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},
+    {"secret", 's', 0, N_("Get OS provisioned secret and mount all volumes encrypted with that secret"), 0, 0},
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -967,6 +968,10 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
 
 static int check_boot, have_it;
 static char *search_uuid;
+static char *os_passwd;
+
+/* variable to hold the passed in secret area. */
+static char *os_secret_area;
 
 static void
 cryptodisk_close (grub_cryptodisk_t dev)
@@ -977,6 +982,17 @@ cryptodisk_close (grub_cryptodisk_t dev)
   grub_free (dev);
 }
 
+static int
+os_password_get(char buf[], unsigned len)
+{
+  /* os_passwd should be null terminated, so just copy everything */
+  grub_strncpy(buf, os_passwd, len);
+  /* and add a terminator just in case */
+  buf[len - 1] = 0;
+
+  return 1;
+}
+
 static grub_err_t
 grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
 {
@@ -996,8 +1012,17 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
       return grub_errno;
     if (!dev)
       continue;
-    
-    err = cr->recover_key (source, dev, grub_password_get);
+
+    if (os_passwd)
+      {
+	err = cr->recover_key (source, dev, os_password_get);
+	if (err)
+	  /* if the key doesn't work ignore the access denied error */
+	  grub_error_pop();
+      }
+    else
+      err = cr->recover_key (source, dev, grub_password_get);
+
     if (err)
     {
       cryptodisk_close (dev);
@@ -1013,6 +1038,14 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
   return GRUB_ERR_NONE;
 }
 
+grub_err_t
+grub_cryptodisk_set_secret (char *secret)
+{
+  os_secret_area = secret;
+
+  return GRUB_ERR_NONE;
+}
+
 #ifdef GRUB_UTIL
 #include <grub/util/misc.h>
 grub_err_t
@@ -1089,7 +1122,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
 {
   struct grub_arg_list *state = ctxt->state;
 
-  if (argc < 1 && !state[1].set && !state[2].set)
+  if (argc < 1 && !state[1].set && !state[2].set && !state[3].set)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
 
   have_it = 0;
@@ -1107,6 +1140,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
 
       check_boot = state[2].set;
       search_uuid = args[0];
+      os_passwd = NULL;
       grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
       search_uuid = NULL;
 
@@ -1117,11 +1151,28 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
   else if (state[1].set || (argc == 0 && state[2].set))
     {
       search_uuid = NULL;
+      os_passwd = NULL;
       check_boot = state[2].set;
       grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
       search_uuid = NULL;
       return GRUB_ERR_NONE;
     }
+  else if (state[3].set)
+    {
+      /* do we have a secret? */
+      if (os_secret_area == NULL)
+	return grub_error (GRUB_ERR_INVALID_COMMAND, "No OS secret is provisioned");
+
+      os_passwd = os_secret_area;
+      search_uuid = NULL;
+      grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
+      os_passwd = NULL;
+
+      if (!have_it)
+	return grub_error (GRUB_ERR_ACCESS_DENIED, "SEV password failed to unlock any volumes");
+
+      return GRUB_ERR_NONE;
+    }
   else
     {
       grub_err_t err;
@@ -1132,6 +1183,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
       grub_size_t len;
 
       search_uuid = NULL;
+      os_passwd = NULL;
       check_boot = state[2].set;
       diskname = args[0];
       len = grub_strlen (diskname);
@@ -1299,7 +1351,7 @@ GRUB_MOD_INIT (cryptodisk)
 {
   grub_disk_dev_register (&grub_cryptodisk_dev);
   cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
-			      N_("SOURCE|-u UUID|-a|-b"),
+			      N_("SOURCE|-u UUID|-a|-b|-s"),
 			      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 45dae5483..55c411754 100644
--- a/include/grub/cryptodisk.h
+++ b/include/grub/cryptodisk.h
@@ -163,4 +163,6 @@ grub_util_get_geli_uuid (const char *dev);
 grub_cryptodisk_t grub_cryptodisk_get_by_uuid (const char *uuid);
 grub_cryptodisk_t grub_cryptodisk_get_by_source_disk (grub_disk_t disk);
 
+grub_err_t grub_cryptodisk_set_secret(char *secret);
+
 #endif
-- 
2.26.2

Re: [PATCH 2/3] cryptodisk: add OS provided secret support

[Dr. David Alan Gilbert]

* James Bottomley (jejb@linux.ibm.com) wrote:
> Make use of the new OS provided secrets API so that if the new '-s'
> option is passed in we try to extract the secret from the API rather
> than prompting for it.
> 
> The primary consumer of this is AMD SEV, which has been programmed to
> provide an injectable secret to the encrypted virtual machine.  OVMF
> provides the secret area and passes it into the EFI Configuration
> Tables.  The grub EFI layer pulls the secret out and primes the
> secrets API with it.  The upshot of all of this is that a SEV
> protected VM can do an encrypted boot with a protected boot secret.
> 
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  grub-core/disk/cryptodisk.c | 60 ++++++++++++++++++++++++++++++++++---
>  include/grub/cryptodisk.h   |  2 ++
>  2 files changed, 58 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 682f5a55d..02104aad4 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},
> +    {"secret", 's', 0, N_("Get OS provisioned secret and mount all volumes encrypted with that secret"), 0, 0},
>      {0, 0, 0, 0, 0, 0}
>    };
>  
> @@ -967,6 +968,10 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
>  
>  static int check_boot, have_it;
>  static char *search_uuid;
> +static char *os_passwd;
> +
> +/* variable to hold the passed in secret area. */
> +static char *os_secret_area;
>  
>  static void
>  cryptodisk_close (grub_cryptodisk_t dev)
> @@ -977,6 +982,17 @@ cryptodisk_close (grub_cryptodisk_t dev)
>    grub_free (dev);
>  }
>  
> +static int
> +os_password_get(char buf[], unsigned len)
> +{
> +  /* os_passwd should be null terminated, so just copy everything */
> +  grub_strncpy(buf, os_passwd, len);
> +  /* and add a terminator just in case */
> +  buf[len - 1] = 0;
> +
> +  return 1;
> +}
> +
>  static grub_err_t
>  grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
>  {
> @@ -996,8 +1012,17 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
>        return grub_errno;
>      if (!dev)
>        continue;
> -    
> -    err = cr->recover_key (source, dev, grub_password_get);
> +
> +    if (os_passwd)
> +      {
> +	err = cr->recover_key (source, dev, os_password_get);
> +	if (err)
> +	  /* if the key doesn't work ignore the access denied error */
> +	  grub_error_pop();
> +      }
> +    else
> +      err = cr->recover_key (source, dev, grub_password_get);
> +
>      if (err)
>      {
>        cryptodisk_close (dev);
> @@ -1013,6 +1038,14 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
>    return GRUB_ERR_NONE;
>  }
>  
> +grub_err_t
> +grub_cryptodisk_set_secret (char *secret)
> +{
> +  os_secret_area = secret;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
>  #ifdef GRUB_UTIL
>  #include <grub/util/misc.h>
>  grub_err_t
> @@ -1089,7 +1122,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>  {
>    struct grub_arg_list *state = ctxt->state;
>  
> -  if (argc < 1 && !state[1].set && !state[2].set)
> +  if (argc < 1 && !state[1].set && !state[2].set && !state[3].set)
>      return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
>  
>    have_it = 0;
> @@ -1107,6 +1140,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>  
>        check_boot = state[2].set;
>        search_uuid = args[0];
> +      os_passwd = NULL;
>        grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
>        search_uuid = NULL;
>  
> @@ -1117,11 +1151,28 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>    else if (state[1].set || (argc == 0 && state[2].set))
>      {
>        search_uuid = NULL;
> +      os_passwd = NULL;
>        check_boot = state[2].set;
>        grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
>        search_uuid = NULL;
>        return GRUB_ERR_NONE;
>      }
> +  else if (state[3].set)
> +    {
> +      /* do we have a secret? */
> +      if (os_secret_area == NULL)
> +	return grub_error (GRUB_ERR_INVALID_COMMAND, "No OS secret is provisioned");
> +
> +      os_passwd = os_secret_area;
> +      search_uuid = NULL;
> +      grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> +      os_passwd = NULL;
> +
> +      if (!have_it)
> +	return grub_error (GRUB_ERR_ACCESS_DENIED, "SEV password failed to unlock any volumes");

That's the only place you break the generality and admit to it being a
SEV password I think.

Dave

> +      return GRUB_ERR_NONE;
> +    }
>    else
>      {
>        grub_err_t err;
> @@ -1132,6 +1183,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
>        grub_size_t len;
>  
>        search_uuid = NULL;
> +      os_passwd = NULL;
>        check_boot = state[2].set;
>        diskname = args[0];
>        len = grub_strlen (diskname);
> @@ -1299,7 +1351,7 @@ GRUB_MOD_INIT (cryptodisk)
>  {
>    grub_disk_dev_register (&grub_cryptodisk_dev);
>    cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
> -			      N_("SOURCE|-u UUID|-a|-b"),
> +			      N_("SOURCE|-u UUID|-a|-b|-s"),
>  			      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 45dae5483..55c411754 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -163,4 +163,6 @@ grub_util_get_geli_uuid (const char *dev);
>  grub_cryptodisk_t grub_cryptodisk_get_by_uuid (const char *uuid);
>  grub_cryptodisk_t grub_cryptodisk_get_by_source_disk (grub_disk_t disk);
>  
> +grub_err_t grub_cryptodisk_set_secret(char *secret);
> +
>  #endif
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 2/3] cryptodisk: add OS provided secret support

[James Bottomley]

On Fri, 2020-11-13 at 13:23 +0000, Dr. David Alan Gilbert wrote:
> * James Bottomley (jejb@linux.ibm.com) wrote:
[...]
> > @@ -1117,11 +1151,28 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args)
> >    else if (state[1].set || (argc == 0 && state[2].set))
> >      {
> >        search_uuid = NULL;
> > +      os_passwd = NULL;
> >        check_boot = state[2].set;
> >        grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> >        search_uuid = NULL;
> >        return GRUB_ERR_NONE;
> >      }
> > +  else if (state[3].set)
> > +    {
> > +      /* do we have a secret? */
> > +      if (os_secret_area == NULL)
> > +	return grub_error (GRUB_ERR_INVALID_COMMAND, "No OS secret is
> > provisioned");
> > +
> > +      os_passwd = os_secret_area;
> > +      search_uuid = NULL;
> > +      grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> > +      os_passwd = NULL;
> > +
> > +      if (!have_it)
> > +	return grub_error (GRUB_ERR_ACCESS_DENIED, "SEV password failed
> > to unlock any volumes");
> 
> That's the only place you break the generality and admit to it being
> a SEV password I think.

Yes, I think it reflects the fact that there's likely a missing
callback into the secret setting module so the message can be very
specific and also that module can handle the secret destruction policy.

James

[PATCH 3/3] efi: Add API for retrieving the AMD SEV injected secret for cryptodisk

[James Bottomley]

This module is designed to provide a sevsecret command which
interrogates the EFI configuration table to find the location of the
sev secret injection and tries to register the secret with the
cryptodisk.

The secret is stored in an injection area, usually a page in size.  The layout of the secret injection area is a header

|GRUB_EFI_SEVSECRET_TABLE_HEADER_GUID|len|

with entries of the form

|guid|len|data|

the guid corresponding to the disk encryption passphrase is
GRUB_EFI_DISKPASSWD_GUID and data must be a zero terminated string.
To get a high entropy string that doesn't need large numbers of
iterations, use a base64 encoding of 33 bytes of random data.

Signed-off-by: James Bottomley <jejb@linux.ibm.com>
---
 grub-core/Makefile.core.def    |   8 +++
 grub-core/disk/efi/sevsecret.c | 118 +++++++++++++++++++++++++++++++++
 include/grub/efi/api.h         |  15 +++++
 3 files changed, 141 insertions(+)
 create mode 100644 grub-core/disk/efi/sevsecret.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index b5f47fc41..76ae5d8fc 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -784,6 +784,14 @@ module = {
   enable = efi;
 };
 
+module = {
+  name = sevsecret;
+
+  common = disk/efi/sevsecret.c;
+
+  enable = efi;
+};
+
 module = {
   name = lsefimmap;
 
diff --git a/grub-core/disk/efi/sevsecret.c b/grub-core/disk/efi/sevsecret.c
new file mode 100644
index 000000000..46f1744bb
--- /dev/null
+++ b/grub-core/disk/efi/sevsecret.c
@@ -0,0 +1,118 @@
+#include <grub/err.h>
+#include <grub/command.h>
+#include <grub/misc.h>
+#include <grub/cryptodisk.h>
+#include <grub/efi/efi.h>
+#include <grub/efi/api.h>
+#include <grub/dl.h>
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+static grub_efi_packed_guid_t sevsecret_guid = GRUB_EFI_SEVSECRET_TABLE_GUID;
+static grub_efi_packed_guid_t tableheader_guid = GRUB_EFI_SEVSECRET_TABLE_HEADER_GUID;
+static grub_efi_packed_guid_t diskpasswd_guid = GRUB_EFI_DISKPASSWD_GUID;
+
+/*
+ * EFI places the secret in the lower 4GB, so it uses a UINT32
+ * for the pointer which we have to transform to the correct type
+ */
+struct sev_secret {
+  grub_uint32_t base;
+  grub_uint32_t size;
+};
+
+struct secret_header {
+  grub_efi_packed_guid_t guid;
+  grub_uint32_t len;
+};
+
+struct secret_entry {
+  grub_efi_packed_guid_t guid;
+  grub_uint32_t len;
+  char data[0];
+};
+
+static grub_err_t
+grub_efi_sevsecret_find (struct sev_secret *s)
+{
+  int len;
+  struct secret_header *h;
+  struct secret_entry *e;
+  unsigned char *ptr = (unsigned char *)(unsigned long)s->base;
+
+  /* the area must be big enough for a guid and a u32 length */
+  if (s->size < sizeof (*h))
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "SEV secret area is too small");
+
+  h = (struct secret_header *)ptr;
+  if (grub_memcmp(&h->guid, &tableheader_guid, sizeof (h->guid)))
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "SEV secret area does not start with correct guid\n");
+  if (h->len < sizeof (*h))
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, "SEV secret area is too small\n");
+
+  len = h->len - sizeof (*h);
+  ptr += sizeof (*h);
+
+  while (len >= (int)sizeof (*e)) {
+    e = (struct secret_entry *)ptr;
+    if (e->len < sizeof(*e) || e->len > (unsigned int)len)
+      return grub_error (GRUB_ERR_BAD_ARGUMENT, "SEV secret area is corrupt\n");
+
+    if (! grub_memcmp (&e->guid, &diskpasswd_guid, sizeof (e->guid))) {
+      int end = e->len - sizeof(*e);
+
+      /*
+       * the passphrase must be a zero terminated string because the
+       * password routines call grub_strlen () to find its size
+       */
+      if (e->data[end - 1] != '\0')
+	return grub_error (GRUB_ERR_BAD_ARGUMENT, "SEV secret area disk encryption password is not zero terminated\n");
+
+      return grub_cryptodisk_set_secret (e->data);
+    }
+    ptr += e->len;
+    len -= e->len;
+  }
+  return grub_error (GRUB_ERR_BAD_ARGUMENT, "SEV secret aread does not contain disk decryption password\n");
+}
+
+static grub_err_t
+grub_efi_sevsecret_init (void)
+{
+  unsigned int i;
+
+  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
+    {
+      grub_efi_packed_guid_t *guid =
+	&grub_efi_system_table->configuration_table[i].vendor_guid;
+
+      if (! grub_memcmp (guid, &sevsecret_guid, sizeof (grub_efi_packed_guid_t))) {
+	struct sev_secret *s =
+	  grub_efi_system_table->configuration_table[i].vendor_table;
+
+	return grub_efi_sevsecret_find(s);
+      }
+    }
+  return grub_error (GRUB_ERR_BAD_ARGUMENT, "No SEV secret found in the EFI configuration table");
+}
+
+static grub_err_t
+grub_cmd_sevsecret (grub_command_t cmd __attribute__ ((unused)),
+		    int argc __attribute__ ((unused)),
+		    char **args __attribute__ ((unused)))
+{
+  return grub_efi_sevsecret_init();
+}
+
+static grub_command_t cmd;
+
+GRUB_MOD_INIT(sevsecret)
+{
+  cmd = grub_register_command ("sevsecret", grub_cmd_sevsecret, 0,
+			       N_("Register a SEV secret with cryptomount if one exists"));
+}
+
+GRUB_MOD_FINI(sevsecret)
+{
+  grub_unregister_command (cmd);
+}
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 1dcaa12f5..80eb880f8 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -299,6 +299,21 @@
     { 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d } \
   }
 
+#define GRUB_EFI_SEVSECRET_TABLE_GUID \
+  { 0xadf956ad, 0xe98c, 0x484c, \
+    { 0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47} \
+  }
+
+#define GRUB_EFI_SEVSECRET_TABLE_HEADER_GUID \
+  { 0x1e74f542, 0x71dd, 0x4d66, \
+    { 0x96, 0x3e, 0xef, 0x42, 0x87, 0xff, 0x17, 0x3b } \
+  }
+
+#define GRUB_EFI_DISKPASSWD_GUID \
+  { 0x736869e5, 0x84f0, 0x4973, \
+    { 0x92, 0xec, 0x06, 0x87, 0x9c, 0xe3, 0xda, 0x0b } \
+  }
+
 #define GRUB_EFI_ACPI_TABLE_GUID	\
   { 0xeb9d2d30, 0x2d88, 0x11d3, \
     { 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d } \
-- 
2.26.2

Re: [PATCH 0/3] Add ability to use SEV provisioned secrets for disk decryption

[Dr. David Alan Gilbert]

* James Bottomley (jejb@linux.ibm.com) wrote:
> To achieve encrypted disk images in the AMD SEV encrypted virtual
> machine, we need to add the ability for grub to retrieve the disk
> passphrase from the SEV launch secret.  To do this, we've modified
> OVMF to set aside an area for the injected secret and pass up a
> configuration table for it:
> 
> https://edk2.groups.io/g/devel/topic/78198617#67339
> 
> The patches in this series modify grub to look for the disk passphrase
> in the secret configuration table and use it to decrypt any disks in
> the system if they are found.  This is so an encrypted image with a
> properly injected password will boot without any user intervention.
> 
> The three patches firstly modify the cryptodisk consumers to allow
> arbitrary password getters instead of the current console based one.
> The next patch adds a '-s' option to cryptodisk to allow it to use a
> saved password and the final one adds a sevsecret command to check for
> the secrets configuration table and provision the disk passphrase from
> it if an entry is found.  With all this in place, the sequence to boot
> an encrypted volume without user intervention is:
> 
> sevsecret
> cryptomount -s
> source (crypto0)/boot/grub.cfg

I was thinking what happens if the evil admin adds an extra disc; I
guess the argument here is that:
  a) Since you specify (crypto0) it can only be a decrypted disc
  b) And since only the guest owner can supply the keys, it can only be
there disc image that can be decrypted.

Right?

Dave

> Assuming there's a standard Linux root partition.
> 
> James
> 
> ---
> 
> James Bottomley (3):
>   cryptodisk: make the password getter and additional argument to
>     recover_key
>   cryptodisk: add OS provided secret support
>   efi: Add API for retrieving the AMD SEV injected secret for cryptodisk
> 
>  grub-core/Makefile.core.def    |   8 +++
>  grub-core/disk/cryptodisk.c    |  60 +++++++++++++++--
>  grub-core/disk/efi/sevsecret.c | 118 +++++++++++++++++++++++++++++++++
>  grub-core/disk/geli.c          |   5 +-
>  grub-core/disk/luks.c          |  12 ++--
>  grub-core/disk/luks2.c         |  12 ++--
>  include/grub/cryptodisk.h      |   8 ++-
>  include/grub/efi/api.h         |  15 +++++
>  8 files changed, 221 insertions(+), 17 deletions(-)
>  create mode 100644 grub-core/disk/efi/sevsecret.c
> 
> -- 
> 2.26.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 0/3] Add ability to use SEV provisioned secrets for disk decryption

[James Bottomley]

On Fri, 2020-11-13 at 17:50 +0000, Dr. David Alan Gilbert wrote:
> * James Bottomley (jejb@linux.ibm.com) wrote:
> > To achieve encrypted disk images in the AMD SEV encrypted virtual
> > machine, we need to add the ability for grub to retrieve the disk
> > passphrase from the SEV launch secret.  To do this, we've modified
> > OVMF to set aside an area for the injected secret and pass up a
> > configuration table for it:
> > 
> > https://edk2.groups.io/g/devel/topic/78198617#67339
> > 
> > The patches in this series modify grub to look for the disk
> > passphrase in the secret configuration table and use it to decrypt
> > any disks in the system if they are found.  This is so an encrypted
> > image with a properly injected password will boot without any user
> > intervention.
> > 
> > The three patches firstly modify the cryptodisk consumers to allow
> > arbitrary password getters instead of the current console based
> > one. The next patch adds a '-s' option to cryptodisk to allow it to
> > use a saved password and the final one adds a sevsecret command to
> > check for the secrets configuration table and provision the disk
> > passphrase from it if an entry is found.  With all this in place,
> > the sequence to boot an encrypted volume without user intervention
> > is:
> > 
> > sevsecret
> > cryptomount -s
> > source (crypto0)/boot/grub.cfg
> 
> I was thinking what happens if the evil admin adds an extra disc; I
> guess the argument here is that:
>   a) Since you specify (crypto0) it can only be a decrypted disc
>   b) And since only the guest owner can supply the keys, it can only
> be there disc image that can be decrypted.
> 
> Right?

Right, cryptomount will mount as (cryptoN) only those devices which can
actually be decrypted by the key.  Since the initial grub.cfg is built
into the grub that executes from the firmware volume only someone who
knows the decryption key can substitute the booted volume.  If you
substitute an unencrypted volume, the grub.cfg script I constructed
simply errors out (because it can't find any encrypted volumes) and
reboots.  The script is more complicated than the simple illustration
above, but it's in this patch:

https://edk2.groups.io/g/devel/message/67341?p=,,,20,0,0,0::Created,,PATCH+2%2F4,20,2,0,78198619

James

Re: [PATCH 0/3] Add ability to use SEV provisioned secrets for disk decryption

[Dr. David Alan Gilbert]

* James Bottomley (jejb@linux.ibm.com) wrote:
> On Fri, 2020-11-13 at 17:50 +0000, Dr. David Alan Gilbert wrote:
> > * James Bottomley (jejb@linux.ibm.com) wrote:
> > > To achieve encrypted disk images in the AMD SEV encrypted virtual
> > > machine, we need to add the ability for grub to retrieve the disk
> > > passphrase from the SEV launch secret.  To do this, we've modified
> > > OVMF to set aside an area for the injected secret and pass up a
> > > configuration table for it:
> > > 
> > > https://edk2.groups.io/g/devel/topic/78198617#67339
> > > 
> > > The patches in this series modify grub to look for the disk
> > > passphrase in the secret configuration table and use it to decrypt
> > > any disks in the system if they are found.  This is so an encrypted
> > > image with a properly injected password will boot without any user
> > > intervention.
> > > 
> > > The three patches firstly modify the cryptodisk consumers to allow
> > > arbitrary password getters instead of the current console based
> > > one. The next patch adds a '-s' option to cryptodisk to allow it to
> > > use a saved password and the final one adds a sevsecret command to
> > > check for the secrets configuration table and provision the disk
> > > passphrase from it if an entry is found.  With all this in place,
> > > the sequence to boot an encrypted volume without user intervention
> > > is:
> > > 
> > > sevsecret
> > > cryptomount -s
> > > source (crypto0)/boot/grub.cfg
> > 
> > I was thinking what happens if the evil admin adds an extra disc; I
> > guess the argument here is that:
> >   a) Since you specify (crypto0) it can only be a decrypted disc
> >   b) And since only the guest owner can supply the keys, it can only
> > be there disc image that can be decrypted.
> > 
> > Right?
> 
> Right, cryptomount will mount as (cryptoN) only those devices which can
> actually be decrypted by the key.  Since the initial grub.cfg is built
> into the grub that executes from the firmware volume only someone who
> knows the decryption key can substitute the booted volume.  If you
> substitute an unencrypted volume, the grub.cfg script I constructed
> simply errors out (because it can't find any encrypted volumes) and
> reboots.  The script is more complicated than the simple illustration
> above, but it's in this patch:
> 
> https://edk2.groups.io/g/devel/message/67341?p=,,,20,0,0,0::Created,,PATCH+2%2F4,20,2,0,78198619

Hmm:

+echo "Entering grub config"
+sevsecret
+if [ $? -ne 0 ]; then
+    echo "Failed to locate anything in the SEV secret area, prompting for =
password"
+    cryptomount -a
+else
+    cryptomount -s
+    if [ $? -ne 0 ]; then
+        echo "Failed to mount root securely, retrying with password prompt"
+        cryptomount -a
+    fi
+fi

if Eviladmin can make it fall down the cryptomount -a  paths with one
of their own discs attached they can decrypt that and boot, and then
if they can later inject the original secret, then mount the original
disc.
I think Brijesh said that the secret could be changed later; so perhaps
if the admin just stopped the secret being injected initially, or caused
the VM to start without waiting for the injection, that would happen?

Dave

> James
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 0/3] Add ability to use SEV provisioned secrets for disk decryption

[James Bottomley]

On Fri, 2020-11-13 at 18:21 +0000, Dr. David Alan Gilbert wrote:
> * James Bottomley (jejb@linux.ibm.com) wrote:
> > On Fri, 2020-11-13 at 17:50 +0000, Dr. David Alan Gilbert wrote:
> > > * James Bottomley (jejb@linux.ibm.com) wrote:
> > > > To achieve encrypted disk images in the AMD SEV encrypted
> > > > virtual
> > > > machine, we need to add the ability for grub to retrieve the
> > > > disk
> > > > passphrase from the SEV launch secret.  To do this, we've
> > > > modified
> > > > OVMF to set aside an area for the injected secret and pass up a
> > > > configuration table for it:
> > > > 
> > > > https://edk2.groups.io/g/devel/topic/78198617#67339
> > > > 
> > > > The patches in this series modify grub to look for the disk
> > > > passphrase in the secret configuration table and use it to
> > > > decrypt
> > > > any disks in the system if they are found.  This is so an
> > > > encrypted
> > > > image with a properly injected password will boot without any
> > > > user
> > > > intervention.
> > > > 
> > > > The three patches firstly modify the cryptodisk consumers to
> > > > allow
> > > > arbitrary password getters instead of the current console based
> > > > one. The next patch adds a '-s' option to cryptodisk to allow
> > > > it to
> > > > use a saved password and the final one adds a sevsecret command
> > > > to
> > > > check for the secrets configuration table and provision the
> > > > disk
> > > > passphrase from it if an entry is found.  With all this in
> > > > place,
> > > > the sequence to boot an encrypted volume without user
> > > > intervention
> > > > is:
> > > > 
> > > > sevsecret
> > > > cryptomount -s
> > > > source (crypto0)/boot/grub.cfg
> > > 
> > > I was thinking what happens if the evil admin adds an extra disc;
> > > I
> > > guess the argument here is that:
> > >   a) Since you specify (crypto0) it can only be a decrypted disc
> > >   b) And since only the guest owner can supply the keys, it can
> > > only
> > > be there disc image that can be decrypted.
> > > 
> > > Right?
> > 
> > Right, cryptomount will mount as (cryptoN) only those devices which
> > can
> > actually be decrypted by the key.  Since the initial grub.cfg is
> > built
> > into the grub that executes from the firmware volume only someone
> > who
> > knows the decryption key can substitute the booted volume.  If you
> > substitute an unencrypted volume, the grub.cfg script I constructed
> > simply errors out (because it can't find any encrypted volumes) and
> > reboots.  The script is more complicated than the simple
> > illustration
> > above, but it's in this patch:
> > 
> > https://edk2.groups.io/g/devel/message/67341?p=,,,20,0,0,0::Created,,PATCH+2%2F4,20,2,0,78198619
> 
> Hmm:
> 
> +echo "Entering grub config"
> +sevsecret
> +if [ $? -ne 0 ]; then
> +    echo "Failed to locate anything in the SEV secret area,
> prompting for =
> password"
> +    cryptomount -a
> +else
> +    cryptomount -s
> +    if [ $? -ne 0 ]; then
> +        echo "Failed to mount root securely, retrying with password
> prompt"
> +        cryptomount -a
> +    fi
> +fi
> 
> if Eviladmin can make it fall down the cryptomount -a  paths with one
> of their own discs attached they can decrypt that and boot, and then
> if they can later inject the original secret, then mount the original
> disc. I think Brijesh said that the secret could be changed later; so
> perhaps if the admin just stopped the secret being injected
> initially, or caused the VM to start without waiting for the
> injection, that would happen?

In the current scheme the secret is destroyed after cryptomount -s
fails, so you still wouldn't get access.  But the grub.cfg is designed
to be mutable ... if a distro wants a more secure sequence that doesn't
try a fallback, it can construct one.

James