[libvirt PATCH v2 09/12] tools: support generating SEV secret injection tables

Wed Oct 26 12:34:00 UTC 2022

On 26/10/2022 12:59, Daniel P. Berrangé wrote:
> On Tue, Oct 25, 2022 at 07:38:43PM -0400, Cole Robinson wrote:
>> On 10/19/22 6:17 AM, Daniel P. Berrangé wrote:
>>> It is possible to build OVMF for SEV with an embedded Grub that can
>>> fetch LUKS disk secrets. This adds support for injecting secrets in
>>> the required format.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
>>> ---
>>
>>> diff --git a/tools/virt-qemu-sev-validate b/tools/virt-qemu-sev-validate
>>> index 5ce5763d5b..2d15edb933 100755
>>> --- a/tools/virt-qemu-sev-validate
>>> +++ b/tools/virt-qemu-sev-validate
>>> @@ -36,16 +36,19 @@
>>>  
>>>  import abc
>>>  import argparse
>>> -from base64 import b64decode
>>> +from base64 import b64decode, b64encode
>>>  from hashlib import sha256
>>>  import hmac
>>>  import logging
>>> +import os
>>>  import re
>>>  import socket
>>>  from struct import pack
>>>  import sys
>>>  import traceback
>>>  from uuid import UUID
>>> +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
>>> +
>>>  
>>>  from lxml import etree
>>>  import libvirt
>>> @@ -573,7 +576,26 @@ class KernelTable(GUIDTable):
>>>          return entries
>>>  
>>>  
>>> -class ConfidentialVM(object):
>>> +class SecretsTable(GUIDTable):
>>> +
>>> +    TABLE_GUID = UUID('{1e74f542-71dd-4d66-963e-ef4287ff173b}').bytes_le
>>> +    DISK_PW_GUID = UUID('{736869e5-84f0-4973-92ec-06879ce3da0b}').bytes_le
>>> +
>>> +    def __init__(self):
>>> +        super().__init__(guid=self.TABLE_GUID,
>>> +                         lenlen=4)
>>> +        self.disk_password = None
>>> +
>>> +    def load_disk_password(self, path):
>>> +        with open(path, 'rb') as fh:
>>> +            self.disk_password = fh.read()
>>> +
>>> +    def entries(self):
>>> +        return self.build_entry(self.DISK_PW_GUID,
>>> +                                self.disk_password + bytes([0]), 4)
>>> +
>>
>> This bytes([0]) NUL byte ends up in the efi_secret /sys path. Dropping
>> it doesn't seem to impact injecting the secret at all
> 
> The specs/sev-guest-firmware.rst files does not specify anything in
> particular about the injected secret format. The rules for the format
> will vary according to the GUID used for the entry. In this case what
> I've called the DISK_PW_GUID is the same as the GUID used in James
> Bottomley's grub patch:
> 
>   https://lists.gnu.org/archive/html/grub-devel/2020-12/msg00260.html
> 
> In particular note
> 
>   +      /*
>   +       * the passphrase must be a zero terminated string because the
>   +       * password routines call grub_strlen () to find its size
>   +       */
> 
> As written, the code just passes around a pointer to the data in the
> secret table, so it can't simply add a NUL terminator itself.
> 
> See also James' python demo script for injection which adds a NUL:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03691.html
> 
>> FWIW once that's dropped, getting automatic luks unlock is really simple
>> with /etc/crypttab + kernel 5.19
>>
>> sed -i -e "s| none |
>> /sys/kernel/security/secrets/coco/736869e5-84f0-4973-92ec-06879ce3da0b
>> |g" /etc/crypttab
>> dracut --force --add-drivers efi_secret
>> shutdown -r now

I'm not an initrd expert, but maybe there's a way to run an early script
(before cryptsetup) that will peel that terminating NUL:

  head -c -1 /sys/kernel/security/secrets/coco/736869e5-84f0-4973-92ec-06879ce3da0b > /tmp/luks_key

and then use /tmp/luks_key in /etc/crypttab.

Of course /tmp/luks_key isn't a secure place, so need to find the correct location for this.

> 
> Now one option here is to invent a different GUID and call it the
> "CRYTPTAB_PW_GUID", and have the host side injection tools not use
> a NUL terminator for this option.

and then modify the Grub patch (option 1 below) to read from the
new GUID, copy the value, and add the NUL byte.

> 
> What I don't like about this is that in both cases we're just passing
> in the same data - a passphrase for unlocking a LUKS keyslot.
> 
> IMHO, we should have a "LUKS_PASSPHARSE_GUID" secret entry which can
> be consumed by any software that wants to unlock LUKS, not have
> software specific GUIDs.
> 
> The crypttab keyfiles can contain arbitrary bytes, so we can't just
> drop the trailing NUL as that might be relevant key material.
> 

Note that grub and cryptsetup are not identical -- I think that (currently) the
grub patches do not support binary passphrases (that may include NUL bytes),
whereas cryptsetup keyfile can be have binary content.  But I'm not following
grub-devel closely.

> Reading cryttab(5) I see that /etc/crypttab supports an option
> called 'keyfile-size', which could be used to truncate the file.
> That's not at all user friendly for us, as the user pw data can be
> arbitrary length and when building the disk image template we
> don't know how many bytes are sensitive.IOW, we can't predict
> what keyfile-size to use.
> 
> So it seems we have a few options
> 
>  1. modify grub patches to remove the requirement for NUL
>     termination
>  2. modify systemd to add a 'keyfile-nul-terminated' option
>     to instruct it to stip a trailing NUL from key-file.
>  3. modify systemd to add a 'efi-secret=UUID' option and
>     ignore key-file field entirely

Debian's crypttab supports keyscript=/path/to/script which should output
the passphrase to stdout.  This is very generic and can allow us to:

1. check a few GUIDs (fallback)
2. drop terminating NUL
3. for SNP-style late attestation: fetch attestation report, contact the KBS,
   and retrieve the key (see AMD's example [1])

[1] https://github.com/AMDESE/sev-guest/tree/main/attestation/cryptsetup-initramfs

But there's no keyscript option in systemd...

> 
> Incidentally I think we should document well known secret GUIDs
> in specs/sev-guest-firmware.rst.

The only others I know are used in confidential-containers
attestation agent (offline_sev_kbc [2] and online_sev_kbc [3]).

[2] https://github.com/confidential-containers/attestation-agent/blob/6e4a249dfbf6fae4a0ec636ae2fac8c0aa368467/src/kbc_modules/offline_sev_kbc/mod.rs#L17
[3] https://github.com/confidential-containers/attestation-agent/blob/6e4a249dfbf6fae4a0ec636ae2fac8c0aa368467/src/kbc_modules/online_sev_kbc/mod.rs#L28

-Dov