On Fri, 2020-11-13 at 19:50 -0600, Glenn Washburn wrote: > On Fri, 13 Nov 2020 14:25:07 -0800 > James Bottomley <j...@linux.ibm.com> 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 > this).
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 release function. If you follow this model, a file use case would have a special filesecret command and the use would go like: filesecret <file> cryptomount -s > 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 attempts. 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. James _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel