mbox series

[v3,0/2] x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline

Message ID 20210624102040.2015280-1-dovmurik@linux.ibm.com (mailing list archive)
Headers show
Series x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline | expand

Message

Dov Murik June 24, 2021, 10:20 a.m. UTC
Currently booting with -kernel/-initrd/-append is not supported in SEV
confidential guests, because the content of these blobs is not measured
and therefore not trusted by the SEV guest.

However, in some cases the kernel, initrd, and cmdline are not secret
but should not be modified by the host.  In such a case, we want to
verify inside the trusted VM that the kernel, initrd, and cmdline are
indeed the ones expected by the Guest Owner, and only if that is the
case go on and boot them up (removing the need for grub inside OVMF in
that mode).

To support that, OVMF adds a special area for hashes of
kernel/initrd/cmdline; that area is expected to be filled by QEMU and
encrypted as part of the initial SEV guest launch.  This in turn makes
the hashes part of the PSP measured content, and OVMF can trust these
inputs if they match the hashes.

This series adds an SEV function to generate the table of hashes for
OVMF and encrypt it (patch 1/2), and calls this function if SEV is
enabled when the kernel/initrd/cmdline are prepared (patch 2/2).

Corresponding OVMF support was submitted to edk2-devel [1] (patch series
"Measured SEV boot with kernel/initrd/cmdline"); it's still under
review.

[1] https://edk2.groups.io/g/devel/topic/patch_v1_0_8_measured_sev/83074450

---

v3 changes:
 - initrd hash is now mandatory; if no -initrd is passed, calculate the
   hash of the empty buffer.  This is now aligned with the OVMF
   behaviour which verifies the empty initrd (correctly).
 - make SevHashTable entries fixed: 3 entries for cmdline, initrd, and kernel.
 - in sev_add_kernel_loader_hashes: first calculate all the hashes, only then
   fill-in the hashes table in the guest's memory.
 - Use g_assert_not_reached in sev-stub.c.
 - Use QEMU_PACKED attribute for structs.
 - Use QemuUUID type for guids.
 - in sev_add_kernel_loader_hashes: use ARRAY_SIZE(iov) instead of literal 2.

v2: https://lore.kernel.org/qemu-devel/20210621190553.1763020-1-dovmurik@linux.ibm.com/
v2 changes:
 - Extract main functionality to sev.c (with empty stub in sev-stub.c)
 - Use sev_enabled() instead of machine->cgs->ready to detect SEV guest
 - Coding style changes

v1: https://lore.kernel.org/qemu-devel/20210525065931.1628554-1-dovmurik@linux.ibm.com/

Dov Murik (2):
  sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux
    boot
  x86/sev: generate SEV kernel loader hashes in x86_load_linux

 target/i386/sev_i386.h |  12 ++++
 hw/i386/x86.c          |  25 +++++++-
 target/i386/sev-stub.c |   5 ++
 target/i386/sev.c      | 137 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 178 insertions(+), 1 deletion(-)


base-commit: b22726abdfa54592d6ad88f65b0297c0e8b363e2

Comments

Connor Kuehl July 8, 2021, 4:41 p.m. UTC | #1
Hi Paolo,

Please consider this series[1] for inclusion into your next pull request.

Just a note that this series has a companion series that is getting
upstreamed into OVMF[2]

[1] Patchwork link, if convenient: https://patchwork.kernel.org/project/qemu-devel/cover/20210624102040.2015280-1-dovmurik@linux.ibm.com/
[2] https://bugzilla.tianocore.org/show_bug.cgi?id=3457#c6

Thank you,

Connor

On 6/24/21 3:20 AM, Dov Murik wrote:
> Currently booting with -kernel/-initrd/-append is not supported in SEV
> confidential guests, because the content of these blobs is not measured
> and therefore not trusted by the SEV guest.
> 
> However, in some cases the kernel, initrd, and cmdline are not secret
> but should not be modified by the host.  In such a case, we want to
> verify inside the trusted VM that the kernel, initrd, and cmdline are
> indeed the ones expected by the Guest Owner, and only if that is the
> case go on and boot them up (removing the need for grub inside OVMF in
> that mode).
> 
> To support that, OVMF adds a special area for hashes of
> kernel/initrd/cmdline; that area is expected to be filled by QEMU and
> encrypted as part of the initial SEV guest launch.  This in turn makes
> the hashes part of the PSP measured content, and OVMF can trust these
> inputs if they match the hashes.
> 
> This series adds an SEV function to generate the table of hashes for
> OVMF and encrypt it (patch 1/2), and calls this function if SEV is
> enabled when the kernel/initrd/cmdline are prepared (patch 2/2).
> 
> Corresponding OVMF support was submitted to edk2-devel [1] (patch series
> "Measured SEV boot with kernel/initrd/cmdline"); it's still under
> review.
> 
> [1] https://edk2.groups.io/g/devel/topic/patch_v1_0_8_measured_sev/83074450
> 
> ---
> 
> v3 changes:
>  - initrd hash is now mandatory; if no -initrd is passed, calculate the
>    hash of the empty buffer.  This is now aligned with the OVMF
>    behaviour which verifies the empty initrd (correctly).
>  - make SevHashTable entries fixed: 3 entries for cmdline, initrd, and kernel.
>  - in sev_add_kernel_loader_hashes: first calculate all the hashes, only then
>    fill-in the hashes table in the guest's memory.
>  - Use g_assert_not_reached in sev-stub.c.
>  - Use QEMU_PACKED attribute for structs.
>  - Use QemuUUID type for guids.
>  - in sev_add_kernel_loader_hashes: use ARRAY_SIZE(iov) instead of literal 2.
> 
> v2: https://lore.kernel.org/qemu-devel/20210621190553.1763020-1-dovmurik@linux.ibm.com/
> v2 changes:
>  - Extract main functionality to sev.c (with empty stub in sev-stub.c)
>  - Use sev_enabled() instead of machine->cgs->ready to detect SEV guest
>  - Coding style changes
> 
> v1: https://lore.kernel.org/qemu-devel/20210525065931.1628554-1-dovmurik@linux.ibm.com/
> 
> Dov Murik (2):
>   sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux
>     boot
>   x86/sev: generate SEV kernel loader hashes in x86_load_linux
> 
>  target/i386/sev_i386.h |  12 ++++
>  hw/i386/x86.c          |  25 +++++++-
>  target/i386/sev-stub.c |   5 ++
>  target/i386/sev.c      | 137 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 178 insertions(+), 1 deletion(-)
> 
> 
> base-commit: b22726abdfa54592d6ad88f65b0297c0e8b363e2
>
Philippe Mathieu-Daudé July 8, 2021, 5:03 p.m. UTC | #2
On 7/8/21 6:41 PM, Connor Kuehl wrote:
> Hi Paolo,
> 
> Please consider this series[1] for inclusion into your next pull request.
> 
> Just a note that this series has a companion series that is getting
> upstreamed into OVMF[2]

Shouldn't we get the OVMF part merged first?

> 
> [1] Patchwork link, if convenient: https://patchwork.kernel.org/project/qemu-devel/cover/20210624102040.2015280-1-dovmurik@linux.ibm.com/
> [2] https://bugzilla.tianocore.org/show_bug.cgi?id=3457#c6
> 
> Thank you,
> 
> Connor
> 
> On 6/24/21 3:20 AM, Dov Murik wrote:
>> Currently booting with -kernel/-initrd/-append is not supported in SEV
>> confidential guests, because the content of these blobs is not measured
>> and therefore not trusted by the SEV guest.
>>
>> However, in some cases the kernel, initrd, and cmdline are not secret
>> but should not be modified by the host.  In such a case, we want to
>> verify inside the trusted VM that the kernel, initrd, and cmdline are
>> indeed the ones expected by the Guest Owner, and only if that is the
>> case go on and boot them up (removing the need for grub inside OVMF in
>> that mode).
>>
>> To support that, OVMF adds a special area for hashes of
>> kernel/initrd/cmdline; that area is expected to be filled by QEMU and
>> encrypted as part of the initial SEV guest launch.  This in turn makes
>> the hashes part of the PSP measured content, and OVMF can trust these
>> inputs if they match the hashes.
>>
>> This series adds an SEV function to generate the table of hashes for
>> OVMF and encrypt it (patch 1/2), and calls this function if SEV is
>> enabled when the kernel/initrd/cmdline are prepared (patch 2/2).
>>
>> Corresponding OVMF support was submitted to edk2-devel [1] (patch series
>> "Measured SEV boot with kernel/initrd/cmdline"); it's still under
>> review.
>>
>> [1] https://edk2.groups.io/g/devel/topic/patch_v1_0_8_measured_sev/83074450
>>
>> ---
Connor Kuehl July 8, 2021, 5:16 p.m. UTC | #3
On 7/8/21 10:03 AM, Philippe Mathieu-Daudé wrote:
> On 7/8/21 6:41 PM, Connor Kuehl wrote:
>> Hi Paolo,
>>
>> Please consider this series[1] for inclusion into your next pull request.
>>
>> Just a note that this series has a companion series that is getting
>> upstreamed into OVMF[2]
> 
> Shouldn't we get the OVMF part merged first?

The approach taken in the OVMF series doesn't seem very controversial,
so I don't anticipate any breaking changes with the current state of
those patches as far as QEMU is concerned.

However, I'm fine with erring on the side of caution.

Connor
Michael S. Tsirkin July 9, 2021, 6:55 a.m. UTC | #4
On Thu, Jun 24, 2021 at 10:20:38AM +0000, Dov Murik wrote:
> Currently booting with -kernel/-initrd/-append is not supported in SEV
> confidential guests, because the content of these blobs is not measured
> and therefore not trusted by the SEV guest.
> 
> However, in some cases the kernel, initrd, and cmdline are not secret
> but should not be modified by the host.  In such a case, we want to
> verify inside the trusted VM that the kernel, initrd, and cmdline are
> indeed the ones expected by the Guest Owner, and only if that is the
> case go on and boot them up (removing the need for grub inside OVMF in
> that mode).
> 
> To support that, OVMF adds a special area for hashes of
> kernel/initrd/cmdline; that area is expected to be filled by QEMU and
> encrypted as part of the initial SEV guest launch.  This in turn makes
> the hashes part of the PSP measured content, and OVMF can trust these
> inputs if they match the hashes.
> 
> This series adds an SEV function to generate the table of hashes for
> OVMF and encrypt it (patch 1/2), and calls this function if SEV is
> enabled when the kernel/initrd/cmdline are prepared (patch 2/2).
> 
> Corresponding OVMF support was submitted to edk2-devel [1] (patch series
> "Measured SEV boot with kernel/initrd/cmdline"); it's still under
> review.
> 
> [1] https://edk2.groups.io/g/devel/topic/patch_v1_0_8_measured_sev/83074450


Now that I figured the measurement angle

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> 
> v3 changes:
>  - initrd hash is now mandatory; if no -initrd is passed, calculate the
>    hash of the empty buffer.  This is now aligned with the OVMF
>    behaviour which verifies the empty initrd (correctly).
>  - make SevHashTable entries fixed: 3 entries for cmdline, initrd, and kernel.
>  - in sev_add_kernel_loader_hashes: first calculate all the hashes, only then
>    fill-in the hashes table in the guest's memory.
>  - Use g_assert_not_reached in sev-stub.c.
>  - Use QEMU_PACKED attribute for structs.
>  - Use QemuUUID type for guids.
>  - in sev_add_kernel_loader_hashes: use ARRAY_SIZE(iov) instead of literal 2.
> 
> v2: https://lore.kernel.org/qemu-devel/20210621190553.1763020-1-dovmurik@linux.ibm.com/
> v2 changes:
>  - Extract main functionality to sev.c (with empty stub in sev-stub.c)
>  - Use sev_enabled() instead of machine->cgs->ready to detect SEV guest
>  - Coding style changes
> 
> v1: https://lore.kernel.org/qemu-devel/20210525065931.1628554-1-dovmurik@linux.ibm.com/
> 
> Dov Murik (2):
>   sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux
>     boot
>   x86/sev: generate SEV kernel loader hashes in x86_load_linux
> 
>  target/i386/sev_i386.h |  12 ++++
>  hw/i386/x86.c          |  25 +++++++-
>  target/i386/sev-stub.c |   5 ++
>  target/i386/sev.c      | 137 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 178 insertions(+), 1 deletion(-)
> 
> 
> base-commit: b22726abdfa54592d6ad88f65b0297c0e8b363e2
> -- 
> 2.25.1
Dov Murik July 29, 2021, 7:31 p.m. UTC | #5
Hi Paolo, Phil, Connor,

On 08/07/2021 20:16, Connor Kuehl wrote:
> On 7/8/21 10:03 AM, Philippe Mathieu-Daudé wrote:
>> On 7/8/21 6:41 PM, Connor Kuehl wrote:
>>> Hi Paolo,
>>>
>>> Please consider this series[1] for inclusion into your next pull request.
>>>
>>> Just a note that this series has a companion series that is getting
>>> upstreamed into OVMF[2]
>>
>> Shouldn't we get the OVMF part merged first?

The OVMF companion series has been reviewed by the new OVMF maintainer
and merged to edk2 master branch as of edk2 commit 514b3aa08ece [1].

[1] https://github.com/tianocore/edk2/commit/514b3aa08ece


Thanks,
Dov


> 
> The approach taken in the OVMF series doesn't seem very controversial,
> so I don't anticipate any breaking changes with the current state of
> those patches as far as QEMU is concerned.
> 
> However, I'm fine with erring on the side of caution.
> 
> Connor
>
Connor Kuehl July 30, 2021, 2:47 p.m. UTC | #6
On Thu Jul 29, 2021 at 2:31 PM CDT, Dov Murik wrote:
> The OVMF companion series has been reviewed by the new OVMF maintainer
> and merged to edk2 master branch as of edk2 commit 514b3aa08ece [1].
>
> [1] https://github.com/tianocore/edk2/commit/514b3aa08ece

Awesome! Unfortunately, it's looking like we'll have to wait[1] for QEMU to
thaw before this series goes in.

Connor

[1] https://wiki.qemu.org/Planning/6.1
Dov Murik July 30, 2021, 6:02 p.m. UTC | #7
On 30/07/2021 17:47, Connor Kuehl wrote:
> On Thu Jul 29, 2021 at 2:31 PM CDT, Dov Murik wrote:
>> The OVMF companion series has been reviewed by the new OVMF maintainer
>> and merged to edk2 master branch as of edk2 commit 514b3aa08ece [1].
>>
>> [1] https://github.com/tianocore/edk2/commit/514b3aa08ece
> 
> Awesome! Unfortunately, it's looking like we'll have to wait[1] for QEMU to
> thaw before this series goes in.
> 

Thanks for explaining this.  Do I need to do anything after 6.1 is
released? Ping? Rebase and re-send?

-Dov


> Connor
> 
> [1] https://wiki.qemu.org/Planning/6.1
>
Connor Kuehl July 30, 2021, 6:14 p.m. UTC | #8
On Fri Jul 30, 2021 at 1:02 PM CDT, Dov Murik wrote:
>
>
> > Awesome! Unfortunately, it's looking like we'll have to wait[1] for QEMU to
> > thaw before this series goes in.
> > 
>
> Thanks for explaining this. Do I need to do anything after 6.1 is
> released? Ping? Rebase and re-send?

Rebase and re-send. I think your patches already have the Reviewed-by
tags in the patch descriptions, but if that's not the case, make sure
you add them for the re-send so it's obvious that the patches have
already been reviewed.

Thank you,

Connor