[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 0/3] Add ability to use SEV provisioned secrets for disk d
Re: [PATCH v2 0/3] Add ability to use SEV provisioned secrets for disk decryption
Fri, 13 Nov 2020 18:48:30 -0800
On Fri, 2020-11-13 at 19:50 -0600, Glenn Washburn wrote:
> On Fri, 13 Nov 2020 14:25:07 -0800
> James Bottomley <firstname.lastname@example.org> wrote:
> > v2: update geli.c to use conditional prompt and add callback for
> > variable message printing and secret destruction
> > 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.
> I like this idea in general.
> > 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.
> I'm not in favor of this approach. This feels like a special case of
> providing a key file to cryptomount. We have working (and I believe
> merge worthy) patches for adding key file support. Unfortunately, due
> to the current position in the grub development cycle, they have not
> been merged. As a side note, it might be interesting to re-work the
> key file patch series to use the arbitrary password getter mechanism
> you've created.
> What I would prefer, because it feels more generic, is to have the
> sevsecret module create a procfs entry (perhaps (proc)/sevsecret),
> which outputs the secret data when read (or NULL string if some error
> in finding the secret). Then to cryptomount all devices that accept
> the sev secret do:
> cryptomount -a -k (proc)/sevsecret
> In this case you could re-use most of the code in
> grub_efi_sevsecret_find and creating the procfs entry would be
> trivial (see bottom of cryptodisk.c for an example on how to do
A file interface feels slightly wrong for this. What we need is a
use/release envelope ... effectively a way of enforcing the lifetime on
the secret use. This allows a wider variety of threat models than the
simply file model because the latter assumes an infinite lifetime (even
though that can be hacked around using temporary filesystems). From a
generality point of view it feels better to implement a file interface
as a special case of a use/release model because it simply has an empty
If you follow this model, a file use case would have a special
filesecret command and the use would go like:
> One potential issue could be getting error messages from
> grub_efi_sevsecret_find back to the user and a solution could be to
> replace the grub_error with grub_dprintf("sev", ...) statements and
> set debug=sev unconditionally. In most cases no output would be
> generated, but some debug log messages should be generated on error.
Right, all this currently goes in the release function.
> Also, if this series does end up adding an option to cryptomount, a
> documentation patch should be added. I think we should start
> documenting procfs paths as well.
> Also, out of curiosity, is it possible that there are multiple
> GRUB_EFI_DISKPASSWD_GUID entries defined? You only get the first
> one, but I'm wondering if the spec allows for more.
Well, I'm currently defining the spec by proposing patches. I envisage
the secret area would contain multiple secrets, some of which may be
consumed before grub but a few may be consumed after grub by the OS. I
also suspect if the cryptomount fails, it might be advisable to destroy
the entire secrets area rather than just the grub passphrase, because
failure to decrypt the /boot partition would indicate potential tamper
Given the use case is an encrypted boot system on an untrusted cloud, I
can't see any reason for having multiple grub passwords ... the image
should only have a single /boot filesystem.